From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 3DB02C433F5 for ; Mon, 24 Jan 2022 17:18:06 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 91D2383049; Mon, 24 Jan 2022 18:18:04 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=gmx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; secure) header.d=gmx.net header.i=@gmx.net header.b="N84Xg2t6"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 5270482FA3; Mon, 24 Jan 2022 18:18:02 +0100 (CET) Received: from mout.gmx.net (mout.gmx.net [212.227.17.22]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 74BB983061 for ; Mon, 24 Jan 2022 18:17:57 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=xypron.glpk@gmx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1643044676; bh=dExizZ+YNwTsy67eacI5kbsnROZ+Zul6jmZIbDCSs6g=; h=X-UI-Sender-Class:Date:Subject:To:Cc:References:From:In-Reply-To; b=N84Xg2t6owUCLca01A9uafIi6HuXk4yW6CyNaPrSSkxaaeSMbvywa4/p6c65rydM4 OepyXZTqNSiKvZWu8CuDKY54pvfkwKN9QK5tDRqR3LdZPLyxLrpkeAINhqI85soTci iEishUxM/OvB3fiqxfEE5r+Q3JQjLztpH0+QpLO0= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from [192.168.123.55] ([88.152.144.107]) by mail.gmx.net (mrgmx104 [212.227.17.168]) with ESMTPSA (Nemesis) id 1MhD2Y-1mgXiL3Gl8-00eJ68; Mon, 24 Jan 2022 18:17:55 +0100 Message-ID: Date: Mon, 24 Jan 2022 18:17:55 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [RFC PATCH] efi_loader: clean up uefi secure boot image verification logic Content-Language: en-US To: Ilias Apalodimas , takahiro.akashi@linaro.org Cc: Ilias Apalodimas , u-boot@lists.denx.de References: <20220124153621.662350-1-ilias.apalodimas@linaro.org> From: Heinrich Schuchardt In-Reply-To: <20220124153621.662350-1-ilias.apalodimas@linaro.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:LsuA9rX+T7tymW3dxl5R99WpDbF7UmkhUKibNpQ8708fkEKl2gp RwHQWpjM6ql8qZD0etT48Q3HBO7oqD5WFfOHtfLbm9JcA8rWB2CLoBwgZrufoYDYPcwefOf JGcpOz5On8MLGrA1cEyjMQZYlb0a8M7W/jLpKFI8wd4gUTpZ+Js4iF1Nv48hPd0Fblxqi7X TO9M3J9t/VValGiZi9q8g== X-UI-Out-Filterresults: notjunk:1;V03:K0:fZXlQ5eVsaE=:oALbY1IAFwCnpawQz+7BMo LIxoBkptMwKlQ5FFkfT91SV5AVaTZUqKMv5/EB2xbtmYtEMrnAq79dyyRfn5QWG8QbjZld5MS PHQItPgPIlolAJlqnHWyF7OAwG3yK0CREyBs0SrjYn/qKo2LNCeaqV0pap4IXJ2PytrSHVwWM UtFAeKOe0cJI0NyVGUK/eMbyMn1ZGthmJ71x7pJ4iALUMEXW9/XflwXK7kVM9M47MtCpo+QTk NKFVZHh8ni6qIBahUi17Jp9E9jB7QNPa3KJ2XOhFVfjgXeXM6ul/q3CH6A2lkbK4M0WanKjCY kXMYGSssOMpQIYhsZCFND749dO87uwVJfoEspgjeIpYcQPUS+pztKLehe/ajEa6yhIIWvzYbU q+wzoO8QhEPQBGBtvPCTASnZ0GpfVoY1mdJcx65wJuCT1ijWGpY29Cix4oapBbDgFHLOLjVgN ENzngrkNW49lUNacijepGTgPfp8ix10HVDPzSZ4q+u9EWkkBzw1rEPvOsjdsIXSuHE9igFv1f srcnN7JrbXET7FRONQy2Ptxi7A0x8PVRhgYYZnhC8kw7NLSUtMX6u8Xsr+VGN1Dg7H29CuSN9 MDB1DQcNEWS2GNHsiMFdZRMKVTKZZ0FAoEe+L4rYdCAWmjnKZ1mufQuNf0umYpbo1bzAFJAfO SlyT1VR1gewJDR1/kf4kxhz/+pMalruft5LHcfSGEGvRpJ6fg79ASirqCCzZAxTFXzQwrcFsr vpLO5IkXPk2hJ5hg7GpVuwH/yyoOblsAJ3FejKxyBQHG5a5fvacRwycnE7hn3U6Ek7kb6KtMD KFY3OTwZPQZky6H58bD287owI2rz+kkDtJKqMhE6AJ9i99T5FM2BNWsQFr/hxXWqF/0DB2jab EHW4CqbDiVWJKdtAMHsfhQ6KiJ/LyQ6RSy+o1JQeo5stBBieYvg3XAT/UdjoYcf4ZMpzyYizz Po9DBLKZVByCJ8aHhf8ne7ImblagpDml17zlYnwe3On8oUFFmAXRUSK0pRRgSaK9ZcLCFJbxA aXkfSm9TOWwomQLAAP4JCbHVLpFSCqWsjYuLf6N7mrYK0NQsy/w3KuQkDo7MywWo1nnStibC0 +uFI6mFAz+8EL8= X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean On 1/24/22 16:36, Ilias Apalodimas wrote: > From: Ilias Apalodimas > > 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. > On sha256 hashes we don't really care if the image is signed or > not. The logic can be implemented in > 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 > So weed out the 'special' case 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 sha verification loop if a signature > happens to be added first. > > It's worth noting that (2) only works if the certificate is self > signed, but that canb be fixed in a following patch. > > Signed-off-by: Ilias Apalodimas > --- > lib/efi_loader/efi_image_loader.c | 84 ++++++------------------------- > lib/efi_loader/efi_signature.c | 2 +- > 2 files changed, 15 insertions(+), 71 deletions(-) > > diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_imag= e_loader.c > index 255613eb72ba..53d54ca42f5c 100644 > --- a/lib/efi_loader/efi_image_loader.c > +++ b/lib/efi_loader/efi_image_loader.c > @@ -516,63 +516,16 @@ 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 *r= egs) > -{ > - struct efi_signature_store *db =3D NULL, *dbx =3D NULL; > - bool ret =3D false; > - > - dbx =3D efi_sigstore_parse_sigdb(L"dbx"); > - if (!dbx) { > - EFI_PRINT("Getting signature database(dbx) failed\n"); > - goto out; > - } > - > - db =3D 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 =3D 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 a table of its PE heade= r. The signature must be stored in the "Certificate Table". See PE-COFF specification. The PE header only contains the Optional Header Data Directory that points to the table. Best regards Heinrich > * So if an image is signed and only if if its signature is verified u= sing > - * 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=3D=3D0, if the image's signature is not found in > * the authorized database, or is found in the forbidden database, > @@ -608,14 +561,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 =3D efi_image_unsigned_authenticate(regs); > - > - goto err; > + goto out; > } > > /* > @@ -624,18 +570,18 @@ static bool efi_image_authenticate(void *efi, size= _t efi_size) > db =3D efi_sigstore_parse_sigdb(L"db"); > if (!db) { > EFI_PRINT("Getting signature database(db) failed\n"); > - goto err; > + goto out; > } > > dbx =3D 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 +675,18 @@ static bool efi_image_authenticate(void *efi, size= _t efi_size) > /* try white-list */ > if (efi_signature_verify(regs, msg, db, dbx)) { > ret =3D true; > - break; > + goto out; > } > > EFI_PRINT("Signature was not verified by \"db\"\n"); > > - if (efi_signature_lookup_digest(regs, db)) { > - ret =3D true; > - break; > - } > - > - EFI_PRINT("Image's digest was not found in \"db\" or \"dbx\"\n"); > } > > -err: > + if (efi_signature_lookup_digest(regs, db)) > + ret =3D 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_signatu= re.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_re= gions *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)) {