nixos/networkmanager: remove networking.networkmanager.dynamicHosts

[flokli]

This option was removed because allowing (multiple) regular users to
override host entries affecting the whole system opens up a huge attack
vector. There seem to be very rare cases where this might be useful.
Consider setting system-wide host entries using networking.hosts,
provide them via the DNS server in your network, or use
networking.networkmanager.appendNameservers to point your system to
another (local) nameserver to set those entries.

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

Notify maintainers

[rickynils]

@flokli I commented on #71322 before seeing you opened this PR: #71322 (comment)

I like to hear a little bit more about why you think this particular option is such a security risk that it needs to be removed from nixpkgs.

I agree with you on the flaws in its implementation (possibly misusing /run/NetworkManager), but my comment linked above offers a solution for that.

But security-wise: The option is disabled by default, and a user has to explicitly enable it and make a decision on which uid should have write permissions to the hosts dirs. I don't see a big security risk here. The documentation could be clearer on the security implications of enabling the option, though.

[flokli]

Moved from @rickynils #71322 (comment):

@flokli The dynamicHosts config was merged to master in d80292d, no PR created.

I'm fine with removing the nm-setup-hostsdirs service and the use of /run/NetworkManager/hostsdirs, and instead adding an option for configuring which host dirs NetworkManager/dnsmasq should look at. I guess a more sensible name for the option would then be networking.networkmanager.extraHostsDirs. It would then be up to the user to configure the directories and their permissions.

[flokli]

I like to hear a little bit more about why you think this particular option is such a security risk that it needs to be removed from nixpkgs.

The option is disabled by default, still it's advertised in the NixOS options documentation, so people might enable it because they think "it's a good idea".

Assume there's two users for whom this was configured , and one of this users gets compromised. It's possible to do system-wide DNS redirection attacks of the whole traffic. There's a reason on why /etc/hosts is only writable by root on regular distros, and why you need to use sudo to change most system-wide things.

On top of that, it's a feature that was implemented solely inside nixpkgs. There's no upstream NM "best practices", explaining that this is the recommended way for users to change /etc/hosts records, and I'm not aware of other distros providing something similar.

[flokli]

I guess a more sensible name for the option would then be networking.networkmanager.extraHostsDirs. It would then be up to the user to configure the directories and their permissions.

I'd assume it's also sufficient to do something like this:

environments.etc = {
  target = "NetworkManager/dnsmasq.d/dyndns.conf";	
  text = ''
    hostsdir=/path/to/imperative-hostsdir
  '';
}

… and this is simply setting a setting documented in NetworkManager and dnsmasq.

I still consider imperatively adding entries in there as a regular user a hack, or what's the usecase?

[rickynils]

My use case is for development/testing. It is a very simple way to dynamically update DNS entries to point to temporary VMs and containers.

I'm completely fine with removing it from nixpkgs, if it is not wanted there. It is simple enough for me to use a local NixOS module for this.

Maybe you should just mention the environment.etc = ... snippet in the release notes, instead of mentioning networking.networkmanager.appendNameservers, since piggy-backing the already running NM-controlled dnsmasq instance is much easier than configure an additional one.

[flokli]

You're right - just configuring the existing dnsmasq (if you happen to use the dnsmasq backend) is easier - I updated the PR and now point towards environment.etc. Does that look good to you now?

This might be a quick & dirty way around it, but at least for local containers this should be handled by NSS - and for anything remote I'd personally want HTTPS and a DNS record anyways.

[worldofpeace]

Fine with me 👍
I believe you supported your claim particularly well, the initial change could have used a PR. Changes to networkmanager in NixOS should have lots of review.