nsswitch: do not reload if "/" changes

  [Note: I tried putting this functionality in the file_change_detection
module, but that didn't have enough persistence.]

[Note: tested by instrumenting test-container.c and observing the
instrumentation with test containers on the root fs and on a separate
fs]

https://sourceware.org/bugzilla/show_bug.cgi?id=27077

Before reloading nsswitch.conf, verify that the root directory
hasn't changed - if it has, it's likely that we've entered a
container and should not trust the nsswitch inside the container
nor load any shared objects therein.
  

* DJ Delorie via Libc-alpha:

> diff --git a/nss/nss_database.c b/nss/nss_database.c
> index e719ec0865..580ea7b963 100644
> --- a/nss/nss_database.c
> +++ b/nss/nss_database.c
> @@ -33,6 +33,11 @@ struct nss_database_state
>  {
>    struct nss_database_data data;
>    __libc_lock_define (, lock);
> +  /* If "/" changes, we switched into a container and do NOT want to
> +     reload anything.  This data must be persistent across
> +     reloads.  */
> +  ino64_t root_ino;
> +  ino64_t root_dev;
>  };

dev_t for root_dev?

> @@ -53,6 +58,8 @@ global_state_allocate (void *closure)
>        memset (result->data.services, 0, sizeof (result->data.services));
>        result->data.initialized = true;
>        result->data.reload_disabled = false;
> +      result->root_ino = 0;
> +      result->root_dev = 0;
>        __libc_lock_init (result->lock);
>      }

Perhaps you can match the declaration order in the initialization?

> diff --git a/nss/nss_database.h b/nss/nss_database.h
> index 1f827e6def..f94c629174 100644
> --- a/nss/nss_database.h
> +++ b/nss/nss_database.h
> @@ -75,6 +75,10 @@ struct nss_database_data
>    nss_action_list services[NSS_DATABASE_COUNT];
>    int reload_disabled;          /* Actually bool; int for atomic access.  */
>    bool initialized;
> +  /* If "/" changes, we switched into a container and do NOT want to
> +     reload anything.  */
> +  ino64_t root_ino;
> +  ino64_t root_dev;
>  };
>  
>  /* Called by fork in the parent process, before forking.  */

This does not seem to be needed?

Rest looks good.

I have one remaining question: Should we load service modules after /
has changed?  Disabling reloading brings us back to the old behavior in
terms of exposure to untrusted /, but maybe we can do even better and
stop loading service modules altogether?  Assuming that this change is
compatible with init systems.

Thanks,
Florian
  

How does that interact with pivot_root?

Andreas.
  

* Andreas Schwab:

> How does that interact with pivot_root?

I expect it to disable reloading, as a safety measure.  Behavior will be
as before: If NSS was initialized before pivot_root, the old
/etc/nsswitch.conf will be used for the remaining life-time of the
process.  If not, the new configuration file will be read.

I don't think there is a way around that.  Maybe we could add a special
__nss_configure_lookup call to re-enable reloading for the cases where
this is safe.

Thanks,
Florian
  

On 1/15/21 7:59 PM, DJ Delorie via Libc-alpha wrote:
> 
> [Note: I tried putting this functionality in the file_change_detection
> module, but that didn't have enough persistence.]
> 
> [Note: tested by instrumenting test-container.c and observing the
> instrumentation with test containers on the root fs and on a separate
> fs]
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=27077
> 
> Before reloading nsswitch.conf, verify that the root directory
> hasn't changed - if it has, it's likely that we've entered a
> container and should not trust the nsswitch inside the container
> nor load any shared objects therein.

Can we create a non-test-container test for this?

I think you can use support_become_root to unshare and then try
to use support_chroot_create/support_chroot_free and xhcroot to 
change root, and then try to do an NSS call that will fail?

The test can start by calling __nss_lookup_configure to set
a known module to provide the NSS values, and then do an IdM
call, verify you're using a known value, and then try to
become root and chroot.

I'm not sure if this is possible though.
  

* Carlos O'Donell via Libc-alpha:

> Can we create a non-test-container test for this?
>
> I think you can use support_become_root to unshare and then try
> to use support_chroot_create/support_chroot_free and xhcroot to 
> change root, and then try to do an NSS call that will fail?

You need to chroot twice, first to get a defined /etc/nsswitch.conf, and
another one to make sure things don't ger reloaded after chroot.

You probably also have to copy different service modules into the two
chroots.

Thanks,
Florian
  

On 1/18/21 11:53 AM, Florian Weimer wrote:
> * Carlos O'Donell via Libc-alpha:
> 
>> Can we create a non-test-container test for this?
>>
>> I think you can use support_become_root to unshare and then try
>> to use support_chroot_create/support_chroot_free and xhcroot to 
>> change root, and then try to do an NSS call that will fail?
> 
> You need to chroot twice, first to get a defined /etc/nsswitch.conf, and
> another one to make sure things don't ger reloaded after chroot.

That would be a perfect solution.

However, I think you could get away with recording some known uid/gid
from the system that was doing the build and then ensure that value
is not present in the container.

> You probably also have to copy different service modules into the two
> chroots.

Correct.
  

* Carlos O'Donell:

> On 1/18/21 11:53 AM, Florian Weimer wrote:
>> * Carlos O'Donell via Libc-alpha:
>> 
>>> Can we create a non-test-container test for this?
>>>
>>> I think you can use support_become_root to unshare and then try
>>> to use support_chroot_create/support_chroot_free and xhcroot to 
>>> change root, and then try to do an NSS call that will fail?
>> 
>> You need to chroot twice, first to get a defined /etc/nsswitch.conf, and
>> another one to make sure things don't ger reloaded after chroot.
>
> That would be a perfect solution.
>
> However, I think you could get away with recording some known uid/gid
> from the system that was doing the build and then ensure that value
> is not present in the container.

The issue is with loading NSS modules from the system.  This can lead to
crashes/unpredictable behavior.  We've been through this already, if I
recall correctly.

Thanks,
Florian