Fix missing check for RSA key length on EE certs

- also adapt tests to use lesser requirement for compatibility with old
  testing material

ChangeLog

@@ -1,5 +1,13 @@
 mbed TLS ChangeLog (Sorted per branch, date)
 
+= mbed TLS 2.2.0 released 2015-10-06
+
+Bugfix
+   * mbedtls_x509_crt_verify(_with_profile)() now also checks the key type and
+     size/curve against the profile. Before that, there was no way to set a
+     minimum key size for end-entity certificates with RSA keys. Found by
+     Matthew Page.
+
 = mbed TLS 2.1.2 released 2015-10-06
 
 Security

include/mbedtls/ssl.h

@@ -1385,6 +1385,10 @@ void mbedtls_ssl_conf_ciphersuites_for_version( mbedtls_ssl_config *conf,
 /**
  * \brief          Set the X.509 security profile used for verification
  *
+ * \note           The restrictions are enforced for all certificates in the
+ *                 chain. However, signatures in the handshake are not covered
+ *                 by this setting but by \b mbedtls_ssl_conf_sig_hashes().
+ *
  * \param conf     SSL configuration
  * \param profile  Profile to use
  */
@@ -1546,16 +1550,14 @@ void mbedtls_ssl_conf_dhm_min_bitlen( mbedtls_ssl_config *conf,
  *                 On client: this affects the list of curves offered for any
  *                 use. The server can override our preference order.
  *
- *                 Both sides: limits the set of curves used by peer to the
+ *                 Both sides: limits the set of curves accepted for use in
- *                 listed curves for any use ECDHE and the end-entity
+ *                 ECDHE and in the peer's end-entity certificate.
- *                 certificate.
  *
- * \note           This has no influence on which curve are allowed inside the
+ * \note           This has no influence on which curves are allowed inside the
  *                 certificate chains, see \c mbedtls_ssl_conf_cert_profile()
- *                 for that. For example, if the peer's certificate chain is
+ *                 for that. For the end-entity certificate however, the key
- *                 EE -> CA_int -> CA_root, then the allowed curves for EE are
+ *                 will be accepted only if it is allowed both by this list
- *                 controlled by \c mbedtls_ssl_conf_curves() but for CA_int
+ *                 and by the cert profile.
- *                 and CA_root it's \c mbedtls_ssl_conf_cert_profile().
  *
  * \note           This list should be ordered by decreasing preference
  *                 (preferred curve first).

include/mbedtls/x509_crt.h

@@ -301,8 +301,8 @@ int mbedtls_x509_crt_verify( mbedtls_x509_crt *crt,
  *                 security profile.
  *
  * \note           The restrictions on keys (RSA minimum size, allowed curves
- *                 for ECDSA) only applys to (intermediate) CAs, not to the
+ *                 for ECDSA) apply to all certificates: trusted root,
- *                 end-entity certificate.
+ *                 intermediate CAs if any, and end entity certificate.
  *
  * \param crt      a certificate to be verified
  * \param trust_ca the trusted CA chain

library/x509_crt.c

@@ -186,8 +186,10 @@ static int x509_profile_check_key( const mbedtls_x509_crt_profile *profile,
     }
 #endif
 
-#if defined(MBEDTLS_ECDSA_C)
+#if defined(MBEDTLS_ECP_C)
-    if( pk_alg == MBEDTLS_PK_ECDSA )
+    if( pk_alg == MBEDTLS_PK_ECDSA ||
+        pk_alg == MBEDTLS_PK_ECKEY ||
+        pk_alg == MBEDTLS_PK_ECKEY_DH )
     {
         mbedtls_ecp_group_id gid = mbedtls_pk_ec( *pk )->grp.id;
 
@@ -2151,6 +2153,7 @@ int mbedtls_x509_crt_verify_with_profile( mbedtls_x509_crt *crt,
     mbedtls_x509_crt *parent;
     mbedtls_x509_name *name;
     mbedtls_x509_sequence *cur = NULL;
+    mbedtls_pk_type_t pk_type;
 
     if( profile == NULL )
         return( MBEDTLS_ERR_X509_BAD_INPUT_DATA );
@@ -2209,6 +2212,15 @@ int mbedtls_x509_crt_verify_with_profile( mbedtls_x509_crt *crt,
         }
     }
 
+    /* Check the type and size of the key */
+    pk_type = mbedtls_pk_get_type( &crt->pk );
+
+    if( x509_profile_check_pk_alg( profile, pk_type ) != 0 )
+        *flags |= MBEDTLS_X509_BADCERT_BAD_PK;
+
+    if( x509_profile_check_key( profile, pk_type, &crt->pk ) != 0 )
+        *flags |= MBEDTLS_X509_BADCERT_BAD_KEY;
+
     /* Look for a parent in trusted CAs */
     for( parent = trust_ca; parent != NULL; parent = parent->next )
     {

tests/suites/test_suite_x509parse.data

@@ -673,7 +673,7 @@ x509_verify:"data_files/server6-ss-child.crt":"data_files/server5-selfsigned.crt
 
 X509 Certificate verification #75 (encoding mismatch)
 depends_on:MBEDTLS_PEM_PARSE_C
-x509_verify:"data_files/enco-cert-utf8str.pem":"data_files/enco-ca-prstr.pem":"data_files/crl.pem":"NULL":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_BAD_KEY:"NULL"
+x509_verify:"data_files/enco-cert-utf8str.pem":"data_files/enco-ca-prstr.pem":"data_files/crl.pem":"NULL":0:0:"NULL"
 
 X509 Certificate verification #76 (multiple CRLs, not revoked)
 depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA256_C:MBEDTLS_RSA_C
@@ -697,7 +697,7 @@ x509_verify:"data_files/server1.crt":"data_files/test-ca_cat12.crt":"data_files/
 
 X509 Certificate verification #81 (multiple CRLs, none relevant)
 depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA256_C:MBEDTLS_RSA_C
-x509_verify:"data_files/enco-cert-utf8str.pem":"data_files/enco-ca-prstr.pem":"data_files/crl_cat_rsa-ec.pem":"NULL":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_BAD_KEY:"NULL"
+x509_verify:"data_files/enco-cert-utf8str.pem":"data_files/enco-ca-prstr.pem":"data_files/crl_cat_rsa-ec.pem":"NULL":0:0:"NULL"
 
 X509 Certificate verification callback: trusted EE cert
 depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED

tests/suites/test_suite_x509parse.function

@@ -6,6 +6,19 @@
 #include "mbedtls/oid.h"
 #include "mbedtls/base64.h"
 
+const mbedtls_x509_crt_profile compat_profile =
+{
+    MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA1 ) |
+    MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_RIPEMD160 ) |
+    MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA224 ) |
+    MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA256 ) |
+    MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA384 ) |
+    MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA512 ),
+    0xFFFFFFF, /* Any PK alg    */
+    0xFFFFFFF, /* Any curve     */
+    1024,
+};
+
 int verify_none( void *data, mbedtls_x509_crt *crt, int certificate_depth, uint32_t *flags )
 {
     ((void) data);
@@ -191,7 +204,11 @@ void x509_verify( char *crt_file, char *ca_file, char *crl_file,
     TEST_ASSERT( mbedtls_x509_crt_parse_file( &ca, ca_file ) == 0 );
     TEST_ASSERT( mbedtls_x509_crl_parse_file( &crl, crl_file ) == 0 );
 
-    res = mbedtls_x509_crt_verify( &crt, &ca, &crl, cn_name, &flags, f_vrfy, NULL );
+    //puts( "" );
+    res = mbedtls_x509_crt_verify_with_profile( &crt, &ca, &crl, &compat_profile, cn_name, &flags, f_vrfy, NULL );
+
+    //printf( "exp: -%04x, %08x\n", result, flags_result );
+    //printf( "got: -%04x, %08x\n", res, flags );
 
     TEST_ASSERT( res == ( result ) );
     TEST_ASSERT( flags == (uint32_t)( flags_result ) );