From 5bc2ca13f2f89114ff82d1ba8b99fe160ad16784 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Eeden?= Date: Tue, 2 Jun 2020 21:47:03 +0200 Subject: [PATCH 1/2] Check SubjectAlternativeName for TLS instead of commonName Related: * https://tools.ietf.org/html/rfc6125#section-6.4.4 * https://bugs.mysql.com/bug.php?id=93301 Connector/C++ * https://bugs.mysql.com/bug.php?id=68052 libmysqlclient Fixes: * https://bugs.mysql.com/bug.php?id=92903 Instead of manually checking SAN and CN it would be nice if there would be a library which knows how to do this correctly. I only did very minimal testing. --- .../mysql/cj/protocol/ExportControlled.java | 54 ++++++++++++++----- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/src/main/core-impl/java/com/mysql/cj/protocol/ExportControlled.java b/src/main/core-impl/java/com/mysql/cj/protocol/ExportControlled.java index 36450941..941d9964 100644 --- a/src/main/core-impl/java/com/mysql/cj/protocol/ExportControlled.java +++ b/src/main/core-impl/java/com/mysql/cj/protocol/ExportControlled.java @@ -61,6 +61,7 @@ import java.security.spec.X509EncodedKeySpec; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.List; import java.util.Properties; import java.util.Set; @@ -410,23 +411,48 @@ public void checkServerTrusted(X509Certificate[] chain, String authType) throws // verify server certificate identity if (this.hostName != null) { - String dn = chain[0].getSubjectX500Principal().getName(X500Principal.RFC2253); - String cn = null; - try { - LdapName ldapDN = new LdapName(dn); - for (Rdn rdn : ldapDN.getRdns()) { - if (rdn.getType().equalsIgnoreCase("CN")) { - cn = rdn.getValue().toString(); - break; + + // Check SubjectAlternativeName + // See https://tools.ietf.org/html/rfc6125#section-6.4.4 + final Collection> sans = chain[0].getSubjectAlternativeNames(); + if (sans != null) { + String sanmatch = null; + for (final List san : sans) { + Integer sanType = (Integer) san.get(0); + String sanVal = (String) san.get(1); + // 2 = dNSName + // 7 = iPAddress + if ((sanType == 2) || ( sanType == 7)) { + if (this.hostName.equalsIgnoreCase(sanVal)) { + sanmatch = sanVal; + break; + } } } - } catch (InvalidNameException e) { - throw new CertificateException("Failed to retrieve the Common Name (CN) from the server certificate."); - } + if (!this.hostName.equalsIgnoreCase(sanmatch)) { + throw new CertificateException("None of the Subject Alternative Names matched the server hostname '" + + this.hostName + "'."); + } + } else { + // Check CommonName + String dn = chain[0].getSubjectX500Principal().getName(X500Principal.RFC2253); + String cn = null; + try { + LdapName ldapDN = new LdapName(dn); + for (Rdn rdn : ldapDN.getRdns()) { + if (rdn.getType().equalsIgnoreCase("CN")) { + cn = rdn.getValue().toString(); + break; + } + } + } catch (InvalidNameException e) { + throw new CertificateException("Failed to retrieve the Common Name (CN) from the server certificate."); + } - if (!this.hostName.equalsIgnoreCase(cn)) { - throw new CertificateException("Server certificate identity check failed. The certificate Common Name '" + cn - + "' does not match with '" + this.hostName + "'."); + if (!this.hostName.equalsIgnoreCase(cn)) { + throw new CertificateException("Server certificate identity check failed. The certificate Common Name '" + cn + + "' does not match with '" + this.hostName + "'."); + } } } } From ba6b5da85f39cdeafc9c2b1d1055142c85b6f58b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Eeden?= Date: Wed, 3 Jun 2020 09:48:43 +0200 Subject: [PATCH 2/2] Fix comment from review: "patch looks good to me, with one exception - this line: `String sanVal = (String) san.get(1);` should be moved after you verified the type is 2 or 7. The second object in that list may be a byte array in some cases, per the javaDoc (though scouting the jdk src, this is never the case)" --- .../core-impl/java/com/mysql/cj/protocol/ExportControlled.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/core-impl/java/com/mysql/cj/protocol/ExportControlled.java b/src/main/core-impl/java/com/mysql/cj/protocol/ExportControlled.java index 941d9964..d873323a 100644 --- a/src/main/core-impl/java/com/mysql/cj/protocol/ExportControlled.java +++ b/src/main/core-impl/java/com/mysql/cj/protocol/ExportControlled.java @@ -419,10 +419,10 @@ public void checkServerTrusted(X509Certificate[] chain, String authType) throws String sanmatch = null; for (final List san : sans) { Integer sanType = (Integer) san.get(0); - String sanVal = (String) san.get(1); // 2 = dNSName // 7 = iPAddress if ((sanType == 2) || ( sanType == 7)) { + String sanVal = (String) san.get(1); if (this.hostName.equalsIgnoreCase(sanVal)) { sanmatch = sanVal; break;