u-boot: signature check for u-boot scripts

[Simon Glass]

Hi,

On Thu, 12 Jan 2023 at 09:03, Sean Anderson <sean.anderson at seco.com> wrote:
>
> On 1/11/23 01:13, Heiko Schocher wrote:
> > Hello Sean,
> >
> > Thanks for your answer!
> >
> > On 10.01.23 17:27, Sean Anderson wrote:
> >> On 1/10/23 08:18, Heiko Schocher wrote:
> >>> Hello Simon,
> > [...]
> >>> While writting this email ... in [3] the line
> >>>
> >>>  require = "conf"
> >>>
> >>> poped into my eyes .... and in fit_image_verify_required_sigs() there is check:
> >>>
> >>>                 if (!required || strcmp(required, "image"))
> >>>                         continue;
> >>>
> >>> and yes! changing in [3]
> >>>
> >>> -required = "conf";
> >>> +required = "image";
> >>>
> >>> makes sourcing the signed script working (error in case of no
> >>> signature or wrong signature)! ... but booting the signed fitimage
> >>> now breaks ... so it seems, I cannot use configuration signing with
> >>> images signing ?
> >>>
> >>> I tried to add two key nodes in signature node of u-boot dtb ... one with
> >>> require = "conf" and one with require = "image" ... but no luck...
> >>>
> >>> Also adding a configurations section to scripts its file did not helped
> >>> (which will not prevent the problem sourcing a not signed script)
> >>
> >> As you discovered, you must either have required = "image", in which case
> >>
> >>     source :
> >>
> >> will be secure. Otherwise, you must use
> >>
> >>     source \#
> >>
> >> Any other way is not secure.
> >
> > My "hack" checks a configuration signature in fitimage with script in it...
> > so also "secure" ...
> >
> > BTW: why we need a env variable to enable checking in cmd/source.c?
> >      I would say, if verify fit images is enabled we always should check
> >      signature ... but this is another question...
>
> I think it's to allow disabling things for debugging. If the variable
> does not exist, it defaults to verifying.
>
> > So I tried your suggestion:
> >
> > => tftp 100000 script.bin.signed;setenv verify 1;source \#100000
> > Speed: 1000, full duplex
> > Using ethernet at 24000 device
> > TFTP from server 192.168.3.1; our IP address is 192.168.3.40
> > Filename 'script.bin.signed'.
> > Load address: 0x100000
> > Loading: #
> >          233.4 KiB/s
> > done
> > Bytes transferred = 1679 (68f hex)
> > ## Executing script at 00000000
> > Wrong image format for "source" command
> > =>
> >
> > same for
> >
> > => source \#100000:script-1
> > ## Executing script at 00000000
> > Wrong image format for "source" command
> > =>
> >
> > Which is the error message from the switch in image_source_script()
> > from cmd/source.c ...
>
> Right.
>
> What kind of image is your script? Do you have CONFIG_FIT (and *only*
> CONFIG_FIT) enabled?
>
> > (check if fitimage "is okay"):
> > => source 100000
> > ## Executing script at 00100000
> > sha256+ sha256,rsa2048:dev+ Hallo from script
> > => source 100000:script-1
> > ## Executing script at 00100000
> > sha256+ sha256,rsa2048:dev+ Hallo from script
> > => source 100000:script-2
> > ## Executing script at 00100000
> > Can't find 'script-2' FIT subimage
> > =>
> >
> > and changing hash in fitimages signature leads to:
> > => mw 1001c0 0 1
> > => source 100000:script-1
> > ## Executing script at 00100000
> > sha256+ sha256,rsa2048:dev- Hallo from script
> > =>
> >
> > As I described ... problem "hash is detected, but script is executed",
> > as public key in u-boots dtb has required = "conf"; (as it is used also
> > for fitimage boot, where we use conf signing)
> >
> > May you have an example (u-boot.dtb, its and complete working command
> > for a signed fitimage script)?
>
> $ cat << EOF >> dm-verity.its
> /dts-v1/;
>
> / {
>     description = "dm-verity boot parameters";
>     #address-cells = <1>;
>
>     images {
>         dm-verity {
>             data = /incbin/("dm-verity.scr");
>             type = "script";
>             arch = "arm64";
>             compression = "none";
>             hash-1 {
>                 algo = "sha256";
>             };
>         };
>     };
>
>     configurations {
>         default = "conf";
>         conf {
>             description = "Load dm-verity boot parameters";
>             script = "dm-verity";
>             signature {
>                 algo = "sha256,rsa2048";
>                 key-name-hint = "u-boot";
>                 sign-images = "script";
>             };
>         };
>     };
> };
> EOF
> $ uboot-mkimage -EB 0x40 -f dm-verity.its dm-verity.itb
> $ uboot-mkimage -EB 0x40 -r -F -k /path/to/keys dm-verity.itb
>
> > The main problem is (I think) that we check for fitimages which are
> > used for booting kernels, a "signed configuration" and in fitimage for scripts
> > only "image" signatures ... and a combination of both is not possible
> > (except I also sign the image nodes in kernel fitimage too ... which
> > than leads in checking configuration signature and image signature on
> > boot... but may a way to go (and disabling hash check) ?
>
> Yes, which is why you need to have # in the source command to force only
> using configurations. IMO we should also check image-only FITs, but
> Simon disagrees.

I'm not sure what I disagree with, exactly. Signing images leaves one
open to a mix-and-match attack, so I believe it is better to put the
script into the config so it is signed along with the kernel, etc. But
I am fine with us supporting signed images containing scripts...in
fact I thought we already did?

Regards,
Simon