Look up TLS context directly in X509ClientUtil, which simplifies a lot!

5 files changed, 87 insertions, 102 deletions

diff --git a/zookeeper-server/zookeeper-server-3.8.0/src/main/java/com/yahoo/vespa/zookeeper/VespaMtlsAuthenticationProvider.java b/zookeeper-server/zookeeper-server-3.8.0/src/main/java/com/yahoo/vespa/zookeeper/VespaMtlsAuthenticationProvider.java
index 68f7459530e..90554910293 100644
--- a/zookeeper-server/zookeeper-server-3.8.0/src/main/java/com/yahoo/vespa/zookeeper/VespaMtlsAuthenticationProvider.java
+++ b/zookeeper-server/zookeeper-server-3.8.0/src/main/java/com/yahoo/vespa/zookeeper/VespaMtlsAuthenticationProvider.java
@@ -2,19 +2,24 @@
 package com.yahoo.vespa.zookeeper;
 
 import com.yahoo.security.X509SslContext;
+import com.yahoo.security.tls.TlsContext;
+import com.yahoo.security.tls.TransportSecurityUtils;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.common.ClientX509Util;
+import org.apache.zookeeper.common.X509Exception;
 import org.apache.zookeeper.data.Id;
 import org.apache.zookeeper.server.ServerCnxn;
 import org.apache.zookeeper.server.auth.AuthenticationProvider;
 import org.apache.zookeeper.server.auth.X509AuthenticationProvider;
 
+import javax.net.ssl.KeyManager;
 import javax.net.ssl.X509KeyManager;
 import javax.net.ssl.X509TrustManager;
 import java.security.cert.X509Certificate;
 import java.util.logging.Logger;
 
 /**
- * A {@link AuthenticationProvider} to be used in combination with Vespa mTLS
+ * A {@link AuthenticationProvider} to be used in combination with Vespa mTLS.
  *
  * @author bjorncs
  */
@@ -23,15 +28,7 @@ public class VespaMtlsAuthenticationProvider extends X509AuthenticationProvider
     private static final Logger log = Logger.getLogger(VespaMtlsAuthenticationProvider.class.getName());
 
     public VespaMtlsAuthenticationProvider() {
-        super(trustManager(), keyManager());
-    }
-
-    private static X509KeyManager keyManager() {
-        return new VespaSslContextProvider().tlsContext().map(X509SslContext::keyManager).orElse(null);
-    }
-
-    private static X509TrustManager trustManager() {
-        return new VespaSslContextProvider().tlsContext().map(X509SslContext::trustManager).orElse(null);
+        super(null, null);
     }
 
     @Override
diff --git a/zookeeper-server/zookeeper-server-common/pom.xml b/zookeeper-server/zookeeper-server-common/pom.xml
index 86734ec6c56..2238f6ad086 100644
--- a/zookeeper-server/zookeeper-server-common/pom.xml
+++ b/zookeeper-server/zookeeper-server-common/pom.xml
@@ -13,6 +13,12 @@
   <version>8-SNAPSHOT</version>
   <dependencies>
     <dependency>
+      <groupId>com.yahoo.vespa</groupId>
+      <artifactId>zookeeper-common</artifactId>
+      <version>${project.version}</version>
+      <scope>compile</scope>
+    </dependency>
+    <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>
       <scope>test</scope>
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 727e369885e..ca18e7ef146 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,6 @@ package com.yahoo.vespa.zookeeper;
 
 import com.yahoo.cloud.config.ZookeeperServerConfig;
 import com.yahoo.cloud.config.ZookeeperServerConfig.Server;
-import com.yahoo.security.tls.ConfigFileBasedTlsContext;
 import com.yahoo.security.tls.MixedMode;
 import com.yahoo.security.tls.TlsContext;
 import com.yahoo.security.tls.TransportSecurityUtils;
@@ -47,9 +46,8 @@ public class Configurator {
         // Doc says that it is max size of data in a zookeeper node, but it goes for everything that
         // needs to be serialized, see https://issues.apache.org/jira/browse/ZOOKEEPER-1162 for details
         System.setProperty(ZOOKEEPER_JUTE_MAX_BUFFER, Integer.valueOf(zookeeperServerConfig.juteMaxBuffer()).toString());
-        // Need to set these as a system properties instead of config, config does not work
+        // Need to set this as a system properties instead of config, config does not work
         System.setProperty("zookeeper.authProvider.x509", "com.yahoo.vespa.zookeeper.VespaMtlsAuthenticationProvider");
-        System.setProperty("zookeeper.ssl.authProvider", "x509");
         // Need to set this as a system property, otherwise it will be parsed for _every_ packet and an exception will be thrown (and handled)
         System.setProperty("zookeeper.globalOutstandingLimit", "1000");
         System.setProperty("zookeeper.snapshot.compression.method", zookeeperServerConfig.snapshotMethod());
@@ -60,13 +58,9 @@ public class Configurator {
     }
 
     void writeConfigToDisk() {
-        VespaTlsConfig config;
-        String cfgFile = zookeeperServerConfig.vespaTlsConfigFile();
-        if (cfgFile.isBlank()) {
-            config = VespaTlsConfig.fromSystem();
-        } else {
-            config = VespaTlsConfig.fromConfig(Paths.get(cfgFile));
-        }
+        VespaTlsConfig config = VespaZookeeperTlsContextUtils.tlsContext()
+                                                             .map(ctx -> new VespaTlsConfig(ctx, TransportSecurityUtils.getInsecureMixedMode()))
+                                                             .orElse(VespaTlsConfig.tlsDisabled());
         writeConfigToDisk(config);
     }
 
@@ -90,7 +84,7 @@ public class Configurator {
         }
     }
 
-    private String transformConfigToString(ZookeeperServerConfig config, VespaTlsConfig vespaTlsConfig, Map<String, String> dynamicConfig) {
+    private static String transformConfigToString(ZookeeperServerConfig config, VespaTlsConfig vespaTlsConfig, Map<String, String> dynamicConfig) {
         Map<String, String> configEntries = new LinkedHashMap<>();
         configEntries.put("tickTime", Integer.toString(config.tickTime()));
         configEntries.put("initLimit", Integer.toString(config.initLimit()));
@@ -118,7 +112,7 @@ public class Configurator {
         return transformConfigToString(configEntries);
     }
 
-    void addServerSpecs(Map<String, String> configEntries, ZookeeperServerConfig config, Map<String, String> dynamicConfig) {
+    static void addServerSpecs(Map<String, String> configEntries, ZookeeperServerConfig config, Map<String, String> dynamicConfig) {
         int myIndex = ensureThisServerIsRepresented(config.myid(), config.server());
 
         // If dynamic config refers to servers that are not in the current config, we must ignore it.
@@ -210,7 +204,7 @@ public class Configurator {
                                     .toList();
     }
 
-    Path makeAbsolutePath(String filename) {
+    static Path makeAbsolutePath(String filename) {
         Path path = Paths.get(filename);
         return path.isAbsolute() ? path : Paths.get(getDefaults().underVespaHome(filename));
     }
@@ -220,8 +214,6 @@ public class Configurator {
 
         default void appendSharedTlsConfig(Map<String, String> configEntries, VespaTlsConfig vespaTlsConfig) {
             vespaTlsConfig.context().ifPresent(ctx -> {
-                VespaSslContextProvider.set(ctx);
-                configEntries.put(configFieldPrefix() + ".context.supplier.class", VespaSslContextProvider.class.getName());
                 String enabledCiphers = Arrays.stream(ctx.parameters().getCipherSuites()).sorted().collect(Collectors.joining(","));
                 configEntries.put(configFieldPrefix() + ".ciphersuites", enabledCiphers);
                 String enabledProtocols = Arrays.stream(ctx.parameters().getProtocols()).sorted().collect(Collectors.joining(","));
@@ -276,19 +268,6 @@ public class Configurator {
             this.mixedMode = mixedMode;
         }
 
-        static VespaTlsConfig fromSystem() {
-            return new VespaTlsConfig(
-                    TransportSecurityUtils.getSystemTlsContext().orElse(null),
-                    TransportSecurityUtils.getInsecureMixedMode());
-        }
-
-        static VespaTlsConfig fromConfig(Path file) {
-            return new VespaTlsConfig(
-                    new ConfigFileBasedTlsContext(file, TransportSecurityUtils.getInsecureAuthorizationMode()),
-                    TransportSecurityUtils.getInsecureMixedMode());
-        }
-
-
         static VespaTlsConfig tlsDisabled() { return new VespaTlsConfig(null, MixedMode.defaultValue()); }
 
         boolean tlsEnabled() { return context != null; }
diff --git a/zookeeper-server/zookeeper-server/src/main/java/com/yahoo/vespa/zookeeper/VespaMtlsAuthenticationProvider.java b/zookeeper-server/zookeeper-server/src/main/java/com/yahoo/vespa/zookeeper/VespaMtlsAuthenticationProvider.java
index 100de4894ae..90554910293 100644
--- a/zookeeper-server/zookeeper-server/src/main/java/com/yahoo/vespa/zookeeper/VespaMtlsAuthenticationProvider.java
+++ b/zookeeper-server/zookeeper-server/src/main/java/com/yahoo/vespa/zookeeper/VespaMtlsAuthenticationProvider.java
@@ -2,7 +2,10 @@
 package com.yahoo.vespa.zookeeper;
 
 import com.yahoo.security.X509SslContext;
+import com.yahoo.security.tls.TlsContext;
+import com.yahoo.security.tls.TransportSecurityUtils;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.common.ClientX509Util;
 import org.apache.zookeeper.common.X509Exception;
 import org.apache.zookeeper.data.Id;
 import org.apache.zookeeper.server.ServerCnxn;
@@ -16,7 +19,7 @@ import java.security.cert.X509Certificate;
 import java.util.logging.Logger;
 
 /**
- * A {@link AuthenticationProvider} to be used in combination with Vespa mTLS
+ * A {@link AuthenticationProvider} to be used in combination with Vespa mTLS.
  *
  * @author bjorncs
  */
@@ -25,15 +28,7 @@ public class VespaMtlsAuthenticationProvider extends X509AuthenticationProvider
     private static final Logger log = Logger.getLogger(VespaMtlsAuthenticationProvider.class.getName());
 
     public VespaMtlsAuthenticationProvider() {
-        super(trustManager(), keyManager());
-    }
-
-    private static X509KeyManager keyManager() {
-        return new VespaSslContextProvider().tlsContext().map(X509SslContext::keyManager).orElse(null);
-    }
-
-    private static X509TrustManager trustManager() {
-        return new VespaSslContextProvider().tlsContext().map(X509SslContext::trustManager).orElse(null);
+        super(null, null);
     }
 
     @Override
diff --git a/zookeeper-server/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java b/zookeeper-server/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java
index c0034a4723f..9eda60ea361 100644
--- a/zookeeper-server/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java
+++ b/zookeeper-server/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java
@@ -18,6 +18,7 @@
 
 package org.apache.zookeeper.common;
 
+import com.yahoo.vespa.zookeeper.VespaZookeeperTlsContextUtils;
 import io.netty.handler.ssl.DelegatingSslContext;
 import io.netty.handler.ssl.SslContext;
 import io.netty.handler.ssl.SslContextBuilder;
@@ -28,21 +29,16 @@ import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLException;
 import javax.net.ssl.SSLParameters;
 import javax.net.ssl.TrustManager;
-
-import org.apache.zookeeper.common.X509Exception.KeyManagerException;
-import org.apache.zookeeper.common.X509Exception.SSLContextException;
-import org.apache.zookeeper.server.auth.ProviderRegistry;
-import org.apache.zookeeper.server.auth.X509AuthenticationProvider;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- *
- * <em>NOTE: Overridden because ZK 3.9 completely broke the SSL setup APIs; for clients, key and trust stores are
- * now mandatory, unlike for servers, where it's still possible to provide a custom authProvider. This patch fixes that.
- * Based on https://github.com/apache/zookeeper/blob/branch-3.9/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java</em>
- * <p>
  * X509 utilities specific for client-server communication framework.
+ * <p>
+ * <em>Modified to use Vespa's TLS context, whenever it is available, instead of the file-based key and trust stores of ZK 3.9.
+ * Based on https://github.com/apache/zookeeper/blob/branch-3.9/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java</em>
+ *
+ * @author jonmv
  */
 public class ClientX509Util extends X509Util {
 
@@ -70,37 +66,31 @@ public class ClientX509Util extends X509Util {
     }
 
     public SslContext createNettySslContextForClient(ZKConfig config)
-        throws X509Exception.KeyManagerException, X509Exception.TrustManagerException, SSLException {
-
+            throws X509Exception.KeyManagerException, X509Exception.TrustManagerException, SSLException {
+        SslContextBuilder sslContextBuilder = SslContextBuilder.forClient();
         KeyManager km;
         TrustManager tm;
-        String authProviderProp = System.getProperty(getSslAuthProviderProperty());
-        if (authProviderProp == null) {
+        if (VespaZookeeperTlsContextUtils.tlsContext().isPresent()) {
+            km = VespaZookeeperTlsContextUtils.tlsContext().get().sslContext().keyManager();
+            tm = VespaZookeeperTlsContextUtils.tlsContext().get().sslContext().trustManager();
+        }
+        else {
             String keyStoreLocation = config.getProperty(getSslKeystoreLocationProperty(), "");
             String keyStorePassword = getPasswordFromConfigPropertyOrFile(config, getSslKeystorePasswdProperty(),
                                                                           getSslKeystorePasswdPathProperty());
             String keyStoreType = config.getProperty(getSslKeystoreTypeProperty());
+
             if (keyStoreLocation.isEmpty()) {
                 LOG.warn("{} not specified", getSslKeystoreLocationProperty());
                 km = null;
-            } else {
+            }
+            else {
                 km = createKeyManager(keyStoreLocation, keyStorePassword, keyStoreType);
             }
-            tm = getTrustManager(config);
-        } else {
-            X509AuthenticationProvider authProvider = (X509AuthenticationProvider) ProviderRegistry.getProvider(
-                    System.getProperty(getSslAuthProviderProperty(), "x509"));
 
-            if (authProvider == null) {
-                LOG.error("Auth provider not found: {}", authProviderProp);
-                throw new SSLException("Could not create SSLContext with specified auth provider: " + authProviderProp);
-            }
-            LOG.info("Using auth provider for client: {}", authProviderProp);
-            km = authProvider.getKeyManager();
-            tm = authProvider.getTrustManager();
+            tm = getTrustManager(config);
         }
 
-        SslContextBuilder sslContextBuilder = SslContextBuilder.forClient();
         if (km != null) {
             sslContextBuilder.keyManager(km);
         }
@@ -108,36 +98,54 @@ public class ClientX509Util extends X509Util {
             sslContextBuilder.trustManager(tm);
         }
 
-        return createNettySslContext(config, sslContextBuilder, "Server");
+        sslContextBuilder.enableOcsp(config.getBoolean(getSslOcspEnabledProperty()));
+        sslContextBuilder.protocols(getEnabledProtocols(config));
+        Iterable<String> enabledCiphers = getCipherSuites(config);
+        if (enabledCiphers != null) {
+            sslContextBuilder.ciphers(enabledCiphers);
+        }
+        sslContextBuilder.sslProvider(getSslProvider(config));
+
+        SslContext sslContext1 = sslContextBuilder.build();
+
+        if (getFipsMode(config) && isServerHostnameVerificationEnabled(config)) {
+            return addHostnameVerification(sslContext1, "Server");
+        } else {
+            return sslContext1;
+        }
     }
 
     public SslContext createNettySslContextForServer(ZKConfig config)
-        throws X509Exception.SSLContextException, X509Exception.KeyManagerException, X509Exception.TrustManagerException, SSLException {
-        String keyStoreLocation = config.getProperty(getSslKeystoreLocationProperty(), "");
-        String keyStorePassword = getPasswordFromConfigPropertyOrFile(config, getSslKeystorePasswdProperty(),
-            getSslKeystorePasswdPathProperty());
-        String keyStoreType = config.getProperty(getSslKeystoreTypeProperty());
-
-        if (keyStoreLocation.isEmpty()) {
-            throw new X509Exception.SSLContextException(
-                "Keystore is required for SSL server: " + getSslKeystoreLocationProperty());
+            throws X509Exception.SSLContextException, X509Exception.KeyManagerException, X509Exception.TrustManagerException, SSLException {
+        KeyManager km;
+        TrustManager tm;
+        if (VespaZookeeperTlsContextUtils.tlsContext().isPresent()) {
+            km = VespaZookeeperTlsContextUtils.tlsContext().get().sslContext().keyManager();
+            tm = VespaZookeeperTlsContextUtils.tlsContext().get().sslContext().trustManager();
         }
+        else {
+            String keyStoreLocation = config.getProperty(getSslKeystoreLocationProperty(), "");
+            String keyStorePassword = getPasswordFromConfigPropertyOrFile(config, getSslKeystorePasswdProperty(),
+                                                                          getSslKeystorePasswdPathProperty());
+            String keyStoreType = config.getProperty(getSslKeystoreTypeProperty());
 
-        KeyManager km = createKeyManager(keyStoreLocation, keyStorePassword, keyStoreType);
-        TrustManager trustManager = getTrustManager(config);
-
-        return createNettySslContextForServer(config, km, trustManager);
+            if (keyStoreLocation.isEmpty()) {
+                throw new X509Exception.SSLContextException(
+                        "Keystore is required for SSL server: " + getSslKeystoreLocationProperty());
+            }
+            km = createKeyManager(keyStoreLocation, keyStorePassword, keyStoreType);
+            tm = getTrustManager(config);
+        }
+        return createNettySslContextForServer(config, km, tm);
     }
 
-    public SslContext createNettySslContextForServer(ZKConfig config, KeyManager km, TrustManager tm) throws SSLException {
-        SslContextBuilder sslContextBuilder = SslContextBuilder.forServer(km);
-        if (tm != null) {
-            sslContextBuilder.trustManager(tm);
+    public SslContext createNettySslContextForServer(ZKConfig config, KeyManager keyManager, TrustManager trustManager) throws SSLException {
+        SslContextBuilder sslContextBuilder = SslContextBuilder.forServer(keyManager);
+
+        if (trustManager != null) {
+            sslContextBuilder.trustManager(trustManager);
         }
-        return createNettySslContext(config, sslContextBuilder, "Client");
-    }
 
-    SslContext createNettySslContext(ZKConfig config, SslContextBuilder sslContextBuilder, String clientOrServer) throws SSLException {
         sslContextBuilder.enableOcsp(config.getBoolean(getSslOcspEnabledProperty()));
         sslContextBuilder.protocols(getEnabledProtocols(config));
         sslContextBuilder.clientAuth(getClientAuth(config).toNettyClientAuth());
@@ -147,12 +155,12 @@ public class ClientX509Util extends X509Util {
         }
         sslContextBuilder.sslProvider(getSslProvider(config));
 
-        SslContext sslContext = sslContextBuilder.build();
+        SslContext sslContext1 = sslContextBuilder.build();
 
         if (getFipsMode(config) && isClientHostnameVerificationEnabled(config)) {
-            return addHostnameVerification(sslContext, clientOrServer);
+            return addHostnameVerification(sslContext1, "Client");
         } else {
-            return sslContext;
+            return sslContext1;
         }
     }
 
@@ -201,7 +209,7 @@ public class ClientX509Util extends X509Util {
     private TrustManager getTrustManager(ZKConfig config) throws X509Exception.TrustManagerException {
         String trustStoreLocation = config.getProperty(getSslTruststoreLocationProperty(), "");
         String trustStorePassword = getPasswordFromConfigPropertyOrFile(config, getSslTruststorePasswdProperty(),
-            getSslTruststorePasswdPathProperty());
+                                                                        getSslTruststorePasswdPathProperty());
         String trustStoreType = config.getProperty(getSslTruststoreTypeProperty());
 
         boolean sslCrlEnabled = config.getBoolean(getSslCrlEnabledProperty());
@@ -214,8 +222,8 @@ public class ClientX509Util extends X509Util {
             return null;
         } else {
             return createTrustManager(trustStoreLocation, trustStorePassword, trustStoreType,
-                sslCrlEnabled, sslOcspEnabled, sslServerHostnameVerificationEnabled,
-                sslClientHostnameVerificationEnabled, getFipsMode(config));
+                                      sslCrlEnabled, sslOcspEnabled, sslServerHostnameVerificationEnabled,
+                                      sslClientHostnameVerificationEnabled, getFipsMode(config));
         }
     }
 }