From 8afe7efc2cef82e5736b43e25c2bbec9cc968342 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sun, 16 Jul 2017 11:05:19 +0200 Subject: [PATCH] workaround for OpenFire: check CN first in self signed certs The self signed certificates created by OpenFire (Not sure if other certs are affected as well) will crash the Java/Android TLS stack when accessing getSubjectAlternativeNames() on the the peer certificate. This usually goes unnoticed in other applications since the DefaultHostnameVerifier checkes the CN first. That however is a violation of RFC6125 section 6.4.4 which requires us to check for the existence of SAN first. This commit adds a work around where in self signed certificates we check for the CN first as well. (Avoiding the call to getSubjectAlternativeNames()) --- .../crypto/XmppDomainVerifier.java | 38 ++++++++++++++++--- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/crypto/XmppDomainVerifier.java b/src/main/java/eu/siacs/conversations/crypto/XmppDomainVerifier.java index 0d7de892b..ba4e10160 100644 --- a/src/main/java/eu/siacs/conversations/crypto/XmppDomainVerifier.java +++ b/src/main/java/eu/siacs/conversations/crypto/XmppDomainVerifier.java @@ -16,12 +16,12 @@ import org.bouncycastle.cert.jcajce.JcaX509CertificateHolder; import java.io.IOException; import java.security.cert.Certificate; +import java.security.cert.CertificateEncodingException; import java.security.cert.X509Certificate; import java.util.ArrayList; import java.util.Collection; import java.util.List; -import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLSession; import de.duenndns.ssl.DomainHostnameVerifier; @@ -41,6 +41,13 @@ public class XmppDomainVerifier implements DomainHostnameVerifier { return false; } X509Certificate certificate = (X509Certificate) chain[0]; + if (isSelfSigned(certificate)) { + List domains = getCommonNames(certificate); + if (domains.size() == 1 && domains.get(0).equals(domain)) { + Log.d(LOGTAG,"accepted CN in cert self signed cert for "+domain); + return true; + } + } Collection> alternativeNames = certificate.getSubjectAlternativeNames(); List xmppAddrs = new ArrayList<>(); List srvNames = new ArrayList<>(); @@ -71,11 +78,7 @@ public class XmppDomainVerifier implements DomainHostnameVerifier { } } if (srvNames.size() == 0 && xmppAddrs.size() == 0 && domains.size() == 0) { - X500Name x500name = new JcaX509CertificateHolder(certificate).getSubject(); - RDN[] rdns = x500name.getRDNs(BCStyle.CN); - for (int i = 0; i < rdns.length; ++i) { - domains.add(IETFUtils.valueToString(x500name.getRDNs(BCStyle.CN)[i].getFirst().getValue())); - } + domains.addAll(domains); } Log.d(LOGTAG, "searching for " + domain + " in srvNames: " + srvNames + " xmppAddrs: " + xmppAddrs + " domains:" + domains); if (hostname != null) { @@ -90,6 +93,20 @@ public class XmppDomainVerifier implements DomainHostnameVerifier { } } + private static List getCommonNames(X509Certificate certificate) { + List domains = new ArrayList<>(); + try { + X500Name x500name = new JcaX509CertificateHolder(certificate).getSubject(); + RDN[] rdns = x500name.getRDNs(BCStyle.CN); + for (int i = 0; i < rdns.length; ++i) { + domains.add(IETFUtils.valueToString(x500name.getRDNs(BCStyle.CN)[i].getFirst().getValue())); + } + return domains; + } catch (CertificateEncodingException e) { + return domains; + } + } + private static Pair parseOtherName(byte[] otherName) { try { ASN1Primitive asn1Primitive = ASN1Primitive.fromByteArray(otherName); @@ -133,6 +150,15 @@ public class XmppDomainVerifier implements DomainHostnameVerifier { return false; } + private boolean isSelfSigned(X509Certificate certificate) { + try { + certificate.verify(certificate.getPublicKey()); + return true; + } catch (Exception e) { + return false; + } + } + @Override public boolean verify(String domain, SSLSession sslSession) { return verify(domain,null,sslSession);