This is an automated email from the ASF dual-hosted git repository.
andor pushed a commit to branch branch-3.9
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/branch-3.9 by this push:
new 66771be4d ZOOKEEPER-4415: Added TLSv1.3 support to server (#1919)
66771be4d is described below
commit 66771be4dedd827a77997389cfda0498ea0cb5a8
Author: Tero Saarni <[email protected]>
AuthorDate: Mon Oct 2 07:49:19 2023 +0300
ZOOKEEPER-4415: Added TLSv1.3 support to server (#1919)
* ZOOKEEPER-4415: Added TLSv1.3 support to server
Enable TLSv1.3 when server is running on JDK that supports it.
- Select TLSv1.3 as default protocol.
- Include both TLSv1.2 and TLSv1.3 as default enabled protocols.
- Include TLSv1.3 ciphers in default ciphers.
Signed-off-by: Tero Saarni <[email protected]>
* Fixed checkstyle errors
Signed-off-by: Tero Saarni <[email protected]>
---------
Signed-off-by: Tero Saarni <[email protected]>
(cherry picked from commit 6cae2cded2a6feeeea886a0f31f4f98450353340)
Signed-off-by: Andor Molnar <[email protected]>
---
.../src/main/resources/markdown/zookeeperAdmin.md | 4 +-
.../zookeeper/common/SSLContextAndOptions.java | 5 +-
.../java/org/apache/zookeeper/common/X509Util.java | 53 ++++++++++++++++++----
.../org/apache/zookeeper/common/X509UtilTest.java | 22 ++++++++-
4 files changed, 71 insertions(+), 13 deletions(-)
diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
index 4653ea08c..80aebcfa6 100644
--- a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
+++ b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
@@ -1709,13 +1709,13 @@ and [SASL authentication for
ZooKeeper](https://cwiki.apache.org/confluence/disp
(Java system properties: **zookeeper.ssl.protocol** and
**zookeeper.ssl.quorum.protocol**)
**New in 3.5.5:**
Specifies to protocol to be used in client and quorum TLS negotiation.
- Default: TLSv1.2
+ Default: TLSv1.3 or TLSv1.2 depending on Java runtime version being used.
* *ssl.enabledProtocols* and *ssl.quorum.enabledProtocols* :
(Java system properties: **zookeeper.ssl.enabledProtocols** and
**zookeeper.ssl.quorum.enabledProtocols**)
**New in 3.5.5:**
Specifies the enabled protocols in client and quorum TLS negotiation.
- Default: value of `protocol` property
+ Default: TLSv1.3, TLSv1.2 if value of `protocol` property is TLSv1.3.
TLSv1.2 if `protocol` is TLSv1.2.
* *ssl.ciphersuites* and *ssl.quorum.ciphersuites* :
(Java system properties: **zookeeper.ssl.ciphersuites** and
**zookeeper.ssl.quorum.ciphersuites**)
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 f5ce023c6..3a2542552 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
@@ -149,7 +149,10 @@ public class SSLContextAndOptions {
private String[] getEnabledProtocols(final ZKConfig config, final
SSLContext sslContext) {
String enabledProtocolsInput =
config.getProperty(x509Util.getSslEnabledProtocolsProperty());
if (enabledProtocolsInput == null) {
- return new String[]{sslContext.getProtocol()};
+ // Use JDK defaults for enabled protocols:
+ // Protocol TLSv1.3 -> enabled protocols TLSv1.3 and TLSv1.2
+ // Protocol TLSv1.2 -> enabled protocols TLSv1.2
+ return sslContext.getDefaultSSLParameters().getProtocols();
}
return enabledProtocolsInput.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 56b9f8ed1..a7a9fb7a3 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
@@ -33,14 +33,19 @@ import java.security.NoSuchAlgorithmException;
import java.security.Security;
import java.security.cert.PKIXBuilderParameters;
import java.security.cert.X509CertSelector;
+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;
@@ -69,6 +74,9 @@ public abstract class X509Util implements Closeable,
AutoCloseable {
private static final String REJECT_CLIENT_RENEGOTIATION_PROPERTY =
"jdk.tls.rejectClientInitiatedRenegotiation";
private static final String FIPS_MODE_PROPERTY = "zookeeper.fips-mode";
+ public static final String TLS_1_1 = "TLSv1.1";
+ public static final String TLS_1_2 = "TLSv1.2";
+ public static final String TLS_1_3 = "TLSv1.3";
static {
// Client-initiated renegotiation in TLS is unsafe and
@@ -82,7 +90,32 @@ public abstract class X509Util implements Closeable,
AutoCloseable {
}
}
- public static final String DEFAULT_PROTOCOL = "TLSv1.2";
+ public static final String DEFAULT_PROTOCOL = defaultTlsProtocol();
+
+ /**
+ * Return TLSv1.3 or TLSv1.2 depending on Java runtime version being used.
+ * TLSv1.3 was first introduced in JDK11 and back-ported to OpenJDK 8u272.
+ */
+ private static String defaultTlsProtocol() {
+ String defaultProtocol = TLS_1_2;
+ List<String> supported = new ArrayList<>();
+ try {
+ supported =
Arrays.asList(SSLContext.getDefault().getSupportedSSLParameters().getProtocols());
+ if (supported.contains(TLS_1_3)) {
+ defaultProtocol = TLS_1_3;
+ }
+ } catch (NoSuchAlgorithmException e) {
+ // Ignore.
+ }
+ LOG.info("Default TLS protocol is {}, supported TLS protocols are {}",
defaultProtocol, supported);
+ 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"};
}
@@ -91,18 +124,22 @@ public abstract class X509Util implements Closeable,
AutoCloseable {
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"};
}
- private static String[] concatArrays(String[] left, String[] right) {
- String[] result = new String[left.length + right.length];
- System.arraycopy(left, 0, result, 0, left.length);
- System.arraycopy(right, 0, result, left.length, right.length);
- return result;
+ /**
+ * 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 =
concatArrays(getCBCCiphers(), getGCMCiphers());
+ 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.
- private static final String[] DEFAULT_CIPHERS_JAVA9 =
concatArrays(getGCMCiphers(), getCBCCiphers());
+ // 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;
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 ddffd426d..1218a00de 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
@@ -31,6 +31,8 @@ import java.net.Socket;
import java.nio.file.Path;
import java.security.NoSuchAlgorithmException;
import java.security.Security;
+import java.util.Arrays;
+import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
@@ -102,6 +104,20 @@ public class X509UtilTest extends
BaseX509ParameterizedTestCase {
init(caKeyType, certKeyType, keyPassword, paramIndex);
SSLContext sslContext = x509Util.getDefaultSSLContext();
assertEquals(X509Util.DEFAULT_PROTOCOL, sslContext.getProtocol());
+
+ // Check that TLSv1.3 is selected in JDKs that support it (OpenJDK
8u272 and later).
+ List<String> supported =
Arrays.asList(SSLContext.getDefault().getSupportedSSLParameters().getProtocols());
+ if (supported.contains(X509Util.TLS_1_3)) {
+ // SSLContext protocol.
+ assertEquals(X509Util.TLS_1_3, sslContext.getProtocol());
+ // Enabled protocols.
+ List<String> protos =
Arrays.asList(sslContext.getDefaultSSLParameters().getProtocols());
+ assertTrue(protos.contains(X509Util.TLS_1_2));
+ assertTrue(protos.contains(X509Util.TLS_1_3));
+ } else {
+ assertEquals(X509Util.TLS_1_2, sslContext.getProtocol());
+ assertArrayEquals(new String[]{X509Util.TLS_1_2},
sslContext.getDefaultSSLParameters().getProtocols());
+ }
}
@ParameterizedTest
@@ -110,7 +126,7 @@ public class X509UtilTest extends
BaseX509ParameterizedTestCase {
public void testCreateSSLContextWithCustomProtocol(
X509KeyType caKeyType, X509KeyType certKeyType, String
keyPassword, Integer paramIndex)
throws Exception {
- final String protocol = "TLSv1.1";
+ final String protocol = X509Util.TLS_1_1;
init(caKeyType, certKeyType, keyPassword, paramIndex);
System.setProperty(x509Util.getSslProtocolProperty(), protocol);
SSLContext sslContext = x509Util.getDefaultSSLContext();
@@ -745,7 +761,8 @@ public class X509UtilTest extends
BaseX509ParameterizedTestCase {
}
// This test makes sure that client-initiated TLS renegotiation does not
- // succeed. We explicitly disable it at the top of X509Util.java.
+ // succeed when using TLSv1.2. We explicitly disable it at the top of
X509Util.java.
+ // Force TLSv1.2 since the renegotiation feature is not supported anymore
in TLSv1.3 and the test becomes invalid.
@ParameterizedTest
@MethodSource("data")
public void testClientRenegotiationFails(
@@ -768,6 +785,7 @@ public class X509UtilTest extends
BaseX509ParameterizedTestCase {
@Override
public SSLSocket call() throws Exception {
SSLSocket sslSocket = (SSLSocket)
listeningSocket.accept();
+ sslSocket.setEnabledProtocols(new
String[]{X509Util.TLS_1_2});
sslSocket.addHandshakeCompletedListener(new
HandshakeCompletedListener() {
@Override
public void
handshakeCompleted(HandshakeCompletedEvent handshakeCompletedEvent) {
