From 5e98086d1629f5c5b73645ba2568de4b09b7d958 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 22 Sep 2020 14:08:05 +0200 Subject: [PATCH 1/3] core: remember when we set ExecContext.mount_apivfs No functional change intended so far. --- src/core/dbus-execute.c | 20 ++++++++++++++++---- src/core/execute.c | 15 ++++++++++++--- src/core/execute.h | 2 ++ src/core/load-fragment.c | 38 ++++++++++++++++++++++++++++++++++++++ src/core/load-fragment.h | 1 + 5 files changed, 69 insertions(+), 7 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 753b91d511..488af98cd3 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -53,6 +53,7 @@ static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_protect_home, protect_home, Pro static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_protect_system, protect_system, ProtectSystem); static BUS_DEFINE_PROPERTY_GET_ENUM(property_get_personality, personality, unsigned long); static BUS_DEFINE_PROPERTY_GET(property_get_ioprio, "i", ExecContext, exec_context_get_effective_ioprio); +static BUS_DEFINE_PROPERTY_GET(property_get_mount_apivfs, "b", ExecContext, exec_context_get_effective_mount_apivfs); static BUS_DEFINE_PROPERTY_GET2(property_get_ioprio_class, "i", ExecContext, exec_context_get_effective_ioprio, IOPRIO_PRIO_CLASS); static BUS_DEFINE_PROPERTY_GET2(property_get_ioprio_priority, "i", ExecContext, exec_context_get_effective_ioprio, IOPRIO_PRIO_DATA); static BUS_DEFINE_PROPERTY_GET_GLOBAL(property_get_empty_string, "s", NULL); @@ -1143,7 +1144,7 @@ const sd_bus_vtable bus_exec_vtable[] = { SD_BUS_PROPERTY("BindPaths", "a(ssbt)", property_get_bind_paths, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("BindReadOnlyPaths", "a(ssbt)", property_get_bind_paths, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("TemporaryFileSystem", "a(ss)", property_get_temporary_filesystems, 0, SD_BUS_VTABLE_PROPERTY_CONST), - SD_BUS_PROPERTY("MountAPIVFS", "b", bus_property_get_bool, offsetof(ExecContext, mount_apivfs), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("MountAPIVFS", "b", property_get_mount_apivfs, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("KeyringMode", "s", property_get_exec_keyring_mode, offsetof(ExecContext, keyring_mode), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("ProtectProc", "s", property_get_protect_proc, offsetof(ExecContext, protect_proc), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("ProcSubset", "s", property_get_proc_subset, offsetof(ExecContext, proc_subset), SD_BUS_VTABLE_PROPERTY_CONST), @@ -1805,9 +1806,6 @@ int bus_exec_context_set_transient_property( if (streq(name, "ProtectControlGroups")) return bus_set_transient_bool(u, name, &c->protect_control_groups, message, flags, error); - if (streq(name, "MountAPIVFS")) - return bus_set_transient_bool(u, name, &c->mount_apivfs, message, flags, error); - if (streq(name, "CPUSchedulingResetOnFork")) return bus_set_transient_bool(u, name, &c->cpu_sched_reset_on_fork, message, flags, error); @@ -2635,6 +2633,20 @@ int bus_exec_context_set_transient_property( return 1; + } else if (streq(name, "MountAPIVFS")) { + bool b; + + r = bus_set_transient_bool(u, name, &b, message, flags, error); + if (r < 0) + return r; + + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { + c->mount_apivfs = b; + c->mount_apivfs_set = true; + } + + return 1; + } else if (streq(name, "WorkingDirectory")) { const char *s; bool missing_ok; diff --git a/src/core/execute.c b/src/core/execute.c index 44f30cb634..fd28f22c4f 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -2027,7 +2027,7 @@ static bool exec_needs_mount_namespace( return true; if (context->root_directory) { - if (context->mount_apivfs) + if (exec_context_get_effective_mount_apivfs(context)) return true; for (ExecDirectoryType t = 0; t < _EXEC_DIRECTORY_TYPE_MAX; t++) { @@ -3147,7 +3147,7 @@ static int apply_mount_namespace( .protect_kernel_modules = context->protect_kernel_modules, .protect_kernel_logs = context->protect_kernel_logs, .protect_hostname = context->protect_hostname, - .mount_apivfs = context->mount_apivfs, + .mount_apivfs = exec_context_get_effective_mount_apivfs(context), .private_mounts = context->private_mounts, .protect_home = context->protect_home, .protect_system = context->protect_system, @@ -5185,7 +5185,7 @@ void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) { prefix, yes_no(c->private_users), prefix, protect_home_to_string(c->protect_home), prefix, protect_system_to_string(c->protect_system), - prefix, yes_no(c->mount_apivfs), + prefix, yes_no(exec_context_get_effective_mount_apivfs(c)), prefix, yes_no(c->ignore_sigpipe), prefix, yes_no(c->memory_deny_write_execute), prefix, yes_no(c->restrict_realtime), @@ -5650,6 +5650,15 @@ int exec_context_get_effective_ioprio(const ExecContext *c) { return p; } +bool exec_context_get_effective_mount_apivfs(const ExecContext *c) { + assert(c); + + if (c->mount_apivfs_set) + return c->mount_apivfs; + + return false; +} + void exec_context_free_log_extra_fields(ExecContext *c) { assert(c); diff --git a/src/core/execute.h b/src/core/execute.h index 02a2c8d1e7..c21154bda2 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -174,6 +174,7 @@ struct ExecContext { bool nice_set:1; bool ioprio_set:1; bool cpu_sched_set:1; + bool mount_apivfs_set:1; /* This is not exposed to the user but available internally. We need it to make sure that whenever we * spawn /usr/bin/mount it is run in the same process group as us so that the autofs logic detects @@ -409,6 +410,7 @@ bool exec_context_may_touch_console(const ExecContext *c); bool exec_context_maintains_privileges(const ExecContext *c); int exec_context_get_effective_ioprio(const ExecContext *c); +bool exec_context_get_effective_mount_apivfs(const ExecContext *c); void exec_context_free_log_extra_fields(ExecContext *c); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 480da2c0dd..df40119175 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -1349,6 +1349,44 @@ int config_parse_exec_cpu_sched_policy(const char *unit, return 0; } +int config_parse_exec_mount_apivfs(const char *unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *lvalue, + int ltype, + const char *rvalue, + void *data, + void *userdata) { + + ExecContext *c = data; + int k; + + assert(filename); + assert(lvalue); + assert(rvalue); + assert(data); + + if (isempty(rvalue)) { + c->mount_apivfs_set = false; + c->mount_apivfs = false; + return 0; + } + + k = parse_boolean(rvalue); + if (k < 0) { + log_syntax(unit, LOG_WARNING, filename, line, k, + "Failed to parse boolean value, ignoring: %s", + rvalue); + return 0; + } + + c->mount_apivfs_set = true; + c->mount_apivfs = k; + return 0; +} + int config_parse_numa_mask(const char *unit, const char *filename, unsigned line, diff --git a/src/core/load-fragment.h b/src/core/load-fragment.h index 3504227cae..d67852a74d 100644 --- a/src/core/load-fragment.h +++ b/src/core/load-fragment.h @@ -43,6 +43,7 @@ CONFIG_PARSER_PROTOTYPE(config_parse_exec_io_priority); CONFIG_PARSER_PROTOTYPE(config_parse_exec_cpu_sched_policy); CONFIG_PARSER_PROTOTYPE(config_parse_exec_cpu_sched_prio); CONFIG_PARSER_PROTOTYPE(config_parse_exec_cpu_affinity); +CONFIG_PARSER_PROTOTYPE(config_parse_exec_mount_apivfs); CONFIG_PARSER_PROTOTYPE(config_parse_exec_secure_bits); CONFIG_PARSER_PROTOTYPE(config_parse_root_image_options); CONFIG_PARSER_PROTOTYPE(config_parse_exec_root_hash); From 6119878480aab4c10ad6af33deab221778683807 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 22 Sep 2020 14:49:49 +0200 Subject: [PATCH 2/3] core: turn on MountAPIVFS=true when RootImage or RootDirectory are specified Lennart wanted to do this back in https://github.com/systemd/systemd/commit/01c33c1effaa2406ff7d2a7de08a3ee87aec9fc8. For better or worse, this wasn't done because I thought that turning on MountAPIVFS is a compat break for RootDirectory and people might be negatively surprised by it. Without this, search for binaries doesn't work (access_fd() requires /proc). Let's turn it on, but still allow overriding to "no". When RootDirectory=/, MountAPIVFS=1 doesn't work. This might be a buglet on its own, but this patch doesn't change the situation. --- src/core/execute.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/core/execute.c b/src/core/execute.c index fd28f22c4f..1030c37ab5 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -5653,9 +5653,14 @@ int exec_context_get_effective_ioprio(const ExecContext *c) { bool exec_context_get_effective_mount_apivfs(const ExecContext *c) { assert(c); + /* Explicit setting wins */ if (c->mount_apivfs_set) return c->mount_apivfs; + /* Default to "yes" if root directory or image are specified */ + if (c->root_image || !empty_or_root(c->root_directory)) + return true; + return false; } From d583cf45b6830ec0e23ede181d17030852e5ac75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 22 Sep 2020 12:36:38 +0200 Subject: [PATCH 3/3] TEST-50-DISSECT: drop now-unneeded MountAPIVFS=yes and full paths to executables With the previous changes we can simplify the invocations in the test a bit. --- test/units/testsuite-50.sh | 54 ++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/test/units/testsuite-50.sh b/test/units/testsuite-50.sh index d6e1a3557a..883c83b8b7 100755 --- a/test/units/testsuite-50.sh +++ b/test/units/testsuite-50.sh @@ -60,11 +60,12 @@ fi umount ${image_dir}/mount umount ${image_dir}/mount2 -systemd-run -t --property RootImage=${image}.raw /usr/bin/cat /usr/lib/os-release | grep -q -F "MARKER=1" +systemd-run -t -p RootImage=${image}.raw cat /usr/lib/os-release | grep -q -F "MARKER=1" mv ${image}.verity ${image}.fooverity mv ${image}.roothash ${image}.foohash -systemd-run -t --property RootImage=${image}.raw --property RootHash=${image}.foohash --property RootVerity=${image}.fooverity /usr/bin/cat /usr/lib/os-release | grep -q -F "MARKER=1" -systemd-run -t --property RootImage=${image}.raw --property RootHash=${roothash} --property RootVerity=${image}.fooverity /usr/bin/cat /usr/lib/os-release | grep -q -F "MARKER=1" +systemd-run -t -p RootImage=${image}.raw -p RootHash=${image}.foohash -p RootVerity=${image}.fooverity cat /usr/lib/os-release | grep -q -F "MARKER=1" +# Let's use the long option name just here as a test +systemd-run -t --property RootImage=${image}.raw --property RootHash=${roothash} --property RootVerity=${image}.fooverity cat /usr/lib/os-release | grep -q -F "MARKER=1" mv ${image}.fooverity ${image}.verity mv ${image}.foohash ${image}.roothash @@ -128,19 +129,19 @@ cat ${image_dir}/mount/etc/os-release | grep -q -F -f $os_release cat ${image_dir}/mount/usr/lib/os-release | grep -q -F "MARKER=1" umount ${image_dir}/mount -systemd-run -t --property RootImage=${image}.gpt --property RootHash=${roothash} /usr/bin/cat /usr/lib/os-release | grep -q -F "MARKER=1" +# add explicit -p MountAPIVFS=yes once to test the parser +systemd-run -t -p RootImage=${image}.gpt -p RootHash=${roothash} -p MountAPIVFS=yes cat /usr/lib/os-release | grep -q -F "MARKER=1" -systemd-run -t --property RootImage=${image}.raw --property RootImageOptions="root:nosuid,dev home:ro,dev ro,noatime" --property MountAPIVFS=yes mount | grep -F "squashfs" | grep -q -F "nosuid" -systemd-run -t --property RootImage=${image}.gpt --property RootImageOptions="root:ro,noatime root:ro,dev" --property MountAPIVFS=yes mount | grep -F "squashfs" | grep -q -F "noatime" +systemd-run -t -p RootImage=${image}.raw -p RootImageOptions="root:nosuid,dev home:ro,dev ro,noatime" mount | grep -F "squashfs" | grep -q -F "nosuid" +systemd-run -t -p RootImage=${image}.gpt -p RootImageOptions="root:ro,noatime root:ro,dev" mount | grep -F "squashfs" | grep -q -F "noatime" mkdir -p mkdir -p ${image_dir}/result -cat > /run/systemd/system/testservice-50a.service </run/systemd/system/testservice-50a.service < /run/result/a" +ExecStart=bash -c "mount >/run/result/a" BindPaths=${image_dir}/result:/run/result TemporaryFileSystem=/run -MountAPIVFS=yes RootImage=${image}.raw RootImageOptions=root:ro,noatime home:ro,dev relatime,dev RootImageOptions=nosuid,dev @@ -149,33 +150,34 @@ systemctl start testservice-50a.service grep -F "squashfs" ${image_dir}/result/a | grep -q -F "noatime" grep -F "squashfs" ${image_dir}/result/a | grep -q -F -v "nosuid" -cat > /run/systemd/system/testservice-50b.service </run/systemd/system/testservice-50b.service < /run/result/b" +ExecStart=bash -c "mount >/run/result/b" BindPaths=${image_dir}/result:/run/result TemporaryFileSystem=/run -MountAPIVFS=yes RootImage=${image}.gpt RootImageOptions=root:ro,noatime,nosuid home:ro,dev nosuid,dev RootImageOptions=home:ro,dev nosuid,dev,%%foo +# this is the default, but let's specify once to test the parser +MountAPIVFS=yes EOF systemctl start testservice-50b.service grep -F "squashfs" ${image_dir}/result/b | grep -q -F "noatime" -# Check that specifier escape is applied %%foo -> %foo +# Check that specifier escape is applied %%foo → %foo busctl get-property org.freedesktop.systemd1 /org/freedesktop/systemd1/unit/testservice_2d50b_2eservice org.freedesktop.systemd1.Service RootImageOptions | grep -F "nosuid,dev,%foo" # Now do some checks with MountImages, both by itself, with options and in combination with RootImage, and as single FS or GPT image -systemd-run -t --property MountImages="${image}.gpt:/run/img1 ${image}.raw:/run/img2" /usr/bin/cat /run/img1/usr/lib/os-release | grep -q -F "MARKER=1" -systemd-run -t --property MountImages="${image}.gpt:/run/img1 ${image}.raw:/run/img2" /usr/bin/cat /run/img2/usr/lib/os-release | grep -q -F "MARKER=1" -systemd-run -t --property MountImages="${image}.gpt:/run/img1 ${image}.raw:/run/img2:nosuid,dev" --property MountAPIVFS=yes mount | grep -F "squashfs" | grep -q -F "nosuid" -systemd-run -t --property MountImages="${image}.gpt:/run/img1:root:nosuid ${image}.raw:/run/img2:home:suid" --property MountAPIVFS=yes mount | grep -F "squashfs" | grep -q -F "nosuid" -systemd-run -t --property MountImages="${image}.raw:/run/img2\:3" /usr/bin/cat /run/img2:3/usr/lib/os-release | grep -q -F "MARKER=1" -systemd-run -t --property MountImages="${image}.raw:/run/img2\:3:nosuid" --property MountAPIVFS=yes mount | grep -F "squashfs" | grep -q -F "nosuid" -systemd-run -t --property TemporaryFileSystem=/run --property RootImage=${image}.raw --property MountImages="${image}.gpt:/run/img1 ${image}.raw:/run/img2" /usr/bin/cat /usr/lib/os-release | grep -q -F "MARKER=1" -systemd-run -t --property TemporaryFileSystem=/run --property RootImage=${image}.raw --property MountImages="${image}.gpt:/run/img1 ${image}.raw:/run/img2" /usr/bin/cat /run/img1/usr/lib/os-release | grep -q -F "MARKER=1" -systemd-run -t --property TemporaryFileSystem=/run --property RootImage=${image}.gpt --property RootHash=${roothash} --property MountImages="${image}.gpt:/run/img1 ${image}.raw:/run/img2" /usr/bin/cat /run/img2/usr/lib/os-release | grep -q -F "MARKER=1" +systemd-run -t -p MountImages="${image}.gpt:/run/img1 ${image}.raw:/run/img2" cat /run/img1/usr/lib/os-release | grep -q -F "MARKER=1" +systemd-run -t -p MountImages="${image}.gpt:/run/img1 ${image}.raw:/run/img2" cat /run/img2/usr/lib/os-release | grep -q -F "MARKER=1" +systemd-run -t -p MountImages="${image}.gpt:/run/img1 ${image}.raw:/run/img2:nosuid,dev" mount | grep -F "squashfs" | grep -q -F "nosuid" +systemd-run -t -p MountImages="${image}.gpt:/run/img1:root:nosuid ${image}.raw:/run/img2:home:suid" mount | grep -F "squashfs" | grep -q -F "nosuid" +systemd-run -t -p MountImages="${image}.raw:/run/img2\:3" cat /run/img2:3/usr/lib/os-release | grep -q -F "MARKER=1" +systemd-run -t -p MountImages="${image}.raw:/run/img2\:3:nosuid" mount | grep -F "squashfs" | grep -q -F "nosuid" +systemd-run -t -p TemporaryFileSystem=/run -p RootImage=${image}.raw -p MountImages="${image}.gpt:/run/img1 ${image}.raw:/run/img2" cat /usr/lib/os-release | grep -q -F "MARKER=1" +systemd-run -t -p TemporaryFileSystem=/run -p RootImage=${image}.raw -p MountImages="${image}.gpt:/run/img1 ${image}.raw:/run/img2" cat /run/img1/usr/lib/os-release | grep -q -F "MARKER=1" +systemd-run -t -p TemporaryFileSystem=/run -p RootImage=${image}.gpt -p RootHash=${roothash} -p MountImages="${image}.gpt:/run/img1 ${image}.raw:/run/img2" cat /run/img2/usr/lib/os-release | grep -q -F "MARKER=1" cat >/run/systemd/system/testservice-50c.service < /run/result/c" -ExecStart=bash -c "cat /run/img2:3/usr/lib/os-release >> /run/result/c" -ExecStart=bash -c "mount >> /run/result/c" +ExecStart=bash -c "cat /run/img1/usr/lib/os-release >/run/result/c" +ExecStart=bash -c "cat /run/img2:3/usr/lib/os-release >>/run/result/c" +ExecStart=bash -c "mount >>/run/result/c" BindPaths=${image_dir}/result:/run/result Type=oneshot EOF @@ -194,6 +196,6 @@ grep -q -F "MARKER=1" ${image_dir}/result/c grep -F "squashfs" ${image_dir}/result/c | grep -q -F "noatime" grep -F "squashfs" ${image_dir}/result/c | grep -q -F -v "nosuid" -echo OK > /testok +echo OK >/testok exit 0