mirror of
https://github.com/ARM-software/arm-trusted-firmware.git
synced 2025-04-17 01:54:22 +00:00
fix(auth): allow hashes of different lengths
Trusted Board Boot supports multiple hash algorithms, including SHA-256, SHA-384, and SHA-512. These algorithms produce hashes of different lengths, so the resulting DER-encoded hash objects are also of different lengths. However, the common Trusted Board Boot code only stores the contents of the object, not its length. Before commitf47547b354
, this was harmless: ASN.1 objects are self-delimiting, and any excess padding was ignored.f47547b354
changed the code to reject excess padding. However, this breaks using a shorter hash in a build that supports longer hashes: the shorter hash will have padding after it, and verify_hash() will reject it. This was found by an Arm customer: TF-A v2.9 refused to boot, even though TF-A v2.6 (which did not havef47547b354
) worked just fine. Storing the length of the hash turns out to be quite difficult. However, it turns out that hashes verified by verify_hash() always come from the ROTPK or an X.509 certificate extension. Furthermore, _all_ X.509 certificate extensions used by Trusted Board Boot are ASN.1 DER encoded, so it is possible to reject padding in get_ext(). Padding after the ROTPK is harmless, and it is better to ignore that padding than to refuse to boot the system. Change-Id: I28a19d7783e6036b65e86426d78c8e5b2ed6f542 Fixes:f47547b354
("fix(auth): reject invalid padding in digests") Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
This commit is contained in:
parent
ec8ba97e4f
commit
22a53545aa
2 changed files with 36 additions and 3 deletions
|
@ -172,17 +172,20 @@ static int verify_hash(void *data_ptr, unsigned int data_len,
|
|||
int rc;
|
||||
|
||||
/*
|
||||
* Digest info should be an MBEDTLS_ASN1_SEQUENCE
|
||||
* and consume all bytes.
|
||||
* Digest info should be an MBEDTLS_ASN1_SEQUENCE, but padding after
|
||||
* it is allowed. This is necessary to support multiple hash
|
||||
* algorithms.
|
||||
*/
|
||||
p = (unsigned char *)digest_info_ptr;
|
||||
end = p + digest_info_len;
|
||||
rc = mbedtls_asn1_get_tag(&p, end, &len, MBEDTLS_ASN1_CONSTRUCTED |
|
||||
MBEDTLS_ASN1_SEQUENCE);
|
||||
if (rc != 0 || ((size_t)(end - p) != len)) {
|
||||
if (rc != 0) {
|
||||
return CRYPTO_ERR_HASH;
|
||||
}
|
||||
|
||||
end = p + len;
|
||||
|
||||
/* Get the hash algorithm */
|
||||
rc = mbedtls_asn1_get_alg(&p, end, &hash_oid, ¶ms);
|
||||
if (rc != 0) {
|
||||
|
|
|
@ -135,8 +135,38 @@ static int get_ext(const char *oid, void **ext, unsigned int *ext_len)
|
|||
if ((oid != NULL) &&
|
||||
((size_t)oid_len == strlen(oid_str)) &&
|
||||
(strcmp(oid, oid_str) == 0)) {
|
||||
/* Extension must be ASN.1 DER */
|
||||
if (len < 2) {
|
||||
/* too short */
|
||||
return IMG_PARSER_ERR_FORMAT;
|
||||
}
|
||||
|
||||
if ((p[0] & 0x1F) == 0x1F) {
|
||||
/* multi-byte ASN.1 DER tag, not allowed */
|
||||
return IMG_PARSER_ERR_FORMAT;
|
||||
}
|
||||
|
||||
if ((p[0] & 0xDF) == 0) {
|
||||
/* UNIVERSAL 0 tag, not allowed */
|
||||
return IMG_PARSER_ERR_FORMAT;
|
||||
}
|
||||
|
||||
*ext = (void *)p;
|
||||
*ext_len = (unsigned int)len;
|
||||
|
||||
/* Advance past the tag byte */
|
||||
p++;
|
||||
|
||||
if (mbedtls_asn1_get_len(&p, end_ext_data, &len)) {
|
||||
/* not valid DER */
|
||||
return IMG_PARSER_ERR_FORMAT;
|
||||
}
|
||||
|
||||
if (p + len != end_ext_data) {
|
||||
/* junk after ASN.1 object */
|
||||
return IMG_PARSER_ERR_FORMAT;
|
||||
}
|
||||
|
||||
return IMG_PARSER_OK;
|
||||
}
|
||||
|
||||
|
|
Loading…
Add table
Reference in a new issue