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 <tero.saa...@est.tech>
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 <tero.saa...@est.tech>
    
    * Fixed checkstyle errors
    
    Signed-off-by: Tero Saarni <tero.saa...@est.tech>
    
    ---------
    
    Signed-off-by: Tero Saarni <tero.saa...@est.tech>
    (cherry picked from commit 6cae2cded2a6feeeea886a0f31f4f98450353340)
    Signed-off-by: Andor Molnar <an...@apache.org>
---
 .../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) {

Reply via email to