From 0cc496b2d21f73d0a03414ce40eceb9e3af76e22 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 19 Oct 2024 14:38:08 +0900 Subject: [PATCH 1/3] core/namespace: honor MountEntry.read_only, .options, and so on in static entries Otherwise, ProtectHome=tmpfs makes /home/ and friends not read-only. Also, mount options for /run/ specified in MountAPIVFS=yes are not applied. The function append_static_mounts() was introduced in 5327c910d2fc1ae91bd0b891be92b30379c7467b, but at that time, there were neither .read_only nor .options in the struct. But, when later the struct is extended, the function was not updated and they were not copied from the static table. The fields has been used in static tables since e4da7d8c796a1fd11ecfa80fb8a48eac9e823f06, and also in 94293d65cd4125347e21b3e423d0e245226b1be2. Fixes #34825. --- src/core/namespace.c | 15 ++++++++++----- test/units/TEST-07-PID1.exec-context.sh | 13 +++++++++++-- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index e1a88f2455..6606d767ed 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -714,11 +714,16 @@ static int append_static_mounts(MountList *ml, const MountEntry *mounts, size_t if (!me) return log_oom_debug(); - *me = (MountEntry) { - .path_const = mount_entry_path(m), - .mode = m->mode, - .ignore = m->ignore || ignore_protect, - }; + /* No dynamic values allowed. */ + assert(m->path_const); + assert(!m->path_malloc); + assert(!m->unprefixed_path_malloc); + assert(!m->source_malloc); + assert(!m->options_malloc); + assert(!m->overlay_layers); + + *me = *m; + me->ignore = me->ignore || ignore_protect; } return 0; diff --git a/test/units/TEST-07-PID1.exec-context.sh b/test/units/TEST-07-PID1.exec-context.sh index 0809460955..46fb1f79e8 100755 --- a/test/units/TEST-07-PID1.exec-context.sh +++ b/test/units/TEST-07-PID1.exec-context.sh @@ -55,13 +55,22 @@ if [[ -z "${COVERAGE_BUILD_DIR:-}" ]]; then bash -xec "test ! -w /home; test ! -w /root; test ! -w /run/user; test ! -e $MARK" systemd-run --wait --pipe -p ProtectHome=read-only \ bash -xec "test ! -w /home; test ! -w /root; test ! -w /run/user; test -e $MARK" - systemd-run --wait --pipe -p ProtectHome=tmpfs \ - bash -xec "test -w /home; test -w /root; test -w /run/user; test ! -e $MARK" + systemd-run --wait --pipe -p ProtectHome=tmpfs -p TemporaryFileSystem=/home/foo \ + bash -xec "test ! -w /home; test ! -w /root; test ! -w /run/user; test ! -e $MARK; test -w /home/foo" systemd-run --wait --pipe -p ProtectHome=no \ bash -xec "test -w /home; test -w /root; test -w /run/user; test -e $MARK" rm -f "$MARK" fi +systemd-run --wait --pipe -p PrivateMounts=true -p MountAPIVFS=yes \ + bash -xec '[[ "$(findmnt --mountpoint /proc --noheadings -o FSTYPE)" == proc ]]; + [[ "$$(findmnt --mountpoint /dev --noheadings -o FSTYPE)" =~ (devtmpfs|tmpfs) ]]; + [[ "$$(findmnt --mountpoint /sys --noheadings -o FSTYPE)" =~ (sysfs|tmpfs) ]]; + [[ "$$(findmnt --mountpoint /run --noheadings -o FSTYPE)" == tmpfs ]]; + [[ "$$(findmnt --mountpoint /run --noheadings -o VFS-OPTIONS)" =~ rw ]]; + [[ "$$(findmnt --mountpoint /run --noheadings -o VFS-OPTIONS)" =~ nosuid ]]; + [[ "$$(findmnt --mountpoint /run --noheadings -o VFS-OPTIONS)" =~ nodev ]]' + if proc_supports_option "hidepid=off"; then systemd-run --wait --pipe -p ProtectProc=noaccess -p User=testuser \ bash -xec 'test -e /proc/1; test ! -r /proc/1; test -r /proc/$$$$/comm' From d69ee5acdccec762b6b58b5b79902be14bb54a98 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 19 Oct 2024 12:46:05 +0900 Subject: [PATCH 2/3] core/namespace: coding style cleanups --- src/core/namespace.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index 6606d767ed..90779ba703 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -221,7 +221,7 @@ static const MountEntry protect_system_full_table[] = { * left writable, as ProtectHome= shall manage those, orthogonally). */ static const MountEntry protect_system_strict_table[] = { - { "/", MOUNT_READ_ONLY, false }, + { "/", MOUNT_READ_ONLY, false }, { "/proc", MOUNT_READ_WRITE_IMPLICIT, false }, /* ProtectKernelTunables= */ { "/sys", MOUNT_READ_WRITE_IMPLICIT, false }, /* ProtectKernelTunables= */ { "/dev", MOUNT_READ_WRITE_IMPLICIT, false }, /* PrivateDevices= */ @@ -280,7 +280,7 @@ static const struct { DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(mount_mode, MountMode); -static const char *mount_entry_path(const MountEntry *p) { +static const char* mount_entry_path(const MountEntry *p) { assert(p); /* Returns the path of this bind mount. If the malloc()-allocated ->path_buffer field is set we return that, @@ -289,7 +289,7 @@ static const char *mount_entry_path(const MountEntry *p) { return p->path_malloc ?: p->path_const; } -static const char *mount_entry_unprefixed_path(const MountEntry *p) { +static const char* mount_entry_unprefixed_path(const MountEntry *p) { assert(p); /* Returns the unprefixed path (ie: before prefix_where_needed() ran), if any */ @@ -330,13 +330,13 @@ static bool mount_entry_exec(const MountEntry *p) { return p->exec || p->mode == MOUNT_EXEC; } -static const char *mount_entry_source(const MountEntry *p) { +static const char* mount_entry_source(const MountEntry *p) { assert(p); return p->source_malloc ?: p->source_const; } -static const char *mount_entry_options(const MountEntry *p) { +static const char* mount_entry_options(const MountEntry *p) { assert(p); return p->options_malloc ?: p->options_const; @@ -363,7 +363,7 @@ static void mount_list_done(MountList *ml) { ml->n_mounts = 0; } -static MountEntry *mount_list_extend(MountList *ml) { +static MountEntry* mount_list_extend(MountList *ml) { assert(ml); if (!GREEDY_REALLOC0(ml->mounts, ml->n_mounts+1)) From 5811a0117cd4c51b166d04cf3c9b9bd0dbe144aa Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 19 Oct 2024 14:40:13 +0900 Subject: [PATCH 3/3] core/namespace: replace MOUNT_PRIVATE_TMP_READ_ONLY with MOUNT_PRIVATE_TMP with .read_only = true --- src/core/namespace.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index 90779ba703..ba308c2957 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -59,7 +59,6 @@ typedef enum MountMode { MOUNT_BIND, MOUNT_BIND_RECURSIVE, MOUNT_PRIVATE_TMP, - MOUNT_PRIVATE_TMP_READ_ONLY, MOUNT_PRIVATE_DEV, MOUNT_BIND_DEV, MOUNT_EMPTY_DIR, @@ -243,7 +242,6 @@ static const char * const mount_mode_table[_MOUNT_MODE_MAX] = { [MOUNT_BIND] = "bind", [MOUNT_BIND_RECURSIVE] = "bind-recursive", [MOUNT_PRIVATE_TMP] = "private-tmp", - [MOUNT_PRIVATE_TMP_READ_ONLY] = "private-tmp-read-only", [MOUNT_PRIVATE_DEV] = "private-dev", [MOUNT_BIND_DEV] = "bind-dev", [MOUNT_EMPTY_DIR] = "empty-dir", @@ -315,7 +313,7 @@ static void mount_entry_consume_prefix(MountEntry *p, char *new_path) { static bool mount_entry_read_only(const MountEntry *p) { assert(p); - return p->read_only || IN_SET(p->mode, MOUNT_READ_ONLY, MOUNT_INACCESSIBLE, MOUNT_PRIVATE_TMP_READ_ONLY); + return p->read_only || IN_SET(p->mode, MOUNT_READ_ONLY, MOUNT_INACCESSIBLE); } static bool mount_entry_noexec(const MountEntry *p) { @@ -1746,7 +1744,6 @@ static int apply_one_mount( return mount_tmpfs(m); case MOUNT_PRIVATE_TMP: - case MOUNT_PRIVATE_TMP_READ_ONLY: what = mount_entry_source(m); make = true; break; @@ -2402,29 +2399,27 @@ int setup_namespace(const NamespaceParameters *p, char **error_path) { assert(p->private_tmp == PRIVATE_TMP_CONNECTED); if (p->tmp_dir) { - bool ro = streq(p->tmp_dir, RUN_SYSTEMD_EMPTY); - MountEntry *me = mount_list_extend(&ml); if (!me) return log_oom_debug(); *me = (MountEntry) { .path_const = "/tmp", - .mode = ro ? MOUNT_PRIVATE_TMP_READ_ONLY : MOUNT_PRIVATE_TMP, + .mode = MOUNT_PRIVATE_TMP, + .read_only = streq(p->tmp_dir, RUN_SYSTEMD_EMPTY), .source_const = p->tmp_dir, }; } if (p->var_tmp_dir) { - bool ro = streq(p->var_tmp_dir, RUN_SYSTEMD_EMPTY); - MountEntry *me = mount_list_extend(&ml); if (!me) return log_oom_debug(); *me = (MountEntry) { .path_const = "/var/tmp", - .mode = ro ? MOUNT_PRIVATE_TMP_READ_ONLY : MOUNT_PRIVATE_TMP, + .mode = MOUNT_PRIVATE_TMP, + .read_only = streq(p->var_tmp_dir, RUN_SYSTEMD_EMPTY), .source_const = p->var_tmp_dir, }; }