From c07ce093ecc6a62e10a9abff39e96a27540c3fbc Mon Sep 17 00:00:00 2001 From: Andreas Rammhold Date: Thu, 7 May 2020 13:12:38 +0200 Subject: [PATCH 01/10] unbound: allow building with systemd support Systemd has to remain an optional (non-default) dependency as otherwise we will have an unpleasant bootstrap cycle. Most (if not all) of the (lib)unbound consumers will likely not care about unbound's systemd integration that only affects the daemon mode, anyway. --- pkgs/tools/networking/unbound/default.nix | 40 ++++++++++++++++++----- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/pkgs/tools/networking/unbound/default.nix b/pkgs/tools/networking/unbound/default.nix index 9b33d53e54f..72653b672ee 100644 --- a/pkgs/tools/networking/unbound/default.nix +++ b/pkgs/tools/networking/unbound/default.nix @@ -1,4 +1,24 @@ -{ stdenv, fetchurl, openssl, nettle, expat, libevent, dns-root-data }: +{ stdenv +, lib +, fetchurl +, openssl +, nettle +, expat +, libevent +, dns-root-data +, pkg-config + # + # By default unbound will not be built with systemd support. Unbound is a very + # commmon dependency. The transitive dependency closure of systemd also + # contains unbound. + # Since most (all?) (lib)unbound users outside of the unbound daemon usage do + # not need the systemd integration it is likely best to just default to no + # systemd integration. + # For the daemon use-case, that needs to notify systemd, use `unbound-with-systemd`. + # +, withSystemd ? false +, systemd ? null +}: stdenv.mkDerivation rec { pname = "unbound"; @@ -11,7 +31,7 @@ stdenv.mkDerivation rec { outputs = [ "out" "lib" "man" ]; # "dev" would only split ~20 kB - buildInputs = [ openssl nettle expat libevent ]; + buildInputs = [ openssl nettle expat libevent ] ++ lib.optionals withSystemd [ pkg-config systemd ]; configureFlags = [ "--with-ssl=${openssl.dev}" @@ -25,6 +45,8 @@ stdenv.mkDerivation rec { "--enable-relro-now" ] ++ stdenv.lib.optional stdenv.hostPlatform.isStatic [ "--disable-flto" + ] ++ lib.optionals withSystemd [ + "--enable-systemd" ]; installFlags = [ "configfile=\${out}/etc/unbound/unbound.conf" ]; @@ -33,7 +55,7 @@ stdenv.mkDerivation rec { make unbound-event-install ''; - preFixup = stdenv.lib.optionalString (stdenv.isLinux && !stdenv.hostPlatform.isMusl) # XXX: revisit + preFixup = lib.optionalString (stdenv.isLinux && !stdenv.hostPlatform.isMusl) # XXX: revisit # Build libunbound again, but only against nettle instead of openssl. # This avoids gnutls.out -> unbound.lib -> openssl.out. # There was some problem with this on Darwin; let's not complicate non-Linux. @@ -43,17 +65,17 @@ stdenv.mkDerivation rec { buildPhase installPhase '' - # get rid of runtime dependencies on $dev outputs + # get rid of runtime dependencies on $dev outputs + ''substituteInPlace "$lib/lib/libunbound.la" '' - + stdenv.lib.concatMapStrings - (pkg: " --replace '-L${pkg.dev}/lib' '-L${pkg.out}/lib' --replace '-R${pkg.dev}/lib' '-R${pkg.out}/lib'") - buildInputs; + + lib.concatMapStrings + (pkg: lib.optionalString (pkg ? dev) " --replace '-L${pkg.dev}/lib' '-L${pkg.out}/lib' --replace '-R${pkg.dev}/lib' '-R${pkg.out}/lib'") + (builtins.filter (p: p != null) buildInputs); - meta = with stdenv.lib; { + meta = with lib; { description = "Validating, recursive, and caching DNS resolver"; license = licenses.bsd3; homepage = "https://www.unbound.net"; maintainers = with maintainers; [ ehmry fpletz globin ]; - platforms = stdenv.lib.platforms.unix; + platforms = platforms.unix; }; } From f6d570b2589b61c4605f19bca0cba2ce1f076d44 Mon Sep 17 00:00:00 2001 From: Andreas Rammhold Date: Thu, 7 May 2020 13:15:04 +0200 Subject: [PATCH 02/10] unbound-with-systemd: init This introduces an unbound variant that is built with systemd support. That means it is able to signal readiness to systemd once it did start or finished reloading. This likely allows us to close a small gap during bootup where the service is assumed up but doesn't respond to DNS queries just yet. --- pkgs/top-level/all-packages.nix | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix index fbad7d9145b..3a75ed3c391 100644 --- a/pkgs/top-level/all-packages.nix +++ b/pkgs/top-level/all-packages.nix @@ -8076,7 +8076,11 @@ in unclutter-xfixes = callPackage ../tools/misc/unclutter-xfixes { }; - unbound = callPackage ../tools/networking/unbound { }; + unbound = callPackage ../tools/networking/unbound {}; + + unbound-with-systemd = unbound.override { + withSystemd = true; + }; unicorn = callPackage ../development/libraries/unicorn { }; From 5e602f88d1e8ba97491dd60c794c2faca273eccf Mon Sep 17 00:00:00 2001 From: Andreas Rammhold Date: Thu, 7 May 2020 13:17:14 +0200 Subject: [PATCH 03/10] nixos/modules/services/networking/unbound: update systemd unit Previously we just applied a very minimal set of restrictions and trusted unbound to properly drop root privs and capabilities. With this change I am (for the most part) just using the upstream example unit file for unbound. The main difference is that we start unbound was `unbound` user with the required capabilities instead of letting unbound do the chroot & uid/gid changes. The upstream unit configuration this is based on is a lot stricter with all kinds of permissions then our previous variant. It also came with the default of having the `Type` set to `notify`, therefore we are also using the `unbound-with-systemd` package here. Unbound will start up, read the configuration files and start listening on the configured ports before systemd will declare the unit "running". This will likely help with startup order and the occasional race condition during system activation where the DNS service is started but not yet ready to answer queries. Aditionally to the much stricter runtime environmet I removed the `/dev/urandom` mount lines we previously had in the code (that would randomly fail during `stop`-phase). The `preStart` script is now only required if we enabled the trust anchor updates (which are still enabled by default). Another beneefit of the refactoring is that we can now issue reloads via either `pkill -HUP unbound` or `systemctl reload unbound` to reload the running configuration without taking the daemon offline. A prerequisite of this was that unbound configuration is available on a well known path on the file system. I went for /etc/unbound/unbound.conf as that is the default in the CLI tooling which in turn enables us to use `unbound-control` without passing a custom configuration location. --- nixos/modules/services/networking/unbound.nix | 100 ++++++++++++------ 1 file changed, 68 insertions(+), 32 deletions(-) diff --git a/nixos/modules/services/networking/unbound.nix b/nixos/modules/services/networking/unbound.nix index baed83591e1..bcb48678b21 100644 --- a/nixos/modules/services/networking/unbound.nix +++ b/nixos/modules/services/networking/unbound.nix @@ -1,9 +1,7 @@ { config, lib, pkgs, ... }: with lib; - let - cfg = config.services.unbound; stateDir = "/var/lib/unbound"; @@ -17,12 +15,12 @@ let forward = optionalString (any isLocalAddress cfg.forwardAddresses) '' do-not-query-localhost: no - '' + - optionalString (cfg.forwardAddresses != []) '' + '' + + optionalString (cfg.forwardAddresses != []) '' forward-zone: name: . - '' + - concatMapStringsSep "\n" (x: " forward-addr: ${x}") cfg.forwardAddresses; + '' + + concatMapStringsSep "\n" (x: " forward-addr: ${x}") cfg.forwardAddresses; rootTrustAnchorFile = "${stateDir}/root.key"; @@ -31,19 +29,20 @@ let confFile = pkgs.writeText "unbound.conf" '' server: + ip-freebind: yes directory: "${stateDir}" username: unbound - chroot: "${stateDir}" + chroot: "" pidfile: "" + # when running under systemd there is no need to daemonize + do-daemonize: no ${interfaces} ${access} ${trustAnchor} ${cfg.extraConfig} ${forward} ''; - in - { ###### interface @@ -55,8 +54,8 @@ in package = mkOption { type = types.package; - default = pkgs.unbound; - defaultText = "pkgs.unbound"; + default = pkgs.unbound-with-systemd; + defaultText = "pkgs.unbound-with-systemd"; description = "The unbound package to use"; }; @@ -69,11 +68,14 @@ in interfaces = mkOption { default = [ "127.0.0.1" ] ++ optional config.networking.enableIPv6 "::1"; type = types.listOf types.str; - description = "What addresses the server should listen on."; + description = '' + What addresses the server should listen on. This supports the interface syntax documented in + unbound.conf8. + ''; }; forwardAddresses = mkOption { - default = [ ]; + default = []; type = types.listOf types.str; description = "What servers to forward queries to."; }; @@ -110,6 +112,9 @@ in networking.resolvconf.useLocalResolver = mkDefault true; + + environment.etc."unbound/unbound.conf".source = confFile; + systemd.services.unbound = { description = "Unbound recursive Domain Name Server"; after = [ "network.target" ]; @@ -117,32 +122,63 @@ in wants = [ "nss-lookup.target" ]; wantedBy = [ "multi-user.target" ]; - preStart = '' - mkdir -m 0755 -p ${stateDir}/dev/ - cp ${confFile} ${stateDir}/unbound.conf - ${optionalString cfg.enableRootTrustAnchor '' - ${cfg.package}/bin/unbound-anchor -a ${rootTrustAnchorFile} || echo "Root anchor updated!" - chown unbound ${stateDir} ${rootTrustAnchorFile} - ''} - touch ${stateDir}/dev/random - ${pkgs.utillinux}/bin/mount --bind -n /dev/urandom ${stateDir}/dev/random + preStart = lib.mkIf cfg.enableRootTrustAnchor '' + ${cfg.package}/bin/unbound-anchor -a ${rootTrustAnchorFile} || echo "Root anchor updated!" ''; - serviceConfig = { - ExecStart = "${cfg.package}/bin/unbound -d -c ${stateDir}/unbound.conf"; - ExecStopPost="${pkgs.utillinux}/bin/umount ${stateDir}/dev/random"; + restartTriggers = [ + confFile + ]; - ProtectSystem = true; - ProtectHome = true; + serviceConfig = { + ExecStart = "${cfg.package}/bin/unbound -p -d -c /etc/unbound/unbound.conf"; + ExecReload = "+/run/current-system/sw/bin/kill -HUP $MAINPID"; + + NotifyAccess = "main"; + Type = "notify"; + + AmbientCapabilities = [ + "CAP_NET_BIND_SERVICE" + "CAP_NET_RAW" + "CAP_SETGID" + "CAP_SETUID" + "CAP_SYS_CHROOT" + "CAP_SYS_RESOURCE" + ]; + + User = "unbound"; + + MemoryDenyWriteExecute = true; + NoNewPrivileges = true; PrivateDevices = true; - Restart = "always"; - RestartSec = "5s"; + PrivateTmp = true; + ProtectHome = true; + ProtectControlGroups = true; + ProtectKernelModules = true; + ProtectSystem = "strict"; + RuntimeDirectory = "unbound"; + ConfigurationDirectory = "unbound"; + StateDirectory = "unbound"; + RestrictAddressFamilies = [ "AF_INET" "AF_INET6" "AF_UNIX" ]; + RestrictRealtime = true; + SystemCallArchitectures = "native"; + SystemCallFilter = [ + "~@clock" + "@cpu-emulation" + "@debug" + "@keyring" + "@module" + "mount" + "@obsolete" + "@resources" + ]; + RestrictNamespaces = true; + LockPersonality = true; + RestrictSUIDSGID = true; + ReadWritePaths = [ "/run/unbound" "${stateDir}" ]; }; }; - # If networkmanager is enabled, ask it to interface with unbound. networking.networkmanager.dns = "unbound"; - }; - } From 72fbf05c17374b01abd7b6b1927de4146a7251eb Mon Sep 17 00:00:00 2001 From: Andreas Rammhold Date: Sun, 1 Nov 2020 22:11:11 +0100 Subject: [PATCH 04/10] nixos/unbound: note about the AmbientCapabilities --- nixos/modules/services/networking/unbound.nix | 1 + 1 file changed, 1 insertion(+) diff --git a/nixos/modules/services/networking/unbound.nix b/nixos/modules/services/networking/unbound.nix index bcb48678b21..bc2d5e550ba 100644 --- a/nixos/modules/services/networking/unbound.nix +++ b/nixos/modules/services/networking/unbound.nix @@ -137,6 +137,7 @@ in NotifyAccess = "main"; Type = "notify"; + # FIXME: Which of these do we actualy need, can we drop the chroot flag? AmbientCapabilities = [ "CAP_NET_BIND_SERVICE" "CAP_NET_RAW" From aadc07618aff8106f888c67383fb32e471dd817e Mon Sep 17 00:00:00 2001 From: Andreas Rammhold Date: Sun, 1 Nov 2020 22:22:08 +0100 Subject: [PATCH 05/10] nixos/unbound: drop ReadWritePaths from systemd unit configuration Both of the configured paths should be implicit due to RuntimeDirectory & StateDirectory. --- nixos/modules/services/networking/unbound.nix | 1 - 1 file changed, 1 deletion(-) diff --git a/nixos/modules/services/networking/unbound.nix b/nixos/modules/services/networking/unbound.nix index bc2d5e550ba..07e58481a77 100644 --- a/nixos/modules/services/networking/unbound.nix +++ b/nixos/modules/services/networking/unbound.nix @@ -176,7 +176,6 @@ in RestrictNamespaces = true; LockPersonality = true; RestrictSUIDSGID = true; - ReadWritePaths = [ "/run/unbound" "${stateDir}" ]; }; }; # If networkmanager is enabled, ask it to interface with unbound. From a040a8a2e3e598d24be81d36b66fd8c195c019da Mon Sep 17 00:00:00 2001 From: Andreas Rammhold Date: Wed, 21 Oct 2020 01:34:24 +0200 Subject: [PATCH 06/10] nixos/tests/unbound: init --- nixos/tests/all-tests.nix | 1 + nixos/tests/unbound.nix | 247 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 248 insertions(+) create mode 100644 nixos/tests/unbound.nix diff --git a/nixos/tests/all-tests.nix b/nixos/tests/all-tests.nix index 4e4d8b5e689..f1d14c9f2d5 100644 --- a/nixos/tests/all-tests.nix +++ b/nixos/tests/all-tests.nix @@ -362,6 +362,7 @@ in trezord = handleTest ./trezord.nix {}; trickster = handleTest ./trickster.nix {}; tuptime = handleTest ./tuptime.nix {}; + unbound = handleTest ./unbound.nix {}; udisks2 = handleTest ./udisks2.nix {}; unit-php = handleTest ./web-servers/unit-php.nix {}; upnp = handleTest ./upnp.nix {}; diff --git a/nixos/tests/unbound.nix b/nixos/tests/unbound.nix new file mode 100644 index 00000000000..bbccfa9c043 --- /dev/null +++ b/nixos/tests/unbound.nix @@ -0,0 +1,247 @@ +/* + Test that our unbound module indeed works as most users would expect. + There are a few settings that we must consider when modifying the test. The + ususal use-cases for unbound are + * running a recursive DNS resolver on the local machine + * running a recursive DNS resolver on the local machine, forwarding to a local DNS server via UDP/53 & TCP/53 + * running a recursive DNS resolver on the local machine, forwarding to a local DNS server via TCP/853 (DoT) + * running a recursive DNS resolver on a machine in the network awaiting input from clients over TCP/53 & UDP/53 + * running a recursive DNS resolver on a machine in the network awaiting input from clients over TCP/853 (DoT) + + In the below test setup we are trying to implement all of those use cases + without creating a bazillion machines. +*/ +import ./make-test-python.nix ({ pkgs, lib, ... }: + let + # common client configuration that we can just use for the multitude of + # clients we are constructing + common = { lib, pkgs, ... }: { + config = { + environment.systemPackages = [ pkgs.knot-dns ]; + + # disable the root anchor update as we do not have internet access during + # the test execution + services.unbound.enableRootTrustAnchor = false; + }; + }; + + cert = pkgs.runCommandNoCC "selfSignedCerts" { buildInputs = [ pkgs.openssl ]; } '' + openssl req -x509 -newkey rsa:4096 -keyout key.pem -out cert.pem -nodes -subj '/CN=dns.example.local' + mkdir -p $out + cp key.pem cert.pem $out + ''; + in + { + name = "unbound"; + meta = with pkgs.stdenv.lib.maintainers; { + maintainers = [ andir ]; + }; + + nodes = { + + # The server that actually serves our zones, this tests unbounds authoriative mode + authoritative = { lib, pkgs, config, ... }: { + imports = [ common ]; + networking.interfaces.eth1.ipv4.addresses = lib.mkForce [ + { address = "192.168.0.1"; prefixLength = 24; } + ]; + networking.interfaces.eth1.ipv6.addresses = lib.mkForce [ + { address = "fd21::1"; prefixLength = 64; } + ]; + networking.firewall.allowedTCPPorts = [ 53 ]; + networking.firewall.allowedUDPPorts = [ 53 ]; + + services.unbound = { + enable = true; + interfaces = [ "192.168.0.1" "fd21::1" "::1" "127.0.0.1" ]; + allowedAccess = [ "192.168.0.0/24" "fd21::/64" "::1" "127.0.0.0/8" ]; + extraConfig = '' + server: + local-data: "example.local. IN A 1.2.3.4" + local-data: "example.local. IN AAAA abcd::eeff" + ''; + }; + }; + + # The resolver that knows that fowards (only) to the authoritative server + # and listens on UDP/53, TCP/53 & TCP/853. + resolver = { lib, nodes, ... }: { + imports = [ common ]; + networking.interfaces.eth1.ipv4.addresses = lib.mkForce [ + { address = "192.168.0.2"; prefixLength = 24; } + ]; + networking.interfaces.eth1.ipv6.addresses = lib.mkForce [ + { address = "fd21::2"; prefixLength = 64; } + ]; + networking.firewall.allowedTCPPorts = [ + 53 # regular DNS + 853 # DNS over TLS + ]; + networking.firewall.allowedUDPPorts = [ 53 ]; + + services.unbound = { + enable = true; + allowedAccess = [ "192.168.0.0/24" "fd21::/64" "::1" "127.0.0.0/8" ]; + interfaces = [ "::1" "127.0.0.1" "192.168.0.2" "fd21::2" "192.168.0.2@853" "fd21::2@853" "::1@853" "127.0.0.1@853" ]; + forwardAddresses = [ + (lib.head nodes.authoritative.config.networking.interfaces.eth1.ipv6.addresses).address + (lib.head nodes.authoritative.config.networking.interfaces.eth1.ipv4.addresses).address + ]; + extraConfig = '' + server: + tls-service-pem: ${cert}/cert.pem + tls-service-key: ${cert}/key.pem + ''; + }; + }; + + # machine that runs a local unbound that will be reconfigured during test execution + local_resolver = { lib, nodes, ... }: { + imports = [ common ]; + networking.interfaces.eth1.ipv4.addresses = lib.mkForce [ + { address = "192.168.0.3"; prefixLength = 24; } + ]; + networking.interfaces.eth1.ipv6.addresses = lib.mkForce [ + { address = "fd21::3"; prefixLength = 64; } + ]; + networking.firewall.allowedTCPPorts = [ + 53 # regular DNS + ]; + networking.firewall.allowedUDPPorts = [ 53 ]; + + services.unbound = { + enable = true; + allowedAccess = [ "::1" "127.0.0.0/8" ]; + interfaces = [ "::1" "127.0.0.1" ]; + extraConfig = '' + include: "/etc/unbound/extra*.conf" + ''; + }; + + environment.etc = { + "unbound-extra1.conf".text = '' + forward-zone: + name: "example.local." + forward-addr: ${(lib.head nodes.resolver.config.networking.interfaces.eth1.ipv6.addresses).address} + forward-addr: ${(lib.head nodes.resolver.config.networking.interfaces.eth1.ipv4.addresses).address} + ''; + "unbound-extra2.conf".text = '' + auth-zone: + name: something.local. + zonefile: ${pkgs.writeText "zone" '' + something.local. IN A 3.4.5.6 + ''} + ''; + }; + }; + + + # plain node that only has network access and doesn't run any part of the + # resolver software locally + client = { lib, nodes, ... }: { + imports = [ common ]; + networking.nameservers = [ + (lib.head nodes.resolver.config.networking.interfaces.eth1.ipv6.addresses).address + (lib.head nodes.resolver.config.networking.interfaces.eth1.ipv4.addresses).address + ]; + networking.interfaces.eth1.ipv4.addresses = [ + { address = "192.168.0.10"; prefixLength = 24; } + ]; + networking.interfaces.eth1.ipv6.addresses = [ + { address = "fd21::10"; prefixLength = 64; } + ]; + }; + }; + + testScript = { nodes, ... }: '' + import typing + import json + + zone = "example.local." + records = [("AAAA", "abcd::eeff"), ("A", "1.2.3.4")] + + + def query( + machine, + host: str, + query_type: str, + query: str, + expected: typing.Optional[str] = None, + args: typing.Optional[typing.List[str]] = None, + ): + """ + Execute a single query and compare the result with expectation + """ + text_args = "" + if args: + text_args = " ".join(args) + + out = machine.succeed( + f"kdig {text_args} {query} {query_type} @{host} +short" + ).strip() + machine.log(f"{host} replied with {out}") + if expected: + assert expected == out, f"Expected `{expected}` but got `{out}`" + + + def test(machine, remotes, /, doh=False, zone=zone, records=records, args=[]): + """ + Run queries for the given remotes on the given machine. + """ + for query_type, expected in records: + for remote in remotes: + query(machine, remote, query_type, zone, expected, args) + query(machine, remote, query_type, zone, expected, ["+tcp"] + args) + if doh: + query( + machine, + remote, + query_type, + zone, + expected, + ["+tcp", "+tls"] + args, + ) + + + client.start() + authoritative.wait_for_unit("unbound.service") + + # verify that we can resolve locally + with subtest("test the authoritative servers local responses"): + test(authoritative, ["::1", "127.0.0.1"]) + + resolver.wait_for_unit("unbound.service") + + # verify that the resolver is able to resolve on all the local protocols + with subtest("test that the resolver resolves on all protocols and transports"): + test(resolver, ["::1", "127.0.0.1"], doh=True) + + resolver.wait_for_unit("multi-user.target") + + with subtest("client should be able to query the resolver"): + test(client, ["${(lib.head nodes.resolver.config.networking.interfaces.eth1.ipv6.addresses).address}", "${(lib.head nodes.resolver.config.networking.interfaces.eth1.ipv4.addresses).address}"], doh=True) + + # discard the client we do not need anymore + client.shutdown() + + local_resolver.wait_for_unit("multi-user.target") + + # link a new config file to /etc/unbound/extra.conf + local_resolver.succeed("ln -s /etc/unbound-extra1.conf /etc/unbound/extra1.conf") + + # reload the server & ensure the forwarding works + with subtest("test that the local resolver resolves on all protocols and transports"): + local_resolver.succeed("systemctl reload unbound") + print(local_resolver.succeed("journalctl -u unbound -n 1000")) + test(local_resolver, ["::1", "127.0.0.1"], args=["+timeout=60"]) + + # link a new config file to /etc/unbound/extra.conf + local_resolver.succeed("ln -sf /etc/unbound-extra2.conf /etc/unbound/extra2.conf") + + # reload the server & ensure the new local zone works + with subtest("test that we can query the new local zone"): + local_resolver.succeed("systemctl reload unbound") + r = [("A", "3.4.5.6")] + test(local_resolver, ["::1", "127.0.0.1"], zone="something.local.", records=r) + ''; + }) From b67cc6298e366aae63a381a895cf21c3b75ed649 Mon Sep 17 00:00:00 2001 From: Andreas Rammhold Date: Fri, 23 Oct 2020 22:14:28 +0200 Subject: [PATCH 07/10] nixos/tests/unbound: add test to verify control sockets work --- nixos/tests/unbound.nix | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/nixos/tests/unbound.nix b/nixos/tests/unbound.nix index bbccfa9c043..9a7a652b405 100644 --- a/nixos/tests/unbound.nix +++ b/nixos/tests/unbound.nix @@ -132,6 +132,12 @@ import ./make-test-python.nix ({ pkgs, lib, ... }: something.local. IN A 3.4.5.6 ''} ''; + "unbound-extra3.conf".text = '' + remote-control: + control-enable: yes + control-interface: /run/unbound/unbound.ctl + ''; + }; }; @@ -243,5 +249,10 @@ import ./make-test-python.nix ({ pkgs, lib, ... }: local_resolver.succeed("systemctl reload unbound") r = [("A", "3.4.5.6")] test(local_resolver, ["::1", "127.0.0.1"], zone="something.local.", records=r) + + with subtest("test that we can enable unbound control sockets on the fly"): + local_resolver.succeed("ln -sf /etc/unbound-extra3.conf /etc/unbound/extra3.conf") + local_resolver.succeed("systemctl reload unbound") + local_resolver.succeed("unbound-control list_forwards") ''; }) From 2aa64e5df5819f7ebeaacfdefb8324736f7f68ba Mon Sep 17 00:00:00 2001 From: Andreas Rammhold Date: Sun, 1 Nov 2020 22:15:42 +0100 Subject: [PATCH 08/10] nixos/unbound: add option to configure the local control socket path This option allows users to specify a local UNIX control socket to "remote control" the daemon. System users, that should be permitted to access the daemon, must be in the `unbound` group in order to access the socket. When a socket path is configured we are also creating the required group. Currently this only supports the UNIX socket mode while unbound actually supports more advanced types. Users are still able to configure more complex scenarios via the `extraConfig` attribute. When this option is set to `null` (the default) it doesn't affect the system configuration at all. The unbound defaults for control sockets apply and no additional groups are created. --- nixos/modules/services/networking/unbound.nix | 36 +++++++++++++ nixos/tests/unbound.nix | 50 +++++++++++++------ 2 files changed, 71 insertions(+), 15 deletions(-) diff --git a/nixos/modules/services/networking/unbound.nix b/nixos/modules/services/networking/unbound.nix index 07e58481a77..2650de4ebeb 100644 --- a/nixos/modules/services/networking/unbound.nix +++ b/nixos/modules/services/networking/unbound.nix @@ -39,6 +39,11 @@ let ${interfaces} ${access} ${trustAnchor} + ${lib.optionalString (cfg.localControlSocketPath != null) '' + remote-control: + control-enable: yes + control-interface: ${cfg.localControlSocketPath} + ''} ${cfg.extraConfig} ${forward} ''; @@ -86,6 +91,28 @@ in description = "Use and update root trust anchor for DNSSEC validation."; }; + localControlSocketPath = mkOption { + default = null; + # FIXME: What is the proper type here so users can specify strings, + # paths and null? + # My guess would be `types.nullOr (types.either types.str types.path)` + # but I haven't verified yet. + type = types.nullOr types.str; + example = "/run/unbound/unbound.ctl"; + description = '' + When not set to null this option defines the path + at which the unbound remote control socket should be created at. The + socket will be owned by the unbound user (unbound) + and group will be nogroup. + + Users that should be permitted to access the socket must be in the + unbound group. + + If this option is null remote control will not be + configured at all. Unbounds default values apply. + ''; + }; + extraConfig = mkOption { default = ""; type = types.lines; @@ -108,6 +135,14 @@ in users.users.unbound = { description = "unbound daemon user"; isSystemUser = true; + group = lib.mkIf (cfg.localControlSocketPath != null) (lib.mkDefault "unbound"); + }; + + # We need a group so that we can give users access to the configured + # control socket. Unbound allows access to the socket only to the unbound + # user and the primary group. + users.groups = lib.mkIf (cfg.localControlSocketPath != null) { + unbound = {}; }; networking.resolvconf.useLocalResolver = mkDefault true; @@ -148,6 +183,7 @@ in ]; User = "unbound"; + Group = lib.mkIf (cfg.localControlSocketPath != null) (lib.mkDefault "unbound"); MemoryDenyWriteExecute = true; NoNewPrivileges = true; diff --git a/nixos/tests/unbound.nix b/nixos/tests/unbound.nix index 9a7a652b405..dc8e5a9d3ed 100644 --- a/nixos/tests/unbound.nix +++ b/nixos/tests/unbound.nix @@ -8,8 +8,13 @@ * running a recursive DNS resolver on a machine in the network awaiting input from clients over TCP/53 & UDP/53 * running a recursive DNS resolver on a machine in the network awaiting input from clients over TCP/853 (DoT) - In the below test setup we are trying to implement all of those use cases - without creating a bazillion machines. + In the below test setup we are trying to implement all of those use cases. + + Another aspect that we cover is access to the local control UNIX socket. It + can optionally be enabled and users can optionally be in a group to gain + access. Users that are not in the group (except for root) should not have + access to that socket. Also, when there is no socket configured, users + shouldn't be able to access the control socket at all. Not even root. */ import ./make-test-python.nix ({ pkgs, lib, ... }: let @@ -96,7 +101,7 @@ import ./make-test-python.nix ({ pkgs, lib, ... }: }; # machine that runs a local unbound that will be reconfigured during test execution - local_resolver = { lib, nodes, ... }: { + local_resolver = { lib, nodes, config, ... }: { imports = [ common ]; networking.interfaces.eth1.ipv4.addresses = lib.mkForce [ { address = "192.168.0.3"; prefixLength = 24; } @@ -113,11 +118,22 @@ import ./make-test-python.nix ({ pkgs, lib, ... }: enable = true; allowedAccess = [ "::1" "127.0.0.0/8" ]; interfaces = [ "::1" "127.0.0.1" ]; + localControlSocketPath = "/run/unbound/unbound.ctl"; extraConfig = '' include: "/etc/unbound/extra*.conf" ''; }; + users.users = { + # user that is permitted to access the unix socket + someuser.extraGroups = [ + config.users.users.unbound.group + ]; + + # user that is not permitted to access the unix socket + unauthorizeduser = {}; + }; + environment.etc = { "unbound-extra1.conf".text = '' forward-zone: @@ -132,12 +148,6 @@ import ./make-test-python.nix ({ pkgs, lib, ... }: something.local. IN A 3.4.5.6 ''} ''; - "unbound-extra3.conf".text = '' - remote-control: - control-enable: yes - control-interface: /run/unbound/unbound.ctl - ''; - }; }; @@ -218,6 +228,10 @@ import ./make-test-python.nix ({ pkgs, lib, ... }: resolver.wait_for_unit("unbound.service") + with subtest("root is unable to use unbounc-control when the socket is not configured"): + resolver.succeed("which unbound-control") # the binary must exist + resolver.fail("unbound-control list_forwards") # the invocation must fail + # verify that the resolver is able to resolve on all the local protocols with subtest("test that the resolver resolves on all protocols and transports"): test(resolver, ["::1", "127.0.0.1"], doh=True) @@ -241,18 +255,24 @@ import ./make-test-python.nix ({ pkgs, lib, ... }: print(local_resolver.succeed("journalctl -u unbound -n 1000")) test(local_resolver, ["::1", "127.0.0.1"], args=["+timeout=60"]) + with subtest("test that we can use the unbound control socket"): + out = local_resolver.succeed( + "sudo -u someuser -- unbound-control list_forwards" + ).strip() + + # Thank you black! Can't really break this line into a readable version. + expected = "example.local. IN forward ${(lib.head nodes.resolver.config.networking.interfaces.eth1.ipv6.addresses).address} ${(lib.head nodes.resolver.config.networking.interfaces.eth1.ipv4.addresses).address}" + assert out == expected, f"Expected `{expected}` but got `{out}` instead." + local_resolver.fail("sudo -u unauthorizeduser -- unbound-control list_forwards") + + # link a new config file to /etc/unbound/extra.conf local_resolver.succeed("ln -sf /etc/unbound-extra2.conf /etc/unbound/extra2.conf") # reload the server & ensure the new local zone works with subtest("test that we can query the new local zone"): - local_resolver.succeed("systemctl reload unbound") + local_resolver.succeed("unbound-control reload") r = [("A", "3.4.5.6")] test(local_resolver, ["::1", "127.0.0.1"], zone="something.local.", records=r) - - with subtest("test that we can enable unbound control sockets on the fly"): - local_resolver.succeed("ln -sf /etc/unbound-extra3.conf /etc/unbound/extra3.conf") - local_resolver.succeed("systemctl reload unbound") - local_resolver.succeed("unbound-control list_forwards") ''; }) From 5c16c31e067573616feda611858f742777af0555 Mon Sep 17 00:00:00 2001 From: Andreas Rammhold Date: Tue, 3 Nov 2020 13:41:00 +0100 Subject: [PATCH 09/10] nixos/unbound: add release notes for the changes that were introduced As part of this patch series a few changes have been made to the unbound serivce the deserve proper documentation. --- nixos/doc/manual/release-notes/rl-2103.xml | 56 ++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/nixos/doc/manual/release-notes/rl-2103.xml b/nixos/doc/manual/release-notes/rl-2103.xml index 85eedfe0ec7..da20c9e8067 100644 --- a/nixos/doc/manual/release-notes/rl-2103.xml +++ b/nixos/doc/manual/release-notes/rl-2103.xml @@ -140,6 +140,62 @@ All services should use or StartLimitIntervalSec in instead. + + + The Unbound DNS resolver service (services.unbound) has been refactored to allow reloading, control sockets and to fix startup ordering issues. + + + + It is now possible to enable a local UNIX control socket for unbound by setting the + option. + + + + Previously we just applied a very minimal set of restrictions and + trusted unbound to properly drop root privs and capabilities. + + + + As of this we are (for the most part) just using the upstream + example unit file for unbound. The main difference is that we start + unbound as unbound user with the required capabilities instead of + letting unbound do the chroot & uid/gid changes. + + + + The upstream unit configuration this is based on is a lot stricter with + all kinds of permissions then our previous variant. It also came with + the default of having the Type set to notify, therefore we are now also + using the unbound-with-systemd package here. Unbound will start up, + read the configuration files and start listening on the configured ports + before systemd will declare the unit active (running). + This will likely help with startup order and the occasional race condition during system + activation where the DNS service is started but not yet ready to answer + queries. Services depending on nss-lookup.target or unbound.service + are now be able to use unbound when those targets have been reached. + + + + Aditionally to the much stricter runtime environmet the + /dev/urandom mount lines we previously had in the code (that would + randomly failed during the stop-phase) have been removed as systemd will take care of those for us. + + + + The preStart script is now only required if we enabled the trust + anchor updates (which are still enabled by default). + + + + Another benefit of the refactoring is that we can now issue reloads via + either pkill -HUP unbound and systemctl reload unbound to reload the + running configuration without taking the daemon offline. A prerequisite + of this was that unbound configuration is available on a well known path + on the file system. We are using the path /etc/unbound/unbound.conf as that is the + default in the CLI tooling which in turn enables us to use + unbound-control without passing a custom configuration location. + + From 5903ea539513c8ec2a1412d88465559b957b6483 Mon Sep 17 00:00:00 2001 From: Andreas Rammhold Date: Tue, 3 Nov 2020 19:21:39 +0100 Subject: [PATCH 10/10] nixos/unbond: unbound should be required for nss-lookup.target Other units depend on nss-lookup.target and expect the DNS resolution to work once that target is reached. The previous version `wants=nss-lookup.target` made this unit require the nss-lookup.target to be reached before this was started. Another change that we can probalby do is drop the before relationship with the nss-lookup.target. That might just be implied with the current version. --- nixos/modules/services/networking/unbound.nix | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/nixos/modules/services/networking/unbound.nix b/nixos/modules/services/networking/unbound.nix index 2650de4ebeb..9a46fa3075f 100644 --- a/nixos/modules/services/networking/unbound.nix +++ b/nixos/modules/services/networking/unbound.nix @@ -154,8 +154,7 @@ in description = "Unbound recursive Domain Name Server"; after = [ "network.target" ]; before = [ "nss-lookup.target" ]; - wants = [ "nss-lookup.target" ]; - wantedBy = [ "multi-user.target" ]; + wantedBy = [ "multi-user.target" "nss-lookup.target" ]; preStart = lib.mkIf cfg.enableRootTrustAnchor '' ${cfg.package}/bin/unbound-anchor -a ${rootTrustAnchorFile} || echo "Root anchor updated!"