From 82047a6aa7230031d2a579e81d5f8871c23e0616 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Wed, 7 Feb 2024 00:36:39 +0000 Subject: [PATCH] portable: add --copy=mixed to copy images and link profiles This new mode copies resources provided by the client, so that they remain available for inspect/detach even if the original images are deleted, but symlinks the profile as that is owned by the OS, so that updates are automatically applied. --- man/org.freedesktop.portable1.xml | 17 ++++-- man/portablectl.xml | 14 +++-- src/portable/portable.c | 92 ++++++++++++++++++------------ src/portable/portable.h | 3 +- src/portable/portablectl.c | 5 +- src/portable/portabled-image-bus.c | 4 ++ test/units/testsuite-29.sh | 19 ++++++ 7 files changed, 104 insertions(+), 50 deletions(-) diff --git a/man/org.freedesktop.portable1.xml b/man/org.freedesktop.portable1.xml index a41da4f5c3c..9b49c610d55 100644 --- a/man/org.freedesktop.portable1.xml +++ b/man/org.freedesktop.portable1.xml @@ -229,16 +229,23 @@ node /org/freedesktop/portable1 { for the current boot session, and a string representing the preferred copy mode (whether to copy the image or to just symlink it) with the following possible values: - (null) + (empty) copy symlink + + mixed - This method returns the list of changes applied to the system (for example, which unit was - added and is now available as a system service). Each change is represented as a triplet of - strings: the type of change applied, the path on which it was applied, and the source - (if any). The type of change applied will be one of the following possible values: + If an empty string is passed the security profile drop-ins and images will be symlinked while unit + files will be copied, copy will copy, symlink will prefer + linking if possible (e.g.: a unit has to be copied out of an image), and mixed will + prefer linking the resources owned by the OS (e.g.: the portable profile located within the host's + /usr/ tree) but will copy the resources owned by the portable image (e.g.: the unit files and the + images). This method returns the list of changes applied to the system (for example, which unit was + added and is now available as a system service). Each change is represented as a triplet of strings: + the type of change applied, the path on which it was applied, and the source (if any). The type of + change applied will be one of the following possible values: copy diff --git a/man/portablectl.xml b/man/portablectl.xml index d0d00cf5d22..d241893d4de 100644 --- a/man/portablectl.xml +++ b/man/portablectl.xml @@ -321,12 +321,14 @@ - When attaching an image, select whether to prefer copying or symlinking of files installed into - the host system. Takes one of copy (to prefer copying of files), symlink - (to prefer creation of symbolic links) or auto for an intermediary mode where security - profile drop-ins are symlinked while unit files are copied. Note that this option expresses a preference only, - in cases where symbolic links cannot be created — for example when the image operated on is a raw disk image, - and hence not directly referentiable from the host file system — copying of files is used + When attaching an image, select whether to prefer copying or symlinking of files + installed into the host system. Takes one of copy (files will be copied), + symlink (to prefer creation of symbolic links), auto for an + intermediary mode where security profile drop-ins and images are symlinked while unit files are + copied, or mixed (since v256) where security profile drop-ins are symlinked while + unit files and images are copied. Note that this option expresses a preference only, in cases where + symbolic links cannot be created — for example when the image operated on is a raw disk image, and + hence not directly referentiable from the host file system — copying of files is used unconditionally. diff --git a/src/portable/portable.c b/src/portable/portable.c index 6054f0f17f8..27c18b117f2 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -33,6 +33,7 @@ #include "path-lookup.h" #include "portable.h" #include "process-util.h" +#include "rm-rf.h" #include "selinux-util.h" #include "set.h" #include "signal-util.h" @@ -1341,7 +1342,7 @@ static int attach_unit_file( return 0; } -static int image_symlink( +static int image_target_path( const char *image_path, PortableFlags flags, char **ret) { @@ -1367,37 +1368,66 @@ static int image_symlink( return 0; } -static int install_image_symlink( +static int install_image( const char *image_path, PortableFlags flags, PortableChange **changes, size_t *n_changes) { - _cleanup_free_ char *sl = NULL; + _cleanup_free_ char *target = NULL; int r; assert(image_path); - /* If the image is outside of the image search also link it into it, so that it can be found with short image - * names and is listed among the images. */ + /* If the image is outside of the image search also link it into it, so that it can be found with + * short image names and is listed among the images. If we are operating in mixed mode, the image is + * copied instead. */ if (image_in_search_path(IMAGE_PORTABLE, NULL, image_path)) return 0; - r = image_symlink(image_path, flags, &sl); + r = image_target_path(image_path, flags, &target); if (r < 0) return log_debug_errno(r, "Failed to generate image symlink path: %m"); - (void) mkdir_parents(sl, 0755); + (void) mkdir_parents(target, 0755); - if (symlink(image_path, sl) < 0) - return log_debug_errno(errno, "Failed to link %s %s %s: %m", image_path, special_glyph(SPECIAL_GLYPH_ARROW_RIGHT), sl); + if (flags & PORTABLE_MIXED_COPY_LINK) { + r = copy_tree(image_path, + target, + UID_INVALID, + GID_INVALID, + COPY_REFLINK | COPY_FSYNC | COPY_FSYNC_FULL | COPY_SYNCFS, + /* denylist= */ NULL, + /* subvolumes= */ NULL); + if (r < 0) + return log_debug_errno( + r, + "Failed to copy %s %s %s: %m", + image_path, + special_glyph(SPECIAL_GLYPH_ARROW_RIGHT), + target); - (void) portable_changes_add(changes, n_changes, PORTABLE_SYMLINK, sl, image_path); + } else { + if (symlink(image_path, target) < 0) + return log_debug_errno( + errno, + "Failed to link %s %s %s: %m", + image_path, + special_glyph(SPECIAL_GLYPH_ARROW_RIGHT), + target); + } + + (void) portable_changes_add( + changes, + n_changes, + (flags & PORTABLE_MIXED_COPY_LINK) ? PORTABLE_COPY : PORTABLE_SYMLINK, + target, + image_path); return 0; } -static int install_image_and_extensions_symlinks( +static int install_image_and_extensions( const Image *image, OrderedHashmap *extension_images, PortableFlags flags, @@ -1410,12 +1440,12 @@ static int install_image_and_extensions_symlinks( assert(image); ORDERED_HASHMAP_FOREACH(ext, extension_images) { - r = install_image_symlink(ext->path, flags, changes, n_changes); + r = install_image(ext->path, flags, changes, n_changes); if (r < 0) return r; } - r = install_image_symlink(image->path, flags, changes, n_changes); + r = install_image(image->path, flags, changes, n_changes); if (r < 0) return r; @@ -1608,9 +1638,9 @@ int portable_attach( return sd_bus_error_set_errnof(error, r, "Failed to attach unit '%s': %m", item->name); } - /* We don't care too much for the image symlink, it's just a convenience thing, it's not necessary for proper - * operation otherwise. */ - (void) install_image_and_extensions_symlinks(image, extension_images, flags, changes, n_changes); + /* We don't care too much for the image symlink/copy, it's just a convenience thing, it's not necessary for + * proper operation otherwise. */ + (void) install_image_and_extensions(image, extension_images, flags, changes, n_changes); log_portable_verb( "attached", @@ -1909,34 +1939,24 @@ int portable_detach( portable_changes_add_with_prefix(changes, n_changes, PORTABLE_UNLINK, where, md, NULL); } - /* Now, also drop any image symlink, for images outside of the sarch path */ + /* Now, also drop any image symlink or copy, for images outside of the sarch path */ SET_FOREACH(item, markers) { - _cleanup_free_ char *sl = NULL; - struct stat st; + _cleanup_free_ char *target = NULL; - r = image_symlink(item, flags, &sl); + r = image_target_path(item, flags, &target); if (r < 0) { - log_debug_errno(r, "Failed to determine image symlink for '%s', ignoring: %m", item); + log_debug_errno(r, "Failed to determine image path for '%s', ignoring: %m", item); continue; } - if (lstat(sl, &st) < 0) { - log_debug_errno(errno, "Failed to stat '%s', ignoring: %m", sl); - continue; - } + r = rm_rf(target, REMOVE_ROOT | REMOVE_PHYSICAL | REMOVE_MISSING_OK | REMOVE_SYNCFS); + if (r < 0) { + log_debug_errno(r, "Can't remove image '%s': %m", target); - if (!S_ISLNK(st.st_mode)) { - log_debug("Image '%s' is not a symlink, ignoring.", sl); - continue; - } - - if (unlink(sl) < 0) { - log_debug_errno(errno, "Can't remove image symlink '%s': %m", sl); - - if (errno != ENOENT && ret >= 0) - ret = -errno; + if (r != -ENOENT) + RET_GATHER(ret, r); } else - portable_changes_add(changes, n_changes, PORTABLE_UNLINK, sl, NULL); + portable_changes_add(changes, n_changes, PORTABLE_UNLINK, target, NULL); } /* Try to remove the unit file directory, if we can */ diff --git a/src/portable/portable.h b/src/portable/portable.h index c4a9d5103e6..f5bb1902bd3 100644 --- a/src/portable/portable.h +++ b/src/portable/portable.h @@ -27,7 +27,8 @@ typedef enum PortableFlags { PORTABLE_FORCE_EXTENSION = 1 << 2, /* Public API via DBUS, do not change */ PORTABLE_PREFER_COPY = 1 << 3, PORTABLE_PREFER_SYMLINK = 1 << 4, - PORTABLE_REATTACH = 1 << 5, + PORTABLE_MIXED_COPY_LINK = 1 << 5, + PORTABLE_REATTACH = 1 << 6, _PORTABLE_MASK_PUBLIC = PORTABLE_RUNTIME | PORTABLE_FORCE_ATTACH | PORTABLE_FORCE_EXTENSION, _PORTABLE_TYPE_MAX, _PORTABLE_TYPE_INVALID = -EINVAL, diff --git a/src/portable/portablectl.c b/src/portable/portablectl.c index cf3122e29c5..1867fc8a1d0 100644 --- a/src/portable/portablectl.c +++ b/src/portable/portablectl.c @@ -1372,12 +1372,13 @@ static int parse_argv(int argc, char *argv[]) { case ARG_COPY: if (streq(optarg, "auto")) arg_copy_mode = NULL; - else if (STR_IN_SET(optarg, "copy", "symlink")) + else if (STR_IN_SET(optarg, "copy", "symlink", "mixed")) arg_copy_mode = optarg; else if (streq(optarg, "help")) { puts("auto\n" "copy\n" - "symlink"); + "symlink\n" + "mixed\n"); return 0; } else return log_error_errno(SYNTHETIC_ERRNO(EINVAL), diff --git a/src/portable/portabled-image-bus.c b/src/portable/portabled-image-bus.c index 63f177eb74b..8db55742425 100644 --- a/src/portable/portabled-image-bus.c +++ b/src/portable/portabled-image-bus.c @@ -366,6 +366,8 @@ int bus_image_common_attach( flags |= PORTABLE_PREFER_SYMLINK; else if (streq(copy_mode, "copy")) flags |= PORTABLE_PREFER_COPY; + else if (streq(copy_mode, "mixed")) + flags |= PORTABLE_MIXED_COPY_LINK; else if (!isempty(copy_mode)) return sd_bus_reply_method_errorf(message, SD_BUS_ERROR_INVALID_ARGS, "Unknown copy mode '%s'", copy_mode); @@ -695,6 +697,8 @@ int bus_image_common_reattach( flags |= PORTABLE_PREFER_SYMLINK; else if (streq(copy_mode, "copy")) flags |= PORTABLE_PREFER_COPY; + else if (streq(copy_mode, "mixed")) + flags |= PORTABLE_MIXED_COPY_LINK; else if (!isempty(copy_mode)) return sd_bus_reply_method_errorf(message, SD_BUS_ERROR_INVALID_ARGS, "Unknown copy mode '%s'", copy_mode); diff --git a/test/units/testsuite-29.sh b/test/units/testsuite-29.sh index 536827311b9..d40cef2551a 100755 --- a/test/units/testsuite-29.sh +++ b/test/units/testsuite-29.sh @@ -203,6 +203,14 @@ portablectl inspect --force --cat --extension /usr/share/app0.raw --extension /u portablectl detach --now --runtime --extension /usr/share/app0.raw --extension /usr/share/conf0.raw /usr/share/minimal_0.raw app0 +# Ensure that mixed mode copies the images and units (client-owned) but symlinks the profile (OS owned) +portablectl "${ARGS[@]}" attach --copy=mixed --runtime --extension /usr/share/app0.raw /usr/share/minimal_0.raw app0 +test -f /run/portables/app0.raw +test -f /run/portables/minimal_0.raw +test -f /run/systemd/system.attached/app0.service +test -L /run/systemd/system.attached/app0.service.d/10-profile.conf +portablectl detach --runtime --extension /usr/share/app0.raw /usr/share/minimal_0.raw app0 + # 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 @@ -255,6 +263,17 @@ grep -q -F "LogExtraFields=PORTABLE_EXTENSION_NAME_AND_VERSION=app_1" /run/syste portablectl detach --now --runtime --extension /tmp/app0 --extension /tmp/app1 /tmp/rootdir app0 app1 +# Ensure that mixed mode copies the images and units (client-owned) but symlinks the profile (OS owned) +portablectl "${ARGS[@]}" attach --copy=mixed --runtime --extension /tmp/app0 --extension /tmp/app1 /tmp/rootdir app0 app1 +test -d /run/portables/app0 +test -d /run/portables/app1 +test -d /run/portables/rootdir +test -f /run/systemd/system.attached/app0.service +test -f /run/systemd/system.attached/app1.service +test -L /run/systemd/system.attached/app0.service.d/10-profile.conf +test -L /run/systemd/system.attached/app1.service.d/10-profile.conf +portablectl detach --runtime --extension /tmp/app0 --extension /tmp/app1 /tmp/rootdir app0 app1 + # Attempt to disable the app unit during detaching. Requires --copy=symlink to reproduce. # Provides coverage for https://github.com/systemd/systemd/issues/23481 portablectl "${ARGS[@]}" attach --copy=symlink --now --runtime /tmp/rootdir minimal-app0