diff --git a/SOURCES/0163-UTIL-Store-UPN-suffixes-when-creating-a-new-subdomai.patch b/SOURCES/0163-UTIL-Store-UPN-suffixes-when-creating-a-new-subdomai.patch new file mode 100644 index 0000000..9418081 --- /dev/null +++ b/SOURCES/0163-UTIL-Store-UPN-suffixes-when-creating-a-new-subdomai.patch @@ -0,0 +1,138 @@ +From c860682bca53bbafe34b6c22ba151faf18ad2ace Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Thu, 2 Mar 2017 13:52:54 +0100 +Subject: [PATCH 163/163] UTIL: Store UPN suffixes when creating a new + subdomain + +We used to store UPN suffixes pointer into the domain structure only if +the domain changed, not when a new domain was created. As an effect, the +enterprise principals flag was not enabled unless a domain changed, +preventing logins with enterprise principals. + +Reviewed-by: Sumit Bose +(cherry picked from commit 8718ff9ccd29f6431bfa8630bfa3576b2692c9ee) +--- + src/db/sysdb_private.h | 1 + + src/db/sysdb_subdomains.c | 11 ++++++++++- + src/tests/cmocka/test_fqnames.c | 2 +- + src/tests/cmocka/test_nss_srv.c | 2 +- + src/tests/sysdb-tests.c | 8 ++++---- + 5 files changed, 17 insertions(+), 7 deletions(-) + +diff --git a/src/db/sysdb_private.h b/src/db/sysdb_private.h +index b6bf3706e6b9e49d8dd4984f3334b317d17ed9bf..bfd24799950ab3b31d57df11b8f91c0b2572f13a 100644 +--- a/src/db/sysdb_private.h ++++ b/src/db/sysdb_private.h +@@ -190,6 +190,7 @@ struct sss_domain_info *new_subdomain(TALLOC_CTX *mem_ctx, + bool mpg, + bool enumerate, + const char *forest, ++ const char **upn_suffixes, + uint32_t trust_direction); + + /* Helper functions to deal with the timestamp cache should not be used +diff --git a/src/db/sysdb_subdomains.c b/src/db/sysdb_subdomains.c +index 780140484f6f023bc6e8c12266e3b81ff016ec10..4f326405f955abd462f892e6013a8c24764afd55 100644 +--- a/src/db/sysdb_subdomains.c ++++ b/src/db/sysdb_subdomains.c +@@ -32,6 +32,7 @@ struct sss_domain_info *new_subdomain(TALLOC_CTX *mem_ctx, + bool mpg, + bool enumerate, + const char *forest, ++ const char **upn_suffixes, + uint32_t trust_direction) + { + struct sss_domain_info *dom; +@@ -108,6 +109,14 @@ struct sss_domain_info *new_subdomain(TALLOC_CTX *mem_ctx, + } + } + ++ if (upn_suffixes != NULL) { ++ dom->upn_suffixes = dup_string_list(dom, upn_suffixes); ++ if (dom->upn_suffixes == NULL) { ++ DEBUG(SSSDBG_OP_FAILURE, "Failed to copy UPN upn_suffixes.\n"); ++ goto fail; ++ } ++ } ++ + dom->enumerate = enumerate; + dom->fqnames = true; + dom->mpg = mpg; +@@ -442,7 +451,7 @@ errno_t sysdb_update_subdomains(struct sss_domain_info *domain) + if (dom == NULL) { + dom = new_subdomain(domain, domain, name, realm, + flat, id, mpg, enumerate, forest, +- trust_direction); ++ upn_suffixes, trust_direction); + if (dom == NULL) { + ret = ENOMEM; + goto done; +diff --git a/src/tests/cmocka/test_fqnames.c b/src/tests/cmocka/test_fqnames.c +index f4cdd80ef94584fe4eb1f0578bf388da3ead824c..19788248a39774bb4509363145ac4ce0815b7d28 100644 +--- a/src/tests/cmocka/test_fqnames.c ++++ b/src/tests/cmocka/test_fqnames.c +@@ -309,7 +309,7 @@ static int parse_name_test_setup(void **state) + * discovered + */ + test_ctx->subdom = new_subdomain(dom, dom, SUBDOMNAME, NULL, SUBFLATNAME, +- NULL, false, false, NULL, 0); ++ NULL, false, false, NULL, NULL, 0); + assert_non_null(test_ctx->subdom); + + check_leaks_push(test_ctx); +diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c +index 41425e76f3b76fafa917f33fcfef0946f2f71c7d..5eee82d78f4e4ab4dcdc0dcdfb24c2e7d017acf5 100644 +--- a/src/tests/cmocka/test_nss_srv.c ++++ b/src/tests/cmocka/test_nss_srv.c +@@ -3084,7 +3084,7 @@ static int nss_subdom_test_setup(void **state) + + subdomain = new_subdomain(nss_test_ctx, nss_test_ctx->tctx->dom, + testdom[0], testdom[1], testdom[2], testdom[3], +- false, false, NULL, 0); ++ false, false, NULL, NULL, 0); + assert_non_null(subdomain); + + ret = sysdb_subdomain_store(nss_test_ctx->tctx->sysdb, +diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c +index d1450015cb0f0b073045e7b6031423e3f5494d78..6fd1988668124dc2dc922b41d3f7387c6d00c486 100644 +--- a/src/tests/sysdb-tests.c ++++ b/src/tests/sysdb-tests.c +@@ -1395,7 +1395,7 @@ START_TEST (test_sysdb_get_user_attr_subdomain) + /* Create subdomain */ + subdomain = new_subdomain(test_ctx, test_ctx->domain, + "test.sub", "TEST.SUB", "test", "S-3", +- false, false, NULL, 0); ++ false, false, NULL, NULL, 0); + fail_if(subdomain == NULL, "Failed to create new subdomain."); + + ret = sss_names_init_from_args(test_ctx, +@@ -5468,7 +5468,7 @@ START_TEST(test_sysdb_subdomain_store_user) + + subdomain = new_subdomain(test_ctx, test_ctx->domain, + testdom[0], testdom[1], testdom[2], testdom[3], +- false, false, NULL, 0); ++ false, false, NULL, NULL, 0); + fail_unless(subdomain != NULL, "Failed to create new subdomin."); + ret = sysdb_subdomain_store(test_ctx->sysdb, + testdom[0], testdom[1], testdom[2], testdom[3], +@@ -5547,7 +5547,7 @@ START_TEST(test_sysdb_subdomain_user_ops) + + subdomain = new_subdomain(test_ctx, test_ctx->domain, + testdom[0], testdom[1], testdom[2], testdom[3], +- false, false, NULL, 0); ++ false, false, NULL, NULL, 0); + fail_unless(subdomain != NULL, "Failed to create new subdomin."); + ret = sysdb_subdomain_store(test_ctx->sysdb, + testdom[0], testdom[1], testdom[2], testdom[3], +@@ -5620,7 +5620,7 @@ START_TEST(test_sysdb_subdomain_group_ops) + + subdomain = new_subdomain(test_ctx, test_ctx->domain, + testdom[0], testdom[1], testdom[2], testdom[3], +- false, false, NULL, 0); ++ false, false, NULL, NULL, 0); + fail_unless(subdomain != NULL, "Failed to create new subdomin."); + ret = sysdb_subdomain_store(test_ctx->sysdb, + testdom[0], testdom[1], testdom[2], testdom[3], +-- +2.9.3 + diff --git a/SOURCES/0164-Move-sized_output_name-and-sized_domain_name-into-re.patch b/SOURCES/0164-Move-sized_output_name-and-sized_domain_name-into-re.patch new file mode 100644 index 0000000..dd5e219 --- /dev/null +++ b/SOURCES/0164-Move-sized_output_name-and-sized_domain_name-into-re.patch @@ -0,0 +1,272 @@ +From ed3bdd8998c5e0e18f9b8cfefa9acd00b8531585 Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Wed, 19 Apr 2017 17:44:40 +0200 +Subject: [PATCH 164/165] Move sized_output_name() and sized_domain_name() into + responder common code +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +These functions are used to format a name into a format that the user +configured for output, including case sensitiveness, replacing +whitespace and qualified format. They were used only in the NSS +responder, which typically returns strings to the NSS client library and +then the user. + +But it makes sense to just reuse the same code in the IFP responder as +well, since it does essentially the same job. + +The patch also renames sized_member_name to sized_domain_name. +Previously, the function was only used to format a group member, the IFP +responder would use the same function to format a group the user is a +member of. + +Related to: + https://pagure.io/SSSD/sssd/issue/3268 + +Reviewed-by: Pavel Březina +(cherry picked from commit 7c074ba2f923985ab0d4f9d6a5e01ff3f2f0a7a8) +--- + src/responder/common/responder.h | 21 ++++++++ + src/responder/common/responder_common.c | 90 +++++++++++++++++++++++++++++++++ + src/responder/nss/nsssrv_cmd.c | 89 +------------------------------- + 3 files changed, 112 insertions(+), 88 deletions(-) + +diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h +index 9e3b2fdbda4e30b859df597374fc7d490b1720e5..fd6a67ba72f28f52d6cc1bbad16e1a7245462c93 100644 +--- a/src/responder/common/responder.h ++++ b/src/responder/common/responder.h +@@ -358,4 +358,25 @@ char *sss_resp_create_fqname(TALLOC_CTX *mem_ctx, + bool name_is_upn, + const char *orig_name); + ++/** ++ * Helper functions to format output names ++ */ ++ ++/* Format orig_name into a sized_string in output format as prescribed ++ * by the name_dom domain ++ */ ++int sized_output_name(TALLOC_CTX *mem_ctx, ++ struct resp_ctx *rctx, ++ const char *orig_name, ++ struct sss_domain_info *name_dom, ++ struct sized_string **_name); ++ ++/* Format orig_name into a sized_string in output format as prescribed ++ * by the domain read from the fully qualified name. ++ */ ++int sized_domain_name(TALLOC_CTX *mem_ctx, ++ struct resp_ctx *rctx, ++ const char *member_name, ++ struct sized_string **_name); ++ + #endif /* __SSS_RESPONDER_H__ */ +diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c +index c604c64a652221521ec7114b8588186f087eb11a..1db8a2283f3e96bccebe9a8443d5d04b9d5f4a54 100644 +--- a/src/responder/common/responder_common.c ++++ b/src/responder/common/responder_common.c +@@ -1248,3 +1248,93 @@ void responder_set_fd_limit(rlim_t fd_limit) + "Proceeding with system values\n"); + } + } ++ ++/** ++ * Helper functions to format output names ++ */ ++int sized_output_name(TALLOC_CTX *mem_ctx, ++ struct resp_ctx *rctx, ++ const char *orig_name, ++ struct sss_domain_info *name_dom, ++ struct sized_string **_name) ++{ ++ TALLOC_CTX *tmp_ctx = NULL; ++ errno_t ret; ++ char *username; ++ struct sized_string *name; ++ ++ tmp_ctx = talloc_new(NULL); ++ if (tmp_ctx == NULL) { ++ return ENOMEM; ++ } ++ ++ username = sss_output_name(tmp_ctx, orig_name, name_dom->case_preserve, ++ rctx->override_space); ++ if (username == NULL) { ++ ret = EIO; ++ goto done; ++ } ++ ++ if (name_dom->fqnames) { ++ username = sss_tc_fqname(tmp_ctx, name_dom->names, name_dom, username); ++ if (username == NULL) { ++ DEBUG(SSSDBG_CRIT_FAILURE, "sss_replace_space failed\n"); ++ ret = EIO; ++ goto done; ++ } ++ } ++ ++ name = talloc_zero(tmp_ctx, struct sized_string); ++ if (name == NULL) { ++ ret = ENOMEM; ++ goto done; ++ } ++ ++ to_sized_string(name, username); ++ name->str = talloc_steal(name, username); ++ *_name = talloc_steal(mem_ctx, name); ++ ret = EOK; ++done: ++ talloc_zfree(tmp_ctx); ++ return ret; ++} ++ ++int sized_domain_name(TALLOC_CTX *mem_ctx, ++ struct resp_ctx *rctx, ++ const char *member_name, ++ struct sized_string **_name) ++{ ++ TALLOC_CTX *tmp_ctx = NULL; ++ errno_t ret; ++ char *domname; ++ struct sss_domain_info *member_dom; ++ ++ tmp_ctx = talloc_new(NULL); ++ if (tmp_ctx == NULL) { ++ return ENOMEM; ++ } ++ ++ ret = sss_parse_internal_fqname(tmp_ctx, member_name, NULL, &domname); ++ if (ret != EOK) { ++ DEBUG(SSSDBG_CRIT_FAILURE, "sss_parse_internal_fqname failed\n"); ++ goto done; ++ } ++ ++ if (domname == NULL) { ++ ret = ERR_WRONG_NAME_FORMAT; ++ goto done; ++ } ++ ++ member_dom = find_domain_by_name(get_domains_head(rctx->domains), ++ domname, true); ++ if (member_dom == NULL) { ++ ret = ERR_DOMAIN_NOT_FOUND; ++ goto done; ++ } ++ ++ ret = sized_output_name(mem_ctx, rctx, member_name, ++ member_dom, _name); ++done: ++ talloc_free(tmp_ctx); ++ return ret; ++} +diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c +index b64cea2a53ec6032904237b0afc1377022c2c804..0a9dd301266d4db1be1977cc8c337abde7cd79f5 100644 +--- a/src/responder/nss/nsssrv_cmd.c ++++ b/src/responder/nss/nsssrv_cmd.c +@@ -253,93 +253,6 @@ static const char *get_shell_override(TALLOC_CTX *mem_ctx, + return talloc_strdup(mem_ctx, NOLOGIN_SHELL); + } + +-static int sized_output_name(TALLOC_CTX *mem_ctx, +- struct resp_ctx *rctx, +- const char *orig_name, +- struct sss_domain_info *name_dom, +- struct sized_string **_name) +-{ +- TALLOC_CTX *tmp_ctx = NULL; +- errno_t ret; +- char *username; +- struct sized_string *name; +- +- tmp_ctx = talloc_new(NULL); +- if (tmp_ctx == NULL) { +- return ENOMEM; +- } +- +- username = sss_output_name(tmp_ctx, orig_name, name_dom->case_preserve, +- rctx->override_space); +- if (username == NULL) { +- ret = EIO; +- goto done; +- } +- +- if (name_dom->fqnames) { +- username = sss_tc_fqname(tmp_ctx, name_dom->names, name_dom, username); +- if (username == NULL) { +- DEBUG(SSSDBG_CRIT_FAILURE, "sss_replace_space failed\n"); +- ret = EIO; +- goto done; +- } +- } +- +- name = talloc_zero(tmp_ctx, struct sized_string); +- if (name == NULL) { +- ret = ENOMEM; +- goto done; +- } +- +- to_sized_string(name, username); +- name->str = talloc_steal(name, username); +- *_name = talloc_steal(mem_ctx, name); +- ret = EOK; +-done: +- talloc_zfree(tmp_ctx); +- return ret; +-} +- +-static int sized_member_name(TALLOC_CTX *mem_ctx, +- struct resp_ctx *rctx, +- const char *member_name, +- struct sized_string **_name) +-{ +- TALLOC_CTX *tmp_ctx = NULL; +- errno_t ret; +- char *domname; +- struct sss_domain_info *member_dom; +- +- tmp_ctx = talloc_new(NULL); +- if (tmp_ctx == NULL) { +- return ENOMEM; +- } +- +- ret = sss_parse_internal_fqname(tmp_ctx, member_name, NULL, &domname); +- if (ret != EOK) { +- DEBUG(SSSDBG_CRIT_FAILURE, "sss_parse_internal_fqname failed\n"); +- goto done; +- } +- +- if (domname == NULL) { +- ret = ERR_WRONG_NAME_FORMAT; +- goto done; +- } +- +- member_dom = find_domain_by_name(get_domains_head(rctx->domains), +- domname, true); +- if (member_dom == NULL) { +- ret = ERR_DOMAIN_NOT_FOUND; +- goto done; +- } +- +- ret = sized_output_name(mem_ctx, rctx, member_name, +- member_dom, _name); +-done: +- talloc_free(tmp_ctx); +- return ret; +-} +- + static int fill_pwent(struct sss_packet *packet, + struct sss_domain_info *dom, + struct nss_ctx *nctx, +@@ -2727,7 +2640,7 @@ static int fill_members(struct sss_packet *packet, + } + } + +- ret = sized_member_name(tmp_ctx, rctx, fqname, &name); ++ ret = sized_domain_name(tmp_ctx, rctx, fqname, &name); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, "sized_member_name failed: %d\n", ret); + goto done; +-- +2.9.3 + diff --git a/SOURCES/0165-IFP-Use-sized_domain_name-to-format-the-groups-the-u.patch b/SOURCES/0165-IFP-Use-sized_domain_name-to-format-the-groups-the-u.patch new file mode 100644 index 0000000..6a83760 --- /dev/null +++ b/SOURCES/0165-IFP-Use-sized_domain_name-to-format-the-groups-the-u.patch @@ -0,0 +1,95 @@ +From e32d3a1074a64a1976047fb9fcd0defac439f97c Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Wed, 19 Apr 2017 17:46:03 +0200 +Subject: [PATCH 165/165] IFP: Use sized_domain_name to format the groups the + user is a member of +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Resolves: + https://pagure.io/SSSD/sssd/issue/3268 + +Uses the common function sized_domain_name() to format a group the user +is a member of to the appropriate format. + +To see the code is working correctly, run: + dbus-send --system --print-reply --dest=org.freedesktop.sssd.infopipe + /org/freedesktop/sssd/infopipe + org.freedesktop.sssd.infopipe.GetUserGroups + string:trusted_user + +Where trusted_user is a user from a trusted domain that is a member of groups +from the joined domain and a trusted domain as well. The groups from the +joined domain should not be qualified, the groups from the trusted +domain should be qualified. + +Reviewed-by: Pavel Březina +(cherry picked from commit c9a73bb6ffa010ef206896a0d1c2801bc056fa45) +--- + src/responder/ifp/ifpsrv_cmd.c | 29 +++++++++++++++-------------- + 1 file changed, 15 insertions(+), 14 deletions(-) + +diff --git a/src/responder/ifp/ifpsrv_cmd.c b/src/responder/ifp/ifpsrv_cmd.c +index 97fad47e90c2155fc7243e68a7110ba9fd28aa39..f14a41da096edd2ace40c6faf8fb6f6dc3b503fe 100644 +--- a/src/responder/ifp/ifpsrv_cmd.c ++++ b/src/responder/ifp/ifpsrv_cmd.c +@@ -369,10 +369,11 @@ ifp_user_get_groups_reply(struct sss_domain_info *domain, + struct ifp_req *ireq, + struct ldb_result *res) + { +- int i, num; ++ int i, gri, num; + const char *name; + const char **groupnames; +- char *out_name; ++ struct sized_string *group_name; ++ errno_t ret; + + /* one less, the first one is the user entry */ + num = res->count - 1; +@@ -381,6 +382,7 @@ ifp_user_get_groups_reply(struct sss_domain_info *domain, + return sbus_request_finish(ireq->dbus_req, NULL); + } + ++ gri = 0; + for (i = 0; i < num; i++) { + name = sss_view_ldb_msg_find_attr_as_string(domain, + res->msgs[i + 1], +@@ -390,22 +392,21 @@ ifp_user_get_groups_reply(struct sss_domain_info *domain, + continue; + } + +- out_name = sss_output_name(ireq, name, domain->case_preserve, +- ireq->ifp_ctx->rctx->override_space); +- if (out_name == NULL) { ++ ret = sized_domain_name(ireq, ireq->ifp_ctx->rctx, name, &group_name); ++ if (ret != EOK) { ++ DEBUG(SSSDBG_MINOR_FAILURE, ++ "Unable to get sized name for %s [%d]: %s\n", ++ name, ret, sss_strerror(ret)); + continue; + } + +- if (domain->fqnames) { +- groupnames[i] = sss_tc_fqname(groupnames, domain->names, +- domain, out_name); +- if (out_name == NULL) { +- DEBUG(SSSDBG_CRIT_FAILURE, "sss_tc_fqname failed\n"); +- continue; +- } +- } else { +- groupnames[i] = talloc_steal(groupnames, out_name); ++ groupnames[gri] = talloc_strndup(groupnames, ++ group_name->str, group_name->len); ++ if (groupnames[gri] == NULL) { ++ DEBUG(SSSDBG_MINOR_FAILURE, "talloc_strndup failed\n"); ++ continue; + } ++ gri++; + + DEBUG(SSSDBG_TRACE_FUNC, "Adding group %s\n", groupnames[i]); + } +-- +2.9.3 + diff --git a/SOURCES/0166-LDAP-AD-Do-not-fail-in-case-rfc2307bis_nested_groups.patch b/SOURCES/0166-LDAP-AD-Do-not-fail-in-case-rfc2307bis_nested_groups.patch new file mode 100644 index 0000000..d04aedf --- /dev/null +++ b/SOURCES/0166-LDAP-AD-Do-not-fail-in-case-rfc2307bis_nested_groups.patch @@ -0,0 +1,49 @@ +From b9460652c3ab86b1b0cfe1e8ea868e6e0bb492ad Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= +Date: Mon, 1 May 2017 14:49:50 +0200 +Subject: [PATCH 166/166] LDAP/AD: Do not fail in case + rfc2307bis_nested_groups_recv() returns ENOENT +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Commit 25699846 introduced a regression seen when an initgroup lookup is +done and there's no nested groups involved. + +In this scenario the whole lookup fails due to an ENOENT returned by +rfc2307bis_nested_groups_recv(), which leads to the user removal from +sysdb causing some authentication issues. + +Resolves: +https://pagure.io/SSSD/sssd/issue/3331 + +Signed-off-by: Fabiano Fidêncio +Reviewed-by: Sumit Bose +(cherry picked from commit df4b24bed15f45bf286fb0102fd397218fdd4186) +(cherry picked from commit 4540d9f6817c78eef7b6e2d79245434811b59ad9) +--- + src/providers/ldap/sdap_async_initgroups_ad.c | 8 +++++++- + 1 file changed, 7 insertions(+), 1 deletion(-) + +diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c b/src/providers/ldap/sdap_async_initgroups_ad.c +index 1fee4ab43a6c13803a088ffa4695dde7f39b3d2b..904cffd820fa1f0aeb86929b41b0d7523f36d315 100644 +--- a/src/providers/ldap/sdap_async_initgroups_ad.c ++++ b/src/providers/ldap/sdap_async_initgroups_ad.c +@@ -1746,7 +1746,13 @@ static void sdap_ad_get_domain_local_groups_done(struct tevent_req *subreq) + + ret = rfc2307bis_nested_groups_recv(subreq); + talloc_zfree(subreq); +- if (ret != EOK) { ++ if (ret == ENOENT) { ++ /* In case of ENOENT we can just proceed without making ++ * sdap_get_initgr_user() fail because there's no nested ++ * groups for this user/group. */ ++ ret = EOK; ++ goto done; ++ } else if (ret != EOK) { + tevent_req_error(req, ret); + return; + } +-- +2.9.3 + diff --git a/SOURCES/0167-HBAC-Do-not-rely-on-originalMemberOf-use-the-sysdb-m.patch b/SOURCES/0167-HBAC-Do-not-rely-on-originalMemberOf-use-the-sysdb-m.patch new file mode 100644 index 0000000..343c654 --- /dev/null +++ b/SOURCES/0167-HBAC-Do-not-rely-on-originalMemberOf-use-the-sysdb-m.patch @@ -0,0 +1,175 @@ +From bef2586e998a3fa2a85475d19e8cf257383b79db Mon Sep 17 00:00:00 2001 +From: Jakub Hrozek +Date: Sun, 9 Apr 2017 20:50:47 +0200 +Subject: [PATCH 167/167] 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 +(cherry picked from commit c92e49144978ad3b6c9fffa8803ebdad8f6f5b18) +--- + 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 b99b75d322930f16412f6abd4cdf0d7e0b59c32c..ba677965a3eb68a54baf99b1875bca2acbb76c99 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.9.4 + diff --git a/SPECS/sssd.spec b/SPECS/sssd.spec index 6d572df..ae74e0a 100644 --- a/SPECS/sssd.spec +++ b/SPECS/sssd.spec @@ -33,7 +33,7 @@ Name: sssd Version: 1.14.0 -Release: 43%{?dist}.14 +Release: 43%{?dist}.18 Group: Applications/System Summary: System Security Services Daemon License: GPLv3+ @@ -203,6 +203,11 @@ Patch0159: 0159-WATCHDOG-define-and-use-_MAX_TICKS-as-3.patch Patch0160: 0160-WATCHDOG-Avoid-non-async-signal-safe-from-the-signal.patch Patch0161: 0161-TESTS-Extending-sysdb-sudo-store-tests.patch Patch0162: 0162-SUDO-Only-store-lowercased-attribute-value-once.patch +Patch0163: 0163-UTIL-Store-UPN-suffixes-when-creating-a-new-subdomai.patch +Patch0164: 0164-Move-sized_output_name-and-sized_domain_name-into-re.patch +Patch0165: 0165-IFP-Use-sized_domain_name-to-format-the-groups-the-u.patch +Patch0166: 0166-LDAP-AD-Do-not-fail-in-case-rfc2307bis_nested_groups.patch +Patch0167: 0167-HBAC-Do-not-rely-on-originalMemberOf-use-the-sysdb-m.patch #This patch should not be removed in RHEL-7 Patch999: 0999-NOUPSTREAM-Default-to-root-if-sssd-user-is-not-spec @@ -1197,6 +1202,22 @@ fi /usr/bin/rm -f /var/tmp/sssd.upgrade || : %changelog +* Fri May 26 2017 Jakub Hrozek - 1.14.0-43.18 +- Resolves: rhbz#1456013 - sssd intermittently failing to resolve groups + for an AD user in IPA-AD trust environment. + +* Fri May 12 2017 Jakub Hrozek - 1.14.0-43.17 +- Resolves: rhbz#1450125 - Wrong pam return code for user from subdomain + with ad_access_filter + +* Tue May 2 2017 Jakub Hrozek - 1.14.0-43.16 +- Resolves: rhbz#1446085 - D-Bus interface of sssd is giving inappropriate + group information for trusted AD users + +* Wed Apr 26 2017 Jakub Hrozek - 1.14.0-43.15 +- Resolves: rhbz#1445821 - sssd does not evaluate AD UPN suffixes which + results in failed user logins + * Wed Feb 15 2017 Jakub Hrozek - 1.14.0-43.14 - Resolves: rhbz#1422183 - Fails to accept any sudo rules if there are two user entries in an ldap role with the same