[PATCH 1/3 v2] efi_capsule: Move signature from DTB to .rodata

[Simon Glass]

Hi Ilias,

On Thu, 22 Jul 2021 at 07:56, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> On Thu, 22 Jul 2021 at 16:30, Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Ilias,
> >
> > On Thu, 22 Jul 2021 at 07:28, Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Ilias,
> > >
> > > On Wed, 21 Jul 2021 at 00:42, Ilias Apalodimas
> > > <ilias.apalodimas at linaro.org> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Tue, 20 Jul 2021 at 20:42, Simon Glass <sjg at chromium.org> wrote:
> > > > >
> > > > > Hi Sughosh,
> > > > >
> > > > > On Tue, 20 Jul 2021 at 07:32, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > > > >
> > > > > > hi Simon,
> > > > > >
> > > > > > On Tue, 20 Jul 2021 at 18:20, Ilias Apalodimas <ilias.apalodimas at linaro.org> wrote:
> > > > > >>
> > > > > >> Hi Simon,
> > > > > >> On Tue, 20 Jul 2021 at 15:33, Simon Glass <sjg at chromium.org> wrote:
> > > > > >> >
> > > > > >> > Hi Ilias,
> > > > > >> >
> > > > > >> > On Sat, 17 Jul 2021 at 08:27, Ilias Apalodimas
> > > > > >> > <ilias.apalodimas at linaro.org> wrote:
> > > > > >> > >
> > > > > >> > > The capsule signature is now part of our DTB.  This is problematic when a
> > > > > >> > > user is allowed to change/fixup that DTB from U-Boots command line since he
> > > > > >> > > can overwrite the signature as well.
> > > > > >> >
> > > > > >> > Just to repeat my question since it looks like I didn't get a response
> > > > > >> > on the last patch:
> > > > > >> >
> > > > > >> > Do you mean with the 'fdt' command?
> > > > > >> > >
> > > > > >> > If you mean the FDT fixups, they happen to a different DT, the one
> > > > > >> > being passed to Linux.
> > > > > >>
> > > > > >> In some platforms the key is derived from the relocated DTB, which we
> > > > > >> can overwrite. But I'll let Sughosh who figured it out explain the
> > > > > >> details.
> > > > > >
> > > > > >
> > > > > > On platforms where the dtb is concatenated with the u-boot image, using CONFIG_OF_SEPARATE, the fdt is also getting relocated to the main memory. We retrieve the public key from this dtb. By default, the fdtcontroladdr env variable is getting set to this relocated dtb address -- this address can also be accessed using the bdinfo command. Thus the public key can be modified before attempting the capsule update. Which is the reason why Ilias is moving the public key to the embedded rodata section.
> > > > >
> > > > > You should be clearer about what problem you are trying to solve. Are
> > > > > you worried about a script changing the DT? Or just it being writable
> > > > > in general?
> > > >
> > > > Being writable in general is my main concern. Doing fixup internally
> > > > from U-Boot might be something we'll always need but the ability to
> > > > completely change it doesn't play well security.
> > > >
> > > > >
> > > > > U-Boot itself is relocated also, including the rodata. So are you
> > > > > using the public key from the original location? What if that is not
> > > > > accessible after relocation?
> > > >
> > > > We are accessing he key from the relocated address.
> > >
> > > Then in what way are you protecting it? This is so confusing. Are you
> > > saying that you are protecting the relocated address? If so, protect
> > > the relocated devicetree too!
> > >
>
> How? DTBs if fixed up and there's a protocol proposal from Heinrich,
> which allows fixups from GRUB2.  So how exactly are you going to put
> it in r/o memory (which is what .rodata is supposed to achieve).

Because they are different DTBs, right? Either it can be read-only or
can't be read-only. At first you said it could not be read-only. Now
you are saying it needs to be changed. Where is all this coming from?

> A big portion of the DTBs we build today are horribly outdated
> compared to the current upstream.  Since nowdays there's a spec

That may be true on some boards but it is not my experience, at least
on ARM. Anyway that is an issue for the board maintainers. I don't
think this has any bearing on the points we are discussing here.

> describing what can and can't go in a DTB, I'd much rather prefer we
> stick to that and make a potential update easier.

There is just so much confusion in all of this and we are going around
in circles. Let me try to state what I think are points of confusion.

1. The U-Boot DT needs to be protected against change for lots of
reasons (drivers misbehaving, etc.). The signature is only one of
them.

2. The U-Boot DT is separate from the one passed to Linux. So
discussion about where U-Boot config should go in the Linux DT is not
germain.

3. U-Boot uses DT for its configuration and that is that. It has done
that for about 7-8 years. U-Boot does not have a user space to provide
policy and configuration . It cannot do what Linux does and run
programs and look up filesystems to figure out how to boot. So
configuration / runtime info go in the DT in U-Boot. I have not seen
any proposal to do it any other way. I hope you can understand how
frustrating to have someone come from the Linux world and say, Oh it's
all wrong...we should put it user space, etc. The alternative to DT is
a mishmash of random places and ideas with no schema and no
discoverability, etc, or a forest of CONFIG options, like it used to
me.

4. The DT is relocated anyway so is not actually read-only just
because you put it in the rodata area.

5. You can make the DT read-only if you want to. You can make any part
of memory read-only. You need to create an API for that if that's what
you want. It would be nice to have a command to look at what is
protected and change it. See for example the 'mtrr' command on x86.

Also I would add that there has always been a DT spec. I think the
latest version is here:
https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.3
So far as I can tell it does not talk about what can and cannot go in a DT

Perhaps the spec you are referring to is a Linux spec. Do you have a
link? So far as I can tell, U-Boot, Zephyr, etc. have had very little
input into that. I know for a fact that no one has asked what I think.
For example, even the u-boot,xxx tags are kept in separate files in
U-Boot because of resistance to putting that in Linux. Zephyr
completely does its own thing with DT. U-Boot very much follows Binux,
BUT it has its own things as well, just as Linux does. I would LOVE to
see that change and if you would like to help with that, or have ideas
on how, please go ahead.

Regards,
Simon

[..]