From 6cfc4692961ba808d801ab2ffbf6e076523ccac0 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Mon, 28 Nov 2022 00:46:00 -0500 Subject: [PATCH] pkcs7: reject signatures with internal data A CMS signature can have internal data, but mbedTLS does not support verifying such signatures. Reject them during parsing. Signed-off-by: Demi Marie Obenour Signed-off-by: Dave Rodgman --- include/mbedtls/pkcs7.h | 31 ++++++++++++------------------- library/pkcs7.c | 25 ++++++++++++++++--------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/include/mbedtls/pkcs7.h b/include/mbedtls/pkcs7.h index fb24604d0..126eac422 100644 --- a/include/mbedtls/pkcs7.h +++ b/include/mbedtls/pkcs7.h @@ -135,21 +135,12 @@ typedef struct mbedtls_pkcs7_signer_info { } mbedtls_pkcs7_signer_info; -/** - * Structure holding attached data as part of PKCS7 signed data format - */ -typedef struct mbedtls_pkcs7_data { - mbedtls_pkcs7_buf MBEDTLS_PRIVATE(data); -} -mbedtls_pkcs7_data; - /** * Structure holding the signed data section */ typedef struct mbedtls_pkcs7_signed_data { int MBEDTLS_PRIVATE(version); mbedtls_pkcs7_buf MBEDTLS_PRIVATE(digest_alg_identifiers); - struct mbedtls_pkcs7_data MBEDTLS_PRIVATE(content); int MBEDTLS_PRIVATE(no_of_certs); mbedtls_x509_crt MBEDTLS_PRIVATE(certs); int MBEDTLS_PRIVATE(no_of_crls); @@ -176,7 +167,7 @@ mbedtls_pkcs7; void mbedtls_pkcs7_init(mbedtls_pkcs7 *pkcs7); /** - * \brief Parse a single DER formatted pkcs7 content. + * \brief Parse a single DER formatted pkcs7 detached signature. * * \param pkcs7 The pkcs7 structure to be filled by parser for the output. * \param buf The buffer holding only the DER encoded pkcs7. @@ -186,6 +177,7 @@ void mbedtls_pkcs7_init(mbedtls_pkcs7 *pkcs7); * \note This function makes an internal copy of the PKCS7 buffer * \p buf. In particular, \p buf may be destroyed or reused * after this call returns. + * \note Signatures with internal data are not supported. * * \return The \c mbedtls_pkcs7_type of \p buf, if successful. * \return A negative error code on failure. @@ -205,7 +197,8 @@ int mbedtls_pkcs7_parse_der(mbedtls_pkcs7 *pkcs7, const unsigned char *buf, * matches. * * This function does not use the certificates held within the - * PKCS7 structure itself. + * PKCS7 structure itself, and does not check that the + * certificate is signed by a trusted certification authority. * * \param pkcs7 PKCS7 structure containing signature. * \param cert Certificate containing key to verify signature. @@ -226,15 +219,15 @@ int mbedtls_pkcs7_signed_data_verify(mbedtls_pkcs7 *pkcs7, * \brief Verification of PKCS7 signature against a caller-supplied * certificate. * - * For each signer in the PKCS structure, this function computes - * a signature over the supplied hash, using the supplied - * certificate and the same digest algorithm as specified by the - * signer. It then compares this signature against the - * signer's signature; verification succeeds if any comparison - * matches. + * For each signer in the PKCS structure, this function + * validates a signature over the supplied hash, using the + * supplied certificate and the same digest algorithm as + * specified by the signer. Verification succeeds if any + * signature is good. * * This function does not use the certificates held within the - * PKCS7 structure itself. + * PKCS7 structure itself, and does not check that the + * certificate is signed by a trusted certification authority. * * \param pkcs7 PKCS7 structure containing signature. * \param cert Certificate containing key to verify signature. @@ -242,7 +235,7 @@ int mbedtls_pkcs7_signed_data_verify(mbedtls_pkcs7 *pkcs7, * \param hashlen Length of the hash. * * \note This function is different from mbedtls_pkcs7_signed_data_verify() - * in a way that it directly receives the hash of the data. + * in that it is directly passed the hash of the data. * * \return 0 if the signature verifies, or a negative error code on failure. */ diff --git a/library/pkcs7.c b/library/pkcs7.c index 9ef76089a..fbe959ef2 100644 --- a/library/pkcs7.c +++ b/library/pkcs7.c @@ -57,9 +57,9 @@ static int pkcs7_get_next_content_len(unsigned char **p, unsigned char *end, ret = mbedtls_asn1_get_tag(p, end, len, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_CONTEXT_SPECIFIC); if (ret != 0) { - ret = MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_FORMAT, ret); + ret = MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO, ret); } else if ((size_t) (end - *p) != *len) { - ret = MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_FORMAT, + ret = MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO, MBEDTLS_ERR_ASN1_LENGTH_MISMATCH); } @@ -187,13 +187,13 @@ static int pkcs7_get_certificates(unsigned char **p, unsigned char *end, size_t len2 = 0; unsigned char *end_set, *end_cert, *start; - if ((ret = mbedtls_asn1_get_tag(p, end, &len1, MBEDTLS_ASN1_CONSTRUCTED - | MBEDTLS_ASN1_CONTEXT_SPECIFIC)) != 0) { - if (ret == MBEDTLS_ERR_ASN1_UNEXPECTED_TAG) { - return 0; - } else { - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_FORMAT, ret); - } + ret = mbedtls_asn1_get_tag(p, end, &len1, MBEDTLS_ASN1_CONSTRUCTED + | MBEDTLS_ASN1_CONTEXT_SPECIFIC); + if (ret == MBEDTLS_ERR_ASN1_UNEXPECTED_TAG) { + return 0; + } + if (ret != 0) { + return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS7_INVALID_FORMAT, ret); } start = *p; end_set = *p + len1; @@ -716,11 +716,15 @@ 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, const unsigned char *data, size_t datalen) { + if (data == NULL) { + return MBEDTLS_ERR_PKCS7_BAD_INPUT_DATA; + } return mbedtls_pkcs7_data_or_hash_verify(pkcs7, cert, data, datalen, 0); } @@ -729,6 +733,9 @@ int mbedtls_pkcs7_signed_hash_verify(mbedtls_pkcs7 *pkcs7, const unsigned char *hash, size_t hashlen) { + if (hash == NULL) { + return MBEDTLS_ERR_PKCS7_BAD_INPUT_DATA; + } return mbedtls_pkcs7_data_or_hash_verify(pkcs7, cert, hash, hashlen, 1); }