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

[Heiko Schocher <hs@denx.de>]

Hello Simon,

Am 09.06.2017 um 05:05 schrieb Simon Glass:
> Hi Heiko,
>
> On 8 June 2017 at 03:52, Heiko Schocher <hs@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@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:
[...]
>>   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.

Ok, changed.

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

I do this, here my configurations node from the its file:

         configurations {
                 default = "conf at 1";
                 conf at 1 {
                         description = "board config 1";
                         kernel = "kernel at 1";
                         fdt = "fdt at 1";
                         ramdisk = "ramdisk at 1";
                         signature at 1 {
                                 algo = "sha256,rsa4096";
                                 key-name-hint = "dev";
                         };
                 };
         };

> As Tom said, can you add a test please?

Hmm... tried with current U-Boot, the steps described in

test/image/test-fit.py

# make O=sandbox sandbox_config
# make O=sandbox
# ./test/image/test-fit.py -u sandbox/u-boot

and get:

pollux:u-boot hs [master] $ ./test/image/test-fit.py -u sandbox/u-boot
FIT Tests
=========
Warning (unit_address_vs_reg): Node /reset at 0 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /images/kernel at 1 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /images/kernel at 2 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /images/fdt at 1 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /images/fdt at 1/signature at 1 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /images/ramdisk at 1 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /images/ramdisk at 2 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /configurations/conf at 1 has a unit name, but no reg property
Kernel load


U-Boot 2017.07-rc1-00997-gad701b1 (Jun 09 2017 - 06:18:46 +0200)

DRAM:  128 MiB
MMC:
Using default environment

In:    serial
Out:   serial
Err:   serial
SCSI:  Net:   No ethernet found.
IDE:   Bus 0: not available
18474 bytes read in 0 ms
## Loading kernel from FIT Image at 00001000 ...
    Using 'conf at 1' configuration
    Verifying Hash Integrity ... OK
    Trying 'kernel at 1' kernel subimage
      Description:  unavailable
      Created:      2017-06-09   4:19:13 UTC
      Type:         Kernel Image
      Compression:  uncompressed
      Data Start:   0x000010c8
      Data Size:    3491 Bytes = 3.4 KiB
      Architecture: Sandbox
      OS:           Linux
      Load Address: 0x00040000
      Entry Point:  0x00000008
    Verifying Hash Integrity ... OK
## Loading fdt from FIT Image at 00001000 ...
    Using 'conf at 1' configuration
    Trying 'fdt at 1' fdt subimage
      Description:  snow
      Created:      2017-06-09   4:19:13 UTC
      Type:         Flat Device Tree
      Compression:  uncompressed
      Data Start:   0x00002d30
      Data Size:    193 Bytes = 193 Bytes
      Architecture: Sandbox
      Sign algo:    sha1,rsa2048:dev
      Sign value:   unavailable
      Timestamp:    unavailable
    Verifying Hash Integrity ... sha1,rsa2048:dev- OK
    Booting using the fdt blob at 0x002d30
    Loading Kernel Image ... OK
3491 bytes written in 0 ms
193 bytes written in 0 ms
4591 bytes written in 0 ms
3491 bytes written in 0 ms
4591 bytes written in 0 ms

Expected '%s' but not found in output:


U-Boot 2017.07-rc1-00997-gad701b1 (Jun 09 2017 - 06:18:46 +0200)

DRAM:  128 MiB
MMC:
Using default environment

In:    serial
Out:   serial
Err:   serial
SCSI:  Net:   No ethernet found.
IDE:   Bus 0: not available
18474 bytes read in 0 ms
## Loading kernel from FIT Image at 00001000 ...
    Using 'conf at 1' configuration
    Verifying Hash Integrity ... OK
    Trying 'kernel at 1' kernel subimage
      Description:  unavailable
      Created:      2017-06-09   4:19:13 UTC
      Type:         Kernel Image
      Compression:  uncompressed
      Data Start:   0x000010c8
      Data Size:    3491 Bytes = 3.4 KiB
      Architecture: Sandbox
      OS:           Linux
      Load Address: 0x00040000
      Entry Point:  0x00000008
    Verifying Hash Integrity ... OK
## Loading fdt from FIT Image at 00001000 ...
    Using 'conf at 1' configuration
    Trying 'fdt at 1' fdt subimage
      Description:  snow
      Created:      2017-06-09   4:19:13 UTC
      Type:         Flat Device Tree
      Compression:  uncompressed
      Data Start:   0x00002d30
      Data Size:    193 Bytes = 193 Bytes
      Architecture: Sandbox
      Sign algo:    sha1,rsa2048:dev
      Sign value:   unavailable
      Timestamp:    unavailable
    Verifying Hash Integrity ... sha1,rsa2048:dev- OK
    Booting using the fdt blob at 0x002d30
    Loading Kernel Image ... OK
3491 bytes written in 0 ms
193 bytes written in 0 ms
4591 bytes written in 0 ms
3491 bytes written in 0 ms
4591 bytes written in 0 ms

Traceback (most recent call last):
   File "./test/image/test-fit.py", line 481, in <module>
     run_tests()
   File "./test/image/test-fit.py", line 470, in run_tests
     run_fit_test(mkimage, options.u_boot)
   File "./test/image/test-fit.py", line 395, in run_fit_test
     line = find_matching(stdout, 'Booting using the FDT blob at ')
   File "./test/image/test-fit.py", line 286, in find_matching
     raise ValueError('Test aborted')
ValueError: Test aborted

:-(

With my patch:
pollux:u-boot hs [master] $ git diff
diff --git a/common/image-sig.c b/common/image-sig.c
index 455f2b9..e5ba85a 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 -EPERM;
         }

         fdt_for_each_subnode(noffset, sig_blob, sig_node) {
pollux:u-boot hs [master] $ ./test/image/test-fit.py -u sandbox/u-boot
FIT Tests
=========
Warning (unit_address_vs_reg): Node /reset at 0 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /images/kernel at 1 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /images/kernel at 2 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /images/fdt at 1 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /images/fdt at 1/signature at 1 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /images/ramdisk at 1 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /images/ramdisk at 2 has a unit name, but no reg property
Warning (unit_address_vs_reg): Node /configurations/conf at 1 has a unit name, but no reg property
Kernel load


U-Boot 2017.07-rc1-00997-gad701b1-dirty (Jun 09 2017 - 06:21:36 +0200)

DRAM:  128 MiB
MMC:
Using default environment

In:    serial
Out:   serial
Err:   serial
SCSI:  Net:   No ethernet found.
IDE:   Bus 0: not available
18474 bytes read in 1 ms (17.6 MiB/s)
## Loading kernel from FIT Image at 00001000 ...
    Using 'conf at 1' configuration
    Verifying Hash Integrity ... OK
    Trying 'kernel at 1' kernel subimage
      Description:  unavailable
      Created:      2017-06-09   4:22:07 UTC
      Type:         Kernel Image
      Compression:  uncompressed
      Data Start:   0x000010c8
      Data Size:    3491 Bytes = 3.4 KiB
      Architecture: Sandbox
      OS:           Linux
      Load Address: 0x00040000
      Entry Point:  0x00000008
    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!
    XIP Invalid Image ... OK
3491 bytes written in 0 ms
193 bytes written in 0 ms
4591 bytes written in 0 ms
3491 bytes written in 0 ms
4591 bytes written in 0 ms



U-Boot 2017.07-rc1-00997-gad701b1-dirty (Jun 09 2017 - 06:21:36 +0200)

DRAM:  128 MiB
MMC:
Using default environment

In:    serial
Out:   serial
Err:   serial
SCSI:  Net:   No ethernet found.
IDE:   Bus 0: not available
18474 bytes read in 1 ms (17.6 MiB/s)
## Loading kernel from FIT Image at 00001000 ...
    Using 'conf at 1' configuration
    Verifying Hash Integrity ... OK
    Trying 'kernel at 1' kernel subimage
      Description:  unavailable
      Created:      2017-06-09   4:22:07 UTC
      Type:         Kernel Image
      Compression:  uncompressed
      Data Start:   0x000010c8
      Data Size:    3491 Bytes = 3.4 KiB
      Architecture: Sandbox
      OS:           Linux
      Load Address: 0x00040000
      Entry Point:  0x00000008
    Verifying Hash Integrity ...  error!
Unable to verify required signature for '' hash node in 'kernel@1' image node
Bad Data Hash
ERROR: can't get kernel image!
    XIP Invalid Image ... OK
3491 bytes written in 0 ms
193 bytes written in 0 ms
4591 bytes written in 0 ms
3491 bytes written in 0 ms
4591 bytes written in 0 ms

Traceback (most recent call last):
   File "./test/image/test-fit.py", line 481, in <module>
     run_tests()
   File "./test/image/test-fit.py", line 470, in run_tests
     run_fit_test(mkimage, options.u_boot)
   File "./test/image/test-fit.py", line 388, in run_fit_test
     fail('Kernel not loaded', stdout)
   File "./test/image/test-fit.py", line 306, in fail
     raise ValueError("Test '%s' failed: %s" % (test_name, msg))
ValueError: Test 'Kernel load' failed: Kernel not loaded
pollux:u-boot hs [master] $

Can you verify this?

Thanks!

bye,
Heiko
>
>>          }
>>
>>          fdt_for_each_subnode(noffset, sig_blob, sig_node) {
>> --
>> 2.7.4
>>
>
> Regards,
> Simon
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany