Skip to content

Commit 4f7da6c

Browse files
DemiMarieashishverma2691
authored andcommitted
fix(auth): require at least one extension to be present
X.509 and RFC5280 allow omitting the extensions entirely, but require that if the extensions field is present at all, it must contain at least one certificate. TF-A already requires the extensions to be present, but allows them to be empty. However, a certificate with an empty extensions field will always fail later on, as the extensions contain the information needed to validate the next stage in the boot chain. Therefore, it is simpler to require the extension field to be present and contain at least one extension. Also add a comment explaining why the extensions field is required, even though it is OPTIONAL in the ASN.1 syntax. Change-Id: Ie26eed8a7924bf50937a6b27ccdf7cc9a390588d Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
1 parent 1c6e9c8 commit 4f7da6c

File tree

1 file changed

+18
-4
lines changed

1 file changed

+18
-4
lines changed

Diff for: drivers/auth/mbedtls/mbedtls_x509_parser.c

+18-4
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,18 @@ static int cert_parse(void *img, unsigned int img_len)
290290

291291
/*
292292
* extensions [3] EXPLICIT Extensions OPTIONAL
293-
* -- must use all remaining bytes in TBSCertificate
293+
* }
294+
*
295+
* X.509 and RFC5280 allow omitting the extensions entirely.
296+
* However, in TF-A, a certificate with no extensions would
297+
* always fail later on, as the extensions contain the
298+
* information needed to authenticate the next stage in the
299+
* boot chain. Furthermore, get_ext() assumes that the
300+
* extensions have been parsed into v3_ext, and allowing
301+
* there to be no extensions would pointlessly complicate
302+
* the code. Therefore, just reject certificates without
303+
* extensions. This is also why version 1 and 2 certificates
304+
* are rejected above.
294305
*/
295306
ret = mbedtls_asn1_get_tag(&p, end, &len,
296307
MBEDTLS_ASN1_CONTEXT_SPECIFIC |
@@ -312,9 +323,12 @@ static int cert_parse(void *img, unsigned int img_len)
312323
v3_ext.len = end - v3_ext.p;
313324

314325
/*
315-
* Check extensions integrity
326+
* Check extensions integrity. At least one extension is
327+
* required: the ASN.1 specifies a minimum size of 1, and at
328+
* least one extension is needed to authenticate the next stage
329+
* in the boot chain.
316330
*/
317-
while (p < end) {
331+
do {
318332
ret = mbedtls_asn1_get_tag(&p, end, &len,
319333
MBEDTLS_ASN1_CONSTRUCTED |
320334
MBEDTLS_ASN1_SEQUENCE);
@@ -342,7 +356,7 @@ static int cert_parse(void *img, unsigned int img_len)
342356
return IMG_PARSER_ERR_FORMAT;
343357
}
344358
p += len;
345-
}
359+
} while (p < end);
346360

347361
if (p != end) {
348362
return IMG_PARSER_ERR_FORMAT;

0 commit comments

Comments
 (0)