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/

11 files changed, 101 insertions, 71 deletions

diff --git a/man/systemd-sysext.xml b/man/systemd-sysext.xml
index 39a16d8e8f..258c7142c9 100644
--- a/man/systemd-sysext.xml
+++ b/man/systemd-sysext.xml
@@ -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
diff --git a/src/core/namespace.c b/src/core/namespace.c
index c75a2cb29f..8b141a2484 100644
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -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"
diff --git a/src/portable/portable.c b/src/portable/portable.c
index dbebebf4ce..adfd846bab 100644
--- a/src/portable/portable.c
+++ b/src/portable/portable.c
@@ -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"
diff --git a/src/shared/discover-image.c b/src/shared/discover-image.c
index fa018cb912..6aff9fbb30 100644
--- a/src/shared/discover-image.c
+++ b/src/shared/discover-image.c
@@ -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);
diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c
index 2920a51dc2..6000af0ce0 100644
--- a/src/shared/dissect-image.c
+++ b/src/shared/dissect-image.c
@@ -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)
diff --git a/src/shared/extension-release.c b/src/shared/extension-util.c
index 2da8e7ea94..fa83f6b6fd 100644
--- a/src/shared/extension-release.c
+++ b/src/shared/extension-util.c
@@ -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;
+}
diff --git a/src/shared/extension-release.h b/src/shared/extension-util.h
index 5c3fee24be..fba8acaf19 100644
--- a/src/shared/extension-release.h
+++ b/src/shared/extension-util.h
@@ -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);
diff --git a/src/shared/meson.build b/src/shared/meson.build
index cb0697d1b9..0f2e2d1a67 100644
--- a/src/shared/meson.build
+++ b/src/shared/meson.build
@@ -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',
diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c
index 539ab5e639..5632b72f3d 100644
--- a/src/sysext/sysext.c
+++ b/src/sysext/sysext.c
@@ -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;
diff --git a/test/units/testsuite-50.sh b/test/units/testsuite-50.sh
index af7d351154..546a915a2e 100755
--- a/test/units/testsuite-50.sh
+++ b/test/units/testsuite-50.sh
@@ -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
diff --git a/units/systemd-sysext.service b/units/systemd-sysext.service
index f8c26f5fbf..9a8d4ebc5f 100644
--- a/units/systemd-sysext.service
+++ b/units/systemd-sysext.service
@@ -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