Merge pull request #3554 from mpg/x509-verify-non-dns-san-dev

X509 verify non-DNS SANs

7 files changed, 63 insertions, 3 deletions

diff --git a/ChangeLog.d/x509-verify-non-dns-san.txt b/ChangeLog.d/x509-verify-non-dns-san.txt
new file mode 100644
index 0000000..0cd81b3
--- /dev/null
+++ b/ChangeLog.d/x509-verify-non-dns-san.txt
@@ -0,0 +1,11 @@
+Security
+   * Fix a vulnerability in the verification of X.509 certificates when
+     matching the expected common name (the cn argument of
+     mbedtls_x509_crt_verify()) with the actual certificate name: when the
+     subjecAltName extension is present, the expected name was compared to any
+     name in that extension regardless of its type. This means that an
+     attacker could for example impersonate a 4-bytes or 16-byte domain by
+     getting a certificate for the corresponding IPv4 or IPv6 (this would
+     require the attacker to control that IP address, though). Similar attacks
+     using other subjectAltName name types might be possible. Found and
+     reported by kFYatek in #3498.
diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h
index ab0d0cd..d24204d 100644
--- a/include/mbedtls/x509_crt.h
+++ b/include/mbedtls/x509_crt.h
@@ -585,8 +585,11 @@ int mbedtls_x509_crt_verify_info( char *buf, size_t size, const char *prefix,
  * \param crt      The certificate chain to be verified.
  * \param trust_ca The list of trusted CAs.
  * \param ca_crl   The list of CRLs for trusted CAs.
- * \param cn       The expected Common Name. This may be \c NULL if the
- *                 CN need not be verified.
+ * \param cn       The expected Common Name. This will be checked to be
+ *                 present in the certificate's subjectAltNames extension or,
+ *                 if this extension is absent, as a CN component in its
+ *                 Subject name. Currently only DNS names are supported. This
+ *                 may be \c NULL if the CN need not be verified.
  * \param flags    The address at which to store the result of the verification.
  *                 If the verification couldn't be completed, the flag value is
  *                 set to (uint32_t) -1.
diff --git a/library/x509_crt.c b/library/x509_crt.c
index 8fd8b86..2627224 100644
--- a/library/x509_crt.c
+++ b/library/x509_crt.c
@@ -3008,6 +3008,25 @@ static int x509_crt_check_cn( const mbedtls_x509_buf *name,
 }
 
 /*
+ * Check for SAN match, see RFC 5280 Section 4.2.1.6
+ */
+static int x509_crt_check_san( const mbedtls_x509_buf *name,
+                               const char *cn, size_t cn_len )
+{
+    const unsigned char san_type = (unsigned char) name->tag &
+                                   MBEDTLS_ASN1_TAG_VALUE_MASK;
+
+    /* dNSName */
+    if( san_type == MBEDTLS_X509_SAN_DNS_NAME )
+        return( x509_crt_check_cn( name, cn, cn_len ) );
+
+    /* (We may handle other types here later.) */
+
+    /* Unrecognized type */
+    return( -1 );
+}
+
+/*
  * Verify the requested CN - only call this if cn is not NULL!
  */
 static void x509_crt_verify_name( const mbedtls_x509_crt *crt,
@@ -3022,7 +3041,7 @@ static void x509_crt_verify_name( const mbedtls_x509_crt *crt,
     {
         for( cur = &crt->subject_alt_names; cur != NULL; cur = cur->next )
         {
-            if( x509_crt_check_cn( &cur->buf, cn, cn_len ) == 0 )
+            if( x509_crt_check_san( &cur->buf, cn, cn_len ) == 0 )
                 break;
         }
 
diff --git a/tests/data_files/Makefile b/tests/data_files/Makefile
index 99d64eb..40c22f5 100644
--- a/tests/data_files/Makefile
+++ b/tests/data_files/Makefile
@@ -270,6 +270,10 @@ server5-unsupported_othername.crt: server5.key
 server5-fan.crt: server5.key
 	$(OPENSSL) req -x509 -new -subj "/C=UK/O=Mbed TLS/CN=Mbed TLS FAN" -set_serial 77 -config $(test_ca_config_file) -extensions fan_cert -days 3650 -sha256 -key server5.key -out $@
 
+server5-tricky-ip-san.crt: server5.key
+	$(OPENSSL) req -x509 -new -subj "/C=UK/O=Mbed TLS/CN=Mbed TLS Tricky IP SAN" -set_serial 77 -config $(test_ca_config_file) -extensions tricky_ip_san -days 3650 -sha256 -key server5.key -out $@
+all_final += server5-tricky-ip-san.crt
+
 server10-badsign.crt: server10.crt
 	{ head -n-2 $<; tail -n-2 $< | sed -e '1s/0\(=*\)$$/_\1/' -e '1s/[^_=]\(=*\)$$/0\1/' -e '1s/_/1/'; } > $@
 all_final += server10-badsign.crt
diff --git a/tests/data_files/server5-tricky-ip-san.crt b/tests/data_files/server5-tricky-ip-san.crt
new file mode 100644
index 0000000..135830f
--- /dev/null
+++ b/tests/data_files/server5-tricky-ip-san.crt
@@ -0,0 +1,11 @@
+-----BEGIN CERTIFICATE-----
+MIIBljCCATygAwIBAgIBTTAKBggqhkjOPQQDAjBBMQswCQYDVQQGEwJVSzERMA8G
+A1UECgwITWJlZCBUTFMxHzAdBgNVBAMMFk1iZWQgVExTIFRyaWNreSBJUCBTQU4w
+HhcNMjAwNzIzMTAyNzQ2WhcNMzAwNzIxMTAyNzQ2WjBBMQswCQYDVQQGEwJVSzER
+MA8GA1UECgwITWJlZCBUTFMxHzAdBgNVBAMMFk1iZWQgVExTIFRyaWNreSBJUCBT
+QU4wWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQ3zFbZdgkeWnI+x1kt/yBu7nz5
+BpF00K0UtfdoIllikk7lANgjEf/qL9I0XV0WvYqIwmt3DVXNiioO+gHItO3/oyUw
+IzAhBgNVHREEGjAYhwRhYmNkhxBhYmNkLmV4YW1wbGUuY29tMAoGCCqGSM49BAMC
+A0gAMEUCIFDc8ZALA/9Zv7dZTWrZOOp/dgPAEJRT+h68nD6KF+XyAiEAs1QqugOo
+Dwru0DSEmpYkmj1Keunpd0VopM0joC1cc5A=
+-----END CERTIFICATE-----
diff --git a/tests/data_files/test-ca.opensslconf b/tests/data_files/test-ca.opensslconf
index 9d34ed6..64347de 100644
--- a/tests/data_files/test-ca.opensslconf
+++ b/tests/data_files/test-ca.opensslconf
@@ -71,3 +71,7 @@ issuingDistributionPoint=@idpdata
 
 [idpdata]
 fullname=URI:http://pki.example.com/
+
+# these IPs are the ascii values for 'abcd' and 'abcd.example.com'
+[tricky_ip_san]
+subjectAltName=IP:97.98.99.100,IP:6162:6364:2e65:7861:6d70:6c65:2e63:6f6d
diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data
index d5f538b..f8e3891 100644
--- a/tests/suites/test_suite_x509parse.data
+++ b/tests/suites/test_suite_x509parse.data
@@ -911,6 +911,14 @@ X509 CRT verification #97 (next profile Valid Cert SHA256 Digest)
 depends_on:MBEDTLS_SHA256_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:MBEDTLS_ECDSA_C:MBEDTLS_SHA1_C
 x509_verify:"data_files/cert_sha256.crt":"data_files/test-ca.crt":"data_files/crl-ec-sha256.pem":"NULL":0:0:"next":"NULL"
 
+X509 CRT verification: domain identical to IPv4 in SubjectAltName
+depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C
+x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"abcd":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL"
+
+X509 CRT verification: domain identical to IPv6 in SubjectAltName
+depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C
+x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"abcd.example.com":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL"
+
 X509 CRT verification with ca callback: failure
 depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_SHA1_C:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK
 x509_verify_ca_cb_failure:"data_files/server1.crt":"data_files/test-ca.crt":"NULL":MBEDTLS_ERR_X509_FATAL_ERROR