Skip to content

Commit a6f1bc9

Browse files
kannanjgithubkannanjgithub
committed
XdsX509TrustManager changes for auto sni san validation.
1 parent a371065 commit a6f1bc9

File tree

7 files changed

+188
-84
lines changed

7 files changed

+188
-84
lines changed

‎xds/src/main/java/io/grpc/xds/EnvoyServerProtoData.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,19 +75,22 @@ public static final class UpstreamTlsContext extends BaseTlsContext {
7575

7676
private final String sni;
7777
private final boolean auto_host_sni;
78+
private final boolean auto_sni_san_validation;
7879

7980
@VisibleForTesting
8081
public UpstreamTlsContext(CommonTlsContext commonTlsContext) {
8182
super(commonTlsContext);
8283
this.sni = null;
8384
this.auto_host_sni = false;
85+
this.auto_sni_san_validation = false;
8486
}
8587

8688
@VisibleForTesting
8789
public UpstreamTlsContext(io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext upstreamTlsContext) {
8890
super(upstreamTlsContext.getCommonTlsContext());
8991
this.sni = upstreamTlsContext.getSni();
9092
this.auto_host_sni = upstreamTlsContext.getAutoHostSni();
93+
this.auto_sni_san_validation = upstreamTlsContext.getAutoSniSanValidation();
9194
}
9295

9396
public static UpstreamTlsContext fromEnvoyProtoUpstreamTlsContext(
@@ -104,6 +107,10 @@ public boolean getAutoHostSni() {
104107
return auto_host_sni;
105108
}
106109

110+
public boolean getAutoSniSanValidation() {
111+
return auto_sni_san_validation;
112+
}
113+
107114
@Override
108115
public String toString() {
109116
return "UpstreamTlsContext{" + "commonTlsContext=" + commonTlsContext + '}';

‎xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProvider.java‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,16 @@
3232
/** A client SslContext provider using CertificateProviderInstance to fetch secrets. */
3333
public final class CertProviderClientSslContextProvider extends CertProviderSslContextProvider {
3434

35+
private final String sniForSanMatching;
36+
3537
CertProviderClientSslContextProvider(
36-
Node node,
37-
@Nullable Map<String, CertificateProviderInfo> certProviders,
38-
CommonTlsContext.CertificateProviderInstance certInstance,
39-
CommonTlsContext.CertificateProviderInstance rootCertInstance,
40-
CertificateValidationContext staticCertValidationContext,
41-
UpstreamTlsContext upstreamTlsContext,
42-
String sni, CertificateProviderStore certificateProviderStore) {
38+
Node node,
39+
@Nullable Map<String, CertificateProviderInfo> certProviders,
40+
CommonTlsContext.CertificateProviderInstance certInstance,
41+
CommonTlsContext.CertificateProviderInstance rootCertInstance,
42+
CertificateValidationContext staticCertValidationContext,
43+
UpstreamTlsContext upstreamTlsContext,
44+
String sniForSanMatching, CertificateProviderStore certificateProviderStore) {
4345
super(
4446
node,
4547
certProviders,
@@ -48,11 +50,12 @@ public final class CertProviderClientSslContextProvider extends CertProviderSslC
4850
staticCertValidationContext,
4951
upstreamTlsContext,
5052
certificateProviderStore);
53+
this.sniForSanMatching = upstreamTlsContext.getAutoSniSanValidation()? sniForSanMatching : null;
5154
}
5255

5356
@Override
5457
protected final SslContextBuilder getSslContextBuilder(
55-
CertificateValidationContext certificateValidationContextdationContext)
58+
CertificateValidationContext certificateValidationContext)
5659
throws CertStoreException {
5760
SslContextBuilder sslContextBuilder = GrpcSslContexts.forClient();
5861
// Null rootCertInstance implies hasSystemRootCerts because of the check in
@@ -62,12 +65,12 @@ protected final SslContextBuilder getSslContextBuilder(
6265
sslContextBuilder = sslContextBuilder.trustManager(
6366
new XdsTrustManagerFactory(
6467
savedSpiffeTrustMap,
65-
certificateValidationContextdationContext));
68+
certificateValidationContext, sniForSanMatching));
6669
} else {
6770
sslContextBuilder = sslContextBuilder.trustManager(
6871
new XdsTrustManagerFactory(
6972
savedTrustedRoots.toArray(new X509Certificate[0]),
70-
certificateValidationContextdationContext));
73+
certificateValidationContext, sniForSanMatching));
7174
}
7275
}
7376
if (isMtls()) {

‎xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderServerSslContextProvider.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,11 @@ protected final SslContextBuilder getSslContextBuilder(
6363
if (isMtls() && savedSpiffeTrustMap != null) {
6464
trustManagerFactory = new XdsTrustManagerFactory(
6565
savedSpiffeTrustMap,
66-
certificateValidationContextdationContext);
66+
certificateValidationContextdationContext, null);
6767
} else if (isMtls()) {
6868
trustManagerFactory = new XdsTrustManagerFactory(
6969
savedTrustedRoots.toArray(new X509Certificate[0]),
70-
certificateValidationContextdationContext);
70+
certificateValidationContextdationContext, null);
7171
}
7272
setClientAuthValues(sslContextBuilder, trustManagerFactory);
7373
sslContextBuilder = GrpcSslContexts.configure(sslContextBuilder);

‎xds/src/main/java/io/grpc/xds/internal/security/trust/XdsTrustManagerFactory.java‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -58,43 +58,46 @@ public XdsTrustManagerFactory(CertificateValidationContext certificateValidation
5858
this(
5959
getTrustedCaFromCertContext(certificateValidationContext),
6060
certificateValidationContext,
61-
false);
61+
false,
62+
null);
6263
}
6364

6465
public XdsTrustManagerFactory(
65-
X509Certificate[] certs, CertificateValidationContext staticCertificateValidationContext)
66+
X509Certificate[] certs, CertificateValidationContext staticCertificateValidationContext, String sniForSanMatching)
6667
throws CertStoreException {
67-
this(certs, staticCertificateValidationContext, true);
68+
this(certs, staticCertificateValidationContext, true, sniForSanMatching);
6869
}
6970

7071
public XdsTrustManagerFactory(Map<String, List<X509Certificate>> spiffeTrustMap,
71-
CertificateValidationContext staticCertificateValidationContext) throws CertStoreException {
72-
this(spiffeTrustMap, staticCertificateValidationContext, true);
72+
CertificateValidationContext staticCertificateValidationContext, String sniForSanMatching) throws CertStoreException {
73+
this(spiffeTrustMap, staticCertificateValidationContext, true, sniForSanMatching);
7374
}
7475

7576
private XdsTrustManagerFactory(
7677
X509Certificate[] certs,
7778
CertificateValidationContext certificateValidationContext,
78-
boolean validationContextIsStatic)
79+
boolean validationContextIsStatic,
80+
String sniForSanMatching)
7981
throws CertStoreException {
8082
if (validationContextIsStatic) {
8183
checkArgument(
8284
certificateValidationContext == null || !certificateValidationContext.hasTrustedCa(),
8385
"only static certificateValidationContext expected");
8486
}
85-
xdsX509TrustManager = createX509TrustManager(certs, certificateValidationContext);
87+
xdsX509TrustManager = createX509TrustManager(certs, certificateValidationContext, sniForSanMatching);
8688
}
8789

8890
private XdsTrustManagerFactory(
8991
Map<String, List<X509Certificate>> spiffeTrustMap,
9092
CertificateValidationContext certificateValidationContext,
91-
boolean validationContextIsStatic)
93+
boolean validationContextIsStatic,
94+
String sniForSanMatching)
9295
throws CertStoreException {
9396
if (validationContextIsStatic) {
9497
checkArgument(
9598
certificateValidationContext == null || !certificateValidationContext.hasTrustedCa(),
9699
"only static certificateValidationContext expected");
97-
xdsX509TrustManager = createX509TrustManager(spiffeTrustMap, certificateValidationContext);
100+
xdsX509TrustManager = createX509TrustManager(spiffeTrustMap, certificateValidationContext, sniForSanMatching);
98101
}
99102
}
100103

@@ -121,21 +124,21 @@ private static X509Certificate[] getTrustedCaFromCertContext(
121124

122125
@VisibleForTesting
123126
static XdsX509TrustManager createX509TrustManager(
124-
X509Certificate[] certs, CertificateValidationContext certContext) throws CertStoreException {
125-
return new XdsX509TrustManager(certContext, createTrustManager(certs));
127+
X509Certificate[] certs, CertificateValidationContext certContext, String sniForSanMatching) throws CertStoreException {
128+
return new XdsX509TrustManager(certContext, createTrustManager(certs), sniForSanMatching);
126129
}
127130

128131
@VisibleForTesting
129132
static XdsX509TrustManager createX509TrustManager(
130-
Map<String, List<X509Certificate>> spiffeTrustMapFile,
131-
CertificateValidationContext certContext) throws CertStoreException {
133+
Map<String, List<X509Certificate>> spiffeTrustMapFile,
134+
CertificateValidationContext certContext, String sniForSanMatching) throws CertStoreException {
132135
checkNotNull(spiffeTrustMapFile, "spiffeTrustMapFile");
133136
Map<String, X509ExtendedTrustManager> delegates = new HashMap<>();
134137
for (Map.Entry<String, List<X509Certificate>> entry:spiffeTrustMapFile.entrySet()) {
135138
delegates.put(entry.getKey(), createTrustManager(
136139
entry.getValue().toArray(new X509Certificate[0])));
137140
}
138-
return new XdsX509TrustManager(certContext, delegates);
141+
return new XdsX509TrustManager(certContext, delegates, sniForSanMatching);
139142
}
140143

141144
private static X509ExtendedTrustManager createTrustManager(X509Certificate[] certs)

‎xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -61,34 +61,29 @@ final class XdsX509TrustManager extends X509ExtendedTrustManager implements X509
6161
private final X509ExtendedTrustManager delegate;
6262
private final Map<String, X509ExtendedTrustManager> spiffeTrustMapDelegates;
6363
private final CertificateValidationContext certContext;
64-
private final String sni;
64+
private final String sniForSanMatching;
6565

6666
XdsX509TrustManager(@Nullable CertificateValidationContext certContext,
6767
X509ExtendedTrustManager delegate) {
6868
this(certContext, delegate, null);
6969
}
7070

7171
XdsX509TrustManager(@Nullable CertificateValidationContext certContext,
72-
X509ExtendedTrustManager delegate, @Nullable String sni) {
72+
X509ExtendedTrustManager delegate, @Nullable String sniForSanMatching) {
7373
checkNotNull(delegate, "delegate");
7474
this.certContext = certContext;
7575
this.delegate = delegate;
7676
this.spiffeTrustMapDelegates = null;
77-
this.sni = sni;
77+
this.sniForSanMatching = sniForSanMatching;
7878
}
7979

8080
XdsX509TrustManager(@Nullable CertificateValidationContext certContext,
81-
Map<String, X509ExtendedTrustManager> spiffeTrustMapDelegates) {
82-
this(certContext, spiffeTrustMapDelegates, null);
83-
}
84-
85-
XdsX509TrustManager(@Nullable CertificateValidationContext certContext,
86-
Map<String, X509ExtendedTrustManager> spiffeTrustMapDelegates, @Nullable String sni) {
81+
Map<String, X509ExtendedTrustManager> spiffeTrustMapDelegates, @Nullable String sniForSanMatching) {
8782
checkNotNull(spiffeTrustMapDelegates, "spiffeTrustMapDelegates");
8883
this.spiffeTrustMapDelegates = ImmutableMap.copyOf(spiffeTrustMapDelegates);
8984
this.certContext = certContext;
9085
this.delegate = null;
91-
this.sni = sni;
86+
this.sniForSanMatching = sniForSanMatching;
9287
}
9388

9489
private static boolean verifyDnsNameInPattern(
@@ -222,7 +217,9 @@ void verifySubjectAltNameInChain(X509Certificate[] peerCertChain) throws Certifi
222217
return;
223218
}
224219
@SuppressWarnings("deprecation") // gRFC A29 predates match_typed_subject_alt_names
225-
List<StringMatcher> verifyList = sni != null? ImmutableList.of(StringMatcher.newBuilder().setExact(sni).build()) : certContext.getMatchSubjectAltNamesList();
220+
List<StringMatcher> verifyList = !Strings.isNullOrEmpty(sniForSanMatching)
221+
? ImmutableList.of(StringMatcher.newBuilder().setExact(sniForSanMatching).build())
222+
: certContext.getMatchSubjectAltNamesList();
226223
if (verifyList.isEmpty()) {
227224
return;
228225
}

‎xds/src/test/java/io/grpc/xds/internal/security/trust/XdsTrustManagerFactoryTest.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public void constructor_fromRootCert()
9191
CertificateValidationContext staticValidationContext = buildStaticValidationContext("san1",
9292
"san2");
9393
XdsTrustManagerFactory factory =
94-
new XdsTrustManagerFactory(new X509Certificate[]{x509Cert}, staticValidationContext);
94+
new XdsTrustManagerFactory(new X509Certificate[]{x509Cert}, staticValidationContext, null);
9595
assertThat(factory).isNotNull();
9696
TrustManager[] tms = factory.getTrustManagers();
9797
assertThat(tms).isNotNull();
@@ -115,7 +115,7 @@ public void constructor_fromSpiffeTrustMap()
115115
"san2");
116116
// Single domain and single cert
117117
XdsTrustManagerFactory factory = new XdsTrustManagerFactory(ImmutableMap
118-
.of("example.com", ImmutableList.of(x509Cert)), staticValidationContext);
118+
.of("example.com", ImmutableList.of(x509Cert)), staticValidationContext, null);
119119
assertThat(factory).isNotNull();
120120
TrustManager[] tms = factory.getTrustManagers();
121121
assertThat(tms).isNotNull();
@@ -131,7 +131,7 @@ public void constructor_fromSpiffeTrustMap()
131131
X509Certificate anotherCert = TestUtils.loadX509Cert(CLIENT_PEM_FILE);
132132
factory = new XdsTrustManagerFactory(ImmutableMap
133133
.of("example.com", ImmutableList.of(x509Cert),
134-
"google.com", ImmutableList.of(x509Cert, anotherCert)), staticValidationContext);
134+
"google.com", ImmutableList.of(x509Cert, anotherCert)), staticValidationContext, null);
135135
assertThat(factory).isNotNull();
136136
tms = factory.getTrustManagers();
137137
assertThat(tms).isNotNull();
@@ -154,7 +154,7 @@ public void constructorRootCert_checkServerTrusted()
154154
CertificateValidationContext staticValidationContext = buildStaticValidationContext("san1",
155155
"waterzooi.test.google.be");
156156
XdsTrustManagerFactory factory =
157-
new XdsTrustManagerFactory(new X509Certificate[]{x509Cert}, staticValidationContext);
157+
new XdsTrustManagerFactory(new X509Certificate[]{x509Cert}, staticValidationContext, null);
158158
XdsX509TrustManager xdsX509TrustManager = (XdsX509TrustManager) factory.getTrustManagers()[0];
159159
X509Certificate[] serverChain =
160160
CertificateUtils.toX509Certificates(TlsTesting.loadCert(SERVER_1_PEM_FILE));
@@ -167,7 +167,7 @@ public void constructorRootCert_nonStaticContext_throwsException()
167167
X509Certificate x509Cert = TestUtils.loadX509Cert(CA_PEM_FILE);
168168
try {
169169
new XdsTrustManagerFactory(
170-
new X509Certificate[] {x509Cert}, getCertContextFromPath(CA_PEM_FILE));
170+
new X509Certificate[] {x509Cert}, getCertContextFromPath(CA_PEM_FILE), null);
171171
Assert.fail("no exception thrown");
172172
} catch (IllegalArgumentException expected) {
173173
assertThat(expected)
@@ -183,7 +183,7 @@ public void constructorRootCert_checkServerTrusted_throwsException()
183183
CertificateValidationContext staticValidationContext = buildStaticValidationContext("san1",
184184
"san2");
185185
XdsTrustManagerFactory factory =
186-
new XdsTrustManagerFactory(new X509Certificate[]{x509Cert}, staticValidationContext);
186+
new XdsTrustManagerFactory(new X509Certificate[]{x509Cert}, staticValidationContext, null);
187187
XdsX509TrustManager xdsX509TrustManager = (XdsX509TrustManager) factory.getTrustManagers()[0];
188188
X509Certificate[] serverChain =
189189
CertificateUtils.toX509Certificates(TlsTesting.loadCert(SERVER_1_PEM_FILE));
@@ -204,7 +204,7 @@ public void constructorRootCert_checkClientTrusted_throwsException()
204204
CertificateValidationContext staticValidationContext = buildStaticValidationContext("san1",
205205
"san2");
206206
XdsTrustManagerFactory factory =
207-
new XdsTrustManagerFactory(new X509Certificate[]{x509Cert}, staticValidationContext);
207+
new XdsTrustManagerFactory(new X509Certificate[]{x509Cert}, staticValidationContext, null);
208208
XdsX509TrustManager xdsX509TrustManager = (XdsX509TrustManager) factory.getTrustManagers()[0];
209209
X509Certificate[] clientChain =
210210
CertificateUtils.toX509Certificates(TlsTesting.loadCert(SERVER_1_PEM_FILE));

0 commit comments

Comments
 (0)