From 10b44ab578496b554ce86a3caf3012def9a40c5f Mon Sep 17 00:00:00 2001
From: Bartek Fabiszewski <github@fabiszewski.net>
Date: Thu, 4 May 2017 19:24:09 +0200
Subject: [PATCH] Improve TLS socket factory

---
 .../fabiszewski/ulogger/TlsSocketFactory.java | 96 +++++++++++++++----
 1 file changed, 77 insertions(+), 19 deletions(-)

diff --git a/app/src/main/java/net/fabiszewski/ulogger/TlsSocketFactory.java b/app/src/main/java/net/fabiszewski/ulogger/TlsSocketFactory.java
index 0ce2645..34b5711 100644
--- a/app/src/main/java/net/fabiszewski/ulogger/TlsSocketFactory.java
+++ b/app/src/main/java/net/fabiszewski/ulogger/TlsSocketFactory.java
@@ -16,8 +16,12 @@
 import java.io.IOException;
 import java.lang.reflect.Method;
 import java.net.InetAddress;
+import java.net.InetSocketAddress;
 import java.net.Socket;
+import java.util.ArrayList;
+
 import javax.net.ssl.HostnameVerifier;
+import javax.net.ssl.SSLException;
 import javax.net.ssl.SSLPeerUnverifiedException;
 import javax.net.ssl.SSLSession;
 import javax.net.ssl.SSLSocket;
@@ -34,18 +38,32 @@ class TlsSocketFactory extends SSLSocketFactory {
     private static final String TAG = TlsSocketFactory.class.getSimpleName();
     private static HostnameVerifier hostnameVerifier;
     private static SSLSocketFactory factory;
+    private static String[] cipherSuites;
 
     TlsSocketFactory(Context context) {
         SSLSessionCache cache = new SSLSessionCache(context);
         factory = SSLCertificateSocketFactory.getDefault(WebHelper.SOCKET_TIMEOUT, cache);
         hostnameVerifier = new org.apache.http.conn.ssl.BrowserCompatHostnameVerifier();
+        cipherSuites = initCipherSuites();
     }
 
+    /**
+     * Returns connected secure socket,
+     * built on SSLCertificateSocketFactory
+     * @param socket Socket
+     * @param host Host name
+     * @param port Port
+     * @param autoClose Auto close
+     * @return Socket
+     * @throws IOException If setup or host name verification fail
+     */
     @Override
     public Socket createSocket(Socket socket, String host, int port, boolean autoClose) throws IOException {
-        if (autoClose) { socket.close(); }
+        if (autoClose && socket != null && !socket.isClosed()) {
+            socket.close();
+        }
 
-        socket = factory.createSocket(InetAddress.getByName(host), port);
+        socket = factory.createSocket();
 
         if (socket != null && socket instanceof SSLSocket) {
             if (Logger.DEBUG) { Log.d(TAG, "[Preparing TLS socket]"); }
@@ -53,29 +71,69 @@ public Socket createSocket(Socket socket, String host, int port, boolean autoClo
 
             // set all protocols including TLS (disabled by default on older APIs)
             sslSocket.setEnabledProtocols(sslSocket.getSupportedProtocols());
-            sslSocket.setEnabledCipherSuites(sslSocket.getSupportedCipherSuites());
-
-            if (host != null && !host.isEmpty()) {
-                // set hostname for SNI
-                if (Logger.DEBUG) { Log.d(TAG, "[Setting SNI for host " + host + "]"); }
-                try {
-                    Method setHostnameMethod = sslSocket.getClass().getMethod("setHostname", String.class);
-                    setHostnameMethod.invoke(sslSocket, host);
-                } catch (Exception e) {
-                    if (Logger.DEBUG) { Log.d(TAG, "[Setting SNI failed: " + e.getMessage() + "]"); }
-                }
-            }
-            // verify hostname and certificate
-            SSLSession session = sslSocket.getSession();
-            if (!hostnameVerifier.verify(host, session)) {
-                throw new SSLPeerUnverifiedException("Hostname '" + host + "' was not verified (" + session.getPeerPrincipal() + ")");
+            sslSocket.setEnabledCipherSuites(cipherSuites);
+
+            // set hostname for SNI
+            if (Logger.DEBUG) { Log.d(TAG, "[Setting SNI for host " + host + "]"); }
+            try {
+                Method setHostnameMethod = sslSocket.getClass().getMethod("setHostname", String.class);
+                setHostnameMethod.invoke(sslSocket, host);
+            } catch (Exception e) {
+                if (Logger.DEBUG) { Log.d(TAG, "[Setting SNI failed: " + e.getMessage() + "]"); }
             }
 
-            if (Logger.DEBUG) { Log.d(TAG, "Connected to " + session.getPeerHost() + " using " + session.getProtocol() + " (" + session.getCipherSuite() + ")"); }
+            try {
+                InetSocketAddress remoteAddress = new InetSocketAddress(host, port);
+                sslSocket.connect(remoteAddress, WebHelper.SOCKET_TIMEOUT);
+                sslSocket.setSoTimeout(WebHelper.SOCKET_TIMEOUT);
+                // verify hostname and certificate
+                verifyHostname(sslSocket, host);
+            } catch (Exception e) {
+                try { sslSocket.close(); } catch (Exception ex) { /* ignore */ }
+                throw e;
+            }
         }
         return socket;
     }
 
+    /**
+     * Verify hostname against certificate
+     * @param sslSocket Socket
+     * @param host Host name
+     * @throws IOException Exception if host name is not verified
+     */
+    private void verifyHostname(SSLSocket sslSocket, String host) throws IOException {
+        // Make sure we started handshake before verifying
+        sslSocket.startHandshake();
+
+        SSLSession session = sslSocket.getSession();
+        if (session == null) {
+            throw new SSLException("Hostname '" + host + "' was not verified (no session)");
+        }
+        if (!hostnameVerifier.verify(host, session)) {
+            throw new SSLPeerUnverifiedException("Hostname '" + host + "' was not verified (" + session.getPeerPrincipal() + ")");
+        }
+        if (Logger.DEBUG) { Log.d(TAG, "Connected to " + session.getPeerHost() + " using " + session.getProtocol() + " (" + session.getCipherSuite() + ")"); }
+    }
+
+    /**
+     * Initialize cipher suites array, based on all supported ciphers without weak ones.
+     * @return Array of ciphers
+     */
+    private String[] initCipherSuites() {
+        // remove weakest suites
+        ArrayList<String> ciphers = new ArrayList<>(20);
+        for (String cipher : factory.getSupportedCipherSuites()) {
+            if (cipher.contains("_EXPORT_") ||
+                    cipher.contains("_anon_") ||
+                    cipher.contains("_NULL_")) {
+                continue;
+            }
+            ciphers.add(cipher);
+        }
+        return ciphers.toArray(new String[0]);
+    }
+
     @Override
     public String[] getDefaultCipherSuites() {
         throw new UnsupportedOperationException("Not implemented");