| Message ID | 20240507175132.1456512-15-raymond.mao@linaro.org |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Integrate MbedTLS v3.6 LTS with U-Boot | expand |
Hi Raymond On Tue, 7 May 2024 at 20:57, Raymond Mao <raymond.mao@linaro.org> wrote: > > Support decoding multiple signer's cert in the signed data within > a PKCS7 message. For all similar external mbedTLS patches, try to explain which belong to mbedTLS must be sent upstream and which ones we need to carry internally. I don't expect us to have to carry anything in U-Boot Thanks /Ilias > > Signed-off-by: Raymond Mao <raymond.mao@linaro.org> > --- > Changes in v2 > - 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 >
Hi Ilias, On Wed, 8 May 2024 at 10:35, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > Hi Raymond > > On Tue, 7 May 2024 at 20:57, Raymond Mao <raymond.mao@linaro.org> wrote: > > > > Support decoding multiple signer's cert in the signed data within > > a PKCS7 message. > > For all similar external mbedTLS patches, try to explain which belong > to mbedTLS must be sent upstream and which ones we need to carry > internally. I don't expect us to have to carry anything in U-Boot > > I can add a reference link to the MbedTLS PR for the patches marked with "mbedtls/external". But for enabling the features, we still need to merge them before the next git subtree update when those patches are merged in a MbedTLS release. Regards, Raymond Thanks > /Ilias > > > > > Signed-off-by: Raymond Mao <raymond.mao@linaro.org> > > --- > > Changes in v2 > > - 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 > > >
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; } /**
Support decoding multiple signer's cert in the signed data within a PKCS7 message. Signed-off-by: Raymond Mao <raymond.mao@linaro.org> --- Changes in v2 - None. lib/mbedtls/external/mbedtls/library/pkcs7.c | 75 ++++++++++++-------- 1 file changed, 47 insertions(+), 28 deletions(-)