Save changes.

Author: kannanjgithub

netty/src/main/java/io/grpc/netty/InternalProtocolNegotiators.java

@@ -72,8 +72,8 @@ public void close() {
    * be negotiated, the {@code handler} is added and writes to the {@link io.netty.channel.Channel}
    * may happen immediately, even before the TLS Handshake is complete.
    */
-  public static InternalProtocolNegotiator.ProtocolNegotiator tls(SslContext sslContext) {
-    return tls(sslContext, null, Optional.absent(), null);
+  public static InternalProtocolNegotiator.ProtocolNegotiator tls(SslContext sslContext, String sni) {
+    return tls(sslContext, null, Optional.absent(), sni);
   }
 
   /**
@@ -171,7 +171,7 @@ public static ChannelHandler clientTlsHandler(
       ChannelHandler next, SslContext sslContext, String authority,
       ChannelLogger negotiationLogger) {
     return new ClientTlsHandler(next, sslContext, authority, null, negotiationLogger,
-        Optional.absent(), null);
+        Optional.absent(), null, null);
   }
 
   public static class ProtocolNegotiationHandler

netty/src/main/java/io/grpc/netty/ProtocolNegotiators.java

@@ -21,6 +21,7 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
 import com.google.errorprone.annotations.ForOverride;
 import io.grpc.Attributes;
 import io.grpc.CallCredentials;
@@ -89,6 +90,7 @@
 import javax.net.ssl.TrustManager;
 import javax.net.ssl.TrustManagerFactory;
 import javax.net.ssl.X509TrustManager;
+
 import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement;
 
 /**
@@ -609,8 +611,8 @@ public ChannelHandler newHandler(GrpcHttp2ConnectionHandler grpcHandler) {
       ChannelHandler gnh = new GrpcNegotiationHandler(grpcHandler);
       ChannelLogger negotiationLogger = grpcHandler.getNegotiationLogger();
       ChannelHandler cth = new ClientTlsHandler(gnh, sslContext,
-          sni != null? sni : grpcHandler.getAuthority(),
-          this.executor, negotiationLogger, handshakeCompleteRunnable, x509ExtendedTrustManager);
+          !Strings.isNullOrEmpty(sni)? sni : grpcHandler.getAuthority(),
+          this.executor, negotiationLogger, handshakeCompleteRunnable, null, x509ExtendedTrustManager);
       return new WaitUntilActiveHandler(cth, negotiationLogger);
     }
 
@@ -637,13 +639,13 @@ static final class ClientTlsHandler extends ProtocolNegotiationHandler {
     private final X509TrustManager x509ExtendedTrustManager;
     private SSLEngine sslEngine;
 
-    ClientTlsHandler(ChannelHandler next, SslContext sslContext, String sni,
+    ClientTlsHandler(ChannelHandler next, SslContext sslContext, String authority,
                      Executor executor, ChannelLogger negotiationLogger,
                      Optional<Runnable> handshakeCompleteRunnable,
-                     X509TrustManager x509ExtendedTrustManager) {
+                     ClientTlsProtocolNegotiator clientTlsProtocolNegotiator, X509TrustManager x509ExtendedTrustManager) {
       super(next, negotiationLogger);
       this.sslContext = Preconditions.checkNotNull(sslContext, "sslContext");
-      HostPort hostPort = parseAuthority(sni);
+      HostPort hostPort = parseAuthority(authority);
       this.host = hostPort.host;
       this.port = hostPort.port;
       this.executor = executor;

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

@@ -19,8 +19,10 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Strings;
 import io.grpc.Attributes;
 import io.grpc.EquivalentAddressGroup;
+import io.grpc.xds.EnvoyServerProtoData.UpstreamTlsContext;
 import io.grpc.Grpc;
 import io.grpc.internal.GrpcUtil;
 import io.grpc.internal.ObjectPool;
@@ -30,6 +32,7 @@
 import io.grpc.netty.InternalProtocolNegotiator.ProtocolNegotiator;
 import io.grpc.netty.InternalProtocolNegotiators;
 import io.grpc.netty.ProtocolNegotiationEvent;
+import io.grpc.xds.EnvoyServerProtoData;
 import io.netty.channel.ChannelHandler;
 import io.netty.channel.ChannelHandlerAdapter;
 import io.netty.channel.ChannelHandlerContext;
@@ -192,12 +195,12 @@ static final class ClientSecurityHandler
       extends InternalProtocolNegotiators.ProtocolNegotiationHandler {
     private final GrpcHttp2ConnectionHandler grpcHandler;
     private final SslContextProviderSupplier sslContextProviderSupplier;
-    private final String hostname;
+    private final String sni;
 
     ClientSecurityHandler(
         GrpcHttp2ConnectionHandler grpcHandler,
         SslContextProviderSupplier sslContextProviderSupplier,
-        String hostname) {
+        String endpointHostname) {
       super(
           // superclass (InternalProtocolNegotiators.ProtocolNegotiationHandler) expects 'next'
           // handler but we don't have a next handler _yet_. So we "disable" superclass's behavior
@@ -211,7 +214,15 @@ public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
       checkNotNull(grpcHandler, "grpcHandler");
       this.grpcHandler = grpcHandler;
       this.sslContextProviderSupplier = sslContextProviderSupplier;
-      this.hostname = hostname;
+      EnvoyServerProtoData.BaseTlsContext tlsContext = sslContextProviderSupplier.getTlsContext();
+      UpstreamTlsContext upstreamTlsContext = ((UpstreamTlsContext) tlsContext);
+      sni = upstreamTlsContext.getAutoHostSni() && !Strings.isNullOrEmpty(endpointHostname)
+              ? endpointHostname : upstreamTlsContext.getSni();
+    }
+
+    @VisibleForTesting
+    String getSni() {
+      return sni;
     }
 
     @Override
@@ -220,10 +231,10 @@ protected void handlerAdded0(final ChannelHandlerContext ctx) {
       ctx.pipeline().addBefore(ctx.name(), null, bufferReads);
 
       sslContextProviderSupplier.updateSslContext(
-          new SslContextProvider.Callback(ctx.executor(), hostname) {
+          new SslContextProvider.Callback(ctx.executor()) {
 
             @Override
-            public void updateSslContext(SslContext sslContext, String sni) {
+            public void updateSslContext(SslContext sslContext) {
               if (ctx.isRemoved()) {
                 return;
               }
@@ -232,7 +243,7 @@ public void updateSslContext(SslContext sslContext, String sni) {
                   "ClientSecurityHandler.updateSslContext authority={0}, ctx.name={1}",
                   new Object[]{grpcHandler.getAuthority(), ctx.name()});
               ChannelHandler handler =
-                  InternalProtocolNegotiators.tls(sslContext).newHandler(grpcHandler);
+                  InternalProtocolNegotiators.tls(sslContext, sni).newHandler(grpcHandler);
 
               // Delegate rest of handshake to TLS handler
               ctx.pipeline().addAfter(ctx.name(), null, handler);
@@ -244,8 +255,8 @@ public void updateSslContext(SslContext sslContext, String sni) {
             public void onException(Throwable throwable) {
               ctx.fireExceptionCaught(throwable);
             }
-          }
-      );
+          },
+          sni);
     }
 
     @Override
@@ -366,7 +377,7 @@ protected void handlerAdded0(final ChannelHandlerContext ctx) {
           new SslContextProvider.Callback(ctx.executor()) {
 
             @Override
-            public void updateSslContext(SslContext sslContext, String sni) {
+            public void updateSslContext(SslContext sslContext) {
               ChannelHandler handler =
                   InternalProtocolNegotiators.serverTls(sslContext).newHandler(grpcHandler);
 
@@ -382,8 +393,8 @@ public void updateSslContext(SslContext sslContext, String sni) {
             public void onException(Throwable throwable) {
               ctx.fireExceptionCaught(throwable);
             }
-          }
-      );
+          },
+              null);
     }
   }
 }

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

@@ -69,7 +69,7 @@ protected String getHostname() {
     }
 
     /** Informs callee of new/updated SslContext. */
-    @VisibleForTesting public abstract void updateSslContext(SslContext sslContext, String sni);
+    @VisibleForTesting public abstract void updateSslContext(SslContext sslContext);
 
     /** Informs callee of an exception that was generated. */
     @VisibleForTesting protected abstract void onException(Throwable throwable);
@@ -132,7 +132,7 @@ protected final void performCallback(
           public void run() {
             try {
               SslContext sslContext = sslContextGetter.get();
-              callback.updateSslContext(sslContext, callback.getHostname());
+              callback.updateSslContext(sslContext);
             } catch (Throwable e) {
               callback.onException(e);
             }

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

@@ -20,17 +20,16 @@
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.MoreObjects;
-import io.grpc.netty.GrpcSslContexts;
+import com.google.common.base.Strings;
 import io.grpc.xds.EnvoyServerProtoData.BaseTlsContext;
 import io.grpc.xds.EnvoyServerProtoData.DownstreamTlsContext;
 import io.grpc.xds.EnvoyServerProtoData.UpstreamTlsContext;
 import io.grpc.xds.TlsContextManager;
-import io.grpc.xds.internal.security.certprovider.CertProviderClientSslContextProvider;
 import io.netty.handler.ssl.SslContext;
+
 import java.util.HashSet;
 import java.util.Objects;
 import java.util.Set;
-import javax.net.ssl.SSLException;
 
 /**
  * Enables Client or server side to initialize this object with the received {@link BaseTlsContext}
@@ -58,29 +57,17 @@ public BaseTlsContext getTlsContext() {
   }
 
   /** Updates SslContext via the passed callback. */
-  public synchronized void updateSslContext(final SslContextProvider.Callback callback) {
+  public synchronized void updateSslContext(final SslContextProvider.Callback callback, String sni) {
     checkNotNull(callback, "callback");
     try {
-      String sni;
-      if (tlsContext instanceof UpstreamTlsContext) {
-        UpstreamTlsContext upstreamTlsContext = ((UpstreamTlsContext) tlsContext);
-        sni = upstreamTlsContext.getAutoHostSni() ? callback.getHostname() : upstreamTlsContext.getSni();
-      } else {
-        sni = null;
-      }
-      if (!shutdown) {
-        if (sslContextProvider == null) {
-          sslContextProvider = getSslContextProvider(sni);
-        }
-      }
       // we want to increment the ref-count so call findOrCreate again...
       final SslContextProvider toRelease = getSslContextProvider(sni);
       toRelease.addCallback(
           new SslContextProvider.Callback(callback.getExecutor()) {
 
             @Override
-            public void updateSslContext(SslContext sslContext, String sni) {
-              callback.updateSslContext(sslContext, sni);
+            public void updateSslContext(SslContext sslContext) {
+              callback.updateSslContext(sslContext);
               releaseSslContextProvider(toRelease, sni);
             }
 

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

@@ -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, null);
   }
 
   private void sendListenerUpdate(

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

@@ -394,7 +394,7 @@ public TestCallback(Executor executor) {
     }
 
     @Override
-    public void updateSslContext(SslContext sslContext, String sni) {
+    public void updateSslContext(SslContext sslContext) {
       updatedSslContext = sslContext;
     }