URL: https://github.com/SSSD/sssd/pull/141 Author: fidencio Title: #141: PAM: Use cache_req_user_by_name_*() Action: opened
PR body: """ PAM responder has been already using a lot from cache_req and one of the missing parts (most likely the only one?) was that pam_check_user_search() has been using sss_dp_get_account_send(), which could be dropped (and a lot of code around it) in favour of using cache_req_user_by_name_*() instead. Resolves: https://fedorahosted.org/sssd/ticket/1126 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> """ To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/141/head:pr141 git checkout pr141
From 381a29056206862d92de8dd2185d19eeeaef5ed0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Fri, 27 Jan 2017 21:49:52 +0100 Subject: [PATCH] PAM: Use cache_req_user_by_name_*() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PAM responder has been already using a lot from cache_req and one of the missing parts (most likely the only one?) was that pam_check_user_search() has been using sss_dp_get_account_send(), which could be dropped (and a lot of code around it) in favour of using cache_req_user_by_name_*() instead. Resolves: https://fedorahosted.org/sssd/ticket/1126 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/responder/pam/pamsrv_cmd.c | 349 ++++++----------------------------------- 1 file changed, 46 insertions(+), 303 deletions(-) diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c index e73a819..b962539 100644 --- a/src/responder/pam/pamsrv_cmd.c +++ b/src/responder/pam/pamsrv_cmd.c @@ -1016,44 +1016,9 @@ static void pam_handle_cached_login(struct pam_auth_req *preq, int ret, static void pam_forwarder_cb(struct tevent_req *req); static void pam_forwarder_cert_cb(struct tevent_req *req); -static void pam_check_user_dp_callback(uint16_t err_maj, uint32_t err_min, - const char *err_msg, void *ptr); static int pam_check_user_search(struct pam_auth_req *preq); static int pam_check_user_done(struct pam_auth_req *preq, int ret); -static errno_t pam_cmd_assume_upn(struct pam_auth_req *preq) -{ - int ret; - - if (!preq->pd->name_is_upn - && preq->pd->logon_name != NULL - && strchr(preq->pd->logon_name, '@') != NULL) { - DEBUG(SSSDBG_TRACE_ALL, - "No entry found so far, trying UPN/email lookup with [%s].\n", - preq->pd->logon_name); - /* Assuming Kerberos principal */ - preq->domain = preq->cctx->rctx->domains; - preq->check_provider = - NEED_CHECK_PROVIDER(preq->domain->provider); - preq->pd->user = talloc_strdup(preq->pd, preq->pd->logon_name); - if (preq->pd->user == NULL) { - DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n"); - return ENOMEM; - } - preq->pd->name_is_upn = true; - preq->pd->domain = NULL; - - ret = pam_check_user_search(preq); - if (ret == EOK) { - pam_dom_forwarder(preq); - } - return EOK; - } - - return ENOENT; -} - - /* TODO: we should probably return some sort of cookie that is set in the * PAM_ENVIRONMENT, so that we can save performing some calls and cache * data. */ @@ -1348,11 +1313,6 @@ static int pam_forwarder(struct cli_ctx *cctx, int pam_cmd) preq->check_provider = NEED_CHECK_PROVIDER(preq->domain->provider); ret = pam_check_user_search(preq); - if (ret == EOK) { - pam_dom_forwarder(preq); - } else if (ret == ENOENT) { - ret = pam_cmd_assume_upn(preq); - } done: return pam_check_user_done(preq, ret); @@ -1392,9 +1352,6 @@ static void pam_forwarder_cert_cb(struct tevent_req *req) ret = ENOENT; } else { ret = pam_check_user_search(preq); - if (ret == EOK) { - pam_dom_forwarder(preq); - } } } @@ -1476,9 +1433,6 @@ static void pam_forwarder_lookup_by_cert_done(struct tevent_req *req) } ret = pam_check_user_search(preq); - if (ret == EOK) { - pam_dom_forwarder(preq); - } done: pam_check_user_done(preq, ret); @@ -1538,11 +1492,6 @@ static void pam_forwarder_cb(struct tevent_req *req) } ret = pam_check_user_search(preq); - if (ret == EOK) { - pam_dom_forwarder(preq); - } else if (ret == ENOENT) { - ret = pam_cmd_assume_upn(preq); - } done: pam_check_user_done(preq, ret); @@ -1551,233 +1500,69 @@ static void pam_forwarder_cb(struct tevent_req *req) static void pam_dp_send_acct_req_done(struct tevent_req *req); static int pam_check_user_search(struct pam_auth_req *preq) { - struct sss_domain_info *dom = preq->domain; - char *name = NULL; - time_t cacheExpire; - int ret; struct tevent_req *dpreq; - struct dp_callback_ctx *cb_ctx; - struct pam_ctx *pctx = - talloc_get_type(preq->cctx->rctx->pvt_ctx, struct pam_ctx); - static const char *user_attrs[] = SYSDB_PW_ATTRS; - struct ldb_message *msg; - struct ldb_result *res; - const char *sysdb_name; - - while (dom) { - /* if it is a domainless search, skip domains that require fully - * qualified names instead */ - while (dom && !preq->pd->domain && !preq->pd->name_is_upn - && dom->fqnames) { - dom = get_next_domain(dom, 0); - } - - if (!dom) break; - - if (dom != preq->domain) { - /* make sure we reset the check_provider flag when we check - * a new domain */ - preq->check_provider = NEED_CHECK_PROVIDER(dom->provider); - } - - /* make sure to update the preq if we changed domain */ - preq->domain = dom; - - talloc_free(name); - - name = sss_resp_create_fqname(preq, pctx->rctx, dom, - preq->pd->name_is_upn, - preq->pd->user); - if (name == NULL) { - return ENOMEM; - } - - /* Refresh the user's cache entry on any PAM query - * We put a timeout in the client context so that we limit - * the number of updates within a reasonable timeout - */ - if (preq->check_provider) { - ret = pam_initgr_check_timeout(pctx->id_table, - preq->pd->logon_name); - if (ret != EOK - && ret != ENOENT) { - DEBUG(SSSDBG_OP_FAILURE, - "Could not look up initgroup timout\n"); - return EIO; - } else if (ret == ENOENT) { - /* Call provider first */ - break; - } - /* Entry is still valid, get it from the sysdb */ - } - - DEBUG(SSSDBG_CONF_SETTINGS, "Requesting info for [%s]\n", name); - - if (dom->sysdb == NULL) { - DEBUG(SSSDBG_FATAL_FAILURE, - "Fatal: Sysdb CTX not found for this domain!\n"); - preq->pd->pam_status = PAM_SYSTEM_ERR; - return EFAULT; - } - - if (preq->pd->name_is_upn) { - ret = sysdb_search_user_by_upn(preq, dom, name, user_attrs, &msg); - if (ret == EOK) { - /* Since sysdb_search_user_by_upn() searches the whole cache we - * have to set the domain so that it matches the result. */ - sysdb_name = ldb_msg_find_attr_as_string(msg, - SYSDB_NAME, NULL); - if (sysdb_name == NULL) { - DEBUG(SSSDBG_CRIT_FAILURE, "Cached entry has no name.\n"); - return EINVAL; - } - preq->domain = find_domain_by_object_name( - get_domains_head(dom), - sysdb_name); - if (preq->domain == NULL) { - DEBUG(SSSDBG_CRIT_FAILURE, - "Cannot find matching domain for [%s].\n", - sysdb_name); - return EINVAL; - } - } - } else { - ret = sysdb_getpwnam_with_views(preq, dom, name, &res); - if (res->count > 1) { - DEBUG(SSSDBG_FATAL_FAILURE, - "getpwnam call returned more than one result !?!\n"); - sss_log(SSS_LOG_ERR, - "More users have the same name [%s@%s] in SSSD cache. " - "SSSD will not work correctly.\n", - name, dom->name); - return ENOENT; - } else if (res->count == 0) { - ret = ENOENT; - } else { - msg = res->msgs[0]; - } - } - if (ret != EOK && ret != ENOENT) { - DEBUG(SSSDBG_CRIT_FAILURE, - "Failed to make request to our cache!\n"); - return EIO; - } - - if (ret == ENOENT) { - if (preq->check_provider == false) { - /* set negative cache only if not result of cache check */ - ret = sss_ncache_set_user(pctx->rctx->ncache, - false, dom, preq->pd->user); - if (ret != EOK) { - /* Should not be fatal, just slower next time */ - DEBUG(SSSDBG_MINOR_FAILURE, - "Cannot set ncache for [%s@%s]\n", name, - dom->name); - } - } - - /* if a multidomain search, try with next */ - if (!preq->pd->domain) { - dom = get_next_domain(dom, 0); - continue; - } - - DEBUG(SSSDBG_OP_FAILURE, "No results for getpwnam call\n"); - - /* TODO: store negative cache ? */ - - return ENOENT; - } - - /* One result found */ - - /* if we need to check the remote account go on */ - if (preq->check_provider) { - cacheExpire = ldb_msg_find_attr_as_uint64(msg, - SYSDB_CACHE_EXPIRE, 0); - if (cacheExpire < time(NULL)) { - break; - } - } - - DEBUG(SSSDBG_TRACE_FUNC, - "Returning info for user [%s@%s]\n", name, dom->name); - - /* We might have searched by alias. Pass on the primary name */ - ret = pd_set_primary_name(msg, preq->pd); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, "Could not canonicalize username\n"); - return ret; - } - return EOK; - } - - if (!dom) { - /* Ensure that we don't try to check a provider without a domain, - * since this will cause a NULL-dereference below. - */ - preq->check_provider = false; + dpreq = cache_req_user_by_name_send(preq, + preq->cctx->rctx->ev, + preq->cctx->rctx, + preq->cctx->rctx->ncache, + 0, + preq->pd->domain, + preq->pd->user); + if (dpreq == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Out of memory sending data provider request\n"); + return ENOMEM; } - if (preq->check_provider) { - - /* dont loop forever :-) */ - preq->check_provider = false; - - dpreq = sss_dp_get_account_send(preq, preq->cctx->rctx, - dom, false, SSS_DP_INITGROUPS, name, 0, - preq->pd->name_is_upn ? EXTRA_NAME_IS_UPN : NULL); - if (!dpreq) { - DEBUG(SSSDBG_CRIT_FAILURE, - "Out of memory sending data provider request\n"); - return ENOMEM; - } - - cb_ctx = talloc_zero(preq, struct dp_callback_ctx); - if(!cb_ctx) { - talloc_zfree(dpreq); - return ENOMEM; - } - - cb_ctx->callback = pam_check_user_dp_callback; - cb_ctx->ptr = preq; - cb_ctx->cctx = preq->cctx; - cb_ctx->mem_ctx = preq; - - tevent_req_set_callback(dpreq, pam_dp_send_acct_req_done, cb_ctx); - - /* tell caller we are in an async call */ - return EAGAIN; - } + tevent_req_set_callback(dpreq, pam_dp_send_acct_req_done, preq); - DEBUG(SSSDBG_MINOR_FAILURE, - "No matching domain found for [%s], fail!\n", preq->pd->user); - return ENOENT; + return EOK; } static void pam_dp_send_acct_req_done(struct tevent_req *req) { - struct dp_callback_ctx *cb_ctx = - tevent_req_callback_data(req, struct dp_callback_ctx); + struct cache_req_result *result; + struct pam_auth_req *preq; + struct pam_ctx *pctx; + int ret; - errno_t ret; - dbus_uint16_t err_maj; - dbus_uint32_t err_min; - char *err_msg; + preq = tevent_req_callback_data(req, struct pam_auth_req); + pctx = talloc_get_type(preq->cctx->rctx->pvt_ctx, struct pam_ctx); - ret = sss_dp_get_account_recv(cb_ctx->mem_ctx, req, - &err_maj, &err_min, - &err_msg); + ret = cache_req_user_by_name_recv(preq, req, &result); talloc_zfree(req); - if (ret != EOK) { + if (ret != EOK && ret != ENOENT) { DEBUG(SSSDBG_CRIT_FAILURE, "Fatal error, killing connection!\n"); - talloc_free(cb_ctx->cctx); return; } - cb_ctx->callback(err_maj, err_min, err_msg, cb_ctx->ptr); + preq->domain = result->domain; + + if (ret == EOK) { + pd_set_primary_name(result->msgs[0], preq->pd); + + ret = pam_initgr_cache_set(pctx->rctx->ev, pctx->id_table, + preq->pd->logon_name, pctx->id_timeout); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "Could not save initgr timestamp. " + "Proceeding with PAM actions\n"); + /* This is non-fatal, we'll just end up going to the + * data provider again next time. + */ + } + + pam_dom_forwarder(preq); + } + + ret = pam_check_user_done(preq, ret); + + if (ret) { + preq->pd->pam_status = PAM_SYSTEM_ERR; + pam_reply(preq); + } } static int pam_check_user_done(struct pam_auth_req *preq, int ret) @@ -1809,48 +1594,6 @@ static int pam_check_user_done(struct pam_auth_req *preq, int ret) return EOK; } -static void pam_check_user_dp_callback(uint16_t err_maj, uint32_t err_min, - const char *err_msg, void *ptr) -{ - struct pam_auth_req *preq = talloc_get_type(ptr, struct pam_auth_req); - int ret; - struct pam_ctx *pctx = - talloc_get_type(preq->cctx->rctx->pvt_ctx, struct pam_ctx); - - if (err_maj) { - DEBUG(SSSDBG_OP_FAILURE, - "Unable to get information from Data Provider\n" - "Error: %u, %u, %s\n", - (unsigned int)err_maj, (unsigned int)err_min, err_msg); - } - - ret = pam_check_user_search(preq); - if (ret == EOK) { - /* Make sure we don't go to the ID provider too often */ - ret = pam_initgr_cache_set(pctx->rctx->ev, pctx->id_table, - preq->pd->logon_name, pctx->id_timeout); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, - "Could not save initgr timestamp. " - "Proceeding with PAM actions\n"); - /* This is non-fatal, we'll just end up going to the - * data provider again next time. - */ - } - - pam_dom_forwarder(preq); - } else if (ret == ENOENT) { - ret = pam_cmd_assume_upn(preq); - } - - ret = pam_check_user_done(preq, ret); - - if (ret) { - preq->pd->pam_status = PAM_SYSTEM_ERR; - pam_reply(preq); - } -} - static errno_t pam_is_last_online_login_fresh(struct sss_domain_info *domain, const char* user, int cached_auth_timeout,
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
