Trust manager not needed on server side when invoking SslProvider.Callback.

Author: kannanjgithub

xds/src/main/java/io/grpc/xds/ClusterImplLoadBalancer.java

@@ -377,7 +377,8 @@ private class RequestLimitingSubchannelPicker extends SubchannelPicker {
       private final Map<String, Struct> filterMetadata;
 
       private RequestLimitingSubchannelPicker(SubchannelPicker delegate,
-                                              List<DropOverload> dropPolicies, long maxConcurrentRequests,
+                                              List<DropOverload> dropPolicies,
+                                              long maxConcurrentRequests,
                                               Map<String, Struct> filterMetadata) {
         this.delegate = delegate;
         this.dropPolicies = dropPolicies;

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

@@ -99,7 +99,8 @@ protected final void updateSslContext() {
   }
 
   protected final void callPerformCallback(
-          Callback callback, final AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContextAndTmCopy) {
+          Callback callback,
+          final AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContextAndTmCopy) {
     performCallback(
         new SslContextGetter() {
           @Override

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

@@ -72,7 +72,8 @@ public synchronized void updateSslContext(final SslContextProvider.Callback call
           new SslContextProvider.Callback(callback.getExecutor()) {
 
           @Override
-          public void updateSslContextAndExtendedX509TrustManager(AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContextAndTm) {
+          public void updateSslContextAndExtendedX509TrustManager(
+              AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContextAndTm) {
             callback.updateSslContextAndExtendedX509TrustManager(sslContextAndTm);
             releaseSslContextProvider(toRelease, sni);
           }

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

@@ -75,9 +75,8 @@ protected final AbstractMap.SimpleImmutableEntry<SslContextBuilder, TrustManager
     }
     setClientAuthValues(sslContextBuilder, trustManagerFactory);
     sslContextBuilder = GrpcSslContexts.configure(sslContextBuilder);
-    return new AbstractMap.SimpleImmutableEntry(sslContextBuilder,
-        CertificateUtils.getX509ExtendedTrustManager(
-            Arrays.asList(trustManagerFactory.getTrustManagers())));
+    // TrustManager in the below return value is not used on the server side, so setting it to null
+    return new AbstractMap.SimpleImmutableEntry(sslContextBuilder, null);
   }
 
 }

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

@@ -361,14 +361,15 @@ private static CommonTlsContext.Builder addNewCertificateValidationContext(
   }
 
   /** Perform some simple checks on sslContext. */
-  public static void doChecksOnSslContext(boolean server, AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContext,
-                                          List<String> expectedApnProtos) {
+  public static void doChecksOnSslContext(boolean server,
+      AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContextAndTm,
+      List<String> expectedApnProtos) {
     if (server) {
-      assertThat(sslContext.getKey().isServer()).isTrue();
+      assertThat(sslContextAndTm.getKey().isServer()).isTrue();
     } else {
-      assertThat(sslContext.getKey().isClient()).isTrue();
+      assertThat(sslContextAndTm.getKey().isClient()).isTrue();
     }
-    List<String> apnProtos = sslContext.getKey().applicationProtocolNegotiator().protocols();
+    List<String> apnProtos = sslContextAndTm.getKey().applicationProtocolNegotiator().protocols();
     assertThat(apnProtos).isNotNull();
     if (expectedApnProtos != null) {
       assertThat(apnProtos).isEqualTo(expectedApnProtos);

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

@@ -203,8 +203,9 @@ public void clientSecurityHandler_addLast()
     sslContextProviderSupplier
         .updateSslContext(new SslContextProvider.Callback(MoreExecutors.directExecutor()) {
           @Override
-          public void updateSslContextAndExtendedX509TrustManager(AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContext) {
-            future.set(sslContext);
+          public void updateSslContextAndExtendedX509TrustManager(
+              AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContextAndTm) {
+            future.set(sslContextAndTm);
           }
 
           @Override
@@ -215,7 +216,7 @@ protected void onException(Throwable throwable) {
     assertThat(executor.runDueTasks()).isEqualTo(1);
     channel.runPendingTasks();
     Object fromFuture = future.get(2, TimeUnit.SECONDS);
-    assertThat(fromFuture).isInstanceOf(SslContext.class);
+    assertThat(fromFuture).isInstanceOf(AbstractMap.SimpleImmutableEntry.class);
     channel.runPendingTasks();
     channelHandlerCtx = pipeline.context(clientSecurityHandler);
     assertThat(channelHandlerCtx).isNull();
@@ -391,8 +392,9 @@ public SocketAddress remoteAddress() {
     sslContextProviderSupplier
         .updateSslContext(new SslContextProvider.Callback(MoreExecutors.directExecutor()) {
           @Override
-          public void updateSslContextAndExtendedX509TrustManager(AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContext) {
-            future.set(sslContext);
+          public void updateSslContextAndExtendedX509TrustManager(
+              AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContextAndTm) {
+            future.set(sslContextAndTm);
           }
 
           @Override
@@ -403,7 +405,7 @@ protected void onException(Throwable throwable) {
     channel.runPendingTasks(); // need this for tasks to execute on eventLoop
     assertThat(executor.runDueTasks()).isEqualTo(1);
     Object fromFuture = future.get(2, TimeUnit.SECONDS);
-    assertThat(fromFuture).isInstanceOf(SslContext.class);
+    assertThat(fromFuture).isInstanceOf(AbstractMap.SimpleImmutableEntry.class);
     channel.runPendingTasks();
     channelHandlerCtx = pipeline.context(SecurityProtocolNegotiators.ServerSecurityHandler.class);
     assertThat(channelHandlerCtx).isNull();
@@ -529,8 +531,9 @@ public void clientSecurityProtocolNegotiatorNewHandler_fireProtocolNegotiationEv
       sslContextProviderSupplier
           .updateSslContext(new SslContextProvider.Callback(MoreExecutors.directExecutor()) {
             @Override
-            public void updateSslContextAndExtendedX509TrustManager(AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContext) {
-              future.set(sslContext);
+            public void updateSslContextAndExtendedX509TrustManager(
+                AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContextAndTm) {
+              future.set(sslContextAndTm);
             }
 
             @Override
@@ -541,7 +544,7 @@ protected void onException(Throwable throwable) {
       executor.runDueTasks();
       channel.runPendingTasks(); // need this for tasks to execute on eventLoop
       Object fromFuture = future.get(5, TimeUnit.SECONDS);
-      assertThat(fromFuture).isInstanceOf(SslContext.class);
+      assertThat(fromFuture).isInstanceOf(AbstractMap.SimpleImmutableEntry.class);
       channel.runPendingTasks();
       channelHandlerCtx = pipeline.context(clientSecurityHandler);
       assertThat(channelHandlerCtx).isNull();