Skip to content

Commit cf65137

Browse files
stotymeszinorbi
stoty
authored and
meszinorbi
committed
OMID-317 Add TLSv1.3 to default protocols if supported by JRE (#197)
1 parent 110f0ca commit cf65137

7 files changed

Lines changed: 50 additions & 25 deletions

File tree

‎common/src/main/java/org/apache/omid/tls/X509Util.java‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,18 +33,19 @@
3333
import java.nio.file.Files;
3434
import java.security.GeneralSecurityException;
3535
import java.security.KeyStore;
36+
import java.security.NoSuchAlgorithmException;
3637
import java.security.Security;
3738
import java.security.cert.PKIXBuilderParameters;
3839
import java.security.cert.X509CertSelector;
40+
import java.util.ArrayList;
3941
import java.util.Arrays;
42+
import java.util.List;
4043

4144

4245
/**
43-
* Utility code for X509 handling Default cipher suites: Performance testing done by Facebook
44-
* engineers shows that on Intel x86_64 machines, Java9 performs better with GCM and Java8 performs
45-
* better with CBC, so these seem like reasonable defaults.
46+
* Utility code for X509 handling Default cipher suites.
4647
* <p/>
47-
* This file has is based on the one in HBase project.
48+
* This file has is based on the one in HBase project, which is based on the one in Zookeeper.
4849
* @see <a href=
4950
* "https://github.com/apache/hbase/blob/d2b0074f7ad4c43d31a1a511a0d74feda72451d1/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/X509Util.java">Base
5051
* revision</a>
@@ -54,15 +55,37 @@ public final class X509Util {
5455
private static final Logger LOG = LoggerFactory.getLogger(X509Util.class);
5556
private static final char[] EMPTY_CHAR_ARRAY = new char[0];
5657

58+
public static final String TLS_1_2 = "TLSv1.2";
59+
public static final String TLS_1_3 = "TLSv1.3";
60+
5761
// Config
5862
public static final int DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS = 5000;
5963

60-
public static final String DEFAULT_PROTOCOL = "TLSv1.2";
64+
public static final String DEFAULT_PROTOCOLS = defaultTlsProtocols();
6165

6266
private X509Util() {
6367
// disabled
6468
}
6569

70+
// For recent JDKs we could have just used the JVM defaults.
71+
// This is only needed for pre JDK-8202343 Java 8 and 11 to avoid a regression of allowing
72+
// pre TLSv1.2 protocols by default.
73+
// As Omid only supports the JRE provider, we don't need to worry about tcnative stuff.
74+
private static String defaultTlsProtocols() {
75+
String defaultProtocol = TLS_1_2;
76+
List<String> supported = new ArrayList<>();
77+
try {
78+
supported = Arrays.asList(SSLContext.getDefault().getSupportedSSLParameters().getProtocols());
79+
if (supported.contains(TLS_1_3)) {
80+
defaultProtocol = TLS_1_3 + "," + TLS_1_2;
81+
}
82+
} catch (NoSuchAlgorithmException e) {
83+
// Ignore.
84+
}
85+
LOG.info("Default TLS protocols are {}, supported TLS protocols are {}", defaultProtocol, supported);
86+
return defaultProtocol;
87+
}
88+
6689
public static SslContext createSslContextForClient(String keyStoreLocation, char[] keyStorePassword,
6790
String keyStoreType, String trustStoreLocation, char[] trustStorePassword, String trustStoreType,
6891
boolean sslCrlEnabled, boolean sslOcspEnabled, String enabledProtocols, String cipherSuites, String tlsConfigProtocols)
@@ -229,7 +252,7 @@ static X509TrustManager createTrustManager(String trustStoreLocation, char[] tru
229252

230253
private static String[] getEnabledProtocols(String enabledProtocolsInput, String tlsConfigProtocols) {
231254
if (enabledProtocolsInput == null) {
232-
return new String[] {tlsConfigProtocols};
255+
return tlsConfigProtocols.split(",");
233256
}
234257
return enabledProtocolsInput.split(",");
235258
}

‎common/src/test/java/org/apache/omid/tls/TestX509Util.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,9 @@ public void cleanUp() {
101101
@Test
102102
public void testCreateSSLContextWithoutCustomProtocol() throws Exception {
103103
init(caKeyType, certKeyType, keyPassword, paramIndex);
104-
SslContext sslContext = X509Util.createSslContextForClient(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOL);
104+
SslContext sslContext = X509Util.createSslContextForClient(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOLS);
105105
ByteBufAllocator byteBufAllocatorMock = mock(ByteBufAllocator.class);
106-
assertEquals(new String[] { X509Util.DEFAULT_PROTOCOL },
106+
assertEquals(X509Util.DEFAULT_PROTOCOLS.split(","),
107107
sslContext.newEngine(byteBufAllocatorMock).getEnabledProtocols());
108108
}
109109

@@ -113,7 +113,7 @@ public void testCreateSSLContextWithCustomProtocol() throws Exception {
113113
init(caKeyType, certKeyType, keyPassword, paramIndex);
114114

115115
ByteBufAllocator byteBufAllocatorMock = mock(ByteBufAllocator.class);
116-
SslContext sslContext = X509Util.createSslContextForClient(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, protocol, null, X509Util.DEFAULT_PROTOCOL);
116+
SslContext sslContext = X509Util.createSslContextForClient(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, protocol, null, X509Util.DEFAULT_PROTOCOLS);
117117
assertEquals(Collections.singletonList(protocol),
118118
Arrays.asList(sslContext.newEngine(byteBufAllocatorMock).getEnabledProtocols()));
119119
}
@@ -122,14 +122,14 @@ public void testCreateSSLContextWithCustomProtocol() throws Exception {
122122
public void testCreateSSLContextWithoutKeyStoreLocationServer() throws Exception {
123123
init(caKeyType, certKeyType, keyPassword, paramIndex);
124124
tlsConfigKeystoreLocation = "";
125-
SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOL);
125+
SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOLS);
126126
}
127127

128128
@Test
129129
public void testCreateSSLContextWithoutKeyStoreLocationClient() throws Exception {
130130
init(caKeyType, certKeyType, keyPassword, paramIndex);
131131
tlsConfigKeystoreLocation = "";
132-
SslContext sslContext = X509Util.createSslContextForClient(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOL);
132+
SslContext sslContext = X509Util.createSslContextForClient(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOLS);
133133
}
134134

135135
@Test(expected = X509Exception.class)
@@ -139,29 +139,29 @@ public void testCreateSSLContextWithoutKeyStorePassword() throws Exception {
139139
throw new X509Exception.SSLContextException("");
140140
}
141141
tlsConfigKeystorePassword = "";
142-
SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOL);
142+
SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOLS);
143143
}
144144

145145
@Test
146146
public void testCreateSSLContextWithoutTrustStoreLocationClient() throws Exception {
147147
init(caKeyType, certKeyType, keyPassword, paramIndex);
148148
tlsConfigTrustLocation = "";
149-
SslContext sslContext = X509Util.createSslContextForClient(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOL);
149+
SslContext sslContext = X509Util.createSslContextForClient(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOLS);
150150
}
151151

152152
@Test
153153
public void testCreateSSLContextWithoutTrustStoreLocationServer() throws Exception {
154154
init(caKeyType, certKeyType, keyPassword, paramIndex);
155155
tlsConfigTrustLocation = "";
156-
SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOL);
156+
SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOLS);
157157
}
158158

159159
// It would be great to test the value of PKIXBuilderParameters#setRevocationEnabled,
160160
// but it does not appear to be possible
161161
@Test
162162
public void testCRLEnabled() throws Exception {
163163
init(caKeyType, certKeyType, keyPassword, paramIndex);
164-
SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, true, false, null, null, X509Util.DEFAULT_PROTOCOL);
164+
SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, true, false, null, null, X509Util.DEFAULT_PROTOCOLS);
165165
assertTrue(Boolean.valueOf(System.getProperty("com.sun.net.ssl.checkRevocation")));
166166
assertTrue(Boolean.valueOf(System.getProperty("com.sun.security.enableCRLDP")));
167167
assertFalse(Boolean.valueOf(Security.getProperty("ocsp.enable")));
@@ -170,7 +170,7 @@ public void testCRLEnabled() throws Exception {
170170
@Test
171171
public void testCRLDisabled() throws Exception {
172172
init(caKeyType, certKeyType, keyPassword, paramIndex);
173-
SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOL);
173+
SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOLS);
174174
assertFalse(Boolean.valueOf(System.getProperty("com.sun.net.ssl.checkRevocation")));
175175
assertFalse(Boolean.valueOf(System.getProperty("com.sun.security.enableCRLDP")));
176176
assertFalse(Boolean.valueOf(Security.getProperty("ocsp.enable")));

‎transaction-client/src/main/java/org/apache/omid/tso/client/OmidClientConfiguration.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import org.apache.phoenix.thirdparty.com.google.common.annotations.VisibleForTesting;
2424

2525
import static org.apache.omid.tls.X509Util.DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS;
26-
import static org.apache.omid.tls.X509Util.DEFAULT_PROTOCOL;
26+
import static org.apache.omid.tls.X509Util.DEFAULT_PROTOCOLS;
2727

2828
/**
2929
* Configuration for Omid client side
@@ -83,7 +83,7 @@ public enum ConflictDetectionLevel {CELL, ROW}
8383

8484
private String cipherSuites;
8585

86-
private String tlsConfigProtocols = DEFAULT_PROTOCOL;
86+
private String tlsConfigProtocols = DEFAULT_PROTOCOLS;
8787

8888
// ----------------------------------------------------------------------------------------------------------------
8989
// Instantiation

‎transaction-client/src/test/java/org/apache/omid/tso/client/TestGetSslContext.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public void testYamlReading() throws Exception {
6868
SslContext sslContext = tsoClient.getSslContext(tsoClientConf);
6969

7070
ByteBufAllocator byteBufAllocatorMock = mock(ByteBufAllocator.class);
71-
Assert.assertEquals(new String[] { X509Util.DEFAULT_PROTOCOL },
71+
Assert.assertEquals(X509Util.DEFAULT_PROTOCOLS.split(","),
7272
sslContext.newEngine(byteBufAllocatorMock).getEnabledProtocols());
7373

7474
Assert.assertEquals(new String[] { cipherSuite },

‎transaction-client/src/test/java/org/apache/omid/tso/client/TestOmidClientConfiguration.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.testng.Assert;
2121
import org.testng.annotations.Test;
2222
import org.apache.omid.tso.client.OmidClientConfiguration.ConnType;
23+
import org.apache.omid.tls.X509Util;
2324
import org.apache.omid.tso.client.OmidClientConfiguration.ConflictDetectionLevel;
2425

2526
public class TestOmidClientConfiguration {
@@ -30,7 +31,7 @@ public void testYamlReading() {
3031
Assert.assertEquals(configuration.getConnectionString(), "localhost:24758");
3132
Assert.assertEquals(configuration.getConnectionType(), ConnType.DIRECT);
3233
Assert.assertEquals(configuration.getEnabledProtocols(), null);
33-
Assert.assertEquals(configuration.getTsConfigProtocols(), "TLSv1.2");
34+
Assert.assertEquals(configuration.getTsConfigProtocols(), X509Util.DEFAULT_PROTOCOLS);
3435
Assert.assertEquals(configuration.getTlsEnabled(), false);
3536
Assert.assertEquals(configuration.getKeyStoreLocation(), "");
3637
Assert.assertEquals(configuration.getKeyStorePassword(), "");
@@ -46,7 +47,7 @@ public void testCustomYamlReading() {
4647
Assert.assertEquals(configuration.getConnectionString(), "localhost:24758");
4748
Assert.assertEquals(configuration.getConnectionType(), ConnType.DIRECT);
4849
Assert.assertEquals(configuration.getEnabledProtocols(), "TLSv1.2");
49-
Assert.assertEquals(configuration.getTsConfigProtocols(), "TLSv1.2");
50+
Assert.assertEquals(configuration.getTsConfigProtocols(), X509Util.DEFAULT_PROTOCOLS);
5051
Assert.assertEquals(configuration.getTlsEnabled(), true);
5152
Assert.assertEquals(configuration.getKeyStoreLocation(), "/asd");
5253
Assert.assertEquals(configuration.getKeyStorePassword(), "pass");

‎tso-server/src/main/java/org/apache/omid/tso/TSOServerConfig.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
import org.slf4j.Logger;
2828
import org.slf4j.LoggerFactory;
2929

30-
import static org.apache.omid.tls.X509Util.DEFAULT_PROTOCOL;
30+
import static org.apache.omid.tls.X509Util.DEFAULT_PROTOCOLS;
3131

3232
/**
3333
* Reads the configuration parameters of a TSO server instance from CONFIG_FILE_NAME.
@@ -118,7 +118,7 @@ public TSOServerConfig() {
118118

119119
private String cipherSuites;
120120

121-
private String tlsConfigProtocols = DEFAULT_PROTOCOL;
121+
private String tlsConfigProtocols = DEFAULT_PROTOCOLS;
122122

123123
public boolean getMonitorContext() {
124124
return monitorContext;

‎tso-server/src/test/java/org/apache/omid/tso/TSOServerConfigTest.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
*/
1818
package org.apache.omid.tso;
1919

20+
import org.apache.omid.tls.X509Util;
2021
import org.testng.Assert;
2122
import org.testng.annotations.Test;
2223

@@ -28,7 +29,7 @@ public void testParsesOK() throws Exception {
2829
Assert.assertEquals(tsoServerConfig.getTlsEnabled(), false);
2930
Assert.assertEquals(tsoServerConfig.getSupportPlainText(), true);
3031
Assert.assertEquals(tsoServerConfig.getEnabledProtocols(), null);
31-
Assert.assertEquals(tsoServerConfig.getTsConfigProtocols(), "TLSv1.2");
32+
Assert.assertEquals(tsoServerConfig.getTsConfigProtocols(), X509Util.DEFAULT_PROTOCOLS);
3233
Assert.assertEquals(tsoServerConfig.getKeyStoreLocation(), "");
3334
Assert.assertEquals(tsoServerConfig.getKeyStorePassword(), "");
3435
Assert.assertEquals(tsoServerConfig.getKeyStoreType(), "");
@@ -45,7 +46,7 @@ public void testCustomParseK() throws Exception {
4546
Assert.assertEquals(tsoServerConfig.getSupportPlainText(), false);
4647

4748
Assert.assertEquals(tsoServerConfig.getEnabledProtocols(), "TLSv1.2");
48-
Assert.assertEquals(tsoServerConfig.getTsConfigProtocols(), "TLSv1.2");
49+
Assert.assertEquals(tsoServerConfig.getTsConfigProtocols(), X509Util.DEFAULT_PROTOCOLS);
4950

5051
Assert.assertEquals(tsoServerConfig.getKeyStoreLocation(), "/asd");
5152
Assert.assertEquals(tsoServerConfig.getKeyStorePassword(), "pass");

0 commit comments

Comments
 (0)