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.
This commit is contained in:
Alexey Tikhonov 2025-08-14 12:58:05 +02:00
parent 646541314c
commit 282bcc8d49
5 changed files with 7 additions and 403 deletions

View File

@ -1,151 +0,0 @@
From 37f6f1aa65120539104acbf611383fcb263f3020 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose@redhat.com>
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 <allopez@redhat.com>
Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Justin Stephenson <jstephen@redhat.com>
(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

View File

@ -1,247 +0,0 @@
From 844cefe14e5aa97c09159dec419dd14b17bd531e Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose@redhat.com>
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 <allopez@redhat.com>
Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Justin Stephenson <jstephen@redhat.com>
(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

View File

@ -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 <atikhono@redhat.com> - 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 <atikhono@redhat.com> - 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