diff --git a/0007-HBAC-Do-not-rely-on-originalMemberOf-use-the-sysdb-m.patch b/0007-HBAC-Do-not-rely-on-originalMemberOf-use-the-sysdb-m.patch new file mode 100644 index 0000000..b5e7ca4 --- /dev/null +++ b/0007-HBAC-Do-not-rely-on-originalMemberOf-use-the-sysdb-m.patch @@ -0,0 +1,174 @@ +From c92e49144978ad3b6c9fffa8803ebdad8f6f5b18 Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Sun, 9 Apr 2017 20:50:47 +0200 +Subject: [PATCH] HBAC: Do not rely on originalMemberOf, use the sysdb memberof + links instead + +The IPA HBAC code used to read the group members from the +originalMemberOf attribute value for performance reasons. However, +especially on IPA clients trusting an AD domain, the originalMemberOf +attribute value is often not synchronized correctly. + +Instead of going through the work of maintaining both member/memberOf +and originalMemberOf, let's just do an ASQ search for the group names of +the groups the user is a member of in the cache and read their +SYSBD_NAME attribute. + +To avoid clashing between similarly-named groups in IPA and in AD, we +look at the container of the group. + +Resolves: +https://pagure.io/SSSD/sssd/issue/3382 + +Reviewed-by: Sumit Bose +--- + src/providers/ipa/ipa_hbac_common.c | 97 +++++++++++++++++++++++++------------ + 1 file changed, 67 insertions(+), 30 deletions(-) + +diff --git a/src/providers/ipa/ipa_hbac_common.c b/src/providers/ipa/ipa_hbac_common.c +index b99b75d32..ba677965a 100644 +--- a/src/providers/ipa/ipa_hbac_common.c ++++ b/src/providers/ipa/ipa_hbac_common.c +@@ -507,15 +507,15 @@ hbac_eval_user_element(TALLOC_CTX *mem_ctx, + struct hbac_request_element **user_element) + { + errno_t ret; +- unsigned int i; + unsigned int num_groups = 0; + TALLOC_CTX *tmp_ctx; +- const char *member_dn; + struct hbac_request_element *users; +- struct ldb_message *msg; +- struct ldb_message_element *el; +- const char *attrs[] = { SYSDB_ORIG_MEMBEROF, NULL }; + char *shortname; ++ const char *fqgroupname = NULL; ++ struct sss_domain_info *ipa_domain; ++ struct ldb_dn *ipa_groups_basedn; ++ struct ldb_result *res; ++ int exp_comp; + + tmp_ctx = talloc_new(mem_ctx); + if (tmp_ctx == NULL) return ENOMEM; +@@ -533,56 +533,93 @@ hbac_eval_user_element(TALLOC_CTX *mem_ctx, + } + users->name = talloc_steal(users, shortname); + +- /* Read the originalMemberOf attribute +- * This will give us the list of both POSIX and +- * non-POSIX groups that this user belongs to. ++ ipa_domain = get_domains_head(domain); ++ if (ipa_domain == NULL) { ++ ret = EINVAL; ++ goto done; ++ } ++ ++ ipa_groups_basedn = ldb_dn_new_fmt(tmp_ctx, sysdb_ctx_get_ldb(domain->sysdb), ++ SYSDB_TMPL_GROUP_BASE, ipa_domain->name); ++ if (ipa_groups_basedn == NULL) { ++ ret = ENOMEM; ++ goto done; ++ } ++ ++ /* +1 because there will be a RDN preceding the base DN */ ++ exp_comp = ldb_dn_get_comp_num(ipa_groups_basedn) + 1; ++ ++ /* ++ * Get all the groups the user is a member of. ++ * This includes both POSIX and non-POSIX groups. + */ +- ret = sysdb_search_user_by_name(tmp_ctx, domain, username, +- attrs, &msg); ++ ret = sysdb_initgroups(tmp_ctx, domain, username, &res); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, +- "Could not determine user memberships for [%s]\n", +- users->name); ++ "sysdb_asq_search failed [%d]: %s\n", ret, sss_strerror(ret)); + goto done; + } + +- el = ldb_msg_find_element(msg, SYSDB_ORIG_MEMBEROF); +- if (el == NULL || el->num_values == 0) { ++ if (res->count == 0) { ++ /* This should not happen at this point */ ++ DEBUG(SSSDBG_MINOR_FAILURE, ++ "User [%s] not found in cache.\n", username); ++ ret = ENOENT; ++ goto done; ++ } else if (res->count == 1) { ++ /* The first item is the user entry */ + DEBUG(SSSDBG_TRACE_LIBS, "No groups for [%s]\n", users->name); + ret = create_empty_grouplist(users); + goto done; + } + DEBUG(SSSDBG_TRACE_LIBS, +- "[%d] groups for [%s]\n", el->num_values, users->name); ++ "[%u] groups for [%s]\n", res->count - 1, username); + +- users->groups = talloc_array(users, const char *, el->num_values + 1); ++ /* This also includes the sentinel, b/c we'll skip the user entry below */ ++ users->groups = talloc_array(users, const char *, res->count); + if (users->groups == NULL) { + ret = ENOMEM; + goto done; + } + +- for (i = 0; i < el->num_values; i++) { +- member_dn = (const char *)el->values[i].data; ++ /* Start counting from 1 to exclude the user entry */ ++ for (size_t i = 1; i < res->count; i++) { ++ /* Only groups from the IPA domain can be referenced from HBAC rules. To ++ * avoid evaluating groups which might even have the same name, but come ++ * from a trusted domain, we first copy the DN to a temporary one.. ++ */ ++ if (ldb_dn_get_comp_num(res->msgs[i]->dn) != exp_comp ++ || ldb_dn_compare_base(ipa_groups_basedn, ++ res->msgs[i]->dn) != 0) { ++ DEBUG(SSSDBG_FUNC_DATA, ++ "Skipping non-IPA group %s\n", ++ ldb_dn_get_linearized(res->msgs[i]->dn)); ++ continue; ++ } + +- ret = get_ipa_groupname(users->groups, domain->sysdb, member_dn, +- &users->groups[num_groups]); +- if (ret != EOK && ret != ERR_UNEXPECTED_ENTRY_TYPE) { ++ fqgroupname = ldb_msg_find_attr_as_string(res->msgs[i], SYSDB_NAME, NULL); ++ if (fqgroupname == NULL) { + DEBUG(SSSDBG_MINOR_FAILURE, +- "Skipping malformed entry [%s]\n", member_dn); ++ "Skipping malformed entry [%s]\n", ++ ldb_dn_get_linearized(res->msgs[i]->dn)); + continue; +- } else if (ret == EOK) { +- DEBUG(SSSDBG_TRACE_LIBS, "Added group [%s] for user [%s]\n", +- users->groups[num_groups], users->name); +- num_groups++; ++ } ++ ++ ret = sss_parse_internal_fqname(tmp_ctx, fqgroupname, ++ &shortname, NULL); ++ if (ret != EOK) { ++ DEBUG(SSSDBG_MINOR_FAILURE, "Malformed name %s, skipping!\n", fqgroupname); + continue; + } +- /* Skip entries that are not groups */ +- DEBUG(SSSDBG_TRACE_INTERNAL, +- "Skipping non-group memberOf [%s]\n", member_dn); ++ ++ users->groups[num_groups] = talloc_steal(users->groups, shortname); ++ DEBUG(SSSDBG_TRACE_LIBS, "Added group [%s] for user [%s]\n", ++ users->groups[num_groups], users->name); ++ num_groups++; + } + users->groups[num_groups] = NULL; + +- if (num_groups < el->num_values) { ++ if (num_groups < (res->count - 1)) { + /* Shrink the array memory */ + users->groups = talloc_realloc(users, users->groups, const char *, + num_groups+1); +-- +2.13.0 + diff --git a/sssd.spec b/sssd.spec index 4794001..f0e35ca 100644 --- a/sssd.spec +++ b/sssd.spec @@ -30,7 +30,7 @@ Name: sssd Version: 1.15.2 -Release: 3%{?dist} +Release: 5%{?dist} Group: Applications/System Summary: System Security Services Daemon License: GPLv3+ @@ -45,6 +45,7 @@ Patch0003: 0003-krb5-return-to-responder-that-pkinit-is-not-availabl.patch Patch0004: 0004-ssh-tools-The-ai-structure-is-not-an-array.patch Patch0005: 0005-ssh-tools-Fix-issues-with-multiple-IP-addresses.patch Patch0006: 0006-ssh-tools-Split-connect-and-communication-phases.patch +Patch0007: 0007-HBAC-Do-not-rely-on-originalMemberOf-use-the-sysdb-m.patch Patch0502: 0502-SYSTEMD-Use-capabilities.patch @@ -1169,6 +1170,10 @@ fi %{_libdir}/%{name}/modules/libwbclient.so %changelog +* Wed May 31 2017 Lukas Slebodnik - 1.15.2-5 +- Resolves upstream#3382 - SSSD should use memberOf, not originalMemberOf to + evaluate group membership for HBAC rules + * Sat Apr 29 2017 Lukas Slebodnik - 1.15.2-3 - Resolves: rhbz#1445680 - Properly fall back to local Smartcard authentication - Resolves: rhbz#1437199 - sssd-nfs-idmap-1.15.2-1.fc25.x86_64 conflicts with