[SSSD] [PATCH] Two proxy netgroups fixes

Jakub Hrozek jhrozek at redhat.com
Fri Mar 9 17:17:44 UTC 2012


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 :-)

[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.
-------------- next part --------------
From 76c2d916577ea6474e6bab9c0b8eeb938f7f950c 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 |   77 ++++++++++++++++++++++++++++------
 1 files changed, 64 insertions(+), 13 deletions(-)

diff --git a/src/providers/proxy/proxy_netgroup.c b/src/providers/proxy/proxy_netgroup.c
index 797f8c6b88ec4c07440f134fb2a1071b1c5c9976..c22869d37d4d8e723dd249f67355ade99aa17044 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,
@@ -108,37 +143,42 @@ errno_t get_netgroup(struct proxy_id_ctx *ctx,
     TALLOC_CTX *tmp_ctx;
     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"));
+        DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_new failed.\n"));
         return ENOMEM;
     }
 
     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)));
+            DEBUG(SSSDBG_OP_FAILURE,
+                  ("getnetgrent_r failed for netgroup [%s]: [%d][%s].\n",
+                   name, ret, strerror(ret)));
+            ret = handle_error(status, sysdb, name);
             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;
             }
         }
@@ -146,8 +186,20 @@ errno_t get_netgroup(struct proxy_id_ctx *ctx,
 
     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;
+    }
+
+    /* For some reason setnetgrent returns NSS_SUCCESS even if the
+     * netgroup does not exist..
+     */
+    if (attrs->num == 0) {
+        DEBUG(SSSDBG_MINOR_FAILURE, ("The netgroup does not exist\n"));
+        ret = sysdb_delete_netgroup(sysdb, name);
+        if (ret != EOK) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("Cannot delete netgroup: %d\n", ret));
+        }
         goto done;
     }
 
@@ -155,7 +207,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 +215,5 @@ errno_t get_netgroup(struct proxy_id_ctx *ctx,
 
 done:
     talloc_free(tmp_ctx);
-
     return ret;
 }
-- 
1.7.7.6

-------------- next part --------------
From f980c16de471cd7345717f197456d0208accde52 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 c22869d37d4d8e723dd249f67355ade99aa17044..0b9f730d22a34cdb61fc421b8b6338efd7c216a5 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