Merge pull request #11503 from vespa-engine/bjorncs/extended-ssl-syntax

Allow config of ssl cipher suites and protocol version

8 files changed, 87 insertions, 47 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/ConfiguredFilebasedSslProvider.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/ConfiguredFilebasedSslProvider.java
index 4f84a01ff94..4a331718985 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/ConfiguredFilebasedSslProvider.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/ConfiguredFilebasedSslProvider.java
@@ -8,6 +8,7 @@ import com.yahoo.jdisc.http.ssl.impl.ConfiguredSslContextFactoryProvider;
 import com.yahoo.osgi.provider.model.ComponentModel;
 import com.yahoo.vespa.model.container.component.SimpleComponent;
 
+import java.util.List;
 import java.util.Optional;
 
 import static com.yahoo.component.ComponentSpecification.fromString;
@@ -16,6 +17,7 @@ import static com.yahoo.component.ComponentSpecification.fromString;
  * Configure SSL using file references
  *
  * @author mortent
+ * @author bjorncs
  */
 public class ConfiguredFilebasedSslProvider extends SimpleComponent implements ConnectorConfig.Producer {
     public static final String COMPONENT_ID_PREFIX = "configured-ssl-provider@";
@@ -26,8 +28,16 @@ public class ConfiguredFilebasedSslProvider extends SimpleComponent implements C
     private final String certificatePath;
     private final String caCertificatePath;
     private final ConnectorConfig.Ssl.ClientAuth.Enum clientAuthentication;
+    private final List<String> cipherSuites;
+    private final List<String> protocolVersions;
 
-    public ConfiguredFilebasedSslProvider(String servername, String privateKeyPath, String certificatePath, String caCertificatePath, String clientAuthentication) {
+    public ConfiguredFilebasedSslProvider(String servername,
+                                          String privateKeyPath,
+                                          String certificatePath,
+                                          String caCertificatePath,
+                                          String clientAuthentication,
+                                          List<String> cipherSuites,
+                                          List<String> protocolVersions) {
         super(new ComponentModel(
                 new BundleInstantiationSpecification(new ComponentId(COMPONENT_ID_PREFIX+servername),
                                                      fromString(COMPONENT_CLASS),
@@ -36,15 +46,21 @@ public class ConfiguredFilebasedSslProvider extends SimpleComponent implements C
         this.certificatePath = certificatePath;
         this.caCertificatePath = caCertificatePath;
         this.clientAuthentication = mapToConfigEnum(clientAuthentication);
+        this.cipherSuites = cipherSuites;
+        this.protocolVersions = protocolVersions;
     }
 
     @Override
     public void getConfig(ConnectorConfig.Builder builder) {
-        builder.ssl.enabled(true);
-        builder.ssl.privateKeyFile(privateKeyPath);
-        builder.ssl.certificateFile(certificatePath);
-        builder.ssl.caCertificateFile(Optional.ofNullable(caCertificatePath).orElse(""));
-        builder.ssl.clientAuth(clientAuthentication);
+        builder.ssl(
+                new ConnectorConfig.Ssl.Builder()
+                        .enabled(true)
+                        .privateKeyFile(privateKeyPath)
+                        .certificateFile(certificatePath)
+                        .caCertificateFile(Optional.ofNullable(caCertificatePath).orElse(""))
+                        .clientAuth(clientAuthentication)
+                        .enabledCipherSuites(cipherSuites)
+                        .enabledProtocols(protocolVersions));
     }
 
     public SimpleComponent getComponent() {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/JettyConnectorBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/JettyConnectorBuilder.java
index 1b457b1250a..499268929b7 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/JettyConnectorBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/JettyConnectorBuilder.java
@@ -8,13 +8,17 @@ import com.yahoo.text.XML;
 import com.yahoo.vespa.model.builder.xml.dom.VespaDomBuilder;
 import com.yahoo.vespa.model.container.component.SimpleComponent;
 import com.yahoo.vespa.model.container.http.ConnectorFactory;
-import com.yahoo.vespa.model.container.http.ssl.CustomSslProvider;
 import com.yahoo.vespa.model.container.http.ssl.ConfiguredFilebasedSslProvider;
+import com.yahoo.vespa.model.container.http.ssl.CustomSslProvider;
 import com.yahoo.vespa.model.container.http.ssl.DefaultSslProvider;
 import org.w3c.dom.Element;
 
+import java.util.Arrays;
+import java.util.List;
 import java.util.Optional;
 
+import static java.util.stream.Collectors.toList;
+
 /**
  * @author Einar M R Rosenvinge
  * @author mortent
@@ -39,12 +43,16 @@ public class JettyConnectorBuilder extends VespaDomBuilder.DomConfigProducerBuil
             String certificateFile = XML.getValue(XML.getChild(sslConfigurator, "certificate-file"));
             Optional<String> caCertificateFile = XmlHelper.getOptionalChildValue(sslConfigurator, "ca-certificates-file");
             Optional<String> clientAuthentication = XmlHelper.getOptionalChildValue(sslConfigurator, "client-authentication");
+            List<String> cipherSuites = extractOptionalCommaSeparatedList(sslConfigurator, "cipher-suites");
+            List<String> protocols = extractOptionalCommaSeparatedList(sslConfigurator, "protocols");
             return new ConfiguredFilebasedSslProvider(
                     serverName,
                     privateKeyFile,
                     certificateFile,
                     caCertificateFile.orElse(null),
-                    clientAuthentication.orElse(null));
+                    clientAuthentication.orElse(null),
+                    cipherSuites,
+                    protocols);
         } else if (sslProviderConfigurator != null) {
             String className = sslProviderConfigurator.getAttribute("class");
             String bundle = sslProviderConfigurator.getAttribute("bundle");
@@ -53,4 +61,14 @@ public class JettyConnectorBuilder extends VespaDomBuilder.DomConfigProducerBuil
             return new DefaultSslProvider(serverName);
         }
     }
+
+    private static List<String> extractOptionalCommaSeparatedList(Element sslElement, String listElementName) {
+        return XmlHelper.getOptionalChildValue(sslElement, listElementName)
+                .map(element ->
+                             Arrays.stream(element.split(","))
+                                     .filter(listEntry -> !listEntry.isBlank())
+                                     .map(String::trim)
+                                     .collect(toList()))
+                .orElse(List.of());
+    }
 }
diff --git a/config-model/src/main/resources/schema/containercluster.rnc b/config-model/src/main/resources/schema/containercluster.rnc
index 142abb5c63b..a8228a233b3 100644
--- a/config-model/src/main/resources/schema/containercluster.rnc
+++ b/config-model/src/main/resources/schema/containercluster.rnc
@@ -95,7 +95,9 @@ Ssl = element ssl {
     element private-key-file { string } &
     element certificate-file { string } &
     element ca-certificates-file { string }? &
-    element client-authentication { string "disabled" | string "want" | string "need" }?
+    element client-authentication { string "disabled" | string "want" | string "need" }? &
+    element cipher-suites { string }? &
+    element protocols { string }?
 }
 
 SslProvider = element ssl-provider {
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JettyContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JettyContainerModelBuilderTest.java
index 929e520f984..4679377ce94 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JettyContainerModelBuilderTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/JettyContainerModelBuilderTest.java
@@ -152,6 +152,14 @@ public class JettyContainerModelBuilderTest extends ContainerModelBuilderTestBas
                 "                <client-authentication>need</client-authentication>",
                 "            </ssl>",
                 "        </server>",
+                "        <server port='9003' id='with-ciphers-and-protocols'>",
+                "            <ssl>",
+                "                <private-key-file>/foo/key</private-key-file>",
+                "                <certificate-file>/foo/cert</certificate-file>",
+                "                <cipher-suites>TLS_AES_128_GCM_SHA256,TLS_AES_256_GCM_SHA384</cipher-suites>",
+                "                <protocols>TLSv1.3</protocols>",
+                "            </ssl>",
+                "        </server>",
                 "    </http>",
                 nodesXml,
                 "",
@@ -179,6 +187,13 @@ public class JettyContainerModelBuilderTest extends ContainerModelBuilderTestBas
         assertThat(needClientAuth.ssl().caCertificateFile(), is(equalTo("")));
         assertThat(needClientAuth.ssl().clientAuth(), is(equalTo(ConnectorConfig.Ssl.ClientAuth.Enum.NEED_AUTH)));
 
+        ConnectorConfig withCiphersAndProtocols = root.getConfig(ConnectorConfig.class, "default/http/jdisc-jetty/with-ciphers-and-protocols/configured-ssl-provider@with-ciphers-and-protocols");
+        assertTrue(withCiphersAndProtocols.ssl().enabled());
+        assertThat(withCiphersAndProtocols.ssl().privateKeyFile(), is(equalTo("/foo/key")));
+        assertThat(withCiphersAndProtocols.ssl().certificateFile(), is(equalTo("/foo/cert")));
+        assertThat(withCiphersAndProtocols.ssl().enabledCipherSuites(), is(equalTo(List.of("TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384"))));
+        assertThat(withCiphersAndProtocols.ssl().enabledProtocols(), is(equalTo(List.of("TLSv1.3"))));
+
         ContainerCluster cluster = (ContainerCluster) root.getChildren().get("default");
         List<ConnectorFactory> connectorFactories = cluster.getChildrenByTypeRecursive(ConnectorFactory.class);
         connectorFactories.forEach(connectorFactory -> assertChildComponentExists(connectorFactory, ConfiguredFilebasedSslProvider.COMPONENT_CLASS));
diff --git a/config-model/src/test/schema-test-files/services.xml b/config-model/src/test/schema-test-files/services.xml
index 2bbd98f72ac..1bf42650123 100644
--- a/config-model/src/test/schema-test-files/services.xml
+++ b/config-model/src/test/schema-test-files/services.xml
@@ -119,6 +119,13 @@
           <certificate-file>/foo/cert</certificate-file>
           <ca-certificates-file>/foo/cacerts</ca-certificates-file>
           <client-authentication>want</client-authentication>
+          <cipher-suites>
+            TLS_AES_128_GCM_SHA256,
+            TLS_AES_256_GCM_SHA384,
+            TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
+            TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
+          </cipher-suites>
+          <protocols>TLSv1.2,TLSv1.3</protocols>
         </ssl>
       </server>
       <server port="4083" id="sslProvider">
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
index 7a683b74656..140feb75026 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
@@ -23,6 +23,7 @@ import org.eclipse.jetty.server.Handler;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnectionStatistics;
 import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.server.SslConnectionFactory;
 import org.eclipse.jetty.server.handler.AbstractHandlerContainer;
 import org.eclipse.jetty.server.handler.HandlerCollection;
 import org.eclipse.jetty.server.handler.StatisticsHandler;
@@ -316,6 +317,7 @@ public class JettyHttpServer extends AbstractServerProvider {
     public void start() {
         try {
             server.start();
+            logEffectiveSslConfiguration();
         } catch (final Exception e) {
             if (e instanceof IOException && e.getCause() instanceof BindException) {
                 throw new RuntimeException("Failed to start server due to BindExecption. ListenPorts = " + listenedPorts.toString(), e.getCause());
@@ -324,6 +326,22 @@ public class JettyHttpServer extends AbstractServerProvider {
         }
     }
 
+    private void logEffectiveSslConfiguration() {
+        if (!server.isStarted()) throw new IllegalStateException();
+        for (Connector connector : server.getConnectors()) {
+            ServerConnector serverConnector = (ServerConnector) connector;
+            int localPort = serverConnector.getLocalPort();
+            var sslConnectionFactory = serverConnector.getConnectionFactory(SslConnectionFactory.class);
+            if (sslConnectionFactory != null) {
+                var sslContextFactory = sslConnectionFactory.getSslContextFactory();
+                log.info(String.format("Enabled SSL cipher suites for port '%d': %s",
+                                       localPort, Arrays.toString(sslContextFactory.getSelectedCipherSuites())));
+                log.info(String.format("Enabled SSL protocols for port '%d': %s",
+                                       localPort, Arrays.toString(sslContextFactory.getSelectedProtocols())));
+            }
+        }
+    }
+
     @Override
     public void close() {
         try {
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/impl/ConfiguredSslContextFactoryProvider.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/impl/ConfiguredSslContextFactoryProvider.java
index b2e7ba1be67..90848f1dfd4 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/impl/ConfiguredSslContextFactoryProvider.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/impl/ConfiguredSslContextFactoryProvider.java
@@ -70,12 +70,12 @@ public class ConfiguredSslContextFactoryProvider implements SslContextFactoryPro
 
         List<String> protocols = !sslConfig.enabledProtocols().isEmpty()
                 ? sslConfig.enabledProtocols()
-                : new ArrayList<>(TlsContext.ALLOWED_PROTOCOLS);
+                : new ArrayList<>(TlsContext.getAllowedProtocols(sslContext));
         setEnabledProtocols(factory, sslContext, protocols);
 
         List<String> ciphers = !sslConfig.enabledCipherSuites().isEmpty()
                 ? sslConfig.enabledCipherSuites()
-                : new ArrayList<>(TlsContext.ALLOWED_CIPHER_SUITES);
+                : new ArrayList<>(TlsContext.getAllowedCipherSuites(sslContext));
         setEnabledCipherSuites(factory, sslContext, ciphers);
 
         return factory;
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/impl/JDiscSslContextFactory.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/impl/JDiscSslContextFactory.java
deleted file mode 100644
index 4d3bb4a280a..00000000000
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/ssl/impl/JDiscSslContextFactory.java
+++ /dev/null
@@ -1,36 +0,0 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.jdisc.http.ssl.impl;
-
-import org.eclipse.jetty.util.resource.Resource;
-import org.eclipse.jetty.util.security.CertificateUtils;
-import org.eclipse.jetty.util.ssl.SslContextFactory;
-
-import java.security.KeyStore;
-import java.util.Objects;
-
-/**
- * A modified {@link SslContextFactory} that allows passwordless truststore in combination with password protected keystore.
- *
- * @author bjorncs
- */
-class JDiscSslContextFactory extends SslContextFactory.Server {
-
-    private String trustStorePassword;
-
-    @Override
-    public void setTrustStorePassword(String password) {
-        super.setTrustStorePassword(password);
-        this.trustStorePassword = password;
-    }
-
-
-    // Overriden to stop Jetty from using the keystore password if no truststore password is specified.
-    @Override
-    protected KeyStore loadTrustStore(Resource resource) throws Exception {
-        return CertificateUtils.getKeyStore(
-                resource != null ? resource : getKeyStoreResource(),
-                Objects.toString(getTrustStoreType(), getKeyStoreType()),
-                Objects.toString(getTrustStoreProvider(), getKeyStoreProvider()),
-                trustStorePassword);
-    }
-}