daandemeyer / rpms / systemd

Forked from rpms/systemd 2 years ago
Clone
b9a53a
From 11f5677752f9b78239214b3064e5a2c3712d71b1 Mon Sep 17 00:00:00 2001
b9a53a
From: Lennart Poettering <lennart@poettering.net>
b9a53a
Date: Wed, 20 Mar 2019 20:19:38 +0100
b9a53a
Subject: [PATCH] core: imply NNP and SUID/SGID restriction for DynamicUser=yes
b9a53a
 service
b9a53a
b9a53a
Let's be safe, rather than sorry. This way DynamicUser=yes services can
b9a53a
neither take benefit of, nor create SUID/SGID binaries.
b9a53a
b9a53a
Given that DynamicUser= is a recent addition only we should be able to
b9a53a
get away with turning this on, even though this is strictly speaking a
b9a53a
binary compatibility breakage.
b9a53a
b9a53a
(cherry picked from commit bf65b7e0c9fc215897b676ab9a7c9d1c688143ba)
b9a53a
Resolves: #1687512
b9a53a
---
b9a53a
 man/systemd.exec.xml | 16 ++++++++++------
b9a53a
 src/core/unit.c      | 10 ++++++++--
b9a53a
 2 files changed, 18 insertions(+), 8 deletions(-)
b9a53a
b9a53a
diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
b9a53a
index 45ed1864f8..bdaed68162 100644
b9a53a
--- a/man/systemd.exec.xml
b9a53a
+++ b/man/systemd.exec.xml
b9a53a
@@ -229,7 +229,9 @@
b9a53a
         created by the executed processes is bound to the runtime of the service, and hence the lifetime of the dynamic
b9a53a
         user/group. Since <filename>/tmp</filename> and <filename>/var/tmp</filename> are usually the only
b9a53a
         world-writable directories on a system this ensures that a unit making use of dynamic user/group allocation
b9a53a
-        cannot leave files around after unit termination. Moreover <varname>ProtectSystem=strict</varname> and
b9a53a
+        cannot leave files around after unit termination. Furthermore <varname>NoNewPrivileges=</varname> and
b9a53a
+        <varname>RestrictSUIDSGID=</varname> are implicitly enabled to ensure that processes invoked cannot take benefit
b9a53a
+        or create SUID/SGID files or directories. Moreover <varname>ProtectSystem=strict</varname> and
b9a53a
         <varname>ProtectHome=read-only</varname> are implied, thus prohibiting the service to write to arbitrary file
b9a53a
         system locations. In order to allow the service to write to certain directories, they have to be whitelisted
b9a53a
         using <varname>ReadWritePaths=</varname>, but care must be taken so that UID/GID recycling doesn't create
b9a53a
@@ -357,11 +359,12 @@ CapabilityBoundingSet=~CAP_B CAP_C</programlisting>
b9a53a
         <varname>RestrictAddressFamilies=</varname>, <varname>RestrictNamespaces=</varname>,
b9a53a
         <varname>PrivateDevices=</varname>, <varname>ProtectKernelTunables=</varname>,
b9a53a
         <varname>ProtectKernelModules=</varname>, <varname>MemoryDenyWriteExecute=</varname>,
b9a53a
-        <varname>RestrictRealtime=</varname>, <varname>RestrictSUIDSGID=</varname> or
b9a53a
-        <varname>LockPersonality=</varname> are specified. Note that even if this setting is overridden by
b9a53a
-        them, <command>systemctl show</command> shows the original value of this setting. Also see 
b9a53a
+        <varname>RestrictRealtime=</varname>, <varname>RestrictSUIDSGID=</varname>,
b9a53a
+        <varname>DynamicUser=</varname> or <varname>LockPersonality=</varname> are specified. Note that even
b9a53a
+        if this setting is overridden by them, <command>systemctl show</command> shows the original value of
b9a53a
+        this setting. Also see 
b9a53a
         url="https://www.kernel.org/doc/html/latest/userspace-api/no_new_privs.html">No New Privileges
b9a53a
-        Flag</ulink>.  </para></listitem>
b9a53a
+        Flag</ulink>.</para></listitem>
b9a53a
       </varlistentry>
b9a53a
 
b9a53a
       <varlistentry>
b9a53a
@@ -1288,7 +1291,8 @@ RestrictNamespaces=~cgroup net</programlisting>
b9a53a
         identity of other users, it is recommended to restrict creation of SUID/SGID files to the few
b9a53a
         programs that actually require them. Note that this restricts marking of any type of file system
b9a53a
         object with these bits, including both regular files and directories (where the SGID is a different
b9a53a
-        meaning than for files, see documentation). Defaults to off.</para></listitem>
b9a53a
+        meaning than for files, see documentation). This option is implied if <varname>DynamicUser=</varname>
b9a53a
+        is enabled. Defaults to off.</para></listitem>
b9a53a
       </varlistentry>
b9a53a
 
b9a53a
       <varlistentry>
b9a53a
diff --git a/src/core/unit.c b/src/core/unit.c
b9a53a
index 115739f4c6..e1f5e6f7bd 100644
b9a53a
--- a/src/core/unit.c
b9a53a
+++ b/src/core/unit.c
b9a53a
@@ -4161,14 +4161,20 @@ int unit_patch_contexts(Unit *u) {
b9a53a
                                         return -ENOMEM;
b9a53a
                         }
b9a53a
 
b9a53a
-                        /* If the dynamic user option is on, let's make sure that the unit can't leave its UID/GID
b9a53a
-                         * around in the file system or on IPC objects. Hence enforce a strict sandbox. */
b9a53a
+                        /* If the dynamic user option is on, let's make sure that the unit can't leave its
b9a53a
+                         * UID/GID around in the file system or on IPC objects. Hence enforce a strict
b9a53a
+                         * sandbox. */
b9a53a
 
b9a53a
                         ec->private_tmp = true;
b9a53a
                         ec->remove_ipc = true;
b9a53a
                         ec->protect_system = PROTECT_SYSTEM_STRICT;
b9a53a
                         if (ec->protect_home == PROTECT_HOME_NO)
b9a53a
                                 ec->protect_home = PROTECT_HOME_READ_ONLY;
b9a53a
+
b9a53a
+                        /* Make sure this service can neither benefit from SUID/SGID binaries nor create
b9a53a
+                         * them. */
b9a53a
+                        ec->no_new_privileges = true;
b9a53a
+                        ec->restrict_suid_sgid = true;
b9a53a
                 }
b9a53a
         }
b9a53a