[PATCH v4 00/16] efi_loader: add secure boot support

[Ilias Apalodimas]

On Thu, Jan 09, 2020 at 08:09:27PM +0100, Heinrich Schuchardt wrote:
> On 1/9/20 9:02 AM, Ilias Apalodimas wrote:
> > On Thu, Jan 09, 2020 at 01:08:35AM +0100, Heinrich Schuchardt wrote:
> > > On 12/18/19 1:44 AM, AKASHI Takahiro wrote:
> > > > One of major missing features in current UEFI implementation is "secure boot."
> > > > The ultimate goal of my attempt is to implement image authentication based
> > > > on signature and provide UEFI secure boot support which would be fully
> > > > compliant with UEFI specification, section 32[1].
> > > > (The code was originally developed by Patrick Wildt.)
> > > > 
> > > > Please note, however, this patch doesn't work on its own; there are
> > > > a couple of functional dependencies[2] and [3], that I have submitted
> > > > before. For complete workable patch set, see my repository[4],
> > > > which also contains experimental timestamp-based revocation suuport.
> > > > 
> > > > My "non-volatile" support[5], which is under discussion, is not mandatory
> > > > and so not included here, but this inevitably implies that, for example,
> > > > signature database variables, like db and dbx, won't be persistent unless you
> > > > explicitly run "env save" command and that UEFI variables are not separated
> > > > from U-Boot environment. Anyhow, Linaro is also working on implementing
> > > > real "secure storage" solution based on TF-A and OP-TEE.
> > > > 
> > > 
> > > Device trees can be used for denial of service or to destroy hardware.
> > > 
> > > How will you address the validation of device trees?
> > 
> > Although this is really simple to solve, factoring in the different vendor
> > needs makes it quite complex.
> > There's a couple of options we can consider and not all of them are sane.
> > 
> > 1. U-Boot embeds the DTB. This is straightforward. On Arm devices TF-A
> > verifies U-boot so you natively end up with a verified DTB. If U-Boot does not
> > include the proper DTB (as is the case for several devices), one can always
> > complite the correct DTB and compile with EXT_DTB.
> > 2. Use https://github.com/jiazhang0/SELoader which verifies non-PE files
> > 3. Add some custom code and use UEFI keyring to verify non PE files. This is a
> > bad idea though since you'll 'polute' the UEFI keyring.
> > 4. FIT for DTB (??)
> > 
> > In any case UEFI job is to verify PE/COFF executables and that's what this patch
> > provides. DTB verification is beyond UEFI secure boot patches imho.
> > 
> > Regards
> > /Ilias
> 
> The UEFI specification does not mention device trees at all. EDK2 is
> based on ACPI tables.

On one of the platforms i kknow of (socionext synquacer), it also provides DTB
as part of the firmware, which is identical to proposeal (1) I mentioned.

> 
> We already have verified boot via signed UEFI FIT images which can
> contain an UEFI image and a device tree. So for verified boot of Linux
> you would simply package shim and the device tree into a FIT image. Shim
> would verify GRUB and GRUB would verify the kernel and the ramdisk. In
> this scenario we don't need the current patch series at all and it
> integrates well with distributions like Debian which provide shim for
> arm64, cf. https://packages.debian.org/de/bullseye/shim-signed .

Of course everything is verified, but that's not UEFI secure boot. It's similar
but the verification does not go through DB/DBX and there are no secure
variables, so the current patchset has value.

> 
> If we implement secure boot according the UEFI specification, one option
> would be to package the device tree as a UEFI driver image and let the
> stub install it as a configuration table. The unload callback could be
> used to remove the device tree.
> 

Sure but this is not in scope for the current patchset is it?
Similarly you can just include the DTB in U-Boot and naturally have it verified.

I am not arguing that DTB verification is needed. We absolutely agree on that.
All i am saying is that the extra functionality can be added in the future,
since we already have a valid way of providing it with the current patchset.

Regards
/Ilias
> 
> > > 
> > > Best regards
> > > 
> > > Heinrich
> > 
>