[SSSD] [PATCH] Two proxy netgroups fixes

Jakub Hrozek jhrozek at redhat.com
Fri Mar 9 19:56:19 UTC 2012


On Fri, Mar 09, 2012 at 02:10:43PM -0500, Stephen Gallagher wrote:
> On Fri, 2012-03-09 at 18:17 +0100, Jakub Hrozek wrote:
> > Hi,
> > 
> > attached are two patches for issues I found in the proxy netgroups code.
> > 
> > [PATCH 1/2] Fix netgroup error handling
> > https://fedorahosted.org/sssd/ticket/1242
> > 
> > The patch improves error handling, and, most importanly, deletes any
> > netgroup that might be in the cache if the search did not yield any
> > results. There's one catch, though. During my testing with
> > nss-pam-ldapd, all the NSS operations returned NSS_STATUS_SUCCESS and an
> > empty "struct __netgrent" structure for cases when the netgroup existed
> > and when the netgroup existed but had no nisNetgroupTriple attributes.
> > This may be a nss-pam-ldapd bug, though..is there any other back end
> > that could be used to test? I'd like to avoid setting up NIS :-)
> > 
> 
> You can create /etc/netgroup and add lines like
> netgroupfile1 (a,b,c) (d,,e)
> 
> And then use proxy_lib_name=files.
> 
> It looks like that IS an nss-pam-ldapd bug. The file provider properly
> returns NSS_STATUS_NOTFOUND if the netgroup doesn't exist.

Yes, I'll bring that up with nss-pam-ldapd upstream.

> 
> It's not actually correct to delete the netgroup if it has no
> attributes. It's technically legal to have a netgroup containing no
> members. I'm not sure it's *useful*, but it's legal.
> 
> Also, there's a segfault here if the netgroup lookup returns
> NSS_STATUS_NOTFOUND because you don't initialize tmp_ctx to NULL in
> get_netgroup(), and the goto done: tries to free it.
> 
> So, nack.
> 
> 
> > [PATCH 2/2] Handle empty elements in proxy netgroups
> > The make_netgroup_attr() function did not check for NULL elements of
> > netgroup triples and could print literal "(null)" into the triple
> > element in the nice case and crash in the worse case.
> 
> Ack.

Fixed & attached both patches even though only patch #1 changed.
-------------- next part --------------
From 98d3a3d09aece77cd085b6794f607ffd82044708 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 9 Mar 2012 11:22:48 -0500
Subject: [PATCH 1/2] Fix netgroup error handling

https://fedorahosted.org/sssd/ticket/1242
---
 src/providers/proxy/proxy_netgroup.c |   76 ++++++++++++++++++++++++++--------
 1 files changed, 59 insertions(+), 17 deletions(-)

diff --git a/src/providers/proxy/proxy_netgroup.c b/src/providers/proxy/proxy_netgroup.c
index 797f8c6b88ec4c07440f134fb2a1071b1c5c9976..0c8f5c66087f68fff30ba07eff4013b591a36106 100644
--- a/src/providers/proxy/proxy_netgroup.c
+++ b/src/providers/proxy/proxy_netgroup.c
@@ -96,6 +96,41 @@ static errno_t save_netgroup(struct sysdb_ctx *sysdb,
     return EOK;
 }
 
+static errno_t handle_error(enum nss_status status,
+                            struct sysdb_ctx *sysdb, const char *name)
+{
+    errno_t ret;
+
+    switch (status) {
+    case NSS_STATUS_SUCCESS:
+        DEBUG(SSSDBG_TRACE_INTERNAL, ("Netgroup lookup succeeded\n"));
+        ret = EOK;
+        break;
+
+    case NSS_STATUS_NOTFOUND:
+        DEBUG(SSSDBG_MINOR_FAILURE, ("The netgroup was not found\n"));
+        ret = sysdb_delete_netgroup(sysdb, name);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Cannot delete netgroup: %d\n", ret));
+            ret = EIO;
+        }
+        break;
+
+    case NSS_STATUS_UNAVAIL:
+        DEBUG(SSSDBG_TRACE_LIBS,
+              ("The proxy target did not respond, going offline\n"));
+        ret = ENXIO;
+        break;
+
+    default:
+        DEBUG(SSSDBG_CRIT_FAILURE, ("Unexpected error looking up netgroup\n"));
+        ret = EIO;
+        break;
+    }
+
+    return ret;
+}
+
 errno_t get_netgroup(struct proxy_id_ctx *ctx,
                      struct sysdb_ctx *sysdb,
                      struct sss_domain_info *dom,
@@ -105,49 +140,57 @@ errno_t get_netgroup(struct proxy_id_ctx *ctx,
     enum nss_status status;
     char buffer[BUFLEN];
     int ret;
-    TALLOC_CTX *tmp_ctx;
+    TALLOC_CTX *tmp_ctx = NULL;
     struct sysdb_attrs *attrs;
 
-    memset(&result, 0 ,sizeof(result));
+    memset(&result, 0, sizeof(result));
     status = ctx->ops.setnetgrent(name, &result);
     if (status != NSS_STATUS_SUCCESS) {
-        DEBUG(5, ("setnetgrent failed for netgroup [%s].\n", name));
-        return EIO;
+        DEBUG(SSSDBG_OP_FAILURE,
+              ("setnetgrent failed for netgroup [%s].\n", name));
+        ret = handle_error(status, sysdb, name);
+        goto done;
     }
 
     tmp_ctx = talloc_new(NULL);
     if (tmp_ctx == NULL) {
-        DEBUG(1, ("talloc_new failed.\n"));
-        return ENOMEM;
+        DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_new failed.\n"));
+        ret = ENOMEM;
+        goto done;
     }
 
     attrs = sysdb_new_attrs(tmp_ctx);
     if (attrs == NULL) {
-        DEBUG(1, ("sysdb_new_attrs failed.\n"));
-        return ENOMEM;
+        DEBUG(SSSDBG_CRIT_FAILURE, ("sysdb_new_attrs failed.\n"));
+        ret = ENOMEM;
+        goto done;
     }
 
     do {
         status = ctx->ops.getnetgrent_r(&result, buffer, BUFLEN, &ret);
-        if (status != NSS_STATUS_SUCCESS && status != NSS_STATUS_RETURN) {
-            DEBUG(1, ("getnetgrent_r failed for netgroup [%s]: [%d][%s].\n",
-                      name, ret, strerror(ret)));
+        if (status != NSS_STATUS_SUCCESS &&
+            status != NSS_STATUS_RETURN &&
+            status != NSS_STATUS_NOTFOUND) {
+            ret = handle_error(status, sysdb, name);
+            DEBUG(SSSDBG_OP_FAILURE,
+                  ("getnetgrent_r failed for netgroup [%s]: [%d][%s].\n",
+                   name, ret, strerror(ret)));
             goto done;
         }
 
         if (status == NSS_STATUS_SUCCESS) {
             ret = make_netgroup_attr(result, attrs);
             if (ret != EOK) {
-                DEBUG(1, ("make_netgroup_attr failed.\n"));
+                DEBUG(SSSDBG_CRIT_FAILURE, ("make_netgroup_attr failed.\n"));
                 goto done;
             }
         }
-    } while (status != NSS_STATUS_RETURN);
+    } while (status != NSS_STATUS_RETURN && status != NSS_STATUS_NOTFOUND);
 
     status = ctx->ops.endnetgrent(&result);
     if (status != NSS_STATUS_SUCCESS) {
-        DEBUG(1, ("endnetgrent failed.\n"));
-        ret = EIO;
+        DEBUG(SSSDBG_OP_FAILURE, ("endnetgrent failed.\n"));
+        ret = handle_error(status, sysdb, name);
         goto done;
     }
 
@@ -155,7 +198,7 @@ errno_t get_netgroup(struct proxy_id_ctx *ctx,
                         !dom->case_sensitive,
                         dom->netgroup_timeout);
     if (ret != EOK) {
-        DEBUG(1, ("sysdb_add_netgroup failed.\n"));
+        DEBUG(SSSDBG_OP_FAILURE, ("sysdb_add_netgroup failed.\n"));
         goto done;
     }
 
@@ -163,6 +206,5 @@ errno_t get_netgroup(struct proxy_id_ctx *ctx,
 
 done:
     talloc_free(tmp_ctx);
-
     return ret;
 }
-- 
1.7.7.6

-------------- next part --------------
From bc1619137d200d0ef50851d6f9f4261e0abe330c Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Fri, 9 Mar 2012 11:49:12 -0500
Subject: [PATCH 2/2] Handle empty elements in proxy netgroups:

---
 src/providers/proxy/proxy_netgroup.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/providers/proxy/proxy_netgroup.c b/src/providers/proxy/proxy_netgroup.c
index 0c8f5c66087f68fff30ba07eff4013b591a36106..afc57ecbe2eac1a765124d898cb77485fbd9c629 100644
--- a/src/providers/proxy/proxy_netgroup.c
+++ b/src/providers/proxy/proxy_netgroup.c
@@ -28,6 +28,8 @@
 
 #define BUFLEN  1024
 
+#define get_triple_el(s) ((s) ? (s) : "")
+
 static errno_t make_netgroup_attr(struct __netgrent netgrent,
                                   struct sysdb_attrs *attrs)
 {
@@ -42,9 +44,10 @@ static errno_t make_netgroup_attr(struct __netgrent netgrent,
             return ret;
         }
     } else if (netgrent.type == triple_val) {
-        dummy = talloc_asprintf(attrs, "(%s,%s,%s)", netgrent.val.triple.host,
-                                netgrent.val.triple.user,
-                                netgrent.val.triple.domain);
+        dummy = talloc_asprintf(attrs, "(%s,%s,%s)",
+                                get_triple_el(netgrent.val.triple.host),
+                                get_triple_el(netgrent.val.triple.user),
+                                get_triple_el(netgrent.val.triple.domain));
         if (dummy == NULL) {
             DEBUG(1, ("talloc_asprintf failed.\n"));
             return ENOMEM;
-- 
1.7.7.6



More information about the sssd-devel mailing list