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 0A56AC32771 for ; Wed, 28 Sep 2022 07:30:31 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id CB35C84594; Wed, 28 Sep 2022 09:30:29 +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="Mskwe2x1"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id A2FC8836C5; Wed, 28 Sep 2022 09:30:28 +0200 (CEST) Received: from mail-ed1-x52e.google.com (mail-ed1-x52e.google.com [IPv6:2a00:1450:4864:20::52e]) (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 942F0849E9 for ; Wed, 28 Sep 2022 09:30: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=etienne.carriere@linaro.org Received: by mail-ed1-x52e.google.com with SMTP id z13so16044226edb.13 for ; Wed, 28 Sep 2022 00:30:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=mA9rQR/oadkT+B+yd1VNHbaTRlnRlYSAk8XDpcSY5rQ=; b=Mskwe2x1aRGwf3xmK65ij3dqxGuJ4oWAUT3ZcT26pwWHL7+JekOkyiShB95+CrYUAC m+2+YbnqsQRzhbP5EUmRgAHmedNcxeh1vKwOqpY2N5BZs3QchAXM6hUFJCk0r1oIdEkb nilj4F/qO/aX6RlHaeq+rK+iu+VLjmogaDV2ZVLc4kqROZF5EsyjOFayOQ19yPhLvNXR cxKYY/qOcnevFPgcJ+RqJTK/QIYOnBc+ZEIHxEFB67k0X+OAkoJHx+vYwW32ooJf40mK 9qtgABLi7UP20V4FlGZ2vejY4PeLFIVmAY0wIqCBqV2lC+PFzGw80Rco3rV0tH2Gm9N7 JJVQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=mA9rQR/oadkT+B+yd1VNHbaTRlnRlYSAk8XDpcSY5rQ=; b=5A8Swqf9XlNLdkgbpxIyPyGhXPOHTcg+mNCvfifqsnr1VlF98dFSTBJwoSwV7JCudQ V/JVaQW7hc0BV4c0EyiBBbs0H8mBULNnTmtxoaKPAzPVB+tJJWSrLOt3FHUlnTcHu/g5 /U0edtNKkCaoLmPWVaTtrZemVedU71fGd59udPn0WGjCs0B7nJozmfWDa25mEJTz3ulF 29wG520ZxASCCEX/QVyfwOuVavcwr2IBCPkbBeesbpzqOGunk0qk6foTVUOKV+vkVZwY 4YnvKHYk+UN6Z3WMC5CoXHa6LJSmz//BN6v0/NR6aL9zu6XCEa/fV0OqdJ522tXeHgNc wJ8w== X-Gm-Message-State: ACrzQf3aPKbwBbn9LSBNlZ+5J+zwgmBuv3DOloXFEalzLwvnO0pbZQ23 8YFgUNRyrzz0cJu1bZ/kyw5SDrRtvOQzpmc07eNB/g== X-Google-Smtp-Source: AMsMyM6ZlQ8m+Sg9pgSm6ia0xDW41QZYJODlc8ZR1pnkWiQb414YA11UqV7lb6EzpnkCC+Yr8UlhXKEG+iTnkXQTCM0= X-Received: by 2002:a50:ff13:0:b0:43e:76d3:63e1 with SMTP id a19-20020a50ff13000000b0043e76d363e1mr31350691edu.271.1664350225098; Wed, 28 Sep 2022 00:30:25 -0700 (PDT) MIME-Version: 1.0 References: <20220915081451.633983-1-sughosh.ganu@linaro.org> <20220915081451.633983-11-sughosh.ganu@linaro.org> In-Reply-To: From: Etienne Carriere Date: Wed, 28 Sep 2022 09:30:13 +0200 Message-ID: Subject: Re: [PATCH v10 10/15] FWU: Add support for the FWU Multi Bank Update feature To: Sughosh Ganu Cc: Jassi Brar , u-boot@lists.denx.de, Heinrich Schuchardt , Ilias Apalodimas , Takahiro Akashi , Patrick Delaunay , Patrice Chotard , Simon Glass , Bin Meng , Tom Rini , Michal Simek , Jassi Brar Content-Type: text/plain; charset="UTF-8" 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 Hello Jassi, Sughosh and all, On Wed, 28 Sept 2022 at 08:23, Sughosh Ganu wrote: > > On Tue, 27 Sept 2022 at 22:19, Jassi Brar wrote: > > > > On Tue, Sep 27, 2022 at 2:22 AM Sughosh Ganu wrote: > > > > > > On Mon, 26 Sept 2022 at 20:24, Jassi Brar wrote: > > > > > > > > On Mon, Sep 26, 2022 at 4:01 AM Sughosh Ganu wrote: > > > > > On Mon, 26 Sept 2022 at 08:25, Jassi Brar wrote: > > > > > > > > > > ..... > > > > > > > > > > > > > > +static __maybe_unused efi_status_t fwu_post_update_process(bool fw_accept_os) > > > > > > > +{ > > > > > > > + int status; > > > > > > > + u32 update_index; > > > > > > > + efi_status_t ret; > > > > > > > + > > > > > > > + status = fwu_plat_get_update_index(&update_index); > > > > > > > + if (status < 0) { > > > > > > > + log_err("Failed to get the FWU update_index value\n"); > > > > > > > + return EFI_DEVICE_ERROR; > > > > > > > + } > > > > > > > + > > > > > > > + /* > > > > > > > + * All the capsules have been updated successfully, > > > > > > > + * update the FWU metadata. > > > > > > > + */ > > > > > > > + log_debug("Update Complete. Now updating active_index to %u\n", > > > > > > > + update_index); > > > > > > > + status = fwu_update_active_index(update_index); > > > > > > > > > > > > > Do we want to check if all images in the bank are updated via capsules > > > > > > before switching the bank? > > > > > > > > > > This function does get called only when the update status for every > > > > > capsule is a success. Even if one of the capsules does not get > > > > > updated, the active index will not get updated. > > > > > > > > > .... but we don't check if the capsule for each image in the bank is > > > > provided for update. > > > > > > Yes, we have had this discussion earlier. Neither the FWU spec, nor > > > the capsule update spec in UEFI puts that restriction that all images > > > on the platform need to be updated. If a user wants to ensure such a > > > behaviour, they may choose some kind of image packaging like FIP or > > > FIT which would mean that all the images on the platform are being > > > updated. But this is not something to be ensured by the FWU update > > > code. > > > > > > > > > > > > > > > > > > > A developer will make sure all images are provided in one go, so that > > > > > > the switch is successful. > > > > > > But a malicious user may force some old vulnerable image back into use > > > > > > by updating all but that image. > > > > > > > > > > That I believe is to be handled through a combination of implementing > > > > > a rollback protection mechanism, along with capsule authentication. > > > > > These are separate to the implementation of the multi bank updates > > > > > that these patches are aiming for. > > > > > > > > > This sounds like : we don't worry about buffer-overflow > > > > vulnerabilities because the system will be secured and hardened by > > > > other mechanisms. > > > > > > Not sure how this is related. The aim of the FWU spec is for providing > > > a means for a platform to maintain multiple partitions of images and > > > to specify the metadata structure for the bookkeeping of the different > > > partitions and images on those partitions. If we need a more secure > > > and hardened system, we do have the Dependable Boot spec, which is > > > talking precisely about these things. Those are indeed separate > > > features or aspects from what the FWU spec is talking about. And this > > > patchset is implementing the FWU spec. > > > > > > > I am out of ways to explain. Best of luck. > > Let me try one last time. If you still do not agree with what I say, > let us agree to disagree :) > > You mentioned earlier that a malicious user may force some old > vulnerable image back into use by updating all but that image. Now a > few points to consider here. The FWU spec recommends that platforms > follow the Platform Security Boot Guide [1] which specifies the > trusted boot flow for platforms where the firmware images being booted > on the platform, including the NWd bootloader(BL33 in tf-a jargon) are > verified before getting booted. Let me add that in tf-a we can bind booted images (actually booted certificates) version id to a platform non-volatile monotonic counter. This adds rollback protection on booted images upgrades. Enabling firmware update, we need this rollback protection to prevent cases like you mentioned above: >>> But a malicious user may force some old vulnerable image back into use >>> by updating all but that image. When the system boots with accepted images (referring to fwu-mdata regular/trial state), the platform monotonic counter is updated against booted image version number if needed, preventing older images to be booted when an accepted image has been deployed. @Jassi, does this answer your question? > So, if the platform is booting with > the trusted boot flow, the above scenario should not occur. Secondly, > at the time of the update, the capsule can be authenticated before > getting written to the storage. This again should be able to identify > a malicious image getting written to the device. So this hypothesis of > a malicious image being present should not occur with authenticated > capsules and a trusted boot flow. Authenticating capsules rather relates to reducing the attack surface IMHO. I don't mean it is useless, far from it. In our system, authenticated boot is the boot guard in the chain of trust. With firmware update, the guard must prevent firmware version rollback. > And lastly, if a supposedly good > image is found to have some vulnerability because of which an image > with a fix should be installed and the vulnerable image not allowed to > boot, this is precisely what we have rollback protection for. > > You also mentioned that a platform might not support these > functionalities. Well, if these are not being supported, we don't have > much of a safety net here. Moreover, one more point to think here is > that if it is possible for the user to install and boot a vulnerable > image on the platform in the first place, it will not be protected by > updating all the images -- the malicious user can still put a corrupt > image as part of the update image bundle. The protection against > booting a corrupt image or a buggy image has to come through > mechanisms like trusted boot and rollback protection. "rollback protection" here should also cover tracking of updatable images version ID. Best regards, Etienne > > -sughosh > > [1] - https://developer.arm.com/documentation/den0072/0101/?lang=en