From ca0ad612a7b40133aba4197dcd0124f00ebb2cd0 Mon Sep 17 00:00:00 2001 From: R Hirner Date: Thu, 1 Jan 2015 17:56:21 +0100 Subject: [PATCH] TlsSniSocketFactory improvements * make it work again with non-proxied connections * add tests for proxied and non-proxied connections * add tests for SNI, host name and certificate verification --- .../webdav/TlsSniSocketFactoryTest.java | 116 ++++++++++++++++-- .../davdroid/webdav/TlsSniSocketFactory.java | 69 ++++++----- 2 files changed, 139 insertions(+), 46 deletions(-) diff --git a/app/src/androidTest/java/at/bitfire/davdroid/webdav/TlsSniSocketFactoryTest.java b/app/src/androidTest/java/at/bitfire/davdroid/webdav/TlsSniSocketFactoryTest.java index f00844d5..f7a3d571 100644 --- a/app/src/androidTest/java/at/bitfire/davdroid/webdav/TlsSniSocketFactoryTest.java +++ b/app/src/androidTest/java/at/bitfire/davdroid/webdav/TlsSniSocketFactoryTest.java @@ -1,27 +1,119 @@ +/* + * Copyright (c) 2015 Ricki Hirner (bitfire web engineering). + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the GNU Public License v3.0 + * which accompanies this distribution, and is available at + * http://www.gnu.org/licenses/gpl.html + */ + package at.bitfire.davdroid.webdav; import java.io.IOException; +import java.net.InetSocketAddress; +import java.net.Socket; +import java.net.SocketAddress; +import java.security.cert.CertPathValidatorException; +import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSocket; import android.util.Log; import junit.framework.TestCase; +import org.apache.commons.lang.ArrayUtils; +import org.apache.commons.lang.exception.ExceptionUtils; +import org.apache.http.HttpHost; + +import lombok.Cleanup; + public class TlsSniSocketFactoryTest extends TestCase { private static final String TAG = "davdroid.TlsSniSocketFactoryTest"; - - public void testCiphers() throws IOException { - SSLSocket socket = (SSLSocket)TlsSniSocketFactory.INSTANCE.createSocket(null); - Log.i(TAG, "Enabled:"); - for (String cipher : socket.getEnabledCipherSuites()) - Log.i(TAG, cipher); - - Log.i(TAG, "Supported:"); - for (String cipher : socket.getSupportedCipherSuites()) - Log.i(TAG, cipher); - - assert(true); + TlsSniSocketFactory factory = TlsSniSocketFactory.INSTANCE; + + private InetSocketAddress sampleTlsEndpoint; + + @Override + protected void setUp() { + // sni.velox.ch is used to test SNI (without SNI support, the certificate is invalid) + sampleTlsEndpoint = new InetSocketAddress("sni.velox.ch", 443); + } + + public void testCreateSocket() { + try { + @Cleanup SSLSocket socket = factory.createSocket(null); + assertFalse(socket.isConnected()); + } catch (IOException e) { + fail(); + } + } + + public void testConnectSocket() { + try { + @Cleanup SSLSocket socket = factory.createSocket(null); + + factory.connectSocket(1000, socket, new HttpHost(sampleTlsEndpoint.getHostName()), sampleTlsEndpoint, null, null); + } catch (IOException e) { + Log.e(TAG, "I/O exception", e); + fail(); + } + } + + public void testCreateLayeredSocket() { + try { + // connect plain socket first + @Cleanup Socket plain = new Socket(); + plain.connect(sampleTlsEndpoint); + assertTrue(plain.isConnected()); + + // then create TLS socket on top of it and establish TLS Connection + @Cleanup SSLSocket socket = factory.createLayeredSocket(plain, sampleTlsEndpoint.getHostName(), sampleTlsEndpoint.getPort(), null); + assertTrue(socket.isConnected()); + + } catch (IOException e) { + Log.e(TAG, "I/O exception", e); + fail(); + } + } + + public void testSetTlsParameters() throws IOException { + @Cleanup SSLSocket socket = factory.createSocket(null); + factory.setTlsParameters(socket); + + String enabledProtocols[] = socket.getEnabledProtocols(); + // SSL (all versions) should be disabled + for (String protocol : enabledProtocols) + assertFalse(protocol.contains("SSL")); + // TLS v1+ should be enabled + assertTrue(ArrayUtils.contains(enabledProtocols, "TLSv1")); + assertTrue(ArrayUtils.contains(enabledProtocols, "TLSv1.1")); + assertTrue(ArrayUtils.contains(enabledProtocols, "TLSv1.2")); + } + + + public void testHostnameNotInCertificate() { + try { + // host with certificate that doesn't match host name + // use the IP address as host name because IP addresses are usually not in the certificate subject + InetSocketAddress host = new InetSocketAddress(sampleTlsEndpoint.getAddress().getHostAddress(), 443); + + @Cleanup SSLSocket socket = factory.connectSocket(0, null, new HttpHost(host.getHostName()), host, null, null); + fail(); + } catch (IOException e) { + assertFalse(ExceptionUtils.indexOfType(e, SSLPeerUnverifiedException.class) == -1); + } + } + + public void testUntrustedCertificate() { + try { + // host with certificate that is not trusted by default + InetSocketAddress host = new InetSocketAddress("cacert.org", 443); + + @Cleanup SSLSocket socket = factory.connectSocket(0, null, new HttpHost(host.getHostName()), host, null, null); + fail(); + } catch (IOException e) { + assertFalse(ExceptionUtils.indexOfType(e, CertPathValidatorException.class) == -1); + } } } diff --git a/app/src/main/java/at/bitfire/davdroid/webdav/TlsSniSocketFactory.java b/app/src/main/java/at/bitfire/davdroid/webdav/TlsSniSocketFactory.java index 2cff28ed..60e74882 100644 --- a/app/src/main/java/at/bitfire/davdroid/webdav/TlsSniSocketFactory.java +++ b/app/src/main/java/at/bitfire/davdroid/webdav/TlsSniSocketFactory.java @@ -1,10 +1,10 @@ -/******************************************************************************* - * Copyright (c) 2014 Ricki Hirner (bitfire web engineering). +/* + * Copyright (c) 2015 Ricki Hirner (bitfire web engineering). * All rights reserved. This program and the accompanying materials * are made available under the terms of the GNU Public License v3.0 * which accompanies this distribution, and is available at * http://www.gnu.org/licenses/gpl.html - ******************************************************************************/ + */ package at.bitfire.davdroid.webdav; import android.annotation.SuppressLint; @@ -13,35 +13,25 @@ import android.net.SSLCertificateSocketFactory; import android.os.Build; import android.util.Log; -import org.apache.commons.lang.ArrayUtils; import org.apache.commons.lang.StringUtils; import org.apache.http.HttpHost; import org.apache.http.conn.socket.LayeredConnectionSocketFactory; -import org.apache.http.conn.ssl.BrowserCompatHostnameVerifierHC4; import org.apache.http.protocol.HttpContext; import java.io.IOException; import java.net.InetSocketAddress; import java.net.Socket; -import java.net.UnknownHostException; -import java.security.NoSuchAlgorithmException; -import java.security.cert.CertPathValidatorException; import java.util.Arrays; import java.util.HashSet; import java.util.LinkedList; import java.util.List; -import javax.net.ssl.HandshakeCompletedEvent; -import javax.net.ssl.HandshakeCompletedListener; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSocket; import javax.net.ssl.SSLSocketFactory; -import javax.net.ssl.TrustManager; -import javax.net.ssl.TrustManagerFactory; -import javax.net.ssl.X509TrustManager; public class TlsSniSocketFactory implements LayeredConnectionSocketFactory { private static final String TAG = "davdroid.SNISocketFactory"; @@ -65,35 +55,48 @@ public class TlsSniSocketFactory implements LayeredConnectionSocketFactory { 2) the plain socket is connected to http://proxy:8080 3) a CONNECT request is sent to the proxy and the response is parsed 4) socket = createLayeredSocket(plain) is called to "upgrade" the plain connection to a TLS connection (but no handshake yet) - 5) SNI is set up for socket - 6) handshake and certificate/host name verification + 5) reasonable encryption settings are applied to socket + 6) SNI is set up for socket + 7) handshake and certificate/host name verification */ @Override - public Socket createSocket(HttpContext context) throws IOException { - return sslSocketFactory.createSocket(); + public SSLSocket createSocket(HttpContext context) throws IOException { + return (SSLSocket)sslSocketFactory.createSocket(); } @Override - public Socket connectSocket(int timeout, Socket sock, HttpHost host, InetSocketAddress remoteAddr, InetSocketAddress localAddr, HttpContext context) throws IOException { - Log.d(TAG, "Preparing direct TLS connection to " + host); - final SSLSocket socket = (SSLSocket)((sock != null) ? sock : createSocket(context)); - connectAndVerify(socket, host.getHostName()); + public SSLSocket connectSocket(int timeout, Socket sock, HttpHost host, InetSocketAddress remoteAddr, InetSocketAddress localAddr, HttpContext context) throws IOException { + Log.d(TAG, "Establishing direct TLS connection to " + host); + final SSLSocket socket = (sock != null) ? (SSLSocket)sock : createSocket(context); + + if (localAddr != null) + socket.bind(localAddr); + + // connect the socket on TCP level + socket.connect(remoteAddr, timeout); + + // establish and verify TLS connection + establishAndVerify(socket, host.getHostName()); return socket; } @Override - public Socket createLayeredSocket(Socket plain, String host, int port, HttpContext context) throws IOException { - Log.d(TAG, "Preparing proxied TLS connection to " + host); + public SSLSocket createLayeredSocket(Socket plain, String host, int port, HttpContext context) throws IOException { + Log.d(TAG, "Establishing layered TLS connection to " + host); + + // create new socket for TLS connection on top of existing socket final SSLSocket socket = (SSLSocket)sslSocketFactory.createSocket(plain, host, port, true); - connectAndVerify(socket, host); + + // establish and verify TLS connection + establishAndVerify(socket, host); return socket; } /** - * Establishes a connection to an unconnected SSLSocket: - * - prepare socket + * Establishes and verifies a TLS connection to a (TCP-)connected SSLSocket: + * - set TLS parameters like allowed protocols and ciphers * - set SNI host name * - verify host name * - verify certificate @@ -101,19 +104,17 @@ public class TlsSniSocketFactory implements LayeredConnectionSocketFactory { * @param host host name for SNI * @throws SSLPeerUnverifiedException */ - private void connectAndVerify(SSLSocket socket, String host) throws IOException, SSLPeerUnverifiedException { - // prepare socket (set encryption etc.) - prepareSSLSocket(socket); - - // set SNI hostname + private void establishAndVerify(SSLSocket socket, String host) throws IOException, SSLPeerUnverifiedException { + setTlsParameters(socket); setSniHostname(socket, host); - // TLS handshake + // TLS handshake, throws an exception for untrusted certificates socket.startHandshake(); // verify hostname and certificate SSLSession session = socket.getSession(); if (!hostnameVerifier.verify(host, session)) + // throw exception for inavlid host names throw new SSLPeerUnverifiedException(host); Log.d(TAG, "Established " + session.getProtocol() + " connection with " + session.getPeerHost() + @@ -122,14 +123,14 @@ public class TlsSniSocketFactory implements LayeredConnectionSocketFactory { /** - * Prepares a TLS/SSL connetion socket by: + * Prepares a TLS/SSL connection socket by: * - setting the default TrustManager (as we have created an "insecure" connection to avoid handshake problems before) * - setting reasonable TLS protocol versions * - setting reasonable cipher suites (if required) * @param socket unconnected SSLSocket to prepare */ @SuppressLint("DefaultLocale") - private void prepareSSLSocket(SSLSocket socket) { + void setTlsParameters(SSLSocket socket) { // Android 5.0+ (API level21) provides reasonable default settings // but it still allows SSLv3 // https://developer.android.com/about/versions/android-5.0-changes.html#ssl