pkcs7: Check that hash algs are in digestAlgorithms
Since only a single hash algorithm is currenlty supported, this avoids
having to perform hashing more than once.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
diff --git a/library/pkcs7.c b/library/pkcs7.c
index fbe959e..36e1960 100644
--- a/library/pkcs7.c
+++ b/library/pkcs7.c
@@ -287,7 +287,8 @@
* and unauthenticatedAttributes.
**/
static int pkcs7_get_signer_info(unsigned char **p, unsigned char *end,
- mbedtls_pkcs7_signer_info *signer)
+ mbedtls_pkcs7_signer_info *signer,
+ mbedtls_x509_buf *alg)
{
unsigned char *end_signer, *end_issuer_and_sn;
int asn1_ret = 0, ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
@@ -345,8 +346,15 @@
goto out;
}
- /* Assume authenticatedAttributes is nonexistent */
+ /* Check that the digest algorithm used matches the one provided earlier */
+ if (signer->alg_identifier.tag != alg->tag ||
+ signer->alg_identifier.len != alg->len ||
+ memcmp(signer->alg_identifier.p, alg->p, alg->len) != 0) {
+ ret = MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO;
+ goto out;
+ }
+ /* Asssume authenticatedAttributes is nonexistent */
ret = pkcs7_get_digest_algorithm(p, end_signer, &signer->sig_alg_identifier);
if (ret != 0) {
goto out;
@@ -379,7 +387,8 @@
* Return negative error code for failure.
**/
static int pkcs7_get_signers_info_set(unsigned char **p, unsigned char *end,
- mbedtls_pkcs7_signer_info *signers_set)
+ mbedtls_pkcs7_signer_info *signers_set,
+ mbedtls_x509_buf *digest_alg)
{
unsigned char *end_set;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
@@ -399,7 +408,7 @@
end_set = *p + len;
- ret = pkcs7_get_signer_info(p, end_set, signers_set);
+ ret = pkcs7_get_signer_info(p, end_set, signers_set, digest_alg);
if (ret != 0) {
return ret;
}
@@ -414,7 +423,7 @@
goto cleanup;
}
- ret = pkcs7_get_signer_info(p, end_set, signer);
+ ret = pkcs7_get_signer_info(p, end_set, signer, digest_alg);
if (ret != 0) {
mbedtls_free(signer);
goto cleanup;
@@ -534,7 +543,10 @@
signed_data->no_of_crls = 0;
/* Get signers info */
- ret = pkcs7_get_signers_info_set(&p, end, &signed_data->signers);
+ ret = pkcs7_get_signers_info_set(&p,
+ end,
+ &signed_data->signers,
+ &signed_data->digest_alg_identifiers);
if (ret < 0) {
return ret;
}
@@ -657,6 +669,39 @@
return MBEDTLS_ERR_PKCS7_CERT_DATE_INVALID;
}
+ ret = mbedtls_oid_get_md_alg(&pkcs7->signed_data.digest_alg_identifiers, &md_alg);
+ if (ret != 0) {
+ return ret;
+ }
+
+ md_info = mbedtls_md_info_from_type(md_alg);
+ if (md_info == NULL) {
+ return MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
+ }
+
+ hash = mbedtls_calloc(mbedtls_md_get_size(md_info), 1);
+ if (hash == NULL) {
+ return MBEDTLS_ERR_PKCS7_ALLOC_FAILED;
+ }
+ /* BEGIN must free hash before jumping out */
+
+ if (is_data_hash) {
+ if (datalen != mbedtls_md_get_size(md_info)) {
+ ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
+ } else {
+ memcpy(hash, data, datalen);
+ }
+ } else {
+ ret = mbedtls_md(md_info, data, datalen, hash);
+ }
+ if (ret != 0) {
+ mbedtls_free(hash);
+ return MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
+ }
+
+ /* assume failure */
+ ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
+
/*
* Potential TODOs
* Currently we iterate over all signers and return success if any of them
@@ -666,54 +711,19 @@
* identification and SignerIdentifier fields first. That would also allow
* us to distinguish between 'no signature for key' and 'signature for key
* failed to validate'.
- *
- * We could also cache hashes by md, so if there are several sigs all using
- * the same algo we don't recalculate the hash each time.
*/
for (signer = &pkcs7->signed_data.signers; signer; signer = signer->next) {
- ret = mbedtls_oid_get_md_alg(&signer->alg_identifier, &md_alg);
- if (ret != 0) {
- ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
- continue;
- }
-
- md_info = mbedtls_md_info_from_type(md_alg);
- if (md_info == NULL) {
- ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
- continue;
- }
-
- hash = mbedtls_calloc(mbedtls_md_get_size(md_info), 1);
- if (hash == NULL) {
- return MBEDTLS_ERR_PKCS7_ALLOC_FAILED;
- }
- /* BEGIN must free hash before jumping out */
- if (is_data_hash) {
- if (datalen != mbedtls_md_get_size(md_info)) {
- ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
- } else {
- memcpy(hash, data, datalen);
- }
- } else {
- ret = mbedtls_md(md_info, data, datalen, hash);
- }
- if (ret != 0) {
- ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
- mbedtls_free(hash);
- continue;
- }
-
ret = mbedtls_pk_verify(&pk_cxt, md_alg, hash,
mbedtls_md_get_size(md_info),
signer->sig.p, signer->sig.len);
- mbedtls_free(hash);
- /* END must free hash before jumping out */
if (ret == 0) {
break;
}
}
+ mbedtls_free(hash);
+ /* END must free hash before jumping out */
return ret;
}