Skip to content

Commit b8dba99

Browse files
kannanjgithubkannanjgithub
committed
Merge branch 'systemrootcerts-ignore-trusted-root-updates' into system-root-changes-needed
# Conflicts: # xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProvider.java # xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java
2 parents 13200fa + e18d6cd commit b8dba99

File tree

2 files changed

+118
-19
lines changed

2 files changed

+118
-19
lines changed

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

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,25 @@
2222
import io.grpc.netty.GrpcSslContexts;
2323
import io.grpc.xds.EnvoyServerProtoData.UpstreamTlsContext;
2424
import io.grpc.xds.client.Bootstrapper.CertificateProviderInfo;
25+
import io.grpc.xds.internal.security.CommonTlsContextUtil;
2526
import io.grpc.xds.internal.security.trust.XdsTrustManagerFactory;
2627
import io.netty.handler.ssl.SslContextBuilder;
28+
import java.io.IOException;
29+
import java.security.KeyStore;
30+
import java.security.KeyStoreException;
31+
import java.security.NoSuchAlgorithmException;
2732
import java.security.cert.CertStoreException;
33+
import java.security.cert.CertificateException;
2834
import java.security.cert.X509Certificate;
35+
import java.util.Arrays;
36+
import java.util.Collection;
37+
import java.util.List;
2938
import java.util.Map;
39+
import java.util.stream.Collectors;
3040
import javax.annotation.Nullable;
31-
import javax.net.ssl.SSLException;
41+
import javax.net.ssl.TrustManager;
42+
import javax.net.ssl.TrustManagerFactory;
43+
import javax.net.ssl.X509TrustManager;
3244

3345
/** A client SslContext provider using CertificateProviderInstance to fetch secrets. */
3446
final class CertProviderClientSslContextProvider extends CertProviderSslContextProvider {
@@ -51,14 +63,14 @@ final class CertProviderClientSslContextProvider extends CertProviderSslContextP
5163
staticCertValidationContext,
5264
upstreamTlsContext,
5365
certificateProviderStore);
54-
// Null rootCertInstance implies hasSystemRootCerts because of the check in
55-
// CertProviderClientSslContextProviderFactory.
56-
if (rootCertInstance == null && !isMtls()) {
66+
if (rootCertInstance == null
67+
&& CommonTlsContextUtil.isUsingSystemRootCerts(tlsContext.getCommonTlsContext())
68+
&& !isMtls()) {
5769
try {
5870
// Instantiate sslContext so that addCallback will immediately update the callback with
5971
// the SslContext.
6072
sslContext = getSslContextBuilder(staticCertificateValidationContext).build();
61-
} catch (SSLException | CertStoreException e) {
73+
} catch (CertStoreException | CertificateException | IOException e) {
6274
throw new RuntimeException(e);
6375
}
6476
}
@@ -68,7 +80,7 @@ final class CertProviderClientSslContextProvider extends CertProviderSslContextP
6880
@Override
6981
protected final SslContextBuilder getSslContextBuilder(
7082
CertificateValidationContext certificateValidationContext)
71-
throws CertStoreException {
83+
throws CertificateException, IOException, CertStoreException {
7284
SslContextBuilder sslContextBuilder = GrpcSslContexts.forClient();
7385
if (rootCertInstance != null) {
7486
if (savedSpiffeTrustMap != null) {
@@ -77,15 +89,35 @@ protected final SslContextBuilder getSslContextBuilder(
7789
savedSpiffeTrustMap,
7890
certificateValidationContext, sniForSanMatching));
7991
} else {
80-
sslContextBuilder = sslContextBuilder.trustManager(
81-
new XdsTrustManagerFactory(
82-
savedTrustedRoots.toArray(new X509Certificate[0]),
83-
certificateValidationContext, sniForSanMatching));
92+
try {
93+
sslContextBuilder = sslContextBuilder.trustManager(
94+
new XdsTrustManagerFactory(
95+
getX509CertificatesFromSystemTrustStore(),
96+
certificateValidationContext));
97+
} catch (KeyStoreException | NoSuchAlgorithmException e) {
98+
throw new CertStoreException(e);
99+
}
84100
}
85101
}
86102
if (isMtls()) {
87103
sslContextBuilder.keyManager(savedKey, savedCertChain);
88104
}
89105
return sslContextBuilder;
90106
}
107+
108+
private X509Certificate[] getX509CertificatesFromSystemTrustStore()
109+
throws KeyStoreException, NoSuchAlgorithmException {
110+
TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(
111+
TrustManagerFactory.getDefaultAlgorithm());
112+
trustManagerFactory.init((KeyStore) null);
113+
114+
List<TrustManager> trustManagers = Arrays.asList(trustManagerFactory.getTrustManagers());
115+
List<X509Certificate> rootCerts = trustManagers.stream()
116+
.filter(X509TrustManager.class::isInstance)
117+
.map(X509TrustManager.class::cast)
118+
.map(trustManager -> Arrays.asList(trustManager.getAcceptedIssuers()))
119+
.flatMap(Collection::stream)
120+
.collect(Collectors.toList());
121+
return rootCerts.toArray(new X509Certificate[rootCerts.size()]);
122+
}
91123
}

‎xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java‎

Lines changed: 76 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import com.google.common.util.concurrent.SettableFuture;
3838
import io.envoyproxy.envoy.config.core.v3.SocketAddress.Protocol;
3939
import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateValidationContext;
40+
import io.envoyproxy.envoy.type.matcher.v3.StringMatcher;
4041
import io.grpc.Attributes;
4142
import io.grpc.EquivalentAddressGroup;
4243
import io.grpc.Grpc;
@@ -117,6 +118,10 @@
117118
@RunWith(Parameterized.class)
118119
public class XdsSecurityClientServerTest {
119120

121+
// TODO: Change this is a specific domain after
122+
// https://github.com/grpc/grpc-java/issues/12326 is fixed
123+
private static final String SAN_TO_MATCH = "*.test.google.fr";
124+
120125
@Parameter
121126
public Boolean enableSpiffe;
122127
private Boolean originalEnableSpiffe;
@@ -206,7 +211,8 @@ public void tlsClientServer_noClientAuthentication() throws Exception {
206211
* Uses common_tls_context.combined_validation_context in upstream_tls_context.
207212
*/
208213
@Test
209-
public void tlsClientServer_useSystemRootCerts_useCombinedValidationContext() throws Exception {
214+
public void tlsClientServer_useSystemRootCerts_noMtls_useCombinedValidationContext()
215+
throws Exception {
210216
Path trustStoreFilePath = getCacertFilePathForTestCa();
211217
try {
212218
setTrustStoreSystemProperties(trustStoreFilePath.toAbsolutePath().toString());
@@ -217,7 +223,7 @@ public void tlsClientServer_useSystemRootCerts_useCombinedValidationContext() th
217223

218224
UpstreamTlsContext upstreamTlsContext =
219225
setBootstrapInfoAndBuildUpstreamTlsContextForUsingSystemRootCerts(CLIENT_KEY_FILE,
220-
CLIENT_PEM_FILE, true);
226+
CLIENT_PEM_FILE, true, SAN_TO_MATCH, false);
221227

222228
SimpleServiceGrpc.SimpleServiceBlockingStub blockingStub =
223229
getBlockingStub(upstreamTlsContext, /* overrideAuthority= */ OVERRIDE_AUTHORITY);
@@ -233,7 +239,7 @@ public void tlsClientServer_useSystemRootCerts_useCombinedValidationContext() th
233239
* Uses common_tls_context.validation_context in upstream_tls_context.
234240
*/
235241
@Test
236-
public void tlsClientServer_useSystemRootCerts_validationContext() throws Exception {
242+
public void tlsClientServer_useSystemRootCerts_noMtls_validationContext() throws Exception {
237243
Path trustStoreFilePath = getCacertFilePathForTestCa().toAbsolutePath();
238244
try {
239245
setTrustStoreSystemProperties(trustStoreFilePath.toAbsolutePath().toString());
@@ -244,7 +250,7 @@ public void tlsClientServer_useSystemRootCerts_validationContext() throws Except
244250

245251
UpstreamTlsContext upstreamTlsContext =
246252
setBootstrapInfoAndBuildUpstreamTlsContextForUsingSystemRootCerts(CLIENT_KEY_FILE,
247-
CLIENT_PEM_FILE, false);
253+
CLIENT_PEM_FILE, false, SAN_TO_MATCH, false);
248254

249255
SimpleServiceGrpc.SimpleServiceBlockingStub blockingStub =
250256
getBlockingStub(upstreamTlsContext, /* overrideAuthority= */ OVERRIDE_AUTHORITY);
@@ -255,6 +261,62 @@ public void tlsClientServer_useSystemRootCerts_validationContext() throws Except
255261
}
256262
}
257263

264+
@Test
265+
public void tlsClientServer_useSystemRootCerts_mtls() throws Exception {
266+
Path trustStoreFilePath = getCacertFilePathForTestCa();
267+
try {
268+
setTrustStoreSystemProperties(trustStoreFilePath.toAbsolutePath().toString());
269+
DownstreamTlsContext downstreamTlsContext =
270+
setBootstrapInfoAndBuildDownstreamTlsContext(SERVER_1_PEM_FILE, null, null, null, null,
271+
null, false, true);
272+
buildServerWithTlsContext(downstreamTlsContext);
273+
274+
UpstreamTlsContext upstreamTlsContext =
275+
setBootstrapInfoAndBuildUpstreamTlsContextForUsingSystemRootCerts(CLIENT_KEY_FILE,
276+
CLIENT_PEM_FILE, true, SAN_TO_MATCH, true);
277+
278+
SimpleServiceGrpc.SimpleServiceBlockingStub blockingStub =
279+
getBlockingStub(upstreamTlsContext, /* overrideAuthority= */ OVERRIDE_AUTHORITY);
280+
assertThat(unaryRpc(/* requestMessage= */ "buddy", blockingStub)).isEqualTo("Hello buddy");
281+
} finally {
282+
Files.deleteIfExists(trustStoreFilePath);
283+
clearTrustStoreSystemProperties();
284+
}
285+
}
286+
287+
/**
288+
* Use system root ca cert for TLS channel - no mTLS.
289+
* Subj Alt Names to match are specified in the validaton context.
290+
*/
291+
@Test
292+
public void tlsClientServer_useSystemRootCerts_failureToMatchSubjAltNames() throws Exception {
293+
Path trustStoreFilePath = getCacertFilePathForTestCa();
294+
try {
295+
setTrustStoreSystemProperties(trustStoreFilePath.toAbsolutePath().toString());
296+
DownstreamTlsContext downstreamTlsContext =
297+
setBootstrapInfoAndBuildDownstreamTlsContext(SERVER_1_PEM_FILE, null, null, null, null,
298+
null, false, false);
299+
buildServerWithTlsContext(downstreamTlsContext);
300+
301+
UpstreamTlsContext upstreamTlsContext =
302+
setBootstrapInfoAndBuildUpstreamTlsContextForUsingSystemRootCerts(CLIENT_KEY_FILE,
303+
CLIENT_PEM_FILE, true, "server1.test.google.in", false);
304+
305+
SimpleServiceGrpc.SimpleServiceBlockingStub blockingStub =
306+
getBlockingStub(upstreamTlsContext, /* overrideAuthority= */ OVERRIDE_AUTHORITY);
307+
unaryRpc(/* requestMessage= */ "buddy", blockingStub);
308+
fail("Expected handshake failure exception");
309+
} catch (StatusRuntimeException e) {
310+
assertThat(e.getCause()).isInstanceOf(SSLHandshakeException.class);
311+
assertThat(e.getCause().getCause()).isInstanceOf(CertificateException.class);
312+
assertThat(e.getCause().getCause().getMessage()).isEqualTo(
313+
"Peer certificate SAN check failed");
314+
} finally {
315+
Files.deleteIfExists(trustStoreFilePath);
316+
clearTrustStoreSystemProperties();
317+
}
318+
}
319+
258320
/**
259321
* Use system root ca cert for TLS channel - mTLS.
260322
* Uses common_tls_context.combined_validation_context in upstream_tls_context.
@@ -266,12 +328,12 @@ public void tlsClientServer_useSystemRootCerts_requireClientAuth() throws Except
266328
setTrustStoreSystemProperties(trustStoreFilePath.toAbsolutePath().toString());
267329
DownstreamTlsContext downstreamTlsContext =
268330
setBootstrapInfoAndBuildDownstreamTlsContext(SERVER_1_PEM_FILE, null, null, null, null,
269-
null, false, false);
331+
null, false, true);
270332
buildServerWithTlsContext(downstreamTlsContext);
271333

272334
UpstreamTlsContext upstreamTlsContext =
273335
setBootstrapInfoAndBuildUpstreamTlsContextForUsingSystemRootCerts(CLIENT_KEY_FILE,
274-
CLIENT_PEM_FILE, true);
336+
CLIENT_PEM_FILE, true, SAN_TO_MATCH, false);
275337

276338
SimpleServiceGrpc.SimpleServiceBlockingStub blockingStub =
277339
getBlockingStub(upstreamTlsContext, /* overrideAuthority= */ OVERRIDE_AUTHORITY);
@@ -549,21 +611,26 @@ private UpstreamTlsContext setBootstrapInfoAndBuildUpstreamTlsContext(String cli
549611
.buildUpstreamTlsContext("google_cloud_private_spiffe-client", hasIdentityCert, null, false);
550612
}
551613

614+
@SuppressWarnings("deprecation") // gRFC A29 predates match_typed_subject_alt_names
552615
private UpstreamTlsContext setBootstrapInfoAndBuildUpstreamTlsContextForUsingSystemRootCerts(
553616
String clientKeyFile,
554617
String clientPemFile,
555-
boolean useCombinedValidationContext) {
618+
boolean useCombinedValidationContext, String sanToMatch, boolean isMtls) {
556619
bootstrapInfoForClient = CommonBootstrapperTestUtils
557620
.buildBootstrapInfo("google_cloud_private_spiffe-client", clientKeyFile, clientPemFile,
558621
CA_PEM_FILE, null, null, null, null, null);
559622
if (useCombinedValidationContext) {
560623
return CommonTlsContextTestsUtil.buildUpstreamTlsContextForCertProviderInstance(
561-
"google_cloud_private_spiffe-client", "ROOT", null,
624+
isMtls ? "google_cloud_private_spiffe-client" : null,
625+
isMtls ? "ROOT" : null, null,
562626
null, null,
563627
CertificateValidationContext.newBuilder()
564628
.setSystemRootCerts(
565629
CertificateValidationContext.SystemRootCerts.newBuilder().build())
566-
.build(), null, false);
630+
.addMatchSubjectAltNames(
631+
StringMatcher.newBuilder()
632+
.setExact(sanToMatch))
633+
.build());
567634
}
568635
return CommonTlsContextTestsUtil.buildNewUpstreamTlsContextForCertProviderInstance(
569636
"google_cloud_private_spiffe-client", "ROOT", null,

0 commit comments

Comments
 (0)