Hi, these two patches add error handling to the code. The first prevents operating on ldb_message if retrieving the message fails. I only tested an SSH login with a UPN to make sure we actually hit this codepath. I don't like the deep indendation nesting, so I welcome suggestions how to fix the code better.
The second just checks a return value.
>From 1e67ab6596ac73d12c97abc5feebe2ee1fca6a3f Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Wed, 3 Aug 2016 17:43:14 +0200 Subject: [PATCH 1/2] PAM: Do not act on ldb_message in case of a failure --- src/responder/pam/pamsrv_cmd.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c index 66564f5d301a53dcdb5967f43ef4afdb897e9974..91ac3673d8965018ceac033c2bfe86edeeef17ea 100644 --- a/src/responder/pam/pamsrv_cmd.c +++ b/src/responder/pam/pamsrv_cmd.c @@ -1534,21 +1534,24 @@ static int pam_check_user_search(struct pam_auth_req *preq) if (preq->pd->name_is_upn) { ret = sysdb_search_user_by_upn(preq, dom, name, user_attrs, &msg); - - /* 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; + 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); -- 2.4.11
>From a27529da7c03a27f8d87c624e4b6660fd5b22edf Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Wed, 3 Aug 2016 18:03:59 +0200 Subject: [PATCH 2/2] IPA: Check the return value of sss_parse_internal_fqname --- src/providers/ipa/ipa_subdomains_id.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/providers/ipa/ipa_subdomains_id.c b/src/providers/ipa/ipa_subdomains_id.c index 76fdaa8a1213069bd6b45e0b69b6cdb0d034d721..886813dc648f04c8fadd234524fce94455f31ee4 100644 --- a/src/providers/ipa/ipa_subdomains_id.c +++ b/src/providers/ipa/ipa_subdomains_id.c @@ -509,6 +509,12 @@ static void ipa_get_subdom_acct_connected(struct tevent_req *subreq) } else { ret = sss_parse_internal_fqname(req_input, state->filter, &shortname, NULL); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Cannot parse internal name [%s]: %d\n", + state->filter, ret); + } + req_input->inp.name = talloc_steal(req_input, shortname); } if (req_input->inp.name == NULL) { -- 2.4.11
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org