From: emekcan.aras@arm.com
To: meta-arm@lists.yoctoproject.org, Ross.Burton@arm.com, Jon.Mason@arm.com
Cc: nd@arm.com, Emekcan Aras <emekcan.aras@arm.com>
Subject: [PATCH nanbield 1/1] arm-bsp/optee: Improve PIN counter handling robustness
Date: Mon, 19 Feb 2024 18:26:37 +0000 [thread overview]
Message-ID: <20240219182637.530206-2-emekcan.aras@arm.com> (raw)
In-Reply-To: <20240219182637.530206-1-emekcan.aras@arm.com>
From: Emekcan Aras <emekcan.aras@arm.com>
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>
---
...rove-PIN-counter-handling-robustness.patch | 205 ++++++++++++++++++
.../recipes-security/optee/optee-os_3.22.0.bb | 1 +
2 files changed, 206 insertions(+)
create mode 100644 meta-arm-bsp/recipes-security/optee/optee-os-3.22.0/0005-ta-pkcs11-Improve-PIN-counter-handling-robustness.patch
diff --git a/meta-arm-bsp/recipes-security/optee/optee-os-3.22.0/0005-ta-pkcs11-Improve-PIN-counter-handling-robustness.patch b/meta-arm-bsp/recipes-security/optee/optee-os-3.22.0/0005-ta-pkcs11-Improve-PIN-counter-handling-robustness.patch
new file mode 100644
index 00000000..d95954fa
--- /dev/null
+++ b/meta-arm-bsp/recipes-security/optee/optee-os-3.22.0/0005-ta-pkcs11-Improve-PIN-counter-handling-robustness.patch
@@ -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
+
+
diff --git a/meta-arm-bsp/recipes-security/optee/optee-os_3.22.0.bb b/meta-arm-bsp/recipes-security/optee/optee-os_3.22.0.bb
index e1220192..16a193c3 100644
--- a/meta-arm-bsp/recipes-security/optee/optee-os_3.22.0.bb
+++ b/meta-arm-bsp/recipes-security/optee/optee-os_3.22.0.bb
@@ -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 \
"
--
2.25.1
prev parent reply other threads:[~2024-02-19 18:27 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-19 18:26 [PATCH nanbield 0/1] arm-bsp/optee: Improve PIN counter handling robustness emekcan.aras
2024-02-19 18:26 ` emekcan.aras [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240219182637.530206-2-emekcan.aras@arm.com \
--to=emekcan.aras@arm.com \
--cc=Jon.Mason@arm.com \
--cc=Ross.Burton@arm.com \
--cc=meta-arm@lists.yoctoproject.org \
--cc=nd@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).