packages/arm-trusted-firmware
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix CVE-2022-47630

Browse Source 
(cherry picked from commit 10789ca79000e9fc813bf12c4cf6af5b5db5704a)
This commit is contained in:
starlet-dx 2023-12-01 17:50:50 +08:00 committed by openeuler-sync-bot
parent 98ff2070dc
commit 5206eb0b42
5 changed files with 301 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
CVE-2022-47630-1.patch Normal file
View File

@ -0,0 +1,50 @@
From fd37982a19a4a2911912ce321b9468993a0919ad Mon Sep 17 00:00:00 2001
From: Demi Marie Obenour <demiobenour@gmail.com>
Date: Thu, 8 Dec 2022 15:23:56 -0500
Subject: fix(auth): forbid junk after extensions
The extensions must use all remaining bytes in the TBSCertificate.
Change-Id: Idf48f7168e146d050ba62dbc732638946fcd6c92
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
---
drivers/auth/mbedtls/mbedtls_x509_parser.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/auth/mbedtls/mbedtls_x509_parser.c b/drivers/auth/mbedtls/mbedtls_x509_parser.c
index 49bc008ed1..8c78003bb2 100644
--- a/drivers/auth/mbedtls/mbedtls_x509_parser.c
+++ b/drivers/auth/mbedtls/mbedtls_x509_parser.c
@@ -304,24 +304,26 @@ static int cert_parse(void *img, unsigned int img_len)
/*
* extensions [3] EXPLICIT Extensions OPTIONAL
+ * -- must use all remaining bytes in TBSCertificate
*/
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
--
cgit v1.2.3

73
CVE-2022-47630-2.patch Normal file
View File

@ -0,0 +1,73 @@
From 72460f50e2437a85ce5229c430931aab8f4a0d5b Mon Sep 17 00:00:00 2001
From: Demi Marie Obenour <demiobenour@gmail.com>
Date: Thu, 8 Dec 2022 15:23:58 -0500
Subject: 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>
---
drivers/auth/mbedtls/mbedtls_x509_parser.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/auth/mbedtls/mbedtls_x509_parser.c b/drivers/auth/mbedtls/mbedtls_x509_parser.c
index 8c78003bb2..9cccd964d4 100644
--- a/drivers/auth/mbedtls/mbedtls_x509_parser.c
+++ b/drivers/auth/mbedtls/mbedtls_x509_parser.c
@@ -304,7 +304,18 @@ static int cert_parse(void *img, unsigned int img_len)
/*
* extensions [3] EXPLICIT Extensions OPTIONAL
- * -- must use all remaining bytes in TBSCertificate
+ * }
+ *
+ * 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 |
@@ -326,9 +337,12 @@ static int cert_parse(void *img, unsigned int img_len)
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);
@@ -356,7 +370,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;
--
cgit v1.2.3

84
CVE-2022-47630-3.patch Normal file
View File

@ -0,0 +1,84 @@
From f5c51855d36e399e6e22cc1eb94f6b58e51b3b6d Mon Sep 17 00:00:00 2001
From: Demi Marie Obenour <demiobenour@gmail.com>
Date: Fri, 9 Dec 2022 17:19:08 -0500
Subject: fix(auth): properly validate X.509 extensions
get_ext() does not check the return value of the various mbedtls_*
functions, as cert_parse() is assumed to have guaranteed that they will
always succeed. However, it passes the end of an extension as the end
pointer to these functions, whereas cert_parse() passes the end of the
TBSCertificate. Furthermore, cert_parse() does *not* check that the
contents of the extension have the same length as the extension itself.
Before fd37982a19a4a291 ("fix(auth): forbid junk after extensions"),
cert_parse() also does not check that the extension block extends to the
end of the TBSCertificate.
This is a problem, as mbedtls_asn1_get_tag() leaves *p and *len
undefined on failure. In practice, this results in get_ext() continuing
to parse at different offsets than were used (and validated) by
cert_parse(), which means that the in-bounds guarantee provided by
cert_parse() no longer holds.
This patch fixes the remaining flaw by enforcing that the contents of an
extension are the same length as the extension itself.
Change-Id: Id4570f911402e34d5d6c799ae01a01f184c68d7c
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
Signed-off-by: Sandrine Bailleux <sandrine.bailleux@arm.com>
---
drivers/auth/mbedtls/mbedtls_x509_parser.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/auth/mbedtls/mbedtls_x509_parser.c b/drivers/auth/mbedtls/mbedtls_x509_parser.c
index 44b25ba72b..bef2f3d0a6 100644
--- a/drivers/auth/mbedtls/mbedtls_x509_parser.c
+++ b/drivers/auth/mbedtls/mbedtls_x509_parser.c
@@ -355,33 +355,39 @@ static int cert_parse(void *img, unsigned int img_len)
* in the boot chain.
*/
do {
+ unsigned char *end_ext_data;
+
ret = mbedtls_asn1_get_tag(&p, end, &len,
MBEDTLS_ASN1_CONSTRUCTED |
MBEDTLS_ASN1_SEQUENCE);
if (ret != 0) {
return IMG_PARSER_ERR_FORMAT;
}
+ end_ext_data = p + len;
/* Get extension ID */
- ret = mbedtls_asn1_get_tag(&p, end, &len, MBEDTLS_ASN1_OID);
+ ret = mbedtls_asn1_get_tag(&p, end_ext_data, &len, MBEDTLS_ASN1_OID);
if (ret != 0) {
return IMG_PARSER_ERR_FORMAT;
}
p += len;
/* Get optional critical */
- ret = mbedtls_asn1_get_bool(&p, end, &is_critical);
+ ret = mbedtls_asn1_get_bool(&p, end_ext_data, &is_critical);
if ((ret != 0) && (ret != MBEDTLS_ERR_ASN1_UNEXPECTED_TAG)) {
return IMG_PARSER_ERR_FORMAT;
}
- /* Data should be octet string type */
- ret = mbedtls_asn1_get_tag(&p, end, &len,
+ /*
+ * Data should be octet string type and must use all bytes in
+ * the Extension.
+ */
+ ret = mbedtls_asn1_get_tag(&p, end_ext_data, &len,
MBEDTLS_ASN1_OCTET_STRING);
- if (ret != 0) {
+ if ((ret != 0) || ((p + len) != end_ext_data)) {
return IMG_PARSER_ERR_FORMAT;
}
- p += len;
+ p = end_ext_data;
} while (p < end);
if (p != end) {
--
cgit v1.2.3

82
CVE-2022-47630-4.patch Normal file
View File

@ -0,0 +1,82 @@
From abb8f936fd0ad085b1966bdc2cddf040ba3865e3 Mon Sep 17 00:00:00 2001
From: Demi Marie Obenour <demiobenour@gmail.com>
Date: Fri, 9 Dec 2022 18:21:47 -0500
Subject: fix(auth): avoid out-of-bounds read in auth_nvctr()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
auth_nvctr() does not check that the buffer provided is long enough to
hold an ASN.1 INTEGER, or even that the buffer is non-empty. Since
auth_nvctr() will only ever read 6 bytes, it is possible to read up to
6 bytes past the end of the buffer.
This out-of-bounds read turns out to be harmless. The only caller of
auth_nvctr() always passes a pointer into an X.509 TBSCertificate, and
all in-tree chains of trust require that the certificate’s signature has
already been validated. This means that the signature algorithm
identifier is at least 4 bytes and the signature itself more than that.
Therefore, the data read will be from the certificate itself. Even if
the certificate signature has not been validated, an out-of-bounds read
is still not possible. Since there are at least two bytes (tag and
length) in both the signature algorithm ID and the signature itself, an
out-of-bounds read would require that the tag byte of the signature
algorithm ID would need to be either the tag or length byte of the
DER-encoded nonvolatile counter. However, this byte must be
(MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE) (0x30), which is
greater than 4 and not equal to MBEDTLS_ASN1_INTEGER (2). Therefore,
auth_nvctr() will error out before reading the integer itself,
preventing an out-of-bounds read.
Change-Id: Ibdf1af702fbeb98a94c0c96456ebddd3d392ad44
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
---
drivers/auth/auth_mod.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/auth/auth_mod.c b/drivers/auth/auth_mod.c
index eb537b6..070f60f 100644
--- a/drivers/auth/auth_mod.c
+++ b/drivers/auth/auth_mod.c
@@ -228,7 +228,7 @@ static int auth_nvctr(const auth_method_param_nv_ctr_t *param,
const auth_img_desc_t *img_desc,
void *img, unsigned int img_len)
{
- char *p;
+ unsigned char *p;
void *data_ptr = NULL;
unsigned int data_len, len, i;
unsigned int cert_nv_ctr, plat_nv_ctr;
@@ -242,16 +242,24 @@ static int auth_nvctr(const auth_method_param_nv_ctr_t *param,
/* Parse the DER encoded integer */
assert(data_ptr);
- p = (char *)data_ptr;
- if (*p != ASN1_INTEGER) {
+ p = (unsigned char *)data_ptr;
+
+ /*
+ * Integers must be at least 3 bytes: 1 for tag, 1 for length, and 1
+ * for value. The first byte (tag) must be ASN1_INTEGER.
+ */
+ if ((data_len < 3) || (*p != ASN1_INTEGER)) {
/* Invalid ASN.1 integer */
return 1;
}
p++;
- /* NV-counters are unsigned integers up to 32-bit */
- len = (unsigned int)(*p & 0x7f);
- if ((*p & 0x80) || (len > 4)) {
+ /*
+ * NV-counters are unsigned integers up to 31 bits. Trailing
+ * padding is not allowed.
+ */
+ len = (unsigned int)*p;
+ if ((len > 4) || (data_len - 2 != len)) {
return 1;
}
p++;
--
2.30.0

13
arm-trusted-firmware.spec
View File

@ -2,11 +2,19 @@
Name: arm-trusted-firmware
Version: 2.3
Release: 2
Release: 3
Summary: ARM Trusted Firmware
License: BSD
URL: https://github.com/ARM-software/arm-trusted-firmware/wiki
Source0: https://github.com/ARM-software/arm-trusted-firmware/archive/v%{version}.tar.gz
# https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=fd37982a19a4a291
Patch0000: CVE-2022-47630-1.patch
# https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=72460f50e2437a85
Patch0001: CVE-2022-47630-2.patch
# https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=f5c51855d36e399e
Patch0002: CVE-2022-47630-3.patch
# https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=abb8f936fd0ad085
Patch0003: CVE-2022-47630-4.patch
ExclusiveArch: aarch64
BuildRequires: dtc
@ -61,6 +69,9 @@ strip %{buildroot}/%{_datadir}/%{name}/rk3368/bl31.elf
%{_datadir}/%{name}
%changelog
* Fri Dec 01 2023 yaoxin <yao_xin001@hoperun.com> - 2.3-3
- Fix CVE-2022-47630
* Wed Dec 07 2022 yaoxin <yaoxin30@h-partners.com> -2.3-2
- Add RELRO,PIE,BIND_NOW flags and fix not striped problem