[PATCH V5 08/12] iot2050: Add script for signing artifacts

[Simon Glass]

Hi Jan,

On Tue, 7 Feb 2023 at 09:45, Jan Kiszka <jan.kiszka at siemens.com> wrote:
>
> On 07.02.23 16:28, Simon Glass wrote:
> > Hi Jan,
> >
> > On Mon, 6 Feb 2023 at 04:57, Jan Kiszka <jan.kiszka at siemens.com> wrote:
> >>
> >> On 06.02.23 11:47, Jan Kiszka wrote:
> >>> On 04.02.23 23:26, Simon Glass wrote:
> >>>> Hi Jan,
> >>>>
> >>>> On Fri, 3 Feb 2023 at 23:35, Jan Kiszka <jan.kiszka at siemens.com> wrote:
> >>>>>
> >>>>> On 03.02.23 19:51, Tom Rini wrote:
> >>>>>> On Fri, Feb 03, 2023 at 01:26:37PM +0100, Jan Kiszka wrote:
> >>>>>>
> >>>>>>> From: Jan Kiszka <jan.kiszka at siemens.com>
> >>>>>>>
> >>>>>>> There are many ways to get a signed firmware for the IOT2050 devices,
> >>>>>>> namely for the parts under user-control. This script documents one way
> >>>>>>> of doing it, given a signing key. Augment the board documentation with
> >>>>>>> the required procedure around it.
> >>>>>>>
> >>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka at siemens.com>
> >>>>>> [snip]
> >>>>>>> +# currently broken in upstream
> >>>>>>> +#source/tools/binman/binman replace -i flash.bin -f fit at 0x380000.fit fit at 0x380000
> >>>>>>> +dd if=fit at 0x380000.fit of=flash.bin bs=$((0x1000)) seek=$((0x380000/0x1000)) conv=notrunc
> >>>>>>
> >>>>>> Is that still a true comment?
> >>>>>>
> >>>>>
> >>>>> binman: Node '/fit at 0x380000/images/u-boot': Offset 0x0 (0) size 0xb8870
> >>>>> (755824) is outside the section '/fit at 0x380000' starting at 0x0 (0) of
> >>>>> size 0x400 (1024)
> >>>>>
> >>>>> And for the second call:
> >>>>>
> >>>>> binman: Node '/fit at 0x380000': Replacing sections is not implemented yet
> >>>>
> >>>> I sent a patch to implement that.
> >>>>
> >>>> I'm not quite sure if this resolves the first problem, too. If not,
> >>>> can you please provide a binman test for the case you need, or
> >>>> instructions on how to cause the failure?
> >>>
> >>> Instructions to reproduce are basically
> >>>  - apply this series
> >>>  - build flash.bin according to doc/board/siemens/iot2050.rst
> >>>  - drop the dd calls and activate binman in this signing script
> >>>  - run it
> >>>
> >>> But I'll try your patch ASAP on my setup.
> >>
> >> Still left with
> >>
> >> binman: Node '/fit at 0x380000/images/u-boot': Offset 0x0 (0) size 0xb8928
> >> (756008) is outside the section '/fit at 0x380000' starting at 0x0 (0) of
> >> size 0x400 (1024)
> >>
> >> and
> >>
> >> binman: 'NoneType' object has no attribute 'props'
> >>
> >> That was for the second call of binman (source/tools/binman/binman
> >> replace -i flash.bin -f fit at 0x380000.fit fit at 0x380000). The "not
> >> implemented messages is gone.
> >>
> >> I've switched back to dd for the first call, but that did not work yet.
> >> So the message above indicates a relevant error.
> >>
> >> Jan
> >
> > I thought I was able to follow all the steps (gosh, so many blobs) but
> > I got something different from the first 'binman' call in your script.
> >
> > binman: Error 1 running 'mkimage -t -F
> > /tmp/binman.l_xl69mi/fit at 0x380000.fit': mkimage: verify_header failed
> > for FIT Image support with exit code 1
> >
> > I will take a look at that...it is trying to rebuild the FIT and it
> > should not. It is another case of rebuilding sections that I didn't
> > think of.
> >
> > But actually, you need to create a new entry type for your signing
> > scheme. It looks like the signature is created by openssl and (rather
> > than putting it in a separate file) it creates a new file containing
> > both the signature and the file contents. That is a bit of a pain, but
> > can be made to work.
> >
> > Basically you need a new entry type (of which the FIT is a child) that
> > gets the contents of its child, signs it and returns that as the
> > contents. You can see vblock for an example, and
> > collection_contents_to_file(). Let me know if you want me to create an
> > example.
> >
> > The way it should work is that you run binman once (as part of the
> > U-Boot build) and it produces a final image. No messing about with
> > scripts, etc. In this case it looks like the key.pem file should be an
> > input to your new entry type.
> >
> > Using binman replace to sign something later is fine, but it is not
> > the intended use. Binman is supposed to be a single-pass image
> > builder.
>
> I strongly suspect we will need split build/sign also in the future
> because those steps are generally separate in corporate production envs.
> Maybe even 3 steps: assemble, extract hashes that should be signed and
> embed signatures of those in the end.

Yes I'm sure you are right, that's what I would expect. There is a
'binman sign' command coming[1] so I hope we can use that to make it
easier, too.

Still, we must have a single-step build in U-Boot, so we do need a new
entry type. Let me know if you want me to hack up something as a
starting point.

>
> >
> > Since we get different results, I suggest pushing a tree somewhere, in
> > case something is different with the patches.
>
> Tree is already at https://github.com/siemens/u-boot/commits/jan/iot2050
>
> Jan
>
> --
> Siemens AG, Technology
> Competence Center Embedded Linux
>

Regards,
Simon

[1] https://patchwork.ozlabs.org/project/uboot/list/?series=291386&state=*