diff options
author | David Vincze <david.vincze@linaro.org> | 2020-06-10 15:54:31 +0200 |
---|---|---|
committer | David Vincze <david.vincze@linaro.org> | 2020-06-24 14:27:50 +0200 |
commit | 5399803fa6724959d0a6ae379540a03e19ff4bb6 (patch) | |
tree | 119b49e5265f50910b8a5aab4750dc6da96bd973 | |
parent | abb527cc7c408750385a81e92412c8293f35a0c0 (diff) | |
download | trusted-firmware-m-5399803fa6724959d0a6ae379540a03e19ff4bb6.tar.gz |
Attest: Handle discrepancy in tlv_len definitions
The definition of the 'tlv_len' field in the shared boot data entry
header differs in upstream MCUboot repo and in its TF-M fork.
In the upstream repo the 'tlv_len' only covers the length of the payload
but not the size of the entry header. But in the TF-M fork the 'tlv_len'
covers the size of entry header and payload together. This discrepancy
is distinguished during the shared boot data processing based on which
MCUboot version is used along with TF-M runtime.
Change-Id: I0cc4b2a2e6c53d125514b1da7e44be474abdd9e4
Signed-off-by: David Vincze <david.vincze@linaro.org>
-rw-r--r-- | CommonConfig.cmake | 5 | ||||
-rw-r--r-- | docs/reference/services/tfm_attestation_integration_guide.rst | 17 | ||||
-rw-r--r-- | secure_fw/CMakeLists.txt | 10 | ||||
-rw-r--r-- | secure_fw/partitions/initial_attestation/CMakeLists.inc | 4 | ||||
-rw-r--r-- | secure_fw/partitions/initial_attestation/attestation_core.c | 47 | ||||
-rw-r--r-- | secure_fw/spm/init/tfm_boot_data.c | 17 |
6 files changed, 82 insertions, 18 deletions
diff --git a/CommonConfig.cmake b/CommonConfig.cmake index 01cc8e4dea..499cdb67d8 100644 --- a/CommonConfig.cmake +++ b/CommonConfig.cmake @@ -426,6 +426,11 @@ if (BL2) if (${MCUBOOT_UPGRADE_STRATEGY} STREQUAL "NO_SWAP") set(LINK_TO_BOTH_MEMORY_REGION ON) endif() + + if (MCUBOOT_REPO STREQUAL "TF-M") + # FixMe: LEGACY_TFM_TLV_HEADER could be removed when MCUBoot fork is deleted. + set(LEGACY_TFM_TLV_HEADER ON) + endif() endif() ##Set Mbed Crypto compiler flags and variables for crypto service diff --git a/docs/reference/services/tfm_attestation_integration_guide.rst b/docs/reference/services/tfm_attestation_integration_guide.rst index d3457cd905..5d2986146e 100644 --- a/docs/reference/services/tfm_attestation_integration_guide.rst +++ b/docs/reference/services/tfm_attestation_integration_guide.rst @@ -356,8 +356,21 @@ The structure of shared data must be the following: entry header structure: ``struct shared_data_tlv_entry`` and the data. In the entry header is a type field ``tlv_type`` which identify the consumer of the entry in the runtime software and specify the subtype of that data item. - There is a size field ``tlv_len`` which covers the size of the entry header - and the data. After this structure comes the actual data. + + .. Note:: + + There is a size field ``tlv_len`` which has different definitions in the + upstream MCUboot repository and in its TF-M forked version: + + - Upstream MCUboot: Covers only the length of data but not the header + size. + - TF-M MCUboot: Covers the size of the entry header and the data + together. + + This difference is handled by TF-M code based on which bootloader is used + along with TF-M runtime. + + After the entry header structure comes the actual data. .. code-block:: c diff --git a/secure_fw/CMakeLists.txt b/secure_fw/CMakeLists.txt index 38a0bb22b2..216c92aee7 100644 --- a/secure_fw/CMakeLists.txt +++ b/secure_fw/CMakeLists.txt @@ -153,18 +153,22 @@ if (DEFINED CMSE_FLAGS) endif() config_setting_shared_compiler_flags(${PROJECT_OBJ_LIB}) -if(NOT DEFINED TARGET_NV_COUNTERS_ENABLE) +if (NOT DEFINED TARGET_NV_COUNTERS_ENABLE) set(TARGET_NV_COUNTERS_ENABLE OFF) endif() -if(TARGET_NV_COUNTERS_ENABLE) +if (TARGET_NV_COUNTERS_ENABLE) embedded_set_target_compile_defines(TARGET ${PROJECT_OBJ_LIB} LANGUAGE C DEFINES TFM_NVCOUNTERS_ENABLE APPEND) endif() -if(BOOT_DATA_AVAILABLE) +if (BOOT_DATA_AVAILABLE) embedded_set_target_compile_defines(TARGET ${PROJECT_OBJ_LIB} LANGUAGE C DEFINES BOOT_DATA_AVAILABLE APPEND) endif() +if (LEGACY_TFM_TLV_HEADER) + embedded_set_target_compile_defines(TARGET ${PROJECT_OBJ_LIB} LANGUAGE C DEFINES LEGACY_TFM_TLV_HEADER APPEND) +endif() + if (NOT DEFINED CORE_TEST) message(FATAL_ERROR "Incomplete build configuration: CORE_TEST is undefined.") elseif(CORE_TEST) diff --git a/secure_fw/partitions/initial_attestation/CMakeLists.inc b/secure_fw/partitions/initial_attestation/CMakeLists.inc index b0c18c16a4..f525000ed3 100644 --- a/secure_fw/partitions/initial_attestation/CMakeLists.inc +++ b/secure_fw/partitions/initial_attestation/CMakeLists.inc @@ -62,6 +62,10 @@ if (ATTEST_INCLUDE_COSE_KEY_ID) set_property(SOURCE ${ATTEST_C_SRC} APPEND PROPERTY COMPILE_DEFINITIONS INCLUDE_COSE_KEY_ID) endif() +if (LEGACY_TFM_TLV_HEADER) + set_property(SOURCE ${ATTEST_C_SRC} APPEND PROPERTY COMPILE_DEFINITIONS LEGACY_TFM_TLV_HEADER) +endif() + #Inform the user about attestation service features selected based on the cmake flags message("The Initial Attestation service compile configuration is as follows:") message("- ATTEST_INCLUDE_OPTIONAL_CLAIMS: ${ATTEST_INCLUDE_OPTIONAL_CLAIMS}") diff --git a/secure_fw/partitions/initial_attestation/attestation_core.c b/secure_fw/partitions/initial_attestation/attestation_core.c index f5059b9dde..e0df017205 100644 --- a/secure_fw/partitions/initial_attestation/attestation_core.c +++ b/secure_fw/partitions/initial_attestation/attestation_core.c @@ -209,13 +209,18 @@ static int32_t attest_get_tlv_by_module(uint8_t module, } else { /* Any subsequent call set to the next TLV entry */ (void)tfm_memcpy(&tlv_entry, *tlv_ptr, SHARED_DATA_ENTRY_HEADER_SIZE); +#ifdef LEGACY_TFM_TLV_HEADER tlv_curr = (*tlv_ptr) + tlv_entry.tlv_len; +#else + tlv_curr = (*tlv_ptr) + SHARED_DATA_ENTRY_HEADER_SIZE + + tlv_entry.tlv_len; +#endif } /* Iterates over the TLV section and returns the address and size of TLVs * with requested module identifier */ - for (; tlv_curr < tlv_end; tlv_curr += tlv_entry.tlv_len) { + while (tlv_curr < tlv_end) { /* Create local copy to avoid unaligned access */ (void)tfm_memcpy(&tlv_entry, tlv_curr, SHARED_DATA_ENTRY_HEADER_SIZE); if (GET_IAS_MODULE(tlv_entry.tlv_type) == module) { @@ -224,6 +229,11 @@ static int32_t attest_get_tlv_by_module(uint8_t module, *tlv_len = tlv_entry.tlv_len; return 1; } +#ifdef LEGACY_TFM_TLV_HEADER + tlv_curr += tlv_entry.tlv_len; +#else + tlv_curr += (SHARED_DATA_ENTRY_HEADER_SIZE + tlv_entry.tlv_len); +#endif } return 0; @@ -287,7 +297,7 @@ attest_add_all_sw_components(struct attest_token_ctx *token_ctx) uint8_t tlv_id; int32_t found; uint32_t cnt = 0; - uint8_t module; + uint8_t module = 0; QCBOREncodeContext *cbor_encode_ctx = NULL; UsefulBufC encoded = NULLUsefulBufC; @@ -296,7 +306,18 @@ attest_add_all_sw_components(struct attest_token_ctx *token_ctx) /* Starting from module 1, because module 0 contains general claims which * are not related to SW module(i.e: boot_seed, etc.) */ - for (module = 1; module < SW_MAX; ++module) { + /* TODO: When TF-M's MCUboot fork is used as the bootloader + * (LEGACY_TFM_TLV_HEADER is defined) it uses different SW module + * identifiers in the shared data entry headers than the upstream + * MCUboot. This is a workaround to be able to get all the claims + * of every SW components, until this discrepancy is handled properly. + */ +#ifdef LEGACY_TFM_TLV_HEADER + module = 1; +#else + module = 0; +#endif + for ( ; module < SW_MAX; ++module) { /* Indicates to restart the look up from the beginning of the shared * data section */ @@ -318,7 +339,10 @@ attest_add_all_sw_components(struct attest_token_ctx *token_ctx) } encoded.ptr = tlv_ptr + SHARED_DATA_ENTRY_HEADER_SIZE; - encoded.len = tlv_len - SHARED_DATA_ENTRY_HEADER_SIZE; + encoded.len = tlv_len; +#ifdef LEGACY_TFM_TLV_HEADER + encoded.len -= SHARED_DATA_ENTRY_HEADER_SIZE; +#endif QCBOREncode_AddEncoded(cbor_encode_ctx, encoded); } } @@ -359,7 +383,10 @@ attest_add_boot_seed_claim(struct attest_token_ctx *token_ctx) found = attest_get_tlv_by_id(BOOT_SEED, &tlv_len, &tlv_ptr); if (found == 1) { claim_value.ptr = tlv_ptr + SHARED_DATA_ENTRY_HEADER_SIZE; - claim_value.len = tlv_len - SHARED_DATA_ENTRY_HEADER_SIZE; + claim_value.len = tlv_len; +#ifdef LEGACY_TFM_TLV_HEADER + claim_value.len -= SHARED_DATA_ENTRY_HEADER_SIZE; +#endif } else { /* If not found in boot status then use callback function to get it * from runtime SW @@ -486,7 +513,10 @@ attest_add_security_lifecycle_claim(struct attest_token_ctx *token_ctx) found = attest_get_tlv_by_id(SECURITY_LIFECYCLE, &tlv_len, &tlv_ptr); if (found == 1) { claim_value.ptr = tlv_ptr + SHARED_DATA_ENTRY_HEADER_SIZE; - claim_value.len = tlv_len - SHARED_DATA_ENTRY_HEADER_SIZE; + claim_value.len = tlv_len; +#ifdef LEGACY_TFM_TLV_HEADER + claim_value.len -= SHARED_DATA_ENTRY_HEADER_SIZE; +#endif res = get_uint(claim_value.ptr, claim_value.len, &slc_value); if (res) { return PSA_ATTEST_ERR_GENERAL; @@ -604,7 +634,10 @@ attest_add_hw_version_claim(struct attest_token_ctx *token_ctx) found = attest_get_tlv_by_id(HW_VERSION, &tlv_len, &tlv_ptr); if (found == 1) { claim_value.ptr = tlv_ptr + SHARED_DATA_ENTRY_HEADER_SIZE; - claim_value.len = tlv_len - SHARED_DATA_ENTRY_HEADER_SIZE; + claim_value.len = tlv_len; +#ifdef LEGACY_TFM_TLV_HEADER + claim_value.len -= SHARED_DATA_ENTRY_HEADER_SIZE; +#endif } else { /* If not found in boot status then use callback function to get it * from runtime SW diff --git a/secure_fw/spm/init/tfm_boot_data.c b/secure_fw/spm/init/tfm_boot_data.c index ee4466e26f..0dc312446b 100644 --- a/secure_fw/spm/init/tfm_boot_data.c +++ b/secure_fw/spm/init/tfm_boot_data.c @@ -142,6 +142,7 @@ void tfm_core_get_boot_data_handler(uint32_t args[]) uint8_t *ptr; struct shared_data_tlv_entry tlv_entry; uintptr_t tlv_end, offset; + size_t next_tlv_offset; #endif /* BOOT_DATA_AVAILABLE */ #ifndef TFM_PSA_API uint32_t running_partition_idx = @@ -216,23 +217,27 @@ void tfm_core_get_boot_data_handler(uint32_t args[]) /* Iterates over the TLV section and copy TLVs with requested major * type to the provided buffer. */ - for (; offset < tlv_end; offset += tlv_entry.tlv_len) { + for (; offset < tlv_end; offset += next_tlv_offset) { /* Create local copy to avoid unaligned access */ (void)tfm_core_util_memcpy(&tlv_entry, (const void *)offset, SHARED_DATA_ENTRY_HEADER_SIZE); +#ifdef LEGACY_TFM_TLV_HEADER + next_tlv_offset = tlv_entry.tlv_len; +#else + next_tlv_offset = SHARED_DATA_ENTRY_HEADER_SIZE + tlv_entry.tlv_len; +#endif if (GET_MAJOR(tlv_entry.tlv_type) == tlv_major) { /* Check buffer overflow */ - if (((ptr - buf_start) + tlv_entry.tlv_len) > buf_size) { + if (((ptr - buf_start) + next_tlv_offset) > buf_size) { args[0] = (uint32_t)TFM_ERROR_INVALID_PARAMETER; return; } (void)tfm_core_util_memcpy(ptr, (const void *)offset, - tlv_entry.tlv_len); - - ptr += tlv_entry.tlv_len; - boot_data->header.tlv_tot_len += tlv_entry.tlv_len; + next_tlv_offset); + ptr += next_tlv_offset; + boot_data->header.tlv_tot_len += next_tlv_offset; } } #endif /* BOOT_DATA_AVAILABLE */ |