Merge changes Ia748b6ae,Id8a48e14,Id25ab231,Ie26eed8a,Idf48f716, ... into integration
* changes:
refactor(auth): partially validate SubjectPublicKeyInfo early
fix(auth): reject padding after BIT STRING in signatures
fix(auth): reject invalid padding in digests
fix(auth): require at least one extension to be present
fix(auth): forbid junk after extensions
fix(auth): only accept v3 X.509 certificates
diff --git a/drivers/auth/mbedtls/mbedtls_crypto.c b/drivers/auth/mbedtls/mbedtls_crypto.c
index d231179..42a0925 100644
--- a/drivers/auth/mbedtls/mbedtls_crypto.c
+++ b/drivers/auth/mbedtls/mbedtls_crypto.c
@@ -115,7 +115,7 @@
end = (unsigned char *)(p + sig_len);
signature.tag = *p;
rc = mbedtls_asn1_get_bitstring_null(&p, end, &signature.len);
- if (rc != 0) {
+ if ((rc != 0) || ((size_t)(end - p) != signature.len)) {
rc = CRYPTO_ERR_SIGNATURE;
goto end1;
}
@@ -170,12 +170,15 @@
size_t len;
int rc;
- /* Digest info should be an MBEDTLS_ASN1_SEQUENCE */
+ /*
+ * Digest info should be an MBEDTLS_ASN1_SEQUENCE
+ * and consume all bytes.
+ */
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) {
+ if (rc != 0 || ((size_t)(end - p) != len)) {
return CRYPTO_ERR_HASH;
}
@@ -195,9 +198,9 @@
return CRYPTO_ERR_HASH;
}
- /* Hash should be octet string type */
+ /* Hash should be octet string type and consume all bytes */
rc = mbedtls_asn1_get_tag(&p, end, &len, MBEDTLS_ASN1_OCTET_STRING);
- if (rc != 0) {
+ if ((rc != 0) || ((size_t)(end - p) != len)) {
return CRYPTO_ERR_HASH;
}
diff --git a/drivers/auth/mbedtls/mbedtls_x509_parser.c b/drivers/auth/mbedtls/mbedtls_x509_parser.c
index 993ef12..244f1c9 100644
--- a/drivers/auth/mbedtls/mbedtls_x509_parser.c
+++ b/drivers/auth/mbedtls/mbedtls_x509_parser.c
@@ -144,8 +144,23 @@
{
int ret, is_critical;
size_t len;
- unsigned char *p, *end, *crt_end;
+ unsigned char *p, *end, *crt_end, *pk_end;
mbedtls_asn1_buf sig_alg1, sig_alg2;
+ /*
+ * The unique ASN.1 DER encoding of [0] EXPLICIT INTEGER { v3(2} }.
+ */
+ static const char v3[] = {
+ /* The outer CONTEXT SPECIFIC 0 tag */
+ MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_CONTEXT_SPECIFIC | 0,
+ /* The number bytes used to encode the inner INTEGER */
+ 3,
+ /* The tag of the inner INTEGER */
+ MBEDTLS_ASN1_INTEGER,
+ /* The number of bytes needed to represent 2 */
+ 1,
+ /* The actual value 2 */
+ 2,
+ };
p = (unsigned char *)img;
len = img_len;
@@ -181,15 +196,14 @@
tbs.len = end - tbs.p;
/*
- * Version ::= INTEGER { v1(0), v2(1), v3(2) }
+ * Version ::= [0] EXPLICIT INTEGER { v1(0), v2(1), v3(2) }
+ * -- only v3 accepted
*/
- ret = mbedtls_asn1_get_tag(&p, end, &len,
- MBEDTLS_ASN1_CONTEXT_SPECIFIC |
- MBEDTLS_ASN1_CONSTRUCTED | 0);
- if (ret != 0) {
+ if (((end - p) <= (ptrdiff_t)sizeof(v3)) ||
+ (memcmp(p, v3, sizeof(v3)) != 0)) {
return IMG_PARSER_ERR_FORMAT;
}
- p += len;
+ p += sizeof(v3);
/*
* CertificateSerialNumber ::= INTEGER
@@ -257,9 +271,22 @@
if (ret != 0) {
return IMG_PARSER_ERR_FORMAT;
}
+ pk_end = p + len;
+ pk.len = pk_end - pk.p;
+
+ ret = mbedtls_asn1_get_tag(&p, pk_end, &len, MBEDTLS_ASN1_CONSTRUCTED |
+ MBEDTLS_ASN1_SEQUENCE);
+ if (ret != 0) {
+ return IMG_PARSER_ERR_FORMAT;
+ }
- pk.len = (p + len) - pk.p;
p += len;
+ ret = mbedtls_asn1_get_tag(&p, pk_end, &len, MBEDTLS_ASN1_BIT_STRING);
+ if ((ret != 0) || (p + len != pk_end)) {
+ return IMG_PARSER_ERR_FORMAT;
+ }
+ p = pk_end;
+
/*
* issuerUniqueID [1] IMPLICIT UniqueIdentifier OPTIONAL,
*/
@@ -290,29 +317,45 @@
/*
* extensions [3] EXPLICIT Extensions OPTIONAL
+ * }
+ *
+ * X.509 and RFC5280 allow omitting the extensions entirely.
+ * However, in TF-A, a certificate with no extensions would
+ * always fail later on, as the extensions contain the
+ * information needed to authenticate the next stage in the
+ * boot chain. Furthermore, get_ext() assumes that the
+ * extensions have been parsed into v3_ext, and allowing
+ * there to be no extensions would pointlessly complicate
+ * the code. Therefore, just reject certificates without
+ * extensions. This is also why version 1 and 2 certificates
+ * are rejected above.
*/
ret = mbedtls_asn1_get_tag(&p, end, &len,
MBEDTLS_ASN1_CONTEXT_SPECIFIC |
MBEDTLS_ASN1_CONSTRUCTED | 3);
- if (ret != 0) {
+ if ((ret != 0) || (len != (size_t)(end - p))) {
return IMG_PARSER_ERR_FORMAT;
}
/*
* Extensions ::= SEQUENCE SIZE (1..MAX) OF Extension
+ * -- must use all remaining bytes in TBSCertificate
*/
v3_ext.p = p;
ret = mbedtls_asn1_get_tag(&p, end, &len, MBEDTLS_ASN1_CONSTRUCTED |
MBEDTLS_ASN1_SEQUENCE);
- if (ret != 0) {
+ if ((ret != 0) || (len != (size_t)(end - p))) {
return IMG_PARSER_ERR_FORMAT;
}
- v3_ext.len = (p + len) - v3_ext.p;
+ v3_ext.len = end - v3_ext.p;
/*
- * Check extensions integrity
+ * Check extensions integrity. At least one extension is
+ * required: the ASN.1 specifies a minimum size of 1, and at
+ * least one extension is needed to authenticate the next stage
+ * in the boot chain.
*/
- while (p < end) {
+ do {
ret = mbedtls_asn1_get_tag(&p, end, &len,
MBEDTLS_ASN1_CONSTRUCTED |
MBEDTLS_ASN1_SEQUENCE);
@@ -340,7 +383,7 @@
return IMG_PARSER_ERR_FORMAT;
}
p += len;
- }
+ } while (p < end);
if (p != end) {
return IMG_PARSER_ERR_FORMAT;