URL: https://github.com/SSSD/sssd/pull/871 Author: sumit-bose Title: #871: pam: set PAM_USER properly with allow_missing_name Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/871/head:pr871 git checkout pr871
From 2a890b606654aa01d6db2f724a08e8bda837eabf Mon Sep 17 00:00:00 2001 From: Sumit Bose <sb...@redhat.com> Date: Thu, 15 Aug 2019 15:01:28 +0200 Subject: [PATCH] pam: set PAM_USER properly with allow_missing_name Currently if the allow_missing_name pam_sss option is used PAM_USER is set to the fully-qualified name only for the files provider it is set to the short name. This might cause issue with other components expecting that the value of PAM_USER corresponds to the name returned by the nss calls getpwnam() and getpwuid(). With this patch PAM_USER is set to the same user name as returned by the NSS responder. For the communication between pam_sss and SSSD's PAM responder the fully-qualified name is kept. Related to https://pagure.io/SSSD/sssd/issue/4069 --- src/responder/pam/pamsrv_p11.c | 35 +++++++-- src/sss_client/pam_sss.c | 42 +++++++---- src/tests/cmocka/test_pam_srv.c | 33 ++++++--- src/tests/intg/Makefile.am | 9 ++- src/tests/intg/test_pam_responder.py | 102 ++++++++++++++++++++++++++- 5 files changed, 189 insertions(+), 32 deletions(-) diff --git a/src/responder/pam/pamsrv_p11.c b/src/responder/pam/pamsrv_p11.c index 67fbd89efb..f46e6bff87 100644 --- a/src/responder/pam/pamsrv_p11.c +++ b/src/responder/pam/pamsrv_p11.c @@ -1077,6 +1077,7 @@ static char *get_cert_prompt(TALLOC_CTX *mem_ctx, static errno_t pack_cert_data(TALLOC_CTX *mem_ctx, const char *sysdb_username, struct cert_auth_info *cert_info, + const char *nss_name, uint8_t **_msg, size_t *_msg_len) { uint8_t *msg = NULL; @@ -1090,12 +1091,18 @@ static errno_t pack_cert_data(TALLOC_CTX *mem_ctx, const char *sysdb_username, size_t module_len; size_t key_id_len; size_t prompt_len; + size_t nss_name_len; const char *username = ""; + const char *nss_username = ""; if (sysdb_username != NULL) { username = sysdb_username; } + if (nss_name != NULL) { + nss_username = nss_name; + } + prompt = get_cert_prompt(mem_ctx, cert_info); if (prompt == NULL) { DEBUG(SSSDBG_OP_FAILURE, "get_cert_prompt failed.\n"); @@ -1111,7 +1118,10 @@ static errno_t pack_cert_data(TALLOC_CTX *mem_ctx, const char *sysdb_username, module_len = strlen(module_name) + 1; key_id_len = strlen(key_id) + 1; prompt_len = strlen(prompt) + 1; - msg_len = user_len + token_len + module_len + key_id_len + prompt_len; + nss_name_len = strlen(nss_username) +1; + + msg_len = user_len + token_len + module_len + key_id_len + prompt_len + + nss_name_len; msg = talloc_zero_size(mem_ctx, msg_len); if (msg == NULL) { @@ -1126,6 +1136,8 @@ static errno_t pack_cert_data(TALLOC_CTX *mem_ctx, const char *sysdb_username, memcpy(msg + user_len + token_len + module_len, key_id, key_id_len); memcpy(msg + user_len + token_len + module_len + key_id_len, prompt, prompt_len); + memcpy(msg + user_len + token_len + module_len + key_id_len + prompt_len, + nss_username, nss_name_len); talloc_free(prompt); if (_msg != NULL) { @@ -1159,6 +1171,8 @@ errno_t add_pam_cert_response(struct pam_data *pd, struct sss_domain_info *dom, char *short_name = NULL; char *domain_name = NULL; const char *cert_info_name = sysdb_username; + struct sss_domain_info *user_dom; + char *nss_name = NULL; if (type != SSS_PAM_CERT_INFO && type != SSS_PAM_CERT_INFO_WITH_HINT) { @@ -1194,15 +1208,24 @@ errno_t add_pam_cert_response(struct pam_data *pd, struct sss_domain_info *dom, "using full name.\n", sysdb_username, ret, sss_strerror(ret)); } else { - if (domain_name != NULL - && is_files_provider(find_domain_by_name(dom, domain_name, - false))) { - cert_info_name = short_name; + if (domain_name != NULL) { + user_dom = find_domain_by_name(dom, domain_name, false); + + if (user_dom != NULL) { + ret = sss_output_fqname(short_name, user_dom, + sysdb_username, false, &nss_name); + if (ret != EOK) { + nss_name = NULL; + } + } } + } } - ret = pack_cert_data(pd, cert_info_name, cert_info, &msg, &msg_len); + ret = pack_cert_data(pd, cert_info_name, cert_info, + nss_name != NULL ? nss_name : sysdb_username, + &msg, &msg_len); talloc_free(short_name); talloc_free(domain_name); if (ret != EOK) { diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c index cfd3e3731d..21a5813486 100644 --- a/src/sss_client/pam_sss.c +++ b/src/sss_client/pam_sss.c @@ -124,6 +124,7 @@ struct cert_auth_info { char *module_name; char *key_id; char *prompt_str; + char *pam_cert_user; struct cert_auth_info *prev; struct cert_auth_info *next; }; @@ -853,7 +854,8 @@ static int eval_user_info_response(pam_handle_t *pamh, size_t buflen, } static int parse_cert_info(struct pam_items *pi, uint8_t *buf, size_t len, - size_t *p, const char **cert_user) + size_t *p, const char **cert_user, + const char **pam_cert_user) { struct cert_auth_info *cai = NULL; size_t offset; @@ -935,11 +937,27 @@ static int parse_cert_info(struct pam_items *pi, uint8_t *buf, size_t len, goto done; } + offset += strlen(cai->prompt_str) + 1; + if (offset >= len) { + D(("Cert message size mismatch")); + ret = EINVAL; + goto done; + } + + cai->pam_cert_user = strdup((char *) &buf[*p + offset]); + if (cai->pam_cert_user == NULL) { + D(("strdup failed")); + ret = ENOMEM; + goto done; + } + if (pam_cert_user != NULL) { + *pam_cert_user = cai->pam_cert_user; + } D(("cert user: [%s] token name: [%s] module: [%s] key id: [%s] " - "prompt: [%s]", + "prompt: [%s] pam cert user: [%s]", cai->cert_user, cai->token_name, cai->module_name, - cai->key_id, cai->prompt_str)); + cai->key_id, cai->prompt_str, cai->pam_cert_user)); DLIST_ADD(pi->cert_list, cai); ret = 0; @@ -964,6 +982,7 @@ static int eval_response(pam_handle_t *pamh, size_t buflen, uint8_t *buf, int32_t pam_status; size_t offset; const char *cert_user; + const char *pam_cert_user; if (buflen < (2*sizeof(int32_t))) { D(("response buffer is too small")); @@ -1126,15 +1145,16 @@ static int eval_response(pam_handle_t *pamh, size_t buflen, uint8_t *buf, pi->user_name_hint = false; } - ret = parse_cert_info(pi, buf, len, &p, &cert_user); + ret = parse_cert_info(pi, buf, len, &p, &cert_user, + &pam_cert_user); if (ret != 0) { D(("Failed to parse cert info")); break; } if ((pi->pam_user == NULL || *(pi->pam_user) == '\0') - && *cert_user != '\0') { - ret = pam_set_item(pamh, PAM_USER, cert_user); + && *cert_user != '\0' && *pam_cert_user != '\0') { + ret = pam_set_item(pamh, PAM_USER, pam_cert_user); if (ret != PAM_SUCCESS) { D(("Failed to set PAM_USER during " "Smartcard authentication [%s]", @@ -1142,15 +1162,7 @@ static int eval_response(pam_handle_t *pamh, size_t buflen, uint8_t *buf, break; } - ret = pam_get_item(pamh, PAM_USER, - (const void **)&(pi->pam_user)); - if (ret != PAM_SUCCESS) { - D(("Failed to get PAM_USER during " - "Smartcard authentication [%s]", - pam_strerror(pamh, ret))); - break; - } - + pi->pam_user = cert_user; pi->pam_user_size = strlen(pi->pam_user) + 1; } break; diff --git a/src/tests/cmocka/test_pam_srv.c b/src/tests/cmocka/test_pam_srv.c index f55e6222ea..6a92622d0d 100644 --- a/src/tests/cmocka/test_pam_srv.c +++ b/src/tests/cmocka/test_pam_srv.c @@ -892,7 +892,8 @@ static int test_pam_cert_check_gdm_smartcard(uint32_t status, uint8_t *body, + sizeof(TEST_TOKEN_NAME) + sizeof(TEST_MODULE_NAME) + sizeof(TEST_KEY_ID) - + sizeof(TEST_PROMPT))); + + sizeof(TEST_PROMPT) + + sizeof("pamuser"))); assert_int_equal(*(body + rp + sizeof("pamuser@"TEST_DOM_NAME) - 1), 0); assert_string_equal(body + rp, "pamuser@"TEST_DOM_NAME); @@ -914,6 +915,10 @@ static int test_pam_cert_check_gdm_smartcard(uint32_t status, uint8_t *body, assert_string_equal(body + rp, TEST_PROMPT); rp += sizeof(TEST_PROMPT); + assert_int_equal(*(body + rp + sizeof("pamuser") - 1), 0); + assert_string_equal(body + rp, "pamuser"); + rp += sizeof("pamuser"); + assert_int_equal(rp, blen); return EOK; } @@ -943,7 +948,7 @@ static size_t check_string_array_len(const char **strs) static int test_pam_cert_check_ex(uint32_t status, uint8_t *body, size_t blen, enum response_type type, const char *name, - const char *name2) + const char *name2, const char *nss_name) { size_t rp = 0; uint32_t val; @@ -955,6 +960,7 @@ static int test_pam_cert_check_ex(uint32_t status, uint8_t *body, size_t blen, TEST_MODULE_NAME, TEST_KEY_ID, TEST_PROMPT, + NULL, NULL }; size_t check2_len = 0; @@ -963,15 +969,19 @@ static int test_pam_cert_check_ex(uint32_t status, uint8_t *body, size_t blen, TEST_MODULE_NAME, TEST2_KEY_ID, TEST2_PROMPT, + NULL, NULL }; assert_int_equal(status, 0); check_strings[0] = name; + check_strings[5] = nss_name; check_len = check_string_array_len(check_strings); check2_strings[0] = name; + check2_strings[5] = nss_name; check2_len = check_string_array_len(check2_strings); + SAFEALIGN_COPY_UINT32(&val, body + rp, &rp); assert_int_equal(val, pam_test_ctx->exp_pam_status); @@ -1042,7 +1052,8 @@ static int test_pam_cert_check_ex(uint32_t status, uint8_t *body, size_t blen, static int test_pam_cert2_token2_check_ex(uint32_t status, uint8_t *body, size_t blen, enum response_type type, - const char *name) + const char *name, + const char *nss_name) { size_t rp = 0; uint32_t val; @@ -1052,11 +1063,13 @@ static int test_pam_cert2_token2_check_ex(uint32_t status, uint8_t *body, TEST_MODULE_NAME, TEST2_KEY_ID, TEST2_PROMPT, + NULL, NULL }; assert_int_equal(status, 0); check2_strings[0] = name; + check2_strings[5] = nss_name; check2_len = check_string_array_len(check2_strings); SAFEALIGN_COPY_UINT32(&val, body + rp, &rp); @@ -1092,13 +1105,13 @@ static int test_pam_cert_check(uint32_t status, uint8_t *body, size_t blen) { return test_pam_cert_check_ex(status, body, blen, SSS_PAM_CERT_INFO, "pamuser@"TEST_DOM_NAME, - NULL); + NULL, "pamuser"); } static int test_pam_cert2_check(uint32_t status, uint8_t *body, size_t blen) { return test_pam_cert2_token2_check_ex(status, body, blen, SSS_PAM_CERT_INFO, - "pamuser@"TEST_DOM_NAME); + "pamuser@"TEST_DOM_NAME, "pamuser"); } static int test_pam_cert_check_auth_success(uint32_t status, uint8_t *body, @@ -1108,7 +1121,7 @@ static int test_pam_cert_check_auth_success(uint32_t status, uint8_t *body, pam_test_ctx->exp_pam_status = PAM_SUCCESS; return test_pam_cert_check_ex(status, body, blen, SSS_PAM_CERT_INFO, "pamuser@"TEST_DOM_NAME, - NULL); + NULL, "pamuser"); } static int test_pam_cert_check_with_hint(uint32_t status, uint8_t *body, @@ -1116,14 +1129,15 @@ static int test_pam_cert_check_with_hint(uint32_t status, uint8_t *body, { return test_pam_cert_check_ex(status, body, blen, SSS_PAM_CERT_INFO_WITH_HINT, - "pamuser@"TEST_DOM_NAME, NULL); + "pamuser@"TEST_DOM_NAME, NULL, + "pamuser"); } static int test_pam_cert_check_with_hint_no_user(uint32_t status, uint8_t *body, size_t blen) { return test_pam_cert_check_ex(status, body, blen, - SSS_PAM_CERT_INFO_WITH_HINT, "", NULL); + SSS_PAM_CERT_INFO_WITH_HINT, "", NULL, ""); } static int test_pam_cert_check_2certs(uint32_t status, uint8_t *body, @@ -1131,7 +1145,8 @@ static int test_pam_cert_check_2certs(uint32_t status, uint8_t *body, { return test_pam_cert_check_ex(status, body, blen, SSS_PAM_CERT_INFO, "pamuser@"TEST_DOM_NAME, - "pamuser@"TEST_DOM_NAME); + "pamuser@"TEST_DOM_NAME, + "pamuser"); } static int test_pam_offline_chauthtok_check(uint32_t status, diff --git a/src/tests/intg/Makefile.am b/src/tests/intg/Makefile.am index 2aa1566e35..820c9ed3cc 100644 --- a/src/tests/intg/Makefile.am +++ b/src/tests/intg/Makefile.am @@ -134,6 +134,13 @@ pam_sss_try_sc: echo "password required $(DESTDIR)$(pammoddir)/pam_sss.so" >> $(PAM_SERVICE_DIR)/$@ echo "session required $(DESTDIR)$(pammoddir)/pam_sss.so" >> $(PAM_SERVICE_DIR)/$@ +pam_sss_allow_missing_name: + $(MKDIR_P) $(PAM_SERVICE_DIR) + echo "auth required $(DESTDIR)$(pammoddir)/pam_sss.so allow_missing_name" > $(PAM_SERVICE_DIR)/$@ + echo "account required $(DESTDIR)$(pammoddir)/pam_sss.so" >> $(PAM_SERVICE_DIR)/$@ + echo "password required $(DESTDIR)$(pammoddir)/pam_sss.so" >> $(PAM_SERVICE_DIR)/$@ + echo "session required $(DESTDIR)$(pammoddir)/pam_sss.so" >> $(PAM_SERVICE_DIR)/$@ + CLEANFILES=config.py config.pyc passwd group clean-local: @@ -148,7 +155,7 @@ PAM_CERT_DB_PATH="$(abs_builddir)/../test_CA/SSSD_test_CA.pem" SOFTHSM2_CONF="$(abs_builddir)/../test_CA/softhsm2_one.conf" endif -intgcheck-installed: config.py passwd group pam_sss_service pam_sss_alt_service pam_sss_sc_required pam_sss_try_sc +intgcheck-installed: config.py passwd group pam_sss_service pam_sss_alt_service pam_sss_sc_required pam_sss_try_sc pam_sss_allow_missing_name pipepath="$(DESTDIR)$(pipepath)"; \ if test $${#pipepath} -gt 80; then \ echo "error: Pipe directory path too long," \ diff --git a/src/tests/intg/test_pam_responder.py b/src/tests/intg/test_pam_responder.py index 8e1fcf126e..74d12d8f99 100644 --- a/src/tests/intg/test_pam_responder.py +++ b/src/tests/intg/test_pam_responder.py @@ -131,7 +131,7 @@ def format_pam_cert_auth_conf(config): [pam] pam_cert_auth = True pam_p11_allowed_services = +pam_sss_service, +pam_sss_sc_required, \ - +pam_sss_try_sc + +pam_sss_try_sc, +pam_sss_allow_missing_name pam_cert_db_path = {config.PAM_CERT_DB_PATH} p11_child_timeout = 5 p11_wait_for_card_timeout = 5 @@ -146,6 +146,37 @@ def format_pam_cert_auth_conf(config): """).format(**locals()) +def format_pam_cert_auth_conf_name_format(config): + """Format SSSD configuration with full_name_format""" + return unindent("""\ + [sssd] + debug_level = 10 + domains = auth_only + services = pam, nss + + [nss] + debug_level = 10 + + [pam] + pam_cert_auth = True + pam_p11_allowed_services = +pam_sss_service, +pam_sss_sc_required, \ + +pam_sss_try_sc, +pam_sss_allow_missing_name + pam_cert_db_path = {config.PAM_CERT_DB_PATH} + p11_child_timeout = 5 + p11_wait_for_card_timeout = 5 + debug_level = 10 + + [domain/auth_only] + use_fully_qualified_names = True + full_name_format = %2$s\%1$s + debug_level = 10 + id_provider = files + + [certmap/auth_only/user1] + matchrule = <SUBJECT>.*CN=SSSD test cert 0001.* + """).format(**locals()) + + def create_conf_file(contents): """Create sssd.conf with specified contents""" conf = open(config.CONF_PATH, "w") @@ -284,6 +315,19 @@ def simple_pam_cert_auth_no_cert(request, passwd_ops_setup): return None +@pytest.fixture +def simple_pam_cert_auth_name_format(request, passwd_ops_setup): + """Setup SSSD with pam_cert_auth=True and full_name_format""" + config.PAM_CERT_DB_PATH = os.environ['PAM_CERT_DB_PATH'] + conf = format_pam_cert_auth_conf_name_format(config) + create_conf_fixture(request, conf) + create_sssd_fixture(request) + create_nssdb_fixture(request) + passwd_ops_setup.useradd(**USER1) + passwd_ops_setup.useradd(**USER2) + return None + + def test_preauth_indicator(simple_pam_cert_auth): """Check if preauth indicator file is created""" statinfo = os.stat(config.PUBCONF_PATH + "/pam_preauth_available") @@ -546,3 +590,59 @@ def test_try_sc_auth_root(simple_pam_cert_auth, env_for_sssctl): assert err.find("pam_authenticate for user [root]: Authentication " + "service cannot retrieve authentication info") != -1 + + +def test_sc_auth_missing_name(simple_pam_cert_auth, env_for_sssctl): + """ + Test pam_sss allow_missing_name feature. + """ + + sssctl = subprocess.Popen(["sssctl", "user-checks", "", + "--action=auth", + "--service=pam_sss_allow_missing_name"], + universal_newlines=True, + env=env_for_sssctl, stdin=subprocess.PIPE, + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + + try: + out, err = sssctl.communicate(input="123456") + except: + sssctl.kill() + out, err = sssctl.communicate() + + sssctl.stdin.close() + sssctl.stdout.close() + + if sssctl.wait() != 0: + raise Exception("sssctl failed") + + assert err.find("pam_authenticate for user [user1]: Success") != -1 + + +def test_sc_auth_name_format(simple_pam_cert_auth_name_format, env_for_sssctl): + """ + Test that full_name_format is respected with pam_sss allow_missing_name + option. + """ + + sssctl = subprocess.Popen(["sssctl", "user-checks", "", + "--action=auth", + "--service=pam_sss_allow_missing_name"], + universal_newlines=True, + env=env_for_sssctl, stdin=subprocess.PIPE, + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + + try: + out, err = sssctl.communicate(input="123456") + except: + sssctl.kill() + out, err = sssctl.communicate() + + sssctl.stdin.close() + sssctl.stdout.close() + + if sssctl.wait() != 0: + raise Exception("sssctl failed") + + assert err.find("pam_authenticate for user [auth_only\user1]: " + + "Success") != -1
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
