Skip to content

xds: Remove isXdsSniEnabled and align SNI logic with gRFC A101 #12625

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

Open
becomeStar wants to merge 1 commit into
base: master
Choose a base branch
from
+143 −186
Open

xds: Remove isXdsSniEnabled and align SNI logic with gRFC A101 #12625

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 10 additions & 12 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 @@ -215,23 +215,21 @@ public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
this.sslContextProviderSupplier = sslContextProviderSupplier;
EnvoyServerProtoData.BaseTlsContext tlsContext = sslContextProviderSupplier.getTlsContext();
UpstreamTlsContext upstreamTlsContext = ((UpstreamTlsContext) tlsContext);
if (CertificateUtils.isXdsSniEnabled) {
String sniToUse = upstreamTlsContext.getAutoHostSni()
&& !Strings.isNullOrEmpty(endpointHostname)
? endpointHostname : upstreamTlsContext.getSni();
if (sniToUse.isEmpty()) {
if (CertificateUtils.useChannelAuthorityIfNoSniApplicable) {
sniToUse = grpcHandler.getAuthority();
}
autoSniSanValidationDoesNotApply = true;

String sniToUse = upstreamTlsContext.getAutoHostSni()
&& !Strings.isNullOrEmpty(endpointHostname)
? endpointHostname : upstreamTlsContext.getSni();
if (sniToUse.isEmpty()) {
if (CertificateUtils.useChannelAuthorityIfNoSniApplicable) {
sniToUse = grpcHandler.getAuthority();
} else {
autoSniSanValidationDoesNotApply = false;
sniToUse = "";
}
sni = sniToUse;
autoSniSanValidationDoesNotApply = true;
} else {
sni = grpcHandler.getAuthority();
autoSniSanValidationDoesNotApply = false;
}
sni = sniToUse;
}

@VisibleForTesting
Expand Down
1 change: 0 additions & 1 deletion 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 @@ -30,7 +30,6 @@
* Contains certificate utility method(s).
*/
public final class CertificateUtils {
public static boolean isXdsSniEnabled = GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_SNI", true);
public static boolean useChannelAuthorityIfNoSniApplicable
= GrpcUtil.getFlag("GRPC_USE_CHANNEL_AUTHORITY_IF_NO_SNI_APPLICABLE", false);

Expand Down
2 changes: 1 addition & 1 deletion xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ public void checkServerTrusted(X509Certificate[] chain, String authType)

private List<StringMatcher> getAutoSniSanMatchers(SSLParameters sslParams) {
List<StringMatcher> sniNamesToMatch = new ArrayList<>();
if (CertificateUtils.isXdsSniEnabled && autoSniSanValidation) {
if (autoSniSanValidation) {
List<SNIServerName> serverNames = sslParams.getServerNames();
if (serverNames != null) {
for (SNIServerName serverName : serverNames) {
Expand Down
7 changes: 0 additions & 7 deletions xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@
import io.grpc.xds.internal.security.SslContextProviderSupplier;
import io.grpc.xds.internal.security.TlsContextManagerImpl;
import io.grpc.xds.internal.security.certprovider.FileWatcherCertificateProviderProvider;
import io.grpc.xds.internal.security.trust.CertificateUtils;
import io.netty.handler.ssl.NotSslRecordException;
import java.io.File;
import java.io.FileOutputStream;
Expand Down Expand Up @@ -317,7 +316,6 @@ public void tlsClientServer_noAutoSniValidation_failureToMatchSubjAltNames()
@Test
public void tlsClientServer_autoSniValidation_sniInUtc()
throws Exception {
CertificateUtils.isXdsSniEnabled = true;
Path trustStoreFilePath = getCacertFilePathForTestCa();
try {
setTrustStoreSystemProperties(trustStoreFilePath.toAbsolutePath().toString());
Expand All @@ -341,14 +339,12 @@ public void tlsClientServer_autoSniValidation_sniInUtc()
} finally {
Files.deleteIfExists(trustStoreFilePath);
clearTrustStoreSystemProperties();
CertificateUtils.isXdsSniEnabled = false;
}
}

@Test
public void tlsClientServer_autoSniValidation_sniFromHostname()
throws Exception {
CertificateUtils.isXdsSniEnabled = true;
Path trustStoreFilePath = getCacertFilePathForTestCa();
try {
setTrustStoreSystemProperties(trustStoreFilePath.toAbsolutePath().toString());
Expand All @@ -375,14 +371,12 @@ public void tlsClientServer_autoSniValidation_sniFromHostname()
} finally {
Files.deleteIfExists(trustStoreFilePath);
clearTrustStoreSystemProperties();
CertificateUtils.isXdsSniEnabled = false;
}
}

@Test
public void tlsClientServer_autoSniValidation_noSniApplicable_usesMatcherFromCmnVdnCtx()
throws Exception {
CertificateUtils.isXdsSniEnabled = true;
Path trustStoreFilePath = getCacertFilePathForTestCa();
try {
setTrustStoreSystemProperties(trustStoreFilePath.toAbsolutePath().toString());
Expand All @@ -406,7 +400,6 @@ public void tlsClientServer_autoSniValidation_noSniApplicable_usesMatcherFromCmn
} finally {
Files.deleteIfExists(trustStoreFilePath);
clearTrustStoreSystemProperties();
CertificateUtils.isXdsSniEnabled = false;
}
}

Expand Down
Loading