From: Luca Boccassi Date: Thu, 22 Jul 2021 19:41:34 +0000 (+0100) Subject: extension-release: search for other files if expected name not found X-Git-Tag: v250-rc1~819^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=9a4b883be234d3bfae7b3b38db490b67b36c09a2;p=thirdparty%2Fsystemd.git extension-release: search for other files if expected name not found In some cases image names are unpredictable - some orchestrators/deployment tools like to mangle names to suit their internal formats. In these cases, the requirement that the extension-release file matches exactly the image name where it's contained cannot work. Allow falling back to loading the first regular file which name starts with 'extension-release' located in /usr/lib/extension-release.d/ and tagged with a user.extension-release.strict extended attribute with a true value, if the one with the expected name cannot be found. --- diff --git a/src/basic/os-util.c b/src/basic/os-util.c index 51c685bc6a8..1d69ecc5862 100644 --- a/src/basic/os-util.c +++ b/src/basic/os-util.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #include "alloc-util.h" +#include "dirent-util.h" #include "env-file.h" #include "env-util.h" #include "fd-util.h" @@ -8,10 +9,13 @@ #include "fs-util.h" #include "macro.h" #include "os-util.h" +#include "parse-util.h" #include "path-util.h" +#include "stat-util.h" #include "string-util.h" #include "strv.h" #include "utf8.h" +#include "xattr-util.h" bool image_name_is_valid(const char *s) { if (!filename_is_valid(s)) @@ -41,8 +45,8 @@ int path_is_extension_tree(const char *path, const char *extension) { if (laccess(path, F_OK) < 0) return -errno; - /* We use /usr/lib/extension-release.d/extension-release.NAME as flag file if something is a system extension, - * and {/etc|/usr/lib}/os-release as flag file if something is an OS (in case extension == NULL) */ + /* We use /usr/lib/extension-release.d/extension-release[.NAME] as flag for something being a system extension, + * and {/etc|/usr/lib}/os-release as a flag for something being an OS (when not an extension). */ r = open_extension_release(path, extension, NULL, NULL); if (r == -ENOENT) /* We got nothing */ return 0; @@ -67,6 +71,91 @@ int open_extension_release(const char *root, const char *extension, char **ret_p r = chase_symlinks(extension_full_path, root, CHASE_PREFIX_ROOT, ret_path ? &q : NULL, ret_fd ? &fd : NULL); + /* Cannot find the expected extension-release file? The image filename might have been + * mangled on deployment, so fallback to checking for any file in the extension-release.d + * directory, and return the first one with a user.extension-release xattr instead. + * The user.extension-release.strict xattr is checked to ensure the author of the image + * considers it OK if names do not match. */ + if (r == -ENOENT) { + _cleanup_free_ char *extension_release_dir_path = NULL; + _cleanup_closedir_ DIR *extension_release_dir = NULL; + + r = chase_symlinks_and_opendir("/usr/lib/extension-release.d/", root, CHASE_PREFIX_ROOT, + &extension_release_dir_path, &extension_release_dir); + if (r < 0) + return r; + + r = -ENOENT; + struct dirent *de; + FOREACH_DIRENT(de, extension_release_dir, return -errno) { + int k; + + if (!IN_SET(de->d_type, DT_REG, DT_UNKNOWN)) + continue; + + const char *image_name = startswith(de->d_name, "extension-release."); + if (!image_name) + continue; + + if (!image_name_is_valid(image_name)) + continue; + + /* We already chased the directory, and checked that + * this is a real file, so we shouldn't fail to open it. */ + _cleanup_close_ int extension_release_fd = openat(dirfd(extension_release_dir), + de->d_name, + O_PATH|O_CLOEXEC|O_NOFOLLOW); + if (extension_release_fd < 0) + return log_debug_errno(errno, + "Failed to open extension-release file %s/%s: %m", + extension_release_dir_path, + de->d_name); + + /* Really ensure it is a regular file after we open it. */ + if (fd_verify_regular(extension_release_fd) < 0) + continue; + + /* No xattr or cannot parse it? Then skip this. */ + _cleanup_free_ char *extension_release_xattr = NULL; + k = fgetxattrat_fake_malloc(extension_release_fd, NULL, "user.extension-release.strict", AT_EMPTY_PATH, &extension_release_xattr); + if (k < 0 && !ERRNO_IS_NOT_SUPPORTED(k) && k != -ENODATA) + log_debug_errno(k, + "Failed to read 'user.extension-release.strict' extended attribute from extension-release file %s/%s: %m", + extension_release_dir_path, + de->d_name); + if (k < 0) + continue; + + /* Explicitly set to request strict matching? Skip it. */ + k = parse_boolean(extension_release_xattr); + if (k < 0) + log_debug_errno(k, + "Failed to parse 'user.extension-release.strict' extended attribute value from extension-release file %s/%s: %m", + extension_release_dir_path, + de->d_name); + if (k < 0 || k > 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 fallback path.*/ + if (r == 0) { + r = -ENOTUNIQ; + break; + } + + r = 0; /* Found it! */ + + if (ret_fd) + fd = TAKE_FD(extension_release_fd); + + if (ret_path) { + q = path_join(extension_release_dir_path, de->d_name); + if (!q) + return -ENOMEM; + } + } + } } else { const char *p; diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index 27b9ac9569e..32b21eb71b0 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -2538,13 +2538,13 @@ int dissected_image_acquire_metadata(DissectedImage *m) { _META_MAX, }; - static const char *paths[_META_MAX] = { + static const char *const paths[_META_MAX] = { [META_HOSTNAME] = "/etc/hostname\0", [META_MACHINE_ID] = "/etc/machine-id\0", [META_MACHINE_INFO] = "/etc/machine-info\0", - [META_OS_RELEASE] = "/etc/os-release\0" - "/usr/lib/os-release\0", - [META_EXTENSION_RELEASE] = NULL, + [META_OS_RELEASE] = ("/etc/os-release\0" + "/usr/lib/os-release\0"), + [META_EXTENSION_RELEASE] = "extension-release\0", /* Used only for logging. */ }; _cleanup_strv_free_ char **machine_info = NULL, **os_release = NULL, **extension_release = NULL; @@ -2561,17 +2561,6 @@ int dissected_image_acquire_metadata(DissectedImage *m) { assert(m); - /* As per the os-release spec, if the image is an extension it will have a file - * named after the image name in extension-release.d/ */ - if (m->image_name) { - char *ext; - - ext = strjoina("/usr/lib/extension-release.d/extension-release.", m->image_name, "0"); - ext[strlen(ext) - 1] = '\0'; /* Extra \0 for NULSTR_FOREACH using placeholder from above */ - paths[META_EXTENSION_RELEASE] = ext; - } else - log_debug("No image name available, will skip extension-release metadata"); - for (; n_meta_initialized < _META_MAX; n_meta_initialized ++) { if (!paths[n_meta_initialized]) { fds[2*n_meta_initialized] = fds[2*n_meta_initialized+1] = -1; @@ -2625,11 +2614,25 @@ int dissected_image_acquire_metadata(DissectedImage *m) { fds[2*k] = safe_close(fds[2*k]); - NULSTR_FOREACH(p, paths[k]) { - fd = chase_symlinks_and_open(p, t, CHASE_PREFIX_ROOT, O_RDONLY|O_CLOEXEC|O_NOCTTY, NULL); - if (fd >= 0) - break; - } + if (k == META_EXTENSION_RELEASE) { + /* As per the os-release spec, if the image is an extension it will have a file + * named after the image name in extension-release.d/ - we use the image name + * and try to resolve it with the extension-release helpers, as sometimes + * the image names are mangled on deployment and do not match anymore. + * Unlike other paths this is not fixed, and the image name + * can be mangled on deployment, so by calling into the helper + * we allow a fallback that matches on the first extension-release + * file found in the directory, if one named after the image cannot + * be found first. */ + r = open_extension_release(t, m->image_name, NULL, &fd); + if (r < 0) + fd = r; /* Propagate the error. */ + } else + NULSTR_FOREACH(p, paths[k]) { + fd = chase_symlinks_and_open(p, t, CHASE_PREFIX_ROOT, O_RDONLY|O_CLOEXEC|O_NOCTTY, NULL); + if (fd >= 0) + break; + } if (fd < 0) { log_debug_errno(fd, "Failed to read %s file of image, ignoring: %m", paths[k]); fds[2*k+1] = safe_close(fds[2*k+1]); diff --git a/test/test-functions b/test/test-functions index abe421c505c..27508957deb 100644 --- a/test/test-functions +++ b/test/test-functions @@ -626,8 +626,9 @@ EOF export initdir="$TESTDIR/app1" mkdir -p "$initdir/usr/lib/extension-release.d" "$initdir/usr/lib/systemd/system" "$initdir/opt" - grep "^ID=" "$os_release" >"$initdir/usr/lib/extension-release.d/extension-release.app1" - echo "${version_id}" >>"$initdir/usr/lib/extension-release.d/extension-release.app1" + grep "^ID=" "$os_release" >"$initdir/usr/lib/extension-release.d/extension-release.app2" + echo "${version_id}" >>"$initdir/usr/lib/extension-release.d/extension-release.app2" + setfattr -n user.extension-release.strict -v false "$initdir/usr/lib/extension-release.d/extension-release.app2" cat >"$initdir/usr/lib/systemd/system/app1.service" <"$initdir/usr/lib/systemd/system/other_file" diff --git a/test/units/testsuite-29.sh b/test/units/testsuite-29.sh index 3408e6d71ac..77bb6db15a7 100755 --- a/test/units/testsuite-29.sh +++ b/test/units/testsuite-29.sh @@ -5,6 +5,11 @@ set -eux set -o pipefail export SYSTEMD_LOG_LEVEL=debug +mkdir -p /run/systemd/system/systemd-portabled.service.d/ +cat </run/systemd/system/systemd-portabled.service.d/override.conf +[Service] +Environment=SYSTEMD_LOG_LEVEL=debug +EOF portablectl attach --now --runtime /usr/share/minimal_0.raw app0 @@ -63,24 +68,31 @@ portablectl detach --now --enable --runtime /tmp/minimal_1 app0 portablectl list | grep -q -F "No images." -root="/usr/share/minimal_0.raw" -app1="/usr/share/app1.raw" +portablectl attach --now --runtime --extension /usr/share/app0.raw /usr/share/minimal_0.raw app0 -portablectl attach --now --runtime --extension ${app1} ${root} app1 +systemctl is-active app0.service + +portablectl reattach --now --runtime --extension /usr/share/app0.raw /usr/share/minimal_1.raw app0 + +systemctl is-active app0.service + +portablectl detach --now --runtime --extension /usr/share/app0.raw /usr/share/minimal_1.raw app0 + +portablectl attach --now --runtime --extension /usr/share/app1.raw /usr/share/minimal_0.raw app1 systemctl is-active app1.service -portablectl reattach --now --runtime --extension ${app1} ${root} app1 +portablectl reattach --now --runtime --extension /usr/share/app1.raw /usr/share/minimal_1.raw app1 systemctl is-active app1.service -portablectl detach --now --runtime --extension ${app1} ${root} app1 +portablectl detach --now --runtime --extension /usr/share/app1.raw /usr/share/minimal_1.raw app1 # portablectl also works with directory paths rather than images mkdir /tmp/rootdir /tmp/app1 /tmp/overlay -mount ${app1} /tmp/app1 -mount ${root} /tmp/rootdir +mount /usr/share/app1.raw /tmp/app1 +mount /usr/share/minimal_0.raw /tmp/rootdir mount -t overlay overlay -o lowerdir=/tmp/app1:/tmp/rootdir /tmp/overlay portablectl attach --copy=symlink --now --runtime /tmp/overlay app1 diff --git a/test/units/testsuite-50.sh b/test/units/testsuite-50.sh index 081a8bcb953..9125918ea9d 100755 --- a/test/units/testsuite-50.sh +++ b/test/units/testsuite-50.sh @@ -235,7 +235,7 @@ systemd-run -P --property ExtensionImages=/usr/share/app0.raw --property RootIma systemd-run -P --property ExtensionImages=/usr/share/app0.raw --property RootImage="${image}.raw" cat /usr/lib/systemd/system/some_file | grep -q -F "MARKER=1" systemd-run -P --property ExtensionImages="/usr/share/app0.raw /usr/share/app1.raw" --property RootImage="${image}.raw" cat /opt/script0.sh | grep -q -F "extension-release.app0" systemd-run -P --property ExtensionImages="/usr/share/app0.raw /usr/share/app1.raw" --property RootImage="${image}.raw" cat /usr/lib/systemd/system/some_file | grep -q -F "MARKER=1" -systemd-run -P --property ExtensionImages="/usr/share/app0.raw /usr/share/app1.raw" --property RootImage="${image}.raw" cat /opt/script1.sh | grep -q -F "extension-release.app1" +systemd-run -P --property ExtensionImages="/usr/share/app0.raw /usr/share/app1.raw" --property RootImage="${image}.raw" cat /opt/script1.sh | grep -q -F "extension-release.app2" systemd-run -P --property ExtensionImages="/usr/share/app0.raw /usr/share/app1.raw" --property RootImage="${image}.raw" cat /usr/lib/systemd/system/other_file | grep -q -F "MARKER=1" cat >/run/systemd/system/testservice-50e.service <