From 101d12296da24436eaf2de7514c4693d11b86851 Mon Sep 17 00:00:00 2001 From: Martin Weinelt Date: Mon, 14 Oct 2024 18:46:04 +0200 Subject: [PATCH 1/3] coturn: make setgroups conditional on privdrop codepath Make coturn only call setgroups, when it actually needs to privdrop. In the nixos module we already run coturn as an unprivileged user, which means we don't need to provide access to the setgroups syscall in the first place. --- pkgs/servers/coturn/default.nix | 4 ++ .../dont-call-setgroups-unconditionally.patch | 46 +++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 pkgs/servers/coturn/dont-call-setgroups-unconditionally.patch diff --git a/pkgs/servers/coturn/default.nix b/pkgs/servers/coturn/default.nix index 061ea520731b..4014d566068e 100644 --- a/pkgs/servers/coturn/default.nix +++ b/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 diff --git a/pkgs/servers/coturn/dont-call-setgroups-unconditionally.patch b/pkgs/servers/coturn/dont-call-setgroups-unconditionally.patch new file mode 100644 index 000000000000..6b7780cbee84 --- /dev/null +++ b/pkgs/servers/coturn/dont-call-setgroups-unconditionally.patch @@ -0,0 +1,46 @@ +From 1b5da9c7c5423eed7a567a02e66c244705116724 Mon Sep 17 00:00:00 2001 +From: networkException +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); From 6d9089c67dec1c62750f21b8fb2862e0e601f189 Mon Sep 17 00:00:00 2001 From: Martin Weinelt Date: Mon, 14 Oct 2024 03:31:09 +0200 Subject: [PATCH 2/3] nixos/coturn: set up sandboxing --- nixos/modules/services/networking/coturn.nix | 71 +++++++++++++++----- nixos/tests/coturn.nix | 2 + 2 files changed, 58 insertions(+), 15 deletions(-) diff --git a/nixos/modules/services/networking/coturn.nix b/nixos/modules/services/networking/coturn.nix index 40c157d1006e..215d679c09dd 100644 --- a/nixos/modules/services/networking/coturn.nix +++ b/nixos/modules/services/networking/coturn.nix @@ -1,4 +1,4 @@ -{ config, lib, pkgs, ... }: +{ config, lib, pkgs, utils, ... }: let cfg = config.services.coturn; pidfile = "/run/turnserver/turnserver.pid"; @@ -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 - -" - ]; }])); } diff --git a/nixos/tests/coturn.nix b/nixos/tests/coturn.nix index b44bf8d06e39..b3c96dba35f8 100644 --- a/nixos/tests/coturn.nix +++ b/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]) ''; }) From 72dd22a02df88371c04216781acff3d55d2c2c03 Mon Sep 17 00:00:00 2001 From: Martin Weinelt Date: Mon, 14 Oct 2024 03:37:03 +0200 Subject: [PATCH 3/3] nixos/coturn: reindent, unclutter Make the module slightly easier to browse. --- nixos/modules/services/networking/coturn.nix | 70 ++++++++++---------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/nixos/modules/services/networking/coturn.nix b/nixos/modules/services/networking/coturn.nix index 215d679c09dd..ab8806dc03a1 100644 --- a/nixos/modules/services/networking/coturn.nix +++ b/nixos/modules/services/networking/coturn.nix @@ -3,39 +3,39 @@ 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"; @@ -402,5 +402,5 @@ in { UMask = "0077"; }; }; - }])); + }]); }