From a9ec0cd77fa3e058ea96e67faaf28f8c62bb1a1e Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Thu, 9 Feb 2017 19:29:33 +0200 Subject: [PATCH 1/2] Restrict MD5 in x509 certificates Remove support for X509 certificates signed with MD5. Issue raised by Harm Verhagen --- ChangeLog | 6 ++++ include/polarssl/config.h | 25 +++++++++++++++++ library/x509_crt.c | 39 ++++++++++++++++++++++++++ tests/suites/test_suite_x509parse.data | 12 ++++++-- 4 files changed, 80 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 509908177..cf2defa01 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS x.x.xx branch released xxxx-xx-xx + +Bugfix + * Remove support for X509 certificates signed with MD5. + Issue raised by Harm Verhagen + = mbed TLS 1.3.19 branch released 2017-03-08 Security diff --git a/include/polarssl/config.h b/include/polarssl/config.h index 498fc5b9a..33a02b3a9 100644 --- a/include/polarssl/config.h +++ b/include/polarssl/config.h @@ -2062,6 +2062,31 @@ */ #define POLARSSL_SHA512_C +/** + * \def MINIMAL_SUPPORTED_MD_ALG + * + * minimal supported md algorithm. + * The value should be one of the enumerations in + * md_type_t defined in md.h + * typedef enum { + * POLARSSL_MD_NONE=0, + * POLARSSL_MD_MD2, + * POLARSSL_MD_MD4, + * POLARSSL_MD_MD5, + * POLARSSL_MD_SHA1, + * POLARSSL_MD_SHA224, + * POLARSSL_MD_SHA256, + * POLARSSL_MD_SHA384, + * POLARSSL_MD_SHA512, + * POLARSSL_MD_RIPEMD160, + * } md_type_t; + * + * Module: library/x509_crt.c + * Caller: + * + */ +#define POLARSSL_MINIMAL_SUPPORTED_MD_ALG POLARSSL_MD_SHA1 + /** * \def POLARSSL_SSL_CACHE_C * diff --git a/library/x509_crt.c b/library/x509_crt.c index a3517f64f..186ecda9b 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1434,6 +1434,18 @@ int x509_crt_verify_info( char *buf, size_t size, const char *prefix, return( (int) ( size - n ) ); } +/* + * Check md_alg against profile + * Return 0 if md_alg acceptable for this profile, -1 otherwise + */ +static int x509_check_md_alg( md_type_t md_alg ) +{ + if( md_alg >= POLARSSL_MINIMAL_SUPPORTED_MD_ALG ) + return( 0 ); + + return( -1 ); +} + #if defined(POLARSSL_X509_CHECK_KEY_USAGE) int x509_crt_check_key_usage( const x509_crt *crt, int usage ) { @@ -1541,6 +1553,15 @@ static int x509_crt_verifycrl( x509_crt *crt, x509_crt *ca, } #endif + /* + * Check if CRL is signed with a valid MD + */ + if( x509_check_md_alg( crl_list->sig_md ) != 0 ) + { + flags |= BADCRL_NOT_TRUSTED; + break; + } + /* * Check if CRL is correctly signed by the trusted CA */ @@ -1788,6 +1809,18 @@ static int x509_crt_verify_top( */ *flags |= BADCERT_NOT_TRUSTED; + /* + * Check if certificate is signed with a valid MD + */ + if( x509_check_md_alg( child->sig_md ) != 0 ) + { + *flags |= BADCERT_NOT_TRUSTED; + /* + * not signed with a valid MD, no need to check trust_ca + */ + trust_ca = NULL; + } + md_info = md_info_from_type( child->sig_md ); if( md_info == NULL ) { @@ -1925,6 +1958,12 @@ static int x509_crt_verify_child( if( x509_time_future( &child->valid_from ) ) *flags |= BADCERT_FUTURE; + /* + * Check if certificate is signed with a valid MD + */ + if( x509_check_md_alg( child->sig_md ) != 0 ) + *flags |= BADCERT_NOT_TRUSTED; + md_info = md_info_from_type( child->sig_md ); if( md_info == NULL ) { diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 717cd6f79..7920fc626 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -417,11 +417,11 @@ x509_verify:"data_files/server2.crt":"data_files/server1.crt":"data_files/crl_ex X509 Certificate verification #12 (Valid Cert MD4 Digest) depends_on:POLARSSL_MD4_C:POLARSSL_PEM_PARSE_C:POLARSSL_SHA1_C:POLARSSL_RSA_C:POLARSSL_PKCS1_V15 -x509_verify:"data_files/cert_md4.crt":"data_files/test-ca.crt":"data_files/crl.pem":"NULL":0:0:"NULL" +x509_verify:"data_files/cert_md4.crt":"data_files/test-ca.crt":"data_files/crl.pem":"NULL":POLARSSL_ERR_X509_CERT_VERIFY_FAILED:BADCERT_NOT_TRUSTED:"NULL" X509 Certificate verification #13 (Valid Cert MD5 Digest) depends_on:POLARSSL_MD5_C:POLARSSL_PEM_PARSE_C:POLARSSL_SHA1_C:POLARSSL_RSA_C:POLARSSL_PKCS1_V15 -x509_verify:"data_files/cert_md5.crt":"data_files/test-ca.crt":"data_files/crl.pem":"NULL":0:0:"NULL" +x509_verify:"data_files/cert_md5.crt":"data_files/test-ca.crt":"data_files/crl.pem":"NULL":POLARSSL_ERR_X509_CERT_VERIFY_FAILED:BADCERT_NOT_TRUSTED:"NULL" X509 Certificate verification #14 (Valid Cert SHA1 Digest) depends_on:POLARSSL_SHA1_C:POLARSSL_PEM_PARSE_C:POLARSSL_SHA1_C:POLARSSL_RSA_C:POLARSSL_PKCS1_V15 @@ -723,6 +723,14 @@ X509 Certificate verification #87 (Expired CA and invalid CA) depends_on:POLARSSL_PEM_PARSE_C:POLARSSL_ECP_C:POLARSSL_ECP_DP_SECP256R1_ENABLED:POLARSSL_ECP_DP_SECP384R1_ENABLED:POLARSSL_SHA1_C:POLARSSL_SHA256_C x509_verify:"data_files/server5.crt":"data_files/test-ca2_cat-past-invalid.crt":"data_files/crl-ec-sha1.pem":"NULL":POLARSSL_ERR_X509_CERT_VERIFY_FAILED:BADCERT_EXPIRED:"NULL" +X509 Certificate verification #88 (MD4 CRL) +depends_on:POLARSSL_SHA256_C:POLARSSL_PEM_PARSE_C:POLARSSL_SHA1_C:POLARSSL_RSA_C:POLARSSL_PKCS1_V15 +x509_verify:"data_files/cert_sha256.crt":"data_files/test-ca.crt":"data_files/crl_md4.pem":"NULL":POLARSSL_ERR_X509_CERT_VERIFY_FAILED:BADCRL_NOT_TRUSTED:"NULL" + +X509 Certificate verification #89 (MD5 CRL) +depends_on:POLARSSL_SHA256_C:POLARSSL_PEM_PARSE_C:POLARSSL_SHA1_C:POLARSSL_RSA_C:POLARSSL_PKCS1_V15 +x509_verify:"data_files/cert_sha256.crt":"data_files/test-ca.crt":"data_files/crl_md5.pem":"NULL":POLARSSL_ERR_X509_CERT_VERIFY_FAILED:BADCRL_NOT_TRUSTED:"NULL" + X509 Certificate verification callback: trusted EE cert depends_on:POLARSSL_PEM_PARSE_C:POLARSSL_ECDSA_C:POLARSSL_SHA256_C:POLARSSL_ECP_DP_SECP256R1_ENABLED x509_verify_callback:"data_files/server5-selfsigned.crt":"data_files/server5-selfsigned.crt":0:"depth 0 - serial 53\:A2\:CB\:4B\:12\:4E\:AD\:83\:7D\:A8\:94\:B2 - subject CN=selfsigned, OU=testing, O=PolarSSL, C=NL\n" From 48ed550b926f8866c4bd7a67d5041ba652af9771 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 8 Jun 2017 17:27:20 +0200 Subject: [PATCH 2/2] Fix name, documentation & location of config flag --- include/polarssl/config.h | 48 +++++++++++++++++++-------------------- library/x509_crt.c | 6 ++++- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/include/polarssl/config.h b/include/polarssl/config.h index 33a02b3a9..60d96ec65 100644 --- a/include/polarssl/config.h +++ b/include/polarssl/config.h @@ -2062,31 +2062,6 @@ */ #define POLARSSL_SHA512_C -/** - * \def MINIMAL_SUPPORTED_MD_ALG - * - * minimal supported md algorithm. - * The value should be one of the enumerations in - * md_type_t defined in md.h - * typedef enum { - * POLARSSL_MD_NONE=0, - * POLARSSL_MD_MD2, - * POLARSSL_MD_MD4, - * POLARSSL_MD_MD5, - * POLARSSL_MD_SHA1, - * POLARSSL_MD_SHA224, - * POLARSSL_MD_SHA256, - * POLARSSL_MD_SHA384, - * POLARSSL_MD_SHA512, - * POLARSSL_MD_RIPEMD160, - * } md_type_t; - * - * Module: library/x509_crt.c - * Caller: - * - */ -#define POLARSSL_MINIMAL_SUPPORTED_MD_ALG POLARSSL_MD_SHA1 - /** * \def POLARSSL_SSL_CACHE_C * @@ -2391,6 +2366,29 @@ /* X509 options */ //#define POLARSSL_X509_MAX_INTERMEDIATE_CA 8 /**< Maximum number of intermediate CAs in a verification chain. */ +/** + * \def POLARSSL_X509_MIN_VERIFY_MD_ALG + * + * Minimal hash algorithm accepted in X.509 chain verification. + * + * The value should be one of the enumerations in md_type_t defined in md.h + * Only algorithms with a value equal or higher are accepted. + * + * typedef enum { + * POLARSSL_MD_NONE=0, + * POLARSSL_MD_MD2, + * POLARSSL_MD_MD4, + * POLARSSL_MD_MD5, + * POLARSSL_MD_SHA1, + * POLARSSL_MD_SHA224, + * POLARSSL_MD_SHA256, + * POLARSSL_MD_SHA384, + * POLARSSL_MD_SHA512, + * POLARSSL_MD_RIPEMD160, + * } md_type_t; + */ +//#define POLARSSL_X509_MIN_VERIFY_MD_ALG POLARSSL_MD_SHA1 + /* \} name SECTION: Module configuration options */ #include "check_config.h" diff --git a/library/x509_crt.c b/library/x509_crt.c index 186ecda9b..3c7bd154e 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -76,6 +76,10 @@ #endif /* !_WIN32 || EFIX64 || EFI32 */ #endif +#if !defined(POLARSSL_X509_MIN_VERIFY_MD_ALG) +#define POLARSSL_X509_MIN_VERIFY_MD_ALG POLARSSL_MD_SHA1 +#endif + /* Implementation that should never be optimized out by the compiler */ static void polarssl_zeroize( void *v, size_t n ) { volatile unsigned char *p = v; while( n-- ) *p++ = 0; @@ -1440,7 +1444,7 @@ int x509_crt_verify_info( char *buf, size_t size, const char *prefix, */ static int x509_check_md_alg( md_type_t md_alg ) { - if( md_alg >= POLARSSL_MINIMAL_SUPPORTED_MD_ALG ) + if( md_alg >= POLARSSL_X509_MIN_VERIFY_MD_ALG ) return( 0 ); return( -1 );