[U-Boot] [PATCH] common, image-sig: [BUG?] if no valid signature node found, do not boot signed FIT image

[Simon Glass]

Hi Heiko,

On 8 June 2017 at 03:52, Heiko Schocher <hs at denx.de> wrote:
> fit_image_verify_required_sigs() must return != 0, on error.
>
> When fit_image_verify_required_sigs() does not find a signature
> node, it returns 0, which leads in booting a signed FIT image.
>
> Fix this!
>
> Signed-off-by: Heiko Schocher <hs at denx.de>
> ---
>
> Found on an imx28 based board, with key dtb appended to u-boot.bin.
>
> Booting signed FIT image without an valid key dtb appended to u-boot.bin
> shows:
>
> Using FEC1 device
> TFTP from server 192.168.1.1; our IP address is 192.168.20.103
> Filename '/tftpboot/xxx/20170509/signed.fit'.
> Load address: 0x42000000
> Loading: #################################################################
> [...]
>          ###########
>          3.3 MiB/s
> done
> Bytes transferred = 12560801 (bfa9a1 hex)
>    Using 'conf at 1' configuration
>    Verifying Hash Integrity ... OK
>    Trying 'kernel at 1' kernel subimage
>      Description:  Linux kernel
>      Created:      2017-06-08   9:10:14 UTC
>      Type:         Kernel Image
>      Compression:  uncompressed
>      Data Start:   0x420000c0
>      Data Size:    4078928 Bytes = 3.9 MiB
>      Architecture: ARM
>      OS:           Linux
>      Load Address: 0x40008000
>      Entry Point:  0x40008000
>      Hash algo:    sha256
>      Hash value:   6d1dce3e08133ac4d34c0e07ce266f5cffc6f5a2713619c9ff76ca4b04df4a5b
>      Sign algo:    sha256,rsa4096:dev
>      Sign value:   xxx
>      Timestamp:    2017-06-08   9:10:15 UTC
>    Verifying Hash Integrity ... sha256+ sha256,rsa4096:dev- OK
>                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
>    Using 'conf at 1' configuration
>    Trying 'ramdisk at 1' ramdisk subimage
>      Description:  miro ramdisk
>      Created:      2017-06-08   9:10:14 UTC
>      Type:         RAMDisk Image
>      Compression:  gzip compressed
>      Data Start:   0x423e92b0
>      Data Size:    8457506 Bytes = 8.1 MiB
>      Architecture: ARM
>      OS:           Linux
>      Load Address: 0x00000000
>      Entry Point:  0x00000000
>      Hash algo:    sha256
>      Hash value:   da60884efa4373e7003940a56271c326f159ff74356ded28d8ebe108af807cda
>      Sign algo:    sha256,rsa4096:dev
>      Sign value:   xxx
>      Timestamp:    2017-06-08   9:10:15 UTC
>    Verifying Hash Integrity ... sha256+ sha256,rsa4096:dev- OK
>                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>    Using 'conf at 1' configuration
>    Trying 'fdt at 1' fdt subimage
>      Description:  miro device tree
>      Created:      2017-06-08   9:10:14 UTC
>      Type:         Flat Device Tree
>      Compression:  uncompressed
>      Data Start:   0x423e41b4
>      Data Size:    19852 Bytes = 19.4 KiB
>      Architecture: ARM
>      Hash algo:    sha256
>      Hash value:   9b39c3ab6227bb8f0bcebc0bb64439248a6670dfe873bb1c6536764e0dc1623c
>      Sign algo:    sha256,rsa4096:dev
>      Sign value:   xxx
>      Timestamp:    2017-06-08   9:10:15 UTC
>    Verifying Hash Integrity ... sha256+ sha256,rsa4096:dev- OK
>                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>    Booting using the fdt blob at 0x423e41b4
>    Loading Kernel Image ... OK
>    Loading Ramdisk to 47303000, end 47b13d22 ... OK
>    Loading Device Tree to 472fb000, end 47302d8b ... OK
>
> Starting kernel ...
>
> [    0.000000] Booting Linux on physical CPU 0x0
> [    0.000000] Linux version 4.11.0 (hs at hercules.denx.de) (gcc version 6.2.0 (GCC) ) #1 Tue May 16 07:31:30 CEST 2017
> [...]
>
> And linux boots ... which is bad, as we have no valid key appended
> to u-boot.bin ...
>
> With this patch it shows:
>
>    Using 'conf at 1' configuration
>    Verifying Hash Integrity ... OK
>    Trying 'kernel at 1' kernel subimage
>      Description:  Linux kernel
>      Created:      2017-06-08   9:34:28 UTC
>      Type:         Kernel Image
>      Compression:  uncompressed
>      Data Start:   0x420000c0
>      Data Size:    4078928 Bytes = 3.9 MiB
>      Architecture: ARM
>      OS:           Linux
>      Load Address: 0x40008000
>      Entry Point:  0x40008000
>      Hash algo:    sha256
>      Hash value:   6d1dce3e08133ac4d34c0e07ce266f5cffc6f5a2713619c9ff76ca4b04df4a5b
>      Sign algo:    sha256,rsa4096:dev
>      Sign value:   xxx
>      Timestamp:    2017-06-08   9:34:29 UTC
>    Verifying Hash Integrity ... error!
> Unable to verify required signature for '' hash node in 'kernel at 1' image node
> Bad Data Hash
> ERROR: can't get kernel image!
> =>
>
>
>  common/image-sig.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/common/image-sig.c b/common/image-sig.c
> index 455f2b9..646fb08 100644
> --- a/common/image-sig.c
> +++ b/common/image-sig.c
> @@ -265,7 +265,7 @@ int fit_image_verify_required_sigs(const void *fit, int image_noffset,
>         if (sig_node < 0) {
>                 debug("%s: No signature node found: %s\n", __func__,
>                       fdt_strerror(sig_node));
> -               return 0;
> +               return 1;

Thanks for finding/fixing this! I suggest returning -EPERM.

Also note that using image-based security is somewhat insecure since
people can mix and match them. Configuration signing is preferred if
you can do it.

As Tom said, can you add a test please?

>         }
>
>         fdt_for_each_subnode(noffset, sig_blob, sig_node) {
> --
> 2.7.4
>

Regards,
Simon