[SSSD] [PATCH] Improve negcache with subdomains

Lukas Slebodnik lslebodn at redhat.com
Thu Apr 2 09:58:41 UTC 2015


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 at 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 at 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 at somedomain" },
>+        { "filter_groups", "testgroup1, testgroup2@"TEST_DOM_NAME", testgroup3 at 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 at somedomain");
>+    assert_int_equal(ret, ENOENT);
>+
>+    ret = sss_ncache_check_group(ncache, 1, dom, "testgroup3 at 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 at somedomain" },
>+        { "filter_groups", "testgroup1, testgroup2@"TEST_DOM_NAME", testgroup3 at 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 at 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 at 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 at 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



More information about the sssd-devel mailing list