Re: [RFC PATCH v3 8/8] tools: gen_pre_load_header.sh: initial import

[Rasmus Villemoes <rasmus.villemoes@prevas.dk>]

On 08/12/2021 19.10, Philippe REYNES wrote:
> Hi Rasmus,
> 
> First, thanks for the feedback.
> 
> 

>>> +echo "$h$i$sig" | xxd -r -p > $output
>> So this sounds like a completely generic way of prepending a signature
>> to an arbitrary blob, whether that is a FIT image, a U-Boot script
>> wrapped in a FIT, some firmware blob or whatnot. So it sounds like it
>> could be generally useful, and a lot simpler than the complexity
>> inherent when trying to add signature data within the signed data
>> structure itself.
> We plan to use it with FIT, but it is very generic and may be used
> with any firmware.

Excellent.

>> So, can we perhaps not tie it to bootm as such? It's not a problem if
>> bootm learns to recognize 0x55425348 as another image format and then
>> automatically knows how to locate the "real" image, and/or automatically
>> verifies it. But I'd really like to be able to
>>
>>    fatload $loadaddr blabla && \
>>    verify $loadaddr && \
>>    source $loadaddr
>>
>> where fatload can be any random command that gets a bunch of bytes into
>> memory at a specific location (tftpboot, mmc read, ubi...). Currently,
>> we simply don't have any sane way to verify a boot script, or random
>> blobs, AFAICT.
> 
> 
> Based on this header, it is quite easy to develop a command verify.
> It wasn't planned but it is a very good idea. I will add it, in the next
> version or in another series a bit after.

Thanks for being open to my suggestions/ideas.

> 
>> To that end, it would be nice if the header was a little more
>> self-describing. Something like
>>
[snip]
> I understand why you want to add a signature for the header and I agree.
> 
> But if we add a signature for the header, I think that we should
> sign everything (even the signature) or include a hash of the
> image signature in the header.
> 
> So I would suggest something like:
> 0 = magic
> 4 = header size (including padding)
> 8 = image size
> 12 = offset to image signature
> 16 = version of the header
> 20 = flags (currently enforced to 0)
> 24 = reserved (currently enforced to 0)
> 28 = reserved (currently enforced to 0)
> 32 = sha256 of the signature
> 64 = signature of the first 64 bytes
> ..
> xx = signature of the image

Yes, I like this.

> Another solution would be to keep the header size in the u-boot device
> tree and
> add the signature of the header at the end of the header.

Hm, no, I don't see any reason to hardcode the header size - though it
is of course not much of a loss as one must already hardcode the public
key and the method to use for verification. However, imagine at some
point we figure out a use for flags/reserved that means the header needs
to expand. Obviously U-Boot would need to be upgraded to understand
those flags, but the upgraded U-Boot should still be able to verify old
blobs with 0 flags and the old header size. Granted, it's a bit of a
thin argument.

> It would become something like:
> 0 = magic
> 4 = image size
> 8 = offset to image signature
> 12 = version of the header
> 16 = flags (currently enforced to 0)
> 20 = reserved (currently enforced to 0)
> 24 = reserved (currently enforced to 0)
> ..
> xx = signature of the image
> ..
> header_size - sig_size = signature of the header (without the header
> signature)

Is sig_size always a known constant, only depending on the algorithm used?

> As you can see I also propose to add the header version.
> I prefer the second solution.
> 
> Everybody agrees with this proposal ?

Well, I prefer the first, but I'll leave it to you.

>>
>>    verify load $loadaddr 'mmc read %l% 0 %s512%'
>>
>> i.e. we could pass a "parametrized shell command" to verify for it to
>> use to read in a bunch of bytes to a given address - with %l% being
>> substituted by the address and %s<optional unit>% by the size to load,
>> optionally specified in the given unit.
> 
> I agree, it would be nice. But I think it could be done in a second step.

Absolutely.

Rasmus