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 648E0C433EF for ; Sun, 2 Jan 2022 21:27:59 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id B630582FB5; Sun, 2 Jan 2022 22:27:56 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=xs4all.nl Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id D111583049; Sun, 2 Jan 2022 22:27:55 +0100 (CET) Received: from sibelius.xs4all.nl (sibelius.xs4all.nl [83.163.83.176]) (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 E75B982F5E for ; Sun, 2 Jan 2022 22:27:52 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=xs4all.nl Authentication-Results: phobos.denx.de; spf=fail smtp.mailfrom=mark.kettenis@xs4all.nl Received: from localhost (bloch.sibelius.xs4all.nl [local]) by bloch.sibelius.xs4all.nl (OpenSMTPD) with ESMTPA id 790962ea; Sun, 2 Jan 2022 22:27:50 +0100 (CET) Date: Sun, 2 Jan 2022 22:27:50 +0100 (CET) From: Mark Kettenis To: Heinrich Schuchardt Cc: ilias.apalodimas@linaro.org, ardb@kernel.org, agraf@csgraf.de, u-boot@lists.denx.de In-Reply-To: <16F01EF8-EE64-4B2D-B48E-D91E1E3DF4A7@gmx.de> (message from Heinrich Schuchardt on Sun, 02 Jan 2022 22:06:11 +0100) Subject: Re: [PATCH v2] efi_loader: Get rid of kaslr-seed References: <20211217070644.2458603-1-ilias.apalodimas@linaro.org> <16F01EF8-EE64-4B2D-B48E-D91E1E3DF4A7@gmx.de> Message-ID: X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.38 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 > Date: Sun, 02 Jan 2022 22:06:11 +0100 > From: Heinrich Schuchardt > > Am 2. Januar 2022 21:50:35 MEZ schrieb Ilias Apalodimas : > >Hi Heinrich, > > > >> > > > > > > > >[...] > > > >> > > > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c > >> > > > > index d77d3b6e943d..57f13ce701ec 100644 > >> > > > > --- a/cmd/bootefi.c > >> > > > > +++ b/cmd/bootefi.c > >> > > > > @@ -310,6 +310,8 @@ efi_status_t efi_install_fdt(void *fdt) > >> > > > > /* Create memory reservations as indicated by the device tree */ > >> > > > > efi_carve_out_dt_rsv(fdt); > >> > > > > > >> > > > > + efi_try_purge_kaslr_seed(fdt); > >> > >> This function should only be invoked for CONFIG_EFI_TCG2_PROTOCOL=y. > > > >Why? As we discussed the kernel ignores the kaslr-seed for the > >physical randomization. The only reason we would like to keep it is > >for the randomization of the virtual address. But if the EFI > >RNG protocol is installed the EFI stub is already doing the right thing. > >So I really think purging it if EFI RNG is installed is the best option > >here (regardless of TPM measurements) > > > > The only reason to delete kaslr-seed is that it conflicts with > measured boot. If an OS prefers the RNG protocol over kaslr-seed is > the decision of the OS and nothing U-Boot has to care about. But it is also up to the OS to decide whether it cares about measured boot or not. > You will have to delete kaslr-seed no matter if you have a RNG > protocol or not if and only if you want to use measured boot. So you shouldn't just unconditionally delete kaslr-seed if the CONFIG_EFI_TCG2_PROTOCOL has been enabled, but only if it is actually used... But is that even possible? So maybe you should just specify the certain properties (such as kaslr-seed) should be skipped when calculating the hash of the device tree. > >> > > > > + > >> > > > > /* Install device tree as UEFI table */ > >> > > > > ret = efi_install_configuration_table(&efi_guid_fdt, fdt); > >> > > > > if (ret != EFI_SUCCESS) { > >> > > > > diff --git a/include/efi_loader.h b/include/efi_loader.h > >> > > > > index 9dd6c2033634..1fe003db69e0 100644 > >> > > > > --- a/include/efi_loader.h > >> > > > > +++ b/include/efi_loader.h > >> > > > > @@ -519,6 +519,8 @@ efi_status_t EFIAPI efi_convert_pointer(efi_uintn_t debug_disposition, > >> > > > > void **address); > >> > > > > /* Carve out DT reserved memory ranges */ > >> > > > > void efi_carve_out_dt_rsv(void *fdt); > >> > > > > +/* Purge unused kaslr-seed */ > >> > > > > +void efi_try_purge_kaslr_seed(void *fdt); > >> > > > > /* Called by bootefi to make console interface available */ > >> > > > > efi_status_t efi_console_register(void); > >> > > > > /* Called by bootefi to make all disk storage accessible as EFI objects */ > >> > > > > diff --git a/lib/efi_loader/efi_dt_fixup.c b/lib/efi_loader/efi_dt_fixup.c > >> > > > > index b6fe5d2e5a34..d3923e5dba1b 100644 > >> > > > > --- a/lib/efi_loader/efi_dt_fixup.c > >> > > > > +++ b/lib/efi_loader/efi_dt_fixup.c > >> > > > > @@ -8,6 +8,7 @@ > >> > > > > #include > >> > > > > #include > >> > > > > #include > >> > > > > +#include > >> > > > > #include > >> > > > > #include > >> > > > > > >> > > > > @@ -40,6 +41,38 @@ static void efi_reserve_memory(u64 addr, u64 size, bool nomap) > >> > > > > addr, size); > >> > > > > } > >> > > > > > >> > > > > +/** > >> > > > > + * efi_try_purge_kaslr_seed() - Remove unused kaslr-seed > >> > > > > + * > >> > > > > + * Kernel's EFI STUB only relies on EFI_RNG_PROTOCOL for randomization > >> > > > > + * and completely ignores the kaslr-seed for its own randomness needs > >> > > > > + * (i.e the randomization of the physical placement of the kernel). > >> > > > > + * Weed it out from the DTB we hand over, which would mess up our DTB > >> > > > > + * TPM measurements as well. > >> > > > > + * > >> > > > > + * @fdt: Pointer to device tree > >> > > > > + */ > >> > > > > +void efi_try_purge_kaslr_seed(void *fdt) > >> > > > > +{ > >> > > > > + const efi_guid_t efi_guid_rng_protocol = EFI_RNG_PROTOCOL_GUID; > >> > >> There is not need to check if the RNG protocol is installed. If > >> CONFIG_EFI_TCG2_PROTOCOL=y, you should unconditionally remove > >> 'kaslr-seed' as it is incompatible with measured boot. > > > >That's not entirely correct. Right now having the kaslr-seed hurts no one, > >since we don't measure the DTB to begin with. What I intend to do is > >expose the RNG hardware of the TPM and use that if the hardware doesn't > >provide one already. This obviously means the kaslr-seed will be removed > >because the RNG protocol will always be installed with the current patch. > > > >I really don't see a connection between a *compile* time option which > >might not even have any effect if a TPM is not present, with an entry in > >the /chosen node. IMHO we should merge this patch since it improves > >existing use cases. I'll work on the rest and send patches soon. > > > >Cheers > >/Ilias > > > > > >> > >> Best regards > >> > >> Heinrich > >> > >> > > > > + struct efi_handler *handler; > >> > > > > + efi_status_t ret; > >> > > > > + int nodeoff = 0; > >> > > > > + int err = 0; > >> > > > > + > >> > > > > + ret = efi_search_protocol(efi_root, &efi_guid_rng_protocol, &handler); > >> > > > > + if (ret != EFI_SUCCESS) > >> > > > > + return; > >> > > > > + > >> > > > > + nodeoff = fdt_path_offset(fdt, "/chosen"); > >> > > > > + if (nodeoff < 0) > >> > > > > + return; > >> > > > > + > >> > > > > + err = fdt_delprop(fdt, nodeoff, "kaslr-seed"); > >> > > > > + if (err < 0 && err != -FDT_ERR_NOTFOUND) > >> > > > > + log_err("Error deleting kaslr-seed\n"); > >> > > > > +} > >> > > > > + > >> > > > > /** > >> > > > > * efi_carve_out_dt_rsv() - Carve out DT reserved memory ranges > >> > > > > * > >> > > > > -- > >> > > > > 2.30.2 > >> > > > > > >> > > > > > >> > > > >