Question on /etc/nsswitch.conf

Question on /etc/nsswitch.conf

[Yury Norov]

Hi all,

Recently Linaro issued the toolchain for arm64/ilp32 [1], and
running LTP compiled with it, I found multiple regressions. I
tracked them down to function family getpw*(). The problem is
that they return NULL, and don't set any errno.

#include <sys/types.h>
#include <pwd.h>
#include <errno.h>
#include <stdio.h>
#include <stddef.h>
#include <unistd.h>

int main(void)
{
        struct passwd *pwent;
        uid_t uid = getuid();
        printf("uid  == %d, errno = %d\n", uid,
                        errno);

        pwent = getpwuid(uid);
        printf("pwent  == %p, errno == %d\n", pwent, errno);

        return 0;
}

Steve Ellcey noticed that it's related to the "passwd: compat"
record in /etc/nsswitch.conf. On my testing system (Ubuntu 14.04) I replaced the
file with one coming in glibc sources (passwd is "db_files" there), and it fixed
the problem.

By this email I'd like to report the issue to community and ask some
questions:
 - I think it shuld be a bug if function returns NULL instead the pointer
   to the structure, and doesn't set errno. The POSIX [2] is not
   specific here: "If getpwuid() returns a null pointer and errno is
   set to non-zero, an error occurred". But if getpwuid() returns null,
   it is an error from user point of view (LTP treats is like this for
   example). But Glibc doesn't set errno, and POSIX doesn't restrict it
   explicitly.
 - system may host many toolchains, but there's a single nsswitch.conf
   for them all. It raises the problem of compatibility of settings,
   like this one. Fortunately, in this case I can modify the
   nsswitch.conf to make it suitable for both lp64 and ilp32 toolchains,
   but it may become a problem in general case... The most straightforward
   solution is to introduce a system variable that allows to alternate the 
   file path, but discussion on resolv.conf [3] shows that there may be
   other considerations. So I don't have the answer what to do.
 - On the other hand, it might be considered as local problem of
   Debian distro. In that case, I would like to ask if there's any way
   to update nsswitch.conf while installing the new library? People
   soon will start use the toolchain, and will have the problem. I
   think that now GLIBC is not informative in this case and doesn't help
   to track the issue down, even if the issue is not in GLIBC itself,
   but in configuration files. Can we change it? I think that simple
   warning would be lot helpful.

Yury

[1] http://snapshots.linaro.org/components/toolchain/binaries/7.1-2017.08-rc1/aarch64-linux-gnu_ilp32/
[2] http://pubs.opengroup.org/onlinepubs/9699919799/
[3] https://sourceware.org/ml/libc-alpha/2017-08/msg00742.html

Re: Question on /etc/nsswitch.conf

[Florian Weimer]

On 09/27/2017 11:50 PM, Yury Norov wrote:
> Hi all,
> 
> Recently Linaro issued the toolchain for arm64/ilp32 [1], and
> running LTP compiled with it, I found multiple regressions. I
> tracked them down to function family getpw*(). The problem is
> that they return NULL, and don't set any errno.
> 
> #include <sys/types.h>
> #include <pwd.h>
> #include <errno.h>
> #include <stdio.h>
> #include <stddef.h>
> #include <unistd.h>
> 
> int main(void)
> {
>          struct passwd *pwent;
>          uid_t uid = getuid();
>          printf("uid  == %d, errno = %d\n", uid,
>                          errno);
> 
>          pwent = getpwuid(uid);
>          printf("pwent  == %p, errno == %d\n", pwent, errno);
> 
>          return 0;
> }
> 
> Steve Ellcey noticed that it's related to the "passwd: compat"
> record in /etc/nsswitch.conf. On my testing system (Ubuntu 14.04) I replaced the
> file with one coming in glibc sources (passwd is "db_files" there), and it fixed
> the problem.
> 
> By this email I'd like to report the issue to community and ask some
> questions:
>   - I think it shuld be a bug if function returns NULL instead the pointer
>     to the structure, and doesn't set errno. The POSIX [2] is not
>     specific here: "If getpwuid() returns a null pointer and errno is
>     set to non-zero, an error occurred". But if getpwuid() returns null,
>     it is an error from user point of view (LTP treats is like this for
>     example). But Glibc doesn't set errno, and POSIX doesn't restrict it
>     explicitly.

POSIX requires that if the user does not exist, NULL is returned.  But 
this is not an error, so errno is not set.

Callers need to set errno to zero before calling those NSS functions. 
(Currently, many of them incorrectly set errno to zero on success, which 
is not allowed by POSIX.)  If LTP tests do not do this, these LTP tests 
need to be fixed.

Does this address your concerns?

Florian

Re: Question on /etc/nsswitch.conf

[Yury Norov]

On Thu, Sep 28, 2017 at 12:13:43PM +0200, Florian Weimer wrote:
> On 09/27/2017 11:50 PM, Yury Norov wrote:
> > Hi all,
> > 
> > Recently Linaro issued the toolchain for arm64/ilp32 [1], and
> > running LTP compiled with it, I found multiple regressions. I
> > tracked them down to function family getpw*(). The problem is
> > that they return NULL, and don't set any errno.
> > 
> > #include <sys/types.h>
> > #include <pwd.h>
> > #include <errno.h>
> > #include <stdio.h>
> > #include <stddef.h>
> > #include <unistd.h>
> > 
> > int main(void)
> > {
> >          struct passwd *pwent;
> >          uid_t uid = getuid();
> >          printf("uid  == %d, errno = %d\n", uid,
> >                          errno);
> > 
> >          pwent = getpwuid(uid);
> >          printf("pwent  == %p, errno == %d\n", pwent, errno);
> > 
> >          return 0;
> > }
> > 
> > Steve Ellcey noticed that it's related to the "passwd: compat"
> > record in /etc/nsswitch.conf. On my testing system (Ubuntu 14.04) I replaced the
> > file with one coming in glibc sources (passwd is "db_files" there), and it fixed
> > the problem.
> > 
> > By this email I'd like to report the issue to community and ask some
> > questions:
> >   - I think it shuld be a bug if function returns NULL instead the pointer
> >     to the structure, and doesn't set errno. The POSIX [2] is not
> >     specific here: "If getpwuid() returns a null pointer and errno is
> >     set to non-zero, an error occurred". But if getpwuid() returns null,
> >     it is an error from user point of view (LTP treats is like this for
> >     example). But Glibc doesn't set errno, and POSIX doesn't restrict it
> >     explicitly.
> 
> POSIX requires that if the user does not exist, NULL is returned.  But this
> is not an error, so errno is not set.
> 
> Callers need to set errno to zero before calling those NSS functions.
> (Currently, many of them incorrectly set errno to zero on success, which is
> not allowed by POSIX.)  If LTP tests do not do this, these LTP tests need to
> be fixed.
> 
> Does this address your concerns?
> 
> Florian

Yes, it is, thanks. It means that getpwuid(), getpwnam() etc are
broken because with NUll in return value and untouched errno they
report that no user record found in passwd. Whilst the record exists.

Below is the problem function from LTP, for reference. It doesn't
reset errno before calling getpwnam(), but this is different issue.

Yury

struct passwd *safe_getpwnam(const char *file, const int lineno,
                             void (*cleanup_fn) (void), const char *name)
{
        struct passwd *rval;

        rval = getpwnam(name);
        if (rval == NULL) {
                tst_brkm(TBROK | TERRNO, cleanup_fn,
                         "%s:%d: getpwnam(%s) failed",
                         file, lineno, name);
        }

        return rval;
}

Re: Question on /etc/nsswitch.conf

[Florian Weimer]

On 09/28/2017 01:28 PM, Yury Norov wrote:
> Yes, it is, thanks. It means that getpwuid(), getpwnam() etc are
> broken because with NUll in return value and untouched errno they
> report that no user record found in passwd. Whilst the record exists.

Is this because your new glibc doesn't have the compat module?

This old discussion is relevant:

   https://sourceware.org/ml/libc-alpha/2017-03/msg00200.html

compat is only loosely coupled with NIS (it's just the default fallback 
module), so we could still build it if it is used with other services as 
well, even if the NIS libraries are disabled.

Thanks,
Florian

Re: Question on /etc/nsswitch.conf

[Yury Norov]

On Thu, Sep 28, 2017 at 01:44:49PM +0200, Florian Weimer wrote:
> On 09/28/2017 01:28 PM, Yury Norov wrote:
> > Yes, it is, thanks. It means that getpwuid(), getpwnam() etc are
> > broken because with NUll in return value and untouched errno they
> > report that no user record found in passwd. Whilst the record exists.
> 
> Is this because your new glibc doesn't have the compat module?
> 

Do you mean libnss_compat? There's no this library in Linaro toolchain.
AFAIR, it was deprecated and there were already issues with it. 
https://sourceware.org/ml/libc-alpha/2017-08/msg00531.html

> This old discussion is relevant:
> 
>   https://sourceware.org/ml/libc-alpha/2017-03/msg00200.html
> 
> compat is only loosely coupled with NIS (it's just the default fallback
> module), so we could still build it if it is used with other services as
> well, even if the NIS libraries are disabled.
> 
> Thanks,
> Florian

Re: Question on /etc/nsswitch.conf

[Florian Weimer]

On 09/28/2017 02:28 PM, Yury Norov wrote:
> On Thu, Sep 28, 2017 at 01:44:49PM +0200, Florian Weimer wrote:
>> On 09/28/2017 01:28 PM, Yury Norov wrote:
>>> Yes, it is, thanks. It means that getpwuid(), getpwnam() etc are
>>> broken because with NUll in return value and untouched errno they
>>> report that no user record found in passwd. Whilst the record exists.
>> Is this because your new glibc doesn't have the compat module?

> Do you mean libnss_compat? There's no this library in Linaro toolchain.
> AFAIR, it was deprecated and there were already issues with it.
> https://sourceware.org/ml/libc-alpha/2017-08/msg00531.html

I believe that it might still be possible that compat without NIS is 
useful, and that people use it.

Thanks,
Florian

Re: Question on /etc/nsswitch.conf

[Zack Weinberg]

On Thu, Sep 28, 2017 at 9:29 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 09/28/2017 02:28 PM, Yury Norov wrote:
>>
>> On Thu, Sep 28, 2017 at 01:44:49PM +0200, Florian Weimer wrote:
>>>
>>> On 09/28/2017 01:28 PM, Yury Norov wrote:
>>>>
>>>> Yes, it is, thanks. It means that getpwuid(), getpwnam() etc are
>>>> broken because with NUll in return value and untouched errno they
>>>> report that no user record found in passwd. Whilst the record exists.
>>>
>>> Is this because your new glibc doesn't have the compat module?
>>
>> Do you mean libnss_compat? There's no this library in Linaro toolchain.
>> AFAIR, it was deprecated and there were already issues with it.
>> https://sourceware.org/ml/libc-alpha/2017-08/msg00531.html
>
> I believe that it might still be possible that compat without NIS is useful,
> and that people use it.

I expect there are, at least, a lot of existing installations with
"passwd: compat" in their nsswitch.conf, but probably most of them
don't use NIS.

I was under the impression that the only thing compat does is look for
the +:::::: notation in /etc/passwd and forward to NIS.  If so, or
even if it's more complicated than that, maybe the Right Thing is to
build a libnss_compat.so even when !BUILD_OBSOLETE_NSL, but have it
error out on +:::::: rather than forwarding to NIS.  The big problem I
see with this idea is it might make it harder for TIRPC to drop in a
replacement.  Thoughts?

zw

Re: Question on /etc/nsswitch.conf

[Florian Weimer]

On 09/28/2017 03:54 PM, Zack Weinberg wrote:
> I expect there are, at least, a lot of existing installations with
> "passwd: compat" in their nsswitch.conf, but probably most of them
> don't use NIS.
> 
> I was under the impression that the only thing compat does is look for
> the +:::::: notation in /etc/passwd and forward to NIS.  If so, or
> even if it's more complicated than that, maybe the Right Thing is to
> build a libnss_compat.so even when !BUILD_OBSOLETE_NSL, but have it
> error out on +:::::: rather than forwarding to NIS.  The big problem I
> see with this idea is it might make it harder for TIRPC to drop in a
> replacement.  Thoughts?

It is possible to redirect the lookup to something else besides NIS, 
using passwd_compat, group_compat, shadow_compat stanzas in 
/etc/nsswitch.conf.

Thanks,
Florian

Re: Question on /etc/nsswitch.conf

[Zack Weinberg]

On Thu, Sep 28, 2017 at 9:57 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 09/28/2017 03:54 PM, Zack Weinberg wrote:
>>
>> I expect there are, at least, a lot of existing installations with
>> "passwd: compat" in their nsswitch.conf, but probably most of them
>> don't use NIS.
>>
>> I was under the impression that the only thing compat does is look for
>> the +:::::: notation in /etc/passwd and forward to NIS.  If so, or
>> even if it's more complicated than that, maybe the Right Thing is to
>> build a libnss_compat.so even when !BUILD_OBSOLETE_NSL, but have it
>> error out on +:::::: rather than forwarding to NIS.  The big problem I
>> see with this idea is it might make it harder for TIRPC to drop in a
>> replacement.  Thoughts?
>
> It is possible to redirect the lookup to something else besides NIS, using
> passwd_compat, group_compat, shadow_compat stanzas in /etc/nsswitch.conf.

OK, so pull libnss_compat out of the nis directory, build and install
it unconditionally (but still leave it out of the default NSS rules)
and make sure it throws a sensible error if it encounters a forwarding
directive and the forwarded-to module isn't available?

zw

Re: Question on /etc/nsswitch.conf

[Andreas Schwab]

On Sep 28 2017, Zack Weinberg <zackw@panix.com> wrote:

> OK, so pull libnss_compat out of the nis directory, build and install
> it unconditionally (but still leave it out of the default NSS rules)
> and make sure it throws a sensible error if it encounters a forwarding
> directive and the forwarded-to module isn't available?

The only problem is that compat-pwd and compat-spwd need to refer to
yp_get_default_domain, for netgroup matches ([+-]@netgroup).

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: Question on /etc/nsswitch.conf

[Florian Weimer]

On 10/02/2017 02:30 PM, Andreas Schwab wrote:
> On Sep 28 2017, Zack Weinberg <zackw@panix.com> wrote:
> 
>> OK, so pull libnss_compat out of the nis directory, build and install
>> it unconditionally (but still leave it out of the default NSS rules)
>> and make sure it throws a sensible error if it encounters a forwarding
>> directive and the forwarded-to module isn't available?
> 
> The only problem is that compat-pwd and compat-spwd need to refer to
> yp_get_default_domain, for netgroup matches ([+-]@netgroup).

Could the code call uname directly instead?  As far as I can tell, 
yp_get_default_domain is just a thin wrapper around getdomainname, which 
is again a wrapper around uname.

Thanks,
Florian

Re: Question on /etc/nsswitch.conf

[Andreas Schwab]

On Okt 02 2017, Florian Weimer <fweimer@redhat.com> wrote:

> Could the code call uname directly instead?

uname has nothing to do with the NIS domain, or any domain.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: Question on /etc/nsswitch.conf

[Florian Weimer]

On 10/02/2017 02:50 PM, Andreas Schwab wrote:
> On Okt 02 2017, Florian Weimer <fweimer@redhat.com> wrote:
> 
>> Could the code call uname directly instead?
> 
> uname has nothing to do with the NIS domain, or any domain.

That's not how I read the implementation.  Could you elaborate?  How can 
the NIS domain and the kernel domain differ?

It's not what I would expect, but based on the code, I don't see how the 
NIS domain can be configured separately.  The local static ypdomainname 
variable in ypclnt.c appears to be set by yp_get_default_domain only, 
and that eventually gets the data from uname.

Thanks,
Florian

Re: Question on /etc/nsswitch.conf

[Andreas Schwab]

On Okt 02 2017, Florian Weimer <fweimer@redhat.com> wrote:

> That's not how I read the implementation.  Could you elaborate?  How can
> the NIS domain and the kernel domain differ?

Sorry, my misunderstanding.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

[PATCH] Move nss_compat from nis to nss subdir and install it unconditionally.

[Andreas Schwab]

This has been tested that local lookup still works with and
without an installed libnss_nis, and that NIS lookup works when
libnss_nis is available.

	* nis/Makefile (services): Remove compat.
	(libnss_compat-routines, libnss_compat-inhibit-o): Don't define.
	($(objpfx)libnss_compat.so): Remove rule.
	* nis/Versions (libnss_compat): Remove.
	* nss/Makefile (services): Add compat.
	(libnss_compat-routines, libnss_compat-inhibit-o): Define.
	* nss/Versions (libnss_compat): Define.
	* nss/nss_compat/compat-grp.c: Moved here from nis/nss_compat.
	Don't include <rpc/types.h>.  Replace bool_t by bool.
	* nss/nss_compat/compat-initgroups.c: Likewise.
	* nss/nss_compat/compat-pwd.c: Likewise.  Include "nisdomain.h"
	instead of <rpcsrv/ypclnt.h>.
	(getpwent_next_nss_netgr): Use __nss_get_default_domain instead of
	yp_get_default_domain.
	* nss/nss_compat/compat-pwd.c: Likewise.
	(getspent_next_nss_netgr): Use __nss_get_default_domain instead of
	yp_get_default_domain.
	* nss/nss_compat/nisdomain.c: New file.
	* nss/nss_compat/nisdomain.h: Likewise.
---
 nis/Makefile                                |  6 +--
 nis/Versions                                | 11 ------
 nss/Makefile                                |  6 ++-
 nss/Versions                                | 11 ++++++
 {nis => nss}/nss_compat/compat-grp.c        | 21 +++++------
 {nis => nss}/nss_compat/compat-initgroups.c |  9 ++---
 {nis => nss}/nss_compat/compat-pwd.c        | 13 +++----
 {nis => nss}/nss_compat/compat-spwd.c       | 11 +++---
 nss/nss_compat/nisdomain.c                  | 58 +++++++++++++++++++++++++++++
 nss/nss_compat/nisdomain.h                  | 20 ++++++++++
 10 files changed, 120 insertions(+), 46 deletions(-)
 rename {nis => nss}/nss_compat/compat-grp.c (97%)
 rename {nis => nss}/nss_compat/compat-initgroups.c (98%)
 rename {nis => nss}/nss_compat/compat-pwd.c (99%)
 rename {nis => nss}/nss_compat/compat-spwd.c (98%)
 create mode 100644 nss/nss_compat/nisdomain.c
 create mode 100644 nss/nss_compat/nisdomain.h

diff --git a/nis/Makefile b/nis/Makefile
index 6b6f5ee72c..023916f446 100644
--- a/nis/Makefile
+++ b/nis/Makefile
@@ -33,7 +33,7 @@ databases		= proto service hosts network grp pwd rpc ethers \
 			  spwd netgrp alias publickey
 
 # Specify rules for the nss_* modules.
-services		:= nis nisplus compat
+services		:= nis nisplus
 endif
 
 extra-libs		= libnsl
@@ -63,9 +63,6 @@ libnsl-routines = yp_xdr ypclnt ypupdate_xdr \
 		  nis_clone_res nss-default
 
 ifeq ($(build-obsolete-nsl),yes)
-libnss_compat-routines	:= $(addprefix compat-,grp pwd spwd initgroups)
-libnss_compat-inhibit-o	= $(filter-out .os,$(object-suffixes))
-
 libnss_nis-routines	:= $(addprefix nis-,$(databases)) nis-initgroups \
 			   nss-nis
 libnss_nis-inhibit-o	= $(filter-out .os,$(object-suffixes))
@@ -79,7 +76,6 @@ include ../Rules
 
 
 ifeq ($(build-obsolete-nsl),yes)
-$(objpfx)libnss_compat.so: $(objpfx)libnsl.so$(libnsl.so-version)
 $(objpfx)libnss_nis.so: $(objpfx)libnsl.so$(libnsl.so-version) \
 			$(common-objpfx)nss/libnss_files.so
 $(objpfx)libnss_nisplus.so: $(objpfx)libnsl.so$(libnsl.so-version)
diff --git a/nis/Versions b/nis/Versions
index ef9a512417..90d3d9dfaa 100644
--- a/nis/Versions
+++ b/nis/Versions
@@ -63,17 +63,6 @@ libnsl {
   }
 }
 
-libnss_compat {
-  GLIBC_PRIVATE {
-    _nss_compat_endgrent; _nss_compat_endpwent; _nss_compat_endspent;
-    _nss_compat_getgrent_r; _nss_compat_getgrgid_r; _nss_compat_getgrnam_r;
-    _nss_compat_getpwent_r; _nss_compat_getpwnam_r; _nss_compat_getpwuid_r;
-    _nss_compat_getspent_r; _nss_compat_getspnam_r;
-    _nss_compat_setgrent; _nss_compat_setpwent; _nss_compat_setspent;
-    _nss_compat_initgroups_dyn;
-  }
-}
-
 libnss_nis {
   GLIBC_PRIVATE {
     _nss_nis_endaliasent; _nss_nis_endetherent; _nss_nis_endgrent;
diff --git a/nss/Makefile b/nss/Makefile
index c9a5200f96..f27bed11fc 100644
--- a/nss/Makefile
+++ b/nss/Makefile
@@ -72,7 +72,7 @@ tests += tst-cancel-getpwuid_r
 endif
 
 # Specify rules for the nss_* modules.  We have some services.
-services		:= files db
+services		:= files db compat
 
 extra-libs		= $(services:%=libnss_%)
 # These libraries will be built in the `others' pass rather than
@@ -95,11 +95,15 @@ libnss_db-routines	:= $(libnss_db-dbs) db-open db-init hash-string
 generated		+= $(filter-out db-alias.c db-netgrp.c, \
 					$(addsuffix .c,$(libnss_db-dbs)))
 
+libnss_compat-routines	:= $(addprefix compat-,grp pwd spwd initgroups) \
+			   nisdomain
+
 install-others		+= $(inst_vardbdir)/Makefile
 
 # Build static module into libc if requested
 libnss_files-inhibit-o	= $(filter-out .os,$(object-suffixes))
 libnss_db-inhibit-o	= $(filter-out .os,$(object-suffixes))
+libnss_compat-inhibit-o	= $(filter-out .os,$(object-suffixes))
 ifeq ($(build-static-nss),yes)
 routines                += $(libnss_files-routines)
 static-only-routines    += $(libnss_files-routines)
diff --git a/nss/Versions b/nss/Versions
index 50268ed9b5..7694998f1d 100644
--- a/nss/Versions
+++ b/nss/Versions
@@ -162,3 +162,14 @@ libnss_db {
     _nss_db_init;
   }
 }
+
+libnss_compat {
+  GLIBC_PRIVATE {
+    _nss_compat_endgrent; _nss_compat_endpwent; _nss_compat_endspent;
+    _nss_compat_getgrent_r; _nss_compat_getgrgid_r; _nss_compat_getgrnam_r;
+    _nss_compat_getpwent_r; _nss_compat_getpwnam_r; _nss_compat_getpwuid_r;
+    _nss_compat_getspent_r; _nss_compat_getspnam_r;
+    _nss_compat_setgrent; _nss_compat_setpwent; _nss_compat_setspent;
+    _nss_compat_initgroups_dyn;
+  }
+}
diff --git a/nis/nss_compat/compat-grp.c b/nss/nss_compat/compat-grp.c
similarity index 97%
rename from nis/nss_compat/compat-grp.c
rename to nss/nss_compat/compat-grp.c
index 0381458c0c..4d51fc95ed 100644
--- a/nis/nss_compat/compat-grp.c
+++ b/nss/nss_compat/compat-grp.c
@@ -24,7 +24,6 @@
 #include <nsswitch.h>
 #include <stdio_ext.h>
 #include <string.h>
-#include <rpc/types.h>
 #include <libc-lock.h>
 #include <kernel-features.h>
 
@@ -58,21 +57,21 @@ struct blacklist_t
 
 struct ent_t
 {
-  bool_t files;
+  bool files;
   enum nss_status setent_status;
   FILE *stream;
   struct blacklist_t blacklist;
 };
 typedef struct ent_t ent_t;
 
-static ent_t ext_ent = { TRUE, NSS_STATUS_SUCCESS, NULL, { NULL, 0, 0 }};
+static ent_t ext_ent = { true, NSS_STATUS_SUCCESS, NULL, { NULL, 0, 0 }};
 
 /* Protect global state against multiple changers.  */
 __libc_lock_define_initialized (static, lock)
 
 /* Prototypes for local functions.  */
 static void blacklist_store_name (const char *, ent_t *);
-static int in_blacklist (const char *, int, ent_t *);
+static bool in_blacklist (const char *, int, ent_t *);
 
 /* Initialize the NSS interface/functions. The calling function must
    hold the lock.  */
@@ -94,7 +93,7 @@ internal_setgrent (ent_t *ent, int stayopen, int needent)
 {
   enum nss_status status = NSS_STATUS_SUCCESS;
 
-  ent->files = TRUE;
+  ent->files = true;
 
   if (ent->blacklist.data != NULL)
     {
@@ -321,7 +320,7 @@ getgrent_next_file (struct group *result, ent_t *ent,
       /* +:... */
       if (result->gr_name[0] == '+' && result->gr_name[1] == '\0')
 	{
-	  ent->files = FALSE;
+	  ent->files = false;
 
 	  return getgrent_next_nss (result, ent, buffer, buflen, errnop);
 	}
@@ -466,7 +465,7 @@ enum nss_status
 _nss_compat_getgrnam_r (const char *name, struct group *grp,
 			char *buffer, size_t buflen, int *errnop)
 {
-  ent_t ent = { TRUE, NSS_STATUS_SUCCESS, NULL, { NULL, 0, 0 }};
+  ent_t ent = { true, NSS_STATUS_SUCCESS, NULL, { NULL, 0, 0 }};
   enum nss_status result;
 
   if (name[0] == '-' || name[0] == '+')
@@ -598,7 +597,7 @@ enum nss_status
 _nss_compat_getgrgid_r (gid_t gid, struct group *grp,
 			char *buffer, size_t buflen, int *errnop)
 {
-  ent_t ent = { TRUE, NSS_STATUS_SUCCESS, NULL, { NULL, 0, 0 }};
+  ent_t ent = { true, NSS_STATUS_SUCCESS, NULL, { NULL, 0, 0 }};
   enum nss_status result;
 
   __libc_lock_lock (lock);
@@ -665,15 +664,15 @@ blacklist_store_name (const char *name, ent_t *ent)
   return;
 }
 
-/* returns TRUE if ent->blacklist contains name, else FALSE */
-static bool_t
+/* Return whether ent->blacklist contains name.  */
+static bool
 in_blacklist (const char *name, int namelen, ent_t *ent)
 {
   char buf[namelen + 3];
   char *cp;
 
   if (ent->blacklist.data == NULL)
-    return FALSE;
+    return false;
 
   buf[0] = '|';
   cp = stpcpy (&buf[1], name);
diff --git a/nis/nss_compat/compat-initgroups.c b/nss/nss_compat/compat-initgroups.c
similarity index 98%
rename from nis/nss_compat/compat-initgroups.c
rename to nss/nss_compat/compat-initgroups.c
index 795213448c..c1a9301a3b 100644
--- a/nis/nss_compat/compat-initgroups.c
+++ b/nss/nss_compat/compat-initgroups.c
@@ -24,7 +24,6 @@
 #include <stdio_ext.h>
 #include <string.h>
 #include <unistd.h>
-#include <rpc/types.h>
 #include <sys/param.h>
 #include <nsswitch.h>
 #include <libc-lock.h>
@@ -79,7 +78,7 @@ typedef struct ent_t ent_t;
 
 /* Prototypes for local functions.  */
 static void blacklist_store_name (const char *, ent_t *);
-static int in_blacklist (const char *, int, ent_t *);
+static bool in_blacklist (const char *, int, ent_t *);
 
 /* Initialize the NSS interface/functions. The calling function must
    hold the lock.  */
@@ -558,15 +557,15 @@ blacklist_store_name (const char *name, ent_t *ent)
   return;
 }
 
-/* returns TRUE if ent->blacklist contains name, else FALSE */
-static bool_t
+/* Return whether ent->blacklist contains name.  */
+static bool
 in_blacklist (const char *name, int namelen, ent_t *ent)
 {
   char buf[namelen + 3];
   char *cp;
 
   if (ent->blacklist.data == NULL)
-    return FALSE;
+    return false;
 
   buf[0] = '|';
   cp = stpcpy (&buf[1], name);
diff --git a/nis/nss_compat/compat-pwd.c b/nss/nss_compat/compat-pwd.c
similarity index 99%
rename from nis/nss_compat/compat-pwd.c
rename to nss/nss_compat/compat-pwd.c
index 0583a10b84..b16eef5d44 100644
--- a/nis/nss_compat/compat-pwd.c
+++ b/nss/nss_compat/compat-pwd.c
@@ -25,12 +25,11 @@
 #include <pwd.h>
 #include <stdio_ext.h>
 #include <string.h>
-#include <rpc/types.h>
-#include <rpcsvc/ypclnt.h>
 #include <libc-lock.h>
 #include <kernel-features.h>
 
 #include "netgroup.h"
+#include "nisdomain.h"
 
 static service_user *ni;
 static enum nss_status (*nss_setpwent) (int stayopen);
@@ -82,7 +81,7 @@ __libc_lock_define_initialized (static, lock)
 
 /* Prototypes for local functions.  */
 static void blacklist_store_name (const char *, ent_t *);
-static int in_blacklist (const char *, int, ent_t *);
+static bool in_blacklist (const char *, int, ent_t *);
 
 /* Initialize the NSS interface/functions. The calling function must
    hold the lock.  */
@@ -346,7 +345,7 @@ getpwent_next_nss_netgr (const char *name, struct passwd *result, ent_t *ent,
       if (domain != NULL)
 	{
 	  if (curdomain == NULL
-	      && yp_get_default_domain (&curdomain) != YPERR_SUCCESS)
+	      && __nss_get_default_domain (&curdomain) != 0)
 	    {
 	      __internal_endnetgrent (&ent->netgrdata);
 	      ent->netgroup = false;
@@ -1114,15 +1113,15 @@ blacklist_store_name (const char *name, ent_t *ent)
   return;
 }
 
-/* Returns TRUE if ent->blacklist contains name, else FALSE.  */
-static bool_t
+/* Returns whether ent->blacklist contains name.  */
+static bool
 in_blacklist (const char *name, int namelen, ent_t *ent)
 {
   char buf[namelen + 3];
   char *cp;
 
   if (ent->blacklist.data == NULL)
-    return FALSE;
+    return false;
 
   buf[0] = '|';
   cp = stpcpy (&buf[1], name);
diff --git a/nis/nss_compat/compat-spwd.c b/nss/nss_compat/compat-spwd.c
similarity index 98%
rename from nis/nss_compat/compat-spwd.c
rename to nss/nss_compat/compat-spwd.c
index eec3af3d15..8a875472fa 100644
--- a/nis/nss_compat/compat-spwd.c
+++ b/nss/nss_compat/compat-spwd.c
@@ -25,12 +25,11 @@
 #include <shadow.h>
 #include <stdio_ext.h>
 #include <string.h>
-#include <rpc/types.h>
-#include <rpcsvc/ypclnt.h>
 #include <libc-lock.h>
 #include <kernel-features.h>
 
 #include "netgroup.h"
+#include "nisdomain.h"
 
 static service_user *ni;
 static enum nss_status (*nss_setspent) (int stayopen);
@@ -79,7 +78,7 @@ __libc_lock_define_initialized (static, lock)
 
 /* Prototypes for local functions.  */
 static void blacklist_store_name (const char *, ent_t *);
-static int in_blacklist (const char *, int, ent_t *);
+static bool in_blacklist (const char *, int, ent_t *);
 
 /* Initialize the NSS interface/functions. The calling function must
    hold the lock.  */
@@ -306,7 +305,7 @@ getspent_next_nss_netgr (const char *name, struct spwd *result, ent_t *ent,
       if (domain != NULL)
 	{
 	  if (curdomain == NULL
-	      && yp_get_default_domain (&curdomain) != YPERR_SUCCESS)
+	      && __nss_get_default_domain (&curdomain) != 0)
 	    {
 	      __internal_endnetgrent (&ent->netgrdata);
 	      ent->netgroup = false;
@@ -840,8 +839,8 @@ blacklist_store_name (const char *name, ent_t *ent)
 }
 
 
-/* Returns TRUE if ent->blacklist contains name, else FALSE.  */
-static bool_t
+/* Returns whether ent->blacklist contains name.  */
+static bool
 in_blacklist (const char *name, int namelen, ent_t *ent)
 {
   char buf[namelen + 3];
diff --git a/nss/nss_compat/nisdomain.c b/nss/nss_compat/nisdomain.c
new file mode 100644
index 0000000000..220ae27234
--- /dev/null
+++ b/nss/nss_compat/nisdomain.c
@@ -0,0 +1,58 @@
+/* Copyright (C) 2017 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+#include <libc-lock.h>
+#include "nisdomain.h"
+
+#define MAXDOMAINNAMELEN 1024
+
+static char domainname[MAXDOMAINNAMELEN];
+
+__libc_lock_define_initialized (static, domainname_lock)
+
+int
+__nss_get_default_domain (char **outdomain)
+{
+  int result = 0;
+  *outdomain = NULL;
+
+  __libc_lock_lock (domainname_lock);
+
+  if (domainname[0] != '\0')
+    {
+      if (getdomainname (domainname, MAXDOMAINNAMELEN) < 0)
+	result = errno;
+      else if (strcmp (domainname, "(none)") == 0)
+	{
+	  /* If domainname is not set, some systems will return "(none)" */
+	  domainname[0] = '\0';
+	  result = ENOENT;
+	}
+      else
+	*outdomain = domainname;
+    }
+  else
+    *outdomain = domainname;
+
+  __libc_lock_unlock (domainname_lock);
+
+  return result;
+}
diff --git a/nss/nss_compat/nisdomain.h b/nss/nss_compat/nisdomain.h
new file mode 100644
index 0000000000..314f3f7c06
--- /dev/null
+++ b/nss/nss_compat/nisdomain.h
@@ -0,0 +1,20 @@
+/* Copyright (C) 2017 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
+   <http://www.gnu.org/licenses/>.  */
+
+/* Set OUTDOMAIN to a pointer to the current NIS domain name, or NULL if
+   not set.  Return zero on success, an error number on failure.  */
+extern int __nss_get_default_domain (char **outdomain);
-- 
2.14.2

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] Move nss_compat from nis to nss subdir and install it unconditionally.

[Florian Weimer]

On 10/04/2017 11:51 AM, Andreas Schwab wrote:
> This has been tested that local lookup still works with and
> without an installed libnss_nis, and that NIS lookup works when
> libnss_nis is available.
> 
> 	* nis/Makefile (services): Remove compat.
> 	(libnss_compat-routines, libnss_compat-inhibit-o): Don't define.
> 	($(objpfx)libnss_compat.so): Remove rule.
> 	* nis/Versions (libnss_compat): Remove.
> 	* nss/Makefile (services): Add compat.
> 	(libnss_compat-routines, libnss_compat-inhibit-o): Define.
> 	* nss/Versions (libnss_compat): Define.
> 	* nss/nss_compat/compat-grp.c: Moved here from nis/nss_compat.
> 	Don't include <rpc/types.h>.  Replace bool_t by bool.
> 	* nss/nss_compat/compat-initgroups.c: Likewise.
> 	* nss/nss_compat/compat-pwd.c: Likewise.  Include "nisdomain.h"
> 	instead of <rpcsrv/ypclnt.h>.
> 	(getpwent_next_nss_netgr): Use __nss_get_default_domain instead of
> 	yp_get_default_domain.
> 	* nss/nss_compat/compat-pwd.c: Likewise.
> 	(getspent_next_nss_netgr): Use __nss_get_default_domain instead of
> 	yp_get_default_domain.
> 	* nss/nss_compat/nisdomain.c: New file.
> 	* nss/nss_compat/nisdomain.h: Likewise.

This looks good to me.  Thanks.

Florian

Re: [PATCH] Move nss_compat from nis to nss subdir and install it unconditionally.

[Zack Weinberg]

On Wed, Oct 4, 2017 at 6:31 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 10/04/2017 11:51 AM, Andreas Schwab wrote:
>>
>> This has been tested that local lookup still works with and
>> without an installed libnss_nis, and that NIS lookup works when
>> libnss_nis is available.
>>
>>         * nis/Makefile (services): Remove compat.
>>         (libnss_compat-routines, libnss_compat-inhibit-o): Don't define.
>>         ($(objpfx)libnss_compat.so): Remove rule.
>>         * nis/Versions (libnss_compat): Remove.
>>         * nss/Makefile (services): Add compat.
>>         (libnss_compat-routines, libnss_compat-inhibit-o): Define.
>>         * nss/Versions (libnss_compat): Define.
>>         * nss/nss_compat/compat-grp.c: Moved here from nis/nss_compat.
>>         Don't include <rpc/types.h>.  Replace bool_t by bool.
>>         * nss/nss_compat/compat-initgroups.c: Likewise.
>>         * nss/nss_compat/compat-pwd.c: Likewise.  Include "nisdomain.h"
>>         instead of <rpcsrv/ypclnt.h>.
>>         (getpwent_next_nss_netgr): Use __nss_get_default_domain instead of
>>         yp_get_default_domain.
>>         * nss/nss_compat/compat-pwd.c: Likewise.
>>         (getspent_next_nss_netgr): Use __nss_get_default_domain instead of
>>         yp_get_default_domain.
>>         * nss/nss_compat/nisdomain.c: New file.
>>         * nss/nss_compat/nisdomain.h: Likewise.
>
>
> This looks good to me.  Thanks.

Looks good to me, too.

zw

Re: [PATCH] Move nss_compat from nis to nss subdir and install it unconditionally.

[Zack Weinberg]

On Wed, Oct 4, 2017 at 5:51 AM, Andreas Schwab <schwab@suse.de> wrote:
> This has been tested that local lookup still works with and
> without an installed libnss_nis, and that NIS lookup works when
> libnss_nis is available.

It now occurs to me that another test might be a good idea: configure
a system with at least one local non-root user and also at least one
+user or +@netgroup line in /etc/passwd, and "passwd: compat" in
/etc/nsswitch.conf, but *don't* install libnss_nis or any NIS
configuration; then make sure that all of the local users can log in
even though libnss_nis is unavailable (that is, _compat only tries to
use _nis if it doesn't find a local match for the query), that the
users included from NIS *cannot* log in (fail closed), and that the
failure to log in produces an error message that will point the
sysadmin at least vaguely in the right direction.

zw

Re: [PATCH] Move nss_compat from nis to nss subdir and install it unconditionally.

[Andreas Schwab]

On Okt 04 2017, Zack Weinberg <zackw@panix.com> wrote:

> It now occurs to me that another test might be a good idea: configure
> a system with at least one local non-root user and also at least one
> +user or +@netgroup line in /etc/passwd, and "passwd: compat" in
> /etc/nsswitch.conf, but *don't* install libnss_nis or any NIS
> configuration; then make sure that all of the local users can log in
> even though libnss_nis is unavailable (that is, _compat only tries to
> use _nis if it doesn't find a local match for the query),

Check.

> that the users included from NIS *cannot* log in (fail closed),

Check.

> and that the failure to log in produces an error message that will
> point the sysadmin at least vaguely in the right direction.

This requires more work because __nss_lookup_function doesn't
distinguish a missing library from a library that does not implement the
function.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] Move nss_compat from nis to nss subdir and install it unconditionally.

[Zack Weinberg]

On Wed, Oct 4, 2017 at 12:01 PM, Andreas Schwab <schwab@suse.de> wrote:
> On Okt 04 2017, Zack Weinberg <zackw@panix.com> wrote:
>> and that the failure to log in produces an error message that will
>> point the sysadmin at least vaguely in the right direction.
>
> This requires more work because __nss_lookup_function doesn't
> distinguish a missing library from a library that does not implement the
> function.

Hmm.  I hate bad error messages, but I'm really not sure how much time
is worth spending on this.  Certainly it shouldn't block this patch.

zw

Re: [PATCH] Move nss_compat from nis to nss subdir and install it unconditionally.

[Florian Weimer]

On 10/04/2017 06:01 PM, Andreas Schwab wrote:

>> and that the failure to log in produces an error message that will
>> point the sysadmin at least vaguely in the right direction.
> 
> This requires more work because __nss_lookup_function doesn't
> distinguish a missing library from a library that does not implement the
> function.

It also does not distinguish a missing library from a transient failure 
during library loading (bug 22041).  This is probably the more 
significant issue here.

I don't expect this to be easy to fix, unfortunately.

Thanks,
Florian

Re: [PATCH] Move nss_compat from nis to nss subdir and install it unconditionally.

[Carlos O'Donell]

On 10/04/2017 07:35 AM, Zack Weinberg wrote:
> On Wed, Oct 4, 2017 at 5:51 AM, Andreas Schwab <schwab@suse.de> wrote:
>> This has been tested that local lookup still works with and
>> without an installed libnss_nis, and that NIS lookup works when
>> libnss_nis is available.
> 
> It now occurs to me that another test might be a good idea: configure
> a system with at least one local non-root user and also at least one
> +user or +@netgroup line in /etc/passwd, and "passwd: compat" in
> /etc/nsswitch.conf, but *don't* install libnss_nis or any NIS
> configuration; then make sure that all of the local users can log in
> even though libnss_nis is unavailable (that is, _compat only tries to
> use _nis if it doesn't find a local match for the query), that the
> users included from NIS *cannot* log in (fail closed), and that the
> failure to log in produces an error message that will point the
> sysadmin at least vaguely in the right direction.

I would also like to see more testing of this kind, and in fact
DJ is looking into this kind of more complicated testing within
upstream glibc's test framework and expanding it as required.

Right now we just don't have the framework pieces in place though
which is why we all manually test NSS stuff.

-- 
Cheers,
Carlos.