Signature on BL updated by DFU

[sjerlhagen]

There doesn't seem to be any validation of the image when updating the bootloader by secure DFU. nrfutil has no switch to generate any crc or ecdsa_signature. The code uses VALIDATE_CRC, but the expected value is calculated on the received image in boot_validate_extract, so when this is later verified in nrf_dfu_validation_boot_validate the result is always success.

There is also a bug in postvalidate_sd_bl causing the boot validation of BL to always fail for a combined SD+BL image. The second call to boot_validation_extract should have index=1 if there was also a SD in the image.

I'm using nRf52832 with SDK 15.3.0.

Best Regards

Sven

[sjerlhagen]

For a bootloader update I only see the ECDSA signature of the init packet being verified before download, I cant find any verification of the image
(except for verifying CRC for the objects to ensure correct transfer). Are you saying there is ECDSA signature check for the bootloader code at every startup ? My concern is with intentional tampering with the bootloader image in a signed DFU package. Before activating the bootloader image it should be authenticated.

When I generate a DFU package with the command:
nrfutil pkg generate --key-file private.key --sd-boot-validation VALIDATE_ECDSA_P256_SHA256 --bootloader-version 2 --hw-version 52 --sd-req 0xB7 --softdevice sd.hex --bootloader bl.hex sd_bl.zip
The binary consist of first SD then BL.

The entire image is first transfered. Then the SD part is verified:

boot_validation_extract(... index=0, sd_start, sd_len ...)  p_init->boot_validation[0] = VALIDATE_ECDSA_P256_SHA256 which is what was specified in the init packet. This succeeds.

10 lines down the BL the BL part is verified:

boot_validation_extract(... index=0, bl_start, bl_len ...)  In this function p_init->boot_validation[0] is still VALIDATE_ECDSA_P256_SHA256, and the signature of the bootloader image is compared to the expected signature of the SD (boot_validation[0].bytes.bytes). This fails.

[Einar Thorsrud]

Hi,

sjerlhagen said:

For a bootloader update I only see the ECDSA signature of the init packet being verified before download, I cant find any verification of the image
(except for verifying CRC for the objects to ensure correct transfer).

Yes, the init packet itself is validated before the firmware is downloaded. But the init packet is also used to validate the received firmware before it is activated. In details:

• When a whole firmware image is received on_data_obj_execute_request_sched() in nrf_dfu_req_handler.c calls nrf_dfu_validation_activation_prepare() in dfu_validation.c.
• nrf_dfu_validation_activation_prepare() calls postvalidate(),
• which calls fw_hash_ok(), and this takes both the init packet and received data as input. It uses nrf_dfu_validation_hash_ok() and only returns true if the hash of the received firmware matches the hash in the signed init packet (which was validated at the beginning of the DFU process).
• Subsequently, postvalidate_sd_bl() is called, but at that point, the received firmware is already validated as described above.

sjerlhagen said:

Are you saying there is ECDSA signature check for the bootloader code at every startup ?

No. (But you can optionally do that for the application.)

sjerlhagen said:

My concern is with intentional tampering with the bootloader image in a signed DFU package. Before activating the bootloader image it should be authenticated.

Absolutely. That is handled by the bootloader.

sjerlhagen said:

boot_validation_extract(... index=0, bl_start, bl_len ...)  In this function p_init->boot_validation[0] is still VALIDATE_ECDSA_P256_SHA256, and the signature of the bootloader image is compared to the expected signature of the SD (boot_validation[0].bytes.bytes). This fails.

It should not be so. Can you show all the modifications you have done to the bootloader?

[sjerlhagen]

I see, I did not catch this additional whole image hash verification. This is ok.

In postvalidate_sd_bl I did the following changes:

Added  uint8_t  index = 0;

Changed the bold parts below in the postvalidate_sd_bl function:

if (!boot_validation_extract(&boot_validation_sd, p_init, index, start_addr, p_init->sd_size, VALIDATE_CRC))
{
return false;
}

bl_start += p_init->sd_size;
bl_size -= p_init->sd_size;
index++;
}
if (with_bl)
{
if (!boot_validation_extract(&boot_validation_bl, p_init, index, bl_start, bl_size, NO_VALIDATION))

[Einar Thorsrud]

I see. Please revert your change. This works with the unmodified bootloader.

[jlubawy]

Please don't close this issue yet.

I ran into this same exact problem, where if I try to update the SD + BL at the same time the call to "boot_validation_extract" passes for SD but fails for BL. This is because as sjerlhagen pointed out, the code is looking at the wrong index (always looking at index zero).

The only way I was able to get past this was to implement the fix he suggested where index starts at zero and is incremented.

Here is what my init packet looks like, index = 0 is for SD, and index = 1 is for BL:

boot_validation_count = 2,
boot_validation = {
    {
      type = DFU_VALIDATION_TYPE_VALIDATE_ECDSA_P256_SHA256,
      bytes = {
        size = 64, 
        bytes = "<some bytes>",
      },
    },
    {
      type = DFU_VALIDATION_TYPE_VALIDATE_GENERATED_CRC,
      bytes = {
        size = 0,
        bytes = '\000' <repeats 63 times>
      },
    },
}

I think this bug would be hidden unless "VALIDATE_ECDSA_P256_SHA256" is used for SD, otherwise the SD and BL would end up using the same validation. Can you elaborate on how you determined it "works as expected"? Did you try the same configuration?

[jlubawy]

By the way I am using nRF52 SDK v15.3.0, but I checked and the firmware bug mentioned above is still present in v16.0.0.

I tried updating the SD+BL using nRF Connect on both iOS and Android, all failed due to this firmware bug.

I believe the only option to workaround this bug is to not use nrfutil pkg generate --sd-boot-validation VALIDATE_ECDSA_P256_SHA256 until the bug is fixed in an update version of the bootloader.

[Einar Thorsrud]

Hi,

Thank you for updating this thread. You are right, this is a bug. I have created an internal ticket and discussed it with the SDK team so that it can be fixed in a future release.