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 <demiobenour@gmail.com>
Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
diff --git a/include/mbedtls/pkcs7.h b/include/mbedtls/pkcs7.h
index fb24604..126eac4 100644
--- a/include/mbedtls/pkcs7.h
+++ b/include/mbedtls/pkcs7.h
@@ -136,20 +136,11 @@
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 @@
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 @@
* \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 @@
* 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 @@
* \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 @@
* \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 9ef7608..fbe959e 100644
--- a/library/pkcs7.c
+++ b/library/pkcs7.c
@@ -57,9 +57,9 @@
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 @@
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 @@
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 @@
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);
}