Support for OTP Flash as trust anchor for keystore #449
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danielinux
commented
May 10, 2024
| @@ -200,7 +200,7 @@ const char Keystore_API[] = | |||
| "uint32_t keystore_get_mask(int id)\n" | |||
| "{\n" | |||
| " if (id >= keystore_num_pubkeys())\n" | |||
| " return -1;\n" | |||
| " return 0;\n" | |||
There was a problem hiding this comment.
This looked like a bug in the generated software keystore, a mask with all "1" being returned for an invalid slot. Spotted while writing the API for the OTP trust anchor.
danielinux
force-pushed
the
stm32h7_otp
branch
2 times, most recently
from
53ff0ad
to
268f187
Compare
+ added non-secure area at boot for OTP to read trust anchor if OTP feature is enabled.
|
Tested provisioning on a new STM32H563. OTP keystore working as expected. Ready for review. |
dgarske
requested changes
May 21, 2024
tools/keytools/otp/Makefile
Outdated
| @$(OBJCOPY) -O binary $(^) $(@) | ||
|
|
||
| otp-keystore-primer.elf: $(PRI_KS_OBJS) | ||
| @$(CC) -o otp-keystore-primer.elf $(LDFLAGS) $(CFLAGS) $(PRI_KS_OBJS) |
There was a problem hiding this comment.
Might consider the V=1 logic and $(Q) logic.
Got a warning about missing entry symbol _start. Still built fine, but is the default entry at 0x8000 really okay?
% make clean && make
arm-none-eabi-gcc -D"__WOLFBOOT" -Werror -Wextra -Wno-array-bounds -mthumb -mlittle-endian -mthumb-interwork -DARCH_ARM -Ihal -mcpu=cortex-m33 -DCORTEX_M33 -mcmse -DWOLFCRYPT_SECURE_MODE -DWOLFSSL_SP_ASM -DWOLFSSL_SP_ARM_CORTEX_M_ASM -DTZEN -DARCH_FLASH_OFFSET=0x08000000 -DWOLFBOOT_DUALBOOT -DWOLFBOOT_ORIGIN=0x0C000000 -DBOOTLOADER_PARTITION_SIZE= -DWOLFBOOT_ARCH_ARM -DTARGET_stm32h5 -D"FLASH_OTP_KEYSTORE" -D"WOLFBOOT_SIGN_ECC256" -D"FILL_BYTE=0xFF" -D"DUALBANK_SWAP=1" -D"NVM_FLASH_WRITEONCE" -Os -D"WOLFBOOT_NO_MPU" -DWOLFBOOT_VERSION=1 -DSECURE_PKCS11 -DWOLFSSL_PKCS11_RW_TOKENS -DCK_CALLABLE="__attribute__((cmse_nonsecure_entry))" -Ilib/wolfPKCS11 -DWP11_HASH_PIN_COST=3 -D"WOLFBOOT_HASH_SHA256" -DIMAGE_HEADER_SIZE=256 "-Wstack-usage=16688" -I"." -I"include/" -I"lib/wolfssl" -Wno-array-bounds -D"WOLFSSL_USER_SETTINGS" -D"WOLFTPM_USER_SETTINGS" -D"PLATFORM_stm32h5" -Wall -Wextra -Wno-main -ffreestanding -Wno-unused -nostartfiles -ffunction-sections -fdata-sections -fomit-frame-pointer -mthumb -mlittle-endian -mthumb-interwork -mcpu=cortex-m33 -Wl,-gc-sections -Wl,-Map=wolfboot.map -ffreestanding -nostartfiles tools/keytools/otp/otp-keystore-primer.c -o tools/keytools/otp/otp-keystore-primer
/usr/local/Cellar/gcc-arm-none-eabi/20200630/bin/../lib/gcc/arm-none-eabi/9.3.1/../../../../arm-none-eabi/bin/ld: warning: cannot find entry symbol _start; defaulting to 0000000000008000
[CC-ARM] hal/stm32h5.o
There was a problem hiding this comment.
This was an actual issue with otp-keystore-primer build, not selecting the right linker script
include/wolfboot/wolfboot.h
Outdated
| @@ -175,6 +175,10 @@ extern "C" { | |||
| #endif | |||
| #endif | |||
|
|
|||
| #endif | |||
|
|
|||
| #if defined __WOLFBOOT || defined __FLASH_OTP_PRIMER | |||
There was a problem hiding this comment.
Maybe use defined(__WOLFBOOT) || with parenthesis for consistency?
include/otp_keystore.h
Outdated
|
|
||
| #define OTP_HDR_SIZE 16 | ||
|
|
||
| struct __attribute__((packed)) wolfBoot_otp_hdr { |
There was a problem hiding this comment.
Eventually I'd like to see our use of PACKED come from a macro, so it's not just GCC.
#if (defined(__IAR_SYSTEMS_ICC__) && (__IAR_SYSTEMS_ICC__ > 8)) || \
defined(__GNUC__)
#define WOLFBOOT_PACK __attribute__ ((packed))
#else
#define WOLFBOOT_PACK
#endif
+ separate object for hal in otp-keystore-primer
dgarske
approved these changes
May 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goals:
keygentool generates a one-time provisioning mechanism(re)based on PR #400