Merge pull request #27044 from bluca/sysext_recursive_dir

Ensure sysexts do not contain an os-release file, do not load sysexts from /usr[/local]/lib/extensions/
This commit is contained in:
Lennart Poettering 2023-04-03 12:38:48 +02:00 committed by GitHub
commit 6b868766eb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 101 additions and 71 deletions

View file

@ -84,9 +84,8 @@
them they may optionally carry Verity authentication information.</para>
<para>System extensions are automatically looked for in the directories
<filename>/etc/extensions/</filename>, <filename>/run/extensions/</filename>,
<filename>/var/lib/extensions/</filename>, <filename>/usr/lib/extensions/</filename> and
<filename>/usr/local/lib/extensions/</filename>. The first two listed directories are not suitable for
<filename>/etc/extensions/</filename>, <filename>/run/extensions/</filename> and
<filename>/var/lib/extensions/</filename>. The first two listed directories are not suitable for
carrying large binary images, however are still useful for carrying symlinks to them. The primary place
for installing system extensions is <filename>/var/lib/extensions/</filename>. Any directories found in
these search directories are considered directory based extension images, any files with the

View file

@ -18,7 +18,7 @@
#include "devnum-util.h"
#include "env-util.h"
#include "escape.h"
#include "extension-release.h"
#include "extension-util.h"
#include "fd-util.h"
#include "format-util.h"
#include "glyph-util.h"

View file

@ -16,7 +16,7 @@
#include "env-util.h"
#include "errno-list.h"
#include "escape.h"
#include "extension-release.h"
#include "extension-util.h"
#include "fd-util.h"
#include "fileio.h"
#include "fs-util.h"

View file

@ -22,6 +22,7 @@
#include "dissect-image.h"
#include "env-file.h"
#include "env-util.h"
#include "extension-util.h"
#include "fd-util.h"
#include "fs-util.h"
#include "hashmap.h"
@ -58,11 +59,13 @@ static const char* const image_search_path[_IMAGE_CLASS_MAX] = {
"/usr/local/lib/portables\0"
"/usr/lib/portables\0",
/* Note that we don't allow storing extensions under /usr/, unlike with other image types. That's
* because extension images are supposed to extend /usr/, so you get into recursive races, especially
* with directory-based extensions, as the kernel's OverlayFS explicitly checks for this and errors
* out with -ELOOP if it finds that a lowerdir= is a child of another lowerdir=. */
[IMAGE_EXTENSION] = "/etc/extensions\0" /* only place symlinks here */
"/run/extensions\0" /* and here too */
"/var/lib/extensions\0" /* the main place for images */
"/usr/local/lib/extensions\0"
"/usr/lib/extensions\0",
"/var/lib/extensions\0", /* the main place for images */
};
static Image *image_free(Image *i) {
@ -1149,6 +1152,16 @@ int image_read_metadata(Image *i) {
_cleanup_free_ char *hostname = NULL;
_cleanup_free_ char *path = NULL;
if (i->class == IMAGE_EXTENSION) {
r = extension_has_forbidden_content(i->path);
if (r < 0)
return r;
if (r > 0)
return log_debug_errno(SYNTHETIC_ERRNO(ENOMEDIUM),
"Conflicting content found in image %s, refusing.",
i->name);
}
r = chase("/etc/hostname", i->path, CHASE_PREFIX_ROOT|CHASE_TRAIL_SLASH, &path, NULL);
if (r < 0 && r != -ENOENT)
log_debug_errno(r, "Failed to chase /etc/hostname in image %s: %m", i->name);

View file

@ -39,7 +39,7 @@
#include "dm-util.h"
#include "env-file.h"
#include "env-util.h"
#include "extension-release.h"
#include "extension-util.h"
#include "fd-util.h"
#include "fileio.h"
#include "fs-util.h"
@ -1787,11 +1787,16 @@ int dissected_image_mount(
ok = true;
}
if (!ok && FLAGS_SET(flags, DISSECT_IMAGE_VALIDATE_OS_EXT)) {
r = path_is_extension_tree(where, m->image_name, FLAGS_SET(flags, DISSECT_IMAGE_RELAX_SYSEXT_CHECK));
r = extension_has_forbidden_content(where);
if (r < 0)
return r;
if (r > 0)
ok = true;
if (r == 0) {
r = path_is_extension_tree(where, m->image_name, FLAGS_SET(flags, DISSECT_IMAGE_RELAX_SYSEXT_CHECK));
if (r < 0)
return r;
if (r > 0)
ok = true;
}
}
if (!ok)

View file

@ -2,8 +2,9 @@
#include "alloc-util.h"
#include "architecture.h"
#include "chase.h"
#include "env-util.h"
#include "extension-release.h"
#include "extension-util.h"
#include "log.h"
#include "os-util.h"
#include "strv.h"
@ -135,3 +136,20 @@ int parse_env_extension_hierarchies(char ***ret_hierarchies) {
*ret_hierarchies = TAKE_PTR(l);
return 0;
}
int extension_has_forbidden_content(const char *root) {
int r;
/* Insist that extension images do not overwrite the underlying OS release file (it's fine if
* they place one in /etc/os-release, i.e. where things don't matter, as they aren't
* merged.) */
r = chase("/usr/lib/os-release", root, CHASE_PREFIX_ROOT, NULL, NULL);
if (r > 0) {
log_debug("Extension contains '/usr/lib/os-release', which is not allowed, refusing.");
return 1;
}
if (r < 0 && r != -ENOENT)
return log_debug_errno(r, "Failed to determine whether '/usr/lib/os-release' exists in the extension: %m");
return 0;
}

View file

@ -14,3 +14,7 @@ int extension_release_validate(
/* Parse SYSTEMD_SYSEXT_HIERARCHIES and if not set, return "/usr /opt" */
int parse_env_extension_hierarchies(char ***ret_hierarchies);
/* Insist that extension images do not overwrite the underlying OS release file (it's fine if they place one
* in /etc/os-release, i.e. where things don't matter, as they aren't merged.) */
int extension_has_forbidden_content(const char *root);

View file

@ -64,7 +64,7 @@ shared_sources = files(
'ethtool-util.c',
'exec-util.c',
'exit-status.c',
'extension-release.c',
'extension-util.c',
'fdset.c',
'fileio-label.c',
'find-esp.c',

View file

@ -15,7 +15,7 @@
#include "dissect-image.h"
#include "env-util.h"
#include "escape.h"
#include "extension-release.h"
#include "extension-util.h"
#include "fd-util.h"
#include "fileio.h"
#include "format-table.h"
@ -408,47 +408,6 @@ static int strverscmp_improvedp(char *const* a, char *const* b) {
return strverscmp_improved(*a, *b);
}
static int validate_version(
const char *root,
const Image *img,
const char *host_os_release_id,
const char *host_os_release_version_id,
const char *host_os_release_sysext_level) {
int r;
assert(root);
assert(img);
if (arg_force) {
log_debug("Force mode enabled, skipping version validation.");
return 1;
}
/* Insist that extension images do not overwrite the underlying OS release file (it's fine if
* they place one in /etc/os-release, i.e. where things don't matter, as they aren't
* merged.) */
r = chase("/usr/lib/os-release", root, CHASE_PREFIX_ROOT, NULL, NULL);
if (r < 0) {
if (r != -ENOENT)
return log_error_errno(r, "Failed to determine whether /usr/lib/os-release exists in the extension image: %m");
} else
return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
"Extension image contains /usr/lib/os-release file, which is not allowed (it may carry /etc/os-release), refusing.");
r = extension_release_validate(
img->name,
host_os_release_id,
host_os_release_version_id,
host_os_release_sysext_level,
in_initrd() ? "initrd" : "system",
img->extension_release);
if (r < 0)
return log_error_errno(r, "Failed to validate extension release information: %m");
return r;
}
static int merge_subprocess(Hashmap *images, const char *workspace) {
_cleanup_free_ char *host_os_release_id = NULL, *host_os_release_version_id = NULL, *host_os_release_sysext_level = NULL,
*buf = NULL;
@ -505,6 +464,17 @@ static int merge_subprocess(Hashmap *images, const char *workspace) {
switch (img->type) {
case IMAGE_DIRECTORY:
case IMAGE_SUBVOLUME:
if (!arg_force) {
r = extension_has_forbidden_content(p);
if (r < 0)
return r;
if (r > 0) {
n_ignored++;
continue;
}
}
r = mount_nofollow_verbose(LOG_ERR, img->path, p, NULL, MS_BIND, NULL);
if (r < 0)
return r;
@ -537,6 +507,9 @@ static int merge_subprocess(Hashmap *images, const char *workspace) {
if (verity_settings.data_path)
flags |= DISSECT_IMAGE_NO_PARTITION_TABLE;
if (!arg_force)
flags |= DISSECT_IMAGE_VALIDATE_OS_EXT;
r = loop_device_make_by_path(
img->path,
O_RDONLY,
@ -576,8 +549,12 @@ static int merge_subprocess(Hashmap *images, const char *workspace) {
UID_INVALID,
UID_INVALID,
flags);
if (r < 0)
if (r < 0 && r != -ENOMEDIUM)
return r;
if (r == -ENOMEDIUM && !arg_force) {
n_ignored++;
continue;
}
r = dissected_image_relinquish(m);
if (r < 0)
@ -588,17 +565,22 @@ static int merge_subprocess(Hashmap *images, const char *workspace) {
assert_not_reached();
}
r = validate_version(
p,
img,
host_os_release_id,
host_os_release_version_id,
host_os_release_sysext_level);
if (r < 0)
return r;
if (r == 0) {
n_ignored++;
continue;
if (arg_force)
log_debug("Force mode enabled, skipping version validation.");
else {
r = extension_release_validate(
img->name,
host_os_release_id,
host_os_release_version_id,
host_os_release_sysext_level,
in_initrd() ? "initrd" : "system",
img->extension_release);
if (r < 0)
return r;
if (r == 0) {
n_ignored++;
continue;
}
}
/* Nice! This one is an extension we want. */
@ -612,7 +594,7 @@ static int merge_subprocess(Hashmap *images, const char *workspace) {
/* Nothing left? Then shortcut things */
if (n_extensions == 0) {
if (n_ignored > 0)
log_info("No suitable extensions found (%u ignored due to incompatible version).", n_ignored);
log_info("No suitable extensions found (%u ignored due to incompatible image(s)).", n_ignored);
else
log_info("No extensions found.");
return 0;

View file

@ -413,6 +413,17 @@ systemd-sysext merge
test ! -e /usr/lib/systemd/system/some_file
systemd-sysext unmerge
rmdir /etc/extensions/app-nodistro
# Check that extensions cannot contain os-release
mkdir -p /run/extensions/app-reject/usr/lib/{extension-release.d/,systemd/system}
echo "ID=_any" >/run/extensions/app-reject/usr/lib/extension-release.d/extension-release.app-reject
echo "ID=_any" >/run/extensions/app-reject/usr/lib/os-release
touch /run/extensions/app-reject/usr/lib/systemd/system/other_file
systemd-sysext merge && { echo 'unexpected success'; exit 1; }
test ! -e /usr/lib/systemd/system/some_file
test ! -e /usr/lib/systemd/system/other_file
systemd-sysext unmerge
rm -rf /run/extensions/app-reject
rm /var/lib/extensions/app-nodistro.raw
mkdir -p /run/machines /run/portables /run/extensions

View file

@ -15,8 +15,6 @@ ConditionCapability=CAP_SYS_ADMIN
ConditionDirectoryNotEmpty=|/etc/extensions
ConditionDirectoryNotEmpty=|/run/extensions
ConditionDirectoryNotEmpty=|/var/lib/extensions
ConditionDirectoryNotEmpty=|/usr/local/lib/extensions
ConditionDirectoryNotEmpty=|/usr/lib/extensions
DefaultDependencies=no
After=local-fs.target