From ee5edd1f90489ec85ae1b23bbdb038a830825aa6 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Mon, 15 Mar 2021 15:35:07 +0100 Subject: Handle client port that is configured to TLS only The client port can no longer be distributed through the ZK dynamic reconfiguration as the protocol does not support SSL client port. The port must be configured through the static config section instead. --- .../com/yahoo/vespa/zookeeper/Configurator.java | 25 ++++++++++------- .../yahoo/vespa/zookeeper/ConfiguratorTest.java | 31 ++++++++++++++-------- 2 files changed, 35 insertions(+), 21 deletions(-) (limited to 'zookeeper-server') 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 aff4bb950f6..3c56741adfb 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 @@ -76,7 +76,7 @@ public class Configurator { sb.append("skipACL=yes").append("\n"); 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())); + config.server().forEach(server -> addServerToCfg(sb, server)); sb.append(new TlsQuorumConfig().createConfig(config, tlsContext)); sb.append(new TlsClientServerConfig().createConfig(config, tlsContext)); return sb.toString(); @@ -101,7 +101,7 @@ public class Configurator { } } - private void addServerToCfg(StringBuilder sb, ZookeeperServerConfig.Server server, int clientPort) { + private void addServerToCfg(StringBuilder sb, ZookeeperServerConfig.Server server) { sb.append("server.") .append(server.id()) .append("=") @@ -119,9 +119,7 @@ public class Configurator { sb.append(":") .append("observer"); } - sb.append(";") - .append(clientPort) - .append("\n"); + sb.append("\n"); } static List zookeeperServerHostnames(ZookeeperServerConfig zookeeperServerConfig) { @@ -176,21 +174,28 @@ public class Configurator { StringBuilder sb = new StringBuilder(); boolean portUnification; + boolean secureClientPort; switch (tlsSetting) { case "OFF": + secureClientPort = false; portUnification = false; + break; case "TLS_ONLY": - portUnification = false; + secureClientPort = true; portUnification = false; break; case "PORT_UNIFICATION": case "TLS_WITH_PORT_UNIFICATION": - portUnification = true; + secureClientPort = false; portUnification = true; break; default: throw new IllegalArgumentException("Unknown value of config setting tlsForClientServerCommunication: " + tlsSetting); } - sb.append("client.portUnification=").append(portUnification).append("\n"); - // TODO This should override "clientPort" if TLS enabled without port unification); - tlsContext.ifPresent(ctx -> sb.append("secureClientPort=").append(config.secureClientPort()).append("\n")); + // ZooKeeper Dynamic Reconfiguration does not support SSL/secure client port + // The secure client port must be configured in the static configuration section instead + // https://issues.apache.org/jira/browse/ZOOKEEPER-3577 + sb.append("client.portUnification=").append(portUnification).append("\n") + .append("clientPort=").append(secureClientPort ? 0 : config.clientPort()).append("\n") + .append("secureClientPort=").append(secureClientPort ? config.clientPort() : 0).append("\n"); + appendTlsConfig(sb, config, tlsContext); return sb.toString(); 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 aee44c90cbf..147b61a804c 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 @@ -182,10 +182,12 @@ public class ConfiguratorTest { private void validateConfigFileSingleHost(File cfgFile) { String expected = commonConfig() + - "server.0=foo:321:123;2181\n" + + "server.0=foo:321:123\n" + "sslQuorum=false\n" + "portUnification=false\n" + - "client.portUnification=false\n"; + "client.portUnification=false\n" + + "clientPort=2181\n" + + "secureClientPort=0\n"; validateConfigFile(cfgFile, expected); } @@ -198,8 +200,7 @@ public class ConfiguratorTest { } private String tlsClientServerConfig() { - return "secureClientPort=2184\n" + - "ssl.context.supplier.class=com.yahoo.vespa.zookeeper.VespaSslContextProvider\n" + + return "ssl.context.supplier.class=com.yahoo.vespa.zookeeper.VespaSslContextProvider\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.enabledProtocols=TLSv1.2\n" + @@ -209,23 +210,27 @@ public class ConfiguratorTest { private void validateConfigFileMultipleHosts(File cfgFile) { String expected = commonConfig() + - "server.0=foo:321:123;2181\n" + - "server.1=bar:432:234;2181\n" + - "server.2=baz:543:345:observer;2181\n" + + "server.0=foo:321:123\n" + + "server.1=bar:432:234\n" + + "server.2=baz:543:345:observer\n" + "sslQuorum=false\n" + "portUnification=false\n" + - "client.portUnification=false\n"; + "client.portUnification=false\n" + + "clientPort=2181\n" + + "secureClientPort=0\n"; validateConfigFile(cfgFile, expected); } private void validateConfigFilePortUnification(File cfgFile) { String expected = commonConfig() + - "server.0=foo:321:123;2181\n" + + "server.0=foo:321:123\n" + "sslQuorum=false\n" + "portUnification=true\n" + tlsQuorumConfig() + "client.portUnification=true\n" + + "clientPort=2181\n" + + "secureClientPort=0\n" + tlsClientServerConfig(); validateConfigFile(cfgFile, expected); } @@ -233,11 +238,13 @@ public class ConfiguratorTest { private void validateConfigFileTlsWithPortUnification(File cfgFile) { String expected = commonConfig() + - "server.0=foo:321:123;2181\n" + + "server.0=foo:321:123\n" + "sslQuorum=true\n" + "portUnification=true\n" + tlsQuorumConfig() + "client.portUnification=true\n" + + "clientPort=2181\n" + + "secureClientPort=0\n" + tlsClientServerConfig(); validateConfigFile(cfgFile, expected); } @@ -245,11 +252,13 @@ public class ConfiguratorTest { private void validateConfigFileTlsOnly(File cfgFile) { String expected = commonConfig() + - "server.0=foo:321:123;2181\n" + + "server.0=foo:321:123\n" + "sslQuorum=true\n" + "portUnification=false\n" + tlsQuorumConfig() + "client.portUnification=false\n" + + "clientPort=0\n" + + "secureClientPort=2181\n" + tlsClientServerConfig(); validateConfigFile(cfgFile, expected); } -- cgit v1.2.3