Add SslConfig.cipherSuiteFilter() (#2798)

Motivation: Sometimes users can configure ciphers that are not supported by the current `SSLEngine`. As a result, they are getting a runtime exception. Modifications: - Add new API `SslConfig.cipherSuiteFilter()` that allows users to configure how Netty should consume the configured ciphers: take them all as-is or filter out unsupported ciphers from the passed list; - Test new behavior; Result: Users can configure `SslConfig` to filter out unsupported ciphers.
apple · Jan 16, 2024 · 4da962a · 4da962a
1 parent f9e3f0b
commit 4da962a
Show file tree

Hide file tree

Showing 8 changed files with 246 additions and 16 deletions.
diff --git a/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/SslCipherSuiteFilterTest.java b/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/SslCipherSuiteFilterTest.java
@@ -0,0 +1,150 @@
+/*
+ * Copyright © 2024 Apple Inc. and the ServiceTalk project authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.servicetalk.http.netty;
+
+import io.servicetalk.http.api.BlockingHttpClient;
+import io.servicetalk.http.api.BlockingHttpConnection;
+import io.servicetalk.http.api.HttpResponse;
+import io.servicetalk.http.api.HttpServerContext;
+import io.servicetalk.test.resources.DefaultTestCerts;
+import io.servicetalk.transport.api.ClientSslConfigBuilder;
+import io.servicetalk.transport.api.ServerSslConfigBuilder;
+import io.servicetalk.transport.api.SslProvider;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
+
+import java.io.IOException;
+import javax.net.ssl.SSLSession;
+
+import static io.servicetalk.http.api.HttpResponseStatus.OK;
+import static io.servicetalk.test.resources.DefaultTestCerts.serverPemHostname;
+import static io.servicetalk.transport.api.SslConfig.CipherSuiteFilter.PROVIDED;
+import static io.servicetalk.transport.api.SslConfig.CipherSuiteFilter.SUPPORTED;
+import static io.servicetalk.transport.netty.internal.AddressUtils.localAddress;
+import static io.servicetalk.transport.netty.internal.AddressUtils.serverHostAndPort;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.containsStringIgnoringCase;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+class SslCipherSuiteFilterTest {
+
+    private static final String SUPPORTED_CIPHER = "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384";
+    private static final String UNSUPPORTED_CIPHER = "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305";
+
+    @ParameterizedTest(name = "{displayName} [{index}] provider={0}")
+    @EnumSource(SslProvider.class)
+    void clientWithIdentityFilterAndUnsupportedCipher(SslProvider provider) throws Exception {
+        try (HttpServerContext serverContext = HttpServers.forAddress(localAddress(0))
+                .sslConfig(new ServerSslConfigBuilder(DefaultTestCerts::loadServerPem, DefaultTestCerts::loadServerKey)
+                        .build())
+                .listenBlockingAndAwait((ctx, request, responseFactory) -> responseFactory.ok())) {
+
+            IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> {
+                try (BlockingHttpClient client = HttpClients.forSingleAddress(serverHostAndPort(serverContext))
+                        .sslConfig(new ClientSslConfigBuilder(DefaultTestCerts::loadServerCAPem)
+                                .peerHost(serverPemHostname())
+                                .provider(provider)
+                                .ciphers(SUPPORTED_CIPHER, UNSUPPORTED_CIPHER)
+                                .cipherSuiteFilter(PROVIDED)
+                                .build())
+                        .buildBlocking()) {
+
+                    if (provider == SslProvider.JDK) {
+                        client.request(client.get("/"));
+                    }
+                }
+            });
+            if (provider == SslProvider.OPENSSL) {
+                e = (IllegalArgumentException) e.getCause().getCause();
+            }
+            assertThat(e.getMessage(), containsStringIgnoringCase("unsupported"));
+            assertThat(e.getMessage(), containsString(UNSUPPORTED_CIPHER));
+        }
+    }
+
+    @Test
+    void serverWithIdentityFilterAndUnsupportedCipherOpenSslProvider() throws Exception {
+        IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> {
+            HttpServers.forAddress(localAddress(0))
+                    .sslConfig(new ServerSslConfigBuilder(
+                            DefaultTestCerts::loadServerPem, DefaultTestCerts::loadServerKey)
+                            .provider(SslProvider.OPENSSL)
+                            .ciphers(SUPPORTED_CIPHER, UNSUPPORTED_CIPHER)
+                            .cipherSuiteFilter(PROVIDED)
+                            .build())
+                    .listenBlockingAndAwait((ctx, request, responseFactory) -> responseFactory.ok());
+        });
+        e = (IllegalArgumentException) e.getCause().getCause();
+        assertThat(e.getMessage(), containsStringIgnoringCase("unsupported"));
+        assertThat(e.getMessage(), containsString(UNSUPPORTED_CIPHER));
+    }
+
+    @Test
+    void serverWithIdentityFilterAndUnsupportedCipherJdkProvider() throws Exception {
+        try (HttpServerContext serverContext = HttpServers.forAddress(localAddress(0))
+                .sslConfig(new ServerSslConfigBuilder(DefaultTestCerts::loadServerPem, DefaultTestCerts::loadServerKey)
+                        .provider(SslProvider.JDK)
+                        .ciphers(SUPPORTED_CIPHER, UNSUPPORTED_CIPHER)
+                        .cipherSuiteFilter(PROVIDED)
+                        .build())
+                .listenBlockingAndAwait((ctx, request, responseFactory) -> responseFactory.ok());
+             BlockingHttpClient client = HttpClients.forSingleAddress(serverHostAndPort(serverContext))
+                     .sslConfig(new ClientSslConfigBuilder(DefaultTestCerts::loadServerCAPem)
+                             .peerHost(serverPemHostname())
+                             .build())
+                     .buildBlocking()) {
+            assertThrows(IOException.class, () -> client.request(client.get("/")));
+        }
+    }
+
+    @ParameterizedTest(name = "{displayName} [{index}] provider={0}")
+    @EnumSource(SslProvider.class)
+    void supportedFilterWithUnsupportedCipher(SslProvider provider) throws Exception {
+        try (HttpServerContext serverContext = HttpServers.forAddress(localAddress(0))
+                .sslConfig(new ServerSslConfigBuilder(DefaultTestCerts::loadServerPem, DefaultTestCerts::loadServerKey)
+                        .provider(provider)
+                        .ciphers(SUPPORTED_CIPHER, UNSUPPORTED_CIPHER)
+                        .cipherSuiteFilter(SUPPORTED)
+                        // Enforce TLSv1.2 because OPENSSL provider ignores ciphers when TLSv1.3 is used
+                        .sslProtocols("TLSv1.2")
+                        .build())
+                .listenBlockingAndAwait((ctx, request, responseFactory) -> responseFactory.ok());
+             BlockingHttpClient client = HttpClients.forSingleAddress(serverHostAndPort(serverContext))
+                     .sslConfig(new ClientSslConfigBuilder(DefaultTestCerts::loadServerCAPem)
+                             .peerHost(serverPemHostname())
+                             .provider(provider)
+                             .ciphers(SUPPORTED_CIPHER, UNSUPPORTED_CIPHER)
+                             .cipherSuiteFilter(SUPPORTED)
+                             // Enforce TLSv1.2 because OPENSSL provider ignores ciphers when TLSv1.3 is used
+                             .sslProtocols("TLSv1.2")
+                             .build())
+                     .buildBlocking();
+             BlockingHttpConnection connection = client.reserveConnection(client.get("/"))) {
+
+            SSLSession sslSession = connection.connectionContext().sslSession();
+            assertThat(sslSession, is(notNullValue()));
+            assertThat(sslSession.getCipherSuite(), is(SUPPORTED_CIPHER));
+
+            HttpResponse response = connection.request(client.get("/"));
+            assertThat(response.status(), is(OK));
+        }
+    }
+}
diff --git a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/AbstractSslConfig.java b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/AbstractSslConfig.java
@@ -42,6 +42,7 @@ abstract class AbstractSslConfig implements SslConfig {
     private final List<String> alpnProtocols;
     @Nullable
     private final List<String> ciphers;
+    private final CipherSuiteFilter cipherSuiteFilter;
     private final long sessionCacheSize;
     private final long sessionTimeout;
     private final int maxCertificateListBytes;
@@ -58,7 +59,8 @@ abstract class AbstractSslConfig implements SslConfig {
                       @Nullable final Supplier<InputStream> keySupplier,
                       @Nullable final String keyPassword, @Nullable final List<String> sslProtocols,
                       @Nullable final List<String> alpnProtocols,
-                      @Nullable final List<String> ciphers, final long sessionCacheSize, final long sessionTimeout,
+                      @Nullable final List<String> ciphers, final CipherSuiteFilter cipherSuiteFilter,
+                      final long sessionCacheSize, final long sessionTimeout,
                       final int maxCertificateListBytes, @Nullable final SslProvider provider,
                       @Nullable final List<CertificateCompressionAlgorithm> certificateCompressionAlgorithms,
                       final Duration handshakeTimeout) {
@@ -71,6 +73,7 @@ abstract class AbstractSslConfig implements SslConfig {
         this.sslProtocols = sslProtocols;
         this.alpnProtocols = alpnProtocols;
         this.ciphers = ciphers;
+        this.cipherSuiteFilter = cipherSuiteFilter;
         this.sessionCacheSize = sessionCacheSize;
         this.sessionTimeout = sessionTimeout;
         this.maxCertificateListBytes = maxCertificateListBytes;
@@ -133,6 +136,11 @@ public final List<String> ciphers() {
         return ciphers;
     }
 
+    @Override
+    public CipherSuiteFilter cipherSuiteFilter() {
+        return cipherSuiteFilter;
+    }
+
     @Override
     public final long sessionCacheSize() {
         return sessionCacheSize;

diff --git a/...lk-transport-api/src/main/java/io/servicetalk/transport/api/AbstractSslConfigBuilder.java b/...lk-transport-api/src/main/java/io/servicetalk/transport/api/AbstractSslConfigBuilder.java
@@ -15,6 +15,8 @@
  */
 package io.servicetalk.transport.api;
 
+import io.servicetalk.transport.api.SslConfig.CipherSuiteFilter;
+
 import java.io.InputStream;
 import java.time.Duration;
 import java.util.Arrays;
@@ -60,6 +62,7 @@ abstract class AbstractSslConfigBuilder<T extends AbstractSslConfigBuilder<T>> {
     private List<String> alpnProtocols;
     @Nullable
     private List<String> ciphers;
+    private CipherSuiteFilter cipherSuiteFilter = CipherSuiteFilter.PROVIDED;
     private long sessionCacheSize;
     private long sessionTimeout;
     private int maxCertificateListBytes = DEFAULT_MAX_CERTIFICATE_LIST_BYTES;
@@ -299,6 +302,24 @@ final List<String> ciphers() {
         return ciphers;
     }
 
+    /**
+     * Set the filtering behavior for ciphers suites.
+     *
+     * @param cipherSuiteFilter {@link CipherSuiteFilter} to use.
+     * @return {@code this}.
+     * @see SslConfig#cipherSuiteFilter()
+     * @see #ciphers(String...)
+     * @see #ciphers(List)
+     */
+    public final T cipherSuiteFilter(final CipherSuiteFilter cipherSuiteFilter) {
+        this.cipherSuiteFilter = requireNonNull(cipherSuiteFilter);
+        return thisT();
+    }
+
+    final CipherSuiteFilter cipherSuiteFilter() {
+        return cipherSuiteFilter;
+    }
+
     /**
      * Get the size of the cache used for storing SSL session objects.
      *

diff --git a/...talk-transport-api/src/main/java/io/servicetalk/transport/api/ClientSslConfigBuilder.java b/...talk-transport-api/src/main/java/io/servicetalk/transport/api/ClientSslConfigBuilder.java
@@ -144,8 +144,9 @@ public ClientSslConfigBuilder sniHostname(String sniHostname) {
     public ClientSslConfig build() {
         return new DefaultClientSslConfig(hostnameVerificationAlgorithm, peerHost, peerPort, sniHostname,
                 trustManager(), trustCertChainSupplier(), keyManager(), keyCertChainSupplier(), keySupplier(),
-                keyPassword(), sslProtocols(), alpnProtocols(), ciphers(), sessionCacheSize(), sessionTimeout(),
-                maxCertificateListBytes(), provider(), certificateCompressionAlgorithms(), handshakeTimeout());
+                keyPassword(), sslProtocols(), alpnProtocols(), ciphers(), cipherSuiteFilter(), sessionCacheSize(),
+                sessionTimeout(), maxCertificateListBytes(), provider(), certificateCompressionAlgorithms(),
+                handshakeTimeout());
     }
 
     @Override
@@ -172,14 +173,15 @@ private static final class DefaultClientSslConfig extends AbstractSslConfig impl
                                @Nullable final Supplier<InputStream> keyCertChainSupplier,
                                @Nullable final Supplier<InputStream> keySupplier, @Nullable final String keyPassword,
                                @Nullable final List<String> sslProtocols, @Nullable final List<String> alpnProtocols,
-                               @Nullable final List<String> ciphers, final long sessionCacheSize,
-                               final long sessionTimeout, final int maxCertificateListBytes,
-                               @Nullable final SslProvider provider,
+                               @Nullable final List<String> ciphers, final CipherSuiteFilter cipherSuiteFilter,
+                               final long sessionCacheSize, final long sessionTimeout,
+                               final int maxCertificateListBytes, @Nullable final SslProvider provider,
                                @Nullable final List<CertificateCompressionAlgorithm> certificateCompressionAlgorithms,
                                final Duration handshakeTimeout) {
             super(trustManagerFactory, trustCertChainSupplier, keyManagerFactory, keyCertChainSupplier, keySupplier,
-                    keyPassword, sslProtocols, alpnProtocols, ciphers, sessionCacheSize, sessionTimeout,
-                    maxCertificateListBytes, provider, certificateCompressionAlgorithms, handshakeTimeout);
+                    keyPassword, sslProtocols, alpnProtocols, ciphers, cipherSuiteFilter, sessionCacheSize,
+                    sessionTimeout, maxCertificateListBytes, provider, certificateCompressionAlgorithms,
+                    handshakeTimeout);
             this.hostnameVerificationAlgorithm = hostnameVerificationAlgorithm;
             this.peerHost = peerHost;
             this.peerPort = peerPort;

diff --git a/...icetalk-transport-api/src/main/java/io/servicetalk/transport/api/DelegatingSslConfig.java b/...icetalk-transport-api/src/main/java/io/servicetalk/transport/api/DelegatingSslConfig.java
@@ -102,6 +102,11 @@ public List<String> ciphers() {
         return delegate.ciphers();
     }
 
+    @Override
+    public CipherSuiteFilter cipherSuiteFilter() {
+        return delegate.cipherSuiteFilter();
+    }
+
     @Override
     public long sessionCacheSize() {
         return delegate.sessionCacheSize();

diff --git a/...talk-transport-api/src/main/java/io/servicetalk/transport/api/ServerSslConfigBuilder.java b/...talk-transport-api/src/main/java/io/servicetalk/transport/api/ServerSslConfigBuilder.java
@@ -108,7 +108,7 @@ public ServerSslConfigBuilder clientAuthMode(SslClientAuthMode clientAuthMode) {
     public ServerSslConfig build() {
         return new DefaultServerSslConfig(clientAuthMode, trustManager(), trustCertChainSupplier(), keyManager(),
                 keyCertChainSupplier(), keySupplier(), keyPassword(), sslProtocols(), alpnProtocols(), ciphers(),
-                sessionCacheSize(), sessionTimeout(), maxCertificateListBytes(), provider(),
+                cipherSuiteFilter(), sessionCacheSize(), sessionTimeout(), maxCertificateListBytes(), provider(),
                 certificateCompressionAlgorithms(), handshakeTimeout());
     }
 
@@ -127,14 +127,15 @@ private static final class DefaultServerSslConfig extends AbstractSslConfig impl
                                @Nullable final Supplier<InputStream> keyCertChainSupplier,
                                @Nullable final Supplier<InputStream> keySupplier, @Nullable final String keyPassword,
                                @Nullable final List<String> sslProtocols, @Nullable final List<String> alpnProtocols,
-                               @Nullable final List<String> ciphers, final long sessionCacheSize,
-                               final long sessionTimeout, final int maxCertificateListBytes,
-                               @Nullable final SslProvider provider,
+                               @Nullable final List<String> ciphers, final CipherSuiteFilter cipherSuiteFilter,
+                               final long sessionCacheSize, final long sessionTimeout,
+                               final int maxCertificateListBytes, @Nullable final SslProvider provider,
                                @Nullable final List<CertificateCompressionAlgorithm> certificateCompressionAlgorithms,
                                final Duration handshakeTimeout) {
             super(trustManagerFactory, trustCertChainSupplier, keyManagerFactory, keyCertChainSupplier, keySupplier,
-                    keyPassword, sslProtocols, alpnProtocols, ciphers, sessionCacheSize, sessionTimeout,
-                    maxCertificateListBytes, provider, certificateCompressionAlgorithms, handshakeTimeout);
+                    keyPassword, sslProtocols, alpnProtocols, ciphers, cipherSuiteFilter, sessionCacheSize,
+                    sessionTimeout, maxCertificateListBytes, provider, certificateCompressionAlgorithms,
+                    handshakeTimeout);
             this.clientAuthMode = clientAuthMode;
         }
 

diff --git a/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/SslConfig.java b/servicetalk-transport-api/src/main/java/io/servicetalk/transport/api/SslConfig.java
@@ -112,10 +112,21 @@ public interface SslConfig {
      * Get the cipher suites to enable, in the order of preference.
      *
      * @return the cipher suites to enable, in the order of preference.
+     * @see #cipherSuiteFilter()
      */
     @Nullable
     List<String> ciphers();
 
+    /**
+     * Defines filtering behavior for ciphers suites.
+     *
+     * @return filtering behavior for ciphers suites.
+     * @see #ciphers()
+     */
+    default CipherSuiteFilter cipherSuiteFilter() {
+        return CipherSuiteFilter.PROVIDED;
+    }
+
     /**
      * Get the size of the cache used for storing SSL session objects.
      *
@@ -186,4 +197,21 @@ default Duration handshakeTimeout() {   // FIXME 0.43 - remove default implement
     default int maxCertificateListBytes() {
         return 0;
     }
+
+    /**
+     * Defines filtering logic for ciphers suites.
+     *
+     * @see #ciphers()
+     */
+    enum CipherSuiteFilter {
+        /**
+         * Will take all provided ciphers suites as-is without any filtering.
+         */
+        PROVIDED,
+
+        /**
+         * Will filter all provided ciphers suites out that are not supported by the current {@link SSLEngine}.
+         */
+        SUPPORTED
+    }
 }