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 7bc29db7b20724f1b63bdd2c225880db35e2df7e 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 1/8] 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 1ad1f0d9bc2287654369d7248aa0e3a9625f8c5d 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 2/8] 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 | 45 ++++++++++++++++++++++++++++ 2 files changed, 47 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..a61f7c59d 100644 --- a/src/providers/data_provider/dp_resp_client.c +++ b/src/providers/data_provider/dp_resp_client.c @@ -189,3 +189,48 @@ 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; + + if (provider == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, "No provider pointer\n"); + return; + } + + 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 813404b2d51fea6de71dc49386b8c1fd1fabb803 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 3/8] 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 2fea07153736e5f39ff5f42bc518d96219965d26 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Mon, 9 Apr 2018 12:36:45 +0200 Subject: [PATCH 4/8] LDAP: Augment the sdap_opts structure with a data provider pointer In order to be able to use the Data Provider methods from the SDAP code to e.g. invalidate memcache when needed, add a new field to the sdap_options structure with the data_provider structure pointer. Fill the pointer value for all LDAP-based providers. --- src/providers/ad/ad_common.c | 18 +++++++++++++----- src/providers/ad/ad_common.h | 4 ++++ src/providers/ad/ad_init.c | 5 ++++- src/providers/ad/ad_subdomains.c | 8 ++++++-- src/providers/ipa/ipa_common.c | 2 ++ src/providers/ipa/ipa_common.h | 1 + src/providers/ipa/ipa_init.c | 5 ++++- src/providers/ipa/ipa_subdomains_server.c | 2 ++ src/providers/ldap/ldap_common.h | 1 + src/providers/ldap/ldap_init.c | 3 ++- src/providers/ldap/ldap_options.c | 2 ++ src/providers/ldap/sdap.h | 1 + src/tests/cmocka/common_mock_sdap.c | 2 +- src/tests/cmocka/test_ad_common.c | 3 +++ 14 files changed, 46 insertions(+), 11 deletions(-) diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c index 2a1647173..d92c68e6f 100644 --- a/src/providers/ad/ad_common.c +++ b/src/providers/ad/ad_common.c @@ -35,7 +35,8 @@ static errno_t ad_set_sdap_options(struct ad_options *ad_opts, struct sdap_options *id_opts); static struct sdap_options * -ad_create_default_sdap_options(TALLOC_CTX *mem_ctx) +ad_create_default_sdap_options(TALLOC_CTX *mem_ctx, + struct data_provider *dp) { struct sdap_options *id_opts; errno_t ret; @@ -44,6 +45,7 @@ ad_create_default_sdap_options(TALLOC_CTX *mem_ctx) if (!id_opts) { return NULL; } + id_opts->dp = dp; ret = dp_copy_defaults(id_opts, ad_def_ldap_opts, @@ -112,6 +114,7 @@ static errno_t ad_create_sdap_options(TALLOC_CTX *mem_ctx, struct confdb_ctx *cdb, const char *conf_path, + struct data_provider *dp, struct sdap_options **_id_opts) { struct sdap_options *id_opts; @@ -119,7 +122,7 @@ ad_create_sdap_options(TALLOC_CTX *mem_ctx, if (cdb == NULL || conf_path == NULL) { /* Fallback to defaults if there is no confdb */ - id_opts = ad_create_default_sdap_options(mem_ctx); + id_opts = ad_create_default_sdap_options(mem_ctx, dp); if (id_opts == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, "Failed to initialize default sdap options\n"); @@ -220,6 +223,7 @@ struct ad_options * ad_create_options(TALLOC_CTX *mem_ctx, struct confdb_ctx *cdb, const char *conf_path, + struct data_provider *dp, struct sss_domain_info *subdom) { struct ad_options *ad_options; @@ -252,6 +256,7 @@ ad_create_options(TALLOC_CTX *mem_ctx, ret = ad_create_sdap_options(ad_options, cdb, conf_path, + dp, &ad_options->id); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "Cannot initialize AD LDAP options\n"); @@ -304,6 +309,7 @@ struct ad_options * ad_create_2way_trust_options(TALLOC_CTX *mem_ctx, struct confdb_ctx *cdb, const char *conf_path, + struct data_provider *dp, const char *realm, struct sss_domain_info *subdom, const char *hostname, @@ -315,7 +321,7 @@ ad_create_2way_trust_options(TALLOC_CTX *mem_ctx, DEBUG(SSSDBG_TRACE_FUNC, "2way trust is defined to domain '%s'\n", subdom->name); - ad_options = ad_create_options(mem_ctx, cdb, conf_path, subdom); + ad_options = ad_create_options(mem_ctx, cdb, conf_path, dp, subdom); if (ad_options == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, "ad_create_options failed\n"); return NULL; @@ -343,6 +349,7 @@ struct ad_options * ad_create_1way_trust_options(TALLOC_CTX *mem_ctx, struct confdb_ctx *cdb, const char *subdom_conf_path, + struct data_provider *dp, struct sss_domain_info *subdom, const char *hostname, const char *keytab, @@ -355,7 +362,7 @@ ad_create_1way_trust_options(TALLOC_CTX *mem_ctx, DEBUG(SSSDBG_TRACE_FUNC, "1way trust is defined to domain '%s'\n", subdom->name); - ad_options = ad_create_options(mem_ctx, cdb, subdom_conf_path, subdom); + ad_options = ad_create_options(mem_ctx, cdb, subdom_conf_path, dp, subdom); if (ad_options == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, "ad_create_options failed\n"); return NULL; @@ -1056,12 +1063,13 @@ errno_t ad_get_id_options(struct ad_options *ad_opts, struct confdb_ctx *cdb, const char *conf_path, + struct data_provider *dp, struct sdap_options **_opts) { struct sdap_options *id_opts; errno_t ret; - ret = ad_create_sdap_options(ad_opts, cdb, conf_path, &id_opts); + ret = ad_create_sdap_options(ad_opts, cdb, conf_path, dp, &id_opts); if (ret != EOK) { return ENOMEM; } diff --git a/src/providers/ad/ad_common.h b/src/providers/ad/ad_common.h index 931aafc6c..6eb2ba7e9 100644 --- a/src/providers/ad/ad_common.h +++ b/src/providers/ad/ad_common.h @@ -112,11 +112,13 @@ ad_get_common_options(TALLOC_CTX *mem_ctx, struct ad_options *ad_create_options(TALLOC_CTX *mem_ctx, struct confdb_ctx *cdb, const char *conf_path, + struct data_provider *dp, struct sss_domain_info *subdom); struct ad_options *ad_create_2way_trust_options(TALLOC_CTX *mem_ctx, struct confdb_ctx *cdb, const char *conf_path, + struct data_provider *dp, const char *realm, struct sss_domain_info *subdom, const char *hostname, @@ -125,6 +127,7 @@ struct ad_options *ad_create_2way_trust_options(TALLOC_CTX *mem_ctx, struct ad_options *ad_create_1way_trust_options(TALLOC_CTX *mem_ctx, struct confdb_ctx *cdb, const char *conf_path, + struct data_provider *dp, struct sss_domain_info *subdom, const char *hostname, const char *keytab, @@ -147,6 +150,7 @@ errno_t ad_get_id_options(struct ad_options *ad_opts, struct confdb_ctx *cdb, const char *conf_path, + struct data_provider *dp, struct sdap_options **_opts); errno_t ad_get_autofs_options(struct ad_options *ad_opts, diff --git a/src/providers/ad/ad_init.c b/src/providers/ad/ad_init.c index 8c485a7c2..b19624782 100644 --- a/src/providers/ad/ad_init.c +++ b/src/providers/ad/ad_init.c @@ -453,7 +453,10 @@ errno_t sssm_ad_init(TALLOC_CTX *mem_ctx, init_ctx->options->id_ctx = init_ctx->id_ctx; - ret = ad_get_id_options(init_ctx->options, be_ctx->cdb, be_ctx->conf_path, + ret = ad_get_id_options(init_ctx->options, + be_ctx->cdb, + be_ctx->conf_path, + be_ctx->provider, &init_ctx->id_ctx->sdap_id_ctx->opts); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "Unable to init AD id options\n"); diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c index bd94ba8ea..74b9f0751 100644 --- a/src/providers/ad/ad_subdomains.c +++ b/src/providers/ad/ad_subdomains.c @@ -265,8 +265,12 @@ ad_subdom_ad_ctx_new(struct be_ctx *be_ctx, return ENOMEM; } - ad_options = ad_create_2way_trust_options(id_ctx, be_ctx->cdb, - subdom_conf_path, realm, subdom, + ad_options = ad_create_2way_trust_options(id_ctx, + be_ctx->cdb, + subdom_conf_path, + be_ctx->provider, + realm, + subdom, hostname, keytab); talloc_free(subdom_conf_path); if (ad_options == NULL) { diff --git a/src/providers/ipa/ipa_common.c b/src/providers/ipa/ipa_common.c index 2b81d7f3f..87ed96767 100644 --- a/src/providers/ipa/ipa_common.c +++ b/src/providers/ipa/ipa_common.c @@ -171,6 +171,7 @@ static errno_t ipa_parse_search_base(TALLOC_CTX *mem_ctx, int ipa_get_id_options(struct ipa_options *ipa_opts, struct confdb_ctx *cdb, const char *conf_path, + struct data_provider *dp, struct sdap_options **_opts) { TALLOC_CTX *tmpctx; @@ -190,6 +191,7 @@ int ipa_get_id_options(struct ipa_options *ipa_opts, ret = ENOMEM; goto done; } + ipa_opts->id->dp = dp; ret = sdap_domain_add(ipa_opts->id, ipa_opts->id_ctx->sdap_id_ctx->be->domain, diff --git a/src/providers/ipa/ipa_common.h b/src/providers/ipa/ipa_common.h index 3a1259ccd..725e0e937 100644 --- a/src/providers/ipa/ipa_common.h +++ b/src/providers/ipa/ipa_common.h @@ -235,6 +235,7 @@ int ipa_get_options(TALLOC_CTX *memctx, int ipa_get_id_options(struct ipa_options *ipa_opts, struct confdb_ctx *cdb, const char *conf_path, + struct data_provider *dp, struct sdap_options **_opts); int ipa_get_auth_options(struct ipa_options *ipa_opts, diff --git a/src/providers/ipa/ipa_init.c b/src/providers/ipa/ipa_init.c index cd2227896..931145985 100644 --- a/src/providers/ipa/ipa_init.c +++ b/src/providers/ipa/ipa_init.c @@ -161,7 +161,10 @@ static errno_t ipa_init_id_ctx(TALLOC_CTX *mem_ctx, ipa_id_ctx->sdap_id_ctx = sdap_id_ctx; ipa_options->id_ctx = ipa_id_ctx; - ret = ipa_get_id_options(ipa_options, be_ctx->cdb, be_ctx->conf_path, + ret = ipa_get_id_options(ipa_options, + be_ctx->cdb, + be_ctx->conf_path, + be_ctx->provider, &sdap_id_ctx->opts); if (ret != EOK) { goto done; diff --git a/src/providers/ipa/ipa_subdomains_server.c b/src/providers/ipa/ipa_subdomains_server.c index d670a156b..1e53e7a95 100644 --- a/src/providers/ipa/ipa_subdomains_server.c +++ b/src/providers/ipa/ipa_subdomains_server.c @@ -148,6 +148,7 @@ ipa_create_1way_trust_ctx(struct ipa_id_ctx *id_ctx, ad_options = ad_create_1way_trust_options(id_ctx, be_ctx->cdb, subdom_conf_path, + be_ctx->provider, subdom, id_ctx->server_mode->hostname, keytab, @@ -186,6 +187,7 @@ static struct ad_options *ipa_ad_options_new(struct be_ctx *be_ctx, ad_options = ad_create_2way_trust_options(id_ctx, be_ctx->cdb, subdom_conf_path, + be_ctx->provider, id_ctx->server_mode->realm, subdom, id_ctx->server_mode->hostname, diff --git a/src/providers/ldap/ldap_common.h b/src/providers/ldap/ldap_common.h index 44dbc3fb0..548f0f985 100644 --- a/src/providers/ldap/ldap_common.h +++ b/src/providers/ldap/ldap_common.h @@ -193,6 +193,7 @@ int ldap_get_options(TALLOC_CTX *memctx, struct sss_domain_info *dom, struct confdb_ctx *cdb, const char *conf_path, + struct data_provider *dp, struct sdap_options **_opts); int ldap_get_sudo_options(struct confdb_ctx *cdb, diff --git a/src/providers/ldap/ldap_init.c b/src/providers/ldap/ldap_init.c index 83075b5d3..44b3e9ab3 100644 --- a/src/providers/ldap/ldap_init.c +++ b/src/providers/ldap/ldap_init.c @@ -458,7 +458,8 @@ errno_t sssm_ldap_init(TALLOC_CTX *mem_ctx, /* Always initialize options since it is needed everywhere. */ ret = ldap_get_options(init_ctx, be_ctx->domain, be_ctx->cdb, - be_ctx->conf_path, &init_ctx->options); + be_ctx->conf_path, be_ctx->provider, + &init_ctx->options); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "Unable to initialize LDAP options " "[%d]: %s\n", ret, sss_strerror(ret)); diff --git a/src/providers/ldap/ldap_options.c b/src/providers/ldap/ldap_options.c index ccc1a2c5b..0b79715d2 100644 --- a/src/providers/ldap/ldap_options.c +++ b/src/providers/ldap/ldap_options.c @@ -27,6 +27,7 @@ int ldap_get_options(TALLOC_CTX *memctx, struct sss_domain_info *dom, struct confdb_ctx *cdb, const char *conf_path, + struct data_provider *dp, struct sdap_options **_opts) { struct sdap_attr_map *default_attr_map; @@ -57,6 +58,7 @@ int ldap_get_options(TALLOC_CTX *memctx, opts = talloc_zero(memctx, struct sdap_options); if (!opts) return ENOMEM; + opts->dp = dp; ret = sdap_domain_add(opts, dom, NULL); if (ret != EOK) { diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h index ecf9c4d2e..e892c4071 100644 --- a/src/providers/ldap/sdap.h +++ b/src/providers/ldap/sdap.h @@ -465,6 +465,7 @@ struct sdap_certmap_ctx; struct sdap_options { struct dp_option *basic; + struct data_provider *dp; struct sdap_attr_map *gen_map; struct sdap_attr_map *user_map; size_t user_map_cnt; diff --git a/src/tests/cmocka/common_mock_sdap.c b/src/tests/cmocka/common_mock_sdap.c index cef321613..fa4787c4b 100644 --- a/src/tests/cmocka/common_mock_sdap.c +++ b/src/tests/cmocka/common_mock_sdap.c @@ -48,7 +48,7 @@ struct sdap_options *mock_sdap_options_ldap(TALLOC_CTX *mem_ctx, struct sdap_options *opts = NULL; errno_t ret; - ret = ldap_get_options(mem_ctx, domain, confdb_ctx, conf_path, &opts); + ret = ldap_get_options(mem_ctx, domain, confdb_ctx, conf_path, NULL, &opts); if (ret != EOK) { return NULL; } diff --git a/src/tests/cmocka/test_ad_common.c b/src/tests/cmocka/test_ad_common.c index 94f351e19..39ebbc633 100644 --- a/src/tests/cmocka/test_ad_common.c +++ b/src/tests/cmocka/test_ad_common.c @@ -449,6 +449,7 @@ static void test_ad_create_1way_trust_options(void **state) test_ctx->ad_ctx, NULL, NULL, + NULL, test_ctx->subdom, ONEWAY_HOST_NAME, ONEWAY_KEYTAB_PATH, @@ -515,6 +516,7 @@ static void test_ad_create_2way_trust_options(void **state) test_ctx->ad_ctx, NULL, NULL, + NULL, REALMNAME, test_ctx->subdom, HOST_NAME, @@ -585,6 +587,7 @@ test_ldap_conn_setup(void **state) ad_ctx, NULL, NULL, + NULL, REALMNAME, test_ctx->subdom, HOST_NAME, From f82a12fe5c01c93ff42d096d34ac16ee03f73cd3 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 5/8] 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 40da81fb9..6ca3ed8d8 100644 --- a/src/providers/ldap/sdap_async.h +++ b/src/providers/ldap/sdap_async.h @@ -412,4 +412,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(struct data_provider *dp, + 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 326294a1c..34747be59 100644 --- a/src/providers/ldap/sdap_async_initgroups.c +++ b/src/providers/ldap/sdap_async_initgroups.c @@ -3543,3 +3543,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(struct data_provider *dp, + 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(dp, gid); + + return EOK; +} From cdb0e5f60d63ee51de59cf195a028fef42a7fd98 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 6/8] 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/ad/ad_pac.c | 3 +++ src/providers/ldap/sdap_async_ad.h | 1 + src/providers/ldap/sdap_async_initgroups.c | 13 +++++++++++++ src/providers/ldap/sdap_async_initgroups_ad.c | 15 +++++++++++++++ 4 files changed, 32 insertions(+) diff --git a/src/providers/ad/ad_pac.c b/src/providers/ad/ad_pac.c index 6b47462cf..1a344725f 100644 --- a/src/providers/ad/ad_pac.c +++ b/src/providers/ad/ad_pac.c @@ -434,6 +434,7 @@ struct ad_handle_pac_initgr_state { const char *err; int dp_error; int sdap_ret; + struct sdap_options *opts; size_t num_missing_sids; char **missing_sids; @@ -471,6 +472,7 @@ struct tevent_req *ad_handle_pac_initgr_send(TALLOC_CTX *mem_ctx, return NULL; } state->user_dom = sdom->dom; + state->opts = id_ctx->opts; /* The following variables are currently unused because no sub-request * returns any of them. But they are needed to allow the same signature as @@ -514,6 +516,7 @@ struct tevent_req *ad_handle_pac_initgr_send(TALLOC_CTX *mem_ctx, DEBUG(SSSDBG_TRACE_ALL, "Running PAC processing with id-mapping.\n"); ret = sdap_ad_save_group_membership_with_idmapping(state->username, + state->opts, sdom->dom, id_ctx->opts->idmap_ctx, num_sids, group_sids); diff --git a/src/providers/ldap/sdap_async_ad.h b/src/providers/ldap/sdap_async_ad.h index 950f5a030..a5f47a1a9 100644 --- a/src/providers/ldap/sdap_async_ad.h +++ b/src/providers/ldap/sdap_async_ad.h @@ -25,6 +25,7 @@ #define SDAP_ASYNC_AD_H_ errno_t sdap_ad_save_group_membership_with_idmapping(const char *username, + struct sdap_options *opts, struct sss_domain_info *user_dom, struct sdap_idmap_ctx *idmap_ctx, size_t num_sids, diff --git a/src/providers/ldap/sdap_async_initgroups.c b/src/providers/ldap/sdap_async_initgroups.c index 34747be59..03f6de01a 100644 --- a/src/providers/ldap/sdap_async_initgroups.c +++ b/src/providers/ldap/sdap_async_initgroups.c @@ -225,6 +225,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( + opts->dp, 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 30f1d3db2..eab103652 100644 --- a/src/providers/ldap/sdap_async_initgroups_ad.c +++ b/src/providers/ldap/sdap_async_initgroups_ad.c @@ -836,6 +836,7 @@ sdap_ad_tokengroups_initgr_mapping_connect_done(struct tevent_req *subreq) } errno_t sdap_ad_save_group_membership_with_idmapping(const char *username, + struct sdap_options *opts, struct sss_domain_info *user_dom, struct sdap_idmap_ctx *idmap_ctx, size_t num_sids, @@ -921,6 +922,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)); @@ -992,6 +1006,7 @@ static void sdap_ad_tokengroups_initgr_mapping_done(struct tevent_req *subreq) } ret = sdap_ad_save_group_membership_with_idmapping(state->username, + state->opts, state->domain, state->idmap_ctx, num_sids, From 14c2a48341c9d5388760517826095a4964a61ec6 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 7/8] 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 5d3cf643d..de4fdb592 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -2377,12 +2377,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 7cd45e61526aa270c959ae665aaf9141488be669 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Mon, 19 Feb 2018 18:26:05 +0100 Subject: [PATCH 8/8] 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: 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 08e30f24e..9bbdd4329 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" @@ -1437,3 +1438,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
