From 7d2a4d873f1ded3f3c1fff591527867e16a0a7a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 23 Jul 2020 12:39:53 +0200 Subject: [PATCH 1/4] Add test: DNS names should not match IP addresses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/data_files/Makefile | 4 ++++ tests/data_files/server5-tricky-ip-san.crt | 11 +++++++++++ tests/data_files/test-ca.opensslconf | 4 ++++ tests/suites/test_suite_x509parse.data | 8 ++++++++ 4 files changed, 27 insertions(+) create mode 100644 tests/data_files/server5-tricky-ip-san.crt diff --git a/tests/data_files/Makefile b/tests/data_files/Makefile index 99d64eb3a..40c22f53b 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 000000000..135830fbe --- /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 9d34ed68d..64347de83 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 d5f538b22..f8e3891a2 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 From f3e4bd8632b71dc491e52e6df87dc3e409d2b869 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 21 Jul 2020 13:22:41 +0200 Subject: [PATCH 2/4] Fix comparison between different name types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/x509_crt.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 8fd8b865d..26272244b 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -3007,6 +3007,25 @@ static int x509_crt_check_cn( const mbedtls_x509_buf *name, return( -1 ); } +/* + * 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! */ @@ -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; } From f58e5cc4f4d0729a8762e98ebcd17028b6aba059 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 24 Jul 2020 10:31:37 +0200 Subject: [PATCH 3/4] Improve documentation of cn in x509_crt_verify() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mention explicitly that only DNS names are supported so far, and while at it explain where the name is searched. Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/x509_crt.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index ab0d0cdbc..d24204db3 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. From 204e05404f568e4868b3fdac70887a097a270770 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 24 Jul 2020 10:33:39 +0200 Subject: [PATCH 4/4] Add ChangeLog entry for X.509 CN-type vulnerability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- ChangeLog.d/x509-verify-non-dns-san.txt | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 ChangeLog.d/x509-verify-non-dns-san.txt 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 000000000..0cd81b385 --- /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.