From c815d74544bd1c961990994a5a24cd70ef32a148 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Fri, 20 Feb 2026 16:29:18 +0100 Subject: [PATCH 1/5] p11_child: ignore failure of C_GetTokenInfo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed-by: Alexey Tikhonov Reviewed-by: Pavel Březina --- src/p11_child/p11_child_openssl.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c index 4780f959bf2..697869121cf 100644 --- a/src/p11_child/p11_child_openssl.c +++ b/src/p11_child/p11_child_openssl.c @@ -2150,10 +2150,9 @@ errno_t do_card(TALLOC_CTX *mem_ctx, struct p11_ctx *p11_ctx, rv = modules[c]->C_GetTokenInfo(slots[s], &token_info); if (rv != CKR_OK) { DEBUG(SSSDBG_OP_FAILURE, - "C_GetTokenInfo failed [%lu][%s].\n", + "C_GetTokenInfo failed [%lu][%s], skipping.\n", rv, p11_kit_strerror(rv)); - ret = EIO; - goto done; + continue; } if (!(token_info.flags & CKF_TOKEN_INITIALIZED)) { From 49816d3b219284c138e2d65d7217b7bc00197298 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Mon, 23 Feb 2026 19:35:41 +0100 Subject: [PATCH 2/5] pam: handle protected authentication path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a Smartcard reader has a built-in keypad or keyboard the flag CKF_PROTECTED_AUTHENTICATION_PATH is set in the token info data. To properly tell the user that the pin must be given at the reader directly and not at the computer this information must be propagated to the pam_sss module. Resolves: https://github.com/SSSD/sssd/issues/5371 Reviewed-by: Alexey Tikhonov Reviewed-by: Pavel Březina --- src/p11_child/p11_child_openssl.c | 18 +++++--- src/responder/pam/pamsrv.h | 1 + src/responder/pam/pamsrv_cmd.c | 5 +- src/responder/pam/pamsrv_p11.c | 53 ++++++++++++++++++++- src/sss_client/pam_sss.c | 77 ++++++++++++++++++++++--------- src/tests/cmocka/test_pam_srv.c | 13 +++++- 6 files changed, 136 insertions(+), 31 deletions(-) diff --git a/src/p11_child/p11_child_openssl.c b/src/p11_child/p11_child_openssl.c index 697869121cf..7fe9274d41b 100644 --- a/src/p11_child/p11_child_openssl.c +++ b/src/p11_child/p11_child_openssl.c @@ -1761,7 +1761,8 @@ errno_t do_slot(CK_FUNCTION_LIST *module, size_t module_id, CK_SLOT_ID slot_id, struct p11_ctx *p11_ctx, enum op_mode mode, const char *pin, const char *module_name_in, const char *token_name_in, const char *key_id_in, const char *label_in, - const char *uri_str, char **_multi) { + const char *uri_str, char **_multi) +{ int ret; CK_RV rv; char *module_file_name = NULL; @@ -1773,6 +1774,7 @@ errno_t do_slot(CK_FUNCTION_LIST *module, size_t module_id, CK_SLOT_ID slot_id, struct cert_list *next_item = NULL; bool pkcs11_session = false; bool pkcs11_login = false; + bool has_protected_authentication_path = false; slot_name = p11_kit_space_strdup(info->slotDescription, sizeof(info->slotDescription)); @@ -1790,6 +1792,8 @@ errno_t do_slot(CK_FUNCTION_LIST *module, size_t module_id, CK_SLOT_ID slot_id, goto done; } + has_protected_authentication_path = (token_info->flags & CKF_PROTECTED_AUTHENTICATION_PATH); + module_file_name = p11_kit_module_get_filename(module); if (module_file_name == NULL) { DEBUG(SSSDBG_OP_FAILURE, "p11_kit_module_get_filename failed.\n"); @@ -1824,10 +1828,9 @@ errno_t do_slot(CK_FUNCTION_LIST *module, size_t module_id, CK_SLOT_ID slot_id, if (mode == OP_AUTH) { DEBUG(SSSDBG_TRACE_ALL, "Login required.\n"); DEBUG(SSSDBG_TRACE_ALL, "Token flags [%lu].\n", token_info->flags); - if ((pin != NULL) - || (token_info->flags & CKF_PROTECTED_AUTHENTICATION_PATH)) { + if ((pin != NULL) || has_protected_authentication_path) { - if (token_info->flags & CKF_PROTECTED_AUTHENTICATION_PATH) { + if (has_protected_authentication_path) { DEBUG(SSSDBG_TRACE_ALL, "Protected authentication path.\n"); pin = NULL; } @@ -1936,9 +1939,12 @@ errno_t do_slot(CK_FUNCTION_LIST *module, size_t module_id, CK_SLOT_ID slot_id, DEBUG(SSSDBG_TRACE_ALL, "Found certificate has key id [%s].\n", item->id); - *_multi = talloc_asprintf_append(*_multi, "%s\n%s\n%s\n%s\n%s\n", + *_multi = talloc_asprintf_append(*_multi, "%s\n%s\n%s\n%s\n%s\n%s\n", token_name, module_file_name, item->id, - item->label, item->cert_b64); + item->label, + has_protected_authentication_path ? + "true" : "false", + item->cert_b64); if (*_multi == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, "Failed to append certificate to the output string.\n"); diff --git a/src/responder/pam/pamsrv.h b/src/responder/pam/pamsrv.h index 1d2e7bc0d14..694b391bb16 100644 --- a/src/responder/pam/pamsrv.h +++ b/src/responder/pam/pamsrv.h @@ -140,6 +140,7 @@ const char *sss_cai_get_token_name(struct cert_auth_info *i); const char *sss_cai_get_module_name(struct cert_auth_info *i); const char *sss_cai_get_key_id(struct cert_auth_info *i); const char *sss_cai_get_label(struct cert_auth_info *i); +bool sss_cai_get_has_protected_authentication_path(struct cert_auth_info *i); struct cert_auth_info *sss_cai_get_next(struct cert_auth_info *i); struct ldb_result *sss_cai_get_cert_user_objs(struct cert_auth_info *i); void sss_cai_set_cert_user_objs(struct cert_auth_info *i, diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c index 80f730c8d83..cd003ff46ff 100644 --- a/src/responder/pam/pamsrv_cmd.c +++ b/src/responder/pam/pamsrv_cmd.c @@ -2904,7 +2904,10 @@ static void pam_dom_forwarder(struct pam_auth_req *preq) found = true; if (preq->pd->cmd == SSS_PAM_PREAUTH) { ret = sss_authtok_set_sc(preq->pd->authtok, - SSS_AUTHTOK_TYPE_SC_PIN, NULL, 0, + sss_cai_get_has_protected_authentication_path(preq->current_cert) + ? SSS_AUTHTOK_TYPE_SC_KEYPAD + : SSS_AUTHTOK_TYPE_SC_PIN, + NULL, 0, sss_cai_get_token_name(preq->current_cert), 0, sss_cai_get_module_name(preq->current_cert), 0, sss_cai_get_key_id(preq->current_cert), 0, diff --git a/src/responder/pam/pamsrv_p11.c b/src/responder/pam/pamsrv_p11.c index 4f1e8d08481..dee40e19c0f 100644 --- a/src/responder/pam/pamsrv_p11.c +++ b/src/responder/pam/pamsrv_p11.c @@ -39,6 +39,7 @@ struct cert_auth_info { char *module_name; char *key_id; char *label; + bool has_protected_authentication_path; struct ldb_result *cert_user_objs; struct cert_auth_info *prev; struct cert_auth_info *next; @@ -69,6 +70,11 @@ const char *sss_cai_get_label(struct cert_auth_info *i) return i != NULL ? i->label : NULL; } +bool sss_cai_get_has_protected_authentication_path(struct cert_auth_info *i) +{ + return i != NULL ? i->has_protected_authentication_path : false; +} + struct cert_auth_info *sss_cai_get_next(struct cert_auth_info *i) { return i != NULL ? i->next : NULL; @@ -659,6 +665,37 @@ static errno_t parse_p11_child_response(TALLOC_CTX *mem_ctx, uint8_t *buf, goto done; } + if (pn == p) { + DEBUG(SSSDBG_OP_FAILURE, + "Missing protected authentication path info in p11_child " + "response.\n"); + ret = EINVAL; + goto done; + } + + if ((pn - p) == 4 && strncmp((char *) p, "true", 4) == 0) { + cert_auth_info->has_protected_authentication_path = true; + } else if ((pn - p) == 5 && strncmp((char *) p, "false", 5) == 0) { + cert_auth_info->has_protected_authentication_path = false; + } else { + DEBUG(SSSDBG_OP_FAILURE, + "Unexpected response where true/false was expected.\n"); + ret = EINVAL; + goto done; + } + DEBUG(SSSDBG_TRACE_ALL, "Found protected authentication path [%s].\n", + cert_auth_info->has_protected_authentication_path ? "true" + : "false"); + + p = ++pn; + pn = memchr(p, '\n', buf_len - (p - buf)); + if (pn == NULL) { + DEBUG(SSSDBG_OP_FAILURE, + "Missing new-line in p11_child response.\n"); + ret = EINVAL; + goto done; + } + if (pn == p) { DEBUG(SSSDBG_OP_FAILURE, "Missing cert in p11_child response.\n"); ret = EINVAL; @@ -1057,8 +1094,10 @@ static errno_t pack_cert_data(TALLOC_CTX *mem_ctx, const char *sysdb_username, size_t label_len; size_t prompt_len; size_t nss_name_len; + size_t has_pap_len; const char *username = ""; const char *nss_username = ""; + const char *has_protected_authentication_path; if (sysdb_username != NULL) { username = sysdb_username; @@ -1068,6 +1107,12 @@ static errno_t pack_cert_data(TALLOC_CTX *mem_ctx, const char *sysdb_username, nss_username = nss_name; } + if (sss_cai_get_has_protected_authentication_path(cert_info)) { + has_protected_authentication_path = "true"; + } else { + has_protected_authentication_path = "false"; + } + prompt = get_cert_prompt(mem_ctx, cert_info); if (prompt == NULL) { DEBUG(SSSDBG_OP_FAILURE, "get_cert_prompt failed.\n"); @@ -1085,10 +1130,11 @@ static errno_t pack_cert_data(TALLOC_CTX *mem_ctx, const char *sysdb_username, key_id_len = strlen(key_id) + 1; label_len = strlen(label) + 1; prompt_len = strlen(prompt) + 1; - nss_name_len = strlen(nss_username) +1; + nss_name_len = strlen(nss_username) + 1; + has_pap_len = strlen(has_protected_authentication_path) + 1; msg_len = user_len + token_len + module_len + key_id_len + label_len - + prompt_len + nss_name_len; + + prompt_len + nss_name_len + has_pap_len; msg = talloc_zero_size(mem_ctx, msg_len); if (msg == NULL) { @@ -1108,6 +1154,9 @@ static errno_t pack_cert_data(TALLOC_CTX *mem_ctx, const char *sysdb_username, memcpy(msg + user_len + token_len + module_len + key_id_len + label_len + prompt_len, nss_username, nss_name_len); + memcpy(msg + user_len + token_len + module_len + key_id_len + label_len + + prompt_len + nss_name_len, + has_protected_authentication_path, has_pap_len); talloc_free(prompt); if (_msg != NULL) { diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c index 5f070ee3692..67b88b5242a 100644 --- a/src/sss_client/pam_sss.c +++ b/src/sss_client/pam_sss.c @@ -138,6 +138,7 @@ struct cert_auth_info { char *module_name; char *key_id; char *label; + bool has_protected_authentication_path; char *prompt_str; char *pam_cert_user; char *choice_list_id; @@ -1041,10 +1042,28 @@ static int parse_cert_info(struct pam_items *pi, uint8_t *buf, size_t len, *pam_cert_user = cai->pam_cert_user; } + offset += strlen(cai->pam_cert_user) + 1; + if (offset >= len) { + D(("Cert message size mismatch")); + ret = EINVAL; + goto done; + } + + if (strcmp((char *) &buf[*p + offset], "true") == 0) { + cai->has_protected_authentication_path = true; + } else if (strcmp((char *) &buf[*p + offset], "false") == 0) { + cai->has_protected_authentication_path = false; + } else { + D(("Expected 'true' or 'false'")); + ret = EINVAL; + goto done; + } + D(("cert user: [%s] token name: [%s] module: [%s] key id: [%s] " - "prompt: [%s] pam cert user: [%s]", + "prompt: [%s] pam cert user: [%s] protected auth path: [%s]", cai->cert_user, cai->token_name, cai->module_name, - cai->key_id, cai->prompt_str, cai->pam_cert_user)); + cai->key_id, cai->prompt_str, cai->pam_cert_user, + cai->has_protected_authentication_path ? "true" : "false")); DLIST_ADD(pi->cert_list, cai); ret = 0; @@ -2111,6 +2130,7 @@ static int auth_selection_conversation_gdm(pam_handle_t *pamh, } #define SC_PROMPT_FMT "PIN for %s: " +#define SC_KEYPAD_FMT "Use external keypad to enter PIN for %s" #ifndef discard_const #define discard_const(ptr) ((void *)((uintptr_t)(ptr))) @@ -2313,6 +2333,7 @@ static int prompt_sc_pin(pam_handle_t *pamh, struct pam_items *pi) struct pam_message m[2] = { { 0 }, { 0 } }; struct pam_response *resp = NULL; struct cert_auth_info *cai = pi->selected_cert; + bool has_protected_authentication_path = false; if (cai == NULL && (SERVICE_IS_GDM_SMARTCARD(pi) || (pi->flags & PAM_CLI_FLAGS_REQUIRE_CERT_AUTH))) { @@ -2321,7 +2342,11 @@ static int prompt_sc_pin(pam_handle_t *pamh, struct pam_items *pi) || *cai->token_name == '\0') { return PAM_SYSTEM_ERR; } else { - ret = asprintf(&prompt, SC_PROMPT_FMT, cai->token_name); + ret = asprintf(&prompt, + cai->has_protected_authentication_path ? SC_KEYPAD_FMT + : SC_PROMPT_FMT, + cai->token_name); + has_protected_authentication_path = cai->has_protected_authentication_path; } if (ret == -1) { @@ -2348,7 +2373,8 @@ static int prompt_sc_pin(pam_handle_t *pamh, struct pam_items *pi) return PAM_SYSTEM_ERR; } - m[0].msg_style = PAM_PROMPT_ECHO_OFF; + m[0].msg_style = has_protected_authentication_path ? PAM_TEXT_INFO + : PAM_PROMPT_ECHO_OFF; m[0].msg = prompt; m[1].msg_style = PAM_PROMPT_ECHO_ON; m[1].msg = "User name hint: "; @@ -2375,20 +2401,24 @@ static int prompt_sc_pin(pam_handle_t *pamh, struct pam_items *pi) return PAM_SYSTEM_ERR; } - if (resp[0].resp == NULL || *(resp[0].resp) == '\0') { - D(("Missing PIN.")); - ret = PAM_CRED_INSUFFICIENT; - goto done; - } + if (has_protected_authentication_path) { + answer = NULL; + } else { + if (resp[0].resp == NULL || *(resp[0].resp) == '\0') { + D(("Missing PIN.")); + ret = PAM_CRED_INSUFFICIENT; + goto done; + } - answer = strndup(resp[0].resp, MAX_AUTHTOK_SIZE); - sss_erase_mem_securely((void *)resp[0].resp, strlen(resp[0].resp)); - free(resp[0].resp); - resp[0].resp = NULL; - if (answer == NULL) { - D(("strndup failed")); - ret = PAM_BUF_ERR; - goto done; + answer = strndup(resp[0].resp, MAX_AUTHTOK_SIZE); + sss_erase_mem_securely((void *)resp[0].resp, strlen(resp[0].resp)); + free(resp[0].resp); + resp[0].resp = NULL; + if (answer == NULL) { + D(("strndup failed")); + ret = PAM_BUF_ERR; + goto done; + } } if (resp[1].resp != NULL && *(resp[1].resp) != '\0') { @@ -2411,8 +2441,10 @@ static int prompt_sc_pin(pam_handle_t *pamh, struct pam_items *pi) pi->pam_user_size = strlen(pi->pam_user) + 1; } } else { - ret = do_pam_conversation(pamh, PAM_PROMPT_ECHO_OFF, prompt, NULL, - &answer); + ret = do_pam_conversation(pamh, + has_protected_authentication_path ? PAM_TEXT_INFO + : PAM_PROMPT_ECHO_OFF, + prompt, NULL, &answer); free(prompt); if (ret != PAM_SUCCESS) { D(("do_pam_conversation failed.")); @@ -2428,7 +2460,8 @@ static int prompt_sc_pin(pam_handle_t *pamh, struct pam_items *pi) goto done; } - if (answer == NULL || *answer == '\0') { + if (!has_protected_authentication_path + && (answer == NULL || *answer == '\0')) { D(("Missing PIN.")); ret = PAM_CRED_INSUFFICIENT; goto done; @@ -2465,7 +2498,9 @@ static int prompt_sc_pin(pam_handle_t *pamh, struct pam_items *pi) goto done; } - pi->pam_authtok_type = SSS_AUTHTOK_TYPE_SC_PIN; + pi->pam_authtok_type = has_protected_authentication_path + ? SSS_AUTHTOK_TYPE_SC_KEYPAD + : SSS_AUTHTOK_TYPE_SC_PIN; pi->pam_authtok_size = needed_size; } diff --git a/src/tests/cmocka/test_pam_srv.c b/src/tests/cmocka/test_pam_srv.c index 715ac04a9e9..d87ae8825c8 100644 --- a/src/tests/cmocka/test_pam_srv.c +++ b/src/tests/cmocka/test_pam_srv.c @@ -1076,7 +1076,8 @@ static int test_pam_cert_check_gdm_smartcard(uint32_t status, uint8_t *body, + sizeof(TEST_KEY_ID) + sizeof(TEST_LABEL) + sizeof(TEST_PROMPT) - + sizeof("pamuser"))); + + sizeof("pamuser") + + sizeof("false"))); assert_int_equal(*(body + rp + sizeof("pamuser@"TEST_DOM_NAME) - 1), 0); assert_string_equal((char *)(body + rp), "pamuser@"TEST_DOM_NAME); @@ -1106,6 +1107,10 @@ static int test_pam_cert_check_gdm_smartcard(uint32_t status, uint8_t *body, assert_string_equal((char *)(body + rp), "pamuser"); rp += sizeof("pamuser"); + assert_int_equal(*(body + rp + sizeof("false") - 1), 0); + assert_string_equal((char *)(body + rp), "false"); + rp += sizeof("false"); + assert_int_equal(rp, blen); return EOK; } @@ -1149,6 +1154,7 @@ static int test_pam_cert_check_ex(uint32_t status, uint8_t *body, size_t blen, TEST_LABEL, TEST_PROMPT, NULL, + "false", NULL }; size_t check2_len = 0; @@ -1159,6 +1165,7 @@ static int test_pam_cert_check_ex(uint32_t status, uint8_t *body, size_t blen, TEST2_LABEL, TEST2_PROMPT, NULL, + "false", NULL }; assert_int_equal(status, 0); @@ -1254,6 +1261,7 @@ static int test_pam_cert2_token2_check_ex(uint32_t status, uint8_t *body, TEST2_LABEL, TEST2_PROMPT, NULL, + "false", NULL }; assert_int_equal(status, 0); @@ -1358,6 +1366,7 @@ static int test_pam_cert5_check(uint32_t status, uint8_t *body, size_t blen) TEST5_LABEL, TEST5_PROMPT, NULL, + "false", NULL }; return test_pam_cert_X_token_X_check_ex(status, body, blen, SSS_PAM_CERT_INFO, @@ -1374,6 +1383,7 @@ static int test_pam_cert8_check(uint32_t status, uint8_t *body, size_t blen) TEST8_LABEL, TEST8_PROMPT, NULL, + "false", NULL }; return test_pam_cert_X_token_X_check_ex(status, body, blen, SSS_PAM_CERT_INFO, @@ -1390,6 +1400,7 @@ static int test_pam_cert9_check(uint32_t status, uint8_t *body, size_t blen) TEST9_LABEL, TEST9_PROMPT, NULL, + "false", NULL }; return test_pam_cert_X_token_X_check_ex(status, body, blen, SSS_PAM_CERT_INFO, From d8e130bc3b2e73f7b6f7110b4c1d27835fa04775 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Fri, 27 Feb 2026 19:09:32 +0100 Subject: [PATCH 3/5] authtok: remove sss_authtok_set_sc_keypad() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sss_authtok_set_sc_keypad() does not set which token and certificate should be used for authentication, just using sss_authtok_set_sc() with SSS_AUTHTOK_TYPE_SC_KEYPAD as type is sufficient. Reviewed-by: Alexey Tikhonov Reviewed-by: Pavel Březina --- src/tests/cmocka/test_authtok.c | 17 ----------------- src/util/authtok.c | 25 +++++++++++-------------- src/util/authtok.h | 12 +++--------- 3 files changed, 14 insertions(+), 40 deletions(-) diff --git a/src/tests/cmocka/test_authtok.c b/src/tests/cmocka/test_authtok.c index 53ad75d16a3..05fb993ac79 100644 --- a/src/tests/cmocka/test_authtok.c +++ b/src/tests/cmocka/test_authtok.c @@ -584,21 +584,6 @@ void test_sss_authtok_2fa_blobs_missing_null(void **state) MISSING_NULL_CHECK; } -void test_sss_authtok_sc_keypad(void **state) -{ - struct test_state *ts; - - ts = talloc_get_type_abort(*state, struct test_state); - - sss_authtok_set_sc_keypad(NULL); - - sss_authtok_set_sc_keypad(ts->authtoken); - assert_int_equal(sss_authtok_get_type(ts->authtoken), - SSS_AUTHTOK_TYPE_SC_KEYPAD); - assert_int_equal(sss_authtok_get_size(ts->authtoken), 0); - assert_null(sss_authtok_get_data(ts->authtoken)); -} - void test_sss_authtok_sc_pin(void **state) { struct test_state *ts; @@ -811,8 +796,6 @@ int main(int argc, const char *argv[]) setup, teardown), cmocka_unit_test_setup_teardown(test_sss_authtok_2fa_blobs_missing_null, setup, teardown), - cmocka_unit_test_setup_teardown(test_sss_authtok_sc_keypad, - setup, teardown), cmocka_unit_test_setup_teardown(test_sss_authtok_sc_pin, setup, teardown), cmocka_unit_test_setup_teardown(test_sss_authtok_sc_blobs, diff --git a/src/util/authtok.c b/src/util/authtok.c index 4c2d7ca4d24..ab443be3fba 100644 --- a/src/util/authtok.c +++ b/src/util/authtok.c @@ -439,9 +439,9 @@ errno_t sss_authtok_set(struct sss_auth_token *tok, case SSS_AUTHTOK_TYPE_2FA: return sss_authtok_set_2fa_from_blob(tok, data, len); case SSS_AUTHTOK_TYPE_SC_PIN: - return sss_authtok_set_sc_from_blob(tok, data, len); + return sss_authtok_set_sc_from_blob(tok, type, data, len); case SSS_AUTHTOK_TYPE_SC_KEYPAD: - return sss_authtok_set_sc_from_blob(tok, data, len); + return sss_authtok_set_sc_from_blob(tok, type, data, len); case SSS_AUTHTOK_TYPE_2FA_SINGLE: return sss_authtok_set_2fa_single(tok, (const char *) data, len); case SSS_AUTHTOK_TYPE_OAUTH2: @@ -926,6 +926,7 @@ errno_t sss_authtok_set_sc(struct sss_auth_token *tok, } errno_t sss_authtok_set_sc_from_blob(struct sss_auth_token *tok, + enum sss_authtok_type type, const uint8_t *data, size_t len) { @@ -949,6 +950,13 @@ errno_t sss_authtok_set_sc_from_blob(struct sss_auth_token *tok, return EINVAL; } + if (type != SSS_AUTHTOK_TYPE_SC_PIN + && type != SSS_AUTHTOK_TYPE_SC_KEYPAD) { + DEBUG(SSSDBG_CRIT_FAILURE, "Invalid type [%d].\n", type); + return EINVAL; + } + + tmp_ctx = talloc_new(NULL); if (tmp_ctx == NULL) { DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n"); @@ -965,7 +973,7 @@ errno_t sss_authtok_set_sc_from_blob(struct sss_auth_token *tok, goto done; } - ret = sss_authtok_set_sc(tok, SSS_AUTHTOK_TYPE_SC_PIN, pin, pin_len, + ret = sss_authtok_set_sc(tok, type, pin, pin_len, token_name, token_name_len, module_name, module_name_len, key_id, key_id_len, label, label_len); @@ -1033,17 +1041,6 @@ errno_t sss_authtok_get_sc_pin(struct sss_auth_token *tok, const char **_pin, return EINVAL; } -void sss_authtok_set_sc_keypad(struct sss_auth_token *tok) -{ - if (tok == NULL) { - return; - } - - sss_authtok_set_empty(tok); - - tok->type = SSS_AUTHTOK_TYPE_SC_KEYPAD; -} - errno_t sss_auth_unpack_sc_blob(TALLOC_CTX *mem_ctx, const uint8_t *blob, size_t blob_len, char **pin, size_t *_pin_len, diff --git a/src/util/authtok.h b/src/util/authtok.h index b681012112b..2b945996c6f 100644 --- a/src/util/authtok.h +++ b/src/util/authtok.h @@ -273,15 +273,6 @@ errno_t sss_authtok_set_sc_pin(struct sss_auth_token *tok, const char *pin, errno_t sss_authtok_get_sc_pin(struct sss_auth_token *tok, const char **pin, size_t *len); -/** - * @brief Sets an auth token to type SSS_AUTHTOK_TYPE_SC_KEYPAD, replacing any - * previous data - * - * @param tok A pointer to an sss_auth_token structure to change, also - * used as a memory context to allocate the internal data. - */ -void sss_authtok_set_sc_keypad(struct sss_auth_token *tok); - /** * @brief Set complete Smart Card authentication blob including PKCS#11 token * name, module name and key id. @@ -326,6 +317,8 @@ errno_t sss_authtok_set_sc(struct sss_auth_token *tok, * * @param tok A pointer to an sss_auth_token structure to change, also * used as a memory context to allocate the internal data. + * @param type Authentication token type, may be SSS_AUTHTOK_TYPE_SC_PIN or + * SSS_AUTHTOK_TYPE_SC_KEYPAD * @param data Smart Card authentication data blob * @param len The length of the blob * @@ -333,6 +326,7 @@ errno_t sss_authtok_set_sc(struct sss_auth_token *tok, * ENOMEM on error */ errno_t sss_authtok_set_sc_from_blob(struct sss_auth_token *tok, + enum sss_authtok_type type, const uint8_t *data, size_t len); From f07b3b235c35034e084e1f17f2bcc2d506ceecae Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Fri, 24 Apr 2026 10:03:08 +0200 Subject: [PATCH 4/5] pam_sss: fix potential memory leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In case the conversation callback allocates memory for a reply we have to free it. Reviewed-by: Alexey Tikhonov Reviewed-by: Pavel Březina --- src/sss_client/pam_sss.c | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c index 67b88b5242a..bd181d099cf 100644 --- a/src/sss_client/pam_sss.c +++ b/src/sss_client/pam_sss.c @@ -39,11 +39,33 @@ #ifdef HAVE_GDM_PAM_EXTENSIONS #include + +#ifndef GDM_PAM_EXTENSION_CHOICE_LIST_RESPONSE_FREE +#define GDM_PAM_EXTENSION_CHOICE_LIST_RESPONSE_FREE(response) \ +{ \ + if ((response)->key != NULL) { \ + sss_erase_mem_securely ((response)->key, strlen ((response)->key)); \ + free ((response)->key); \ + } \ + free (response); \ +} #endif +#endif /* HAVE_GDM_PAM_EXTENSIONS */ #ifdef HAVE_GDM_CUSTOM_JSON_PAM_EXTENSION #include + +#ifndef GDM_PAM_EXTENSION_CUSTOM_JSON_RESPONSE_FREE +#define GDM_PAM_EXTENSION_CUSTOM_JSON_RESPONSE_FREE(response) \ +{ \ + if ((response)->json != NULL) { \ + sss_erase_mem_securely ((response)->json, strlen ((response)->json)); \ + free ((response)->json); \ + } \ + free (response); \ +} #endif +#endif /* HAVE_GDM_CUSTOM_JSON_PAM_EXTENSION */ #include "sss_pam_compat.h" #include "sss_pam_macros.h" @@ -349,6 +371,17 @@ static int do_pam_conversation(pam_handle_t *pamh, const int msg_style, resp = NULL; } + if (resp != NULL) { + if (resp[0].resp != NULL) { + /* We do not know what might be the content, better be on the + * safe side and clean the memory. */ + sss_erase_mem_securely((void *)resp[0].resp, strlen(resp[0].resp)); + free(resp[0].resp); + } + free(resp); + resp = NULL; + } + if (reenter_msg != NULL && state == SSS_PAM_CONV_STD) { state = SSS_PAM_CONV_REENTER; } else { @@ -2121,7 +2154,8 @@ static int auth_selection_conversation_gdm(pam_handle_t *pamh, if (request != NULL) { free(request); } - free(response); + GDM_PAM_EXTENSION_CUSTOM_JSON_RESPONSE_FREE(response); + free(reply); return ret; #else @@ -2231,7 +2265,8 @@ static int prompt_multi_cert_gdm(pam_handle_t *pamh, struct pam_items *pi) } free(request); } - free(response); + GDM_PAM_EXTENSION_CHOICE_LIST_RESPONSE_FREE(response); + free(reply); return ret; #else From 330455c47a1e2b3a11bfb98eb0920d97fdc7edf9 Mon Sep 17 00:00:00 2001 From: Sumit Bose Date: Fri, 24 Apr 2026 13:22:06 +0200 Subject: [PATCH 5/5] pam: refactor pack_cert_data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use safealign_memcpy() instead of plain memcpy() and add a consistency check. Reviewed-by: Alexey Tikhonov Reviewed-by: Pavel Březina --- src/responder/pam/pamsrv_p11.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/responder/pam/pamsrv_p11.c b/src/responder/pam/pamsrv_p11.c index dee40e19c0f..191bf40c585 100644 --- a/src/responder/pam/pamsrv_p11.c +++ b/src/responder/pam/pamsrv_p11.c @@ -1098,6 +1098,7 @@ static errno_t pack_cert_data(TALLOC_CTX *mem_ctx, const char *sysdb_username, const char *username = ""; const char *nss_username = ""; const char *has_protected_authentication_path; + size_t offset = 0; if (sysdb_username != NULL) { username = sysdb_username; @@ -1143,21 +1144,23 @@ static errno_t pack_cert_data(TALLOC_CTX *mem_ctx, const char *sysdb_username, return ENOMEM; } - memcpy(msg, username, user_len); - memcpy(msg + user_len, token_name, token_len); - memcpy(msg + user_len + token_len, module_name, module_len); - memcpy(msg + user_len + token_len + module_len, key_id, key_id_len); - memcpy(msg + user_len + token_len + module_len + key_id_len, - label, label_len); - memcpy(msg + user_len + token_len + module_len + key_id_len + label_len, - prompt, prompt_len); - memcpy(msg + user_len + token_len + module_len + key_id_len + label_len - + prompt_len, - nss_username, nss_name_len); - memcpy(msg + user_len + token_len + module_len + key_id_len + label_len - + prompt_len + nss_name_len, - has_protected_authentication_path, has_pap_len); + safealign_memcpy(msg, username, user_len, &offset); + safealign_memcpy(msg + offset, token_name, token_len, &offset); + safealign_memcpy(msg + offset, module_name, module_len, &offset); + safealign_memcpy(msg + offset, key_id, key_id_len, &offset); + safealign_memcpy(msg + offset, label, label_len, &offset); + safealign_memcpy(msg + offset, prompt, prompt_len, &offset); + safealign_memcpy(msg + offset, nss_username, nss_name_len, &offset); + safealign_memcpy(msg + offset, has_protected_authentication_path, + has_pap_len, &offset); talloc_free(prompt); + if (offset != msg_len) { + DEBUG(SSSDBG_OP_FAILURE, + "Expected [%zu] and copied [%zu] number of bytes do not match.\n", + msg_len, offset); + talloc_free(msg); + return EIO; + } if (_msg != NULL) { *_msg = msg;