On (30/03/15 11:47), Jakub Hrozek wrote:
Hi,
I noticed this bug while working on the SSH fix with
default_domain_suffix. It turns out our filter_users/filter_groups don't
work well in this scenario, which might have effect on the server load
even. Attached are patches that re-initialize negcache with filter_lists
contents after the subdomains are (re)discovered. All patches have
tests.
From c00a8d74bfe74c3f00e29d1b5c0271bc1bfdb24a Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Fri, 27 Mar 2015 12:57:17 +0100
Subject: [PATCH 1/5] ncache: Fix sss_ncache_reset_permanent
There was an off-by-one error in sss_ncache_reset_permanent that
prevented the reset from working.
---
src/responder/common/negcache.c | 2 +-
src/tests/cmocka/test_negcache.c | 11 +++++++++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/responder/common/negcache.c b/src/responder/common/negcache.c
index 88dd18fa5348fca223a0b2db844e3d17286045b7..04c9a53f556497173daf1ef9c562896cb2d5bbc9
100644
--- a/src/responder/common/negcache.c
+++ b/src/responder/common/negcache.c
@@ -556,7 +556,7 @@ static int delete_permanent(struct tdb_context *tdb,
char *ep;
if (strncmp((char *)key.dptr,
- NC_ENTRY_PREFIX, sizeof(NC_ENTRY_PREFIX)) != 0) {
+ NC_ENTRY_PREFIX, sizeof(NC_ENTRY_PREFIX) - 1) != 0) {
/* not interested in this key */
return 0;
}
IMHO this test is not very useful.
All entries in negative cache should start with NC_ENTRY_PREFIX.
But your patch fixes the bug.
ACK
From d2e30c6083b5082b90343ca6be10e1899f44d911 Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Fri, 27 Mar 2015 12:30:53 +0100
Subject: [PATCH 2/5] ncache: Silence critical error from filter_users when
default_domain_suffix is set
When default_domain_suffix is used and filter_users is set (at least
root is always, by default), SSSD tried to add the negcache entry to the
default domain. But since the default domain is not known after start
up, adding the entries fail with a verbose error message.
This patch handles EAGAIN returned from the parsing function while
setting negcache entries gracefully and also makes the debug message in
parsing function more precise.
---
src/responder/common/negcache.c | 18 +++++++++--
src/tests/cmocka/test_negcache.c | 69 ++++++++++++++++++++++++++++++++++++++--
src/util/usertools.c | 3 +-
3 files changed, 82 insertions(+), 8 deletions(-)
diff --git a/src/tests/cmocka/test_negcache.c b/src/tests/cmocka/test_negcache.c
index 4502c0294e772b15bf230d485e02c40e2ffbf2d6..5ade3fd4b347a6c7c74754730bca4b82c71ddf06
100644
--- a/src/tests/cmocka/test_negcache.c
+++ b/src/tests/cmocka/test_negcache.c
@@ -590,8 +590,8 @@ static void test_sss_ncache_prepopulate(void **state)
struct sss_domain_info *dom;
struct sss_test_conf_param params[] = {
- { "filter_users", "testuser1" },
- { "filter_groups", "testgroup1" },
+ { "filter_users", "testuser1, testuser2@"TEST_DOM_NAME",
testuser3@somedomain" },
+ { "filter_groups", "testgroup1,
testgroup2@"TEST_DOM_NAME", testgroup3@somedomain" },
{ NULL, NULL },
};
@@ -628,6 +628,67 @@ static void test_sss_ncache_prepopulate(void **state)
ret = sss_ncache_check_group(ncache, 1, dom, "testgroup1");
assert_int_equal(ret, EEXIST);
+
+ ret = sss_ncache_check_user(ncache, 1, dom, "testuser2");
+ assert_int_equal(ret, EEXIST);
+
+ ret = sss_ncache_check_group(ncache, 1, dom, "testgroup2");
+ assert_int_equal(ret, EEXIST);
+
+ ret = sss_ncache_check_user(ncache, 1, dom, "testuser3");
+ assert_int_equal(ret, ENOENT);
+
+ ret = sss_ncache_check_group(ncache, 1, dom, "testgroup3");
+ assert_int_equal(ret, ENOENT);
+
+ ret = sss_ncache_check_user(ncache, 1, dom, "testuser3@somedomain");
+ assert_int_equal(ret, ENOENT);
+
+ ret = sss_ncache_check_group(ncache, 1, dom, "testgroup3@somedomain");
+ assert_int_equal(ret, ENOENT);
+}
+
+static void test_sss_ncache_default_domain_suffix(void **state)
+{
+ int ret;
+ struct test_state *ts;
+ struct tevent_context *ev;
+ struct sss_nc_ctx *ncache;
+ struct sss_test_ctx *tc;
+ struct sss_domain_info *dom;
+
+ struct sss_test_conf_param params[] = {
+ { "filter_users", "testuser1, testuser2@"TEST_DOM_NAME",
testuser3@somedomain" },
+ { "filter_groups", "testgroup1,
testgroup2@"TEST_DOM_NAME", testgroup3@somedomain" },
+ { NULL, NULL },
+ };
+
+ ts = talloc_get_type_abort(*state, struct test_state);
+
+ ev = tevent_context_init(ts);
+ assert_non_null(ev);
+
+ dom = talloc_zero(ts, struct sss_domain_info);
+ assert_non_null(dom);
+ dom->name = discard_const_p(char, TEST_DOM_NAME);
+
+ ts->nctx = mock_nctx(ts);
+ assert_non_null(ts->nctx);
+
+ tc = create_dom_test_ctx(ts, TESTS_PATH, TEST_CONF_DB,
+ TEST_DOM_NAME, TEST_ID_PROVIDER, params);
+ assert_non_null(tc);
+
+ ncache = ts->ctx;
+ ts->rctx = mock_rctx(ts, ev, dom, ts->nctx);
+ assert_non_null(ts->rctx);
+ ts->rctx->default_domain = discard_const(TEST_DOM_NAME);
+
+ ret = sss_names_init(ts, tc->confdb, TEST_DOM_NAME, &dom->names);
+ assert_int_equal(ret, EOK);
+
+ ret = sss_ncache_prepopulate(ncache, tc->confdb, ts->rctx);
+ assert_int_equal(ret, EOK);
Could you test "filtered entries" after
pre populating negative cache?
From e5745bb5384772f25850203c27a355cba5a5c374 Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Sun, 29 Mar 2015 16:30:27 +0200
Subject: [PATCH 3/5] ncache: Add sss_ncache_reset_repopulate_permanent
This new function resets the negative cache and then re-adds the
permanent entries.
---
src/responder/common/negcache.c | 14 ++++++
src/responder/common/negcache.h | 6 +++
src/tests/cmocka/test_negcache.c | 93 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 113 insertions(+)
ACK
From 0570429e64f8c42feeed8bed75bf5530d1c1fed2 Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Sun, 29 Mar 2015 16:31:19 +0200
Subject: [PATCH 4/5] responders: reset ncache after domains are discovered
during startup
After responders start, they add a lookup operation that discovers the
subdomains so that qualifying users works. After this operation is
finishes, we need to reset negcache to allow users to be added into the
newly discovered domains.
---
ACK
From 1d9bc4c91131dc8af1cbddeb414f0cb71069724d Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Sun, 29 Mar 2015 20:30:44 +0200
Subject: [PATCH 5/5] NSS: Reset negcache after checking domains
The NSS responder periodically re-checks subdomains. We need to reset
the negative cache each time the check finishes to allow the negative
cache to contain entries from different domains.
---
ACK