From 6cae2cded2a6feeeea886a0f31f4f98450353340 Mon Sep 17 00:00:00 2001 From: Tero Saarni Date: Mon, 2 Oct 2023 07:49:19 +0300 Subject: [PATCH] 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 * Fixed checkstyle errors Signed-off-by: Tero Saarni --------- Signed-off-by: Tero Saarni --- .../main/resources/markdown/zookeeperAdmin.md | 4 +- .../common/SSLContextAndOptions.java | 5 +- .../org/apache/zookeeper/common/X509Util.java | 53 ++++++++++++++++--- .../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 7a99cd346c6..59b995e2f15 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 f5ce023c653..3a254255292 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 @@ private void configureSslParameters(SSLParameters sslParameters, boolean isClien 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 56b9f8ed1c6..a7a9fb7a3d5 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.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 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 @@ 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"}; } - 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 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 ddffd426dd8..1218a00de30 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.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 void testCreateSSLContextWithoutCustomProtocol( 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 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 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 void testCreateSSLContextWithoutCustomProtocol( 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 @@ private static void forceClose(ServerSocket s) { } // 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 void testClientRenegotiationFails( @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) {