Bug#560333: libc6: getpwnam shows shadow passwords of NIS users
On Tue, Dec 22, 2009 at 12:51:30PM +0100, Christoph Pleger wrote:
> The attached patch seems to solve the problems. It works with nscd as well as
> without nscd. Authentication works fine now.
>
> 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)
>
> Maybe some people can have a look over my patch to see if I missed anything.
Thanks for the patch. I don't know a lot neither about NIS nor about the
NIS API, but it basically looks ok. Some minor changes and code indentation
will probably be need before it can be accepted upstream. See my comments
inline.
In the meanwhile, I'll include it in the next glibc upload to unstable.
If no bug reports are reported after a few days, we'll also fix stable.
> 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),
I guess the test above including (endp - encrypted) and that doesn't
appear in this patch should also be updated. Not doing so does not break
anything though.
Same issues on the similar portion of code below.
> 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;
This should probably be free(result). Also there is indentation issues.
Same issues on the similar portion of code below.
> +
> + 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, "::");
Upstream will probably say there is more optimised code to do that, but
I am fine with it.
> + }
>
> - 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);
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
Reply to: