From 8874d58e68f251bb3a9367caa686e6ef08e55ce8 Mon Sep 17 00:00:00 2001 From: Christian Schneppe Date: Sat, 24 Jun 2017 10:05:30 +0200 Subject: also check for hostname in in certs if hostname is from trusted source --- .../de/duenndns/ssl/DomainHostnameVerifier.java | 10 ++++ .../de/duenndns/ssl/MemorizingTrustManager.java | 68 +++++++++------------- .../messenger/crypto/XmppDomainVerifier.java | 20 +++++-- .../messenger/http/HttpConnectionManager.java | 12 +--- .../de/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,23 +825,9 @@ public class MemorizingTrustManager { } @Override - public boolean verify(String hostname, SSLSession session) { - return verify(hostname, session, true); - } - } - - class NonInteractiveMemorizingHostnameVerifier extends MemorizingHostnameVerifier { - - public NonInteractiveMemorizingHostnameVerifier(HostnameVerifier wrapped) { - super(wrapped); + public boolean verify(String domain, SSLSession sslSession) { + return verify(domain, null, sslSession); } - - @Override - public boolean verify(String hostname, SSLSession session) { - return verify(hostname, session, false); - } - - } public X509TrustManager getNonInteractive(String 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 results = Resolver.resolve(account.getJid().getDomainpart()); - Log.d(Config.LOGTAG, "results: " + results); for (Iterator 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); } -- cgit v1.2.3