Add stubby resolver daemon service module #38667

baroncharlus · 2018-04-10T01:59:24Z

This change implements stubby, the DNS-over-TLS stub resolver daemon.
The package itself is maintained by @leenaars.

The motivation for this change was the desire to use stubby's
DNS-over-TLS functionality in tandem with unbound, which requires
passing certain configuration parameters. This module implements those
config parameters by exposing them for use in configuration.nix.

This is a reopening of PR #38586, which was based on the incorrect branch.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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 nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

This change implements stubby, the DNS-over-TLS stub resolver daemon. The motivation for this change was the desire to use stubby's DNS-over-TLS funcitonality in tandem with unbound, which requires passing certain configuration parameters. This module implements those config parameters by exposing them for use in configuration.nix.

re-merging the module list to remove unecessary changes.

baroncharlus · 2018-04-10T02:01:27Z

nixos/modules/services/networking/stubby.nix

+        '';
+      };
+
+      enableNetBindService = mkOption {


instead of trying to parse the listenAddresses, which required the very inefficient splitString, I have simply provided the option to disable this capability if the user knows they are going to bind to ports > 1024.

baroncharlus · 2018-04-10T02:28:49Z

nixos/modules/services/networking/stubby.nix

+
+      serviceConfig = {
+        AmbientCapabilities = "${optionalString cfg.enableNetBindService "CAP_NET_BIND_SERVICE"}";
+        CapabilitiesBoundingSet = "${optionalString cfg.enableNetBindService "CAP_NET_BIND_SERVICE"}";


this part I'm not 100% sure about and would welcome input. obviously, having AmbientCapabilities= set to nothing (instead of just being altogether absent in the false case) isn't the prettiest. however, it does seem to achieve the desired effect (for now, anyway).

Eh, I think providing an option for this is a bit too much. Should be fine to just set the values by default, if the user wants to override it he can still do it with something like systemd.services.stubby.serviceConfig.AmbientCapabilities = mkForce "";. I mean it's a DNS module after all, it should be expected to be able to bind to privileged ports.

This change removes the unecessary flag for toggling the capabilities which allows the daemon to bind to low ports.

Adding the option to turn on debug logging.

wizeman · 2018-04-13T18:32:30Z

Shouldn't this service be ordered before nss-lookup.target, like the unbound and dnscrypt-proxy services?

wizeman · 2018-04-13T18:35:50Z

Also, it would be useful to mention that the idleTimeout option is in ms units.

BTW, thanks for contributing this service, I am already using it successfully (with unbound as a caching layer)!

baroncharlus · 2018-04-18T02:12:48Z

@wizeman both good points. very pleased to hear someone found it useful!

Improving docs to note that idleTimeout is expressed in ms. Adding the nss-lookup `before' target to the systemd service definition.

infinisil · 2018-04-20T16:30:56Z

nixos/modules/services/networking/stubby.nix

+  fallbacks = concatMapStringsSep "\n  " (x: "- ${x}") cfg.fallbackProtocols;
+  listeners = concatMapStringsSep "\n  " (x: "- ${x}") cfg.listenAddresses;
+  # By default, the recursive resolvers maintained by the getdns
+  # project itself are enabled. More information about both getdns's servers, # as well as third party options for upstream resolvers, can be found here:


Missed a line break here

infinisil · 2018-04-20T16:33:16Z

nixos/modules/services/networking/stubby.nix

+
+      fallbackProtocols = mkOption {
+        default = [ "GETDNS_TRANSPORT_TLS" ];
+        type = types.listOf types.str;


You could use types.enum [ "GETDNS_TRANSPORT_TLS" "GETDNS_TRANSPORT_TCP" "GETDNS_TRANSPORT_UDP" ] here

infinisil · 2018-04-20T16:35:02Z

nixos/modules/services/networking/stubby.nix

+      };
+
+      authenticationMode = mkOption {
+        default = "GETDNS_AUTHENTICATION_REQUIRED";


You could change this to a boolean option authenticationRequired when there's only the values required/none. But only if you're sure that other options won't be added in the future (i suspect that won't be the case).

And if you don't want to make it a boolean, it can be an enum like above instead.

wizeman · 2018-04-20T16:42:35Z

BTW, I forgot to mention that I'm getting the following message in the systemd journal:

Apr 20 15:26:09 wizylap systemd[1]: /nix/store/dh0mk6yh4nhnmh9j3yqimvwcch5h9sp4-unit-stubby.service/stubby.service:15: Unknown lvalue 'CapabilitiesBoundingSet' in section 'Service'

baroncharlus · 2018-04-21T20:53:11Z

@wizeman whups! You're right. that should be CapabilityBindingSet, not Capabilities. Thanks!

This change restricts fallbackProtocol and authenticationMode to accept only valid options instead of any list or str types (respectively). This change also fixes typo in the CapabilityBoundingSet systemd setting.

Cleaning up docs, adding literal tags to settings, and removing whitespace.

infinisil · 2018-04-21T21:53:54Z

nixos/modules/services/networking/stubby.nix

+  listeners = concatMapStringsSep "\n  " (x: "- ${x}") cfg.listenAddresses;
+
+  # By default, the recursive resolvers maintained by the getdns
+  # project itself are enabled. More information about both getdns's servers, # as well as third party options for upstream resolvers, can be found here:


I meant a line break in this comment line, there's a # in the middle of it.

infinisil · 2018-04-22T00:43:11Z

nixos/modules/services/networking/stubby.nix

+
+  # By default, the recursive resolvers maintained by the getdns
+  # project itself are enabled. More information about both getdns's servers,
+  # # as well as third party options for upstream resolvers, can be found here:


Now there's two # here :)

oh dear... what are we going to do with me

infinisil

Looks perfect to me now, well done! I have also tested this locally and it worked without problems.

baroncharlus · 2018-04-22T14:46:43Z

🎉🎉! thanks for your help, @infinisil and @wizeman.

jfrankenau · 2018-05-14T07:38:32Z

I've been running this since a few days together with systemd-resolved and it works fine. I guess you'll just have to squash the commits to have this PR merged.

@baroncharlus, have you had a look at the upstream systemd service? I wonder if we should set the working directory as well. This seems to be of interest for zero configuration DNSSEC. (See also appdata_dir in Stubby's config.)

baroncharlus added 2 commits April 9, 2018 20:45

networking/stubby.nix: merging back module list

a17967e

re-merging the module list to remove unecessary changes.

baroncharlus commented Apr 10, 2018

View reviewed changes

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Apr 10, 2018

baroncharlus commented Apr 10, 2018

View reviewed changes

baroncharlus added 2 commits April 10, 2018 07:26

networking/stubby.nix: removing unecessary capabilities flag

4cc3276

This change removes the unecessary flag for toggling the capabilities which allows the daemon to bind to low ports.

networking/stubby.nix: adding debug level logging bool

dbe6db4

Adding the option to turn on debug logging.

networking/stubby.nix: clarifying idleTimeout and adding systemd target

b99e337

Improving docs to note that idleTimeout is expressed in ms. Adding the nss-lookup `before' target to the systemd service definition.

infinisil requested changes Apr 20, 2018

View reviewed changes

baroncharlus added 2 commits April 21, 2018 16:15

networking/stubby.nix: Restrict options with types.enum

e7df6f0

This change restricts fallbackProtocol and authenticationMode to accept only valid options instead of any list or str types (respectively). This change also fixes typo in the CapabilityBoundingSet systemd setting.

networking/stubby.nix: cleaning up documentation

0780475

Cleaning up docs, adding literal tags to settings, and removing whitespace.

infinisil reviewed Apr 21, 2018

View reviewed changes

networking/stubby.nix: fixing missing linebreak in comments

ecbac60

infinisil reviewed Apr 22, 2018

View reviewed changes

networking/stubby.nix: cleaning errant comments

16fbe8a

infinisil approved these changes Apr 22, 2018

View reviewed changes

infinisil mentioned this pull request May 14, 2018

stubby: 0.2.2 -> 0.2.3 #40465

Merged

xeji merged commit 380cdd8 into NixOS:master May 16, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add stubby resolver daemon service module #38667

Add stubby resolver daemon service module #38667

baroncharlus commented Apr 10, 2018 •

edited

baroncharlus Apr 10, 2018

baroncharlus Apr 10, 2018 •

edited

infinisil Apr 10, 2018 •

edited

wizeman commented Apr 13, 2018 •

edited

wizeman commented Apr 13, 2018

baroncharlus commented Apr 18, 2018

infinisil Apr 20, 2018

infinisil Apr 20, 2018

infinisil Apr 20, 2018

infinisil Apr 20, 2018

wizeman commented Apr 20, 2018

baroncharlus commented Apr 21, 2018

infinisil Apr 21, 2018 •

edited

infinisil Apr 22, 2018 •

edited

baroncharlus Apr 22, 2018

infinisil left a comment

baroncharlus commented Apr 22, 2018

jfrankenau commented May 14, 2018 •

edited

Add stubby resolver daemon service module #38667

Add stubby resolver daemon service module #38667

Conversation

baroncharlus commented Apr 10, 2018 • edited

Things done

baroncharlus Apr 10, 2018

Choose a reason for hiding this comment

baroncharlus Apr 10, 2018 • edited

Choose a reason for hiding this comment

infinisil Apr 10, 2018 • edited

Choose a reason for hiding this comment

wizeman commented Apr 13, 2018 • edited

wizeman commented Apr 13, 2018

baroncharlus commented Apr 18, 2018

infinisil Apr 20, 2018

Choose a reason for hiding this comment

infinisil Apr 20, 2018

Choose a reason for hiding this comment

infinisil Apr 20, 2018

Choose a reason for hiding this comment

infinisil Apr 20, 2018

Choose a reason for hiding this comment

wizeman commented Apr 20, 2018

baroncharlus commented Apr 21, 2018

infinisil Apr 21, 2018 • edited

Choose a reason for hiding this comment

infinisil Apr 22, 2018 • edited

Choose a reason for hiding this comment

baroncharlus Apr 22, 2018

Choose a reason for hiding this comment

infinisil left a comment

Choose a reason for hiding this comment

baroncharlus commented Apr 22, 2018

jfrankenau commented May 14, 2018 • edited

baroncharlus commented Apr 10, 2018 •

edited

baroncharlus Apr 10, 2018 •

edited

infinisil Apr 10, 2018 •

edited

wizeman commented Apr 13, 2018 •

edited

infinisil Apr 21, 2018 •

edited

infinisil Apr 22, 2018 •

edited

jfrankenau commented May 14, 2018 •

edited