diff options
author | Bjørn Christian Seime <bjorncs@verizonmedia.com> | 2021-03-15 16:59:54 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-03-15 16:59:54 +0100 |
commit | 5507bdcfb6eb0cdf06d04d0dd317302e4cb4362d (patch) | |
tree | c298234c7bab79f9ba48fd691fa250b2cef73c05 | |
parent | ac6df3096625d646982883ce4968cf125a1f83f6 (diff) | |
parent | 6d110dc030fa648a2f99efbe7c2ac2ed0a87ba59 (diff) |
Merge pull request #16962 from vespa-engine/bjorncs/zk-server-client-port-tls-only
Bjorncs/zk server client port tls only [run-systemtest]
3 files changed, 41 insertions, 24 deletions
diff --git a/configdefinitions/src/vespa/zookeeper-server.def b/configdefinitions/src/vespa/zookeeper-server.def index 536cd993105..006e266916c 100644 --- a/configdefinitions/src/vespa/zookeeper-server.def +++ b/configdefinitions/src/vespa/zookeeper-server.def @@ -16,6 +16,7 @@ maxClientConnections int default=0 dataDir string default="var/zookeeper" clientPort int default=2181 +# TODO(bjorncs): remove setting - no longer in use secureClientPort int default=2184 snapshotCount int default=50000 @@ -42,7 +43,9 @@ server[].joining bool default=false trustEmptySnapshot bool default=true # TLS options +# TODO(bjorncs): todo cleanup after migrating to unified Vespa TLS configuration tlsForQuorumCommunication enum { OFF, PORT_UNIFICATION, TLS_WITH_PORT_UNIFICATION, TLS_ONLY } default=OFF +# TODO(bjorncs): todo cleanup after migrating to unified Vespa TLS configuration tlsForClientServerCommunication enum { OFF, PORT_UNIFICATION, TLS_WITH_PORT_UNIFICATION, TLS_ONLY } default=OFF # TODO(bjorncs): remove setting - no longer in use jksKeyStoreFile string default="conf/zookeeper/zookeeper.jks" 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..9f2144966e0 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<String> zookeeperServerHostnames(ZookeeperServerConfig zookeeperServerConfig) { @@ -154,7 +152,7 @@ public class Configurator { String configFieldPrefix(); - default void appendTlsConfig(StringBuilder builder, ZookeeperServerConfig config, Optional<TlsContext> tlsContext) { + default void appendTlsConfig(StringBuilder builder, Optional<TlsContext> tlsContext) { tlsContext.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(",")); @@ -176,22 +174,29 @@ 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")); - appendTlsConfig(sb, config, tlsContext); + // 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, tlsContext); return sb.toString(); } @@ -234,7 +239,7 @@ public class Configurator { } sb.append("sslQuorum=").append(sslQuorum).append("\n"); sb.append("portUnification=").append(portUnification).append("\n"); - appendTlsConfig(sb, config, tlsContext); + appendTlsConfig(sb, 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); } |