Merge pull request #34468 from cescoffier/cve-2023-2974

Enforce the configured TLS version
quarkusio · Jul 4, 2023 · 468397a · 468397a
2 parents 9bcc5ba + cdd9ce5
commit 468397a
Show file tree

Hide file tree

Showing 10 changed files with 227 additions and 23 deletions.
diff --git a/extensions/grpc/runtime/src/main/java/io/quarkus/grpc/runtime/GrpcSslUtils.java b/extensions/grpc/runtime/src/main/java/io/quarkus/grpc/runtime/GrpcSslUtils.java
@@ -117,12 +117,7 @@ static boolean applySslOptions(GrpcServerConfiguration config, HttpServerOptions
         for (String cipher : sslConfig.cipherSuites.orElse(Collections.emptyList())) {
             options.addEnabledCipherSuite(cipher);
         }
-
-        for (String protocol : sslConfig.protocols) {
-            if (!protocol.isEmpty()) {
-                options.addEnabledSecureTransportProtocol(protocol);
-            }
-        }
+        options.setEnabledSecureTransportProtocols(sslConfig.protocols);
         options.setClientAuth(sslConfig.clientAuth);
         return false;
     }

diff --git a/extensions/grpc/runtime/src/main/java/io/quarkus/grpc/runtime/GrpcTestPortUtils.java b/extensions/grpc/runtime/src/main/java/io/quarkus/grpc/runtime/GrpcTestPortUtils.java
@@ -1,6 +1,6 @@
 package io.quarkus.grpc.runtime;
 
-import java.util.List;
+import java.util.Set;
 
 import org.eclipse.microprofile.config.ConfigProvider;
 
@@ -36,7 +36,7 @@ private static boolean isHttpsConfigured(SslServerConfig ssl) {
                 || !isDefaultProtocols(ssl.protocols);
     }
 
-    private static boolean isDefaultProtocols(List<String> protocols) {
+    private static boolean isDefaultProtocols(Set<String> protocols) {
         return protocols.size() == 2 && protocols.contains("TLSv1.3") && protocols.contains("TLSv1.2");
     }
 

diff --git a/extensions/grpc/runtime/src/main/java/io/quarkus/grpc/runtime/config/SslServerConfig.java b/extensions/grpc/runtime/src/main/java/io/quarkus/grpc/runtime/config/SslServerConfig.java
@@ -3,6 +3,7 @@
 import java.nio.file.Path;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 
 import io.quarkus.runtime.annotations.ConfigGroup;
 import io.quarkus.runtime.annotations.ConfigItem;
@@ -49,7 +50,7 @@ public class SslServerConfig {
 
     /**
      * An optional trust store which holds the certificate information of the certificates to trust
-     *
+     * <p>
      * The trust store can be either on classpath or an external file.
      */
     @ConfigItem
@@ -75,11 +76,18 @@ public class SslServerConfig {
     public Optional<List<String>> cipherSuites;
 
     /**
-     * The list of protocols to explicitly enable.
+     * Sets the ordered list of enabled SSL/TLS protocols.
+     * <p>
+     * If not set, it defaults to {@code "TLSv1.3, TLSv1.2"}.
+     * The following list of protocols are supported: {@code TLSv1, TLSv1.1, TLSv1.2, TLSv1.3}.
+     * To only enable {@code TLSv1.3}, set the value to {@code to "TLSv1.3"}.
+     * <p>
+     * Note that setting an empty list, and enabling SSL/TLS is invalid.
+     * You must at least have one protocol.
      */
     @DefaultConverter
     @ConfigItem(defaultValue = "TLSv1.3,TLSv1.2")
-    public List<String> protocols;
+    public Set<String> protocols;
 
     /**
      * Configures the engine to require/request client authentication.

diff --git a/...sions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/ServerSslConfig.java b/...sions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/ServerSslConfig.java
@@ -2,6 +2,7 @@
 
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 
 import io.quarkus.runtime.annotations.ConfigGroup;
 import io.quarkus.runtime.annotations.ConfigItem;
@@ -24,11 +25,18 @@ public class ServerSslConfig {
     public Optional<List<String>> cipherSuites;
 
     /**
-     * The list of protocols to explicitly enable.
+     * Sets the ordered list of enabled SSL/TLS protocols.
+     * <p>
+     * If not set, it defaults to {@code "TLSv1.3, TLSv1.2"}.
+     * The following list of protocols are supported: {@code TLSv1, TLSv1.1, TLSv1.2, TLSv1.3}.
+     * To only enable {@code TLSv1.3}, set the value to {@code to "TLSv1.3"}.
+     * <p>
+     * Note that setting an empty list, and enabling SSL/TLS is invalid.
+     * You must at least have one protocol.
      */
     @DefaultConverter
     @ConfigItem(defaultValue = "TLSv1.3,TLSv1.2")
-    public List<String> protocols;
+    public Set<String> protocols;
 
     /**
      * Enables Server Name Indication (SNI), an TLS extension allowing the server to use multiple certificates.

diff --git a/...p/runtime/src/main/java/io/quarkus/vertx/http/runtime/options/HttpServerOptionsUtils.java b/...p/runtime/src/main/java/io/quarkus/vertx/http/runtime/options/HttpServerOptionsUtils.java
@@ -119,11 +119,7 @@ public static HttpServerOptions createSslOptions(HttpBuildTimeConfig buildTimeCo
             serverOptions.addEnabledCipherSuite(cipher);
         }
 
-        for (String protocol : sslConfig.protocols) {
-            if (!protocol.isEmpty()) {
-                serverOptions.addEnabledSecureTransportProtocol(protocol);
-            }
-        }
+        serverOptions.setEnabledSecureTransportProtocols(sslConfig.protocols);
         serverOptions.setSsl(true);
         serverOptions.setSni(sslConfig.sni);
         int sslPort = httpConfiguration.determineSslPort(launchMode);
@@ -224,11 +220,8 @@ public static HttpServerOptions createSslOptionsForManagementInterface(Managemen
             serverOptions.addEnabledCipherSuite(cipher);
         }
 
-        for (String protocol : sslConfig.protocols) {
-            if (!protocol.isEmpty()) {
-                serverOptions.addEnabledSecureTransportProtocol(protocol);
-            }
-        }
+        serverOptions.setEnabledSecureTransportProtocols(sslConfig.protocols);
+
         serverOptions.setSsl(true);
         serverOptions.setSni(sslConfig.sni);
         int sslPort = httpConfiguration.determinePort(launchMode);

diff --git a/integration-tests/vertx-http/pom.xml b/integration-tests/vertx-http/pom.xml
@@ -47,6 +47,11 @@
             <artifactId>awaitility</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>io.smallrye.reactive</groupId>
+            <artifactId>smallrye-mutiny-vertx-web-client</artifactId>
+            <scope>test</scope>
+        </dependency>
 
         <!-- Minimal test dependencies to *-deployment artifacts for consistent build order -->
         <dependency>

diff --git a/integration-tests/vertx-http/src/main/java/io/quarkus/it/vertx/HelloResource.java b/integration-tests/vertx-http/src/main/java/io/quarkus/it/vertx/HelloResource.java
@@ -0,0 +1,13 @@
+package io.quarkus.it.vertx;
+
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+
+@Path("/hello")
+public class HelloResource {
+
+    @GET
+    public String hello() {
+        return "hello";
+    }
+}
diff --git a/integration-tests/vertx-http/src/test/java/io/quarkus/it/vertx/ServerWithTLS13Only.java b/integration-tests/vertx-http/src/test/java/io/quarkus/it/vertx/ServerWithTLS13Only.java
@@ -0,0 +1,14 @@
+package io.quarkus.it.vertx;
+
+import java.util.Map;
+
+import io.quarkus.test.junit.QuarkusTestProfile;
+
+public class ServerWithTLS13Only implements QuarkusTestProfile {
+
+    @Override
+    public Map<String, String> getConfigOverrides() {
+        return Map.of(
+                "quarkus.http.ssl.protocols", "TLSv1.3");
+    }
+}
diff --git a/...tests/vertx-http/src/test/java/io/quarkus/it/vertx/TlsProtocolVersionDefaultTestCase.java b/...tests/vertx-http/src/test/java/io/quarkus/it/vertx/TlsProtocolVersionDefaultTestCase.java
@@ -0,0 +1,83 @@
+package io.quarkus.it.vertx;
+
+import java.util.Set;
+import java.util.concurrent.CompletionException;
+
+import javax.net.ssl.SSLHandshakeException;
+
+import jakarta.inject.Inject;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import io.quarkus.test.common.http.TestHTTPResource;
+import io.quarkus.test.junit.QuarkusTest;
+import io.vertx.core.net.JksOptions;
+import io.vertx.ext.web.client.WebClientOptions;
+import io.vertx.mutiny.core.Vertx;
+import io.vertx.mutiny.ext.web.client.WebClient;
+
+@QuarkusTest
+public class TlsProtocolVersionDefaultTestCase {
+
+    @TestHTTPResource(value = "/hello", ssl = true)
+    String url;
+
+    @Inject
+    Vertx vertx;
+
+    @Test
+    void testWithWebClientRequestingMultipleTlsVersions() {
+        // The Web client is requesting TLS 1.2 or 1.3, the server is exposing 1.3 - all good
+        WebClient client = WebClient.create(vertx, new WebClientOptions().setSsl(true)
+                .setKeyStoreOptions(
+                        new JksOptions().setPath("src/test/resources/client-keystore-1.jks").setPassword("password"))
+                .setTrustStoreOptions(
+                        new JksOptions().setPath("src/test/resources/client-truststore.jks").setPassword("password"))
+                .setVerifyHost(false));
+        var resp = client.getAbs(url).sendAndAwait();
+        Assertions.assertEquals(200, resp.statusCode());
+    }
+
+    @Test
+    void testWithWebClientRequestingTls13() {
+        // The Web client is requesting TLS 1.3, the server is exposing 1.3 - all good
+        WebClient client = WebClient.create(vertx, new WebClientOptions().setSsl(true)
+                .setEnabledSecureTransportProtocols(Set.of("TLSv1.3"))
+                .setKeyStoreOptions(
+                        new JksOptions().setPath("src/test/resources/client-keystore-1.jks").setPassword("password"))
+                .setTrustStoreOptions(
+                        new JksOptions().setPath("src/test/resources/client-truststore.jks").setPassword("password"))
+                .setVerifyHost(false));
+        var resp = client.getAbs(url).sendAndAwait();
+        Assertions.assertEquals(200, resp.statusCode());
+    }
+
+    @Test
+    void testWithWebClientRequestingTls12() {
+        // The Web client is requesting TLS 1.2, the server is exposing 1.2 and 1.3 - all good
+        WebClient client = WebClient.create(vertx, new WebClientOptions().setSsl(true)
+                .setEnabledSecureTransportProtocols(Set.of("TLSv1.2"))
+                .setKeyStoreOptions(
+                        new JksOptions().setPath("src/test/resources/client-keystore-1.jks").setPassword("password"))
+                .setTrustStoreOptions(
+                        new JksOptions().setPath("src/test/resources/client-truststore.jks").setPassword("password"))
+                .setVerifyHost(false));
+        var resp = client.getAbs(url).sendAndAwait();
+        Assertions.assertEquals(200, resp.statusCode());
+    }
+
+    @Test
+    void testWithWebClientRequestingTls11() {
+        // The Web client is requesting TLS 1.1, the server is exposing 1.2 and 1.3 - KO
+        WebClient client = WebClient.create(vertx, new WebClientOptions().setSsl(true)
+                .setEnabledSecureTransportProtocols(Set.of("TLSv1.1"))
+                .setKeyStoreOptions(
+                        new JksOptions().setPath("src/test/resources/client-keystore-1.jks").setPassword("password"))
+                .setTrustStoreOptions(
+                        new JksOptions().setPath("src/test/resources/client-truststore.jks").setPassword("password"))
+                .setVerifyHost(false));
+        Throwable exception = Assertions.assertThrows(CompletionException.class, () -> client.getAbs(url).sendAndAwait());
+        Assertions.assertTrue(exception.getCause() instanceof SSLHandshakeException);
+    }
+}
diff --git a/...sts/vertx-http/src/test/java/io/quarkus/it/vertx/TlsProtocolVersionSelectionTestCase.java b/...sts/vertx-http/src/test/java/io/quarkus/it/vertx/TlsProtocolVersionSelectionTestCase.java
@@ -0,0 +1,85 @@
+package io.quarkus.it.vertx;
+
+import java.util.Set;
+import java.util.concurrent.CompletionException;
+
+import javax.net.ssl.SSLHandshakeException;
+
+import jakarta.inject.Inject;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import io.quarkus.test.common.http.TestHTTPResource;
+import io.quarkus.test.junit.QuarkusTest;
+import io.quarkus.test.junit.TestProfile;
+import io.vertx.core.net.JksOptions;
+import io.vertx.ext.web.client.WebClientOptions;
+import io.vertx.mutiny.core.Vertx;
+import io.vertx.mutiny.ext.web.client.WebClient;
+
+@QuarkusTest
+@TestProfile(ServerWithTLS13Only.class)
+public class TlsProtocolVersionSelectionTestCase {
+
+    @TestHTTPResource(value = "/hello", ssl = true)
+    String url;
+
+    @Inject
+    Vertx vertx;
+
+    @Test
+    void testWithWebClientRequestingMultipleTlsVersions() {
+        // The Web client is requesting TLS 1.2 or 1.3, the server is exposing 1.3 - all good
+        WebClient client = WebClient.create(vertx, new WebClientOptions().setSsl(true)
+                .setKeyStoreOptions(
+                        new JksOptions().setPath("src/test/resources/client-keystore-1.jks").setPassword("password"))
+                .setTrustStoreOptions(
+                        new JksOptions().setPath("src/test/resources/client-truststore.jks").setPassword("password"))
+                .setVerifyHost(false));
+        var resp = client.getAbs(url).sendAndAwait();
+        Assertions.assertEquals(200, resp.statusCode());
+    }
+
+    @Test
+    void testWithWebClientRequestingTls13() {
+        // The Web client is requesting TLS 1.3, the server is exposing 1.3 - all good
+        WebClient client = WebClient.create(vertx, new WebClientOptions().setSsl(true)
+                .setEnabledSecureTransportProtocols(Set.of("TLSv1.3"))
+                .setKeyStoreOptions(
+                        new JksOptions().setPath("src/test/resources/client-keystore-1.jks").setPassword("password"))
+                .setTrustStoreOptions(
+                        new JksOptions().setPath("src/test/resources/client-truststore.jks").setPassword("password"))
+                .setVerifyHost(false));
+        var resp = client.getAbs(url).sendAndAwait();
+        Assertions.assertEquals(200, resp.statusCode());
+    }
+
+    @Test
+    void testWithWebClientRequestingTls12() {
+        // The Web client is requesting TLS 1.2, the server is exposing 1.3 - KO
+        WebClient client = WebClient.create(vertx, new WebClientOptions().setSsl(true)
+                .setEnabledSecureTransportProtocols(Set.of("TLSv1.2"))
+                .setKeyStoreOptions(
+                        new JksOptions().setPath("src/test/resources/client-keystore-1.jks").setPassword("password"))
+                .setTrustStoreOptions(
+                        new JksOptions().setPath("src/test/resources/client-truststore.jks").setPassword("password"))
+                .setVerifyHost(false));
+        Throwable exception = Assertions.assertThrows(CompletionException.class, () -> client.getAbs(url).sendAndAwait());
+        Assertions.assertTrue(exception.getCause() instanceof SSLHandshakeException);
+    }
+
+    @Test
+    void testWithWebClientRequestingTls11() {
+        // The Web client is requesting TLS 1.1, the server is exposing 1.3 - KO
+        WebClient client = WebClient.create(vertx, new WebClientOptions().setSsl(true)
+                .setEnabledSecureTransportProtocols(Set.of("TLSv1.1"))
+                .setKeyStoreOptions(
+                        new JksOptions().setPath("src/test/resources/client-keystore-1.jks").setPassword("password"))
+                .setTrustStoreOptions(
+                        new JksOptions().setPath("src/test/resources/client-truststore.jks").setPassword("password"))
+                .setVerifyHost(false));
+        Throwable exception = Assertions.assertThrows(CompletionException.class, () -> client.getAbs(url).sendAndAwait());
+        Assertions.assertTrue(exception.getCause() instanceof SSLHandshakeException);
+    }
+}