ddclient nixos module: follow best practice for running daemons

Couple of changes:

 - move home to /var/lib/ddclient so we can enable ProtectSystem=full
 - do not stick binary into systemPackages as it will only run as a daemon
 - run as dedicated user/group
 - document why we cannot run as type=forking (output is swallowed)
 - secure things by running with ProtectSystem and PrivateTmp
 - .pid file goes into /run/ddclient
 - let nix create the home directory instead of handling it manually
 - make the interval configurable

1 files changed, 56 insertions, 44 deletions

diff --git a/nixos/modules/services/networking/ddclient.nix b/nixos/modules/services/networking/ddclient.nix
index 28c96a9baef..9e56545f746 100644
--- a/nixos/modules/services/networking/ddclient.nix
+++ b/nixos/modules/services/networking/ddclient.nix
@@ -1,17 +1,33 @@
 { config, pkgs, lib, ... }:
 
 let
-
-  inherit (lib) mkOption mkIf singleton;
-  inherit (pkgs) ddclient;
-
-  stateDir = "/var/spool/ddclient";
-  ddclientUser = "ddclient";
-  ddclientFlags = "-foreground -file ${config.services.ddclient.configFile}";
-  ddclientPIDFile = "${stateDir}/ddclient.pid";
+  cfg = config.services.ddclient;
+  boolToStr = bool: if bool then "yes" else "no";
+
+  configText = ''
+    # This file can be used as a template for configFile or is automatically generated by Nix options.
+    daemon=${toString cfg.interval}
+    cache=${cfg.homeDir}/ddclient.cache
+    pid=/run/ddclient/ddclient.pid
+    foreground=NO
+    use=${cfg.use}
+    login=${cfg.username}
+    password=${cfg.password}
+    protocol=${cfg.protocol}
+    ${let server = cfg.server; in
+      lib.optionalString (server != "") "server=${server}"}
+    ssl=${boolToStr cfg.ssl}
+    wildcard=YES
+    quiet=${boolToStr cfg.quiet}
+    verbose=${boolToStr cfg.verbose}
+    ${cfg.domain}
+    ${cfg.extraConfig}
+  '';
 
 in
 
+with lib;
+
 {
 
   ###### interface
@@ -28,6 +44,12 @@ in
         '';
       };
 
+      homeDir = mkOption {
+        default = "/var/lib/ddclient";
+        type = str;
+        description = "Home directory for the daemon user.";
+      };
+
       domain = mkOption {
         default = "";
         type = str;
@@ -52,6 +74,12 @@ in
         '';
       };
 
+      interval = mkOption {
+        default = 600;
+        type = int;
+        description = "The interval at which to run the check and update.";
+      };
+
       configFile = mkOption {
         default = "/etc/ddclient.conf";
         type = path;
@@ -126,37 +154,24 @@ in
 
   config = mkIf config.services.ddclient.enable {
 
-    environment.systemPackages = [ ddclient ];
+    users = {
+      extraGroups.ddclient.gid = config.ids.gids.ddclient;
 
-    users.extraUsers = singleton {
-      name = ddclientUser;
-      uid = config.ids.uids.ddclient;
-      description = "ddclient daemon user";
-      home = stateDir;
+      extraUsers.ddclient = {
+        uid = config.ids.uids.ddclient;
+        description = "ddclient daemon user";
+        group = "ddclient";
+        home = cfg.homeDir;
+        createHome = true;
+      };
     };
 
     environment.etc."ddclient.conf" = {
-      enable = config.services.ddclient.configFile == "/etc/ddclient.conf";
+      enable = cfg.configFile == "/etc/ddclient.conf";
       uid = config.ids.uids.ddclient;
+      gid = config.ids.gids.ddclient;
       mode = "0600";
-      text = ''
-        # This file can be used as a template for configFile or is automatically generated by Nix options.
-        daemon=600
-        cache=${stateDir}/ddclient.cache
-        pid=${ddclientPIDFile}
-        use=${config.services.ddclient.use}
-        login=${config.services.ddclient.username}
-        password=${config.services.ddclient.password}
-        protocol=${config.services.ddclient.protocol}
-        ${let server = config.services.ddclient.server; in
-          lib.optionalString (server != "") "server=${server}"}
-        ssl=${if config.services.ddclient.ssl then "yes" else "no"}
-        wildcard=YES
-        quiet=${if config.services.ddclient.quiet then "yes" else "no"}
-        verbose=${if config.services.ddclient.verbose then "yes" else "no"}
-        ${config.services.ddclient.domain}
-        ${config.services.ddclient.extraConfig}
-      '';
+      text = configText;
     };
 
     systemd.services.ddclient = {
@@ -166,17 +181,14 @@ in
       restartTriggers = [ config.environment.etc."ddclient.conf".source ];
 
       serviceConfig = {
-        # Uncomment this if too many problems occur:
-        # Type = "forking";
-        User = ddclientUser;
-        Group = "nogroup"; #TODO get this to work
-        PermissionsStartOnly = "true";
-        PIDFile = ddclientPIDFile;
-        ExecStartPre = ''
-          ${pkgs.stdenv.shell} -c "${pkgs.coreutils}/bin/mkdir -m 0755 -p ${stateDir} && ${pkgs.coreutils}/bin/chown ${ddclientUser} ${stateDir}"
-        '';
-        ExecStart = "${ddclient}/bin/ddclient ${ddclientFlags}";
-        #ExecStartPost = "${pkgs.coreutils}/bin/rm -r ${stateDir}"; # Should we have this?
+        RuntimeDirectory = "ddclient";
+        # we cannot run in forking mode as it swallows all the program output
+        Type = "simple";
+        User = "ddclient";
+        Group = "ddclient";
+        ExecStart = "${lib.getBin pkgs.ddclient}/bin/ddclient -foreground -file ${cfg.configFile}";
+        ProtectSystem = "full";
+        PrivateTmp = true;
       };
     };
   };