Skip to content

Commit

Permalink
passkey: Skip processing non-passkey mapping data
Browse files Browse the repository at this point in the history
In the AD case, the user altSecurityIdentities attribute can
store passkey, smartcard, or ssh public key mapping data. Check
to ensure we are handling passkey data before continuing in
PAM passkey processing.

:relnote: Fixes a crash when PAM passkey processing incorrectly
handles non-passkey data.

Resolves: #7061

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
(cherry picked from commit 6ed1eff)
  • Loading branch information
justin-stephenson authored and alexey-tikhonov committed Dec 12, 2023
1 parent f6faf12 commit 4d01e11
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 1 deletion.
21 changes: 20 additions & 1 deletion src/responder/pam/pamsrv_passkey.c
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,22 @@ errno_t process_passkey_data(TALLOC_CTX *mem_ctx,
goto done;
}

/* This attribute may contain other mapping data unrelated to passkey. In that case
* let's omit it. For example, AD user altSecurityIdentities may store ssh public key
* or smart card mapping data */
ret = split_on_separator(tmp_ctx, (const char *) el->values[0].data, ':', true, true,
&mappings, NULL);
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE, "Incorrectly formatted passkey data [%d]: %s\n",
ret, sss_strerror(ret));
ret = ENOENT;
goto done;
} else if (strcasecmp(mappings[0], "passkey") != 0) {
DEBUG(SSSDBG_TRACE_FUNC, "Mapping data found is not passkey related\n");
ret = ENOENT;
goto done;
}

kh_mappings = talloc_zero_array(tmp_ctx, const char *, el->num_values + 1);
if (kh_mappings == NULL) {
DEBUG(SSSDBG_OP_FAILURE, "talloc_zero_array failed.\n");
Expand Down Expand Up @@ -624,7 +640,10 @@ void pam_passkey_get_user_done(struct tevent_req *req)
/* Get passkey data */
DEBUG(SSSDBG_TRACE_ALL, "Processing passkey data\n");
ret = process_passkey_data(pk_data, result->msgs[0], domain_name, pk_data);
if (ret == ENOENT) {
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE,
"process_passkey_data failed: [%d]: %s\n",
ret, sss_strerror(ret));
goto done;
}

Expand Down
60 changes: 60 additions & 0 deletions src/tests/cmocka/test_pam_srv.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,15 @@
"MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEEKhSQWMPgAUcz4d7Fjz2hZK7QUlnAttuEW5Xr" \
"xD06VBaQvIRYJT7e6wM+vFU4z+uQgU9B5ERbgMiBVe99rBL9w=="

#define SSSD_TEST_PUBKEY \
"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQCa+l8uZ6Q5G58PVMe1na7NrOMTzo2wOZfFwo" \
"0fM3RbvfAdlz/wsGwln2+EXA19FiXu/nNj4EwYGP9hymKuYaXzpq40k0VbhEL1v/qzXQvuKZgN" \
"x42vxi7NITaaAXuYj8OZQsZTvv+xgkREZmhQ6YqEjTJ0JzpD9fj8Gf8Mgn8pdsb/ZODLMAwEKt" \
"Q2DaWqH5jCqzoGEJlRl+kRbnrHc+RQrmj7NnY1voEJNrmzCyJFH5awZyBl/ZdbvpnwCKnVEleB" \
"FULrOIfJ9lc/QMmURCMa6RfW5CFrxdtjUwiIxfMiHe+zUY5T9L0Q6FWnlfNz/63Xdcrw1Gc90O" \
"CZKcqf/4P9N5flGSGSfiO5fD8gCCJ0c3WhxSVMREDP3ibKDsz8yhw2OuyGcfRo4nnchxy9G703" \
"1m2t9rUXc12eS1EKGJiPiT9IuTQ9nCG2PslkqR+KUMiYoS9MqTsAj9HhuTMkFhcYFyufxFmt/S" \
"4rIqVwmP8lY4GwwJwOnZwNLj/I2HwC+pk= testuser@fedora.test.local"

int no_cleanup;

Expand Down Expand Up @@ -4403,6 +4412,55 @@ void test_pam_passkey_auth(void **state)
assert_int_equal(ret, EOK);
}

void test_pam_passkey_bad_mapping(void **state)
{
int ret;
struct sysdb_attrs *attrs;
const char *pubkey = SSSD_TEST_PUBKEY;
size_t pk_size;
const char *user_verification = "on";

set_passkey_auth_param(pam_test_ctx->pctx);

/* Add user verification attribute */
ret = sysdb_domain_update_passkey_user_verification(
pam_test_ctx->tctx->dom->sysdb,
pam_test_ctx->tctx->dom->name,
user_verification);
assert_int_equal(ret, EOK);

mock_input_pam_passkey(pam_test_ctx, "pamuser", "1234", NULL,
NULL, SSSD_TEST_PASSKEY);
mock_parse_inp("pamuser", NULL, EOK);

will_return(__wrap_sss_packet_get_cmd, SSS_PAM_AUTHENTICATE);
will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);

/* Add the test invalid pubkey data for this user */
pk_size = strlen(pubkey) + 1;

attrs = sysdb_new_attrs(pam_test_ctx);
assert_non_null(attrs);

ret = sysdb_attrs_add_mem(attrs, SYSDB_USER_PASSKEY, pubkey, pk_size);
assert_int_equal(ret, EOK);

ret = sysdb_set_user_attr(pam_test_ctx->tctx->dom,
pam_test_ctx->pam_user_fqdn,
attrs,
LDB_FLAG_MOD_ADD);
assert_int_equal(ret, EOK);

pam_test_ctx->exp_pam_status = PAM_SUCCESS;
set_cmd_cb(test_pam_passkey_auth_check);
ret = sss_cmd_execute(pam_test_ctx->cctx, SSS_PAM_AUTHENTICATE,
pam_test_ctx->pam_cmds);
assert_int_equal(ret, EOK);

ret = test_ev_loop(pam_test_ctx->tctx);
assert_int_equal(ret, EOK);
}


void test_pam_passkey_auth_send(void **state)
{
Expand Down Expand Up @@ -4630,6 +4688,8 @@ int main(int argc, const char *argv[])
pam_test_setup_passkey, pam_test_teardown),
cmocka_unit_test_setup_teardown(test_pam_passkey_auth,
pam_test_setup_passkey, pam_test_teardown),
cmocka_unit_test_setup_teardown(test_pam_passkey_bad_mapping,
pam_test_setup_passkey, pam_test_teardown),
cmocka_unit_test_setup_teardown(test_pam_passkey_auth_send,
pam_test_setup_passkey, pam_test_teardown),
cmocka_unit_test_setup_teardown(test_pam_prompting_passkey_interactive,
Expand Down

0 comments on commit 4d01e11

Please sign in to comment.