Merge pull request #1638 from valeriosetti/issue1598

[development] PKCS7 accepts weak hashes

ChangeLog.d/pkcs7-reject-weak-hashes.txt

@@ -0,0 +1,7 @@
+Security
+   * PKCS7 now rejects weak hash algorithms (RIPEMD160, MD5, SHA-1, SHA-224,
+     SHA3-224) on signature verification.
+
+Features
+   * SHA3-256, SHA3-384 and SHA3-512 are now accepted in the default X.509
+     certificate profile.

framework

@@ -1 +1 @@
-Subproject commit e0f6275eff2e45e295fdd6b6877de4cbdaddae83
+Subproject commit d6dec17adfb5bf283c6a7a5bb6c451ecdc0b8201

include/mbedtls/pkcs7.h

@@ -31,6 +31,10 @@
  *    assumed these fields are empty.
  *  - The RFC allows for the signed Data type to contain contentInfo. This
  *    implementation assumes the type is DATA and the content is empty.
+ *  - The RFC doesn't put any constrain on the hash algorithm to be used, but
+ *    this implementation rejects weak hash algorithms (i.e. RIPEMD160, MD5,
+ *    SHA-1, SHA-224, SHA3-224). In general accepted hash and PK algorithms are
+ *    the ones belonging to `mbedtls_x509_crt_profile_default`.
  */
 
 #ifndef MBEDTLS_PKCS7_H
@@ -190,6 +194,9 @@ int mbedtls_pkcs7_parse_der(mbedtls_pkcs7 *pkcs7, const unsigned char *buf,
  * \note           This function internally calculates the hash on the supplied
  *                 plain data for signature verification.
  *
+ * \note           For limitation on the supported hash algorithms, please refer
+ *                 to the note at the top of this document.
+ *
  * \return         0 if the signature verifies, or a negative error code on failure.
  */
 int mbedtls_pkcs7_signed_data_verify(mbedtls_pkcs7 *pkcs7,
@@ -219,6 +226,9 @@ int mbedtls_pkcs7_signed_data_verify(mbedtls_pkcs7 *pkcs7,
  * \note           This function is different from mbedtls_pkcs7_signed_data_verify()
  *                 in that it is directly passed the hash of the data.
  *
+ * \note           For limitation on the supported hash algorithms, please refer
+ *                 to the note at the top of this document.
+ *
  * \return         0 if the signature verifies, or a negative error code on failure.
  */
 int mbedtls_pkcs7_signed_hash_verify(mbedtls_pkcs7 *pkcs7,

library/pkcs7.c

@@ -665,6 +665,18 @@ static int mbedtls_pkcs7_data_or_hash_verify(mbedtls_pkcs7 *pkcs7,
         return ret;
     }
 
+    /* Ensure the MD alg from the PKCS#7 context and signature algorithm from
+     * the certificate belong to the list of secure algorithms
+     * (i.e. mbedtls_x509_crt_profile_default). */
+    ret = mbedtls_x509_profile_check_md_alg(&mbedtls_x509_crt_profile_default, md_alg);
+    if (ret != 0) {
+        return MBEDTLS_ERR_PKCS7_INVALID_ALG;
+    }
+    ret = mbedtls_x509_profile_check_pk_alg(&mbedtls_x509_crt_profile_default, cert->sig_pk);
+    if (ret != 0) {
+        return MBEDTLS_ERR_PKCS7_INVALID_ALG;
+    }
+
     md_info = mbedtls_md_info_from_type(md_alg);
     if (md_info == NULL) {
         return MBEDTLS_ERR_PKCS7_VERIFY_FAIL;

library/x509_crt.c

@@ -87,11 +87,13 @@ typedef struct {
  * concerns. */
 const mbedtls_x509_crt_profile mbedtls_x509_crt_profile_default =
 {
-    /* Hashes from SHA-256 and above. Note that this selection
-     * should be aligned with ssl_preset_default_hashes in ssl_tls.c. */
+    /* Hashes from SHA-256 and above. */
     MBEDTLS_X509_ID_FLAG(MBEDTLS_MD_SHA256) |
     MBEDTLS_X509_ID_FLAG(MBEDTLS_MD_SHA384) |
-    MBEDTLS_X509_ID_FLAG(MBEDTLS_MD_SHA512),
+    MBEDTLS_X509_ID_FLAG(MBEDTLS_MD_SHA512) |
+    MBEDTLS_X509_ID_FLAG(MBEDTLS_MD_SHA3_256) |
+    MBEDTLS_X509_ID_FLAG(MBEDTLS_MD_SHA3_384) |
+    MBEDTLS_X509_ID_FLAG(MBEDTLS_MD_SHA3_512),
     0xFFFFFFF, /* Any PK alg    */
 #if defined(PSA_WANT_KEY_TYPE_ECC_PUBLIC_KEY)
     /* Curves at or above 128-bit security level. Note that this selection
@@ -165,11 +167,7 @@ const mbedtls_x509_crt_profile mbedtls_x509_crt_profile_none =
     (uint32_t) -1,
 };
 
-/*
- * Check md_alg against profile
- * Return 0 if md_alg is acceptable for this profile, -1 otherwise
- */
-static int x509_profile_check_md_alg(const mbedtls_x509_crt_profile *profile,
+int mbedtls_x509_profile_check_md_alg(const mbedtls_x509_crt_profile *profile,
                                       mbedtls_md_type_t md_alg)
 {
     if (md_alg == MBEDTLS_MD_NONE) {
@@ -183,11 +181,7 @@ static int x509_profile_check_md_alg(const mbedtls_x509_crt_profile *profile,
     return -1;
 }
 
-/*
- * Check pk_alg against profile
- * Return 0 if pk_alg is acceptable for this profile, -1 otherwise
- */
-static int x509_profile_check_pk_alg(const mbedtls_x509_crt_profile *profile,
+int mbedtls_x509_profile_check_pk_alg(const mbedtls_x509_crt_profile *profile,
                                       mbedtls_pk_sigalg_t pk_alg)
 {
     if (pk_alg == MBEDTLS_PK_SIGALG_NONE) {
@@ -2048,11 +2042,11 @@ static int x509_crt_verifycrl(mbedtls_x509_crt *crt, mbedtls_x509_crt *ca,
         /*
          * Check if CRL is correctly signed by the trusted CA
          */
-        if (x509_profile_check_md_alg(profile, crl_list->sig_md) != 0) {
+        if (mbedtls_x509_profile_check_md_alg(profile, crl_list->sig_md) != 0) {
             flags |= MBEDTLS_X509_BADCRL_BAD_MD;
         }
 
-        if (x509_profile_check_pk_alg(profile, crl_list->sig_pk) != 0) {
+        if (mbedtls_x509_profile_check_pk_alg(profile, crl_list->sig_pk) != 0) {
             flags |= MBEDTLS_X509_BADCRL_BAD_PK;
         }
 
@@ -2561,11 +2555,11 @@ static int x509_crt_verify_chain(
         }
 
         /* Check signature algorithm: MD & PK algs */
-        if (x509_profile_check_md_alg(profile, child->sig_md) != 0) {
+        if (mbedtls_x509_profile_check_md_alg(profile, child->sig_md) != 0) {
             *flags |= MBEDTLS_X509_BADCERT_BAD_MD;
         }
 
-        if (x509_profile_check_pk_alg(profile, child->sig_pk) != 0) {
+        if (mbedtls_x509_profile_check_pk_alg(profile, child->sig_pk) != 0) {
             *flags |= MBEDTLS_X509_BADCERT_BAD_PK;
         }
 
@@ -3073,7 +3067,7 @@ static int x509_crt_verify_restartable_ca_cb(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, (mbedtls_pk_sigalg_t) pk_type) != 0) {
+    if (mbedtls_x509_profile_check_pk_alg(profile, (mbedtls_pk_sigalg_t) pk_type) != 0) {
         ee_flags |= MBEDTLS_X509_BADCERT_BAD_PK;
     }
 

library/x509_internal.h

@@ -15,6 +15,7 @@
 #include "mbedtls/private_access.h"
 
 #include "mbedtls/x509.h"
+#include "mbedtls/x509_crt.h"
 #include "mbedtls/asn1.h"
 
 #include "pk_internal.h" // for a lot of things, including in SSL
@@ -79,4 +80,18 @@ int mbedtls_x509_info_key_usage(char **buf, size_t *size,
 int mbedtls_x509_write_set_san_common(mbedtls_asn1_named_data **extensions,
                                       const mbedtls_x509_san_list *san_list);
 
+/*
+ * Check md_alg against profile
+ * Return 0 if md_alg is acceptable for this profile, -1 otherwise
+ */
+int mbedtls_x509_profile_check_md_alg(const mbedtls_x509_crt_profile *profile,
+                                      mbedtls_md_type_t md_alg);
+
+/*
+ * Check pk_alg against profile
+ * Return 0 if pk_alg is acceptable for this profile, -1 otherwise
+ */
+int mbedtls_x509_profile_check_pk_alg(const mbedtls_x509_crt_profile *profile,
+                                      mbedtls_pk_sigalg_t pk_alg);
+
 #endif /* MBEDTLS_X509_INTERNAL_H */

tests/suites/test_suite_pkcs7.data

@@ -94,9 +94,17 @@ PKCS7 Signed Data Verification Pass SHA256 #9.1
 depends_on:PSA_WANT_ALG_SHA_256:PSA_HAVE_ALG_RSA_PKCS1V15_VERIFY
 pkcs7_verify:"../framework/data_files/pkcs7_data_cert_signed_sha256.der":"../framework/data_files/pkcs7-rsa-sha256-1.der":"../framework/data_files/pkcs7_data.bin":MBEDTLS_MD_SHA256:0
 
-PKCS7 Signed Data Verification Pass SHA1 #10
+PKCS7 Signed Data Verification Fail SHA1 #10
 depends_on:PSA_WANT_ALG_SHA_1:PSA_WANT_ALG_SHA_256:PSA_HAVE_ALG_RSA_PKCS1V15_VERIFY
-pkcs7_verify:"../framework/data_files/pkcs7_data_cert_signed_sha1.der":"../framework/data_files/pkcs7-rsa-sha256-1.der":"../framework/data_files/pkcs7_data.bin":0:0
+pkcs7_verify:"../framework/data_files/pkcs7_data_cert_signed_sha1.der":"../framework/data_files/pkcs7-rsa-sha256-1.der":"../framework/data_files/pkcs7_data.bin":0:MBEDTLS_ERR_PKCS7_INVALID_ALG
+
+PKCS7 Signed Data Verification Fail MD5 #10.1
+depends_on:PSA_WANT_ALG_MD5:PSA_WANT_ALG_SHA_256:PSA_HAVE_ALG_RSA_PKCS1V15_VERIFY
+pkcs7_verify:"../framework/data_files/pkcs7_data_cert_signed_md5.der":"../framework/data_files/pkcs7-rsa-sha256-1.der":"../framework/data_files/pkcs7_data.bin":0:MBEDTLS_ERR_PKCS7_INVALID_ALG
+
+PKCS7 Signed Data Verification Fail RIPEMD160 #10.2
+depends_on:PSA_WANT_ALG_RIPEMD160:PSA_WANT_ALG_SHA_256:PSA_HAVE_ALG_RSA_PKCS1V15_VERIFY
+pkcs7_verify:"../framework/data_files/pkcs7_data_cert_signed_ripemd160.der":"../framework/data_files/pkcs7-rsa-sha256-1.der":"../framework/data_files/pkcs7_data.bin":0:MBEDTLS_ERR_PKCS7_INVALID_ALG
 
 PKCS7 Signed Data Verification Pass SHA512 #11
 depends_on:PSA_WANT_ALG_SHA_512:PSA_WANT_ALG_SHA_256:PSA_HAVE_ALG_RSA_PKCS1V15_VERIFY