[PATCH] efi_loader: correctly handle mixed hashes and signatures in db

[Heinrich Schuchardt]

On 1/27/22 23:36, Ilias Apalodimas wrote:
> A mix of signatures and hashes in db doesn't always work as intended.
> Currently if the digest algorithm is not supported we stop walking the
> security database and reject the image.
> That's problematic in case we find and try to check a signature before
> inspecting the sha256 hash.  If the image is unsigned we will reject it
> even if the digest matches
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> ---
>   lib/efi_loader/efi_signature.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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);

According to the commit message siglist->sig_type could be
EFI_CERT_RSA2048_SHA256_GUID which does not relate to a 'Digest
algorithm'. So the debug message text is wrong. As we expect guidcmp()
to report a mismatch we could remove the message.

> -			break;
> +			continue;

This still is not correct:

dbx containing a signature type that we do not support must disable the
loading of any image.

The UEFI specification defines EFI_CERT_SHA1_GUID, EFI_CERT_SHA384_GUID
and EFI_CERT_SHA512_GUID. We should support all of these for dbx.

For security reasons we should not support EFI_CERT_SHA1_GUID for db.

The function lacks an argument indicating if we are dealing with db or
dbx which have to be treated differently.

>   		}
>
>   		if (!efi_hash_regions(regs->reg, regs->num, &hash, &size)) {

The subsequent code has a performance issue:

We should not hash the image once per entry in db but once per hash
algorithm.

Best regards

Heinrich