Skip to content

Commit bdabd40

Browse files
rmichelarmichela
authored
Merge pull request #82 from salesforce/feature/XFCC
Minor XFCC refactoring
2 parents b3cc0ab + e0c519f commit bdabd40

File tree

5 files changed

+160
-93
lines changed

5 files changed

+160
-93
lines changed

‎grpc-contrib/src/main/java/com/salesforce/grpc/contrib/xfcc/XForwardedClientCert.java‎

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,20 @@
1414
import java.util.Collections;
1515
import java.util.List;
1616

17+
import static com.salesforce.grpc.contrib.xfcc.XfccQuoteUtil.*;
18+
1719
/**
1820
* x-forwarded-client-cert (XFCC) is a proxy header which indicates certificate information of part or all of the
1921
* clients or proxies that a request has flowed through, on its way from the client to the server.
2022
*/
2123
public final class XForwardedClientCert {
24+
static final String XFCC_BY = "By";
25+
static final String XFCC_HASH = "Hash";
26+
static final String XFCC_SAN = "SAN";
27+
static final String XFCC_URI = "URI";
28+
static final String XFCC_DNS = "DNS";
29+
static final String XFCC_SUBJECT = "Subject";
30+
2231
/**
2332
* The metadata key used to access any present {@link XForwardedClientCert} objects.
2433
*/
@@ -89,33 +98,27 @@ public String getSubject() {
8998
public String toString() {
9099
List<String> kvp = new ArrayList<>();
91100
if (!by.isEmpty()) {
92-
kvp.add("By=" + enquote(by));
101+
kvp.add(toKvp(XFCC_BY, enquote(by)));
93102
}
94103
if (!hash.isEmpty()) {
95-
kvp.add("Hash=" + enquote(hash));
104+
kvp.add(toKvp(XFCC_HASH, enquote(hash)));
96105
}
97106
if (!sanUri.isEmpty()) {
98-
kvp.add("URI=" + enquote(sanUri));
107+
kvp.add(toKvp(XFCC_URI, enquote(sanUri)));
99108
}
100-
for (String dns : sanDns) {
101-
kvp.add("DNS=" + enquote(dns));
109+
if (!sanDns.isEmpty()) {
110+
for (String dns : sanDns) {
111+
kvp.add(toKvp(XFCC_DNS, enquote(dns)));
112+
}
102113
}
103114
if (!subject.isEmpty()) {
104-
kvp.add("Subject=" + enquote(subject));
115+
kvp.add(toKvp(XFCC_SUBJECT, enquote(subject)));
105116
}
106117

107118
return String.join(";", kvp);
108119
}
109120

110-
private String enquote(String value) {
111-
// Escape inner quotes with \"
112-
value = value.replace("\"", "\\\"");
113-
114-
// Wrap in quotes if ,;= is present
115-
if (value.contains(",") || value.contains(";") || value.contains("=")) {
116-
value = "\"" + value + "\"";
117-
}
118-
119-
return value;
121+
private String toKvp(String key, String value) {
122+
return key + "=" + value;
120123
}
121124
}

‎grpc-contrib/src/main/java/com/salesforce/grpc/contrib/xfcc/XfccParser.java‎

Lines changed: 19 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010
import java.util.ArrayList;
1111
import java.util.List;
1212

13+
import static com.salesforce.grpc.contrib.xfcc.XfccQuoteUtil.*;
14+
import static com.salesforce.grpc.contrib.xfcc.XForwardedClientCert.*;
15+
1316
/**
1417
* {@code XfccParser} parses the {@code x-forwarded-client-cert} (XFCC) header populated by TLS-terminating
1518
* reverse proxies.
@@ -27,93 +30,32 @@ static List<XForwardedClientCert> parse(String header) {
2730

2831
for (String element : quoteAwareSplit(header, ',')) {
2932
XForwardedClientCert cert = new XForwardedClientCert();
30-
List<String> kvps = quoteAwareSplit(element, ';');
31-
for (String kvp : kvps) {
32-
List<String> l = quoteAwareSplit(kvp, '=');
33-
34-
if (l.get(0).toLowerCase().equals("by")) {
35-
cert.setBy(dequote(l.get(1)));
33+
List<String> substrings = quoteAwareSplit(element, ';');
34+
for (String substring : substrings) {
35+
List<String> kvp = quoteAwareSplit(substring, '=');
36+
String key = kvp.get(0).toLowerCase();
37+
String value = kvp.get(1);
38+
39+
if (key.equalsIgnoreCase(XFCC_BY)) {
40+
cert.setBy(dequote(value));
3641
}
37-
if (l.get(0).toLowerCase().equals("hash")) {
38-
cert.setHash(dequote(l.get(1)));
42+
if (key.equalsIgnoreCase(XFCC_HASH)) {
43+
cert.setHash(dequote(value));
3944
}
4045
// Use "SAN:" instead of "URI:" for backward compatibility with previous mesh proxy releases.
41-
if (l.get(0).toLowerCase().equals("san") || l.get(0).toLowerCase().equals("uri")) {
42-
cert.setSanUri(dequote(l.get(1)));
46+
if (key.equalsIgnoreCase(XFCC_SAN) || key.equalsIgnoreCase(XFCC_URI)) {
47+
cert.setSanUri(dequote(value));
4348
}
44-
if (l.get(0).toLowerCase().equals("dns")) {
45-
cert.addSanDns(dequote(l.get(1)));
49+
if (key.equalsIgnoreCase(XFCC_DNS)) {
50+
cert.addSanDns(dequote(value));
4651
}
47-
if (l.get(0).toLowerCase().equals("subject")) {
48-
cert.setSubject(dequote(l.get(1)));
52+
if (key.equalsIgnoreCase(XFCC_SUBJECT)) {
53+
cert.setSubject(dequote(value));
4954
}
5055
}
5156
certs.add(cert);
5257
}
5358

5459
return certs;
5560
}
56-
57-
// Break str into individual elements, splitting on delim (not in quotes)
58-
private static List<String> quoteAwareSplit(String str, char delim) {
59-
boolean inQuotes = false;
60-
boolean inEscape = false;
61-
62-
List<String> elements = new ArrayList<>();
63-
StringBuilder buffer = new StringBuilder();
64-
for (char c : str.toCharArray()) {
65-
if (c == delim && !inQuotes) {
66-
elements.add(buffer.toString());
67-
buffer = new StringBuilder();
68-
inEscape = false;
69-
continue;
70-
}
71-
72-
if (c == '"') {
73-
if (inQuotes) {
74-
if (!inEscape) {
75-
inQuotes = false;
76-
}
77-
} else {
78-
inQuotes = true;
79-
80-
}
81-
inEscape = false;
82-
buffer.append(c);
83-
continue;
84-
}
85-
86-
if (c == '\\') {
87-
if (!inEscape) {
88-
inEscape = true;
89-
buffer.append(c);
90-
continue;
91-
}
92-
}
93-
94-
// all other characters
95-
inEscape = false;
96-
buffer.append(c);
97-
}
98-
99-
if (inQuotes) {
100-
throw new RuntimeException("Quoted string not closed");
101-
}
102-
103-
elements.add(buffer.toString());
104-
105-
return elements;
106-
}
107-
108-
// Remove leading and tailing unescaped quotes, remove escaping from escaped internal quotes
109-
private static String dequote(String str) {
110-
str = str.replace("\\\"", "\"");
111-
if (str.startsWith("\"")) {
112-
str = str.substring(1);
113-
}
114-
if (str.endsWith("\"")) {
115-
str = str.substring(0, str.length() - 1);
116-
}
117-
return str;
118-
}
11961
}

‎grpc-contrib/src/main/java/com/salesforce/grpc/contrib/xfcc/XfccQuoteUtil.java‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/*
2+
* Copyright (c) 2017, salesforce.com, inc.
3+
* All rights reserved.
4+
* Licensed under the BSD 3-Clause license.
5+
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
6+
*/
7+
package com.salesforce.grpc.contrib.xfcc;
8+
9+
import java.util.ArrayList;
10+
import java.util.List;
11+
12+
/**
13+
* Utility methods for quote escaping in XFCC headers.
14+
*/
15+
final class XfccQuoteUtil {
16+
private XfccQuoteUtil() { }
17+
18+
/**
19+
* Break str into individual elements, splitting on delim (not in quotes).
20+
*/
21+
static List<String> quoteAwareSplit(String str, char delim) {
22+
boolean inQuotes = false;
23+
boolean inEscape = false;
24+
25+
List<String> elements = new ArrayList<>();
26+
StringBuilder buffer = new StringBuilder();
27+
for (char c : str.toCharArray()) {
28+
if (c == delim && !inQuotes) {
29+
elements.add(buffer.toString());
30+
buffer.setLength(0); // clear
31+
inEscape = false;
32+
continue;
33+
}
34+
35+
if (c == '"') {
36+
if (inQuotes) {
37+
if (!inEscape) {
38+
inQuotes = false;
39+
}
40+
} else {
41+
inQuotes = true;
42+
43+
}
44+
inEscape = false;
45+
buffer.append(c);
46+
continue;
47+
}
48+
49+
if (c == '\\') {
50+
if (!inEscape) {
51+
inEscape = true;
52+
buffer.append(c);
53+
continue;
54+
}
55+
}
56+
57+
// all other characters
58+
inEscape = false;
59+
buffer.append(c);
60+
}
61+
62+
if (inQuotes) {
63+
throw new RuntimeException("Quoted string not closed");
64+
}
65+
66+
elements.add(buffer.toString());
67+
68+
return elements;
69+
}
70+
71+
/**
72+
* Add escaping around double quote characters; wrap with quotes if special characters are present.
73+
*/
74+
static String enquote(String value) {
75+
// Escape inner quotes with \"
76+
value = value.replace("\"", "\\\"");
77+
78+
// Wrap in quotes if ,;= is present
79+
if (value.contains(",") || value.contains(";") || value.contains("=")) {
80+
value = "\"" + value + "\"";
81+
}
82+
83+
return value;
84+
}
85+
86+
/**
87+
* Remove leading and tailing unescaped quotes; remove escaping from escaped internal quotes.
88+
*/
89+
static String dequote(String str) {
90+
str = str.replace("\\\"", "\"");
91+
if (str.startsWith("\"")) {
92+
str = str.substring(1);
93+
}
94+
if (str.endsWith("\"")) {
95+
str = str.substring(0, str.length() - 1);
96+
}
97+
return str;
98+
}
99+
}

‎grpc-contrib/src/test/java/com/salesforce/grpc/contrib/xfcc/XfccMarshallerTest.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import java.util.List;
1313

1414
import static org.assertj.core.api.Assertions.assertThat;
15+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
1516

1617
public class XfccMarshallerTest {
1718
@Test
@@ -80,7 +81,14 @@ public void roundTripSimpleTest() {
8081

8182
@Test
8283
public void roundTripUriAndDnsTest() {
84+
XfccMarshaller marshaller = new XfccMarshaller();
85+
86+
String header = "By=http://frontend.lyft.com;Hash=468ed33be74eee6556d90c0149c1309e9ba61d6425303443c0748a02dd8de688;URI=http://testclient.lyft.com;DNS=lyft.com;DNS=www.lyft.com;Subject=\"/C=US/ST=CA/L=San Francisco/OU=Lyft/CN=Test Client\"";
87+
88+
List<XForwardedClientCert> certs = marshaller.parseAsciiString(header);
89+
String serialized = marshaller.toAsciiString(certs);
8390

91+
assertThat(serialized).isEqualTo(header);
8492
}
8593

8694
@Test
@@ -121,4 +129,11 @@ public void roundTripEscapedQuotedTest() {
121129

122130
assertThat(serialized).isEqualTo(header);
123131
}
132+
133+
@Test
134+
public void malformedHeaderThrows() {
135+
XfccMarshaller marshaller = new XfccMarshaller();
136+
String header = "KABOOM!";
137+
assertThatThrownBy(() -> marshaller.parseAsciiString(header)).isInstanceOf(RuntimeException.class);
138+
}
124139
}

‎grpc-contrib/src/test/java/com/salesforce/grpc/contrib/xfcc/XfccParserTest.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,4 +107,12 @@ public void mismatchedQuotesThrows() {
107107

108108
assertThatThrownBy(() -> XfccParser.parse(header)).isInstanceOf(RuntimeException.class);
109109
}
110+
111+
@Test
112+
public void quotedKeyThrows() {
113+
String header = "\"By\"=http://frontend.lyft.com;Hash=468ed33be74eee6556d90c0149c1309e9ba61d6425303443c0748a02dd8de688;" +
114+
"Subject=\"/C=US/ST=CA/L=\\\"San Francisco\"/OU=Lyft/CN=Test Client\";SAN=http://testclient.lyft.com";
115+
116+
assertThatThrownBy(() -> XfccParser.parse(header)).isInstanceOf(RuntimeException.class);
117+
}
110118
}

0 commit comments

Comments
 (0)