Re: [RFC PATCH 1/2] efi_loader: fix dual signed image certification

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

> > > >   		msg = pkcs7_parse_message(auth, auth_size);

[...]

> > > > @@ -717,32 +665,32 @@ static bool efi_image_authenticate(void *efi, size_t efi_size)
> > > >   		 */
> > > >   		/* try black-list first */
> > > >   		if (efi_signature_verify_one(regs, msg, dbx)) {
> > > > +			ret = false;
> > > >   			EFI_PRINT("Signature was rejected by \"dbx\"\n");
> > > > -			continue;
> > > > +			goto out;
> > > 
> > > If we go to "out" here, we have no chance to verify some cases:
> > > 1) An image has two signatures, for instance, one signed by SHA1 cert
> > >     and the other signed by SHA256 cert. A user wants to reject SHA1 cert
> > >     and put the cert in dbx.
> > 
> > I am not sure I am following,  what does he gain be rejecting the SHA1
> > portion only?  Avoid potential collisions?
> 
> If an image has a SHA1 and a SHA256 signature attached and SHA1 *or*
> SHA256 is in dbx, we must reject the image. Don't expect a dbx entry for
> each of the hashes. - But isn't this what your are doing here: for all
> signatures of the image look for one hit in dbx?
> 

Yes exactly. Any match on dbx of any certificate or sha256 of a certificate
or a sha256 of the executable will reject the image.

Regards
/Ilias
> Best regards
> 
> Heinrich
> 
> > 
> > >     But this image can and should yet be verified by SHA256 cert.
> > 
> > Why should it be verified?  My understanding of the EFI spec is that any match
> > in dbx of any certificate in the signing chain of the signature being verified means
> > reject the image.
> > 
> > > 2) A user knows that a given image is safe for some reason even though
> > >     he or she doesn't trust the certficate which is used for signing
> > >     the image.
> > > 
> > > -Takahiro Akashi
> > > 
> > > >   		}
> > > > 
> > > >   		if (!efi_signature_check_signers(msg, dbx)) {
> > > > +			ret = false;
> > > >   			EFI_PRINT("Signer(s) in \"dbx\"\n");
> > > > -			continue;
> > > > +			goto out;
> > > >   		}
> > > > 
> > > >   		/* try white-list */
> > > >   		if (efi_signature_verify(regs, msg, db, dbx)) {
> > > >   			ret = true;
> > > > -			break;
> > > > +			continue;
> > > >   		}
> > > > 
> > > >   		EFI_PRINT("Signature was not verified by \"db\"\n");
> > > > +	}
> > > > 
> > > > -		if (efi_signature_lookup_digest(regs, db, false)) {
> > > > -			ret = true;
> > > > -			break;
> > > > -		}
> > > > 
> > > > -		EFI_PRINT("Image's digest was not found in \"db\" or \"dbx\"\n");
> > > > -	}
> > > > +	/* last resort try the image sha256 hash in db */
> > > > +	if (!ret && efi_signature_lookup_digest(regs, db, false))
> > > > +		ret = true;
> > > > 
> > > > -err:
> > > > +out:
> > > >   	efi_sigstore_free(db);
> > > >   	efi_sigstore_free(dbx);
> > > >   	pkcs7_free_message(msg);
> > > > --
> > > > 2.32.0
> > > > 
> > 
> > Thanks
> > /Ilias
>