Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide NSS modules globally, make nscd unnecessary (v2) #155655

Closed
wants to merge 3 commits into from

Conversation

erikarvstedt
Copy link
Member

@erikarvstedt erikarvstedt commented Jan 19, 2022

This is a follow-up to #138178 (diff) which fixes the binary incompatibilities of the original PR.

This PR allows glibc client binaries to access NSS modules configured via system.nssModules without nscd.
nscd has significant caching bugs and causes friction in general (#95107, #154928).
For details about nscd bugs, see issue DNS responses are cached and the Fedora nscd deprecation notes.

Some services set LD_LIBRARY_PATH to allow running them without nscd. These workarounds are now obsolete and are removed by this PR.

Implementation

Provide global NSS modules at /run/nss-modules-${word_size}-${glibc_version}/lib (e.g. /run/nss-modules-64-2.34/lib) and patch glibc to use this path.
The versioning suffix ensures that only binary compatible glibc client binaries will use this path.

Repo erikarvstedt/check-glibc-compatibilities shows that different NSS modules and glibc clients are compatible with each other, as long as they share the same minor glibc release (e.g. 2.34).

Because the patched code region is never inlined, the patch affects all binaries that dynamically link glibc. This includes binaries prebuilt with a non-nixpkgs libc that are processed with patchelf (like slack).

Todo

  • nscd is still enabled to provide NSS modules for binaries with non-system glibc versions and for 32-bit binaries on 64-bit hosts.
    In light of its defects and lack of maintenance, it might be sensible to disable nscd by default.
    Note: unscd is no replacement for nscd because it doesn't implement all nsswitch functions (src thread).
  • We'll add release notes for this PR as soon as it reaches community consensus.
  • To support 32-bit binaries on 64-bit hosts without nscd, we could add options analogous to opengl.driSupport32Bit and opengl.extraPackages32. As a minimum, systemd NSS modules should be provided. This can be addressed in another PR.

Appendix

Fixes: #135888
Fixes: #105353
Fixes: #55276
Cc: #52411 (comment)
Closes: #111194

This PR is co-authored by @flokli.

flokli and others added 3 commits January 19, 2022 13:05
…stem.nssModules.path

This makes this point to a single folder, not a colon-separated list of
directories, which makes it much easier to symlink to it (what the
next commit does).

This makes overriding *already existing* NSS modules harder, as we can't
just pretend to the list, but it's probably a good idea to explicitly
handle this, instead of silently shadowing - plus, I'm not aware of
anything in nixpkgs actually overwriting existing NSS modules.
NSS modules are now globally provided by a symlink in `/run`.

See the description in `add-extra-module-load-path.patch` for further details.

Fixes: NixOS#55276
Fixes: NixOS#135888
Fixes: NixOS#105353
Cc:    NixOS#52411 (comment)

Co-authored-by: Erik Arvstedt <[email protected]>
@erikarvstedt erikarvstedt changed the base branch from master to staging January 19, 2022 15:26
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Jan 19, 2022
@flokli
Copy link
Contributor

flokli commented Jan 19, 2022

@dasJ this PR mostly provides the plumbing (glibc patch) to make some of these things possible, which is the heavy rebuild (and why this targets staging)

IMHO, we can do the discussions on what nss modules should be available by default (old glibc versions, 32bit or not) in the PR switching the nscd default to false.

As long as nscd is enabled (which this PR doesn't change), NSS requests still go through it, so all existing usecases are still supported.

@flokli flokli mentioned this pull request Jan 19, 2022
13 tasks
@flokli
Copy link
Contributor

flokli commented Jan 26, 2022

@edolstra since you initially 👎 this on #138178 (comment), do you have an opinion on this iteration?

As written in the PR description, this should address the binary incompatibilities and 32bit concerns mentioned in case of a disabled nscd, and still keeps nscd enabled by default, using a similar approach as the one you suggested.

systemd.tmpfiles.rules = let
glibcPlatform = "${if pkgs.stdenv.hostPlatform.is64bit then "64" else "32"}-${pkgs.glibc.version}";
in [
"L+ /run/nss-modules-${glibcPlatform} - - - - ${config.system.nssModules.path}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this will "leak" symlinks over time as glibc versions change? Couldn't this be part of /run/current-system?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will create symlinks to nix store paths there (until it's rebooted). Nix might eventually garbage collect them away, I'm not sure if /run is checked. In that case, the NSS lookup will fail using that NSS module, which is probably still better than segfaulting ;-)

Having these NSS moduels inside /run/current-system would mean one would need to keep multiple glibc versions around as part of the system closure.

As this is only meant to prevent breakage of still running old versions of binaries, compiled against an old glibc (and even those will use nscd by default), I'd consider this to be not much of a problem.

@edolstra
Copy link
Member

edolstra commented Jan 26, 2022

I'm not in favor of this. While nscd has its problems, every other proposed solution is significantly worse and more painful. E.g. requiring a bunch of impure /run/nss-modules-${word_size}-${glibc_version}/lib symlinks means that you just can't run an arbitrary Nix closure anymore; you have to make sure that a symlink for the exact version of Glibc used by that closure exists in /run, and there is no obvious way for users to figure out how to do that. It's also not clear how this would work on non-NixOS systems.

My experience with OpenGL (/run/opengl-drivers) is that after any major NixOS upgrade, old closures that use OpenGL simply stop working. So I have very little faith in this approach.

I think it may be worthwhile to allow the NSS modules location to be overriden via environment variables. That allows people to build pure closures containing the NSS modules they need in particular environments, rather than requiring them to create /run/nss-modules symlinks that they may not be privileged to do. See #111194.

@edolstra
Copy link
Member

It's also not clear to me how this is going to work in practical terms, especially on non-NixOS systems. For instance, suppose I have previously installed some package using nix-env -i and I upgrade my NixOS system. Now suddenly the package stops working because I don't have the appropriate /run/nss-modules-${word_size}-${glibc_version} symlink. Whereas with nscd, it just works. We must not break that scenario.

On non-NixOS systems this is even worse, since there is no obvious way to create/manage /run/nss-modules-${word_size}-${glibc_version}. At least with #111194, I could use makeWrapper to create a closure that contains all the required non-standard NSS modules, which is fairly straightforward and doesn't require any outside configuration.

@edolstra
Copy link
Member

Really what I'm trying to say here is that /run/opengl-driver and /run/current-system/sw/lib/locale/locale-archive are hacks that violate the spirit of Nix (hermeticism) and we should avoid making that problem bigger. With NSS modules it was kind of lucky that there is a mechanism (nscd) that allows us to get around the ABI issue.

@picnoir
Copy link
Member

picnoir commented Mar 1, 2022

FYI: The Guix folks poked upstream about the Nscd removal: https://sourceware.org/pipermail/libc-alpha/2022-February/136741.html

Relevant part:

Ludovic and I compared notes on the future of nscd, and we discussed a what-if
scenario. What if we kept nscd, but removed all the caches, and left it as a
daemon that could answer requests from system processes but the only goal was
to decouple the libc required to service the request and the libc in the process?

This is already starting to sound like Florian's getent idea where by a static
process uses a known binary to carry out the NSS lookup and resolution [1].

What if instead of getent we just thinned down nscd to something we could audit
and trust? If we really wanted to we could even compile the same code into getent
as a fallback e.g. try nscd first (long lived) then getent (short lived).

You might say "Well then nscd is mis-named!" I'd argue it's a "cache" for the shared
objects and state required to do the lookup, but not the data itself, and if you
didn't run nscd then you'd go through NSS normally, or "getent" if you were a
statically compiled binary.

Full thread: https://sourceware.org/pipermail/libc-alpha/2022-March/thread.html#136745

@edolstra
Copy link
Member

edolstra commented Mar 2, 2022

What if we kept nscd, but removed all the caches

I think we're already using nscd in that way, since all the TTLs in /etc/nscd.conf are set to 0.

@Ericson2314
Copy link
Member

Where do these files go today? Say they are just in an output of libc, and programs reference that output. What goes wrong?

@flokli
Copy link
Contributor

flokli commented Mar 4, 2022

Where do these files go today? Say they are just in an output of libc, and programs reference that output. What goes wrong?

I'm not sure I understand the question. Without nscd, glibc lookup nss modules defined in /etc/nsswitch.conf by loading libnss_*.so files, from ${glibc.out}/lib/and everything else in LD_LIBRARY_PATH.

This is all fine for glibc-provided NSS modules, but doesn't allow discovery of NSS modules provided by other packages (like the systemd ones, which are quite critical for dynamic user support, or the avahi or ldap ones, …).
That's usually highly dependent on the system that's running a binary, needs to be admin-configurable. And to tell glibc where to find these nss modules, we probably don't want to set LD_LIBRARY_PATH globally ;-)

Right now we do this by piggy-backing on nscd (without the caching part), and set LD_LIBRARY_PATH for only that process to the list of nss modules. glibc has some code to always ask nscd first, NixOS has nscd running, and nscd knows about all the system-wide nss modules.

This PR proposes adding another path to the glibc lookup path, before asking nscd (/run/nss-modules-${word_size}-${glibc_version}). Idea is that NixOS systems can provide a NSS module closure at that location (and not need nscd anymore).

@edolstra you mentioned this is worse on non-NixOS systems. However, non-NixOS systems already need nscd running so they can do lookups with non-glibc NSS modules. This has been a problem all the time (we just ignored it so far) and doesn't change with this PR - it just puts nscd (theoretically) out of the critical path in NixOS systems.

We could probably do a compromise - use the logic from the patch here, look in /run/nss-modules-${word_size}-${glibc_version}, but allowing to override this path with a NIX_IMPURE_RUNTIME_DIR env var (name suggested in #111194 (comment)).

With this approach:

  • For NixOS systems: We can move nscd out of the critical path
  • For non-NixOS: people who really want to ship a closure of NSS modules to non-NixOS systems (even though I'd argue it depends highly on the target system) could just makeWrapper their binary and set NIX_IMPURE_RUNTIME_DIR to a nix-provided path
  • For non-NixOS: people who really want to use the system-provided one (and are certain they're binary-compatible) can set NIX_IMPURE_RUNTIME_DIR to /usr/lib
  • For non-NixOS: people who don't want to control NSS modules can rely on nscd running on the non-NixOS system (like before)

@Ericson2314
Copy link
Member

Thanks @flokli for explaining. I think I will email the list to second the Guix thing regardless of what we do here.

@Ericson2314
Copy link
Member

Ericson2314 commented Mar 6, 2022

My preference is nscd as a "lingua franca" is still a good idea, unless we want to just statically link the systemd-resolved and sssd clients, which is....a big move.

@Ericson2314
Copy link
Member

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Apr 15, 2022

Really what I'm trying to say here is that /run/opengl-driver and /run/current-system/sw/lib/locale/locale-archive are hacks that violate the spirit of Nix (hermeticism) and we should avoid making that problem bigger.

This is not only a philosophical issue: the /run/opengl-driver impurity is already making debugging OpenGL errors in Nixpkgs a nightmare. Just look at this issue: #80936 (comment). There's also the fact that nixGL is mandatory on non-NixOS and most users don't know about it.

I dread the thought of this sort of situation extending to all programs through glibc. I understand that using a daemon just to discover a bunch of shared objects is a waste of resources and that the nscd DNS caching is a problem, but introducing another impurity is a strictly worse one.

@flokli
Copy link
Contributor

flokli commented Apr 16, 2022

The conversation in the guix ML also seems to lean towards keeping nscd around.

I'm still curious about how we can fix the issues we have with it right now:

@Ericson2314
Copy link
Member

Thanks for linking #124019. It is good to un-hardcode the choice of nscd like that. I hope we can reopen and merge that PR.

@SuperSandro2000
Copy link
Member

Thanks for linking #124019. It is good to un-hardcode the choice of nscd like that. I hope we can reopen and merge that PR.

unscd has the last release 2014, so it is pretty safe ot assume that development is dead. Also sounds like a suboptimal solution.

@Ericson2314
Copy link
Member

As I said on that PR, I am less enamored with the idea of unscd in particular, than the principle that the choice of daemon should be configurable in NixOS, rather than hardcoded conditional code. I am happy to add the unscd package too in case anyone wants to try it out, even if I am not optomistic that package in particular would save the day.

The best solution does seem to team up with Guix to ensure that the upstream NSCD becomes a lot simpler.

@ctheune
Copy link
Contributor

ctheune commented Sep 16, 2022

I also think that this idea seems most practical. Also, there is a bit of urgency on this topic in general. The latent caching bugs (like faulty cached NXDOMAIN for lookups that once lost a UDP packet) keep creeping up every now and then and there's little we can do (except restart nscd if it happens and disable nscd if the affected node doesn't use any of the modules).

However, I wish @edolstra's statement about the caching was true. We do set it to 0 but there are significant caching bugs in nscd that still trigger even when turned off ... 😢

What if we kept nscd, but removed all the caches

I think we're already using nscd in that way, since all the TTLs in /etc/nscd.conf are set to 0.

@dasJ
Copy link
Member

dasJ commented Sep 16, 2022

Just from skimming the glibc source code, it may be enough to LD_PRELOAD a small library into nscd that has a cache_add function that acts as a noop.
Signature:

/* Add a new entry to the cache.  The return value is zero if the function
   call was successful.
   This function must be called with the read-lock held.
   We modify the table but we nevertheless only acquire a read-lock.
   This is ok since we use operations which would be safe even without
   locking, given that the `prune_cache' function never runs.  Using
   the readlock reduces the chance of conflicts.  */
int
cache_add (int type, const void *key, size_t len, struct datahead *packet,
	   bool first, struct database_dyn *table,
	   uid_t owner, bool prune_wakeup)

Source

@dasJ
Copy link
Member

dasJ commented Sep 16, 2022

This could work but is untested:

#include <stddef.h>
#include <stdbool.h>
#include <unistd.h>

int cache_add (int type, const void *key, size_t len, struct datahead *packet, bool first, struct database_dyn *table, uid_t owner, bool prune_wakeup) {
	return 0;
}

Build with:

gcc -Wall -Wextra -O3 -shared -fPIC -static-libgcc -o test.so test.c

Use with:

{
  systemd.services.nscd.environment.LD_PRELOAD = /path/to/test.so;
}

@xaverdh
Copy link
Contributor

xaverdh commented Nov 13, 2022

This can be closed, since we have #196917, right?

@flokli
Copy link
Contributor

flokli commented Nov 13, 2022

Hmmh, this specific PR has probably bitrotten.

I think NixOS should probably be using nsncd, but on some non-NixOS distributions it might still make sense to have an environment variable that you can set to steer nix-built binaries to a set of NSS modules, without polluting all of LD_LIBRARY_PATH, in case you can't run nscd or nsncd on that system.

This would be the $NIX_GLIBC_NSS_PATH patch to glibc from #111194, but without any of the module system changes.

--

NixOS with nsncd also still has the problem of DNS requests/host lookups leaking out of network namespaces. This could maybe be solved in nsncd, if we can detect the network namespace of the nss client in a race-free fashion.

@flokli
Copy link
Contributor

flokli commented Nov 18, 2022

I summarized the different approaches in https://flokli.de/posts/2022-11-18-nsncd/.

I think this can be closed.

@flokli flokli closed this Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: clean-up 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ 10.rebuild-linux-stdenv This PR causes stdenv to rebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DNS responses are cached sslh depends on nscd