From 282bcc8d49b5387cb06becc98d49525b0a4d39ef Mon Sep 17 00:00:00 2001 From: Alexey Tikhonov Date: Thu, 14 Aug 2025 12:58:05 +0200 Subject: [PATCH] Related: RHEL-87530 - AD user in external group is not cleared when expiring the cache [rhel-9] Patch used to fix this ticket causes a regression (RHEL-106987) and is being reverted. --- ...0001-authtok-add-IS_PW_OR_ST_AUTHTOK.patch | 0 ...ysdb-add-sysdb_get_direct_parents_ex.patch | 151 ----------- ...ndling-of-external-group-memberships.patch | 247 ------------------ ...ne-with-SSS_AUTHTOK_TYPE_PAM_STACKED.patch | 0 sssd.spec | 12 +- 5 files changed, 7 insertions(+), 403 deletions(-) rename 0003-authtok-add-IS_PW_OR_ST_AUTHTOK.patch => 0001-authtok-add-IS_PW_OR_ST_AUTHTOK.patch (100%) delete mode 100644 0001-sysdb-add-sysdb_get_direct_parents_ex.patch delete mode 100644 0002-ipa-improve-handling-of-external-group-memberships.patch rename 0004-krb5-offline-with-SSS_AUTHTOK_TYPE_PAM_STACKED.patch => 0002-krb5-offline-with-SSS_AUTHTOK_TYPE_PAM_STACKED.patch (100%) diff --git a/0003-authtok-add-IS_PW_OR_ST_AUTHTOK.patch b/0001-authtok-add-IS_PW_OR_ST_AUTHTOK.patch similarity index 100% rename from 0003-authtok-add-IS_PW_OR_ST_AUTHTOK.patch rename to 0001-authtok-add-IS_PW_OR_ST_AUTHTOK.patch diff --git a/0001-sysdb-add-sysdb_get_direct_parents_ex.patch b/0001-sysdb-add-sysdb_get_direct_parents_ex.patch deleted file mode 100644 index 7a42337..0000000 --- a/0001-sysdb-add-sysdb_get_direct_parents_ex.patch +++ /dev/null @@ -1,151 +0,0 @@ -From 37f6f1aa65120539104acbf611383fcb263f3020 Mon Sep 17 00:00:00 2001 -From: Sumit Bose -Date: Tue, 6 May 2025 16:41:44 +0200 -Subject: [PATCH 1/2] sysdb: add sysdb_get_direct_parents_ex() -MIME-Version: 1.0 -Content-Type: text/plain; charset=UTF-8 -Content-Transfer-Encoding: 8bit - -sysdb_get_direct_parents_ex() is similar to sysdb_get_direct_parents() -but allows to request a different attribute than SYSDB_NAME be returned -for the found groups. - -Reviewed-by: Alejandro López -Reviewed-by: Alexey Tikhonov -Reviewed-by: Justin Stephenson -(cherry picked from commit 16d61ee1acdeb73d22a756a1c68c3dfc08d9b94d) ---- - src/db/sysdb.h | 33 ++++++++++++++++++++++++++++++++- - src/db/sysdb_search.c | 38 ++++++++++++++++++++++++++++---------- - 2 files changed, 60 insertions(+), 11 deletions(-) - -diff --git a/src/db/sysdb.h b/src/db/sysdb.h -index 009e6a490..2a76f2745 100644 ---- a/src/db/sysdb.h -+++ b/src/db/sysdb.h -@@ -1419,7 +1419,7 @@ errno_t sysdb_remove_attrs(struct sss_domain_info *domain, - char **remove_attrs); - - /** -- * @brief Return direct parents of an object in the cache -+ * @brief Return name of direct parents of an object in the cache - * - * @param[in] mem_ctx Memory context the result should be allocated - * on -@@ -1445,6 +1445,37 @@ errno_t sysdb_get_direct_parents(TALLOC_CTX *mem_ctx, - const char *name, - char ***_direct_parents); - -+/** -+ * @brief Return requested attribute of direct parents of an object in the cache -+ * -+ * @param[in] mem_ctx Memory context the result should be allocated -+ * on -+ * @param[in] dom domain the object is in -+ * @param[in] parent_dom domain which should be searched for direct -+ * parents if NULL all domains in the given cache -+ * are searched -+ * @param[in] mtype Type of the object, SYSDB_MEMBER_USER or -+ * SYSDB_MEMBER_GROUP -+ * @param[in] name Name of the object -+ * @param[in] attr_name Name of the attribute to return, if NULL -+ * SYSDB_NAME will be used -+ * @param[out] _direct_parents List of the requested attribute of the direct -+ * parent groups -+ * -+ * -+ * @return -+ * - EOK: success -+ * - EINVAL: wrong mtype -+ * - ENOMEM: Memory allocation failed -+ */ -+errno_t sysdb_get_direct_parents_ex(TALLOC_CTX *mem_ctx, -+ struct sss_domain_info *dom, -+ struct sss_domain_info *parent_dom, -+ enum sysdb_member_type mtype, -+ const char *name, -+ const char *attr_name, -+ char ***_direct_parents); -+ - /* === Functions related to ID-mapping === */ - - #define SYSDB_IDMAP_CONTAINER "cn=id_mappings" -diff --git a/src/db/sysdb_search.c b/src/db/sysdb_search.c -index 79293a3ee..1c938a58c 100644 ---- a/src/db/sysdb_search.c -+++ b/src/db/sysdb_search.c -@@ -2430,18 +2430,19 @@ done: - return ret; - } - --errno_t sysdb_get_direct_parents(TALLOC_CTX *mem_ctx, -- struct sss_domain_info *dom, -- struct sss_domain_info *parent_dom, -- enum sysdb_member_type mtype, -- const char *name, -- char ***_direct_parents) -+errno_t sysdb_get_direct_parents_ex(TALLOC_CTX *mem_ctx, -+ struct sss_domain_info *dom, -+ struct sss_domain_info *parent_dom, -+ enum sysdb_member_type mtype, -+ const char *name, -+ const char *attr_name, -+ char ***_direct_parents) - { - errno_t ret; - const char *dn; - char *sanitized_dn; - struct ldb_dn *basedn; -- static const char *group_attrs[] = { SYSDB_NAME, NULL }; -+ const char *group_attrs[] = { NULL, NULL }; - const char *member_filter; - size_t direct_sysdb_count = 0; - struct ldb_message **direct_sysdb_groups = NULL; -@@ -2494,6 +2495,11 @@ errno_t sysdb_get_direct_parents(TALLOC_CTX *mem_ctx, - DEBUG(SSSDBG_TRACE_INTERNAL, - "searching sysdb with filter [%s]\n", member_filter); - -+ if (attr_name == NULL) { -+ attr_name = SYSDB_NAME; -+ } -+ group_attrs[0] = attr_name; -+ - ret = sysdb_search_entry(tmp_ctx, dom->sysdb, basedn, - LDB_SCOPE_SUBTREE, member_filter, group_attrs, - &direct_sysdb_count, &direct_sysdb_groups); -@@ -2515,10 +2521,11 @@ errno_t sysdb_get_direct_parents(TALLOC_CTX *mem_ctx, - - pi = 0; - for(i = 0; i < direct_sysdb_count; i++) { -- tmp_str = ldb_msg_find_attr_as_string(direct_sysdb_groups[i], -- SYSDB_NAME, NULL); -+ tmp_str = ldb_msg_find_attr_as_string(direct_sysdb_groups[i], attr_name, -+ NULL); - if (!tmp_str) { -- DEBUG(SSSDBG_CRIT_FAILURE, "A group with no name?\n"); -+ DEBUG(SSSDBG_CRIT_FAILURE, "A group with no attribute [%s]?\n", -+ attr_name); - /* This should never happen, but if it does, just continue */ - continue; - } -@@ -2542,6 +2549,17 @@ done: - return ret; - } - -+errno_t sysdb_get_direct_parents(TALLOC_CTX *mem_ctx, -+ struct sss_domain_info *dom, -+ struct sss_domain_info *parent_dom, -+ enum sysdb_member_type mtype, -+ const char *name, -+ char ***_direct_parents) -+{ -+ return sysdb_get_direct_parents_ex(mem_ctx, dom, parent_dom, mtype, name, -+ NULL, _direct_parents); -+} -+ - errno_t sysdb_get_real_name(TALLOC_CTX *mem_ctx, - struct sss_domain_info *domain, - const char *name_or_upn_or_sid, --- -2.50.0 - diff --git a/0002-ipa-improve-handling-of-external-group-memberships.patch b/0002-ipa-improve-handling-of-external-group-memberships.patch deleted file mode 100644 index ef454ef..0000000 --- a/0002-ipa-improve-handling-of-external-group-memberships.patch +++ /dev/null @@ -1,247 +0,0 @@ -From 844cefe14e5aa97c09159dec419dd14b17bd531e Mon Sep 17 00:00:00 2001 -From: Sumit Bose -Date: Tue, 6 May 2025 16:43:26 +0200 -Subject: [PATCH 2/2] ipa: improve handling of external group memberships -MIME-Version: 1.0 -Content-Type: text/plain; charset=UTF-8 -Content-Transfer-Encoding: 8bit - -Currently add_ad_user_to_cached_groups() expects that all IPA -group-memberships of users from a trusted domain are removed when the -group-memberships from the trusted domain are updated. This is currently -only true for the code path where the tokenGroups request is used. The -code path without tokenGroups does not remove the IPA group-memberships. - -Removing the IPA group-memberships is also not very efficient especially -if there are no changes to those at all. With this patch in -add_ad_user_to_cached_groups() it is checked which group-memberships -have to be added or removed. In this function the SYSDB_ORIG_MEMBEROF -attribute of the user is handled as well for the IPA group-memberships. -Since this attribute is removed in all code paths all IPA -group-memberships are added here again. But instead of doing it one by -one as in the previous version, the attribute is added for all groups in -a single operation which should help to improved the performance as -well. - -Resolves: https://github.com/SSSD/sssd/issues/7921 - -Reviewed-by: Alejandro López -Reviewed-by: Alexey Tikhonov -Reviewed-by: Justin Stephenson -(cherry picked from commit 2a19873c84948a6451da80e6c8d3f689c8228c04) ---- - src/providers/ipa/ipa_subdomains_ext_groups.c | 152 +++++++++++++++--- - 1 file changed, 130 insertions(+), 22 deletions(-) - -diff --git a/src/providers/ipa/ipa_subdomains_ext_groups.c b/src/providers/ipa/ipa_subdomains_ext_groups.c -index ba3fb3953..f86130d89 100644 ---- a/src/providers/ipa/ipa_subdomains_ext_groups.c -+++ b/src/providers/ipa/ipa_subdomains_ext_groups.c -@@ -312,11 +312,19 @@ static errno_t add_ad_user_to_cached_groups(struct ldb_dn *user_dn, - bool *missing_groups) - { - size_t c; -+ size_t d = 0; - struct sysdb_attrs *user_attrs; - size_t msgs_count; - struct ldb_message **msgs; - TALLOC_CTX *tmp_ctx; - int ret; -+ const struct ldb_val *val; -+ char *user_name; -+ char **sysdb_ipa_group_memberships; -+ char **add_groups; -+ char **del_groups; -+ errno_t sret; -+ bool in_transaction = false; - - *missing_groups = false; - -@@ -326,18 +334,96 @@ static errno_t add_ad_user_to_cached_groups(struct ldb_dn *user_dn, - return ENOMEM; - } - -+ val = ldb_dn_get_rdn_val(user_dn); -+ if (val == NULL) { -+ DEBUG(SSSDBG_OP_FAILURE, "user_dn has no RDN.\n"); -+ ret = EINVAL; -+ goto done; -+ } -+ user_name = talloc_strndup(tmp_ctx, (char *) val->data, val->length); -+ if (user_name == NULL) { -+ DEBUG(SSSDBG_OP_FAILURE, "Failed to copy user name.\n"); -+ ret = ENOMEM; -+ goto done; -+ } -+ -+ ret = sysdb_transaction_start(user_dom->sysdb); -+ if (ret != EOK) { -+ DEBUG(SSSDBG_FATAL_FAILURE, "Failed to start update transaction\n"); -+ goto done; -+ } -+ -+ in_transaction = true; -+ -+ ret = sysdb_get_direct_parents_ex(tmp_ctx, user_dom, group_dom, -+ SYSDB_MEMBER_USER, user_name, -+ SYSDB_ORIG_DN, -+ &sysdb_ipa_group_memberships); -+ if (ret != EOK) { -+ DEBUG(SSSDBG_OP_FAILURE, "Failed to get current IPA group memberships " -+ "of user [%s].\n", user_name); -+ goto done; -+ } -+ -+ ret = diff_string_lists(tmp_ctx, groups, sysdb_ipa_group_memberships, -+ &add_groups, &del_groups, NULL); -+ if (ret != EOK) { -+ DEBUG(SSSDBG_OP_FAILURE, "Failed to get difference in group lists.\n"); -+ goto done; -+ } -+ -+ user_attrs = sysdb_new_attrs(tmp_ctx); -+ if (user_attrs == NULL) { -+ DEBUG(SSSDBG_OP_FAILURE, "sysdb_new_attrs failed.\n"); -+ ret = ENOMEM; -+ goto done; -+ } -+ -+ /* Add all new IPA groups to SYSDB_ORIG_MEMBEROF because they are most -+ * probably removed by the previous user update and mark all new groups as -+ * processed. */ - for (c = 0; groups[c] != NULL; c++) { -- if (groups[c][0] == '\0') { -- continue; -+ ret = sysdb_attrs_add_string(user_attrs, SYSDB_ORIG_MEMBEROF, -+ groups[c]); -+ if (ret != EOK) { -+ DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_add_string failed.\n"); -+ goto done; - } - -- ret = sysdb_search_groups_by_orig_dn(tmp_ctx, group_dom, groups[c], -+ groups[c][0] = '\0'; -+ } -+ -+ if (DEBUG_IS_SET(SSSDBG_TRACE_ALL)) { -+ DEBUG(SSSDBG_TRACE_ALL, "New IPA groups [%zu].\n", c); -+ -+ for (c = 0; sysdb_ipa_group_memberships[c] != NULL; c++); -+ DEBUG(SSSDBG_TRACE_ALL, "Cached IPA groups [%zu].\n", c); -+ -+ for (c = 0; add_groups[c] != NULL; c++); -+ DEBUG(SSSDBG_TRACE_ALL, "Groups to add [%zu].\n", c); -+ -+ for (c = 0; del_groups[c] != NULL; c++); -+ DEBUG(SSSDBG_TRACE_ALL, "Groups to delete [%zu].\n", c); -+ } -+ -+ /* TODO: there is a similar functionality (adding and removing group -+ * memberships in sysdb_update_members_ex(), but the missing group feature -+ * is missing. It might be worth to evaluate if either the missing group -+ * feature can be added there or if group which are missing in the cache -+ * can bew handled differently here. */ -+ -+ for (c = 0; add_groups[c] != NULL; c++) { -+ -+ ret = sysdb_search_groups_by_orig_dn(tmp_ctx, group_dom, add_groups[c], - NULL, &msgs_count, &msgs); - if (ret != EOK) { - if (ret == ENOENT) { - DEBUG(SSSDBG_TRACE_ALL, "Group [%s] not in the cache.\n", -- groups[c]); -+ add_groups[c]); - *missing_groups = true; -+ talloc_free(groups[d]); -+ /* add missing group back to the list */ -+ groups[d++] = talloc_steal(groups, add_groups[c]); - continue; - } else { - DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_entry failed.\n"); -@@ -345,9 +431,6 @@ static errno_t add_ad_user_to_cached_groups(struct ldb_dn *user_dn, - } - } - --/* TODO? Do we have to remove members as well? I think not because the AD -- * query before removes all memberships. */ -- - ret = sysdb_mod_group_member(group_dom, user_dn, msgs[0]->dn, - LDB_FLAG_MOD_ADD); - if (ret != EOK && ret != EEXIST) { -@@ -355,33 +438,58 @@ static errno_t add_ad_user_to_cached_groups(struct ldb_dn *user_dn, - goto done; - } - -- user_attrs = sysdb_new_attrs(tmp_ctx); -- if (user_attrs == NULL) { -- DEBUG(SSSDBG_OP_FAILURE, "sysdb_new_attrs failed.\n"); -- ret = ENOMEM; -- goto done; -- } -+ } -+ talloc_free(groups[d]); -+ groups[d] = NULL; - -- ret = sysdb_attrs_add_string(user_attrs, SYSDB_ORIG_MEMBEROF, -- groups[c]); -+ for (c = 0; del_groups[c] != NULL; c++) { -+ ret = sysdb_search_groups_by_orig_dn(tmp_ctx, group_dom, del_groups[c], -+ NULL, &msgs_count, &msgs); - if (ret != EOK) { -- DEBUG(SSSDBG_OP_FAILURE, "sysdb_attrs_add_string failed.\n"); -- goto done; -+ if (ret == ENOENT) { -+ DEBUG(SSSDBG_TRACE_ALL, -+ "Group [%s] not in the cache, skipping.\n", -+ del_groups[c]); -+ continue; -+ } else { -+ DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_entry failed.\n"); -+ goto done; -+ } - } - -- ret = sysdb_set_entry_attr(user_dom->sysdb, user_dn, user_attrs, -- LDB_FLAG_MOD_ADD); -+ ret = sysdb_mod_group_member(group_dom, user_dn, msgs[0]->dn, -+ LDB_FLAG_MOD_DELETE); - if (ret != EOK && ret != EEXIST) { -- DEBUG(SSSDBG_OP_FAILURE, "sysdb_set_entry_attr failed.\n"); -+ DEBUG(SSSDBG_OP_FAILURE, -+ "sysdb_mod_group_member failed to delete member.\n"); - goto done; - } -+ } - -- /* mark group as already processed */ -- groups[c][0] = '\0'; -+ /* Update SYSDB_ORIG_MEMBEROF with the IPA groups. */ -+ ret = sysdb_set_entry_attr(user_dom->sysdb, user_dn, user_attrs, -+ LDB_FLAG_MOD_ADD); -+ if (ret != EOK && ret != EEXIST) { -+ DEBUG(SSSDBG_OP_FAILURE, "Failed to add original IPA group DNs, ignored.\n"); - } - -+ ret = sysdb_transaction_commit(user_dom->sysdb); -+ if (ret != EOK) { -+ DEBUG(SSSDBG_CRIT_FAILURE, "Failed to commit transaction\n"); -+ goto done; -+ } -+ -+ in_transaction = false; -+ - ret = EOK; - done: -+ if (in_transaction) { -+ sret = sysdb_transaction_cancel(user_dom->sysdb); -+ if (sret != EOK) { -+ DEBUG(SSSDBG_CRIT_FAILURE, "Could not cancel transaction\n"); -+ } -+ } -+ - talloc_free(tmp_ctx); - - return ret; --- -2.50.0 - diff --git a/0004-krb5-offline-with-SSS_AUTHTOK_TYPE_PAM_STACKED.patch b/0002-krb5-offline-with-SSS_AUTHTOK_TYPE_PAM_STACKED.patch similarity index 100% rename from 0004-krb5-offline-with-SSS_AUTHTOK_TYPE_PAM_STACKED.patch rename to 0002-krb5-offline-with-SSS_AUTHTOK_TYPE_PAM_STACKED.patch diff --git a/sssd.spec b/sssd.spec index 3e7dcf3..87b7451 100644 --- a/sssd.spec +++ b/sssd.spec @@ -27,17 +27,15 @@ Name: sssd Version: 2.9.7 -Release: 3%{?dist} +Release: 4%{?dist} Summary: System Security Services Daemon License: GPLv3+ URL: https://github.com/SSSD/sssd/ Source0: https://github.com/SSSD/sssd/releases/download/%{version}/sssd-%{version}.tar.gz ### Patches ### -Patch0001: 0001-sysdb-add-sysdb_get_direct_parents_ex.patch -Patch0002: 0002-ipa-improve-handling-of-external-group-memberships.patch -Patch0003: 0003-authtok-add-IS_PW_OR_ST_AUTHTOK.patch -Patch0004: 0004-krb5-offline-with-SSS_AUTHTOK_TYPE_PAM_STACKED.patch +Patch0001: 0001-authtok-add-IS_PW_OR_ST_AUTHTOK.patch +Patch0002: 0002-krb5-offline-with-SSS_AUTHTOK_TYPE_PAM_STACKED.patch ### Dependencies ### @@ -1087,6 +1085,10 @@ fi %systemd_postun_with_restart sssd.service %changelog +* Thu Aug 14 2025 Alexey Tikhonov - 2.9.7-4 +- Related: RHEL-87530 - AD user in external group is not cleared when expiring the cache [rhel-9] + Patch used to fix this ticket causes a regression (RHEL-106987) and is being reverted. + * Mon Jul 14 2025 Alexey Tikhonov - 2.9.7-3 - Resolves: RHEL-87530 - AD user in external group is not cleared when expiring the cache [rhel-9] - Resolves: RHEL-103434 - cache_credentials = true not working