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.

Sign up for GitHub

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

nixos/systemd: move systemd-provided NSS modules to systemd module #86940

Merged
merged 2 commits into from
May 5, 2020
Merged

nixos/systemd: move systemd-provided NSS modules to systemd module #86940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7426bec
nixos/systemd/resolved: add resolve to nss hosts database if enabled
flokli May 5, 2020
c0995d2
nixos/systemd: move NSS module logic to systemd module
flokli May 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 0 additions & 20 deletions nixos/modules/config/nsswitch.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,22 @@ let

# only with nscd up and running we can load NSS modules that are not integrated in NSS
canLoadExternalModules = config.services.nscd.enable;
myhostname = canLoadExternalModules;
mymachines = canLoadExternalModules;
# XXX Move these to their respective modules
nssmdns = canLoadExternalModules && config.services.avahi.nssmdns;
nsswins = canLoadExternalModules && config.services.samba.nsswins;
ldap = canLoadExternalModules && (config.users.ldap.enable && config.users.ldap.nsswitch);
resolved = canLoadExternalModules && config.services.resolved.enable;

hostArray = mkMerge [
(mkBefore [ "files" ])
(mkIf mymachines [ "mymachines" ])
(mkIf nssmdns [ "mdns_minimal [NOTFOUND=return]" ])
(mkIf nsswins [ "wins" ])
(mkIf resolved [ "resolve [!UNAVAIL=return]" ])
(mkAfter [ "dns" ])
(mkIf nssmdns (mkOrder 1501 [ "mdns" ])) # 1501 to ensure it's after dns
(mkIf myhostname (mkOrder 1600 [ "myhostname" ])) # 1600 to ensure it's always the last
];

passwdArray = mkMerge [
(mkBefore [ "files" ])
(mkIf ldap [ "ldap" ])
(mkIf mymachines [ "mymachines" ])
(mkIf canLoadExternalModules (mkAfter [ "systemd" ]))
];

shadowArray = mkMerge [
Expand Down Expand Up @@ -134,11 +126,6 @@ in {
assertion = config.system.nssModules.path != "" -> canLoadExternalModules;
message = "Loading NSS modules from path ${config.system.nssModules.path} requires nscd being enabled.";
}
{
# resolved does not need to add to nssModules, therefore needs an extra assertion
assertion = resolved -> canLoadExternalModules;
message = "Loading systemd-resolved's nss-resolve NSS module requires nscd being enabled.";
}
];

# Name Service Switch configuration file. Required by the C
Expand All @@ -164,12 +151,5 @@ in {
hosts = hostArray;
services = mkBefore [ "files" ];
};

# Systemd provides nss-myhostname to ensure that our hostname
# always resolves to a valid IP address. It returns all locally
# configured IP addresses, or ::1 and 127.0.0.2 as
# fallbacks. Systemd also provides nss-mymachines to return IP
# addresses of local containers.
system.nssModules = (optionals canLoadExternalModules [ config.systemd.package.out ]);
};
}
4 changes: 4 additions & 0 deletions nixos/modules/system/boot/resolved.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ in

users.users.resolved.group = "systemd-resolve";

# add resolve to nss hosts database if enabled and nscd enabled
# system.nssModules is configured in nixos/modules/system/boot/systemd.nix
Copy link
Member

Choose a reason for hiding this comment

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

This isn't enough. We need to mkOrder in such a way that we're sure dns follows resolve

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dns is still added with (mkAfter [ "dns" ]) - try nix-build nixos/tests/networking.nix --arg networkd true -A dhcpOneIf.driver && result/bin/nixos-run-vms and cat /etc/nsswitch.conf on the client:

cat /etc/nsswitch.conf | grep hosts
hosts:    files mymachines resolve [!UNAVAIL=return] dns myhostname

Copy link
Member

Choose a reason for hiding this comment

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

perfect

system.nssDatabases.hosts = optional config.services.nscd.enable "resolve [!UNAVAIL=return]";
flokli marked this conversation as resolved.
Show resolved Hide resolved

systemd.additionalUpstreamSystemUnits = [
"systemd-resolved.service"
];
Expand Down
21 changes: 21 additions & 0 deletions nixos/modules/system/boot/systemd.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,27 @@ in

system.build.units = cfg.units;

# Systemd provides various NSS modules to look up dynamic users, locally
# configured IP adresses and local container hostnames.
# On NixOS, these can only be passed to the NSS system via nscd (and its
# LD_LIBRARY_PATH), which is why it's usually a very good idea to have nscd
# enabled (also see the config.nscd.enable description).
# While there is already an assertion in place complaining loudly about
# having nssModules configured and nscd disabled, for some reason we still
# check for nscd being enabled before adding to nssModules.
Comment on lines +835 to +837
Copy link
Contributor Author

@flokli flokli May 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@florianjacob as the author of e370e97, can you elaborate on why we silently disable these nss modules if nscd is disabled, even though there's an assertion in https://github.com/NixOS/nixpkgs/pull/86940/files#diff-5796c52b71eee35842f408f4126430d6R126-R127 which should complain if nss modules are present, but nscd disabled (so nssModules are not respected)?

Maybe instead of silently ignoring these (and breaking dynamic user support, as well as other NSS modules), can't we ask the user to mkForce system.nssModules = [] in the assertion message if they really doesn't want any external NSS modules?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also see #43607 and #86010 for context.

Copy link
Member

Choose a reason for hiding this comment

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

I really like your idea. There is no reason for the current behaviour and system.nssModules = systemd.out seems like the most elegant solution and also prevents users from accidentially breaking their systems by disabling nscd.

Copy link
Member

Choose a reason for hiding this comment

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

I hope @florianjacob can shed some light on this. I think we should never have to add vague comments like for some reason we still check for nscd being enabled before adding to nssModules. The source should be a place of truth :) If we can't figure it out and the original authors don't respond we might as well change the implementation until we encounter errors and can properly document them.

system.nssModules = optional config.services.nscd.enable systemd.out;
system.nssDatabases = mkIf config.services.nscd.enable {
hosts = (mkMerge [
[ "mymachines" ]
(mkOrder 1600 [ "myhostname" ] # 1600 to ensure it's always the last
)
]);
passwd = (mkMerge [
[ "mymachines" ]
(mkAfter [ "systemd" ])
]);
};

environment.systemPackages = [ systemd ];

environment.etc = let
Expand Down