diff mbox series

[v8,10/27] mbedtls/external: support decoding multiple signer's cert

Message ID 20241003215112.3103601-11-raymond.mao@linaro.org
State Accepted
Commit 6e7acd36de4fffc9a993bba948792bfb677b90fa
Delegated to: Tom Rini
Headers show
Series Integrate MbedTLS v3.6 LTS with U-Boot | expand

Commit Message

Raymond Mao Oct. 3, 2024, 9:50 p.m. UTC
Support decoding multiple signer's cert in the signed data within
a PKCS7 message.

The PR for this patch is at:
https://github.com/Mbed-TLS/mbedtls/pull/9001

For enabling EFI loader PKCS7 features with MbedTLS build,
we need this patch on top of MbedTLS v3.6.0 before it is merged into
the next MbedTLS LTS release.

Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
---
Changes in v2
- None.
Changes in v3
- Update commit message.
Changes in v4
- None.
Changes in v5
- None.
Changes in v6
- None.
Changes in v7
- None.
Changes in v8
- None

 lib/mbedtls/external/mbedtls/library/pkcs7.c | 75 ++++++++++++--------
 1 file changed, 47 insertions(+), 28 deletions(-)

Comments

Ilias Apalodimas Oct. 9, 2024, 9:15 a.m. UTC | #1
On Fri, 4 Oct 2024 at 00:56, Raymond Mao <raymond.mao@linaro.org> wrote:
>
> Support decoding multiple signer's cert in the signed data within
> a PKCS7 message.
>
> The PR for this patch is at:
> https://github.com/Mbed-TLS/mbedtls/pull/9001
>
> For enabling EFI loader PKCS7 features with MbedTLS build,
> we need this patch on top of MbedTLS v3.6.0 before it is merged into
> the next MbedTLS LTS release.
>
> Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> ---
> Changes in v2
> - None.
> Changes in v3
> - Update commit message.
> Changes in v4
> - None.
> Changes in v5
> - None.
> Changes in v6
> - None.
> Changes in v7
> - None.
> Changes in v8
> - None
>
>  lib/mbedtls/external/mbedtls/library/pkcs7.c | 75 ++++++++++++--------
>  1 file changed, 47 insertions(+), 28 deletions(-)
>
> diff --git a/lib/mbedtls/external/mbedtls/library/pkcs7.c b/lib/mbedtls/external/mbedtls/library/pkcs7.c
> index da73fb341d6..01105227d7a 100644
> --- a/lib/mbedtls/external/mbedtls/library/pkcs7.c
> +++ b/lib/mbedtls/external/mbedtls/library/pkcs7.c
> @@ -61,6 +61,36 @@ static int pkcs7_get_next_content_len(unsigned char **p, unsigned char *end,
>      return ret;
>  }
>
> +/**
> + * Get and decode one cert from a sequence.
> + * Return 0 for success,
> + * Return negative error code for failure.
> + **/
> +static int pkcs7_get_one_cert(unsigned char **p, unsigned char *end,
> +                              mbedtls_x509_crt *certs)
> +{
> +    int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
> +    size_t len = 0;
> +    unsigned char *start = *p;
> +    unsigned char *end_cert;
> +
> +    ret = mbedtls_asn1_get_tag(p, end, &len, MBEDTLS_ASN1_CONSTRUCTED
> +                               | MBEDTLS_ASN1_SEQUENCE);
> +    if (ret != 0) {
> +        return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_CERT, ret);
> +    }
> +
> +    end_cert = *p + len;
> +
> +    if ((ret = mbedtls_x509_crt_parse_der(certs, start, end_cert - start)) < 0) {
> +        return MBEDTLS_ERR_PKCS7_INVALID_CERT;
> +    }
> +
> +    *p = end_cert;
> +
> +    return 0;
> +}
> +
>  /**
>   * version Version
>   * Version ::= INTEGER
> @@ -178,11 +208,12 @@ static int pkcs7_get_certificates(unsigned char **p, unsigned char *end,
>                                    mbedtls_x509_crt *certs)
>  {
>      int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
> -    size_t len1 = 0;
> -    size_t len2 = 0;
> -    unsigned char *end_set, *end_cert, *start;
> +    size_t len = 0;
> +    unsigned char *end_set;
> +    int num_of_certs = 0;
>
> -    ret = mbedtls_asn1_get_tag(p, end, &len1, MBEDTLS_ASN1_CONSTRUCTED
> +    /* Get the set of certs */
> +    ret = mbedtls_asn1_get_tag(p, end, &len, MBEDTLS_ASN1_CONSTRUCTED
>                                 | MBEDTLS_ASN1_CONTEXT_SPECIFIC);
>      if (ret == MBEDTLS_ERR_ASN1_UNEXPECTED_TAG) {
>          return 0;
> @@ -190,38 +221,26 @@ static int pkcs7_get_certificates(unsigned char **p, unsigned char *end,
>      if (ret != 0) {
>          return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_FORMAT, ret);
>      }
> -    start = *p;
> -    end_set = *p + len1;
> +    end_set = *p + len;
>
> -    ret = mbedtls_asn1_get_tag(p, end_set, &len2, MBEDTLS_ASN1_CONSTRUCTED
> -                               | MBEDTLS_ASN1_SEQUENCE);
> +    ret = pkcs7_get_one_cert(p, end_set, certs);
>      if (ret != 0) {
> -        return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_CERT, ret);
> +        return ret;
>      }
>
> -    end_cert = *p + len2;
> +    num_of_certs++;
>
> -    /*
> -     * 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;
> -    }
> -
> -    if ((ret = mbedtls_x509_crt_parse_der(certs, start, len1)) < 0) {
> -        return MBEDTLS_ERR_PKCS7_INVALID_CERT;
> +    while (*p != end_set) {
> +        ret = pkcs7_get_one_cert(p, end_set, certs);
> +        if (ret != 0) {
> +            return ret;
> +        }
> +        num_of_certs++;
>      }
>
> -    *p = end_cert;
> +    *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 num_of_certs;
>  }
>
>  /**
> --
> 2.25.1
>

Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
diff mbox series

Patch

diff --git a/lib/mbedtls/external/mbedtls/library/pkcs7.c b/lib/mbedtls/external/mbedtls/library/pkcs7.c
index da73fb341d6..01105227d7a 100644
--- a/lib/mbedtls/external/mbedtls/library/pkcs7.c
+++ b/lib/mbedtls/external/mbedtls/library/pkcs7.c
@@ -61,6 +61,36 @@  static int pkcs7_get_next_content_len(unsigned char **p, unsigned char *end,
     return ret;
 }
 
+/**
+ * Get and decode one cert from a sequence.
+ * Return 0 for success,
+ * Return negative error code for failure.
+ **/
+static int pkcs7_get_one_cert(unsigned char **p, unsigned char *end,
+                              mbedtls_x509_crt *certs)
+{
+    int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
+    size_t len = 0;
+    unsigned char *start = *p;
+    unsigned char *end_cert;
+
+    ret = mbedtls_asn1_get_tag(p, end, &len, MBEDTLS_ASN1_CONSTRUCTED
+                               | MBEDTLS_ASN1_SEQUENCE);
+    if (ret != 0) {
+        return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_CERT, ret);
+    }
+
+    end_cert = *p + len;
+
+    if ((ret = mbedtls_x509_crt_parse_der(certs, start, end_cert - start)) < 0) {
+        return MBEDTLS_ERR_PKCS7_INVALID_CERT;
+    }
+
+    *p = end_cert;
+
+    return 0;
+}
+
 /**
  * version Version
  * Version ::= INTEGER
@@ -178,11 +208,12 @@  static int pkcs7_get_certificates(unsigned char **p, unsigned char *end,
                                   mbedtls_x509_crt *certs)
 {
     int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
-    size_t len1 = 0;
-    size_t len2 = 0;
-    unsigned char *end_set, *end_cert, *start;
+    size_t len = 0;
+    unsigned char *end_set;
+    int num_of_certs = 0;
 
-    ret = mbedtls_asn1_get_tag(p, end, &len1, MBEDTLS_ASN1_CONSTRUCTED
+    /* Get the set of certs */
+    ret = mbedtls_asn1_get_tag(p, end, &len, MBEDTLS_ASN1_CONSTRUCTED
                                | MBEDTLS_ASN1_CONTEXT_SPECIFIC);
     if (ret == MBEDTLS_ERR_ASN1_UNEXPECTED_TAG) {
         return 0;
@@ -190,38 +221,26 @@  static int pkcs7_get_certificates(unsigned char **p, unsigned char *end,
     if (ret != 0) {
         return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_FORMAT, ret);
     }
-    start = *p;
-    end_set = *p + len1;
+    end_set = *p + len;
 
-    ret = mbedtls_asn1_get_tag(p, end_set, &len2, MBEDTLS_ASN1_CONSTRUCTED
-                               | MBEDTLS_ASN1_SEQUENCE);
+    ret = pkcs7_get_one_cert(p, end_set, certs);
     if (ret != 0) {
-        return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_CERT, ret);
+        return ret;
     }
 
-    end_cert = *p + len2;
+    num_of_certs++;
 
-    /*
-     * 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;
-    }
-
-    if ((ret = mbedtls_x509_crt_parse_der(certs, start, len1)) < 0) {
-        return MBEDTLS_ERR_PKCS7_INVALID_CERT;
+    while (*p != end_set) {
+        ret = pkcs7_get_one_cert(p, end_set, certs);
+        if (ret != 0) {
+            return ret;
+        }
+        num_of_certs++;
     }
 
-    *p = end_cert;
+    *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 num_of_certs;
 }
 
 /**