Merge with changes to not special case system root certs in SslContextProviderSupplier itself but have it be handled only in the ClientCertificateSslContextProviderSupplier.

Author: kannanjgithub

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

@@ -16,11 +16,8 @@
 
 package io.grpc.xds.internal.security;
 
-import static com.google.common.base.Preconditions.checkNotNull;
-
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.MoreObjects;
-import io.grpc.netty.GrpcSslContexts;
 import io.grpc.xds.EnvoyServerProtoData.BaseTlsContext;
 import io.grpc.xds.EnvoyServerProtoData.DownstreamTlsContext;
 import io.grpc.xds.EnvoyServerProtoData.UpstreamTlsContext;
@@ -29,9 +26,10 @@
 
 import java.util.HashSet;
 import java.util.Objects;
-import javax.net.ssl.SSLException;
 import java.util.Set;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 /**
  * Enables Client or server side to initialize this object with the received {@link BaseTlsContext}
  * and communicate it to the consumer i.e. {@link SecurityProtocolNegotiators}
@@ -67,39 +65,22 @@ public synchronized void updateSslContext(final SslContextProvider.Callback call
         }
       }
       // we want to increment the ref-count so call findOrCreate again...
-      final SslContextProvider toRelease = getSslContextProvider();
+      final SslContextProvider toRelease = getSslContextProvider(sni);
       toRelease.addCallback(
           new SslContextProvider.Callback(callback.getExecutor()) {
-      final SslContextProvider toRelease = getSslContextProvider(sni);
-      // When using system root certs on client side, SslContext updates via CertificateProvider is
-      // only required if Mtls is also enabled, i.e. tlsContext has a cert provider instance.
-      if (tlsContext instanceof UpstreamTlsContext
-          && !CommonTlsContextUtil.hasCertProviderInstance(tlsContext.getCommonTlsContext())
-          && CommonTlsContextUtil.isUsingSystemRootCerts(tlsContext.getCommonTlsContext())) {
-        callback.getExecutor().execute(() -> {
-          try {
-            callback.updateSslContext(GrpcSslContexts.forClient().build());
+
+          @Override
+          public void updateSslContext(SslContext sslContext) {
+            callback.updateSslContext(sslContext);
+            releaseSslContextProvider(toRelease, sni);
+          }
+
+          @Override
+          public void onException(Throwable throwable) {
+            callback.onException(throwable);
             releaseSslContextProvider(toRelease, sni);
-          } catch (SSLException e) {
-            callback.onException(e);
           }
         });
-      } else {
-        toRelease.addCallback(
-            new SslContextProvider.Callback(callback.getExecutor()) {
-
-            @Override
-            public void updateSslContext(SslContext sslContext) {
-              callback.updateSslContext(sslContext);
-              releaseSslContextProvider(toRelease, sni);
-            }
-
-            @Override
-            public void onException(Throwable throwable) {
-              callback.onException(throwable);
-              releaseSslContextProvider(toRelease, sni);
-            }
-          });
     } catch (final Throwable throwable) {
       callback.getExecutor().execute(new Runnable() {
         @Override

xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProvider.java

@@ -33,6 +33,8 @@
 /** A client SslContext provider using CertificateProviderInstance to fetch secrets. */
 final class CertProviderClientSslContextProvider extends CertProviderSslContextProvider {
 
+  private final String sniForSanMatching;
+
   CertProviderClientSslContextProvider(
           Node node,
           @Nullable Map<String, CertificateProviderInfo> certProviders,

xds/src/test/java/io/grpc/xds/internal/security/SslContextProviderSupplierTest.java

@@ -178,31 +178,6 @@ public void systemRootCertsWithMtls_callbackExecutedFromProvider() {
         .findOrCreateClientSslContextProvider(eq(upstreamTlsContext), eq(SNI));
   }
 
-  @Test
-  public void systemRootCertsWithRegularTls_callbackExecutedFromSupplier() {
-    upstreamTlsContext =
-        CommonTlsContextTestsUtil.buildNewUpstreamTlsContextForCertProviderInstance(
-            null,
-            null,
-            null,
-            "root-default",
-            null,
-            CertificateValidationContext.newBuilder()
-                .setSystemRootCerts(
-                        CertificateValidationContext.SystemRootCerts.getDefaultInstance())
-                .build());
-    supplier = new SslContextProviderSupplier(upstreamTlsContext, mockTlsContextManager);
-    reset(mockTlsContextManager);
-
-    callUpdateSslContext();
-    ArgumentCaptor<Runnable> runnableArgumentCaptor = ArgumentCaptor.forClass(Runnable.class);
-    verify(mockExecutor).execute(runnableArgumentCaptor.capture());
-    runnableArgumentCaptor.getValue().run();
-    verify(mockCallback, times(1)).updateSslContext(any(SslContext.class));
-    verify(mockTlsContextManager, times(1))
-            .releaseClientSslContextProvider(eq(mockSslContextProvider), eq(SNI));
-  }
-
   @Test
   public void testClose() {
     prepareSupplier(true);