This is an automated email from the ASF dual-hosted git repository.
kezhuw pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/master by this push:
new 2aaeff840 ZOOKEEPER-4912: Remove default TLS cipher overrides
2aaeff840 is described below
commit 2aaeff840e8e5b61a30530b261b8c05c181d078f
Author: Istvan Toth <[email protected]>
AuthorDate: Sun May 11 04:16:38 2025 +0200
ZOOKEEPER-4912: Remove default TLS cipher overrides
Reviewers: cnauroth, anmolnar, kezhuw
Author: stoty
Closes #2239 from stoty/ZOOKEEPER-4912
---
.../src/main/resources/markdown/zookeeperAdmin.md | 16 +++++-
.../apache/zookeeper/common/ClientX509Util.java | 17 +++---
.../zookeeper/common/SSLContextAndOptions.java | 2 +-
.../java/org/apache/zookeeper/common/X509Util.java | 60 +-------------------
.../org/apache/zookeeper/common/X509UtilTest.java | 66 ----------------------
5 files changed, 28 insertions(+), 133 deletions(-)
diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
index 70ce27b72..a4fccf1f9 100644
--- a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
+++ b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
@@ -43,6 +43,7 @@ limitations under the License.
* [Advanced Configuration](#sc_advancedConfiguration)
* [Cluster Options](#sc_clusterOptions)
* [Encryption, Authentication, Authorization Options](#sc_authOptions)
+ * [TLS Cipher Suites](#sc_tls_cipher_suites)
* [Experimental Options/Features](#Experimental+Options%2FFeatures)
* [Unsafe Options](#Unsafe+Options)
* [Disabling data directory
autocreation](#Disabling+data+directory+autocreation)
@@ -1738,7 +1739,7 @@ and [SASL authentication for
ZooKeeper](https://cwiki.apache.org/confluence/disp
(Java system properties: **zookeeper.ssl.ciphersuites** and
**zookeeper.ssl.quorum.ciphersuites**)
**New in 3.5.5:**
Specifies the enabled cipher suites to be used in client and quorum TLS
negotiation.
- Default: Enabled cipher suites depend on the Java runtime version being
used.
+ Default: JDK defaults since 3.10.0, and hard coded cipher suites for 3.9
and earlier versions. See [TLS Cipher Suites](#sc_tls_cipher_suites).
* *ssl.context.supplier.class* and *ssl.quorum.context.supplier.class* :
(Java system properties: **zookeeper.ssl.context.supplier.class** and
**zookeeper.ssl.quorum.context.supplier.class**)
@@ -1877,6 +1878,19 @@ and [SASL authentication for
ZooKeeper](https://cwiki.apache.org/confluence/disp
Default: **true** (3.9.0+), **false** (3.8.x)
+<a name="sc_tls_cipher_suites"></a>
+
+##### TLS Cipher Suites
+
+From 3.5.5 to 3.9 a hard coded default cipher list was used, with the ordering
+dependent on whether it is run Java 8 or a later version.
+
+The list on Java 8 includes TLSv1.2 CBC, GCM and TLSv1.3 ciphers in ordering:
*TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384,
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384, TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_ECDSA_WITH_AES_ [...]
+
+The list on Java 9+ includes TLSv1.2 GCM, CBC and TLSv1.3 ciphers in ordering:
*TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384,
TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384, TLS_ECDHE_ECDSA_WI [...]
+
+Since 3.10 there is no hardcoded list, and the JDK defaults are used.
+
<a name="Experimental+Options%2FFeatures"></a>
#### Experimental Options/Features
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 7bb453903..9aa03ae49 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
@@ -80,7 +80,10 @@ public SslContext createNettySslContextForClient(ZKConfig
config)
}
sslContextBuilder.enableOcsp(config.getBoolean(getSslOcspEnabledProperty()));
- sslContextBuilder.protocols(getEnabledProtocols(config));
+ String[] enabledProtocols = getEnabledProtocols(config);
+ if (enabledProtocols != null) {
+ sslContextBuilder.protocols(enabledProtocols);
+ }
Iterable<String> enabledCiphers = getCipherSuites(config);
if (enabledCiphers != null) {
sslContextBuilder.ciphers(enabledCiphers);
@@ -121,7 +124,10 @@ public SslContext createNettySslContextForServer(ZKConfig
config, KeyManager key
}
sslContextBuilder.enableOcsp(config.getBoolean(getSslOcspEnabledProperty()));
- sslContextBuilder.protocols(getEnabledProtocols(config));
+ String[] enabledProtocols = getEnabledProtocols(config);
+ if (enabledProtocols != null) {
+ sslContextBuilder.protocols(enabledProtocols);
+ }
sslContextBuilder.clientAuth(getClientAuth(config).toNettyClientAuth());
Iterable<String> enabledCiphers = getCipherSuites(config);
if (enabledCiphers != null) {
@@ -155,7 +161,7 @@ protected void initEngine(SSLEngine sslEngine) {
private String[] getEnabledProtocols(final ZKConfig config) {
String enabledProtocolsInput =
config.getProperty(getSslEnabledProtocolsProperty());
if (enabledProtocolsInput == null) {
- return new String[]{ config.getProperty(getSslProtocolProperty(),
DEFAULT_PROTOCOL) };
+ return null;
}
return enabledProtocolsInput.split(",");
}
@@ -167,10 +173,7 @@ private X509Util.ClientAuth getClientAuth(final ZKConfig
config) {
private Iterable<String> getCipherSuites(final ZKConfig config) {
String cipherSuitesInput =
config.getProperty(getSslCipherSuitesProperty());
if (cipherSuitesInput == null) {
- if (getSslProvider(config) != SslProvider.JDK) {
- return null;
- }
- return Arrays.asList(X509Util.getDefaultCipherSuites());
+ return null;
} else {
return Arrays.asList(cipherSuitesInput.split(","));
}
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/common/SSLContextAndOptions.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/common/SSLContextAndOptions.java
index 3a2542552..c712f6acb 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/common/SSLContextAndOptions.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/common/SSLContextAndOptions.java
@@ -160,7 +160,7 @@ private String[] getEnabledProtocols(final ZKConfig config,
final SSLContext ssl
private String[] getCipherSuites(final ZKConfig config) {
String cipherSuitesInput =
config.getProperty(x509Util.getSslCipherSuitesProperty());
if (cipherSuitesInput == null) {
- return X509Util.getDefaultCipherSuites();
+ return null;
} else {
return cipherSuitesInput.split(",");
}
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 17818207e..153a826ba 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
@@ -36,16 +36,13 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
-import java.util.Objects;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Supplier;
-import java.util.stream.Collectors;
import javax.net.ssl.CertPathTrustManagerParameters;
import javax.net.ssl.KeyManager;
import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLServerSocket;
-import javax.net.ssl.SSLServerSocketFactory;
import javax.net.ssl.SSLSocket;
import javax.net.ssl.TrustManager;
import javax.net.ssl.TrustManagerFactory;
@@ -62,11 +59,6 @@
/**
* Utility code for X509 handling
- *
- * Default cipher suites:
- *
- * Performance testing done by Facebook engineers shows that on Intel x86_64
machines, Java9 performs better with
- * GCM and Java8 performs better with CBC, so these seem like reasonable
defaults.
*/
public abstract class X509Util implements Closeable, AutoCloseable {
@@ -102,6 +94,8 @@ private static String defaultTlsProtocol() {
List<String> supported = new ArrayList<>();
try {
supported =
Arrays.asList(SSLContext.getDefault().getSupportedSSLParameters().getProtocols());
+ // We cannot use the default protocols directly, because the
SSLContext factory methods
+ // only accept a single protocol
if (supported.contains(TLS_1_3)) {
defaultProtocol = TLS_1_3;
}
@@ -112,36 +106,6 @@ private static String defaultTlsProtocol() {
return defaultProtocol;
}
- // ChaCha20 was introduced in OpenJDK 11.0.15 and it is not supported by
JDK8.
- private static String[] getTLSv13Ciphers() {
- return new String[]{"TLS_AES_256_GCM_SHA384",
"TLS_AES_128_GCM_SHA256", "TLS_CHACHA20_POLY1305_SHA256"};
- }
-
- private static String[] getGCMCiphers() {
- return new String[]{"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"};
- }
-
- private static String[] getCBCCiphers() {
- return new String[]{"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256",
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256",
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384",
"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384",
"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA"};
- }
-
- /**
- * Returns a filtered set of ciphers, where ciphers not supported by the
JDK are removed.
- */
- private static String[] getSupportedCiphers(String[]... cipherLists) {
- List<String> supported = Arrays.asList(
- ((SSLServerSocketFactory)
SSLServerSocketFactory.getDefault()).getSupportedCipherSuites());
-
- return
Arrays.stream(cipherLists).flatMap(Arrays::stream).filter(supported::contains).collect(Collectors.toList()).toArray(new
String[0]);
- }
-
- // On Java 8, prefer CBC ciphers since AES-NI support is lacking and GCM
is slower than CBC.
- private static final String[] DEFAULT_CIPHERS_JAVA8 =
getSupportedCiphers(getCBCCiphers(), getGCMCiphers(), getTLSv13Ciphers());
- // On Java 9 and later, prefer GCM ciphers due to improved AES-NI support.
- // Note that this performance assumption might not hold true for
architectures other than x86_64.
- // TLSv1.3 ciphers can be added at the end of the list without impacting
the priority of TLSv1.3 vs TLSv1.2.
- private static final String[] DEFAULT_CIPHERS_JAVA9 =
getSupportedCiphers(getGCMCiphers(), getCBCCiphers(), getTLSv13Ciphers());
-
public static final int DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS = 5000;
/**
@@ -636,26 +600,6 @@ public SSLServerSocket createSSLServerSocket(int port)
throws X509Exception, IOE
return getDefaultSSLContextAndOptions().createSSLServerSocket(port);
}
- static String[] getDefaultCipherSuites() {
- return
getDefaultCipherSuitesForJavaVersion(System.getProperty("java.specification.version"));
- }
-
- static String[] getDefaultCipherSuitesForJavaVersion(String javaVersion) {
- Objects.requireNonNull(javaVersion);
- if (javaVersion.matches("\\d+")) {
- // Must be Java 9 or later
- LOG.debug("Using Java9+ optimized cipher suites for Java version
{}", javaVersion);
- return DEFAULT_CIPHERS_JAVA9;
- } else if (javaVersion.startsWith("1.")) {
- // Must be Java 1.8 or earlier
- LOG.debug("Using Java8 optimized cipher suites for Java version
{}", javaVersion);
- return DEFAULT_CIPHERS_JAVA8;
- } else {
- LOG.debug("Could not parse java version {}, using Java8 optimized
cipher suites", javaVersion);
- return DEFAULT_CIPHERS_JAVA8;
- }
- }
-
private FileChangeWatcher newFileChangeWatcher(String fileLocation) throws
IOException {
if (fileLocation == null || fileLocation.isEmpty()) {
return null;
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 1218a00de..e4f19d77f 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
@@ -829,72 +829,6 @@ public void handshakeCompleted(HandshakeCompletedEvent
handshakeCompletedEvent)
});
}
- @ParameterizedTest
- @MethodSource("data")
- public void testGetDefaultCipherSuitesJava8(
- X509KeyType caKeyType, X509KeyType certKeyType, String
keyPassword, Integer paramIndex)
- throws Exception {
- init(caKeyType, certKeyType, keyPassword, paramIndex);
- String[] cipherSuites =
X509Util.getDefaultCipherSuitesForJavaVersion("1.8");
- // Java 8 default should have the CBC suites first
- assertTrue(cipherSuites[0].contains("CBC"));
- }
-
- @ParameterizedTest
- @MethodSource("data")
- public void testGetDefaultCipherSuitesJava9(
- X509KeyType caKeyType, X509KeyType certKeyType, String
keyPassword, Integer paramIndex)
- throws Exception {
- init(caKeyType, certKeyType, keyPassword, paramIndex);
- String[] cipherSuites =
X509Util.getDefaultCipherSuitesForJavaVersion("9");
- // Java 9+ default should have the GCM suites first
- assertTrue(cipherSuites[0].contains("GCM"));
- }
-
- @ParameterizedTest
- @MethodSource("data")
- public void testGetDefaultCipherSuitesJava10(
- X509KeyType caKeyType, X509KeyType certKeyType, String
keyPassword, Integer paramIndex)
- throws Exception {
- init(caKeyType, certKeyType, keyPassword, paramIndex);
- String[] cipherSuites =
X509Util.getDefaultCipherSuitesForJavaVersion("10");
- // Java 9+ default should have the GCM suites first
- assertTrue(cipherSuites[0].contains("GCM"));
- }
-
- @ParameterizedTest
- @MethodSource("data")
- public void testGetDefaultCipherSuitesJava11(
- X509KeyType caKeyType, X509KeyType certKeyType, String
keyPassword, Integer paramIndex)
- throws Exception {
- init(caKeyType, certKeyType, keyPassword, paramIndex);
- String[] cipherSuites =
X509Util.getDefaultCipherSuitesForJavaVersion("11");
- // Java 9+ default should have the GCM suites first
- assertTrue(cipherSuites[0].contains("GCM"));
- }
-
- @ParameterizedTest
- @MethodSource("data")
- public void testGetDefaultCipherSuitesUnknownVersion(
- X509KeyType caKeyType, X509KeyType certKeyType, String
keyPassword, Integer paramIndex)
- throws Exception {
- init(caKeyType, certKeyType, keyPassword, paramIndex);
- String[] cipherSuites =
X509Util.getDefaultCipherSuitesForJavaVersion("notaversion");
- // If version can't be parsed, use the more conservative Java 8 default
- assertTrue(cipherSuites[0].contains("CBC"));
- }
-
- @ParameterizedTest
- @MethodSource("data")
- public void testGetDefaultCipherSuitesNullVersion(
- X509KeyType caKeyType, X509KeyType certKeyType, String
keyPassword, Integer paramIndex)
- throws Exception {
- init(caKeyType, certKeyType, keyPassword, paramIndex);
- assertThrows(NullPointerException.class, () -> {
- X509Util.getDefaultCipherSuitesForJavaVersion(null);
- });
- }
-
// Warning: this will reset the x509Util
private void setCustomCipherSuites() {
System.setProperty(x509Util.getCipherSuitesProperty(),
customCipherSuites[0] + "," + customCipherSuites[1]);
