From 2ed71df01a426e8addfcb20a710f08f9bc56425f Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sat, 17 Jun 2017 19:54:09 +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 | 65 ++++++++----------- .../crypto/XmppDomainVerifier.java | 19 +++++- .../http/HttpConnectionManager.java | 12 +--- .../conversations/xmpp/XmppConnection.java | 31 ++++----- 5 files changed, 72 insertions(+), 65 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 014790280..01a8c8e2a 100644 --- a/libs/MemorizingTrustManager/src/de/duenndns/ssl/MemorizingTrustManager.java +++ b/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); diff --git a/src/main/java/eu/siacs/conversations/crypto/XmppDomainVerifier.java b/src/main/java/eu/siacs/conversations/crypto/XmppDomainVerifier.java index 1fca865ed..0d7de892b 100644 --- a/src/main/java/eu/siacs/conversations/crypto/XmppDomainVerifier.java +++ b/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); + } } diff --git a/src/main/java/eu/siacs/conversations/http/HttpConnectionManager.java b/src/main/java/eu/siacs/conversations/http/HttpConnectionManager.java index 7e44cef71..d0d571818 100644 --- a/src/main/java/eu/siacs/conversations/http/HttpConnectionManager.java +++ b/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()); diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index a2528078a..10cf576c5 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/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 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()) { @@ -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); }