Fix nscd lookup for innetgr when netgroup has wildcards (BZ #16758)

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

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