Skip to content

Commit ac44e96

Browse files
authored
xds: Remove isXdsSniEnabled and align SNI logic with gRFC A101 (#12625)
## Description Remove the `isXdsSniEnabled` (GRPC_EXPERIMENTAL_XDS_SNI) guard so that SNI determination via xDS is always enabled. This aligns the behavior with **gRFC A101**, where SNI is determined by xDS configurations such as `auto_host_sni` or `UpstreamTlsContext.sni`, without relying on an experimental toggle. This change does **not** remove the `GRPC_USE_CHANNEL_AUTHORITY_IF_NO_SNI_APPLICABLE` fallback logic, which remains unchanged. ## Changes - Remove the `isXdsSniEnabled` flag and the related conditional logic. - Remove test cases that specifically covered behavior when the experimental flag was disabled, since the flag is no longer supported. Ref #11784
1 parent 4f2b110 commit ac44e96

File tree

5 files changed

+128
-190
lines changed

5 files changed

+128
-190
lines changed

xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -215,23 +215,19 @@ public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
215215
this.sslContextProviderSupplier = sslContextProviderSupplier;
216216
EnvoyServerProtoData.BaseTlsContext tlsContext = sslContextProviderSupplier.getTlsContext();
217217
UpstreamTlsContext upstreamTlsContext = ((UpstreamTlsContext) tlsContext);
218-
if (CertificateUtils.isXdsSniEnabled) {
219-
String sniToUse = upstreamTlsContext.getAutoHostSni()
220-
&& !Strings.isNullOrEmpty(endpointHostname)
221-
? endpointHostname : upstreamTlsContext.getSni();
222-
if (sniToUse.isEmpty()) {
223-
if (CertificateUtils.useChannelAuthorityIfNoSniApplicable) {
224-
sniToUse = grpcHandler.getAuthority();
225-
}
226-
autoSniSanValidationDoesNotApply = true;
227-
} else {
228-
autoSniSanValidationDoesNotApply = false;
218+
219+
String sniToUse = upstreamTlsContext.getAutoHostSni()
220+
&& !Strings.isNullOrEmpty(endpointHostname)
221+
? endpointHostname : upstreamTlsContext.getSni();
222+
if (sniToUse.isEmpty()) {
223+
if (CertificateUtils.useChannelAuthorityIfNoSniApplicable) {
224+
sniToUse = grpcHandler.getAuthority();
229225
}
230-
sni = sniToUse;
226+
autoSniSanValidationDoesNotApply = true;
231227
} else {
232-
sni = grpcHandler.getAuthority();
233228
autoSniSanValidationDoesNotApply = false;
234229
}
230+
sni = sniToUse;
235231
}
236232

237233
@VisibleForTesting

xds/src/main/java/io/grpc/xds/internal/security/trust/CertificateUtils.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
* Contains certificate utility method(s).
3131
*/
3232
public final class CertificateUtils {
33-
public static boolean isXdsSniEnabled = GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_SNI", true);
3433
public static boolean useChannelAuthorityIfNoSniApplicable
3534
= GrpcUtil.getFlag("GRPC_USE_CHANNEL_AUTHORITY_IF_NO_SNI_APPLICABLE", false);
3635

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ public void checkServerTrusted(X509Certificate[] chain, String authType)
308308

309309
private List<StringMatcher> getAutoSniSanMatchers(SSLParameters sslParams) {
310310
List<StringMatcher> sniNamesToMatch = new ArrayList<>();
311-
if (CertificateUtils.isXdsSniEnabled && autoSniSanValidation) {
311+
if (autoSniSanValidation) {
312312
List<SNIServerName> serverNames = sslParams.getServerNames();
313313
if (serverNames != null) {
314314
for (SNIServerName serverName : serverNames) {

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@
7777
import io.grpc.xds.internal.security.SslContextProviderSupplier;
7878
import io.grpc.xds.internal.security.TlsContextManagerImpl;
7979
import io.grpc.xds.internal.security.certprovider.FileWatcherCertificateProviderProvider;
80-
import io.grpc.xds.internal.security.trust.CertificateUtils;
8180
import io.netty.handler.ssl.NotSslRecordException;
8281
import java.io.File;
8382
import java.io.FileOutputStream;
@@ -317,7 +316,6 @@ public void tlsClientServer_noAutoSniValidation_failureToMatchSubjAltNames()
317316
@Test
318317
public void tlsClientServer_autoSniValidation_sniInUtc()
319318
throws Exception {
320-
CertificateUtils.isXdsSniEnabled = true;
321319
Path trustStoreFilePath = getCacertFilePathForTestCa();
322320
try {
323321
setTrustStoreSystemProperties(trustStoreFilePath.toAbsolutePath().toString());
@@ -341,14 +339,12 @@ public void tlsClientServer_autoSniValidation_sniInUtc()
341339
} finally {
342340
Files.deleteIfExists(trustStoreFilePath);
343341
clearTrustStoreSystemProperties();
344-
CertificateUtils.isXdsSniEnabled = false;
345342
}
346343
}
347344

348345
@Test
349346
public void tlsClientServer_autoSniValidation_sniFromHostname()
350347
throws Exception {
351-
CertificateUtils.isXdsSniEnabled = true;
352348
Path trustStoreFilePath = getCacertFilePathForTestCa();
353349
try {
354350
setTrustStoreSystemProperties(trustStoreFilePath.toAbsolutePath().toString());
@@ -375,14 +371,12 @@ public void tlsClientServer_autoSniValidation_sniFromHostname()
375371
} finally {
376372
Files.deleteIfExists(trustStoreFilePath);
377373
clearTrustStoreSystemProperties();
378-
CertificateUtils.isXdsSniEnabled = false;
379374
}
380375
}
381376

382377
@Test
383378
public void tlsClientServer_autoSniValidation_noSniApplicable_usesMatcherFromCmnVdnCtx()
384379
throws Exception {
385-
CertificateUtils.isXdsSniEnabled = true;
386380
Path trustStoreFilePath = getCacertFilePathForTestCa();
387381
try {
388382
setTrustStoreSystemProperties(trustStoreFilePath.toAbsolutePath().toString());
@@ -406,7 +400,6 @@ public void tlsClientServer_autoSniValidation_noSniApplicable_usesMatcherFromCmn
406400
} finally {
407401
Files.deleteIfExists(trustStoreFilePath);
408402
clearTrustStoreSystemProperties();
409-
CertificateUtils.isXdsSniEnabled = false;
410403
}
411404
}
412405

0 commit comments

Comments
 (0)