From f3c17d718b1253eaf176ab3393e4bcb3f982300a Mon Sep 17 00:00:00 2001 From: Joakim Sindholt 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