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.
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.
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.
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.
On Fri, 2012-03-09 at 20:56 +0100, Jakub Hrozek wrote:
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.
Ack to both and pushed to master and sssd-1-8.
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Stephen Gallagher