[PATCH] nss: Remove cryptographic key from nss_files

[PATCH] nss: Remove cryptographic key from nss_files

[Florian Weimer]

The interface has hard-coded buffer sizes and is therefore tied to
DES.  It also does not match current practice where different
services on the same host use different key material.

This change simplifies removal of the sunrpc code.

---
Tested on x86_64-linux-gnu (with the glibc test suite).

 NEWS                      |   3 ++
 nss/Makefile              |   3 +-
 nss/nss_files/files-key.c | 113 ----------------------------------------------
 3 files changed, 5 insertions(+), 114 deletions(-)

diff --git a/NEWS b/NEWS
index 1f70b73edd..ffb389161b 100644
--- a/NEWS
+++ b/NEWS
@@ -57,6 +57,9 @@ Deprecated and removed features, and other changes affecting compatibility:
 * ldconfig now defaults to the new format for ld.so.cache. glibc has
   already supported this format for almost 20 years.
 
+* The "files" NSS module can no longer process DES public or private
+  keys.  The contents of the /etc/publickey file is ignored.
+
 Changes to build and runtime requirements:
 
 * powerpc64le requires GCC 7.4 or newer.  This is required for supporting
diff --git a/nss/Makefile b/nss/Makefile
index 97bab5bb75..cbb70167a9 100644
--- a/nss/Makefile
+++ b/nss/Makefile
@@ -93,7 +93,8 @@ subdir-dirs = $(services:%=nss_%)
 vpath %.c $(subdir-dirs) ../locale/programs ../intl
 
 
-libnss_files-routines	:= $(addprefix files-,$(databases)) \
+libnss_files-routines	:= $(addprefix files-, \
+			     $(filter-out key, $(databases))) \
 			   files-initgroups files-init
 
 libnss_db-dbs		:= $(addprefix db-,\
diff --git a/nss/nss_files/files-key.c b/nss/nss_files/files-key.c
deleted file mode 100644
index cf0a7d9ad9..0000000000
--- a/nss/nss_files/files-key.c
+++ /dev/null
@@ -1,113 +0,0 @@
-/* Public key file parser in nss_files module.
-   Copyright (C) 1996-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 <stdio.h>
-#include <errno.h>
-#include <string.h>
-#include <netdb.h>
-#include <rpc/key_prot.h>
-#include <rpc/des_crypt.h>
-#include "nsswitch.h"
-
-NSS_DECLARE_MODULE_FUNCTIONS (files)
-
-#define DATAFILE "/etc/publickey"
-
-
-static enum nss_status
-search (const char *netname, char *result, int *errnop, int secret)
-{
-  FILE *stream = fopen (DATAFILE, "rce");
-  if (stream == NULL)
-    return errno == EAGAIN ? NSS_STATUS_TRYAGAIN : NSS_STATUS_UNAVAIL;
-
-  for (;;)
-    {
-      char buffer[HEXKEYBYTES * 2 + KEYCHECKSUMSIZE + MAXNETNAMELEN + 17];
-      char *p;
-      char *save_ptr;
-
-      buffer[sizeof (buffer) - 1] = '\xff';
-      p = fgets_unlocked (buffer, sizeof (buffer), stream);
-      if (p == NULL)
-	{
-	  /* End of file or read error.  */
-	  *errnop = errno;
-	  fclose (stream);
-	  return NSS_STATUS_NOTFOUND;
-	}
-      else if (buffer[sizeof (buffer) - 1] != '\xff')
-	{
-	  /* Invalid line in file?  Skip remainder of line.  */
-	  if (buffer[sizeof (buffer) - 2] != '\0')
-	    while (getc_unlocked (stream) != '\n')
-	      continue;
-	  continue;
-	}
-
-      /* Parse line.  */
-      p = __strtok_r (buffer, "# \t:\n", &save_ptr);
-      if (p == NULL) /* Skip empty and comment lines.  */
-	continue;
-      if (strcmp (p, netname) != 0)
-	continue;
-
-      /* A hit!  Find the field we want and return.  */
-      p = __strtok_r (NULL, ":\n", &save_ptr);
-      if (p == NULL)  /* malformed line? */
-	continue;
-      if (secret)
-	p = __strtok_r (NULL, ":\n", &save_ptr);
-      if (p == NULL)  /* malformed line? */
-	continue;
-      fclose (stream);
-      strcpy (result, p);
-      return NSS_STATUS_SUCCESS;
-    }
-}
-
-enum nss_status
-_nss_files_getpublickey (const char *netname, char *pkey, int *errnop)
-{
-  return search (netname, pkey, errnop, 0);
-}
-
-enum nss_status
-_nss_files_getsecretkey (const char *netname, char *skey, char *passwd,
-			 int *errnop)
-{
-  enum nss_status status;
-  char buf[HEXKEYBYTES + KEYCHECKSUMSIZE + 16];
-
-  skey[0] = 0;
-
-  status = search (netname, buf, errnop, 1);
-  if (status != NSS_STATUS_SUCCESS)
-    return status;
-
-  if (!xdecrypt (buf, passwd))
-    return NSS_STATUS_SUCCESS;
-
-  if (memcmp (buf, &(buf[HEXKEYBYTES]), KEYCHECKSUMSIZE) != 0)
-    return NSS_STATUS_SUCCESS;
-
-  buf[HEXKEYBYTES] = 0;
-  strcpy (skey, buf);
-
-  return NSS_STATUS_SUCCESS;
-}

Re: [PATCH] nss: Remove cryptographic key from nss_files

[Petr Vorel]

Hi Florian,

> The interface has hard-coded buffer sizes and is therefore tied to
> DES.  It also does not match current practice where different
> services on the same host use different key material.

> This change simplifies removal of the sunrpc code.
Thanks!

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

Re: [PATCH] nss: Remove cryptographic key from nss_files

[Zack Weinberg]

On Tue, Jun 30, 2020 at 11:52 AM Florian Weimer via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> The interface has hard-coded buffer sizes and is therefore tied to
> DES.  It also does not match current practice where different
> services on the same host use different key material.

I just want to suggest a small tweak to the wording in NEWS:

> +* The "files" NSS module can no longer process DES public or private
> +  keys.  The contents of the /etc/publickey file is ignored.

This will be confusing to anyone who knows what DES is but not how
Sun's "secure" RPC extension used it, because DES is a symmetric
cipher.  I had to look this up myself and I'm still not sure I get it.
I suggest instead

+ * The "files" NSS module no longer supports the "key" database
+ (used for secure RPC).  The contents of the /etc/publickey file
+ will be ignored, regardless of the settings in /etc/nsswitch.conf.
+ (This method of storing RPC keys only supported the obsolete and
+ insecure AUTH_DES flavor of secure RPC.)

zw

Re: [PATCH] nss: Remove cryptographic key from nss_files

[Florian Weimer]

* Zack Weinberg:

> On Tue, Jun 30, 2020 at 11:52 AM Florian Weimer via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> The interface has hard-coded buffer sizes and is therefore tied to
>> DES.  It also does not match current practice where different
>> services on the same host use different key material.
>
> I just want to suggest a small tweak to the wording in NEWS:
>
>> +* The "files" NSS module can no longer process DES public or private
>> +  keys.  The contents of the /etc/publickey file is ignored.
>
> This will be confusing to anyone who knows what DES is but not how
> Sun's "secure" RPC extension used it, because DES is a symmetric
> cipher.  I had to look this up myself and I'm still not sure I get it.
> I suggest instead
>
> + * The "files" NSS module no longer supports the "key" database
> + (used for secure RPC).  The contents of the /etc/publickey file
> + will be ignored, regardless of the settings in /etc/nsswitch.conf.
> + (This method of storing RPC keys only supported the obsolete and
> + insecure AUTH_DES flavor of secure RPC.)

I've picked this up for V2, thanks.

Florian