This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC: Test hook for nss_files testing


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]