From e3f754b6bac836d45374a5a57e426aa9bfc7bead Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Wed, 17 Mar 2021 18:02:19 +0100 Subject: 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. --- .../com/yahoo/vespa/zookeeper/Configurator.java | 130 ++++++++------------- .../com/yahoo/vespa/zookeeper/ZooKeeperRunner.java | 3 +- .../yahoo/vespa/zookeeper/ConfiguratorTest.java | 56 +++------ 3 files changed, 67 insertions(+), 122 deletions(-) (limited to 'zookeeper-server/zookeeper-server-common') 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) { + 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) 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) { + 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); - - default Optional getEnvironmentVariable(String variableName) { - return Optional.ofNullable(System.getenv().get(variableName)) - .filter(var -> !var.isEmpty()); - } - - default void validateOptions(Optional 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.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) { - 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) { - 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 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" + -- cgit v1.2.3