mirrors/meta-arm
1
0
Fork 0
mirror of https://git.yoctoproject.org/meta-arm synced 2026-01-11 15:00:39 +00:00
Code Issues Projects Releases Wiki Activity

arm-bsp/optee: Improve PIN counter handling robustness

Browse Source 
This patches a security issue discovered lately in OP-TEE version
earlier than v4.1. The detailed report can be found here:

https://github.com/OP-TEE/optee_os/security/advisories/GHSA-2f5m-q4w3-865p

Signed-off-by: Emekcan Aras <emekcan.aras@arm.com>
This commit is contained in:
Emekcan Aras
2024-02-19 18:26:37 +00:00
committed by Ross Burton
parent 12e80bade1
commit 9a4ae38e84
2 changed files with 206 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

205
meta-arm-bsp/recipes-security/optee/optee-os-3.22.0/0005-ta-pkcs11-Improve-PIN-counter-handling-robustness.patch Normal file
View File

@@ -0,0 +1,205 @@
From d75c42ff2847b090d5b1f11c49067cd41fcc2734 Mon Sep 17 00:00:00 2001
From: Loic Poulain <loic.poulain@linaro.org>
Date: Tue, 31 Oct 2023 11:07:00 +0100
Subject: [PATCH] ta: pkcs11: Improve PIN counter handling robustness
Make sure PIN check attempt is saved persistently before continuing with
the actual PIN verification, improving counter and flags coherency in
case of subsequent failure with persistent saving.
Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Upstream-Status: Backport [https://github.com/OP-TEE/optee_os/pull/6445/commits/0a74733d9437d94a5b4b2db6c40c5755cabc5393]
---
ta/pkcs11/src/pkcs11_token.c | 126 +++++++++++++++++------------------
1 file changed, 62 insertions(+), 64 deletions(-)
diff --git a/ta/pkcs11/src/pkcs11_token.c b/ta/pkcs11/src/pkcs11_token.c
index ab0fc291e..c5271e449 100644
--- a/ta/pkcs11/src/pkcs11_token.c
+++ b/ta/pkcs11/src/pkcs11_token.c
@@ -1132,117 +1132,115 @@ static enum pkcs11_rc check_so_pin(struct pkcs11_session *session,
uint8_t *pin, size_t pin_size)
{
struct ck_token *token = session->token;
+ struct token_persistent_main *db = token->db_main;
enum pkcs11_rc rc = PKCS11_CKR_OK;
- assert(token->db_main->flags & PKCS11_CKFT_TOKEN_INITIALIZED);
+ assert(db->flags & PKCS11_CKFT_TOKEN_INITIALIZED);
if (IS_ENABLED(CFG_PKCS11_TA_AUTH_TEE_IDENTITY) &&
- token->db_main->flags & PKCS11_CKFT_PROTECTED_AUTHENTICATION_PATH)
+ db->flags & PKCS11_CKFT_PROTECTED_AUTHENTICATION_PATH)
return verify_identity_auth(token, PKCS11_CKU_SO);
- if (token->db_main->flags & PKCS11_CKFT_SO_PIN_LOCKED)
+ if (db->flags & PKCS11_CKFT_SO_PIN_LOCKED)
return PKCS11_CKR_PIN_LOCKED;
- rc = verify_pin(PKCS11_CKU_SO, pin, pin_size,
- token->db_main->so_pin_salt,
- token->db_main->so_pin_hash);
- if (rc) {
- unsigned int pin_count = 0;
+ /*
+ * Preset the counter and flags conservatively in the database so that
+ * the tentative is saved whatever happens next.
+ */
+ db->flags |= PKCS11_CKFT_SO_PIN_COUNT_LOW;
+ db->so_pin_count++;
- if (rc != PKCS11_CKR_PIN_INCORRECT)
- return rc;
+ if (db->so_pin_count == PKCS11_TOKEN_SO_PIN_COUNT_MAX - 1)
+ db->flags |= PKCS11_CKFT_SO_PIN_FINAL_TRY;
+ else if (db->so_pin_count == PKCS11_TOKEN_SO_PIN_COUNT_MAX)
+ db->flags |= PKCS11_CKFT_SO_PIN_LOCKED;
- token->db_main->flags |= PKCS11_CKFT_SO_PIN_COUNT_LOW;
- token->db_main->so_pin_count++;
-
- pin_count = token->db_main->so_pin_count;
- if (pin_count == PKCS11_TOKEN_SO_PIN_COUNT_MAX - 1)
- token->db_main->flags |= PKCS11_CKFT_SO_PIN_FINAL_TRY;
- if (pin_count == PKCS11_TOKEN_SO_PIN_COUNT_MAX)
- token->db_main->flags |= PKCS11_CKFT_SO_PIN_LOCKED;
-
- update_persistent_db(token);
+ update_persistent_db(token);
- if (token->db_main->flags & PKCS11_CKFT_SO_PIN_LOCKED)
+ rc = verify_pin(PKCS11_CKU_SO, pin, pin_size,
+ db->so_pin_salt,
+ db->so_pin_hash);
+ if (rc == PKCS11_CKR_PIN_INCORRECT) {
+ if (db->flags & PKCS11_CKFT_SO_PIN_LOCKED)
return PKCS11_CKR_PIN_LOCKED;
return PKCS11_CKR_PIN_INCORRECT;
}
- if (token->db_main->so_pin_count) {
- token->db_main->so_pin_count = 0;
+ if (rc)
+ db->so_pin_count--;
+ else
+ db->so_pin_count = 0;
- update_persistent_db(token);
+ db->flags &= ~PKCS11_CKFT_SO_PIN_LOCKED;
+ if (db->so_pin_count < PKCS11_TOKEN_SO_PIN_COUNT_MAX - 1) {
+ db->flags &= ~PKCS11_CKFT_SO_PIN_FINAL_TRY;
+ if (!db->so_pin_count)
+ db->flags &= ~PKCS11_CKFT_SO_PIN_COUNT_LOW;
}
- if (token->db_main->flags & (PKCS11_CKFT_SO_PIN_COUNT_LOW |
- PKCS11_CKFT_SO_PIN_FINAL_TRY)) {
- token->db_main->flags &= ~(PKCS11_CKFT_SO_PIN_COUNT_LOW |
- PKCS11_CKFT_SO_PIN_FINAL_TRY);
-
- update_persistent_db(token);
- }
+ update_persistent_db(token);
- return PKCS11_CKR_OK;
+ return rc;
}
static enum pkcs11_rc check_user_pin(struct pkcs11_session *session,
uint8_t *pin, size_t pin_size)
{
struct ck_token *token = session->token;
+ struct token_persistent_main *db = token->db_main;
enum pkcs11_rc rc = PKCS11_CKR_OK;
if (IS_ENABLED(CFG_PKCS11_TA_AUTH_TEE_IDENTITY) &&
- token->db_main->flags & PKCS11_CKFT_PROTECTED_AUTHENTICATION_PATH)
+ db->flags & PKCS11_CKFT_PROTECTED_AUTHENTICATION_PATH)
return verify_identity_auth(token, PKCS11_CKU_USER);
- if (!token->db_main->user_pin_salt)
+ if (!db->user_pin_salt)
return PKCS11_CKR_USER_PIN_NOT_INITIALIZED;
- if (token->db_main->flags & PKCS11_CKFT_USER_PIN_LOCKED)
+ if (db->flags & PKCS11_CKFT_USER_PIN_LOCKED)
return PKCS11_CKR_PIN_LOCKED;
- rc = verify_pin(PKCS11_CKU_USER, pin, pin_size,
- token->db_main->user_pin_salt,
- token->db_main->user_pin_hash);
- if (rc) {
- unsigned int pin_count = 0;
-
- if (rc != PKCS11_CKR_PIN_INCORRECT)
- return rc;
-
- token->db_main->flags |= PKCS11_CKFT_USER_PIN_COUNT_LOW;
- token->db_main->user_pin_count++;
+ /*
+ * Preset the counter and flags conservatively in the database so that
+ * the tentative is saved whatever happens next.
+ */
+ db->flags |= PKCS11_CKFT_USER_PIN_COUNT_LOW;
+ db->user_pin_count++;
- pin_count = token->db_main->user_pin_count;
- if (pin_count == PKCS11_TOKEN_USER_PIN_COUNT_MAX - 1)
- token->db_main->flags |= PKCS11_CKFT_USER_PIN_FINAL_TRY;
- if (pin_count == PKCS11_TOKEN_USER_PIN_COUNT_MAX)
- token->db_main->flags |= PKCS11_CKFT_USER_PIN_LOCKED;
+ if (db->user_pin_count == PKCS11_TOKEN_USER_PIN_COUNT_MAX - 1)
+ db->flags |= PKCS11_CKFT_USER_PIN_FINAL_TRY;
+ else if (db->user_pin_count == PKCS11_TOKEN_USER_PIN_COUNT_MAX)
+ db->flags |= PKCS11_CKFT_USER_PIN_LOCKED;
- update_persistent_db(token);
+ update_persistent_db(token);
- if (token->db_main->flags & PKCS11_CKFT_USER_PIN_LOCKED)
+ rc = verify_pin(PKCS11_CKU_USER, pin, pin_size,
+ db->user_pin_salt,
+ db->user_pin_hash);
+ if (rc == PKCS11_CKR_PIN_INCORRECT) {
+ if (db->flags & PKCS11_CKFT_USER_PIN_LOCKED)
return PKCS11_CKR_PIN_LOCKED;
return PKCS11_CKR_PIN_INCORRECT;
}
- if (token->db_main->user_pin_count) {
- token->db_main->user_pin_count = 0;
+ if (rc)
+ db->user_pin_count--;
+ else
+ db->user_pin_count = 0;
- update_persistent_db(token);
+ db->flags &= ~PKCS11_CKFT_USER_PIN_LOCKED;
+ if (db->user_pin_count < PKCS11_TOKEN_USER_PIN_COUNT_MAX - 1) {
+ db->flags &= ~PKCS11_CKFT_USER_PIN_FINAL_TRY;
+ if (!db->user_pin_count)
+ db->flags &= ~PKCS11_CKFT_USER_PIN_COUNT_LOW;
}
- if (token->db_main->flags & (PKCS11_CKFT_USER_PIN_COUNT_LOW |
- PKCS11_CKFT_USER_PIN_FINAL_TRY)) {
- token->db_main->flags &= ~(PKCS11_CKFT_USER_PIN_COUNT_LOW |
- PKCS11_CKFT_USER_PIN_FINAL_TRY);
-
- update_persistent_db(token);
- }
+ update_persistent_db(token);
- return PKCS11_CKR_OK;
+ return rc;
}
enum pkcs11_rc entry_ck_set_pin(struct pkcs11_client *client,
--
2.25.1

1
meta-arm-bsp/recipes-security/optee/optee-os_3.22.0.bb
View File

@@ -10,4 +10,5 @@ SRC_URI += " \
file://0002-core-Define-section-attributes-for-clang.patch \
file://0003-optee-enable-clang-support.patch \
file://0004-core-link-add-no-warn-rwx-segments.patch \
file://0005-ta-pkcs11-Improve-PIN-counter-handling-robustness.patch \
"