Merge branch 'fix-anon-ssl' of https://github.com/tomasp8/h2database into tomasp8-fix-anon-ssl

h2/src/docsrc/html/changelog.html

@@ -21,6 +21,8 @@ Change Log
 
 <h2>Next Version (unreleased)</h2>
 <ul>
+<li>Issue #235: Anonymous SSL connections fail in many situations
+</li>
 <li>Fix race condition in FILE_LOCK=SOCKET, which could result in the watchdog thread not running  
 </li>
 <li>Experimental support for datatype TIMESTAMP WITH TIMEZONE

h2/src/main/org/h2/security/CipherFactory.java

@@ -17,13 +17,18 @@ import java.net.Socket;
 import java.security.KeyFactory;
 import java.security.KeyStore;
 import java.security.PrivateKey;
+import java.security.Security;
 import java.security.cert.Certificate;
 import java.security.cert.CertificateFactory;
 import java.security.spec.PKCS8EncodedKeySpec;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.LinkedList;
+import java.util.List;
 import java.util.Properties;
+
 import javax.net.ServerSocketFactory;
 import javax.net.ssl.SSLServerSocket;
 import javax.net.ssl.SSLServerSocketFactory;
@@ -47,6 +52,18 @@ public class CipherFactory {
      */
     public static final String KEYSTORE_PASSWORD =
             "h2pass";
+    /**
+     * The security property which can prevent anonymous TLS connections.
+     * Introduced into Java 6,7,8 in updates from July 2015.
+     */
+    public static final String LEGACY_ALGORITHMS_SECURITY_KEY =
+            "jdk.tls.legacyAlgorithms";
+    /**
+     * The value of {@value #LEGACY_ALGORITHMS_SECURITY_KEY} security
+     * property at the time of class initialization.
+     * Null if it is not set.
+     */
+    public static final String DEFAULT_LEGACY_ALGORITHMS = getLegacyAlgoritmsSilently();
 
     private static final String KEYSTORE =
             "~/.h2.keystore";
@@ -55,6 +72,7 @@ public class CipherFactory {
     private static final String KEYSTORE_PASSWORD_KEY =
             "javax.net.ssl.keyStorePassword";
 
+
     private CipherFactory() {
         // utility class
     }
@@ -105,8 +123,13 @@ public class CipherFactory {
     }
 
 /**
-     * Create a secure server socket. If a bind address is specified, the socket
-     * is only bound to this address.
+     * Create a secure server socket. If a bind address is specified, the
+     * socket is only bound to this address.
+     * If h2.enableAnonymousTLS is true, an attempt is made to modify
+     * the security property jdk.tls.legacyAlgorithms (in newer JVMs) to allow
+     * anonymous TLS. This system change is effectively permanent for the
+     * lifetime of the JVM.
+     * @see #removeAnonFromLegacyAlgorithms()
      *
      * @param port the port to listen on
      * @param bindAddress the address to bind to, or null to bind to all
@@ -116,6 +139,9 @@ public class CipherFactory {
     public static ServerSocket createServerSocket(int port,
             InetAddress bindAddress) throws IOException {
         ServerSocket socket = null;
+        if (SysProperties.ENABLE_ANONYMOUS_TLS) {
+            removeAnonFromLegacyAlgorithms();
+        }
         setKeystore();
         ServerSocketFactory f = SSLServerSocketFactory.getDefault();
         SSLServerSocket secureSocket;
@@ -137,6 +163,95 @@ public class CipherFactory {
         return socket;
     }
 
+    /**
+     * Removes DH_anon and ECDH_anon from a comma separated list of ciphers.
+     * Only the first occurrence is removed.
+     * If there is nothing to remove, returns the reference to the argument.
+     * @param commaSepList  a list of names separated by commas (and spaces)
+     * @return  a new string without DH_anon and ECDH_anon items,
+     *          or the original if none were found
+     */
+    public static String removeDhAnonFromCommaSepList(String commaSepList) {
+        if (commaSepList == null) {
+            return commaSepList;
+        }
+        List<String> algos = new LinkedList<String>(Arrays.asList(commaSepList.split("\\s*,\\s*")));
+        boolean dhAnonRemoved = algos.remove("DH_anon");
+        boolean ecdhAnonRemoved = algos.remove("ECDH_anon");
+        if (dhAnonRemoved || ecdhAnonRemoved) {
+            String algosStr = Arrays.toString(algos.toArray(new String[algos.size()]));
+            return (algos.size() > 0) ? algosStr.substring(1, algosStr.length() - 1): "";
+        } else {
+            return commaSepList;
+        }
+    }
+
+    /**
+     * Attempts to weaken the security properties to allow anonymous TLS.
+     * New JREs would not choose an anonymous cipher suite in a TLS handshake
+     * if server-side security property
+     * {@value #LEGACY_ALGORITHMS_SECURITY_KEY}
+     * were not modified from the default value.
+     * <p>
+     * NOTE: In current (as of 2016) default implementations of JSSE which use
+     * this security property, the value is permanently cached inside the
+     * ServerHandshake class upon its first use.
+     * Therefore the modification accomplished by this method has to be done
+     * before the first use of a server SSL socket.
+     * Later changes to this property will not have any effect on server socket
+     * behavior.
+     */
+    public static synchronized void removeAnonFromLegacyAlgorithms() {
+        String legacyAlgosOrig = getLegacyAlgoritmsSilently();
+        if (legacyAlgosOrig == null) {
+            return;
+        }
+        String legacyAlgosNew = removeDhAnonFromCommaSepList(legacyAlgosOrig);
+        if (!legacyAlgosOrig.equals(legacyAlgosNew)) {
+            setLegacyAlgorithmsSilently(legacyAlgosNew);
+        }
+    }
+
+    /**
+     * Attempts to resets the security property to the default value.
+     * The default value of {@value #LEGACY_ALGORITHMS_SECURITY_KEY} was
+     * obtained at time of class initialization.
+     * <p>
+     * NOTE: Resetting the property might not have any effect on server
+     * socket behavior.
+     * @see #removeAnonFromLegacyAlgorithms()
+     */
+    public static synchronized void resetDefaultLegacyAlgorithms() {
+        setLegacyAlgorithmsSilently(DEFAULT_LEGACY_ALGORITHMS);
+    }
+
+    /**
+     * Returns the security property {@value #LEGACY_ALGORITHMS_SECURITY_KEY}.
+     * Ignores security exceptions.
+     * @return  the value of the security property, or null if not set
+     *          or not accessible
+     */
+    public static String getLegacyAlgoritmsSilently() {
+        String defaultLegacyAlgorithms = null;
+        try {
+            defaultLegacyAlgorithms = Security.getProperty(LEGACY_ALGORITHMS_SECURITY_KEY);
+        } catch (SecurityException e) {
+            // ignore
+        }
+        return defaultLegacyAlgorithms;
+    }
+
+    private static void setLegacyAlgorithmsSilently(String legacyAlgos) {
+        if (legacyAlgos == null) {
+            return;
+        }
+        try {
+            Security.setProperty(LEGACY_ALGORITHMS_SECURITY_KEY, legacyAlgos);
+        } catch (SecurityException e) {
+            // ignore
+        }
+    }
+
     private static byte[] getKeyStoreBytes(KeyStore store, String password)
             throws IOException {
         ByteArrayOutputStream bout = new ByteArrayOutputStream();
@@ -270,16 +385,16 @@ public class CipherFactory {
     }
 
     private static String[] enableAnonymous(String[] enabled, String[] supported) {
-        HashSet<String> set = new HashSet<String>();
-        Collections.addAll(set, enabled);
+        LinkedHashSet<String> set = new LinkedHashSet<String>();
         for (String x : supported) {
             if (!x.startsWith("SSL") &&
                     x.indexOf("_anon_") >= 0 &&
-                    x.indexOf("_AES_") >= 0 &&
+                    (x.indexOf("_AES_") >= 0 || x.indexOf("_3DES_") >= 0) &&
                     x.indexOf("_SHA") >= 0) {
                 set.add(x);
             }
         }
+        Collections.addAll(set, enabled);
         return set.toArray(new String[0]);
     }
 

h2/src/main/org/h2/util/NetUtils.java

@@ -146,7 +146,12 @@ public class NetUtils {
 
     /**
      * Create a server socket. The system property h2.bindAddress is used if
-     * set.
+     * set. If SSL is used and h2.enableAnonymousTLS is true, an attempt is
+     * made to modify the security property jdk.tls.legacyAlgorithms
+     * (in newer JVMs) to allow anonymous TLS.
+     * <p>
+     * This system change is effectively permanent for the lifetime of the JVM.
+     * @see CipherFactory#removeAnonFromLegacyAlgorithms()
      *
      * @param port the port to listen on
      * @param ssl if SSL should be used

h2/src/test/org/h2/test/unit/TestNetUtils.java

@@ -11,19 +11,30 @@ import java.net.Socket;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLServerSocket;
+import javax.net.ssl.SSLSession;
+import javax.net.ssl.SSLSocket;
+
+import org.h2.engine.SysProperties;
 import org.h2.test.TestBase;
 import org.h2.util.NetUtils;
 import org.h2.util.Task;
 
 /**
- * Test the network utilities.
+ * Test the network utilities from {@link NetUtils}.
  *
  * @author Sergi Vladykin
+ * @author Tomas Pospichal
  */
 public class TestNetUtils extends TestBase {
 
     private static final int WORKER_COUNT = 10;
     private static final int PORT = 9111;
+    private static final int WAIT_MILLIS = 100;
+    private static final int WAIT_LONGER_MILLIS = 2 * WAIT_MILLIS;
+    private static final String TASK_PREFIX = "ServerSocketThread-";
 
     /**
      * Run just this test.
@@ -36,10 +47,130 @@ public class TestNetUtils extends TestBase {
 
     @Override
     public void test() throws Exception {
+        testAnonymousTlsSession();
+        testTlsSessionWithServerSideAnonymousDisabled();
         testFrequentConnections(true, 100);
         testFrequentConnections(false, 1000);
     }
 
+    /**
+     * With default settings, H2 client SSL socket should be able to connect
+     * to an H2 server SSL socket using an anonymous cipher suite
+     * (no SSL certificate is needed).
+     * @throws Exception
+     */
+    private void testAnonymousTlsSession() throws Exception {
+        assertTrue("Failed assumption: the default value of ENABLE_ANONYMOUS_TLS" +
+                " property should be true", SysProperties.ENABLE_ANONYMOUS_TLS);
+        boolean ssl = true;
+        Task task = null;
+        ServerSocket serverSocket = null;
+        Socket socket = null;
+
+        try {
+            serverSocket = NetUtils.createServerSocket(PORT, ssl);
+            serverSocket.setSoTimeout(WAIT_LONGER_MILLIS);
+            task = createServerSocketTask(serverSocket);
+            task.execute(TASK_PREFIX + "AnonEnabled");
+            Thread.sleep(WAIT_MILLIS);
+            socket = NetUtils.createLoopbackSocket(PORT, ssl);
+            assertTrue("loopback anon socket should be connected", socket.isConnected());
+            SSLSession session = ((SSLSocket) socket).getSession();
+            assertTrue("TLS session should be valid when anonymous TLS is enabled",
+                    session.isValid());
+            // in case of handshake failure:
+            // the cipher suite is the pre-handshake SSL_NULL_WITH_NULL_NULL
+            assertContains(session.getCipherSuite(), "_anon_");
+        } finally {
+            closeSilently(socket);
+            closeSilently(serverSocket);
+            if (task != null) {
+                // SSL server socket should succeed using an anonymous cipher
+                // suite, and not throw javax.net.ssl.SSLHandshakeException
+                assertNull(task.getException());
+                task.join();
+            }
+        }
+    }
+
+    /**
+     * TLS connections (without trusted certificates) should fail if the server
+     * does not allow anonymous TLS.
+     * The global property ENABLE_ANONYMOUS_TLS cannot be modified for the test;
+     * instead, the server socket is altered.
+     * @throws Exception
+     */
+    private void testTlsSessionWithServerSideAnonymousDisabled() throws Exception {
+        boolean ssl = true;
+        Task task = null;
+        ServerSocket serverSocket = null;
+        Socket socket = null;
+        try {
+            serverSocket = NetUtils.createServerSocket(PORT, ssl);
+            serverSocket.setSoTimeout(WAIT_LONGER_MILLIS);
+            // emulate the situation ENABLE_ANONYMOUS_TLS=false on server side
+            String[] defaultCipherSuites = SSLContext.getDefault().getServerSocketFactory()
+                    .getDefaultCipherSuites();
+            ((SSLServerSocket) serverSocket).setEnabledCipherSuites(defaultCipherSuites);
+            task = createServerSocketTask(serverSocket);
+            task.execute(TASK_PREFIX + "AnonDisabled");
+            Thread.sleep(WAIT_MILLIS);
+            socket = NetUtils.createLoopbackSocket(PORT, ssl);
+            assertTrue("loopback socket should be connected", socket.isConnected());
+            // Java 6 API does not have getHandshakeSession() which could
+            // reveal the actual cipher selected in the attempted handshake
+            SSLSession session = ((SSLSocket) socket).getSession();
+            assertFalse("TLS session should be invalid when the server" +
+                    "disables anonymous TLS", session.isValid());
+            // the SSL handshake should fail, because non-anon ciphers require
+            // a trusted certificate
+            assertEquals("SSL_NULL_WITH_NULL_NULL", session.getCipherSuite());
+        } finally {
+            closeSilently(socket);
+            closeSilently(serverSocket);
+            if (task != null) {
+                assertTrue(task.getException() != null);
+                assertEquals(javax.net.ssl.SSLHandshakeException.class.getName(),
+                        task.getException().getClass().getName());
+                assertContains(task.getException().getMessage(), "certificate_unknown");
+                task.join();
+            }
+        }
+    }
+
+    private Task createServerSocketTask(final ServerSocket serverSocket) {
+        Task task = new Task() {
+            
+            @Override
+            public void call() throws Exception {
+                Socket ss = null;
+                try {
+                    ss = serverSocket.accept();
+                    ss.getOutputStream().write(123);
+                } finally {
+                   closeSilently(ss);
+                }
+            }
+        };
+        return task;
+    }
+
+    private void closeSilently(Socket socket) {
+        try {
+            socket.close();
+        } catch (Exception e) {
+            // ignore
+        }
+    }
+
+    private void closeSilently(ServerSocket socket) {
+        try {
+            socket.close();
+        } catch (Exception e) {
+            // ignore
+        }
+    }
+
     private void testFrequentConnections(boolean ssl, int count) throws Exception {
         final ServerSocket serverSocket = NetUtils.createServerSocket(PORT, ssl);
         final AtomicInteger counter = new AtomicInteger(count);
@@ -96,7 +227,7 @@ public class TestNetUtils extends TestBase {
         private final AtomicInteger counter;
         private Exception exception;
 
-        public ConnectWorker(boolean ssl, AtomicInteger counter) {
+        ConnectWorker(boolean ssl, AtomicInteger counter) {
             this.ssl = ssl;
             this.counter = counter;
         }

h2/src/test/org/h2/test/unit/TestSecurity.java

@@ -9,6 +9,7 @@ import java.sql.Connection;
 import java.sql.DriverManager;
 import java.sql.SQLException;
 import java.util.Arrays;
+
 import org.h2.security.BlockCipher;
 import org.h2.security.CipherFactory;
 import org.h2.security.SHA256;
@@ -35,6 +36,8 @@ public class TestSecurity extends TestBase {
         testSHA();
         testAES();
         testBlockCiphers();
+        testRemoveAnonFromLegacyAlgos();
+        //testResetLegacyAlgos();
     }
 
     private static void testConnectWithHash() throws SQLException {
@@ -251,4 +254,43 @@ public class TestSecurity extends TestBase {
         return len * r < len * 120;
     }
 
+    private void testRemoveAnonFromLegacyAlgos() {
+        String legacyAlgos = "K_NULL, C_NULL, M_NULL, DHE_DSS_EXPORT" +
+                ", DHE_RSA_EXPORT, DH_anon_EXPORT, DH_DSS_EXPORT, DH_RSA_EXPORT, RSA_EXPORT" +
+                ", DH_anon, ECDH_anon, RC4_128, RC4_40, DES_CBC, DES40_CBC";
+        String expectedLegacyAlgosWithoutDhAnon = "K_NULL, C_NULL, M_NULL, DHE_DSS_EXPORT" +
+                ", DHE_RSA_EXPORT, DH_anon_EXPORT, DH_DSS_EXPORT, DH_RSA_EXPORT, RSA_EXPORT" +
+                ", RC4_128, RC4_40, DES_CBC, DES40_CBC";
+        assertEquals(expectedLegacyAlgosWithoutDhAnon,
+                CipherFactory.removeDhAnonFromCommaSepList(legacyAlgos));
+
+        legacyAlgos = "ECDH_anon, DH_anon_EXPORT, DH_anon";
+        expectedLegacyAlgosWithoutDhAnon = "DH_anon_EXPORT";
+        assertEquals(expectedLegacyAlgosWithoutDhAnon,
+                CipherFactory.removeDhAnonFromCommaSepList(legacyAlgos));
+
+        legacyAlgos = null;
+        assertNull(CipherFactory.removeDhAnonFromCommaSepList(legacyAlgos));
+    }
+
+    /**
+     * This test is meaningful when run in isolation. However, tests of server
+     * sockets or ssl connections may modify the global state given by the
+     * jdk.tls.legacyAlgorithms security property (for a good reason).
+     * It is best to avoid running it in test suites, as it could itself lead
+     * to a modification of the global state with hard-to-track consequences.
+     */
+    @SuppressWarnings("unused")
+    private void testResetLegacyAlgos() {
+        String legacyAlgorithmsBefore = CipherFactory.getLegacyAlgoritmsSilently();
+        assertEquals("Failed assumption: jdk.tls.legacyAlgorithms" +
+                " has been modified from its initial setting",
+                CipherFactory.DEFAULT_LEGACY_ALGORITHMS, legacyAlgorithmsBefore);
+        CipherFactory.removeAnonFromLegacyAlgorithms();
+        CipherFactory.resetDefaultLegacyAlgorithms();
+        String legacyAlgorithmsAfter = CipherFactory.getLegacyAlgoritmsSilently();
+        assertEquals(CipherFactory.DEFAULT_LEGACY_ALGORITHMS, legacyAlgorithmsAfter);
+    }
+
+
 }

h2/src/test/org/h2/test/unit/TestTools.java

@@ -33,6 +33,7 @@ import java.sql.Time;
 import java.sql.Timestamp;
 import java.sql.Types;
 import java.util.ArrayList;
+import java.util.List;
 import java.util.Random;
 
 import org.h2.api.ErrorCode;
@@ -64,6 +65,7 @@ public class TestTools extends TestBase {
 
     private static String lastUrl;
     private Server server;
+    private List<Server> remainingServers = new ArrayList<Server>(3);
 
     /**
      * Run just this test.
@@ -171,7 +173,6 @@ public class TestTools extends TestBase {
             assertThrows(ErrorCode.EXCEPTION_OPENING_PORT_2, c).runTool("-web",
                     "-webPort", "9002", "-tcp", "-tcpPort", "9002");
             c.runTool("-web", "-webPort", "9002");
-            c.shutdown();
 
         } finally {
             if (old != null) {
@@ -179,6 +180,7 @@ public class TestTools extends TestBase {
             } else {
                 System.clearProperty(SysProperties.H2_BROWSER);
             }
+            c.shutdown();
         }
     }
 
@@ -486,6 +488,7 @@ public class TestTools extends TestBase {
                 }
             }
         };
+        try {
             task.execute();
             Thread.sleep(100);
             try {
@@ -494,9 +497,11 @@ public class TestTools extends TestBase {
             } catch (SQLException e) {
                 assertEquals(ErrorCode.CONNECTION_BROKEN_1, e.getErrorCode());
             }
+        } finally {
             serverSocket.close();
             task.getException();
         }
+    }
 
     private void testDeleteFiles() throws SQLException {
         deleteDb("testDeleteFiles");
@@ -527,6 +532,7 @@ public class TestTools extends TestBase {
         String result;
         Connection conn;
 
+        try {
             result = runServer(0, new String[]{"-?"});
             assertTrue(result.contains("Starts the H2 Console"));
             assertTrue(result.indexOf("Unknown option") < 0);
@@ -545,12 +551,16 @@ public class TestTools extends TestBase {
             result = runServer(0, new String[]{"-tcpShutdown",
                     "tcp://localhost:9001", "-tcpPassword", "abc", "-tcpShutdownForce"});
             assertTrue(result.contains("Shutting down"));
+        } finally {
+            shutdownServers();
+        }
     }
 
     private void testSSL() throws SQLException {
         String result;
         Connection conn;
 
+        try {
             result = runServer(0, new String[]{"-tcp",
                     "-tcpAllowOthers", "-tcpPort", "9001", "-tcpPassword", "abcdef", "-tcpSSL"});
             assertTrue(result.contains("ssl://"));
@@ -589,11 +599,17 @@ public class TestTools extends TestBase {
             stop.shutdown();
             assertThrows(ErrorCode.CONNECTION_BROKEN_1, this).
             getConnection("jdbc:h2:tcp://localhost:9006/mem:", "sa", "sa");
+        } finally {
+            shutdownServers();
+        }
     }
 
     private String runServer(int exitCode, String... args) {
         ByteArrayOutputStream buff = new ByteArrayOutputStream();
         PrintStream ps = new PrintStream(buff);
+        if (server != null) {
+            remainingServers.add(server);
+        }
         server = new Server();
         server.setOut(ps);
         int result = 0;
@@ -609,6 +625,18 @@ public class TestTools extends TestBase {
         return s;
     }
 
+    private void shutdownServers() {
+        for (Server remainingServer : remainingServers) {
+            if (remainingServer != null) {
+                remainingServer.shutdown();
+            }
+        }
+        remainingServers.clear();
+        if (server != null) {
+            server.shutdown();
+        }
+    }
+
     private void testConvertTraceFile() throws Exception {
         deleteDb("toolsConvertTraceFile");
         org.h2.Driver.load();
@@ -1018,11 +1046,13 @@ public class TestTools extends TestBase {
 
     private void testServer() throws SQLException {
         Connection conn;
+        try {
             deleteDb("test");
             Server tcpServer = Server.createTcpServer(
                             "-baseDir", getBaseDir(),
                             "-tcpPort", "9192",
                             "-tcpAllowOthers").start();
+            remainingServers.add(tcpServer);
             conn = getConnection("jdbc:h2:tcp://localhost:9192/test", "sa", "");
             conn.close();
             // must not be able to use a different base dir
@@ -1037,11 +1067,12 @@ public class TestTools extends TestBase {
                     getConnection("jdbc:h2:tcp://localhost:9192/../test2/test", "sa", "");
             }};
             tcpServer.stop();
-        Server.createTcpServer(
+            Server tcpServerWithPassword = Server.createTcpServer(
                             "-ifExists",
                             "-tcpPassword", "abc",
                             "-baseDir", getBaseDir(),
                             "-tcpPort", "9192").start();
+            remainingServers.add(tcpServerWithPassword);
             // must not be able to create new db
             new AssertThrows(ErrorCode.DATABASE_NOT_FOUND_1) {
                 @Override
@@ -1083,6 +1114,9 @@ public class TestTools extends TestBase {
 
             server.stop();
             deleteDb("testSplit");
+        } finally {
+            shutdownServers();
+        }
     }
 
     /**