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 873B3C433EF for ; Wed, 6 Jul 2022 01:46:54 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 816828451F; Wed, 6 Jul 2022 03:46:52 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="LyNGO63j"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 7A75684508; Wed, 6 Jul 2022 03:46:51 +0200 (CEST) Received: from mail-pf1-x42f.google.com (mail-pf1-x42f.google.com [IPv6:2607:f8b0:4864:20::42f]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 9D3F084528 for ; Wed, 6 Jul 2022 03:46:48 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=takahiro.akashi@linaro.org Received: by mail-pf1-x42f.google.com with SMTP id w185so9363941pfb.4 for ; Tue, 05 Jul 2022 18:46:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to; bh=WSF3AGq4BPjqueT2CIbhEC3+E3qMFj5r4ZJ7qHwemrI=; b=LyNGO63jtiIKMed4jdQVuoKPHm8C1G44BH5WEmk/XvtqjqQHDOZfw0mbPk43JbIOrB hLP9wsxEn7TE3zxtimBU0T2+7iOPY+HwXuFsHjOlyY4m6dy3UgA3UrJb00SrRqP4qXM6 nXKkxs+cKrNsqYo0qyRKqrWbmtVC9BST8WGkuiLrc82dhpmU4TkY/0e2KMeo/nhpy9Ht x16JfIxNlJ4mef7XoavslButR9RXu1ntnjqzupgLI+iYIbJJ6MqAtBquQHB7h/kjJr5f OmW3SthngCYLJZBJUbtm8wxCPyx/FDtAs7V8KMVB36v8blaiK5g7GcoJ9iiRZokJIM5A p8Gg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id :mail-followup-to:references:mime-version:content-disposition :in-reply-to; bh=WSF3AGq4BPjqueT2CIbhEC3+E3qMFj5r4ZJ7qHwemrI=; b=f5z0dvLVKQOqidrErtacnI7zKQYpaGHyNj6v5KjO9DwccTdKt0EBcHQjDbilSzsNIs v4xKWm6VwAYitBZXYytze2QR6JDEP6vTc1rYMJsPFqDHuqpgOP2V6cyrRte7//KfAtLE iLh48OUMZ49y/Zjmrqspn2Yp26XODlwUhCYmv/xfaQqrLoAoa0pa3SrJt+FV5YpxOn6G xdPUI3/g0qgoMH4Ubf+GnzToI5VdSfxC1GLbD99w621vz40TLJKwKUFTESS6G+a7VxFg 20GdclqFogdsSr+D3wBJy+Wfgl/N1Q+WOYVbeg/wo2TdaipPIZGv7qV9cnqzXn4dbOr9 FgUA== X-Gm-Message-State: AJIora8nmdNHbiafA9Prhgxt8uqJ0eX5qHyXxP+HOZbeoql8Ydtqq6Nm KpQyb0GW8i0QEFEVYFCbN5wt2Q== X-Google-Smtp-Source: AGRyM1sUKSRyBOu/bdf9O0sbac5DmcrTUMUvtoGawewGIWTTwQfp+ey4w5YWgFnHCmERwVZg6jJzOA== X-Received: by 2002:a63:2cd5:0:b0:411:3eca:da41 with SMTP id s204-20020a632cd5000000b004113ecada41mr30837252pgs.502.1657072006895; Tue, 05 Jul 2022 18:46:46 -0700 (PDT) Received: from laputa ([2400:4050:c3e1:100:a4a2:c8c3:a3dd:4eca]) by smtp.gmail.com with ESMTPSA id m7-20020a1709026bc700b0016b8b5ef703sm17059457plt.55.2022.07.05.18.46.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Jul 2022 18:46:46 -0700 (PDT) Date: Wed, 6 Jul 2022 10:46:42 +0900 From: AKASHI Takahiro To: Heinrich Schuchardt Cc: ilias.apalodimas@linaro.org, baocheng.su@siemens.com, jan.kiszka@siemens.com, u-boot@lists.denx.de Subject: Re: [PATCH 4/5] efi_loader: image_loader: add a missing digest verification for signed PE image Message-ID: <20220706014642.GD42673@laputa> Mail-Followup-To: AKASHI Takahiro , Heinrich Schuchardt , ilias.apalodimas@linaro.org, baocheng.su@siemens.com, jan.kiszka@siemens.com, u-boot@lists.denx.de References: <20220705054815.30318-1-takahiro.akashi@linaro.org> <20220705054815.30318-5-takahiro.akashi@linaro.org> <0e843651-d3ac-f012-c24e-9296ca3e2542@gmx.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0e843651-d3ac-f012-c24e-9296ca3e2542@gmx.de> 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.6 at phobos.denx.de X-Virus-Status: Clean Heinrich, On Tue, Jul 05, 2022 at 05:29:07PM +0200, Heinrich Schuchardt wrote: > On 7/5/22 07:48, AKASHI Takahiro wrote: > > At the last step of PE image authentication, an image's hash value must be > > compared with a message digest stored as the content (of SpcPeImageData type) > > of pkcs7's contentInfo. > > > > Fixes: commit 4540dabdcaca ("efi_loader: image_loader: support image authentication") > > Signed-off-by: AKASHI Takahiro > > --- > > lib/efi_loader/Kconfig | 1 + > > lib/efi_loader/efi_image_loader.c | 62 ++++++++++++++++++++++++++++++- > > 2 files changed, 61 insertions(+), 2 deletions(-) > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index e2a1a5a69a24..e3f2402d0e8e 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -366,6 +366,7 @@ config EFI_SECURE_BOOT > > select X509_CERTIFICATE_PARSER > > select PKCS7_MESSAGE_PARSER > > select PKCS7_VERIFY > > + select MSCODE_PARSER > > select EFI_SIGNATURE_SUPPORT > > help > > Select this option to enable EFI secure boot support. > > diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c > > index fe8e4a89082c..eaf75a5803d4 100644 > > --- a/lib/efi_loader/efi_image_loader.c > > +++ b/lib/efi_loader/efi_image_loader.c > > @@ -16,6 +16,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > > > @@ -516,6 +517,51 @@ err: > > } > > > > #ifdef CONFIG_EFI_SECURE_BOOT > > +/** > > + * efi_image_verify_digest - verify image's message digest > > + * @regs: Array of memory regions to digest > > + * @msg: Signature in pkcs7 structure > > + * > > + * @regs contains all the data in a PE image to digest. Calculate > > + * a hash value based on @regs and compare it with a messaged digest > > + * in the content (SpcPeImageData) of @msg's contentInfo. > > + * > > + * Return: true if verified, false if not > > + */ > > +static bool efi_image_verify_digest(struct efi_image_regions *regs, > > + struct pkcs7_message *msg) > > +{ > > + struct pefile_context ctx; > > + void *hash; > > + int hash_len, ret; > > + > > + const void *data; > > + size_t data_len; > > + size_t asn1hdrlen; > > + > > + /* get pkcs7's contentInfo */ > > + ret = pkcs7_get_content_data(msg, &data, &data_len, &asn1hdrlen); > > + if (ret < 0 || !data) > > + return false; > > + > > + /* parse data and retrieve a message digest into ctx */ > > + ret = mscode_parse(&ctx, data, data_len, asn1hdrlen); > > + if (ret < 0) > > + return false; > > + > > + /* calculate a hash value of PE image */ > > + hash = NULL; > > + if (!efi_hash_regions(regs->reg, regs->num, &hash, ctx.digest_algo, > > + &hash_len)) > > + return false; > > + > > + /* match the digest */ > > + if (ctx.digest_len != hash_len || memcmp(ctx.digest, hash, hash_len)) > > + return false; > > + > > + return true; > > +} > > + > > /** > > * efi_image_authenticate() - verify a signature of signed image > > * @efi: Pointer to image > > @@ -645,6 +691,9 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) > > } > > > > /* > > + * verify signatures in pkcs7's signedInfos which are > > + * to authenticate the integrity of pkcs7's contentInfo. > > + * > > * NOTE: > > * UEFI specification defines two signature types possible > > * in signature database: > > @@ -677,12 +726,21 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) > > } > > > > /* try white-list */ > > - if (efi_signature_verify(regs, msg, db, dbx)) { > > + if (!efi_signature_verify(regs, msg, db, dbx)) { > > + log_debug("Signature was not verified by \"db\"\n"); > > Shouldn't this be log_err()? I think that I have already explained my idea behind here in my previous reply. > > + continue; > > + } > > + > > + /* > > + * now calculate an image's hash value and compare it with > > + * a messaged digest embedded in pkcs7's contentInfo > > + */ > > + if (efi_image_verify_digest(regs, msg)) { > > ret = true; > > continue; > > } > > > > - log_debug("Signature was not verified by \"db\"\n"); > > ditto? > > Or does the caller write an error message somewhere? Yes: efi_load_pe() ... /* Authenticate an image */ if (efi_image_authenticate(efi, efi_size)) { handle->auth_status = EFI_IMAGE_AUTH_PASSED; } else { handle->auth_status = EFI_IMAGE_AUTH_FAILED; log_err("Image not authenticated\n"); } -Takahiro Akashi > Best regards > > Heinrich > > > + log_debug("Message digest doesn't match\n"); > > } > > > > >