aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Vincze <david.vincze@linaro.org>2020-06-10 15:54:31 +0200
committerDavid Vincze <david.vincze@linaro.org>2020-06-24 14:27:50 +0200
commit5399803fa6724959d0a6ae379540a03e19ff4bb6 (patch)
tree119b49e5265f50910b8a5aab4750dc6da96bd973
parentabb527cc7c408750385a81e92412c8293f35a0c0 (diff)
downloadtrusted-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.cmake5
-rw-r--r--docs/reference/services/tfm_attestation_integration_guide.rst17
-rw-r--r--secure_fw/CMakeLists.txt10
-rw-r--r--secure_fw/partitions/initial_attestation/CMakeLists.inc4
-rw-r--r--secure_fw/partitions/initial_attestation/attestation_core.c47
-rw-r--r--secure_fw/spm/init/tfm_boot_data.c17
6 files changed, 82 insertions, 18 deletions
diff --git a/CommonConfig.cmake b/CommonConfig.cmake
index 01cc8e4de..499cdb67d 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 d3457cd90..5d2986146 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 38a0bb22b..216c92aee 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 b0c18c16a..f525000ed 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 f5059b9dd..e0df01720 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 ee4466e26..0dc312446 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 */