From f3c17d718b1253eaf176ab3393e4bcb3f982300a Mon Sep 17 00:00:00 2001
From: Joakim Sindholt <opensource@zhasha.com>
Date: Tue, 21 Feb 2023 11:43:20 +0100
Subject: [PATCH 2/2] pkcs7: add support for internal certificates

This adds support for reading out multiple certificates and filters them
using their serial numbers. The cert given to the verify functions now
acts as a trust anchor, meaning any chain of embedded certificates
leading to it is now also considered trusted.
The previous 1-cert-restriction seemed arbitrary and wrong as the read
cert was never actually used anywhere.
---
 include/mbedtls/pkcs7.h |  12 ++--
 library/pkcs7.c         | 120 +++++++++++++++++++++++++++-------------
 2 files changed, 90 insertions(+), 42 deletions(-)

diff --git a/include/mbedtls/pkcs7.h b/include/mbedtls/pkcs7.h
index 45d77c76e..8fab8b491 100644
--- a/include/mbedtls/pkcs7.h
+++ b/include/mbedtls/pkcs7.h
@@ -212,7 +212,9 @@ int mbedtls_pkcs7_parse_der(mbedtls_pkcs7 *pkcs7, const unsigned char *buf,
  *                 PKCS7 structure itself.
  *
  * \param pkcs7    PKCS7 structure containing signature.
- * \param cert     Certificate containing key to verify signature.
+ * \param cert     Trusted certificates either containing keys to verify the
+ *                 signature or to verify an embedded certificate containing
+ *                 the key.
  * \param data     Plain data on which signature has to be verified.
  * \param datalen  Length of the data.
  *
@@ -222,7 +224,7 @@ int mbedtls_pkcs7_parse_der(mbedtls_pkcs7 *pkcs7, const unsigned char *buf,
  * \return         0 if the signature verifies, or a negative error code on failure.
  */
 int mbedtls_pkcs7_signed_data_verify(mbedtls_pkcs7 *pkcs7,
-                                     const mbedtls_x509_crt *cert,
+                                     mbedtls_x509_crt *cert,
                                      const unsigned char *data,
                                      size_t datalen);
 
@@ -241,7 +243,9 @@ int mbedtls_pkcs7_signed_data_verify(mbedtls_pkcs7 *pkcs7,
  *                 PKCS7 structure itself.
  *
  * \param pkcs7    PKCS7 structure containing signature.
- * \param cert     Certificate containing key to verify signature.
+ * \param cert     Trusted certificates either containing keys to verify the
+ *                 signature or to verify an embedded certificate containing
+ *                 the key.
  * \param hash     Hash of the plain data on which signature has to be verified.
  * \param hashlen  Length of the hash.
  *
@@ -251,7 +255,7 @@ int mbedtls_pkcs7_signed_data_verify(mbedtls_pkcs7 *pkcs7,
  * \return         0 if the signature verifies, or a negative error code on failure.
  */
 int mbedtls_pkcs7_signed_hash_verify(mbedtls_pkcs7 *pkcs7,
-                                     const mbedtls_x509_crt *cert,
+                                     mbedtls_x509_crt *cert,
                                      const unsigned char *hash, size_t hashlen);
 
 /**
diff --git a/library/pkcs7.c b/library/pkcs7.c
index 32b7267b9..2cc6d9c72 100644
--- a/library/pkcs7.c
+++ b/library/pkcs7.c
@@ -268,6 +268,7 @@ static int pkcs7_get_certificates(unsigned char **p, unsigned char *end,
     size_t len1 = 0;
     size_t len2 = 0;
     unsigned char *end_set, *end_cert, *start;
+    int n = 0;
 
     if ((ret = mbedtls_asn1_get_tag(p, end, &len1, MBEDTLS_ASN1_CONSTRUCTED
                                     | MBEDTLS_ASN1_CONTEXT_SPECIFIC)) != 0) {
@@ -277,39 +278,30 @@ static int pkcs7_get_certificates(unsigned char **p, unsigned char *end,
             return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_FORMAT, ret);
         }
     }
-    start = *p;
     end_set = *p + len1;
 
-    ret = mbedtls_asn1_get_tag(p, end_set, &len2, MBEDTLS_ASN1_CONSTRUCTED
-                               | MBEDTLS_ASN1_SEQUENCE);
-    if (ret != 0) {
-        return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_CERT, ret);
-    }
+    while (*p != end_set) {
+        start = *p;
+        ret = mbedtls_asn1_get_tag(p, end_set, &len2, MBEDTLS_ASN1_CONSTRUCTED
+                                   | MBEDTLS_ASN1_SEQUENCE);
+        if (ret != 0) {
+            return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_CERT, ret);
+        }
 
-    end_cert = *p + len2;
+        end_cert = *p + len2;
 
-    /*
-     * This is to verify that there is only one signer certificate. It seems it is
-     * not easy to differentiate between the chain vs different signer's certificate.
-     * So, we support only the root certificate and the single signer.
-     * The behaviour would be improved with addition of multiple signer support.
-     */
-    if (end_cert != end_set) {
-        return MBEDTLS_ERR_PKCS7_FEATURE_UNAVAILABLE;
-    }
-
-    *p = start;
-    if ((ret = mbedtls_x509_crt_parse_der(certs, *p, len1)) < 0) {
-        return MBEDTLS_ERR_PKCS7_INVALID_CERT;
+        *p = start;
+        if ((ret = mbedtls_x509_crt_parse_der(certs, *p,
+                                              (size_t)(end_cert - *p))) < 0) {
+            return MBEDTLS_ERR_PKCS7_INVALID_CERT;
+        }
+        *p = end_cert;
+        n++;
     }
 
-    *p = *p + len1;
+    *p = end_set;
 
-    /*
-     * Since in this version we strictly support single certificate, and reaching
-     * here implies we have parsed successfully, we return 1.
-     */
-    return 1;
+    return n;
 }
 
 /**
@@ -754,29 +746,40 @@ static int pkcs7_signed_attrs_match(mbedtls_asn1_named_data *attrs,
     return 0;
 }
 
+static mbedtls_x509_crt *pkcs7_find_cert(mbedtls_x509_buf *serial,
+                                         mbedtls_x509_crt *cert)
+{
+    for (; cert != NULL; cert = cert->next)
+    {
+        if (serial->len == cert->serial.len &&
+            memcmp(serial->p, cert->serial.p, serial->len) == 0) {
+            return cert;
+        }
+    }
+
+    return NULL;
+}
+
 static int mbedtls_pkcs7_data_or_hash_verify(mbedtls_pkcs7 *pkcs7,
-                                             const mbedtls_x509_crt *cert,
+                                             mbedtls_x509_crt *cert,
                                              const unsigned char *data,
                                              size_t datalen,
                                              const int is_data_hash)
 {
     int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
     unsigned char *hash;
-    mbedtls_pk_context pk_cxt = cert->pk;
+    mbedtls_pk_context pk_ctx;
+    mbedtls_x509_crt *pk_cert;
     const mbedtls_md_info_t *md_info;
     mbedtls_md_type_t md_alg;
     mbedtls_md_context_t md_ctx;
     mbedtls_pkcs7_signer_info *signer;
+    uint32_t flags;
 
     if (pkcs7->signed_data.no_of_signers == 0) {
         return MBEDTLS_ERR_PKCS7_INVALID_CERT;
     }
 
-    if (mbedtls_x509_time_is_past(&cert->valid_to) ||
-        mbedtls_x509_time_is_future(&cert->valid_from)) {
-        return MBEDTLS_ERR_PKCS7_CERT_DATE_INVALID;
-    }
-
     /*
      * Potential TODOs
      * Currently we iterate over all signers and return success if any of them
@@ -847,9 +850,50 @@ static int mbedtls_pkcs7_data_or_hash_verify(mbedtls_pkcs7 *pkcs7,
             mbedtls_md_free(&md_ctx);
         }
 
-        ret = mbedtls_pk_verify(&pk_cxt, md_alg, hash,
-                                mbedtls_md_get_size(md_info),
-                                signer->sig.p, signer->sig.len);
+        ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
+
+        /* try trusted certificates */
+        for (pk_cert = pkcs7_find_cert(&signer->serial, cert);
+             pk_cert != NULL;
+             pk_cert = pkcs7_find_cert(&signer->serial, pk_cert->next)) {
+            if (mbedtls_x509_time_is_past(&pk_cert->valid_to) ||
+                mbedtls_x509_time_is_future(&pk_cert->valid_from)) {
+                ret = MBEDTLS_ERR_PKCS7_CERT_DATE_INVALID;
+                continue;
+            }
+            pk_ctx = pk_cert->pk;
+            ret = mbedtls_pk_verify(&pk_ctx, md_alg, hash,
+                                    mbedtls_md_get_size(md_info),
+                                    signer->sig.p, signer->sig.len);
+            if (ret == 0) {
+                break;
+            }
+            ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
+        }
+
+        /* try embedded certs with a chain of trust back to a trusted cert */
+        if (ret != 0) {
+            for (pk_cert = pkcs7_find_cert(&signer->serial,
+                                           &pkcs7->signed_data.certs);
+                 pk_cert != NULL;
+                 pk_cert = pkcs7_find_cert(&signer->serial, pk_cert->next)) {
+                if (mbedtls_x509_crt_verify(pk_cert, cert,
+                                            &pkcs7->signed_data.crl, NULL,
+                                            &flags, NULL, NULL) != 0) {
+                    ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
+                    continue;
+                }
+                pk_ctx = pk_cert->pk;
+                ret = mbedtls_pk_verify(&pk_ctx, md_alg, hash,
+                                        mbedtls_md_get_size(md_info),
+                                        signer->sig.p, signer->sig.len);
+                if (ret == 0) {
+                    break;
+                }
+                ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
+            }
+        }
+
         mbedtls_free(hash);
         /* END must free hash before jumping out */
 
@@ -861,7 +905,7 @@ static int mbedtls_pkcs7_data_or_hash_verify(mbedtls_pkcs7 *pkcs7,
     return ret;
 }
 int mbedtls_pkcs7_signed_data_verify(mbedtls_pkcs7 *pkcs7,
-                                     const mbedtls_x509_crt *cert,
+                                     mbedtls_x509_crt *cert,
                                      const unsigned char *data,
                                      size_t datalen)
 {
@@ -869,7 +913,7 @@ int mbedtls_pkcs7_signed_data_verify(mbedtls_pkcs7 *pkcs7,
 }
 
 int mbedtls_pkcs7_signed_hash_verify(mbedtls_pkcs7 *pkcs7,
-                                     const mbedtls_x509_crt *cert,
+                                     mbedtls_x509_crt *cert,
                                      const unsigned char *hash,
                                      size_t hashlen)
 {
-- 
2.26.3

