Merge pull request #4619 from TRodziewicz/remove_MBEDTLS_X509_CHECK_x_KEY_USAGE_options

Remove MBEDTLS_X509_CHECK_*_KEY_USAGE options but enable the code

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.

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

docs/3.0-migration-guide.d/remove_MBEDTLS_X509_CHECK_x_KEY_USAGE_options.md

@@ -0,0 +1,18 @@
+Remove `MBEDTLS_X509_CHECK_*_KEY_USAGE` options from `config.h`
+-------------------------------------------------------------------
+
+This change affects users who have chosen the configuration options to disable the
+library's verification of the `keyUsage` and `extendedKeyUsage` fields of x509
+certificates.
+
+The `MBEDTLS_X509_CHECK_KEY_USAGE` and `MBEDTLS_X509_CHECK_EXTENDED_KEY_USAGE`
+configuration options are removed and the X509 code now behaves as if they were
+always enabled. It is consequently not possible anymore to disable at compile
+time the verification of the `keyUsage` and `extendedKeyUsage` fields of X509
+certificates.
+
+The verification of the `keyUsage` and `extendedKeyUsage` fields is important,
+disabling it can cause security issues and it is thus not recommended. If the
+verification is for some reason undesirable, it can still be disabled by means
+of the verification callback function passed to `mbedtls_x509_crt_verify()` (see
+the documentation of this function for more information).

include/mbedtls/config.h

@@ -1832,33 +1832,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
  *

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)
 /**

library/ssl_tls.c

@@ -6544,22 +6544,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 */
@@ -6601,11 +6589,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;
@@ -6622,7 +6606,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 );
 }

library/x509_crt.c

@@ -2231,7 +2231,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 )
 {
@@ -2254,9 +2253,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 )
@@ -2286,7 +2283,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)
 /*
@@ -2337,14 +2333,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
@@ -2481,13 +2475,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 );
 }

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)

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
                                     )
 {