BL2: Update key id mapping for built in keys

For MCUBOOT_BUILTIN_KEY, create config for secure and non-secure key id.
This will be added to image as separate TLV entry. It will be parsed by
mcuboot during verification and will be used to load key from OTP.

Signed-off-by: Maulik Patel <maulik.patel@arm.com>
Change-Id: Ia6148a993c908d0306461e91e449a0582fecaeaa

Author: maulik-arm

bl2/ext/mcuboot/CMakeLists.txt

@@ -77,6 +77,9 @@ target_sources(bl2
 target_compile_definitions(bl2
     PRIVATE
         MCUBOOT_SIGNATURE_TYPE=${MCUBOOT_SIGNATURE_TYPE}
+        $<$<BOOL:${MCUBOOT_BUILTIN_KEY}>:TFM_S_KEY_ID=${TFM_S_KEY_ID}>
+        $<$<BOOL:${MCUBOOT_BUILTIN_KEY}>:TFM_NS_KEY_ID=${TFM_NS_KEY_ID}>
+        MCUBOOT_ROTPK_MAX_KEYS_PER_IMAGE=${MCUBOOT_ROTPK_MAX_KEYS_PER_IMAGE}
 )
 
 target_link_libraries(bl2
@@ -216,7 +219,6 @@ if (PLATFORM_DEFAULT_IMAGE_SIGNING)
     )
 
     if(MCUBOOT_BUILTIN_KEY)
-        set(TFM_S_KEY_ID 0)
         set(wrapper_args ${wrapper_args} --psa-key-ids ${TFM_S_KEY_ID})
     endif()
 

bl2/ext/mcuboot/keys.c

@@ -24,6 +24,7 @@
  * Modifications are Copyright (c) 2019-2024 Arm Limited.
  */
 
+#include <assert.h>
 #include <stddef.h>
 #include <stdbool.h>
 #include <bootutil/sign_key.h>
@@ -533,14 +534,17 @@ int boot_retrieve_public_key_hash(uint8_t image_index,
                                    (uint32_t *)key_hash_size);
 }
 #elif defined(MCUBOOT_BUILTIN_KEY)
+/* Maximum number of keys per image */
+#define MAX_KEYS_PER_IMAGE MCUBOOT_ROTPK_MAX_KEYS_PER_IMAGE
+
 /**
  * @note When using builtin keys the signature verification happens based on key IDs.
  *       During verification MCUboot feeds the image index as a key ID and it is the
  *       underlying crypto library's responsibility to do a mapping (if required)
  *       between the image indexes and the builtin key IDs. Therefore it only allows
- *       as many IDs as there are images.
+ *       as many IDs as there are images times number of keys per image.
  */
-const int bootutil_key_cnt = MCUBOOT_IMAGE_NUMBER;
+const int bootutil_key_cnt = (MCUBOOT_IMAGE_NUMBER * MAX_KEYS_PER_IMAGE);
 
 /**
  * @brief Loader function to retrieve the Root of Trust Public Key
@@ -570,10 +574,11 @@ static enum tfm_plat_err_t tfm_plat_get_bl2_rotpk(const void *ctx,
 {
     enum tfm_plat_err_t err;
     const tfm_plat_builtin_key_descriptor_t *descriptor = (const tfm_plat_builtin_key_descriptor_t *)ctx;
-    /* ToDo: sort out the mapping between Key IDs and image indexes at runtime */
-    uint32_t image_idx = descriptor->key_id - 1;
 
-    err = tfm_plat_otp_read(PLAT_OTP_ID_BL2_ROTPK_0 + image_idx, buf_len, buf);
+    assert(descriptor->key_id >= 0);
+
+    uint32_t otp_id = PLAT_OTP_ID_BL2_ROTPK_0 + descriptor->key_id;
+    err = tfm_plat_otp_read(otp_id, buf_len, buf);
     if (err != TFM_PLAT_ERR_SUCCESS) {
         return err;
     }
@@ -583,7 +588,7 @@ static enum tfm_plat_err_t tfm_plat_get_bl2_rotpk(const void *ctx,
      * encoding of the key (RSA) or alignment requirements of the stored key
      * material, which in OTP must be 4-bytes aligned
      */
-    err = tfm_plat_otp_get_size(PLAT_OTP_ID_BL2_ROTPK_0 + image_idx, key_len);
+    err = tfm_plat_otp_get_size(otp_id, key_len);
     if (err != TFM_PLAT_ERR_SUCCESS) {
         return err;
     }
@@ -625,67 +630,119 @@ static enum tfm_plat_err_t tfm_plat_get_bl2_rotpk(const void *ctx,
     return TFM_PLAT_ERR_SUCCESS;
 }
 
+typedef struct {
+    uint32_t key_id[MAX_KEYS_PER_IMAGE];  /*!< Key id of built in keys */
+}image_key_id_mapping_t;
+
+/* Platform specific image to key id (otp id offset) map */
+static const image_key_id_mapping_t tfm_image_key_map[] = {
+    {
+        /* Image 0: Only one key is provided and required */
+        .key_id = { TFM_S_KEY_ID },
+    },
+    {
+        /* Image 1: Only one key is provided and required */
+        .key_id = { TFM_NS_KEY_ID },
+    },
+#if (MCUBOOT_IMAGE_NUMBER > 2)
+    {
+        /* Image 2: Only one key is provided and required */
+        .key_id = { TFM_S_KEY_ID_3 },
+    },
+#endif /* MCUBOOT_IMAGE_NUMBER > 2 */
+#if (MCUBOOT_IMAGE_NUMBER > 3)
+    {
+        /* Image 3: Only one key is provided and required */
+        .key_id = { TFM_S_KEY_ID_4 },
+    },
+#endif /* MCUBOOT_IMAGE_NUMBER > 3 */
+};
+
+static int get_key_id(uint8_t img_idx, uint8_t key_idx)
+{
+    assert(img_idx < MCUBOOT_IMAGE_NUMBER);
+    assert(key_idx < MAX_KEYS_PER_IMAGE);
+    return tfm_image_key_map[img_idx].key_id[key_idx];
+}
+
 /* The policy table is built dynamically at runtime to allow for an arbitrary
  * number of MCUBOOT_IMAGE_NUMBER entries, without hardcoding at build time
  */
 size_t tfm_plat_builtin_key_get_policy_table_ptr(const tfm_plat_builtin_key_policy_t *policy_ptr[])
 {
-#if defined(MCUBOOT_BUILTIN_KEY)
-    static tfm_plat_builtin_key_policy_t policy_table[MCUBOOT_IMAGE_NUMBER];
+    static tfm_plat_builtin_key_policy_t policy_table[MCUBOOT_IMAGE_NUMBER][MAX_KEYS_PER_IMAGE];
     static bool policy_table_is_initalized = false;
 
     if (!policy_table_is_initalized) {
-        for (uint32_t idx = 0; idx < MCUBOOT_IMAGE_NUMBER; idx++) {
-            tfm_plat_builtin_key_policy_t policy = {
-                .key_id = idx + 1, /* ToDo: Relationship between key_id and image_idx is simply hardcoded */
-                .per_user_policy = 0,
-                .usage = PSA_KEY_USAGE_VERIFY_HASH,
-            };
-
-            policy_table[idx] = policy;
-        }
+        for (uint32_t i = 0; i < MCUBOOT_IMAGE_NUMBER; i++) {
+            for (uint32_t j = 0; j < MAX_KEYS_PER_IMAGE; j++) {
+                tfm_plat_builtin_key_policy_t policy = {
+                    .per_user_policy = 0,
+                    .usage = PSA_KEY_USAGE_VERIFY_HASH,
+                };
 
+                policy.key_id = get_key_id(i,j);
+                policy_table[i][j] = policy;
+            }
+        }
         policy_table_is_initalized = true;
     }
 
-    *policy_ptr = &policy_table[0];
-    return MCUBOOT_IMAGE_NUMBER;
-#else
-    *policy_ptr = NULL;
-    return 0;
-#endif
+    *policy_ptr = &policy_table[0][0];
+    return MCUBOOT_IMAGE_NUMBER * MAX_KEYS_PER_IMAGE;
+
 }
 
 /* The descriptor table is built dynamically at runtime to allow for an arbitrary
  * number of MCUBOOT_IMAGE_NUMBER entries, without hardcoding at build time
  */
 size_t tfm_plat_builtin_key_get_desc_table_ptr(const tfm_plat_builtin_key_descriptor_t *desc_ptr[])
 {
-#if defined(MCUBOOT_BUILTIN_KEY)
-    static tfm_plat_builtin_key_descriptor_t descriptor_table[MCUBOOT_IMAGE_NUMBER];
+    static tfm_plat_builtin_key_descriptor_t descriptor_table[MCUBOOT_IMAGE_NUMBER][MAX_KEYS_PER_IMAGE];
     static bool descriptor_table_is_initalized = false;
 
     if (!descriptor_table_is_initalized) {
-        for (uint32_t idx = 0; idx < MCUBOOT_IMAGE_NUMBER; idx++) {
-            tfm_plat_builtin_key_descriptor_t descriptor = {
-                .key_id = idx + 1, /* ToDo: Relationship between key_id and image_idx is simply hardcoded */
-                .slot_number = 0, /* Unused */
-                .lifetime = PSA_KEY_LIFETIME_PERSISTENT,
-                .loader_key_func = tfm_plat_get_bl2_rotpk,
-                .loader_key_ctx = &descriptor_table[idx],
-            };
-
-            descriptor_table[idx] = descriptor;
+        for (uint32_t i = 0; i < MCUBOOT_IMAGE_NUMBER; i++) {
+            for (uint32_t j = 0; j < MAX_KEYS_PER_IMAGE; j++) {
+                tfm_plat_builtin_key_descriptor_t descriptor = {
+                    .slot_number = 0, /* Unused */
+                    .lifetime = PSA_KEY_LIFETIME_PERSISTENT,
+                    .loader_key_func = tfm_plat_get_bl2_rotpk,
+                    .loader_key_ctx = &descriptor_table[i][j],
+                };
+                descriptor.key_id = get_key_id(i,j);
+                descriptor_table[i][j] = descriptor;
+            }
         }
 
         descriptor_table_is_initalized = true;
     }
 
-    *desc_ptr = &descriptor_table[0];
-    return MCUBOOT_IMAGE_NUMBER;
-#else
+    *desc_ptr = &descriptor_table[0][0];
+    return MCUBOOT_IMAGE_NUMBER * MAX_KEYS_PER_IMAGE;
+}
+
+int boot_verify_key_id_for_image(uint8_t image_index, int32_t key_id)
+{
+    for (int i = 0; i < MAX_KEYS_PER_IMAGE; i++) {
+        if (key_id == get_key_id(image_index, i)) {
+            return 0;
+        }
+    }
+    /* If the key id is not found in the image key id mapping, return -1 */
+    return -1;
+}
+
+#else /* MCUBOOT_BUILTIN_KEY */
+size_t tfm_plat_builtin_key_get_policy_table_ptr(const tfm_plat_builtin_key_policy_t *policy_ptr[])
+{
+    *policy_ptr = NULL;
+    return 0;
+}
+
+size_t tfm_plat_builtin_key_get_desc_table_ptr(const tfm_plat_builtin_key_descriptor_t *desc_ptr[])
+{
     *desc_ptr = NULL;
     return 0;
-#endif
 }
-#endif /* MCUBOOT_USE_PSA_CRYPTO */
+#endif /* MCUBOOT_BUILTIN_KEY */

bl2/ext/mcuboot/mcuboot_default_config.cmake

@@ -55,6 +55,9 @@ set(MCUBOOT_SIGNATURE_TYPE              "RSA-3072"       CACHE STRING    "Algori
 set(MCUBOOT_GENERATE_SIGNING_KEYPAIR    OFF              CACHE BOOL      "Generate new keypair for signing and use that instead of MCUBOOT_KEY_S and MCUBOOT_KEY_NS")
 set(MCUBOOT_KEY_S                       "${CMAKE_SOURCE_DIR}/bl2/ext/mcuboot/root-${MCUBOOT_SIGNATURE_TYPE}.pem" CACHE FILEPATH "Path to key with which to sign secure binary")
 set(MCUBOOT_KEY_NS                      "${CMAKE_SOURCE_DIR}/bl2/ext/mcuboot/root-${MCUBOOT_SIGNATURE_TYPE}_1.pem" CACHE FILEPATH "Path to key with which to sign non-secure binary")
+set(TFM_S_KEY_ID                        0                CACHE STRING    "Key ID of the key used to sign the secure image")
+set(TFM_NS_KEY_ID                       1                CACHE STRING    "Key ID of the key used to sign the non-secure image")
+set(MCUBOOT_ROTPK_MAX_KEYS_PER_IMAGE    1                CACHE STRING    "Maximum number of RoTPK keys per image to be used in BL2")
 
 if (MCUBOOT_SIGNATURE_TYPE STREQUAL EC-P384)
     set(MCUBOOT_ROTPK_HASH_ALG          SHA384           CACHE STRING "Algoritm to use to hash mcuboot ROTPKs")

bl2/ext/mcuboot/scripts/wrapper/wrapper.py

@@ -88,7 +88,7 @@
               'the image manifest: full key or hash of the key.')
 @click.option('--psa-key-ids', multiple=True, type=int, required=False,
               help='List of integer key IDs for each signature.')
-@click.option('-k', '--key', metavar='filename')
+@click.option('-k', '--key', multiple=True, metavar='filename')
 @click.command(help='''Create a signed or unsigned image\n
                INFILE and OUTFILE are parsed as Intel HEX if the params have
                .hex extension, otherwise binary format is used''')
@@ -130,16 +130,23 @@ def wrap(key, align, version, header_size, pad_header, layout, pad, confirm,
                               max_align=max_align)
 
     img.load(infile)
-    img.set_key_ids(psa_key_ids)
-    key = imgtool.main.load_key(key) if key else None
+    print (f"PSA key ids values: {psa_key_ids}")
+    if psa_key_ids is not None:
+        img.set_key_ids(psa_key_ids)
+
+    if key:
+        keys = [imgtool.main.load_key(k) for k in key]
+    else:
+        keys = None
+
     enckey = imgtool.main.load_key(encrypt) if encrypt else None
     if enckey and key:
         if (isinstance(key, imgtool.keys.RSA) and
            not isinstance(enckey, imgtool.keys.RSAPublic)):
             # FIXME
             raise click.UsageError("Signing and encryption must use the same "
                                    "type of key")
-    img.create(key, public_key_format, enckey, dependencies, record_sw_type,
+    img.create(keys, public_key_format, enckey, dependencies, record_sw_type,
                None, encrypt_keylen=int(encrypt_keylen))
     img.save(outfile, hex_addr)
 

cmake/spe-CMakeLists.cmake

@@ -184,7 +184,6 @@ if(BL2 AND PLATFORM_DEFAULT_IMAGE_SIGNING)
         )
 
         if(MCUBOOT_BUILTIN_KEY)
-            set(TFM_NS_KEY_ID 1)
             set(wrapper_args ${wrapper_args} --psa-key-ids ${TFM_NS_KEY_ID})
         endif()
 
@@ -234,6 +233,29 @@ if(BL2 AND PLATFORM_DEFAULT_IMAGE_SIGNING)
                 --output ${CMAKE_BINARY_DIR}/bin/tfm_s_ns.bin
         )
 
+        set(wrapper_args
+            --version    ${MCUBOOT_IMAGE_VERSION_S}
+            --layout     ${CMAKE_CURRENT_SOURCE_DIR}/image_signing/layout_files/signing_layout_s_ns.o
+            --key        ${CMAKE_CURRENT_SOURCE_DIR}/image_signing/keys/image_s_signing_private_key.pem
+            --public-key-format  $<IF:$<BOOL:${MCUBOOT_HW_KEY}>,full,hash>
+            --align      ${MCUBOOT_ALIGN_VAL}
+            --pad
+            --pad-header
+            -H           ${BL2_HEADER_SIZE}
+            -s           ${MCUBOOT_SECURITY_COUNTER_S}
+            -L           ${MCUBOOT_ENC_KEY_LEN}
+            $<$<STREQUAL:${MCUBOOT_UPGRADE_STRATEGY},OVERWRITE_ONLY>:--overwrite-only>
+            $<$<BOOL:${MCUBOOT_CONFIRM_IMAGE}>:--confirm>
+            $<$<BOOL:${MCUBOOT_ENC_IMAGES}>:-E${CMAKE_CURRENT_SOURCE_DIR}/image_signing/keys/image_enc_key.pem>
+            $<$<BOOL:${MCUBOOT_MEASURED_BOOT}>:--measured-boot-record>
+            ${CMAKE_BINARY_DIR}/bin/tfm_s_ns.bin
+            ${CMAKE_BINARY_DIR}/tfm_s_ns_signed.bin
+        )
+
+        if(MCUBOOT_BUILTIN_KEY)
+            set(wrapper_args ${wrapper_args} --psa-key-ids ${TFM_S_KEY_ID})
+        endif()
+
         add_custom_command(OUTPUT ${CMAKE_BINARY_DIR}/tfm_s_ns_signed.bin
             DEPENDS tfm_s_ns_bin ${CMAKE_BINARY_DIR}/bin/tfm_s_ns.bin
             DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/image_signing/layout_files/signing_layout_s_ns.o
@@ -242,22 +264,7 @@ if(BL2 AND PLATFORM_DEFAULT_IMAGE_SIGNING)
             # sign the combined tfm_s_ns.bin file
             COMMAND ${Python3_EXECUTABLE}
                 ${CMAKE_CURRENT_SOURCE_DIR}/image_signing/scripts/wrapper/wrapper.py
-                --version ${MCUBOOT_IMAGE_VERSION_S}
-                --layout ${CMAKE_CURRENT_SOURCE_DIR}/image_signing/layout_files/signing_layout_s_ns.o
-                --key ${CMAKE_CURRENT_SOURCE_DIR}/image_signing/keys/image_s_signing_private_key.pem
-                --public-key-format $<IF:$<BOOL:${MCUBOOT_HW_KEY}>,full,hash>
-                --align ${MCUBOOT_ALIGN_VAL}
-                --pad
-                --pad-header
-                -H ${BL2_HEADER_SIZE}
-                -s ${MCUBOOT_SECURITY_COUNTER_S}
-                -L ${MCUBOOT_ENC_KEY_LEN}
-                $<$<STREQUAL:${MCUBOOT_UPGRADE_STRATEGY},OVERWRITE_ONLY>:--overwrite-only>
-                $<$<BOOL:${MCUBOOT_CONFIRM_IMAGE}>:--confirm>
-                $<$<BOOL:${MCUBOOT_ENC_IMAGES}>:-E${CMAKE_CURRENT_SOURCE_DIR}/image_signing/keys/image_enc_key.pem>
-                $<$<BOOL:${MCUBOOT_MEASURED_BOOT}>:--measured-boot-record>
-                ${CMAKE_BINARY_DIR}/bin/tfm_s_ns.bin
-                ${CMAKE_BINARY_DIR}/tfm_s_ns_signed.bin
+                ${wrapper_args}
         )
     endif()
 endif()

config/spe_config.cmake.in

@@ -1,5 +1,5 @@
 #-------------------------------------------------------------------------------
-# Copyright (c) 2023, Arm Limited. All rights reserved.
+# SPDX-FileCopyrightText: Copyright The TrustedFirmware-M Contributors
 #
 # SPDX-License-Identifier: BSD-3-Clause
 #
@@ -24,6 +24,8 @@ set(BL2                                    @BL2@)
 set(BL2_HEADER_SIZE                        @BL2_HEADER_SIZE@)
 set(BL2_TRAILER_SIZE                       @BL2_TRAILER_SIZE@)
 set(MCUBOOT_IMAGE_NUMBER                   @MCUBOOT_IMAGE_NUMBER@)
+set(TFM_S_KEY_ID                           @TFM_S_KEY_ID@)
+set(TFM_NS_KEY_ID                          @TFM_NS_KEY_ID@)
 set(MCUBOOT_CONFIRM_IMAGE                  @MCUBOOT_CONFIRM_IMAGE@)
 set(MCUBOOT_ENC_IMAGES                     @MCUBOOT_ENC_IMAGES@)
 set(MCUBOOT_ENC_KEY_LEN                    @MCUBOOT_ENC_KEY_LEN@)
@@ -35,6 +37,7 @@ set(MCUBOOT_S_IMAGE_MIN_VER                @MCUBOOT_S_IMAGE_MIN_VER@)
 
 set(MCUBOOT_MEASURED_BOOT                  @MCUBOOT_MEASURED_BOOT@)
 set(MCUBOOT_HW_KEY                         @MCUBOOT_HW_KEY@)
+set(MCUBOOT_BUILTIN_KEY                    @MCUBOOT_BUILTIN_KEY@)
 
 set(MCUBOOT_SECURITY_COUNTER_S             @MCUBOOT_SECURITY_COUNTER_S@)
 set(MCUBOOT_IMAGE_VERSION_S                @MCUBOOT_IMAGE_VERSION_S@)