nss: fix nss_database_lookup2's alternate handling [27416]

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

Commit Message

DJ Delorie Feb. 19, 2021, 10:22 p.m. UTC
  https://sourceware.org/bugzilla/show_bug.cgi?id=27416
  

Comments

Carlos O'Donell Feb. 20, 2021, 4:19 p.m. UTC | #1
On 2/19/21 5:22 PM, DJ Delorie via Libc-alpha wrote:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=27416

Could you please provide the full commit message you plan to
use for this commit?

If you commit your change locally then do:
git format-patch HEAD~1
git send-email mydiff.patch

That will do everything you need automatically. It avoids
having to anything manually like generating a subject like
with lowercase 'patch' or failing to include the exact commit
message as it will be when you apply the patch.

Please repost v2.

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

OK. New container test.

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

OK. Define the three compat databases for use in the generic code.

>  
>  /*
>     Local Variables:
> diff --git a/nss/nss_database.c b/nss/nss_database.c
> index fb72d0cc03..9ff3bb6ffb 100644
> --- a/nss/nss_database.c
> +++ b/nss/nss_database.c
> @@ -172,7 +172,7 @@ nss_database_select_default (struct nss_database_default_cache *cache,
>  
>  /* database_name must be large enough for each individual name plus a
>     null terminator.  */
> -typedef char database_name[11];
> +typedef char database_name[14];

OK. Increase array size.

>  #define DEFINE_DATABASE(name) \
>    _Static_assert (sizeof (#name) <= sizeof (database_name), #name);
>  #include "databases.def"
> diff --git a/nss/nsswitch.c b/nss/nsswitch.c
> index 46f232d720..6d8673b507 100644
> --- a/nss/nsswitch.c
> +++ b/nss/nsswitch.c
> @@ -76,24 +76,26 @@ __nss_database_lookup2 (const char *database, const char *alternate_name,
>  
>    for (database_id = 0; database_names[database_id]; database_id++)
>      if (strcmp (database_names[database_id], database) == 0)
> -	break;
> -
> -  if (database_names[database_id] == NULL)
> -    return -1;
> -
> -  /* 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;
> -    }
> -  else
> -    {
> -      /* Failure.  */
> -      return -1;
> -    }
> +      /* 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)
> +	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) && *ni != NULL)
> +	  return 0;
> +
> +  /* Neither found, use default config.  */
> +  *ni = __nss_action_parse (defconfig);
> +  if (*ni != NULL)
> +    return 0;
> +
> +  /* Failure.  */
> +  return -1;

OK. Add code to try the alternate or default.

>  }
>  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..45355ef225
> --- /dev/null
> +++ b/nss/tst-nss-compat1.c
> @@ -0,0 +1,64 @@
> +/* 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 <support/support.h>
> +#include <support/check.h>
> +
> +#include "nss_test.h"
> +
> +static struct passwd pwd_table[] = {
> +    PWD (100),
> +    PWD (30),
> +    PWD_LAST ()
> +  };
> +
> +void
> +_nss_test1_init_hook(test_tables *t)
> +{
> +  t->pwd_table = pwd_table;
> +}
> +
> +static int
> +do_test (void)
> +{
> +  struct passwd *p = 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");

OK. Password uses compat which redirects to test1 instead of files.
So seeing name100 means files was used and that's wrong.

> +
> +  /* 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");

OK.

> +
> +  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/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
>
  
Andreas Schwab Feb. 22, 2021, 12:13 p.m. UTC | #2
Still broken.

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..9ff3bb6ffb 100644
--- a/nss/nss_database.c
+++ b/nss/nss_database.c
@@ -172,7 +172,7 @@  nss_database_select_default (struct nss_database_default_cache *cache,
 
 /* 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"
diff --git a/nss/nsswitch.c b/nss/nsswitch.c
index 46f232d720..6d8673b507 100644
--- a/nss/nsswitch.c
+++ b/nss/nsswitch.c
@@ -76,24 +76,26 @@  __nss_database_lookup2 (const char *database, const char *alternate_name,
 
   for (database_id = 0; database_names[database_id]; database_id++)
     if (strcmp (database_names[database_id], database) == 0)
-	break;
-
-  if (database_names[database_id] == NULL)
-    return -1;
-
-  /* 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;
-    }
-  else
-    {
-      /* Failure.  */
-      return -1;
-    }
+      /* 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)
+	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) && *ni != NULL)
+	  return 0;
+
+  /* Neither found, use default config.  */
+  *ni = __nss_action_parse (defconfig);
+  if (*ni != NULL)
+    return 0;
+
+  /* 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..45355ef225
--- /dev/null
+++ b/nss/tst-nss-compat1.c
@@ -0,0 +1,64 @@ 
+/* 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 <support/support.h>
+#include <support/check.h>
+
+#include "nss_test.h"
+
+static struct passwd pwd_table[] = {
+    PWD (100),
+    PWD (30),
+    PWD_LAST ()
+  };
+
+void
+_nss_test1_init_hook(test_tables *t)
+{
+  t->pwd_table = pwd_table;
+}
+
+static int
+do_test (void)
+{
+  struct passwd *p = 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");
+
+  /* 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/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