From ac7855a332e869d66061fb80c41f708a283a1602 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Mon, 3 May 2021 08:28:03 +0200 Subject: [PATCH] show domains in manual cert accept dialog --- .../crypto/DomainHostnameVerifier.java | 3 +- .../crypto/XmppDomainVerifier.java | 138 ++++++++++-------- .../services/MemorizingTrustManager.java | 34 +++-- .../conversations/xmpp/XmppConnection.java | 12 +- 4 files changed, 117 insertions(+), 70 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/crypto/DomainHostnameVerifier.java b/src/main/java/eu/siacs/conversations/crypto/DomainHostnameVerifier.java index b6695611e..4349db45e 100644 --- a/src/main/java/eu/siacs/conversations/crypto/DomainHostnameVerifier.java +++ b/src/main/java/eu/siacs/conversations/crypto/DomainHostnameVerifier.java @@ -1,10 +1,11 @@ package eu.siacs.conversations.crypto; import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSession; public interface DomainHostnameVerifier extends HostnameVerifier { - boolean verify(String domain, String hostname, SSLSession sslSession); + boolean verify(String domain, String hostname, SSLSession sslSession) throws SSLPeerUnverifiedException; } diff --git a/src/main/java/eu/siacs/conversations/crypto/XmppDomainVerifier.java b/src/main/java/eu/siacs/conversations/crypto/XmppDomainVerifier.java index 7b741b864..ed17c5695 100644 --- a/src/main/java/eu/siacs/conversations/crypto/XmppDomainVerifier.java +++ b/src/main/java/eu/siacs/conversations/crypto/XmppDomainVerifier.java @@ -3,6 +3,8 @@ package eu.siacs.conversations.crypto; import android.util.Log; import android.util.Pair; +import com.google.common.collect.ImmutableList; + import org.bouncycastle.asn1.ASN1Primitive; import org.bouncycastle.asn1.DERIA5String; import org.bouncycastle.asn1.DERTaggedObject; @@ -18,15 +20,17 @@ import java.io.IOException; import java.net.IDN; import java.security.cert.Certificate; import java.security.cert.CertificateEncodingException; +import java.security.cert.CertificateParsingException; import java.security.cert.X509Certificate; import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Locale; +import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSession; -public class XmppDomainVerifier implements DomainHostnameVerifier { +public class XmppDomainVerifier { private static final String LOGTAG = "XmppDomainVerifier"; @@ -94,68 +98,93 @@ public class XmppDomainVerifier implements DomainHostnameVerifier { return false; } - @Override - public boolean verify(final String unicodeDomain,final String unicodeHostname, SSLSession sslSession) { + public boolean verify(final String unicodeDomain, final String unicodeHostname, SSLSession sslSession) throws SSLPeerUnverifiedException { final String domain = IDN.toASCII(unicodeDomain); final String hostname = unicodeHostname == null ? null : IDN.toASCII(unicodeHostname); + final Certificate[] chain = sslSession.getPeerCertificates(); + if (chain.length == 0 || !(chain[0] instanceof X509Certificate)) { + return false; + } + final X509Certificate certificate = (X509Certificate) chain[0]; + final List commonNames = getCommonNames(certificate); + if (isSelfSigned(certificate)) { + if (commonNames.size() == 1 && matchDomain(domain, commonNames)) { + Log.d(LOGTAG, "accepted CN in self signed cert as work around for " + domain); + return true; + } + } try { - final Certificate[] chain = sslSession.getPeerCertificates(); - if (chain.length == 0 || !(chain[0] instanceof X509Certificate)) { - return false; - } - final X509Certificate certificate = (X509Certificate) chain[0]; - final List commonNames = getCommonNames(certificate); - if (isSelfSigned(certificate)) { - if (commonNames.size() == 1 && matchDomain(domain, commonNames)) { - Log.d(LOGTAG, "accepted CN in self signed cert as work around for " + domain); - return true; - } - } - final Collection> alternativeNames = certificate.getSubjectAlternativeNames(); - final List xmppAddrs = new ArrayList<>(); - final List srvNames = new ArrayList<>(); - final List domains = new ArrayList<>(); - if (alternativeNames != null) { - for (List san : alternativeNames) { - final Integer type = (Integer) san.get(0); - if (type == 0) { - final Pair otherName = parseOtherName((byte[]) san.get(1)); - if (otherName != null && otherName.first != null && otherName.second != null) { - switch (otherName.first) { - case SRV_NAME: - srvNames.add(otherName.second.toLowerCase(Locale.US)); - break; - case XMPP_ADDR: - xmppAddrs.add(otherName.second.toLowerCase(Locale.US)); - break; - default: - Log.d(LOGTAG, "oid: " + otherName.first + " value: " + otherName.second); - } - } - } else if (type == 2) { - final Object value = san.get(1); - if (value instanceof String) { - domains.add(((String) value).toLowerCase(Locale.US)); - } - } - } - } - if (srvNames.size() == 0 && xmppAddrs.size() == 0 && domains.size() == 0) { - domains.addAll(commonNames); - } - Log.d(LOGTAG, "searching for " + domain + " in srvNames: " + srvNames + " xmppAddrs: " + xmppAddrs + " domains:" + domains); + final ValidDomains validDomains = parseValidDomains(certificate); + Log.d(LOGTAG, "searching for " + domain + " in srvNames: " + validDomains.srvNames + " xmppAddrs: " + validDomains.xmppAddrs + " domains:" + validDomains.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)); + return validDomains.xmppAddrs.contains(domain) + || validDomains.srvNames.contains("_xmpp-client." + domain) + || matchDomain(domain, validDomains.domains) + || (hostname != null && matchDomain(hostname, validDomains.domains)); } catch (final Exception e) { return false; } } + public static ValidDomains parseValidDomains(final X509Certificate certificate) throws CertificateParsingException { + final List commonNames = getCommonNames(certificate); + final Collection> alternativeNames = certificate.getSubjectAlternativeNames(); + final List xmppAddrs = new ArrayList<>(); + final List srvNames = new ArrayList<>(); + final List domains = new ArrayList<>(); + if (alternativeNames != null) { + for (List san : alternativeNames) { + final Integer type = (Integer) san.get(0); + if (type == 0) { + final Pair otherName = parseOtherName((byte[]) san.get(1)); + if (otherName != null && otherName.first != null && otherName.second != null) { + switch (otherName.first) { + case SRV_NAME: + srvNames.add(otherName.second.toLowerCase(Locale.US)); + break; + case XMPP_ADDR: + xmppAddrs.add(otherName.second.toLowerCase(Locale.US)); + break; + default: + Log.d(LOGTAG, "oid: " + otherName.first + " value: " + otherName.second); + } + } + } else if (type == 2) { + final Object value = san.get(1); + if (value instanceof String) { + domains.add(((String) value).toLowerCase(Locale.US)); + } + } + } + } + if (srvNames.size() == 0 && xmppAddrs.size() == 0 && domains.size() == 0) { + domains.addAll(commonNames); + } + return new ValidDomains(xmppAddrs, srvNames, domains); + } + + public static final class ValidDomains { + final List xmppAddrs; + final List srvNames; + final List domains; + + private ValidDomains(List xmppAddrs, List srvNames, List domains) { + this.xmppAddrs = xmppAddrs; + this.srvNames = srvNames; + this.domains = domains; + } + + public List all() { + ImmutableList.Builder all = new ImmutableList.Builder<>(); + all.addAll(xmppAddrs); + all.addAll(srvNames); + all.addAll(domains); + return all.build(); + } + } + private boolean isSelfSigned(X509Certificate certificate) { try { certificate.verify(certificate.getPublicKey()); @@ -164,9 +193,4 @@ public class XmppDomainVerifier implements DomainHostnameVerifier { return false; } } - - @Override - public boolean verify(String domain, SSLSession sslSession) { - return verify(domain, null, sslSession); - } } diff --git a/src/main/java/eu/siacs/conversations/services/MemorizingTrustManager.java b/src/main/java/eu/siacs/conversations/services/MemorizingTrustManager.java index 2f22911c1..b51b8de41 100644 --- a/src/main/java/eu/siacs/conversations/services/MemorizingTrustManager.java +++ b/src/main/java/eu/siacs/conversations/services/MemorizingTrustManager.java @@ -42,6 +42,7 @@ import android.util.SparseArray; import androidx.appcompat.app.AppCompatActivity; import com.google.common.base.Charsets; +import com.google.common.base.Joiner; import com.google.common.io.CharStreams; import org.json.JSONArray; @@ -61,12 +62,13 @@ import java.security.NoSuchAlgorithmException; import java.security.cert.Certificate; import java.security.cert.CertificateEncodingException; import java.security.cert.CertificateException; -import java.security.cert.CertificateExpiredException; +import java.security.cert.CertificateParsingException; import java.security.cert.X509Certificate; import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.Enumeration; import java.util.List; +import java.util.Locale; import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Pattern; @@ -76,6 +78,7 @@ import javax.net.ssl.TrustManagerFactory; import javax.net.ssl.X509TrustManager; import eu.siacs.conversations.R; +import eu.siacs.conversations.crypto.XmppDomainVerifier; import eu.siacs.conversations.entities.MTMDecision; import eu.siacs.conversations.http.HttpConnectionManager; import eu.siacs.conversations.persistance.FileBackend; @@ -93,6 +96,8 @@ import eu.siacs.conversations.ui.MemorizingActivity; */ public class MemorizingTrustManager { + private static final SimpleDateFormat DATE_FORMAT = new SimpleDateFormat("yyyy-MM-dd", Locale.US); + final static String DECISION_INTENT = "de.duenndns.ssl.DECISION"; public final static String DECISION_INTENT_ID = DECISION_INTENT + ".decisionId"; public final static String DECISION_INTENT_CERT = DECISION_INTENT + ".cert"; @@ -521,14 +526,24 @@ public class MemorizingTrustManager { return myId; } - private void certDetails(StringBuffer si, X509Certificate c) { - SimpleDateFormat validityDateFormater = new SimpleDateFormat("yyyy-MM-dd"); + private void certDetails(final StringBuffer si, final X509Certificate c, final boolean showValidFor) { + si.append("\n"); - si.append(c.getSubjectDN().toString()); + if (showValidFor) { + try { + si.append("Valid for: "); + si.append(Joiner.on(", ").join(XmppDomainVerifier.parseValidDomains(c).all())); + } catch (final CertificateParsingException e) { + si.append("Unable to parse Certificate"); + } + si.append("\n"); + } else { + si.append(c.getSubjectDN()); + } si.append("\n"); - si.append(validityDateFormater.format(c.getNotBefore())); + si.append(DATE_FORMAT.format(c.getNotBefore())); si.append(" - "); - si.append(validityDateFormater.format(c.getNotAfter())); + si.append(DATE_FORMAT.format(c.getNotAfter())); si.append("\nSHA-256: "); si.append(certHash(c, "SHA-256")); si.append("\nSHA-1: "); @@ -541,7 +556,7 @@ public class MemorizingTrustManager { private String certChainMessage(final X509Certificate[] chain, CertificateException cause) { Throwable e = cause; LOGGER.log(Level.FINE, "certChainMessage for " + e); - StringBuffer si = new StringBuffer(); + final StringBuffer si = new StringBuffer(); if (e.getCause() != null) { e = e.getCause(); // HACK: there is no sane way to check if the error is a "trust anchor @@ -556,8 +571,9 @@ public class MemorizingTrustManager { si.append(master.getString(R.string.mtm_connect_anyway)); si.append("\n\n"); si.append(master.getString(R.string.mtm_cert_details)); - for (X509Certificate c : chain) { - certDetails(si, c); + si.append('\n'); + for(int i = 0; i < chain.length; ++i) { + certDetails(si, chain[i], i == 0); } return si.toString(); } diff --git a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java index cd9cc2832..abe5d161f 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java @@ -46,6 +46,7 @@ import java.util.regex.Matcher; import javax.net.ssl.KeyManager; import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLSocket; import javax.net.ssl.SSLSocketFactory; import javax.net.ssl.X509KeyManager; @@ -826,10 +827,15 @@ public class XmppConnection implements Runnable { SSLSocketHelper.setHostname(sslSocket, IDN.toASCII(account.getServer())); SSLSocketHelper.setApplicationProtocol(sslSocket, "xmpp-client"); final XmppDomainVerifier xmppDomainVerifier = new XmppDomainVerifier(); - if (!xmppDomainVerifier.verify(account.getServer(), this.verifiedHostname, sslSocket.getSession())) { - Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": TLS certificate domain verification failed"); + try { + if (!xmppDomainVerifier.verify(account.getServer(), this.verifiedHostname, sslSocket.getSession())) { + Log.d(Config.LOGTAG, account.getJid().asBareJid() + ": TLS certificate domain verification failed"); + FileBackend.close(sslSocket); + throw new StateChangingException(Account.State.TLS_ERROR_DOMAIN); + } + } catch (final SSLPeerUnverifiedException e) { FileBackend.close(sslSocket); - throw new StateChangingException(Account.State.TLS_ERROR_DOMAIN); + throw new StateChangingException(Account.State.TLS_ERROR); } return sslSocket; }