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.
From 98d3a3d09aece77cd085b6794f607ffd82044708 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <[email protected]> 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
From bc1619137d200d0ef50851d6f9f4261e0abe330c Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <[email protected]> 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
_______________________________________________ sssd-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/sssd-devel
