From 3375e9dcfc49528a301c89d1008f5de36fc48f8e Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 27 Oct 2024 10:20:29 -0400 Subject: [PATCH] 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 --- man/ostree-checkout.xml | 18 ++++++ src/libostree/ostree-repo-checkout.c | 46 ++++++++++++--- src/libostree/ostree-repo-composefs.c | 85 +++++++++++++++------------ src/libostree/ostree-repo-private.h | 6 +- src/libostree/ostree-sysroot-deploy.c | 19 +++++- src/ostree/ot-builtin-checkout.c | 15 ++++- tests/test-composefs.sh | 15 ++++- 7 files changed, 151 insertions(+), 53 deletions(-) diff --git a/man/ostree-checkout.xml b/man/ostree-checkout.xml index 8f7d4f9b28..fd2094cdf7 100644 --- a/man/ostree-checkout.xml +++ b/man/ostree-checkout.xml @@ -211,6 +211,24 @@ License along with this library. If not, see . (may be /). This implies --force-copy. + + + + + + Only generate a composefs, not a directory. + + + + + + + + Only generate a composefs, not a directory; fsverity digests + will not be included. This is best used for "opportunistic" + use of composefs. + + diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c index e83713d8ce..dc07c9d18a 100644 --- a/src/libostree/ostree-repo-checkout.c +++ b/src/libostree/ostree-repo-checkout.c @@ -1273,14 +1273,18 @@ 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, @@ -1288,8 +1292,31 @@ ostree_repo_checkout_composefs (OstreeRepo *self, GVariant *options, int destina 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,8 +1338,8 @@ 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; @@ -1320,8 +1347,11 @@ ostree_repo_checkout_composefs (OstreeRepo *self, GVariant *options, int destina 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 index e2fae6898c..3366afb0b1 100644 --- 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 index 21b0fc14e9..1e27fc772d 100644 --- 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 index ba414c75b6..abc97292bb 100644 --- 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 index db4e0a0f73..bfbe5d3c3b 100644 --- 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 index d7ae8ec350..fa6ce8c514 100755 --- 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