From d76f0de746f4ee7c9014f42b531ba0449b834214 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 27 Jan 2022 14:07:20 +0000 Subject: [PATCH 1/4] test: rename service used in TEST-29-PORTABLE to avoid conflict There's an app0.service in the extension app0.raw, so don't use the same name for a unit in minimal.raw --- test/test-functions | 14 +++++++++----- test/units/testsuite-29.sh | 36 ++++++++++++++++++------------------ 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/test/test-functions b/test/test-functions index ba18a0ea2d..40c7d2ac38 100644 --- a/test/test-functions +++ b/test/test-functions @@ -598,17 +598,21 @@ install_verity_minimal() { touch "$initdir/etc/machine-id" "$initdir/etc/resolv.conf" touch "$initdir/opt/some_file" echo MARKER=1 >>"$initdir/usr/lib/os-release" - echo "PORTABLE_PREFIXES=app0 minimal" >>"$initdir/usr/lib/os-release" - echo -e "[Service]\nExecStartPre=cat /usr/lib/os-release\nExecStart=sleep 120" >"$initdir/usr/lib/systemd/system/app0.service" - cp "$initdir/usr/lib/systemd/system/app0.service" "$initdir/usr/lib/systemd/system/app0-foo.service" + echo "PORTABLE_PREFIXES=app0 minimal minimal-app0" >>"$initdir/usr/lib/os-release" + cat >"$initdir/usr/lib/systemd/system/minimal-app0.service" <"$oldinitdir/usr/share/minimal_0.roothash" sed -i "s/MARKER=1/MARKER=2/g" "$initdir/usr/lib/os-release" - rm "$initdir/usr/lib/systemd/system/app0-foo.service" - cp "$initdir/usr/lib/systemd/system/app0.service" "$initdir/usr/lib/systemd/system/app0-bar.service" + rm "$initdir/usr/lib/systemd/system/minimal-app0-foo.service" + cp "$initdir/usr/lib/systemd/system/minimal-app0.service" "$initdir/usr/lib/systemd/system/minimal-app0-bar.service" mksquashfs "$initdir" "$oldinitdir/usr/share/minimal_1.raw" veritysetup format "$oldinitdir/usr/share/minimal_1.raw" "$oldinitdir/usr/share/minimal_1.verity" | \ diff --git a/test/units/testsuite-29.sh b/test/units/testsuite-29.sh index fdb84b6b45..4dbb3a0752 100755 --- a/test/units/testsuite-29.sh +++ b/test/units/testsuite-29.sh @@ -24,29 +24,29 @@ cat </run/systemd/system/systemd-portabled.service.d/override.conf Environment=SYSTEMD_LOG_LEVEL=debug EOF -portablectl "${ARGS[@]}" attach --now --runtime /usr/share/minimal_0.raw app0 +portablectl "${ARGS[@]}" attach --now --runtime /usr/share/minimal_0.raw minimal-app0 -systemctl is-active app0.service -systemctl is-active app0-foo.service +systemctl is-active minimal-app0.service +systemctl is-active minimal-app0-foo.service set +o pipefail set +e -systemctl is-active app0-bar.service && exit 1 +systemctl is-active minimal-app0-bar.service && exit 1 set -e set -o pipefail -portablectl "${ARGS[@]}" reattach --now --runtime /usr/share/minimal_1.raw app0 +portablectl "${ARGS[@]}" reattach --now --runtime /usr/share/minimal_1.raw minimal-app0 -systemctl is-active app0.service -systemctl is-active app0-bar.service +systemctl is-active minimal-app0.service +systemctl is-active minimal-app0-bar.service set +o pipefail set +e -systemctl is-active app0-foo.service && exit 1 +systemctl is-active minimal-app0-foo.service && exit 1 set -e set -o pipefail portablectl list | grep -q -F "minimal_1" -portablectl detach --now --runtime /usr/share/minimal_1.raw app0 +portablectl detach --now --runtime /usr/share/minimal_1.raw minimal-app0 portablectl list | grep -q -F "No images." @@ -55,29 +55,29 @@ portablectl list | grep -q -F "No images." unsquashfs -dest /tmp/minimal_0 /usr/share/minimal_0.raw unsquashfs -dest /tmp/minimal_1 /usr/share/minimal_1.raw -portablectl "${ARGS[@]}" attach --copy=symlink --now --runtime /tmp/minimal_0 app0 +portablectl "${ARGS[@]}" attach --copy=symlink --now --runtime /tmp/minimal_0 minimal-app0 -systemctl is-active app0.service -systemctl is-active app0-foo.service +systemctl is-active minimal-app0.service +systemctl is-active minimal-app0-foo.service set +o pipefail set +e -systemctl is-active app0-bar.service && exit 1 +systemctl is-active minimal-app0-bar.service && exit 1 set -e set -o pipefail -portablectl "${ARGS[@]}" reattach --now --enable --runtime /tmp/minimal_1 app0 +portablectl "${ARGS[@]}" reattach --now --enable --runtime /tmp/minimal_1 minimal-app0 -systemctl is-active app0.service -systemctl is-active app0-bar.service +systemctl is-active minimal-app0.service +systemctl is-active minimal-app0-bar.service set +o pipefail set +e -systemctl is-active app0-foo.service && exit 1 +systemctl is-active minimal-app0-foo.service && exit 1 set -e set -o pipefail portablectl list | grep -q -F "minimal_1" -portablectl detach --now --enable --runtime /tmp/minimal_1 app0 +portablectl detach --now --enable --runtime /tmp/minimal_1 minimal-app0 portablectl list | grep -q -F "No images." From 392d46d7a8f78169ff6b0d2740f82924e6fdc878 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 27 Jan 2022 14:08:05 +0000 Subject: [PATCH 2/4] test: use mksquashfs -noappend Makes the setup idempotent, as mksquashfs by default attempts to append to an existing image --- test/test-functions | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/test-functions b/test/test-functions index 40c7d2ac38..f03a63ac04 100644 --- a/test/test-functions +++ b/test/test-functions @@ -606,7 +606,7 @@ ExecStart=sleep 120 EOF cp "$initdir/usr/lib/systemd/system/minimal-app0.service" "$initdir/usr/lib/systemd/system/minimal-app0-foo.service" - mksquashfs "$initdir" "$oldinitdir/usr/share/minimal_0.raw" + mksquashfs "$initdir" "$oldinitdir/usr/share/minimal_0.raw" -noappend veritysetup format "$oldinitdir/usr/share/minimal_0.raw" "$oldinitdir/usr/share/minimal_0.verity" | \ grep '^Root hash:' | cut -f2 | tr -d '\n' >"$oldinitdir/usr/share/minimal_0.roothash" @@ -614,7 +614,7 @@ EOF rm "$initdir/usr/lib/systemd/system/minimal-app0-foo.service" cp "$initdir/usr/lib/systemd/system/minimal-app0.service" "$initdir/usr/lib/systemd/system/minimal-app0-bar.service" - mksquashfs "$initdir" "$oldinitdir/usr/share/minimal_1.raw" + mksquashfs "$initdir" "$oldinitdir/usr/share/minimal_1.raw" -noappend veritysetup format "$oldinitdir/usr/share/minimal_1.raw" "$oldinitdir/usr/share/minimal_1.verity" | \ grep '^Root hash:' | cut -f2 | tr -d '\n' >"$oldinitdir/usr/share/minimal_1.roothash" @@ -642,7 +642,7 @@ cat /usr/lib/extension-release.d/extension-release.app0 EOF chmod +x "$initdir/opt/script0.sh" echo MARKER=1 >"$initdir/usr/lib/systemd/system/some_file" - mksquashfs "$initdir" "$oldinitdir/usr/share/app0.raw" + mksquashfs "$initdir" "$oldinitdir/usr/share/app0.raw" -noappend export initdir="$TESTDIR/app1" mkdir -p "$initdir/usr/lib/extension-release.d" "$initdir/usr/lib/systemd/system" "$initdir/opt" @@ -665,7 +665,7 @@ cat /usr/lib/extension-release.d/extension-release.app2 EOF chmod +x "$initdir/opt/script1.sh" echo MARKER=1 >"$initdir/usr/lib/systemd/system/other_file" - mksquashfs "$initdir" "$oldinitdir/usr/share/app1.raw" + mksquashfs "$initdir" "$oldinitdir/usr/share/app1.raw" -noappend ) } From 6d7c999ab5958d6f1f192e7d0a63b8c330a077cb Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 27 Jan 2022 14:08:44 +0000 Subject: [PATCH 3/4] core: add clearer debug log when setting up ExecDirectories symlinks fails --- src/core/namespace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index f3c6b58f86..c8dc6c0503 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -1804,7 +1804,7 @@ static int apply_mounts( * exist, which means this will be a no-op. */ r = create_symlinks_from_tuples(root, exec_dir_symlinks); if (r < 0) - return r; + return log_debug_errno(r, "Failed to set up ExecDirectories symlinks inside mount namespace: %m"); /* Create a deny list we can pass to bind_mount_recursive() */ deny_list = new(char*, (*n_mounts)+1); From 3fa80e5e75a98ef6f9a84b01770b71a1774478dc Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Thu, 27 Jan 2022 14:10:34 +0000 Subject: [PATCH 4/4] core: do not attempt to add 'private' symlinks when RootImage/RootDirectory are used A bind mount is added directly from private on the host to the actual destination directory, no need for the symlinks (which cannot be created as the bind mount happens first and creates the target as an actual directory) Fixes https://github.com/systemd/systemd/issues/22264 --- src/core/execute.c | 2 +- test/test-functions | 10 +++++++++- test/units/testsuite-29.sh | 9 +++++++++ test/units/testsuite-50.sh | 6 ++++-- 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index acc59ef9db..d3266a9ab5 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -3373,7 +3373,7 @@ static int compile_symlinks( return r; } - if (!exec_directory_is_private(context, dt)) + if (!exec_directory_is_private(context, dt) || exec_context_with_rootfs(context)) continue; private_path = path_join(params->prefix[dt], "private", context->directories[dt].items[i].path); diff --git a/test/test-functions b/test/test-functions index f03a63ac04..dcb54ec243 100644 --- a/test/test-functions +++ b/test/test-functions @@ -576,7 +576,8 @@ install_verity_minimal() { oldinitdir="$initdir" rm -rfv "$TESTDIR/minimal" export initdir="$TESTDIR/minimal" - mkdir -p "$initdir/usr/lib/systemd/system" "$initdir/usr/lib/extension-release.d" "$initdir/etc" "$initdir/var/tmp" "$initdir/opt" + # app0 will use TemporaryFileSystem=/var/lib, app1 will need the mount point in the base image + mkdir -p "$initdir/usr/lib/systemd/system" "$initdir/usr/lib/extension-release.d" "$initdir/etc" "$initdir/var/tmp" "$initdir/opt" "$initdir/var/lib/app1" setup_basic_dirs install_basic_tools # Shellcheck treats [[ -v VAR ]] as an assignment to avoid a different @@ -633,11 +634,15 @@ EOF Type=oneshot RemainAfterExit=yes ExecStart=/opt/script0.sh +TemporaryFileSystem=/var/lib +StateDirectory=app0 +RuntimeDirectory=app0 EOF cat >"$initdir/opt/script0.sh" < \${STATE_DIRECTORY}/foo cat /usr/lib/extension-release.d/extension-release.app0 EOF chmod +x "$initdir/opt/script0.sh" @@ -656,11 +661,14 @@ EOF Type=oneshot RemainAfterExit=yes ExecStart=/opt/script1.sh +StateDirectory=app1 +RuntimeDirectory=app1 EOF cat >"$initdir/opt/script1.sh" < \${STATE_DIRECTORY}/foo cat /usr/lib/extension-release.d/extension-release.app2 EOF chmod +x "$initdir/opt/script1.sh" diff --git a/test/units/testsuite-29.sh b/test/units/testsuite-29.sh index 4dbb3a0752..1b927a8305 100755 --- a/test/units/testsuite-29.sh +++ b/test/units/testsuite-29.sh @@ -6,10 +6,13 @@ set -eux set -o pipefail ARGS=() +state_directory=/var/lib/private/ if [[ -v ASAN_OPTIONS || -v UBSAN_OPTIONS ]]; then # If we're running under sanitizers, we need to use a less restrictive # profile, otherwise LSan syscall would get blocked by seccomp ARGS+=(--profile=trusted) + # With the trusted profile DynamicUser is disabled, so the storage is not in private/ + state_directory=/var/lib/ fi systemd-dissect --no-pager /usr/share/minimal_0.raw | grep -q '✓ portable service' @@ -109,6 +112,12 @@ status="$(portablectl is-attached --extension app1 minimal_1)" portablectl detach --now --runtime --extension /usr/share/app1.raw /usr/share/minimal_1.raw app1 +# Ensure that the combination of read-only images, state directory and dynamic user works, and that +# state is retained. Check after detaching, as on slow systems (eg: sanitizers) it might take a while +# after the service is attached before the file appears. +grep -q -F bar "${state_directory}/app0/foo" +grep -q -F baz "${state_directory}/app1/foo" + # portablectl also works with directory paths rather than images mkdir /tmp/rootdir /tmp/app0 /tmp/app1 /tmp/overlay /tmp/os-release-fix /tmp/os-release-fix/etc diff --git a/test/units/testsuite-50.sh b/test/units/testsuite-50.sh index ff4f77def2..c79adba5e1 100755 --- a/test/units/testsuite-50.sh +++ b/test/units/testsuite-50.sh @@ -308,7 +308,8 @@ systemd-run -P --property ExtensionImages="/usr/share/app0.raw /usr/share/app1.r cat >/run/systemd/system/testservice-50e.service <