Generate server ZK TLS config using Vespa mTLS config

Server ZK TLS config follows Vespa mTLS config and is no longer controlled by feature flag.

3 files changed, 67 insertions, 122 deletions

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 d662bab8463..f302798589c 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
@@ -3,7 +3,9 @@
 package com.yahoo.vespa.zookeeper;
 
 import com.yahoo.cloud.config.ZookeeperServerConfig;
+import com.yahoo.security.tls.MixedMode;
 import com.yahoo.security.tls.TlsContext;
+import com.yahoo.security.tls.TransportSecurityUtils;
 import com.yahoo.vespa.defaults.Defaults;
 
 import java.io.FileWriter;
@@ -40,11 +42,14 @@ public class Configurator {
         System.setProperty("zookeeper.authProvider.x509", "com.yahoo.vespa.zookeeper.VespaMtlsAuthenticationProvider");
     }
 
-    void writeConfigToDisk(Optional<TlsContext> tlsContext) {
+    void writeConfigToDisk() { writeConfigToDisk(VespaTlsConfig.fromSystem()); }
+
+    // override of Vespa TLS config for unit testing
+    void writeConfigToDisk(VespaTlsConfig vespaTlsConfig) {
         configFilePath.toFile().getParentFile().mkdirs();
 
         try {
-            writeZooKeeperConfigFile(zookeeperServerConfig, tlsContext);
+            writeZooKeeperConfigFile(zookeeperServerConfig, vespaTlsConfig);
             writeMyIdFile(zookeeperServerConfig);
         } catch (IOException e) {
             throw new RuntimeException("Error writing zookeeper config", e);
@@ -52,13 +57,13 @@ public class Configurator {
     }
 
     private void writeZooKeeperConfigFile(ZookeeperServerConfig config,
-                                          Optional<TlsContext> tlsContext) throws IOException {
+                                          VespaTlsConfig vespaTlsConfig) throws IOException {
         try (FileWriter writer = new FileWriter(configFilePath.toFile())) {
-            writer.write(transformConfigToString(config, tlsContext));
+            writer.write(transformConfigToString(config, vespaTlsConfig));
         }
     }
 
-    private String transformConfigToString(ZookeeperServerConfig config, Optional<TlsContext> tlsContext) {
+    private String transformConfigToString(ZookeeperServerConfig config, VespaTlsConfig vespaTlsConfig) {
         StringBuilder sb = new StringBuilder();
         sb.append("tickTime=").append(config.tickTime()).append("\n");
         sb.append("initLimit=").append(config.initLimit()).append("\n");
@@ -80,8 +85,8 @@ public class Configurator {
         sb.append("skipACL=yes").append("\n");
         ensureThisServerIsRepresented(config.myid(), config.server());
         config.server().forEach(server -> addServerToCfg(sb, server, config.clientPort()));
-        sb.append(new TlsQuorumConfig().createConfig(config, tlsContext));
-        sb.append(new TlsClientServerConfig().createConfig(config, tlsContext));
+        sb.append(new TlsQuorumConfig().createConfig(vespaTlsConfig));
+        sb.append(new TlsClientServerConfig().createConfig(vespaTlsConfig));
         return sb.toString();
     }
 
@@ -143,22 +148,10 @@ public class Configurator {
     }
 
     private interface TlsConfig {
-        String createConfig(ZookeeperServerConfig config, Optional<TlsContext> tlsContext);
-
-        default Optional<String> getEnvironmentVariable(String variableName) {
-            return Optional.ofNullable(System.getenv().get(variableName))
-                    .filter(var -> !var.isEmpty());
-        }
-
-        default void validateOptions(Optional<TlsContext> tlsContext, String tlsSetting) {
-            if (tlsContext.isEmpty() && !tlsSetting.equals("OFF"))
-                throw new RuntimeException("Could not retrieve transport security options");
-        }
-
         String configFieldPrefix();
 
-        default void appendSharedTlsConfig(StringBuilder builder, Optional<TlsContext> tlsContext) {
-            tlsContext.ifPresent(ctx -> {
+        default void appendSharedTlsConfig(StringBuilder builder, VespaTlsConfig vespaTlsConfig) {
+            vespaTlsConfig.context().ifPresent(ctx -> {
                 builder.append(configFieldPrefix()).append(".context.supplier.class=").append(VespaSslContextProvider.class.getName()).append("\n");
                 String enabledCiphers = Arrays.stream(ctx.parameters().getCipherSuites()).sorted().collect(Collectors.joining(","));
                 builder.append(configFieldPrefix()).append(".ciphersuites=").append(enabledCiphers).append("\n");
@@ -167,39 +160,23 @@ public class Configurator {
                 builder.append(configFieldPrefix()).append(".clientAuth=NEED\n");
             });
         }
+
+        default boolean enablePortUnification(VespaTlsConfig config) {
+            return config.tlsEnabled()
+                    && (config.mixedMode() == MixedMode.TLS_CLIENT_MIXED_SERVER || config.mixedMode() == MixedMode.PLAINTEXT_CLIENT_MIXED_SERVER);
+        }
     }
 
     static class TlsClientServerConfig implements TlsConfig {
 
-        @Override
-        public String createConfig(ZookeeperServerConfig config, Optional<TlsContext> tlsContext) {
-            String tlsSetting = getEnvironmentVariable("VESPA_TLS_FOR_ZOOKEEPER_CLIENT_SERVER_COMMUNICATION")
-                    .orElse(config.tlsForClientServerCommunication().name());
-            validateOptions(tlsContext, tlsSetting);
-
-            StringBuilder sb = new StringBuilder();
-            boolean portUnification;
-            boolean secureClientPort;
-            switch (tlsSetting) {
-                case "OFF":
-                    secureClientPort = false; portUnification = false;
-                    break;
-                case "TLS_ONLY":
-                    secureClientPort = true; portUnification = false;
-                    break;
-                case "PORT_UNIFICATION":
-                case "TLS_WITH_PORT_UNIFICATION":
-                    secureClientPort = false; portUnification = true;
-                    break;
-                default:
-                    throw new IllegalArgumentException("Unknown value of config setting tlsForClientServerCommunication: " + tlsSetting);
-            }
-            sb.append("client.portUnification=").append(portUnification).append("\n");
+        public String createConfig(VespaTlsConfig vespaTlsConfig) {
+            StringBuilder sb = new StringBuilder()
+                    .append("client.portUnification=").append(enablePortUnification(vespaTlsConfig)).append("\n");
             // ZooKeeper Dynamic Reconfiguration requires the "non-secure" client port to exist
             // This is a hack to override the secure parameter through our connection factory wrapper
             // https://issues.apache.org/jira/browse/ZOOKEEPER-3577
-            VespaNettyServerCnxnFactory_isSecure = secureClientPort;
-            appendSharedTlsConfig(sb, tlsContext);
+            VespaNettyServerCnxnFactory_isSecure = vespaTlsConfig.tlsEnabled() && vespaTlsConfig.mixedMode() == MixedMode.DISABLED;
+            appendSharedTlsConfig(sb, vespaTlsConfig);
 
             return sb.toString();
         }
@@ -212,38 +189,11 @@ public class Configurator {
 
     static class TlsQuorumConfig implements TlsConfig {
 
-        @Override
-        public String createConfig(ZookeeperServerConfig config, Optional<TlsContext> tlsContext) {
-            String tlsSetting = getEnvironmentVariable("VESPA_TLS_FOR_ZOOKEEPER_QUORUM_COMMUNICATION")
-                    .orElse(config.tlsForQuorumCommunication().name());
-            validateOptions(tlsContext, tlsSetting);
-
-            StringBuilder sb = new StringBuilder();
-            boolean sslQuorum;
-            boolean portUnification;
-            switch (tlsSetting) {
-                case "OFF":
-                    sslQuorum = false;
-                    portUnification = false;
-                    break;
-                case "PORT_UNIFICATION":
-                    sslQuorum = false;
-                    portUnification = true;
-                    break;
-                case "TLS_WITH_PORT_UNIFICATION":
-                    sslQuorum = true;
-                    portUnification = true;
-                    break;
-                case "TLS_ONLY":
-                    sslQuorum = true;
-                    portUnification = false;
-                    break;
-                default: throw new IllegalArgumentException("Unknown value of config setting tlsForQuorumCommunication: " + tlsSetting);
-            }
-            sb.append("sslQuorum=").append(sslQuorum).append("\n");
-            sb.append("portUnification=").append(portUnification).append("\n");
-            appendSharedTlsConfig(sb, tlsContext);
-
+        public String createConfig(VespaTlsConfig vespaTlsConfig) {
+            StringBuilder sb = new StringBuilder()
+                    .append("sslQuorum=").append(vespaTlsConfig.tlsEnabled()).append("\n")
+                    .append("portUnification=").append(enablePortUnification(vespaTlsConfig)).append("\n");
+            appendSharedTlsConfig(sb, vespaTlsConfig);
             return sb.toString();
         }
 
@@ -253,4 +203,26 @@ public class Configurator {
         }
     }
 
+    static class VespaTlsConfig {
+        private final TlsContext context;
+        private final MixedMode mixedMode;
+
+        VespaTlsConfig(TlsContext context, MixedMode mixedMode) {
+            this.context = context;
+            this.mixedMode = mixedMode;
+        }
+
+        static VespaTlsConfig fromSystem() {
+            return new VespaTlsConfig(
+                    TransportSecurityUtils.getSystemTlsContext().orElse(null),
+                    TransportSecurityUtils.getInsecureMixedMode());
+        }
+
+        static VespaTlsConfig tlsDisabled() { return new VespaTlsConfig(null, MixedMode.defaultValue()); }
+
+        boolean tlsEnabled() { return context != null; }
+        Optional<TlsContext> context() { return Optional.ofNullable(context); }
+        MixedMode mixedMode() { return mixedMode; }
+    }
+
 }
diff --git a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperRunner.java b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperRunner.java
index adbc7a369b3..8c748250503 100644
--- a/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperRunner.java
+++ b/zookeeper-server/zookeeper-server-common/src/main/java/com/yahoo/vespa/zookeeper/ZooKeeperRunner.java
@@ -4,7 +4,6 @@ package com.yahoo.vespa.zookeeper;
 import com.yahoo.cloud.config.ZookeeperServerConfig;
 import com.yahoo.concurrent.DaemonThreadFactory;
 import com.yahoo.protect.Process;
-import com.yahoo.security.tls.TransportSecurityUtils;
 
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -39,7 +38,7 @@ public class ZooKeeperRunner implements Runnable {
     public ZooKeeperRunner(ZookeeperServerConfig zookeeperServerConfig, VespaZooKeeperServer server) {
         this.zookeeperServerConfig = zookeeperServerConfig;
         this.server = server;
-        new Configurator(zookeeperServerConfig).writeConfigToDisk(TransportSecurityUtils.getSystemTlsContext());
+        new Configurator(zookeeperServerConfig).writeConfigToDisk();
         executorService = Executors.newSingleThreadExecutor(new DaemonThreadFactory("zookeeper server"));
         executorService.submit(this);
     }
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 47fed6fceac..c40b7cb7b52 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
@@ -7,6 +7,7 @@ import com.yahoo.security.X509CertificateBuilder;
 import com.yahoo.security.tls.AuthorizationMode;
 import com.yahoo.security.tls.DefaultTlsContext;
 import com.yahoo.security.tls.HostnameVerification;
+import com.yahoo.security.tls.MixedMode;
 import com.yahoo.security.tls.PeerAuthentication;
 import com.yahoo.security.tls.TlsContext;
 import com.yahoo.security.tls.policy.AuthorizedPeers;
@@ -24,14 +25,12 @@ import java.nio.file.Files;
 import java.security.KeyPair;
 import java.security.cert.X509Certificate;
 import java.util.List;
-import java.util.Optional;
 import java.util.Set;
 
-import static com.yahoo.cloud.config.ZookeeperServerConfig.TlsForClientServerCommunication;
-import static com.yahoo.cloud.config.ZookeeperServerConfig.TlsForQuorumCommunication;
 import static com.yahoo.security.KeyAlgorithm.EC;
 import static com.yahoo.security.SignatureAlgorithm.SHA256_WITH_ECDSA;
 import static com.yahoo.vespa.defaults.Defaults.getDefaults;
+import static com.yahoo.vespa.zookeeper.Configurator.VespaTlsConfig;
 import static com.yahoo.vespa.zookeeper.Configurator.ZOOKEEPER_JUTE_MAX_BUFFER;
 import static java.time.Instant.EPOCH;
 import static java.time.temporal.ChronoUnit.DAYS;
@@ -57,7 +56,7 @@ public class ConfiguratorTest {
     @Test
     public void config_is_written_correctly_when_one_server() {
         ZookeeperServerConfig.Builder builder = createConfigBuilderForSingleHost(cfgFile, idFile);
-        new Configurator(builder.build()).writeConfigToDisk(Optional.empty());
+        new Configurator(builder.build()).writeConfigToDisk(VespaTlsConfig.tlsDisabled());
         validateConfigFileSingleHost(cfgFile);
         validateIdFile(idFile, "0\n");
     }
@@ -71,39 +70,25 @@ public class ConfiguratorTest {
         builder.server(newServer(2, "baz", 345, 543, true));
         builder.myidFile(idFile.getAbsolutePath());
         builder.myid(1);
-        new Configurator(builder.build()).writeConfigToDisk(Optional.empty());
+        new Configurator(builder.build()).writeConfigToDisk(VespaTlsConfig.tlsDisabled());
         validateConfigFileMultipleHosts(cfgFile);
         validateIdFile(idFile, "1\n");
     }
 
     @Test
-    public void config_is_written_correctly_with_tls_for_quorum_communication_port_unification() {
+    public void config_is_written_correctly_with_tls_for_quorum_communication_tls_with_mixed_mode() {
         ZookeeperServerConfig.Builder builder = createConfigBuilderForSingleHost(cfgFile, idFile);
-        builder.tlsForQuorumCommunication(TlsForQuorumCommunication.PORT_UNIFICATION);
-        builder.tlsForClientServerCommunication(TlsForClientServerCommunication.PORT_UNIFICATION);
         TlsContext tlsContext = createTlsContext();
-        new Configurator(builder.build()).writeConfigToDisk(Optional.of(tlsContext));
-        validateConfigFilePortUnification(cfgFile);
+        new Configurator(builder.build()).writeConfigToDisk(new VespaTlsConfig(tlsContext, MixedMode.TLS_CLIENT_MIXED_SERVER));
+        validateConfigFileTlsWithMixedMode(cfgFile);
     }
 
     @Test
-    public void config_is_written_correctly_with_tls_for_quorum_communication_tls_with_port_unification() {
+    public void config_is_written_correctly_with_tls_for_quorum_communication_tls_without_mixed_mode() {
         ZookeeperServerConfig.Builder builder = createConfigBuilderForSingleHost(cfgFile, idFile);
-        builder.tlsForQuorumCommunication(TlsForQuorumCommunication.TLS_WITH_PORT_UNIFICATION);
-        builder.tlsForClientServerCommunication(TlsForClientServerCommunication.TLS_WITH_PORT_UNIFICATION);
         TlsContext tlsContext = createTlsContext();
-        new Configurator(builder.build()).writeConfigToDisk(Optional.of(tlsContext));
-        validateConfigFileTlsWithPortUnification(cfgFile);
-    }
-
-    @Test
-    public void config_is_written_correctly_with_tls_for_quorum_communication_tls_only() {
-        ZookeeperServerConfig.Builder builder = createConfigBuilderForSingleHost(cfgFile, idFile);
-        builder.tlsForQuorumCommunication(TlsForQuorumCommunication.TLS_ONLY);
-        builder.tlsForClientServerCommunication(TlsForClientServerCommunication.TLS_ONLY);
-        TlsContext tlsContext = createTlsContext();
-        new Configurator(builder.build()).writeConfigToDisk(Optional.of(tlsContext));
-        validateConfigFileTlsOnly(cfgFile);
+        new Configurator(builder.build()).writeConfigToDisk(new VespaTlsConfig(tlsContext, MixedMode.DISABLED));
+        validateConfigFileTlsWithoutMixedMode(cfgFile);
     }
 
     @Test(expected = RuntimeException.class)
@@ -113,7 +98,7 @@ public class ConfiguratorTest {
         builder.server(newServer(1, "bar", 234, 432, false));
         builder.server(newServer(2, "baz", 345, 543, false));
         builder.myid(0);
-        new Configurator(builder.build()).writeConfigToDisk(Optional.empty());
+        new Configurator(builder.build()).writeConfigToDisk(VespaTlsConfig.tlsDisabled());
     }
 
     @Test
@@ -127,12 +112,12 @@ public class ConfiguratorTest {
         builder.zooKeeperConfigFile(cfgFile.getAbsolutePath());
         builder.myidFile(idFile.getAbsolutePath());
 
-        new Configurator(builder.build()).writeConfigToDisk(Optional.empty());
+        new Configurator(builder.build()).writeConfigToDisk(VespaTlsConfig.tlsDisabled());
         assertEquals("" + new ZookeeperServerConfig(builder).juteMaxBuffer(), System.getProperty(ZOOKEEPER_JUTE_MAX_BUFFER));
 
         final int max_buffer = 1;
         builder.juteMaxBuffer(max_buffer);
-        new Configurator(builder.build()).writeConfigToDisk(Optional.empty());
+        new Configurator(builder.build()).writeConfigToDisk(VespaTlsConfig.tlsDisabled());
         assertEquals("" + max_buffer, System.getProperty(ZOOKEEPER_JUTE_MAX_BUFFER));
     }
 
@@ -216,19 +201,8 @@ public class ConfiguratorTest {
         validateConfigFile(cfgFile, expected);
     }
 
-    private void validateConfigFilePortUnification(File cfgFile) {
-        String expected =
-                commonConfig() +
-                "server.0=foo:321:123;2181\n" +
-                "sslQuorum=false\n" +
-                "portUnification=true\n" +
-                tlsQuorumConfig() +
-                "client.portUnification=true\n" +
-                tlsClientServerConfig();
-        validateConfigFile(cfgFile, expected);
-    }
 
-    private void validateConfigFileTlsWithPortUnification(File cfgFile) {
+    private void validateConfigFileTlsWithMixedMode(File cfgFile) {
         String expected =
                 commonConfig() +
                 "server.0=foo:321:123;2181\n" +
@@ -240,7 +214,7 @@ public class ConfiguratorTest {
         validateConfigFile(cfgFile, expected);
     }
 
-    private void validateConfigFileTlsOnly(File cfgFile) {
+    private void validateConfigFileTlsWithoutMixedMode(File cfgFile) {
         String expected =
                 commonConfig() +
                 "server.0=foo:321:123;2181\n" +