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=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no 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 CE391C04EB5 for ; Fri, 7 Feb 2020 15:31:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 8136820838 for ; Fri, 7 Feb 2020 15:31:07 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="VZnjA+Tu" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727012AbgBGPbH (ORCPT ); Fri, 7 Feb 2020 10:31:07 -0500 Received: from mail-wr1-f68.google.com ([209.85.221.68]:38024 "EHLO mail-wr1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726674AbgBGPbH (ORCPT ); Fri, 7 Feb 2020 10:31:07 -0500 Received: by mail-wr1-f68.google.com with SMTP id y17so3181593wrh.5 for ; Fri, 07 Feb 2020 07:31:03 -0800 (PST) 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; bh=TC+VMUCo88KEW4kESs34vu0qfl39+6/hso0IL8x9w98=; b=VZnjA+TumuB9BSvFyh4hS2+e+maLKjIHYfP10KANlMbMBt7jAxedYJHqK8VqbofkHG tdKoWr94JpI0nCdRJKO3wGDdUGKx7E3mPKJt7fnYINvSAOKEeqZTRfvXGPofwC4dizcW w2Nu8xbFDoSXbGtB9qfhIZCTPGWgSG791sLycUy0QA1BWgjjPS/4ncLlLYKzbuaRMjEN MJ2mDCiCd5STRtrTQ+G8NH9QkIznkUedeqsQQu58tNFtk4VKjF9k1JYLMKfAgbIJ6lJq eR8Zsp6DsHNWYhk1/npzTjHFgT29W4cXuR72nydik6c1qc9Hh79pOMDOH0QNWeagaOpt ZIaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=TC+VMUCo88KEW4kESs34vu0qfl39+6/hso0IL8x9w98=; b=GTHokafhmCzH8QG5O1d+lHWBw6TCbnW93eDwRsJKWyy6N5ZjOu78hx7uKNkN1/fZzO czzGnVz4Z0122h9eMyEhPlYaApVRq8H6apVp845mvthl5nGeNl564T4FCGq9NrcHGyBg wFwpBks9oW3JKmNQT6p6FZ+4V8bRSPzW8580fUYoj4YP5xLeet5DUkzJmNl2Q7W50lyJ 9okeNAiBKPq0bagDrqy6ZGDwFq+VYhcB+f8YBBqS7CXuhYFp90WP2ZL1jPgao4XqKxvb CeJPz34HZe+MJC7A7JWWZsAbjjrJ15R85JYZy8foODp+39XM46LpO6Z5fwWVcyrUChjg BE4g== X-Gm-Message-State: APjAAAWn4BNZU4u5mQHIPXOVfd8ktSWsltV4h1m74mQUatxAOfy9tJIH 1cOoFRUf5Iwxmgsn5siJiUx+OHp2Gf2fqE1p+uhqqQ== X-Google-Smtp-Source: APXvYqy4fZAGpGMxjx1ZG/U+BDZQ0Rgl0AFW8U/lBlW24hdddXAoh12Ug1AveGr2mgYltI/rv00o1Rwt4FDUo9UOsKA= X-Received: by 2002:adf:fd8d:: with SMTP id d13mr5344851wrr.208.1581089461726; Fri, 07 Feb 2020 07:31:01 -0800 (PST) MIME-Version: 1.0 References: <20200206140352.6300-1-ardb@kernel.org> <20200206140352.6300-2-ardb@kernel.org> <081d152a-fa9b-886a-565d-b244dea08cd5@gmx.de> <9e7c378a-782f-f56e-2ce3-0af6386b0bff@gmx.de> In-Reply-To: From: Ard Biesheuvel Date: Fri, 7 Feb 2020 15:30:50 +0000 Message-ID: Subject: Re: [PATCH 1/2] efi/libstub: add support for loading the initrd from a device path To: Alexander Graf Cc: Heinrich Schuchardt , Ard Biesheuvel , linux-efi , linux-arm-kernel , Laszlo Ersek , Leif Lindholm , Peter Jones , Matthew Garrett , Ilias Apalodimas , Daniel Kiper Content-Type: text/plain; charset="UTF-8" Sender: linux-efi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-efi@vger.kernel.org On Fri, 7 Feb 2020 at 14:18, Alexander Graf wrote: > > > On 07.02.20 13:58, Ard Biesheuvel wrote: > > On Fri, 7 Feb 2020 at 13:30, Heinrich Schuchardt wrote: > >> > >> > >> On 2/7/20 9:12 AM, Ard Biesheuvel wrote: > >>> On Fri, 7 Feb 2020 at 00:58, Heinrich Schuchardt wrote: > >>>> On 2/7/20 1:21 AM, Ard Biesheuvel wrote: > >>>>> On Fri, 7 Feb 2020 at 00:01, Heinrich Schuchardt wrote: > >>>>>> On 2/6/20 11:35 PM, Ard Biesheuvel wrote: > >>>>>>> On Thu, 6 Feb 2020 at 18:26, Heinrich Schuchardt wrote: > >>> ... > >>>>>>>> Please, indicate which software you expect to expose the initrd related > >>>>>>>> EFI_LOAD_FILE2_PROTOCOL. > >>>>>>>> > >>>>>>> The primary use case is GRUB and other intermediate loaders, since it > >>>>>>> would remove any need for these components to know any such details. > >>>>>>> My aim is to make the next architecture that gets added to GRUB for > >>>>>>> EFI boot 100% generic. > >>>>>>> > >>>>>>>> Using an UEFI variable for passing the initrd device path would be a > >>>>>>>> leaner solution on the bootloader side than requiring an extra > >>>>>>>> EFI_LOAD_FILE2_PROTOCOL implementation. > >>>>>>>> > >>>>>>> This would also require kernel changes, since we don't currently load > >>>>>>> initrds from arbitrary device paths. The EFI_FILE_PROTOCOL is much > >>>>>>> more complicated than needed, and it doesn't work well with mixed > >>>>>>> mode. It also requires GRUB to expose the filesystem it loads the > >>>>>>> initrd from via EFI protocols, which is currently unnecessary and > >>>>>>> therefore not implemented. > >>>>>> This means you move the complexity of EFI_FILE_PROTOCOL from Linux to GRUB. > >>>>>> > >>>>> No. I am not interested in EFI_FILE_PROTOCOL, only in LoadFile2, which > >>>>> is a single method that needs to be implemented. > >>>> I said you move complexity because GRUB will need to use the > >>>> EFI_FILE_PROTOCOL to do the job that you do not want to do in Linux. > >>>> > >>>>>> I would not have a problem if this would only touch GRUB. But if listen > >>>>>> to Ilias we are replacing one implementation in Linux by one in GRUB and > >>>>>> one in U-Boot and one in EDK2 and one in any other firmware. > >>>>>> > >>>>> If u-boot will be used to boot RISC-V in EFI mode without GRUB, then I > >>>>> expect that we will need an implementation of this in u-boot. > >>>> What sets RISC-V apart? GRUB for RISC-V is available. > >> >> > >>> RISC-V EFI boot is not supported yet in upstream Linux. > >> It is currently prepared Atish Patra of WDC. > >> > > Exactly. So it is not in the upstream yet, and I want to converge on a > > sane generic interface before it gets merged. > > > >>>>>>> Also, using an EFI variable defeats the purpose. The whole point of > >>>>>>> this is making it more likely that the kernel loaded the initrd that > >>>>>>> the bootloader or firmware intended it to load, and having a piece of > >>>>>>> simple [signed] code that implements this is the easiest way to > >>>>>>> achieve that. > >>>>>> At least on my Debian system it is the operating system creating initrd > >>>>>> and defining which initrd matches which kernel. GRUB simply assumes that > >>>>>> files ending on the same version number match. Therefore I would say > >>>>>> Linux hopes that GRUB loads what Linux intended. > >>>>>> > >>>>>> The chain of trust would not be broken if the kernel were responsible > >>>>>> for loading the initrd and for checking if it matches the kernel. Linux > >>>>>> already does this for the kernel modules in initrd. > >>>>>> > >>>>> We can still sign the initrd and Linux can verify the signature. What > >>>>> I am after is an interface that does not require the initrd to > >>>>> originate from a EFI file system protocol, and which doesn't require > >>>>> the loaded initrd to sit in memory for an unspecified amount of time > >>>>> and its information passed via DT properties or bootparams structs. > >>>>> > >>>>> So invoking EFI_FILE_PROTOCOL directly is not going to work, > >>>>> regardless of whether we get the devicepath from the command line or > >>>>> from a EFI variable. > >>>> What do you mean by "is not going to work"? > >>>> > >>>> With the device path you can find the handle implementing the > >>>> EFI_SIMPLE_FIL_SYSTEM_PROTOCOL. > >>>> > >>>>>>> For u-boot, it should be trivial to implement a simple LoadFile2 > >>>>>>> protocol wrapper around EFI_FILE_PROTOCOL that can be installed on a > >>>>>>> handle that also carries EFI_FILE_PROTOCOL. > >>>>>>> > >>>>>> A U-Boot implementation of the EFI_LOAD_FILE2_PROTOCOL would need a > >>>>>> device path variable to find the block device and to open the > >>>>>> EFI_SIMPLE_FILE_SYSTEM_PROTOCOL before accessing the file. > >>>>>> > >>>>>> Linux would not be needing more lines and we would not repeat the same > >>>>>> code in GRUB, U-Boot, EDK2, etc. > >>>>>> > >>>>>> As said Linux updates the initrd often. If that file is not signed by > >>>>>> Linux in a well defined way, do not expect any security at all. > >>>>>> > >>>>> It is not only about security. The primary goal is to remove the need > >>>>> for arch specific knowledge in the firmware about DT, bootparams and > >>>>> initrd allocation policies without being forced to load the initrd > >>>>> from a filesystem that is exposed via a EFI protocol. > >>>> Where are device-trees touched by this patch? > >>>> > >>>> When booting via UEFI there is no need for knowledge of initrd > >>>> allocation policies in U-Boot because up to now Linux or GRUB or iPXE > >>>> load initrd. > >>>> > >>>> Furthermore I need no knowledge of bootparams in U-Boot once we properly > >>>> we support UEFI variables at runtime because grub-update will pass the > >>>> command line in one of the Bootxxxx UEFI variables. > >>>> > >>>> But most importantly I do not have to implement anything Linux specific > >>>> in U-Boot for booting via UEFI up to now. > >>>> > >>> Adding Linux specific stuff to u-boot is arguably more appropriate > >>> than adding architecture specific stuff to EFI loaders that could > >>> otherwise be entirely generic. > >>> > >>> ... > >>>> Your patch claims to fend off a specific threat scenario: A user puts an > >>>> untrusted initrd on the disk and references it in the Linux command line. > >>>> > >>>> If he is able to do so with your current bootloader (signed or not > >>>> signed), he most probably will also be able to delete a good initrd from > >>>> the filesystem and thus force your code into the unsafe path. > >>>> > >>>> That is why I say that with the current fallback logic this patch > >>>> achieves no increase in security. Of cause you could remove the fallback > >>>> logic. But in this case your Linux will not boot with any legacy > >>>> bootloader or firmware. > >>>> > >>> If there is a better way to expose the initrd that > >>> a) does not require the initrd to reside on a file system that is > >>> accessible via EFI protocols, and > >>> b) does not require the loader to know about arch specific policies > >>> regarding the placement of the initrd in memory, and > >>> c) does not leave a time window between the time that the initrd is > >>> loaded/verified/measured by the firmware and the time that the kernel > >>> gets handed the buffer > >>> > >>> then I am happy to discuss it. This proposal is the best I could come > >>> up with to achieve the above. > >>> > >> Hello Ard, > >> > >> I think part of our different views is that we are thinking about two > >> different use cases which both have their relevance: > >> > >> If I understand you correctly, you are thinking about an embedded device > >> where the kernel and the initrd is essentially part of the firmware > >> provided by the device. > >> > >> I am thinking of a system running a standard Linux distribution like > >> Debian where the initrd is generated by the operating system > >> > >> In both use cases verifying the initrd is of importance. > >> > >> Now concerning the requirements: > >> > >> a) In U-Boot all file systems on block devices can be made accessible > >> via EFI protocols. Are you thinking about initrds that are not in a file > >> system? > >> > > The typical GRUB deployment keeps the core GRUB itself (or the entire > > thing if it is built as standalone) in the ESP, and the GRUB modules, > > kernel images and initrds are in /boot, which is typically not a file > > system that EFI understands. So in that case, initrd= does not work, > > which is why GRUB loads the initrd into memory directly and passes the > > base address and size via DT or bootparams structure. > > > >> b) My suggestion to use a UEFI variable for communicating the device > >> path would not require any arch specific policies either. > >> > > Passing the EFI device path is not going to help us since the initrd > > may not be representable as a device path. That is the whole point, > > actually - this series makes the initrd representable as a device > > path, but in a simple way that doesn't rely on EFI_FILE_PROTOCOL but > > only on EFI_LOAD_FILE2_PROTOCOL, which is *much* simpler. > > > So if we had support in grub to just export its own file systems as UEFI > protocols, that problem would disappear, right? What other reasons are > left to not just use normal file load operations from the Linux EFI stub? > 1) complexity """ typedef struct { u64 size; u64 file_size; u64 phys_size; efi_time_t create_time; efi_time_t last_access_time; efi_time_t modification_time; __aligned_u64 attribute; efi_char16_t filename[]; } efi_file_info_t; typedef struct efi_file_protocol efi_file_protocol_t; struct efi_file_protocol { u64 revision; efi_status_t (__efiapi *open) (efi_file_protocol_t *, efi_file_protocol_t **, efi_char16_t *, u64, u64); efi_status_t (__efiapi *close) (efi_file_protocol_t *); efi_status_t (__efiapi *delete) (efi_file_protocol_t *); efi_status_t (__efiapi *read) (efi_file_protocol_t *, unsigned long *, void *); efi_status_t (__efiapi *write) (efi_file_protocol_t *, unsigned long, void *); efi_status_t (__efiapi *get_position)(efi_file_protocol_t *, u64 *); efi_status_t (__efiapi *set_position)(efi_file_protocol_t *, u64); efi_status_t (__efiapi *get_info) (efi_file_protocol_t *, efi_guid_t *, unsigned long *, void *); efi_status_t (__efiapi *set_info) (efi_file_protocol_t *, efi_guid_t *, unsigned long, void *); efi_status_t (__efiapi *flush) (efi_file_protocol_t *); }; typedef struct efi_simple_file_system_protocol efi_simple_file_system_protocol_t; struct efi_simple_file_system_protocol { u64 revision; int (__efiapi *open_volume)(efi_simple_file_system_protocol_t *, efi_file_protocol_t **); }; #define EFI_FILE_MODE_READ 0x0000000000000001 #define EFI_FILE_MODE_WRITE 0x0000000000000002 #define EFI_FILE_MODE_CREATE 0x8000000000000000 """ versus """ union efi_load_file_protocol { struct { efi_status_t (__efiapi *load_file)(efi_load_file_protocol_t *, efi_device_path_protocol_t *, bool, unsigned long *, void *); }; }; """ 2) mixed mode is problematic due to the use of u64 arguments in the prototypes 3) you have to trust that the device path that was specified was intended to be loaded as an initrd 4) if you want to authenticate or measure the initrd from the firmware as it is read, you have to disambiguate EFI_FILE_PROTOCOL calls involving the initrd from arbitrary other calls. > IIRC you mentioned that the fwcfg -kernel and -initrd parameters are > already exposed as pseudo filesystem inside AAVMF, so that one is solved. > Yes, but this only works if you use the PE entry point, which rules out mixed mode. > I think that only leaves the UEFI shell case? But if you have a UEFI > shell, then you can only load from an existing file system as well, no? > Yes. > What I can't quite grasp yet is how you would handle multiple initrds > with a single device path. How would that work? > The core kernel does not care about multiple initrds, they have to be concatenated and passed as one anyway. In this case, that means the implementation of LoadFile2 is in charge of this. > > > > >> c) I proposed that the kernel does the verification. So there would be > >> equally nothing in between loading the file and its verification. Yet > >> responsibilities would be changed. > >> > >> But possibly I missed some requirements you have in mind that I should > >> consider. > >> > > 1) The assumption that the initrd can always be loaded from a EFI > > device path directly does not hold. > > > Can you think of good reasons why this is true? I understand the grub > one, but that's solvable. What other cases are there? > Thin EFI implementations in enclaves, LinuxBoot on arm64, ... > > > 2) Loading the initrd into memory and passing the address and size is > > not acceptable. > > > This would basically be the option to pass the initrd as configuration > table, right? The only reason that definitely goes against that one that > I can think of right now is to avoid double copying? > That basically forces the stub to reallocate it, unless we put the [arch specific] allocation policy in the loader. initrds can be large, and so having to copy them can be a problem. > > > 3) Having a special device path that designates the initrd > > specifically (which will be treated in a special way by the kernel) > > makes it very easy for the boot firmware to attach policy to the > > loading of the initrd that can be enforced right when the file is > > being passed into the kernel. > > > I don't fully understand this one. Can you give examples for such > policies? :) > If you want to measure the initrd into PCR x, it gives you a natural place to do it, since the data is measured right at the point where the stub consumes it, and the protocol will never be invoked if the initrd is not requested (e.g., if the boot environment exposes a initrd but 'noinitrd' is passed on the command line, in which case we would prefer not to measure the initrd at all) > > > Putting an arbitrary device path in a EFI variable doesn't address 1), > > and it complicates 3), since you cannot easily distinguish whether the > > file load that is occurring is the EFI stub loading the initrd at > > boot. > > > Why do we need to distinguish? I'm missing creativity for the use case > right now. For 1), we just need to make sure that boot loaders that > implement their own file systems also expose those file systems as UEFI > protocols, right? > Because an arbitrary file read call cannot be identified as being the one issued by the stub to load and invoke the initrd. > > That said, I don't think the proposed approach of using > EFI_LOAD_FILE2_PROTOCOL is bad. Whatever we do, we always will need to > treat initrds special in one way or another; at least by exposing a > command to specify which one(s) you want to load. > 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=-0.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no 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 C19F9C04EB5 for ; Fri, 7 Feb 2020 15:31:13 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 85A0D20838 for ; Fri, 7 Feb 2020 15:31:13 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="KDWwoxjR"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="VZnjA+Tu" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 85A0D20838 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=3NnCP98IqNqPtLBna5mJDGTTYqBLC4M7ekkDAo4Orzk=; b=KDWwoxjRRzE0Rg a2sEDQ6uI6q6dmX7K5sJYMvzCdAC/4Navq+05LKWnDmw9hC1RkmwKgbrDEKnqaPMyTp3CJYHLcbGD yeLPKhmM3QbS2Xe+jzaoqw6Hk/PfOzmXh9HXhyucbkFKSH5BlnCjVArRuYZgizm2utBVIKf/YyGG5 ksVQyT8jdNhFz4RVp/v3u113lvOq9K5tKKyT5SUh0gKOzI5CBXBUAtlUPwQboNalbaGlFlLAuIVEa EIzcWbJYA/0PCyet8Ap5GhVquacSia+MWa9nIQ+c4Z6Z7EvN6uvR3326hLu627/IZ12EYpwWVyueK 3NurqPzpGWPCy1eA+TLg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1j05b1-0002sL-Mj; Fri, 07 Feb 2020 15:31:07 +0000 Received: from mail-wr1-x442.google.com ([2a00:1450:4864:20::442]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1j05ay-0002rm-2v for linux-arm-kernel@lists.infradead.org; Fri, 07 Feb 2020 15:31:06 +0000 Received: by mail-wr1-x442.google.com with SMTP id u6so3214648wrt.0 for ; Fri, 07 Feb 2020 07:31:03 -0800 (PST) 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; bh=TC+VMUCo88KEW4kESs34vu0qfl39+6/hso0IL8x9w98=; b=VZnjA+TumuB9BSvFyh4hS2+e+maLKjIHYfP10KANlMbMBt7jAxedYJHqK8VqbofkHG tdKoWr94JpI0nCdRJKO3wGDdUGKx7E3mPKJt7fnYINvSAOKEeqZTRfvXGPofwC4dizcW w2Nu8xbFDoSXbGtB9qfhIZCTPGWgSG791sLycUy0QA1BWgjjPS/4ncLlLYKzbuaRMjEN MJ2mDCiCd5STRtrTQ+G8NH9QkIznkUedeqsQQu58tNFtk4VKjF9k1JYLMKfAgbIJ6lJq eR8Zsp6DsHNWYhk1/npzTjHFgT29W4cXuR72nydik6c1qc9Hh79pOMDOH0QNWeagaOpt ZIaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=TC+VMUCo88KEW4kESs34vu0qfl39+6/hso0IL8x9w98=; b=Hqy2shbmdwPGZpX/Bp5WP9rkRe10JB2ETxjxwA2jFD3/HtKJ9/t8mZWyukkVqDvsT5 fMBWbEvrX4rUEGrntS4dff3YRQt3vmm2F1TZHElfOZKhhNmtV45KEd7JqxaX8J93O2N1 YIUrMr8cWJ5fx8H2f4KValb47wCQ4wXS4mhHfCWIP8rIb0BjkiDkkfNRWJ60o7DOqF1I eewa7xjV5baTs6C0gZBV9WGooQhfrljc63R+bL+uy2y82JI58dWMVtKT0I6lysq99AKc rgY/KzoTmedxP0F1sIx/wUiEpy4V7Qmorewsd9KLD/SR3SBeIhcsOqaJSuonhRiOlqch gxSQ== X-Gm-Message-State: APjAAAVJ6pycxyDiwEWDlQuX1G1uVuBruLtZWEnGH1zCABH6Cqc2jC2i 50FrgH1IR2W1L2xjPu8F3BiVwVkyY2xJ97pie8rDpA== X-Google-Smtp-Source: APXvYqy4fZAGpGMxjx1ZG/U+BDZQ0Rgl0AFW8U/lBlW24hdddXAoh12Ug1AveGr2mgYltI/rv00o1Rwt4FDUo9UOsKA= X-Received: by 2002:adf:fd8d:: with SMTP id d13mr5344851wrr.208.1581089461726; Fri, 07 Feb 2020 07:31:01 -0800 (PST) MIME-Version: 1.0 References: <20200206140352.6300-1-ardb@kernel.org> <20200206140352.6300-2-ardb@kernel.org> <081d152a-fa9b-886a-565d-b244dea08cd5@gmx.de> <9e7c378a-782f-f56e-2ce3-0af6386b0bff@gmx.de> In-Reply-To: From: Ard Biesheuvel Date: Fri, 7 Feb 2020 15:30:50 +0000 Message-ID: Subject: Re: [PATCH 1/2] efi/libstub: add support for loading the initrd from a device path To: Alexander Graf X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200207_073104_137698_13AF50E5 X-CRM114-Status: GOOD ( 48.38 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-efi , Heinrich Schuchardt , Daniel Kiper , Ilias Apalodimas , Matthew Garrett , Peter Jones , Leif Lindholm , Laszlo Ersek , Ard Biesheuvel , linux-arm-kernel Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, 7 Feb 2020 at 14:18, Alexander Graf wrote: > > > On 07.02.20 13:58, Ard Biesheuvel wrote: > > On Fri, 7 Feb 2020 at 13:30, Heinrich Schuchardt wrote: > >> > >> > >> On 2/7/20 9:12 AM, Ard Biesheuvel wrote: > >>> On Fri, 7 Feb 2020 at 00:58, Heinrich Schuchardt wrote: > >>>> On 2/7/20 1:21 AM, Ard Biesheuvel wrote: > >>>>> On Fri, 7 Feb 2020 at 00:01, Heinrich Schuchardt wrote: > >>>>>> On 2/6/20 11:35 PM, Ard Biesheuvel wrote: > >>>>>>> On Thu, 6 Feb 2020 at 18:26, Heinrich Schuchardt wrote: > >>> ... > >>>>>>>> Please, indicate which software you expect to expose the initrd related > >>>>>>>> EFI_LOAD_FILE2_PROTOCOL. > >>>>>>>> > >>>>>>> The primary use case is GRUB and other intermediate loaders, since it > >>>>>>> would remove any need for these components to know any such details. > >>>>>>> My aim is to make the next architecture that gets added to GRUB for > >>>>>>> EFI boot 100% generic. > >>>>>>> > >>>>>>>> Using an UEFI variable for passing the initrd device path would be a > >>>>>>>> leaner solution on the bootloader side than requiring an extra > >>>>>>>> EFI_LOAD_FILE2_PROTOCOL implementation. > >>>>>>>> > >>>>>>> This would also require kernel changes, since we don't currently load > >>>>>>> initrds from arbitrary device paths. The EFI_FILE_PROTOCOL is much > >>>>>>> more complicated than needed, and it doesn't work well with mixed > >>>>>>> mode. It also requires GRUB to expose the filesystem it loads the > >>>>>>> initrd from via EFI protocols, which is currently unnecessary and > >>>>>>> therefore not implemented. > >>>>>> This means you move the complexity of EFI_FILE_PROTOCOL from Linux to GRUB. > >>>>>> > >>>>> No. I am not interested in EFI_FILE_PROTOCOL, only in LoadFile2, which > >>>>> is a single method that needs to be implemented. > >>>> I said you move complexity because GRUB will need to use the > >>>> EFI_FILE_PROTOCOL to do the job that you do not want to do in Linux. > >>>> > >>>>>> I would not have a problem if this would only touch GRUB. But if listen > >>>>>> to Ilias we are replacing one implementation in Linux by one in GRUB and > >>>>>> one in U-Boot and one in EDK2 and one in any other firmware. > >>>>>> > >>>>> If u-boot will be used to boot RISC-V in EFI mode without GRUB, then I > >>>>> expect that we will need an implementation of this in u-boot. > >>>> What sets RISC-V apart? GRUB for RISC-V is available. > >> >> > >>> RISC-V EFI boot is not supported yet in upstream Linux. > >> It is currently prepared Atish Patra of WDC. > >> > > Exactly. So it is not in the upstream yet, and I want to converge on a > > sane generic interface before it gets merged. > > > >>>>>>> Also, using an EFI variable defeats the purpose. The whole point of > >>>>>>> this is making it more likely that the kernel loaded the initrd that > >>>>>>> the bootloader or firmware intended it to load, and having a piece of > >>>>>>> simple [signed] code that implements this is the easiest way to > >>>>>>> achieve that. > >>>>>> At least on my Debian system it is the operating system creating initrd > >>>>>> and defining which initrd matches which kernel. GRUB simply assumes that > >>>>>> files ending on the same version number match. Therefore I would say > >>>>>> Linux hopes that GRUB loads what Linux intended. > >>>>>> > >>>>>> The chain of trust would not be broken if the kernel were responsible > >>>>>> for loading the initrd and for checking if it matches the kernel. Linux > >>>>>> already does this for the kernel modules in initrd. > >>>>>> > >>>>> We can still sign the initrd and Linux can verify the signature. What > >>>>> I am after is an interface that does not require the initrd to > >>>>> originate from a EFI file system protocol, and which doesn't require > >>>>> the loaded initrd to sit in memory for an unspecified amount of time > >>>>> and its information passed via DT properties or bootparams structs. > >>>>> > >>>>> So invoking EFI_FILE_PROTOCOL directly is not going to work, > >>>>> regardless of whether we get the devicepath from the command line or > >>>>> from a EFI variable. > >>>> What do you mean by "is not going to work"? > >>>> > >>>> With the device path you can find the handle implementing the > >>>> EFI_SIMPLE_FIL_SYSTEM_PROTOCOL. > >>>> > >>>>>>> For u-boot, it should be trivial to implement a simple LoadFile2 > >>>>>>> protocol wrapper around EFI_FILE_PROTOCOL that can be installed on a > >>>>>>> handle that also carries EFI_FILE_PROTOCOL. > >>>>>>> > >>>>>> A U-Boot implementation of the EFI_LOAD_FILE2_PROTOCOL would need a > >>>>>> device path variable to find the block device and to open the > >>>>>> EFI_SIMPLE_FILE_SYSTEM_PROTOCOL before accessing the file. > >>>>>> > >>>>>> Linux would not be needing more lines and we would not repeat the same > >>>>>> code in GRUB, U-Boot, EDK2, etc. > >>>>>> > >>>>>> As said Linux updates the initrd often. If that file is not signed by > >>>>>> Linux in a well defined way, do not expect any security at all. > >>>>>> > >>>>> It is not only about security. The primary goal is to remove the need > >>>>> for arch specific knowledge in the firmware about DT, bootparams and > >>>>> initrd allocation policies without being forced to load the initrd > >>>>> from a filesystem that is exposed via a EFI protocol. > >>>> Where are device-trees touched by this patch? > >>>> > >>>> When booting via UEFI there is no need for knowledge of initrd > >>>> allocation policies in U-Boot because up to now Linux or GRUB or iPXE > >>>> load initrd. > >>>> > >>>> Furthermore I need no knowledge of bootparams in U-Boot once we properly > >>>> we support UEFI variables at runtime because grub-update will pass the > >>>> command line in one of the Bootxxxx UEFI variables. > >>>> > >>>> But most importantly I do not have to implement anything Linux specific > >>>> in U-Boot for booting via UEFI up to now. > >>>> > >>> Adding Linux specific stuff to u-boot is arguably more appropriate > >>> than adding architecture specific stuff to EFI loaders that could > >>> otherwise be entirely generic. > >>> > >>> ... > >>>> Your patch claims to fend off a specific threat scenario: A user puts an > >>>> untrusted initrd on the disk and references it in the Linux command line. > >>>> > >>>> If he is able to do so with your current bootloader (signed or not > >>>> signed), he most probably will also be able to delete a good initrd from > >>>> the filesystem and thus force your code into the unsafe path. > >>>> > >>>> That is why I say that with the current fallback logic this patch > >>>> achieves no increase in security. Of cause you could remove the fallback > >>>> logic. But in this case your Linux will not boot with any legacy > >>>> bootloader or firmware. > >>>> > >>> If there is a better way to expose the initrd that > >>> a) does not require the initrd to reside on a file system that is > >>> accessible via EFI protocols, and > >>> b) does not require the loader to know about arch specific policies > >>> regarding the placement of the initrd in memory, and > >>> c) does not leave a time window between the time that the initrd is > >>> loaded/verified/measured by the firmware and the time that the kernel > >>> gets handed the buffer > >>> > >>> then I am happy to discuss it. This proposal is the best I could come > >>> up with to achieve the above. > >>> > >> Hello Ard, > >> > >> I think part of our different views is that we are thinking about two > >> different use cases which both have their relevance: > >> > >> If I understand you correctly, you are thinking about an embedded device > >> where the kernel and the initrd is essentially part of the firmware > >> provided by the device. > >> > >> I am thinking of a system running a standard Linux distribution like > >> Debian where the initrd is generated by the operating system > >> > >> In both use cases verifying the initrd is of importance. > >> > >> Now concerning the requirements: > >> > >> a) In U-Boot all file systems on block devices can be made accessible > >> via EFI protocols. Are you thinking about initrds that are not in a file > >> system? > >> > > The typical GRUB deployment keeps the core GRUB itself (or the entire > > thing if it is built as standalone) in the ESP, and the GRUB modules, > > kernel images and initrds are in /boot, which is typically not a file > > system that EFI understands. So in that case, initrd= does not work, > > which is why GRUB loads the initrd into memory directly and passes the > > base address and size via DT or bootparams structure. > > > >> b) My suggestion to use a UEFI variable for communicating the device > >> path would not require any arch specific policies either. > >> > > Passing the EFI device path is not going to help us since the initrd > > may not be representable as a device path. That is the whole point, > > actually - this series makes the initrd representable as a device > > path, but in a simple way that doesn't rely on EFI_FILE_PROTOCOL but > > only on EFI_LOAD_FILE2_PROTOCOL, which is *much* simpler. > > > So if we had support in grub to just export its own file systems as UEFI > protocols, that problem would disappear, right? What other reasons are > left to not just use normal file load operations from the Linux EFI stub? > 1) complexity """ typedef struct { u64 size; u64 file_size; u64 phys_size; efi_time_t create_time; efi_time_t last_access_time; efi_time_t modification_time; __aligned_u64 attribute; efi_char16_t filename[]; } efi_file_info_t; typedef struct efi_file_protocol efi_file_protocol_t; struct efi_file_protocol { u64 revision; efi_status_t (__efiapi *open) (efi_file_protocol_t *, efi_file_protocol_t **, efi_char16_t *, u64, u64); efi_status_t (__efiapi *close) (efi_file_protocol_t *); efi_status_t (__efiapi *delete) (efi_file_protocol_t *); efi_status_t (__efiapi *read) (efi_file_protocol_t *, unsigned long *, void *); efi_status_t (__efiapi *write) (efi_file_protocol_t *, unsigned long, void *); efi_status_t (__efiapi *get_position)(efi_file_protocol_t *, u64 *); efi_status_t (__efiapi *set_position)(efi_file_protocol_t *, u64); efi_status_t (__efiapi *get_info) (efi_file_protocol_t *, efi_guid_t *, unsigned long *, void *); efi_status_t (__efiapi *set_info) (efi_file_protocol_t *, efi_guid_t *, unsigned long, void *); efi_status_t (__efiapi *flush) (efi_file_protocol_t *); }; typedef struct efi_simple_file_system_protocol efi_simple_file_system_protocol_t; struct efi_simple_file_system_protocol { u64 revision; int (__efiapi *open_volume)(efi_simple_file_system_protocol_t *, efi_file_protocol_t **); }; #define EFI_FILE_MODE_READ 0x0000000000000001 #define EFI_FILE_MODE_WRITE 0x0000000000000002 #define EFI_FILE_MODE_CREATE 0x8000000000000000 """ versus """ union efi_load_file_protocol { struct { efi_status_t (__efiapi *load_file)(efi_load_file_protocol_t *, efi_device_path_protocol_t *, bool, unsigned long *, void *); }; }; """ 2) mixed mode is problematic due to the use of u64 arguments in the prototypes 3) you have to trust that the device path that was specified was intended to be loaded as an initrd 4) if you want to authenticate or measure the initrd from the firmware as it is read, you have to disambiguate EFI_FILE_PROTOCOL calls involving the initrd from arbitrary other calls. > IIRC you mentioned that the fwcfg -kernel and -initrd parameters are > already exposed as pseudo filesystem inside AAVMF, so that one is solved. > Yes, but this only works if you use the PE entry point, which rules out mixed mode. > I think that only leaves the UEFI shell case? But if you have a UEFI > shell, then you can only load from an existing file system as well, no? > Yes. > What I can't quite grasp yet is how you would handle multiple initrds > with a single device path. How would that work? > The core kernel does not care about multiple initrds, they have to be concatenated and passed as one anyway. In this case, that means the implementation of LoadFile2 is in charge of this. > > > > >> c) I proposed that the kernel does the verification. So there would be > >> equally nothing in between loading the file and its verification. Yet > >> responsibilities would be changed. > >> > >> But possibly I missed some requirements you have in mind that I should > >> consider. > >> > > 1) The assumption that the initrd can always be loaded from a EFI > > device path directly does not hold. > > > Can you think of good reasons why this is true? I understand the grub > one, but that's solvable. What other cases are there? > Thin EFI implementations in enclaves, LinuxBoot on arm64, ... > > > 2) Loading the initrd into memory and passing the address and size is > > not acceptable. > > > This would basically be the option to pass the initrd as configuration > table, right? The only reason that definitely goes against that one that > I can think of right now is to avoid double copying? > That basically forces the stub to reallocate it, unless we put the [arch specific] allocation policy in the loader. initrds can be large, and so having to copy them can be a problem. > > > 3) Having a special device path that designates the initrd > > specifically (which will be treated in a special way by the kernel) > > makes it very easy for the boot firmware to attach policy to the > > loading of the initrd that can be enforced right when the file is > > being passed into the kernel. > > > I don't fully understand this one. Can you give examples for such > policies? :) > If you want to measure the initrd into PCR x, it gives you a natural place to do it, since the data is measured right at the point where the stub consumes it, and the protocol will never be invoked if the initrd is not requested (e.g., if the boot environment exposes a initrd but 'noinitrd' is passed on the command line, in which case we would prefer not to measure the initrd at all) > > > Putting an arbitrary device path in a EFI variable doesn't address 1), > > and it complicates 3), since you cannot easily distinguish whether the > > file load that is occurring is the EFI stub loading the initrd at > > boot. > > > Why do we need to distinguish? I'm missing creativity for the use case > right now. For 1), we just need to make sure that boot loaders that > implement their own file systems also expose those file systems as UEFI > protocols, right? > Because an arbitrary file read call cannot be identified as being the one issued by the stub to load and invoke the initrd. > > That said, I don't think the proposed approach of using > EFI_LOAD_FILE2_PROTOCOL is bad. Whatever we do, we always will need to > treat initrds special in one way or another; at least by exposing a > command to specify which one(s) you want to load. > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel