From 11f5677752f9b78239214b3064e5a2c3712d71b1 Mon Sep 17 00:00:00 2001

From: Lennart Poettering <lennart@poettering.net>

Date: Wed, 20 Mar 2019 20:19:38 +0100

Subject: [PATCH] core: imply NNP and SUID/SGID restriction for DynamicUser=yes

 service

Let's be safe, rather than sorry. This way DynamicUser=yes services can

neither take benefit of, nor create SUID/SGID binaries.

Given that DynamicUser= is a recent addition only we should be able to

get away with turning this on, even though this is strictly speaking a

binary compatibility breakage.

(cherry picked from commit bf65b7e0c9fc215897b676ab9a7c9d1c688143ba)

Resolves: #1687512

---

 man/systemd.exec.xml | 16 ++++++++++------

 src/core/unit.c      | 10 ++++++++--

 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml

index 45ed1864f8..bdaed68162 100644

--- a/man/systemd.exec.xml

+++ b/man/systemd.exec.xml

@@ -229,7 +229,9 @@

         created by the executed processes is bound to the runtime of the service, and hence the lifetime of the dynamic

         user/group. Since <filename>/tmp</filename> and <filename>/var/tmp</filename> are usually the only

         world-writable directories on a system this ensures that a unit making use of dynamic user/group allocation

-        cannot leave files around after unit termination. Moreover <varname>ProtectSystem=strict</varname> and

+        cannot leave files around after unit termination. Furthermore <varname>NoNewPrivileges=</varname> and

+        <varname>RestrictSUIDSGID=</varname> are implicitly enabled to ensure that processes invoked cannot take benefit

+        or create SUID/SGID files or directories. Moreover <varname>ProtectSystem=strict</varname> and

         <varname>ProtectHome=read-only</varname> are implied, thus prohibiting the service to write to arbitrary file

         system locations. In order to allow the service to write to certain directories, they have to be whitelisted

         using <varname>ReadWritePaths=</varname>, but care must be taken so that UID/GID recycling doesn't create

@@ -357,11 +359,12 @@ CapabilityBoundingSet=~CAP_B CAP_C</programlisting>

         <varname>RestrictAddressFamilies=</varname>, <varname>RestrictNamespaces=</varname>,

         <varname>PrivateDevices=</varname>, <varname>ProtectKernelTunables=</varname>,

         <varname>ProtectKernelModules=</varname>, <varname>MemoryDenyWriteExecute=</varname>,

-        <varname>RestrictRealtime=</varname>, <varname>RestrictSUIDSGID=</varname> or

-        <varname>LockPersonality=</varname> are specified. Note that even if this setting is overridden by

-        them, <command>systemctl show</command> shows the original value of this setting. Also see 

+        <varname>RestrictRealtime=</varname>, <varname>RestrictSUIDSGID=</varname>,

+        <varname>DynamicUser=</varname> or <varname>LockPersonality=</varname> are specified. Note that even

+        if this setting is overridden by them, <command>systemctl show</command> shows the original value of

+        this setting. Also see 

         url="https://www.kernel.org/doc/html/latest/userspace-api/no_new_privs.html">No New Privileges

-        Flag</ulink>.  </para></listitem>

+        Flag</ulink>.</para></listitem>

       </varlistentry>

       <varlistentry>

@@ -1288,7 +1291,8 @@ RestrictNamespaces=~cgroup net</programlisting>

         identity of other users, it is recommended to restrict creation of SUID/SGID files to the few

         programs that actually require them. Note that this restricts marking of any type of file system

         object with these bits, including both regular files and directories (where the SGID is a different

-        meaning than for files, see documentation). Defaults to off.</para></listitem>

+        meaning than for files, see documentation). This option is implied if <varname>DynamicUser=</varname>

+        is enabled. Defaults to off.</para></listitem>

       </varlistentry>

       <varlistentry>

diff --git a/src/core/unit.c b/src/core/unit.c

index 115739f4c6..e1f5e6f7bd 100644

--- a/src/core/unit.c

+++ b/src/core/unit.c

@@ -4161,14 +4161,20 @@ int unit_patch_contexts(Unit *u) {

                                         return -ENOMEM;

                         }

-                        /* If the dynamic user option is on, let's make sure that the unit can't leave its UID/GID

-                         * around in the file system or on IPC objects. Hence enforce a strict sandbox. */

+                        /* If the dynamic user option is on, let's make sure that the unit can't leave its

+                         * UID/GID around in the file system or on IPC objects. Hence enforce a strict

+                         * sandbox. */

                         ec->private_tmp = true;

                         ec->remove_ipc = true;

                         ec->protect_system = PROTECT_SYSTEM_STRICT;

                         if (ec->protect_home == PROTECT_HOME_NO)

                                 ec->protect_home = PROTECT_HOME_READ_ONLY;

+

+                        /* Make sure this service can neither benefit from SUID/SGID binaries nor create

+                         * them. */

+                        ec->no_new_privileges = true;

+                        ec->restrict_suid_sgid = true;

                 }

         }