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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4BD1CC433EF for ; Mon, 25 Oct 2021 07:05:24 +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 6BAE660FD7 for ; Mon, 25 Oct 2021 07:05:23 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 6BAE660FD7 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 685CD832C4; Mon, 25 Oct 2021 09:05:21 +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="rNO3ycrH"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id C081C82DD7; Mon, 25 Oct 2021 09:05:18 +0200 (CEST) Received: from mail-io1-xd31.google.com (mail-io1-xd31.google.com [IPv6:2607:f8b0:4864:20::d31]) (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 4964983311 for ; Mon, 25 Oct 2021 09:05:09 +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=masami.hiramatsu@linaro.org Received: by mail-io1-xd31.google.com with SMTP id q6so10028909iod.7 for ; Mon, 25 Oct 2021 00:05:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=fx0CynzfSp8uwznYdlVyKdbQhiq6nhcL11QsXwTb2ms=; b=rNO3ycrHo6AwZZzDNnN6w1T+QuWk4ZM0hlgcbnXNOqHsZjp8CxBnjCa+VptdiKVGre QLPTZjEc2sdcuybkM+wL8ujPfWmcS2XLHSjM78t+WwzZJ5RBAhIsLf1UgQ2QFfbiuSFF zsur1aKXYmth77fBNORglhCAUXTbpe5+adYfLqE56fiIuozPNJdnw3WNAHFkcJAaOnF4 e78lbbMBGApypGx7xl5bTrAmlw+jc5xglkLiFNsVhB+YV0zJMtKb4x1JEEHlcyRYwlEl 8zqMttAkvCe3YLbaTrwUkUypz9KsX+5AYQHEsAk1OnDeWumSQ7JW0tdFtcW3azESmsUf OCww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=fx0CynzfSp8uwznYdlVyKdbQhiq6nhcL11QsXwTb2ms=; b=DPz2FiAS2b2CSq0d/bAFmlrG7Tt2YR0VZyKDAipthdAzpCTXoWgzKrl3YOMzaiNwl5 +/mXGYcwyESDaclveU4G+xAfk7skEl/TAb65lvTdrabw6FtMdilGUVdabUvfAXZyMuBl zoUxrSCl1PWSDtNFXNbzmzUlXks2SEiqkKHcxNOzShKnGJYe/cjLSSpn7Vpn3zYP1GOx Cc4ez+h+5FjD4q608fzf1HS/3QxX8YIW4DA1uB9hhcqh2FRxBjjrf4FF4Xo5q5IdqzP2 m+LY8KA0FNR96TJOTM7LgTdu+oOk0LwIv2VkqRbYnYtoqK9cTzlq4K3NpgNN/PIaqWq8 XiIg== X-Gm-Message-State: AOAM5335HSqiDpocNqsyBEyBvAVSw1Bg18pCWI0vI35nyqEjBtEXYDvX 3IKILhzfJ/XdWOqR77G9urfopZg4jGJXGIdgvAPuHQ== X-Google-Smtp-Source: ABdhPJzY/KDTNBU3n1llZ6lZ9ciXlk5AVxBQa/Z1aosR9boA5PjgEZiHJbSiinhjOBQ1ITLwoTABMTXgUc5hL3VpuQ4= X-Received: by 2002:a05:6638:4090:: with SMTP id m16mr9484649jam.147.1635145507906; Mon, 25 Oct 2021 00:05:07 -0700 (PDT) MIME-Version: 1.0 References: <20211007062340.72207-1-takahiro.akashi@linaro.org> <20211007062340.72207-4-takahiro.akashi@linaro.org> <20211025051434.GC44989@laputa> In-Reply-To: From: Masami Hiramatsu Date: Mon, 25 Oct 2021 16:04:56 +0900 Message-ID: Subject: Re: [PATCH v4 03/11] efi_loader: capsule: add back efi_get_public_key_data() To: =?UTF-8?Q?Fran=C3=A7ois_Ozog?= Cc: AKASHI Takahiro , Alex Graf , Heinrich Schuchardt , Ilias Apalodimas , Simon Glass , Sughosh Ganu , U-Boot Mailing List Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 Hi Francois, 2021=E5=B9=B410=E6=9C=8825=E6=97=A5(=E6=9C=88) 15:28 Fran=C3=A7ois Ozog : > > > > Le lun. 25 oct. 2021 =C3=A0 07:14, AKASHI Takahiro a =C3=A9crit : >> >> On Wed, Oct 20, 2021 at 07:39:37AM -0600, Simon Glass wrote: >> > Hi Masami, >> > >> > On Wed, 20 Oct 2021 at 02:18, Masami Hiramatsu >> > wrote: >> > > >> > > Hi Simon, >> > > >> > > 2021=E5=B9=B410=E6=9C=8815=E6=97=A5(=E9=87=91) 9:40 Simon Glass : >> > > > >> > > > Hi Takahiro, >> > > > >> > > > On Thu, 7 Oct 2021 at 00:25, AKASHI Takahiro wrote: >> > > > > >> > > > > The commit 47a25e81d35c ("Revert "efi_capsule: Move signature fr= om DTB to >> > > > > .rodata"") failed to revert the removal of efi_get_public_key_da= ta(). >> > > > > >> > > > > Add back this function and move it under lib/efi_loader so that = other >> > > > > platforms can utilize it. It is now declared as a weak function = so that >> > > > > it can be replaced with a platform-specific implementation. >> > > > > >> > > > > Fixes: 47a25e81d35c ("Revert "efi_capsule: Move signature from D= TB to >> > > > > .rodata"") >> > > > > Signed-off-by: AKASHI Takahiro >> > > > > --- >> > > > > lib/efi_loader/efi_capsule.c | 36 +++++++++++++++++++++++++++++= +++++++ >> > > > > 1 file changed, 36 insertions(+) >> > > > > >> > > > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_c= apsule.c >> > > > > index b75e4bcba1a9..44f5da61a9be 100644 >> > > > > --- a/lib/efi_loader/efi_capsule.c >> > > > > +++ b/lib/efi_loader/efi_capsule.c >> > > > > @@ -11,15 +11,20 @@ >> > > > > #include >> > > > > #include >> > > > > #include >> > > > > +#include >> > > > > +#include >> > > > > #include >> > > > > #include >> > > > > #include >> > > > > #include >> > > > > +#include >> > > > > >> > > > > #include >> > > > > #include >> > > > > #include >> > > > > >> > > > > +DECLARE_GLOBAL_DATA_PTR; >> > > > > + >> > > > > const efi_guid_t efi_guid_capsule_report =3D EFI_CAPSULE_REPORT= _GUID; >> > > > > static const efi_guid_t efi_guid_firmware_management_capsule_id= =3D >> > > > > EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID; >> > > > > @@ -251,6 +256,37 @@ out: >> > > > > } >> > > > > >> > > > > #if defined(CONFIG_EFI_CAPSULE_AUTHENTICATE) >> > > > > +int __weak efi_get_public_key_data(void **pkey, efi_uintn_t *pk= ey_len) >> > > > >> > > > I don't think this should be weak. What other way is there of hand= ling >> > > > this and why would it be platform-specific? >> > > >> > > I have a question about the current design of the capsule auth key. >> > > If the platform has its own key-storage, how can the platform use th= e >> > > platform specific storage? Does such platform load the key from the = storage >> > > and generate the dtb node in the platform initialization code? (or >> > > device driver?) >> > >> > Are we talking about a public key (which I assume from the function >> > naming) or some secret key. What is an auth key? >> >> Surely, a public key (more strictly, x509 certificate under the current >> implementation) >> >> > As I understand it, the public key should be provided by the platform >> > in devicetree that U-Boot uses, by whatever prior stage has access to >> > the key. >> >> I still believe that some platform provider may want to save the key >> in a *safer* storage, which should be at least read-only for non-authori= zed >> writers. > > > indeed. And U-Boot may not be entitled at all to check the capsule signat= ure. For example updating SCP firmware with a key that is in secure world a= nd will never leave this world. I think secure world firmware updates should be discussed in another thread, like with FWU. At this point, the capsule signature will be only authenticated by U-Boot, because we haven't passed the capsule image to the secure world yet. >> But if this issue (__weak or not) is the only blocking factor >> for my entire patch series, I'm willing to drop __weak for now since >> someone with production system may change it in the future when he has >> a good reason for that :) > > > If that need be=E2=80=A6. Agreed. Thank you, > >> >> >> -Takahiro Akashi >> >> >> > Regards, >> > Simon > > -- > Fran=C3=A7ois-Fr=C3=A9d=C3=A9ric Ozog | Director Business Development > T: +33.67221.6485 > francois.ozog@linaro.org | Skype: ffozog > -- Masami Hiramatsu