URL: https://github.com/SSSD/sssd/pull/128 Author: fidencio Title: #128: Fix group renaming issue when "id_provider = ldap" is set Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/128/head:pr128 git checkout pr128
From 36b52887d4b9028a7315790addf7a4432aa56c1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Fri, 16 Feb 2018 13:55:53 +0100 Subject: [PATCH 01/15] NSS: Add InvalidateGroupById handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are some situations where, from the backend, the NSS responder will have to be notified to invalidate a group. In order to achieve this in a clean way, let's add the InvalidateGroupById handler and make use of it later in this very same series. Related: https://pagure.io/SSSD/sssd/issue/2653 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/responder/nss/nss_iface.c | 16 ++++++++++++++ src/responder/nss/nss_iface.xml | 3 +++ src/responder/nss/nss_iface_generated.c | 38 +++++++++++++++++++++++++++++++++ src/responder/nss/nss_iface_generated.h | 5 +++++ 4 files changed, 62 insertions(+) diff --git a/src/responder/nss/nss_iface.c b/src/responder/nss/nss_iface.c index 415af9550..805e4fcdf 100644 --- a/src/responder/nss/nss_iface.c +++ b/src/responder/nss/nss_iface.c @@ -199,12 +199,28 @@ int nss_memorycache_update_initgroups(struct sbus_request *sbus_req, return iface_nss_memorycache_UpdateInitgroups_finish(sbus_req); } +int nss_memorycache_invalidate_group_by_id(struct sbus_request *sbus_req, + void *data, + gid_t gid) +{ + struct resp_ctx *rctx = talloc_get_type(data, struct resp_ctx); + struct nss_ctx *nctx = talloc_get_type(rctx->pvt_ctx, struct nss_ctx); + + DEBUG(SSSDBG_TRACE_LIBS, + "Invalidating group %"PRIu32" from memory cache\n", gid); + + sss_mmap_cache_gr_invalidate_gid(nctx->grp_mc_ctx, gid); + + return iface_nss_memorycache_InvalidateGroupById_finish(sbus_req); +} + struct iface_nss_memorycache iface_nss_memorycache = { { &iface_nss_memorycache_meta, 0 }, .UpdateInitgroups = nss_memorycache_update_initgroups, .InvalidateAllUsers = nss_memorycache_invalidate_users, .InvalidateAllGroups = nss_memorycache_invalidate_groups, .InvalidateAllInitgroups = nss_memorycache_invalidate_initgroups, + .InvalidateGroupById = nss_memorycache_invalidate_group_by_id, }; static struct sbus_iface_map iface_map[] = { diff --git a/src/responder/nss/nss_iface.xml b/src/responder/nss/nss_iface.xml index 27aae0197..4d8cf14f9 100644 --- a/src/responder/nss/nss_iface.xml +++ b/src/responder/nss/nss_iface.xml @@ -14,5 +14,8 @@ </method> <method name="InvalidateAllInitgroups"> </method> + <method name="InvalidateGroupById"> + <arg name="gid" type="u" direction="in" /> + </method> </interface> </node> diff --git a/src/responder/nss/nss_iface_generated.c b/src/responder/nss/nss_iface_generated.c index 4a8b704da..8d5a4584b 100644 --- a/src/responder/nss/nss_iface_generated.c +++ b/src/responder/nss/nss_iface_generated.c @@ -12,6 +12,9 @@ /* invokes a handler with a 'ssau' DBus signature */ static int invoke_ssau_method(struct sbus_request *dbus_req, void *function_ptr); +/* invokes a handler with a 'u' DBus signature */ +static int invoke_u_method(struct sbus_request *dbus_req, void *function_ptr); + /* arguments for org.freedesktop.sssd.nss.MemoryCache.UpdateInitgroups */ const struct sbus_arg_meta iface_nss_memorycache_UpdateInitgroups__in[] = { { "user", "s" }, @@ -44,6 +47,18 @@ int iface_nss_memorycache_InvalidateAllInitgroups_finish(struct sbus_request *re DBUS_TYPE_INVALID); } +/* arguments for org.freedesktop.sssd.nss.MemoryCache.InvalidateGroupById */ +const struct sbus_arg_meta iface_nss_memorycache_InvalidateGroupById__in[] = { + { "gid", "u" }, + { NULL, } +}; + +int iface_nss_memorycache_InvalidateGroupById_finish(struct sbus_request *req) +{ + return sbus_request_return_and_finish(req, + DBUS_TYPE_INVALID); +} + /* methods for org.freedesktop.sssd.nss.MemoryCache */ const struct sbus_method_meta iface_nss_memorycache__methods[] = { { @@ -74,6 +89,13 @@ const struct sbus_method_meta iface_nss_memorycache__methods[] = { offsetof(struct iface_nss_memorycache, InvalidateAllInitgroups), NULL, /* no invoker */ }, + { + "InvalidateGroupById", /* name */ + iface_nss_memorycache_InvalidateGroupById__in, + NULL, /* no out_args */ + offsetof(struct iface_nss_memorycache, InvalidateGroupById), + invoke_u_method, + }, { NULL, } }; @@ -86,6 +108,22 @@ const struct sbus_interface_meta iface_nss_memorycache_meta = { sbus_invoke_get_all, /* GetAll invoker */ }; +/* invokes a handler with a 'u' DBus signature */ +static int invoke_u_method(struct sbus_request *dbus_req, void *function_ptr) +{ + uint32_t arg_0; + int (*handler)(struct sbus_request *, void *, uint32_t) = function_ptr; + + if (!sbus_request_parse_or_finish(dbus_req, + DBUS_TYPE_UINT32, &arg_0, + DBUS_TYPE_INVALID)) { + return EOK; /* request handled */ + } + + return (handler)(dbus_req, dbus_req->intf->handler_data, + arg_0); +} + /* invokes a handler with a 'ssau' DBus signature */ static int invoke_ssau_method(struct sbus_request *dbus_req, void *function_ptr) { diff --git a/src/responder/nss/nss_iface_generated.h b/src/responder/nss/nss_iface_generated.h index 11fac7916..27a6d0853 100644 --- a/src/responder/nss/nss_iface_generated.h +++ b/src/responder/nss/nss_iface_generated.h @@ -18,6 +18,7 @@ #define IFACE_NSS_MEMORYCACHE_INVALIDATEALLUSERS "InvalidateAllUsers" #define IFACE_NSS_MEMORYCACHE_INVALIDATEALLGROUPS "InvalidateAllGroups" #define IFACE_NSS_MEMORYCACHE_INVALIDATEALLINITGROUPS "InvalidateAllInitgroups" +#define IFACE_NSS_MEMORYCACHE_INVALIDATEGROUPBYID "InvalidateGroupById" /* ------------------------------------------------------------------------ * DBus handlers @@ -44,6 +45,7 @@ struct iface_nss_memorycache { int (*InvalidateAllUsers)(struct sbus_request *req, void *data); int (*InvalidateAllGroups)(struct sbus_request *req, void *data); int (*InvalidateAllInitgroups)(struct sbus_request *req, void *data); + int (*InvalidateGroupById)(struct sbus_request *req, void *data, uint32_t arg_gid); }; /* finish function for UpdateInitgroups */ @@ -58,6 +60,9 @@ int iface_nss_memorycache_InvalidateAllGroups_finish(struct sbus_request *req); /* finish function for InvalidateAllInitgroups */ int iface_nss_memorycache_InvalidateAllInitgroups_finish(struct sbus_request *req); +/* finish function for InvalidateGroupById */ +int iface_nss_memorycache_InvalidateGroupById_finish(struct sbus_request *req); + /* ------------------------------------------------------------------------ * DBus Interface Metadata * From 99f7fd7c1104c2571323c4a25572880dcb7dd1a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 19 Feb 2018 08:42:10 +0100 Subject: [PATCH 02/15] DP: Add dp_sbus_invalidate_group_memcache() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function will be called from the data provider to the NSS responder, which will invalidate a group in the memcache. Related: https://pagure.io/SSSD/sssd/issue/2653 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/providers/data_provider/dp.h | 2 ++ src/providers/data_provider/dp_resp_client.c | 40 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/src/providers/data_provider/dp.h b/src/providers/data_provider/dp.h index ceb49da53..e8b2f9c8f 100644 --- a/src/providers/data_provider/dp.h +++ b/src/providers/data_provider/dp.h @@ -179,6 +179,8 @@ void dp_sbus_reset_groups_ncache(struct data_provider *provider, void dp_sbus_reset_users_memcache(struct data_provider *provider); void dp_sbus_reset_groups_memcache(struct data_provider *provider); void dp_sbus_reset_initgr_memcache(struct data_provider *provider); +void dp_sbus_invalidate_group_memcache(struct data_provider *provider, + gid_t gid); /* * A dummy handler for DPM_ACCT_DOMAIN_HANDLER. diff --git a/src/providers/data_provider/dp_resp_client.c b/src/providers/data_provider/dp_resp_client.c index 5735188a6..d48af3288 100644 --- a/src/providers/data_provider/dp_resp_client.c +++ b/src/providers/data_provider/dp_resp_client.c @@ -189,3 +189,43 @@ void dp_sbus_reset_initgr_memcache(struct data_provider *provider) return dp_sbus_reset_memcache(provider, IFACE_NSS_MEMORYCACHE_INVALIDATEALLINITGROUPS); } + +void dp_sbus_invalidate_group_memcache(struct data_provider *provider, + gid_t gid) +{ + struct dp_client *dp_cli; + DBusMessage *msg; + dbus_bool_t dbret; + + dp_cli = provider->clients[DPC_NSS]; + if (dp_cli == NULL) { + return; + } + + msg = dbus_message_new_method_call(NULL, + NSS_MEMORYCACHE_PATH, + IFACE_NSS_MEMORYCACHE, + IFACE_NSS_MEMORYCACHE_INVALIDATEGROUPBYID); + if (msg == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "Out of memory?!\n"); + return; + } + + dbret = dbus_message_append_args(msg, + DBUS_TYPE_UINT32, &gid, + DBUS_TYPE_INVALID); + if (!dbret) { + DEBUG(SSSDBG_CRIT_FAILURE, "Out of memory?!\n"); + dbus_message_unref(msg); + return; + } + + DEBUG(SSSDBG_TRACE_FUNC, + "Ordering NSS responder to invalidate the group %"PRIu32" \n", + gid); + + sbus_conn_send_reply(dp_client_conn(dp_cli), msg); + dbus_message_unref(msg); + + return; +} From 4795659cf6e0aa6747e85419b0d77b8ead3a3050 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 19 Feb 2018 10:00:42 +0100 Subject: [PATCH 03/15] SDAP: Pass struct data_provider to sdap_get_groups_send() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is needed in order to properly treat group id-collision when adding an incomplete group. The struct data_provider had to be passed to this function and also added to the struct sdap_get_groups_state as it'll have to be passed down to sdap_add_incomplete_groups(). Related: https://pagure.io/SSSD/sssd/issue/2653 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/providers/ldap/ldap_id.c | 3 ++- src/providers/ldap/sdap_async.h | 3 ++- src/providers/ldap/sdap_async_enum.c | 3 ++- src/providers/ldap/sdap_async_groups.c | 9 ++++++++- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c index 3bd68780a..bb6cd5c26 100644 --- a/src/providers/ldap/ldap_id.c +++ b/src/providers/ldap/ldap_id.c @@ -1036,7 +1036,8 @@ static void groups_get_search(struct tevent_req *req) dp_opt_get_int(state->ctx->opts->basic, SDAP_SEARCH_TIMEOUT), lookup_type, - state->no_members); + state->no_members, + state->ctx->be->provider); if (!subreq) { tevent_req_error(req, ENOMEM); return; diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h index 40da81fb9..d736f8bd3 100644 --- a/src/providers/ldap/sdap_async.h +++ b/src/providers/ldap/sdap_async.h @@ -104,7 +104,8 @@ struct tevent_req *sdap_get_groups_send(TALLOC_CTX *memctx, const char *filter, int timeout, enum sdap_entry_lookup_type lookup_type, - bool no_members); + bool no_members, + void *provider); int sdap_get_groups_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, char **timestamp); diff --git a/src/providers/ldap/sdap_async_enum.c b/src/providers/ldap/sdap_async_enum.c index ea9d51adc..6623e6753 100644 --- a/src/providers/ldap/sdap_async_enum.c +++ b/src/providers/ldap/sdap_async_enum.c @@ -814,7 +814,8 @@ static struct tevent_req *enum_groups_send(TALLOC_CTX *memctx, state->attrs, state->filter, dp_opt_get_int(state->ctx->opts->basic, SDAP_ENUM_SEARCH_TIMEOUT), - SDAP_LOOKUP_ENUMERATE, false); + SDAP_LOOKUP_ENUMERATE, false, + state->ctx->be->provider); if (!subreq) { ret = ENOMEM; goto fail; diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c index 77acded7a..1c7b2c610 100644 --- a/src/providers/ldap/sdap_async_groups.c +++ b/src/providers/ldap/sdap_async_groups.c @@ -1817,6 +1817,11 @@ struct sdap_get_groups_state { struct sdap_handle *ldap_sh; struct sdap_id_op *op; + + /* Provider will be used to send a d-bus message to NSS responder in case + * group id collision has been detected. In this case we'd have to also + * invalidate the group in the memcache. */ + void *provider; }; static errno_t sdap_get_groups_next_base(struct tevent_req *req); @@ -1833,7 +1838,8 @@ struct tevent_req *sdap_get_groups_send(TALLOC_CTX *memctx, const char *filter, int timeout, enum sdap_entry_lookup_type lookup_type, - bool no_members) + bool no_members, + void *provider) { errno_t ret; struct tevent_req *req; @@ -1860,6 +1866,7 @@ struct tevent_req *sdap_get_groups_send(TALLOC_CTX *memctx, state->base_filter = filter; state->base_iter = 0; state->search_bases = sdom->group_search_bases; + state->provider = provider; if (!state->search_bases) { DEBUG(SSSDBG_CRIT_FAILURE, From 24e5432141a738534e883a18d7247eb1f988c0fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 19 Feb 2018 10:16:20 +0100 Subject: [PATCH 04/15] SDAP: Pass struct data_provider to sdap_initgr_rfc2307_send() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is needed in order to properly treat group id-collision when adding an incomplete group. The struct data_provider had to be passed to this function and also added to the struct sdap_initgr_rfc2307_state as it'll have to be passed down to sdap_initgr_common_store(), which will pass it down to sdap_add_incomplete_groups(). Related: https://pagure.io/SSSD/sssd/issue/2653 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/providers/ldap/sdap_async_initgroups.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c index 326294a1c..99f345074 100644 --- a/src/providers/ldap/sdap_async_initgroups.c +++ b/src/providers/ldap/sdap_async_initgroups.c @@ -388,6 +388,11 @@ struct sdap_initgr_rfc2307_state { size_t base_iter; struct sdap_search_base **search_bases; + + /* Provider will be used to send a d-bus message to NSS responder in case + * group id collision has been detected. In this case we'd have to also + * invalidate the group in the memcache. */ + void *provider; }; static errno_t sdap_initgr_rfc2307_next_base(struct tevent_req *req); @@ -398,7 +403,8 @@ struct tevent_req *sdap_initgr_rfc2307_send(TALLOC_CTX *memctx, struct sysdb_ctx *sysdb, struct sss_domain_info *domain, struct sdap_handle *sh, - const char *name) + const char *name, + void *provider) { struct tevent_req *req; struct sdap_initgr_rfc2307_state *state; @@ -422,6 +428,7 @@ struct tevent_req *sdap_initgr_rfc2307_send(TALLOC_CTX *memctx, state->ldap_groups_count = 0; state->base_iter = 0; state->search_bases = opts->sdom->group_search_bases; + state->provider = provider; if (!state->search_bases) { DEBUG(SSSDBG_CRIT_FAILURE, @@ -3041,7 +3048,8 @@ static void sdap_get_initgr_user(struct tevent_req *subreq) case SDAP_SCHEMA_RFC2307: subreq = sdap_initgr_rfc2307_send(state, state->ev, state->opts, state->sysdb, state->dom, state->sh, - cname); + cname, + state->id_ctx->be->provider); if (!subreq) { tevent_req_error(req, ENOMEM); return; From 30dc4c1cbfce9db3a2df473eb4e92f1889e7b28c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 19 Feb 2018 10:41:09 +0100 Subject: [PATCH 05/15] SDAP: Pass struct data_provider to sdap_initgr_rfc2307bis_send() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is needed in order to properly treat group id-collision when adding an incomplete group. The struct data_provider had to be passed to this function and also added to the struct sdap_initgr_rfc2307bis_state as it'll have to be passed down to sdap_initgr_rfc2307bis_send(), which will pass it down to sdap_initgr_common_store(), which will pass it down to sdap_add_incomplete_groups(). Related: https://pagure.io/SSSD/sssd/issue/2653 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/providers/ldap/sdap_async_initgroups.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c index 99f345074..4593e8ca9 100644 --- a/src/providers/ldap/sdap_async_initgroups.c +++ b/src/providers/ldap/sdap_async_initgroups.c @@ -1567,6 +1567,11 @@ struct sdap_initgr_rfc2307bis_state { hash_table_t *group_hash; size_t num_direct_parents; struct sysdb_attrs **direct_groups; + + /* Provider will be used to send a d-bus message to NSS responder in case + * group id collision has been detected. In this case we'd have to also + * invalidate the group in the memcache. */ + void *provider; }; struct sdap_nested_group { @@ -1588,7 +1593,8 @@ static struct tevent_req *sdap_initgr_rfc2307bis_send( struct sdap_domain *sdom, struct sdap_handle *sh, const char *name, - const char *orig_dn) + const char *orig_dn, + void *provider) { errno_t ret; struct tevent_req *req; @@ -1614,6 +1620,7 @@ static struct tevent_req *sdap_initgr_rfc2307bis_send( state->base_iter = 0; state->search_bases = sdom->group_search_bases; state->orig_dn = orig_dn; + state->provider = provider; if (!state->search_bases) { DEBUG(SSSDBG_CRIT_FAILURE, @@ -3099,7 +3106,8 @@ static void sdap_get_initgr_user(struct tevent_req *subreq) subreq = sdap_initgr_rfc2307bis_send( state, state->ev, state->opts, state->sdom, state->sh, - cname, orig_dn); + cname, orig_dn, + state->id_ctx->be->provider); } if (!subreq) { tevent_req_error(req, ENOMEM); From 2b225888613afb9a6b94d5f9334f07f407ae49ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 19 Feb 2018 12:01:54 +0100 Subject: [PATCH 06/15] SDAP: Pass struct data_provider to sdap_get_ad_match_rule_initgroups_send() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is needed in order to properly treat group id-collision when adding an incomplete group. The struct data_provider had to be passed to this function and also added to the struct sdap_ad_match_rule_initgr_state as it'll have to be passed down to sdap_initgr_common_store(), which will pass it down to which will pass it down to sdap_add_incomplete_groups(). Related: https://pagure.io/SSSD/sssd/issue/2653 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/providers/ldap/sdap_async.h | 3 ++- src/providers/ldap/sdap_async_initgroups.c | 16 +++++++++------- src/providers/ldap/sdap_async_initgroups_ad.c | 9 ++++++++- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h index d736f8bd3..ece0c5570 100644 --- a/src/providers/ldap/sdap_async.h +++ b/src/providers/ldap/sdap_async.h @@ -390,7 +390,8 @@ sdap_get_ad_match_rule_initgroups_send(TALLOC_CTX *mem_ctx, struct sdap_handle *sh, const char *name, const char *orig_dn, - int timeout); + int timeout, + void *provider); errno_t sdap_get_ad_match_rule_initgroups_recv(struct tevent_req *req); diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c index 4593e8ca9..bad348dc8 100644 --- a/src/providers/ldap/sdap_async_initgroups.c +++ b/src/providers/ldap/sdap_async_initgroups.c @@ -3095,13 +3095,15 @@ static void sdap_get_initgr_user(struct tevent_req *subreq) /* Take advantage of AD's extensibleMatch filter to look up * all parent groups in a single request. */ - subreq = sdap_get_ad_match_rule_initgroups_send(state, state->ev, - state->opts, - state->sysdb, - state->dom, - state->sh, - cname, orig_dn, - state->timeout); + subreq = sdap_get_ad_match_rule_initgroups_send( + state, state->ev, + state->opts, + state->sysdb, + state->dom, + state->sh, + cname, orig_dn, + state->timeout, + state->id_ctx->be->provider); } else { subreq = sdap_initgr_rfc2307bis_send( state, state->ev, state->opts, diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c b/src/providers/ldap/sdap_async_initgroups_ad.c index 9da671a99..df922d0ff 100644 --- a/src/providers/ldap/sdap_async_initgroups_ad.c +++ b/src/providers/ldap/sdap_async_initgroups_ad.c @@ -47,6 +47,11 @@ struct sdap_ad_match_rule_initgr_state { size_t base_iter; struct sdap_search_base **search_bases; + + /* Provider will be used to send a d-bus message to NSS responder in case + * group id collision has been detected. In this case we'd have to also + * invalidate the group in the memcache. */ + void *provider; }; static errno_t @@ -64,7 +69,8 @@ sdap_get_ad_match_rule_initgroups_send(TALLOC_CTX *mem_ctx, struct sdap_handle *sh, const char *name, const char *orig_dn, - int timeout) + int timeout, + void *provider) { errno_t ret; struct tevent_req *req; @@ -86,6 +92,7 @@ sdap_get_ad_match_rule_initgroups_send(TALLOC_CTX *mem_ctx, state->orig_dn = orig_dn; state->base_iter = 0; state->search_bases = opts->sdom->group_search_bases; + state->provider = provider; /* Request all of the group attributes that we know * about, except for 'member' because that wastes a From 41ef94d4f8e7470b16a501325e268d16d793a49f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 19 Feb 2018 10:19:09 +0100 Subject: [PATCH 07/15] SDAP: Pass struct data_provider to sdap_initgr_common_store() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is needed in order to properly treat group id-collision when adding an incomplete group. The struct data_provider had to be passed to this function as it'll have to be passed down to sdap_add_incomplete_groups(). Related: https://pagure.io/SSSD/sssd/issue/2653 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/providers/ldap/sdap_async_initgroups.c | 6 ++++-- src/providers/ldap/sdap_async_initgroups_ad.c | 3 ++- src/providers/ldap/sdap_async_private.h | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c index bad348dc8..cd05ac9ba 100644 --- a/src/providers/ldap/sdap_async_initgroups.c +++ b/src/providers/ldap/sdap_async_initgroups.c @@ -266,7 +266,8 @@ int sdap_initgr_common_store(struct sysdb_ctx *sysdb, enum sysdb_member_type type, char **sysdb_grouplist, struct sysdb_attrs **ldap_groups, - int ldap_groups_count) + int ldap_groups_count, + void *provider) { TALLOC_CTX *tmp_ctx; char **ldap_grouplist = NULL; @@ -627,7 +628,8 @@ static void sdap_initgr_rfc2307_process(struct tevent_req *subreq) SYSDB_MEMBER_USER, sysdb_grouplist, state->ldap_groups, - state->ldap_groups_count); + state->ldap_groups_count, + state->provider); if (ret != EOK) { tevent_req_error(req, ret); return; diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c b/src/providers/ldap/sdap_async_initgroups_ad.c index df922d0ff..0557817f2 100644 --- a/src/providers/ldap/sdap_async_initgroups_ad.c +++ b/src/providers/ldap/sdap_async_initgroups_ad.c @@ -291,7 +291,8 @@ sdap_get_ad_match_rule_initgroups_step(struct tevent_req *subreq) SYSDB_MEMBER_USER, sysdb_grouplist, state->groups, - state->count); + state->count, + state->provider); if (ret != EOK) { DEBUG(SSSDBG_MINOR_FAILURE, "Could not store groups for user [%s]: [%s]\n", diff --git a/src/providers/ldap/sdap_async_private.h b/src/providers/ldap/sdap_async_private.h index 72507442a..2476ca7b8 100644 --- a/src/providers/ldap/sdap_async_private.h +++ b/src/providers/ldap/sdap_async_private.h @@ -104,7 +104,8 @@ int sdap_initgr_common_store(struct sysdb_ctx *sysdb, enum sysdb_member_type type, char **sysdb_grouplist, struct sysdb_attrs **ldap_groups, - int ldap_groups_count); + int ldap_groups_count, + void *provider); errno_t get_sysdb_grouplist(TALLOC_CTX *mem_ctx, struct sysdb_ctx *sysdb, From 4e4c307cfc1d44fd2ee791eb85331d77160130d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 19 Feb 2018 12:12:58 +0100 Subject: [PATCH 08/15] SDAP: Pass struct data_provider to sdap_ad_get_domain_local_groups_parse_parent() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is needed in order to properly treat group id-collision when adding an incomplete group. The struct data_provider had to be passed to this function as it'll have to be sdap_nested_groups_store(), which will pass it down to sdap_add_incomplete_groups(). Related: https://pagure.io/SSSD/sssd/issue/2653 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/providers/ldap/sdap_async_initgroups_ad.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c b/src/providers/ldap/sdap_async_initgroups_ad.c index 0557817f2..3aef0b076 100644 --- a/src/providers/ldap/sdap_async_initgroups_ad.c +++ b/src/providers/ldap/sdap_async_initgroups_ad.c @@ -1552,6 +1552,7 @@ sdap_ad_get_domain_local_groups_parse_parents(TALLOC_CTX *mem_ctx, struct sss_domain_info *dom, struct sysdb_ctx *sysdb, struct sdap_options *opts, + void *provider, const char **_sysdb_name, enum sysdb_member_type *_type, char ***_add_list, @@ -1781,14 +1782,16 @@ static void sdap_ad_get_domain_local_groups_done(struct tevent_req *subreq) * nested parents found during the request. The nested parents contain * the processed LDAP data and can be identified by a missing * objectclass attribute. */ - ret = sdap_ad_get_domain_local_groups_parse_parents(state, gr, - state->dom, - state->sysdb, - state->opts, - &sysdb_name, - &type, - &add_list, - &del_list); + ret = sdap_ad_get_domain_local_groups_parse_parents( + state, gr, + state->dom, + state->sysdb, + state->opts, + state->conn->id_ctx->be->provider, + &sysdb_name, + &type, + &add_list, + &del_list); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "sdap_ad_get_domain_local_groups_parse_parents failed.\n"); From 6d02630daea8686ba05925fe587cf3c6c9a0a708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 19 Feb 2018 12:16:39 +0100 Subject: [PATCH 09/15] SDAP: Pass struct data_provider to sdap_nested_groups_store() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is needed in order to properly treat group id-collision when adding an incomplete group. The struct data_provider had to be passed to this function and also added to the struct sdap_initgr_nested_state as it'll have to be passed down to sdap_add_incomplete_groups(). Related: https://pagure.io/SSSD/sssd/issue/2653 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/providers/ldap/sdap_async_initgroups.c | 13 ++++++++++--- src/providers/ldap/sdap_async_initgroups_ad.c | 5 +++-- src/providers/ldap/sdap_async_private.h | 3 ++- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c index cd05ac9ba..1a9f89b2c 100644 --- a/src/providers/ldap/sdap_async_initgroups.c +++ b/src/providers/ldap/sdap_async_initgroups.c @@ -651,7 +651,8 @@ sdap_nested_groups_store(struct sysdb_ctx *sysdb, struct sss_domain_info *domain, struct sdap_options *opts, struct sysdb_attrs **groups, - unsigned long count) + unsigned long count, + void *provider) { errno_t ret, tret; TALLOC_CTX *tmp_ctx; @@ -790,6 +791,11 @@ struct sdap_initgr_nested_state { struct sysdb_attrs **groups; int groups_cur; + + /* Provider will be used to send a d-bus message to NSS responder in case + * group id collision has been detected. In this case we'd have to also + * invalidate the group in the memcache. */ + void *provider; }; static errno_t sdap_initgr_nested_deref_search(struct tevent_req *req); @@ -1145,7 +1151,8 @@ sdap_initgr_store_groups(struct sdap_initgr_nested_state *state) { return sdap_nested_groups_store(state->sysdb, state->dom, state->opts, state->groups, - state->groups_cur); + state->groups_cur, + state->provider); } static errno_t @@ -1952,7 +1959,7 @@ save_rfc2307bis_groups(struct sdap_initgr_rfc2307bis_state *state) talloc_zfree(values); ret = sdap_nested_groups_store(state->sysdb, state->dom, state->opts, - groups, count); + groups, count, state->provider); if (ret != EOK) { DEBUG(SSSDBG_MINOR_FAILURE, "Could not save groups [%d]: %s\n", ret, strerror(ret)); diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c b/src/providers/ldap/sdap_async_initgroups_ad.c index 3aef0b076..27ad3cb1f 100644 --- a/src/providers/ldap/sdap_async_initgroups_ad.c +++ b/src/providers/ldap/sdap_async_initgroups_ad.c @@ -1590,7 +1590,8 @@ sdap_ad_get_domain_local_groups_parse_parents(TALLOC_CTX *mem_ctx, if (gr->parents_count != 0) { /* Store the parents if needed */ ret = sdap_nested_groups_store(sysdb, dom, opts, - gr->ldap_parents, gr->parents_count); + gr->ldap_parents, gr->parents_count, + provider); if (ret != EOK) { DEBUG(SSSDBG_MINOR_FAILURE, "Could not save groups [%d]: %s\n", ret, strerror(ret)); @@ -1626,7 +1627,7 @@ sdap_ad_get_domain_local_groups_parse_parents(TALLOC_CTX *mem_ctx, /* make sure group exists in cache */ groups[0]= gr->group; - ret = sdap_nested_groups_store(sysdb, dom, opts, groups, 1); + ret = sdap_nested_groups_store(sysdb, dom, opts, groups, 1, provider); if (ret != EOK) { DEBUG(SSSDBG_MINOR_FAILURE, "Could not save groups [%d]: %s\n", ret, strerror(ret)); diff --git a/src/providers/ldap/sdap_async_private.h b/src/providers/ldap/sdap_async_private.h index 2476ca7b8..53c0135ce 100644 --- a/src/providers/ldap/sdap_async_private.h +++ b/src/providers/ldap/sdap_async_private.h @@ -173,7 +173,8 @@ errno_t sdap_nested_groups_store(struct sysdb_ctx *sysdb, struct sss_domain_info *domain, struct sdap_options *opts, struct sysdb_attrs **groups, - unsigned long count); + unsigned long count, + void *provider); struct tevent_req * sdap_ad_get_domain_local_groups_send(TALLOC_CTX *mem_ctx, From e71f277a4cf6ab4ccf5ab3551545142c2f7bc59a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 19 Feb 2018 09:44:14 +0100 Subject: [PATCH 10/15] SDAP: Pass struct data_provider to sdap_add_incomplete_groups() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is needed in order to properly treat group id-collision when adding an incomplete group. Related: https://pagure.io/SSSD/sssd/issue/2653 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/providers/ldap/sdap_async_groups.c | 2 +- src/providers/ldap/sdap_async_initgroups.c | 7 ++++--- src/providers/ldap/sdap_async_private.h | 3 ++- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c index 1c7b2c610..a69738747 100644 --- a/src/providers/ldap/sdap_async_groups.c +++ b/src/providers/ldap/sdap_async_groups.c @@ -2076,7 +2076,7 @@ static void sdap_get_groups_process(struct tevent_req *subreq) ret = sdap_add_incomplete_groups(state->sysdb, state->dom, state->opts, sysdb_groupnamelist, state->groups, - state->count); + state->count, state->provider); if (ret == EOK) { DEBUG(SSSDBG_TRACE_LIBS, "Writing only group data without members was successful.\n"); diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c index 1a9f89b2c..cfb0e6728 100644 --- a/src/providers/ldap/sdap_async_initgroups.c +++ b/src/providers/ldap/sdap_async_initgroups.c @@ -35,7 +35,8 @@ errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb, struct sdap_options *opts, char **sysdb_groupnames, struct sysdb_attrs **ldap_groups, - int ldap_groups_count) + int ldap_groups_count, + void *provider) { TALLOC_CTX *tmp_ctx; struct ldb_message *msg; @@ -331,7 +332,7 @@ int sdap_initgr_common_store(struct sysdb_ctx *sysdb, if (add_groups && add_groups[0]) { ret = sdap_add_incomplete_groups(sysdb, domain, opts, add_groups, ldap_groups, - ldap_groups_count); + ldap_groups_count, provider); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "Adding incomplete users failed\n"); goto done; @@ -683,7 +684,7 @@ sdap_nested_groups_store(struct sysdb_ctx *sysdb, in_transaction = true; ret = sdap_add_incomplete_groups(sysdb, domain, opts, groupnamelist, - groups, count); + groups, count, provider); if (ret != EOK) { DEBUG(SSSDBG_TRACE_FUNC, "Could not add incomplete groups [%d]: %s\n", ret, strerror(ret)); diff --git a/src/providers/ldap/sdap_async_private.h b/src/providers/ldap/sdap_async_private.h index 53c0135ce..b53d08ea7 100644 --- a/src/providers/ldap/sdap_async_private.h +++ b/src/providers/ldap/sdap_async_private.h @@ -151,7 +151,8 @@ errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb, struct sdap_options *opts, char **sysdb_groupnames, struct sysdb_attrs **ldap_groups, - int ldap_groups_count); + int ldap_groups_count, + void *provider); /* from sdap_ad_groups.c */ errno_t sdap_check_ad_group_type(struct sss_domain_info *dom, From f89b77a0943b9af637d59a2bbde210a6865f0511 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 19 Feb 2018 08:29:36 +0100 Subject: [PATCH 11/15] ERRORS: Add ERR_GID_DUPLICATED MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This new error will be returned from sysdb_add_incomplete_group() when renaming a group which will case gid collision. Related: https://pagure.io/SSSD/sssd/issue/2653 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/util/util_errors.c | 1 + src/util/util_errors.h | 1 + 2 files changed, 2 insertions(+) diff --git a/src/util/util_errors.c b/src/util/util_errors.c index 39ce3d7dc..e2bb2a014 100644 --- a/src/util/util_errors.c +++ b/src/util/util_errors.c @@ -118,6 +118,7 @@ struct err_string error_to_str[] = { { "GetAccountDomain() not supported" }, /* ERR_GET_ACCT_DOM_NOT_SUPPORTED */ { "The last GetAccountDomain() result is still valid" }, /* ERR_GET_ACCT_DOM_CACHED */ { "ID is outside the allowed range" }, /* ERR_ID_OUTSIDE_RANGE */ + { "Group ID is duplicated" }, /* ERR_GID_DUPLICATED */ { "ERR_LAST" } /* ERR_LAST */ }; diff --git a/src/util/util_errors.h b/src/util/util_errors.h index ad4dad5f8..49501727d 100644 --- a/src/util/util_errors.h +++ b/src/util/util_errors.h @@ -140,6 +140,7 @@ enum sssd_errors { ERR_GET_ACCT_DOM_NOT_SUPPORTED, ERR_GET_ACCT_DOM_CACHED, ERR_ID_OUTSIDE_RANGE, + ERR_GID_DUPLICATED, ERR_LAST /* ALWAYS LAST */ }; From fc567fb37c670c51a8079390648b606b6f0e45a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 19 Feb 2018 12:43:06 +0100 Subject: [PATCH 12/15] SDAP: Add sdap_handle_id_collision_for_incomplete_groups() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This newly added function is a helper to properly hadle group id-collisions when renaming incomplete groups and it does: - Deletes the group from sysdb - Adds the new incomplete group - Notifies the NSS responder that the entry also has to be deleted from the memory cache This function will be called from sdap_ad_save_group_membership_with_idmapping() and from sdap_add_incomplete_groups(). Related: https://pagure.io/SSSD/sssd/issue/2653 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/providers/ldap/sdap_async.h | 11 ++++++++++ src/providers/ldap/sdap_async_initgroups.c | 34 ++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/src/providers/ldap/sdap_async.h b/src/providers/ldap/sdap_async.h index ece0c5570..362599fa2 100644 --- a/src/providers/ldap/sdap_async.h +++ b/src/providers/ldap/sdap_async.h @@ -414,4 +414,15 @@ sdap_ad_tokengroups_initgroups_send(TALLOC_CTX *mem_ctx, errno_t sdap_ad_tokengroups_initgroups_recv(struct tevent_req *req); +errno_t +sdap_handle_id_collision_for_incomplete_groups(void *provider, + struct sss_domain_info *domain, + const char *name, + gid_t gid, + const char *original_dn, + const char *sid_str, + const char *uuid, + bool posix, + time_t now); + #endif /* _SDAP_ASYNC_H_ */ diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c index cfb0e6728..275522c4f 100644 --- a/src/providers/ldap/sdap_async_initgroups.c +++ b/src/providers/ldap/sdap_async_initgroups.c @@ -3571,3 +3571,37 @@ errno_t get_sysdb_grouplist_dn(TALLOC_CTX *mem_ctx, return get_sysdb_grouplist_ex(mem_ctx, sysdb, domain, name, grouplist, true); } + +errno_t +sdap_handle_id_collision_for_incomplete_groups(void *provider, + struct sss_domain_info *domain, + const char *name, + gid_t gid, + const char *original_dn, + const char *sid_str, + const char *uuid, + bool posix, + time_t now) +{ + errno_t ret; + + ret = sysdb_delete_group(domain, NULL, gid); + if (ret != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, + "Due to an id collision, the new group with gid [\"%"PRIu32"\"] " + "will not be added as the old group (with the same gid) could " + "not be removed from the sysdb!", + gid); + return ret; + } + + ret = sysdb_add_incomplete_group(domain, name, gid, original_dn, sid_str, + uuid, posix, now); + if (ret != EOK) { + return ret; + } + + dp_sbus_invalidate_group_memcache(provider, gid); + + return EOK; +} From baa25c9891c490ec261d8045e41b6995baeaec98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 19 Feb 2018 12:51:57 +0100 Subject: [PATCH 13/15] SDAP: Properly handle group id-collision when renaming incomplete groups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves: https://pagure.io/SSSD/sssd/issue/2653 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/providers/ldap/sdap_async_initgroups.c | 13 +++++++++++++ src/providers/ldap/sdap_async_initgroups_ad.c | 13 +++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c index 275522c4f..38dfa1d7d 100644 --- a/src/providers/ldap/sdap_async_initgroups.c +++ b/src/providers/ldap/sdap_async_initgroups.c @@ -226,6 +226,19 @@ errno_t sdap_add_incomplete_groups(struct sysdb_ctx *sysdb, ret = sysdb_add_incomplete_group(domain, groupname, gid, original_dn, sid_str, uuid, posix, now); + if (ret == ERR_GID_DUPLICATED) { + /* In case o group id-collision, do: + * - Delete the group from sysdb + * - Add the new incomplete group + * - Notify the NSS responder that the entry has also to be + * removed from the memory cache + */ + ret = sdap_handle_id_collision_for_incomplete_groups( + provider, domain, groupname, gid, + original_dn, sid_str, uuid, posix, + now); + } + if (ret != EOK) { goto done; } diff --git a/src/providers/ldap/sdap_async_initgroups_ad.c b/src/providers/ldap/sdap_async_initgroups_ad.c index 27ad3cb1f..8ee9d3b52 100644 --- a/src/providers/ldap/sdap_async_initgroups_ad.c +++ b/src/providers/ldap/sdap_async_initgroups_ad.c @@ -932,6 +932,19 @@ errno_t sdap_ad_save_group_membership_with_idmapping(const char *username, ret = sysdb_add_incomplete_group(domain, name, gid, NULL, sid, NULL, false, now); + if (ret == ERR_GID_DUPLICATED) { + /* In case o group id-collision, do: + * - Delete the group from sysdb + * - Add the new incomplete group + * - Notify the NSS responder that the entry has also to be + * removed from the memory cache + */ + ret = sdap_handle_id_collision_for_incomplete_groups( + idmap_ctx->id_ctx->be->provider, + domain, name, gid, NULL, sid, NULL, + false, now); + } + if (ret != EOK) { DEBUG(SSSDBG_MINOR_FAILURE, "Could not create incomplete " "group: [%s]\n", strerror(ret)); From 7ee05019ad220620cd57dfcabaa67b0b1f91cef6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 19 Feb 2018 08:53:56 +0100 Subject: [PATCH 14/15] SYSDB_OPS: Error out on id-collision when adding an incomplete group MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This situation can be hit when renaming a group. For now, let's just error this out so the caller can handle it properly on its own layer. Related: https://pagure.io/SSSD/sssd/issue/2653 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/db/sysdb_ops.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index 15915101e..e88ff04ef 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -2340,12 +2340,34 @@ int sysdb_add_incomplete_group(struct sss_domain_info *domain, TALLOC_CTX *tmp_ctx; int ret; struct sysdb_attrs *attrs; + struct ldb_message *msg; + const char *previous = NULL; + const char *group_attrs[] = { SYSDB_SID_STR, SYSDB_UUID, SYSDB_ORIG_DN, NULL }; + const char *values[] = { sid_str, uuid, original_dn, NULL }; + bool same = false; tmp_ctx = talloc_new(NULL); if (!tmp_ctx) { return ENOMEM; } + ret = sysdb_search_group_by_gid(tmp_ctx, domain, gid, group_attrs, &msg); + if (ret == EOK) { + for (int i = 0; !same && group_attrs[i] != NULL; i++) { + previous = ldb_msg_find_attr_as_string(msg, + group_attrs[i], + NULL); + if (previous != NULL && values[i] != NULL) { + same = strcmp(previous, values[i]) == 0; + } + } + } + + if (same) { + ret = ERR_GID_DUPLICATED; + goto done; + } + /* try to add the group */ ret = sysdb_add_basic_group(domain, name, gid); if (ret) goto done; From 948c27903fa857a1144330d7e0bb2d29cdb8aa1c Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Mon, 19 Feb 2018 18:26:05 +0100 Subject: [PATCH 15/15] TESTS: Add an integration test for renaming incomplete groups during initgroups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As we implemented the group renaming heuristics to rename only if we can use another "hint" like the original DN or the SID to know the group is the same, this patch adds two tests (positive and negative) to make sure a group with a totally different RDN and hence different originalDN cannot be renamed but a group whose name changed but the RDN stays the same can be renamed. Related: https://pagure.io/SSSD/sssd/issue/3282 Reviewed-by: Lukáš Slebodník <lsleb...@redhat.com> Reviewed-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/tests/intg/test_ldap.py | 131 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 129 insertions(+), 2 deletions(-) diff --git a/src/tests/intg/test_ldap.py b/src/tests/intg/test_ldap.py index a6659b1b7..10205be1f 100644 --- a/src/tests/intg/test_ldap.py +++ b/src/tests/intg/test_ldap.py @@ -94,10 +94,11 @@ def create_ldap_cleanup(request, ldap_conn, ent_list=None): request.addfinalizer(lambda: cleanup_ldap_entries(ldap_conn, ent_list)) -def create_ldap_fixture(request, ldap_conn, ent_list=None): +def create_ldap_fixture(request, ldap_conn, ent_list=None, cleanup=True): """Add LDAP entries and add teardown for removing them""" create_ldap_entries(ldap_conn, ent_list) - create_ldap_cleanup(request, ldap_conn, ent_list) + if cleanup: + create_ldap_cleanup(request, ldap_conn, ent_list) SCHEMA_RFC2307 = "rfc2307" @@ -1383,3 +1384,129 @@ def test_ldap_auto_private_groups_direct_no_gid(ldap_conn, mpg_setup_no_gid): ", ".join(["%s" % s for s in sorted(gids)]), ", ".join(["%s" % s for s in sorted(user1_expected_gids)]) ) + + +def rename_setup_no_cleanup(request, ldap_conn, cleanup_ent=None): + ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) + ent_list.add_user("user1", 1001, 2001) + ent_list.add_group_bis("user1_private", 2001) + ent_list.add_group_bis("group1", 2015, ["user1"]) + + if cleanup_ent is None: + create_ldap_fixture(request, ldap_conn, ent_list) + else: + # Since the entries were renamed, we need to clean up + # the renamed entries.. + create_ldap_fixture(request, ldap_conn, ent_list, cleanup=False) + create_ldap_cleanup(request, ldap_conn, None) + + +@pytest.fixture +def rename_setup_cleanup(request, ldap_conn): + cleanup_ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn) + cleanup_ent_list.add_user("user1", 1001, 2001) + cleanup_ent_list.add_group_bis("new_user1_private", 2001) + cleanup_ent_list.add_group_bis("new_group1", 2015, ["user1"]) + + rename_setup_no_cleanup(request, ldap_conn, cleanup_ent_list) + + conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS) + create_conf_fixture(request, conf) + create_sssd_fixture(request) + return None + + +@pytest.fixture +def rename_setup_with_name(request, ldap_conn): + rename_setup_no_cleanup(request, ldap_conn) + + conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS) + \ + unindent(""" + [nss] + [domain/LDAP] + ldap_group_name = name + timeout = 3000 + """).format(**locals()) + create_conf_fixture(request, conf) + create_sssd_fixture(request) + return None + + +def test_rename_incomplete_group_same_dn(ldap_conn, rename_setup_with_name): + """ + Test that if a group's name attribute changes, but the DN stays the same, + the incomplete group object will be renamed. + + Because the RDN attribute must be present in the entry, we add another + attribute "name" that is purposefully different from the CN and make + sure the group names are reflected in name + + Regression test for https://pagure.io/SSSD/sssd/issue/3282 + """ + pvt_dn = 'cn=user1_private,ou=Groups,' + ldap_conn.ds_inst.base_dn + group1_dn = 'cn=group1,ou=Groups,' + ldap_conn.ds_inst.base_dn + + # Add the name we want for both private and secondary group + old = {'name': []} + new = {'name': [b"user1_group1"]} + ldif = ldap.modlist.modifyModlist(old, new) + ldap_conn.modify_s(group1_dn, ldif) + + new = {'name': [b"pvt_user1"]} + ldif = ldap.modlist.modifyModlist(old, new) + ldap_conn.modify_s(pvt_dn, ldif) + + # Make sure the old name shows up in the id output + (res, errno, grp_list) = sssd_id.get_user_groups("user1") + assert res == sssd_id.NssReturnCode.SUCCESS, \ + "Could not find groups for user1, %d" % errno + + assert sorted(grp_list) == sorted(["pvt_user1", "user1_group1"]) + + # Rename the group by changing the cn attribute, but keep the DN the same + old = {'name': [b"user1_group1"]} + new = {'name': [b"new_user1_group1"]} + ldif = ldap.modlist.modifyModlist(old, new) + ldap_conn.modify_s(group1_dn, ldif) + + if subprocess.call(["sss_cache", "-GU"]) != 0: + raise Exception("sssd_cache failed") + + (res, errno, grp_list) = sssd_id.get_user_groups("user1") + assert res == sssd_id.NssReturnCode.SUCCESS, \ + "Could not find groups for user1, %d" % errno + + assert sorted(grp_list) == sorted(["pvt_user1", "new_user1_group1"]) + + +def test_rename_incomplete_group_rdn_changed(ldap_conn, rename_setup_cleanup): + """ + Test that if a group's name attribute changes, and the DN changes with + the RDN. Then adding the second group will fail because we can't tell if + there are two duplicate groups in LDAP when saving the group or if the + group was renamed. + + Please note that with many directories (AD, IPA), the code can rely on + other heuristics (SID, UUID) to find out the group is in fact the same. + + Regression test for https://pagure.io/SSSD/sssd/issue/3282 + """ + pvt_dn = 'cn=user1_private,ou=Groups,' + ldap_conn.ds_inst.base_dn + group1_dn = 'cn=group1,ou=Groups,' + ldap_conn.ds_inst.base_dn + + # Make sure the old name shows up in the id output + (res, errno, grp_list) = sssd_id.get_user_groups("user1") + assert res == sssd_id.NssReturnCode.SUCCESS, \ + "Could not find groups for user1, %d" % errno + + assert sorted(grp_list) == sorted(["user1_private", "group1"]) + + # Rename the groups, changing the RDN + ldap_conn.rename_s(group1_dn, "cn=new_group1") + ldap_conn.rename_s(pvt_dn, "cn=new_user1_private") + + if subprocess.call(["sss_cache", "-GU"]) != 0: + raise Exception("sssd_cache failed") + + # The initgroups fails and we fall back to the old names in the cache + assert sorted(grp_list) == sorted(["user1_private", "group1"])
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
