Skip to content

xds: Introduce flag for fallback to use the xds channel authority if no SNI is determined to be used. #12422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 21, 2025
Merged

xds: Introduce flag for fallback to use the xds channel authority if no SNI is determined to be used. #12422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a734f83
Save changes.
kannanjgithub Oct 16, 2025
79ebe40
Introduce flag for fallback to use the xds channel authority if no SN…
kannanjgithub Oct 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions netty/src/main/java/io/grpc/netty/ProtocolNegotiators.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions xds/src/main/java/io/grpc/xds/internal/security/DynamicSslContextProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public abstract class DynamicSslContextProvider extends SslContextProvider {
@Nullable protected final CertificateValidationContext staticCertificateValidationContext;
@Nullable protected AbstractMap.SimpleImmutableEntry<SslContext, X509TrustManager>
sslContextAndTrustManager;
protected boolean autoSniSanValidationDoesNotApply;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this a settable field that is set separately instead of making it a final field in CertProviderClientSslContextProvider where it ideally belongs, in order to avoid having to propagate the boolean via the cert provider factory methods and having to add the boolean to the cache key as well.


protected DynamicSslContextProvider(
BaseTlsContext tlsContext, CertificateValidationContext staticCertValidationContext) {
Expand All @@ -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<SslContextBuilder, X509TrustManager>
getSslContextBuilderAndTrustManager(
Expand Down
20 changes: 15 additions & 5 deletions xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
}
}

Expand Down Expand Up @@ -260,8 +270,8 @@ public void updateSslContextAndExtendedX509TrustManager(
public void onException(Throwable throwable) {
ctx.fireExceptionCaught(throwable);
}
}
);
},
autoSniSanValidationDoesNotApply);
}

@Override
Expand Down Expand Up @@ -399,8 +409,8 @@ public void updateSslContextAndExtendedX509TrustManager(
public void onException(Throwable throwable) {
ctx.fireExceptionCaught(throwable);
}
}
);
},
false);
}
}
}
5 changes: 4 additions & 1 deletion xds/src/main/java/io/grpc/xds/internal/security/SslContextProviderSupplier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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...
Expand Down
6 changes: 4 additions & 2 deletions ...java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
2 changes: 2 additions & 0 deletions xds/src/main/java/io/grpc/xds/internal/security/trust/CertificateUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion xds/src/test/java/io/grpc/xds/XdsClientWrapperForServerSdsTestMisc.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
6 changes: 3 additions & 3 deletions xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions xds/src/test/java/io/grpc/xds/internal/security/SslContextProviderSupplierTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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));
}
Expand All @@ -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));
}
Expand Down Expand Up @@ -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));
}
Expand All @@ -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))
Expand Down