Add SslConfig APIs and fix SNI and SSL hostname bugs (#1387)

Motivation:
The server SSL config APIs for SNI didn't allow configuring SSL
associated with each individual SNI host. The client APIs coupled the
host name verification algorithm with the non-authoritative peer
host/port. This doesn't correctly represent the semantics of SSL.

Modifications:
- Deprecate all APIs related to SecurityConfigurator and secure()
  builder method. The secure() builder methods require that users call
  commit() which is easy to miss if the builder is used in a conditional
  block, and prototypes of a composable server SNI API results in code
  that is difficult to read and understand which configurations apply.
- Add SslConfig interface types and builders that can be used to create
  the ssl configuration independent of the protocol builders. This
  allows for folks to configure ssl independently and we can overlay any
  values which maybe automatically inferred in the protocol builder
  (e.g. peerHost on the client, alpn protocols, etc.).
- Server SSL allows for providing SNI configuration, and Netty's
  DomainWildcardMappingBuilder is used to match hostnames.
- Client side SSL config object removes methods which couple the host
  name verification algorithm to the non-authoritative peer host/port,
  and adds new methods which allow setting them independently.
- Update all tests to enable host name verification, and avoid
  deprecated method use when possible.

Result:
SSL config APIs now correctly represent SNI semantics for client and
server. The client SSL config decouples the non-authoritative peer
host/port from the verification algorithm.

The deprecated types and methods will be removed in a future release.

servicetalk-examples/http/http2/src/main/java/io/servicetalk/examples/http/http2/alpn/HttpClientWithAlpn.java

@@ -19,6 +19,7 @@
 import io.servicetalk.http.api.HttpResponse;
 import io.servicetalk.http.netty.HttpClients;
 import io.servicetalk.test.resources.DefaultTestCerts;
+import io.servicetalk.transport.api.ClientSslConfigBuilder;
 
 import static io.servicetalk.http.api.HttpSerializationProviders.textDeserializer;
 import static io.servicetalk.http.netty.HttpProtocolConfigs.h1Default;
@@ -36,11 +37,7 @@ public static void main(String[] args) throws Exception {
         // streaming API see helloworld examples.
         try (BlockingHttpClient client = HttpClients.forSingleAddress("localhost", 8080)
                 .protocols(h2Default(), h1Default()) // Configure support for HTTP/2 and HTTP/1.1 protocols
-                .secure()   // Start TLS configuration
-                .disableHostnameVerification()  // Our self-signed certificates do not support hostname verification,
-                // but this MUST NOT be disabled in production because it may leave you vulnerable to MITM attacks
-                .trustManager(DefaultTestCerts::loadServerCAPem)    // Custom trust manager for test certificates
-                .commit()   // Finish TLS configuration
+                .sslConfig(new ClientSslConfigBuilder(DefaultTestCerts::loadServerCAPem).build())
                 .buildBlocking()) {
             HttpResponse response = client.request(client.get("/"));
             System.out.println(response.toString((name, value) -> value));

servicetalk-examples/http/http2/src/main/java/io/servicetalk/examples/http/http2/alpn/HttpServerWithAlpn.java

@@ -17,6 +17,7 @@
 
 import io.servicetalk.http.netty.HttpServers;
 import io.servicetalk.test.resources.DefaultTestCerts;
+import io.servicetalk.transport.api.ServerSslConfigBuilder;
 
 import static io.servicetalk.http.api.HttpSerializationProviders.textSerializer;
 import static io.servicetalk.http.netty.HttpProtocolConfigs.h1Default;
@@ -32,8 +33,8 @@ public final class HttpServerWithAlpn {
     public static void main(String[] args) throws Exception {
         HttpServers.forPort(8080)
                 .protocols(h2Default(), h1Default()) // Configure support for HTTP/2 and HTTP/1.1 protocols
-                // Configure TLS certificates:
-                .secure().commit(DefaultTestCerts::loadServerPem, DefaultTestCerts::loadServerKey)
+                .sslConfig(new ServerSslConfigBuilder(DefaultTestCerts::loadServerPem, DefaultTestCerts::loadServerKey)
+                        .build())
                 // Note: this example demonstrates only blocking-aggregated programming paradigm, for asynchronous and
                 // streaming API see helloworld examples.
                 .listenBlockingAndAwait((ctx, request, responseFactory) ->

servicetalk-examples/http/mutual-tls/src/main/java/io/servicetalk/examples/http/mutualtls/HttpClientMutualTLS.java

@@ -19,6 +19,7 @@
 import io.servicetalk.http.api.HttpResponse;
 import io.servicetalk.http.netty.HttpClients;
 import io.servicetalk.test.resources.DefaultTestCerts;
+import io.servicetalk.transport.api.ClientSslConfigBuilder;
 
 import static io.servicetalk.http.api.HttpSerializationProviders.textDeserializer;
 
@@ -31,15 +32,9 @@ public static void main(String[] args) throws Exception {
         // Note: this example demonstrates only blocking-aggregated programming paradigm, for asynchronous and
         // streaming API see helloworld examples.
         try (BlockingHttpClient client = HttpClients.forSingleAddress("localhost", 8080)
-                .secure()   // Start TLS configuration
-                // Our self-signed certificates do not support hostname verification, but this MUST NOT be disabled in
-                // production because it may leave you vulnerable to MITM attacks.
-                .disableHostnameVerification()
-                // The client only trusts the CA which signed the example server's certificate.
-                .trustManager(DefaultTestCerts::loadServerCAPem)
-                // Specify the client's certificate/key pair to use to authenticate to the server.
-                .keyManager(DefaultTestCerts::loadClientPem, DefaultTestCerts::loadClientKey)
-                .commit()   // Finish TLS configuration
+                .sslConfig(new ClientSslConfigBuilder(DefaultTestCerts::loadServerCAPem)
+                        // Specify the client's certificate/key pair to use to authenticate to the server.
+                        .keyManager(DefaultTestCerts::loadClientPem, DefaultTestCerts::loadClientKey).build())
                 .buildBlocking()) {
             HttpResponse response = client.request(client.get("/"));
             System.out.println(response.toString((name, value) -> value));

servicetalk-examples/http/mutual-tls/src/main/java/io/servicetalk/examples/http/mutualtls/HttpServerMutualTLS.java

@@ -17,9 +17,10 @@
 
 import io.servicetalk.http.netty.HttpServers;
 import io.servicetalk.test.resources.DefaultTestCerts;
+import io.servicetalk.transport.api.ServerSslConfigBuilder;
+import io.servicetalk.transport.api.SslClientAuthMode;
 
 import static io.servicetalk.http.api.HttpSerializationProviders.textSerializer;
-import static io.servicetalk.transport.api.ServerSecurityConfigurator.ClientAuth.REQUIRE;
 
 /**
  * A server that does mutual TLS.
@@ -28,14 +29,11 @@ public final class HttpServerMutualTLS {
 
     public static void main(String[] args) throws Exception {
         HttpServers.forPort(8080)
-                .secure()
-                // Require clients to authenticate them selves, otherwise a handshake may succeed without authenticating
-                // the client.
-                .clientAuth(REQUIRE)
-                // The server only trusts the CA which signed the example clients's certificate.
-                .trustManager(DefaultTestCerts::loadClientCAPem)
-                // Specify the server's certificate/key pair to use to authenticate to the server.
-                .commit(DefaultTestCerts::loadServerPem, DefaultTestCerts::loadServerKey)
+                .sslConfig(new ServerSslConfigBuilder(DefaultTestCerts::loadServerPem, DefaultTestCerts::loadServerKey)
+                        // Require clients to authenticate, otherwise a handshake may succeed without authentication.
+                        .clientAuthMode(SslClientAuthMode.REQUIRE)
+                        // The server only trusts the CA which signed the example client's certificate.
+                        .trustManager(DefaultTestCerts::loadClientCAPem).build())
                 // Note: this example demonstrates only blocking-aggregated programming paradigm, for asynchronous and
                 // streaming API see helloworld examples.
                 .listenBlockingAndAwait((ctx, request, responseFactory) ->

servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcClientBuilder.java

@@ -32,6 +32,7 @@
 import io.servicetalk.http.api.StreamingHttpRequester;
 import io.servicetalk.http.api.StreamingHttpResponse;
 import io.servicetalk.logging.api.LogLevel;
+import io.servicetalk.transport.api.ClientSslConfig;
 import io.servicetalk.transport.api.IoExecutor;
 
 import java.net.SocketOption;
@@ -86,9 +87,13 @@ public abstract GrpcClientBuilder<U, R> appendConnectionFactoryFilter(
     public abstract GrpcClientBuilder<U, R> appendConnectionFilter(Predicate<StreamingHttpRequest> predicate,
                                                                    StreamingHttpConnectionFilterFactory factory);
 
+    @Deprecated
     @Override
     public abstract GrpcClientSecurityConfigurator<U, R> secure();
 
+    @Override
+    public abstract GrpcClientBuilder<U, R> sslConfig(ClientSslConfig sslConfig);
+
     @Override
     public abstract GrpcClientBuilder<U, R> autoRetryStrategy(
             AutoRetryStrategyProvider autoRetryStrategyProvider);

servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcClientSecurityConfigurator.java

@@ -16,6 +16,7 @@
 package io.servicetalk.grpc.api;
 
 import io.servicetalk.transport.api.ClientSecurityConfigurator;
+import io.servicetalk.transport.api.ClientSslConfig;
 
 import java.io.InputStream;
 import java.util.function.Supplier;
@@ -24,9 +25,11 @@
 
 /**
  * A {@link ClientSecurityConfigurator} for {@link SingleAddressGrpcClientBuilder}.
+ * @deprecated Use {@link GrpcClientBuilder#sslConfig(ClientSslConfig)}.
  * @param <U> the type of address before resolution (unresolved address)
  * @param <R> the type of address after resolution (resolved address)
  */
+@Deprecated
 public interface GrpcClientSecurityConfigurator<U, R> extends ClientSecurityConfigurator {
     /**
      * Commit configuring client security.

servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcServerBuilder.java

@@ -32,12 +32,14 @@
 import io.servicetalk.transport.api.ConnectionAcceptorFactory;
 import io.servicetalk.transport.api.IoExecutor;
 import io.servicetalk.transport.api.ServerContext;
+import io.servicetalk.transport.api.ServerSslConfig;
 import io.servicetalk.transport.api.ServiceTalkSocketOptions;
 import io.servicetalk.transport.api.TransportObserver;
 
 import java.net.SocketOption;
 import java.net.StandardSocketOptions;
 import java.util.Arrays;
+import java.util.Map;
 import java.util.function.BooleanSupplier;
 import java.util.function.Predicate;
 
@@ -54,8 +56,8 @@ public abstract class GrpcServerBuilder {
     /**
      * Configurations of various underlying protocol versions.
      * <p>
-     * <b>Note:</b> the order of specified protocols will reflect on priorities for ALPN in case the connections are
-     * {@link #secure() secured}.
+     * <b>Note:</b> the order of specified protocols will reflect on priorities for ALPN in case the connections use
+     * {@link #sslConfig(ServerSslConfig)}.
      *
      * @param protocols {@link HttpProtocolConfig} for each protocol that should be supported.
      * @return {@code this}.
@@ -74,31 +76,29 @@ public abstract class GrpcServerBuilder {
     /**
      * Initiate security configuration for this server. Calling any {@code commit} method on the returned
      * {@link GrpcServerSecurityConfigurator} will commit the configuration.
-     * <p>
-     * Additionally use {@link #secure(String...)} to define configurations for specific
-     * <a href="https://tools.ietf.org/html/rfc6066#section-3">SNI</a> hostnames. If such configuration is additionally
-     * defined then configuration using this method is used as default if the hostname does not match any of the
-     * specified hostnames.
-     *
+     * @deprecated Use {@link #sslConfig(ServerSslConfig)}.
      * @return {@link GrpcServerSecurityConfigurator} to configure security for this server. It is
      * mandatory to call any one of the {@code commit} methods after all configuration is done.
      */
+    @Deprecated
     public abstract GrpcServerSecurityConfigurator secure();
 
     /**
-     * Initiate security configuration for this server for the passed {@code sniHostnames}.
-     * Calling any {@code commit} method on the returned {@link GrpcServerSecurityConfigurator} will commit the
-     * configuration.
-     * <p>
-     * When using this method, it is mandatory to also define the default configuration using {@link #secure()} which
-     * is used when the hostname does not match any of the specified {@code sniHostnames}.
-     *
-     * @param sniHostnames <a href="https://tools.ietf.org/html/rfc6066#section-3">SNI</a> hostnames for which this
-     * config is being defined.
-     * @return {@link GrpcServerSecurityConfigurator} to configure security for this server. It is
-     * mandatory to call any one of the {@code commit} methods after all configuration is done.
+     * Set the SSL/TLS configuration.
+     * @param config The configuration to use.
+     * @return {@code this}.
+     */
+    public abstract GrpcServerBuilder sslConfig(ServerSslConfig config);
+
+    /**
+     * Set the SSL/TLS and <a href="https://tools.ietf.org/html/rfc6066#section-3">SNI</a> configuration.
+     * @param defaultConfig The configuration to use is the client certificate's SNI extension isn't present or the
+     * SNI hostname doesn't match any values in {@code sniMap}.
+     * @param sniMap A map where the keys are matched against the client certificate's SNI extension value in order
+     * to provide the corresponding {@link ServerSslConfig}.
+     * @return {@code this}.
      */
-    public abstract GrpcServerSecurityConfigurator secure(String... sniHostnames);
+    public abstract GrpcServerBuilder sslConfig(ServerSslConfig defaultConfig, Map<String, ServerSslConfig> sniMap);
 
     /**
      * Add a {@link SocketOption} that is applied.

servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcServerSecurityConfigurator.java

@@ -16,6 +16,7 @@
 package io.servicetalk.grpc.api;
 
 import io.servicetalk.transport.api.ServerSecurityConfigurator;
+import io.servicetalk.transport.api.ServerSslConfig;
 
 import java.io.InputStream;
 import java.util.function.Supplier;
@@ -24,7 +25,9 @@
 
 /**
  * A {@link ServerSecurityConfigurator} for {@link GrpcServerBuilder}.
+ * @deprecated Use {@link GrpcServerBuilder#sslConfig(ServerSslConfig)}.
  */
+@Deprecated
 public interface GrpcServerSecurityConfigurator extends ServerSecurityConfigurator {
     @Override
     GrpcServerSecurityConfigurator trustManager(Supplier<InputStream> trustCertChainSupplier);

servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/SingleAddressGrpcClientBuilder.java

@@ -31,6 +31,7 @@
 import io.servicetalk.http.api.StreamingHttpConnectionFilterFactory;
 import io.servicetalk.http.api.StreamingHttpRequest;
 import io.servicetalk.logging.api.LogLevel;
+import io.servicetalk.transport.api.ClientSslConfig;
 import io.servicetalk.transport.api.IoExecutor;
 import io.servicetalk.transport.api.TransportObserver;
 
@@ -54,10 +55,10 @@ interface SingleAddressGrpcClientBuilder<U, R,
     @Override
     <T> SingleAddressGrpcClientBuilder<U, R, SDE> socketOption(SocketOption<T> option, T value);
 
+    @Deprecated
     @Override
     SingleAddressGrpcClientBuilder<U, R, SDE> enableWireLogging(String loggerName);
 
-    @Deprecated
     @Override
     SingleAddressGrpcClientBuilder<U, R, SDE> enableWireLogging(String loggerName, LogLevel logLevel,
                                                                 BooleanSupplier logUserData);
@@ -98,13 +99,21 @@ SingleAddressGrpcClientBuilder<U, R, SDE> appendConnectionFilter(Predicate<Strea
      * Initiate security configuration for this client. Calling
      * {@link GrpcClientSecurityConfigurator#commit()} on the returned {@link GrpcClientSecurityConfigurator} will
      * commit the configuration.
-     *
+     * @deprecated Use {@link #sslConfig(ClientSslConfig)}.
      * @return {@link GrpcClientSecurityConfigurator} to configure security for this client. It is
      * mandatory to call {@link GrpcClientSecurityConfigurator#commit() commit} after all configuration is
      * done.
      */
+    @Deprecated
     GrpcClientSecurityConfigurator<U, R> secure();
 
+    /**
+     * Set the SSL/TLS configuration.
+     * @param sslConfig The configuration to use.
+     * @return {@code this}.
+     */
+    SingleAddressGrpcClientBuilder<U, R, SDE> sslConfig(ClientSslConfig sslConfig);
+
     /**
      * Updates the automatic retry strategy for the clients generated by this builder. Automatic retries are done by
      * the clients automatically when allowed by the passed {@link AutoRetryStrategyProvider}. These retries are not a

servicetalk-grpc-netty/src/main/java/io/servicetalk/grpc/netty/DefaultGrpcClientBuilder.java

@@ -33,6 +33,7 @@
 import io.servicetalk.http.api.StreamingHttpConnectionFilterFactory;
 import io.servicetalk.http.api.StreamingHttpRequest;
 import io.servicetalk.logging.api.LogLevel;
+import io.servicetalk.transport.api.ClientSslConfig;
 import io.servicetalk.transport.api.IoExecutor;
 
 import java.net.SocketOption;
@@ -74,6 +75,7 @@ public <T> GrpcClientBuilder<U, R> socketOption(final SocketOption<T> option, fi
         return this;
     }
 
+    @Deprecated
     @Override
     public GrpcClientBuilder<U, R> enableWireLogging(final String loggerName) {
         httpClientBuilder.enableWireLogging(loggerName);
@@ -114,12 +116,19 @@ public GrpcClientBuilder<U, R> appendConnectionFilter(
         return this;
     }
 
+    @Deprecated
     @Override
     public GrpcClientSecurityConfigurator<U, R> secure() {
         SingleAddressHttpClientSecurityConfigurator<U, R> httpConfigurator = httpClientBuilder.secure();
         return new DefaultGrpcClientSecurityConfigurator<>(httpConfigurator, this);
     }
 
+    @Override
+    public GrpcClientBuilder<U, R> sslConfig(final ClientSslConfig sslConfig) {
+        httpClientBuilder.sslConfig(sslConfig);
+        return this;
+    }
+
     @Override
     public GrpcClientBuilder<U, R> autoRetryStrategy(
             final AutoRetryStrategyProvider autoRetryStrategyProvider) {

servicetalk-grpc-netty/src/main/java/io/servicetalk/grpc/netty/DefaultGrpcClientSecurityConfigurator.java

@@ -24,6 +24,7 @@
 import javax.net.ssl.KeyManagerFactory;
 import javax.net.ssl.TrustManagerFactory;
 
+@Deprecated
 final class DefaultGrpcClientSecurityConfigurator<U, R> implements GrpcClientSecurityConfigurator<U, R> {
     private final SingleAddressHttpClientSecurityConfigurator<U, R> delegate;
     private final GrpcClientBuilder<U, R> original;

servicetalk-grpc-netty/src/main/java/io/servicetalk/grpc/netty/DefaultGrpcServerBuilder.java

@@ -35,10 +35,12 @@
 import io.servicetalk.transport.api.ConnectionAcceptorFactory;
 import io.servicetalk.transport.api.IoExecutor;
 import io.servicetalk.transport.api.ServerContext;
+import io.servicetalk.transport.api.ServerSslConfig;
 import io.servicetalk.transport.api.TransportObserver;
 import io.servicetalk.transport.netty.internal.ExecutionContextBuilder;
 
 import java.net.SocketOption;
+import java.util.Map;
 import java.util.function.BooleanSupplier;
 import java.util.function.Predicate;
 
@@ -69,16 +71,23 @@ public GrpcServerBuilder backlog(final int backlog) {
         return this;
     }
 
+    @Deprecated
     @Override
     public GrpcServerSecurityConfigurator secure() {
         HttpServerSecurityConfigurator secure = httpServerBuilder.secure();
         return new DefaultGrpcServerSecurityConfigurator(secure, this);
     }
 
     @Override
-    public GrpcServerSecurityConfigurator secure(final String... sniHostnames) {
-        HttpServerSecurityConfigurator secure = httpServerBuilder.secure(sniHostnames);
-        return new DefaultGrpcServerSecurityConfigurator(secure, this);
+    public GrpcServerBuilder sslConfig(final ServerSslConfig config) {
+        httpServerBuilder.sslConfig(config);
+        return this;
+    }
+
+    @Override
+    public GrpcServerBuilder sslConfig(final ServerSslConfig defaultConfig, final Map<String, ServerSslConfig> sniMap) {
+        httpServerBuilder.sslConfig(defaultConfig, sniMap);
+        return this;
     }
 
     @Override

servicetalk-grpc-netty/src/main/java/io/servicetalk/grpc/netty/DefaultGrpcServerSecurityConfigurator.java

@@ -24,6 +24,7 @@
 import javax.net.ssl.KeyManagerFactory;
 import javax.net.ssl.TrustManagerFactory;
 
+@Deprecated
 final class DefaultGrpcServerSecurityConfigurator implements GrpcServerSecurityConfigurator {
     private final HttpServerSecurityConfigurator delegate;
     private final GrpcServerBuilder original;