RFC: Test hook for nss_files testing
Commit Message
The attached patch adds a test hook which allows us to select different
files (not those under /etc) for testing nss_files inside the build
tree. It contains a regression test for bug 17363 as an example.
If the direction is acceptable, I'll add further tests.
This change increases the size of nss_files slightly (on x86_64):
Old:
text data bss dec hex filename
40895 992 26040 67927 10957 nss/libnss_files.so.2
New:
text data bss dec hex filename
41426 1088 26040 68554 10bca nss/libnss_files.so.2
If this is a problem, I can make additional cleanups which reduce the
DSO size to the previous level.
There are also a few additional relocations (for the string constants),
but they should not matter.
Florian
Comments
On 12/06/2015 03:22 PM, Florian Weimer wrote:
> The attached patch adds a test hook which allows us to select different
> files (not those under /etc) for testing nss_files inside the build
> tree. It contains a regression test for bug 17363 as an example.
>
> If the direction is acceptable, I'll add further tests.
[snip]
> There are also a few additional relocations (for the string constants),
> but they should not matter.
I have two concerns.
(1) Security.
What security implications are there in exposing this interface?
(2) Test what we ship.
We need to get away from build-tree testing and move to installed tree
testing to verify that we are testing is what we are shipping.
The testing would look like this:
- Setup an installed tree.
- Setup the test.
- Run the test in some kind of isolation with configuration changes
made to the sysroot that would otherwise be impossible on the host.
- Return status.
- Repeat for all tests that need a sysroot e.g. ldconfig, network, nss...
That is not to say that whitebox hook-based testing is bad, but it
deviates from (1) and (2) in ways which make me uncomfortable.
A more appealing alternative would be to run the test under a systemtap
script which did all the work of updating the paths to the databases
without the hook changes.
Thoughts?
Cheers,
Carlos.
On 12/07/2015 05:55 PM, Carlos O'Donell wrote:
> I have two concerns.
>
> (1) Security.
>
> What security implications are there in exposing this interface?
I added a function that has to be called instead of an environment
variable precisely so that there are no security concerns. The function
is prefixed with _nss_files, which is what we use for namespacing the
NSS service modules.
> (2) Test what we ship.
>
> We need to get away from build-tree testing and move to installed tree
> testing to verify that we are testing is what we are shipping.
My proposed tests do that, which is why there is a hook. An alternative
would be to compile nss_files twice, with different settings. But then
we aren't testing anymore what we are shipping.
> The testing would look like this:
>
> - Setup an installed tree.
> - Setup the test.
> - Run the test in some kind of isolation with configuration changes
> made to the sysroot that would otherwise be impossible on the host.
> - Return status.
> - Repeat for all tests that need a sysroot e.g. ldconfig, network, nss...
I agree that installed-tree testing is better. At least the nss_files
tests should be straightforward to migrate when installed-tree testing
arrives. You just omit the path override, and copy the test files to
/etc in the test environment.
> A more appealing alternative would be to run the test under a systemtap
> script which did all the work of updating the paths to the databases
> without the hook changes.
I think this would be far more brittle and difficult to implement
because the existing path names are just string literals. Run-time
patching also means that it's not really what we ship. At that point,
we may be better off with something like cwrap, or an xtest with chroot.
Florian
On 12/07/2015 01:34 PM, Florian Weimer wrote:
> On 12/07/2015 05:55 PM, Carlos O'Donell wrote:
>
>> I have two concerns.
>>
>> (1) Security.
>>
>> What security implications are there in exposing this interface?
>
> I added a function that has to be called instead of an environment
> variable precisely so that there are no security concerns. The function
> is prefixed with _nss_files, which is what we use for namespacing the
> NSS service modules.
I agree that an environment variable would not have been acceptable.
My question is: Have we thought through any security implications?
We went from having a number of RO-data strings scattered about to
having a single structure that allows access to all of them in RW-data?
It's the same concerns I have at about tunables actually, but in this
context it's a risk reward of "better testing" vs. "new way to change
/etc/passwd lookup".
Then again, you'd need to have enough control of the target process
to make a call, so you might as well do other things?
What I'm asking is: Have you given thought to the security implications?
Your answer is: Yes. A function is the best we can do.
Did I understand that right?
Can we harden this global table in any way?
>> (2) Test what we ship.
>>
>> We need to get away from build-tree testing and move to installed tree
>> testing to verify that we are testing is what we are shipping.
>
> My proposed tests do that, which is why there is a hook. An alternative
> would be to compile nss_files twice, with different settings. But then
> we aren't testing anymore what we are shipping.
In retrospect I see what you mean here. In that the interface for initialization
is always present as a private API and never removed. I like that aspect of the
testing. What I don't like is that the test program doesn't behave like a normal
application that would be using the APIs in question. It calls a special hook
and that's really the problem I have with whitebox testing.
I'm not against the new function, I'm against the test program having to use it
in a special way that makes it not behave like a regular application.
>> The testing would look like this:
>>
>> - Setup an installed tree.
>> - Setup the test.
>> - Run the test in some kind of isolation with configuration changes
>> made to the sysroot that would otherwise be impossible on the host.
>> - Return status.
>> - Repeat for all tests that need a sysroot e.g. ldconfig, network, nss...
>
> I agree that installed-tree testing is better. At least the nss_files
> tests should be straightforward to migrate when installed-tree testing
> arrives. You just omit the path override, and copy the test files to
> /etc in the test environment.
Agreed.
>> A more appealing alternative would be to run the test under a systemtap
>> script which did all the work of updating the paths to the databases
>> without the hook changes.
>
> I think this would be far more brittle and difficult to implement
> because the existing path names are just string literals. Run-time
> patching also means that it's not really what we ship. At that point,
> we may be better off with something like cwrap, or an xtest with chroot.
I like both of those options better.
If we are going to add hooks for testing like you propose it's going
to be because we absolutely need the hooks and there is no other way
to get at the information in question. It must be a last resort that
we add hooks like this since they incur maintenance burden forever.
I think xtest+chroot is really the ideal solution here and would help
further containerized testing.
To be honest I like 99% of what you did, in fact coalescing the db names
into a single struct prepares us for a set of tunables that might allow
you to run applications under alternate databases by tweaking those
options.
Cheers,
Carlos.
On 12/07/2015 09:50 PM, Carlos O'Donell wrote:
> What I'm asking is: Have you given thought to the security implications?
>
> Your answer is: Yes. A function is the best we can do.
>
> Did I understand that right?
>
> Can we harden this global table in any way?
To be honest, I'm confused by this request. We do not usually take such
issues into account when making changes to the library.
I think a function is indeed the best way, yes.
> I'm not against the new function, I'm against the test program having to use it
> in a special way that makes it not behave like a regular application.
I see, and to some extend, I share that concern.
>>> A more appealing alternative would be to run the test under a systemtap
>>> script which did all the work of updating the paths to the databases
>>> without the hook changes.
>>
>> I think this would be far more brittle and difficult to implement
>> because the existing path names are just string literals. Run-time
>> patching also means that it's not really what we ship. At that point,
>> we may be better off with something like cwrap, or an xtest with chroot.
>
> I like both of those options better.
>
> If we are going to add hooks for testing like you propose it's going
> to be because we absolutely need the hooks and there is no other way
> to get at the information in question. It must be a last resort that
> we add hooks like this since they incur maintenance burden forever.
>
> I think xtest+chroot is really the ideal solution here and would help
> further containerized testing.
I'm not so sure. Setting up the chroot in a realistic way is quite
difficult. You need to populat /dev and /dev/pts in some way, and
arrange for the necessary GCC and platform bits being present, whatever
they are. “make install” may not actually produce a tree layout that is
used by any downstream distribution. And so on. The list of issues is
quite long.
I think we could package such tests that they rewrite an existing chroot
(or live VM image) and then do the testing they need. But these tests
still would not run as part of the build process.
I'll keep my patch around for local nss_files testing, but it seems that
we don't want it yet in glibc proper.
Florian
On 12/10/2015 09:42 AM, Florian Weimer wrote:
> On 12/07/2015 09:50 PM, Carlos O'Donell wrote:
>
>> What I'm asking is: Have you given thought to the security implications?
>>
>> Your answer is: Yes. A function is the best we can do.
>>
>> Did I understand that right?
>>
>> Can we harden this global table in any way?
>
> To be honest, I'm confused by this request. We do not usually take such
> issues into account when making changes to the library.
>
> I think a function is indeed the best way, yes.
Perfect. No further questions.
>> I'm not against the new function, I'm against the test program having to use it
>> in a special way that makes it not behave like a regular application.
>
> I see, and to some extend, I share that concern.
Good.
>>>> A more appealing alternative would be to run the test under a systemtap
>>>> script which did all the work of updating the paths to the databases
>>>> without the hook changes.
>>>
>>> I think this would be far more brittle and difficult to implement
>>> because the existing path names are just string literals. Run-time
>>> patching also means that it's not really what we ship. At that point,
>>> we may be better off with something like cwrap, or an xtest with chroot.
>>
>> I like both of those options better.
>>
>> If we are going to add hooks for testing like you propose it's going
>> to be because we absolutely need the hooks and there is no other way
>> to get at the information in question. It must be a last resort that
>> we add hooks like this since they incur maintenance burden forever.
>>
>> I think xtest+chroot is really the ideal solution here and would help
>> further containerized testing.
>
> I'm not so sure. Setting up the chroot in a realistic way is quite
> difficult. You need to populat /dev and /dev/pts in some way, and
> arrange for the necessary GCC and platform bits being present, whatever
> they are. “make install” may not actually produce a tree layout that is
> used by any downstream distribution. And so on. The list of issues is
> quite long.
In which regard do you think this is not ideal?
Should we rely on distro images?
Let the user choose which images to use for their target?
At the end of the day glibc has to test it's own functionality on what it
considers to be a normalized system root. When the distributions file bugs
we can compare their configuration to what we used in testing.
I don't think it matters that our `make install` is never used by downstream,
but it has to represent a reality that we are willing to support as glibc
developers.
Relying on downstream to test our work has very high latency and can result
in a lot of lost productivity. We really need to know immediately if a
change broke something.
Lastly, we may have to rely on some basic building blocks for doing this kind
of testing. Without those building blocks we mark the test UNSUPPORTED.
It is entirely plausible that at the start the test only works on x86_64,
requires docker, and runs slim Fedora container for testing. Other targets
need to fill in the technology pieces that would allow them to launch a
target system that can be configured for testing and which can be thrown
away or broken if the test fails.
Note: As a target test it will require quite a bit of resources on the target
to run, including root, so it should continue to be xcheck.
> I think we could package such tests that they rewrite an existing chroot
> (or live VM image) and then do the testing they need. But these tests
> still would not run as part of the build process.
That is one solution and was discussed at GNU Cauldron, we would create
a "libc-tests" project that is an add-on to glibc (using the add-on mechanism)
which would allow you to add-on additional tests that are more complex.
> I'll keep my patch around for local nss_files testing, but it seems that
> we don't want it yet in glibc proper.
I just don't think it's the right solution.
Cheers,
Carlos.
On 12/11/2015 07:51 AM, Carlos O'Donell wrote:
>> I'm not so sure. Setting up the chroot in a realistic way is quite
>> difficult. You need to populat /dev and /dev/pts in some way, and
>> arrange for the necessary GCC and platform bits being present, whatever
>> they are. “make install” may not actually produce a tree layout that is
>> used by any downstream distribution. And so on. The list of issues is
>> quite long.
>
> In which regard do you think this is not ideal?
It's quite a bit of work and very tightly integrated with individual
distributions. Test hooks and in-tree testing directly benefit everyone
who looks at test results.
> Should we rely on distro images?
It's not just that, you'd also need to build glibc according to the
distribution's needs, regarding file system layout and so on. So it's
really difficult to see how this can be part of the upstream testing.
> Lastly, we may have to rely on some basic building blocks for doing this kind
> of testing. Without those building blocks we mark the test UNSUPPORTED.
> It is entirely plausible that at the start the test only works on x86_64,
> requires docker, and runs slim Fedora container for testing.
But that still looks like it needs network access, which builders
generally lack (and rightly so).
Florian
On 12/11/2015 07:15 AM, Florian Weimer wrote:
> On 12/11/2015 07:51 AM, Carlos O'Donell wrote:
>
>>> I'm not so sure. Setting up the chroot in a realistic way is quite
>>> difficult. You need to populat /dev and /dev/pts in some way, and
>>> arrange for the necessary GCC and platform bits being present, whatever
>>> they are. “make install” may not actually produce a tree layout that is
>>> used by any downstream distribution. And so on. The list of issues is
>>> quite long.
>>
>> In which regard do you think this is not ideal?
>
> It's quite a bit of work and very tightly integrated with individual
> distributions. Test hooks and in-tree testing directly benefit everyone
> who looks at test results.
You raise a good point here and that's perhaps the strongest argument
I've heard in favour of white-box testing for this particular interface.
Test hooks do result in immediate benefits, and I'm not against them for
testing intermediate state that can't otherwise be accessed via whole
system tests.
We need to move developers beyond the immediate `make check` and get
them doing broader testing upstream and earlier to accomodate the changes
they are working on.
>> Should we rely on distro images?
>
> It's not just that, you'd also need to build glibc according to the
> distribution's needs, regarding file system layout and so on. So it's
> really difficult to see how this can be part of the upstream testing.
Agreed. However, I don't see how that means it can't be part of the upstream
testing. There is no line in the sand that says upstream stops here and the
distribution starts here.
>> Lastly, we may have to rely on some basic building blocks for doing this kind
>> of testing. Without those building blocks we mark the test UNSUPPORTED.
>> It is entirely plausible that at the start the test only works on x86_64,
>> requires docker, and runs slim Fedora container for testing.
>
> But that still looks like it needs network access, which builders
> generally lack (and rightly so).
You always need network access to clone the repository. Therefore this
would be an extension of the checkout process that would require you to
run enough initialization to be able to use the results to do the testing.
Beyond the image download there won't be much else required.
Cheers,
Carlos.
On Fri, Dec 11, 2015 at 9:08 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 12/11/2015 07:15 AM, Florian Weimer wrote:
> > On 12/11/2015 07:51 AM, Carlos O'Donell wrote:
> >
> >>> I'm not so sure. Setting up the chroot in a realistic way is quite
> >>> difficult. You need to populat /dev and /dev/pts in some way, and
> >>> arrange for the necessary GCC and platform bits being present, whatever
> >>> they are. “make install” may not actually produce a tree layout that is
> >>> used by any downstream distribution. And so on. The list of issues is
> >>> quite long.
> >>
> >> In which regard do you think this is not ideal?
> >
> > It's quite a bit of work and very tightly integrated with individual
> > distributions. Test hooks and in-tree testing directly benefit everyone
> > who looks at test results.
>
> You raise a good point here and that's perhaps the strongest argument
> I've heard in favour of white-box testing for this particular interface.
>
One of the bits of religion I've gotten at Google is that one wants white-box
hooks for unit testing because the tests may be running in exotic and/or
highly-restricted contexts, and your chances of getting a code path executed
are better if you can write a custom test executable that drives the library
down that path.
Stan
2015-12-06 Florian Weimer <fweimer@redhat.com>
Implement test hook for nss_files database file selection.
* nss/Makefile (tests): Add tst-nss-files.
(libnss_files-routines): Add files-config.
(LDLIBS-tst-nss-files): New variable.
* nss/Versions (libnss_files): Export _nss_files_set_config.
* nss/nss_db/db-XXX.c (DBFILE_): New macro.
(DBFILE): Use it to stringify DATABASE.
* nss/nss_files.h: New file.
* nss/nss_files/files-XXX.c: Update comment: DATABASE is now an
identifier.
(DATAFILE): Adjust.
* nss/nss_files/files-config.c: New file.
* nss/nss_files/files-ethers.c (DATABASE): Adjust.
* nss/nss_files/files-group.c (DATABASE): Likewise.
* nss/nss_files/files-hosts.c (DATABASE): Likewise.
* nss/nss_files/files-key.c (DATABASE): Likewise.
* nss/nss_files/files-netgrp.c (DATABASE): Likewise.
* nss/nss_files/files-network.c (DATABASE): Likewise.
* nss/nss_files/files-parse.c: Update comment: DATABASE is now an
identifier.
* nss/nss_files/files-proto.c (DATABASE): Adjust.
* nss/nss_files/files-pwd.c (DATABASE): Likewise.
* nss/nss_files/files-rpc.c (DATABASE): Likewise.
* nss/nss_files/files-services.c (DATABASE): Likewise.
* nss/nss_files/files-sgrp.c (DATABASE): Likewise.
* nss/nss_files/files-spwd.c (DATABASE): Likewise.
* nss/test-data/files/netgroup: New file.
* nss/test-data/files/passwd: Likewise.
* nss/tst-nss-files.c: Likewise.
@@ -50,7 +50,7 @@ extra-objs += $(makedb-modules:=.o)
tests-static = tst-field
tests = test-netdb tst-nss-test1 test-digits-dots \
- tst-nss-getpwent bug17079 \
+ tst-nss-getpwent bug17079 tst-nss-files \
$(tests-static)
xtests = bug-erange
@@ -68,7 +68,8 @@ vpath %.c $(subdir-dirs) ../locale/programs ../intl
libnss_files-routines := $(addprefix files-,$(databases)) \
- files-initgroups files-have_o_cloexec files-init
+ files-initgroups files-have_o_cloexec files-init \
+ files-config
libnss_db-dbs := $(addprefix db-,\
$(filter-out hosts network key alias,\
@@ -124,3 +125,5 @@ $(objpfx)/libnss_test1.so$(libnss_test1.so-version): $(objpfx)/libnss_test1.so
$(make-link)
endif
$(objpfx)tst-nss-test1.out: $(objpfx)/libnss_test1.so$(libnss_test1.so-version)
+
+LDLIBS-tst-nss-files = -ldl
@@ -100,6 +100,7 @@ libnss_files {
_nss_files_initgroups_dyn;
_nss_files_init;
+ _nss_files_set_config;
}
}
@@ -39,7 +39,8 @@
#define ENTNAME_r CONCAT(ENTNAME,_r)
#include <paths.h>
-#define DBFILE _PATH_VARDB DATABASE ".db"
+#define DBFILE_(name) _PATH_VARDB #name ".db"
+#define DBFILE DBFILE_(name)
#ifdef NEED_H_ERRNO
# define H_ERRNO_PROTO , int *herrnop
new file mode 100644
@@ -0,0 +1,51 @@
+/* Internal interfaces of the nss_files service module.
+ Copyright (C) 2015 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/>. */
+
+#ifndef NSS_FILES_H
+#define NSS_FILES_H
+
+struct nss_files_config
+{
+ const char *path_ethers;
+ const char *path_group;
+ const char *path_gshadow;
+ const char *path_hosts;
+ const char *path_netgroup;
+ const char *path_networks;
+ const char *path_passwd;
+ const char *path_protocols;
+ const char *path_publickey;
+ const char *path_rpc;
+ const char *path_services;
+ const char *path_shadow;
+};
+
+/* Internal configuration setting. Only available within the DSO. */
+extern struct nss_files_config _nss_files_config attribute_hidden;
+
+/* Internal access to the file name based on the database name. */
+#define NSS_FILES_DATABASE_(name) _nss_files_config.path_##name
+#define NSS_FILES_DATABASE(name) NSS_FILES_DATABASE_ (name)
+
+/* Internal function to set _nss_files_config. Only copies the
+ struct, but not the value of the strings. Outside the DSO, this
+ function has to be looked up with dlsym. */
+void _nss_files_set_config (const struct nss_files_config *)
+ internal_function;
+
+#endif /* NSS_FILES_H */
@@ -22,6 +22,7 @@
#include <fcntl.h>
#include <libc-lock.h>
#include "nsswitch.h"
+#include <nss_files.h>
#include <kernel-features.h>
@@ -29,7 +30,7 @@
ENTNAME -- database name of the structure and functions (hostent, pwent).
STRUCTURE -- struct name, define only if not ENTNAME (passwd, group).
- DATABASE -- string of the database file's name ("hosts", "passwd").
+ DATABASE -- name of the database file's name ("hosts", "passwd").
NEED_H_ERRNO - defined iff an arg `int *herrnop' is used.
@@ -38,7 +39,7 @@
#define ENTNAME_r CONCAT(ENTNAME,_r)
-#define DATAFILE "/etc/" DATABASE
+#define DATAFILE NSS_FILES_DATABASE (DATABASE)
#ifdef NEED_H_ERRNO
# include <netdb.h>
new file mode 100644
@@ -0,0 +1,42 @@
+/* Files configuration for nss_files.
+ Copyright (C) 2015 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 <nss_files.h>
+
+struct nss_files_config _nss_files_config =
+ {
+ .path_ethers = "/etc/ethers",
+ .path_group = "/etc/group",
+ .path_gshadow = "/etc/gshadow",
+ .path_hosts = "/etc/hosts",
+ .path_netgroup = "/etc/netgroup",
+ .path_networks = "/etc/networks",
+ .path_passwd = "/etc/passwd",
+ .path_protocols = "/etc/protocols",
+ .path_publickey = "/etc/publickey",
+ .path_rpc = "/etc/rpc",
+ .path_services = "/etc/services",
+ .path_shadow = "/etc/shadow",
+ };
+
+void
+internal_function
+_nss_files_set_config (const struct nss_files_config *config)
+{
+ _nss_files_config = *config;
+}
@@ -22,7 +22,7 @@
struct etherent_data {};
#define ENTNAME etherent
-#define DATABASE "ethers"
+#define DATABASE ethers
#include "files-parse.c"
LINE_PARSER
("#",
@@ -20,7 +20,7 @@
#define STRUCTURE group
#define ENTNAME grent
-#define DATABASE "group"
+#define DATABASE group
struct grent_data {};
/* Our parser function is already defined in fgetgrent.c, so use that.
@@ -30,7 +30,7 @@
#define ENTNAME hostent
-#define DATABASE "hosts"
+#define DATABASE hosts
#define NEED_H_ERRNO
#define EXTRA_ARGS , af, flags
@@ -23,8 +23,9 @@
#include <rpc/key_prot.h>
#include <rpc/des_crypt.h>
#include "nsswitch.h"
+#include <nss_files.h>
-#define DATAFILE "/etc/publickey"
+#define DATAFILE _nss_files_config.path_publickey
static enum nss_status
@@ -26,8 +26,9 @@
#include <string.h>
#include "nsswitch.h"
#include "netgroup.h"
+#include <nss_files.h>
-#define DATAFILE "/etc/netgroup"
+#define DATAFILE _nss_files_config.path_netgroup
libnss_files_hidden_proto (_nss_files_endnetgrent)
@@ -22,7 +22,7 @@
#include <stdint.h>
#define ENTNAME netent
-#define DATABASE "networks"
+#define DATABASE networks
#define NEED_H_ERRNO
struct netent_data {};
@@ -26,7 +26,7 @@
ENTNAME -- database name of the structure and functions (hostent, pwent).
STRUCTURE -- struct name, define only if not ENTNAME (passwd, group).
- DATABASE -- string of the database file's name ("hosts", "passwd").
+ DATABASE -- name of the database (hosts, passwd).
ENTDATA -- if defined, `struct ENTDATA' is used by the parser to store
things pointed to by the resultant `struct STRUCTURE'.
@@ -20,7 +20,7 @@
#define ENTNAME protoent
-#define DATABASE "protocols"
+#define DATABASE protocols
struct protoent_data {};
@@ -20,7 +20,7 @@
#define STRUCTURE passwd
#define ENTNAME pwent
-#define DATABASE "passwd"
+#define DATABASE passwd
struct pwent_data {};
/* Our parser function is already defined in fgetpwent_r.c, so use that
@@ -20,7 +20,7 @@
#define ENTNAME rpcent
-#define DATABASE "rpc"
+#define DATABASE rpc
struct rpcent_data {};
@@ -21,7 +21,7 @@
#define ENTNAME servent
-#define DATABASE "services"
+#define DATABASE services
struct servent_data {};
@@ -20,7 +20,7 @@
#define STRUCTURE sgrp
#define ENTNAME sgent
-#define DATABASE "gshadow"
+#define DATABASE gshadow
struct sgent_data {};
/* Our parser function is already defined in sgetspent_r.c, so use that
@@ -20,7 +20,7 @@
#define STRUCTURE spwd
#define ENTNAME spent
-#define DATABASE "shadow"
+#define DATABASE shadow
struct spent_data {};
/* Our parser function is already defined in sgetspent_r.c, so use that
new file mode 100644
@@ -0,0 +1,6 @@
+netgr gr1 gr2 gr3
+gr1 (,u1,)
+gr2 (,u2,)
+gr3 gr5 gr6
+gr5
+gr6 (,u3,)
new file mode 100644
@@ -0,0 +1 @@
+root:x:0:0:glibc root test user:/root:/bin/bash
new file mode 100644
@@ -0,0 +1,211 @@
+/* Test nss_files parsing.
+ Copyright (C) 2015 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 <dlfcn.h>
+#include <errno.h>
+#include <gnu/lib-names.h>
+#include <netdb.h>
+#include <nss_files.h>
+#include <pwd.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+static void
+configure_nss (void)
+{
+ __nss_configure_lookup ("passwd", "files");
+}
+
+static char *
+file_name_for_database (const char *name)
+{
+ char *curdir = realpath (".", NULL);
+ if (curdir == NULL)
+ {
+ printf ("error: realpath (\".\", NULL): %m\n");
+ exit (1);
+ }
+
+ char *result;
+ int ret = asprintf (&result, "%s/test-data/files/%s", curdir, name);
+ if (ret < 0)
+ {
+ printf ("error: asprintf: %m\n");
+ exit (1);
+ }
+ return result;
+}
+
+static struct nss_files_config test_config;
+
+static void
+init_test_config (void)
+{
+ test_config = (struct nss_files_config)
+ {
+ .path_ethers = file_name_for_database ("ethers"),
+ .path_group = file_name_for_database ("group"),
+ .path_gshadow = file_name_for_database ("gshadow"),
+ .path_hosts = file_name_for_database ("hosts"),
+ .path_netgroup = file_name_for_database ("netgroup"),
+ .path_networks = file_name_for_database ("networks"),
+ .path_passwd = file_name_for_database ("passwd"),
+ .path_protocols = file_name_for_database ("protocols"),
+ .path_publickey = file_name_for_database ("publickey"),
+ .path_rpc = file_name_for_database ("rpc"),
+ .path_services = file_name_for_database ("services"),
+ .path_shadow = file_name_for_database ("shadow"),
+ };
+}
+
+static bool errors;
+
+static int
+sort_strings (const void *a, const void *b)
+{
+ return strcmp (*(const char **) a, *(const char **) b);
+}
+
+
+/* Test case from bug 17363: getnetgrent does not return the complete
+ list of entries if one of the netgroups in its definition tree is
+ empty. */
+static void
+test_bug17363 (void)
+{
+ int ret = setnetgrent ("netgr");
+ if (ret != 1)
+ {
+ printf ("error: setnetgrent (\"netgr\"): %m\n");
+ errors = true;
+ }
+
+#define COUNT 3
+ static const char *const expected[COUNT] =
+ {
+ ",u1,",
+ ",u2,",
+ ",u3,",
+ };
+
+ char *actual[COUNT] = {};
+
+ for (size_t i = 0; ; ++i)
+ {
+ char *host;
+ char *user;
+ char *domain;
+ ret = getnetgrent (&host, &user, &domain);
+ if (ret != 1)
+ {
+ if (i == COUNT)
+ break;
+ printf ("error: getnetgrent (%zu)\n", i);
+ errors = true;
+ break;
+ }
+ if (i == COUNT)
+ {
+ printf ("error: getnetgrent (%zu): unexpected success\n", i);
+ errors = true;
+ break;
+ }
+
+ if (host == NULL)
+ host = (char *) "";
+ if (user == NULL)
+ user = (char *) "";
+ if (domain == NULL)
+ domain = (char *) "";
+
+ ret = asprintf (actual + i, "%s,%s,%s", host, user, domain);
+ if (ret < 0)
+ {
+ printf ("error: asprintf: %m\n");
+ exit (1);
+ }
+ }
+
+ endnetgrent ();
+
+ if (!errors)
+ {
+ qsort (actual, COUNT, sizeof (actual[0]), sort_strings);
+ for (size_t i = 0; i < COUNT; ++i)
+ {
+ if (strcmp (actual[i], expected[i]) != 0)
+ {
+ printf ("error: getnetgrent (%zu): \"%s\" instead of \"%s\"\n",
+ i, actual[i], expected[i]);
+ errors = true;
+ }
+ free (actual[i]);
+ }
+ }
+#undef COUNT
+}
+
+static int
+do_test (void)
+{
+ configure_nss ();
+
+ void *handle = dlopen (LIBNSS_FILES_SO, RTLD_LAZY);
+ if (handle == NULL)
+ {
+ printf ("error: cannot open " LIBNSS_FILES_SO ": %s\n",
+ dlerror ());
+ return 1;
+ }
+
+ __typeof__ (&_nss_files_set_config) config_function =
+ dlsym (handle, "_nss_files_set_config");
+ if (config_function == NULL)
+ {
+ printf ("error: cannot find configuration function symbol: %s\n",
+ dlerror ());
+ return 1;
+ }
+
+ init_test_config ();
+ config_function (&test_config);
+
+ /* Check that the reconfiguration to the test files was
+ successful. */
+ const struct passwd *pw = getpwuid (0);
+ if (pw == NULL)
+ {
+ printf ("error: root user lookup failed: %m\n");
+ return 1;
+ }
+ if (strcmp (pw->pw_gecos, "glibc root test user") != 0)
+ {
+ printf ("error: incorrect GECOS field for root user: \"%s\n\"",
+ pw->pw_gecos);
+ return 1;
+ }
+
+ test_bug17363 ();
+ dlclose (handle);
+
+ return errors;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"