| 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 |
| |
| |