All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heinrich Schuchardt <xypron.glpk@gmx.de>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ard Biesheuvel <ardb@kernel.org>,
	linux-efi <linux-efi@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Laszlo Ersek <lersek@redhat.com>,
	Leif Lindholm <leif@nuviainc.com>,
	Peter Jones <pjones@redhat.com>,
	Matthew Garrett <mjg59@google.com>,
	Alexander Graf <agraf@csgraf.de>,
	Ilias Apalodimas <ilias.apalodimas@linaro.org>,
	Daniel Kiper <daniel.kiper@oracle.com>
Subject: Re: [PATCH 1/2] efi/libstub: add support for loading the initrd from a device path
Date: Fri, 7 Feb 2020 14:30:10 +0100	[thread overview]
Message-ID: <9e7c378a-782f-f56e-2ce3-0af6386b0bff@gmx.de> (raw)
In-Reply-To: <CAKv+Gu-f9O4eon_E7=OUu_tNwybi7u6bF2zMowPHbA-MhiTjNg@mail.gmail.com>



On 2/7/20 9:12 AM, Ard Biesheuvel wrote:
> On Fri, 7 Feb 2020 at 00:58, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On 2/7/20 1:21 AM, Ard Biesheuvel wrote:
>>> On Fri, 7 Feb 2020 at 00:01, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>
>>>> On 2/6/20 11:35 PM, Ard Biesheuvel wrote:
>>>>> On Thu, 6 Feb 2020 at 18:26, Heinrich Schuchardt <xypron.glpk@gmx.de> 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.

>
>>>
>>>>>
>>>>> 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?

b) My suggestion to use a UEFI variable for communicating the device
path would not require any arch specific policies either.

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.

Best regards

Heinrich


WARNING: multiple messages have this Message-ID
From: Heinrich Schuchardt <xypron.glpk@gmx.de>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: linux-efi <linux-efi@vger.kernel.org>,
	Alexander Graf <agraf@csgraf.de>,
	Daniel Kiper <daniel.kiper@oracle.com>,
	Ilias Apalodimas <ilias.apalodimas@linaro.org>,
	Matthew Garrett <mjg59@google.com>,
	Peter Jones <pjones@redhat.com>,
	Leif Lindholm <leif@nuviainc.com>,
	Laszlo Ersek <lersek@redhat.com>,
	Ard Biesheuvel <ardb@kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/2] efi/libstub: add support for loading the initrd from a device path
Date: Fri, 7 Feb 2020 14:30:10 +0100	[thread overview]
Message-ID: <9e7c378a-782f-f56e-2ce3-0af6386b0bff@gmx.de> (raw)
In-Reply-To: <CAKv+Gu-f9O4eon_E7=OUu_tNwybi7u6bF2zMowPHbA-MhiTjNg@mail.gmail.com>



On 2/7/20 9:12 AM, Ard Biesheuvel wrote:
> On Fri, 7 Feb 2020 at 00:58, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On 2/7/20 1:21 AM, Ard Biesheuvel wrote:
>>> On Fri, 7 Feb 2020 at 00:01, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>
>>>> On 2/6/20 11:35 PM, Ard Biesheuvel wrote:
>>>>> On Thu, 6 Feb 2020 at 18:26, Heinrich Schuchardt <xypron.glpk@gmx.de> 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.

>
>>>
>>>>>
>>>>> 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?

b) My suggestion to use a UEFI variable for communicating the device
path would not require any arch specific policies either.

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.

Best regards

Heinrich


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-02-07 13:30 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-06 14:03 [PATCH 0/2] arch-agnostic initrd loading method for EFI systems Ard Biesheuvel
2020-02-06 14:03 ` Ard Biesheuvel
2020-02-06 14:03 ` [PATCH 1/2] efi/libstub: add support for loading the initrd from a device path Ard Biesheuvel
2020-02-06 14:03   ` Ard Biesheuvel
2020-02-06 18:26   ` Heinrich Schuchardt
2020-02-06 18:26     ` Heinrich Schuchardt
2020-02-06 18:46     ` Ilias Apalodimas
2020-02-06 18:46       ` Ilias Apalodimas
2020-02-06 19:15       ` Heinrich Schuchardt
2020-02-06 19:15         ` Heinrich Schuchardt
2020-02-06 20:09         ` Ilias Apalodimas
2020-02-06 20:09           ` Ilias Apalodimas
2020-02-06 22:49           ` Heinrich Schuchardt
2020-02-06 22:49             ` Heinrich Schuchardt
2020-02-07  7:35             ` Ilias Apalodimas
2020-02-07  7:35               ` Ilias Apalodimas
2020-02-06 22:35     ` Ard Biesheuvel
2020-02-06 22:35       ` Ard Biesheuvel
2020-02-07  0:01       ` Heinrich Schuchardt
2020-02-07  0:01         ` Heinrich Schuchardt
2020-02-07  0:21         ` Ard Biesheuvel
2020-02-07  0:21           ` Ard Biesheuvel
2020-02-07  0:57           ` Heinrich Schuchardt
2020-02-07  0:57             ` Heinrich Schuchardt
2020-02-07  8:12             ` Ard Biesheuvel
2020-02-07  8:12               ` Ard Biesheuvel
2020-02-07 13:30               ` Heinrich Schuchardt [this message]
2020-02-07 13:30                 ` Heinrich Schuchardt
2020-02-07 13:58                 ` Ard Biesheuvel
2020-02-07 13:58                   ` Ard Biesheuvel
2020-02-07 14:18                   ` Alexander Graf
2020-02-07 14:18                     ` Alexander Graf
2020-02-07 15:30                     ` Ard Biesheuvel
2020-02-07 15:30                       ` Ard Biesheuvel
2020-02-07 15:35                     ` Heinrich Schuchardt
2020-02-07 15:35                       ` Heinrich Schuchardt
2020-02-07 11:09       ` Laszlo Ersek
2020-02-07 11:09         ` Laszlo Ersek
2020-02-07 11:03     ` Laszlo Ersek
2020-02-07 11:03       ` Laszlo Ersek
2020-02-07  9:48   ` Laszlo Ersek
2020-02-07  9:48     ` Laszlo Ersek
2020-02-07 12:36     ` Ard Biesheuvel
2020-02-07 12:36       ` Ard Biesheuvel
2020-02-10 14:26       ` Laszlo Ersek
2020-02-10 14:26         ` Laszlo Ersek
2020-02-09  6:39   ` Lukas Wunner
2020-02-09 11:35     ` Ard Biesheuvel
2020-02-09 11:35       ` Ard Biesheuvel
2020-02-06 14:03 ` [PATCH 2/2] efi/libstub: take noinitrd cmdline argument into account for devpath initrd Ard Biesheuvel
2020-02-06 14:03   ` Ard Biesheuvel
2020-02-06 18:33   ` Heinrich Schuchardt
2020-02-06 18:33     ` Heinrich Schuchardt
2020-02-06 23:44     ` Ard Biesheuvel
2020-02-06 23:44       ` Ard Biesheuvel
2020-02-12 16:01   ` Peter Jones
2020-02-12 16:01     ` Peter Jones
2020-02-07  9:09 ` [PATCH 0/2] arch-agnostic initrd loading method for EFI systems Laszlo Ersek
2020-02-07  9:09   ` Laszlo Ersek
2020-02-07  9:22   ` Laszlo Ersek
2020-02-07  9:22     ` Laszlo Ersek
2020-02-07 12:23     ` Ard Biesheuvel
2020-02-07 12:23       ` Ard Biesheuvel
2020-02-07 16:20       ` James Bottomley
2020-02-07 16:20         ` James Bottomley
2020-02-07 18:31         ` Ard Biesheuvel
2020-02-07 18:31           ` Ard Biesheuvel
2020-02-07 19:54           ` James Bottomley
2020-02-07 19:54             ` James Bottomley
2020-02-07 20:03             ` Ard Biesheuvel
2020-02-07 20:03               ` Ard Biesheuvel
2020-02-07 18:45 ` Arvind Sankar
2020-02-07 18:45   ` Arvind Sankar
2020-02-07 19:47   ` Ard Biesheuvel
2020-02-07 19:47     ` Ard Biesheuvel
2020-02-07 20:26     ` Arvind Sankar
2020-02-07 20:26       ` Arvind Sankar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9e7c378a-782f-f56e-2ce3-0af6386b0bff@gmx.de \
    --to=xypron.glpk@gmx.de \
    --cc=agraf@csgraf.de \
    --cc=ard.biesheuvel@linaro.org \
    --cc=ardb@kernel.org \
    --cc=daniel.kiper@oracle.com \
    --cc=ilias.apalodimas@linaro.org \
    --cc=leif@nuviainc.com \
    --cc=lersek@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=mjg59@google.com \
    --cc=pjones@redhat.com \
    --subject='Re: [PATCH 1/2] efi/libstub: add support for loading the initrd from a device path' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.