Return NULL for wildcard values in getnetgrent from nscd (BZ #16759)

Message ID 20140326095041.GA9937@spoyarek.pnq.redhat.com
State Committed
Headers

Commit Message

Siddhesh Poyarekar March 26, 2014, 9:50 a.m. UTC
  getnetgrent is supposed to return NULL for values that are wildcards
in the (host, user, domain) triplet.  This works correctly with nscd
disabled, but with it enabled, it returns a blank ("") instead of a
NULL.  This is easily seen with the output of `getent netgroup foonet`
for a netgroup foonet defined as follows in /etc/netgroup:

    foonet (,foo,)

The output with nscd disabled is:

    foonet ( ,foo,)

while with nscd enabled, it is:

    foonet (,foo,)

The extra space with nscd disabled is due to the fact that `getent
netgroup` adds it if the return value from getnetgrent is NULL for
either host or user.

Tested on x86_64.

Siddhesh

	[BZ #16759]
	* inet/getnetgrent_r.c (get_nonempty_val): New function.
	(nscd_getnetgrent): Use it.

---
 inet/getnetgrent_r.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)
  

Comments

Carlos O'Donell March 27, 2014, 4:31 a.m. UTC | #1
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/26/2014 05:50 AM, Siddhesh Poyarekar wrote:
> getnetgrent is supposed to return NULL for values that are wildcards
> in the (host, user, domain) triplet.  This works correctly with nscd
> disabled, but with it enabled, it returns a blank ("") instead of a
> NULL.  This is easily seen with the output of `getent netgroup foonet`
> for a netgroup foonet defined as follows in /etc/netgroup:
> 
>     foonet (,foo,)
> 
> The output with nscd disabled is:
> 
>     foonet ( ,foo,)
> 
> while with nscd enabled, it is:
> 
>     foonet (,foo,)
> 
> The extra space with nscd disabled is due to the fact that `getent
> netgroup` adds it if the return value from getnetgrent is NULL for
> either host or user.
> 
> Tested on x86_64.
> 
> Siddhesh
> 
> 	[BZ #16759]
> 	* inet/getnetgrent_r.c (get_nonempty_val): New function.
> 	(nscd_getnetgrent): Use it.

Looks good to me as long as you tested a wildcard in all
of the positions.

I also noticed this difference while doing netgroup testing
with and without nscd, but I'll be honest I didn't know why
there was "" or " " in the argument to be wildcarded. Now
that I've looked at the code I see why, and I expect this
probably matches what traditional unices were doing.
 
> ---
>  inet/getnetgrent_r.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/inet/getnetgrent_r.c b/inet/getnetgrent_r.c
> index 62cdfda..f6d064d 100644
> --- a/inet/getnetgrent_r.c
> +++ b/inet/getnetgrent_r.c
> @@ -235,6 +235,14 @@ endnetgrent (void)
>  }
>  
>  #ifdef USE_NSCD
> +static const char *
> +get_nonempty_val (const char *in)
> +{
> +  if (*in == '\0')
> +    return NULL;
> +  return in;
> +}

OK.

> +
>  static enum nss_status
>  nscd_getnetgrent (struct __netgrent *datap, char *buffer, size_t buflen,
>  		  int *errnop)
> @@ -243,11 +251,11 @@ nscd_getnetgrent (struct __netgrent *datap, char *buffer, size_t buflen,
>      return NSS_STATUS_UNAVAIL;
>  
>    datap->type = triple_val;
> -  datap->val.triple.host = datap->cursor;
> +  datap->val.triple.host = get_nonempty_val (datap->cursor);

OK.

>    datap->cursor = (char *) __rawmemchr (datap->cursor, '\0') + 1;
> -  datap->val.triple.user = datap->cursor;
> +  datap->val.triple.user = get_nonempty_val (datap->cursor);

OK.

>    datap->cursor = (char *) __rawmemchr (datap->cursor, '\0') + 1;
> -  datap->val.triple.domain = datap->cursor;
> +  datap->val.triple.domain = get_nonempty_val (datap->cursor);

OK.

>    datap->cursor = (char *) __rawmemchr (datap->cursor, '\0') + 1;
>  
>    return NSS_STATUS_SUCCESS;
> 

Cheers,
Carlos.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTM6mrAAoJECXvCkNsKkr/0MEH/2R4vtGqYlyFokXMwcpynmzV
QzQMXe7GLMnMFaM0ttoL1nFh4ETpxrZTWyTNrEBaHdQcKCwjmKYTDbUihYdZTSrz
bDv+fBEFe5bWinUJdD+tByLa6b0lA7Jk8e/JTpoPxN3WAZrVCon3mCe6e1GC3Mf9
nm3faPWk5qMDQa4esoVqHmwm3af7QubDHcD3SRuA0GLOmF+al9itVjIvxpKyJrXI
GbBwfUv8XYrVAlkI1GCv56t3EvyCYTETAL9+QmMoSIqiRRmuC/ch81BiujOlcdDv
OETqGRuHKliNTlybUhkTBqSwaLXirpl3F3TWYHeb8l4MvC0AXVjiV9yetOHp5Ew=
=Yilu
-----END PGP SIGNATURE-----
  
Siddhesh Poyarekar March 27, 2014, 2:27 p.m. UTC | #2
On Thu, Mar 27, 2014 at 12:31:39AM -0400, Carlos O'Donell wrote:
> Looks good to me as long as you tested a wildcard in all
> of the positions.

Yep, tested wildcards in all positions using the same inputs as in the
earlier patch.

Siddhesh
  

Patch

diff --git a/inet/getnetgrent_r.c b/inet/getnetgrent_r.c
index 62cdfda..f6d064d 100644
--- a/inet/getnetgrent_r.c
+++ b/inet/getnetgrent_r.c
@@ -235,6 +235,14 @@  endnetgrent (void)
 }
 
 #ifdef USE_NSCD
+static const char *
+get_nonempty_val (const char *in)
+{
+  if (*in == '\0')
+    return NULL;
+  return in;
+}
+
 static enum nss_status
 nscd_getnetgrent (struct __netgrent *datap, char *buffer, size_t buflen,
 		  int *errnop)
@@ -243,11 +251,11 @@  nscd_getnetgrent (struct __netgrent *datap, char *buffer, size_t buflen,
     return NSS_STATUS_UNAVAIL;
 
   datap->type = triple_val;
-  datap->val.triple.host = datap->cursor;
+  datap->val.triple.host = get_nonempty_val (datap->cursor);
   datap->cursor = (char *) __rawmemchr (datap->cursor, '\0') + 1;
-  datap->val.triple.user = datap->cursor;
+  datap->val.triple.user = get_nonempty_val (datap->cursor);
   datap->cursor = (char *) __rawmemchr (datap->cursor, '\0') + 1;
-  datap->val.triple.domain = datap->cursor;
+  datap->val.triple.domain = get_nonempty_val (datap->cursor);
   datap->cursor = (char *) __rawmemchr (datap->cursor, '\0') + 1;
 
   return NSS_STATUS_SUCCESS;