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

[Simon Glass]

Hi,

On Wed, 8 Dec 2021 at 11:10, Philippe REYNES
<philippe.reynes at softathome.com> wrote:
>
> Hi Rasmus,
>
> First, thanks for the feedback.
>
>
> Le 06/12/2021 à 09:23, Rasmus Villemoes a écrit :
> > On 17/11/2021 18.52, Philippe Reynes wrote:
> >> This commit adds a script gen_pre_load_header.sh
> >> that generate the header used by the image pre-load
> >> stage.
> >>
> >> Signed-off-by: Philippe Reynes <philippe.reynes at softathome.com>
> >> ---
> >>   tools/gen_pre_load_header.sh | 174 +++++++++++++++++++++++++++++++++++
> >>   1 file changed, 174 insertions(+)
> >>   create mode 100755 tools/gen_pre_load_header.sh
> >>
> >> diff --git a/tools/gen_pre_load_header.sh b/tools/gen_pre_load_header.sh
> >> new file mode 100755
> >> index 0000000000..8256fa80ee
> >> --- /dev/null
> >> +++ b/tools/gen_pre_load_header.sh
> >> @@ -0,0 +1,174 @@
> >> +#!/bin/bash
> >> +# SPDX-License-Identifier: GPL-2.0+
> >> +
> >> +#
> >> +# default value
> >> +#
> >> +size='4096'
> >> +algo='sha256,rsa2048'
> >> +padding='pkcs-1.5'
> >> +key=''
> >> +verbose='false'
> >> +input=''
> >> +output=''
> >> +
> >> +usage() {
> >> +    printf "Usage: $0 -a <algo> -k <key> [-p <padding>] [-s <size>] [-v] -i <input> -o <output>\n"
> >> +}
> >> +
> >> +#
> >> +# parse arguments
> >> +#
> >> +while getopts 'a:hi:k:o:p:s:v' flag; do
> >> +    case "${flag}" in
> >> +            a) algo="${OPTARG}" ;;
> >> +            h) usage
> >> +               exit 0 ;;
> >> +            i) input="${OPTARG}" ;;
> >> +            k) key="${OPTARG}" ;;
> >> +            o) output="${OPTARG}" ;;
> >> +            p) padding="${OPTARG}" ;;
> >> +            s) size="${OPTARG}" ;;
> >> +            v) verbose='true' ;;
> >> +            *) usage
> >> +               exit 1 ;;
> >> +    esac
> >> +done
> >> +
> >> +#
> >> +# check that mandatory arguments are provided
> >> +#
> >> +if [ -z "$key" -o -z "$input" -o -z "$output" ]
> >> +then
> >> +    usage
> >> +    exit 0
> >> +fi
> >> +
> >> +hash=$(echo $algo | cut -d',' -f1)
> >> +sign=$(echo $algo | cut -d',' -f2)
> >> +
> >> +echo "status:"
> >> +echo "size    = $size"
> >> +echo "algo    = $algo"
> >> +echo "hash    = $hash"
> >> +echo "sign    = $sign"
> >> +echo "padding = $padding"
> >> +echo "key     = $key"
> >> +echo "verbose = $verbose"
> >> +
> >> +#
> >> +# check if input file exist
> >> +#
> >> +if [ ! -f "$input" ]
> >> +then
> >> +    echo "Error: file '$input' doesn't exist"
> >> +    exit 1
> >> +fi
> >> +
> >> +#
> >> +# check if output is not empty
> >> +#
> >> +if [ -z "$output" ]
> >> +then
> >> +    echo "Error: output is empty"
> >> +    exit 1
> >> +fi
> >> +
> >> +#
> >> +# check that size is bigger than 0
> >> +#
> >> +if [ $size -le 0 ]
> >> +then
> >> +    echo "Error: $size lower than 0"
> >> +    exit 1
> >> +fi
> >> +
> >> +#
> >> +# check if the key file exist
> >> +#
> >> +if [ ! -f "$key" ]
> >> +then
> >> +    echo "Error: file $key doesn't exist\n"
> >> +    exit 1
> >> +fi
> >> +
> >> +#
> >> +# check if the hash is valid and supported
> >> +#
> >> +print_supported_hash() {
> >> +    echo "Supported hash:"
> >> +    echo "- sha1"
> >> +    echo "- sha256"
> >> +    echo "- sha384"
> >> +    echo "- sha512"
> >> +}
> >> +
> >> +case "$hash" in
> >> +    "sha1") hashOption="-sha1" ;;
> >> +    "sha256") hashOption="-sha256" ;;
> >> +    "sha384") hashOption="-sha384" ;;
> >> +    "sha512") hashOption="-sha512" ;;
> >> +    *) echo "Error: $hash is an invalid hash"
> >> +       print_supported_hash
> >> +       exit 1;;
> >> +esac
> >> +
> >> +#
> >> +# check if the sign is valid and supported
> >> +#
> >> +print_supported_sign() {
> >> +    echo "Supported sign:"
> >> +    echo "- rsa1024"
> >> +    echo "- rsa2048"
> >> +    echo "- rsa4096"
> >> +}
> >> +
> >> +case "$sign" in
> >> +    "rsa1024") ;;
> >> +    "rsa2048") ;;
> >> +    "rsa4096") ;;
> >> +    *) echo "Error: $sign is an invalid signature type"
> >> +       print_supported_sign
> >> +       exit 1;;
> >> +esac
> >> +
> >> +#
> >> +# check if the padding is valid and supported
> >> +#
> >> +print_supported_padding() {
> >> +    echo "Supported padding:"
> >> +    echo "- pkcs-1.5"
> >> +    echo "- pss"
> >> +}
> >> +
> >> +case "$padding" in
> >> +    "pkcs-1.5") optionPadding='' ;;
> >> +    "pss") optionPadding='-sigopt rsa_padding_mode:pss -sigopt rsa_pss_saltlen:-2' ;;
> >> +    *) echo "Error: $padding is an invalid padding"
> >> +       print_supported_padding
> >> +       exit 1;;
> >> +esac
> >> +
> >> +
> >> +#
> >> +# generate the sigature
> >> +#
> >> +sig=$(openssl dgst $optionHash -sign $key $optionPadding $input | xxd -p)
> >> +
> >> +#
> >> +# generate the header
> >> +#
> >> +# 0 = magic
> >> +# 4 = image size
> >> +# 8 = signature
> >> +#
> >> +h=$(printf "%08x" 0x55425348)
> >> +i=$(stat --printf="%s" $input)
> >> +i=$(printf "%08x" $i)
> >> +
> >> +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.
> > 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.
>
>
> > To that end, it would be nice if the header was a little more
> > self-describing. Something like
> >
> > 0 = magic
> > 4 = header size (including padding)
> > 8 = image size
> > 12 = offset to image signature
> > 16 = flags (currently enforced to 0)
> > 20 = reserved (currently enforced to 0)
> > 24 = signature of first 24 bytes
> >
> > xx = signature of image
> >
> > Why do I want the image size signed? Because I'd like to be able to
> > store the whole thing in a raw partition (or ubi volume, or...), where
> > there's no concept of "file size" available. So I'd like to be able to
> > read in some fixed size chunk (24+whatever I expect the signature could
> > be, so 4096 is certainly enough), and from that compute the whole size I
> > need to read. But I don't want to blindly trust the "image size" field.
> > So, for such a case, I'd also like a "verify header $loadaddr"
> > subcommand (and "verify image $loadaddr", with "verify $loadaddr" being
> > shorthand for doing both).
> 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
>
> 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.
> 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)
>
> As you can see I also propose to add the header version.
> I prefer the second solution.
>
>
> Everybody agrees with this proposal ?

So long as this is not a shell script and is done with a binman entry, yes.

But why not use struct image_header, if we are going to have this as
an ad-hoc thing?

Regards,
Simon


>
>
> >
> > And continuing the wishlist, it could be even better if we had
> >
> >    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.
>
> >
> > Rasmus
>
>
> Regards,
> Philippe
>
>
> -- This message and any attachments herein are confidential, intended solely for the addressees and are SoftAtHome’s ownership. Any unauthorized use or dissemination is prohibited. If you are not the intended addressee of this message, please cancel it immediately and inform the sender.