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

2023-03-10 Thread Simon Glass
Hi Jan,

On Sun, 12 Feb 2023 at 22:33, Jan Kiszka  wrote:
>
> On 13.02.23 05:26, Simon Glass wrote:
> > Hi Jan,
> >
> > On Tue, 7 Feb 2023 at 11:39, Simon Glass  wrote:
> >>
> >> Hi Jan,
> >>
> >> On Tue, 7 Feb 2023 at 09:45, Jan Kiszka  wrote:
> >>>
> >>> On 07.02.23 16:28, Simon Glass wrote:
>  Hi Jan,
> 
>  On Mon, 6 Feb 2023 at 04:57, Jan Kiszka  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  
> >>> 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 
> >>
> >> 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 
> > [snip]
> >> +# currently broken in upstream
> >> +#source/tools/binman/binman replace -i flash.bin -f 
> >> f...@0x38.fit fit@0x38
> >> +dd if=f...@0x38.fit of=flash.bin bs=$((0x1000)) 
> >> seek=$((0x38/0x1000)) conv=notrunc
> >
> > Is that still a true comment?
> >
> 
>  binman: Node '/fit@0x38/images/u-boot': Offset 0x0 (0) size 
>  0xb8870
>  (755824) is outside the section '/fit@0x38' starting at 0x0 (0) 
>  of
>  size 0x400 (1024)
> 
>  And for the second call:
> 
>  binman: Node '/fit@0x38': 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@0x38/images/u-boot': Offset 0x0 (0) size 0xb8928
> > (756008) is outside the section '/fit@0x38' 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 f...@0x38.fit fit@0x38). 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/f...@0x38.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

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

2023-02-12 Thread Jan Kiszka
On 13.02.23 05:26, Simon Glass wrote:
> Hi Jan,
> 
> On Tue, 7 Feb 2023 at 11:39, Simon Glass  wrote:
>>
>> Hi Jan,
>>
>> On Tue, 7 Feb 2023 at 09:45, Jan Kiszka  wrote:
>>>
>>> On 07.02.23 16:28, Simon Glass wrote:
 Hi Jan,

 On Mon, 6 Feb 2023 at 04:57, Jan Kiszka  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  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 
>>
>> 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 
> [snip]
>> +# currently broken in upstream
>> +#source/tools/binman/binman replace -i flash.bin -f 
>> f...@0x38.fit fit@0x38
>> +dd if=f...@0x38.fit of=flash.bin bs=$((0x1000)) 
>> seek=$((0x38/0x1000)) conv=notrunc
>
> Is that still a true comment?
>

 binman: Node '/fit@0x38/images/u-boot': Offset 0x0 (0) size 0xb8870
 (755824) is outside the section '/fit@0x38' starting at 0x0 (0) of
 size 0x400 (1024)

 And for the second call:

 binman: Node '/fit@0x38': 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@0x38/images/u-boot': Offset 0x0 (0) size 0xb8928
> (756008) is outside the section '/fit@0x38' 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 f...@0x38.fit fit@0x38). 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/f...@0x38.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.
> 
> Please see 

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

2023-02-12 Thread Simon Glass
Hi Jan,

On Tue, 7 Feb 2023 at 11:39, Simon Glass  wrote:
>
> Hi Jan,
>
> On Tue, 7 Feb 2023 at 09:45, Jan Kiszka  wrote:
> >
> > On 07.02.23 16:28, Simon Glass wrote:
> > > Hi Jan,
> > >
> > > On Mon, 6 Feb 2023 at 04:57, Jan Kiszka  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  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 
> > >>>
> > >>> 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 
> > >> [snip]
> > >>> +# currently broken in upstream
> > >>> +#source/tools/binman/binman replace -i flash.bin -f 
> > >>> f...@0x38.fit fit@0x38
> > >>> +dd if=f...@0x38.fit of=flash.bin bs=$((0x1000)) 
> > >>> seek=$((0x38/0x1000)) conv=notrunc
> > >>
> > >> Is that still a true comment?
> > >>
> > >
> > > binman: Node '/fit@0x38/images/u-boot': Offset 0x0 (0) size 
> > > 0xb8870
> > > (755824) is outside the section '/fit@0x38' starting at 0x0 (0) of
> > > size 0x400 (1024)
> > >
> > > And for the second call:
> > >
> > > binman: Node '/fit@0x38': 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@0x38/images/u-boot': Offset 0x0 (0) size 0xb8928
> > >> (756008) is outside the section '/fit@0x38' 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 f...@0x38.fit fit@0x38). 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/f...@0x38.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

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

2023-02-07 Thread Simon Glass
Hi Jan,

On Tue, 7 Feb 2023 at 09:45, Jan Kiszka  wrote:
>
> On 07.02.23 16:28, Simon Glass wrote:
> > Hi Jan,
> >
> > On Mon, 6 Feb 2023 at 04:57, Jan Kiszka  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  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 
> >>>
> >>> 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 
> >> [snip]
> >>> +# currently broken in upstream
> >>> +#source/tools/binman/binman replace -i flash.bin -f 
> >>> f...@0x38.fit fit@0x38
> >>> +dd if=f...@0x38.fit of=flash.bin bs=$((0x1000)) 
> >>> seek=$((0x38/0x1000)) conv=notrunc
> >>
> >> Is that still a true comment?
> >>
> >
> > binman: Node '/fit@0x38/images/u-boot': Offset 0x0 (0) size 0xb8870
> > (755824) is outside the section '/fit@0x38' starting at 0x0 (0) of
> > size 0x400 (1024)
> >
> > And for the second call:
> >
> > binman: Node '/fit@0x38': 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@0x38/images/u-boot': Offset 0x0 (0) size 0xb8928
> >> (756008) is outside the section '/fit@0x38' 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 f...@0x38.fit fit@0x38). 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/f...@0x38.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
>

Regar

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

2023-02-07 Thread Jan Kiszka
On 07.02.23 16:28, Simon Glass wrote:
> Hi Jan,
> 
> On Mon, 6 Feb 2023 at 04:57, Jan Kiszka  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  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 
>>>
>>> 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 
>> [snip]
>>> +# currently broken in upstream
>>> +#source/tools/binman/binman replace -i flash.bin -f f...@0x38.fit 
>>> fit@0x38
>>> +dd if=f...@0x38.fit of=flash.bin bs=$((0x1000)) 
>>> seek=$((0x38/0x1000)) conv=notrunc
>>
>> Is that still a true comment?
>>
>
> binman: Node '/fit@0x38/images/u-boot': Offset 0x0 (0) size 0xb8870
> (755824) is outside the section '/fit@0x38' starting at 0x0 (0) of
> size 0x400 (1024)
>
> And for the second call:
>
> binman: Node '/fit@0x38': 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@0x38/images/u-boot': Offset 0x0 (0) size 0xb8928
>> (756008) is outside the section '/fit@0x38' 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 f...@0x38.fit fit@0x38). 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/f...@0x38.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.

> 
> 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



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

2023-02-07 Thread Simon Glass
Hi Jan,

On Mon, 6 Feb 2023 at 04:57, Jan Kiszka  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  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 
> >
> > 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 
>  [snip]
> > +# currently broken in upstream
> > +#source/tools/binman/binman replace -i flash.bin -f f...@0x38.fit 
> > fit@0x38
> > +dd if=f...@0x38.fit of=flash.bin bs=$((0x1000)) 
> > seek=$((0x38/0x1000)) conv=notrunc
> 
>  Is that still a true comment?
> 
> >>>
> >>> binman: Node '/fit@0x38/images/u-boot': Offset 0x0 (0) size 0xb8870
> >>> (755824) is outside the section '/fit@0x38' starting at 0x0 (0) of
> >>> size 0x400 (1024)
> >>>
> >>> And for the second call:
> >>>
> >>> binman: Node '/fit@0x38': 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@0x38/images/u-boot': Offset 0x0 (0) size 0xb8928
> (756008) is outside the section '/fit@0x38' 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 f...@0x38.fit fit@0x38). 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/f...@0x38.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.

Since we get different results, I suggest pushing a tree somewhere, in
case something is different with the patches.

Regards,
Simon


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

2023-02-06 Thread Jan Kiszka
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  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 
>
> 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 
 [snip]
> +# currently broken in upstream
> +#source/tools/binman/binman replace -i flash.bin -f f...@0x38.fit 
> fit@0x38
> +dd if=f...@0x38.fit of=flash.bin bs=$((0x1000)) 
> seek=$((0x38/0x1000)) conv=notrunc

 Is that still a true comment?

>>>
>>> binman: Node '/fit@0x38/images/u-boot': Offset 0x0 (0) size 0xb8870
>>> (755824) is outside the section '/fit@0x38' starting at 0x0 (0) of
>>> size 0x400 (1024)
>>>
>>> And for the second call:
>>>
>>> binman: Node '/fit@0x38': 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@0x38/images/u-boot': Offset 0x0 (0) size 0xb8928
(756008) is outside the section '/fit@0x38' 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 f...@0x38.fit fit@0x38). 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

-- 
Siemens AG, Technology
Competence Center Embedded Linux



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

2023-02-06 Thread Jan Kiszka
On 04.02.23 23:26, Simon Glass wrote:
> Hi Jan,
> 
> On Fri, 3 Feb 2023 at 23:35, Jan Kiszka  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 

 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 
>>> [snip]
 +# currently broken in upstream
 +#source/tools/binman/binman replace -i flash.bin -f f...@0x38.fit 
 fit@0x38
 +dd if=f...@0x38.fit of=flash.bin bs=$((0x1000)) 
 seek=$((0x38/0x1000)) conv=notrunc
>>>
>>> Is that still a true comment?
>>>
>>
>> binman: Node '/fit@0x38/images/u-boot': Offset 0x0 (0) size 0xb8870
>> (755824) is outside the section '/fit@0x38' starting at 0x0 (0) of
>> size 0x400 (1024)
>>
>> And for the second call:
>>
>> binman: Node '/fit@0x38': 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.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux



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

2023-02-04 Thread Simon Glass
Hi Jan,

On Fri, 3 Feb 2023 at 23:35, Jan Kiszka  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 
> >>
> >> 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 
> > [snip]
> >> +# currently broken in upstream
> >> +#source/tools/binman/binman replace -i flash.bin -f f...@0x38.fit 
> >> fit@0x38
> >> +dd if=f...@0x38.fit of=flash.bin bs=$((0x1000)) 
> >> seek=$((0x38/0x1000)) conv=notrunc
> >
> > Is that still a true comment?
> >
>
> binman: Node '/fit@0x38/images/u-boot': Offset 0x0 (0) size 0xb8870
> (755824) is outside the section '/fit@0x38' starting at 0x0 (0) of
> size 0x400 (1024)
>
> And for the second call:
>
> binman: Node '/fit@0x38': 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?

>
> We tried fixing but eventually had to give up. binman is not easy to
> understand in these corners.

Regards,
Simon


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

2023-02-04 Thread Tom Rini
On Sat, Feb 04, 2023 at 07:34:46AM +0100, Jan Kiszka 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 
> >>
> >> 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 
> > [snip]
> >> +# currently broken in upstream
> >> +#source/tools/binman/binman replace -i flash.bin -f f...@0x38.fit 
> >> fit@0x38
> >> +dd if=f...@0x38.fit of=flash.bin bs=$((0x1000)) 
> >> seek=$((0x38/0x1000)) conv=notrunc
> > 
> > Is that still a true comment?
> > 
> 
> binman: Node '/fit@0x38/images/u-boot': Offset 0x0 (0) size 0xb8870
> (755824) is outside the section '/fit@0x38' starting at 0x0 (0) of
> size 0x400 (1024)
> 
> And for the second call:
> 
> binman: Node '/fit@0x38': Replacing sections is not implemented yet
> 
> We tried fixing but eventually had to give up. binman is not easy to
> understand in these corners.

OK, thanks.

-- 
Tom


signature.asc
Description: PGP signature


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

2023-02-03 Thread Jan Kiszka
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 
>>
>> 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 
> [snip]
>> +# currently broken in upstream
>> +#source/tools/binman/binman replace -i flash.bin -f f...@0x38.fit 
>> fit@0x38
>> +dd if=f...@0x38.fit of=flash.bin bs=$((0x1000)) 
>> seek=$((0x38/0x1000)) conv=notrunc
> 
> Is that still a true comment?
> 

binman: Node '/fit@0x38/images/u-boot': Offset 0x0 (0) size 0xb8870
(755824) is outside the section '/fit@0x38' starting at 0x0 (0) of
size 0x400 (1024)

And for the second call:

binman: Node '/fit@0x38': Replacing sections is not implemented yet

We tried fixing but eventually had to give up. binman is not easy to
understand in these corners.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux



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

2023-02-03 Thread Tom Rini
On Fri, Feb 03, 2023 at 01:26:37PM +0100, Jan Kiszka wrote:

> From: Jan Kiszka 
> 
> 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 
[snip]
> +# currently broken in upstream
> +#source/tools/binman/binman replace -i flash.bin -f f...@0x38.fit 
> fit@0x38
> +dd if=f...@0x38.fit of=flash.bin bs=$((0x1000)) 
> seek=$((0x38/0x1000)) conv=notrunc

Is that still a true comment?

-- 
Tom


signature.asc
Description: PGP signature