From a2cf73f0b602a93a32107cfc066a5e307263c577 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 20 May 2022 12:05:34 +0100 Subject: [PATCH 1/5] dissect: ID from os-release should be non-empty, not just non-NULL --- src/shared/dissect-image.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index 5b0b12a265..1ab88839aa 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -3157,7 +3157,7 @@ int verity_dissect_and_mount( * First, check the distro ID. If that matches, then check the new SYSEXT_LEVEL value if * available, or else fallback to VERSION_ID. If neither is present (eg: rolling release), * then a simple match on the ID will be performed. */ - if (required_host_os_release_id) { + if (!isempty(required_host_os_release_id)) { _cleanup_strv_free_ char **extension_release = NULL; r = load_extension_release_pairs(dest, dissected_image->image_name, &extension_release); From 7b2e763242e7736ef941f275977aa0c30d832c63 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 20 May 2022 12:24:45 +0100 Subject: [PATCH 2/5] portable: reject root directories without an ID field in os-release We always require at least ID to be set in os-release, reject and propagate error to the caller instead of asserting later --- src/portable/portable.c | 2 ++ test/units/testsuite-29.sh | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/src/portable/portable.c b/src/portable/portable.c index 2191d6c653..c4f42f9500 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -582,6 +582,8 @@ static int extract_image_and_extensions( "PORTABLE_PREFIXES", &prefixes); if (r < 0) return r; + if (isempty(id)) + return sd_bus_error_set_errnof(error, SYNTHETIC_ERRNO(ESTALE), "Image %s os-release metadata lacks the ID field", name_or_path); if (prefixes) { valid_prefixes = strv_split(prefixes, WHITESPACE); diff --git a/test/units/testsuite-29.sh b/test/units/testsuite-29.sh index 47fd31b9b3..2a2aa95799 100755 --- a/test/units/testsuite-29.sh +++ b/test/units/testsuite-29.sh @@ -152,6 +152,16 @@ umount /tmp/rootdir umount /tmp/app0 umount /tmp/app1 +# Lack of ID field in os-release should be rejected, but it caused a crash in the past instead +mkdir -p /tmp/emptyroot/usr/lib +mkdir -p /tmp/emptyext/usr/lib/extension-release.d +touch /tmp/emptyroot/usr/lib/os-release +touch /tmp/emptyext/usr/lib/extension-release.d/extension-release.emptyext + +# Remote peer disconnected -> portabled crashed +res="$(! portablectl attach --extension /tmp/emptyext /tmp/emptyroot 2> >(grep "Remote peer disconnected"))" +test -z "${res}" + echo OK >/testok exit 0 From d316426eeef830e44a05248595eba8cdc7085d3b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 20 May 2022 16:49:14 +0900 Subject: [PATCH 3/5] shared/extension-release: drop unnecessary strna() wrapper These are not NULL. --- src/shared/extension-release.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/shared/extension-release.c b/src/shared/extension-release.c index dccc999907..db87b23a71 100644 --- a/src/shared/extension-release.c +++ b/src/shared/extension-release.c @@ -51,13 +51,13 @@ int extension_release_validate( extension_release_id = strv_env_pairs_get(extension_release, "ID"); if (isempty(extension_release_id)) { log_debug("Extension '%s' does not contain ID in extension-release but requested to match '%s'", - name, strna(host_os_release_id)); + name, host_os_release_id); return 0; } - if (!streq_ptr(host_os_release_id, extension_release_id)) { + if (!streq(host_os_release_id, extension_release_id)) { log_debug("Extension '%s' is for OS '%s', but deployed on top of '%s'.", - name, strna(extension_release_id), strna(host_os_release_id)); + name, extension_release_id, host_os_release_id); return 0; } From 4313e2b69fe1bcddd7b551e171f1fa3554155968 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 20 May 2022 14:53:22 +0200 Subject: [PATCH 4/5] portabled: refuse queries for empty image name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I took inspiration from pid1: bus_unit_find() → find_unit() → manager_load_unit_from_dbus_path() → unit_name_from_dbus_path() → !startswith(path, "/org/freedesktop/systemd1/unit/") → return -EINVAL ← ← ← ← if (r < 0) return 0 ← 0 ← i.e. we return 0 when queried for "/org/freedesktop/systemd1/unit". Fixes #23445. --- src/portable/portabled-image-bus.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/portable/portabled-image-bus.c b/src/portable/portabled-image-bus.c index 3fe0eca4c5..7d69d776eb 100644 --- a/src/portable/portabled-image-bus.c +++ b/src/portable/portabled-image-bus.c @@ -1120,6 +1120,9 @@ int bus_image_object_find( return 0; if (r == 0) goto not_found; + if (isempty(e)) + /* The path is "/org/freedesktop/portable1/image" itself */ + goto not_found; r = bus_image_acquire(m, sd_bus_get_current_message(bus), e, NULL, BUS_IMAGE_REFUSE_BY_PATH, NULL, &image, error); if (r == -ENOENT) From 5943d85f34bc39742291cb1a43f040d4bd581477 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 20 May 2022 14:53:50 +0200 Subject: [PATCH 5/5] portabled: wrap long lines and fix typo in error message --- src/portable/portabled-image-bus.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/portable/portabled-image-bus.c b/src/portable/portabled-image-bus.c index 7d69d776eb..55f09d6285 100644 --- a/src/portable/portabled-image-bus.c +++ b/src/portable/portabled-image-bus.c @@ -1045,19 +1045,23 @@ int bus_image_acquire( /* If it's a short name, let's search for it */ r = image_find(IMAGE_PORTABLE, name_or_path, NULL, &loaded); if (r == -ENOENT) - return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_PORTABLE_IMAGE, "No image '%s' found.", name_or_path); + return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_PORTABLE_IMAGE, + "No image '%s' found.", name_or_path); /* other errors are handled below… */ } else { /* Don't accept path if this is always forbidden */ if (mode == BUS_IMAGE_REFUSE_BY_PATH) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Expected image name, not path in place of '%s'.", name_or_path); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Expected image name, not path in place of '%s'.", name_or_path); if (!path_is_absolute(name_or_path)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image name '%s' is not valid or not a valid path.", name_or_path); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Image name '%s' is not valid or not a valid path.", name_or_path); if (!path_is_normalized(name_or_path)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Image path '%s' is not normalized.", name_or_path); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Image path '%s' is not normalized.", name_or_path); if (mode == BUS_IMAGE_AUTHENTICATE_BY_PATH) { r = bus_verify_polkit_async( @@ -1080,7 +1084,9 @@ int bus_image_acquire( r = image_from_path(name_or_path, &loaded); } if (r == -EMEDIUMTYPE) { - sd_bus_error_setf(error, BUS_ERROR_BAD_PORTABLE_IMAGE_TYPE, "Typ of image '%s' not recognized; supported image types are directories/btrfs subvolumes, block devices, and raw disk image files with suffix '.raw'.", name_or_path); + sd_bus_error_setf(error, BUS_ERROR_BAD_PORTABLE_IMAGE_TYPE, + "Type of image '%s' not recognized; supported image types are directories/btrfs subvolumes, block devices, and raw disk image files with suffix '.raw'.", + name_or_path); return r; } if (r < 0)