core: deduplicate identical dm-verity ExtensionImages=

It turns out OverlayFS doesn't handle gracefully when the same source is
specified multiple times in lowerdir= and it fails with ELOOP:

Failed to mount overlay (type overlay) on /run/systemd/mount-rootfs/opt (MS_RDONLY "lowerdir=/run/systemd/unit-extensions/1/opt:/run/systemd/unit-extensions/0/opt:/run/systemd/mount-rootfs/opt"): Too many levels of symbolic links

This happens even if we mount each image in a different internal mount
path, as OverlayFS will resolve it and look for the backing device, which
will be the same device mapper entity, and return a hard error.
This error does not appear if dm-verity is not used, so it is very
confusing for users, and unnecessary.

When mounting ExtensionImages, check if an image is dm-veritied,
and drop duplicates if the root hashes match, to avoid this user-unfriendly
hard error.

diff --git a/src/core/namespace.c b/src/core/namespace.c
index 6d3cadf05cbb13a72a181c95c70ad844d891d745..57518ad7ff8402a73570d98b3d353800ca374f71 100644 (file)
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -112,6 +112,7 @@ typedef struct MountEntry {
         unsigned n_followed;
         LIST_HEAD(MountOptions, image_options_const);
         char **overlay_layers;
+        VeritySettings verity;
 } MountEntry;
 
 typedef struct MountList {
@@ -343,6 +344,7 @@ static void mount_entry_done(MountEntry *p) {
         p->source_malloc = mfree(p->source_malloc);
         p->options_malloc = mfree(p->options_malloc);
         p->overlay_layers = strv_free(p->overlay_layers);
+        verity_settings_done(&p->verity);
 }
 
 static void mount_list_done(MountList *ml) {
@@ -505,6 +507,7 @@ static int append_extensions(
         /* First, prepare a mount for each image, but these won't be visible to the unit, instead
          * they will be mounted in our propagate directory, and used as a source for the overlay. */
         for (size_t i = 0; i < n_mount_images; i++) {
+                _cleanup_(verity_settings_done) VeritySettings verity = VERITY_SETTINGS_DEFAULT;
                 _cleanup_(pick_result_done) PickResult result = PICK_RESULT_NULL;
                 _cleanup_free_ char *mount_point = NULL;
                 const MountImage *m = mount_images + i;
@@ -523,6 +526,10 @@ static int append_extensions(
                                         "No matching entry in .v/ directory %s found.",
                                         m->source);
 
+                r = verity_settings_load(&verity, result.path, /* root_hash_path= */ NULL, /* root_hash_sig_path= */ NULL);
+                if (r < 0)
+                        return log_debug_errno(r, "Failed to check verity root hash of %s: %m", result.path);
+
                 if (asprintf(&mount_point, "%s/unit-extensions/%zu", private_namespace_dir, i) < 0)
                         return -ENOMEM;
 
@@ -547,6 +554,7 @@ static int append_extensions(
                         .source_malloc = TAKE_PTR(result.path),
                         .mode = MOUNT_EXTENSION_IMAGE,
                         .has_prefix = true,
+                        .verity = TAKE_GENERIC(verity, VeritySettings, VERITY_SETTINGS_DEFAULT),
                 };
         }
 
@@ -787,6 +795,36 @@ static int prefix_where_needed(MountList *ml, const char *root_directory) {
         return 0;
 }
 
+static bool verity_has_later_duplicates(MountList *ml, const MountEntry *needle) {
+
+        assert(ml);
+        assert(needle);
+        assert(needle >= ml->mounts && needle < ml->mounts + ml->n_mounts);
+        assert(needle->mode == MOUNT_EXTENSION_IMAGE);
+
+        if (needle->verity.root_hash_size == 0)
+                return false;
+
+        /* Overlayfs rejects supplying the same directory inode twice as determined by filesystem UUID and
+         * file handle in lowerdir=, even if they are mounted on different paths, as it resolves each mount
+         * to its source filesystem, so drop duplicates, and keep the last one. This only covers non-DDI
+         * verity images. Note that the list is ordered, so we only check for the reminder of the list for
+         * each item, rather than the full list from the beginning, as any earlier duplicates will have
+         * already been pruned. */
+
+        for (const MountEntry *m = needle + 1; m < ml->mounts + ml->n_mounts; m++) {
+                if (m->mode != MOUNT_EXTENSION_IMAGE)
+                        continue;
+                if (memcmp_nn(m->verity.root_hash,
+                              m->verity.root_hash_size,
+                              needle->verity.root_hash,
+                              needle->verity.root_hash_size) == 0)
+                        return true;
+        }
+
+        return false;
+}
+
 static void drop_duplicates(MountList *ml) {
         MountEntry *f, *t, *previous;
 
@@ -810,6 +848,12 @@ static void drop_duplicates(MountList *ml) {
                         continue;
                 }
 
+                if (f->mode == MOUNT_EXTENSION_IMAGE && verity_has_later_duplicates(ml, f)) {
+                        log_debug("Skipping duplicate extension image %s", mount_entry_source(f));
+                        mount_entry_done(f);
+                        continue;
+                }
+
                 *t = *f;
                 previous = t;
                 t++;
@@ -1347,7 +1391,7 @@ static int mount_mqueuefs(const MountEntry *m) {
 }
 
 static int mount_image(
-                const MountEntry *m,
+                MountEntry *m,
                 const char *root_directory,
                 const ImagePolicy *image_policy) {
 
@@ -1387,6 +1431,7 @@ static int mount_image(
                         host_os_release_sysext_level,
                         host_os_release_confext_level,
                         /* required_sysext_scope= */ NULL,
+                        &m->verity,
                         /* ret_image= */ NULL);
         if (r == -ENOENT && m->ignore)
                 return 0;

diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c
index 0d23e5193103566eb266595833bfc5c4b1ffb82d..1a3ed0cf7f41378b9d98ec466f29f9f13385c796 100644 (file)
--- a/src/shared/dissect-image.c
+++ b/src/shared/dissect-image.c
@@ -4034,11 +4034,12 @@ int verity_dissect_and_mount(
                 const char *required_host_os_release_sysext_level,
                 const char *required_host_os_release_confext_level,
                 const char *required_sysext_scope,
+                VeritySettings *verity,
                 DissectedImage **ret_image) {
 
         _cleanup_(loop_device_unrefp) LoopDevice *loop_device = NULL;
         _cleanup_(dissected_image_unrefp) DissectedImage *dissected_image = NULL;
-        _cleanup_(verity_settings_done) VeritySettings verity = VERITY_SETTINGS_DEFAULT;
+        _cleanup_(verity_settings_done) VeritySettings local_verity = VERITY_SETTINGS_DEFAULT;
         DissectImageFlags dissect_image_flags;
         bool relax_extension_release_check;
         int r;
@@ -4050,13 +4051,19 @@ int verity_dissect_and_mount(
 
         relax_extension_release_check = mount_options_relax_extension_release_checks(options);
 
-        /* We might get an FD for the image, but we use the original path to look for the dm-verity files */
-        r = verity_settings_load(&verity, src, NULL, NULL);
-        if (r < 0)
-                return log_debug_errno(r, "Failed to load root hash: %m");
+        /* We might get an FD for the image, but we use the original path to look for the dm-verity files.
+         * The caller might also give us a pre-loaded VeritySettings, in which case we just use it. It will
+         * also be extended, as dissected_image_load_verity_sig_partition() is invoked. */
+        if (!verity) {
+                r = verity_settings_load(&local_verity, src, NULL, NULL);
+                if (r < 0)
+                        return log_debug_errno(r, "Failed to load root hash: %m");
+
+                verity = &local_verity;
+        }
 
         dissect_image_flags =
-                (verity.data_path ? DISSECT_IMAGE_NO_PARTITION_TABLE : 0) |
+                (verity->data_path ? DISSECT_IMAGE_NO_PARTITION_TABLE : 0) |
                 (relax_extension_release_check ? DISSECT_IMAGE_RELAX_EXTENSION_CHECK : 0) |
                 DISSECT_IMAGE_ADD_PARTITION_DEVICES |
                 DISSECT_IMAGE_PIN_PARTITION_DEVICES |
@@ -4068,7 +4075,7 @@ int verity_dissect_and_mount(
                         src_fd >= 0 ? FORMAT_PROC_FD_PATH(src_fd) : src,
                         /* open_flags= */ -1,
                         /* sector_size= */ UINT32_MAX,
-                        verity.data_path ? 0 : LO_FLAGS_PARTSCAN,
+                        verity->data_path ? 0 : LO_FLAGS_PARTSCAN,
                         LOCK_SH,
                         &loop_device);
         if (r < 0)
@@ -4076,16 +4083,16 @@ int verity_dissect_and_mount(
 
         r = dissect_loop_device(
                         loop_device,
-                        &verity,
+                        verity,
                         options,
                         image_policy,
                         dissect_image_flags,
                         &dissected_image);
         /* No partition table? Might be a single-filesystem image, try again */
-        if (!verity.data_path && r == -ENOPKG)
+        if (!verity->data_path && r == -ENOPKG)
                  r = dissect_loop_device(
                                 loop_device,
-                                &verity,
+                                verity,
                                 options,
                                 image_policy,
                                 dissect_image_flags | DISSECT_IMAGE_NO_PARTITION_TABLE,
@@ -4093,14 +4100,14 @@ int verity_dissect_and_mount(
         if (r < 0)
                 return log_debug_errno(r, "Failed to dissect image: %m");
 
-        r = dissected_image_load_verity_sig_partition(dissected_image, loop_device->fd, &verity);
+        r = dissected_image_load_verity_sig_partition(dissected_image, loop_device->fd, verity);
         if (r < 0)
                 return r;
 
         r = dissected_image_decrypt(
                         dissected_image,
                         NULL,
-                        &verity,
+                        verity,
                         dissect_image_flags);
         if (r < 0)
                 return log_debug_errno(r, "Failed to decrypt dissected image: %m");

diff --git a/src/shared/dissect-image.h b/src/shared/dissect-image.h
index 02aa2d5734bc8b7abc7479af5eea8cbcbbe9a9e9..2c118b97ef5cf466f75a36e5fda2a4aacddc7ed6 100644 (file)
--- a/src/shared/dissect-image.h
+++ b/src/shared/dissect-image.h
@@ -226,7 +226,7 @@ bool dissected_image_verity_sig_ready(const DissectedImage *image, PartitionDesi
 
 int mount_image_privately_interactively(const char *path, const ImagePolicy *image_policy, DissectImageFlags flags, char **ret_directory, int *ret_dir_fd, LoopDevice **ret_loop_device);
 
-int verity_dissect_and_mount(int src_fd, const char *src, const char *dest, const MountOptions *options, const ImagePolicy *image_policy, const char *required_host_os_release_id, const char *required_host_os_release_version_id, const char *required_host_os_release_sysext_level, const char *required_host_os_release_confext_level, const char *required_sysext_scope, DissectedImage **ret_image);
+int verity_dissect_and_mount(int src_fd, const char *src, const char *dest, const MountOptions *options, const ImagePolicy *image_policy, const char *required_host_os_release_id, const char *required_host_os_release_version_id, const char *required_host_os_release_sysext_level, const char *required_host_os_release_confext_level, const char *required_sysext_scope, VeritySettings *verity, DissectedImage **ret_image);
 
 int dissect_fstype_ok(const char *fstype);
 

diff --git a/src/shared/mount-util.c b/src/shared/mount-util.c
index 4ddfb9a82acde0afc5740f1e99a4e5b37737a51b..a1f4bee77e17953fe381c5f7ad6c85bff155d189 100644 (file)
--- a/src/shared/mount-util.c
+++ b/src/shared/mount-util.c
@@ -941,6 +941,7 @@ static int mount_in_namespace_legacy(
                                 /* required_host_os_release_sysext_level= */ NULL,
                                 /* required_host_os_release_confext_level= */ NULL,
                                 /* required_sysext_scope= */ NULL,
+                                /* verity= */ NULL,
                                 /* ret_image= */ NULL);
         else
                 r = mount_follow_verbose(LOG_DEBUG, FORMAT_PROC_FD_PATH(chased_src_fd), mount_tmp, NULL, MS_BIND, NULL);
@@ -1166,6 +1167,7 @@ static int mount_in_namespace(
                                 /* required_host_os_release_sysext_level= */ NULL,
                                 /* required_host_os_release_confext_level= */ NULL,
                                 /* required_sysext_scope= */ NULL,
+                                /* verity= */ NULL,
                                 &img);
                 if (r < 0)
                         return log_debug_errno(

diff --git a/test/TEST-50-DISSECT/test.sh b/test/TEST-50-DISSECT/test.sh
index a0cd677f075c3745fd27f17642c9d5bdfc8ba84a..de20ac4681b5d96c82bf72e00cce2cd2beccc95a 100755 (executable)
--- a/test/TEST-50-DISSECT/test.sh
+++ b/test/TEST-50-DISSECT/test.sh
@@ -37,6 +37,7 @@ test_append_files() {
     inst_binary mksquashfs
     inst_binary unsquashfs
     inst_binary pkcheck
+    inst_binary veritysetup
     install_verity_minimal
 }
 

diff --git a/test/units/TEST-50-DISSECT.dissect.sh b/test/units/TEST-50-DISSECT.dissect.sh
index 563206ce7e40ad216a8b9f4df50349ca99116f9d..9a05899cabcf68bad69ecfb59a9adbcf3dfe276c 100755 (executable)
--- a/test/units/TEST-50-DISSECT.dissect.sh
+++ b/test/units/TEST-50-DISSECT.dissect.sh
@@ -389,6 +389,22 @@ systemd-run -P \
             --property RootImage="$MINIMAL_IMAGE.raw" \
             "${BIND_LOG_SOCKETS[@]}" \
             cat /etc/systemd/system/some_file | grep -q -F "MARKER_CONFEXT_123"
+
+# Check that two identical verity images at different paths do not fail with -ELOOP from OverlayFS
+mkdir -p /tmp/loop
+cp /tmp/app0.raw /tmp/loop/app0.raw
+veritysetup format /tmp/loop/app0.raw /tmp/loop/app0.verity --root-hash-file /tmp/loop/app0.roothash
+cp /tmp/loop/app0.raw /tmp/loop/app0_copy.raw
+cp /tmp/loop/app0.verity /tmp/loop/app0_copy.verity
+cp /tmp/loop/app0.roothash /tmp/loop/app0_copy.roothash
+systemd-run -P \
+            --property ExtensionImages=/tmp/loop/app0.raw \
+            --property ExtensionImages=/tmp/loop/app0_copy.raw \
+            --property RootImage="$MINIMAL_IMAGE.raw" \
+            "${BIND_LOG_SOCKETS[@]}" \
+            cat /opt/script0.sh | grep -q -F "extension-release.app0"
+rm -rf /tmp/loop/
+
 # Check that using a symlink to NAME-VERSION.raw works as long as the symlink has the correct name NAME.raw
 mkdir -p /tmp/symlink-test/
 cp /tmp/app-nodistro.raw /tmp/symlink-test/app-nodistro-v1.raw