Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 6cae2cd

Browse files
authoredOct 2, 2023
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]>
1 parent 5823a3f commit 6cae2cd

File tree

4 files changed

+71
-13
lines changed

4 files changed

+71
-13
lines changed
 

‎zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1709,13 +1709,13 @@ and [SASL authentication for ZooKeeper](https://cwiki.apache.org/confluence/disp
17091709
(Java system properties: **zookeeper.ssl.protocol** and **zookeeper.ssl.quorum.protocol**)
17101710
**New in 3.5.5:**
17111711
Specifies to protocol to be used in client and quorum TLS negotiation.
1712-
Default: TLSv1.2
1712+
Default: TLSv1.3 or TLSv1.2 depending on Java runtime version being used.
17131713

17141714
* *ssl.enabledProtocols* and *ssl.quorum.enabledProtocols* :
17151715
(Java system properties: **zookeeper.ssl.enabledProtocols** and **zookeeper.ssl.quorum.enabledProtocols**)
17161716
**New in 3.5.5:**
17171717
Specifies the enabled protocols in client and quorum TLS negotiation.
1718-
Default: value of `protocol` property
1718+
Default: TLSv1.3, TLSv1.2 if value of `protocol` property is TLSv1.3. TLSv1.2 if `protocol` is TLSv1.2.
17191719

17201720
* *ssl.ciphersuites* and *ssl.quorum.ciphersuites* :
17211721
(Java system properties: **zookeeper.ssl.ciphersuites** and **zookeeper.ssl.quorum.ciphersuites**)

‎zookeeper-server/src/main/java/org/apache/zookeeper/common/SSLContextAndOptions.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,10 @@ private void configureSslParameters(SSLParameters sslParameters, boolean isClien
149149
private String[] getEnabledProtocols(final ZKConfig config, final SSLContext sslContext) {
150150
String enabledProtocolsInput = config.getProperty(x509Util.getSslEnabledProtocolsProperty());
151151
if (enabledProtocolsInput == null) {
152-
return new String[]{sslContext.getProtocol()};
152+
// Use JDK defaults for enabled protocols:
153+
// Protocol TLSv1.3 -> enabled protocols TLSv1.3 and TLSv1.2
154+
// Protocol TLSv1.2 -> enabled protocols TLSv1.2
155+
return sslContext.getDefaultSSLParameters().getProtocols();
153156
}
154157
return enabledProtocolsInput.split(",");
155158
}

‎zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,19 @@
3333
import java.security.Security;
3434
import java.security.cert.PKIXBuilderParameters;
3535
import java.security.cert.X509CertSelector;
36+
import java.util.ArrayList;
37+
import java.util.Arrays;
38+
import java.util.List;
3639
import java.util.Objects;
3740
import java.util.concurrent.atomic.AtomicReference;
3841
import java.util.function.Supplier;
42+
import java.util.stream.Collectors;
3943
import javax.net.ssl.CertPathTrustManagerParameters;
4044
import javax.net.ssl.KeyManager;
4145
import javax.net.ssl.KeyManagerFactory;
4246
import javax.net.ssl.SSLContext;
4347
import javax.net.ssl.SSLServerSocket;
48+
import javax.net.ssl.SSLServerSocketFactory;
4449
import javax.net.ssl.SSLSocket;
4550
import javax.net.ssl.TrustManager;
4651
import javax.net.ssl.TrustManagerFactory;
@@ -69,6 +74,9 @@ public abstract class X509Util implements Closeable, AutoCloseable {
6974

7075
private static final String REJECT_CLIENT_RENEGOTIATION_PROPERTY = "jdk.tls.rejectClientInitiatedRenegotiation";
7176
private static final String FIPS_MODE_PROPERTY = "zookeeper.fips-mode";
77+
public static final String TLS_1_1 = "TLSv1.1";
78+
public static final String TLS_1_2 = "TLSv1.2";
79+
public static final String TLS_1_3 = "TLSv1.3";
7280

7381
static {
7482
// Client-initiated renegotiation in TLS is unsafe and
@@ -82,7 +90,32 @@ public abstract class X509Util implements Closeable, AutoCloseable {
8290
}
8391
}
8492

85-
public static final String DEFAULT_PROTOCOL = "TLSv1.2";
93+
public static final String DEFAULT_PROTOCOL = defaultTlsProtocol();
94+
95+
/**
96+
* Return TLSv1.3 or TLSv1.2 depending on Java runtime version being used.
97+
* TLSv1.3 was first introduced in JDK11 and back-ported to OpenJDK 8u272.
98+
*/
99+
private static String defaultTlsProtocol() {
100+
String defaultProtocol = TLS_1_2;
101+
List<String> supported = new ArrayList<>();
102+
try {
103+
supported = Arrays.asList(SSLContext.getDefault().getSupportedSSLParameters().getProtocols());
104+
if (supported.contains(TLS_1_3)) {
105+
defaultProtocol = TLS_1_3;
106+
}
107+
} catch (NoSuchAlgorithmException e) {
108+
// Ignore.
109+
}
110+
LOG.info("Default TLS protocol is {}, supported TLS protocols are {}", defaultProtocol, supported);
111+
return defaultProtocol;
112+
}
113+
114+
// ChaCha20 was introduced in OpenJDK 11.0.15 and it is not supported by JDK8.
115+
private static String[] getTLSv13Ciphers() {
116+
return new String[]{"TLS_AES_256_GCM_SHA384", "TLS_AES_128_GCM_SHA256", "TLS_CHACHA20_POLY1305_SHA256"};
117+
}
118+
86119
private static String[] getGCMCiphers() {
87120
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"};
88121
}
@@ -91,18 +124,22 @@ private static String[] getCBCCiphers() {
91124
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"};
92125
}
93126

94-
private static String[] concatArrays(String[] left, String[] right) {
95-
String[] result = new String[left.length + right.length];
96-
System.arraycopy(left, 0, result, 0, left.length);
97-
System.arraycopy(right, 0, result, left.length, right.length);
98-
return result;
127+
/**
128+
* Returns a filtered set of ciphers, where ciphers not supported by the JDK are removed.
129+
*/
130+
private static String[] getSupportedCiphers(String[]... cipherLists) {
131+
List<String> supported = Arrays.asList(
132+
((SSLServerSocketFactory) SSLServerSocketFactory.getDefault()).getSupportedCipherSuites());
133+
134+
return Arrays.stream(cipherLists).flatMap(Arrays::stream).filter(supported::contains).collect(Collectors.toList()).toArray(new String[0]);
99135
}
100136

101137
// On Java 8, prefer CBC ciphers since AES-NI support is lacking and GCM is slower than CBC.
102-
private static final String[] DEFAULT_CIPHERS_JAVA8 = concatArrays(getCBCCiphers(), getGCMCiphers());
138+
private static final String[] DEFAULT_CIPHERS_JAVA8 = getSupportedCiphers(getCBCCiphers(), getGCMCiphers(), getTLSv13Ciphers());
103139
// On Java 9 and later, prefer GCM ciphers due to improved AES-NI support.
104140
// Note that this performance assumption might not hold true for architectures other than x86_64.
105-
private static final String[] DEFAULT_CIPHERS_JAVA9 = concatArrays(getGCMCiphers(), getCBCCiphers());
141+
// TLSv1.3 ciphers can be added at the end of the list without impacting the priority of TLSv1.3 vs TLSv1.2.
142+
private static final String[] DEFAULT_CIPHERS_JAVA9 = getSupportedCiphers(getGCMCiphers(), getCBCCiphers(), getTLSv13Ciphers());
106143

107144
public static final int DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS = 5000;
108145

‎zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
import java.nio.file.Path;
3232
import java.security.NoSuchAlgorithmException;
3333
import java.security.Security;
34+
import java.util.Arrays;
35+
import java.util.List;
3436
import java.util.concurrent.Callable;
3537
import java.util.concurrent.CountDownLatch;
3638
import java.util.concurrent.ExecutionException;
@@ -102,6 +104,20 @@ public void testCreateSSLContextWithoutCustomProtocol(
102104
init(caKeyType, certKeyType, keyPassword, paramIndex);
103105
SSLContext sslContext = x509Util.getDefaultSSLContext();
104106
assertEquals(X509Util.DEFAULT_PROTOCOL, sslContext.getProtocol());
107+
108+
// Check that TLSv1.3 is selected in JDKs that support it (OpenJDK 8u272 and later).
109+
List<String> supported = Arrays.asList(SSLContext.getDefault().getSupportedSSLParameters().getProtocols());
110+
if (supported.contains(X509Util.TLS_1_3)) {
111+
// SSLContext protocol.
112+
assertEquals(X509Util.TLS_1_3, sslContext.getProtocol());
113+
// Enabled protocols.
114+
List<String> protos = Arrays.asList(sslContext.getDefaultSSLParameters().getProtocols());
115+
assertTrue(protos.contains(X509Util.TLS_1_2));
116+
assertTrue(protos.contains(X509Util.TLS_1_3));
117+
} else {
118+
assertEquals(X509Util.TLS_1_2, sslContext.getProtocol());
119+
assertArrayEquals(new String[]{X509Util.TLS_1_2}, sslContext.getDefaultSSLParameters().getProtocols());
120+
}
105121
}
106122

107123
@ParameterizedTest
@@ -110,7 +126,7 @@ public void testCreateSSLContextWithoutCustomProtocol(
110126
public void testCreateSSLContextWithCustomProtocol(
111127
X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex)
112128
throws Exception {
113-
final String protocol = "TLSv1.1";
129+
final String protocol = X509Util.TLS_1_1;
114130
init(caKeyType, certKeyType, keyPassword, paramIndex);
115131
System.setProperty(x509Util.getSslProtocolProperty(), protocol);
116132
SSLContext sslContext = x509Util.getDefaultSSLContext();
@@ -745,7 +761,8 @@ private static void forceClose(ServerSocket s) {
745761
}
746762

747763
// This test makes sure that client-initiated TLS renegotiation does not
748-
// succeed. We explicitly disable it at the top of X509Util.java.
764+
// succeed when using TLSv1.2. We explicitly disable it at the top of X509Util.java.
765+
// Force TLSv1.2 since the renegotiation feature is not supported anymore in TLSv1.3 and the test becomes invalid.
749766
@ParameterizedTest
750767
@MethodSource("data")
751768
public void testClientRenegotiationFails(
@@ -768,6 +785,7 @@ public void testClientRenegotiationFails(
768785
@Override
769786
public SSLSocket call() throws Exception {
770787
SSLSocket sslSocket = (SSLSocket) listeningSocket.accept();
788+
sslSocket.setEnabledProtocols(new String[]{X509Util.TLS_1_2});
771789
sslSocket.addHandshakeCompletedListener(new HandshakeCompletedListener() {
772790
@Override
773791
public void handshakeCompleted(HandshakeCompletedEvent handshakeCompletedEvent) {

0 commit comments

Comments
 (0)
Please sign in to comment.