From 37543971aff79f3a37646ffc2bb5845c9394797b Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Wed, 21 Feb 2024 19:31:14 +0000 Subject: [PATCH] os-util: allow matching versioned image with extension-release file Currently app_1.0.raw is refused if it contains extension-release.d/extension-release.app, which stops one from using versioned images without using the force flag to disable the check. Relax it so that only the actual name, and not the version, is compared, like it already happens in other places. --- src/basic/os-util.c | 55 ++++++++++++++++++++++++++++++++-- src/basic/os-util.h | 1 + src/portable/portable.c | 61 ++++++-------------------------------- test/units/testsuite-29.sh | 13 ++++++++ 4 files changed, 75 insertions(+), 55 deletions(-) diff --git a/src/basic/os-util.c b/src/basic/os-util.c index dbd067fd446..985d89bc7eb 100644 --- a/src/basic/os-util.c +++ b/src/basic/os-util.c @@ -61,6 +61,39 @@ bool image_name_is_valid(const char *s) { return true; } +int path_extract_image_name(const char *path, char **ret) { + _cleanup_free_ char *fn = NULL; + int r; + + assert(path); + assert(ret); + + /* Extract last component from path, without any "/" suffixes. */ + r = path_extract_filename(path, &fn); + if (r < 0) + return r; + + if (r != O_DIRECTORY) { + /* Chop off any image suffixes we recognize (unless we already know this must refer to some dir */ + FOREACH_STRING(suffix, ".sysext.raw", ".confext.raw", ".raw") { + char *m = endswith(fn, suffix); + if (m) { + *m = 0; + break; + } + } + } + + /* Truncate the version/counting suffixes */ + fn[strcspn(fn, "_+")] = 0; + + if (!image_name_is_valid(fn)) + return -EINVAL; + + *ret = TAKE_PTR(fn); + return 0; +} + int path_is_extension_tree(ImageClass image_class, const char *path, const char *extension, bool relax_extension_release_check) { int r; @@ -230,9 +263,25 @@ int open_extension_release_at( continue; } - if (!relax_extension_release_check && - extension_release_strict_xattr_value(fd, dir_path, de->d_name) != 0) - continue; + if (!relax_extension_release_check) { + _cleanup_free_ char *base_image_name = NULL, *base_extension = NULL; + + r = path_extract_image_name(image_name, &base_image_name); + if (r < 0) { + log_debug_errno(r, "Failed to extract image name from %s/%s, ignoring: %m", dir_path, de->d_name); + continue; + } + + r = path_extract_image_name(extension, &base_extension); + if (r < 0) { + log_debug_errno(r, "Failed to extract image name from %s, ignoring: %m", extension); + continue; + } + + if (!streq(base_image_name, base_extension) && + extension_release_strict_xattr_value(fd, dir_path, image_name) != 0) + continue; + } /* We already found what we were looking for, but there's another candidate? We treat this as * an error, as we want to enforce that there are no ambiguities in case we are in the diff --git a/src/basic/os-util.h b/src/basic/os-util.h index 7cee3dd119f..f6a12a3fb1c 100644 --- a/src/basic/os-util.h +++ b/src/basic/os-util.h @@ -25,6 +25,7 @@ ImageClass image_class_from_string(const char *s) _pure_; * in accordance with the OS extension specification, rather than for /usr/lib/ or /etc/os-release. */ bool image_name_is_valid(const char *s) _pure_; +int path_extract_image_name(const char *path, char **ret); int path_is_extension_tree(ImageClass image_class, const char *path, const char *extension, bool relax_extension_release_check); static inline int path_is_os_tree(const char *path) { diff --git a/src/portable/portable.c b/src/portable/portable.c index a3bec83f0dd..a83f7b68cbb 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -1666,7 +1666,6 @@ int portable_attach( static bool marker_matches_images(const char *marker, const char *name_or_path, char **extension_image_paths) { _cleanup_strv_free_ char **root_and_extensions = NULL; - const char *a; int r; assert(marker); @@ -1687,7 +1686,7 @@ static bool marker_matches_images(const char *marker, const char *name_or_path, return r; STRV_FOREACH(image_name_or_path, root_and_extensions) { - _cleanup_free_ char *image = NULL; + _cleanup_free_ char *image = NULL, *base_image = NULL, *base_image_name_or_path = NULL; r = extract_first_word(&marker, &image, ":", EXTRACT_UNQUOTE|EXTRACT_RETAIN_ESCAPE); if (r < 0) @@ -1695,58 +1694,16 @@ static bool marker_matches_images(const char *marker, const char *name_or_path, if (r == 0) return false; - a = last_path_component(image); - - if (image_name_is_valid(*image_name_or_path)) { - const char *e, *underscore; - - /* We shall match against an image name. In that case let's compare the last component, and optionally - * allow either a suffix of ".raw" or a series of "/". - * But allow matching on a different version of the same image, when a "_" is used as a separator. */ - underscore = strchr(*image_name_or_path, '_'); - if (underscore) { - if (strneq(a, *image_name_or_path, underscore - *image_name_or_path)) - continue; - return false; - } + r = path_extract_image_name(image, &base_image); + if (r < 0) + return log_debug_errno(r, "Failed to extract image name from %s, ignoring: %m", image); - e = startswith(a, *image_name_or_path); - if (!e) - return false; - - if(!(e[strspn(e, "/")] == 0 || streq(e, ".raw"))) - return false; - } else { - const char *b, *underscore; - size_t l; - - /* We shall match against a path. Let's ignore any prefix here though, as often there are many ways to - * reach the same file. However, in this mode, let's validate any file suffix. - * But also ensure that we don't fail if both components don't have a '/' at all - * (strcspn returns the full length of the string in that case, which might not - * match as the versions might differ). */ - - l = strcspn(a, "/"); - b = last_path_component(*image_name_or_path); - - if ((a[l] != '/') != !strchr(b, '/')) /* One is a directory, the other is not */ - return false; - - if (a[l] != 0 && strcspn(b, "/") != l) - return false; - - underscore = strchr(b, '_'); - if (underscore) - l = underscore - b; - else { /* Either component could be versioned */ - underscore = strchr(a, '_'); - if (underscore) - l = underscore - a; - } + r = path_extract_image_name(*image_name_or_path, &base_image_name_or_path); + if (r < 0) + return log_debug_errno(r, "Failed to extract image name from %s, ignoring: %m", *image_name_or_path); - if (!strneq(a, b, l)) - return false; - } + if (!streq(base_image, base_image_name_or_path)) + return false; } return true; diff --git a/test/units/testsuite-29.sh b/test/units/testsuite-29.sh index d40cef2551a..adafdbbecc5 100755 --- a/test/units/testsuite-29.sh +++ b/test/units/testsuite-29.sh @@ -139,6 +139,19 @@ grep -q -F "LogExtraFields=PORTABLE_EXTENSION_NAME_AND_VERSION=app" /run/systemd portablectl detach --now --runtime --extension /usr/share/app0.raw /usr/share/minimal_1.raw app0 +# Ensure versioned images are accepted without needing to use --force to override the extension-release +# matching + +cp /usr/share/app0.raw /tmp/app0_1.0.raw +portablectl "${ARGS[@]}" attach --now --runtime --extension /tmp/app0_1.0.raw /usr/share/minimal_0.raw app0 + +systemctl is-active app0.service +status="$(portablectl is-attached --extension app0_1 minimal_0)" +[[ "${status}" == "running-runtime" ]] + +portablectl detach --now --runtime --extension /tmp/app0_1.0.raw /usr/share/minimal_1.raw app0 +rm -f /tmp/app0_1.0.raw + portablectl "${ARGS[@]}" attach --now --runtime --extension /usr/share/app1.raw /usr/share/minimal_0.raw app1 systemctl is-active app1.service -- 2.47.3