From 1a2ff37a4b18d94c53e44411fc1ac4d1cfac07a5 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Mon, 20 Apr 2026 13:29:12 -0400 Subject: [PATCH 1/2] ZOOKEEPER-4835: Make ZooKeeper's use of Netty optional when no SSL is used (#1) * ZOOKEEPER-4835: Make Netty optional when no SSL is used Agent-Logs-Url: https://github.com/dsmiley/zookeeper/sessions/f61cc9b9-0ac8-4f8f-a366-2a7752cdbedc Co-authored-by: dsmiley <377295+dsmiley@users.noreply.github.com> * Address code review: improve error messages and add constants for magic numbers Agent-Logs-Url: https://github.com/dsmiley/zookeeper/sessions/f61cc9b9-0ac8-4f8f-a366-2a7752cdbedc Co-authored-by: dsmiley <377295+dsmiley@users.noreply.github.com> * Remove non-essential log lines; add ArchUnit test to enforce Netty is optional Agent-Logs-Url: https://github.com/dsmiley/zookeeper/sessions/34abe384-a396-4ef6-9ba0-006a7318faa8 Co-authored-by: dsmiley <377295+dsmiley@users.noreply.github.com> * Refine ArchUnit test using dsmiley's approach; fix remaining Netty hard-refs in ZooKeeper/ZooKeeperServer Agent-Logs-Url: https://github.com/dsmiley/zookeeper/sessions/698a4b0f-1e5b-46f8-bf10-f21f9a65a382 Co-authored-by: dsmiley <377295+dsmiley@users.noreply.github.com> * Fix NoClassDefFoundError: split ClientX509Util into Netty-free base and ClientNettyX509Util subclass Agent-Logs-Url: https://github.com/dsmiley/zookeeper/sessions/2dc4e1b6-53f1-4851-91ed-b4427d332564 Co-authored-by: dsmiley <377295+dsmiley@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: dsmiley <377295+dsmiley@users.noreply.github.com> --- pom.xml | 6 + zookeeper-assembly/pom.xml | 15 ++ zookeeper-server/pom.xml | 8 + .../zookeeper/ClientCnxnSocketNetty.java | 4 +- .../java/org/apache/zookeeper/ZooKeeper.java | 27 ++- .../zookeeper/cli/HexDumpOutputFormatter.java | 39 +++- .../zookeeper/common/ClientNettyX509Util.java | 176 ++++++++++++++++++ .../zookeeper/common/ClientX509Util.java | 143 +------------- .../org/apache/zookeeper/common/X509Util.java | 20 +- .../server/NettyServerCnxnFactory.java | 8 +- .../zookeeper/server/ServerCnxnFactory.java | 51 +++++ .../zookeeper/server/ZooKeeperServer.java | 4 +- .../zookeeper/server/ZooKeeperServerMain.java | 2 +- .../server/quorum/QuorumPeerMain.java | 2 +- .../zookeeper/NettyOptionalArchTest.java | 68 +++++++ .../apache/zookeeper/common/X509UtilTest.java | 2 +- 16 files changed, 401 insertions(+), 174 deletions(-) create mode 100644 zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientNettyX509Util.java create mode 100644 zookeeper-server/src/test/java/org/apache/zookeeper/NettyOptionalArchTest.java diff --git a/pom.xml b/pom.xml index 2811e0dfa06..79afb1c9b03 100644 --- a/pom.xml +++ b/pom.xml @@ -574,6 +574,7 @@ 2.17.0 0.25.4 3.5.1 + 1.3.0 4.4.1 3.7.0.1746 @@ -710,6 +711,11 @@ ${mockito.version} test + + com.tngtech.archunit + archunit-junit5 + ${archunit.version} + io.netty netty-bom diff --git a/zookeeper-assembly/pom.xml b/zookeeper-assembly/pom.xml index 317233d6f3b..cb53787c0b1 100644 --- a/zookeeper-assembly/pom.xml +++ b/zookeeper-assembly/pom.xml @@ -115,6 +115,21 @@ org.xerial.snappy snappy-java + + + io.netty + netty-handler + + + io.netty + netty-transport-native-epoll + linux-x86_64 + + + io.netty + netty-tcnative-boringssl-static + diff --git a/zookeeper-server/pom.xml b/zookeeper-server/pom.xml index f4b76095d2e..cd2c0d717ba 100644 --- a/zookeeper-server/pom.xml +++ b/zookeeper-server/pom.xml @@ -65,15 +65,18 @@ io.netty netty-handler + true io.netty netty-transport-native-epoll linux-x86_64 + true io.netty netty-tcnative-boringssl-static + true org.slf4j @@ -180,6 +183,11 @@ tools test + + com.tngtech.archunit + archunit-junit5 + test + org.xerial.snappy snappy-java diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java index 82364dcb55b..023c4316e2b 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java @@ -51,7 +51,7 @@ import org.apache.zookeeper.ClientCnxn.EndOfStreamException; import org.apache.zookeeper.ClientCnxn.Packet; import org.apache.zookeeper.client.ZKClientConfig; -import org.apache.zookeeper.common.ClientX509Util; +import org.apache.zookeeper.common.ClientNettyX509Util; import org.apache.zookeeper.common.NettyUtils; import org.apache.zookeeper.common.X509Exception; import org.slf4j.Logger; @@ -445,7 +445,7 @@ protected void initChannel(SocketChannel ch) throws Exception { private synchronized void initSSL(ChannelPipeline pipeline) throws X509Exception.KeyManagerException, X509Exception.TrustManagerException, SSLException { if (sslContext == null) { - try (ClientX509Util x509Util = new ClientX509Util()) { + try (ClientNettyX509Util x509Util = new ClientNettyX509Util()) { sslContext = x509Util.createNettySslContextForClient(clientConfig); } } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java index 32893d46af4..d86d7cdf771 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java @@ -3142,10 +3142,17 @@ protected SocketAddress testableLocalSocketAddress() { private ClientCnxnSocket getClientCnxnSocket() throws IOException { String clientCnxnSocketName = getClientConfig().getProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET); - if (clientCnxnSocketName == null || clientCnxnSocketName.equals(ClientCnxnSocketNIO.class.getSimpleName())) { + if (clientCnxnSocketName == null) { + boolean secureClient = getClientConfig().getBoolean(ZKClientConfig.SECURE_CLIENT); + if (secureClient) { + clientCnxnSocketName = "org.apache.zookeeper.ClientCnxnSocketNetty"; + } else { + clientCnxnSocketName = ClientCnxnSocketNIO.class.getName(); + } + } else if (clientCnxnSocketName.equals(ClientCnxnSocketNIO.class.getSimpleName())) { clientCnxnSocketName = ClientCnxnSocketNIO.class.getName(); - } else if (clientCnxnSocketName.equals(ClientCnxnSocketNetty.class.getSimpleName())) { - clientCnxnSocketName = ClientCnxnSocketNetty.class.getName(); + } else if (clientCnxnSocketName.equals("ClientCnxnSocketNetty")) { + clientCnxnSocketName = "org.apache.zookeeper.ClientCnxnSocketNetty"; } try { @@ -3154,7 +3161,19 @@ private ClientCnxnSocket getClientCnxnSocket() throws IOException { ClientCnxnSocket clientCxnSocket = (ClientCnxnSocket) clientCxnConstructor.newInstance(getClientConfig()); return clientCxnSocket; } catch (Exception e) { - throw new IOException("Couldn't instantiate " + clientCnxnSocketName, e); + String msg = "Couldn't instantiate " + clientCnxnSocketName; + if (getClientConfig().getBoolean(ZKClientConfig.SECURE_CLIENT)) { + msg += ". SSL/TLS support requires Netty; please add netty-handler" + + " (and optionally netty-tcnative-boringssl-static) to your project's dependencies."; + } + throw new IOException(msg, e); + } catch (NoClassDefFoundError e) { + String msg = "Couldn't instantiate " + clientCnxnSocketName; + if (getClientConfig().getBoolean(ZKClientConfig.SECURE_CLIENT)) { + msg += ". SSL/TLS support requires Netty; please add netty-handler" + + " (and optionally netty-tcnative-boringssl-static) to your project's dependencies."; + } + throw new IOException(msg, e); } } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/cli/HexDumpOutputFormatter.java b/zookeeper-server/src/main/java/org/apache/zookeeper/cli/HexDumpOutputFormatter.java index d545a422efc..6891d414229 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/cli/HexDumpOutputFormatter.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/cli/HexDumpOutputFormatter.java @@ -18,17 +18,44 @@ package org.apache.zookeeper.cli; -import io.netty.buffer.ByteBuf; -import io.netty.buffer.ByteBufUtil; -import io.netty.buffer.Unpooled; - public class HexDumpOutputFormatter implements OutputFormatter { public static final HexDumpOutputFormatter INSTANCE = new HexDumpOutputFormatter(); + private static final int BYTES_PER_ROW = 16; + private static final int ASCII_PRINTABLE_MIN = 0x20; // space + private static final int ASCII_PRINTABLE_MAX = 0x7f; // DEL (exclusive) + private static final String HEADER_LINE = + " +-------------------------------------------------+\n" + + " | 0 1 2 3 4 5 6 7 8 9 a b c d e f |\n" + + "+--------+-------------------------------------------------+----------------+"; + private static final String FOOTER_LINE = + "+--------+-------------------------------------------------+----------------+"; + @Override public String format(byte[] data) { - ByteBuf buf = Unpooled.wrappedBuffer(data); - return ByteBufUtil.prettyHexDump(buf); + if (data == null || data.length == 0) { + return ""; + } + StringBuilder sb = new StringBuilder(); + sb.append(HEADER_LINE).append('\n'); + for (int offset = 0; offset < data.length; offset += BYTES_PER_ROW) { + sb.append(String.format("|%08x|", offset)); + StringBuilder charPart = new StringBuilder(); + for (int i = 0; i < BYTES_PER_ROW; i++) { + if (offset + i < data.length) { + int b = data[offset + i] & 0xFF; + sb.append(String.format(" %02x", b)); + char c = (char) b; + charPart.append(c >= ASCII_PRINTABLE_MIN && c < ASCII_PRINTABLE_MAX ? c : '.'); + } else { + sb.append(" "); + charPart.append(' '); + } + } + sb.append(" |").append(charPart).append("|\n"); + } + sb.append(FOOTER_LINE); + return sb.toString(); } } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientNettyX509Util.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientNettyX509Util.java new file mode 100644 index 00000000000..be9365cad18 --- /dev/null +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientNettyX509Util.java @@ -0,0 +1,176 @@ +/* + * 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 io.netty.handler.ssl.DelegatingSslContext; +import io.netty.handler.ssl.OpenSsl; +import io.netty.handler.ssl.SslContext; +import io.netty.handler.ssl.SslContextBuilder; +import io.netty.handler.ssl.SslProvider; +import java.security.Security; +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; + +/** + * Extends {@link ClientX509Util} with Netty-specific SSL context creation + * methods. This class is only loaded when Netty is present on the classpath. + * Code that only needs SSL property names should use {@link ClientX509Util} + * directly so that Netty remains an optional dependency. + */ +public class ClientNettyX509Util extends ClientX509Util { + + private static final Logger LOG = LoggerFactory.getLogger(ClientNettyX509Util.class); + + public SslContext createNettySslContextForClient(ZKConfig config) + throws X509Exception.KeyManagerException, X509Exception.TrustManagerException, SSLException { + SslContextBuilder sslContextBuilder = SslContextBuilder.forClient(); + + KeyManager km = buildKeyManager(config); + if (km != null) { + sslContextBuilder.keyManager(km); + } + + TrustManager tm = buildTrustManager(config); + if (tm != null) { + sslContextBuilder.trustManager(tm); + } + + handleTcnativeOcspStapling(sslContextBuilder, config); + String[] enabledProtocols = getEnabledProtocols(config); + if (enabledProtocols != null) { + sslContextBuilder.protocols(enabledProtocols); + } + Iterable enabledCiphers = getCipherSuites(config); + if (enabledCiphers != null) { + sslContextBuilder.ciphers(enabledCiphers); + } + sslContextBuilder.sslProvider(getSslProvider(config)); + + SslContext sslContext1 = sslContextBuilder.build(); + + if ((getFipsMode(config) || tm == null) && isServerHostnameVerificationEnabled(config)) { + return addHostnameVerification(sslContext1, "Server"); + } else { + return sslContext1; + } + } + + public SslContext createNettySslContextForServer(ZKConfig config) + throws X509Exception.SSLContextException, X509Exception.KeyManagerException, X509Exception.TrustManagerException, SSLException { + KeyManager km = buildKeyManager(config); + if (km == null) { + throw new X509Exception.SSLContextException( + "Keystore is required for SSL server: " + getSslKeystoreLocationProperty()); + } + return createNettySslContextForServer(config, km, buildTrustManager(config)); + } + + public SslContext createNettySslContextForServer(ZKConfig config, KeyManager keyManager, TrustManager trustManager) throws SSLException { + SslContextBuilder sslContextBuilder = SslContextBuilder.forServer(keyManager); + + if (trustManager != null) { + sslContextBuilder.trustManager(trustManager); + } + + handleTcnativeOcspStapling(sslContextBuilder, config); + String[] enabledProtocols = getEnabledProtocols(config); + if (enabledProtocols != null) { + sslContextBuilder.protocols(enabledProtocols); + } + sslContextBuilder.clientAuth(toNettyClientAuth(getClientAuth(config))); + Iterable enabledCiphers = getCipherSuites(config); + if (enabledCiphers != null) { + sslContextBuilder.ciphers(enabledCiphers); + } + sslContextBuilder.sslProvider(getSslProvider(config)); + + SslContext sslContext1 = sslContextBuilder.build(); + + if ((getFipsMode(config) || trustManager == null) && isClientHostnameVerificationEnabled(config)) { + return addHostnameVerification(sslContext1, "Client"); + } else { + return sslContext1; + } + } + + private SslContextBuilder handleTcnativeOcspStapling(SslContextBuilder builder, ZKConfig config) { + SslProvider sslProvider = getSslProvider(config); + boolean tcnative = sslProvider == SslProvider.OPENSSL || sslProvider == SslProvider.OPENSSL_REFCNT; + boolean ocspEnabled = config.getBoolean(getSslOcspEnabledProperty(), Boolean.parseBoolean(Security.getProperty("ocsp.enable"))); + + if (tcnative && ocspEnabled && OpenSsl.isOcspSupported()) { + builder.enableOcsp(ocspEnabled); + } + return builder; + } + + 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 null; + } + return enabledProtocolsInput.split(","); + } + + private X509Util.ClientAuth getClientAuth(final ZKConfig config) { + return X509Util.ClientAuth.fromPropertyValue(config.getProperty(getSslClientAuthProperty())); + } + + private static io.netty.handler.ssl.ClientAuth toNettyClientAuth(X509Util.ClientAuth clientAuth) { + switch (clientAuth) { + case NONE: return io.netty.handler.ssl.ClientAuth.NONE; + case WANT: return io.netty.handler.ssl.ClientAuth.OPTIONAL; + case NEED: return io.netty.handler.ssl.ClientAuth.REQUIRE; + default: throw new IllegalArgumentException("Unknown ClientAuth: " + clientAuth); + } + } + + private Iterable getCipherSuites(final ZKConfig config) { + String cipherSuitesInput = config.getProperty(getSslCipherSuitesProperty()); + if (cipherSuitesInput == null) { + return null; + } else { + return Arrays.asList(cipherSuitesInput.split(",")); + } + } + + public SslProvider getSslProvider(ZKConfig config) { + return SslProvider.valueOf(config.getProperty(getSslProviderProperty(), "JDK")); + } +} diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java index 1e50b84257c..473aab88047 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java @@ -18,28 +18,15 @@ package org.apache.zookeeper.common; -import io.netty.handler.ssl.DelegatingSslContext; -import io.netty.handler.ssl.OpenSsl; -import io.netty.handler.ssl.SslContext; -import io.netty.handler.ssl.SslContextBuilder; -import io.netty.handler.ssl.SslProvider; -import java.security.Security; -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. + * + *

This class is intentionally free of Netty dependencies so it can be loaded + * without Netty on the classpath. For Netty SSL context creation use + * {@link ClientNettyX509Util}. */ 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"; @@ -60,126 +47,4 @@ public String getSslAuthProviderProperty() { public String getSslProviderProperty() { return sslProviderProperty; } - - public SslContext createNettySslContextForClient(ZKConfig config) - throws X509Exception.KeyManagerException, X509Exception.TrustManagerException, SSLException { - SslContextBuilder sslContextBuilder = SslContextBuilder.forClient(); - - KeyManager km = buildKeyManager(config); - if (km != null) { - sslContextBuilder.keyManager(km); - } - - TrustManager tm = buildTrustManager(config); - if (tm != null) { - sslContextBuilder.trustManager(tm); - } - - handleTcnativeOcspStapling(sslContextBuilder, config); - String[] enabledProtocols = getEnabledProtocols(config); - if (enabledProtocols != null) { - sslContextBuilder.protocols(enabledProtocols); - } - Iterable enabledCiphers = getCipherSuites(config); - if (enabledCiphers != null) { - sslContextBuilder.ciphers(enabledCiphers); - } - sslContextBuilder.sslProvider(getSslProvider(config)); - - SslContext sslContext1 = sslContextBuilder.build(); - - if ((getFipsMode(config) || tm == null) && isServerHostnameVerificationEnabled(config)) { - return addHostnameVerification(sslContext1, "Server"); - } else { - return sslContext1; - } - } - - public SslContext createNettySslContextForServer(ZKConfig config) - throws X509Exception.SSLContextException, X509Exception.KeyManagerException, X509Exception.TrustManagerException, SSLException { - KeyManager km = buildKeyManager(config); - if (km == null) { - throw new X509Exception.SSLContextException( - "Keystore is required for SSL server: " + getSslKeystoreLocationProperty()); - } - return createNettySslContextForServer(config, km, buildTrustManager(config)); - } - - public SslContext createNettySslContextForServer(ZKConfig config, KeyManager keyManager, TrustManager trustManager) throws SSLException { - SslContextBuilder sslContextBuilder = SslContextBuilder.forServer(keyManager); - - if (trustManager != null) { - sslContextBuilder.trustManager(trustManager); - } - - handleTcnativeOcspStapling(sslContextBuilder, config); - String[] enabledProtocols = getEnabledProtocols(config); - if (enabledProtocols != null) { - sslContextBuilder.protocols(enabledProtocols); - } - 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) || trustManager == null) && isClientHostnameVerificationEnabled(config)) { - return addHostnameVerification(sslContext1, "Client"); - } else { - return sslContext1; - } - } - - private SslContextBuilder handleTcnativeOcspStapling(SslContextBuilder builder, ZKConfig config) { - SslProvider sslProvider = getSslProvider(config); - boolean tcnative = sslProvider == SslProvider.OPENSSL || sslProvider == SslProvider.OPENSSL_REFCNT; - boolean ocspEnabled = config.getBoolean(getSslOcspEnabledProperty(), Boolean.parseBoolean(Security.getProperty("ocsp.enable"))); - - if (tcnative && ocspEnabled && OpenSsl.isOcspSupported()) { - builder.enableOcsp(ocspEnabled); - } - return builder; - } - - 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 null; - } - 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) { - return null; - } else { - return Arrays.asList(cipherSuitesInput.split(",")); - } - } - - public SslProvider getSslProvider(ZKConfig config) { - return SslProvider.valueOf(config.getProperty(getSslProviderProperty(), "JDK")); - } } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java index fee410cf057..644873ac8d0 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java @@ -56,7 +56,6 @@ import org.apache.zookeeper.common.X509Exception.KeyManagerException; import org.apache.zookeeper.common.X509Exception.SSLContextException; import org.apache.zookeeper.common.X509Exception.TrustManagerException; -import org.apache.zookeeper.server.NettyServerCnxnFactory; import org.apache.zookeeper.server.auth.ProviderRegistry; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -70,6 +69,7 @@ public abstract class X509Util implements Closeable, AutoCloseable { private static final String REJECT_CLIENT_RENEGOTIATION_PROPERTY = "jdk.tls.rejectClientInitiatedRenegotiation"; public static final String FIPS_MODE_PROPERTY = "zookeeper.fips-mode"; + public static final String CLIENT_CERT_RELOAD_KEY = "zookeeper.client.certReload"; private static final boolean FIPS_MODE_DEFAULT = true; public static final String TLS_1_1 = "TLSv1.1"; public static final String TLS_1_2 = "TLSv1.2"; @@ -123,15 +123,9 @@ private static String defaultTlsProtocol() { * If the config property is not set, the default value is NEED. */ public enum ClientAuth { - NONE(io.netty.handler.ssl.ClientAuth.NONE), - WANT(io.netty.handler.ssl.ClientAuth.OPTIONAL), - NEED(io.netty.handler.ssl.ClientAuth.REQUIRE); - - private final io.netty.handler.ssl.ClientAuth nettyAuth; - - ClientAuth(io.netty.handler.ssl.ClientAuth nettyAuth) { - this.nettyAuth = nettyAuth; - } + NONE, + WANT, + NEED; /** * Converts a property value to a ClientAuth enum. If the input string is empty or null, returns @@ -146,10 +140,6 @@ public static ClientAuth fromPropertyValue(String prop) { } return ClientAuth.valueOf(prop.toUpperCase()); } - - public io.netty.handler.ssl.ClientAuth toNettyClientAuth() { - return nettyAuth; - } } private final String sslProtocolProperty = getConfigPrefix() + "protocol"; @@ -316,7 +306,7 @@ private void resetDefaultSSLContextAndOptions() throws X509Exception.SSLContextE SSLContextAndOptions newContext = createSSLContextAndOptions(); defaultSSLContextAndOptions.set(newContext); - if (Boolean.getBoolean(NettyServerCnxnFactory.CLIENT_CERT_RELOAD_KEY)) { + if (Boolean.getBoolean(CLIENT_CERT_RELOAD_KEY)) { ProviderRegistry.addOrUpdateProvider(ProviderRegistry.AUTHPROVIDER_PROPERTY_PREFIX + "x509"); } } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java index 6f65787442c..d10acf8c38c 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java @@ -61,11 +61,13 @@ import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSession; import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.common.ClientNettyX509Util; import org.apache.zookeeper.common.ClientX509Util; import org.apache.zookeeper.common.ConfigException; import org.apache.zookeeper.common.NettyUtils; import org.apache.zookeeper.common.X509Exception; import org.apache.zookeeper.common.X509Exception.SSLContextException; +import org.apache.zookeeper.common.X509Util; import org.apache.zookeeper.common.ZKConfig; import org.apache.zookeeper.server.NettyServerCnxn.HandshakeState; import org.apache.zookeeper.server.auth.ProviderRegistry; @@ -112,7 +114,7 @@ public void setOutstandingHandshakeLimit(int limit) { private InetSocketAddress localAddress; private int maxClientCnxns = 60; int listenBacklog = -1; - private final ClientX509Util x509Util; + private final ClientNettyX509Util x509Util; public static final String NETTY_ADVANCED_FLOW_CONTROL = "zookeeper.netty.advancedFlowControl.enabled"; private boolean advancedFlowControlEnabled = false; @@ -121,7 +123,7 @@ public void setOutstandingHandshakeLimit(int limit) { private static final AtomicReference TEST_ALLOCATOR = new AtomicReference<>(null); - public static final String CLIENT_CERT_RELOAD_KEY = "zookeeper.client.certReload"; + public static final String CLIENT_CERT_RELOAD_KEY = X509Util.CLIENT_CERT_RELOAD_KEY; /** * A handler that detects whether the client would like to use @@ -511,7 +513,7 @@ private ServerBootstrap configureBootstrapAllocator(ServerBootstrap bootstrap) { } NettyServerCnxnFactory() { - x509Util = new ClientX509Util(); + x509Util = new ClientNettyX509Util(); boolean useClientReload = Boolean.getBoolean(CLIENT_CERT_RELOAD_KEY); LOG.info("{}={}", CLIENT_CERT_RELOAD_KEY, useClientReload); diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerCnxnFactory.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerCnxnFactory.java index f63c1eec4ba..d06ce09c70b 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerCnxnFactory.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerCnxnFactory.java @@ -161,6 +161,14 @@ public final void setZooKeeperServer(ZooKeeperServer zks) { public abstract void closeAll(ServerCnxn.DisconnectReason reason); + /** + * Returns the number of connections that are currently performing a TLS handshake. + * Non-Netty implementations return 0. + */ + public int getOutstandingHandshakeNum() { + return 0; + } + /** * Attempts to shed approximately the specified percentage of connections. * @@ -228,6 +236,49 @@ public static ServerCnxnFactory createFactory() throws IOException { } } + /** + * Creates a ServerCnxnFactory, defaulting to NettyServerCnxnFactory when + * {@code secure} is {@code true} and no explicit factory is configured via + * the {@value #ZOOKEEPER_SERVER_CNXN_FACTORY} system property. SSL/TLS + * requires Netty; if Netty is not present on the classpath a helpful + * {@link IOException} is thrown. + * + * @param secure {@code true} when the factory will be used for secure (SSL/TLS) connections + * @return a new ServerCnxnFactory instance + * @throws IOException if the factory cannot be instantiated + */ + public static ServerCnxnFactory createFactory(boolean secure) throws IOException { + String serverCnxnFactoryName = System.getProperty(ZOOKEEPER_SERVER_CNXN_FACTORY); + if (serverCnxnFactoryName == null) { + if (secure) { + serverCnxnFactoryName = "org.apache.zookeeper.server.NettyServerCnxnFactory"; + } else { + serverCnxnFactoryName = NIOServerCnxnFactory.class.getName(); + } + } + try { + ServerCnxnFactory serverCnxnFactory = (ServerCnxnFactory) Class.forName(serverCnxnFactoryName) + .getDeclaredConstructor() + .newInstance(); + LOG.info("Using {} as server connection factory", serverCnxnFactoryName); + return serverCnxnFactory; + } catch (Exception e) { + String msg = "Couldn't instantiate " + serverCnxnFactoryName; + if (secure) { + msg += ". SSL/TLS support requires Netty; please add netty-handler" + + " (and optionally netty-tcnative-boringssl-static) to your project's dependencies."; + } + throw new IOException(msg, e); + } catch (NoClassDefFoundError e) { + String msg = "Couldn't instantiate " + serverCnxnFactoryName; + if (secure) { + msg += ". SSL/TLS support requires Netty; please add netty-handler" + + " (and optionally netty-tcnative-boringssl-static) to your project's dependencies."; + } + throw new IOException(msg, e); + } + } + public static ServerCnxnFactory createFactory(int clientPort, int maxClientCnxns) throws IOException { return createFactory(new InetSocketAddress(clientPort), maxClientCnxns, -1); } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java index 080f4f8638d..511cdd5838e 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java @@ -2379,8 +2379,8 @@ public boolean authWriteRequest(Request request) { } public int getOutstandingHandshakeNum() { - if (serverCnxnFactory instanceof NettyServerCnxnFactory) { - return ((NettyServerCnxnFactory) serverCnxnFactory).getOutstandingHandshakeNum(); + if (serverCnxnFactory != null) { + return serverCnxnFactory.getOutstandingHandshakeNum(); } else { return 0; } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerMain.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerMain.java index c721e685bb6..0545827c893 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerMain.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerMain.java @@ -162,7 +162,7 @@ public void runFromConfig(ServerConfig config) throws IOException, AdminServerEx needStartZKServer = false; } if (config.getSecureClientPortAddress() != null) { - secureCnxnFactory = ServerCnxnFactory.createFactory(); + secureCnxnFactory = ServerCnxnFactory.createFactory(true); secureCnxnFactory.configure(config.getSecureClientPortAddress(), config.getMaxClientCnxns(), config.getClientPortListenBacklog(), true); secureCnxnFactory.startup(zkServer, needStartZKServer); } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerMain.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerMain.java index c126ea2e290..e909eeb0ac3 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerMain.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerMain.java @@ -170,7 +170,7 @@ public void runFromConfig(QuorumPeerConfig config) throws IOException, AdminServ } if (config.getSecureClientPortAddress() != null) { - secureCnxnFactory = ServerCnxnFactory.createFactory(); + secureCnxnFactory = ServerCnxnFactory.createFactory(true); secureCnxnFactory.configure(config.getSecureClientPortAddress(), config.getMaxClientCnxns(), config.getClientPortListenBacklog(), true); } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/NettyOptionalArchTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/NettyOptionalArchTest.java new file mode 100644 index 00000000000..4d806755477 --- /dev/null +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/NettyOptionalArchTest.java @@ -0,0 +1,68 @@ +/* + * 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; + +import com.tngtech.archunit.base.DescribedPredicate; +import com.tngtech.archunit.core.domain.JavaClass; +import com.tngtech.archunit.core.domain.JavaClasses; +import com.tngtech.archunit.core.importer.ClassFileImporter; +import com.tngtech.archunit.core.importer.ImportOption; +import com.tngtech.archunit.lang.ArchRule; +import org.junit.jupiter.api.Test; + +import java.util.Collections; + +import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses; + +/** + * Architectural test to enforce that Netty is an optional dependency. + * + *

Only classes whose name contains "Netty" (e.g. {@code NettyServerCnxnFactory}, + * {@code ClientCnxnSocketNetty}, {@code ClientNettyX509Util}) and a small set of + * explicitly allowed SSL/TLS utility classes ({@code UnifiedServerSocket}) may depend + * on {@code io.netty} packages. All other ZooKeeper classes must remain Netty-free so + * that Netty can be an optional dependency for users who do not need SSL/TLS. + */ +public class NettyOptionalArchTest { + + @Test + public void nonNettyClassesShouldNotDependOnNetty() { + JavaClasses importedClasses = new ClassFileImporter( + Collections.singletonList(new ImportOption.DoNotIncludeTests())) + .importPackages("org.apache.zookeeper") + .that(new DescribedPredicate("ZK Non-Netty classes") { + @Override + public boolean test(JavaClass javaClass) { + // Exclude classes with "Netty" in their name (e.g. NettyServerCnxnFactory, + // ClientCnxnSocketNetty, NettyServerCnxn, NettyUtils, ClientNettyX509Util). + // Also exclude UnifiedServerSocket (and its inner classes) which legitimately + // uses the Netty SSL API to detect SSL vs plain-text connections. + String name = javaClass.getName(); + return !name.contains("Netty") + && !name.contains("UnifiedServerSocket"); + } + }); + + ArchRule rule = noClasses().should() + .dependOnClassesThat().resideInAnyPackage("io.netty..") + .orShould().dependOnClassesThat().haveSimpleNameContaining("Netty"); + + rule.check(importedClasses); + } +} diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java index 1c5104e784b..548c313c384 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java @@ -730,7 +730,7 @@ public void testCreateSSLContext_hostnameVerificationNoCustomTrustStore(X509KeyT // Verify client hostname too System.setProperty(x509Util.getSslClientHostnameVerificationEnabledProperty(), "true"); ZKConfig zkConfig = new ZKConfig(); - try (ClientX509Util clientX509Util = new ClientX509Util();) { + try (ClientNettyX509Util clientX509Util = new ClientNettyX509Util();) { UnpooledByteBufAllocator byteBufAllocator = new UnpooledByteBufAllocator(false); SslContext clientContext = clientX509Util.createNettySslContextForClient(zkConfig); SSLEngine clientEngine = clientContext.newEngine(byteBufAllocator); From a1c0fbdcb95729900eb912a991dfd741b0b02b7c Mon Sep 17 00:00:00 2001 From: David Smiley Date: Mon, 20 Apr 2026 22:51:34 -0400 Subject: [PATCH 2/2] checkstyle fixes --- .../org/apache/zookeeper/server/NettyServerCnxnFactory.java | 1 - .../java/org/apache/zookeeper/NettyOptionalArchTest.java | 6 ++---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java index d10acf8c38c..061caf85cae 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java @@ -62,7 +62,6 @@ import javax.net.ssl.SSLSession; import org.apache.zookeeper.KeeperException; import org.apache.zookeeper.common.ClientNettyX509Util; -import org.apache.zookeeper.common.ClientX509Util; import org.apache.zookeeper.common.ConfigException; import org.apache.zookeeper.common.NettyUtils; import org.apache.zookeeper.common.X509Exception; diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/NettyOptionalArchTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/NettyOptionalArchTest.java index 4d806755477..b95050541ef 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/NettyOptionalArchTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/NettyOptionalArchTest.java @@ -18,17 +18,15 @@ package org.apache.zookeeper; +import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses; import com.tngtech.archunit.base.DescribedPredicate; import com.tngtech.archunit.core.domain.JavaClass; import com.tngtech.archunit.core.domain.JavaClasses; import com.tngtech.archunit.core.importer.ClassFileImporter; import com.tngtech.archunit.core.importer.ImportOption; import com.tngtech.archunit.lang.ArchRule; -import org.junit.jupiter.api.Test; - import java.util.Collections; - -import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses; +import org.junit.jupiter.api.Test; /** * Architectural test to enforce that Netty is an optional dependency.