Merge remote-tracking branch 'upstream-restricted/pr/456' into mbedtls-2.7

library/rsa.c

@@ -75,6 +75,7 @@ static void mbedtls_zeroize( void *v, size_t n ) {
     volatile unsigned char *p = (unsigned char*)v; while( n-- ) *p++ = 0;
 }
 
+#if defined(MBEDTLS_PKCS1_V15)
 /* constant-time buffer comparison */
 static inline int mbedtls_safer_memcmp( const void *a, const void *b, size_t n )
 {
@@ -88,6 +89,7 @@ static inline int mbedtls_safer_memcmp( const void *a, const void *b, size_t n )
 
     return( diff );
 }
+#endif /* MBEDTLS_PKCS1_V15 */
 
 int mbedtls_rsa_import( mbedtls_rsa_context *ctx,
                         const mbedtls_mpi *N,

library/x509_crt.c

@@ -717,7 +717,7 @@ static int x509_crt_parse_der_core( mbedtls_x509_crt *crt, const unsigned char *
 
     memcpy( p, buf, crt->raw.len );
 
-    // Direct pointers to the new buffer 
+    // Direct pointers to the new buffer
     p += crt->raw.len - len;
     end = crt_end = p + len;
 
@@ -1634,7 +1634,7 @@ int mbedtls_x509_crt_is_revoked( const mbedtls_x509_crt *crt, const mbedtls_x509
 
 /*
  * Check that the given certificate is not revoked according to the CRL.
- * Skip validation is no CRL for the given CA is present.
+ * Skip validation if no CRL for the given CA is present.
  */
 static int x509_crt_verifycrl( mbedtls_x509_crt *crt, mbedtls_x509_crt *ca,
                                mbedtls_x509_crl *crl_list,
@@ -1679,17 +1679,13 @@ static int x509_crt_verifycrl( mbedtls_x509_crt *crt, mbedtls_x509_crt *ca,
             flags |= MBEDTLS_X509_BADCRL_BAD_PK;
 
         md_info = mbedtls_md_info_from_type( crl_list->sig_md );
-        if( md_info == NULL )
+        if( mbedtls_md( md_info, crl_list->tbs.p, crl_list->tbs.len, hash ) != 0 )
         {
-            /*
-             * Cannot check 'unknown' hash
-             */
+            /* Note: this can't happen except after an internal error */
             flags |= MBEDTLS_X509_BADCRL_NOT_TRUSTED;
             break;
         }
 
-        mbedtls_md( md_info, crl_list->tbs.p, crl_list->tbs.len, hash );
-
         if( x509_profile_check_key( profile, crl_list->sig_pk, &ca->pk ) != 0 )
             flags |= MBEDTLS_X509_BADCERT_BAD_KEY;
 
@@ -1901,6 +1897,27 @@ static int x509_crt_check_parent( const mbedtls_x509_crt *child,
     return( 0 );
 }
 
+/*
+ * Verify a certificate with no parent inside the chain
+ * (either the parent is a trusted root, or there is no parent)
+ *
+ * See comments for mbedtls_x509_crt_verify_with_profile()
+ * (also for notation used below)
+ *
+ * This function is called in two cases:
+ *  - child was found to have a parent in trusted roots, in which case we're
+ *    called with trust_ca pointing directly to that parent (not the full list)
+ *      - this is cases 1, 2 and 3 of the comment on verify_with_profile()
+ *      - case 1 is special as child and trust_ca point to copies of the same
+ *        certificate then
+ *  - child was found to have no parent either in the chain or in trusted CAs
+ *      - this is cases 4 and 5 of the comment on verify_with_profile()
+ *
+ * For historical reasons, the function currently does not assume that
+ * trust_ca points directly to the right root in the first case, and it
+ * doesn't know in which case it starts, so it always starts by searching for
+ * a parent in trust_ca.
+ */
 static int x509_crt_verify_top(
                 mbedtls_x509_crt *child, mbedtls_x509_crt *trust_ca,
                 mbedtls_x509_crl *ca_crl,
@@ -1934,15 +1951,12 @@ static int x509_crt_verify_top(
     *flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED;
 
     md_info = mbedtls_md_info_from_type( child->sig_md );
-    if( md_info == NULL )
+    if( mbedtls_md( md_info, child->tbs.p, child->tbs.len, hash ) != 0 )
     {
-        /*
-         * Cannot check 'unknown', no need to try any CA
-         */
+        /* Note: this can't happen except after an internal error */
+        /* Cannot check signature, no need to try any CA */
         trust_ca = NULL;
     }
-    else
-        mbedtls_md( md_info, child->tbs.p, child->tbs.len, hash );
 
     for( /* trust_ca */ ; trust_ca != NULL; trust_ca = trust_ca->next )
     {
@@ -1957,7 +1971,7 @@ static int x509_crt_verify_top(
          */
         if( child->subject_raw.len == trust_ca->subject_raw.len &&
             memcmp( child->subject_raw.p, trust_ca->subject_raw.p,
-                            child->issuer_raw.len ) == 0 )
+                    child->subject_raw.len ) == 0 )
         {
             check_path_cnt--;
         }
@@ -2007,7 +2021,7 @@ static int x509_crt_verify_top(
     if( trust_ca != NULL &&
         ( child->subject_raw.len != trust_ca->subject_raw.len ||
           memcmp( child->subject_raw.p, trust_ca->subject_raw.p,
-                            child->issuer_raw.len ) != 0 ) )
+                  child->subject_raw.len ) != 0 ) )
     {
 #if defined(MBEDTLS_X509_CRL_PARSE_C)
         /* Check trusted CA's CRL for the chain's top crt */
@@ -2044,6 +2058,11 @@ static int x509_crt_verify_top(
     return( 0 );
 }
 
+/*
+ * Verify a certificate with a parent inside the chain
+ *
+ * See comments for mbedtls_x509_crt_verify_with_profile()
+ */
 static int x509_crt_verify_child(
                 mbedtls_x509_crt *child, mbedtls_x509_crt *parent,
                 mbedtls_x509_crt *trust_ca, mbedtls_x509_crl *ca_crl,
@@ -2082,17 +2101,13 @@ static int x509_crt_verify_child(
         *flags |= MBEDTLS_X509_BADCERT_BAD_PK;
 
     md_info = mbedtls_md_info_from_type( child->sig_md );
-    if( md_info == NULL )
+    if( mbedtls_md( md_info, child->tbs.p, child->tbs.len, hash ) != 0 )
     {
-        /*
-         * Cannot check 'unknown' hash
-         */
+        /* Note: this can't happen except after an internal error */
         *flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED;
     }
     else
     {
-        mbedtls_md( md_info, child->tbs.p, child->tbs.len, hash );
-
         if( x509_profile_check_key( profile, child->sig_pk, &parent->pk ) != 0 )
             *flags |= MBEDTLS_X509_BADCERT_BAD_KEY;
 
@@ -2193,6 +2208,34 @@ int mbedtls_x509_crt_verify( mbedtls_x509_crt *crt,
 
 /*
  * Verify the certificate validity, with profile
+ *
+ * The chain building/verification is spread accross 4 functions:
+ *  - this one
+ *  - x509_crt_verify_child()
+ *  - x509_crt_verify_top()
+ *  - x509_crt_check_parent()
+ *
+ * There are five main cases to consider. Let's introduce some notation:
+ *  - E means the end-entity certificate
+ *  - I an intermediate CA
+ *  - R the trusted root CA this chain anchors to
+ *  - T the list of trusted roots (R and possible some others)
+ *
+ * The main cases with the calling sequence of the crt_verify_xxx() are:
+ *  1. E = R (explicitly trusted EE cert)
+ *      verify(E, T) -> verify_top(E, R)
+ *  2. E -> R (EE signed by trusted root)
+ *      verify(E, T) -> verify_top(E, R)
+ *  3. E -> I -> R (EE signed by intermediate signed by trusted root)
+ *      verify(E, T) -> verify_child(E, I, T) -> verify_top(I, R)
+ *      (plus variant with multiple intermediates)
+ *  4. E -> I (EE signed by intermediate that's not trusted)
+ *      verify(E, T) -> verify_child(E, I, T) -> verify_top(I, T)
+ *      (plus variant with multiple intermediates)
+ *  5. E (EE not trusted)
+ *      verify(E, T) -> verify_top(E, T)
+ *
+ * Note: this notation and case numbering is also used in x509_crt_verify_top()
  */
 int mbedtls_x509_crt_verify_with_profile( mbedtls_x509_crt *crt,
                      mbedtls_x509_crt *trust_ca,