Bug 11134 - getpwnam shows shadow passwords of NIS users
Summary: getpwnam shows shadow passwords of NIS users
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: unspecified
: P1 critical
Target Milestone: ---
Assignee: Ulrich Drepper
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-05 09:26 UTC by Christoph Pleger
Modified: 2014-06-30 20:29 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments
Patch for NIS shadow problems (1.39 KB, patch)
2010-01-05 09:28 UTC, Christoph Pleger
Details | Diff
New patch (1.38 KB, patch)
2010-01-06 07:58 UTC, Christoph Pleger
Details | Diff
Another try (1.85 KB, patch)
2010-02-17 13:18 UTC, Christoph Pleger
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christoph Pleger 2010-01-05 09:26:32 UTC
Hello,
I have several machines where almost all user accounts come by NIS. The NIS
server is running on a Solaris machine. As usual, the Solaris NIS server
exports the passwd data in the map "passwd" and the shadow data in the map
"passwd.adjunct.byname". These two maps are mangled together in some calls
of libc6, for example in getpwnam. This makes it possible for every user who
has an account on the NIS client machine to see the encrypted passwords of
all NIS users. This is a grave security bug.

Furthermore, getspnam returns a NULL pointer for all NIS users, even if
getspnam is called by root.

I wrote a patch nis_shadow.diff that solves these problems. It makes the 
following changes:

* In nis-pwd.c, do not mangle encrypted password from 
  passwd.adjunct.byname map  into the password field
  of passwd map, instead mangle an 'x' into the field

* In nis-spwd.c, look for key in passwd.adjunct.byname if shadow.byname
  does not exist and add the two missing fields (passwd.adjunct.byname
  has two fields less than shadow)

diff -Naurp glibc-2.7.original/nis/nss_nis/nis-pwd.c 
glibc-2.7/nis/nss_nis/nis-pwd.c
--- glibc-2.7.original/nis/nss_nis/nis-pwd.c	2006-05-02 00:31:15.000000000 
+0200
+++ glibc-2.7/nis/nss_nis/nis-pwd.c	2009-12-22 09:04:46.000000000 +0100
@@ -275,8 +275,8 @@ internal_nis_getpwent_r (struct passwd *
 	      yp_match (domain, "passwd.adjunct.byname", result, namelen,
 			&result2, &len2)) == YPERR_SUCCESS)
 	{
-	  /* We found a passwd.adjunct entry.  Merge encrypted
-	     password therein into original result.  */
+	  /* We found a passwd.adjunct entry.  Merge "x"
+	     into original result.  */
 	  char *encrypted = strchr (result2, ':');
 	  char *endp;
 	  size_t restlen;
@@ -304,7 +304,7 @@ internal_nis_getpwent_r (struct passwd *
 
 	  mempcpy (mempcpy (mempcpy (mempcpy (buffer, result, namelen),
 				     ":", 1),
-			    encrypted, endp - encrypted),
+			    "x", 1),
 		   p, restlen + 1);
 	  p = buffer;
 
@@ -408,8 +408,8 @@ _nss_nis_getpwnam_r (const char *name, s
       && yp_match (domain, "passwd.adjunct.byname", name, namelen,
 		   &result2, &len2) == YPERR_SUCCESS)
     {
-      /* We found a passwd.adjunct entry.  Merge encrypted password
-	 therein into original result.  */
+      /* We found a passwd.adjunct entry.  Merge "x"
+	 into original result.  */
       char *encrypted = strchr (result2, ':');
       char *endp;
 
@@ -436,7 +436,7 @@ _nss_nis_getpwnam_r (const char *name, s
 
       __mempcpy (__mempcpy (__mempcpy (__mempcpy (buffer, name, namelen),
 				       ":", 1),
-			    encrypted, endp - encrypted),
+			    "x", 1),
 		 p, restlen + 1);
       p = buffer;
 
@@ -509,8 +509,8 @@ _nss_nis_getpwuid_r (uid_t uid, struct p
 	  yp_match (domain, "passwd.adjunct.byname", result, namelen,
 		    &result2, &len2)) == YPERR_SUCCESS)
     {
-      /* We found a passwd.adjunct entry.  Merge encrypted password
-	 therein into original result.  */
+      /* We found a passwd.adjunct entry.  Merge "x"
+	 into original result.  */
       char *encrypted = strchr (result2, ':');
       char *endp;
       size_t restlen;
@@ -538,7 +538,7 @@ _nss_nis_getpwuid_r (uid_t uid, struct p
 
       __mempcpy (__mempcpy (__mempcpy (__mempcpy (buffer, result, namelen),
 				       ":", 1),
-			    encrypted, endp - encrypted),
+			    "x", 1),
 		 p, restlen + 1);
       p = buffer;
 
diff -Naurp glibc-2.7.original/nis/nss_nis/nis-spwd.c 
glibc-2.7/nis/nss_nis/nis-spwd.c
--- glibc-2.7.original/nis/nss_nis/nis-spwd.c	2006-04-29 03:09:49.000000000 
+0200
+++ glibc-2.7/nis/nss_nis/nis-spwd.c	2009-12-22 10:02:25.000000000 +0100
@@ -78,17 +78,42 @@ internal_nis_getspent_r (struct spwd *sp
     {
       char *result;
       char *outkey;
+      char *p;
       int len;
       int keylen;
       int yperr;
+      int adjunct_used = 0;
 
-      if (new_start)
+      if (new_start) {
         yperr = yp_first (domain, "shadow.byname", &outkey, &keylen, &result,
 			  &len);
-      else
+        
+        if (yperr == YPERR_MAP) {
+	  if (result != NULL)
+	    free result;
+
+	  yperr = yp_first (domain, "passwd.adjunct.byname", &outkey, &keylen, 
&result,
+			    &len);
+
+	  adjunct_used = 1;
+	}
+      }
+          
+      else {
         yperr = yp_next (domain, "shadow.byname", oldkey, oldkeylen, &outkey,
 			 &keylen, &result, &len);
 
+        if (yperr == YPERR_MAP) {
+	  if (result != NULL)
+	    free result;
+
+	  yperr = yp_next (domain, "passwd.adjunct.byname", oldkey, oldkeylen, 
&outkey,
+			   &keylen, &result, &len);
+	  
+	  adjunct_used = 1;
+	}
+      }
+
       if (__builtin_expect (yperr != YPERR_SUCCESS, 0))
         {
 	  enum nss_status retval = yperr2nss (yperr);
@@ -98,15 +123,32 @@ internal_nis_getspent_r (struct spwd *sp
           return retval;
         }
 
-      if (__builtin_expect ((size_t) (len + 1) > buflen, 0))
-        {
-          free (result);
-	  *errnop = ERANGE;
-          return NSS_STATUS_TRYAGAIN;
-        }
+      if (! adjunct_used)
+	{
+	  if (__builtin_expect ((size_t) (len + 1) > buflen, 0))
+	    {
+	      free (result);
+	      *errnop = ERANGE;
+	      return NSS_STATUS_TRYAGAIN;
+	    }
+
+	  p = strncpy (buffer, result, len);
+	  buffer[len] = '\0';  
+	}
+      else
+	{
+	  if (__builtin_expect ((size_t) (len + 3) > buflen, 0))
+	    {
+	      free (result);
+	      *errnop = ERANGE;
+	      return NSS_STATUS_TRYAGAIN;
+	    }
+
+	  p = strncpy (buffer, result, len);
+	  buffer[len] = '\0';  
+	  p = strcat (buffer, "::");
+	}
 
-      char *p = strncpy (buffer, result, len);
-      buffer[len] = '\0';
       while (isspace (*p))
         ++p;
       free (result);
@@ -149,6 +191,9 @@ enum nss_status
 _nss_nis_getspnam_r (const char *name, struct spwd *sp,
 		     char *buffer, size_t buflen, int *errnop)
 {
+  int adjunct_used = 0;
+  char *p;
+
   if (name == NULL)
     {
       *errnop = EINVAL;
@@ -164,6 +209,15 @@ _nss_nis_getspnam_r (const char *name, s
   int yperr = yp_match (domain, "shadow.byname", name, strlen (name), &result,
 			&len);
 
+  if (yperr == YPERR_MAP) {
+    if (result != NULL)
+      free result;
+
+    yperr = yp_match (domain, "passwd.adjunct.byname", name, strlen (name), 
&result,
+		      &len);
+    adjunct_used = 1;
+  }
+
   if (__builtin_expect (yperr != YPERR_SUCCESS, 0))
     {
       enum nss_status retval = yperr2nss (yperr);
@@ -173,15 +227,32 @@ _nss_nis_getspnam_r (const char *name, s
       return retval;
     }
 
-  if (__builtin_expect ((size_t) (len + 1) > buflen, 0))
+  if (! adjunct_used)
     {
-      free (result);
-      *errnop = ERANGE;
-      return NSS_STATUS_TRYAGAIN;
+      if (__builtin_expect ((size_t) (len + 1) > buflen, 0))
+	{
+	  free (result);
+	  *errnop = ERANGE;
+	  return NSS_STATUS_TRYAGAIN;
+	}
+
+      p = strncpy (buffer, result, len);
+      buffer[len] = '\0';  
     }
+  else
+    {
+      if (__builtin_expect ((size_t) (len + 3) > buflen, 0))
+	{
+	  free (result);
+	  *errnop = ERANGE;
+	  return NSS_STATUS_TRYAGAIN;
+	}
 
-  char *p = strncpy (buffer, result, len);
-  buffer[len] = '\0';
+      p = strncpy (buffer, result, len);
+      buffer[len] = '\0';  
+      p = strcat (buffer, "::");
+    }
+  
   while (isspace (*p))
     ++p;
   free (result);


Regards
  Christoph
Comment 1 Christoph Pleger 2010-01-05 09:28:16 UTC
Created attachment 4491 [details]
Patch for NIS shadow problems

Here is my patch in form of an attachment.
Comment 2 Christoph Pleger 2010-01-06 07:58:54 UTC
Created attachment 4498 [details]
New patch

I was told that there are missing parantheses in  free result. Obviously, I
sent an obsolete version of the patch. Here is the version that I used to
compile glibc successfully.
Comment 3 Christoph Pleger 2010-02-17 13:15:27 UTC
Hello,

I am sorry that my patch for the NIS shadow password security
vulnerability introduced a new bug. One of my NIS users informed me
that she could not login any more after she had used chsh to change her
login shell. The reason was that in the shadow file, the encrypted
password had been replaced by an 'x'. This happens because in my
patch, file nis-pwd.c, the string "##<username>" is replaced with "x". 

I thought that this replacement is necessary to let libc6 search for
the encrypted password in the shadow map. But now I found out that it
is not necessary and that without it everything works fine: logging in,
changing password and changing the shell.


I have attached a new patch that simply lets the password field of the
passwd.byname map alone.

Regards
  Christoph
Comment 4 Christoph Pleger 2010-02-17 13:18:13 UTC
Created attachment 4605 [details]
Another try
Comment 5 Ulrich Drepper 2010-04-06 22:53:23 UTC
I'm not so sure about either change.

The server can regulate which process can read the passwd.adjunct database using
the source port number.  A value < 1024 would indicate privileges.  If an
attacker can illegally bind a socket to a low port security is already
compromised.  The code in libc will ignore the error from being denied access
and will use the original entry from /etc/passwd as-is.

That's how it is meant to be used.  In this model processes with privileges can
get to the information.  Especially because I don't think imitating the shadow
file using the passwd.adjunct content is going to work.

You say there are two fields missing in passwd.adjunct.  In theory perhaps true
but I have not found anywhere any indication that usually the file contains any
information except the first two fields.  That's not really the correct content
for the file.  It means no password aging etc happens.


Changing the implementation along your patch sounds arbitrary.  The current
behavior re filling in the password might be used by some people.  There is no
way in Sun's implementation to enable behavior like this?  There is no setting
in Sun's ypserv to restrict access based on ports?  I cannot change it without a
good reason.

The bigger problem is the synthetic shadow file.  I don't like this at all.  If
you want a shadow file, why don't you export one from the server?  I realize
that if you say you don't want a shadow file and restricted access to passwd and
the server doesn't have port-based access control that you then want these
changes.  But these are lots of ifs.

The current libc implementation works perfectly if you use the model I
described.  You get a full passwd file for privileged users and a version
without the password for non-privileged users.  This is a sensible model and
your patch would cause it to stop working.
Comment 6 Christoph Pleger 2010-04-07 07:56:19 UTC
Subject: Re:  getpwnam shows shadow passwords of NIS users

Hello,

> The server can regulate which process can read the passwd.adjunct
> database using the source port number.  A value < 1024 would indicate
> privileges.  If an attacker can illegally bind a socket to a low port
> security is already compromised.

Normal permissions should prevent an attacker from illegally binding a
socket to a port < 1024. Of course there can be a security hole that
gives root privileges. But a security hole of that kind gives access to
everything, in spite of that no sensible administrator gives
permissions 777 to all files. So, we must find a solution for the
normal case which says that no ordinary user can use a port < 1024,
not a solution for a case where another security hole is already
present.

Of course a user can connect his own notebook to the network, be root
on it, which allows to use a port < 1024, and read the encrypted
passwords. But that problem can be solved by other means, for example
by IPSec authentication.

> That's how it is meant to be used.  In this model processes with
> privileges can get to the information.  Especially because I don't
> think imitating the shadow file using the passwd.adjunct content is
> going to work.

Where do you see a problem? I've been using this for some time now and
the only problem I found was the overwriting of the password field,
which I solved by the modified patch.

> You say there are two fields missing in passwd.adjunct.  In theory
> perhaps true but I have not found anywhere any indication that
> usually the file contains any information except the first two
> fields.

Right, that is why I put empty strings into these fields. These
field are defined in libc6, what will getspnam do if they are not
present?

> There is no way in Sun's implementation to enable behavior
> like this?  There is no setting in Sun's ypserv to restrict access
> based on ports?  I cannot change it without a good reason.

The access IS restricted on ports. But that does not help when,
on the Linux client side, nscd is in use.

> The current libc implementation works perfectly if you use the model I
> described.  You get a full passwd file for privileged users and a
> version without the password for non-privileged users.

Unfortunately, that is not true. The current implementation allows
EVERY user to use the getpwnam library call to see the encrypted
password of any NIS user.

> This is a
> sensible model and your patch would cause it to stop working.

No, my patch MAKES it working.

Regards
  Christoph
Comment 7 Ulrich Drepper 2010-04-07 14:41:26 UTC
I decided to implement this but only as a non-default mode.  It can be selected
by a new variable in /etc/default/nss.  This is as far as I'm willing to go.  As
I explained, the current code has its own justification and is not broken.

Your last patch still contained a bunch of mistakes.  The change I checked in
really has not much to do with it.
Comment 8 Jackie Rosen 2014-02-16 19:35:03 UTC Comment hidden (spam)
Comment 9 Florian Weimer 2014-06-30 20:29:44 UTC
This is not a security issue because the data is still available through tools like ypcat, even if glibc's NSS module filters it.