OMID-317 Add TLSv1.3 to default protocols if supported by JRE

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

@@ -33,18 +33,19 @@
 import java.nio.file.Files;
 import java.security.GeneralSecurityException;
 import java.security.KeyStore;
+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;
 
 
 /**
- * Utility code for X509 handling Default cipher suites: Performance testing done by Facebook
- * engineers shows that on Intel x86_64 machines, Java9 performs better with GCM and Java8 performs
- * better with CBC, so these seem like reasonable defaults.
+ * Utility code for X509 handling Default cipher suites.
  * <p/>
- * This file has is based on the one in HBase project.
+ * This file has is based on the one in HBase project, which is based on the one in Zookeeper.
  * @see <a href=
  *      "https://github.com/apache/hbase/blob/d2b0074f7ad4c43d31a1a511a0d74feda72451d1/hbase-common/src/main/java/org/apache/hadoop/hbase/io/crypto/tls/X509Util.java">Base
  *      revision</a>
@@ -54,15 +55,37 @@ public final class X509Util {
     private static final Logger LOG = LoggerFactory.getLogger(X509Util.class);
     private static final char[] EMPTY_CHAR_ARRAY = new char[0];
 
+    public static final String TLS_1_2 = "TLSv1.2";
+    public static final String TLS_1_3 = "TLSv1.3";
+
     // Config
     public static final int DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS = 5000;
 
-    public static final String DEFAULT_PROTOCOL = "TLSv1.2";
+    public static final String DEFAULT_PROTOCOLS = defaultTlsProtocols();
 
     private X509Util() {
         // disabled
     }
 
+    // For recent JDKs we could have just used the JVM defaults.
+    // This is only needed for pre JDK-8202343 Java 8 and 11 to avoid a regression of allowing
+    // pre TLSv1.2 protocols by default.
+    // As Omid only supports the JRE provider, we don't need to worry about tcnative stuff.
+    private static String defaultTlsProtocols() {
+        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 + "," + TLS_1_2;
+            }
+        } catch (NoSuchAlgorithmException e) {
+            // Ignore.
+        }
+        LOG.info("Default TLS protocols are {}, supported TLS protocols are {}", defaultProtocol, supported);
+        return defaultProtocol;
+    }
+
     public static SslContext createSslContextForClient(String keyStoreLocation, char[] keyStorePassword,
                                                        String keyStoreType, String trustStoreLocation, char[] trustStorePassword, String trustStoreType,
                                                        boolean sslCrlEnabled, boolean sslOcspEnabled, String enabledProtocols, String cipherSuites, String tlsConfigProtocols)
@@ -229,7 +252,7 @@ static X509TrustManager createTrustManager(String trustStoreLocation, char[] tru
 
     private static String[] getEnabledProtocols(String enabledProtocolsInput, String tlsConfigProtocols) {
         if (enabledProtocolsInput == null) {
-            return new String[] {tlsConfigProtocols};
+            return tlsConfigProtocols.split(",");
         }
         return enabledProtocolsInput.split(",");
     }

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

@@ -101,9 +101,9 @@ public void cleanUp() {
     @Test
     public void testCreateSSLContextWithoutCustomProtocol() throws Exception {
         init(caKeyType, certKeyType, keyPassword, paramIndex);
-        SslContext sslContext = X509Util.createSslContextForClient(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOL);
+        SslContext sslContext = X509Util.createSslContextForClient(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOLS);
         ByteBufAllocator byteBufAllocatorMock = mock(ByteBufAllocator.class);
-        assertEquals(new String[] { X509Util.DEFAULT_PROTOCOL },
+        assertEquals(X509Util.DEFAULT_PROTOCOLS.split(","),
                 sslContext.newEngine(byteBufAllocatorMock).getEnabledProtocols());
     }
 
@@ -113,7 +113,7 @@ public void testCreateSSLContextWithCustomProtocol() throws Exception {
         init(caKeyType, certKeyType, keyPassword, paramIndex);
 
         ByteBufAllocator byteBufAllocatorMock = mock(ByteBufAllocator.class);
-        SslContext sslContext = X509Util.createSslContextForClient(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, protocol, null, X509Util.DEFAULT_PROTOCOL);
+        SslContext sslContext = X509Util.createSslContextForClient(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, protocol, null, X509Util.DEFAULT_PROTOCOLS);
         assertEquals(Collections.singletonList(protocol),
                 Arrays.asList(sslContext.newEngine(byteBufAllocatorMock).getEnabledProtocols()));
     }
@@ -122,14 +122,14 @@ public void testCreateSSLContextWithCustomProtocol() throws Exception {
     public void testCreateSSLContextWithoutKeyStoreLocationServer() throws Exception {
         init(caKeyType, certKeyType, keyPassword, paramIndex);
         tlsConfigKeystoreLocation = "";
-        SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOL);
+        SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOLS);
     }
 
     @Test
     public void testCreateSSLContextWithoutKeyStoreLocationClient() throws Exception {
         init(caKeyType, certKeyType, keyPassword, paramIndex);
         tlsConfigKeystoreLocation = "";
-        SslContext sslContext = X509Util.createSslContextForClient(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOL);
+        SslContext sslContext = X509Util.createSslContextForClient(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOLS);
     }
 
     @Test(expected = X509Exception.class)
@@ -139,29 +139,29 @@ public void testCreateSSLContextWithoutKeyStorePassword() throws Exception {
             throw new X509Exception.SSLContextException("");
         }
         tlsConfigKeystorePassword = "";
-        SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOL);
+        SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOLS);
     }
 
     @Test
     public void testCreateSSLContextWithoutTrustStoreLocationClient() throws Exception {
         init(caKeyType, certKeyType, keyPassword, paramIndex);
         tlsConfigTrustLocation = "";
-        SslContext sslContext = X509Util.createSslContextForClient(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOL);
+        SslContext sslContext = X509Util.createSslContextForClient(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOLS);
     }
 
     @Test
     public void testCreateSSLContextWithoutTrustStoreLocationServer() throws Exception {
         init(caKeyType, certKeyType, keyPassword, paramIndex);
         tlsConfigTrustLocation = "";
-        SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOL);
+        SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOLS);
     }
 
     // It would be great to test the value of PKIXBuilderParameters#setRevocationEnabled,
     // but it does not appear to be possible
     @Test
     public void testCRLEnabled() throws Exception {
         init(caKeyType, certKeyType, keyPassword, paramIndex);
-        SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, true, false, null, null, X509Util.DEFAULT_PROTOCOL);
+        SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, true, false, null, null, X509Util.DEFAULT_PROTOCOLS);
         assertTrue(Boolean.valueOf(System.getProperty("com.sun.net.ssl.checkRevocation")));
         assertTrue(Boolean.valueOf(System.getProperty("com.sun.security.enableCRLDP")));
         assertFalse(Boolean.valueOf(Security.getProperty("ocsp.enable")));
@@ -170,7 +170,7 @@ public void testCRLEnabled() throws Exception {
     @Test
     public void testCRLDisabled() throws Exception {
         init(caKeyType, certKeyType, keyPassword, paramIndex);
-        SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOL);
+        SslContext sslContext = X509Util.createSslContextForServer(tlsConfigKeystoreLocation, tlsConfigKeystorePassword.toCharArray(), tlsConfigKeystoreType, tlsConfigTrustLocation, tlsConfigTrustPassword.toCharArray(), tlsConfigTrustType, false, false, null, null, X509Util.DEFAULT_PROTOCOLS);
         assertFalse(Boolean.valueOf(System.getProperty("com.sun.net.ssl.checkRevocation")));
         assertFalse(Boolean.valueOf(System.getProperty("com.sun.security.enableCRLDP")));
         assertFalse(Boolean.valueOf(Security.getProperty("ocsp.enable")));

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

@@ -23,7 +23,7 @@
 import org.apache.phoenix.thirdparty.com.google.common.annotations.VisibleForTesting;
 
 import static org.apache.omid.tls.X509Util.DEFAULT_HANDSHAKE_DETECTION_TIMEOUT_MILLIS;
-import static org.apache.omid.tls.X509Util.DEFAULT_PROTOCOL;
+import static org.apache.omid.tls.X509Util.DEFAULT_PROTOCOLS;
 
 /**
  * Configuration for Omid client side
@@ -83,7 +83,7 @@ public enum ConflictDetectionLevel {CELL, ROW}
 
     private String cipherSuites;
 
-    private String tlsConfigProtocols = DEFAULT_PROTOCOL;
+    private String tlsConfigProtocols = DEFAULT_PROTOCOLS;
 
     // ----------------------------------------------------------------------------------------------------------------
     // Instantiation

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

@@ -68,7 +68,7 @@ public void testYamlReading() throws Exception {
         SslContext sslContext = tsoClient.getSslContext(tsoClientConf);
 
         ByteBufAllocator byteBufAllocatorMock = mock(ByteBufAllocator.class);
-        Assert.assertEquals(new String[] { X509Util.DEFAULT_PROTOCOL },
+        Assert.assertEquals(X509Util.DEFAULT_PROTOCOLS.split(","),
                 sslContext.newEngine(byteBufAllocatorMock).getEnabledProtocols());
 
         Assert.assertEquals(new String[] { cipherSuite },

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

@@ -20,6 +20,7 @@
 import org.testng.Assert;
 import org.testng.annotations.Test;
 import org.apache.omid.tso.client.OmidClientConfiguration.ConnType;
+import org.apache.omid.tls.X509Util;
 import org.apache.omid.tso.client.OmidClientConfiguration.ConflictDetectionLevel;
 
 public class TestOmidClientConfiguration {
@@ -30,7 +31,7 @@ public void testYamlReading() {
         Assert.assertEquals(configuration.getConnectionString(), "localhost:24758");
         Assert.assertEquals(configuration.getConnectionType(), ConnType.DIRECT);
         Assert.assertEquals(configuration.getEnabledProtocols(), null);
-        Assert.assertEquals(configuration.getTsConfigProtocols(), "TLSv1.2");
+        Assert.assertEquals(configuration.getTsConfigProtocols(), X509Util.DEFAULT_PROTOCOLS);
         Assert.assertEquals(configuration.getTlsEnabled(), false);
         Assert.assertEquals(configuration.getKeyStoreLocation(), "");
         Assert.assertEquals(configuration.getKeyStorePassword(), "");
@@ -46,7 +47,7 @@ public void testCustomYamlReading() {
         Assert.assertEquals(configuration.getConnectionString(), "localhost:24758");
         Assert.assertEquals(configuration.getConnectionType(), ConnType.DIRECT);
         Assert.assertEquals(configuration.getEnabledProtocols(), "TLSv1.2");
-        Assert.assertEquals(configuration.getTsConfigProtocols(), "TLSv1.2");
+        Assert.assertEquals(configuration.getTsConfigProtocols(), X509Util.DEFAULT_PROTOCOLS);
         Assert.assertEquals(configuration.getTlsEnabled(), true);
         Assert.assertEquals(configuration.getKeyStoreLocation(), "/asd");
         Assert.assertEquals(configuration.getKeyStorePassword(), "pass");

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

@@ -27,7 +27,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static org.apache.omid.tls.X509Util.DEFAULT_PROTOCOL;
+import static org.apache.omid.tls.X509Util.DEFAULT_PROTOCOLS;
 
 /**
  * Reads the configuration parameters of a TSO server instance from CONFIG_FILE_NAME.
@@ -118,7 +118,7 @@ public TSOServerConfig() {
 
     private String cipherSuites;
 
-    private String tlsConfigProtocols = DEFAULT_PROTOCOL;
+    private String tlsConfigProtocols = DEFAULT_PROTOCOLS;
 
     public boolean getMonitorContext() {
         return monitorContext;

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

@@ -17,6 +17,7 @@
  */
 package org.apache.omid.tso;
 
+import org.apache.omid.tls.X509Util;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
@@ -28,7 +29,7 @@ public void testParsesOK() throws Exception {
         Assert.assertEquals(tsoServerConfig.getTlsEnabled(), false);
         Assert.assertEquals(tsoServerConfig.getSupportPlainText(), true);
         Assert.assertEquals(tsoServerConfig.getEnabledProtocols(), null);
-        Assert.assertEquals(tsoServerConfig.getTsConfigProtocols(), "TLSv1.2");
+        Assert.assertEquals(tsoServerConfig.getTsConfigProtocols(), X509Util.DEFAULT_PROTOCOLS);
         Assert.assertEquals(tsoServerConfig.getKeyStoreLocation(), "");
         Assert.assertEquals(tsoServerConfig.getKeyStorePassword(), "");
         Assert.assertEquals(tsoServerConfig.getKeyStoreType(), "");
@@ -45,7 +46,7 @@ public void testCustomParseK() throws Exception {
         Assert.assertEquals(tsoServerConfig.getSupportPlainText(), false);
 
         Assert.assertEquals(tsoServerConfig.getEnabledProtocols(), "TLSv1.2");
-        Assert.assertEquals(tsoServerConfig.getTsConfigProtocols(), "TLSv1.2");
+        Assert.assertEquals(tsoServerConfig.getTsConfigProtocols(), X509Util.DEFAULT_PROTOCOLS);
 
         Assert.assertEquals(tsoServerConfig.getKeyStoreLocation(), "/asd");
         Assert.assertEquals(tsoServerConfig.getKeyStorePassword(), "pass");