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 X-Spam-Level: X-Spam-Status: No, score=-15.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 284CBC07E95 for ; Tue, 20 Jul 2021 02:17:30 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 3F6B661029 for ; Tue, 20 Jul 2021 02:17:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3F6B661029 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id BEFE781BE8; Tue, 20 Jul 2021 04:17:25 +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="IYy9RGxE"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 95A5481BF4; Tue, 20 Jul 2021 04:17:23 +0200 (CEST) Received: from mail-pl1-x635.google.com (mail-pl1-x635.google.com [IPv6:2607:f8b0:4864:20::635]) (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 0540681BDA for ; Tue, 20 Jul 2021 04:17:20 +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-pl1-x635.google.com with SMTP id o8so10701517plg.11 for ; Mon, 19 Jul 2021 19:17:19 -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=0Sxn9+OJCjqxoSSbvfq+j4OP13UjZVu/1smSBnN0ihc=; b=IYy9RGxE3pGQ0bJDVHfDhlR3pc5X1HRACwL0UXv6uf0ZsS/PmvqImlZ5WdqhrQMiQr mSJ5Xu7viENqCSWJjMCRDfwDd6fTD88S44vbwHIIlB2wHNwYbRpmPkS25xaDT1KDF8Ps I/juMdS3LM4OZVrk8jLH6w1PhTdRoyGW5udrhkScw0mzEigW0N3SoYh3vM7tigvuCX/R caXMhCARLoGcCMxa+DysUbMstmGMs28h+ZS0pj1TWOnoI6KazfULMI9ioxiPSxIdrmve CRsB5gVlN/ZCC81qVEExxGC3m3koG6E+iWAWa84Uw8nQwebnp5H1WVwnwg4A4IyB0BWn KeZw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id :mail-followup-to:references:mime-version:content-disposition :in-reply-to; bh=0Sxn9+OJCjqxoSSbvfq+j4OP13UjZVu/1smSBnN0ihc=; b=eKL+Pc74FwE2Ri0mhyiMHH+Hg9/bTttOFvlpLso40p8QXZBGadnXBGSM5iVafjrxTK hujRi/CWuGHRh+0yBfyB52uEpXnS3d8LRI8razS9Fyfg5S5o9yq8Jaqr81inbz+lIM0+ usvE/XTnd7e4Hf9G4gI3lLXyvmTdvoZ0YuIxye7xKz/SndxMiOoX/as3y4vROonuhI7W QFIrgPxBJDjyRh1B4jzo6UVFaKu8PqAOAkntN8J2rq11P5VdORxcxaZVKVqHuPvSsmNE BrbZPMlKuBJXmpayusY7tq9SEFQtQU2pRh5Nkgm4oHS8ZfFu/k73LaVR/jjiuon2x/wO 90RA== X-Gm-Message-State: AOAM533VhR5J1E3HhnCYcAI1/j/ffSFMODc29+jPGDfO9HhJsfiC1bV9 9H2J3WNVpXvH5XgK5uaXZpMyKqVF0gPK0A== X-Google-Smtp-Source: ABdhPJySUoBsFImrtqIka7TmVmSvV/jzcj3+zFYgJDvrFs9Psx9OyxgErwTezxOwUKhPD5TArXmFXA== X-Received: by 2002:a17:90b:4b08:: with SMTP id lx8mr27797667pjb.66.1626747434859; Mon, 19 Jul 2021 19:17:14 -0700 (PDT) Received: from laputa (p784a236a.tkyea130.ap.so-net.ne.jp. [120.74.35.106]) by smtp.gmail.com with ESMTPSA id 73sm18278006pjz.24.2021.07.19.19.17.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 Jul 2021 19:17:14 -0700 (PDT) Date: Tue, 20 Jul 2021 11:17:06 +0900 From: AKASHI Takahiro To: Heinrich Schuchardt Cc: Ilias Apalodimas , Alexander Graf , Sughosh Ganu , U-Boot Mailing List Subject: Re: [PATCH] efi_loader: capsule: remove authentication data Message-ID: <20210720021706.GC77259@laputa> Mail-Followup-To: AKASHI Takahiro , Heinrich Schuchardt , Ilias Apalodimas , Alexander Graf , Sughosh Ganu , U-Boot Mailing List References: <20210510082034.44102-1-takahiro.akashi@linaro.org> <90167cde-4ba4-0ada-2f36-cf8b87cdd552@gmx.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <90167cde-4ba4-0ada-2f36-cf8b87cdd552@gmx.de> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 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.2 at phobos.denx.de X-Virus-Status: Clean On Thu, May 20, 2021 at 04:37:58AM +0200, Heinrich Schuchardt wrote: > On 5/10/21 10:20 AM, AKASHI Takahiro wrote: > > If capsule authentication is disabled and yet a capsule file is signed, > > its signature must be removed from image data to flush. > > Otherwise, the firmware will be corrupted after update. > > > > Fixes: 04be98bd6bcf ("efi: capsule: Add support for uefi capsule > > authentication") > > Signed-off-by: AKASHI Takahiro > > --- > > lib/efi_loader/efi_capsule.c | 70 +++++++++++++++++++++++++++++------- > > 1 file changed, 57 insertions(+), 13 deletions(-) > > > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c > > index b0dffd3ac9ce..5d156c730faa 100644 > > --- a/lib/efi_loader/efi_capsule.c > > +++ b/lib/efi_loader/efi_capsule.c > > @@ -206,6 +206,39 @@ skip: > > return NULL; > > } > > > > +/** > > + * efi_remove_auth_hdr - remove authentication data from image > > + * @image: Pointer to pointer to Image > > + * @image_size: Pointer to Image size > > + * > > + * Remove the authentication data from image if possible. > > + * Update @image and @image_size. > > + * > > + * Return: status code > > + */ > > +static efi_status_t efi_remove_auth_hdr(void **image, efi_uintn_t *image_size) > > +{ > > + struct efi_firmware_image_authentication *auth_hdr; > > + efi_status_t ret = EFI_INVALID_PARAMETER; > > + > > + auth_hdr = (struct efi_firmware_image_authentication *)*image; > > + if (*image_size < sizeof(*auth_hdr)) > > + goto out; > > + > > + if (auth_hdr->auth_info.hdr.dwLength <= > > + offsetof(struct win_certificate_uefi_guid, cert_data)) > > + goto out; > > + > > + *image = (uint8_t *)*image + sizeof(auth_hdr->monotonic_count) + > > + auth_hdr->auth_info.hdr.dwLength; > > + *image_size = *image_size - auth_hdr->auth_info.hdr.dwLength - > > + sizeof(auth_hdr->monotonic_count); > > + > > + ret = EFI_SUCCESS; > > +out: > > + return ret; > > +} > > + > > #if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE) > > > > #if defined(CONFIG_EFI_PKEY_DTB_EMBED) > > The string EFI_PKEY_DTB_EMBED does not yet exist in U-boot. The patch > seems to depend on Sughosh series "Add support for embedding public key > in platform's dtb". Please, state dependencies in future patches. > > In the discussion of Sughosh's patch series the conclusion was that > embedding the public key in the DTB is not preferable. > > I will mark this patch as not applicable. Now that Ilias posted a patch, I will respin my patch. -Takahiro Akashi > Best regards > > Heinrich > > > @@ -271,21 +304,15 @@ efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t capsule_s > > if (capsule == NULL || capsule_size == 0) > > goto out; > > > > - auth_hdr = (struct efi_firmware_image_authentication *)capsule; > > - if (capsule_size < sizeof(*auth_hdr)) > > - goto out; > > - > > - if (auth_hdr->auth_info.hdr.dwLength <= > > - offsetof(struct win_certificate_uefi_guid, cert_data)) > > + *image = (uint8_t *)capsule; > > + *image_size = capsule_size; > > + if (efi_remove_auth_hdr(image, image_size) != EFI_SUCCESS) > > goto out; > > > > + auth_hdr = (struct efi_firmware_image_authentication *)capsule; > > if (guidcmp(&auth_hdr->auth_info.cert_type, &efi_guid_cert_type_pkcs7)) > > goto out; > > > > - *image = (uint8_t *)capsule + sizeof(auth_hdr->monotonic_count) + > > - auth_hdr->auth_info.hdr.dwLength; > > - *image_size = capsule_size - auth_hdr->auth_info.hdr.dwLength - > > - sizeof(auth_hdr->monotonic_count); > > memcpy(&monotonic_count, &auth_hdr->monotonic_count, > > sizeof(monotonic_count)); > > > > @@ -367,7 +394,7 @@ static efi_status_t efi_capsule_update_firmware( > > { > > struct efi_firmware_management_capsule_header *capsule; > > struct efi_firmware_management_capsule_image_header *image; > > - size_t capsule_size; > > + size_t capsule_size, image_binary_size; > > void *image_binary, *vendor_code; > > efi_handle_t *handles; > > efi_uintn_t no_handles; > > @@ -429,13 +456,30 @@ static efi_status_t efi_capsule_update_firmware( > > } > > > > /* do update */ > > + if (IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE) && > > + !(image->image_capsule_support & > > + CAPSULE_SUPPORT_AUTHENTICATION)) { > > + /* no signature */ > > + ret = EFI_SECURITY_VIOLATION; > > + goto out; > > + } > > + > > image_binary = (void *)image + sizeof(*image); > > - vendor_code = image_binary + image->update_image_size; > > + image_binary_size = image->update_image_size; > > + vendor_code = image_binary + image_binary_size; > > + if (!IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE) && > > + (image->image_capsule_support & > > + CAPSULE_SUPPORT_AUTHENTICATION)) { > > + ret = efi_remove_auth_hdr(&image_binary, > > + &image_binary_size); > > + if (ret != EFI_SUCCESS) > > + goto out; > > + } > > > > abort_reason = NULL; > > ret = EFI_CALL(fmp->set_image(fmp, image->update_image_index, > > image_binary, > > - image->update_image_size, > > + image_binary_size, > > vendor_code, NULL, > > &abort_reason)); > > if (ret != EFI_SUCCESS) { > > >