also check for hostname in in certs if hostname is from trusted source

Daniel Gultsch created

Change summary

libs/MemorizingTrustManager/src/de/duenndns/ssl/DomainHostnameVerifier.java | 10 
libs/MemorizingTrustManager/src/de/duenndns/ssl/MemorizingTrustManager.java | 65 
src/main/java/eu/siacs/conversations/crypto/XmppDomainVerifier.java         | 19 
src/main/java/eu/siacs/conversations/http/HttpConnectionManager.java        | 12 
src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java               | 31 
5 files changed, 72 insertions(+), 65 deletions(-)

Detailed changes

libs/MemorizingTrustManager/src/de/duenndns/ssl/MemorizingTrustManager.java 🔗

@@ -292,18 +292,11 @@ public class MemorizingTrustManager {
 	 *
 	 * @throws IllegalArgumentException if the defaultVerifier parameter is null
 	 */
-	public HostnameVerifier wrapHostnameVerifier(final HostnameVerifier defaultVerifier) {
+	public DomainHostnameVerifier wrapHostnameVerifier(final HostnameVerifier defaultVerifier, final boolean interactive) {
 		if (defaultVerifier == null)
 			throw new IllegalArgumentException("The default verifier may not be null");
 		
-		return new MemorizingHostnameVerifier(defaultVerifier);
-	}
-	
-	public HostnameVerifier wrapHostnameVerifierNonInteractive(final HostnameVerifier defaultVerifier) {
-		if (defaultVerifier == null)
-			throw new IllegalArgumentException("The default verifier may not be null");
-		
-		return new NonInteractiveMemorizingHostnameVerifier(defaultVerifier);
+		return new MemorizingHostnameVerifier(defaultVerifier, interactive);
 	}
 	
 	X509TrustManager getTrustManager(KeyStore ks) {
@@ -781,31 +774,41 @@ public class MemorizingTrustManager {
 		}
 	}
 	
-	class MemorizingHostnameVerifier implements HostnameVerifier {
-		private HostnameVerifier defaultVerifier;
+	class MemorizingHostnameVerifier implements DomainHostnameVerifier {
+		private final HostnameVerifier defaultVerifier;
+		private final boolean interactive;
 		
-		public MemorizingHostnameVerifier(HostnameVerifier wrapped) {
-			defaultVerifier = wrapped;
+		public MemorizingHostnameVerifier(HostnameVerifier wrapped, boolean interactive) {
+			this.defaultVerifier = wrapped;
+			this.interactive = interactive;
 		}
 
-		protected boolean verify(String hostname, SSLSession session, boolean interactive) {
-			LOGGER.log(Level.FINE, "hostname verifier for " + hostname + ", trying default verifier first");
+		@Override
+		public boolean verify(String domain, String hostname, SSLSession session) {
+			LOGGER.log(Level.FINE, "hostname verifier for " + domain + ", trying default verifier first");
 			// if the default verifier accepts the hostname, we are done
-			if (defaultVerifier.verify(hostname, session)) {
-				LOGGER.log(Level.FINE, "default verifier accepted " + hostname);
-				return true;
+			if (defaultVerifier instanceof DomainHostnameVerifier) {
+				if (((DomainHostnameVerifier) defaultVerifier).verify(domain,hostname, session)) {
+					return true;
+				}
+			} else {
+				if (defaultVerifier.verify(domain, session)) {
+					return true;
+				}
 			}
+
+
 			// otherwise, we check if the hostname is an alias for this cert in our keystore
 			try {
 				X509Certificate cert = (X509Certificate)session.getPeerCertificates()[0];
 				//Log.d(TAG, "cert: " + cert);
-				if (cert.equals(appKeyStore.getCertificate(hostname.toLowerCase(Locale.US)))) {
-					LOGGER.log(Level.FINE, "certificate for " + hostname + " is in our keystore. accepting.");
+				if (cert.equals(appKeyStore.getCertificate(domain.toLowerCase(Locale.US)))) {
+					LOGGER.log(Level.FINE, "certificate for " + domain + " is in our keystore. accepting.");
 					return true;
 				} else {
-					LOGGER.log(Level.FINE, "server " + hostname + " provided wrong certificate, asking user.");
+					LOGGER.log(Level.FINE, "server " + domain + " provided wrong certificate, asking user.");
 					if (interactive) {
-						return interactHostname(cert, hostname);
+						return interactHostname(cert, domain);
 					} else {
 						return false;
 					}
@@ -815,25 +818,13 @@ public class MemorizingTrustManager {
 				return false;
 			}
 		}
-		
-		@Override
-		public boolean verify(String hostname, SSLSession session) {
-			return verify(hostname, session, true);
-		}
-	}
-	
-	class NonInteractiveMemorizingHostnameVerifier extends MemorizingHostnameVerifier {
 
-		public NonInteractiveMemorizingHostnameVerifier(HostnameVerifier wrapped) {
-			super(wrapped);
-		}
 		@Override
-		public boolean verify(String hostname, SSLSession session) {
-			return verify(hostname, session, false);
+		public boolean verify(String domain, SSLSession sslSession) {
+			return verify(domain,null,sslSession);
 		}
-		
-		
 	}
+
 	
 	public X509TrustManager getNonInteractive(String domain) {
 		return new NonInteractiveMemorizingTrustManager(domain);

src/main/java/eu/siacs/conversations/crypto/XmppDomainVerifier.java 🔗

@@ -24,7 +24,9 @@ import java.util.List;
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLSession;
 
-public class XmppDomainVerifier implements HostnameVerifier {
+import de.duenndns.ssl.DomainHostnameVerifier;
+
+public class XmppDomainVerifier implements DomainHostnameVerifier {
 
 	private static final String LOGTAG = "XmppDomainVerifier";
 
@@ -32,7 +34,7 @@ public class XmppDomainVerifier implements HostnameVerifier {
 	private final String xmppAddr = "1.3.6.1.5.5.7.8.5";
 
 	@Override
-	public boolean verify(String domain, SSLSession sslSession) {
+	public boolean verify(String domain, String hostname, SSLSession sslSession) {
 		try {
 			Certificate[] chain = sslSession.getPeerCertificates();
 			if (chain.length == 0 || !(chain[0] instanceof X509Certificate)) {
@@ -76,7 +78,13 @@ public class XmppDomainVerifier implements HostnameVerifier {
 				}
 			}
 			Log.d(LOGTAG, "searching for " + domain + " in srvNames: " + srvNames + " xmppAddrs: " + xmppAddrs + " domains:" + domains);
-			return xmppAddrs.contains(domain) || srvNames.contains("_xmpp-client." + domain) || matchDomain(domain, domains);
+			if (hostname != null) {
+				Log.d(LOGTAG,"also trying to verify hostname "+hostname);
+			}
+			return xmppAddrs.contains(domain)
+					|| srvNames.contains("_xmpp-client." + domain)
+					|| matchDomain(domain, domains)
+					|| (hostname != null && matchDomain(hostname,domains));
 		} catch (Exception e) {
 			return false;
 		}
@@ -124,4 +132,9 @@ public class XmppDomainVerifier implements HostnameVerifier {
 		}
 		return false;
 	}
+
+	@Override
+	public boolean verify(String domain, SSLSession sslSession) {
+		return verify(domain,null,sslSession);
+	}
 }

src/main/java/eu/siacs/conversations/http/HttpConnectionManager.java 🔗

@@ -58,19 +58,11 @@ public class HttpConnectionManager extends AbstractConnectionManager {
 
 	public void setupTrustManager(final HttpsURLConnection connection, final boolean interactive) {
 		final X509TrustManager trustManager;
-		final HostnameVerifier hostnameVerifier;
+		final HostnameVerifier hostnameVerifier = mXmppConnectionService.getMemorizingTrustManager().wrapHostnameVerifier(new StrictHostnameVerifier(), interactive);
 		if (interactive) {
 			trustManager = mXmppConnectionService.getMemorizingTrustManager().getInteractive();
-			hostnameVerifier = mXmppConnectionService
-					.getMemorizingTrustManager().wrapHostnameVerifier(
-							new StrictHostnameVerifier());
 		} else {
-			trustManager = mXmppConnectionService.getMemorizingTrustManager()
-					.getNonInteractive();
-			hostnameVerifier = mXmppConnectionService
-					.getMemorizingTrustManager()
-					.wrapHostnameVerifierNonInteractive(
-							new StrictHostnameVerifier());
+			trustManager = mXmppConnectionService.getMemorizingTrustManager().getNonInteractive();
 		}
 		try {
 			final SSLSocketFactory sf = new TLSSocketFactory(new X509TrustManager[]{trustManager}, mXmppConnectionService.getRNG());

src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java 🔗

@@ -48,6 +48,7 @@ import javax.net.ssl.SSLSocketFactory;
 import javax.net.ssl.X509KeyManager;
 import javax.net.ssl.X509TrustManager;
 
+import de.duenndns.ssl.DomainHostnameVerifier;
 import de.duenndns.ssl.MemorizingTrustManager;
 import eu.siacs.conversations.Config;
 import eu.siacs.conversations.crypto.XmppDomainVerifier;
@@ -138,6 +139,7 @@ public class XmppConnection implements Runnable {
 
 	private SaslMechanism saslMechanism;
 	private String webRegistrationUrl = null;
+	private String verifiedHostname = null;
 
 	private class MyKeyManager implements X509KeyManager {
 		@Override
@@ -268,6 +270,7 @@ public class XmppConnection implements Runnable {
 		Log.d(Config.LOGTAG, account.getJid().toBareJid().toString() + ": connecting");
 		features.encryptionEnabled = false;
 		this.attempt++;
+		this.verifiedHostname = null; //will be set if user entered hostname is being used or hostname was verified with dnssec
 		try {
 			Socket localSocket;
 			shouldAuthenticate = needsBinding = !account.isOptionSet(Account.OPTION_REGISTER);
@@ -276,10 +279,11 @@ public class XmppConnection implements Runnable {
 			final boolean extended = mXmppConnectionService.showExtendedConnectionOptions();
 			if (useTor) {
 				String destination;
-				if (account.getHostname() == null || account.getHostname().isEmpty()) {
+				if (account.getHostname().isEmpty()) {
 					destination = account.getServer().toString();
 				} else {
 					destination = account.getHostname();
+					this.verifiedHostname = destination;
 				}
 				Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": connect to " + destination + " via Tor");
 				localSocket = SocksSocketFactory.createSocketOverTor(destination, account.getPort());
@@ -291,9 +295,11 @@ public class XmppConnection implements Runnable {
 				} catch (Exception e) {
 					throw new IOException(e.getMessage());
 				}
-			} else if (extended && account.getHostname() != null && !account.getHostname().isEmpty()) {
+			} else if (extended && !account.getHostname().isEmpty()) {
 
-				InetSocketAddress address = new InetSocketAddress(account.getHostname(), account.getPort());
+				this.verifiedHostname = account.getHostname();
+
+				InetSocketAddress address = new InetSocketAddress(this.verifiedHostname, account.getPort());
 
 				features.encryptionEnabled = account.getPort() == 5223;
 
@@ -304,7 +310,8 @@ public class XmppConnection implements Runnable {
 							localSocket = tlsFactoryVerifier.factory.createSocket();
 							localSocket.connect(address, Config.SOCKET_TIMEOUT * 1000);
 							final SSLSession session = ((SSLSocket) localSocket).getSession();
-							if (!tlsFactoryVerifier.verifier.verify(account.getServer().getDomainpart(), session)) {
+							final String domain = account.getJid().getDomainpart();
+							if (!tlsFactoryVerifier.verifier.verify(domain, this.verifiedHostname, session)) {
 								Log.d(Config.LOGTAG, account.getJid().toBareJid() + ": TLS certificate verification failed");
 								throw new StateChangingException(Account.State.TLS_ERROR);
 							}
@@ -344,7 +351,6 @@ public class XmppConnection implements Runnable {
 				}
 			} else {
 				List<Resolver.Result> results = Resolver.resolve(account.getJid().getDomainpart());
-				Log.d(Config.LOGTAG,"results: "+results);
 				for (Iterator<Resolver.Result> iterator = results.iterator(); iterator.hasNext(); ) {
 					final Resolver.Result result = iterator.next();
 					if (Thread.currentThread().isInterrupted()) {
@@ -354,6 +360,7 @@ public class XmppConnection implements Runnable {
 					try {
 						// if tls is true, encryption is implied and must not be started
 						features.encryptionEnabled = result.isDirectTls();
+						verifiedHostname = result.isAuthenticated() ? result.getHostname().toString() : null;
 						final InetSocketAddress addr;
 						if (result.getIp() != null) {
 							addr = new InetSocketAddress(result.getIp(), result.getPort());
@@ -459,9 +466,9 @@ public class XmppConnection implements Runnable {
 
 	private static class TlsFactoryVerifier {
 		private final SSLSocketFactory factory;
-		private final HostnameVerifier verifier;
+		private final DomainHostnameVerifier verifier;
 
-		public TlsFactoryVerifier(final SSLSocketFactory factory, final HostnameVerifier verifier) throws IOException {
+		public TlsFactoryVerifier(final SSLSocketFactory factory, final DomainHostnameVerifier verifier) throws IOException {
 			this.factory = factory;
 			this.verifier = verifier;
 			if (factory == null || verifier == null) {
@@ -482,13 +489,7 @@ public class XmppConnection implements Runnable {
 		String domain = account.getJid().getDomainpart();
 		sc.init(keyManager, new X509TrustManager[]{mInteractive ? trustManager.getInteractive(domain) : trustManager.getNonInteractive(domain)}, mXmppConnectionService.getRNG());
 		final SSLSocketFactory factory = sc.getSocketFactory();
-		final HostnameVerifier verifier;
-		if (mInteractive) {
-			verifier = trustManager.wrapHostnameVerifier(new XmppDomainVerifier());
-		} else {
-			verifier = trustManager.wrapHostnameVerifierNonInteractive(new XmppDomainVerifier());
-		}
-
+		final DomainHostnameVerifier verifier = trustManager.wrapHostnameVerifier(new XmppDomainVerifier(), mInteractive);
 		return new TlsFactoryVerifier(factory, verifier);
 	}
 
@@ -812,7 +813,7 @@ public class XmppConnection implements Runnable {
 
 			SSLSocketHelper.setSecurity(sslSocket);
 
-			if (!tlsFactoryVerifier.verifier.verify(account.getServer().getDomainpart(), sslSocket.getSession())) {
+			if (!tlsFactoryVerifier.verifier.verify(account.getServer().getDomainpart(), this.verifiedHostname, sslSocket.getSession())) {
 				Log.d(Config.LOGTAG,account.getJid().toBareJid()+": TLS certificate verification failed");
 				throw new StateChangingException(Account.State.TLS_ERROR);
 			}