diff mbox series

[v2] nsswitch: handle missing actions properly

Message ID xn360daysn.fsf@greed.delorie.com
State New
Headers show
Series [v2] nsswitch: handle missing actions properly | expand

Commit Message

DJ Delorie Dec. 11, 2020, 1:58 a.m. UTC
v2: syntax fixed, added testcases.

------------------------------

Some internal functions need to know if a database has a nonzero
list of actions; success getting the database does not guarantee
that.  Add checks for such as needed.

Skip the ":" in each nsswitch.conf line so as not to add a dummy
action libnss_:.so

See also https://bugzilla.redhat.com/show_bug.cgi?id=1906066

Comments

Siddhesh Poyarekar Dec. 14, 2020, 3:33 p.m. UTC | #1
On 12/11/20 7:28 AM, DJ Delorie via Libc-alpha wrote:
> 
> v2: syntax fixed, added testcases.
> 
> ------------------------------
> 
> Some internal functions need to know if a database has a nonzero
> list of actions; success getting the database does not guarantee
> that.  Add checks for such as needed.
> 
> Skip the ":" in each nsswitch.conf line so as not to add a dummy
> action libnss_:.so
> 
> See also https://bugzilla.redhat.com/show_bug.cgi?id=1906066
> 

LGTM.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
diff mbox series

Patch

diff --git a/grp/Makefile b/grp/Makefile
index 9d1dae91db..20683f649d 100644
--- a/grp/Makefile
+++ b/grp/Makefile
@@ -31,6 +31,10 @@  routines := fgetgrent initgroups setgroups \
 
 tests := testgrp tst-putgrent
 
+tests-container = \
+	tst-initgroups1 \
+	tst-initgroups2
+
 ifeq (yes,$(build-shared))
 test-srcs :=  tst_fgetgrent
 ifeq ($(run-built-tests),yes)
diff --git a/grp/initgroups.c b/grp/initgroups.c
index a60ca1c395..22736a17e2 100644
--- a/grp/initgroups.c
+++ b/grp/initgroups.c
@@ -72,11 +72,13 @@  internal_getgrouplist (const char *user, gid_t group, long int *size,
 
   nss_action_list nip;
 
-  if (__nss_database_get (nss_database_initgroups, &nip))
+  if (__nss_database_get (nss_database_initgroups, &nip)
+      && nip != NULL)
     {
       use_initgroups_entry = true;
     }
-  else if (__nss_database_get (nss_database_group, &nip))
+  else if (__nss_database_get (nss_database_group, &nip)
+	   && nip != NULL)
     {
       use_initgroups_entry = false;
     }
diff --git a/grp/tst-initgroups1.c b/grp/tst-initgroups1.c
new file mode 100644
index 0000000000..f6551cae9b
--- /dev/null
+++ b/grp/tst-initgroups1.c
@@ -0,0 +1,56 @@ 
+/* Test that initgroups works.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <nss.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/types.h>
+#include <errno.h>
+#include <unistd.h>
+#include <grp.h>
+
+#include <support/support.h>
+#include <support/check.h>
+
+/* Test that initgroups includes secondary groups.
+   https://bugzilla.redhat.com/show_bug.cgi?id=1906066  */
+
+/* This version uses the wrapper around the groups module.  */
+
+#define EXPECTED_N_GROUPS 4
+static gid_t expected_groups[] =
+  { 20, 30, 50, 51 };
+
+static int
+do_test (void)
+{
+  gid_t mygroups [50];
+  int i, n;
+
+  n = 50;
+  getgrouplist ("dj", 20, mygroups, &n);
+
+  TEST_COMPARE (n, EXPECTED_N_GROUPS);
+  for (i=0; i<n; i++)
+    TEST_COMPARE (mygroups[i], expected_groups[i]);
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/grp/tst-initgroups1.root/etc/group b/grp/tst-initgroups1.root/etc/group
new file mode 100644
index 0000000000..0dac1cc2ba
--- /dev/null
+++ b/grp/tst-initgroups1.root/etc/group
@@ -0,0 +1,7 @@ 
+abc:x:10:
+def:x:20:
+ghi:x:30:dj
+jkl:x:40:
+m:x:50:not,dj
+n:x:51:dj,not
+np:x:60:djx
diff --git a/grp/tst-initgroups1.root/etc/nsswitch.conf b/grp/tst-initgroups1.root/etc/nsswitch.conf
new file mode 100644
index 0000000000..8d0a1aea13
--- /dev/null
+++ b/grp/tst-initgroups1.root/etc/nsswitch.conf
@@ -0,0 +1 @@ 
+group : files 
diff --git a/grp/tst-initgroups1.root/etc/passwd b/grp/tst-initgroups1.root/etc/passwd
new file mode 100644
index 0000000000..5e3a2a5eea
--- /dev/null
+++ b/grp/tst-initgroups1.root/etc/passwd
@@ -0,0 +1 @@ 
+dj:x:84:20:DJ:/:/bin/sh
diff --git a/grp/tst-initgroups2.c b/grp/tst-initgroups2.c
new file mode 100644
index 0000000000..776e560ec9
--- /dev/null
+++ b/grp/tst-initgroups2.c
@@ -0,0 +1,21 @@ 
+/* Test that initgroups works.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include "tst-initgroups1.c"
+
+/* This version uses the initgroups built in to the files module.  */
diff --git a/grp/tst-initgroups2.root/etc/group b/grp/tst-initgroups2.root/etc/group
new file mode 100644
index 0000000000..0dac1cc2ba
--- /dev/null
+++ b/grp/tst-initgroups2.root/etc/group
@@ -0,0 +1,7 @@ 
+abc:x:10:
+def:x:20:
+ghi:x:30:dj
+jkl:x:40:
+m:x:50:not,dj
+n:x:51:dj,not
+np:x:60:djx
diff --git a/grp/tst-initgroups2.root/etc/nsswitch.conf b/grp/tst-initgroups2.root/etc/nsswitch.conf
new file mode 100644
index 0000000000..c61f3624f6
--- /dev/null
+++ b/grp/tst-initgroups2.root/etc/nsswitch.conf
@@ -0,0 +1,2 @@ 
+initgroups : files
+group : notfiles 
diff --git a/grp/tst-initgroups2.root/etc/passwd b/grp/tst-initgroups2.root/etc/passwd
new file mode 100644
index 0000000000..5e3a2a5eea
--- /dev/null
+++ b/grp/tst-initgroups2.root/etc/passwd
@@ -0,0 +1 @@ 
+dj:x:84:20:DJ:/:/bin/sh
diff --git a/nss/nss_database.c b/nss/nss_database.c
index e8c307d1f3..a036e95fbf 100644
--- a/nss/nss_database.c
+++ b/nss/nss_database.c
@@ -212,7 +212,8 @@  process_line (struct nss_database_data *data, char *line)
   if (line[0] == '\0' || name == line)
     /* Syntax error.  Skip this line.  */
     return true;
-  *line++ = '\0';
+  while (line[0] != '\0' && (isspace (line[0]) || line[0] == ':'))
+    *line++ = '\0';
 
   int db = name_to_database_index (name);
   if (db < 0)
diff --git a/nss/nsswitch.c b/nss/nsswitch.c
index 40109c744d..5b46d06c6a 100644
--- a/nss/nsswitch.c
+++ b/nss/nsswitch.c
@@ -81,7 +81,10 @@  __nss_database_lookup2 (const char *database, const char *alternate_name,
   if (database_names[database_id] == NULL)
     return -1;
 
-  if (__nss_database_get (database_id, ni))
+  /* If *NI is NULL, the database was not mentioned in nsswitch.conf.
+     If *NI is not NULL, but *NI->module is NULL, the database was in
+     nsswitch.conf but listed no actions.  We test for the former.  */
+  if (__nss_database_get (database_id, ni) && *ni != NULL)
     {
       /* Success.  */
       return 0;