Merge pull request #16885 from vespa-engine/revert-16884-revert-16881-bjorncs/zookeeper-tls

Revert "Revert "Specify TLS configuration when enabling secure ZK client""

4 files changed, 67 insertions, 37 deletions

diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java
index adfd9bd051f..4cbb6c95cb4 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java
@@ -10,6 +10,7 @@ import com.yahoo.text.Utf8;
 import com.yahoo.vespa.curator.api.VespaCurator;
 import com.yahoo.vespa.curator.recipes.CuratorCounter;
 import com.yahoo.vespa.defaults.Defaults;
+import com.yahoo.vespa.zookeeper.VespaSslContextProvider;
 import com.yahoo.vespa.zookeeper.VespaZooKeeperServer;
 import org.apache.curator.RetryPolicy;
 import org.apache.curator.framework.CuratorFramework;
@@ -124,9 +125,15 @@ public class Curator implements VespaCurator, AutoCloseable {
     private static ZKClientConfig createClientConfig(Optional<File> clientConfigFile) {
         if (clientConfigFile.isPresent()) {
             boolean useSecureClient = Boolean.parseBoolean(getEnvironmentVariable("VESPA_USE_TLS_FOR_ZOOKEEPER_CLIENT").orElse("false"));
-            String config = "zookeeper.client.secure=" + useSecureClient + "\n";
+            StringBuilder configBuilder = new StringBuilder("zookeeper.client.secure=").append(useSecureClient).append("\n");
+            if (useSecureClient) {
+                configBuilder.append("zookeeper.ssl.context.supplier.class=").append(VespaSslContextProvider.class.getName()).append("\n")
+                        .append("zookeeper.ssl.enabledProtocols=").append(VespaSslContextProvider.enabledTlsProtocolConfigValue()).append("\n")
+                        .append("zookeeper.ssl.ciphersuites=").append(VespaSslContextProvider.enabledTlsCiphersConfigValue()).append("\n")
+                        .append("zookeeper.ssl.clientAuth=NEED\n");
+            }
             clientConfigFile.get().getParentFile().mkdirs();
-            IOUtils.writeFile(clientConfigFile.get(), Utf8.toBytes(config));
+            IOUtils.writeFile(clientConfigFile.get(), Utf8.toBytes(configBuilder.toString()));
             try {
                 return new ZKClientConfig(clientConfigFile.get());
             } catch (QuorumPeerConfig.ConfigException e) {
diff --git a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java
index ba79969469a..ebf5032a4a7 100644
--- a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java
+++ b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/Configurator.java
@@ -7,14 +7,11 @@ import com.yahoo.security.KeyStoreBuilder;
 import com.yahoo.security.KeyStoreType;
 import com.yahoo.security.KeyStoreUtils;
 import com.yahoo.security.KeyUtils;
-import com.yahoo.security.SslContextBuilder;
 import com.yahoo.security.X509CertificateUtils;
-import com.yahoo.security.tls.TlsContext;
 import com.yahoo.security.tls.TransportSecurityOptions;
 import com.yahoo.text.Utf8;
 import com.yahoo.vespa.defaults.Defaults;
 
-import javax.net.ssl.SSLContext;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.nio.file.Files;
@@ -24,8 +21,6 @@ import java.security.PrivateKey;
 import java.security.cert.X509Certificate;
 import java.util.List;
 import java.util.Optional;
-import java.util.Set;
-import java.util.TreeSet;
 import java.util.logging.Level;
 import java.util.stream.Collectors;
 
@@ -95,9 +90,8 @@ public class Configurator {
         sb.append("metricsProvider.className=org.apache.zookeeper.metrics.impl.NullMetricsProvider\n");
         ensureThisServerIsRepresented(config.myid(), config.server());
         config.server().forEach(server -> addServerToCfg(sb, server, config.clientPort()));
-        SSLContext sslContext = new SslContextBuilder().build();
-        sb.append(new TlsQuorumConfig(sslContext, jksKeyStoreFilePath).createConfig(config, transportSecurityOptions));
-        sb.append(new TlsClientServerConfig(sslContext, jksKeyStoreFilePath).createConfig(config, transportSecurityOptions));
+        sb.append(new TlsQuorumConfig(jksKeyStoreFilePath).createConfig(config, transportSecurityOptions));
+        sb.append(new TlsClientServerConfig(jksKeyStoreFilePath).createConfig(config, transportSecurityOptions));
         return sb.toString();
     }
 
@@ -178,10 +172,6 @@ public class Configurator {
     }
 
     private interface TlsConfig {
-        default Set<String> allowedCiphers(SSLContext sslContext) { return new TreeSet<>(TlsContext.getAllowedCipherSuites(sslContext)); }
-
-        default Set<String> allowedProtocols(SSLContext sslContext) { return new TreeSet<>(TlsContext.getAllowedProtocols(sslContext)); }
-
         default Optional<String> getEnvironmentVariable(String variableName) {
             return Optional.ofNullable(System.getenv().get(variableName))
                     .filter(var -> !var.isEmpty());
@@ -196,8 +186,6 @@ public class Configurator {
 
         Path jksKeyStoreFilePath();
 
-        SSLContext sslContext();
-
         default String createCommonKeyStoreTrustStoreOptions(Optional<TransportSecurityOptions> transportSecurityOptions) {
             StringBuilder sb = new StringBuilder();
             transportSecurityOptions.ifPresent(options -> {
@@ -215,10 +203,9 @@ public class Configurator {
             StringBuilder sb = new StringBuilder();
             sb.append(configFieldPrefix()).append(".hostnameVerification=false\n");
             sb.append(configFieldPrefix()).append(".clientAuth=NEED\n");
-            sb.append(configFieldPrefix()).append(".ciphersuites=").append(String.join(",", allowedCiphers(sslContext()))).append("\n");
-            sb.append(configFieldPrefix()).append(".enabledProtocols=").append(String.join(",", allowedProtocols(sslContext()))).append("\n");
-            sb.append(configFieldPrefix()).append(".protocol=").append(sslContext().getProtocol()).append("\n");
-
+            sb.append(configFieldPrefix()).append(".ciphersuites=").append(VespaSslContextProvider.enabledTlsCiphersConfigValue()).append("\n");
+            sb.append(configFieldPrefix()).append(".enabledProtocols=").append(VespaSslContextProvider.enabledTlsProtocolConfigValue()).append("\n");
+            sb.append(configFieldPrefix()).append(".protocol=").append(VespaSslContextProvider.sslContextVersion()).append("\n");
             return sb.toString();
         }
 
@@ -226,11 +213,9 @@ public class Configurator {
 
     static class TlsClientServerConfig implements TlsConfig {
 
-        private final SSLContext sslContext;
         private final Path jksKeyStoreFilePath;
 
-        TlsClientServerConfig(SSLContext sslContext, Path jksKeyStoreFilePath) {
-            this.sslContext = sslContext;
+        TlsClientServerConfig(Path jksKeyStoreFilePath) {
             this.jksKeyStoreFilePath = jksKeyStoreFilePath;
         }
 
@@ -269,19 +254,13 @@ public class Configurator {
             return jksKeyStoreFilePath;
         }
 
-        @Override
-        public SSLContext sslContext() {
-            return sslContext;
-        }
     }
 
     static class TlsQuorumConfig implements TlsConfig {
 
-        private final SSLContext sslContext;
         private final Path jksKeyStoreFilePath;
 
-        TlsQuorumConfig(SSLContext sslContext, Path jksKeyStoreFilePath) {
-            this.sslContext = sslContext;
+        TlsQuorumConfig(Path jksKeyStoreFilePath) {
             this.jksKeyStoreFilePath = jksKeyStoreFilePath;
         }
 
@@ -329,11 +308,6 @@ public class Configurator {
             return jksKeyStoreFilePath;
         }
 
-        @Override
-        public SSLContext sslContext() {
-            return sslContext;
-        }
-
     }
 
 }
diff --git a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/VespaSslContextProvider.java b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/VespaSslContextProvider.java
new file mode 100644
index 00000000000..dc254ade071
--- /dev/null
+++ b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/VespaSslContextProvider.java
@@ -0,0 +1,43 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.zookeeper;
+
+import com.yahoo.security.tls.TlsContext;
+import com.yahoo.security.tls.TransportSecurityUtils;
+
+import javax.net.ssl.SSLContext;
+import java.util.Collection;
+import java.util.List;
+import java.util.function.Supplier;
+import java.util.stream.Collectors;
+
+/**
+ * Provider for Vespa {@link SSLContext} instance to Zookeeper + misc utility methods for providing Vespa TLS specific ZK configuration.
+ *
+ * @author bjorncs
+ */
+public class VespaSslContextProvider implements Supplier<SSLContext> {
+
+    private static final TlsContext tlsContext = TransportSecurityUtils.getSystemTlsContext().orElse(null);
+
+    @Override
+    public SSLContext get() {
+        if (!tlsEnabled()) throw new IllegalStateException("Vespa TLS is not enabled");
+        return tlsContext.context();
+    }
+
+    public static boolean tlsEnabled() { return tlsContext != null; }
+
+    public static String enabledTlsProtocolConfigValue() {
+        // Fallback to all allowed protocols if we cannot determine which are actually supported by runtime
+        Collection<String> enabledProtocols = tlsEnabled() ? List.of(tlsContext.parameters().getProtocols()) : TlsContext.ALLOWED_PROTOCOLS;
+        return enabledProtocols.stream().sorted().collect(Collectors.joining(","));
+    }
+
+    public static String enabledTlsCiphersConfigValue() {
+        // Fallback to all allowed ciphers if we cannot determine which are actually supported by runtime
+        Collection<String> enabledCiphers = tlsEnabled() ? List.of(tlsContext.parameters().getCipherSuites()) : TlsContext.ALLOWED_CIPHER_SUITES;
+        return enabledCiphers.stream().sorted().collect(Collectors.joining(","));
+    }
+
+    public static String sslContextVersion() { return tlsEnabled() ? tlsContext.context().getProtocol() : TlsContext.SSL_CONTEXT_VERSION; }
+}
diff --git a/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ConfiguratorTest.java b/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ConfiguratorTest.java
index 0f43fb45d9d..a7994531b93 100644
--- a/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ConfiguratorTest.java
+++ b/zookeeper-server/zookeeper-server-common/src/test/java/com/yahoo/vespa/zookeeper/ConfiguratorTest.java
@@ -218,7 +218,10 @@ public class ConfiguratorTest {
     private String commonTlsQuorumConfig() {
         return "ssl.quorum.hostnameVerification=false\n" +
                "ssl.quorum.clientAuth=NEED\n" +
-               "ssl.quorum.ciphersuites=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,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384\n" +
+               "ssl.quorum.ciphersuites=TLS_AES_128_GCM_SHA256,TLS_AES_256_GCM_SHA384,TLS_CHACHA20_POLY1305_SHA256," +
+                "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384," +
+                "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256," +
+                "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256\n" +
                "ssl.quorum.enabledProtocols=TLSv1.2\n" +
                "ssl.quorum.protocol=TLS\n";
     }
@@ -226,7 +229,10 @@ public class ConfiguratorTest {
     private String commonTlsClientServerConfig() {
         return "ssl.hostnameVerification=false\n" +
                "ssl.clientAuth=NEED\n" +
-               "ssl.ciphersuites=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,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384\n" +
+               "ssl.ciphersuites=TLS_AES_128_GCM_SHA256,TLS_AES_256_GCM_SHA384,TLS_CHACHA20_POLY1305_SHA256," +
+                "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384," +
+                "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256," +
+                "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256\n" +
                "ssl.enabledProtocols=TLSv1.2\n" +
                "ssl.protocol=TLS\n";
     }