[RFC PATCH] efi_loader: fix uefi secure boot with intermediate certs

[Heinrich Schuchardt]

On 2/14/22 10:14, Ilias Apalodimas wrote:
> The general rule of accepting or rejecting an image is
>   1. Is the sha256 of the image in dbx
>   2. Is the image signed with a certificate that's found in db and
>      not in dbx
>   3. 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
>   4. Is the sha256 of the image in db
>
> For example SHIM is signed by "CN=Microsoft Windows UEFI Driver Publisher",
> which is issued by "CN=Microsoft Corporation UEFI CA 2011", which in it's
> turn is issued by "CN=Microsoft Corporation Third Party Marketplace Root".
> The latter is a self-signed CA certificate and with our current implementation
> allows shim to execute if we insert it in db.
>
> However it's the CA cert in the middle of the chain which usually ends up
> in the system's db.  pkcs7_verify_one() might or might not return the root
> certificate for a given chain.  But when verifying executables in UEFI,  the
> trust anchor can be in the middle of the chain, as long as that certificate
> is present in db.  Currently we only allow this check on self-signed
> certificates,  so let's remove that check and allow all certs to try a
> match an entry in db.
>
> Open questions:
> - Does this break any aspect of variable authentication since
>    efi_signature_verify() is used on those as well?
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> ---
>   lib/efi_loader/efi_signature.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
> index 1bd1fdc95fce..79ed077ae7dd 100644
> --- a/lib/efi_loader/efi_signature.c
> +++ b/lib/efi_loader/efi_signature.c
> @@ -518,12 +518,11 @@ bool efi_signature_verify(struct efi_image_regions *regs,
>   			goto out;
>
>   		EFI_PRINT("Verifying last certificate in chain\n");
> -		if (signer->self_signed) {
> -			if (efi_lookup_certificate(signer, db))
> -				if (efi_signature_check_revocation(sinfo,
> -								   signer, dbx))
> -					break;
> -		} else if (efi_verify_certificate(signer, db, &root)) {
> +		if (efi_lookup_certificate(signer, db))

Why should we only check dbx if the certificate is in db? Shouldn't we
always check dbx?

Best regards

Heinrich

> +			if (efi_signature_check_revocation(sinfo, signer, dbx))
> +				break;
> +		if (!signer->self_signed &&
> +		    efi_verify_certificate(signer, db, &root)) {
>   			bool check;
>
>   			check = efi_signature_check_revocation(sinfo, root,