[PATCH 1/7] capsule: authenticate: Embed capsule public key in platform's dtb

[Simon Glass]

Hi Ilias,

On Mon, 26 Jun 2023 at 10:53, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Simon,
>
> [...]
>
> > > > > > > +
> > > > > > > +gen_capsule_signature_file signature.$$.dts > /dev/null 2>&1
> > > > > > > +$CPP $dtc_cpp_flags -x assembler-with-cpp -o signature.$$.tmp signature.$$.dts > /dev/null 2>&1
> > > > > > > +dtc -@ -O dtb -o signature.$$.dtbo signature.$$.tmp > /dev/null 2>&1
> > > > > > > +fdtoverlay -i $1 -o temp.$$.dtb -v signature.$$.dtbo > /dev/null 2>&1
> > > > > > > +mv temp.$$.dtb $1 > /dev/null 2>&1
> > > > > > > +rm -f signature.$$.* > /dev/null 2>&1
> > > > > > > --
> > > > > > > 2.34.1
> > > > > > >
> > > > > >
> > > > > > Can you please add this to binman instead?
> > > > >
> > > > > I had looked at using binman for this work earlier because I very much
> > > > > expected this comment from you :). Having said that, I am very much
> > > > > open to using binman instead if it turns out to be the better way of
> > > > > achieving this. What this patch does is that, with capsule
> > > > > authentication enabled, it embeds the public key esl file into the
> > > > > dtb's as they get built. As per my understanding, binman gets called
> > > > > at the end of the u-boot build, once the constituent images( e..g
> > > > > u-boot.bin = u-boot-no-dtb.bin + dtb) have been generated. So, if we
> > > > > call binman _after_ the requisite image(s) have been generated, we
> > > > > would need to 1) identify the dtb's in which the esl needs to be
> > > > > embedded, and then 2) generate the final image all over again. Don't
> > > > > you think this is non optimal? Or is there a way of generating the
> > > > > constituent images(including the dtb's) through binman instead?
> > > >
> > > > The best way to do that IMO is to generate a second file, .e.g.
> > > > u-boot-capsule.bin
> > >
>
> This make no sense to me whatsoever.  Do we have an example in u-boot
> generating multiple dtb versions for other reasons/subsystems?
>
> > > That would break the scripts for platforms which might be using
> > > u-boot.bin as the image to boot from. I know that the ST platform
> > > which does enable capsule updates uses the u-boot-nodtb.bin as the
> > > BL33 image and the u-boot.dtb as BL33_CFG. Hence my question, if we
> > > have to use binman, is there a way to 1) modify the u-boot.dtb and
> > > then 2) regenerate u-boot.bin image.
> > >
> > > I know this is software, and everything can be done in a hacky way.
> > > But I was exploring using the u-boot node as a section entry, so that
> > > the u-boot.dtb can be modified and then binman would
> > > repackage/regenerate the u-boot.bin. But this is not working.
> >
> > NO, please do not do that.  You should create a new file, not modify
> > u-boot.bin or u-boot.dtb. Please just don't mess around with this, it
> > will lead to all sorts of confusion.
> >
> > I thought we already had this discussion a while back?
>
> No we haven't.  In fact I am struggling to see the confusion part.  It's
> fine for the u-boot dtb to include all the internal nodes DM needs, but
> suddenly having the capsule signature is problematic?
>
> In the past the .esl file was part of the U-Boot binary and things were
> working perfectly fine.  In fact you could update/downgrade u-boot and the
> signatures naturally followed along instead of having to update u-boot
> *and* the dtb, which we have to do today. You could also build a capsule
> way easier without injecting/removing signatures to the dtb.
> You were the one that insisted on reverting that and instead adding it on
> the dtb.  We explained most of the downsides back then, along with some
> security concerns.  We also mentioned that the signature in the dtb makes
> little sense since it's difference *per class of boards* and it's not
> something we could include in static dtb files, but that lead nowhere...
>
> As Sughosh already said there are platforms that use the generated u-boot
> dtb and the raw binary to assemble a FIP image.  So why exactly adding the
> capsule signature to the default dtb is wrong?  Why should we add an
> *extra* .dtb with one additional node -- which is very much needed by the
> design you proposed.  This will only lead to extra confusion.

1. I thought a capsule update was going to be a separate file, e.g.
u-boot-capture.bin ?

2. You can't put the signature into U-Boot. It needs to be in the
capsule update so that U-Boot can check it. If you are talking about
the public key, then yes that needs to be in U-Boot

3. Where does the public key come from? With the normal verified boot
flow it is created (and added to the dtb) as a separate signing step
after U-Boot is built.

4. Off topic, but re FIP, can someone just kill that off and use FIT?
It is such a shame that a new format was invented...

Regards,
Simon