Merge pull request #3685 from kinvolk/alessandro/inaccessible-paths

namespace: unify limit behavior on non-directory paths

13 files changed, 132 insertions, 80 deletions

diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
index ed02666daf..49fea98a95 100644
--- a/man/systemd.exec.xml
+++ b/man/systemd.exec.xml
@@ -848,38 +848,40 @@
       </varlistentry>
 
       <varlistentry>
-        <term><varname>ReadWriteDirectories=</varname></term>
-        <term><varname>ReadOnlyDirectories=</varname></term>
-        <term><varname>InaccessibleDirectories=</varname></term>
+        <term><varname>ReadWritePaths=</varname></term>
+        <term><varname>ReadOnlyPaths=</varname></term>
+        <term><varname>InaccessiblePaths=</varname></term>
 
         <listitem><para>Sets up a new file system namespace for
         executed processes. These options may be used to limit access
         a process might have to the main file system hierarchy. Each
-        setting takes a space-separated list of directory paths relative to
+        setting takes a space-separated list of paths relative to
         the host's root directory (i.e. the system running the service manager).
-        Directories listed in
-        <varname>ReadWriteDirectories=</varname> are accessible from
+        Note that if entries contain symlinks, they are resolved from the host's root directory as well.
+        Entries (files or directories) listed in
+        <varname>ReadWritePaths=</varname> are accessible from
         within the namespace with the same access rights as from
-        outside. Directories listed in
-        <varname>ReadOnlyDirectories=</varname> are accessible for
+        outside. Entries listed in
+        <varname>ReadOnlyPaths=</varname> are accessible for
         reading only, writing will be refused even if the usual file
-        access controls would permit this. Directories listed in
-        <varname>InaccessibleDirectories=</varname> will be made
+        access controls would permit this. Entries listed in
+        <varname>InaccessiblePaths=</varname> will be made
         inaccessible for processes inside the namespace, and may not
         countain any other mountpoints, including those specified by
-        <varname>ReadWriteDirectories=</varname> or
-        <varname>ReadOnlyDirectories=</varname>.
+        <varname>ReadWritePaths=</varname> or
+        <varname>ReadOnlyPaths=</varname>.
         Note that restricting access with these options does not extend
-        to submounts of a directory that are created later on. These
+        to submounts of a directory that are created later on.
+        Non-directory paths can be specified as well. These
         options may be specified more than once, in which case all
-        directories listed will have limited access from within the
+        paths listed will have limited access from within the
         namespace. If the empty string is assigned to this option, the
         specific list is reset, and all prior assignments have no
         effect.</para>
         <para>Paths in
-        <varname>ReadOnlyDirectories=</varname>
+        <varname>ReadOnlyPaths=</varname>
         and
-        <varname>InaccessibleDirectories=</varname>
+        <varname>InaccessiblePaths=</varname>
         may be prefixed with
         <literal>-</literal>, in which case
         they will be ignored when they do not
@@ -1034,9 +1036,9 @@
         <varname>PrivateDevices=</varname>,
         <varname>ProtectSystem=</varname>,
         <varname>ProtectHome=</varname>,
-        <varname>ReadOnlyDirectories=</varname>,
-        <varname>InaccessibleDirectories=</varname> and
-        <varname>ReadWriteDirectories=</varname>) require that mount
+        <varname>ReadOnlyPaths=</varname>,
+        <varname>InaccessiblePaths=</varname> and
+        <varname>ReadWritePaths=</varname>) require that mount
         and unmount propagation from the unit's file system namespace
         is disabled, and hence downgrade <option>shared</option> to
         <option>slave</option>. </para></listitem>
diff --git a/shell-completion/bash/systemd-run b/shell-completion/bash/systemd-run
index 8152b021e7..022331e6a9 100644
--- a/shell-completion/bash/systemd-run
+++ b/shell-completion/bash/systemd-run
@@ -84,8 +84,8 @@ _systemd_run() {
                          LimitNICE= LimitRTPRIO= LimitRTTIME= PrivateTmp= PrivateDevices=
                          PrivateNetwork= NoNewPrivileges= WorkingDirectory= RootDirectory=
                          TTYPath= SyslogIdentifier= SyslogLevelPrefix= SyslogLevel=
-                         SyslogFacility= TimerSlackNSec= OOMScoreAdjust= ReadWriteDirectories=
-                         ReadOnlyDirectories= InaccessibleDirectories= EnvironmentFile=
+                         SyslogFacility= TimerSlackNSec= OOMScoreAdjust= ReadWritePaths=
+                         ReadOnlyPaths= InaccessiblePaths= EnvironmentFile=
                          ProtectSystem= ProtectHome= RuntimeDirectory= PassEnvironment='
 
             COMPREPLY=( $(compgen -W '$comps' -- "$cur") )
diff --git a/shell-completion/zsh/_systemd-run b/shell-completion/zsh/_systemd-run
index c425085cd8..6362b97766 100644
--- a/shell-completion/zsh/_systemd-run
+++ b/shell-completion/zsh/_systemd-run
@@ -37,8 +37,8 @@ _arguments \
                 LimitNICE= LimitRTPRIO= LimitRTTIME= PrivateTmp= PrivateDevices= \
                 PrivateNetwork= NoNewPrivileges= WorkingDirectory= RootDirectory= \
                 TTYPath= SyslogIdentifier= SyslogLevelPrefix= SyslogLevel= \
-                SyslogFacility= TimerSlackNSec= OOMScoreAdjust= ReadWriteDirectories= \
-                ReadOnlyDirectories= InaccessibleDirectories= EnvironmentFile= \
+                SyslogFacility= TimerSlackNSec= OOMScoreAdjust= ReadWritePaths= \
+                ReadOnlyPaths= InaccessiblePaths= EnvironmentFile= \
                 ProtectSystem= ProtectHome= RuntimeDirectory= PassEnvironment= \
                 ))' \
         '--description=[Description for unit]:description' \
diff --git a/src/basic/mount-util.c b/src/basic/mount-util.c
index 90b7a885a8..63dff3dd5c 100644
--- a/src/basic/mount-util.c
+++ b/src/basic/mount-util.c
@@ -532,3 +532,21 @@ int repeat_unmount(const char *path, int flags) {
                 done = true;
         }
 }
+
+const char* mode_to_inaccessible_node(mode_t mode) {
+        switch(mode & S_IFMT) {
+                case S_IFREG:
+                        return "/run/systemd/inaccessible/reg";
+                case S_IFDIR:
+                        return "/run/systemd/inaccessible/dir";
+                case S_IFCHR:
+                        return "/run/systemd/inaccessible/chr";
+                case S_IFBLK:
+                        return "/run/systemd/inaccessible/blk";
+                case S_IFIFO:
+                        return "/run/systemd/inaccessible/fifo";
+                case S_IFSOCK:
+                        return "/run/systemd/inaccessible/sock";
+        }
+        return NULL;
+}
diff --git a/src/basic/mount-util.h b/src/basic/mount-util.h
index bdb525d6b0..f46989ebb3 100644
--- a/src/basic/mount-util.h
+++ b/src/basic/mount-util.h
@@ -49,4 +49,6 @@ union file_handle_union {
         char padding[sizeof(struct file_handle) + MAX_HANDLE_SZ];
 };
 
+const char* mode_to_inaccessible_node(mode_t mode);
+
 #define FILE_HANDLE_INIT { .handle.handle_bytes = MAX_HANDLE_SZ }
diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
index 644b9561b5..b2ef3db491 100644
--- a/src/core/dbus-execute.c
+++ b/src/core/dbus-execute.c
@@ -695,9 +695,12 @@ const sd_bus_vtable bus_exec_vtable[] = {
         SD_BUS_PROPERTY("Group", "s", NULL, offsetof(ExecContext, group), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("SupplementaryGroups", "as", NULL, offsetof(ExecContext, supplementary_groups), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("PAMName", "s", NULL, offsetof(ExecContext, pam_name), SD_BUS_VTABLE_PROPERTY_CONST),
-        SD_BUS_PROPERTY("ReadWriteDirectories", "as", NULL, offsetof(ExecContext, read_write_dirs), SD_BUS_VTABLE_PROPERTY_CONST),
-        SD_BUS_PROPERTY("ReadOnlyDirectories", "as", NULL, offsetof(ExecContext, read_only_dirs), SD_BUS_VTABLE_PROPERTY_CONST),
-        SD_BUS_PROPERTY("InaccessibleDirectories", "as", NULL, offsetof(ExecContext, inaccessible_dirs), SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("ReadWriteDirectories", "as", NULL, offsetof(ExecContext, read_write_paths), SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("ReadOnlyDirectories", "as", NULL, offsetof(ExecContext, read_only_paths), SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("InaccessibleDirectories", "as", NULL, offsetof(ExecContext, inaccessible_paths), SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("ReadWritePaths", "as", NULL, offsetof(ExecContext, read_write_paths), SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("ReadOnlyPaths", "as", NULL, offsetof(ExecContext, read_only_paths), SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("InaccessiblePaths", "as", NULL, offsetof(ExecContext, inaccessible_paths), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("MountFlags", "t", bus_property_get_ulong, offsetof(ExecContext, mount_flags), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("PrivateTmp", "b", bus_property_get_bool, offsetof(ExecContext, private_tmp), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("PrivateNetwork", "b", bus_property_get_bool, offsetof(ExecContext, private_network), SD_BUS_VTABLE_PROPERTY_CONST),
@@ -1323,8 +1326,8 @@ int bus_exec_context_set_transient_property(
 
                 return 1;
 
-        } else if (STR_IN_SET(name, "ReadWriteDirectories", "ReadOnlyDirectories", "InaccessibleDirectories")) {
-
+        } else if (STR_IN_SET(name, "ReadWriteDirectories", "ReadOnlyDirectories", "InaccessibleDirectories",
+                              "ReadWritePaths", "ReadOnlyPaths", "InaccessiblePaths")) {
                 _cleanup_strv_free_ char **l = NULL;
                 char ***dirs;
                 char **p;
@@ -1346,12 +1349,12 @@ int bus_exec_context_set_transient_property(
                 if (mode != UNIT_CHECK) {
                         _cleanup_free_ char *joined = NULL;
 
-                        if (streq(name, "ReadWriteDirectories"))
-                                dirs = &c->read_write_dirs;
-                        else if (streq(name, "ReadOnlyDirectories"))
-                                dirs = &c->read_only_dirs;
-                        else /* "InaccessibleDirectories" */
-                                dirs = &c->inaccessible_dirs;
+                        if (STR_IN_SET(name, "ReadWriteDirectories", "ReadWritePaths"))
+                                dirs = &c->read_write_paths;
+                        else if (STR_IN_SET(name, "ReadOnlyDirectories", "ReadOnlyPaths"))
+                                dirs = &c->read_only_paths;
+                        else /* "InaccessiblePaths" */
+                                dirs = &c->inaccessible_paths;
 
                         if (strv_length(l) == 0) {
                                 *dirs = strv_free(*dirs);
diff --git a/src/core/execute.c b/src/core/execute.c
index f4f5723c35..05dc1aaec1 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -1507,9 +1507,9 @@ static bool exec_needs_mount_namespace(
         assert(context);
         assert(params);
 
-        if (!strv_isempty(context->read_write_dirs) ||
-            !strv_isempty(context->read_only_dirs) ||
-            !strv_isempty(context->inaccessible_dirs))
+        if (!strv_isempty(context->read_write_paths) ||
+            !strv_isempty(context->read_only_paths) ||
+            !strv_isempty(context->inaccessible_paths))
                 return true;
 
         if (context->mount_flags != 0)
@@ -1933,9 +1933,9 @@ static int exec_child(
 
                 r = setup_namespace(
                                 params->apply_chroot ? context->root_directory : NULL,
-                                context->read_write_dirs,
-                                context->read_only_dirs,
-                                context->inaccessible_dirs,
+                                context->read_write_paths,
+                                context->read_only_paths,
+                                context->inaccessible_paths,
                                 tmp,
                                 var,
                                 context->private_devices,
@@ -2324,9 +2324,9 @@ void exec_context_done(ExecContext *c) {
 
         c->pam_name = mfree(c->pam_name);
 
-        c->read_only_dirs = strv_free(c->read_only_dirs);
-        c->read_write_dirs = strv_free(c->read_write_dirs);
-        c->inaccessible_dirs = strv_free(c->inaccessible_dirs);
+        c->read_only_paths = strv_free(c->read_only_paths);
+        c->read_write_paths = strv_free(c->read_write_paths);
+        c->inaccessible_paths = strv_free(c->inaccessible_paths);
 
         if (c->cpuset)
                 CPU_FREE(c->cpuset);
@@ -2732,21 +2732,21 @@ void exec_context_dump(ExecContext *c, FILE* f, const char *prefix) {
         if (c->pam_name)
                 fprintf(f, "%sPAMName: %s\n", prefix, c->pam_name);
 
-        if (strv_length(c->read_write_dirs) > 0) {
-                fprintf(f, "%sReadWriteDirs:", prefix);
-                strv_fprintf(f, c->read_write_dirs);
+        if (strv_length(c->read_write_paths) > 0) {
+                fprintf(f, "%sReadWritePaths:", prefix);
+                strv_fprintf(f, c->read_write_paths);
                 fputs("\n", f);
         }
 
-        if (strv_length(c->read_only_dirs) > 0) {
-                fprintf(f, "%sReadOnlyDirs:", prefix);
-                strv_fprintf(f, c->read_only_dirs);
+        if (strv_length(c->read_only_paths) > 0) {
+                fprintf(f, "%sReadOnlyPaths:", prefix);
+                strv_fprintf(f, c->read_only_paths);
                 fputs("\n", f);
         }
 
-        if (strv_length(c->inaccessible_dirs) > 0) {
-                fprintf(f, "%sInaccessibleDirs:", prefix);
-                strv_fprintf(f, c->inaccessible_dirs);
+        if (strv_length(c->inaccessible_paths) > 0) {
+                fprintf(f, "%sInaccessiblePaths:", prefix);
+                strv_fprintf(f, c->inaccessible_paths);
                 fputs("\n", f);
         }
 
diff --git a/src/core/execute.h b/src/core/execute.h
index cacf66cf51..73b8a119b0 100644
--- a/src/core/execute.h
+++ b/src/core/execute.h
@@ -152,7 +152,7 @@ struct ExecContext {
         bool smack_process_label_ignore;
         char *smack_process_label;
 
-        char **read_write_dirs, **read_only_dirs, **inaccessible_dirs;
+        char **read_write_paths, **read_only_paths, **inaccessible_paths;
         unsigned long mount_flags;
 
         uint64_t capability_bounding_set;
diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4
index fe1006830b..6a5c16a000 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -80,9 +80,12 @@ $1.LimitMSGQUEUE,                config_parse_limit,                 RLIMIT_MSGQ
 $1.LimitNICE,                    config_parse_limit,                 RLIMIT_NICE,                   offsetof($1, exec_context.rlimit)
 $1.LimitRTPRIO,                  config_parse_limit,                 RLIMIT_RTPRIO,                 offsetof($1, exec_context.rlimit)
 $1.LimitRTTIME,                  config_parse_limit,                 RLIMIT_RTTIME,                 offsetof($1, exec_context.rlimit)
-$1.ReadWriteDirectories,         config_parse_namespace_path_strv,   0,                             offsetof($1, exec_context.read_write_dirs)
-$1.ReadOnlyDirectories,          config_parse_namespace_path_strv,   0,                             offsetof($1, exec_context.read_only_dirs)
-$1.InaccessibleDirectories,      config_parse_namespace_path_strv,   0,                             offsetof($1, exec_context.inaccessible_dirs)
+$1.ReadWriteDirectories,         config_parse_namespace_path_strv,   0,                             offsetof($1, exec_context.read_write_paths)
+$1.ReadOnlyDirectories,          config_parse_namespace_path_strv,   0,                             offsetof($1, exec_context.read_only_paths)
+$1.InaccessibleDirectories,      config_parse_namespace_path_strv,   0,                             offsetof($1, exec_context.inaccessible_paths)
+$1.ReadWritePaths,               config_parse_namespace_path_strv,   0,                             offsetof($1, exec_context.read_write_paths)
+$1.ReadOnlyPaths,                config_parse_namespace_path_strv,   0,                             offsetof($1, exec_context.read_only_paths)
+$1.InaccessiblePaths,            config_parse_namespace_path_strv,   0,                             offsetof($1, exec_context.inaccessible_paths)
 $1.PrivateTmp,                   config_parse_bool,                  0,                             offsetof($1, exec_context.private_tmp)
 $1.PrivateNetwork,               config_parse_bool,                  0,                             offsetof($1, exec_context.private_network)
 $1.PrivateDevices,               config_parse_bool,                  0,                             offsetof($1, exec_context.private_devices)
diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c
index f9c9b4a91f..5d8ab0ec70 100644
--- a/src/core/mount-setup.c
+++ b/src/core/mount-setup.c
@@ -28,6 +28,7 @@
 #include "cgroup-util.h"
 #include "dev-setup.h"
 #include "efivars.h"
+#include "fs-util.h"
 #include "label.h"
 #include "log.h"
 #include "macro.h"
@@ -403,9 +404,16 @@ int mount_setup(bool loaded_policy) {
          * really needs to stay for good, otherwise software that
          * copied sd-daemon.c into their sources will misdetect
          * systemd. */
-        mkdir_label("/run/systemd", 0755);
-        mkdir_label("/run/systemd/system", 0755);
-        mkdir_label("/run/systemd/inaccessible", 0000);
+        (void) mkdir_label("/run/systemd", 0755);
+        (void) mkdir_label("/run/systemd/system", 0755);
+        (void) mkdir_label("/run/systemd/inaccessible", 0000);
+        /* Set up inaccessible items */
+        (void) mknod("/run/systemd/inaccessible/reg", S_IFREG | 0000, 0);
+        (void) mkdir_label("/run/systemd/inaccessible/dir", 0000);
+        (void) mknod("/run/systemd/inaccessible/chr", S_IFCHR | 0000, makedev(0, 0));
+        (void) mknod("/run/systemd/inaccessible/blk", S_IFBLK | 0000, makedev(0, 0));
+        (void) mkfifo("/run/systemd/inaccessible/fifo", 0000);
+        (void) mknod("/run/systemd/inaccessible/sock", S_IFSOCK | 0000, 0);
 
         return 0;
 }
diff --git a/src/core/namespace.c b/src/core/namespace.c
index 203d122810..722538caf1 100644
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -278,6 +278,7 @@ static int apply_mount(
 
         const char *what;
         int r;
+        struct stat target;
 
         assert(m);
 
@@ -287,12 +288,22 @@ static int apply_mount(
 
                 /* First, get rid of everything that is below if there
                  * is anything... Then, overmount it with an
-                 * inaccessible directory. */
+                 * inaccessible path. */
                 umount_recursive(m->path, 0);
 
-                what = "/run/systemd/inaccessible";
-                break;
+                r = lstat(m->path, &target);
+                if (r != 0) {
+                        if (m->ignore && errno == ENOENT)
+                                return 0;
+                        return -errno;
+                }
 
+                what = mode_to_inaccessible_node(target.st_mode);
+                if (what == NULL) {
+                        log_debug("File type not supported. Note that symlinks are not allowed");
+                        return -ELOOP;
+                }
+                break;
         case READONLY:
         case READWRITE:
                 /* Nothing to mount here, we just later toggle the
@@ -317,12 +328,16 @@ static int apply_mount(
         assert(what);
 
         r = mount(what, m->path, NULL, MS_BIND|MS_REC, NULL);
-        if (r >= 0)
+        if (r >= 0) {
                 log_debug("Successfully mounted %s to %s", what, m->path);
-        else if (m->ignore && errno == ENOENT)
-                return 0;
-
-        return r;
+                return r;
+        }
+        else {
+                if (m->ignore && errno == ENOENT)
+                        return 0;
+                log_debug("Failed mounting %s to %s: %s", what, m->path, strerror(errno));
+                return -errno;
+        }
 }
 
 static int make_read_only(BindMount *m) {
@@ -347,9 +362,9 @@ static int make_read_only(BindMount *m) {
 
 int setup_namespace(
                 const char* root_directory,
-                char** read_write_dirs,
-                char** read_only_dirs,
-                char** inaccessible_dirs,
+                char** read_write_paths,
+                char** read_only_paths,
+                char** inaccessible_paths,
                 const char* tmp_dir,
                 const char* var_tmp_dir,
                 bool private_dev,
@@ -368,9 +383,9 @@ int setup_namespace(
                 return -errno;
 
         n = !!tmp_dir + !!var_tmp_dir +
-                strv_length(read_write_dirs) +
-                strv_length(read_only_dirs) +
-                strv_length(inaccessible_dirs) +
+                strv_length(read_write_paths) +
+                strv_length(read_only_paths) +
+                strv_length(inaccessible_paths) +
                 private_dev +
                 (protect_home != PROTECT_HOME_NO ? 3 : 0) +
                 (protect_system != PROTECT_SYSTEM_NO ? 2 : 0) +
@@ -378,15 +393,15 @@ int setup_namespace(
 
         if (n > 0) {
                 m = mounts = (BindMount *) alloca0(n * sizeof(BindMount));
-                r = append_mounts(&m, read_write_dirs, READWRITE);
+                r = append_mounts(&m, read_write_paths, READWRITE);
                 if (r < 0)
                         return r;
 
-                r = append_mounts(&m, read_only_dirs, READONLY);
+                r = append_mounts(&m, read_only_paths, READONLY);
                 if (r < 0)
                         return r;
 
-                r = append_mounts(&m, inaccessible_dirs, INACCESSIBLE);
+                r = append_mounts(&m, inaccessible_paths, INACCESSIBLE);
                 if (r < 0)
                         return r;
 
diff --git a/src/core/namespace.h b/src/core/namespace.h
index b54b7b47d6..1aedf5f208 100644
--- a/src/core/namespace.h
+++ b/src/core/namespace.h
@@ -40,9 +40,9 @@ typedef enum ProtectSystem {
 } ProtectSystem;
 
 int setup_namespace(const char *chroot,
-                    char **read_write_dirs,
-                    char **read_only_dirs,
-                    char **inaccessible_dirs,
+                    char **read_write_paths,
+                    char **read_only_paths,
+                    char **inaccessible_paths,
                     const char *tmp_dir,
                     const char *var_tmp_dir,
                     bool private_dev,
diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c
index 04471e2373..94ffa8af87 100644
--- a/src/shared/bus-unit-util.c
+++ b/src/shared/bus-unit-util.c
@@ -453,7 +453,8 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen
                 }
 
                 r = sd_bus_message_append(m, "v", "i", oa);
-        } else if (STR_IN_SET(field, "ReadWriteDirectories", "ReadOnlyDirectories", "InaccessibleDirectories")) {
+        } else if (STR_IN_SET(field, "ReadWriteDirectories", "ReadOnlyDirectories", "InaccessibleDirectories",
+                              "ReadWritePaths", "ReadOnlyPaths", "InaccessiblePaths")) {
                 const char *p;
 
                 r = sd_bus_message_open_container(m, 'v', "as");