deploy: Don't recompute verity checksums if not enabled

This fixes a truly horrific performance bug when composefs is enabled, but fsverity is not supported by the filesystem. We'd fall back to doing *userspace* checksumming of all files at deployment time which was absolutely not expected or required. There's really an immense amount of technical debt here, such as the confusion between `ex-integity.composefs` vs the prepare-root config, how we handle "torn" states where some objects don't have verity enabled but some do, etc. The ostree composefs state has two modes: - signed: We need to enforce fsverity - unsigned: Best effort resilience So we fix this by making the deploy path to make verity "opportunistic" - if the ioctl gives us the data, then we add it to the composefs. However, this code path is also invoked when we're computing the expected composefs digest to inject as commit metadata, and *that* API must work regardless of whether the target repo has fsverity enabled as it may operate on a build server. One lucky thing in all of this: When I went to add the "checkout composefs" API I added a stub `GVariant` for options extensibility, which we now use. Signed-off-by: Colin Walters <[email protected]>
ostreedev · Oct 27, 2024 · 3375e9d · 3375e9d
1 parent 3625130
commit 3375e9d
Show file tree

Hide file tree

Showing 7 changed files with 151 additions and 53 deletions.
diff --git a/man/ostree-checkout.xml b/man/ostree-checkout.xml
@@ -211,6 +211,24 @@ License along with this library. If not, see <https://www.gnu.org/licenses/>.
                     (may be /). This implies <literal>--force-copy</literal>.
                 </para></listitem>
             </varlistentry>
+
+            <varlistentry>
+                <term><option>--composefs</option></term>
+
+                <listitem><para>
+                    Only generate a composefs, not a directory.
+                </para></listitem>
+            </varlistentry>
+
+            <varlistentry>
+                <term><option>--composefs-noverity</option></term>
+
+                <listitem><para>
+                    Only generate a composefs, not a directory; fsverity digests
+                    will not be included. This is best used for "opportunistic"
+                    use of composefs.
+                </para></listitem>
+            </varlistentry>
         </variablelist>
     </refsect1>
 

diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c
@@ -1273,23 +1273,50 @@ compare_verity_digests (GVariant *metadata_composefs, const guchar *fsverity_dig
 /**
  * ostree_repo_checkout_composefs:
  * @self: A repo
- * @options: (nullable): Future expansion space; must currently be %NULL
+ * @options: (nullable): If non-NULL, must be a GVariant of type a{sv}. See below.
  * @destination_dfd: Parent directory fd
  * @destination_path: Filename
  * @checksum: OStree commit digest
  * @cancellable: Cancellable
  * @error: Error
  *
- * Create a composefs filesystem metadata blob from an OSTree commit.
+ * Create a composefs filesystem metadata blob from an OSTree commit. Supported
+ * options:
+ *
+ *  - verity: `u`: 0 = disabled, 1 = set if present on file, 2 = enabled; any other value is a fatal
+ * error
  */
 gboolean
 ostree_repo_checkout_composefs (OstreeRepo *self, GVariant *options, int destination_dfd,
                                 const char *destination_path, const char *checksum,
                                 GCancellable *cancellable, GError **error)
 {
 #ifdef HAVE_COMPOSEFS
-  /* Force this for now */
-  g_assert (options == NULL);
+  OtTristate verity = OT_TRISTATE_YES;
+
+  if (options != NULL)
+    {
+      g_auto (GVariantDict) options_dict;
+      g_variant_dict_init (&options_dict, options);
+      guint32 verity_v = 0;
+      if (g_variant_dict_lookup (&options_dict, "verity", "u", &verity_v))
+        {
+          switch (verity_v)
+            {
+            case 0:
+              verity = OT_TRISTATE_NO;
+              break;
+            case 1:
+              verity = OT_TRISTATE_MAYBE;
+              break;
+            case 2:
+              verity = OT_TRISTATE_YES;
+              break;
+            default:
+              g_assert_not_reached ();
+            }
+        }
+    }
 
   g_auto (GLnxTmpfile) tmpf = {
     0,
@@ -1311,17 +1338,20 @@ ostree_repo_checkout_composefs (OstreeRepo *self, GVariant *options, int destina
 
   g_autoptr (OstreeComposefsTarget) target = ostree_composefs_target_new ();
 
-  if (!_ostree_repo_checkout_composefs (self, target, (OstreeRepoFile *)commit_root, cancellable,
-                                        error))
+  if (!_ostree_repo_checkout_composefs (self, verity, target, (OstreeRepoFile *)commit_root,
+                                        cancellable, error))
     return FALSE;
 
   g_autofree guchar *fsverity_digest = NULL;
   if (!ostree_composefs_target_write (target, tmpf.fd, &fsverity_digest, cancellable, error))
     return FALSE;
 
   /* If the commit specified a composefs digest, verify it */
-  if (!compare_verity_digests (metadata_composefs, fsverity_digest, error))
-    return FALSE;
+  if (verity == OT_TRISTATE_YES)
+    {
+      if (!compare_verity_digests (metadata_composefs, fsverity_digest, error))
+        return FALSE;
+    }
 
   if (!glnx_fchmod (tmpf.fd, 0644, error))
     return FALSE;

diff --git a/src/libostree/ostree-repo-composefs.c b/src/libostree/ostree-repo-composefs.c
@@ -265,9 +265,9 @@ _ostree_composefs_set_xattrs (struct lcfs_node_s *node, GVariant *xattrs, GCance
 }
 
 static gboolean
-checkout_one_composefs_file_at (OstreeRepo *repo, const char *checksum, struct lcfs_node_s *parent,
-                                const char *destination_name, GCancellable *cancellable,
-                                GError **error)
+checkout_one_composefs_file_at (OstreeRepo *repo, OtTristate verity, const char *checksum,
+                                struct lcfs_node_s *parent, const char *destination_name,
+                                GCancellable *cancellable, GError **error)
 {
   g_autoptr (GInputStream) input = NULL;
   g_autoptr (GVariant) xattrs = NULL;
@@ -322,30 +322,37 @@ checkout_one_composefs_file_at (OstreeRepo *repo, const char *checksum, struct l
 
       guchar *known_digest = NULL;
 
-#ifdef HAVE_LINUX_FSVERITY_H
-      /* First try to get the digest directly from the bare repo file.
-       * This is the typical case when we're pulled into the target
-       * system repo with verity on and are recreating the composefs
-       * image during deploy. */
-      char buf[sizeof (struct fsverity_digest) + OSTREE_SHA256_DIGEST_LEN];
-
-      if (G_IS_UNIX_INPUT_STREAM (input))
+      if (verity != OT_TRISTATE_NO)
         {
-          int content_fd = g_unix_input_stream_get_fd (G_UNIX_INPUT_STREAM (input));
-          struct fsverity_digest *d = (struct fsverity_digest *)&buf;
-          d->digest_size = OSTREE_SHA256_DIGEST_LEN;
-
-          if (ioctl (content_fd, FS_IOC_MEASURE_VERITY, d) == 0
-              && d->digest_size == OSTREE_SHA256_DIGEST_LEN
-              && d->digest_algorithm == FS_VERITY_HASH_ALG_SHA256)
-            known_digest = d->digest;
-        }
+#ifdef HAVE_LINUX_FSVERITY_H
+          /* First try to get the digest directly from the bare repo file.
+           * This is the typical case when we're pulled into the target
+           * system repo with verity on and are recreating the composefs
+           * image during deploy. */
+          char buf[sizeof (struct fsverity_digest) + OSTREE_SHA256_DIGEST_LEN];
+
+          if (G_IS_UNIX_INPUT_STREAM (input))
+            {
+              int content_fd = g_unix_input_stream_get_fd (G_UNIX_INPUT_STREAM (input));
+              struct fsverity_digest *d = (struct fsverity_digest *)&buf;
+              d->digest_size = OSTREE_SHA256_DIGEST_LEN;
+
+              if (ioctl (content_fd, FS_IOC_MEASURE_VERITY, d) == 0
+                  && d->digest_size == OSTREE_SHA256_DIGEST_LEN
+                  && d->digest_algorithm == FS_VERITY_HASH_ALG_SHA256)
+                known_digest = d->digest;
+            }
 #endif
 
-      if (known_digest)
-        lcfs_node_set_fsverity_digest (node, known_digest);
-      else if (lcfs_node_set_fsverity_from_content (node, input, _composefs_read_cb) != 0)
-        return glnx_throw_errno (error);
+          if (known_digest)
+            lcfs_node_set_fsverity_digest (node, known_digest);
+          else if (verity == OT_TRISTATE_YES)
+            {
+              // Only fall back to userspace computation if explicitly requested
+              if (lcfs_node_set_fsverity_from_content (node, input, _composefs_read_cb) != 0)
+                return glnx_throw_errno (error);
+            }
+        }
     }
 
   if (xattrs)
@@ -360,7 +367,7 @@ checkout_one_composefs_file_at (OstreeRepo *repo, const char *checksum, struct l
 }
 
 static gboolean
-checkout_composefs_recurse (OstreeRepo *self, const char *dirtree_checksum,
+checkout_composefs_recurse (OstreeRepo *self, OtTristate verity, const char *dirtree_checksum,
                             const char *dirmeta_checksum, struct lcfs_node_s *parent,
                             const char *name, GCancellable *cancellable, GError **error)
 {
@@ -422,8 +429,8 @@ checkout_composefs_recurse (OstreeRepo *self, const char *dirtree_checksum,
         char tmp_checksum[OSTREE_SHA256_STRING_LEN + 1];
         _ostree_checksum_inplace_from_bytes_v (contents_csum_v, tmp_checksum);
 
-        if (!checkout_one_composefs_file_at (self, tmp_checksum, directory, fname, cancellable,
-                                             error))
+        if (!checkout_one_composefs_file_at (self, verity, tmp_checksum, directory, fname,
+                                             cancellable, error))
           return glnx_prefix_error (error, "Processing %s", tmp_checksum);
       }
     contents_csum_v = NULL; /* iter_loop freed it */
@@ -453,8 +460,8 @@ checkout_composefs_recurse (OstreeRepo *self, const char *dirtree_checksum,
         _ostree_checksum_inplace_from_bytes_v (subdirtree_csum_v, subdirtree_checksum);
         char subdirmeta_checksum[OSTREE_SHA256_STRING_LEN + 1];
         _ostree_checksum_inplace_from_bytes_v (subdirmeta_csum_v, subdirmeta_checksum);
-        if (!checkout_composefs_recurse (self, subdirtree_checksum, subdirmeta_checksum, directory,
-                                         dname, cancellable, error))
+        if (!checkout_composefs_recurse (self, verity, subdirtree_checksum, subdirmeta_checksum,
+                                         directory, dname, cancellable, error))
           return FALSE;
       }
     /* Freed by iter-loop */
@@ -467,8 +474,9 @@ checkout_composefs_recurse (OstreeRepo *self, const char *dirtree_checksum,
 
 /* Begin a checkout process */
 static gboolean
-checkout_composefs_tree (OstreeRepo *self, OstreeComposefsTarget *target, OstreeRepoFile *source,
-                         GFileInfo *source_info, GCancellable *cancellable, GError **error)
+checkout_composefs_tree (OstreeRepo *self, OtTristate verity, OstreeComposefsTarget *target,
+                         OstreeRepoFile *source, GFileInfo *source_info, GCancellable *cancellable,
+                         GError **error)
 {
   if (g_file_info_get_file_type (source_info) != G_FILE_TYPE_DIRECTORY)
     return glnx_throw (error, "Root checkout of composefs must be directory");
@@ -483,8 +491,8 @@ checkout_composefs_tree (OstreeRepo *self, OstreeComposefsTarget *target, Ostree
 
   const char *dirtree_checksum = ostree_repo_file_tree_get_contents_checksum (source);
   const char *dirmeta_checksum = ostree_repo_file_tree_get_metadata_checksum (source);
-  return checkout_composefs_recurse (self, dirtree_checksum, dirmeta_checksum, target->dest, "root",
-                                     cancellable, error);
+  return checkout_composefs_recurse (self, verity, dirtree_checksum, dirmeta_checksum, target->dest,
+                                     "root", cancellable, error);
 }
 
 static struct lcfs_node_s *
@@ -515,6 +523,7 @@ ensure_lcfs_dir (struct lcfs_node_s *parent, const char *name, GError **error)
  * _ostree_repo_checkout_composefs:
  * @self: Repo
  * @target: A target for the checkout
+ * @verity: Use fsverity
  * @source: Source tree
  * @cancellable: Cancellable
  * @error: Error
@@ -530,7 +539,7 @@ ensure_lcfs_dir (struct lcfs_node_s *parent, const char *name, GError **error)
  * Returns: %TRUE on success, %FALSE on failure
  */
 gboolean
-_ostree_repo_checkout_composefs (OstreeRepo *self, OstreeComposefsTarget *target,
+_ostree_repo_checkout_composefs (OstreeRepo *self, OtTristate verity, OstreeComposefsTarget *target,
                                  OstreeRepoFile *source, GCancellable *cancellable, GError **error)
 {
 #ifdef HAVE_COMPOSEFS
@@ -545,7 +554,7 @@ _ostree_repo_checkout_composefs (OstreeRepo *self, OstreeComposefsTarget *target
   if (!target_info)
     return glnx_prefix_error (error, "Failed to query");
 
-  if (!checkout_composefs_tree (self, target, source, target_info, cancellable, error))
+  if (!checkout_composefs_tree (self, verity, target, source, target_info, cancellable, error))
     return FALSE;
 
   /* We need a root dir */
@@ -593,7 +602,11 @@ ostree_repo_commit_add_composefs_metadata (OstreeRepo *self, guint format_versio
 
   g_autoptr (OstreeComposefsTarget) target = ostree_composefs_target_new ();
 
-  if (!_ostree_repo_checkout_composefs (self, target, repo_root, cancellable, error))
+  // We unconditionally add the expected verity digest. Note that for repositories
+  // on filesystems without fsverity, this operation currently requires re-checksumming
+  // all objects.
+  if (!_ostree_repo_checkout_composefs (self, OT_TRISTATE_YES, target, repo_root, cancellable,
+                                        error))
     return FALSE;
 
   g_autofree guchar *fsverity_digest = NULL;

diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h
@@ -473,9 +473,9 @@ gboolean ostree_composefs_target_write (OstreeComposefsTarget *target, int fd,
                                         guchar **out_fsverity_digest, GCancellable *cancellable,
                                         GError **error);
 
-gboolean _ostree_repo_checkout_composefs (OstreeRepo *self, OstreeComposefsTarget *target,
-                                          OstreeRepoFile *source, GCancellable *cancellable,
-                                          GError **error);
+gboolean _ostree_repo_checkout_composefs (OstreeRepo *self, OtTristate verity,
+                                          OstreeComposefsTarget *target, OstreeRepoFile *source,
+                                          GCancellable *cancellable, GError **error);
 static inline gboolean
 composefs_not_supported (GError **error)
 {

diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
@@ -665,8 +665,23 @@ checkout_deployment_tree (OstreeSysroot *sysroot, OstreeRepo *repo, OstreeDeploy
     composefs_enabled = repo->composefs_wanted;
   if (composefs_enabled == OT_TRISTATE_YES)
     {
-      if (!ostree_repo_checkout_composefs (repo, NULL, ret_deployment_dfd, OSTREE_COMPOSEFS_NAME,
-                                           csum, cancellable, error))
+      // TODO: Clean up our mess around composefs/fsverity...we have duplication
+      // between the repo config and the sysroot config, *and* we need to better
+      // handle skew between repo config and repo state (e.g. "post-copy" should
+      // support transitioning verity on and off in general).
+      // For now we configure things such that the fsverity digest is only added
+      // if present on disk in the unsigned case, and in the signed case unconditionally
+      // require it.
+      g_autoptr(GVariantBuilder) cfs_checkout_opts_builder = g_variant_builder_new (G_VARIANT_TYPE ("a{sv}"));
+      guint32 composefs_requested = 1;
+      if (composefs_config->is_signed)
+        composefs_requested = 2;
+      g_variant_builder_add (cfs_checkout_opts_builder, "{sv}", "verity",
+                               g_variant_new_uint32 (composefs_requested));
+      g_debug ("composefs requested: %u", composefs_requested);
+      g_autoptr (GVariant) cfs_checkout_opts = g_variant_builder_end (cfs_checkout_opts_builder);
+      if (!ostree_repo_checkout_composefs (repo, cfs_checkout_opts, ret_deployment_dfd,
+                                           OSTREE_COMPOSEFS_NAME, csum, cancellable, error))
         return FALSE;
     }
   else

diff --git a/src/ostree/ot-builtin-checkout.c b/src/ostree/ot-builtin-checkout.c
@@ -29,6 +29,7 @@
 #include "otutil.h"
 
 static gboolean opt_composefs;
+static gboolean opt_composefs_noverity;
 static gboolean opt_user_mode;
 static gboolean opt_allow_noent;
 static gboolean opt_disable_cache;
@@ -109,6 +110,8 @@ static GOptionEntry options[] = {
   { "selinux-prefix", 0, 0, G_OPTION_ARG_STRING, &opt_selinux_prefix,
     "When setting SELinux labels, prefix all paths by PREFIX", "PREFIX" },
   { "composefs", 0, 0, G_OPTION_ARG_NONE, &opt_composefs, "Only create a composefs blob", NULL },
+  { "composefs-noverity", 0, 0, G_OPTION_ARG_NONE, &opt_composefs_noverity,
+    "Only create a composefs blob, and disable fsverity", NULL },
   { NULL }
 };
 
@@ -145,12 +148,18 @@ process_one_checkout (OstreeRepo *repo, const char *resolved_commit, const char
                              || opt_process_passthrough_whiteouts;
 
   /* If we're doing composefs, then this is it */
-  if (opt_composefs)
+  if (opt_composefs || opt_composefs_noverity)
     {
       if (new_options_set)
         return glnx_throw (error, "Specified options are incompatible with --composefs");
-      return ostree_repo_checkout_composefs (repo, NULL, AT_FDCWD, destination, resolved_commit,
-                                             cancellable, error);
+      g_autoptr (GVariantBuilder) checkout_opts_builder
+          = g_variant_builder_new (G_VARIANT_TYPE ("a{sv}"));
+      if (opt_composefs_noverity)
+        g_variant_builder_add (checkout_opts_builder, "{sv}", "verity",
+                               g_variant_new_uint32 (0));
+      g_autoptr (GVariant) checkout_opts = g_variant_builder_end (checkout_opts_builder);
+      return ostree_repo_checkout_composefs (repo, checkout_opts, AT_FDCWD, destination,
+                                             resolved_commit, cancellable, error);
     }
 
   if (new_options_set)

diff --git a/tests/test-composefs.sh b/tests/test-composefs.sh
@@ -45,7 +45,20 @@ digest=$(sha256sum < test2-co.cfs | cut -f 1 -d ' ')
 # we're operating on predictable data (fixed uid, gid, timestamps, xattrs, permissions).
 assert_streq "${digest}" "031fab2c7f390b752a820146dc89f6880e5739cba7490f64024e0c7d11aad7c9"
 # Verify it with composefs tooling
-composefs-info dump test2-co.cfs >/dev/null
+composefs-info dump test2-co.cfs > dump.txt
+# Verify we have a verity digest
+assert_file_has_content_literal dump.txt '/baz/cow 4 100644 1 0 0 0 0.0 f6/a517d53831a40cff3886a965c70d57aa50797a8e5ea965b2c49cc575a6ff51.file - ebaa23af194a798df610e5fe2bd10725c9c4a3a56a6b62d4d0ee551d4fc4be27'
+rm -vf dump.txt test2-co.cfs
+
+$OSTREE checkout --composefs-noverity test-composefs test2-co-noverity.cfs
+digest=$(sha256sum < test2-co-noverity.cfs | cut -f 1 -d ' ')
+# Should be reproducible per above
+assert_streq "${digest}" "78f873a76ccfea3ad7c86312ba0e06f8e0bca54ab4912b23871b31caafe59c24"
+# Verify it with composefs tooling
+composefs-info dump test2-co-noverity.cfs > dump.txt
+# No verity digest here
+assert_file_has_content_literal dump.txt '/baz/cow 4 100644 1 0 0 0 0.0 f6/a517d53831a40cff3886a965c70d57aa50797a8e5ea965b2c49cc575a6ff51.file - -'
+
 tap_ok "checkout composefs"
 
 tap_end