From a734f837118c41380122fc9b206ca0cc7d3dbb39 Mon Sep 17 00:00:00 2001 From: Kannan J Date: Thu, 16 Oct 2025 15:55:25 +0000 Subject: [PATCH 1/2] Save changes. --- .../internal/security/DynamicSslContextProvider.java | 5 +++++ .../internal/security/SecurityProtocolNegotiators.java | 8 ++++---- .../internal/security/SslContextProviderSupplier.java | 5 ++++- .../CertProviderClientSslContextProvider.java | 6 ++++-- .../grpc/xds/XdsClientWrapperForServerSdsTestMisc.java | 2 +- .../security/SecurityProtocolNegotiatorsTest.java | 6 +++--- .../security/SslContextProviderSupplierTest.java | 10 +++++----- 7 files changed, 26 insertions(+), 16 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/internal/security/DynamicSslContextProvider.java b/xds/src/main/java/io/grpc/xds/internal/security/DynamicSslContextProvider.java index e7b27cd644a..68c04a3a052 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/DynamicSslContextProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/DynamicSslContextProvider.java @@ -44,6 +44,7 @@ public abstract class DynamicSslContextProvider extends SslContextProvider { @Nullable protected final CertificateValidationContext staticCertificateValidationContext; @Nullable protected AbstractMap.SimpleImmutableEntry sslContextAndTrustManager; + private boolean autoSniSanValidationDoesNotApply; protected DynamicSslContextProvider( BaseTlsContext tlsContext, CertificateValidationContext staticCertValidationContext) { @@ -59,6 +60,10 @@ protected DynamicSslContextProvider( protected abstract CertificateValidationContext generateCertificateValidationContext(); + public void setAutoSniSanValidationDoesNotApply() { + autoSniSanValidationDoesNotApply = true; + } + /** Gets a server or client side SslContextBuilder. */ protected abstract AbstractMap.SimpleImmutableEntry getSslContextBuilderAndTrustManager( diff --git a/xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java b/xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java index 10e3a0bcda1..3ff206269bc 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java @@ -260,8 +260,8 @@ public void updateSslContextAndExtendedX509TrustManager( public void onException(Throwable throwable) { ctx.fireExceptionCaught(throwable); } - } - ); + }, + false); } @Override @@ -399,8 +399,8 @@ public void updateSslContextAndExtendedX509TrustManager( public void onException(Throwable throwable) { ctx.fireExceptionCaught(throwable); } - } - ); + }, + false); } } } diff --git a/xds/src/main/java/io/grpc/xds/internal/security/SslContextProviderSupplier.java b/xds/src/main/java/io/grpc/xds/internal/security/SslContextProviderSupplier.java index 38ae15a88aa..e5960dd95e8 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/SslContextProviderSupplier.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/SslContextProviderSupplier.java @@ -55,12 +55,15 @@ public BaseTlsContext getTlsContext() { /** Updates SslContext via the passed callback. */ public synchronized void updateSslContext( - final SslContextProvider.Callback callback) { + final SslContextProvider.Callback callback, boolean autoSniSanValidationDoesNotApply) { checkNotNull(callback, "callback"); try { if (!shutdown) { if (sslContextProvider == null) { sslContextProvider = getSslContextProvider(); + if (tlsContext instanceof UpstreamTlsContext && autoSniSanValidationDoesNotApply) { + ((DynamicSslContextProvider) sslContextProvider).setAutoSniSanValidationDoesNotApply(); + } } } // we want to increment the ref-count so call findOrCreate again... 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 e92f9ad1e54..b4b72ae11c6 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 @@ -64,13 +64,15 @@ final class CertProviderClientSslContextProvider extends CertProviderSslContextP new XdsTrustManagerFactory( savedSpiffeTrustMap, certificateValidationContext, - ((UpstreamTlsContext) tlsContext).getAutoSniSanValidation())); + autoSniSanValidationDoesNotApply + ? false : ((UpstreamTlsContext) tlsContext).getAutoSniSanValidation())); } else if (savedTrustedRoots != null) { sslContextBuilder = sslContextBuilder.trustManager( new XdsTrustManagerFactory( savedTrustedRoots.toArray(new X509Certificate[0]), certificateValidationContext, - ((UpstreamTlsContext) tlsContext).getAutoSniSanValidation())); + autoSniSanValidationDoesNotApply + ? false : ((UpstreamTlsContext) tlsContext).getAutoSniSanValidation())); } else { // Should be impossible because of the check in CertProviderClientSslContextProviderFactory throw new IllegalStateException("There must be trusted roots or a SPIFFE trust map"); diff --git a/xds/src/test/java/io/grpc/xds/XdsClientWrapperForServerSdsTestMisc.java b/xds/src/test/java/io/grpc/xds/XdsClientWrapperForServerSdsTestMisc.java index ff97afe6916..f997f583898 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientWrapperForServerSdsTestMisc.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientWrapperForServerSdsTestMisc.java @@ -301,7 +301,7 @@ public void releaseOldSupplierOnTemporaryError_noClose() throws Exception { private void callUpdateSslContext(SslContextProviderSupplier sslContextProviderSupplier) { assertThat(sslContextProviderSupplier).isNotNull(); SslContextProvider.Callback callback = mock(SslContextProvider.Callback.class); - sslContextProviderSupplier.updateSslContext(callback); + sslContextProviderSupplier.updateSslContext(callback, false); } private void sendListenerUpdate( diff --git a/xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java b/xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java index 061e6bad581..dcb2fa051a3 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java @@ -215,7 +215,7 @@ public void updateSslContextAndExtendedX509TrustManager( protected void onException(Throwable throwable) { future.set(throwable); } - }); + }, false); assertThat(executor.runDueTasks()).isEqualTo(1); channel.runPendingTasks(); Object fromFuture = future.get(2, TimeUnit.SECONDS); @@ -401,7 +401,7 @@ public void updateSslContextAndExtendedX509TrustManager( protected void onException(Throwable throwable) { future.set(throwable); } - }); + }, false); channel.runPendingTasks(); // need this for tasks to execute on eventLoop assertThat(executor.runDueTasks()).isEqualTo(1); Object fromFuture = future.get(2, TimeUnit.SECONDS); @@ -540,7 +540,7 @@ public void updateSslContextAndExtendedX509TrustManager( protected void onException(Throwable throwable) { future.set(throwable); } - }); + }, false); executor.runDueTasks(); channel.runPendingTasks(); // need this for tasks to execute on eventLoop Object fromFuture = future.get(5, TimeUnit.SECONDS); diff --git a/xds/src/test/java/io/grpc/xds/internal/security/SslContextProviderSupplierTest.java b/xds/src/test/java/io/grpc/xds/internal/security/SslContextProviderSupplierTest.java index 9b6e1ecbc74..70a53c53205 100644 --- a/xds/src/test/java/io/grpc/xds/internal/security/SslContextProviderSupplierTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/security/SslContextProviderSupplierTest.java @@ -69,7 +69,7 @@ private void prepareSupplier() { private void callUpdateSslContext() { mockCallback = mock(SslContextProvider.Callback.class); doReturn(mockExecutor).when(mockCallback).getExecutor(); - supplier.updateSslContext(mockCallback); + supplier.updateSslContext(mockCallback, false); } @Test @@ -94,7 +94,7 @@ public void get_updateSecret() { verify(mockTlsContextManager, times(1)) .releaseClientSslContextProvider(eq(mockSslContextProvider)); SslContextProvider.Callback mockCallback = mock(SslContextProvider.Callback.class); - supplier.updateSslContext(mockCallback); + supplier.updateSslContext(mockCallback, false); verify(mockTlsContextManager, times(3)) .findOrCreateClientSslContextProvider(eq(upstreamTlsContext)); } @@ -121,7 +121,7 @@ public void autoHostSniFalse_usesSniFromUpstreamTlsContext() { verify(mockTlsContextManager, times(1)) .releaseClientSslContextProvider(eq(mockSslContextProvider)); SslContextProvider.Callback mockCallback = mock(SslContextProvider.Callback.class); - supplier.updateSslContext(mockCallback); + supplier.updateSslContext(mockCallback, false); verify(mockTlsContextManager, times(3)) .findOrCreateClientSslContextProvider(eq(upstreamTlsContext)); } @@ -178,7 +178,7 @@ public void systemRootCertsWithMtls_callbackExecutedFromProvider() { verify(mockTlsContextManager, times(1)) .releaseClientSslContextProvider(eq(mockSslContextProvider)); SslContextProvider.Callback mockCallback = mock(SslContextProvider.Callback.class); - supplier.updateSslContext(mockCallback); + supplier.updateSslContext(mockCallback, false); verify(mockTlsContextManager, times(3)) .findOrCreateClientSslContextProvider(eq(upstreamTlsContext)); } @@ -190,7 +190,7 @@ public void testClose() { supplier.close(); verify(mockTlsContextManager, times(1)) .releaseClientSslContextProvider(eq(mockSslContextProvider)); - supplier.updateSslContext(mockCallback); + supplier.updateSslContext(mockCallback, false); verify(mockTlsContextManager, times(3)) .findOrCreateClientSslContextProvider(eq(upstreamTlsContext)); verify(mockTlsContextManager, times(1)) From 79ebe403549c59f09e6f26664e5736e628422bb1 Mon Sep 17 00:00:00 2001 From: Kannan J Date: Thu, 16 Oct 2025 17:32:16 +0000 Subject: [PATCH 2/2] Introduce flag for fallback to use the xds channel authority if no SNI is determined to be used. --- .../java/io/grpc/netty/ProtocolNegotiators.java | 3 --- .../security/DynamicSslContextProvider.java | 2 +- .../security/SecurityProtocolNegotiators.java | 14 ++++++++++++-- .../internal/security/trust/CertificateUtils.java | 2 ++ 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/netty/src/main/java/io/grpc/netty/ProtocolNegotiators.java b/netty/src/main/java/io/grpc/netty/ProtocolNegotiators.java index 59e7e96b4a5..d30a6292d38 100644 --- a/netty/src/main/java/io/grpc/netty/ProtocolNegotiators.java +++ b/netty/src/main/java/io/grpc/netty/ProtocolNegotiators.java @@ -634,9 +634,6 @@ static final class ClientTlsHandler extends ProtocolNegotiationHandler { X509TrustManager x509TrustManager) { super(next, negotiationLogger); this.sslContext = Preconditions.checkNotNull(sslContext, "sslContext"); - // TODO: For empty authority and fallback flag - // GRPC_USE_CHANNEL_AUTHORITY_IF_NO_SNI_APPLICABLE present, we should parse authority - // but prevent it from being used for SAN validation in the TrustManager. if (authority != null) { HostPort hostPort = parseAuthority(authority); this.host = hostPort.host; diff --git a/xds/src/main/java/io/grpc/xds/internal/security/DynamicSslContextProvider.java b/xds/src/main/java/io/grpc/xds/internal/security/DynamicSslContextProvider.java index 68c04a3a052..59e114a89ff 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/DynamicSslContextProvider.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/DynamicSslContextProvider.java @@ -44,7 +44,7 @@ public abstract class DynamicSslContextProvider extends SslContextProvider { @Nullable protected final CertificateValidationContext staticCertificateValidationContext; @Nullable protected AbstractMap.SimpleImmutableEntry sslContextAndTrustManager; - private boolean autoSniSanValidationDoesNotApply; + protected boolean autoSniSanValidationDoesNotApply; protected DynamicSslContextProvider( BaseTlsContext tlsContext, CertificateValidationContext staticCertValidationContext) { diff --git a/xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java b/xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java index 3ff206269bc..0da06b3a753 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java @@ -194,6 +194,7 @@ static final class ClientSecurityHandler private final GrpcHttp2ConnectionHandler grpcHandler; private final SslContextProviderSupplier sslContextProviderSupplier; private final String sni; + private final boolean autoSniSanValidationDoesNotApply; ClientSecurityHandler( GrpcHttp2ConnectionHandler grpcHandler, @@ -215,10 +216,19 @@ public void handlerAdded(ChannelHandlerContext ctx) throws Exception { EnvoyServerProtoData.BaseTlsContext tlsContext = sslContextProviderSupplier.getTlsContext(); UpstreamTlsContext upstreamTlsContext = ((UpstreamTlsContext) tlsContext); if (CertificateUtils.isXdsSniEnabled) { - sni = upstreamTlsContext.getAutoHostSni() && !Strings.isNullOrEmpty(endpointHostname) + String sniToUse = upstreamTlsContext.getAutoHostSni() + && !Strings.isNullOrEmpty(endpointHostname) ? endpointHostname : upstreamTlsContext.getSni(); + if (sniToUse.isEmpty() && CertificateUtils.useChannelAuthorityIfNoSniApplicable) { + sniToUse = grpcHandler.getAuthority(); + autoSniSanValidationDoesNotApply = true; + } else { + autoSniSanValidationDoesNotApply = false; + } + sni = sniToUse; } else { sni = grpcHandler.getAuthority(); + autoSniSanValidationDoesNotApply = false; } } @@ -261,7 +271,7 @@ public void onException(Throwable throwable) { ctx.fireExceptionCaught(throwable); } }, - false); + autoSniSanValidationDoesNotApply); } @Override diff --git a/xds/src/main/java/io/grpc/xds/internal/security/trust/CertificateUtils.java b/xds/src/main/java/io/grpc/xds/internal/security/trust/CertificateUtils.java index 89b4abd3029..30535df836e 100644 --- a/xds/src/main/java/io/grpc/xds/internal/security/trust/CertificateUtils.java +++ b/xds/src/main/java/io/grpc/xds/internal/security/trust/CertificateUtils.java @@ -31,6 +31,8 @@ */ public final class CertificateUtils { public static boolean isXdsSniEnabled = GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_SNI", false); + public static boolean useChannelAuthorityIfNoSniApplicable + = GrpcUtil.getFlag("GRPC_USE_CHANNEL_AUTHORITY_IF_NO_SNI_APPLICABLE", false); /** * Generates X509Certificate array from a file on disk.