From 8874d58e68f251bb3a9367caa686e6ef08e55ce8 Mon Sep 17 00:00:00 2001
From: Christian Schneppe <christian@pix-art.de>
Date: Sat, 24 Jun 2017 10:05:30 +0200
Subject: [PATCH] also check for hostname in in certs if hostname is from
 trusted source

---
 .../duenndns/ssl/DomainHostnameVerifier.java  | 10 +++
 .../duenndns/ssl/MemorizingTrustManager.java  | 68 ++++++++-----------
 .../messenger/crypto/XmppDomainVerifier.java  | 20 ++++--
 .../messenger/http/HttpConnectionManager.java | 12 +---
 .../pixart/messenger/xmpp/XmppConnection.java | 32 ++++-----
 5 files changed, 72 insertions(+), 70 deletions(-)
 create mode 100644 libs/MemorizingTrustManager/src/de/duenndns/ssl/DomainHostnameVerifier.java

diff --git a/libs/MemorizingTrustManager/src/de/duenndns/ssl/DomainHostnameVerifier.java b/libs/MemorizingTrustManager/src/de/duenndns/ssl/DomainHostnameVerifier.java
new file mode 100644
index 000000000..62b625ab9
--- /dev/null
+++ b/libs/MemorizingTrustManager/src/de/duenndns/ssl/DomainHostnameVerifier.java
@@ -0,0 +1,10 @@
+package de.duenndns.ssl;
+
+import javax.net.ssl.HostnameVerifier;
+import javax.net.ssl.SSLSession;
+
+public interface DomainHostnameVerifier extends HostnameVerifier {
+
+    boolean verify(String domain, String hostname, SSLSession sslSession);
+
+}
diff --git a/libs/MemorizingTrustManager/src/de/duenndns/ssl/MemorizingTrustManager.java b/libs/MemorizingTrustManager/src/de/duenndns/ssl/MemorizingTrustManager.java
index e46b6d5fb..a3aa561ef 100644
--- a/libs/MemorizingTrustManager/src/de/duenndns/ssl/MemorizingTrustManager.java
+++ b/libs/MemorizingTrustManager/src/de/duenndns/ssl/MemorizingTrustManager.java
@@ -298,18 +298,11 @@ public class MemorizingTrustManager {
      * @return a new hostname verifier using the MTM's key store
      * @throws IllegalArgumentException if the defaultVerifier parameter is null
      */
-    public HostnameVerifier wrapHostnameVerifier(final HostnameVerifier defaultVerifier) {
-        if (defaultVerifier == null)
+    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) {
@@ -787,31 +780,40 @@ 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;
                     }
@@ -823,25 +825,11 @@ public class MemorizingTrustManager {
         }
 
         @Override
-        public boolean verify(String hostname, SSLSession session) {
-            return verify(hostname, session, true);
+        public boolean verify(String domain, SSLSession sslSession) {
+            return verify(domain, null, sslSession);
         }
     }
 
-    class NonInteractiveMemorizingHostnameVerifier extends MemorizingHostnameVerifier {
-
-        public NonInteractiveMemorizingHostnameVerifier(HostnameVerifier wrapped) {
-            super(wrapped);
-        }
-
-        @Override
-        public boolean verify(String hostname, SSLSession session) {
-            return verify(hostname, session, false);
-        }
-
-
-    }
-
     public X509TrustManager getNonInteractive(String domain) {
         return new NonInteractiveMemorizingTrustManager(domain);
     }
diff --git a/src/main/java/de/pixart/messenger/crypto/XmppDomainVerifier.java b/src/main/java/de/pixart/messenger/crypto/XmppDomainVerifier.java
index d305a33d5..26aa268e4 100644
--- a/src/main/java/de/pixart/messenger/crypto/XmppDomainVerifier.java
+++ b/src/main/java/de/pixart/messenger/crypto/XmppDomainVerifier.java
@@ -21,10 +21,11 @@ import java.util.ArrayList;
 import java.util.Collection;
 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 +33,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 +77,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 +131,9 @@ public class XmppDomainVerifier implements HostnameVerifier {
         }
         return false;
     }
+
+    @Override
+    public boolean verify(String domain, SSLSession sslSession) {
+        return verify(domain, null, sslSession);
+    }
 }
diff --git a/src/main/java/de/pixart/messenger/http/HttpConnectionManager.java b/src/main/java/de/pixart/messenger/http/HttpConnectionManager.java
index 20213521f..340bc6d7a 100644
--- a/src/main/java/de/pixart/messenger/http/HttpConnectionManager.java
+++ b/src/main/java/de/pixart/messenger/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());
diff --git a/src/main/java/de/pixart/messenger/xmpp/XmppConnection.java b/src/main/java/de/pixart/messenger/xmpp/XmppConnection.java
index a62f5c2d3..f7f1ce8e9 100644
--- a/src/main/java/de/pixart/messenger/xmpp/XmppConnection.java
+++ b/src/main/java/de/pixart/messenger/xmpp/XmppConnection.java
@@ -40,7 +40,6 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.regex.Matcher;
 
-import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.KeyManager;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLSession;
@@ -49,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 de.pixart.messenger.Config;
 import de.pixart.messenger.crypto.XmppDomainVerifier;
@@ -142,6 +142,7 @@ public class XmppConnection implements Runnable {
 
     private SaslMechanism saslMechanism;
     private String webRegistrationUrl = null;
+    private String verifiedHostname = null;
 
     private class MyKeyManager implements X509KeyManager {
         @Override
@@ -278,6 +279,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);
@@ -304,10 +306,11 @@ public class XmppConnection implements Runnable {
                 }
             } else 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());
@@ -319,9 +322,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;
 
@@ -332,7 +337,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);
                             }
@@ -372,7 +378,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()) {
@@ -382,6 +387,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());
@@ -489,9 +495,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) {
@@ -512,13 +518,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);
     }
 
@@ -841,7 +841,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);
             }