[PATCH 1/1] efi_loader: clean up uefi secure boot image verification logic
Ilias Apalodimas
ilias.apalodimas at linaro.org
Thu Jan 27 22:47:17 CET 2022
Heinrich,
On Tue, 25 Jan 2022 at 16:46, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> We currently distinguish between signed and non signed PE/COFF executables
> while trying to authenticate signatures and/or sha256 hashes in db and dbx.
> That code duplication can be avoided. When checking for sha256 hashes,
> in db, we don't really care if the image is signed or not. The sequence
> for verifying the binary can be
> 1. Is the sha256 of the image in dbx, reject the image
> 2. Certification checking
> 2a. Is the image signed with a certificate that's found in db and
> not in dbx
> 2b. The image carries a cert which is signed by a cert in db (and
> not in dbx) and the image can be verified against the former
> 3. Is the sha256 of the image in db
>
> So weed out the 'special' case, which is handling unsigned images.
>
> While at it move the checking of a hash outside the certificate
> verification loop which isn't really needed and check for the hash
> only once. Also allow a mix of signatures and hashes in db instead
> of explicitly breaking out the SHA256 verification loop if a signature
> happens to be the present before the hash.
>
> It's worth noting that (2a) only works if the certificate is self
> signed, but that can be fixed in a following patch.
I've found more unsupported cases in our existing code while testing.
Please ignore this patchset I'll send updates which supersede it
Thanks
/Ilias
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> ---
> changes since RFC:
> - added a comment indication sha256 approval is the last resort case
> - reworded the commit message, based on Takahiro's proposal
>
> lib/efi_loader/efi_image_loader.c | 89 +++++++------------------------
> lib/efi_loader/efi_signature.c | 2 +-
> 2 files changed, 20 insertions(+), 71 deletions(-)
>
> diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c
> index 255613eb72ba..90594acf9623 100644
> --- a/lib/efi_loader/efi_image_loader.c
> +++ b/lib/efi_loader/efi_image_loader.c
> @@ -516,63 +516,17 @@ err:
> }
>
> #ifdef CONFIG_EFI_SECURE_BOOT
> -/**
> - * efi_image_unsigned_authenticate() - authenticate unsigned image with
> - * SHA256 hash
> - * @regs: List of regions to be verified
> - *
> - * If an image is not signed, it doesn't have a signature. In this case,
> - * its message digest is calculated and it will be compared with one of
> - * hash values stored in signature databases.
> - *
> - * Return: true if authenticated, false if not
> - */
> -static bool efi_image_unsigned_authenticate(struct efi_image_regions *regs)
> -{
> - struct efi_signature_store *db = NULL, *dbx = NULL;
> - bool ret = false;
> -
> - dbx = efi_sigstore_parse_sigdb(L"dbx");
> - if (!dbx) {
> - EFI_PRINT("Getting signature database(dbx) failed\n");
> - goto out;
> - }
> -
> - db = efi_sigstore_parse_sigdb(L"db");
> - if (!db) {
> - EFI_PRINT("Getting signature database(db) failed\n");
> - goto out;
> - }
> -
> - /* try black-list first */
> - if (efi_signature_lookup_digest(regs, dbx)) {
> - EFI_PRINT("Image is not signed and its digest found in \"dbx\"\n");
> - goto out;
> - }
> -
> - /* try white-list */
> - if (efi_signature_lookup_digest(regs, db))
> - ret = true;
> - else
> - EFI_PRINT("Image is not signed and its digest not found in \"db\" or \"dbx\"\n");
> -
> -out:
> - efi_sigstore_free(db);
> - efi_sigstore_free(dbx);
> -
> - return ret;
> -}
> -
> /**
> * efi_image_authenticate() - verify a signature of signed image
> * @efi: Pointer to image
> * @efi_size: Size of @efi
> *
> - * A signed image should have its signature stored in a table of its PE header.
> + * An image should have its signature stored in the image certificate table
> + *
> * So if an image is signed and only if if its signature is verified using
> - * signature databases, an image is authenticated.
> - * If an image is not signed, its validity is checked by using
> - * efi_image_unsigned_authenticated().
> + * signature databases or if it's sha256 is found in db an image is
> + * authenticated.
> + *
> * TODO:
> * When AuditMode==0, if the image's signature is not found in
> * the authorized database, or is found in the forbidden database,
> @@ -608,14 +562,7 @@ static bool efi_image_authenticate(void *efi, size_t efi_size)
> if (!efi_image_parse(new_efi, efi_size, ®s, &wincerts,
> &wincerts_len)) {
> EFI_PRINT("Parsing PE executable image failed\n");
> - goto err;
> - }
> -
> - if (!wincerts) {
> - /* The image is not signed */
> - ret = efi_image_unsigned_authenticate(regs);
> -
> - goto err;
> + goto out;
> }
>
> /*
> @@ -624,18 +571,18 @@ static bool efi_image_authenticate(void *efi, size_t efi_size)
> db = efi_sigstore_parse_sigdb(L"db");
> if (!db) {
> EFI_PRINT("Getting signature database(db) failed\n");
> - goto err;
> + goto out;
> }
>
> dbx = efi_sigstore_parse_sigdb(L"dbx");
> if (!dbx) {
> EFI_PRINT("Getting signature database(dbx) failed\n");
> - goto err;
> + goto out;
> }
>
> if (efi_signature_lookup_digest(regs, dbx)) {
> EFI_PRINT("Image's digest was found in \"dbx\"\n");
> - goto err;
> + goto out;
> }
>
> /*
> @@ -729,20 +676,22 @@ static bool efi_image_authenticate(void *efi, size_t efi_size)
> /* try white-list */
> if (efi_signature_verify(regs, msg, db, dbx)) {
> ret = true;
> - break;
> + goto out;
> }
>
> EFI_PRINT("Signature was not verified by \"db\"\n");
>
> - if (efi_signature_lookup_digest(regs, db)) {
> - ret = true;
> - break;
> - }
> -
> - EFI_PRINT("Image's digest was not found in \"db\" or \"dbx\"\n");
> }
>
> -err:
> + /*
> + * This our last resort, the image is not found in dbx and is not
> + * authenticated by any certs in db. Try the image hash in db
> + */
> + if (efi_signature_lookup_digest(regs, db))
> + ret = true;
> + else
> + EFI_PRINT("Image's digest was not found in \"db\" or \"dbx\"\n");
> +out:
> efi_sigstore_free(db);
> efi_sigstore_free(dbx);
> pkcs7_free_message(msg);
> diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
> index 3243e2c60de0..8fa82f8cea4c 100644
> --- a/lib/efi_loader/efi_signature.c
> +++ b/lib/efi_loader/efi_signature.c
> @@ -176,7 +176,7 @@ bool efi_signature_lookup_digest(struct efi_image_regions *regs,
> if (guidcmp(&siglist->sig_type, &efi_guid_sha256)) {
> EFI_PRINT("Digest algorithm is not supported: %pUs\n",
> &siglist->sig_type);
> - break;
> + continue;
> }
>
> if (!efi_hash_regions(regs->reg, regs->num, &hash, &size)) {
> --
> 2.30.2
>
More information about the U-Boot
mailing list