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.

diff --git a/NEWS b/NEWS
index e0829002b91a101215efefdccab1d4ee60e93638..603aa635504125744e177a500ea7cfb0951050bb 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -201,6 +201,21 @@ CHANGES WITH 242 in spe:
           done anymore, and instead calling `systemctl preset-all` is
           recommended after the first installation of systemd.
 
+        * A new boolean sandboxing option RestrictSUIDSGID= has been added that
+          is built on seccomp. When turned on creation of SUID/SGID files is
+          prohibited.
+
+        * The NoNewPrivileges= and the new RestrictSUIDSGID= options are now
+          implied if DynamicUser= is turned on for a service. This hardens
+          these services, so that they neither can benefit from nor create
+          SUID/SGID executables. This is a minor compatibility breakage, given
+          that when DynamicUser= was first introduced SUID/SGID behaviour was
+          unaffected. However, the security benefit of these two options is
+          substantial, and the setting is still relatively new, hence we opted
+          to make it mandatory for services with dynamic users.
+
+        …
+
 CHANGES WITH 241:
 
         * The default locale can now be configured at compile time. Otherwise,

diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
index 46f2d856e0fe88055b28bcb5d1ce2b75fd7f532b..688147ea329423988fc0fdbe6aee00a01a526f3c 100644 (file)
--- a/man/systemd.exec.xml
+++ b/man/systemd.exec.xml
@@ -252,7 +252,9 @@
         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
+        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
@@ -388,11 +390,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 <ulink
+        <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 <ulink
         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>
@@ -1406,7 +1409,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 35c268cd3b454849441cb20b17f315a827a1ed71..2cde494a7ecf67562128ef2af007bb38079e3724 100644 (file)
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -4088,14 +4088,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;
                 }
         }
 

diff --git a/units/systemd-journal-gatewayd.service.in b/units/systemd-journal-gatewayd.service.in
index 0f16ae4ccba1927cb85cb95b0acf7595d5b82d0c..50f774512b8c7d1fced3fb670c4e88dc634ec1a6 100644 (file)
--- a/units/systemd-journal-gatewayd.service.in
+++ b/units/systemd-journal-gatewayd.service.in
@@ -17,7 +17,6 @@ DynamicUser=yes
 ExecStart=@rootlibexecdir@/systemd-journal-gatewayd
 LockPersonality=yes
 MemoryDenyWriteExecute=yes
-NoNewPrivileges=yes
 PrivateDevices=yes
 PrivateNetwork=yes
 ProtectControlGroups=yes

diff --git a/units/systemd-journal-upload.service.in b/units/systemd-journal-upload.service.in
index 10e4d657d3ac319f4c9c0b716f5bfce403dd4281..e3800473ec5912ac1ade53e0cd93cf67c25df966 100644 (file)
--- a/units/systemd-journal-upload.service.in
+++ b/units/systemd-journal-upload.service.in
@@ -18,7 +18,6 @@ DynamicUser=yes
 ExecStart=@rootlibexecdir@/systemd-journal-upload --save-state
 LockPersonality=yes
 MemoryDenyWriteExecute=yes
-NoNewPrivileges=yes
 PrivateDevices=yes
 ProtectControlGroups=yes
 ProtectHome=yes