Merge "docs(security): security advisory for CVE-2022-47630" into lts-v2.8lts-v2.8-rc0lts-test

2 files changed, 160 insertions, 0 deletions

diff --git a/docs/security_advisories/index.rst b/docs/security_advisories/index.rst
index b80ba34d3..c9b0f7819 100644
--- a/docs/security_advisories/index.rst
+++ b/docs/security_advisories/index.rst
@@ -14,3 +14,4 @@ Security Advisories
    security-advisory-tfv-7.rst
    security-advisory-tfv-8.rst
    security-advisory-tfv-9.rst
+   security-advisory-tfv-10.rst
diff --git a/docs/security_advisories/security-advisory-tfv-10.rst b/docs/security_advisories/security-advisory-tfv-10.rst
new file mode 100644
index 000000000..91dba074a
--- /dev/null
+++ b/docs/security_advisories/security-advisory-tfv-10.rst
@@ -0,0 +1,159 @@
+Advisory TFV-10 (CVE-2022-47630)
+================================
+
++----------------+-------------------------------------------------------------+
+| Title          | Incorrect validation of X.509 certificate extensions can    |
+|                | result in an out-of-bounds read.                            |
++================+=============================================================+
+| CVE ID         | `CVE-2022-47630`_                                           |
++----------------+-------------------------------------------------------------+
+| Date           | Reported on 12 Dec 2022                                     |
++----------------+-------------------------------------------------------------+
+| Versions       | v1.2 to v2.8                                                |
+| Affected       |                                                             |
++----------------+-------------------------------------------------------------+
+| Configurations | BL1 and BL2 with Trusted Boot enabled with custom,          |
+| Affected       | downstream usages of ``get_ext()`` and/or ``auth_nvctr()``  |
+|                | interfaces. Not exploitable in upstream TF-A code.          |
++----------------+-------------------------------------------------------------+
+| Impact         | Out-of-bounds read.                                         |
++----------------+-------------------------------------------------------------+
+| Fix Version    | - `fd37982a19a4a291`_ "fix(auth): forbid junk after         |
+|                |   extensions"                                               |
+|                |                                                             |
+|                | - `72460f50e2437a85`_ "fix(auth): require at least one      |
+|                |   extension to be present"                                  |
+|                |                                                             |
+|                | - `f5c51855d36e399e`_ "fix(auth): properly validate X.509   |
+|                |   extensions"                                               |
+|                |                                                             |
+|                | - `abb8f936fd0ad085`_ "fix(auth): avoid out-of-bounds read  |
+|                |   in auth_nvctr()"                                          |
+|                |                                                             |
+|                | Note that `72460f50e2437a85`_ is not fixing any             |
+|                | vulnerability per se but it is required for                 |
+|                | `f5c51855d36e399e`_ to apply cleanly.                       |
++----------------+-------------------------------------------------------------+
+| Credit         | Demi Marie Obenour, Invisible Things Lab                    |
++----------------+-------------------------------------------------------------+
+
+This security advisory describes a vulnerability in the X.509 parser used to
+parse boot certificates in TF-A trusted boot: it is possible for a crafted
+certificate to cause an out-of-bounds memory read.
+
+Note that upstream platforms are **not** affected by this. Only downstream
+platforms may be, if (and only if) the interfaces described below are used in a
+different context than seen in upstream code. Details of such context is
+described in the rest of this document.
+
+To fully understand this security advisory, it is recommended to refer to the
+following standards documents:
+
+ - `RFC 5280`_, *Internet X.509 Public Key Infrastructure Certificate and
+   Certificate Revocation List (CRL) Profile*.
+
+ - `ITU-T X.690`_, *ASN.1 encoding rules: Specification of Basic Encoding Rules
+   (BER), Canonical Encoding Rules (CER) and Distinguished Encoding Rules
+   (DER).*
+
+Bug 1: Insufficient certificate validation
+------------------------------------------
+
+The vulnerability lies in the following source file:
+``drivers/auth/mbedtls/mbedtls_x509_parser.c``. By design, ``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. It 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.  The result is that it is possible for ``get_ext()`` to read memory past
+the end of the certificate.  This could potentially access memory with dangerous
+read side effects, or leak microarchitectural state that could theoretically be
+retrieved through some side-channel attacks as part of a more complex attack.
+
+Bug 2: Missing bounds check in ``auth_nvctr()``
+-----------------------------------------------
+``auth_nvctr()`` does not check that the buffer provided is
+long enough to hold an ``ASN.1 INTEGER``.  Since ``auth_nvctr()`` will only ever
+read 6 bytes, it is possible to read up to 6 bytes past the end of the buffer.
+
+Exploitability Analysis
+-----------------------
+
+Upstream TF-A Code
+~~~~~~~~~~~~~~~~~~
+
+In upstream TF-A code, the only caller of ``auth_nvctr()`` takes its input from
+``get_ext()``, which means that the second bug is exploitable, so is the first.
+Therefore, only the first bug need be considered.
+
+All standard chains of trust provided in TF-A source tree (that is, under
+``drivers/auth/``) require that the certificate's signature has already been
+validated prior to calling ``get_ext()``, or any function that calls ``get_ext()``.
+Platforms taking their chain of trust from a dynamic configuration file (such as
+``fdts/cot_descriptors.dtsi``) are also safe, as signature verification will
+always be done prior to any calls to ``get_ext()`` or ``auth_nvctr()`` in this
+case, no matter the order of the properties in the file.  Therefore, it is not
+possible to exploit this vulnerability pre-authentication in upstream TF-A.
+
+Furthermore, the data read through ``get_ext()`` only
+ever gets used by the authentication framework (``drivers/auth/auth_mod.c``),
+which greatly reduces the range of inputs it will ever receive and thus the
+impact this has. Specifically, the authentication framework uses ``get_ext()``
+in three cases:
+
+ 1. Retrieving a hash from an X.509 certificate to check the integrity of a
+    child certificate (see ``auth_hash()``).
+
+ 2. Retrieving the signature details from an X.509 certificate to check its
+    authenticity and integrity (see ``auth_signature()``).
+
+ 3. Retrieving the security counter value from an X.509 certificate to protect
+    it from unauthorized rollback to a previous version (see ``auth_nvctr()``).
+
+None of these uses authentication framework write to the out-of-bounds memory,
+so no memory corruption is possible.
+
+In summary, there are 2 separate issues - one in ``get_ext()`` and another one
+in ``auth_nvctr()`` - but neither of these can be exploited in the context of
+TF-A upstream code.
+
+Only in the following 2 cases do we expect this vulnerability to be triggerable
+prior to authentication:
+
+ - The platform uses a custom chain of trust which uses the non-volatile counter
+   authentication method (``AUTH_METHOD_NV_CTR``) before the cryptographic
+   authentication method (``AUTH_METHOD_SIG``).
+
+ - The chain of trust uses a custom authentication method that calls
+   ``get_ext()`` before cryptographic authentication.
+
+Custom Image Parsers
+~~~~~~~~~~~~~~~~~~~~
+
+If the platform uses a custom image parser instead of the certificate parser,
+the bug in the certificate parser is obviously not relevant.  The bug in
+``auth_nvctr()`` *may* be relevant, but only if the returned data is:
+
+- Taken from an untrusted source (meaning that it is read prior to
+  authentication).
+
+- Not already checked to be a primitively-encoded ASN.1 tag.
+
+In particular, if the custom image parser implementation wraps a 32-bit integer
+in an ASN.1 ``INTEGER``, it is not affected.
+
+.. _CVE-2022-47630: http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-47630
+.. _fd37982a19a4a291: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=fd37982a19a4a291
+.. _72460f50e2437a85: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=72460f50e2437a85
+.. _f5c51855d36e399e: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=f5c51855d36e399e
+.. _abb8f936fd0ad085: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/?id=abb8f936fd0ad085
+.. _RFC 5280: https://www.ietf.org/rfc/rfc5280.txt
+.. _ITU-T X.690: https://www.itu.int/ITU-T/studygroups/com10/languages/X.690_1297.pdf