[PATCH 02/13] efi_loader: image_loader: add a check against certificate type of authenticode

AKASHI Takahiro
2020-05-29T06:41:19Z

UEFI specification requires that we shall support three type of
certificates of authenticode in PE image:
WIN_CERT_TYPE_EFI_GUID with the guid, EFI_CERT_TYPE_PCKS7_GUID
WIN_CERT_TYPE_PKCS_SIGNED_DATA
WIN_CERT_TYPE_EFI_PKCS1_15

As EDK2 does, we will support the first two that are pkcs7 SignedData.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
lib/efi_loader/efi_image_loader.c | 55 ++++++++++++++++++++++++-------
1 file changed, 43 insertions(+), 12 deletions(-)

diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
index 7f35b1e58fe5..a13ba3692ec2 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -375,7 +375,7 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,
}

/* Return Certificates Table */
- if (authsz) {
+ if (authsz > 0) {
if (len < authoff + authsz) {
debug("%s: Size for auth too large: %u >= %zu\n",
__func__, authsz, len - authoff);
@@ -480,8 +480,8 @@ static bool efi_image_authenticate(void *efi, size_t efi_size)
struct pkcs7_message *msg = NULL;
struct efi_signature_store *db = NULL, *dbx = NULL;
struct x509_certificate *cert = NULL;
- void *new_efi = NULL;
- size_t new_efi_size;
+ void *new_efi = NULL, *auth, *wincerts_end;
+ size_t new_efi_size, auth_size;
bool ret = false;

if (!efi_secure_boot_enabled())
@@ -530,20 +530,51 @@ static bool efi_image_authenticate(void *efi, size_t efi_size)
}

/* go through WIN_CERTIFICATE list */
- for (wincert = wincerts;
- (void *)wincert < (void *)wincerts + wincerts_len;
+ for (wincert = wincerts, wincerts_end = (void *)wincerts + wincerts_len;
+ (void *)wincert < wincerts_end;
wincert = (void *)wincert + ALIGN(wincert->dwLength, 8)) {
- if (wincert->dwLength < sizeof(*wincert)) {
- debug("%s: dwLength too small: %u < %zu\n",
- __func__, wincert->dwLength, sizeof(*wincert));
- goto err;
+ if ((void *)wincert + sizeof(*wincert) >= wincerts_end)
+ break;
+
+ if (wincert->dwLength <= sizeof(*wincert)) {
+ debug("dwLength too small: %u < %zu\n",
+ wincert->dwLength, sizeof(*wincert));
+ continue;
}
- msg = pkcs7_parse_message((void *)wincert + sizeof(*wincert),
- wincert->dwLength - sizeof(*wincert));
+
+ debug("WIN_CERTIFICATE_TYPE: 0x%x\n",
+ wincert->wCertificateType);
+
+ auth = (void *)wincert + sizeof(*wincert);
+ auth_size = wincert->dwLength - sizeof(*wincert);
+ if (wincert->wCertificateType == WIN_CERT_TYPE_EFI_GUID) {
+ if (auth + sizeof(efi_guid_t) >= wincerts_end)
+ break;
+
+ if (auth_size <= sizeof(efi_guid_t)) {
+ debug("dwLength too small: %u < %zu\n",
+ wincert->dwLength, sizeof(*wincert));
+ continue;
+ }
+ if (guidcmp(auth, &efi_guid_cert_type_pkcs7)) {
+ debug("Certificate type not supported: %pUl\n",
+ auth);
+ continue;
+ }
+
+ auth += sizeof(efi_guid_t);
+ auth_size -= sizeof(efi_guid_t);
+ } else if (wincert->wCertificateType
+ != WIN_CERT_TYPE_PKCS_SIGNED_DATA) {
+ debug("Certificate type not supported\n");
+ continue;
+ }
+
+ msg = pkcs7_parse_message(auth, auth_size);
if (IS_ERR(msg)) {
debug("Parsing image's signature failed\n");
msg = NULL;
- goto err;
+ continue;
}

/* try black-list first */
--
2.25.2