nixos/coturn: set up sandboxing (#348396)

nixos/modules/services/networking/coturn.nix

@@ -1,41 +1,41 @@
-{ config, lib, pkgs, ... }:
+{ config, lib, pkgs, utils, ... }:
 let
   cfg = config.services.coturn;
   pidfile = "/run/turnserver/turnserver.pid";
   configFile = pkgs.writeText "turnserver.conf" ''
-listening-port=${toString cfg.listening-port}
-tls-listening-port=${toString cfg.tls-listening-port}
-alt-listening-port=${toString cfg.alt-listening-port}
-alt-tls-listening-port=${toString cfg.alt-tls-listening-port}
-${lib.concatStringsSep "\n" (map (x: "listening-ip=${x}") cfg.listening-ips)}
-${lib.concatStringsSep "\n" (map (x: "relay-ip=${x}") cfg.relay-ips)}
-min-port=${toString cfg.min-port}
-max-port=${toString cfg.max-port}
-${lib.optionalString cfg.lt-cred-mech "lt-cred-mech"}
-${lib.optionalString cfg.no-auth "no-auth"}
-${lib.optionalString cfg.use-auth-secret "use-auth-secret"}
-${lib.optionalString (cfg.static-auth-secret != null) ("static-auth-secret=${cfg.static-auth-secret}")}
-${lib.optionalString (cfg.static-auth-secret-file != null) ("static-auth-secret=#static-auth-secret#")}
-realm=${cfg.realm}
-${lib.optionalString cfg.no-udp "no-udp"}
-${lib.optionalString cfg.no-tcp "no-tcp"}
-${lib.optionalString cfg.no-tls "no-tls"}
-${lib.optionalString cfg.no-dtls "no-dtls"}
-${lib.optionalString cfg.no-udp-relay "no-udp-relay"}
-${lib.optionalString cfg.no-tcp-relay "no-tcp-relay"}
-${lib.optionalString (cfg.cert != null) "cert=${cfg.cert}"}
-${lib.optionalString (cfg.pkey != null) "pkey=${cfg.pkey}"}
-${lib.optionalString (cfg.dh-file != null) ("dh-file=${cfg.dh-file}")}
-no-stdout-log
-syslog
-pidfile=${pidfile}
-${lib.optionalString cfg.secure-stun "secure-stun"}
-${lib.optionalString cfg.no-cli "no-cli"}
-cli-ip=${cfg.cli-ip}
-cli-port=${toString cfg.cli-port}
-${lib.optionalString (cfg.cli-password != null) ("cli-password=${cfg.cli-password}")}
-${cfg.extraConfig}
-'';
+    listening-port=${toString cfg.listening-port}
+    tls-listening-port=${toString cfg.tls-listening-port}
+    alt-listening-port=${toString cfg.alt-listening-port}
+    alt-tls-listening-port=${toString cfg.alt-tls-listening-port}
+    ${lib.concatStringsSep "\n" (map (x: "listening-ip=${x}") cfg.listening-ips)}
+    ${lib.concatStringsSep "\n" (map (x: "relay-ip=${x}") cfg.relay-ips)}
+    min-port=${toString cfg.min-port}
+    max-port=${toString cfg.max-port}
+    ${lib.optionalString cfg.lt-cred-mech "lt-cred-mech"}
+    ${lib.optionalString cfg.no-auth "no-auth"}
+    ${lib.optionalString cfg.use-auth-secret "use-auth-secret"}
+    ${lib.optionalString (cfg.static-auth-secret != null) "static-auth-secret=${cfg.static-auth-secret}"}
+    ${lib.optionalString (cfg.static-auth-secret-file != null) "static-auth-secret=#static-auth-secret#"}
+    realm=${cfg.realm}
+    ${lib.optionalString cfg.no-udp "no-udp"}
+    ${lib.optionalString cfg.no-tcp "no-tcp"}
+    ${lib.optionalString cfg.no-tls "no-tls"}
+    ${lib.optionalString cfg.no-dtls "no-dtls"}
+    ${lib.optionalString cfg.no-udp-relay "no-udp-relay"}
+    ${lib.optionalString cfg.no-tcp-relay "no-tcp-relay"}
+    ${lib.optionalString (cfg.cert != null) "cert=${cfg.cert}"}
+    ${lib.optionalString (cfg.pkey != null) "pkey=${cfg.pkey}"}
+    ${lib.optionalString (cfg.dh-file != null) "dh-file=${cfg.dh-file}"}
+    no-stdout-log
+    syslog
+    pidfile=${pidfile}
+    ${lib.optionalString cfg.secure-stun "secure-stun"}
+    ${lib.optionalString cfg.no-cli "no-cli"}
+    cli-ip=${cfg.cli-ip}
+    cli-port=${toString cfg.cli-port}
+    ${lib.optionalString (cfg.cli-password != null) "cli-password=${cfg.cli-password}"}
+    ${cfg.extraConfig}
+  '';
 in {
   options = {
     services.coturn = {
@@ -301,7 +301,7 @@ in {
     };
   };
 
-  config = lib.mkIf cfg.enable (lib.mkMerge ([
+  config = lib.mkIf cfg.enable (lib.mkMerge [
     { assertions = [
       { assertion = cfg.static-auth-secret != null -> cfg.static-auth-secret-file == null ;
         message = "static-auth-secret and static-auth-secret-file cannot be set at the same time";
@@ -341,25 +341,66 @@ in {
           '' }
           chmod 640 ${runConfig}
         '';
-        serviceConfig = {
+        serviceConfig = rec {
           Type = "simple";
-          ExecStart = "${pkgs.coturn}/bin/turnserver -c ${runConfig}";
-          RuntimeDirectory = "turnserver";
+          ExecStart = utils.escapeSystemdExecArgs [
+            (lib.getExe' pkgs.coturn "turnserver")
+            "-c"
+            runConfig
+          ];
           User = "turnserver";
           Group = "turnserver";
-          AmbientCapabilities =
-            lib.mkIf (
-              cfg.listening-port < 1024 ||
-              cfg.alt-listening-port < 1024 ||
-              cfg.tls-listening-port < 1024 ||
-              cfg.alt-tls-listening-port < 1024 ||
-              cfg.min-port < 1024
-            ) "cap_net_bind_service";
+          RuntimeDirectory = [
+            "coturn"
+            "turnserver"
+          ];
+          RuntimeDirectoryMode = "0700";
           Restart = "on-abort";
+
+          # Hardening
+          AmbientCapabilities = if
+            cfg.listening-port < 1024 ||
+            cfg.alt-listening-port < 1024 ||
+            cfg.tls-listening-port < 1024 ||
+            cfg.alt-tls-listening-port < 1024 ||
+            cfg.min-port < 1024
+            then [ "CAP_NET_BIND_SERVICE" ] else [ "" ];
+          CapabilityBoundingSet = AmbientCapabilities;
+          DevicePolicy = "closed";
+          LockPersonality = true;
+          MemoryDenyWriteExecute = true;
+          NoNewPrivileges = true;
+          PrivateDevices = true;
+          PrivateTmp = true;
+          PrivateUsers = true;
+          ProcSubset = "pid";
+          ProtectClock = true;
+          ProtectControlGroups = true;
+          ProtectHome = true;
+          ProtectHostname = true;
+          ProtectKernelLogs = true;
+          ProtectKernelModules = true;
+          ProtectKernelTunables = true;
+          ProtectProc = "invisible";
+          ProtectSystem = "strict";
+          RemoveIPC = true;
+          RestrictAddressFamilies = [
+            "AF_INET"
+            "AF_INET6"
+          ] ++ lib.optionals (cfg.listening-ips == [ ]) [
+            # only used for interface discovery when no listening ips are configured
+            "AF_NETLINK"
+          ];
+          RestrictNamespaces = true;
+          RestrictRealtime = true;
+          RestrictSUIDSGID = true;
+          SystemCallArchitectures = "native";
+          SystemCallFilter = [
+            "@system-service"
+            "~@privileged @resources"
+          ];
+          UMask = "0077";
         };
       };
-    systemd.tmpfiles.rules = [
-      "d  /run/coturn 0700 turnserver turnserver - -"
-    ];
-  }]));
+  }]);
 }

nixos/tests/coturn.nix

@@ -30,5 +30,7 @@ import ./make-test-python.nix ({ pkgs, ... }: {
           secretsfile.fail("${pkgs.coturn}/bin/turnutils_uclient -W some-very-secret-string 127.0.0.1 -DgX -e 127.0.0.1 -n 1 -c -y")
           # allowed-peer-ip, should succeed:
           secretsfile.succeed("${pkgs.coturn}/bin/turnutils_uclient -W some-very-secret-string 192.168.1.2 -DgX -e 192.168.1.2 -n 1 -c -y")
+
+      default.log(default.execute("systemd-analyze security coturn.service | grep -v '✓'")[1])
     '';
 })

pkgs/servers/coturn/default.nix

@@ -37,6 +37,10 @@ stdenv.mkDerivation rec {
 
   patches = [
     ./pure-configure.patch
+
+    # Don't call setgroups unconditionally in mainrelay
+    # https://github.com/coturn/coturn/pull/1508
+    ./dont-call-setgroups-unconditionally.patch
   ];
 
   # Workaround build failure on -fno-common toolchains like upstream

pkgs/servers/coturn/dont-call-setgroups-unconditionally.patch

@@ -0,0 +1,46 @@
+From 1b5da9c7c5423eed7a567a02e66c244705116724 Mon Sep 17 00:00:00 2001
+From: networkException <git@nwex.de>
+Date: Thu, 30 May 2024 02:07:04 +0200
+Subject: [PATCH] Don't call `setgroups` unconditionally in mainrelay
+
+This patch moves the call to `setgroups` from the beginning of the
+`drop_priviliges` function to branch in which `setuid` is actually
+called. This still fulfills the intention of
+acbf7e15c9290e0891a6b6b5ce6e81bbaa77ce5a, initially introducting
+the call to `setgroups`:
+
+> Fix related to POS36-C and rpmlint error
+> "missing-call-to-setgroups-before-setuid".
+
+As per this intention is is not required to call `setgroups`
+otherwise, reducing the more exotic (as in not part of POSIX and
+considered priviliged by systemd) system calls coturn needs to make
+at startup.
+---
+ src/apps/relay/mainrelay.c | 6 +++++-
+ 1 file changed, 5 insertions(+), 1 deletion(-)
+
+diff --git a/src/apps/relay/mainrelay.c b/src/apps/relay/mainrelay.c
+index cf370ec8a..56eaf82d0 100644
+--- a/src/apps/relay/mainrelay.c
++++ b/src/apps/relay/mainrelay.c
+@@ -2913,7 +2913,6 @@ static void drop_privileges(void) {
+ #if defined(WINDOWS)
+   // TODO: implement it!!!
+ #else
+-  setgroups(0, NULL);
+   if (procgroupid_set) {
+     if (getgid() != procgroupid) {
+       if (setgid(procgroupid) != 0) {
+@@ -2929,6 +2928,11 @@ static void drop_privileges(void) {
+ 
+   if (procuserid_set) {
+     if (procuserid != getuid()) {
++      if (setgroups(0, NULL) != 0) {
++        perror("setgroups: Unable drop supplementary groups");
++        exit(-1);
++      }
++
+       if (setuid(procuserid) != 0) {
+         perror("setuid: Unable to change user privileges");
+         exit(-1);