Allow commas in SystemCallLog=/RestrictFileSystems=/RestrictNetworkInterfaces=

The previous commits made SystemCallFilter= and RestrictAddressFamilies=
accept commas as separators, and the shared serialization helper in
bus-unit-util.c now splits on commas for all the filter-list settings it
handles. That left unit files and 'systemctl set-property'/'systemd-run
-p' disagreeing for the remaining three settings. Accept commas in the
unit file parsers too, and update the documentation.

Note that a comma is a valid character in a network interface name, so an
interface with a comma in its name needs to be quoted or escaped. Add a
hint in the man page. Such names should be exceedingly rare so this
shouldn't matter in practice.

The manager writes transient settings back into a drop-in which is parsed
again on daemon-reload. Quote the interface names there, so that a name
containing a comma (or a quote or backslash, which are valid in interface
names too) survives the round trip.

man/systemd.exec.xml

@@ -2496,11 +2496,12 @@ BindReadOnlyPaths=/var/lib/systemd</programlisting>
       <varlistentry>
         <term><varname>RestrictFileSystems=</varname></term>
 
-        <listitem><para>Restricts the set of filesystems processes of this unit can open files on. Takes a space-separated
-        list of filesystem names. Any filesystem listed is made accessible to the unit's processes, access to filesystem
-        types not listed is prohibited (allow-listing). If the first character of the list is <literal>~</literal>, the
-        effect is inverted: access to the filesystems listed is prohibited (deny-listing). If the empty string is assigned,
-        access to filesystems is not restricted.</para>
+        <listitem><para>Restricts the set of filesystems processes of this unit can open files on. Takes a
+        comma- or space-separated list of filesystem names. Any filesystem listed is made accessible to the
+        unit's processes, access to filesystem types not listed is prohibited (allow-listing). If the first
+        character of the list is <literal>~</literal>, the effect is inverted: access to the filesystems listed
+        is prohibited (deny-listing). If the empty string is assigned, access to filesystems is not
+        restricted.</para>
 
         <para>If you specify both types of this option (i.e. allow-listing and deny-listing), the first encountered will take
         precedence and will dictate the default action (allow access to the filesystem or deny it). Then the next occurrences
@@ -3197,8 +3198,8 @@ SystemCallErrorNumber=EPERM</programlisting>
       <varlistentry>
         <term><varname>SystemCallLog=</varname></term>
 
-        <listitem><para>Takes a space-separated list of system call names. If this setting is used, all
-        system calls executed by the unit processes for the listed ones will be logged. If the first
+        <listitem><para>Takes a comma- or space-separated list of system call names. If this setting is used,
+        all system calls executed by the unit processes for the listed ones will be logged. If the first
         character of the list is <literal>~</literal>, the effect is inverted: all system calls except the
         listed system calls will be logged. This feature makes use of the Secure Computing Mode 2 interfaces
         of the kernel ('seccomp filtering') and is useful for auditing or setting up a minimal sandboxing

man/systemd.resource-control.xml

@@ -992,11 +992,11 @@ SocketBindDeny=any
         <term><varname>RestrictNetworkInterfaces=</varname></term>
 
         <listitem>
-          <para>Takes a list of space-separated network interface names. This option restricts the network
-          interfaces that processes of this unit can use. By default, processes can only use the network interfaces
-          listed (allow-list). If the first character of the rule is <literal>~</literal>, the effect is inverted:
-          the processes can only use network interfaces not listed (deny-list).
-          </para>
+          <para>Takes a comma- or space-separated list of network interface names. This option restricts the
+          network interfaces that processes of this unit can use. By default, processes can only use the network
+          interfaces listed (allow-list). If the first character of the rule is <literal>~</literal>, the effect is
+          inverted: the processes can only use network interfaces not listed (deny-list). Note: to specify
+          an interface with a comma in the name, use quotes.</para>
 
           <para>This option can appear multiple times, in which case the network interface names are merged. If the
           empty string is assigned the set is reset, all prior assignments will have not effect.
@@ -1011,6 +1011,7 @@ SocketBindDeny=any
           <para>The loopback interface ("lo") is not treated in any special way, you have to configure it explicitly
           in the unit file.
           </para>
+
           <para>Example 1: allow-list
           <programlisting>
 RestrictNetworkInterfaces=eth1

src/core/dbus-cgroup.c

@@ -1966,7 +1966,9 @@ int bus_cgroup_set_property(
                                 }
                         }
 
-                        joined = strv_join(l, " ");
+                        /* Quote the names, so that names containing the separators (a comma is a valid character in
+                         * an interface name) or quotes survive being read back from the drop-in. */
+                        joined = unit_concat_strv(l, UNIT_ESCAPE_EXEC_SYNTAX);
                         if (!joined)
                                 return -ENOMEM;
 

src/core/load-fragment.c

@@ -3433,7 +3433,7 @@ int config_parse_syscall_log(
         for (;;) {
                 _cleanup_free_ char *word = NULL;
 
-                r = extract_first_word(&p, &word, NULL, 0);
+                r = extract_first_word(&p, &word, /* separators= */ WHITESPACE ",", 0);
                 if (r == -ENOMEM)
                         return log_oom();
                 if (r < 0) {
@@ -3686,7 +3686,7 @@ int config_parse_restrict_filesystems(
         for (const char *p = rvalue;;) {
                 _cleanup_free_ char *word = NULL;
 
-                r = extract_first_word(&p, &word, NULL, EXTRACT_UNQUOTE);
+                r = extract_first_word(&p, &word, /* separators= */ WHITESPACE ",", EXTRACT_UNQUOTE);
                 if (r == 0)
                         break;
                 if (r == -ENOMEM)
@@ -6015,7 +6015,7 @@ int config_parse_restrict_network_interfaces(
         for (const char *p = rvalue;;) {
                 _cleanup_free_ char *word = NULL;
 
-                r = extract_first_word(&p, &word, NULL, EXTRACT_UNQUOTE);
+                r = extract_first_word(&p, &word, /* separators= */ WHITESPACE ",", EXTRACT_UNQUOTE);
                 if (r == 0)
                         break;
                 if (r == -ENOMEM)

test/integration-tests/TEST-62-RESTRICT-IFACES/TEST-62-RESTRICT-IFACES.units/TEST-62-RESTRICT-IFACES-5.service

@@ -7,5 +7,6 @@ ExecStart=bash -c 'ping -c 1 -W 0.2 192.168.113.5'
 ExecStart=bash -c '! ping -c 1 -W 0.2 192.168.113.9'
 RestrictNetworkInterfaces=veth0
 RestrictNetworkInterfaces=veth0 veth1
+RestrictNetworkInterfaces=veth0,veth1
 RestrictNetworkInterfaces=~veth0
 Type=oneshot

test/units/TEST-07-PID1.exec-context.sh

@@ -348,6 +348,7 @@ if [[ ! -v ASAN_OPTIONS ]] && systemctl --version | grep "+BPF_FRAMEWORK" && ker
     ROOTFS="$(df --output=fstype /usr/bin | sed --quiet 2p)"
     systemd-run --wait --pipe -p RestrictFileSystems="" ls /
     systemd-run --wait --pipe -p RestrictFileSystems="$ROOTFS foo bar" ls /
+    systemd-run --wait --pipe -p RestrictFileSystems="$ROOTFS,foo,bar" ls /
     (! systemd-run --wait --pipe -p RestrictFileSystems="$ROOTFS" ls /proc)
     (! systemd-run --wait --pipe -p GCOV_ERROR_LOG=/dev/null -p RestrictFileSystems="foo" ls /)
     systemd-run --wait --pipe -p RestrictFileSystems="$ROOTFS foo bar baz proc" ls /proc

test/units/TEST-62-RESTRICT-IFACES.sh

@@ -49,4 +49,14 @@ systemctl start --wait TEST-62-RESTRICT-IFACES-3.service
 systemctl start --wait TEST-62-RESTRICT-IFACES-4.service
 systemctl start --wait TEST-62-RESTRICT-IFACES-5.service
 
+# An interface name containing a comma must survive the transient drop-in and a daemon-reload.
+# The interface does not need to exist, unknown names are ignored with a warning.
+systemd-run --unit=TEST-62-RESTRICT-IFACES-transient.service \
+            -p 'RestrictNetworkInterfaces=~"veth,comma"' \
+            sleep infinity
+test "$(systemctl show -P RestrictNetworkInterfaces TEST-62-RESTRICT-IFACES-transient.service)" = '~veth,comma'
+systemctl daemon-reload
+test "$(systemctl show -P RestrictNetworkInterfaces TEST-62-RESTRICT-IFACES-transient.service)" = '~veth,comma'
+systemctl stop TEST-62-RESTRICT-IFACES-transient.service
+
 touch /testok