Merge pull request #883 from SamWhited/ciphers

Harden TLS cipher suites

5 files changed, 94 insertions, 43 deletions

diff --git a/src/main/java/eu/siacs/conversations/Config.java b/src/main/java/eu/siacs/conversations/Config.java
index 6fe13d93..b269dedf 100644
--- a/src/main/java/eu/siacs/conversations/Config.java
+++ b/src/main/java/eu/siacs/conversations/Config.java
@@ -29,6 +29,32 @@ public final class Config {
 	public static final long MAM_MAX_CATCHUP =  MILLISECONDS_IN_DAY / 2;
 	public static final int MAM_MAX_MESSAGES = 500;
 
+	public static final String ENABLED_CIPHERS[] = {
+		"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
+		"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA384",
+		"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA256",
+		"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384",
+		"TLS_ECDHE_RSA_AES_128_SHA",
+		"TLS_ECDHE_RSA_AES_256_SHA",
+
+		"TLS_DHE_RSA_WITH_AES_128_GCM_SHA256",
+		"TLS_DHE_RSA_WITH_AES_128_GCM_SHA384",
+		"TLS_DHE_RSA_WITH_AES_256_GCM_SHA256",
+		"TLS_DHE_RSA_WITH_AES_256_GCM_SHA384",
+
+		"TLS_DHE_RSA_WITH_CAMELLIA_256_SHA",
+
+		// Fallback.
+		"TLS_RSA_WITH_AES_128_GCM_SHA256",
+		"TLS_RSA_WITH_AES_128_GCM_SHA384",
+		"TLS_RSA_WITH_AES_256_GCM_SHA256",
+		"TLS_RSA_WITH_AES_256_GCM_SHA384",
+		"TLS_RSA_WITH_AES_128_CBC_SHA256",
+		"TLS_RSA_WITH_AES_128_CBC_SHA384",
+		"TLS_RSA_WITH_AES_256_CBC_SHA256",
+		"TLS_RSA_WITH_AES_256_CBC_SHA384"
+	};
+
 	private Config() {
 
 	}
diff --git a/src/main/java/eu/siacs/conversations/http/HttpConnection.java b/src/main/java/eu/siacs/conversations/http/HttpConnection.java
index 8951de74..4bff5251 100644
--- a/src/main/java/eu/siacs/conversations/http/HttpConnection.java
+++ b/src/main/java/eu/siacs/conversations/http/HttpConnection.java
@@ -20,6 +20,7 @@ import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.HttpsURLConnection;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLHandshakeException;
+import javax.net.ssl.SSLSocketFactory;
 import javax.net.ssl.X509TrustManager;
 
 import eu.siacs.conversations.Config;
@@ -90,7 +91,7 @@ public class HttpConnection implements Downloadable {
 			if (this.message.getEncryption() == Message.ENCRYPTION_OTR
 					&& this.file.getKey() == null) {
 				this.message.setEncryption(Message.ENCRYPTION_NONE);
-			}
+					}
 			checkFileSize(false);
 		} catch (MalformedURLException e) {
 			this.cancel();
@@ -124,33 +125,39 @@ public class HttpConnection implements Downloadable {
 		mXmppConnectionService.updateConversationUi();
 	}
 
-	private void setupTrustManager(HttpsURLConnection connection,
-								   boolean interactive) {
-		X509TrustManager trustManager;
-		HostnameVerifier hostnameVerifier;
+	private void setupTrustManager(final HttpsURLConnection connection,
+			final boolean interactive) {
+		final X509TrustManager trustManager;
+		final HostnameVerifier hostnameVerifier;
 		if (interactive) {
 			trustManager = mXmppConnectionService.getMemorizingTrustManager();
 			hostnameVerifier = mXmppConnectionService
-					.getMemorizingTrustManager().wrapHostnameVerifier(
-							new StrictHostnameVerifier());
+				.getMemorizingTrustManager().wrapHostnameVerifier(
+						new StrictHostnameVerifier());
 		} else {
 			trustManager = mXmppConnectionService.getMemorizingTrustManager()
-					.getNonInteractive();
+				.getNonInteractive();
 			hostnameVerifier = mXmppConnectionService
-					.getMemorizingTrustManager()
-					.wrapHostnameVerifierNonInteractive(
-							new StrictHostnameVerifier());
+				.getMemorizingTrustManager()
+				.wrapHostnameVerifierNonInteractive(
+						new StrictHostnameVerifier());
 		}
 		try {
-			SSLContext sc = SSLContext.getInstance("TLS");
+			final SSLContext sc = SSLContext.getInstance("TLS");
 			sc.init(null, new X509TrustManager[]{trustManager},
 					mXmppConnectionService.getRNG());
-			connection.setSSLSocketFactory(sc.getSocketFactory());
+
+			final SSLSocketFactory sf = sc.getSocketFactory();
+			final String[] cipherSuites = CryptoHelper.getSupportedCipherSuites(
+					sf.getSupportedCipherSuites());
+			if (cipherSuites.length > 0) {
+				sc.getDefaultSSLParameters().setCipherSuites(cipherSuites);
+
+			}
+
+			connection.setSSLSocketFactory(sf);
 			connection.setHostnameVerifier(hostnameVerifier);
-		} catch (KeyManagementException e) {
-			return;
-		} catch (NoSuchAlgorithmException e) {
-			return;
+		} catch (final KeyManagementException | NoSuchAlgorithmException ignored) {
 		}
 	}
 
@@ -188,24 +195,24 @@ public class HttpConnection implements Downloadable {
 		}
 
 		private long retrieveFileSize() throws IOException,
-				SSLHandshakeException {
-			changeStatus(STATUS_CHECKING);
-			HttpURLConnection connection = (HttpURLConnection) mUrl
-					.openConnection();
-			connection.setRequestMethod("HEAD");
-			if (connection instanceof HttpsURLConnection) {
-				setupTrustManager((HttpsURLConnection) connection, interactive);
-			}
-			connection.connect();
-			String contentLength = connection.getHeaderField("Content-Length");
-			if (contentLength == null) {
-				throw new IOException();
-			}
-			try {
-				return Long.parseLong(contentLength, 10);
-			} catch (NumberFormatException e) {
-				throw new IOException();
-			}
+						SSLHandshakeException {
+							changeStatus(STATUS_CHECKING);
+							HttpURLConnection connection = (HttpURLConnection) mUrl
+								.openConnection();
+							connection.setRequestMethod("HEAD");
+							if (connection instanceof HttpsURLConnection) {
+								setupTrustManager((HttpsURLConnection) connection, interactive);
+							}
+							connection.connect();
+							String contentLength = connection.getHeaderField("Content-Length");
+							if (contentLength == null) {
+								throw new IOException();
+							}
+							try {
+								return Long.parseLong(contentLength, 10);
+							} catch (NumberFormatException e) {
+								throw new IOException();
+							}
 		}
 
 	}
@@ -234,7 +241,7 @@ public class HttpConnection implements Downloadable {
 
 		private void download() throws SSLHandshakeException, IOException {
 			HttpURLConnection connection = (HttpURLConnection) mUrl
-					.openConnection();
+				.openConnection();
 			if (connection instanceof HttpsURLConnection) {
 				setupTrustManager((HttpsURLConnection) connection, interactive);
 			}
@@ -300,4 +307,4 @@ public class HttpConnection implements Downloadable {
 	public String getMimeType() {
 		return "";
 	}
-}
\ No newline at end of file
+}
diff --git a/src/main/java/eu/siacs/conversations/utils/CryptoHelper.java b/src/main/java/eu/siacs/conversations/utils/CryptoHelper.java
index 7a36e2ba..48b0cdf3 100644
--- a/src/main/java/eu/siacs/conversations/utils/CryptoHelper.java
+++ b/src/main/java/eu/siacs/conversations/utils/CryptoHelper.java
@@ -2,12 +2,17 @@ package eu.siacs.conversations.utils;
 
 import java.security.SecureRandom;
 import java.text.Normalizer;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.LinkedHashSet;
 
-public class CryptoHelper {
+import eu.siacs.conversations.Config;
+
+public final class CryptoHelper {
 	public static final String FILETRANSFER = "?FILETRANSFERv1:";
-	final protected static char[] hexArray = "0123456789abcdef".toCharArray();
-	final protected static char[] vowels = "aeiou".toCharArray();
-	final protected static char[] consonants = "bcdfghjklmnpqrstvwxyz".toCharArray();
+	private final static char[] hexArray = "0123456789abcdef".toCharArray();
+	private final static char[] vowels = "aeiou".toCharArray();
+	private final static char[] consonants = "bcdfghjklmnpqrstvwxyz".toCharArray();
 	final public static byte[] ONE = new byte[] { 0, 0, 0, 1 };
 
 	public static String bytesToHex(byte[] bytes) {
@@ -45,7 +50,7 @@ public class CryptoHelper {
 		return randomWord(3, random) + "." + randomWord(7, random);
 	}
 
-	protected static String randomWord(int lenght, SecureRandom random) {
+	private static String randomWord(int lenght, SecureRandom random) {
 		StringBuilder builder = new StringBuilder(lenght);
 		for (int i = 0; i < lenght; ++i) {
 			if (i % 2 == 0) {
@@ -91,4 +96,10 @@ public class CryptoHelper {
 		builder.insert(35, " ");
 		return builder.toString();
 	}
+
+	public static String[] getSupportedCipherSuites(final String[] platformSupportedCipherSuites) {
+		final Collection<String> cipherSuites = new LinkedHashSet<>(Arrays.asList(Config.ENABLED_CIPHERS));
+		cipherSuites.retainAll(Arrays.asList(platformSupportedCipherSuites));
+		return cipherSuites.toArray(new String[cipherSuites.size()]);
+	}
 }
diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java
index 30437575..6424b1d5 100644
--- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java
+++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java
@@ -53,6 +53,7 @@ import eu.siacs.conversations.crypto.sasl.ScramSha1;
 import eu.siacs.conversations.entities.Account;
 import eu.siacs.conversations.generator.IqGenerator;
 import eu.siacs.conversations.services.XmppConnectionService;
+import eu.siacs.conversations.utils.CryptoHelper;
 import eu.siacs.conversations.utils.DNSHelper;
 import eu.siacs.conversations.utils.Xmlns;
 import eu.siacs.conversations.xml.Element;
@@ -514,6 +515,12 @@ public class XmppConnection implements Runnable {
 				supportedProtocols.remove("SSLv3");
 				supportProtocols = new String[supportedProtocols.size()];
 				supportedProtocols.toArray(supportProtocols);
+
+				final String[] cipherSuites = CryptoHelper.getSupportedCipherSuites(
+						sslSocket.getSupportedCipherSuites());
+				if (cipherSuites.length > 0) {
+					sslSocket.setEnabledCipherSuites(cipherSuites);
+				}
 			}
 			sslSocket.setEnabledProtocols(supportProtocols);
 
diff --git a/src/main/res/values/strings.xml b/src/main/res/values/strings.xml
index 7db008ea..54e81a2d 100644
--- a/src/main/res/values/strings.xml
+++ b/src/main/res/values/strings.xml
@@ -274,7 +274,7 @@
     <string name="pref_dont_save_encrypted">Don’t save encrypted messages</string>
     <string name="pref_dont_save_encrypted_summary">Warning: This could lead to message loss</string>
     <string name="pref_enable_legacy_ssl">Enable legacy SSL</string>
-    <string name="pref_enable_legacy_ssl_summary">Enables SSLv3 support for legacy servers. Warning: SSLv3 is considered insecure.</string>
+    <string name="pref_enable_legacy_ssl_summary">Enables legacy SSLv3 support and insecure SSL ciphers.</string>
     <string name="pref_expert_options">Expert options</string>
     <string name="pref_expert_options_summary">Please be careful with these</string>
     <string name="title_activity_about">About Conversations</string>