| Message ID | 20140326094838.GA9707@spoyarek.pnq.redhat.com |
|---|---|
| State | New |
| Headers | show |
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 03/26/2014 05:48 AM, Siddhesh Poyarekar wrote: > nscd works correctly when the request in innetgr is a wildcard, > i.e. when one or more of host, user or domain parameters is NULL. > However, it does not work when the the triplet in the netgroup > definition has a wildcard. This is easy to reproduce for a triplet > defined as follows: > > foonet (,foo,) > > Here, an innetgr call that looks like this: > > innetgr ("foonet", "foohost", "foo", NULL); > > should succeed and so should: > > innetgr ("foonet", NULL, "foo", "foodomain"); > > It does succeed with nscd disabled, but not with nscd enabled. This > fix adds this additional check for all three parts of the triplet so > that it gives the correct result. > > Tested on x86_64. > > Siddhesh > > [BZ #16758] > * nscd/netgroupcache.c (addinnetgrX): Succeed if triplet has > blank values. OK to checkin as long as for the sake of completeness tested the other two missing combinations of NULLs in calls to innetgr with and without nscd and it worked. I reviewed the code around this, the netgroup cache code, and this looks correct to me. I frown every time I have to review this code without adding a regression test. We're going to have to talk about this at Cauldron 2014 e.g. the need for network testing and whole system integration testing. > --- > nscd/netgroupcache.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c > index 5ba1e1f..5d15aa4 100644 > --- a/nscd/netgroupcache.c > +++ b/nscd/netgroupcache.c > @@ -560,15 +560,19 @@ addinnetgrX (struct database_dyn *db, int fd, request_header *req, > { > bool success = true; > > - if (host != NULL) > + /* For the host, user and domain in each triplet, we assume success > + if the value is blank because that is how the wildcard entry to > + match anything is stored in the netgroup cache. */ > + if (host != NULL && *triplets != '\0') OK. > success = strcmp (host, triplets) == 0; > triplets = (const char *) rawmemchr (triplets, '\0') + 1; > > - if (success && user != NULL) > + if (success && user != NULL && *triplets != '\0') OK. > success = strcmp (user, triplets) == 0; > triplets = (const char *) rawmemchr (triplets, '\0') + 1; > > - if (success && (domain == NULL || strcmp (domain, triplets) == 0)) > + if (success && (domain == NULL || *triplets == '\0' > + || strcmp (domain, triplets) == 0)) OK. > { > dataset->resp.result = 1; > break; > Cheers, Carlos. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTM0ynAAoJECXvCkNsKkr/neAH/14um4ud99PomrLI6opYgKK0 qssxRjKQeYAePze952dBRYpSiH2dTS4TfYFCmYynxvNlJ1ICcbn8ty4CV6M3RCbb 1ehTPHLvo//3r0dwBDlMAd/91g+H1OKpCvkC1q9TUDJVcwvI0hakBPYmJRufw6p1 WFv3EHujsJh10efUIdZUTE4eA7dsiPZ2JtIuD+Ho4tKaYoJgoBsEohtT3U4hnWF+ qX4nI8kpTkc7sX5HZU588uAiA6FEAHugUwC4s20a+E1j4aEafJlbswhXWqHNuHtC fMxo4UnmdkrB+NJGn3OYKja0t7LG4YR6lO3NIZF1R/0mv7rs+BoHV/Bg573e+1M= =BTTa -----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 03/26/2014 05:54 PM, Carlos O'Donell wrote: > On 03/26/2014 05:48 AM, Siddhesh Poyarekar wrote: >> nscd works correctly when the request in innetgr is a wildcard, >> i.e. when one or more of host, user or domain parameters is NULL. >> However, it does not work when the the triplet in the netgroup >> definition has a wildcard. This is easy to reproduce for a triplet >> defined as follows: > >> foonet (,foo,) > >> Here, an innetgr call that looks like this: > >> innetgr ("foonet", "foohost", "foo", NULL); > >> should succeed and so should: > >> innetgr ("foonet", NULL, "foo", "foodomain"); > >> It does succeed with nscd disabled, but not with nscd enabled. This >> fix adds this additional check for all three parts of the triplet so >> that it gives the correct result. > >> Tested on x86_64. > >> Siddhesh > >> [BZ #16758] >> * nscd/netgroupcache.c (addinnetgrX): Succeed if triplet has >> blank values. > > OK to checkin as long as for the sake of completeness tested the > other two missing combinations of NULLs in calls to innetgr with > and without nscd and it worked. You would also make me very happy to see the testing permutations include permuting the triplet in /etc/netgroups so the null moves to all possible spaces and is tested that way to flush out any other corner cases. Cheers, Carlos. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTM02xAAoJECXvCkNsKkr/K9YIAMLIbEERF/rE4qJ+ysKd0fbB 1VYtwX5br4ckD+vMCS/PCmq3oRvmhexELrYyyYAG8XD6NP9/M9pAXi5g5zB3vkj/ cBM2BXGu78aUUXBGjWu/Sutj+CCnoR17wsgnRX2DPLuuXiMTdADwJtvCJTu6ZeKn LQLiFuojtbw6AoAwFPFyM6/PHgu97jjLLKLqV/nH1AdFESMQkQEgee39Rr/Qpdhi Jov18z3LnkLqAD1PX2zSGSifkdJTm2K41Ka86NLmhqUqzHBkVPcsX1lJFT8qJQbw S0o0WtICv1yt96fLx2wjZbdw3q2PAjyj0SZ1p6MORTI+xVEwDgnQO41b9viur+4= =/6Rw -----END PGP SIGNATURE-----
On Wed, Mar 26, 2014 at 05:59:13PM -0400, Carlos O'Donell wrote: > > OK to checkin as long as for the sake of completeness tested the > > other two missing combinations of NULLs in calls to innetgr with > > and without nscd and it worked. > > You would also make me very happy to see the testing permutations > include permuting the triplet in /etc/netgroups so the null moves > to all possible spaces and is tested that way to flush out any > other corner cases. ack, this is the program I used: #include <stdio.h> #include <netdb.h> int main (int argc, char **argv) { int ret = 0; if (argc != 5) { printf ("Usage: %s netgroup host user domain\n", argv[0]); return 1; } const char *netgr = argv[1]; const char *host = argv[2][0] ? argv[2] : NULL; const char *user = argv[3][0] ? argv[3] : NULL; const char *domain = argv[4][0] ? argv[4] : NULL; ret = innetgr (netgr, host, user, domain); if (ret) printf ("Found it\n"); else printf ("Not found\n"); return 0; } and the outputs: [root@localhost ~]# cat /etc/netgroup bbb (defhost,,) ccc (,defuser,) ddd (,,defdom) [root@localhost ~]# ./a.out bbb defhost "" "" Found it [root@localhost ~]# ./a.out bbb defhost "a" "" Found it [root@localhost ~]# ./a.out bbb defhost "a" "a" Found it [root@localhost ~]# ./a.out bbb defhost "" "" Found it [root@localhost ~]# ./a.out bbb defhost "a" "" Found it [root@localhost ~]# ./a.out bbb defhost "" "a" Found it [root@localhost ~]# ./a.out bbb defhost "a" "a" Found it [root@localhost ~]# ./a.out ccc "" defuser "" Found it [root@localhost ~]# ./a.out ccc "a" defuser "" Found it [root@localhost ~]# ./a.out ccc "" defuser "a" Found it [root@localhost ~]# ./a.out ccc "a" defuser "a" Found it [root@localhost ~]# ./a.out ddd "" "" defdom Found it [root@localhost ~]# ./a.out ddd "a" "" defdom Found it [root@localhost ~]# ./a.out ddd "a" "a" defdom Found it [root@localhost ~]# ./a.out ddd "" "a" defdom Found it Siddhesh
diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c index 5ba1e1f..5d15aa4 100644 --- a/nscd/netgroupcache.c +++ b/nscd/netgroupcache.c @@ -560,15 +560,19 @@ addinnetgrX (struct database_dyn *db, int fd, request_header *req, { bool success = true; - if (host != NULL) + /* For the host, user and domain in each triplet, we assume success + if the value is blank because that is how the wildcard entry to + match anything is stored in the netgroup cache. */ + if (host != NULL && *triplets != '\0') success = strcmp (host, triplets) == 0; triplets = (const char *) rawmemchr (triplets, '\0') + 1; - if (success && user != NULL) + if (success && user != NULL && *triplets != '\0') success = strcmp (user, triplets) == 0; triplets = (const char *) rawmemchr (triplets, '\0') + 1; - if (success && (domain == NULL || strcmp (domain, triplets) == 0)) + if (success && (domain == NULL || *triplets == '\0' + || strcmp (domain, triplets) == 0)) { dataset->resp.result = 1; break;
nscd works correctly when the request in innetgr is a wildcard, i.e. when one or more of host, user or domain parameters is NULL. However, it does not work when the the triplet in the netgroup definition has a wildcard. This is easy to reproduce for a triplet defined as follows: foonet (,foo,) Here, an innetgr call that looks like this: innetgr ("foonet", "foohost", "foo", NULL); should succeed and so should: innetgr ("foonet", NULL, "foo", "foodomain"); It does succeed with nscd disabled, but not with nscd enabled. This fix adds this additional check for all three parts of the triplet so that it gives the correct result. Tested on x86_64. Siddhesh [BZ #16758] * nscd/netgroupcache.c (addinnetgrX): Succeed if triplet has blank values. --- nscd/netgroupcache.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)