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

2 files changed, 65 insertions, 19 deletions

diff --git a/drivers/auth/mbedtls/mbedtls_crypto.c b/drivers/auth/mbedtls/mbedtls_crypto.c
index d231179f5..42a09255d 100644
--- a/drivers/auth/mbedtls/mbedtls_crypto.c
+++ b/drivers/auth/mbedtls/mbedtls_crypto.c
@@ -115,7 +115,7 @@ static int verify_signature(void *data_ptr, unsigned int data_len,
 	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 @@ static int verify_hash(void *data_ptr, unsigned int data_len,
 	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 @@ static int verify_hash(void *data_ptr, unsigned int data_len,
 		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 993ef1264..244f1c95e 100644
--- a/drivers/auth/mbedtls/mbedtls_x509_parser.c
+++ b/drivers/auth/mbedtls/mbedtls_x509_parser.c
@@ -144,8 +144,23 @@ static int cert_parse(void *img, unsigned int img_len)
 {
 	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 @@ static int cert_parse(void *img, unsigned int img_len)
 	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 @@ static int cert_parse(void *img, unsigned int img_len)
 	if (ret != 0) {
 		return IMG_PARSER_ERR_FORMAT;
 	}
-	pk.len = (p + len) - pk.p;
+	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;
+	}
 	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 @@ static int cert_parse(void *img, unsigned int img_len)
 
 	/*
 	 * 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 @@ static int cert_parse(void *img, unsigned int img_len)
 			return IMG_PARSER_ERR_FORMAT;
 		}
 		p += len;
-	}
+	} while (p < end);
 
 	if (p != end) {
 		return IMG_PARSER_ERR_FORMAT;