Re: [PATCH 0/3] Add support for signing grub with an appended signature

[Michal Suchánek]

Hello,

thanks for looking into this

On Thu, Oct 22, 2020 at 03:25:33PM +1100, Daniel Axtens wrote:
> Hi Michal,
> 
> >> A simpler scheme would be for grub-install to parse the signature
> >> footer, split-off the signature, write the ELF binary at the start of
> >> the PReP partition, and the signature at the end. Then the grub
> >> signature can use exactly same format as the kernel and modules.
> >
> > I got part way through implementing this today in SLOF and realised that
> > it's not immediately clear what the size of the signed data is for
> > verifying the appended signature - that is, how much of the PReP
> > partition should I be hashing? As I said, the appended signature
> > metadata doesn't encode this particular size value.
> >
> > One thing that we might be able to do is to base the size of the
> > verified data on a size calculated from the ELF header: basically figure
> > out the maximum file offset + size for all the program headers, capped
> > at (partition size - appended signature size), and hash that many
> > bytes. If I understand correctly, and assuming grub-mkimage constructs
> > valid ELF files, this should be correct.
> >
> > I'll give this a go in SLOF tomorrow, and I'll also need to confer with
> > the team that develops the proprietary firmware used by PowerVM.
> >
> 
> I talked to the firmware team this morning.
> 
> So grub is usually loaded from the PReP partition if you are booting
> from disk. But, if you are booting from a CD/USB/etc, we first parse
> /ppc/boot-info.txt and then load whatever file it identifies. If you're
> netbooting we load the file we get from the network.
> 
> One strength of the ELF-note based scheme is that verification of the
> appended signature is exactly the same in all these scenarios, and in
> each case it can live entirely in the ELF parsing and loading code.
> 
> In contrast, if we don't have an elf note, we have to handle PReP
> partitions, CHRP boot-info.txt and netboot separately. At least in the
> PReP case we also have to do parse enough of the ELF header to determine
> how much data we need to be hashing for signature verification, and this
> crosses a couple of previously separate steps of the process.
> 
> To illustrate from SLOF, currently the ELF note stuff lives almost
> entirely in elf32.c, but for the scheme you describe we need to patch
> load-from-dos-boot-partition, load-from-gpt-prep-partition, whatever
> code we use for where we boot a file directly from disk for CHRP
> boot-info.txt loading, and something somewhere in the netboot code.

Why do you have to handle different boot media separately, though?

There is this bug, supposedly in 'old firmware' that when the PReP
partition is large (gigabytes in size) the firmware crashes trying to
load the whole thing to memory and hand it over to the elf parser. If
this is fixed today you need to parse the header anyway to know how much
to load, or you need to just hand over some handle to the partition and
let the elf parser do the reading.

The appended signature format is exactly the same in each case: there is
a container such as a partition, a file, or a block of memory that has
an elf binary at the start, optional padding, and optinal signature at
the end. There is nothing stopping the elf parser from parsing both
the elf header and the signature footer when it gets the data.

In the case of the partition containing the elf binary it is expected
that the padding is non-zero, in the other cases having padding is
suspicious.

In the case of a partition you have to read the data in some
device-specific sectors while files can be read at arbitrary sized
chunks.

If you choose to write separate code to fetch the elf binary from
different media due to these differences then these separate code paths
must be adjusted. However, that is inherent to lack of suitable
abstraction in the code, and not the appended signature format.

I think it is more productive to clean up the code to handle all media
the same way rather than inventing new unique quirky and deficient
signature format.

Thanks

Michal

> 
> We can still support multiple signers disjoint in time with the scheme I
> suggest at
> https://lists.gnu.org/archive/html/grub-devel/2020-09/msg00081.html
> although I do agree it's ugly in its own separate way.
> 
> Kind regards,
> Daniel
> 
> >> The disadvantage is that for signed grub dd is no longer an alternative
> >> to grub-install.
> >>
> >> There was also concern about distinguishing signed and un-signed grub.
> >> That is that writing an un-signed grub might lease a stale signature
> >> leading to an error.
> >>
> >> However, secure boot is something that should be enabled or disabled in
> >> firmware settings, and not triggered by the PPeP partition containing a
> >> signature. 
> >>
> >> When secure boot is enabled checking the grub signature is required and
> >> un-signed grub is invalid. When secure boot is disabled the signature
> >> is irrelevant and stale signature should not cause any error.
> >
> > What I'm concerned about here - and it's possible I explained it poorly
> > at Plumbers - is how a systems administrator would distinguish between
> > having accidentally installed an unsigned grub, and having installed a
> > grub with a broken or untrusted signature.
> >
> > Having said that, ...
> >
> >> grub-install can also remove the signature magic when installing
> >> un-signed grub for consistency. Users using dd to install un-signed
> >> grub might still have an old signature at the end of the partition.
> >
> > if we make grub-install do the right thing, I think that sufficiently
> > mitigates my concern.
> >
> > Thanks again, and I'll let you know how I get on shortly.
> >
> > Kind regards,
> > Daniel