From patchwork Wed May 22 16:04:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: bence.balogh@arm.com X-Patchwork-Id: 44049 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from aws-us-west-2-korg-lkml-1.web.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.lore.kernel.org (Postfix) with ESMTP id 70DB6C27C43 for ; Wed, 22 May 2024 16:05:05 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web11.1228.1716393897642219740 for ; Wed, 22 May 2024 09:04:57 -0700 Authentication-Results: mx.groups.io; dkim=none (message not signed); spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: bence.balogh@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 68BDB339; Wed, 22 May 2024 09:05:21 -0700 (PDT) Received: from e126523.arm.com (unknown [10.57.84.120]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 38E713F766; Wed, 22 May 2024 09:04:56 -0700 (PDT) From: bence.balogh@arm.com To: meta-arm@lists.yoctoproject.org Cc: Bence Balogh Subject: [PATCH scarthgap 1/5] arm-bsp/trusted-services: corstone1000: add EFI var handling fixes Date: Wed, 22 May 2024 18:04:39 +0200 Message-Id: <20240522160443.89173-2-bence.balogh@arm.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20240522160443.89173-1-bence.balogh@arm.com> References: <20240522160443.89173-1-bence.balogh@arm.com> MIME-Version: 1.0 List-Id: X-Webhook-Received: from li982-79.members.linode.com [45.33.32.79] by aws-us-west-2-korg-lkml-1.web.codeaurora.org with HTTPS for ; Wed, 22 May 2024 16:05:05 -0000 X-Groupsio-URL: https://lists.yoctoproject.org/g/meta-arm/message/5756 From: Bence Balogh Signed-off-by: Bence Balogh --- ...-Fix-Avoid-redefinition-of-variables.patch | 28 + ...x-GetNextVariableName-NameSize-input.patch | 495 ++++++++++++++++++ ...r-handling-of-variable-index-loading.patch | 82 +++ .../trusted-services/ts-arm-platforms.inc | 3 + 4 files changed, 608 insertions(+) create mode 100644 meta-arm-bsp/recipes-security/trusted-services/corstone1000/0011-Fix-Avoid-redefinition-of-variables.patch create mode 100644 meta-arm-bsp/recipes-security/trusted-services/corstone1000/0012-Fix-GetNextVariableName-NameSize-input.patch create mode 100644 meta-arm-bsp/recipes-security/trusted-services/corstone1000/0013-Fix-error-handling-of-variable-index-loading.patch diff --git a/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0011-Fix-Avoid-redefinition-of-variables.patch b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0011-Fix-Avoid-redefinition-of-variables.patch new file mode 100644 index 00000000..d5c43bd5 --- /dev/null +++ b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0011-Fix-Avoid-redefinition-of-variables.patch @@ -0,0 +1,28 @@ +From c7f2861e5c5ee209373a8dba15a608f78a97078b Mon Sep 17 00:00:00 2001 +From: Gabor Toth +Date: Wed, 10 Apr 2024 11:17:50 +0200 +Subject: [PATCH 1/3] Fix: Avoid redefinition of variables + +Remove variable redefinition which shadows the original one. + +Signed-off-by: Gabor Toth +Upstream-Status: Submitted [https://review.trustedfirmware.org/c/TS/trusted-services/+/27954] +--- + .../service/uefi/smm_variable/client/cpp/smm_variable_client.cpp | 1 - + 1 file changed, 1 deletion(-) + +diff --git a/components/service/uefi/smm_variable/client/cpp/smm_variable_client.cpp b/components/service/uefi/smm_variable/client/cpp/smm_variable_client.cpp +index f71d0c864..d39448900 100644 +--- a/components/service/uefi/smm_variable/client/cpp/smm_variable_client.cpp ++++ b/components/service/uefi/smm_variable/client/cpp/smm_variable_client.cpp +@@ -166,7 +166,6 @@ efi_status_t smm_variable_client::get_variable(const EFI_GUID &guid, const std:: + + if (call_handle) { + uint8_t *resp_buf; +- size_t resp_len; + service_status_t service_status; + + SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *access_var = +-- +2.25.1 + diff --git a/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0012-Fix-GetNextVariableName-NameSize-input.patch b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0012-Fix-GetNextVariableName-NameSize-input.patch new file mode 100644 index 00000000..06efbb0e --- /dev/null +++ b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0012-Fix-GetNextVariableName-NameSize-input.patch @@ -0,0 +1,495 @@ +From cc4cc9f3f5f02f713cf4da1854f3085bf31e71cf Mon Sep 17 00:00:00 2001 +From: Gabor Toth +Date: Sat, 13 Apr 2024 14:52:23 +0200 +Subject: [PATCH 2/3] Fix GetNextVariableName NameSize input + +Based on the specification the NameSize shall be set to the available +buffer size at the first call instead of the NameSize of the +provided variable. +Change smm-gateway and the tests according this. Also remove +sanitize_get_next_var_name_param utility function, which is not +compilant with this solution. + +Signed-off-by: Gabor Toth +Upstream-Status: Submitted [https://review.trustedfirmware.org/c/TS/trusted-services/+/28022] +--- + .../backend/test/variable_store_tests.cpp | 48 +++++++-------- + .../backend/uefi_variable_store.c | 60 ++++++++++++------- + .../backend/uefi_variable_store.h | 5 +- + .../smm_variable/backend/variable_index.c | 3 + + .../provider/smm_variable_provider.c | 59 +++++------------- + .../service/smm_variable_attack_tests.cpp | 29 ++++----- + .../service/smm_variable_service_tests.cpp | 7 ++- + 7 files changed, 98 insertions(+), 113 deletions(-) + +diff --git a/components/service/uefi/smm_variable/backend/test/variable_store_tests.cpp b/components/service/uefi/smm_variable/backend/test/variable_store_tests.cpp +index fd48f13fb..72772821c 100644 +--- a/components/service/uefi/smm_variable/backend/test/variable_store_tests.cpp ++++ b/components/service/uefi/smm_variable/backend/test/variable_store_tests.cpp +@@ -501,15 +501,13 @@ TEST(UefiVariableStoreTests, bootServiceAccess) + std::vector msg_buffer(VARIABLE_BUFFER_SIZE); + SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *next_name = + (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *) msg_buffer.data(); +- size_t max_name_len = +- VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET; + + size_t total_len = 0; +- next_name->NameSize = sizeof(int16_t); ++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET; + next_name->Name[0] = 0; + + status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name, +- max_name_len, &total_len); ++ &total_len); + + UNSIGNED_LONGLONGS_EQUAL(EFI_NOT_FOUND, status); + } +@@ -574,47 +572,48 @@ TEST(UefiVariableStoreTests, enumerateStoreContents) + std::vector msg_buffer(VARIABLE_BUFFER_SIZE); + SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *next_name = + (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *) msg_buffer.data(); +- size_t max_name_len = +- VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET; + + /* First check handling of invalid variable name */ + std::u16string bogus_name = to_variable_name(u"bogus_variable"); + size_t bogus_name_size = string_get_size_in_bytes(bogus_name); + next_name->Guid = m_common_guid; +- next_name->NameSize = bogus_name_size; ++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET; + memcpy(next_name->Name, bogus_name.data(), bogus_name_size); + + status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name, +- max_name_len, &total_len); ++ &total_len); + UNSIGNED_LONGLONGS_EQUAL(EFI_INVALID_PARAMETER, status); + + /* Enumerate store contents */ + next_name->NameSize = sizeof(int16_t); + next_name->Name[0] = 0; +- /* Check if the correct NameSize is returned if max_name_len is too small */ ++ /* Check if the correct NameSize is returned if namesize is too small */ + status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name, +- 0, &total_len); ++ &total_len); + UNSIGNED_LONGLONGS_EQUAL(EFI_BUFFER_TOO_SMALL, status); + UNSIGNED_LONGLONGS_EQUAL(sizeof(var_name_1), next_name->NameSize); + +- /* And then used the previously received next_name->NameSize as max_name_len */ ++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET; + status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name, +- next_name->NameSize, &total_len); ++ &total_len); + UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status); + CHECK_TRUE(compare_variable_name(var_name_1, next_name->Name, next_name->NameSize)); + ++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET; + status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name, +- max_name_len, &total_len); ++ &total_len); + UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status); + CHECK_TRUE(compare_variable_name(var_name_2, next_name->Name, next_name->NameSize)); + ++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET; + status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name, +- max_name_len, &total_len); ++ &total_len); + UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status); + CHECK_TRUE(compare_variable_name(var_name_3, next_name->Name, next_name->NameSize)); + ++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET; + status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name, +- max_name_len, &total_len); ++ &total_len); + UNSIGNED_LONGLONGS_EQUAL(EFI_NOT_FOUND, status); + + power_cycle(); +@@ -622,21 +621,23 @@ TEST(UefiVariableStoreTests, enumerateStoreContents) + /* Enumerate again - should be left with just NV variables. + * Use a different but equally valid null name. + */ +- next_name->NameSize = 10 * sizeof(int16_t); ++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET; + memset(next_name->Name, 0, next_name->NameSize); + + status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name, +- max_name_len, &total_len); ++ &total_len); + UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status); + CHECK_TRUE(compare_variable_name(var_name_1, next_name->Name, next_name->NameSize)); + ++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET; + status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name, +- max_name_len, &total_len); ++ &total_len); + UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status); + CHECK_TRUE(compare_variable_name(var_name_3, next_name->Name, next_name->NameSize)); + ++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET; + status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name, +- max_name_len, &total_len); ++ &total_len); + UNSIGNED_LONGLONGS_EQUAL(EFI_NOT_FOUND, status); + } + +@@ -672,21 +673,20 @@ TEST(UefiVariableStoreTests, failedNvSet) + std::vector msg_buffer(VARIABLE_BUFFER_SIZE); + SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *next_name = + (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *) msg_buffer.data(); +- size_t max_name_len = +- VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET; + + /* Enumerate store contents */ + size_t total_len = 0; +- next_name->NameSize = sizeof(int16_t); ++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET; + next_name->Name[0] = 0; + + status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name, +- max_name_len, &total_len); ++ &total_len); + UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, status); + CHECK_TRUE(compare_variable_name(var_name_1, next_name->Name, next_name->NameSize)); + ++ next_name->NameSize = VARIABLE_BUFFER_SIZE - SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_NAME_OFFSET; + status = uefi_variable_store_get_next_variable_name(&m_uefi_variable_store, next_name, +- max_name_len, &total_len); ++ &total_len); + UNSIGNED_LONGLONGS_EQUAL(EFI_NOT_FOUND, status); + } + +diff --git a/components/service/uefi/smm_variable/backend/uefi_variable_store.c b/components/service/uefi/smm_variable/backend/uefi_variable_store.c +index 5b46c1371..caf6698aa 100644 +--- a/components/service/uefi/smm_variable/backend/uefi_variable_store.c ++++ b/components/service/uefi/smm_variable/backend/uefi_variable_store.c +@@ -404,9 +404,27 @@ efi_status_t uefi_variable_store_get_variable(const struct uefi_variable_store * + efi_status_t + uefi_variable_store_get_next_variable_name(const struct uefi_variable_store *context, + SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *cur, +- size_t max_name_len, size_t *total_length) ++ size_t *total_length) + { +- efi_status_t status = check_name_terminator(cur->Name, cur->NameSize); ++ efi_status_t status = EFI_SUCCESS; ++ size_t buffer_size = 0; ++ ++ if (!cur) ++ return EFI_INVALID_PARAMETER; ++ /* ++ * NameSize is set to the buffer size to store the names, ++ * let's calculate the size actually being used. ++ */ ++ buffer_size = cur->NameSize; ++ for (int i = 0; i < buffer_size / sizeof(int16_t); i++) { ++ if (cur->Name[i] == 0) { ++ /* With null terminator */ ++ cur->NameSize = 2*(i+1); ++ break; ++ } ++ } ++ ++ status = check_name_terminator(cur->Name, cur->NameSize); + + if (status != EFI_SUCCESS) + return status; +@@ -418,21 +436,11 @@ uefi_variable_store_get_next_variable_name(const struct uefi_variable_store *con + &context->variable_index, &cur->Guid, cur->NameSize, cur->Name, &status); + + if (info && (status == EFI_SUCCESS)) { +- /* The NameSize has to be set in every case according to the UEFI specs. +- * In case of EFI_BUFFER_TOO_SMALL it has to reflect the size of buffer +- * needed. +- */ +- cur->NameSize = info->metadata.name_size; +- *total_length = sizeof(SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME); +- +- if (info->metadata.name_size <= max_name_len) { ++ if (info->metadata.name_size <= buffer_size) { + cur->Guid = info->metadata.guid; ++ cur->NameSize = info->metadata.name_size; + memcpy(cur->Name, info->metadata.name, info->metadata.name_size); + +- *total_length = +- SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME_TOTAL_SIZE( +- cur); +- + /* + * Check if variable is accessible (e.g boot variable is not + * accessible at runtime) +@@ -442,6 +450,10 @@ uefi_variable_store_get_next_variable_name(const struct uefi_variable_store *con + if (status == EFI_SUCCESS) + break; + } else { ++ /* The VariableNameSize is updated to reflect the size of buffer needed */ ++ cur->NameSize = info->metadata.name_size; ++ memset(cur->Name, 0, buffer_size); ++ memset(&cur->Guid, 0, sizeof(EFI_GUID)); + status = EFI_BUFFER_TOO_SMALL; + break; + } +@@ -450,18 +462,24 @@ uefi_variable_store_get_next_variable_name(const struct uefi_variable_store *con + /* Do not hide original error if there is any */ + if (status == EFI_SUCCESS) + status = EFI_NOT_FOUND; ++ ++ memset(cur->Name, 0, buffer_size); ++ memset(&cur->Guid, 0, sizeof(EFI_GUID)); ++ cur->NameSize = 0; + break; + } + } + +- /* If we found no accessible variable clear the fields for security */ +- if (status != EFI_SUCCESS) { +- memset(cur->Name, 0, max_name_len); +- memset(&cur->Guid, 0, sizeof(EFI_GUID)); +- if (status != EFI_BUFFER_TOO_SMALL) +- cur->NameSize = 0; ++ if (status == EFI_SUCCESS) { ++ /* Store everything including the name */ ++ *total_length = ++ SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME_TOTAL_SIZE( ++ cur); ++ } else { ++ /* Do not store the name, only the size */ ++ *total_length = ++ SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME_NAME_OFFSET; + } +- + return status; + } + +diff --git a/components/service/uefi/smm_variable/backend/uefi_variable_store.h b/components/service/uefi/smm_variable/backend/uefi_variable_store.h +index 8be5f36e6..2493ff6b4 100644 +--- a/components/service/uefi/smm_variable/backend/uefi_variable_store.h ++++ b/components/service/uefi/smm_variable/backend/uefi_variable_store.h +@@ -134,8 +134,7 @@ efi_status_t uefi_variable_store_get_variable(const struct uefi_variable_store * + * Used for enumerating the store contents + * + * @param[in] context uefi_variable_store instance +- * @param[out] cur Current variable name +- * @param[in] max_name_len The maximum variable name length ++ * @param[inout] cur The size of the VariableName buffer + * @param[out] total_len The total length of the output + * + * @return EFI_SUCCESS if successful +@@ -143,7 +142,7 @@ efi_status_t uefi_variable_store_get_variable(const struct uefi_variable_store * + efi_status_t + uefi_variable_store_get_next_variable_name(const struct uefi_variable_store *context, + SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *cur, +- size_t max_name_len, size_t *total_length); ++ size_t *total_length); + + /** + * @brief Query for variable info +diff --git a/components/service/uefi/smm_variable/backend/variable_index.c b/components/service/uefi/smm_variable/backend/variable_index.c +index d850dbe18..e2fe6dd38 100644 +--- a/components/service/uefi/smm_variable/backend/variable_index.c ++++ b/components/service/uefi/smm_variable/backend/variable_index.c +@@ -27,6 +27,9 @@ static uint64_t name_hash(const EFI_GUID *guid, size_t name_size, const int16_t + + /* Extend to cover name up to but not including null terminator */ + for (size_t i = 0; i < (name_size - sizeof(int16_t)) / sizeof(int16_t); ++i) { ++ /* Only hash till the first null terminator */ ++ if (name[i] == 0) ++ break; + hash = ((hash << 5) + hash) + name[i]; + } + +diff --git a/components/service/uefi/smm_variable/provider/smm_variable_provider.c b/components/service/uefi/smm_variable/provider/smm_variable_provider.c +index ca3f7e5e5..1a5269338 100644 +--- a/components/service/uefi/smm_variable/provider/smm_variable_provider.c ++++ b/components/service/uefi/smm_variable/provider/smm_variable_provider.c +@@ -81,30 +81,6 @@ static efi_status_t sanitize_access_variable_param(struct rpc_request *req, size + return efi_status; + } + +-static efi_status_t sanitize_get_next_var_name_param(struct rpc_request *req, size_t *param_len) +-{ +- efi_status_t efi_status = EFI_INVALID_PARAMETER; +- *param_len = 0; +- const struct rpc_buffer *req_buf = &req->request; +- +- if (req_buf->data_length >= SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME_NAME_OFFSET) { +- const SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *param = +- (const SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *)req_buf->data; +- +- size_t max_space_for_name = +- req_buf->data_length - +- SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME_NAME_OFFSET; +- +- if (param->NameSize <= max_space_for_name) { +- *param_len = +- SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME_TOTAL_SIZE(param); +- efi_status = EFI_SUCCESS; +- } +- } +- +- return efi_status; +-} +- + static efi_status_t sanitize_var_check_property_param(struct rpc_request *req, size_t *param_len) + { + efi_status_t efi_status = EFI_INVALID_PARAMETER; +@@ -146,7 +122,7 @@ static rpc_status_t get_variable_handler(void *context, struct rpc_request *req) + struct rpc_buffer *req_buf = &req->request; + size_t max_data_len = resp_buf->size - param_len; + +- memmove(resp_buf->data, req_buf->data, param_len); ++ memcpy(resp_buf->data, req_buf->data, param_len); + + efi_status = uefi_variable_store_get_variable( + &this_instance->variable_store, +@@ -167,28 +143,21 @@ static rpc_status_t get_next_variable_name_handler(void *context, struct rpc_req + { + struct smm_variable_provider *this_instance = (struct smm_variable_provider *)context; + +- size_t param_len = 0; +- efi_status_t efi_status = sanitize_get_next_var_name_param(req, ¶m_len); ++ efi_status_t efi_status = EFI_SUCCESS; ++ size_t variable_size = 0; + +- if (efi_status == EFI_SUCCESS) { +- /* Valid get next variable name header */ +- struct rpc_buffer *resp_buf = &req->response; ++ /* Valid get next variable name header */ ++ struct rpc_buffer *resp_buf = &req->response; ++ struct rpc_buffer *req_buf = &req->request; + +- if (resp_buf->size >= param_len) { +- struct rpc_buffer *req_buf = &req->request; ++ memcpy(resp_buf->data, req_buf->data, req_buf->data_length); + +- memmove(resp_buf->data, req_buf->data, param_len); ++ efi_status = uefi_variable_store_get_next_variable_name( ++ &this_instance->variable_store, ++ (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *)resp_buf->data, ++ &variable_size); + +- efi_status = uefi_variable_store_get_next_variable_name( +- &this_instance->variable_store, +- (SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME *)resp_buf->data, +- ((SMM_VARIABLE_COMMUNICATE_GET_NEXT_VARIABLE_NAME*)resp_buf->data)->NameSize, +- &resp_buf->data_length); +- } else { +- /* Reponse buffer not big enough */ +- efi_status = EFI_BAD_BUFFER_SIZE; +- } +- } ++ resp_buf->data_length = variable_size; + + req->service_status = efi_status; + +@@ -240,7 +209,7 @@ static rpc_status_t query_variable_info_handler(void *context, struct rpc_reques + struct rpc_buffer *resp_buf = &req->response; + + if (resp_buf->size >= req_buf->data_length) { +- memmove(resp_buf->data, req_buf->data, req_buf->data_length); ++ memcpy(resp_buf->data, req_buf->data, req_buf->data_length); + + efi_status = uefi_variable_store_query_variable_info( + &this_instance->variable_store, +@@ -308,7 +277,7 @@ static rpc_status_t get_var_check_property_handler(void *context, struct rpc_req + + if (resp_buf->size >= param_len) { + struct rpc_buffer *req_buf = &req->request; +- memmove(resp_buf->data, req_buf->data, param_len); ++ memcpy(resp_buf->data, req_buf->data, param_len); + resp_buf->data_length = param_len; + + efi_status = uefi_variable_store_get_var_check_property( +diff --git a/components/service/uefi/smm_variable/test/service/smm_variable_attack_tests.cpp b/components/service/uefi/smm_variable/test/service/smm_variable_attack_tests.cpp +index 76b62fd35..98e61fec0 100644 +--- a/components/service/uefi/smm_variable/test/service/smm_variable_attack_tests.cpp ++++ b/components/service/uefi/smm_variable/test/service/smm_variable_attack_tests.cpp +@@ -176,19 +176,6 @@ TEST(SmmVariableAttackTests, setAndGetWithSizeMaxNameSize) + UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status); + } + +-TEST(SmmVariableAttackTests, enumerateWithOversizeName) +-{ +- efi_status_t efi_status = EFI_SUCCESS; +- std::u16string var_name = null_name; +- EFI_GUID guid; +- memset(&guid, 0, sizeof(guid)); +- +- efi_status = m_client->get_next_variable_name(guid, var_name, +- (var_name.size() + 1) * sizeof(int16_t) + 1); +- +- UNSIGNED_LONGLONGS_EQUAL(EFI_INVALID_PARAMETER, efi_status); +-} +- + TEST(SmmVariableAttackTests, enumerateWithSizeMaxNameSize) + { + efi_status_t efi_status = EFI_SUCCESS; +@@ -202,17 +189,23 @@ TEST(SmmVariableAttackTests, enumerateWithSizeMaxNameSize) + + UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status); + +- /* Initial iteration uses good name length */ +- efi_status = m_client->get_next_variable_name(guid, var_name); ++ /* Initial iteration uses good name length for next variable */ ++ efi_status = m_client->get_next_variable_name(guid, var_name, std::numeric_limits::max()); + + UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status); + +- /* Next iteration uses invalid name length */ +- efi_status = m_client->get_next_variable_name(guid, var_name, +- std::numeric_limits::max()); ++ /* Next iteration uses invalid name length, so a null terminator can not fit */ ++ var_name = null_name; ++ efi_status = m_client->get_next_variable_name(guid, var_name, 1); + + UNSIGNED_LONGLONGS_EQUAL(EFI_INVALID_PARAMETER, efi_status); + ++ /* Next iteration uses invalid name length, so a null terminator can not fit */ ++ var_name = null_name; ++ efi_status = m_client->get_next_variable_name(guid, var_name, 2); ++ ++ UNSIGNED_LONGLONGS_EQUAL(EFI_BUFFER_TOO_SMALL, efi_status); ++ + /* Expect to be able to remove the variable */ + efi_status = m_client->remove_variable(m_common_guid, var_name_1); + UNSIGNED_LONGLONGS_EQUAL(EFI_SUCCESS, efi_status); +diff --git a/components/service/uefi/smm_variable/test/service/smm_variable_service_tests.cpp b/components/service/uefi/smm_variable/test/service/smm_variable_service_tests.cpp +index e82a90c37..8fa4f8077 100644 +--- a/components/service/uefi/smm_variable/test/service/smm_variable_service_tests.cpp ++++ b/components/service/uefi/smm_variable/test/service/smm_variable_service_tests.cpp +@@ -9,6 +9,7 @@ + #include + #include + #include ++#include + + #include "util.h" + +@@ -154,7 +155,7 @@ TEST_GROUP(SmmVariableServiceTests) + #endif + + do { +- status = m_client->get_next_variable_name(guid, var_name); ++ status = m_client->get_next_variable_name(guid, var_name, max_variable_size); + + /* There are no more variables in the persistent store */ + if (status == EFI_NOT_FOUND) { +@@ -223,6 +224,8 @@ TEST_GROUP(SmmVariableServiceTests) + std::u16string m_ro_variable = to_variable_name(u"ro_variable"); + std::u16string m_boot_finished_var_name = to_variable_name(u"finished"); + ++ uint32_t max_variable_size = 4096; ++ + /* Cleanup skips these variables */ + std::vector m_non_rm_vars{ &m_ro_variable, &m_boot_finished_var_name }; + +@@ -654,7 +657,7 @@ TEST(SmmVariableServiceTests, enumerateStoreContents) + std::u16string *expected_variables[] = { &var_name_1, &var_name_2, &var_name_3 }; + + do { +- efi_status = m_client->get_next_variable_name(guid, var_name); ++ efi_status = m_client->get_next_variable_name(guid, var_name, max_variable_size); + if (efi_status != EFI_SUCCESS) + break; + +-- +2.25.1 + diff --git a/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0013-Fix-error-handling-of-variable-index-loading.patch b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0013-Fix-error-handling-of-variable-index-loading.patch new file mode 100644 index 00000000..978f2e52 --- /dev/null +++ b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0013-Fix-error-handling-of-variable-index-loading.patch @@ -0,0 +1,82 @@ +From c62e728bb86981219984c8b39819fb8926a41e10 Mon Sep 17 00:00:00 2001 +From: Gabor Toth +Date: Fri, 19 Apr 2024 18:25:23 +0200 +Subject: [PATCH 3/3] Fix error handling of variable index loading + +If loading of the variable index from Protected Storage fails, SmmGW +will silently continue with empty variable store. This is a serious +fault and a potential security risk. +Change the code to produce a log output when this happens and stop +loading the SP. + +Signed-off-by: Gabor Toth +Upstream-Status: Submitted [https://review.trustedfirmware.org/c/TS/trusted-services/+/28300] +--- + .../backend/uefi_variable_store.c | 28 ++++++++++++++----- + 1 file changed, 21 insertions(+), 7 deletions(-) + +diff --git a/components/service/uefi/smm_variable/backend/uefi_variable_store.c b/components/service/uefi/smm_variable/backend/uefi_variable_store.c +index caf6698aa..c1691dc8f 100644 +--- a/components/service/uefi/smm_variable/backend/uefi_variable_store.c ++++ b/components/service/uefi/smm_variable/backend/uefi_variable_store.c +@@ -27,7 +27,7 @@ + #include "service/crypto/client/psa/crypto_client.h" + #endif + +-static void load_variable_index(struct uefi_variable_store *context); ++static efi_status_t load_variable_index(struct uefi_variable_store *context); + + static efi_status_t sync_variable_index(const struct uefi_variable_store *context); + +@@ -165,8 +165,10 @@ efi_status_t uefi_variable_store_init(struct uefi_variable_store *context, uint3 + + /* Load the variable index with NV variable info from the persistent store */ + if (context->index_sync_buffer) { +- load_variable_index(context); +- purge_orphan_index_entries(context); ++ status = load_variable_index(context); ++ ++ if (status == EFI_SUCCESS) ++ purge_orphan_index_entries(context); + } + } + +@@ -571,7 +573,7 @@ efi_status_t uefi_variable_store_get_var_check_property( + return status; + } + +-static void load_variable_index(struct uefi_variable_store *context) ++static efi_status_t load_variable_index(struct uefi_variable_store *context) + { + struct storage_backend *persistent_store = context->persistent_store.storage_backend; + +@@ -583,11 +585,23 @@ static void load_variable_index(struct uefi_variable_store *context) + SMM_VARIABLE_INDEX_STORAGE_UID, 0, context->index_sync_buffer_size, + context->index_sync_buffer, &data_len); + +- if (psa_status == PSA_SUCCESS) { +- variable_index_restore(&context->variable_index, data_len, +- context->index_sync_buffer); ++ switch(psa_status) { ++ case PSA_SUCCESS: ++ (void) variable_index_restore(&context->variable_index, data_len, ++ context->index_sync_buffer); ++ break; ++ ++ case PSA_ERROR_DOES_NOT_EXIST: ++ IMSG("Index variable does not exist in NV store, continuing with empty index"); ++ break; ++ ++ default: ++ EMSG("Loading variable index failed: %d", psa_status); ++ return EFI_LOAD_ERROR; + } + } ++ ++ return EFI_SUCCESS; + } + + static efi_status_t sync_variable_index(const struct uefi_variable_store *context) +-- +2.25.1 + diff --git a/meta-arm-bsp/recipes-security/trusted-services/ts-arm-platforms.inc b/meta-arm-bsp/recipes-security/trusted-services/ts-arm-platforms.inc index 2612c411..f8a94750 100644 --- a/meta-arm-bsp/recipes-security/trusted-services/ts-arm-platforms.inc +++ b/meta-arm-bsp/recipes-security/trusted-services/ts-arm-platforms.inc @@ -12,6 +12,9 @@ SRC_URI:append:corstone1000 = " \ file://0008-plat-corstone1000-add-client_id-for-FMP-service.patch \ file://0009-Remove-Werror-flag.patch \ file://0010-Remove-PLATFORM_HAS_ATTEST_PK-define-from-IAT-test.patch \ + file://0011-Fix-Avoid-redefinition-of-variables.patch \ + file://0012-Fix-GetNextVariableName-NameSize-input.patch \ + file://0013-Fix-error-handling-of-variable-index-loading.patch \ " COMPATIBLE_MACHINE:fvp-base = "fvp-base" From patchwork Wed May 22 16:04:40 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: bence.balogh@arm.com X-Patchwork-Id: 44050 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from aws-us-west-2-korg-lkml-1.web.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.lore.kernel.org (Postfix) with ESMTP id 85C01C27C41 for ; Wed, 22 May 2024 16:05:05 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web11.1230.1716393899562678019 for ; Wed, 22 May 2024 09:04:59 -0700 Authentication-Results: mx.groups.io; dkim=none (message not signed); spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: bence.balogh@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4BE86DA7; Wed, 22 May 2024 09:05:23 -0700 (PDT) Received: from e126523.arm.com (unknown [10.57.84.120]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 025F53F766; Wed, 22 May 2024 09:04:57 -0700 (PDT) From: bence.balogh@arm.com To: meta-arm@lists.yoctoproject.org Cc: Bence Balogh Subject: [PATCH scarthgap 2/5] arm-bsp/trusted-services: corstone1000: add fixes for private auth vars Date: Wed, 22 May 2024 18:04:40 +0200 Message-Id: <20240522160443.89173-3-bence.balogh@arm.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20240522160443.89173-1-bence.balogh@arm.com> References: <20240522160443.89173-1-bence.balogh@arm.com> MIME-Version: 1.0 List-Id: X-Webhook-Received: from li982-79.members.linode.com [45.33.32.79] by aws-us-west-2-korg-lkml-1.web.codeaurora.org with HTTPS for ; Wed, 22 May 2024 16:05:05 -0000 X-Groupsio-URL: https://lists.yoctoproject.org/g/meta-arm/message/5757 From: Bence Balogh Signed-off-by: Bence Balogh --- ...pi-to-create-uefi-priv-var-fingerpri.patch | 758 ++++++++++++++++++ ...estamp-validation-for-uefi-variables.patch | 146 ++++ ...n-uefi-variable-authentication-steps.patch | 282 +++++++ ...e-Authenticated-Variable-verificatio.patch | 292 +++++++ .../trusted-services/ts-arm-platforms.inc | 4 + .../ts-sp-smm-gateway_%.bbappend | 1 + 6 files changed, 1483 insertions(+) create mode 100644 meta-arm-bsp/recipes-security/trusted-services/corstone1000/0014-Provide-crypto-api-to-create-uefi-priv-var-fingerpri.patch create mode 100644 meta-arm-bsp/recipes-security/trusted-services/corstone1000/0015-Add-timestamp-validation-for-uefi-variables.patch create mode 100644 meta-arm-bsp/recipes-security/trusted-services/corstone1000/0016-Isolate-common-uefi-variable-authentication-steps.patch create mode 100644 meta-arm-bsp/recipes-security/trusted-services/corstone1000/0017-Implement-Private-Authenticated-Variable-verificatio.patch diff --git a/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0014-Provide-crypto-api-to-create-uefi-priv-var-fingerpri.patch b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0014-Provide-crypto-api-to-create-uefi-priv-var-fingerpri.patch new file mode 100644 index 00000000..ae9a53fa --- /dev/null +++ b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0014-Provide-crypto-api-to-create-uefi-priv-var-fingerpri.patch @@ -0,0 +1,758 @@ +From 370811420cfa1c14146f45de308bbccf70408eb8 Mon Sep 17 00:00:00 2001 +From: Gabor Toth +Date: Fri, 5 Apr 2024 11:19:37 +0200 +Subject: [PATCH] Provide crypto api to create uefi priv var fingerprint +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Add new call to the crypto backend to calculate a hash of the common +name of the signing certificate’s Subject and the tbsCertificate +of the top-level issuer certificate. + +Signed-off-by: Gabor Toth +Upstream-Status: Submitted [https://review.trustedfirmware.org/c/TS/trusted-services/+/27953] +--- + .../client/caller/packed-c/crypto_caller.h | 1 + + ...aller_get_uefi_priv_auth_var_fingerprint.h | 90 ++++++++ + .../packed-c/packedc_crypto_client.cpp | 8 + + .../protocol/packed-c/packedc_crypto_client.h | 4 + + .../service/crypto/client/psa/component.cmake | 1 + + .../service/crypto/client/psa/crypto_client.h | 5 + + .../psa/get_uefi_priv_auth_var_fingerprint.c | 21 ++ + .../service/crypto/provider/crypto_provider.c | 212 +++++++++++++++--- + .../serializer/crypto_provider_serializer.h | 8 + + .../packedc_crypto_provider_serializer.c | 54 +++++ + .../backend/direct/uefi_direct_backend.c | 90 ++++++++ + deployments/smm-gateway/smm-gateway.cmake | 5 + + .../get_uefi_priv_auth_var_fingerprint.h | 21 ++ + protocols/service/crypto/packed-c/opcodes.h | 1 + + 14 files changed, 488 insertions(+), 33 deletions(-) + create mode 100644 components/service/crypto/client/caller/packed-c/crypto_caller_get_uefi_priv_auth_var_fingerprint.h + create mode 100644 components/service/crypto/client/psa/get_uefi_priv_auth_var_fingerprint.c + create mode 100644 protocols/service/crypto/packed-c/get_uefi_priv_auth_var_fingerprint.h + +diff --git a/components/service/crypto/client/caller/packed-c/crypto_caller.h b/components/service/crypto/client/caller/packed-c/crypto_caller.h +index d834bc207..d5dd0f70d 100644 +--- a/components/service/crypto/client/caller/packed-c/crypto_caller.h ++++ b/components/service/crypto/client/caller/packed-c/crypto_caller.h +@@ -31,5 +31,6 @@ + #include "crypto_caller_sign_hash.h" + #include "crypto_caller_verify_hash.h" + #include "crypto_caller_verify_pkcs7_signature.h" ++#include "crypto_caller_get_uefi_priv_auth_var_fingerprint.h" + + #endif /* PACKEDC_CRYPTO_CALLER_H */ +diff --git a/components/service/crypto/client/caller/packed-c/crypto_caller_get_uefi_priv_auth_var_fingerprint.h b/components/service/crypto/client/caller/packed-c/crypto_caller_get_uefi_priv_auth_var_fingerprint.h +new file mode 100644 +index 000000000..d3446e445 +--- /dev/null ++++ b/components/service/crypto/client/caller/packed-c/crypto_caller_get_uefi_priv_auth_var_fingerprint.h +@@ -0,0 +1,90 @@ ++/* ++ * Copyright (c) 2024, Arm Limited and Contributors. All rights reserved. ++ * ++ * SPDX-License-Identifier: BSD-3-Clause ++ */ ++ ++#ifndef PACKEDC_CRYPTO_CALLER_GET_UEFI_PRIV_AUTH_VAR_FINGERPRINT_H ++#define PACKEDC_CRYPTO_CALLER_GET_UEFI_PRIV_AUTH_VAR_FINGERPRINT_H ++ ++#include ++#include ++#include ++#include ++#include ++#include ++#include ++#include ++ ++#ifdef __cplusplus ++extern "C" { ++#endif ++ ++static inline int crypto_caller_get_uefi_priv_auth_var_fingerprint(struct service_client *context, ++ const uint8_t *signature_cert, ++ uint64_t signature_cert_len, ++ uint8_t *output) ++{ ++ efi_status_t efi_status = EFI_SUCCESS; ++ size_t req_len = 0; ++ ++ if (signature_cert_len > UINT16_MAX) ++ return RPC_ERROR_INVALID_VALUE; ++ ++ struct tlv_record signature_record = { ++ .tag = TS_CRYPTO_GET_UEFI_PRIV_AUTH_VAR_FINGERPRINT_IN_TAG_SIGNATURE, ++ .length = (uint16_t)signature_cert_len, ++ .value = signature_cert ++ }; ++ ++ req_len += tlv_required_space(signature_record.length); ++ ++ rpc_call_handle call_handle; ++ uint8_t *req_buf; ++ ++ call_handle = rpc_caller_session_begin(context->session, &req_buf, req_len, 0); ++ ++ if (call_handle) { ++ uint8_t *resp_buf; ++ size_t resp_len; ++ service_status_t service_status; ++ struct tlv_iterator req_iter; ++ ++ tlv_iterator_begin(&req_iter, req_buf, req_len); ++ tlv_encode(&req_iter, &signature_record); ++ ++ context->rpc_status = rpc_caller_session_invoke( ++ call_handle, TS_CRYPTO_OPCODE_GET_UEFI_PRIV_AUTH_VAR_FINGERPRINT, &resp_buf, &resp_len, ++ &service_status); ++ ++ if (context->rpc_status == RPC_SUCCESS) { ++ ++ if (service_status == EFI_SUCCESS) { ++ ++ struct tlv_const_iterator resp_iter; ++ struct tlv_record decoded_record; ++ tlv_const_iterator_begin(&resp_iter, resp_buf, resp_len); ++ ++ if (tlv_find_decode(&resp_iter, ++ TS_CRYPTO_GET_UEFI_PRIV_AUTH_VAR_FINGERPRINT_OUT_TAG_IDENTIFIER, &decoded_record)) { ++ ++ memcpy(output, decoded_record.value, PSA_HASH_MAX_SIZE); ++ } ++ else { ++ /* Mandatory response parameter missing */ ++ efi_status = EFI_INVALID_PARAMETER; ++ } ++ } ++ } ++ ++ rpc_caller_session_end(call_handle); ++ } ++ ++ return efi_status; ++} ++ ++#ifdef __cplusplus ++} ++#endif ++ ++#endif /* PACKEDC_CRYPTO_CALLER_GET_UEFI_PRIV_AUTH_VAR_FINGERPRINT_H */ +diff --git a/components/service/crypto/client/cpp/protocol/packed-c/packedc_crypto_client.cpp b/components/service/crypto/client/cpp/protocol/packed-c/packedc_crypto_client.cpp +index aaa71f0c8..e0f6a15a8 100644 +--- a/components/service/crypto/client/cpp/protocol/packed-c/packedc_crypto_client.cpp ++++ b/components/service/crypto/client/cpp/protocol/packed-c/packedc_crypto_client.cpp +@@ -428,3 +428,11 @@ int packedc_crypto_client::verify_pkcs7_signature(const uint8_t *signature_cert, + hash, hash_len, public_key_cert, + public_key_cert_len); + } ++ ++int packedc_crypto_client::get_uefi_priv_auth_var_fingerprint(const uint8_t *signature_cert, ++ uint64_t signature_cert_len, ++ uint8_t *output) ++{ ++ return crypto_caller_get_uefi_priv_auth_var_fingerprint(&m_client, signature_cert, signature_cert_len, ++ output); ++} +diff --git a/components/service/crypto/client/cpp/protocol/packed-c/packedc_crypto_client.h b/components/service/crypto/client/cpp/protocol/packed-c/packedc_crypto_client.h +index 8d4f60cf9..ec6c51c7f 100644 +--- a/components/service/crypto/client/cpp/protocol/packed-c/packedc_crypto_client.h ++++ b/components/service/crypto/client/cpp/protocol/packed-c/packedc_crypto_client.h +@@ -236,6 +236,10 @@ public: + int verify_pkcs7_signature(const uint8_t *signature_cert, uint64_t signature_cert_len, + const uint8_t *hash, uint64_t hash_len, + const uint8_t *public_key_cert, uint64_t public_key_cert_len); ++ ++ int get_uefi_priv_auth_var_fingerprint(const uint8_t *signature_cert, ++ uint64_t signature_cert_len, ++ uint8_t *output); + }; + + #endif /* PACKEDC_CRYPTO_CLIENT_H */ +diff --git a/components/service/crypto/client/psa/component.cmake b/components/service/crypto/client/psa/component.cmake +index 359db3b4a..5bee0c652 100644 +--- a/components/service/crypto/client/psa/component.cmake ++++ b/components/service/crypto/client/psa/component.cmake +@@ -32,4 +32,5 @@ target_sources(${TGT} PRIVATE + "${CMAKE_CURRENT_LIST_DIR}/psa_sign_message.c" + "${CMAKE_CURRENT_LIST_DIR}/psa_verify_message.c" + "${CMAKE_CURRENT_LIST_DIR}/verify_pkcs7_signature.c" ++ "${CMAKE_CURRENT_LIST_DIR}/get_uefi_priv_auth_var_fingerprint.c" + ) +diff --git a/components/service/crypto/client/psa/crypto_client.h b/components/service/crypto/client/psa/crypto_client.h +index 4b59bbe32..af04df11e 100644 +--- a/components/service/crypto/client/psa/crypto_client.h ++++ b/components/service/crypto/client/psa/crypto_client.h +@@ -7,10 +7,15 @@ + #ifndef CRYPTO_CLIENT_H + #define CRYPTO_CLIENT_H + ++#include + #include + + int verify_pkcs7_signature(const uint8_t *signature_cert, uint64_t signature_cert_len, + const uint8_t *hash, uint64_t hash_len, const uint8_t *public_key_cert, + uint64_t public_key_cert_len); + ++int get_uefi_priv_auth_var_fingerprint_handler(const uint8_t *signature_cert, ++ uint64_t signature_cert_len, ++ uint8_t *output); ++ + #endif /* CRYPTO_CLIENT_H */ +diff --git a/components/service/crypto/client/psa/get_uefi_priv_auth_var_fingerprint.c b/components/service/crypto/client/psa/get_uefi_priv_auth_var_fingerprint.c +new file mode 100644 +index 000000000..702aaa0c4 +--- /dev/null ++++ b/components/service/crypto/client/psa/get_uefi_priv_auth_var_fingerprint.c +@@ -0,0 +1,21 @@ ++/* ++ * Copyright (c) 2024, Arm Limited and Contributors. All rights reserved. ++ * ++ * SPDX-License-Identifier: BSD-3-Clause ++ */ ++ ++#include "crypto_caller_selector.h" ++#include "crypto_client.h" ++#include "psa_crypto_client.h" ++ ++int get_uefi_priv_auth_var_fingerprint_handler(const uint8_t *signature_cert, ++ uint64_t signature_cert_len, ++ uint8_t *output) ++{ ++ if (psa_crypto_client_instance.init_status != PSA_SUCCESS) ++ return psa_crypto_client_instance.init_status; ++ ++ return crypto_caller_get_uefi_priv_auth_var_fingerprint(&psa_crypto_client_instance.base, ++ signature_cert, signature_cert_len, ++ output); ++} +diff --git a/components/service/crypto/provider/crypto_provider.c b/components/service/crypto/provider/crypto_provider.c +index 9cd520859..4535d6dbe 100644 +--- a/components/service/crypto/provider/crypto_provider.c ++++ b/components/service/crypto/provider/crypto_provider.c +@@ -3,12 +3,15 @@ + * + * SPDX-License-Identifier: BSD-3-Clause + */ ++#include + #include + #include + #include + #include ++#include + #include + #include ++#include + + #include "crypto_partition.h" + #include "crypto_uuid.h" +@@ -28,25 +31,27 @@ static rpc_status_t copy_key_handler(void *context, struct rpc_request *req); + static rpc_status_t purge_key_handler(void *context, struct rpc_request *req); + static rpc_status_t get_key_attributes_handler(void *context, struct rpc_request *req); + static rpc_status_t verify_pkcs7_signature_handler(void *context, struct rpc_request *req); ++static rpc_status_t get_uefi_priv_auth_var_fingerprint_handler(void *context, struct rpc_request *req); + + /* Handler mapping table for service */ + static const struct service_handler handler_table[] = { +- { TS_CRYPTO_OPCODE_GENERATE_KEY, generate_key_handler }, +- { TS_CRYPTO_OPCODE_DESTROY_KEY, destroy_key_handler }, +- { TS_CRYPTO_OPCODE_EXPORT_KEY, export_key_handler }, +- { TS_CRYPTO_OPCODE_EXPORT_PUBLIC_KEY, export_public_key_handler }, +- { TS_CRYPTO_OPCODE_IMPORT_KEY, import_key_handler }, +- { TS_CRYPTO_OPCODE_SIGN_HASH, asymmetric_sign_handler }, +- { TS_CRYPTO_OPCODE_VERIFY_HASH, asymmetric_verify_handler }, +- { TS_CRYPTO_OPCODE_ASYMMETRIC_DECRYPT, asymmetric_decrypt_handler }, +- { TS_CRYPTO_OPCODE_ASYMMETRIC_ENCRYPT, asymmetric_encrypt_handler }, +- { TS_CRYPTO_OPCODE_GENERATE_RANDOM, generate_random_handler }, +- { TS_CRYPTO_OPCODE_COPY_KEY, copy_key_handler }, +- { TS_CRYPTO_OPCODE_PURGE_KEY, purge_key_handler }, +- { TS_CRYPTO_OPCODE_GET_KEY_ATTRIBUTES, get_key_attributes_handler }, +- { TS_CRYPTO_OPCODE_SIGN_MESSAGE, asymmetric_sign_handler }, +- { TS_CRYPTO_OPCODE_VERIFY_MESSAGE, asymmetric_verify_handler }, +- { TS_CRYPTO_OPCODE_VERIFY_PKCS7_SIGNATURE, verify_pkcs7_signature_handler }, ++ { TS_CRYPTO_OPCODE_GENERATE_KEY, generate_key_handler }, ++ { TS_CRYPTO_OPCODE_DESTROY_KEY, destroy_key_handler }, ++ { TS_CRYPTO_OPCODE_EXPORT_KEY, export_key_handler }, ++ { TS_CRYPTO_OPCODE_EXPORT_PUBLIC_KEY, export_public_key_handler }, ++ { TS_CRYPTO_OPCODE_IMPORT_KEY, import_key_handler }, ++ { TS_CRYPTO_OPCODE_SIGN_HASH, asymmetric_sign_handler }, ++ { TS_CRYPTO_OPCODE_VERIFY_HASH, asymmetric_verify_handler }, ++ { TS_CRYPTO_OPCODE_ASYMMETRIC_DECRYPT, asymmetric_decrypt_handler }, ++ { TS_CRYPTO_OPCODE_ASYMMETRIC_ENCRYPT, asymmetric_encrypt_handler }, ++ { TS_CRYPTO_OPCODE_GENERATE_RANDOM, generate_random_handler }, ++ { TS_CRYPTO_OPCODE_COPY_KEY, copy_key_handler }, ++ { TS_CRYPTO_OPCODE_PURGE_KEY, purge_key_handler }, ++ { TS_CRYPTO_OPCODE_GET_KEY_ATTRIBUTES, get_key_attributes_handler }, ++ { TS_CRYPTO_OPCODE_SIGN_MESSAGE, asymmetric_sign_handler }, ++ { TS_CRYPTO_OPCODE_VERIFY_MESSAGE, asymmetric_verify_handler }, ++ { TS_CRYPTO_OPCODE_VERIFY_PKCS7_SIGNATURE, verify_pkcs7_signature_handler }, ++ { TS_CRYPTO_OPCODE_GET_UEFI_PRIV_AUTH_VAR_FINGERPRINT, get_uefi_priv_auth_var_fingerprint_handler }, + }; + + struct rpc_service_interface * +@@ -664,33 +669,44 @@ static rpc_status_t verify_pkcs7_signature_handler(void *context, struct rpc_req + } + + if (rpc_status == RPC_SUCCESS) { +- /* Parse the public key certificate */ +- mbedtls_x509_crt signer_certificate; ++ /* Parse the PKCS#7 DER encoded signature block */ ++ mbedtls_pkcs7 pkcs7_structure; + +- mbedtls_x509_crt_init(&signer_certificate); ++ mbedtls_pkcs7_init(&pkcs7_structure); + +- mbedtls_status = mbedtls_x509_crt_parse_der(&signer_certificate, public_key_cert, +- public_key_cert_len); ++ mbedtls_status = mbedtls_pkcs7_parse_der(&pkcs7_structure, signature_cert, ++ signature_cert_len); + +- if (mbedtls_status == 0) { +- /* Parse the PKCS#7 DER encoded signature block */ +- mbedtls_pkcs7 pkcs7_structure; ++ if (mbedtls_status == MBEDTLS_PKCS7_SIGNED_DATA) { + +- mbedtls_pkcs7_init(&pkcs7_structure); ++ /* ++ * If a separate public key is provided, verify the signature with it, ++ * else use the key from the pkcs7 signature structure, because it is ++ * a self-signed certificate. ++ */ ++ if(public_key_cert_len) { ++ /* Parse the public key certificate */ ++ mbedtls_x509_crt signer_certificate; + +- mbedtls_status = mbedtls_pkcs7_parse_der(&pkcs7_structure, signature_cert, +- signature_cert_len); ++ mbedtls_x509_crt_init(&signer_certificate); + +- if (mbedtls_status == MBEDTLS_PKCS7_SIGNED_DATA) { +- /* Verify hash against signed hash */ ++ mbedtls_status = mbedtls_x509_crt_parse_der(&signer_certificate, public_key_cert, ++ public_key_cert_len); ++ ++ if (mbedtls_status == 0) { ++ /* Verify hash against signed hash */ ++ mbedtls_status = mbedtls_pkcs7_signed_hash_verify( ++ &pkcs7_structure, &signer_certificate, hash, hash_len); ++ } ++ ++ mbedtls_x509_crt_free(&signer_certificate); ++ } else { + mbedtls_status = mbedtls_pkcs7_signed_hash_verify( +- &pkcs7_structure, &signer_certificate, hash, hash_len); ++ &pkcs7_structure, &pkcs7_structure.private_signed_data.private_certs, hash, hash_len); + } +- +- mbedtls_pkcs7_free(&pkcs7_structure); + } + +- mbedtls_x509_crt_free(&signer_certificate); ++ mbedtls_pkcs7_free(&pkcs7_structure); + } + + free(signature_cert); +@@ -702,6 +718,128 @@ static rpc_status_t verify_pkcs7_signature_handler(void *context, struct rpc_req + + return rpc_status; + } ++ ++/* ++ * Official value: http://www.oid-info.com/get/2.5.4.3 ++ * Hex converter: https://misc.daniel-marschall.de/asn.1/oid-converter/online.php ++ */ ++static const mbedtls_asn1_buf* findCommonName(const mbedtls_x509_name *name) ++{ ++ uint8_t CN_oid_tag = 0x06; ++ uint8_t CN_oid_len = 0x03; ++ uint8_t CN_oid_val[3] = {0x55, 0x04, 0x03}; ++ ++ while (name) ++ { ++ if (name->oid.tag == CN_oid_tag && name->oid.len == CN_oid_len) { ++ if (name->oid.p != NULL) { ++ if (!memcmp(name->oid.p, CN_oid_val, CN_oid_len)) ++ return &name->val; ++ } ++ } ++ ++ name = name->next; ++ } ++ ++ return NULL; ++} ++ ++static rpc_status_t get_uefi_priv_auth_var_fingerprint_handler(void *context, struct rpc_request *req) ++{ ++ rpc_status_t rpc_status = RPC_ERROR_INTERNAL; ++ struct rpc_buffer *req_buf = &req->request; ++ const struct crypto_provider_serializer *serializer = get_crypto_serializer(context, req); ++ ++ int mbedtls_status = MBEDTLS_ERR_PKCS7_VERIFY_FAIL; ++ ++ uint8_t *signature_cert = NULL; ++ uint64_t signature_cert_len = 0; ++ ++ if (serializer) { ++ /* First collect the lengths of the field */ ++ rpc_status = serializer->deserialize_get_uefi_priv_auth_var_fingerprint_req( ++ req_buf, NULL, &signature_cert_len); ++ ++ if (rpc_status == RPC_SUCCESS) { ++ /* Allocate the needed space and get the data */ ++ signature_cert = (uint8_t *)malloc(signature_cert_len); ++ ++ if (signature_cert) { ++ rpc_status = serializer->deserialize_get_uefi_priv_auth_var_fingerprint_req( ++ req_buf, signature_cert, &signature_cert_len); ++ } else { ++ rpc_status = RPC_ERROR_RESOURCE_FAILURE; ++ } ++ } ++ } ++ ++ if (rpc_status == RPC_SUCCESS) { ++ /* Parse the PKCS#7 DER encoded signature block */ ++ mbedtls_pkcs7 pkcs7_structure; ++ ++ mbedtls_pkcs7_init(&pkcs7_structure); ++ ++ mbedtls_status = mbedtls_pkcs7_parse_der(&pkcs7_structure, signature_cert, ++ signature_cert_len); ++ ++ if (mbedtls_status == MBEDTLS_PKCS7_SIGNED_DATA) { ++ ++ uint8_t output_buffer[PSA_HASH_MAX_SIZE] = { 0 }; ++ size_t __maybe_unused output_size = 0; ++ const mbedtls_asn1_buf *signerCertCN = NULL; ++ const mbedtls_x509_crt *topLevelCert = &pkcs7_structure.private_signed_data.private_certs; ++ const mbedtls_x509_buf *toplevelCertTbs = NULL; ++ struct rpc_buffer *resp_buf = &req->response;; ++ psa_hash_operation_t op = PSA_HASH_OPERATION_INIT; ++ ++ /* Find common name field of the signing certificate, which is the first in the chain */ ++ signerCertCN = findCommonName(&topLevelCert->subject); ++ if (!signerCertCN) ++ mbedtls_status = MBEDTLS_ERR_PKCS7_VERIFY_FAIL; ++ ++ /* Get the TopLevel certificate which is the last in the chain */ ++ while(topLevelCert->next) ++ topLevelCert = topLevelCert->next; ++ toplevelCertTbs = &topLevelCert->tbs; ++ ++ /* Hash the data to create the fingerprint */ ++ op = psa_hash_operation_init(); ++ ++ if (psa_hash_setup(&op, PSA_ALG_SHA_256) != PSA_SUCCESS) ++ mbedtls_status = MBEDTLS_ERR_PKCS7_VERIFY_FAIL; ++ ++ if (psa_hash_update(&op, signerCertCN->p, signerCertCN->len)) { ++ psa_hash_abort(&op); ++ mbedtls_status = MBEDTLS_ERR_PKCS7_VERIFY_FAIL; ++ } ++ ++ if (psa_hash_update(&op, toplevelCertTbs->p, toplevelCertTbs->len)) { ++ psa_hash_abort(&op); ++ mbedtls_status = MBEDTLS_ERR_PKCS7_VERIFY_FAIL; ++ } ++ ++ if (psa_hash_finish(&op, (uint8_t*)&output_buffer, PSA_HASH_MAX_SIZE, &output_size)) { ++ psa_hash_abort(&op); ++ mbedtls_status = MBEDTLS_ERR_PKCS7_VERIFY_FAIL; ++ } ++ ++ /* Clear the remaining part of the buffer for consistency */ ++ memset(&output_buffer[output_size], 0, PSA_HASH_MAX_SIZE - output_size); ++ ++ rpc_status = serializer->serialize_get_uefi_priv_auth_var_fingerprint_resp( ++ resp_buf, (uint8_t*)&output_buffer); ++ } ++ ++ mbedtls_pkcs7_free(&pkcs7_structure); ++ } ++ ++ free(signature_cert); ++ ++ /* Provide the result of the verification */ ++ req->service_status = (mbedtls_status == MBEDTLS_PKCS7_SIGNED_DATA) ? EFI_SUCCESS : EFI_COMPROMISED_DATA; ++ ++ return rpc_status; ++} + #else + static rpc_status_t verify_pkcs7_signature_handler(void *context, struct rpc_request *req) + { +@@ -710,4 +848,12 @@ static rpc_status_t verify_pkcs7_signature_handler(void *context, struct rpc_req + + return RPC_ERROR_INTERNAL; + } ++ ++static rpc_status_t get_uefi_priv_auth_var_fingerprint_handler(void *context, struct rpc_request *req) ++{ ++ (void)context; ++ (void)req; ++ ++ return RPC_ERROR_INTERNAL; ++} + #endif +diff --git a/components/service/crypto/provider/serializer/crypto_provider_serializer.h b/components/service/crypto/provider/serializer/crypto_provider_serializer.h +index bd5336c3d..2b965afdb 100644 +--- a/components/service/crypto/provider/serializer/crypto_provider_serializer.h ++++ b/components/service/crypto/provider/serializer/crypto_provider_serializer.h +@@ -126,6 +126,14 @@ struct crypto_provider_serializer { + uint8_t *hash, uint64_t *hash_len, + uint8_t *public_key_cert, + uint64_t *public_key_cert_len); ++ ++ /* Operation: get_uefi_priv_auth_var_fingerprintentifier */ ++ rpc_status_t (*deserialize_get_uefi_priv_auth_var_fingerprint_req)(const struct rpc_buffer *req_buf, ++ uint8_t *signed_data, ++ uint64_t *signed_data_len); ++ ++ rpc_status_t (*serialize_get_uefi_priv_auth_var_fingerprint_resp)(struct rpc_buffer *resp_buf, ++ const uint8_t *output); + }; + + #endif /* CRYPTO_PROVIDER_SERIALIZER_H */ +diff --git a/components/service/crypto/provider/serializer/packed-c/packedc_crypto_provider_serializer.c b/components/service/crypto/provider/serializer/packed-c/packedc_crypto_provider_serializer.c +index 050ef2f7d..89e07e2c8 100644 +--- a/components/service/crypto/provider/serializer/packed-c/packedc_crypto_provider_serializer.c ++++ b/components/service/crypto/provider/serializer/packed-c/packedc_crypto_provider_serializer.c +@@ -22,6 +22,7 @@ + #include + #include + #include ++#include + #include + #include + #include +@@ -675,6 +676,57 @@ static rpc_status_t deserialize_verify_pkcs7_signature_req( + return rpc_status; + } + ++/* Operation: get_uefi_priv_auth_var_fingerprintentifier */ ++static rpc_status_t deserialize_get_uefi_priv_auth_var_fingerprint_req(const struct rpc_buffer *req_buf, ++ uint8_t *signed_data, ++ uint64_t *signed_data_len) ++{ ++ rpc_status_t rpc_status = RPC_ERROR_INVALID_REQUEST_BODY; ++ ++ if (req_buf->data_length) { ++ struct tlv_const_iterator req_iter; ++ struct tlv_record decoded_record; ++ ++ rpc_status = RPC_SUCCESS; ++ ++ tlv_const_iterator_begin(&req_iter, (uint8_t *)req_buf->data, req_buf->data_length); ++ ++ if (tlv_find_decode(&req_iter, TS_CRYPTO_GET_UEFI_PRIV_AUTH_VAR_FINGERPRINT_IN_TAG_SIGNATURE, ++ &decoded_record)) { ++ *signed_data_len = decoded_record.length; ++ ++ if (signed_data) ++ memcpy(signed_data, decoded_record.value, decoded_record.length); ++ } else { ++ /* Default to a zero length */ ++ *signed_data_len = 0; ++ } ++ } ++ ++ return rpc_status; ++} ++ ++static rpc_status_t serialize_get_uefi_priv_auth_var_fingerprint_resp(struct rpc_buffer *resp_buf, ++ const uint8_t *output) ++{ ++ rpc_status_t rpc_status = RPC_ERROR_INTERNAL; ++ struct tlv_iterator resp_iter; ++ struct tlv_record out_record; ++ ++ out_record.tag = TS_CRYPTO_GET_UEFI_PRIV_AUTH_VAR_FINGERPRINT_OUT_TAG_IDENTIFIER; ++ out_record.length = PSA_HASH_MAX_SIZE; ++ out_record.value = output; ++ ++ tlv_iterator_begin(&resp_iter, resp_buf->data, resp_buf->size); ++ ++ if (tlv_encode(&resp_iter, &out_record)) { ++ resp_buf->data_length = tlv_required_space(PSA_HASH_MAX_SIZE); ++ rpc_status = RPC_SUCCESS; ++ } ++ ++ return rpc_status; ++} ++ + /* Singleton method to provide access to the serializer instance */ + const struct crypto_provider_serializer *packedc_crypto_provider_serializer_instance(void) + { +@@ -704,6 +756,8 @@ const struct crypto_provider_serializer *packedc_crypto_provider_serializer_inst + deserialize_generate_random_req, + serialize_generate_random_resp, + deserialize_verify_pkcs7_signature_req, ++ deserialize_get_uefi_priv_auth_var_fingerprint_req, ++ serialize_get_uefi_priv_auth_var_fingerprint_resp + }; + + return &instance; +diff --git a/components/service/uefi/smm_variable/backend/direct/uefi_direct_backend.c b/components/service/uefi/smm_variable/backend/direct/uefi_direct_backend.c +index bf978c5dd..c7ca07254 100644 +--- a/components/service/uefi/smm_variable/backend/direct/uefi_direct_backend.c ++++ b/components/service/uefi/smm_variable/backend/direct/uefi_direct_backend.c +@@ -9,6 +9,8 @@ + #include + #include + #include ++#include ++#include + + int verify_pkcs7_signature(const uint8_t *signature_cert, uint64_t signature_cert_len, + const uint8_t *hash, uint64_t hash_len, const uint8_t *public_key_cert, +@@ -46,3 +48,91 @@ int verify_pkcs7_signature(const uint8_t *signature_cert, uint64_t signature_cer + + return mbedtls_status; + } ++ ++/* ++ * Official value: http://www.oid-info.com/get/2.5.4.3 ++ * Hex converter: https://misc.daniel-marschall.de/asn.1/oid-converter/online.php ++ */ ++static const mbedtls_asn1_buf* findCommonName(const mbedtls_x509_name *name) ++{ ++ uint8_t CN_oid_tag = 0x06; ++ uint8_t CN_oid_len = 0x03; ++ uint8_t CN_oid_val[3] = {0x55, 0x04, 0x03}; ++ ++ while (name) ++ { ++ if (name->oid.tag == CN_oid_tag && name->oid.len == CN_oid_len) { ++ if (name->oid.p != NULL) { ++ if (!memcmp(name->oid.p, CN_oid_val, CN_oid_len)) ++ return &name->val; ++ } ++ } ++ ++ name = name->next; ++ } ++ ++ return NULL; ++} ++ ++int get_uefi_priv_auth_var_fingerprint_handler(const uint8_t *signature_cert, ++ uint64_t signature_cert_len, ++ uint8_t *output) ++{ ++ int mbedtls_status = MBEDTLS_ERR_PKCS7_VERIFY_FAIL; ++ ++ /* Parse the PKCS#7 DER encoded signature block */ ++ mbedtls_pkcs7 pkcs7_structure; ++ ++ mbedtls_pkcs7_init(&pkcs7_structure); ++ ++ mbedtls_status = mbedtls_pkcs7_parse_der(&pkcs7_structure, signature_cert, ++ signature_cert_len); ++ ++ if (mbedtls_status == MBEDTLS_PKCS7_SIGNED_DATA) { ++ ++ uint8_t output_buffer[PSA_HASH_MAX_SIZE] = { 0 }; ++ size_t __maybe_unused output_size = 0; ++ const mbedtls_asn1_buf *signerCertCN = NULL; ++ const mbedtls_x509_crt *topLevelCert = &pkcs7_structure.private_signed_data.private_certs; ++ const mbedtls_x509_buf *toplevelCertTbs = NULL; ++ psa_hash_operation_t op = PSA_HASH_OPERATION_INIT; ++ ++ /* Find common name field of the signing certificate, which is the first in the chain */ ++ signerCertCN = findCommonName(&topLevelCert->subject); ++ if (!signerCertCN) ++ mbedtls_status = MBEDTLS_ERR_PKCS7_VERIFY_FAIL; ++ ++ /* Get the TopLevel certificate which is the last in the chain */ ++ while(topLevelCert->next) ++ topLevelCert = topLevelCert->next; ++ toplevelCertTbs = &topLevelCert->tbs; ++ ++ /* Hash the data to create the fingerprint */ ++ op = psa_hash_operation_init(); ++ ++ if (psa_hash_setup(&op, PSA_ALG_SHA_256) != PSA_SUCCESS) ++ mbedtls_status = MBEDTLS_ERR_PKCS7_VERIFY_FAIL; ++ ++ if (psa_hash_update(&op, signerCertCN->p, signerCertCN->len)) { ++ psa_hash_abort(&op); ++ mbedtls_status = MBEDTLS_ERR_PKCS7_VERIFY_FAIL; ++ } ++ ++ if (psa_hash_update(&op, toplevelCertTbs->p, toplevelCertTbs->len)) { ++ psa_hash_abort(&op); ++ mbedtls_status = MBEDTLS_ERR_PKCS7_VERIFY_FAIL; ++ } ++ ++ if (psa_hash_finish(&op, (uint8_t*)&output_buffer, PSA_HASH_MAX_SIZE, &output_size)) { ++ psa_hash_abort(&op); ++ mbedtls_status = MBEDTLS_ERR_PKCS7_VERIFY_FAIL; ++ } ++ ++ /* Clear the remaining part of the buffer for consistency */ ++ memset(&output_buffer[output_size], 0, PSA_HASH_MAX_SIZE - output_size); ++ } ++ ++ mbedtls_pkcs7_free(&pkcs7_structure); ++ ++ return mbedtls_status; ++} +diff --git a/deployments/smm-gateway/smm-gateway.cmake b/deployments/smm-gateway/smm-gateway.cmake +index e5ee03b60..de519892d 100644 +--- a/deployments/smm-gateway/smm-gateway.cmake ++++ b/deployments/smm-gateway/smm-gateway.cmake +@@ -17,6 +17,11 @@ include(${TS_ROOT}/external/MbedTLS/MbedTLS.cmake) + target_link_libraries(smm-gateway PRIVATE MbedTLS::mbedcrypto) + target_link_libraries(smm-gateway PRIVATE MbedTLS::mbedx509) + ++# Pass the location of the mbedtls config file to C preprocessor. ++target_compile_definitions(smm-gateway PRIVATE ++ MBEDTLS_USER_CONFIG_FILE="${MBEDTLS_USER_CONFIG_FILE}" ++) ++ + target_compile_definitions(smm-gateway PRIVATE + -DUEFI_INTERNAL_CRYPTO + ) +diff --git a/protocols/service/crypto/packed-c/get_uefi_priv_auth_var_fingerprint.h b/protocols/service/crypto/packed-c/get_uefi_priv_auth_var_fingerprint.h +new file mode 100644 +index 000000000..29964b33c +--- /dev/null ++++ b/protocols/service/crypto/packed-c/get_uefi_priv_auth_var_fingerprint.h +@@ -0,0 +1,21 @@ ++/* ++ * Copyright (c) 2024, Arm Limited and Contributors. All rights reserved. ++ * SPDX-License-Identifier: BSD-3-Clause ++ */ ++ ++#ifndef TS_CRYPTO_GET_UEFI_PRIV_AUTH_VAR_FINGERPRINT_H ++#define TS_CRYPTO_GET_UEFI_PRIV_AUTH_VAR_FINGERPRINT_H ++ ++#include ++ ++/* Variable length output parameter tags */ ++enum { ++ TS_CRYPTO_GET_UEFI_PRIV_AUTH_VAR_FINGERPRINT_OUT_TAG_IDENTIFIER = 1, ++}; ++ ++/* Variable length input parameter tags */ ++enum { ++ TS_CRYPTO_GET_UEFI_PRIV_AUTH_VAR_FINGERPRINT_IN_TAG_SIGNATURE = 1, ++}; ++ ++#endif /* TS_CRYPTO_GET_UEFI_PRIV_AUTH_VAR_FINGERPRINT_H */ +diff --git a/protocols/service/crypto/packed-c/opcodes.h b/protocols/service/crypto/packed-c/opcodes.h +index 35b81599b..8bc2b49b0 100644 +--- a/protocols/service/crypto/packed-c/opcodes.h ++++ b/protocols/service/crypto/packed-c/opcodes.h +@@ -28,6 +28,7 @@ + #define TS_CRYPTO_OPCODE_SIGN_MESSAGE (TS_CRYPTO_OPCODE_BASE + 16) + #define TS_CRYPTO_OPCODE_VERIFY_MESSAGE (TS_CRYPTO_OPCODE_BASE + 17) + #define TS_CRYPTO_OPCODE_VERIFY_PKCS7_SIGNATURE (TS_CRYPTO_OPCODE_BASE + 18) ++#define TS_CRYPTO_OPCODE_GET_UEFI_PRIV_AUTH_VAR_FINGERPRINT (TS_CRYPTO_OPCODE_BASE + 19) + + /* Hash operations */ + #define TS_CRYPTO_OPCODE_HASH_BASE (0x0200) +-- +2.25.1 + diff --git a/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0015-Add-timestamp-validation-for-uefi-variables.patch b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0015-Add-timestamp-validation-for-uefi-variables.patch new file mode 100644 index 00000000..26e7df5f --- /dev/null +++ b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0015-Add-timestamp-validation-for-uefi-variables.patch @@ -0,0 +1,146 @@ +From 5b418e141aadcb6604406f75e156317bd143d898 Mon Sep 17 00:00:00 2001 +From: Gabor Toth +Date: Fri, 5 Apr 2024 11:27:15 +0200 +Subject: [PATCH 1/3] Add timestamp validation for uefi variables + +Return failure if uefi variable creation or update is not +requested with newer timestamp. + +Signed-off-by: Gabor Toth +Upstream-Status: Submitted [https://review.trustedfirmware.org/c/TS/trusted-services/+/27955] +--- + .../backend/uefi_variable_store.c | 35 +++++++++++++++---- + .../smm_variable/backend/variable_index.c | 1 + + .../smm_variable/backend/variable_index.h | 1 + + 3 files changed, 30 insertions(+), 7 deletions(-) + +diff --git a/components/service/uefi/smm_variable/backend/uefi_variable_store.c b/components/service/uefi/smm_variable/backend/uefi_variable_store.c +index c1691dc8f..1b624f0c9 100644 +--- a/components/service/uefi/smm_variable/backend/uefi_variable_store.c ++++ b/components/service/uefi/smm_variable/backend/uefi_variable_store.c +@@ -76,6 +76,7 @@ static efi_status_t verify_var_by_key_var(const efi_data_map *new_var, + const uint8_t *hash_buffer, size_t hash_len); + + static efi_status_t authenticate_variable(const struct uefi_variable_store *context, ++ EFI_TIME *timestamp, + SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var); + #endif + +@@ -197,6 +198,7 @@ efi_status_t uefi_variable_store_set_variable(const struct uefi_variable_store * + const SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var) + { + bool should_sync_index = false; ++ EFI_TIME timestamp = { 0 }; + + /* Validate incoming request */ + efi_status_t status = check_name_terminator(var->Name, var->NameSize); +@@ -225,6 +227,9 @@ efi_status_t uefi_variable_store_set_variable(const struct uefi_variable_store * + return EFI_OUT_OF_RESOURCES; + } + ++ /* Save the timestamp into a buffer, which can be overwritten by the authentication function */ ++ memcpy(×tamp, &info->metadata.timestamp, sizeof(EFI_TIME)); ++ + /* Control access */ + status = check_access_permitted_on_set(context, info, var); + +@@ -240,7 +245,7 @@ efi_status_t uefi_variable_store_set_variable(const struct uefi_variable_store * + if (info->metadata.attributes & + EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) { + status = authenticate_variable( +- context, (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *)var); ++ context, ×tamp, (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *)var); + + if (status != EFI_SUCCESS) + return status; +@@ -326,7 +331,7 @@ efi_status_t uefi_variable_store_set_variable(const struct uefi_variable_store * + */ + if (var->Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) { + status = authenticate_variable( +- context, (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *)var); ++ context, ×tamp, (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *)var); + + if (status != EFI_SUCCESS) + return status; +@@ -358,9 +363,11 @@ efi_status_t uefi_variable_store_set_variable(const struct uefi_variable_store * + if (should_sync_index) + status = sync_variable_index(context); + +- /* Store any variable data to the storage backend */ +- if (info->is_variable_set && (status == EFI_SUCCESS)) ++ /* Store any variable data to the storage backend with the updated metadata */ ++ if (info->is_variable_set && (status == EFI_SUCCESS)) { ++ memcpy(&info->metadata.timestamp, ×tamp, sizeof(EFI_TIME)); + status = store_variable_data(context, info, var); ++ } + } + + variable_index_remove_unused_entry(&context->variable_index, info); +@@ -1106,6 +1113,7 @@ static efi_status_t verify_var_by_key_var(const efi_data_map *new_var, + * then verifies it. + */ + static efi_status_t authenticate_variable(const struct uefi_variable_store *context, ++ EFI_TIME *timestamp, + SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var) + { + efi_status_t status = EFI_SUCCESS; +@@ -1223,9 +1231,7 @@ static efi_status_t authenticate_variable(const struct uefi_variable_store *cont + * + * UEFI: Page 253 + * 2. Verify that Pad1, Nanosecond, TimeZone, Daylight and Pad2 components +- * of the TimeStamp value are set to zero. Unless the EFI_VARIABLE_APPEND_WRITE +- * attribute is set, verify that the TimeStamp value is later than the current +- * timestamp value associated with the variable ++ * of the TimeStamp value are set to zero. + */ + if ((var_map.efi_auth_descriptor->TimeStamp.Pad1 != 0) || + (var_map.efi_auth_descriptor->TimeStamp.Pad2 != 0) || +@@ -1235,6 +1241,21 @@ static efi_status_t authenticate_variable(const struct uefi_variable_store *cont + return EFI_SECURITY_VIOLATION; + } + ++ /** ++ * UEFI: Page 253 ++ * Unless the EFI_VARIABLE_APPEND_WRITE attribute is set, verify ++ * that the TimeStamp value is later than the current ++ * timestamp value associated with the variable ++ */ ++ if (!(var->Attributes & EFI_VARIABLE_APPEND_WRITE)) { ++ if (memcmp(&var_map.efi_auth_descriptor->TimeStamp, timestamp, sizeof(EFI_GUID)) <= 0) { ++ EMSG("Timestamp violation"); ++ return EFI_SECURITY_VIOLATION; ++ } ++ ++ /* Save new timestamp */ ++ memcpy(timestamp, &var_map.efi_auth_descriptor->TimeStamp, sizeof(EFI_TIME)); ++ } + /* Calculate hash for the variable only once */ + hash_result = calc_variable_hash(&var_map, (uint8_t *)&hash_buffer, sizeof(hash_buffer), + &hash_len); +diff --git a/components/service/uefi/smm_variable/backend/variable_index.c b/components/service/uefi/smm_variable/backend/variable_index.c +index e2fe6dd38..f4194d2d3 100644 +--- a/components/service/uefi/smm_variable/backend/variable_index.c ++++ b/components/service/uefi/smm_variable/backend/variable_index.c +@@ -198,6 +198,7 @@ static struct variable_entry *add_entry(const struct variable_index *context, co + /* Initialize metadata */ + info->metadata.uid = generate_uid(context, guid, name_size, name); + info->metadata.guid = *guid; ++ memset(&info->metadata.timestamp, 0, sizeof(EFI_TIME)); + info->metadata.attributes = 0; + info->metadata.name_size = name_size; + memcpy(info->metadata.name, name, name_size); +diff --git a/components/service/uefi/smm_variable/backend/variable_index.h b/components/service/uefi/smm_variable/backend/variable_index.h +index 5d3b7a7c6..7eef7b86b 100644 +--- a/components/service/uefi/smm_variable/backend/variable_index.h ++++ b/components/service/uefi/smm_variable/backend/variable_index.h +@@ -32,6 +32,7 @@ extern "C" { + */ + struct variable_metadata { + EFI_GUID guid; ++ EFI_TIME timestamp; + size_t name_size; + int16_t name[VARIABLE_INDEX_MAX_NAME_SIZE]; + uint32_t attributes; +-- +2.25.1 + diff --git a/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0016-Isolate-common-uefi-variable-authentication-steps.patch b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0016-Isolate-common-uefi-variable-authentication-steps.patch new file mode 100644 index 00000000..16ca63b3 --- /dev/null +++ b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0016-Isolate-common-uefi-variable-authentication-steps.patch @@ -0,0 +1,282 @@ +From 19e79008e0fa3193b54bf6499516dc75cb10f6ec Mon Sep 17 00:00:00 2001 +From: Gabor Toth +Date: Thu, 11 Apr 2024 13:42:03 +0200 +Subject: [PATCH 2/3] Isolate common uefi variable authentication steps + +Currently all auth variables are authenticated with the secure boot +keys. To introduce corrent check for Private Authenticated Variables +first separate the common steps from the secure boot related steps. + +Signed-off-by: Gabor Toth +Upstream-Status: Submitted [https://review.trustedfirmware.org/c/TS/trusted-services/+/27956] +--- + .../backend/uefi_variable_store.c | 191 ++++++++++-------- + 1 file changed, 103 insertions(+), 88 deletions(-) + +diff --git a/components/service/uefi/smm_variable/backend/uefi_variable_store.c b/components/service/uefi/smm_variable/backend/uefi_variable_store.c +index 1b624f0c9..1384d0def 100644 +--- a/components/service/uefi/smm_variable/backend/uefi_variable_store.c ++++ b/components/service/uefi/smm_variable/backend/uefi_variable_store.c +@@ -78,6 +78,12 @@ static efi_status_t verify_var_by_key_var(const efi_data_map *new_var, + static efi_status_t authenticate_variable(const struct uefi_variable_store *context, + EFI_TIME *timestamp, + SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var); ++ ++static efi_status_t authenticate_secure_boot_variable(const struct uefi_variable_store *context, ++ efi_data_map* var_map, ++ uint8_t* hash_buffer, ++ size_t hash_len, ++ uint64_t max_variable_size); + #endif + + static efi_status_t store_variable_data(const struct uefi_variable_store *context, +@@ -1118,30 +1124,109 @@ static efi_status_t authenticate_variable(const struct uefi_variable_store *cont + { + efi_status_t status = EFI_SUCCESS; + EFI_GUID pkcs7_guid = EFI_CERT_TYPE_PKCS7_GUID; +- EFI_GUID global_variable_guid = EFI_GLOBAL_VARIABLE; +- EFI_GUID security_database_guid = EFI_IMAGE_SECURITY_DATABASE_GUID; + SMM_VARIABLE_COMMUNICATE_QUERY_VARIABLE_INFO variable_info = { 0, 0, 0, 0 }; +- SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *pk_variable = NULL; +- size_t pk_payload_size = 0; + efi_data_map var_map = { NULL, NULL, NULL, 0, 0, NULL, 0, NULL }; + uint8_t hash_buffer[PSA_HASH_MAX_SIZE]; + size_t hash_len = 0; +- bool hash_result = false; + + /* Create a map of the fields of the new variable including the auth header */ + if (!init_efi_data_map(var, true, &var_map)) + return EFI_SECURITY_VIOLATION; + +- /* database variables can be verified by either PK or KEK while images +- * should be checked by db and dbx so the length of two will be enough. +- */ +- SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *allowed_key_store_variables[] = { NULL, NULL }; +- + /* Find the maximal size of variables for the GetVariable operation */ + status = uefi_variable_store_query_variable_info(context, &variable_info); + if (status != EFI_SUCCESS) + return EFI_SECURITY_VIOLATION; + ++ /** ++ * UEFI: Page 246 ++ * If the EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS attribute is set in a ++ * SetVariable() call, and firmware does not support signature type of the certificate ++ * included in the EFI_VARIABLE_AUTHENTICATION_2 descriptor, then the SetVariable() call ++ * shall return EFI_INVALID_PARAMETER. The list of signature types supported by the ++ * firmware is defined by the SignatureSupport variable. Signature type of the certificate ++ * is defined by its digest and encryption algorithms. ++ */ ++ /* TODO: Should support WIN_CERT_TYPE_PKCS_SIGNED_DATA and WIN_CERT_TYPE_EFI_PKCS115 */ ++ if (var_map.efi_auth_descriptor->AuthInfo.Hdr.wCertificateType != WIN_CERT_TYPE_EFI_GUID) ++ return EFI_INVALID_PARAMETER; ++ ++ /* Only a CertType of EFI_CERT_TYPE_PKCS7_GUID is accepted */ ++ if (!compare_guid(&var_map.efi_auth_descriptor->AuthInfo.CertType, &pkcs7_guid)) ++ return EFI_SECURITY_VIOLATION; ++ ++ /** ++ * Time associated with the authentication descriptor. For the TimeStamp value, ++ * components Pad1, Nanosecond, TimeZone, Daylight and Pad2 shall be set to 0. ++ * This means that the time shall always be expressed in GMT. ++ * ++ * UEFI: Page 253 ++ * 2. Verify that Pad1, Nanosecond, TimeZone, Daylight and Pad2 components ++ * of the TimeStamp value are set to zero. ++ */ ++ if ((var_map.efi_auth_descriptor->TimeStamp.Pad1 != 0) || ++ (var_map.efi_auth_descriptor->TimeStamp.Pad2 != 0) || ++ (var_map.efi_auth_descriptor->TimeStamp.Nanosecond != 0) || ++ (var_map.efi_auth_descriptor->TimeStamp.TimeZone != 0) || ++ (var_map.efi_auth_descriptor->TimeStamp.Daylight != 0)) { ++ return EFI_SECURITY_VIOLATION; ++ } ++ ++ /** ++ * UEFI: Page 253 ++ * Unless the EFI_VARIABLE_APPEND_WRITE attribute is set, verify ++ * that the TimeStamp value is later than the current ++ * timestamp value associated with the variable ++ */ ++ if (!(var->Attributes & EFI_VARIABLE_APPEND_WRITE)) { ++ if (memcmp(&var_map.efi_auth_descriptor->TimeStamp, timestamp, sizeof(EFI_GUID)) <= 0) { ++ EMSG("Timestamp violation"); ++ return EFI_SECURITY_VIOLATION; ++ } ++ ++ /* Save new timestamp */ ++ memcpy(timestamp, &var_map.efi_auth_descriptor->TimeStamp, sizeof(EFI_TIME)); ++ } ++ /* Calculate hash for the variable only once */ ++ if (calc_variable_hash(&var_map, (uint8_t *)&hash_buffer, sizeof(hash_buffer), &hash_len) == 0) { ++ status = EFI_SECURITY_VIOLATION; ++ } ++ ++ /* Run Secure Boot related authentication steps */ ++ status = authenticate_secure_boot_variable(context, &var_map, (uint8_t*) &hash_buffer, hash_len, variable_info.MaximumVariableSize); ++ ++ /* Remove the authentication header from the variable if the authentication is successful */ ++ if (status == EFI_SUCCESS) { ++ uint8_t *smm_payload = ++ (uint8_t *)var + SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_DATA_OFFSET(var); ++ ++ memmove(smm_payload, var_map.payload, var_map.payload_len); ++ memset((uint8_t *)smm_payload + var_map.payload_len, 0, ++ var_map.efi_auth_descriptor_len); ++ ++ var->DataSize -= var_map.efi_auth_descriptor_len; ++ } ++ ++ return status; ++} ++ ++static efi_status_t authenticate_secure_boot_variable(const struct uefi_variable_store *context, ++ efi_data_map* var_map, ++ uint8_t* hash_buffer, ++ size_t hash_len, ++ uint64_t max_variable_size) ++{ ++ efi_status_t status = EFI_SUCCESS; ++ EFI_GUID global_variable_guid = EFI_GLOBAL_VARIABLE; ++ EFI_GUID security_database_guid = EFI_IMAGE_SECURITY_DATABASE_GUID; ++ SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *pk_variable = NULL; ++ size_t pk_payload_size = 0; ++ ++ /* database variables can be verified by either PK or KEK while images ++ * should be checked by db and dbx so the length of two will be enough. ++ */ ++ SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *allowed_key_store_variables[] = { NULL, NULL }; ++ + /** + * UEFI: Page 253 + * 3. If the variable SetupMode==1, and the variable is a secure +@@ -1166,14 +1251,14 @@ static efi_status_t authenticate_variable(const struct uefi_variable_store *cont + * Platform Key is checked to enable or disable authentication. + */ + create_smm_variable(&pk_variable, sizeof(EFI_PLATFORM_KEY_NAME), +- variable_info.MaximumVariableSize, (uint8_t *)EFI_PLATFORM_KEY_NAME, ++ max_variable_size, (uint8_t *)EFI_PLATFORM_KEY_NAME, + &global_variable_guid); + + if (!pk_variable) + return EFI_OUT_OF_RESOURCES; + + status = uefi_variable_store_get_variable( +- context, pk_variable, variable_info.MaximumVariableSize, &pk_payload_size); ++ context, pk_variable, max_variable_size, &pk_payload_size); + + /* If PK does not exist authentication is disabled */ + if (status != EFI_SUCCESS) { +@@ -1207,66 +1292,8 @@ static efi_status_t authenticate_variable(const struct uefi_variable_store *cont + goto end; + } + +- /** +- * UEFI: Page 246 +- * If the EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS attribute is set in a +- * SetVariable() call, and firmware does not support signature type of the certificate +- * included in the EFI_VARIABLE_AUTHENTICATION_2 descriptor, then the SetVariable() call +- * shall return EFI_INVALID_PARAMETER. The list of signature types supported by the +- * firmware is defined by the SignatureSupport variable. Signature type of the certificate +- * is defined by its digest and encryption algorithms. +- */ +- /* TODO: Should support WIN_CERT_TYPE_PKCS_SIGNED_DATA and WIN_CERT_TYPE_EFI_PKCS115 */ +- if (var_map.efi_auth_descriptor->AuthInfo.Hdr.wCertificateType != WIN_CERT_TYPE_EFI_GUID) +- return EFI_INVALID_PARAMETER; +- +- /* Only a CertType of EFI_CERT_TYPE_PKCS7_GUID is accepted */ +- if (!compare_guid(&var_map.efi_auth_descriptor->AuthInfo.CertType, &pkcs7_guid)) +- return EFI_SECURITY_VIOLATION; +- +- /** +- * Time associated with the authentication descriptor. For the TimeStamp value, +- * components Pad1, Nanosecond, TimeZone, Daylight and Pad2 shall be set to 0. +- * This means that the time shall always be expressed in GMT. +- * +- * UEFI: Page 253 +- * 2. Verify that Pad1, Nanosecond, TimeZone, Daylight and Pad2 components +- * of the TimeStamp value are set to zero. +- */ +- if ((var_map.efi_auth_descriptor->TimeStamp.Pad1 != 0) || +- (var_map.efi_auth_descriptor->TimeStamp.Pad2 != 0) || +- (var_map.efi_auth_descriptor->TimeStamp.Nanosecond != 0) || +- (var_map.efi_auth_descriptor->TimeStamp.TimeZone != 0) || +- (var_map.efi_auth_descriptor->TimeStamp.Daylight != 0)) { +- return EFI_SECURITY_VIOLATION; +- } +- +- /** +- * UEFI: Page 253 +- * Unless the EFI_VARIABLE_APPEND_WRITE attribute is set, verify +- * that the TimeStamp value is later than the current +- * timestamp value associated with the variable +- */ +- if (!(var->Attributes & EFI_VARIABLE_APPEND_WRITE)) { +- if (memcmp(&var_map.efi_auth_descriptor->TimeStamp, timestamp, sizeof(EFI_GUID)) <= 0) { +- EMSG("Timestamp violation"); +- return EFI_SECURITY_VIOLATION; +- } +- +- /* Save new timestamp */ +- memcpy(timestamp, &var_map.efi_auth_descriptor->TimeStamp, sizeof(EFI_TIME)); +- } +- /* Calculate hash for the variable only once */ +- hash_result = calc_variable_hash(&var_map, (uint8_t *)&hash_buffer, sizeof(hash_buffer), +- &hash_len); +- +- if (!hash_result) { +- status = EFI_SECURITY_VIOLATION; +- goto end; +- } +- +- status = select_verification_keys(var_map, global_variable_guid, security_database_guid, +- variable_info.MaximumVariableSize, ++ status = select_verification_keys(*var_map, global_variable_guid, security_database_guid, ++ max_variable_size, + &allowed_key_store_variables[0]); + + if (status != EFI_SUCCESS) +@@ -1280,8 +1307,8 @@ static efi_status_t authenticate_variable(const struct uefi_variable_store *cont + continue; + + status = uefi_variable_store_get_variable(context, allowed_key_store_variables[i], +- variable_info.MaximumVariableSize, +- &actual_variable_length); ++ max_variable_size, ++ &actual_variable_length); + + if (status) { + /* When the parent does not exist it is considered verification failure */ +@@ -1297,8 +1324,8 @@ static efi_status_t authenticate_variable(const struct uefi_variable_store *cont + goto end; + } + +- status = verify_var_by_key_var(&var_map, &allowed_key_store_var_map, +- (uint8_t *)&hash_buffer, hash_len); ++ status = verify_var_by_key_var(var_map, &allowed_key_store_var_map, ++ hash_buffer, hash_len); + + if (status == EFI_SUCCESS) + goto end; +@@ -1311,18 +1338,6 @@ end: + free(allowed_key_store_variables[i]); + } + +- /* Remove the authentication header from the variable if the authentication is successful */ +- if (status == EFI_SUCCESS) { +- uint8_t *smm_payload = +- (uint8_t *)var + SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE_DATA_OFFSET(var); +- +- memmove(smm_payload, var_map.payload, var_map.payload_len); +- memset((uint8_t *)smm_payload + var_map.payload_len, 0, +- var_map.efi_auth_descriptor_len); +- +- var->DataSize -= var_map.efi_auth_descriptor_len; +- } +- + return status; + } + #endif +-- +2.25.1 + diff --git a/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0017-Implement-Private-Authenticated-Variable-verificatio.patch b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0017-Implement-Private-Authenticated-Variable-verificatio.patch new file mode 100644 index 00000000..eb7852f0 --- /dev/null +++ b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0017-Implement-Private-Authenticated-Variable-verificatio.patch @@ -0,0 +1,292 @@ +From a172c6e8269915db1b25e2749bae06dc0220cfb8 Mon Sep 17 00:00:00 2001 +From: Gabor Toth +Date: Thu, 11 Apr 2024 13:48:14 +0200 +Subject: [PATCH 3/3] Implement Private Authenticated Variable verification + +Refactor the implementation to only use the PK, KEK, DB authentication +chain for boot variables, and implement the self authentication for +private authenticated variables. + +Signed-off-by: Gabor Toth +Upstream-Status: Submitted [https://review.trustedfirmware.org/c/TS/trusted-services/+/27957] +--- + .../backend/uefi_variable_store.c | 126 +++++++++++++++--- + .../smm_variable/backend/variable_index.c | 1 + + .../smm_variable/backend/variable_index.h | 2 + + .../config/default-opteesp/CMakeLists.txt | 2 +- + .../config/default-sp/CMakeLists.txt | 2 +- + 5 files changed, 112 insertions(+), 21 deletions(-) + +diff --git a/components/service/uefi/smm_variable/backend/uefi_variable_store.c b/components/service/uefi/smm_variable/backend/uefi_variable_store.c +index 1384d0def..97c43dc74 100644 +--- a/components/service/uefi/smm_variable/backend/uefi_variable_store.c ++++ b/components/service/uefi/smm_variable/backend/uefi_variable_store.c +@@ -75,15 +75,25 @@ static efi_status_t verify_var_by_key_var(const efi_data_map *new_var, + const efi_data_map *key_store_var, + const uint8_t *hash_buffer, size_t hash_len); + ++static bool isPrivateAuthVar(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var); ++ + static efi_status_t authenticate_variable(const struct uefi_variable_store *context, +- EFI_TIME *timestamp, +- SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var); ++ EFI_TIME *timestamp, uint8_t (*fingerprint)[FINGERPRINT_SIZE], ++ bool new_variable, SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var); + + static efi_status_t authenticate_secure_boot_variable(const struct uefi_variable_store *context, + efi_data_map* var_map, + uint8_t* hash_buffer, + size_t hash_len, + uint64_t max_variable_size); ++ ++static efi_status_t authenticate_private_variable(const struct uefi_variable_store *context, ++ efi_data_map* var_map, ++ uint8_t* hash_buffer, ++ size_t hash_len, ++ uint64_t max_variable_size, ++ bool new_variable, ++ uint8_t (*fingerprint)[FINGERPRINT_SIZE]); + #endif + + static efi_status_t store_variable_data(const struct uefi_variable_store *context, +@@ -205,6 +215,7 @@ efi_status_t uefi_variable_store_set_variable(const struct uefi_variable_store * + { + bool should_sync_index = false; + EFI_TIME timestamp = { 0 }; ++ uint8_t fingerprint[FINGERPRINT_SIZE] = { 0 }; + + /* Validate incoming request */ + efi_status_t status = check_name_terminator(var->Name, var->NameSize); +@@ -233,8 +244,9 @@ efi_status_t uefi_variable_store_set_variable(const struct uefi_variable_store * + return EFI_OUT_OF_RESOURCES; + } + +- /* Save the timestamp into a buffer, which can be overwritten by the authentication function */ ++ /* Save the timestamp and fingerprints into a buffer, which can be overwritten by the authentication function */ + memcpy(×tamp, &info->metadata.timestamp, sizeof(EFI_TIME)); ++ memcpy(&fingerprint, &info->metadata.fingerprint, FINGERPRINT_SIZE); + + /* Control access */ + status = check_access_permitted_on_set(context, info, var); +@@ -251,7 +263,8 @@ efi_status_t uefi_variable_store_set_variable(const struct uefi_variable_store * + if (info->metadata.attributes & + EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) { + status = authenticate_variable( +- context, ×tamp, (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *)var); ++ context, ×tamp, &fingerprint, false, ++ (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *)var); + + if (status != EFI_SUCCESS) + return status; +@@ -337,7 +350,8 @@ efi_status_t uefi_variable_store_set_variable(const struct uefi_variable_store * + */ + if (var->Attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) { + status = authenticate_variable( +- context, ×tamp, (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *)var); ++ context, ×tamp, &fingerprint, true, ++ (SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *)var); + + if (status != EFI_SUCCESS) + return status; +@@ -372,6 +386,7 @@ efi_status_t uefi_variable_store_set_variable(const struct uefi_variable_store * + /* Store any variable data to the storage backend with the updated metadata */ + if (info->is_variable_set && (status == EFI_SUCCESS)) { + memcpy(&info->metadata.timestamp, ×tamp, sizeof(EFI_TIME)); ++ memcpy(&info->metadata.fingerprint, &fingerprint, FINGERPRINT_SIZE); + status = store_variable_data(context, info, var); + } + } +@@ -1030,15 +1045,6 @@ select_verification_keys(const efi_data_map new_var, EFI_GUID global_variable_gu + create_smm_variable(&(allowed_key_store_variables[1]), + sizeof(EFI_KEY_EXCHANGE_KEY_NAME), maximum_variable_size, + (uint8_t *)EFI_KEY_EXCHANGE_KEY_NAME, &global_variable_guid); +- } else { +- /* +- * Any other variable is considered Private Authenticated Variable. +- * These are verified by db +- */ +- create_smm_variable(&(allowed_key_store_variables[0]), +- sizeof(EFI_IMAGE_SECURITY_DATABASE), maximum_variable_size, +- (uint8_t *)EFI_IMAGE_SECURITY_DATABASE, +- &security_database_guid); + } + + return EFI_SUCCESS; +@@ -1114,13 +1120,39 @@ static efi_status_t verify_var_by_key_var(const efi_data_map *new_var, + return EFI_SECURITY_VIOLATION; + } + +-/* Basic verification of the authentication header of the new variable. ++static bool isPrivateAuthVar(SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var) ++{ ++ if (compare_name_to_key_store_name(var->Name, ++ var->NameSize, EFI_PLATFORM_KEY_NAME, ++ sizeof(EFI_PLATFORM_KEY_NAME)) || ++ compare_name_to_key_store_name( ++ var->Name, var->NameSize, ++ EFI_KEY_EXCHANGE_KEY_NAME, sizeof(EFI_KEY_EXCHANGE_KEY_NAME)) || ++ compare_name_to_key_store_name( ++ var->Name, var->NameSize, ++ EFI_IMAGE_SECURITY_DATABASE, sizeof(EFI_IMAGE_SECURITY_DATABASE)) || ++ compare_name_to_key_store_name( ++ var->Name, var->NameSize, ++ EFI_IMAGE_SECURITY_DATABASE1, sizeof(EFI_IMAGE_SECURITY_DATABASE1)) || ++ compare_name_to_key_store_name( ++ var->Name, var->NameSize, ++ EFI_IMAGE_SECURITY_DATABASE2, sizeof(EFI_IMAGE_SECURITY_DATABASE2)) || ++ compare_name_to_key_store_name( ++ var->Name, var->NameSize, ++ EFI_IMAGE_SECURITY_DATABASE3, sizeof(EFI_IMAGE_SECURITY_DATABASE3))) ++ return false; ++ ++ return true; ++} ++ ++/* ++ * Basic verification of the authentication header of the new variable. + * First finds the key variable responsible for the authentication of the new variable, + * then verifies it. + */ + static efi_status_t authenticate_variable(const struct uefi_variable_store *context, +- EFI_TIME *timestamp, +- SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var) ++ EFI_TIME *timestamp, uint8_t (*fingerprint)[FINGERPRINT_SIZE], ++ bool new_variable, SMM_VARIABLE_COMMUNICATE_ACCESS_VARIABLE *var) + { + efi_status_t status = EFI_SUCCESS; + EFI_GUID pkcs7_guid = EFI_CERT_TYPE_PKCS7_GUID; +@@ -1192,8 +1224,13 @@ static efi_status_t authenticate_variable(const struct uefi_variable_store *cont + status = EFI_SECURITY_VIOLATION; + } + +- /* Run Secure Boot related authentication steps */ +- status = authenticate_secure_boot_variable(context, &var_map, (uint8_t*) &hash_buffer, hash_len, variable_info.MaximumVariableSize); ++ if (isPrivateAuthVar(var)) { ++ /* Run Private Authenticated Variable related authentication steps */ ++ status = authenticate_private_variable(context, &var_map, (uint8_t*) &hash_buffer, hash_len, variable_info.MaximumVariableSize, new_variable, fingerprint); ++ } else { ++ /* Run Secure Boot related authentication steps */ ++ status = authenticate_secure_boot_variable(context, &var_map, (uint8_t*) &hash_buffer, hash_len, variable_info.MaximumVariableSize); ++ } + + /* Remove the authentication header from the variable if the authentication is successful */ + if (status == EFI_SUCCESS) { +@@ -1340,6 +1377,57 @@ end: + + return status; + } ++ ++static efi_status_t authenticate_private_variable(const struct uefi_variable_store *context, ++ efi_data_map* var_map, ++ uint8_t* hash_buffer, ++ size_t hash_len, ++ uint64_t max_variable_size, ++ bool new_variable, ++ uint8_t (*fingerprint)[FINGERPRINT_SIZE]) ++{ ++ efi_status_t status = EFI_SUCCESS; ++ uint8_t new_fingerprint[PSA_HASH_MAX_SIZE] = { 0 }; ++ ++ /* Verify the signature of the variable */ ++ if (verify_pkcs7_signature( ++ var_map->efi_auth_descriptor->AuthInfo.CertData, ++ var_map->efi_auth_descriptor_certdata_len, hash_buffer, ++ hash_len, NULL, 0) == 0) ++ status = EFI_SUCCESS; ++ else ++ return EFI_SECURITY_VIOLATION; ++ ++ /** ++ * UEFI: Page 254 ++ * CN of the signing certificate’s Subject and the hash of the tbsCertificate of the top-level issuer certificate ++ * (or the signing certificate itself if no other certificates are present or the certificate chain is of length 1) ++ * in SignedData.certificates is registered for use in subsequent verifications of this variable. Implementations ++ * may store just a single hash of these two elements to reduce storage requirements. ++ */ ++ if (get_uefi_priv_auth_var_fingerprint_handler(var_map->efi_auth_descriptor->AuthInfo.CertData, ++ var_map->efi_auth_descriptor_certdata_len, ++ (uint8_t*)&new_fingerprint)) { ++ EMSG("Failed to querry variable fingerprint input"); ++ return EFI_SECURITY_VIOLATION; ++ } ++ ++ /* ++ * The hash is SHA256 so only 32 bytes contain non zero values. ++ * Use only that part to decrease metadata size. ++ */ ++ if (!new_variable) { ++ if (memcmp(&new_fingerprint, fingerprint, FINGERPRINT_SIZE)) { ++ EMSG("Fingerprint verification failed"); ++ return EFI_SECURITY_VIOLATION; ++ } ++ } else { ++ /* Save fingerprint */ ++ memcpy(fingerprint, &new_fingerprint, FINGERPRINT_SIZE); ++ } ++ ++ return status; ++} + #endif + + static efi_status_t store_variable_data(const struct uefi_variable_store *context, +diff --git a/components/service/uefi/smm_variable/backend/variable_index.c b/components/service/uefi/smm_variable/backend/variable_index.c +index f4194d2d3..7f2fbe0ba 100644 +--- a/components/service/uefi/smm_variable/backend/variable_index.c ++++ b/components/service/uefi/smm_variable/backend/variable_index.c +@@ -199,6 +199,7 @@ static struct variable_entry *add_entry(const struct variable_index *context, co + info->metadata.uid = generate_uid(context, guid, name_size, name); + info->metadata.guid = *guid; + memset(&info->metadata.timestamp, 0, sizeof(EFI_TIME)); ++ memset(&info->metadata.fingerprint, 0, sizeof(FINGERPRINT_SIZE)); + info->metadata.attributes = 0; + info->metadata.name_size = name_size; + memcpy(info->metadata.name, name, name_size); +diff --git a/components/service/uefi/smm_variable/backend/variable_index.h b/components/service/uefi/smm_variable/backend/variable_index.h +index 7eef7b86b..726bc985a 100644 +--- a/components/service/uefi/smm_variable/backend/variable_index.h ++++ b/components/service/uefi/smm_variable/backend/variable_index.h +@@ -24,6 +24,7 @@ extern "C" { + * Implementation limits + */ + #define VARIABLE_INDEX_MAX_NAME_SIZE (64) ++#define FINGERPRINT_SIZE (32) + + /** + * \brief variable_metadata structure definition +@@ -33,6 +34,7 @@ extern "C" { + struct variable_metadata { + EFI_GUID guid; + EFI_TIME timestamp; ++ uint8_t fingerprint[FINGERPRINT_SIZE]; + size_t name_size; + int16_t name[VARIABLE_INDEX_MAX_NAME_SIZE]; + uint32_t attributes; +diff --git a/deployments/smm-gateway/config/default-opteesp/CMakeLists.txt b/deployments/smm-gateway/config/default-opteesp/CMakeLists.txt +index 0e281a377..d3df61ded 100644 +--- a/deployments/smm-gateway/config/default-opteesp/CMakeLists.txt ++++ b/deployments/smm-gateway/config/default-opteesp/CMakeLists.txt +@@ -42,7 +42,7 @@ set(SP_BOOT_ORDER "8" CACHE STRING "Boot order of the SP") + add_platform(TARGET "smm-gateway") + + # SMM variable and RPC caller settings +-set(SMM_GATEWAY_MAX_UEFI_VARIABLES 40 CACHE STRING "Maximum UEFI variable count") ++set(SMM_GATEWAY_MAX_UEFI_VARIABLES 35 CACHE STRING "Maximum UEFI variable count") + set(SMM_RPC_CALLER_SESSION_SHARED_MEMORY_SIZE 2*4096 CACHE STRING "RPC caller buffer size in SMMGW") + if (UEFI_AUTH_VAR) + set(SMM_SP_HEAP_SIZE 64*1024 CACHE STRING "SMM gateway SP heap size") +diff --git a/deployments/smm-gateway/config/default-sp/CMakeLists.txt b/deployments/smm-gateway/config/default-sp/CMakeLists.txt +index 8df9256e4..bb97cf8e3 100644 +--- a/deployments/smm-gateway/config/default-sp/CMakeLists.txt ++++ b/deployments/smm-gateway/config/default-sp/CMakeLists.txt +@@ -47,7 +47,7 @@ set(SP_BOOT_ORDER "8" CACHE STRING "Boot order of the SP") + add_platform(TARGET "smm-gateway") + + # SMM variable and RPC caller settings +-set(SMM_GATEWAY_MAX_UEFI_VARIABLES 40 CACHE STRING "Maximum UEFI variable count") ++set(SMM_GATEWAY_MAX_UEFI_VARIABLES 35 CACHE STRING "Maximum UEFI variable count") + set(SMM_RPC_CALLER_SESSION_SHARED_MEMORY_SIZE 2*4096 CACHE STRING "RPC caller buffer size in SMMGW") + if (UEFI_AUTH_VAR) + set(SMM_SP_HEAP_SIZE 64*1024 CACHE STRING "SMM gateway SP heap size") +-- +2.25.1 + diff --git a/meta-arm-bsp/recipes-security/trusted-services/ts-arm-platforms.inc b/meta-arm-bsp/recipes-security/trusted-services/ts-arm-platforms.inc index f8a94750..84c40aa8 100644 --- a/meta-arm-bsp/recipes-security/trusted-services/ts-arm-platforms.inc +++ b/meta-arm-bsp/recipes-security/trusted-services/ts-arm-platforms.inc @@ -15,6 +15,10 @@ SRC_URI:append:corstone1000 = " \ file://0011-Fix-Avoid-redefinition-of-variables.patch \ file://0012-Fix-GetNextVariableName-NameSize-input.patch \ file://0013-Fix-error-handling-of-variable-index-loading.patch \ + file://0014-Provide-crypto-api-to-create-uefi-priv-var-fingerpri.patch \ + file://0015-Add-timestamp-validation-for-uefi-variables.patch \ + file://0016-Isolate-common-uefi-variable-authentication-steps.patch \ + file://0017-Implement-Private-Authenticated-Variable-verificatio.patch \ " COMPATIBLE_MACHINE:fvp-base = "fvp-base" diff --git a/meta-arm-bsp/recipes-security/trusted-services/ts-sp-smm-gateway_%.bbappend b/meta-arm-bsp/recipes-security/trusted-services/ts-sp-smm-gateway_%.bbappend index 931d567f..628dfb48 100644 --- a/meta-arm-bsp/recipes-security/trusted-services/ts-sp-smm-gateway_%.bbappend +++ b/meta-arm-bsp/recipes-security/trusted-services/ts-sp-smm-gateway_%.bbappend @@ -4,6 +4,7 @@ EXTRA_OECMAKE:append:corstone1000 = " -DMM_COMM_BUFFER_ADDRESS="0x00000000 0x81F -DMM_COMM_BUFFER_PAGE_COUNT="1" \ -DUEFI_AUTH_VAR=ON \ -DUEFI_INTERNAL_CRYPTO=ON \ + -DSMM_GATEWAY_MAX_UEFI_VARIABLES=60 \ " EXTRA_OECMAKE:append:fvp-base = " -DMM_COMM_BUFFER_ADDRESS="0x00000000 0x81000000" \ From patchwork Wed May 22 16:04:41 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: bence.balogh@arm.com X-Patchwork-Id: 44048 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from aws-us-west-2-korg-lkml-1.web.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6AF40C25B7F for ; Wed, 22 May 2024 16:05:05 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web10.1292.1716393900492295664 for ; Wed, 22 May 2024 09:05:00 -0700 Authentication-Results: mx.groups.io; dkim=none (message not signed); spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: bence.balogh@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3B03B1480; Wed, 22 May 2024 09:05:24 -0700 (PDT) Received: from e126523.arm.com (unknown [10.57.84.120]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 917723F766; Wed, 22 May 2024 09:04:59 -0700 (PDT) From: bence.balogh@arm.com To: meta-arm@lists.yoctoproject.org Cc: Bence Balogh Subject: [PATCH scarthgap 3/5] arm-bsp/trusted-firmware-m: corstone1000: increase PS sizes Date: Wed, 22 May 2024 18:04:41 +0200 Message-Id: <20240522160443.89173-4-bence.balogh@arm.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20240522160443.89173-1-bence.balogh@arm.com> References: <20240522160443.89173-1-bence.balogh@arm.com> MIME-Version: 1.0 List-Id: X-Webhook-Received: from li982-79.members.linode.com [45.33.32.79] by aws-us-west-2-korg-lkml-1.web.codeaurora.org with HTTPS for ; Wed, 22 May 2024 16:05:05 -0000 X-Groupsio-URL: https://lists.yoctoproject.org/g/meta-arm/message/5758 From: Bence Balogh The private authenticated variable changes increased the variables metadata. The PS max asset size and related buffer sizes have to be increased because of this. Signed-off-by: Bence Balogh --- ...ne1000-Increase-buffers-for-EFI-vars.patch | 45 +++++++++++++++++++ .../trusted-firmware-m-corstone1000.inc | 1 + 2 files changed, 46 insertions(+) create mode 100644 meta-arm-bsp/recipes-bsp/trusted-firmware-m/files/corstone1000/0011-Platform-corstone1000-Increase-buffers-for-EFI-vars.patch diff --git a/meta-arm-bsp/recipes-bsp/trusted-firmware-m/files/corstone1000/0011-Platform-corstone1000-Increase-buffers-for-EFI-vars.patch b/meta-arm-bsp/recipes-bsp/trusted-firmware-m/files/corstone1000/0011-Platform-corstone1000-Increase-buffers-for-EFI-vars.patch new file mode 100644 index 00000000..abf70389 --- /dev/null +++ b/meta-arm-bsp/recipes-bsp/trusted-firmware-m/files/corstone1000/0011-Platform-corstone1000-Increase-buffers-for-EFI-vars.patch @@ -0,0 +1,45 @@ +From d7725e629c9ba93523589cc9d8af3186db19d4e8 Mon Sep 17 00:00:00 2001 +From: Bence Balogh +Date: Wed, 15 May 2024 22:37:51 +0200 +Subject: [PATCH] Platform: corstone1000: Increase buffers for EFI vars + +The UEFI variables are stored in the Protected Storage. The size of +the variables metadata have been increased so the related buffer sizes +have to be increased. + +Signed-off-by: Bence Balogh +Upstream-Status: Pending +--- + .../ext/target/arm/corstone1000/config_tfm_target.h | 13 ++++++++++++- + 1 file changed, 12 insertions(+), 1 deletion(-) + +diff --git a/platform/ext/target/arm/corstone1000/config_tfm_target.h b/platform/ext/target/arm/corstone1000/config_tfm_target.h +index 2eb0924770..6ee823a7dc 100644 +--- a/platform/ext/target/arm/corstone1000/config_tfm_target.h ++++ b/platform/ext/target/arm/corstone1000/config_tfm_target.h +@@ -1,5 +1,5 @@ + /* +- * Copyright (c) 2022, Arm Limited. All rights reserved. ++ * Copyright (c) 2022-2024, Arm Limited. All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + * +@@ -24,4 +24,15 @@ + #undef ITS_MAX_ASSET_SIZE + #define ITS_MAX_ASSET_SIZE 2048 + ++/* The maximum asset size to be stored in the Protected Storage */ ++#undef PS_MAX_ASSET_SIZE ++#define PS_MAX_ASSET_SIZE 2592 ++ ++/* This is needed to be able to process the EFI variables during PS writes. */ ++#undef CRYPTO_ENGINE_BUF_SIZE ++#define CRYPTO_ENGINE_BUF_SIZE 0x5000 ++ ++/* This is also has to be increased to fit the EFI variables into the iovecs. */ ++#undef CRYPTO_IOVEC_BUFFER_SIZE ++#define CRYPTO_IOVEC_BUFFER_SIZE 6000 + #endif /* __CONFIG_TFM_TARGET_H__ */ +-- +2.25.1 + diff --git a/meta-arm-bsp/recipes-bsp/trusted-firmware-m/trusted-firmware-m-corstone1000.inc b/meta-arm-bsp/recipes-bsp/trusted-firmware-m/trusted-firmware-m-corstone1000.inc index 1e835b6f..55cfee5c 100644 --- a/meta-arm-bsp/recipes-bsp/trusted-firmware-m/trusted-firmware-m-corstone1000.inc +++ b/meta-arm-bsp/recipes-bsp/trusted-firmware-m/trusted-firmware-m-corstone1000.inc @@ -27,6 +27,7 @@ SRC_URI:append:corstone1000 = " \ file://0008-Platform-CS1000-Replace-OpenAMP-with-RSE_COMMS.patch \ file://0009-platform-CS1000-Increase-RSE_COMMS-buffer-size.patch \ file://0010-CC312-alignment-of-cc312-differences-between-fvp-and.patch \ + file://0011-Platform-corstone1000-Increase-buffers-for-EFI-vars.patch \ " # TF-M ships patches for external dependencies that needs to be applied From patchwork Wed May 22 16:04:42 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: bence.balogh@arm.com X-Patchwork-Id: 44046 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from aws-us-west-2-korg-lkml-1.web.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.lore.kernel.org (Postfix) with ESMTP id 64A09C25B7E for ; Wed, 22 May 2024 16:05:05 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web11.1231.1716393901432226558 for ; Wed, 22 May 2024 09:05:01 -0700 Authentication-Results: mx.groups.io; dkim=none (message not signed); spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: bence.balogh@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 25394339; Wed, 22 May 2024 09:05:25 -0700 (PDT) Received: from e126523.arm.com (unknown [10.57.84.120]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7AB053F766; Wed, 22 May 2024 09:05:00 -0700 (PDT) From: bence.balogh@arm.com To: meta-arm@lists.yoctoproject.org Cc: Bence Balogh Subject: [PATCH scarthgap 4/5] arm-bsp/trusted-services: corstone1000: increase comm buffer size Date: Wed, 22 May 2024 18:04:42 +0200 Message-Id: <20240522160443.89173-5-bence.balogh@arm.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20240522160443.89173-1-bence.balogh@arm.com> References: <20240522160443.89173-1-bence.balogh@arm.com> MIME-Version: 1.0 List-Id: X-Webhook-Received: from li982-79.members.linode.com [45.33.32.79] by aws-us-west-2-korg-lkml-1.web.codeaurora.org with HTTPS for ; Wed, 22 May 2024 16:05:05 -0000 X-Groupsio-URL: https://lists.yoctoproject.org/g/meta-arm/message/5759 From: Bence Balogh The increased EFI variable metadata need bigger buffer so it can be transfered to the Secure Enclave without memory overflow issues. The heap and buffer sizes had to be aligned with the. Signed-off-by: Bence Balogh --- ..._COMMS-cmake-variables-to-cahce-vars.patch | 37 +++++++++++++++++++ .../trusted-services/ts-arm-platforms.inc | 1 + .../ts-sp-se-proxy_%.bbappend | 1 + 3 files changed, 39 insertions(+) create mode 100644 meta-arm-bsp/recipes-security/trusted-services/corstone1000/0018-Change-RSS_COMMS-cmake-variables-to-cahce-vars.patch diff --git a/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0018-Change-RSS_COMMS-cmake-variables-to-cahce-vars.patch b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0018-Change-RSS_COMMS-cmake-variables-to-cahce-vars.patch new file mode 100644 index 00000000..76e78fa3 --- /dev/null +++ b/meta-arm-bsp/recipes-security/trusted-services/corstone1000/0018-Change-RSS_COMMS-cmake-variables-to-cahce-vars.patch @@ -0,0 +1,37 @@ +From e8b577d02d1d4ed2492bb0b6c3a5bb7d2656f13a Mon Sep 17 00:00:00 2001 +From: Bence Balogh +Date: Fri, 17 May 2024 13:21:07 +0200 +Subject: [PATCH] Change RSS_COMMS cmake variables to cahce vars + +This way they can be set externally as well for the corstone1000 +platform. + +Signed-off-by: Bence Balogh +Upstream-Status: Pending +--- + platform/providers/arm/corstone1000/platform.cmake | 6 ++++-- + 1 file changed, 4 insertions(+), 2 deletions(-) + +diff --git a/platform/providers/arm/corstone1000/platform.cmake b/platform/providers/arm/corstone1000/platform.cmake +index 16139c80e..82ac14f0b 100644 +--- a/platform/providers/arm/corstone1000/platform.cmake ++++ b/platform/providers/arm/corstone1000/platform.cmake +@@ -9,11 +9,13 @@ + set(SMM_GATEWAY_MAX_UEFI_VARIABLES 80 CACHE STRING "Maximum UEFI variable count") + set(SMM_RPC_CALLER_SESSION_SHARED_MEMORY_SIZE 4*4096 CACHE STRING "RPC caller buffer size in SMMGW") + set(SMM_SP_HEAP_SIZE 80*1024 CACHE STRING "SMM gateway SP heap size") ++set(PLAT_RSS_COMMS_PAYLOAD_MAX_SIZE 0x43C0 CACHE STRING "Size of the RSS_COMMS_PAYLOAD buffer") ++set(COMMS_MHU_MSG_SIZE 0x4500 CACHE STRING "Max message size that can be transfered via MHU") + + target_compile_definitions(${TGT} PRIVATE + SMM_VARIABLE_INDEX_STORAGE_UID=0x787 +- PLAT_RSS_COMMS_PAYLOAD_MAX_SIZE=0x2080 +- COMMS_MHU_MSG_SIZE=0x3500 ++ PLAT_RSS_COMMS_PAYLOAD_MAX_SIZE=${PLAT_RSS_COMMS_PAYLOAD_MAX_SIZE} ++ COMMS_MHU_MSG_SIZE=${COMMS_MHU_MSG_SIZE} + ) + + get_property(_platform_driver_dependencies TARGET ${TGT} +-- +2.25.1 + diff --git a/meta-arm-bsp/recipes-security/trusted-services/ts-arm-platforms.inc b/meta-arm-bsp/recipes-security/trusted-services/ts-arm-platforms.inc index 84c40aa8..837f6871 100644 --- a/meta-arm-bsp/recipes-security/trusted-services/ts-arm-platforms.inc +++ b/meta-arm-bsp/recipes-security/trusted-services/ts-arm-platforms.inc @@ -19,6 +19,7 @@ SRC_URI:append:corstone1000 = " \ file://0015-Add-timestamp-validation-for-uefi-variables.patch \ file://0016-Isolate-common-uefi-variable-authentication-steps.patch \ file://0017-Implement-Private-Authenticated-Variable-verificatio.patch \ + file://0018-Change-RSS_COMMS-cmake-variables-to-cahce-vars.patch \ " COMPATIBLE_MACHINE:fvp-base = "fvp-base" diff --git a/meta-arm-bsp/recipes-security/trusted-services/ts-sp-se-proxy_%.bbappend b/meta-arm-bsp/recipes-security/trusted-services/ts-sp-se-proxy_%.bbappend index 31e4ea55..64ab5bea 100644 --- a/meta-arm-bsp/recipes-security/trusted-services/ts-sp-se-proxy_%.bbappend +++ b/meta-arm-bsp/recipes-security/trusted-services/ts-sp-se-proxy_%.bbappend @@ -2,6 +2,7 @@ require ts-arm-platforms.inc EXTRA_OECMAKE:append:corstone1000 = " -DMM_COMM_BUFFER_ADDRESS="0x00000000 0x81FFF000" \ -DMM_COMM_BUFFER_PAGE_COUNT="1" \ + -DSP_HEAP_SIZE=70*1024 \ " # Proxy is pointless on fvp-base as there is no dedicated security subsystem. It could be From patchwork Wed May 22 16:04:43 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: bence.balogh@arm.com X-Patchwork-Id: 44047 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from aws-us-west-2-korg-lkml-1.web.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.lore.kernel.org (Postfix) with ESMTP id 56729C25B79 for ; Wed, 22 May 2024 16:05:05 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web11.1232.1716393902333546935 for ; Wed, 22 May 2024 09:05:02 -0700 Authentication-Results: mx.groups.io; dkim=none (message not signed); spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: bence.balogh@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 117A6DA7; Wed, 22 May 2024 09:05:26 -0700 (PDT) Received: from e126523.arm.com (unknown [10.57.84.120]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6344B3F766; Wed, 22 May 2024 09:05:01 -0700 (PDT) From: bence.balogh@arm.com To: meta-arm@lists.yoctoproject.org Cc: Bence Balogh Subject: [PATCH scarthgap 5/5] arm-bsp/trusted-firmware-m: corstone1000: increase RSE_COMMS buff size Date: Wed, 22 May 2024 18:04:43 +0200 Message-Id: <20240522160443.89173-6-bence.balogh@arm.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20240522160443.89173-1-bence.balogh@arm.com> References: <20240522160443.89173-1-bence.balogh@arm.com> MIME-Version: 1.0 List-Id: X-Webhook-Received: from li982-79.members.linode.com [45.33.32.79] by aws-us-west-2-korg-lkml-1.web.codeaurora.org with HTTPS for ; Wed, 22 May 2024 16:05:05 -0000 X-Groupsio-URL: https://lists.yoctoproject.org/g/meta-arm/message/5760 From: Bence Balogh The buffer size has to be increased to fit the EFI variables which got increased metadata sizes. Signed-off-by: Bence Balogh --- ...latform-corstone1000-Increase-RSE_COMMS-buffer-size.patch} | 4 ++-- .../trusted-firmware-m/trusted-firmware-m-corstone1000.inc | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) rename meta-arm-bsp/recipes-bsp/trusted-firmware-m/files/corstone1000/{0009-platform-CS1000-Increase-RSE_COMMS-buffer-size.patch => 0009-platform-corstone1000-Increase-RSE_COMMS-buffer-size.patch} (87%) diff --git a/meta-arm-bsp/recipes-bsp/trusted-firmware-m/files/corstone1000/0009-platform-CS1000-Increase-RSE_COMMS-buffer-size.patch b/meta-arm-bsp/recipes-bsp/trusted-firmware-m/files/corstone1000/0009-platform-corstone1000-Increase-RSE_COMMS-buffer-size.patch similarity index 87% rename from meta-arm-bsp/recipes-bsp/trusted-firmware-m/files/corstone1000/0009-platform-CS1000-Increase-RSE_COMMS-buffer-size.patch rename to meta-arm-bsp/recipes-bsp/trusted-firmware-m/files/corstone1000/0009-platform-corstone1000-Increase-RSE_COMMS-buffer-size.patch index 6613a415..3269c0e0 100644 --- a/meta-arm-bsp/recipes-bsp/trusted-firmware-m/files/corstone1000/0009-platform-CS1000-Increase-RSE_COMMS-buffer-size.patch +++ b/meta-arm-bsp/recipes-bsp/trusted-firmware-m/files/corstone1000/0009-platform-corstone1000-Increase-RSE_COMMS-buffer-size.patch @@ -1,7 +1,7 @@ From 21b0c9f028b6b04fa2f027510ec90969735f4dd1 Mon Sep 17 00:00:00 2001 From: Bence Balogh Date: Wed, 17 Apr 2024 19:31:03 +0200 -Subject: [PATCH] platform: CS1000: Increase RSE_COMMS buffer size +Subject: [PATCH] platform: corstone1000: Increase RSE_COMMS buffer size Signed-off-by: Bence Balogh Upstream-Status: Pending @@ -18,7 +18,7 @@ index 6d79dd3bf..f079f6504 100644 /* size suits to fit the largest message too (EFI variables) */ -#define RSE_COMMS_PAYLOAD_MAX_SIZE (0x2100) -+#define RSE_COMMS_PAYLOAD_MAX_SIZE (0x3B00) ++#define RSE_COMMS_PAYLOAD_MAX_SIZE (0x43C0) /* * Allocated for each client request. diff --git a/meta-arm-bsp/recipes-bsp/trusted-firmware-m/trusted-firmware-m-corstone1000.inc b/meta-arm-bsp/recipes-bsp/trusted-firmware-m/trusted-firmware-m-corstone1000.inc index 55cfee5c..eafb1fc7 100644 --- a/meta-arm-bsp/recipes-bsp/trusted-firmware-m/trusted-firmware-m-corstone1000.inc +++ b/meta-arm-bsp/recipes-bsp/trusted-firmware-m/trusted-firmware-m-corstone1000.inc @@ -25,7 +25,7 @@ SRC_URI:append:corstone1000 = " \ file://0006-Platform-Corstone1000-Enable-host-firewall-in-FVP.patch \ file://0007-platform-corstone1000-Increase-ITS-max-asset-size.patch \ file://0008-Platform-CS1000-Replace-OpenAMP-with-RSE_COMMS.patch \ - file://0009-platform-CS1000-Increase-RSE_COMMS-buffer-size.patch \ + file://0009-platform-corstone1000-Increase-RSE_COMMS-buffer-size.patch \ file://0010-CC312-alignment-of-cc312-differences-between-fvp-and.patch \ file://0011-Platform-corstone1000-Increase-buffers-for-EFI-vars.patch \ "