diff --git a/ChangeLog.d/issue4405.txt b/ChangeLog.d/issue4405.txt new file mode 100644 index 000000000..c36aefa15 --- /dev/null +++ b/ChangeLog.d/issue4405.txt @@ -0,0 +1,4 @@ +Removals + * Remove the MBEDTLS_X509_CHECK_KEY_USAGE and + MBEDTLS_X509_CHECK_EXTENDED_KEY_USAGE config.h options and let the code + behave as if they were always enabled. Fixes #4405. diff --git a/configs/config-no-entropy.h b/configs/config-no-entropy.h index 09b3cf5e0..7ca33c381 100644 --- a/configs/config-no-entropy.h +++ b/configs/config-no-entropy.h @@ -49,8 +49,6 @@ #define MBEDTLS_PKCS1_V21 #define MBEDTLS_SELF_TEST #define MBEDTLS_VERSION_FEATURES -#define MBEDTLS_X509_CHECK_KEY_USAGE -#define MBEDTLS_X509_CHECK_EXTENDED_KEY_USAGE /* mbed TLS modules */ #define MBEDTLS_AES_C diff --git a/docs/3.0-migration-guide.d/remove_MBEDTLS_X509_CHECK_x_KEY_USAGE_options.md b/docs/3.0-migration-guide.d/remove_MBEDTLS_X509_CHECK_x_KEY_USAGE_options.md new file mode 100644 index 000000000..348fe32f7 --- /dev/null +++ b/docs/3.0-migration-guide.d/remove_MBEDTLS_X509_CHECK_x_KEY_USAGE_options.md @@ -0,0 +1,20 @@ +Remove `MBEDTLS_X509_CHECK_*_KEY_USAGE` options from `config.h` +-- + +This change affects users who have chosen the compilation time options to disable +the library's verification of the `keyUsage` and `extendedKeyUsage` fields of an x509 +certificate. + +The change is to remove MBEDTLS_X509_CHECK_KEY_USAGE and +MBEDTLS_X509_CHECK_EXTENDED_KEY_USAGE from the configuration. + +After the change the options are removed and the compilation is done in a way that +the verification of the key usage fields is allways enabled by default. + +This verification is an important step and disabling it can cause security issues. +If the verification is for some reason undesirable it can still be disabled at +a runtime with even more flexibility by using the callback parameter in +`mbedtls_x509_crt_verify()`. + +For example the user can disable the verification by using the callback which +clears the corresponding flags when they've been set. diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 9cce3cd8e..42f9867ac 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1910,33 +1910,6 @@ */ //#define MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK -/** - * \def MBEDTLS_X509_CHECK_KEY_USAGE - * - * Enable verification of the keyUsage extension (CA and leaf certificates). - * - * Disabling this avoids problems with mis-issued and/or misused - * (intermediate) CA and leaf certificates. - * - * \warning Depending on your PKI use, disabling this can be a security risk! - * - * Comment to skip keyUsage checking for both CA and leaf certificates. - */ -#define MBEDTLS_X509_CHECK_KEY_USAGE - -/** - * \def MBEDTLS_X509_CHECK_EXTENDED_KEY_USAGE - * - * Enable verification of the extendedKeyUsage extension (leaf certificates). - * - * Disabling this avoids problems with mis-issued and/or misused certificates. - * - * \warning Depending on your PKI use, disabling this can be a security risk! - * - * Comment to skip extendedKeyUsage checking for certificates. - */ -#define MBEDTLS_X509_CHECK_EXTENDED_KEY_USAGE - /** * \def MBEDTLS_X509_REMOVE_INFO * diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index 23a20d10b..18b03738c 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -827,7 +827,6 @@ int mbedtls_x509_crt_verify_with_ca_cb( mbedtls_x509_crt *crt, #endif /* MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK */ -#if defined(MBEDTLS_X509_CHECK_KEY_USAGE) /** * \brief Check usage of certificate against keyUsage extension. * @@ -851,9 +850,7 @@ int mbedtls_x509_crt_verify_with_ca_cb( mbedtls_x509_crt *crt, */ int mbedtls_x509_crt_check_key_usage( const mbedtls_x509_crt *crt, unsigned int usage ); -#endif /* MBEDTLS_X509_CHECK_KEY_USAGE) */ -#if defined(MBEDTLS_X509_CHECK_EXTENDED_KEY_USAGE) /** * \brief Check usage of certificate against extendedKeyUsage. * @@ -870,7 +867,6 @@ int mbedtls_x509_crt_check_key_usage( const mbedtls_x509_crt *crt, int mbedtls_x509_crt_check_extended_key_usage( const mbedtls_x509_crt *crt, const char *usage_oid, size_t usage_len ); -#endif /* MBEDTLS_X509_CHECK_EXTENDED_KEY_USAGE */ #if defined(MBEDTLS_X509_CRL_PARSE_C) /** diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 9b8c05f76..b9666e9b0 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6526,22 +6526,10 @@ int mbedtls_ssl_check_cert_usage( const mbedtls_x509_crt *cert, uint32_t *flags ) { int ret = 0; -#if defined(MBEDTLS_X509_CHECK_KEY_USAGE) int usage = 0; -#endif -#if defined(MBEDTLS_X509_CHECK_EXTENDED_KEY_USAGE) const char *ext_oid; size_t ext_len; -#endif -#if !defined(MBEDTLS_X509_CHECK_KEY_USAGE) && \ - !defined(MBEDTLS_X509_CHECK_EXTENDED_KEY_USAGE) - ((void) cert); - ((void) cert_endpoint); - ((void) flags); -#endif - -#if defined(MBEDTLS_X509_CHECK_KEY_USAGE) if( cert_endpoint == MBEDTLS_SSL_IS_SERVER ) { /* Server part of the key exchange */ @@ -6583,11 +6571,7 @@ int mbedtls_ssl_check_cert_usage( const mbedtls_x509_crt *cert, *flags |= MBEDTLS_X509_BADCERT_KEY_USAGE; ret = -1; } -#else - ((void) ciphersuite); -#endif /* MBEDTLS_X509_CHECK_KEY_USAGE */ -#if defined(MBEDTLS_X509_CHECK_EXTENDED_KEY_USAGE) if( cert_endpoint == MBEDTLS_SSL_IS_SERVER ) { ext_oid = MBEDTLS_OID_SERVER_AUTH; @@ -6604,7 +6588,6 @@ int mbedtls_ssl_check_cert_usage( const mbedtls_x509_crt *cert, *flags |= MBEDTLS_X509_BADCERT_EXT_KEY_USAGE; ret = -1; } -#endif /* MBEDTLS_X509_CHECK_EXTENDED_KEY_USAGE */ return( ret ); } diff --git a/library/x509_crt.c b/library/x509_crt.c index 8387de618..2cc3381d1 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -2238,7 +2238,6 @@ int mbedtls_x509_crt_verify_info( char *buf, size_t size, const char *prefix, } #endif /* MBEDTLS_X509_REMOVE_INFO */ -#if defined(MBEDTLS_X509_CHECK_KEY_USAGE) int mbedtls_x509_crt_check_key_usage( const mbedtls_x509_crt *crt, unsigned int usage ) { @@ -2261,9 +2260,7 @@ int mbedtls_x509_crt_check_key_usage( const mbedtls_x509_crt *crt, return( 0 ); } -#endif -#if defined(MBEDTLS_X509_CHECK_EXTENDED_KEY_USAGE) int mbedtls_x509_crt_check_extended_key_usage( const mbedtls_x509_crt *crt, const char *usage_oid, size_t usage_len ) @@ -2293,7 +2290,6 @@ int mbedtls_x509_crt_check_extended_key_usage( const mbedtls_x509_crt *crt, return( MBEDTLS_ERR_X509_BAD_INPUT_DATA ); } -#endif /* MBEDTLS_X509_CHECK_EXTENDED_KEY_USAGE */ #if defined(MBEDTLS_X509_CRL_PARSE_C) /* @@ -2344,14 +2340,12 @@ static int x509_crt_verifycrl( mbedtls_x509_crt *crt, mbedtls_x509_crt *ca, /* * Check if the CA is configured to sign CRLs */ -#if defined(MBEDTLS_X509_CHECK_KEY_USAGE) if( mbedtls_x509_crt_check_key_usage( ca, MBEDTLS_X509_KU_CRL_SIGN ) != 0 ) { flags |= MBEDTLS_X509_BADCRL_NOT_TRUSTED; break; } -#endif /* * Check if CRL is correctly signed by the trusted CA @@ -2488,13 +2482,11 @@ static int x509_crt_check_parent( const mbedtls_x509_crt *child, if( need_ca_bit && ! parent->ca_istrue ) return( -1 ); -#if defined(MBEDTLS_X509_CHECK_KEY_USAGE) if( need_ca_bit && mbedtls_x509_crt_check_key_usage( parent, MBEDTLS_X509_KU_KEY_CERT_SIGN ) != 0 ) { return( -1 ); } -#endif return( 0 ); } diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 59acc667a..d9611e5da 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -720,7 +720,7 @@ depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_S x509_verify:"data_files/server5.crt":"data_files/test-ca2.ku-crt_crl.crt":"data_files/crl-ec-sha256.pem":"NULL":0:0:"compat":"NULL" X509 CRT verification #53 (CA keyUsage missing cRLSign) -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_X509_CHECK_KEY_USAGE:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_SECP384R1_ENABLED +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_SECP384R1_ENABLED x509_verify:"data_files/server5.crt":"data_files/test-ca2.ku-crt.crt":"data_files/crl-ec-sha256.pem":"NULL":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCRL_NOT_TRUSTED:"compat":"NULL" X509 CRT verification #54 (CA keyUsage missing cRLSign, no CRL) @@ -728,11 +728,11 @@ depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_S x509_verify:"data_files/server5.crt":"data_files/test-ca2.ku-crt.crt":"data_files/crl.pem":"NULL":0:0:"compat":"NULL" X509 CRT verification #55 (CA keyUsage missing keyCertSign) -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_X509_CHECK_KEY_USAGE:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_SECP384R1_ENABLED +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_SECP384R1_ENABLED x509_verify:"data_files/server5.crt":"data_files/test-ca2.ku-crl.crt":"data_files/crl-ec-sha256.pem":"NULL":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_NOT_TRUSTED:"compat":"NULL" X509 CRT verification #56 (CA keyUsage plain wrong) -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_X509_CHECK_KEY_USAGE:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_SECP384R1_ENABLED +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_SECP384R1_ENABLED x509_verify:"data_files/server5.crt":"data_files/test-ca2.ku-ds.crt":"data_files/crl-ec-sha256.pem":"NULL":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_NOT_TRUSTED:"compat":"NULL" X509 CRT verification #57 (Valid, RSASSA-PSS, SHA-1) diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index a6361d83a..fea02f362 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -1173,7 +1173,7 @@ void x509_oid_numstr( data_t * oid_buf, char * numstr, int blen, int ret ) } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_FS_IO:MBEDTLS_X509_CRT_PARSE_C:MBEDTLS_X509_CHECK_KEY_USAGE */ +/* BEGIN_CASE depends_on:MBEDTLS_FS_IO:MBEDTLS_X509_CRT_PARSE_C */ void x509_check_key_usage( char * crt_file, int usage, int ret ) { mbedtls_x509_crt crt; @@ -1189,7 +1189,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_FS_IO:MBEDTLS_X509_CRT_PARSE_C:MBEDTLS_X509_CHECK_EXTENDED_KEY_USAGE */ +/* BEGIN_CASE depends_on:MBEDTLS_FS_IO:MBEDTLS_X509_CRT_PARSE_C */ void x509_check_extended_key_usage( char * crt_file, data_t * oid, int ret ) {