URL: https://github.com/SSSD/sssd/pull/347 Author: fidencio Title: #347: Fixes related to negative cache and "root" user/group Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/347/head:pr347 git checkout pr347
From 336d887979a097213456541be51e6dbb9223b228 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 14 Aug 2017 15:28:41 +0200 Subject: [PATCH 01/11] NEGCACHE: Add some comments about each step of sss_ncache_prepopulate() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comments help to understand which part of the code is dealing with users or groups of specific or non-specific domain filters. Related: https://pagure.io/SSSD/sssd/issue/3460 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/responder/common/negcache.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/responder/common/negcache.c b/src/responder/common/negcache.c index 084c47aa8..376c3e656 100644 --- a/src/responder/common/negcache.c +++ b/src/responder/common/negcache.c @@ -786,7 +786,7 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, return ENOMEM; } - /* Populate domain-specific negative cache entries */ + /* Populate domain-specific negative cache user entries */ for (dom = domain_list; dom; dom = get_next_domain(dom, 0)) { conf_path = talloc_asprintf(tmpctx, CONFDB_DOMAIN_PATH_TMPL, dom->name); @@ -844,6 +844,7 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, } } + /* Populate non domain-specific negative cache user entries */ ret = confdb_get_string_as_list(cdb, tmpctx, CONFDB_NSS_CONF_ENTRY, CONFDB_NSS_FILTER_USERS, &filter_list); if (ret == ENOENT) { @@ -920,6 +921,7 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, } } + /* Populate domain-specific negative cache group entries */ filter_set = false; for (dom = domain_list; dom; dom = get_next_domain(dom, 0)) { conf_path = talloc_asprintf(tmpctx, CONFDB_DOMAIN_PATH_TMPL, dom->name); @@ -970,6 +972,7 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, } } + /* Populate non domain-specific negative cache group entries */ ret = confdb_get_string_as_list(cdb, tmpctx, CONFDB_NSS_CONF_ENTRY, CONFDB_NSS_FILTER_GROUPS, &filter_list); if (ret == ENOENT) { From 48ef721064cbb850a9f5bd55b0d224719c9f1b96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 14 Aug 2017 15:46:10 +0200 Subject: [PATCH 02/11] NEGCACHE: Always add "root" to the negative cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current code only adds "root" to the negative cache in case there's any other user or group set up in to be added. As SSSD doesn't handle "root", it should *always* be added to the negative cache. Related: https://pagure.io/SSSD/sssd/issue/3460 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/responder/common/negcache.c | 88 +++++++++++++++++++++++++---------------- 1 file changed, 54 insertions(+), 34 deletions(-) diff --git a/src/responder/common/negcache.c b/src/responder/common/negcache.c index 376c3e656..fc5ae76bc 100644 --- a/src/responder/common/negcache.c +++ b/src/responder/common/negcache.c @@ -771,8 +771,8 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, struct resp_ctx *rctx) { errno_t ret; - bool filter_set = false; char **filter_list = NULL; + char **default_list = NULL; char *name = NULL; struct sss_domain_info *dom = NULL; struct sss_domain_info *domain_list = rctx->domains; @@ -801,7 +801,6 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, &filter_list); if (ret == ENOENT) continue; if (ret != EOK) goto done; - filter_set = true; for (i = 0; (filter_list && filter_list[i]); i++) { ret = sss_parse_name_for_domains(tmpctx, domain_list, @@ -847,22 +846,9 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, /* Populate non domain-specific negative cache user entries */ ret = confdb_get_string_as_list(cdb, tmpctx, CONFDB_NSS_CONF_ENTRY, CONFDB_NSS_FILTER_USERS, &filter_list); - if (ret == ENOENT) { - if (!filter_set) { - filter_list = talloc_array(tmpctx, char *, 2); - if (!filter_list) { - ret = ENOMEM; - goto done; - } - filter_list[0] = talloc_strdup(tmpctx, "root"); - if (!filter_list[0]) { - ret = ENOMEM; - goto done; - } - filter_list[1] = NULL; - } + if (ret != EOK && ret != ENOENT) { + goto done; } - else if (ret != EOK) goto done; for (i = 0; (filter_list && filter_list[i]); i++) { ret = sss_parse_name_for_domains(tmpctx, domain_list, @@ -922,7 +908,6 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, } /* Populate domain-specific negative cache group entries */ - filter_set = false; for (dom = domain_list; dom; dom = get_next_domain(dom, 0)) { conf_path = talloc_asprintf(tmpctx, CONFDB_DOMAIN_PATH_TMPL, dom->name); if (!conf_path) { @@ -935,7 +920,6 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, CONFDB_NSS_FILTER_GROUPS, &filter_list); if (ret == ENOENT) continue; if (ret != EOK) goto done; - filter_set = true; for (i = 0; (filter_list && filter_list[i]); i++) { ret = sss_parse_name(tmpctx, dom->names, filter_list[i], @@ -975,22 +959,9 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, /* Populate non domain-specific negative cache group entries */ ret = confdb_get_string_as_list(cdb, tmpctx, CONFDB_NSS_CONF_ENTRY, CONFDB_NSS_FILTER_GROUPS, &filter_list); - if (ret == ENOENT) { - if (!filter_set) { - filter_list = talloc_array(tmpctx, char *, 2); - if (!filter_list) { - ret = ENOMEM; - goto done; - } - filter_list[0] = talloc_strdup(tmpctx, "root"); - if (!filter_list[0]) { - ret = ENOMEM; - goto done; - } - filter_list[1] = NULL; - } + if (ret != EOK && ret != ENOENT) { + goto done; } - else if (ret != EOK) goto done; for (i = 0; (filter_list && filter_list[i]); i++) { ret = sss_parse_name_for_domains(tmpctx, domain_list, @@ -1049,6 +1020,55 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, } } + /* SSSD doesn't handle "root", thus it'll be added to the negative cache + * nonetheless what's already added there. */ + default_list = talloc_array(tmpctx, char *, 2); + if (default_list == NULL) { + ret= ENOMEM; + goto done; + } + default_list[0] = talloc_strdup(tmpctx, "root"); + if (default_list[0] == NULL) { + ret = ENOMEM; + goto done; + } + default_list[1] = NULL; + + /* Populate negative cache users and groups entries for the + * "default_list" */ + for (i = 0; (default_list != NULL && default_list[i] != NULL); i++) { + for (dom = domain_list; + dom != NULL; + dom = get_next_domain(dom, SSS_GND_ALL_DOMAINS)) { + fqname = sss_create_internal_fqname(tmpctx, + default_list[i], + dom->name); + if (fqname == NULL) { + continue; + } + + ret = sss_ncache_set_user(ncache, true, dom, fqname); + if (ret != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, + "Failed to store permanent user filter for" + " [%s:%s] (%d [%s])\n", + dom->name, default_list[i], + ret, strerror(ret)); + continue; + } + + ret = sss_ncache_set_group(ncache, true, dom, fqname); + if (ret != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, + "Failed to store permanent group filter for" + " [%s:%s] (%d [%s])\n", + dom->name, default_list[i], + ret, strerror(ret)); + continue; + } + } + } + ret = EOK; done: From 855ab971dcf660226178d820f771b02163dc8fd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Wed, 16 Aug 2017 10:45:19 +0200 Subject: [PATCH 03/11] TEST_NEGCACHE: Test that "root" is always added to ncache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simply modify test_sss_ncache_prepopulate() in order to ensure that "root" user and group are always added to the negative cache, no matter whether they're set as part of the filter_users or filter_groups options. Related: https://pagure.io/SSSD/sssd/issue/3460 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/tests/cmocka/test_negcache.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/tests/cmocka/test_negcache.c b/src/tests/cmocka/test_negcache.c index d608c20ad..163e4653e 100644 --- a/src/tests/cmocka/test_negcache.c +++ b/src/tests/cmocka/test_negcache.c @@ -630,6 +630,12 @@ static void test_sss_ncache_prepopulate(void **state) ret = check_group_in_ncache(ncache, dom, "testgroup3@somedomain"); assert_int_equal(ret, ENOENT); + + ret = check_user_in_ncache(ncache, dom, "root"); + assert_int_equal(ret, EEXIST); + + ret = check_group_in_ncache(ncache, dom, "root"); + assert_int_equal(ret, EEXIST); } static void test_sss_ncache_default_domain_suffix(void **state) From bf3e341a4c7eac4b1e1955f34a042bf6d6ac75bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 14 Aug 2017 12:15:42 +0200 Subject: [PATCH 04/11] NEGCACHE: Descend to all subdomains when adding user/groups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a user or group is added to the negative cache, we should descend to all subdomains as well. Related: https://pagure.io/SSSD/sssd/issue/3460 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/responder/common/negcache.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/responder/common/negcache.c b/src/responder/common/negcache.c index fc5ae76bc..00487a224 100644 --- a/src/responder/common/negcache.c +++ b/src/responder/common/negcache.c @@ -887,7 +887,9 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, continue; } } else { - for (dom = domain_list; dom; dom = get_next_domain(dom, 0)) { + for (dom = domain_list; + dom != NULL; + dom = get_next_domain(dom, SSS_GND_ALL_DOMAINS)) { fqname = sss_create_internal_fqname(tmpctx, name, dom->name); if (fqname == NULL) { continue; @@ -1000,7 +1002,9 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, continue; } } else { - for (dom = domain_list; dom; dom = get_next_domain(dom, 0)) { + for (dom = domain_list; + dom != NULL; + dom = get_next_domain(dom, SSS_GND_ALL_DOMAINS)) { fqname = sss_create_internal_fqname(tmpctx, name, dom->name); if (fqname == NULL) { continue; From 2b91703af8c015e4412741b301507db026b66671 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 14 Aug 2017 13:35:20 +0200 Subject: [PATCH 05/11] CACHE_REQ: Don't error out when searching by id = 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This code path can be easily triggered by calling `id 0` and SSSD should not error out in this case. Previous patches in this series already add uid and gid 0 to the negative cache and we can properly handle this situation. Related: https://pagure.io/SSSD/sssd/issue/3460 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/responder/common/cache_req/cache_req_data.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/responder/common/cache_req/cache_req_data.c b/src/responder/common/cache_req/cache_req_data.c index 5ab1493b8..8726e139f 100644 --- a/src/responder/common/cache_req/cache_req_data.c +++ b/src/responder/common/cache_req/cache_req_data.c @@ -119,12 +119,6 @@ cache_req_data_create(TALLOC_CTX *mem_ctx, case CACHE_REQ_USER_BY_ID: case CACHE_REQ_GROUP_BY_ID: case CACHE_REQ_OBJECT_BY_ID: - if (input->id == 0) { - DEBUG(SSSDBG_CRIT_FAILURE, "Bug: id cannot be 0!\n"); - ret = ERR_INTERNAL; - goto done; - } - data->id = input->id; break; case CACHE_REQ_OBJECT_BY_SID: From 16a840d13bfab26a939ecdd48370fd7895c36801 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 14 Aug 2017 13:40:58 +0200 Subject: [PATCH 06/11] NSS: Don't error out when deleting an entry which has id = 0 from the memcache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This code path can be easily triggered by calling `id 0` after applying the previous patch in this series and SSSD should not error out in this case. As SSSD doesn't handle "root", this entry never will be part of the memcache and EOK can be safely returned there. Related: https://pagure.io/SSSD/sssd/issue/3460 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/responder/nss/nss_get_object.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/responder/nss/nss_get_object.c b/src/responder/nss/nss_get_object.c index 9058793ea..e56480af5 100644 --- a/src/responder/nss/nss_get_object.c +++ b/src/responder/nss/nss_get_object.c @@ -125,6 +125,12 @@ memcache_delete_entry(struct nss_ctx *nss_ctx, name, dom->name); continue; } + } else if (id == 0) { + /* + * As "root" is not handled by SSSD, let's just return EOK here + * instead of erroring out. + */ + return EOK; } else if (id != 0) { ret = memcache_delete_entry_by_id(nss_ctx, id, type); if (ret != EOK) { From 4470678db19df5d0ad6077390041bc900abc2fdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Mon, 14 Aug 2017 13:31:45 +0200 Subject: [PATCH 07/11] NEGCACHE: Add root's uid/gid to ncache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As "root" is not handled by SSSD, let's add its uid and gid to the negative cache as well. The reason it's added without specifying a domain is to follow how the negative cache is used by cache req's code when searching something by id. As the negative cache check for uid/gid, in the cache req code, is done after resolving the name, we can save one LDAP call to the data provider. Related: https://pagure.io/SSSD/sssd/issue/3460 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/responder/common/negcache.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/responder/common/negcache.c b/src/responder/common/negcache.c index 00487a224..b751d89ee 100644 --- a/src/responder/common/negcache.c +++ b/src/responder/common/negcache.c @@ -1073,6 +1073,23 @@ errno_t sss_ncache_prepopulate(struct sss_nc_ctx *ncache, } } + /* Also add "root" uid and gid to the negative cache */ + ret = sss_ncache_set_uid(ncache, true, NULL, 0); + if (ret != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, + "Failed to store permanent uid filter for root (0) " + "(%d [%s])\n", + ret, strerror(ret)); + } + + ret = sss_ncache_set_gid(ncache, true, NULL, 0); + if (ret != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, + "Failed to store permanent gid filter for root (0) " + "(%d [%s])\n", + ret, strerror(ret)); + } + ret = EOK; done: From be903cf057259fe80f38ed789d47873bcadde60d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Wed, 16 Aug 2017 10:51:47 +0200 Subject: [PATCH 08/11] TEST_NEGCACHE: Ensure root's uid and gid are always added to ncache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In order to do so two new functions have been introduced and test_sss_ncache_prepopulate() has been modified in order to ensure that root's uid and gid are always added to the negative cache. Related: https://pagure.io/SSSD/sssd/issue/3460 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/tests/cmocka/test_negcache.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/tests/cmocka/test_negcache.c b/src/tests/cmocka/test_negcache.c index 163e4653e..ba39f778d 100644 --- a/src/tests/cmocka/test_negcache.c +++ b/src/tests/cmocka/test_negcache.c @@ -564,6 +564,24 @@ static int check_group_in_ncache(struct sss_nc_ctx *ctx, return ret; } +static int check_uid_in_ncache(struct sss_nc_ctx *ctx, + uid_t uid) +{ + int ret; + + ret = sss_ncache_check_uid(ctx, NULL, uid); + return ret; +} + +static int check_gid_in_ncache(struct sss_nc_ctx *ctx, + gid_t gid) +{ + int ret; + + ret = sss_ncache_check_gid(ctx, NULL, gid); + return ret; +} + static void test_sss_ncache_prepopulate(void **state) { int ret; @@ -636,6 +654,12 @@ static void test_sss_ncache_prepopulate(void **state) ret = check_group_in_ncache(ncache, dom, "root"); assert_int_equal(ret, EEXIST); + + ret = check_uid_in_ncache(ncache, 0); + assert_int_equal(ret, EEXIST); + + ret = check_gid_in_ncache(ncache, 0); + assert_int_equal(ret, EEXIST); } static void test_sss_ncache_default_domain_suffix(void **state) From 49f2711a9498df8ae100a39f8a470d974e3b1c2a Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Tue, 22 Aug 2017 12:25:31 +0200 Subject: [PATCH 09/11] TESTS: Add wrappers to request a user or a group by ID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/tests/intg/sssd_group.py | 43 ++++++++++++++++++++++++++++++++++++++++++- src/tests/intg/sssd_passwd.py | 43 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/src/tests/intg/sssd_group.py b/src/tests/intg/sssd_group.py index ab873a726..32d12cfae 100644 --- a/src/tests/intg/sssd_group.py +++ b/src/tests/intg/sssd_group.py @@ -52,6 +52,27 @@ def getgrnam_r(name, result_p, buffer_p, buflen): return (int(res), int(errno[0]), result_p) +def getgrgid_r(gid, result_p, buffer_p, buflen): + """ + ctypes wrapper for: + enum nss_status _nss_sss_getgrgid_r(gid_t gid, + struct passwd *result, + char *buffer, + size_t buflen, + int *errnop) + """ + func = nss_sss_ctypes_loader("_nss_sss_getgrgid_r") + func.restype = c_int + func.argtypes = [c_ulong, POINTER(Group), + c_char_p, c_ulong, POINTER(c_int)] + + errno = POINTER(c_int)(c_int(0)) + + res = func(gid, result_p, buffer_p, buflen, errno) + + return (int(res), int(errno[0]), result_p) + + def set_group_dict(res, result_p): if res != NssReturnCode.SUCCESS: return dict() @@ -72,7 +93,7 @@ def set_group_dict(res, result_p): def call_sssd_getgrnam(name): """ - A Python wrapper to retrieve a group. Returns: + A Python wrapper to retrieve a group by name. Returns: (res, group_dict) if res is NssReturnCode.SUCCESS, then group_dict contains the keys corresponding to the C passwd structure fields. Otherwise, the dictionary @@ -88,3 +109,23 @@ def call_sssd_getgrnam(name): group_dict = set_group_dict(res, result_p) return res, group_dict + + +def call_sssd_getgrgid(gid): + """ + A Python wrapper to retrieve a group by GID. Returns: + (res, group_dict) + if res is NssReturnCode.SUCCESS, then group_dict contains the keys + corresponding to the C passwd structure fields. Otherwise, the dictionary + is empty and errno indicates the error code + """ + result = Group() + result_p = POINTER(Group)(result) + buff = create_string_buffer(GROUP_BUFLEN) + + res, errno, result_p = getgrgid_r(gid, result_p, buff, GROUP_BUFLEN) + if errno != 0: + raise SssdNssError(errno, "getgrgid_r") + + group_dict = set_group_dict(res, result_p) + return res, group_dict diff --git a/src/tests/intg/sssd_passwd.py b/src/tests/intg/sssd_passwd.py index f285b4971..e97b0c11b 100644 --- a/src/tests/intg/sssd_passwd.py +++ b/src/tests/intg/sssd_passwd.py @@ -70,6 +70,27 @@ def getpwnam_r(name, result_p, buffer_p, buflen): return (int(res), int(errno[0]), result_p) +def getpwuid_r(uid, result_p, buffer_p, buflen): + """ + ctypes wrapper for: + enum nss_status _nss_sss_getpwuid_r(uid_t uid, + struct passwd *result, + char *buffer, + size_t buflen, + int *errnop) + """ + func = nss_sss_ctypes_loader("_nss_sss_getpwuid_r") + func.restype = c_int + func.argtypes = [c_ulong, POINTER(Passwd), + c_char_p, c_ulong, POINTER(c_int)] + + errno = POINTER(c_int)(c_int(0)) + + res = func(uid, result_p, buffer_p, buflen, errno) + + return (int(res), int(errno[0]), result_p) + + def setpwent(): """ ctypes wrapper for: @@ -134,7 +155,7 @@ def getpwent(): def call_sssd_getpwnam(name): """ - A Python wrapper to retrieve a user. Returns: + A Python wrapper to retrieve a user by name. Returns: (res, user_dict) if res is NssReturnCode.SUCCESS, then user_dict contains the keys corresponding to the C passwd structure fields. Otherwise, the dictionary @@ -152,6 +173,26 @@ def call_sssd_getpwnam(name): return res, user_dict +def call_sssd_getpwuid(uid): + """ + A Python wrapper to retrieve a user by UID. Returns: + (res, user_dict) + if res is NssReturnCode.SUCCESS, then user_dict contains the keys + corresponding to the C passwd structure fields. Otherwise, the dictionary + is empty and errno indicates the error code + """ + result = Passwd() + result_p = POINTER(Passwd)(result) + buff = create_string_buffer(PASSWD_BUFLEN) + + res, errno, result_p = getpwuid_r(uid, result_p, buff, PASSWD_BUFLEN) + if errno != 0: + raise SssdNssError(errno, "getpwuid_r") + + user_dict = set_user_dict(res, result_p) + return res, user_dict + + def call_sssd_enumeration(): """ enumerate users from sssd module only From c3cf78ad875dc8e528386cf6d250029d14bb002e Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Tue, 22 Aug 2017 12:25:58 +0200 Subject: [PATCH 10/11] TESTS: Add files provider tests that request a user and group by ID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/tests/intg/test_files_provider.py | 97 ++++++++++++++++++++++++++++++++--- 1 file changed, 91 insertions(+), 6 deletions(-) diff --git a/src/tests/intg/test_files_provider.py b/src/tests/intg/test_files_provider.py index b26977e06..e507ea10d 100644 --- a/src/tests/intg/test_files_provider.py +++ b/src/tests/intg/test_files_provider.py @@ -29,8 +29,10 @@ import ent import sssd_id from sssd_nss import NssReturnCode -from sssd_passwd import call_sssd_getpwnam, call_sssd_enumeration -from sssd_group import call_sssd_getgrnam +from sssd_passwd import (call_sssd_getpwnam, + call_sssd_enumeration, + call_sssd_getpwuid) +from sssd_group import call_sssd_getgrnam, call_sssd_getgrgid from files_ops import passwd_ops_setup, group_ops_setup from util import unindent @@ -258,6 +260,14 @@ def sssd_getpwnam_sync(name): return call_sssd_getpwnam(name) +def sssd_getpwuid_sync(uid): + ret = poll_canary(call_sssd_getpwnam, CANARY["name"]) + if ret is False: + return NssReturnCode.NOTFOUND, None + + return call_sssd_getpwuid(uid) + + def sssd_getgrnam_sync(name): ret = poll_canary(call_sssd_getgrnam, CANARY_GR["name"]) if ret is False: @@ -266,6 +276,14 @@ def sssd_getgrnam_sync(name): return call_sssd_getgrnam(name) +def sssd_getgrgid_sync(name): + ret = poll_canary(call_sssd_getgrnam, CANARY_GR["name"]) + if ret is False: + return NssReturnCode.NOTFOUND, None + + return call_sssd_getgrgid(name) + + def sssd_id_sync(name): sssd_getpwnam_sync(CANARY["name"]) res, _, groups = sssd_id.get_user_groups(name) @@ -307,6 +325,15 @@ def check_group(exp_group, delay=1.0): assert found_group == exp_group +def check_group_by_gid(exp_group, delay=1.0): + if delay > 0: + time.sleep(delay) + + res, found_group = sssd_getgrgid_sync(exp_group["gid"]) + assert res == NssReturnCode.SUCCESS + assert found_group == exp_group + + def check_group_list(exp_groups_list): for exp_group in exp_groups_list: check_group(exp_group) @@ -349,6 +376,16 @@ def test_getpwnam_after_start(add_user_with_canary, files_domain_only): assert user == USER1 +def test_getpwuid_after_start(add_user_with_canary, files_domain_only): + """ + Test that after startup without any additional operations, a user + can be resolved through sssd + """ + res, user = sssd_getpwuid_sync(USER1["uid"]) + assert res == NssReturnCode.SUCCESS + assert user == USER1 + + def test_user_overriden(add_user_with_canary, files_domain_only): """ Test that user override works with files domain only @@ -373,8 +410,8 @@ def test_group_overriden(add_group_with_canary, files_domain_only): """ # Override subprocess.check_call(["sss_override", "group-add", GROUP1["name"], - "-n", OV_GROUP1["name"], - "-g", str(OV_GROUP1["gid"])]) + "-n", OV_GROUP1["name"], + "-g", str(OV_GROUP1["gid"])]) restart_sssd() @@ -383,12 +420,20 @@ def test_group_overriden(add_group_with_canary, files_domain_only): def test_getpwnam_neg(files_domain_only): """ - Test that a nonexistant user cannot be resolved + Test that a nonexistant user cannot be resolved by name """ res, _ = call_sssd_getpwnam("nosuchuser") assert res == NssReturnCode.NOTFOUND +def test_getpwuid_neg(files_domain_only): + """ + Test that a nonexistant user cannot be resolved by UID + """ + res, _ = call_sssd_getpwuid(12345) + assert res == NssReturnCode.NOTFOUND + + def test_root_does_not_resolve(files_domain_only): """ SSSD currently does not resolve the root user even though it can @@ -401,6 +446,18 @@ def test_root_does_not_resolve(files_domain_only): assert res == NssReturnCode.NOTFOUND +def test_uid_zero_does_not_resolve(files_domain_only): + """ + SSSD currently does not resolve the UID 0 even though it can + be resolved through the NSS interface + """ + nss_root = pwd.getpwuid(0) + assert nss_root is not None + + res, _ = call_sssd_getpwuid(0) + assert res == NssReturnCode.NOTFOUND + + def test_add_remove_add_file_user(setup_pw_with_canary, files_domain_only): """ Test that removing a user is detected and the user @@ -522,11 +579,19 @@ def test_incomplete_user_fail(setup_pw_with_canary, files_domain_only): def test_getgrnam_after_start(add_group_with_canary, files_domain_only): """ Test that after startup without any additional operations, a group - can be resolved through sssd + can be resolved through sssd by name """ check_group(GROUP1) +def test_getgrgid_after_start(add_group_with_canary, files_domain_only): + """ + Test that after startup without any additional operations, a group + can be resolved through sssd by GID + """ + check_group_by_gid(GROUP1) + + def test_getgrnam_neg(files_domain_only): """ Test that a nonexistant group cannot be resolved @@ -535,6 +600,14 @@ def test_getgrnam_neg(files_domain_only): assert res == NssReturnCode.NOTFOUND +def test_getgrgid_neg(files_domain_only): + """ + Test that a nonexistant group cannot be resolved + """ + res, user = sssd_getgrgid_sync(123456) + assert res == NssReturnCode.NOTFOUND + + def test_root_group_does_not_resolve(files_domain_only): """ SSSD currently does not resolve the root group even though it can @@ -547,6 +620,18 @@ def test_root_group_does_not_resolve(files_domain_only): assert res == NssReturnCode.NOTFOUND +def test_gid_zero_does_not_resolve(files_domain_only): + """ + SSSD currently does not resolve the group with GID 0 even though it + can be resolved through the NSS interface + """ + nss_root = grp.getgrgid(0) + assert nss_root is not None + + res, user = call_sssd_getgrgid(0) + assert res == NssReturnCode.NOTFOUND + + def test_add_remove_add_file_group(setup_gr_with_canary, files_domain_only): """ Test that removing a group is detected and the group From 835abcf20cfe998017d2aa3c618f50096efdf2df Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <jhro...@redhat.com> Date: Tue, 22 Aug 2017 11:48:15 +0200 Subject: [PATCH 11/11] TESTS: Add regression tests to try if resolving root and ID 0 fails as expected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/tests/intg/test_ldap.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/tests/intg/test_ldap.py b/src/tests/intg/test_ldap.py index 7906508e1..f2467f1ff 100644 --- a/src/tests/intg/test_ldap.py +++ b/src/tests/intg/test_ldap.py @@ -34,6 +34,9 @@ import sssd_id import sssd_ldb from util import unindent +from sssd_nss import NssReturnCode +from sssd_passwd import call_sssd_getpwnam, call_sssd_getpwuid +from sssd_group import call_sssd_getgrnam, call_sssd_getgrgid LDAP_BASE_DN = "dc=example,dc=com" INTERACTIVE_TIMEOUT = 4 @@ -1102,10 +1105,14 @@ def sanity_nss_filter_cached(request, ldap_conn): ent_list.add_user("user1", 1001, 2001) ent_list.add_user("user2", 1002, 2002) ent_list.add_user("user3", 1003, 2003) + ent_list.add_user("root", 1004, 2004) + ent_list.add_user("zerouid", 0, 0) ent_list.add_group_bis("group1", 2001) ent_list.add_group_bis("group2", 2002) ent_list.add_group_bis("group3", 2003) + ent_list.add_group_bis("root", 2004) + ent_list.add_group_bis("zerogid", 0) create_ldap_fixture(request, ldap_conn, ent_list) conf = format_basic_conf(ldap_conn, SCHEMA_RFC2307_BIS) + \ @@ -1148,3 +1155,17 @@ def test_nss_filters_cached(ldap_conn, sanity_nss_filter_cached): time.sleep(2) with pytest.raises(KeyError): grp.getgrgid(2002) + + # test that root is always filtered even if filter_users contains other + # entries. This is a regression test for upstream ticket #3460 + res, _ = call_sssd_getpwnam("root") + assert res == NssReturnCode.NOTFOUND + + res, _ = call_sssd_getgrnam("root") + assert res == NssReturnCode.NOTFOUND + + res, _ = call_sssd_getpwuid(0) + assert res == NssReturnCode.NOTFOUND + + res, _ = call_sssd_getgrgid(0) + assert res == NssReturnCode.NOTFOUND
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
