[SSSD] [PATCH] NSS: invalidate memcache user entry on initgr, too

Jakub Hrozek jhrozek at redhat.com
Tue Jan 15 14:23:04 UTC 2013


On Tue, Jan 15, 2013 at 08:27:32AM -0500, Simo Sorce wrote:
> On Tue, 2013-01-15 at 09:02 +0100, Jakub Hrozek wrote:
> > https://fedorahosted.org/sssd/ticket/1757
> > 
> > When the user entry was missing completely after initgroups, we would
> > never invalidate the user entry from cache. This led to dangling cache
> > entried in memory cache if the user was removed from the server while
> > still being in memory cache.
> 
> 0001: Why are we unconditionally deleting the user from the memory cache
> even if just groups changed ?
> 

I was trying to keep the flow as simple as possible because I think that
user deletes or even group modifications updates on the server are so rare
that if we loose the memcache, it's not a big deal.

But I moved the user update separately in this version.

> 0002: Please add a comment in be_initgroups_prereq() that the first id
> it he user's primary gid and 'might' be duplicated later ?

Done

> Also why do we skip verifying the primary gid ?
> 

Ah, not sure, I think I was just copying the previous flow that skipped
the primary gid too. The attached version checks them all.

Thank you for the review.
-------------- next part --------------
>From 4d09fab923053504fc22b6c38f274d09768256e7 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 15 Jan 2013 07:05:56 +0100
Subject: [PATCH 1/2] NSS: invalidate memcache user entry on initgr, too

https://fedorahosted.org/sssd/ticket/1757

When the user entry was missing completely after initgroups, we would
never invalidate the user entry from cache. This led to dangling cache
entried in memory cache if the user was removed from the server while
still being in memory cache.
---
 src/responder/nss/nsssrv_cmd.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index 63d82e22c2510c1c7ffa8d4b8f535d9f92bbadd7..b1db367ee1c27455d1a2216a3c63625b9ce61143 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -3410,6 +3410,7 @@ void nss_update_initgr_memcache(struct nss_ctx *nctx,
     TALLOC_CTX *tmp_ctx = NULL;
     struct sss_domain_info *dom;
     struct ldb_result *res;
+    struct sized_string delete_name;
     bool changed = false;
     uint32_t id;
     uint32_t gids[gnum];
@@ -3448,6 +3449,16 @@ void nss_update_initgr_memcache(struct nss_ctx *nctx,
     memcpy(gids, groups, gnum * sizeof(uint32_t));
 
     if (ret == ENOENT || res->count == 0) {
+        /* The user is gone. Invalidate the mc record */
+        to_sized_string(&delete_name, name);
+        ret = sss_mmap_cache_pw_invalidate(nctx->pwd_mc_ctx, &delete_name);
+        if (ret != EOK && ret != ENOENT) {
+            DEBUG(SSSDBG_CRIT_FAILURE,
+                  ("Internal failure in memory cache code: %d [%s]\n",
+                  ret, strerror(ret)));
+        }
+
+        /* Also invalidate his groups */
         changed = true;
     } else {
         /* we skip the first entry, it's the user itself */
-- 
1.8.0.2

-------------- next part --------------
>From 3f406810c6db85bbddbb527e13b5c00ccdcb2bcb Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 15 Jan 2013 07:54:03 +0100
Subject: [PATCH 2/2] Invalidate user entry even if there are no groups

Related to https://fedorahosted.org/sssd/ticket/1757

Previously we would optimize the mc invalidate code for cases where the
user was a member of some groups. But if the user was removed from the
server while being in memory cache, we would only invalidate the mc
record if he was a member of at least one supplementary group.
---
 src/providers/data_provider_be.c | 12 +++++++-----
 src/responder/nss/nsssrv_cmd.c   |  7 +------
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index 452979c0d6720f2c505a5e487cac31370d0443f3..789071f76bb198387861d3e8a484e19777beb7ad 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -669,9 +669,9 @@ static errno_t be_initgroups_prereq(struct be_req *be_req)
     if (ret && ret != ENOENT) {
         return ret;
     }
-    /* if the user is completely missing or has no group memberships
-     * at all there is no need to contact NSS, it would be a noop */
-    if (ret == ENOENT || res->count == 0 || res->count == 1) {
+    /* if the user is completely missing there is no need to contact NSS,
+     * it would be a noop */
+    if (ret == ENOENT || res->count == 0) {
         /* yet unknown, ignore */
         return EOK;
     }
@@ -680,7 +680,7 @@ static errno_t be_initgroups_prereq(struct be_req *be_req)
     if (!pr) {
         return ENOMEM;
     }
-    pr->groups = talloc_array(pr, gid_t, res->count - 1);
+    pr->groups = talloc_array(pr, gid_t, res->count);
     if (!pr->groups) {
         return ENOMEM;
     }
@@ -696,7 +696,9 @@ static errno_t be_initgroups_prereq(struct be_req *be_req)
     if (!pr->domain) {
         return ENOMEM;
     }
-    for (pr->gnum = 0, i = 1; i < res->count; i++) {
+    /* The first GID is the primary so it might be duplicated
+     * later in the list */
+    for (pr->gnum = 0, i = 0; i < res->count; i++) {
         pr->groups[pr->gnum] = ldb_msg_find_attr_as_uint(res->msgs[i],
                                                          SYSDB_GIDNUM, 0);
         /* if 0 it may be a non-posix group, so we skip it */
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index b1db367ee1c27455d1a2216a3c63625b9ce61143..b2dae4a3a0d2ecf39fca0f238645d05029b1c3df 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -3417,11 +3417,6 @@ void nss_update_initgr_memcache(struct nss_ctx *nctx,
     int ret;
     int i, j;
 
-    if (gnum == 0) {
-        /* there are no groups to invalidate in any case, just return */
-        return;
-    }
-
     for (dom = nctx->rctx->domains; dom != NULL; dom = dom->next) {
         if (strcasecmp(dom->name, domain) == 0) {
             break;
@@ -3462,7 +3457,7 @@ void nss_update_initgr_memcache(struct nss_ctx *nctx,
         changed = true;
     } else {
         /* we skip the first entry, it's the user itself */
-        for (i = 1; i < res->count; i++) {
+        for (i = 0; i < res->count; i++) {
             id = ldb_msg_find_attr_as_uint(res->msgs[i], SYSDB_GIDNUM, 0);
             if (id == 0) {
                 /* probably non-posix group, skip */
-- 
1.8.0.2



More information about the sssd-devel mailing list