MCUBOOT Hardware Key

[Jasonho]

Hi,

I trying to enable Hardware Key for mcuboot.

I do the following at mcuboot.conf

CONFIG_BOOT_HW_KEY=y

But I got the following error:

C:/nrfconnect/v1.5.0/bootloader/mcuboot/boot/zephyr/keys.c:70:21: error: conflicting types for 'bootutil_keys'        
   70 | struct bootutil_key bootutil_keys[1] = {
      |                     ^~~~~~~~~~~~~
In file included from C:/nrfconnect/v1.5.0/bootloader/mcuboot/boot/zephyr/keys.c:20:
C:/nrfconnect/v1.5.0/bootloader/mcuboot/boot/bootutil/include/bootutil/sign_key.h:38:34: note: previous declaration of 'bootutil_keys' was here
   38 | extern const struct bootutil_key bootutil_keys[];
      |                                  ^~~~~~~~~~~~~

Do I miss something? Thanks,

 

[hyper]

This looks to me to be a bug in keys.c, because sign_key.h is being #include'd before mcuboot_config.h rather than after it.

As a result, MCUBOOT_HW_KEY is not defined when sign_key.h is processed at the start of compiling keys.c.

If you swap those two #includes around, you should get further, and will then have to figure out the best way to incorporate an implementation of boot_retrieve_public_key_hash() into MCUBoot when it is built. I am not yet sure the best way to do that without having to maintain our own fork of the sdk-nrf repo (which we are trying hard to avoid).

[hyper]

BTW, I am referring to a recent version of sdk-nrf main branch in my posts here.

I think that a similar #include order issue may also be present in single_loader.c, because, for example, bootutil_priv.h depends on MCUBOOT_ENC_IMAGES. But that appears to be set in mcuboot_config.h, which is included later.

I'm thinking I must be wrong about this though, as surely these options have been tested by someone upstream?

[Sigurd]

Hi,

hyper said:

This looks to me to be a bug in keys.c, because sign_key.h is being #include'd before mcuboot_config.h rather than after it.

The issue is resolved upstream: https://github.com/mcu-tools/mcuboot/pull/1156

We have opened a PR to include the fix in our fork here: https://github.com/nrfconnect/sdk-mcuboot/pull/190

[hyper]

Thanks, that's good to see.

AFAICT, there is still the same sort of bug in single_loader.c that hasn't been fixed upstream yet.

[Sigurd]

hyper said:

AFAICT, there is still the same sort of bug in single_loader.c that hasn't been fixed upstream yet.

hyper said:

I think that a similar #include order issue may also be present in single_loader.c, because, for example, bootutil_priv.h depends on MCUBOOT_ENC_IMAGES. But that appears to be set in mcuboot_config.h, which is included later.

Are you able to trigger an issue here? I think this should be ok, since bootutil_priv.h includes mcuboot_config.h

[hyper]

Yeah, sorry about wasting your time there - I was in a rush and I just looked at the code rather than trying to compile it, and I missed the additional mcuboot_config.h #include in bootutil_priv.h.