[v3] nss: fix nss_database_lookup2's alternate handling [BZ #27416]

Message ID xnr1l351sz.fsf@greed.delorie.com
State Superseded
Headers
Series [v3] nss: fix nss_database_lookup2's alternate handling [BZ #27416] |

Commit Message

DJ Delorie Feb. 25, 2021, 8:29 p.m. UTC
  Andreas Schwab <schwab@linux-m68k.org> writes:
> Please add a test case where this block is reached.

__nss_database_lookup2's extra arguments were left unused in the
nsswitch reloading patch set; this broke compat (default config
ignored) and shadow files (secondary name ignored).

This patch adds in the previous behavior.  The test case
verifies that compat targets work (passwd) and that
the default configuration works (group).  Tested on x86-64.
---
 nss/Makefile                                  |  1 +
 nss/databases.def                             |  3 +
 nss/nss_database.c                            | 29 +++++-
 nss/nss_database.h                            | 12 +++
 nss/nss_test.h                                |  7 ++
 nss/nss_test1.c                               | 93 +++++++++++++++++++
 nss/nsswitch.c                                | 82 +++++++++++++---
 nss/tst-nss-compat1.c                         | 81 ++++++++++++++++
 nss/tst-nss-compat1.root/etc/group            |  1 +
 nss/tst-nss-compat1.root/etc/nsswitch.conf    |  3 +
 nss/tst-nss-compat1.root/etc/passwd           |  3 +
 nss/tst-nss-compat1.root/etc/shadow           |  2 +
 .../tst-nss-compat1.script                    |  1 +
 13 files changed, 299 insertions(+), 19 deletions(-)
 create mode 100644 nss/tst-nss-compat1.c
 create mode 100644 nss/tst-nss-compat1.root/etc/group
 create mode 100644 nss/tst-nss-compat1.root/etc/nsswitch.conf
 create mode 100644 nss/tst-nss-compat1.root/etc/passwd
 create mode 100644 nss/tst-nss-compat1.root/etc/shadow
 create mode 100644 nss/tst-nss-compat1.root/tst-nss-compat1.script
  

Comments

Andreas Schwab March 1, 2021, 11:14 a.m. UTC | #1
On Feb 25 2021, DJ Delorie via Libc-alpha wrote:

> diff --git a/nss/nss_database.h b/nss/nss_database.h
> index 1f827e6def..3590de3a78 100644
> --- a/nss/nss_database.h
> +++ b/nss/nss_database.h
> @@ -52,6 +52,13 @@ enum nss_database
>    NSS_DATABASE_COUNT
>  };
>  
> +enum nss_service_origin
> +{
> + NSS_ORIGIN_MISSING,
> + NSS_ORIGIN_DEFAULT,
> + NSS_ORIGIN_NSSWITCH
> +};
> +
>  
>  /* Looks up the action list for DB and stores it in *ACTIONS.  Returns
>     true on success or false on failure.  Success can mean that
> @@ -59,6 +66,10 @@ enum nss_database
>  bool __nss_database_get (enum nss_database db, nss_action_list *actions)
>    attribute_hidden;
>  
> +/* Looks up origin of the action list associated with this database.  */
> +enum nss_service_origin __nss_database_origin (enum nss_database db)
> +  attribute_hidden;
> +

Why do you need that?  If you remove the duplicated default handling
from nss_database.c, the defconfig parameter of __nss_database_lookup2
could just be used as intended, while reducing the complexity.

Andreas.
  

Patch

diff --git a/nss/Makefile b/nss/Makefile
index 0906202db9..71fbe583bf 100644
--- a/nss/Makefile
+++ b/nss/Makefile
@@ -63,6 +63,7 @@  tests			= test-netdb test-digits-dots tst-nss-getpwent bug17079 \
 xtests			= bug-erange
 
 tests-container = \
+			  tst-nss-compat1 \
 			  tst-nss-test3 \
 			  tst-nss-files-hosts-long \
 			  tst-nss-db-endpwent \
diff --git a/nss/databases.def b/nss/databases.def
index df5fab4168..3dc95648a8 100644
--- a/nss/databases.def
+++ b/nss/databases.def
@@ -23,17 +23,20 @@ 
 DEFINE_DATABASE (aliases)
 DEFINE_DATABASE (ethers)
 DEFINE_DATABASE (group)
+DEFINE_DATABASE (group_compat)
 DEFINE_DATABASE (gshadow)
 DEFINE_DATABASE (hosts)
 DEFINE_DATABASE (initgroups)
 DEFINE_DATABASE (netgroup)
 DEFINE_DATABASE (networks)
 DEFINE_DATABASE (passwd)
+DEFINE_DATABASE (passwd_compat)
 DEFINE_DATABASE (protocols)
 DEFINE_DATABASE (publickey)
 DEFINE_DATABASE (rpc)
 DEFINE_DATABASE (services)
 DEFINE_DATABASE (shadow)
+DEFINE_DATABASE (shadow_compat)
 
 /*
    Local Variables:
diff --git a/nss/nss_database.c b/nss/nss_database.c
index fb72d0cc03..0c0b7dd453 100644
--- a/nss/nss_database.c
+++ b/nss/nss_database.c
@@ -56,6 +56,7 @@  global_state_allocate (void *closure)
     {
       result->data.nsswitch_conf.size = -1; /* Force reload.  */
       memset (result->data.services, 0, sizeof (result->data.services));
+      memset (result->data.origins, 0, sizeof (result->data.origins));
       result->data.initialized = true;
       result->data.reload_disabled = false;
       __libc_lock_init (result->lock);
@@ -109,7 +110,8 @@  struct nss_database_default_cache
 
 static bool
 nss_database_select_default (struct nss_database_default_cache *cache,
-                             enum nss_database db, nss_action_list *result)
+                             enum nss_database db, nss_action_list *result,
+			     enum nss_service_origin *so)
 {
   enum nss_database_default def = per_database_defaults[db];
   *result = cache->caches[def];
@@ -166,13 +168,14 @@  nss_database_select_default (struct nss_database_default_cache *cache,
       assert (errno == ENOMEM);
       return false;
     }
-  else
-    return true;
+
+  *so = NSS_ORIGIN_DEFAULT;
+  return true;
 }
 
 /* database_name must be large enough for each individual name plus a
    null terminator.  */
-typedef char database_name[11];
+typedef char database_name[14];
 #define DEFINE_DATABASE(name) \
   _Static_assert (sizeof (#name) <= sizeof (database_name), #name);
 #include "databases.def"
@@ -231,6 +234,7 @@  process_line (struct nss_database_data *data, char *line)
   if (result == NULL)
     return false;
   data->services[db] = result;
+  data->origins[db] = NSS_ORIGIN_NSSWITCH;
   return true;
 }
 
@@ -259,6 +263,7 @@  __nss_configure_lookup (const char *dbname, const char *service_line)
 
   atomic_store_release (&local->data.reload_disabled, 1);
   local->data.services[db] = result;
+  local->data.origins[db] = NSS_ORIGIN_NSSWITCH;
 
 #ifdef USE_NSCD
   __nss_database_custom[db] = true;
@@ -335,7 +340,8 @@  nss_database_reload (struct nss_database_data *staging,
         if (staging->services[i] == NULL)
           {
             ok = nss_database_select_default (&cache, i,
-                                              &staging->services[i]);
+                                              &staging->services[i],
+					      &staging->origins[i]);
             if (!ok)
               break;
           }
@@ -454,6 +460,19 @@  __nss_database_get_noreload (enum nss_database db)
   return result;
 }
 
+enum nss_service_origin
+__nss_database_origin (enum nss_database db)
+{
+  /* There must have been a previous __nss_database_get call.  */
+  struct nss_database_state *local = atomic_load_acquire (&global_database_state);
+  assert (local != NULL);
+
+  __libc_lock_lock (local->lock);
+  enum nss_service_origin result = local->data.origins[db];
+  __libc_lock_unlock (local->lock);
+  return result;
+}
+
 void __libc_freeres_fn_section
 __nss_database_freeres (void)
 {
diff --git a/nss/nss_database.h b/nss/nss_database.h
index 1f827e6def..3590de3a78 100644
--- a/nss/nss_database.h
+++ b/nss/nss_database.h
@@ -52,6 +52,13 @@  enum nss_database
   NSS_DATABASE_COUNT
 };
 
+enum nss_service_origin
+{
+ NSS_ORIGIN_MISSING,
+ NSS_ORIGIN_DEFAULT,
+ NSS_ORIGIN_NSSWITCH
+};
+
 
 /* Looks up the action list for DB and stores it in *ACTIONS.  Returns
    true on success or false on failure.  Success can mean that
@@ -59,6 +66,10 @@  enum nss_database
 bool __nss_database_get (enum nss_database db, nss_action_list *actions)
   attribute_hidden;
 
+/* Looks up origin of the action list associated with this database.  */
+enum nss_service_origin __nss_database_origin (enum nss_database db)
+  attribute_hidden;
+
 /* Like __nss_database_get, but does not reload /etc/nsswitch.conf
    from disk.  This assumes that there has been a previous successful
    __nss_database_get call (which may not have returned any data).  */
@@ -73,6 +84,7 @@  struct nss_database_data
 {
   struct file_change_detection nsswitch_conf;
   nss_action_list services[NSS_DATABASE_COUNT];
+  enum nss_service_origin origins[NSS_DATABASE_COUNT];
   int reload_disabled;          /* Actually bool; int for atomic access.  */
   bool initialized;
 };
diff --git a/nss/nss_test.h b/nss/nss_test.h
index f8b81c27a7..db3d617585 100644
--- a/nss/nss_test.h
+++ b/nss/nss_test.h
@@ -33,11 +33,13 @@ 
 
 #include <pwd.h>
 #include <grp.h>
+#include <shadow.h>
 #include <netdb.h>
 
 typedef struct test_tables {
   struct passwd *pwd_table;
   struct group *grp_table;
+  struct spwd *spwd_table;
   struct hostent *host_table;
 } test_tables;
 
@@ -46,10 +48,12 @@  extern void _nss_test2_init_hook (test_tables *) __attribute__((weak));
 
 #define PWD_LAST()    { .pw_name = NULL, .pw_uid = 0 }
 #define GRP_LAST()    { .gr_name = NULL, .gr_gid = 0 }
+#define SPWD_LAST()    { .sp_namp = NULL, .sp_pwdp = NULL }
 #define HOST_LAST()    { .h_name = NULL, .h_aliases = NULL, .h_length = 0, .h_addr_list = NULL }
 
 #define PWD_ISLAST(p)    ((p)->pw_name == NULL && (p)->pw_uid == 0)
 #define GRP_ISLAST(g)    ((g)->gr_name == NULL && (g)->gr_gid == 0)
+#define SPWD_ISLAST(s)    ((s)->sp_namp == NULL && (s)->sp_pwdp == 0)
 #define HOST_ISLAST(h)    ((h)->h_name == NULL && (h)->h_length == 0)
 
 /* Macros to fill in the tables easily.  */
@@ -76,6 +80,9 @@  extern void _nss_test2_init_hook (test_tables *) __attribute__((weak));
     { .gr_name = (char *) n, .gr_passwd = (char *) "*", .gr_gid = u, \
       .gr_mem = (char **) m }
 
+#define SPWD(u) \
+    { .sp_namp = (char *) "name" #u, .sp_pwdp = (char *) "passwd" #u }
+
 #define HOST(u)								\
     { .h_name = (char *) "name" #u, .h_aliases = NULL, .h_addrtype = u,	\
       .h_length = 4,							\
diff --git a/nss/nss_test1.c b/nss/nss_test1.c
index f73c7a6cd8..f8d81831ee 100644
--- a/nss/nss_test1.c
+++ b/nss/nss_test1.c
@@ -66,6 +66,9 @@  static int npwd_data = default_npwd_data;
 static struct group *grp_data = NULL;
 static int ngrp_data = 0;
 
+static struct spwd *spwd_data = NULL;
+static int nspwd_data = 0;
+
 static struct hostent *host_data = NULL;
 static int nhost_data = 0;
 
@@ -102,6 +105,13 @@  init(void)
 	    ;
 	  ngrp_data = i;
 	}
+      if (t.spwd_table)
+	{
+	  spwd_data = t.spwd_table;
+	  for (i=0; ! SPWD_ISLAST(& spwd_data[i]); i++)
+	    ;
+	  nspwd_data = i;
+	}
       if (t.host_table)
 	{
 	  host_data = t.host_table;
@@ -323,6 +333,89 @@  NAME(getgrnam_r) (const char *name, struct group *result, char *buffer,
   return NSS_STATUS_NOTFOUND;
 }
 
+/* -------------------------------------------------- */
+/* Shadow password handling.  */
+
+static size_t spwd_iter;
+#define CURSPWD spwd_data[spwd_iter]
+
+static pthread_mutex_t spwd_lock = PTHREAD_MUTEX_INITIALIZER;
+
+enum nss_status
+NAME(setspent) (int stayopen)
+{
+  init();
+  spwd_iter = 0;
+  return NSS_STATUS_SUCCESS;
+}
+
+
+enum nss_status
+NAME(endspwent) (void)
+{
+  init();
+  return NSS_STATUS_SUCCESS;
+}
+
+static enum nss_status
+copy_shadow (struct spwd *result, struct spwd *local,
+	    char *buffer, size_t buflen, int *errnop)
+{
+  struct alloc_buffer buf = alloc_buffer_create (buffer, buflen);
+
+  result->sp_namp = alloc_buffer_maybe_copy_string (&buf, local->sp_namp);
+  result->sp_pwdp = alloc_buffer_maybe_copy_string (&buf, local->sp_pwdp);
+  result->sp_lstchg = local->sp_lstchg;
+  result->sp_min = local->sp_min;
+  result->sp_max = local->sp_max;
+  result->sp_warn = local->sp_warn;
+  result->sp_inact = local->sp_inact;
+  result->sp_expire = local->sp_expire;
+  result->sp_flag = local->sp_flag;
+
+  if (alloc_buffer_has_failed (&buf))
+    {
+      *errnop = ERANGE;
+      return NSS_STATUS_TRYAGAIN;
+    }
+
+  return NSS_STATUS_SUCCESS;
+}
+
+enum nss_status
+NAME(getspent_r) (struct spwd *result, char *buffer, size_t buflen,
+		  int *errnop)
+{
+  int res = NSS_STATUS_SUCCESS;
+
+  init();
+  pthread_mutex_lock (&spwd_lock);
+
+  if (spwd_iter >= nspwd_data)
+    res = NSS_STATUS_NOTFOUND;
+  else
+    {
+      res = copy_shadow (result, &CURSPWD, buffer, buflen, errnop);
+      ++spwd_iter;
+    }
+
+  pthread_mutex_unlock (&spwd_lock);
+
+  return res;
+}
+
+enum nss_status
+NAME(getspnam_r) (const char *name, struct spwd *result, char *buffer,
+		  size_t buflen, int *errnop)
+{
+  init();
+  for (size_t idx = 0; idx < nspwd_data; ++idx)
+    if (strcmp (spwd_data[idx].sp_namp, name) == 0)
+      return copy_shadow (result, &spwd_data[idx], buffer, buflen, errnop);
+
+  return NSS_STATUS_NOTFOUND;
+}
+
 /* -------------------------------------------------- */
 /* Host handling.  */
 
diff --git a/nss/nsswitch.c b/nss/nsswitch.c
index 46f232d720..4aba505583 100644
--- a/nss/nsswitch.c
+++ b/nss/nsswitch.c
@@ -63,6 +63,26 @@  static const char * database_names[] = {
 bool __nss_database_custom[NSS_DBSIDX_max];
 #endif
 
+/* Left for future developers who need to debug the lookup
+   function.  */
+#define DEBUG_LOOKUP 0
+
+#if DEBUG_LOOKUP
+static void
+debug_action_list (nss_action_list a)
+{
+  int i;
+  if (a == NULL)
+    {
+      printf("  (null-action-list)\n");
+      return;
+    }
+  printf("  action_list = [");
+  for (i=0; a[i].module != NULL; i++)
+    printf(" %s.%x", a[i].module->name, a[i].action_bits);
+  printf(" ]\n");
+}
+#endif
 
 /*__libc_lock_define_initialized (static, lock)*/
 
@@ -74,26 +94,60 @@  __nss_database_lookup2 (const char *database, const char *alternate_name,
 {
   int database_id;
 
+#if DEBUG_LOOKUP
+  printf("__nss_database_lookup2 %s %s %s\n",
+	 database ? database : "(null)",
+	 alternate_name ? alternate_name : "(null)",
+	 defconfig ? defconfig : "(null)");
   for (database_id = 0; database_names[database_id]; database_id++)
-    if (strcmp (database_names[database_id], database) == 0)
-	break;
+    {
+      __nss_database_get (database_id, ni);
+      printf(" - %s : %p %p %d :", database_names[database_id], *ni,
+	     *ni ? (void *)((*ni)->module) : (void *)"(nomod)",
+	     (int)__nss_database_origin (database_id));
+      debug_action_list (*ni);
+    }
+#endif
 
-  if (database_names[database_id] == NULL)
-    return -1;
+  for (database_id = 0; database_names[database_id]; database_id++)
+    if (strcmp (database_names[database_id], database) == 0)
+      if (__nss_database_get (database_id, ni)
+	  && __nss_database_origin (database_id) == NSS_ORIGIN_NSSWITCH)
+	{
+#if DEBUG_LOOKUP
+	  printf("lookup: primary found :");
+	  debug_action_list (*ni);
+#endif
+	  return 0;
+	}
+
+  /* Primary name not found, try alternate.  */
+  if (alternate_name)
+    for (database_id = 0; database_names[database_id]; database_id++)
+      if (strcmp (database_names[database_id], alternate_name) == 0)
+	if (__nss_database_get (database_id, ni)
+	  && __nss_database_origin (database_id) == NSS_ORIGIN_NSSWITCH)
+	  {
+#if DEBUG_LOOKUP
+	    printf("lookup: alternate found :");
+	    debug_action_list (*ni);
+#endif
+	    return 0;
+	  }
 
-  /* 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)
+  /* Neither found, use default config.  */
+  *ni = __nss_action_parse (defconfig);
+  if (*ni != NULL)
     {
-      /* Success.  */
+#if DEBUG_LOOKUP
+      printf("lookup: default used :");
+      debug_action_list (*ni);
+#endif
       return 0;
     }
-  else
-    {
-      /* Failure.  */
-      return -1;
-    }
+
+  /* Failure.  */
+  return -1;
 }
 libc_hidden_def (__nss_database_lookup2)
 
diff --git a/nss/tst-nss-compat1.c b/nss/tst-nss-compat1.c
new file mode 100644
index 0000000000..670cffe538
--- /dev/null
+++ b/nss/tst-nss-compat1.c
@@ -0,0 +1,81 @@ 
+/* Test error checking for group entries.
+   Copyright (C) 2021 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 <shadow.h>
+
+#include <support/support.h>
+#include <support/check.h>
+
+#include "nss_test.h"
+
+static struct passwd pwd_table[] = {
+    PWD (100),
+    PWD (30),
+    PWD_LAST ()
+  };
+
+static struct spwd spwd_table[] = {
+    SPWD (100),
+    SPWD (30),
+    SPWD_LAST ()
+  };
+
+void
+_nss_test1_init_hook(test_tables *t)
+{
+  t->pwd_table = pwd_table;
+  t->spwd_table = spwd_table;
+}
+
+static int
+do_test (void)
+{
+  struct passwd *p = NULL;
+  struct spwd *s = NULL;
+  struct group *g = NULL;
+
+  /* Test that compat-to-test works.  */
+  p = getpwuid (100);
+  if (p == NULL)
+    FAIL_EXIT1("getpwuid-compat-test1 p");
+  else if (strcmp (p->pw_name, "name100") != 0)
+    FAIL_EXIT1("getpwuid-compat-test1 name100");
+
+  /* Shadow compat should use passwd via the alternate name.  */
+  s = getspnam ("name30");
+  if (s == NULL)
+    FAIL_EXIT1("getspnam-compat-test1 s");
+  else if (strcmp (s->sp_namp, "name30") != 0)
+    FAIL_EXIT1("getpwuid-compat-test1 name30");
+
+  /* Test that internal defconfig works.  */
+  g = getgrgid (100);
+  if (g == NULL)
+    FAIL_EXIT1("getgrgid-compat-null");
+  if (strcmp (g->gr_name, "wilma") != 0)
+    FAIL_EXIT1("getgrgid-compat-name");
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/nss/tst-nss-compat1.root/etc/group b/nss/tst-nss-compat1.root/etc/group
new file mode 100644
index 0000000000..ee467c7950
--- /dev/null
+++ b/nss/tst-nss-compat1.root/etc/group
@@ -0,0 +1 @@ 
+wilma:x:100:
diff --git a/nss/tst-nss-compat1.root/etc/nsswitch.conf b/nss/tst-nss-compat1.root/etc/nsswitch.conf
new file mode 100644
index 0000000000..7fe69d5ffa
--- /dev/null
+++ b/nss/tst-nss-compat1.root/etc/nsswitch.conf
@@ -0,0 +1,3 @@ 
+passwd : compat
+passwd_compat : test1
+
diff --git a/nss/tst-nss-compat1.root/etc/passwd b/nss/tst-nss-compat1.root/etc/passwd
new file mode 100644
index 0000000000..84635587bd
--- /dev/null
+++ b/nss/tst-nss-compat1.root/etc/passwd
@@ -0,0 +1,3 @@ 
+name5:x:5:555:name5 for testing:/home/name5:/bin/nologin
++name100
++name30
diff --git a/nss/tst-nss-compat1.root/etc/shadow b/nss/tst-nss-compat1.root/etc/shadow
new file mode 100644
index 0000000000..cba3152172
--- /dev/null
+++ b/nss/tst-nss-compat1.root/etc/shadow
@@ -0,0 +1,2 @@ 
++name100
++name30
diff --git a/nss/tst-nss-compat1.root/tst-nss-compat1.script b/nss/tst-nss-compat1.root/tst-nss-compat1.script
new file mode 100644
index 0000000000..fe6e863f01
--- /dev/null
+++ b/nss/tst-nss-compat1.root/tst-nss-compat1.script
@@ -0,0 +1 @@ 
+cp $B/nss/libnss_test1.so $L/libnss_test1.so.2