diff --git a/xds/src/main/java/io/grpc/xds/EnvoyServerProtoData.java b/xds/src/main/java/io/grpc/xds/EnvoyServerProtoData.java index 978e6663cbe..4a6213277e7 100644 --- a/xds/src/main/java/io/grpc/xds/EnvoyServerProtoData.java +++ b/xds/src/main/java/io/grpc/xds/EnvoyServerProtoData.java @@ -16,6 +16,8 @@ package io.grpc.xds; +import static com.google.common.base.Preconditions.checkNotNull; + import com.google.auto.value.AutoValue; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; @@ -41,13 +43,13 @@ private EnvoyServerProtoData() { } public abstract static class BaseTlsContext { - @Nullable protected final CommonTlsContext commonTlsContext; + protected final CommonTlsContext commonTlsContext; - protected BaseTlsContext(@Nullable CommonTlsContext commonTlsContext) { - this.commonTlsContext = commonTlsContext; + protected BaseTlsContext(CommonTlsContext commonTlsContext) { + this.commonTlsContext = checkNotNull(commonTlsContext, "commonTlsContext cannot be null."); } - @Nullable public CommonTlsContext getCommonTlsContext() { + public CommonTlsContext getCommonTlsContext() { return commonTlsContext; } diff --git a/xds/src/main/java/io/grpc/xds/XdsClusterResource.java b/xds/src/main/java/io/grpc/xds/XdsClusterResource.java index c6340156d49..c7789f3d7dd 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClusterResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsClusterResource.java @@ -39,6 +39,7 @@ import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext; import io.grpc.LoadBalancerRegistry; import io.grpc.NameResolver; +import io.grpc.internal.GrpcUtil; import io.grpc.internal.ServiceConfigUtil; import io.grpc.internal.ServiceConfigUtil.LbConfig; import io.grpc.xds.EnvoyServerProtoData.OutlierDetection; @@ -46,6 +47,7 @@ import io.grpc.xds.XdsClusterResource.CdsUpdate; import io.grpc.xds.client.XdsClient.ResourceUpdate; import io.grpc.xds.client.XdsResourceType; +import io.grpc.xds.internal.security.CommonTlsContextUtil; import java.util.List; import java.util.Locale; import java.util.Set; @@ -57,6 +59,9 @@ class XdsClusterResource extends XdsResourceType { !Strings.isNullOrEmpty(System.getenv("GRPC_EXPERIMENTAL_ENABLE_LEAST_REQUEST")) ? Boolean.parseBoolean(System.getenv("GRPC_EXPERIMENTAL_ENABLE_LEAST_REQUEST")) : Boolean.parseBoolean(System.getProperty("io.grpc.xds.experimentalEnableLeastRequest")); + @VisibleForTesting + public static boolean enableSystemRootCerts = + GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_SYSTEM_ROOT_CERTS", false); @VisibleForTesting static final String AGGREGATE_CLUSTER_TYPE_NAME = "envoy.clusters.aggregate"; @@ -430,9 +435,11 @@ static void validateCommonTlsContext( } String rootCaInstanceName = getRootCertInstanceName(commonTlsContext); if (rootCaInstanceName == null) { - if (!server) { + if (!server && (!enableSystemRootCerts + || !CommonTlsContextUtil.isUsingSystemRootCerts(commonTlsContext))) { throw new ResourceInvalidException( - "ca_certificate_provider_instance is required in upstream-tls-context"); + "ca_certificate_provider_instance or system_root_certs is required in " + + "upstream-tls-context"); } } else { if (certProviderInstances == null || !certProviderInstances.contains(rootCaInstanceName)) { diff --git a/xds/src/main/java/io/grpc/xds/internal/security/ClientSslContextProviderFactory.java b/xds/src/main/java/io/grpc/xds/internal/security/ClientSslContextProviderFactory.java index 90202b4820a..37d289c1c47 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/ClientSslContextProviderFactory.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/ClientSslContextProviderFactory.java @@ -16,8 +16,6 @@ package io.grpc.xds.internal.security; -import static com.google.common.base.Preconditions.checkNotNull; - import io.grpc.xds.EnvoyServerProtoData.UpstreamTlsContext; import io.grpc.xds.client.Bootstrapper.BootstrapInfo; import io.grpc.xds.internal.security.ReferenceCountingMap.ValueFactory; @@ -44,17 +42,9 @@ final class ClientSslContextProviderFactory /** Creates an SslContextProvider from the given UpstreamTlsContext. */ @Override public SslContextProvider create(UpstreamTlsContext upstreamTlsContext) { - checkNotNull(upstreamTlsContext, "upstreamTlsContext"); - checkNotNull( - upstreamTlsContext.getCommonTlsContext(), - "upstreamTlsContext should have CommonTlsContext"); - if (CommonTlsContextUtil.hasCertProviderInstance( - upstreamTlsContext.getCommonTlsContext())) { - return certProviderClientSslContextProviderFactory.getProvider( - upstreamTlsContext, - bootstrapInfo.node().toEnvoyProtoNode(), - bootstrapInfo.certProviders()); - } - throw new UnsupportedOperationException("Unsupported configurations in UpstreamTlsContext!"); + return certProviderClientSslContextProviderFactory.getProvider( + upstreamTlsContext, + bootstrapInfo.node().toEnvoyProtoNode(), + bootstrapInfo.certProviders()); } } diff --git a/xds/src/main/java/io/grpc/xds/internal/security/CommonTlsContextUtil.java b/xds/src/main/java/io/grpc/xds/internal/security/CommonTlsContextUtil.java index d3003b4a792..e5a8c115361 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/CommonTlsContextUtil.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/CommonTlsContextUtil.java @@ -25,7 +25,7 @@ public final class CommonTlsContextUtil { private CommonTlsContextUtil() {} - static boolean hasCertProviderInstance(CommonTlsContext commonTlsContext) { + public static boolean hasCertProviderInstance(CommonTlsContext commonTlsContext) { if (commonTlsContext == null) { return false; } @@ -65,4 +65,15 @@ public static CommonTlsContext.CertificateProviderInstance convert( .setInstanceName(pluginInstance.getInstanceName()) .setCertificateName(pluginInstance.getCertificateName()).build(); } + + public static boolean isUsingSystemRootCerts(CommonTlsContext commonTlsContext) { + if (commonTlsContext.hasCombinedValidationContext()) { + return commonTlsContext.getCombinedValidationContext().getDefaultValidationContext() + .hasSystemRootCerts(); + } + if (commonTlsContext.hasValidationContext()) { + return commonTlsContext.getValidationContext().hasSystemRootCerts(); + } + return false; + } } diff --git a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProvider.java b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProvider.java index d4080101c1a..8aa74b2b50f 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProvider.java @@ -16,8 +16,6 @@ package io.grpc.xds.internal.security.certprovider; -import static com.google.common.base.Preconditions.checkNotNull; - import io.envoyproxy.envoy.config.core.v3.Node; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateValidationContext; import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext; @@ -46,7 +44,7 @@ final class CertProviderClientSslContextProvider extends CertProviderSslContextP node, certProviders, certInstance, - checkNotNull(rootCertInstance, "Client SSL requires rootCertInstance"), + rootCertInstance, staticCertValidationContext, upstreamTlsContext, certificateProviderStore); @@ -56,12 +54,15 @@ final class CertProviderClientSslContextProvider extends CertProviderSslContextP protected final SslContextBuilder getSslContextBuilder( CertificateValidationContext certificateValidationContextdationContext) throws CertStoreException { - SslContextBuilder sslContextBuilder = - GrpcSslContexts.forClient() - .trustManager( - new XdsTrustManagerFactory( - savedTrustedRoots.toArray(new X509Certificate[0]), - certificateValidationContextdationContext)); + SslContextBuilder sslContextBuilder = GrpcSslContexts.forClient(); + // Null rootCertInstance implies hasSystemRootCerts because of the check in + // CertProviderClientSslContextProviderFactory. + if (rootCertInstance != null) { + sslContextBuilder.trustManager( + new XdsTrustManagerFactory( + savedTrustedRoots.toArray(new X509Certificate[0]), + certificateValidationContextdationContext)); + } if (isMtls()) { sslContextBuilder.keyManager(savedKey, savedCertChain); } diff --git a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProviderFactory.java b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProviderFactory.java index 21782741c2c..6205c1c3a63 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProviderFactory.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProviderFactory.java @@ -25,6 +25,7 @@ import io.grpc.Internal; import io.grpc.xds.EnvoyServerProtoData.UpstreamTlsContext; import io.grpc.xds.client.Bootstrapper.CertificateProviderInfo; +import io.grpc.xds.internal.security.CommonTlsContextUtil; import io.grpc.xds.internal.security.SslContextProvider; import java.util.Map; import javax.annotation.Nullable; @@ -64,13 +65,17 @@ public SslContextProvider getProvider( = CertProviderSslContextProvider.getRootCertProviderInstance(commonTlsContext); CommonTlsContext.CertificateProviderInstance certInstance = CertProviderSslContextProvider.getCertProviderInstance(commonTlsContext); - return new CertProviderClientSslContextProvider( - node, - certProviders, - certInstance, - rootCertInstance, - staticCertValidationContext, - upstreamTlsContext, - certificateProviderStore); + if (CommonTlsContextUtil.hasCertProviderInstance(upstreamTlsContext.getCommonTlsContext()) + || CommonTlsContextUtil.isUsingSystemRootCerts(upstreamTlsContext.getCommonTlsContext())) { + return new CertProviderClientSslContextProvider( + node, + certProviders, + certInstance, + rootCertInstance, + staticCertValidationContext, + upstreamTlsContext, + certificateProviderStore); + } + throw new UnsupportedOperationException("Unsupported configurations in UpstreamTlsContext!"); } } diff --git a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderSslContextProvider.java b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderSslContextProvider.java index 6570c619913..b68f705fedb 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderSslContextProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderSslContextProvider.java @@ -37,10 +37,11 @@ abstract class CertProviderSslContextProvider extends DynamicSslContextProvider @Nullable private final CertificateProviderStore.Handle certHandle; @Nullable private final CertificateProviderStore.Handle rootCertHandle; @Nullable private final CertificateProviderInstance certInstance; - @Nullable private final CertificateProviderInstance rootCertInstance; + @Nullable protected final CertificateProviderInstance rootCertInstance; @Nullable protected PrivateKey savedKey; @Nullable protected List savedCertChain; @Nullable protected List savedTrustedRoots; + private final boolean isUsingSystemRootCerts; protected CertProviderSslContextProvider( Node node, @@ -83,6 +84,8 @@ protected CertProviderSslContextProvider( } else { rootCertHandle = null; } + this.isUsingSystemRootCerts = rootCertInstance == null + && CommonTlsContextUtil.isUsingSystemRootCerts(tlsContext.getCommonTlsContext()); } private static CertificateProviderInfo getCertProviderConfig( @@ -151,7 +154,7 @@ public final void updateTrustedRoots(List trustedRoots) { private void updateSslContextWhenReady() { if (isMtls()) { - if (savedKey != null && savedTrustedRoots != null) { + if (savedKey != null && (savedTrustedRoots != null || isUsingSystemRootCerts)) { updateSslContext(); clearKeysAndCerts(); } @@ -175,7 +178,7 @@ private void clearKeysAndCerts() { } protected final boolean isMtls() { - return certInstance != null && rootCertInstance != null; + return certInstance != null && (rootCertInstance != null || isUsingSystemRootCerts); } protected final boolean isClientSideTls() { diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java index 3c159ba7055..a23e7177c29 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java @@ -173,11 +173,13 @@ public class GrpcXdsClientImplDataTest { private final FilterRegistry filterRegistry = FilterRegistry.getDefaultRegistry(); private boolean originalEnableRouteLookup; private boolean originalEnableLeastRequest; + private boolean originalEnableUseSystemRootCerts; @Before public void setUp() { originalEnableRouteLookup = XdsRouteConfigureResource.enableRouteLookup; originalEnableLeastRequest = XdsClusterResource.enableLeastRequest; + originalEnableUseSystemRootCerts = XdsClusterResource.enableSystemRootCerts; assertThat(originalEnableLeastRequest).isFalse(); } @@ -185,6 +187,7 @@ public void setUp() { public void tearDown() { XdsRouteConfigureResource.enableRouteLookup = originalEnableRouteLookup; XdsClusterResource.enableLeastRequest = originalEnableLeastRequest; + XdsClusterResource.enableSystemRootCerts = originalEnableUseSystemRootCerts; } @Test @@ -2503,7 +2506,8 @@ public void validateCommonTlsContext_validationContext() throws ResourceInvalidE .setValidationContext(CertificateValidationContext.getDefaultInstance()) .build(); thrown.expect(ResourceInvalidException.class); - thrown.expectMessage("ca_certificate_provider_instance is required in upstream-tls-context"); + thrown.expectMessage("ca_certificate_provider_instance or system_root_certs is required " + + "in upstream-tls-context"); XdsClusterResource.validateCommonTlsContext(commonTlsContext, null, false); } @@ -2613,6 +2617,87 @@ public void validateCommonTlsContext_validationContextProviderInstance() .validateCommonTlsContext(commonTlsContext, ImmutableSet.of("name1", "name2"), false); } + @Test + public void + validateCommonTlsContext_combinedValidationContextSystemRootCerts_envVarNotSet_throws() { + XdsClusterResource.enableSystemRootCerts = false; + CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() + .setCombinedValidationContext( + CommonTlsContext.CombinedCertificateValidationContext.newBuilder() + .setDefaultValidationContext( + CertificateValidationContext.newBuilder() + .setSystemRootCerts( + CertificateValidationContext.SystemRootCerts.newBuilder().build()) + .build() + ) + .build()) + .build(); + try { + XdsClusterResource + .validateCommonTlsContext(commonTlsContext, ImmutableSet.of(), false); + fail("Expected exception"); + } catch (ResourceInvalidException ex) { + assertThat(ex.getMessage()).isEqualTo( + "ca_certificate_provider_instance or system_root_certs is required in" + + " upstream-tls-context"); + } + } + + @Test + public void validateCommonTlsContext_combinedValidationContextSystemRootCerts() + throws ResourceInvalidException { + XdsClusterResource.enableSystemRootCerts = true; + CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() + .setCombinedValidationContext( + CommonTlsContext.CombinedCertificateValidationContext.newBuilder() + .setDefaultValidationContext( + CertificateValidationContext.newBuilder() + .setSystemRootCerts( + CertificateValidationContext.SystemRootCerts.newBuilder().build()) + .build() + ) + .build()) + .build(); + XdsClusterResource + .validateCommonTlsContext(commonTlsContext, ImmutableSet.of(), false); + } + + @Test + public void validateCommonTlsContext_validationContextSystemRootCerts_envVarNotSet_throws() { + XdsClusterResource.enableSystemRootCerts = false; + CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() + .setValidationContext( + CertificateValidationContext.newBuilder() + .setSystemRootCerts( + CertificateValidationContext.SystemRootCerts.newBuilder().build()) + .build()) + .build(); + try { + XdsClusterResource + .validateCommonTlsContext(commonTlsContext, ImmutableSet.of(), false); + fail("Expected exception"); + } catch (ResourceInvalidException ex) { + assertThat(ex.getMessage()).isEqualTo( + "ca_certificate_provider_instance or system_root_certs is required in " + + "upstream-tls-context"); + } + } + + @Test + public void validateCommonTlsContext_validationContextSystemRootCerts() + throws ResourceInvalidException { + XdsClusterResource.enableSystemRootCerts = true; + CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() + .setValidationContext( + CertificateValidationContext.newBuilder() + .setSystemRootCerts( + CertificateValidationContext.SystemRootCerts.newBuilder().build()) + .build()) + .build(); + XdsClusterResource + .validateCommonTlsContext(commonTlsContext, ImmutableSet.of(), false); + } + @Test @SuppressWarnings("deprecation") public void validateCommonTlsContext_validationContextProviderInstance_absentInBootstrapFile() @@ -2674,7 +2759,8 @@ public void validateCommonTlsContext_combinedValidationContext_isRequiredForClie CommonTlsContext commonTlsContext = CommonTlsContext.newBuilder() .build(); thrown.expect(ResourceInvalidException.class); - thrown.expectMessage("ca_certificate_provider_instance is required in upstream-tls-context"); + thrown.expectMessage("ca_certificate_provider_instance or system_root_certs is required " + + "in upstream-tls-context"); XdsClusterResource.validateCommonTlsContext(commonTlsContext, null, false); } @@ -2687,7 +2773,8 @@ public void validateCommonTlsContext_combinedValidationContextWithoutCertProvide .build(); thrown.expect(ResourceInvalidException.class); thrown.expectMessage( - "ca_certificate_provider_instance is required in upstream-tls-context"); + "ca_certificate_provider_instance or system_root_certs is required in " + + "upstream-tls-context"); XdsClusterResource.validateCommonTlsContext(commonTlsContext, null, false); } diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java index a9fda599183..f516dc87e98 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java @@ -2217,7 +2217,8 @@ public void cdsResponseErrorHandling_badUpstreamTlsContext() { String errorMsg = "CDS response Cluster 'cluster.googleapis.com' validation error: " + "Cluster cluster.googleapis.com: malformed UpstreamTlsContext: " + "io.grpc.xds.client.XdsResourceType$ResourceInvalidException: " - + "ca_certificate_provider_instance is required in upstream-tls-context"; + + "ca_certificate_provider_instance or system_root_certs is required in " + + "upstream-tls-context"; call.verifyRequestNack(CDS, CDS_RESOURCE, "", "0000", NODE, ImmutableList.of(errorMsg)); verify(cdsResourceWatcher).onError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, errorMsg); diff --git a/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java b/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java index c6b8e7515b2..f56a7c367bb 100644 --- a/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java @@ -31,6 +31,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.SettableFuture; +import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateValidationContext; import io.grpc.Attributes; import io.grpc.EquivalentAddressGroup; import io.grpc.Grpc; @@ -67,10 +68,21 @@ import io.grpc.xds.internal.security.SslContextProviderSupplier; import io.grpc.xds.internal.security.TlsContextManagerImpl; import io.netty.handler.ssl.NotSslRecordException; +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; import java.net.Inet4Address; import java.net.InetSocketAddress; import java.net.URI; import java.net.URISyntaxException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.NoSuchAlgorithmException; +import java.security.cert.CertificateException; +import java.security.cert.CertificateFactory; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -104,7 +116,7 @@ public class XdsSecurityClientServerTest { private static final String OVERRIDE_AUTHORITY = "foo.test.google.fr"; @After - public void tearDown() { + public void tearDown() throws IOException { if (fakeNameResolverFactory != null) { NameResolverRegistry.getDefaultRegistry().deregister(fakeNameResolverFactory); } @@ -147,6 +159,99 @@ public void tlsClientServer_noClientAuthentication() throws Exception { assertThat(unaryRpc(/* requestMessage= */ "buddy", blockingStub)).isEqualTo("Hello buddy"); } + /** + * Use system root ca cert for TLS channel - no mTLS. + * Uses common_tls_context.combined_validation_context in upstream_tls_context. + */ + @Test + public void tlsClientServer_useSystemRootCerts_useCombinedValidationContext() throws Exception { + Path trustStoreFilePath = getCacertFilePathForTestCa(); + try { + setTrustStoreSystemProperties(trustStoreFilePath.toAbsolutePath().toString()); + DownstreamTlsContext downstreamTlsContext = + setBootstrapInfoAndBuildDownstreamTlsContext(null, null, null, null, false, false); + buildServerWithTlsContext(downstreamTlsContext); + + UpstreamTlsContext upstreamTlsContext = + setBootstrapInfoAndBuildUpstreamTlsContextForUsingSystemRootCerts(CLIENT_KEY_FILE, + CLIENT_PEM_FILE, true); + + SimpleServiceGrpc.SimpleServiceBlockingStub blockingStub = + getBlockingStub(upstreamTlsContext, /* overrideAuthority= */ OVERRIDE_AUTHORITY); + assertThat(unaryRpc(/* requestMessage= */ "buddy", blockingStub)).isEqualTo("Hello buddy"); + } finally { + Files.deleteIfExists(trustStoreFilePath); + clearTrustStoreSystemProperties(); + } + } + + /** + * Use system root ca cert for TLS channel - no mTLS. + * Uses common_tls_context.validation_context in upstream_tls_context. + */ + @Test + public void tlsClientServer_useSystemRootCerts_validationContext() throws Exception { + Path trustStoreFilePath = getCacertFilePathForTestCa().toAbsolutePath(); + try { + setTrustStoreSystemProperties(trustStoreFilePath.toAbsolutePath().toString()); + DownstreamTlsContext downstreamTlsContext = + setBootstrapInfoAndBuildDownstreamTlsContext(null, null, null, null, false, false); + buildServerWithTlsContext(downstreamTlsContext); + + UpstreamTlsContext upstreamTlsContext = + setBootstrapInfoAndBuildUpstreamTlsContextForUsingSystemRootCerts(CLIENT_KEY_FILE, + CLIENT_PEM_FILE, false); + + SimpleServiceGrpc.SimpleServiceBlockingStub blockingStub = + getBlockingStub(upstreamTlsContext, /* overrideAuthority= */ OVERRIDE_AUTHORITY); + assertThat(unaryRpc(/* requestMessage= */ "buddy", blockingStub)).isEqualTo("Hello buddy"); + } finally { + Files.deleteIfExists(trustStoreFilePath.toAbsolutePath()); + clearTrustStoreSystemProperties(); + } + } + + /** + * Use system root ca cert for TLS channel - mTLS. + * Uses common_tls_context.combined_validation_context in upstream_tls_context. + */ + @Test + public void tlsClientServer_useSystemRootCerts_requireClientAuth() throws Exception { + Path trustStoreFilePath = getCacertFilePathForTestCa().toAbsolutePath(); + try { + setTrustStoreSystemProperties(trustStoreFilePath.toAbsolutePath().toString()); + DownstreamTlsContext downstreamTlsContext = + setBootstrapInfoAndBuildDownstreamTlsContext(null, null, null, null, false, false); + buildServerWithTlsContext(downstreamTlsContext); + + UpstreamTlsContext upstreamTlsContext = + setBootstrapInfoAndBuildUpstreamTlsContextForUsingSystemRootCerts(CLIENT_KEY_FILE, + CLIENT_PEM_FILE, true); + + SimpleServiceGrpc.SimpleServiceBlockingStub blockingStub = + getBlockingStub(upstreamTlsContext, /* overrideAuthority= */ OVERRIDE_AUTHORITY); + assertThat(unaryRpc(/* requestMessage= */ "buddy", blockingStub)).isEqualTo("Hello buddy"); + } finally { + Files.deleteIfExists(trustStoreFilePath.toAbsolutePath()); + clearTrustStoreSystemProperties(); + } + } + + private Path getCacertFilePathForTestCa() + throws IOException, KeyStoreException, CertificateException, NoSuchAlgorithmException { + KeyStore keystore = KeyStore.getInstance(KeyStore.getDefaultType()); + keystore.load(null, null); + InputStream caCertStream = getClass().getResource("/certs/ca.pem").openStream(); + keystore.setCertificateEntry("testca", CertificateFactory.getInstance("X.509") + .generateCertificate(caCertStream)); + caCertStream.close(); + File trustStoreFile = File.createTempFile("testca-truststore", "jks"); + FileOutputStream out = new FileOutputStream(trustStoreFile); + keystore.store(out, "changeit".toCharArray()); + out.close(); + return trustStoreFile.toPath(); + } + @Test public void requireClientAuth_noClientCert_expectException() throws Exception { @@ -323,6 +428,30 @@ private UpstreamTlsContext setBootstrapInfoAndBuildUpstreamTlsContext(String cli .buildUpstreamTlsContext("google_cloud_private_spiffe-client", hasIdentityCert); } + private UpstreamTlsContext setBootstrapInfoAndBuildUpstreamTlsContextForUsingSystemRootCerts( + String clientKeyFile, + String clientPemFile, + boolean useCombinedValidationContext) { + bootstrapInfoForClient = CommonBootstrapperTestUtils + .buildBootstrapInfo("google_cloud_private_spiffe-client", clientKeyFile, clientPemFile, + CA_PEM_FILE, null, null, null, null); + if (useCombinedValidationContext) { + return CommonTlsContextTestsUtil.buildUpstreamTlsContextForCertProviderInstance( + "google_cloud_private_spiffe-client", "ROOT", null, + null, null, + CertificateValidationContext.newBuilder() + .setSystemRootCerts( + CertificateValidationContext.SystemRootCerts.newBuilder().build()) + .build()); + } + return CommonTlsContextTestsUtil.buildNewUpstreamTlsContextForCertProviderInstance( + "google_cloud_private_spiffe-client", "ROOT", null, + null, null, CertificateValidationContext.newBuilder() + .setSystemRootCerts( + CertificateValidationContext.SystemRootCerts.newBuilder().build()) + .build()); + } + private void buildServerWithTlsContext(DownstreamTlsContext downstreamTlsContext) throws Exception { buildServerWithTlsContext(downstreamTlsContext, InsecureServerCredentials.create()); @@ -450,6 +579,18 @@ public void run() { return settableFuture; } + private void setTrustStoreSystemProperties(String trustStoreFilePath) { + System.setProperty("javax.net.ssl.trustStore", trustStoreFilePath); + System.setProperty("javax.net.ssl.trustStorePassword", "changeit"); + System.setProperty("javax.net.ssl.trustStoreType", "JKS"); + } + + private void clearTrustStoreSystemProperties() { + System.clearProperty("javax.net.ssl.trustStore"); + System.clearProperty("javax.net.ssl.trustStorePassword"); + System.clearProperty("javax.net.ssl.trustStoreType"); + } + private static class SimpleServiceImpl extends SimpleServiceGrpc.SimpleServiceImplBase { @Override diff --git a/xds/src/test/java/io/grpc/xds/internal/security/ClientSslContextProviderFactoryTest.java b/xds/src/test/java/io/grpc/xds/internal/security/ClientSslContextProviderFactoryTest.java index 4de881c710e..23e30883307 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/ClientSslContextProviderFactoryTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/ClientSslContextProviderFactoryTest.java @@ -38,8 +38,6 @@ import io.grpc.xds.internal.security.certprovider.CertificateProviderRegistry; import io.grpc.xds.internal.security.certprovider.CertificateProviderStore; import io.grpc.xds.internal.security.certprovider.TestCertificateProvider; -import java.io.IOException; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -285,22 +283,6 @@ public void createNewCertProviderClientSslContextProvider_onlyRootCert() { verifyWatcher(sslContextProvider, watcherCaptor[0]); } - @Test - public void createNullCommonTlsContext_exception() throws IOException { - clientSslContextProviderFactory = - new ClientSslContextProviderFactory( - null, certProviderClientSslContextProviderFactory); - UpstreamTlsContext upstreamTlsContext = new UpstreamTlsContext(null); - try { - clientSslContextProviderFactory.create(upstreamTlsContext); - Assert.fail("no exception thrown"); - } catch (NullPointerException expected) { - assertThat(expected) - .hasMessageThat() - .isEqualTo("upstreamTlsContext should have CommonTlsContext"); - } - } - static void createAndRegisterProviderProvider( CertificateProviderRegistry certificateProviderRegistry, final CertificateProvider.DistributorWatcher[] watcherCaptor, diff --git a/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java b/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java index 8a04a3d02a7..db82a8682e0 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/CommonTlsContextTestsUtil.java @@ -250,22 +250,28 @@ private static CommonTlsContext.Builder addCertificateValidationContext( String rootInstanceName, String rootCertName, CertificateValidationContext staticCertValidationContext) { + CertificateProviderInstance providerInstance = null; if (rootInstanceName != null) { - CertificateProviderInstance providerInstance = - CertificateProviderInstance.newBuilder() - .setInstanceName(rootInstanceName) - .setCertificateName(rootCertName) - .build(); - if (staticCertValidationContext != null) { - CombinedCertificateValidationContext combined = - CombinedCertificateValidationContext.newBuilder() - .setDefaultValidationContext(staticCertValidationContext) - .setValidationContextCertificateProviderInstance(providerInstance) - .build(); - return builder.setCombinedValidationContext(combined); - } + providerInstance = CertificateProviderInstance.newBuilder() + .setInstanceName(rootInstanceName) + .setCertificateName(rootCertName) + .build(); + } + if (providerInstance != null) { builder = builder.setValidationContextCertificateProviderInstance(providerInstance); } + CombinedCertificateValidationContext.Builder combined = + CombinedCertificateValidationContext.newBuilder(); + if (providerInstance != null) { + combined = combined.setValidationContextCertificateProviderInstance(providerInstance); + } + if (staticCertValidationContext != null) { + combined = combined.setDefaultValidationContext(staticCertValidationContext); + } + if (combined.hasValidationContextCertificateProviderInstance() + || combined.hasDefaultValidationContext()) { + builder = builder.setCombinedValidationContext(combined.build()); + } return builder; } @@ -274,19 +280,19 @@ private static CommonTlsContext.Builder addNewCertificateValidationContext( String rootInstanceName, String rootCertName, CertificateValidationContext staticCertValidationContext) { + CertificateValidationContext.Builder validationContextBuilder = + staticCertValidationContext != null ? staticCertValidationContext.toBuilder() + : CertificateValidationContext.newBuilder(); if (rootInstanceName != null) { CertificateProviderPluginInstance providerInstance = CertificateProviderPluginInstance.newBuilder() .setInstanceName(rootInstanceName) .setCertificateName(rootCertName) .build(); - CertificateValidationContext.Builder validationContextBuilder = - staticCertValidationContext != null ? staticCertValidationContext.toBuilder() - : CertificateValidationContext.newBuilder(); - return builder.setValidationContext( - validationContextBuilder.setCaCertificateProviderInstance(providerInstance)); + validationContextBuilder = validationContextBuilder.setCaCertificateProviderInstance( + providerInstance); } - return builder; + return builder.setValidationContext(validationContextBuilder); } /** Helper method to build UpstreamTlsContext for CertProvider tests. */ diff --git a/xds/src/test/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProviderTest.java b/xds/src/test/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProviderTest.java index 5925c5f03b1..7c300c88297 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProviderTest.java @@ -338,7 +338,8 @@ public void testProviderForClient_sslContextException_onError() throws Exception } @Test - public void testProviderForClient_rootInstanceNull_expectError() throws Exception { + public void testProviderForClient_rootInstanceNull_and_notUsingSystemRootCerts_expectError() + throws Exception { final CertificateProvider.DistributorWatcher[] watcherCaptor = new CertificateProvider.DistributorWatcher[1]; TestCertificateProvider.createAndRegisterProviderProvider( @@ -351,11 +352,30 @@ public void testProviderForClient_rootInstanceNull_expectError() throws Exceptio /* alpnProtocols= */ null, /* staticCertValidationContext= */ null); fail("exception expected"); - } catch (NullPointerException expected) { - assertThat(expected).hasMessageThat().contains("Client SSL requires rootCertInstance"); + } catch (UnsupportedOperationException expected) { + assertThat(expected).hasMessageThat().contains("Unsupported configurations in " + + "UpstreamTlsContext!"); } } + @Test + public void testProviderForClient_rootInstanceNull_but_isUsingSystemRootCerts_valid() + throws Exception { + final CertificateProvider.DistributorWatcher[] watcherCaptor = + new CertificateProvider.DistributorWatcher[1]; + TestCertificateProvider.createAndRegisterProviderProvider( + certificateProviderRegistry, watcherCaptor, "testca", 0); + getSslContextProvider( + /* certInstanceName= */ null, + /* rootInstanceName= */ null, + CommonBootstrapperTestUtils.getTestBootstrapInfo(), + /* alpnProtocols= */ null, + CertificateValidationContext.newBuilder() + .setSystemRootCerts( + CertificateValidationContext.SystemRootCerts.newBuilder().build()) + .build()); + } + static class QueuedExecutor implements Executor { /** A list of Runnables to be run in order. */ @VisibleForTesting final Queue runQueue = new ConcurrentLinkedQueue<>();