From cee303d96079ec1ba05f421ff2791105a8fc0ce4 Mon Sep 17 00:00:00 2001 From: jonmv Date: Wed, 13 Dec 2023 15:16:27 +0100 Subject: Look up TLS context directly in X509ClientUtil, which simplifies a lot! --- zookeeper-client-common/pom.xml | 12 ++ .../zookeeper/client/VespaSslContextProvider.java | 25 --- .../zookeeper/client/ZkClientConfigBuilder.java | 11 +- .../apache/zookeeper/common/ClientX509Util.java | 229 +++++++++++++++++++++ .../client/ZkClientConfigBuilderTest.java | 1 - 5 files changed, 243 insertions(+), 35 deletions(-) delete mode 100644 zookeeper-client-common/src/main/java/com/yahoo/vespa/zookeeper/client/VespaSslContextProvider.java create mode 100644 zookeeper-client-common/src/main/java/org/apache/zookeeper/common/ClientX509Util.java (limited to 'zookeeper-client-common') diff --git a/zookeeper-client-common/pom.xml b/zookeeper-client-common/pom.xml index bb86e759dd2..77451c5bec0 100644 --- a/zookeeper-client-common/pom.xml +++ b/zookeeper-client-common/pom.xml @@ -20,6 +20,12 @@ ${project.version} provided + + com.yahoo.vespa + defaults + ${project.version} + provided + org.slf4j slf4j-api @@ -27,6 +33,12 @@ + + com.yahoo.vespa + zookeeper-common + ${project.version} + compile + org.apache.zookeeper zookeeper diff --git a/zookeeper-client-common/src/main/java/com/yahoo/vespa/zookeeper/client/VespaSslContextProvider.java b/zookeeper-client-common/src/main/java/com/yahoo/vespa/zookeeper/client/VespaSslContextProvider.java deleted file mode 100644 index 9cc71eab96e..00000000000 --- a/zookeeper-client-common/src/main/java/com/yahoo/vespa/zookeeper/client/VespaSslContextProvider.java +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.zookeeper.client; - -import com.yahoo.security.tls.TransportSecurityUtils; - -import javax.net.ssl.SSLContext; -import java.util.function.Supplier; - -/** - * Provider for Vespa {@link SSLContext} instance to Zookeeper + misc utility methods for providing Vespa TLS specific ZK configuration. - * - * @author bjorncs - */ -public class VespaSslContextProvider implements Supplier { - - private static final SSLContext sslContext = TransportSecurityUtils.getSystemTlsContext() - .map(tc -> tc.sslContext().context()).orElse(null); - - @Override - public SSLContext get() { - if (sslContext == null) throw new IllegalStateException("Vespa TLS is not enabled"); - return sslContext; - } - -} diff --git a/zookeeper-client-common/src/main/java/com/yahoo/vespa/zookeeper/client/ZkClientConfigBuilder.java b/zookeeper-client-common/src/main/java/com/yahoo/vespa/zookeeper/client/ZkClientConfigBuilder.java index 5c969454d11..1b240aa4785 100644 --- a/zookeeper-client-common/src/main/java/com/yahoo/vespa/zookeeper/client/ZkClientConfigBuilder.java +++ b/zookeeper-client-common/src/main/java/com/yahoo/vespa/zookeeper/client/ZkClientConfigBuilder.java @@ -1,9 +1,8 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.zookeeper.client; -import com.yahoo.security.tls.MixedMode; import com.yahoo.security.tls.TlsContext; -import com.yahoo.security.tls.TransportSecurityUtils; +import com.yahoo.vespa.zookeeper.VespaZookeeperTlsContextUtils; import org.apache.zookeeper.client.ZKClientConfig; import org.apache.zookeeper.server.quorum.QuorumPeerConfig; @@ -14,7 +13,6 @@ import java.nio.file.StandardCopyOption; import java.util.Arrays; import java.util.HashMap; import java.util.Map; -import java.util.Optional; import java.util.stream.Collectors; /** @@ -31,7 +29,7 @@ public class ZkClientConfigBuilder { public static final String SSL_CLIENTAUTH_PROPERTY = "zookeeper.ssl.clientAuth"; public static final String CLIENT_CONNECTION_SOCKET = "zookeeper.clientCnxnSocket"; - private static final TlsContext defaultTlsContext = getTlsContext().orElse(null); + private static final TlsContext defaultTlsContext = VespaZookeeperTlsContextUtils.tlsContext().orElse(null); private final TlsContext tlsContext; @@ -71,7 +69,6 @@ public class ZkClientConfigBuilder { builder.put(CLIENT_SECURE_PROPERTY, Boolean.toString(tlsContext != null)); builder.put(CLIENT_CONNECTION_SOCKET, "org.apache.zookeeper.ClientCnxnSocketNetty"); if (tlsContext != null) { - builder.put(SSL_CONTEXT_SUPPLIER_CLASS_PROPERTY, VespaSslContextProvider.class.getName()); String protocolsConfigValue = Arrays.stream(tlsContext.parameters().getProtocols()).sorted().collect(Collectors.joining(",")); builder.put(SSL_ENABLED_PROTOCOLS_PROPERTY, protocolsConfigValue); String ciphersConfigValue = Arrays.stream(tlsContext.parameters().getCipherSuites()).sorted().collect(Collectors.joining(",")); @@ -81,8 +78,4 @@ public class ZkClientConfigBuilder { return Map.copyOf(builder); } - private static Optional getTlsContext() { - if (TransportSecurityUtils.getInsecureMixedMode() == MixedMode.PLAINTEXT_CLIENT_MIXED_SERVER) return Optional.empty(); - return TransportSecurityUtils.getSystemTlsContext(); - } } diff --git a/zookeeper-client-common/src/main/java/org/apache/zookeeper/common/ClientX509Util.java b/zookeeper-client-common/src/main/java/org/apache/zookeeper/common/ClientX509Util.java new file mode 100644 index 00000000000..9eda60ea361 --- /dev/null +++ b/zookeeper-client-common/src/main/java/org/apache/zookeeper/common/ClientX509Util.java @@ -0,0 +1,229 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +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; +import io.netty.handler.ssl.SslProvider; +import java.util.Arrays; +import javax.net.ssl.KeyManager; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLException; +import javax.net.ssl.SSLParameters; +import javax.net.ssl.TrustManager; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * X509 utilities specific for client-server communication framework. + *

+ * 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 + * + * @author jonmv + */ +public class ClientX509Util extends X509Util { + + private static final Logger LOG = LoggerFactory.getLogger(ClientX509Util.class); + + private final String sslAuthProviderProperty = getConfigPrefix() + "authProvider"; + private final String sslProviderProperty = getConfigPrefix() + "sslProvider"; + + @Override + protected String getConfigPrefix() { + return "zookeeper.ssl."; + } + + @Override + protected boolean shouldVerifyClientHostname() { + return false; + } + + public String getSslAuthProviderProperty() { + return sslAuthProviderProperty; + } + + public String getSslProviderProperty() { + return sslProviderProperty; + } + + public SslContext createNettySslContextForClient(ZKConfig config) + throws X509Exception.KeyManagerException, X509Exception.TrustManagerException, SSLException { + SslContextBuilder sslContextBuilder = SslContextBuilder.forClient(); + 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()); + + if (keyStoreLocation.isEmpty()) { + LOG.warn("{} not specified", getSslKeystoreLocationProperty()); + km = null; + } + else { + km = createKeyManager(keyStoreLocation, keyStorePassword, keyStoreType); + } + + tm = getTrustManager(config); + } + + if (km != null) { + sslContextBuilder.keyManager(km); + } + if (tm != null) { + sslContextBuilder.trustManager(tm); + } + + sslContextBuilder.enableOcsp(config.getBoolean(getSslOcspEnabledProperty())); + sslContextBuilder.protocols(getEnabledProtocols(config)); + Iterable 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 { + 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()); + + 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 keyManager, TrustManager trustManager) throws SSLException { + SslContextBuilder sslContextBuilder = SslContextBuilder.forServer(keyManager); + + if (trustManager != null) { + sslContextBuilder.trustManager(trustManager); + } + + sslContextBuilder.enableOcsp(config.getBoolean(getSslOcspEnabledProperty())); + sslContextBuilder.protocols(getEnabledProtocols(config)); + sslContextBuilder.clientAuth(getClientAuth(config).toNettyClientAuth()); + Iterable enabledCiphers = getCipherSuites(config); + if (enabledCiphers != null) { + sslContextBuilder.ciphers(enabledCiphers); + } + sslContextBuilder.sslProvider(getSslProvider(config)); + + SslContext sslContext1 = sslContextBuilder.build(); + + if (getFipsMode(config) && isClientHostnameVerificationEnabled(config)) { + return addHostnameVerification(sslContext1, "Client"); + } else { + return sslContext1; + } + } + + private SslContext addHostnameVerification(SslContext sslContext, String clientOrServer) { + return new DelegatingSslContext(sslContext) { + @Override + protected void initEngine(SSLEngine sslEngine) { + SSLParameters sslParameters = sslEngine.getSSLParameters(); + sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); + sslEngine.setSSLParameters(sslParameters); + if (LOG.isDebugEnabled()) { + LOG.debug("{} hostname verification: enabled HTTPS style endpoint identification algorithm", clientOrServer); + } + } + }; + } + + private String[] getEnabledProtocols(final ZKConfig config) { + String enabledProtocolsInput = config.getProperty(getSslEnabledProtocolsProperty()); + if (enabledProtocolsInput == null) { + return new String[]{ config.getProperty(getSslProtocolProperty(), DEFAULT_PROTOCOL) }; + } + return enabledProtocolsInput.split(","); + } + + private X509Util.ClientAuth getClientAuth(final ZKConfig config) { + return X509Util.ClientAuth.fromPropertyValue(config.getProperty(getSslClientAuthProperty())); + } + + private Iterable getCipherSuites(final ZKConfig config) { + String cipherSuitesInput = config.getProperty(getSslCipherSuitesProperty()); + if (cipherSuitesInput == null) { + if (getSslProvider(config) != SslProvider.JDK) { + return null; + } + return Arrays.asList(X509Util.getDefaultCipherSuites()); + } else { + return Arrays.asList(cipherSuitesInput.split(",")); + } + } + + public SslProvider getSslProvider(ZKConfig config) { + return SslProvider.valueOf(config.getProperty(getSslProviderProperty(), "JDK")); + } + + private TrustManager getTrustManager(ZKConfig config) throws X509Exception.TrustManagerException { + String trustStoreLocation = config.getProperty(getSslTruststoreLocationProperty(), ""); + String trustStorePassword = getPasswordFromConfigPropertyOrFile(config, getSslTruststorePasswdProperty(), + getSslTruststorePasswdPathProperty()); + String trustStoreType = config.getProperty(getSslTruststoreTypeProperty()); + + boolean sslCrlEnabled = config.getBoolean(getSslCrlEnabledProperty()); + boolean sslOcspEnabled = config.getBoolean(getSslOcspEnabledProperty()); + boolean sslServerHostnameVerificationEnabled = isServerHostnameVerificationEnabled(config); + boolean sslClientHostnameVerificationEnabled = isClientHostnameVerificationEnabled(config); + + if (trustStoreLocation.isEmpty()) { + LOG.warn("{} not specified", getSslTruststoreLocationProperty()); + return null; + } else { + return createTrustManager(trustStoreLocation, trustStorePassword, trustStoreType, + sslCrlEnabled, sslOcspEnabled, sslServerHostnameVerificationEnabled, + sslClientHostnameVerificationEnabled, getFipsMode(config)); + } + } +} diff --git a/zookeeper-client-common/src/test/java/com/yahoo/vespa/zookeeper/client/ZkClientConfigBuilderTest.java b/zookeeper-client-common/src/test/java/com/yahoo/vespa/zookeeper/client/ZkClientConfigBuilderTest.java index 56bfe8381c2..697d76dc53f 100644 --- a/zookeeper-client-common/src/test/java/com/yahoo/vespa/zookeeper/client/ZkClientConfigBuilderTest.java +++ b/zookeeper-client-common/src/test/java/com/yahoo/vespa/zookeeper/client/ZkClientConfigBuilderTest.java @@ -39,7 +39,6 @@ public class ZkClientConfigBuilderTest { ZKClientConfig config = builder.toConfig(); assertEquals("true", config.getProperty(CLIENT_SECURE_PROPERTY)); assertEquals("org.apache.zookeeper.ClientCnxnSocketNetty", config.getProperty(CLIENT_CONNECTION_SOCKET)); - assertEquals(com.yahoo.vespa.zookeeper.client.VespaSslContextProvider.class.getName(), config.getProperty(SSL_CONTEXT_SUPPLIER_CLASS_PROPERTY)); assertEquals("TLSv1.3", config.getProperty(SSL_ENABLED_PROTOCOLS_PROPERTY)); assertEquals("TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", config.getProperty(SSL_ENABLED_CIPHERSUITES_PROPERTY)); assertEquals("NEED", config.getProperty(SSL_CLIENTAUTH_PROPERTY)); -- cgit v1.2.3