From 618617d68b6ea82a71b3394ccd726e6b4b3e156e Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 18 May 2017 18:12:33 -0400 Subject: [PATCH] lib/pull: Add support for timestamp-check option, use in upgrader For both flatpak and ostree-as-host, we really want to verify up front during pulls that we're not being downgraded. Currently both flatpak and `OstreeSysrootUpgrader` do this before deployments, but at that point we've already downloaded all the data, which is annoying. Closes: https://github.com/ostreedev/ostree/issues/687 Closes: #1055 Approved by: jlebon --- src/libostree/ostree-core-private.h | 7 +++ src/libostree/ostree-core.c | 32 ++++++++++++++ src/libostree/ostree-repo-pull.c | 54 +++++++++++++++++++++++ src/libostree/ostree-sysroot-upgrader.c | 45 +++++++++---------- src/ostree/ot-builtin-pull.c | 5 +++ tests/libtest.sh | 10 ++++- tests/pull-test.sh | 27 +++++++++++- tests/test-admin-upgrade-not-backwards.sh | 30 ++++++++++--- 8 files changed, 178 insertions(+), 32 deletions(-) diff --git a/src/libostree/ostree-core-private.h b/src/libostree/ostree-core-private.h index 799bd228..5a2835d5 100644 --- a/src/libostree/ostree-core-private.h +++ b/src/libostree/ostree-core-private.h @@ -179,6 +179,13 @@ _ostree_raw_file_to_archive_stream (GInputStream *input, gboolean ostree_validate_collection_id (const char *collection_id, GError **error); #endif /* !OSTREE_ENABLE_EXPERIMENTAL_API */ +gboolean +_ostree_compare_timestamps (const char *current_rev, + guint64 current_ts, + const char *new_rev, + guint64 new_ts, + GError **error); + #if (defined(OSTREE_COMPILATION) || GLIB_CHECK_VERSION(2, 44, 0)) && !defined(OSTREE_ENABLE_EXPERIMENTAL_API) #include #include "ostree-ref.h" diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index c13d2f2e..4118cf7e 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -2086,6 +2086,38 @@ ostree_commit_get_timestamp (GVariant *commit_variant) return GUINT64_FROM_BE (ret); } +/* Used in pull/deploy to validate we're not being downgraded */ +gboolean +_ostree_compare_timestamps (const char *current_rev, + guint64 current_ts, + const char *new_rev, + guint64 new_ts, + GError **error) +{ + /* Newer timestamp is OK */ + if (new_ts > current_ts) + return TRUE; + /* If they're equal, ensure they're the same rev */ + if (new_ts == current_ts || strcmp (current_rev, new_rev) == 0) + return TRUE; + + /* Looks like a downgrade, format an error message */ + g_autoptr(GDateTime) current_dt = g_date_time_new_from_unix_utc (current_ts); + g_autoptr(GDateTime) new_dt = g_date_time_new_from_unix_utc (new_ts); + + if (current_dt == NULL || new_dt == NULL) + return glnx_throw (error, "Upgrade target revision '%s' timestamp (%" G_GINT64_FORMAT ") or current revision '%s' timestamp (%" G_GINT64_FORMAT ") is invalid", + new_rev, new_ts, + current_rev, current_ts); + + g_autofree char *current_ts_str = g_date_time_format (current_dt, "%c"); + g_autofree char *new_ts_str = g_date_time_format (new_dt, "%c"); + + return glnx_throw (error, "Upgrade target revision '%s' with timestamp '%s' is chronologically older than current revision '%s' with timestamp '%s'", + new_rev, new_ts_str, current_rev, current_ts_str); +} + + GVariant * _ostree_detached_metadata_append_gpg_sig (GVariant *existing_metadata, GBytes *signature_bytes) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index e21342fe..0bb1bc04 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -104,6 +104,7 @@ typedef struct { GBytes *summary_data_sig; GVariant *summary; GHashTable *summary_deltas_checksums; + GHashTable *ref_original_commits; /* Maps checksum to commit, used by timestamp checks */ GPtrArray *static_delta_superblocks; GHashTable *expected_commit_sizes; /* Maps commit checksum to known size */ GHashTable *commit_to_depth; /* Maps commit checksum maximum depth */ @@ -136,6 +137,7 @@ typedef struct { guint n_fetched_localcache_metadata; guint n_fetched_localcache_content; + gboolean timestamp_check; /* Verify commit timestamps */ int maxdepth; guint64 start_time; @@ -1647,6 +1649,33 @@ scan_commit_object (OtPullData *pull_data, goto out; } + if (pull_data->timestamp_check) + { + /* We don't support timestamp checking while recursing right now */ + g_assert (ref); + g_assert_cmpint (recursion_depth, ==, 0); + const char *orig_rev = NULL; + if (!g_hash_table_lookup_extended (pull_data->ref_original_commits, + ref, NULL, (void**)&orig_rev)) + g_assert_not_reached (); + + g_autoptr(GVariant) orig_commit = NULL; + if (orig_rev) + { + if (!ostree_repo_load_commit (pull_data->repo, orig_rev, + &orig_commit, NULL, error)) + { + g_prefix_error (error, "Reading %s for timestamp-check: ", ref->ref_name); + goto out; + } + + guint64 orig_ts = ostree_commit_get_timestamp (orig_commit); + guint64 new_ts = ostree_commit_get_timestamp (commit); + if (!_ostree_compare_timestamps (orig_rev, orig_ts, checksum, new_ts, error)) + goto out; + } + } + /* If we found a legacy transaction flag, assume all commits are partial */ is_partial = commitstate_is_partial (pull_data, commitstate); @@ -3199,6 +3228,7 @@ initiate_request (OtPullData *pull_data, * * disable-static-deltas (b): Do not use static deltas * * require-static-deltas (b): Require static deltas * * override-commit-ids (as): Array of specific commit IDs to fetch for refs + * * timestamp-check (b): Verify commit timestamps are newer than current (when pulling via ref); Since: 2017.11 * * dry-run (b): Only print information on what will be downloaded (requires static deltas) * * override-url (s): Fetch objects from this URL if remote specifies no metalink in options * * inherit-transaction (b): Don't initiate, finish or abort a transaction, useful to do multiple pulls in one transaction. @@ -3275,10 +3305,12 @@ ostree_repo_pull_with_options (OstreeRepo *self, (void) g_variant_lookup (options, "http-headers", "@a(ss)", &pull_data->extra_headers); (void) g_variant_lookup (options, "update-frequency", "u", &update_frequency); (void) g_variant_lookup (options, "localcache-repos", "^a&s", &opt_localcache_repos); + (void) g_variant_lookup (options, "timestamp-check", "b", &pull_data->timestamp_check); } g_return_val_if_fail (OSTREE_IS_REPO (self), FALSE); g_return_val_if_fail (pull_data->maxdepth >= -1, FALSE); + g_return_val_if_fail (!pull_data->timestamp_check || pull_data->maxdepth == 0, FALSE); g_return_val_if_fail (!opt_collection_refs_set || (refs_to_fetch == NULL && override_commit_ids == NULL), FALSE); if (refs_to_fetch && override_commit_ids) @@ -3322,6 +3354,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, pull_data->summary_deltas_checksums = g_hash_table_new_full (g_str_hash, g_str_equal, (GDestroyNotify)g_free, (GDestroyNotify)g_free); + pull_data->ref_original_commits = g_hash_table_new_full (ostree_collection_ref_hash, ostree_collection_ref_equal, + (GDestroyNotify)NULL, + (GDestroyNotify)g_variant_unref); pull_data->scanned_metadata = g_hash_table_new_full (ostree_hash_object_name, g_variant_equal, (GDestroyNotify)g_variant_unref, NULL); pull_data->fetched_detached_metadata = g_hash_table_new_full (g_str_hash, g_str_equal, @@ -3923,6 +3958,24 @@ ostree_repo_pull_with_options (OstreeRepo *self, ref_with_collection = ostree_collection_ref_dup (ref); } + /* If we have timestamp checking enabled, find the current value of + * the ref, and store its timestamp in the hash map, to check later. + */ + if (pull_data->timestamp_check) + { + g_autofree char *from_rev = NULL; + if (!ostree_repo_resolve_rev (pull_data->repo, ref_with_collection->ref_name, TRUE, + &from_rev, error)) + goto out; + /* Explicitly store NULL if there's no previous revision. We do + * this so we can assert() if we somehow didn't find a ref in the + * hash at all. Note we don't copy the collection-ref, so the + * lifetime of this hash must be equal to `requested_refs_to_fetch`. + */ + g_hash_table_insert (pull_data->ref_original_commits, ref_with_collection, + g_steal_pointer (&from_rev)); + } + g_hash_table_replace (updated_requested_refs_to_fetch, g_steal_pointer (&ref_with_collection), g_steal_pointer (&contents)); @@ -4223,6 +4276,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_clear_pointer (&pull_data->scanned_metadata, (GDestroyNotify) g_hash_table_unref); g_clear_pointer (&pull_data->fetched_detached_metadata, (GDestroyNotify) g_hash_table_unref); g_clear_pointer (&pull_data->summary_deltas_checksums, (GDestroyNotify) g_hash_table_unref); + g_clear_pointer (&pull_data->ref_original_commits, (GDestroyNotify) g_hash_table_unref); g_clear_pointer (&pull_data->requested_content, (GDestroyNotify) g_hash_table_unref); g_clear_pointer (&pull_data->requested_fallback_content, (GDestroyNotify) g_hash_table_unref); g_clear_pointer (&pull_data->requested_metadata, (GDestroyNotify) g_hash_table_unref); diff --git a/src/libostree/ostree-sysroot-upgrader.c b/src/libostree/ostree-sysroot-upgrader.c index 8afea3a6..f028fa7c 100644 --- a/src/libostree/ostree-sysroot-upgrader.c +++ b/src/libostree/ostree-sysroot-upgrader.c @@ -24,6 +24,7 @@ #include "ostree.h" #include "ostree-sysroot-upgrader.h" +#include "ostree-core-private.h" /** * SECTION:ostree-sysroot-upgrader @@ -429,26 +430,10 @@ ostree_sysroot_upgrader_check_timestamps (OstreeRepo *repo, error)) return FALSE; - if (ostree_commit_get_timestamp (old_commit) > ostree_commit_get_timestamp (new_commit)) - { - GDateTime *old_ts = g_date_time_new_from_unix_utc (ostree_commit_get_timestamp (old_commit)); - GDateTime *new_ts = g_date_time_new_from_unix_utc (ostree_commit_get_timestamp (new_commit)); - g_autofree char *old_ts_str = NULL; - g_autofree char *new_ts_str = NULL; - - if (old_ts == NULL || new_ts == NULL) - return glnx_throw (error, "Upgrade target revision '%s' timestamp (%" G_GINT64_FORMAT ") or current revision '%s' timestamp (%" G_GINT64_FORMAT ") is invalid", - to_rev, ostree_commit_get_timestamp (new_commit), - from_rev, ostree_commit_get_timestamp (old_commit)); - - old_ts_str = g_date_time_format (old_ts, "%c"); - new_ts_str = g_date_time_format (new_ts, "%c"); - g_date_time_unref (old_ts); - g_date_time_unref (new_ts); - - return glnx_throw (error, "Upgrade target revision '%s' with timestamp '%s' is chronologically older than current revision '%s' with timestamp '%s'; use --allow-downgrade to permit", - to_rev, new_ts_str, from_rev, old_ts_str); - } + if (!_ostree_compare_timestamps (from_rev, ostree_commit_get_timestamp (old_commit), + to_rev, ostree_commit_get_timestamp (new_commit), + error)) + return FALSE; return TRUE; } @@ -536,9 +521,23 @@ ostree_sysroot_upgrader_pull_one_dir (OstreeSysrootUpgrader *self, if (self->origin_remote && (upgrader_flags & OSTREE_SYSROOT_UPGRADER_PULL_FLAGS_SYNTHETIC) == 0) { - if (!ostree_repo_pull_one_dir (repo, self->origin_remote, dir_to_pull, refs_to_fetch, - flags, progress, - cancellable, error)) + g_autoptr(GVariantBuilder) optbuilder = g_variant_builder_new (G_VARIANT_TYPE ("a{sv}")); + if (dir_to_pull && *dir_to_pull) + g_variant_builder_add (optbuilder, "{s@v}", "subdir", + g_variant_new_variant (g_variant_new_string (dir_to_pull))); + g_variant_builder_add (optbuilder, "{s@v}", "flags", + g_variant_new_variant (g_variant_new_int32 (flags))); + /* Add the timestamp check, unless disabled */ + if ((upgrader_flags & OSTREE_SYSROOT_UPGRADER_PULL_FLAGS_ALLOW_OLDER) == 0) + g_variant_builder_add (optbuilder, "{s@v}", "timestamp-check", + g_variant_new_variant (g_variant_new_boolean (TRUE))); + + g_variant_builder_add (optbuilder, "{s@v}", "refs", + g_variant_new_variant (g_variant_new_strv ((const char *const*) refs_to_fetch, -1))); + g_autoptr(GVariant) opts = g_variant_ref_sink (g_variant_builder_end (optbuilder)); + if (!ostree_repo_pull_with_options (repo, self->origin_remote, + opts, progress, + cancellable, error)) return FALSE; if (progress) diff --git a/src/ostree/ot-builtin-pull.c b/src/ostree/ot-builtin-pull.c index 37cfd143..eceddb0f 100644 --- a/src/ostree/ot-builtin-pull.c +++ b/src/ostree/ot-builtin-pull.c @@ -34,6 +34,7 @@ static gboolean opt_dry_run; static gboolean opt_disable_static_deltas; static gboolean opt_require_static_deltas; static gboolean opt_untrusted; +static gboolean opt_timestamp_check; static gboolean opt_bareuseronly_files; static char** opt_subpaths; static char** opt_http_headers; @@ -64,6 +65,7 @@ static GOptionEntry options[] = { { "http-header", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_http_headers, "Add NAME=VALUE as HTTP header to all requests", "NAME=VALUE" }, { "update-frequency", 0, 0, G_OPTION_ARG_INT, &opt_frequency, "Sets the update frequency, in milliseconds (0=1000ms) (default: 0)", "FREQUENCY" }, { "localcache-repo", 'L', 0, G_OPTION_ARG_FILENAME_ARRAY, &opt_localcache_repos, "Add REPO as local cache source for objects during this pull", "REPO" }, + { "timestamp-check", 'T', 0, G_OPTION_ARG_NONE, &opt_timestamp_check, "Require fetched commits to have newer timestamps", NULL }, { NULL } }; @@ -288,6 +290,9 @@ ostree_builtin_pull (int argc, char **argv, GCancellable *cancellable, GError ** g_variant_builder_add (&builder, "{s@v}", "dry-run", g_variant_new_variant (g_variant_new_boolean (opt_dry_run))); + if (opt_timestamp_check) + g_variant_builder_add (&builder, "{s@v}", "timestamp-check", + g_variant_new_variant (g_variant_new_boolean (opt_timestamp_check))); if (override_commit_ids) g_variant_builder_add (&builder, "{s@v}", "override-commit-ids", diff --git a/tests/libtest.sh b/tests/libtest.sh index 1381a69e..4db8b730 100755 --- a/tests/libtest.sh +++ b/tests/libtest.sh @@ -550,6 +550,14 @@ ostree_file_path_to_checksum() { $CMD_PREFIX ostree --repo=$repo ls -C $ref $path | awk '{ print $5 }' } +# Given an object checksum, print its relative file path +ostree_checksum_to_relative_object_path() { + repo=$1 + checksum=$2 + if grep -Eq -e '^mode=archive' ${repo}/config; then suffix=z; else suffix=''; fi + echo objects/${checksum:0:2}/${checksum:2}.file${suffix} +} + # Given a path to a file in a repo for a ref, print the (relative) path to its # object ostree_file_path_to_relative_object_path() { @@ -558,7 +566,7 @@ ostree_file_path_to_relative_object_path() { path=$3 checksum=$(ostree_file_path_to_checksum $repo $ref $path) test -n "${checksum}" - echo objects/${checksum:0:2}/${checksum:2}.file + ostree_checksum_to_relative_object_path ${repo} ${checksum} } # Given a path to a file in a repo for a ref, print the path to its object diff --git a/tests/pull-test.sh b/tests/pull-test.sh index f51d4445..f44c2ced 100644 --- a/tests/pull-test.sh +++ b/tests/pull-test.sh @@ -35,7 +35,7 @@ function verify_initial_contents() { assert_file_has_content baz/cow '^moo$' } -echo "1..28" +echo "1..29" # Try both syntaxes repo_init --no-gpg-verify @@ -205,6 +205,31 @@ ${CMD_PREFIX} ostree --repo=parentpullrepo rev-parse origin:main > main.txt assert_file_has_content main.txt ${rev} echo "ok pull specific commit" +# test pull -T +cd ${test_tmpdir} +repo_init --no-gpg-verify +${CMD_PREFIX} ostree --repo=repo pull origin main +origrev=$(${CMD_PREFIX} ostree --repo=repo rev-parse main) +# Check we can pull the same commit with timestamp checking enabled +${CMD_PREFIX} ostree --repo=repo pull -T origin main +assert_streq ${origrev} "$(${CMD_PREFIX} ostree --repo=repo rev-parse main)" +newrev=$(${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo commit -b main --tree=ref=main) +${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo summary -u +# New commit with timestamp checking +${CMD_PREFIX} ostree --repo=repo pull -T origin main +assert_not_streq "${origrev}" "${newrev}" +assert_streq ${newrev} "$(${CMD_PREFIX} ostree --repo=repo rev-parse main)" +newrev2=$(${CMD_PREFIX} ostree --timestamp="October 25 1985" --repo=ostree-srv/gnomerepo commit -b main --tree=ref=main) +${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo summary -u +if ${CMD_PREFIX} ostree --repo=repo pull -T origin main 2>err.txt; then + fatal "pulled older commit with timestamp checking enabled?" +fi +assert_file_has_content err.txt "Upgrade.*is chronologically older" +assert_streq ${newrev} "$(${CMD_PREFIX} ostree --repo=repo rev-parse main)" +# But we can pull it without timestamp checking +${CMD_PREFIX} ostree --repo=repo pull origin main +echo "ok pull timestamp checking" + cd ${test_tmpdir} repo_init --no-gpg-verify ${CMD_PREFIX} ostree --repo=repo pull origin main diff --git a/tests/test-admin-upgrade-not-backwards.sh b/tests/test-admin-upgrade-not-backwards.sh index fd1e1108..67d51737 100755 --- a/tests/test-admin-upgrade-not-backwards.sh +++ b/tests/test-admin-upgrade-not-backwards.sh @@ -26,26 +26,42 @@ setup_os_repository "archive-z2" "syslinux" echo "1..2" +ref=testos/buildmaster/x86_64-runtime cd ${test_tmpdir} ${CMD_PREFIX} ostree --repo=sysroot/ostree/repo remote add --set=gpg-verify=false testos $(cat httpd-address)/ostree/testos-repo -${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull testos testos/buildmaster/x86_64-runtime -rev=$(${CMD_PREFIX} ostree --repo=sysroot/ostree/repo rev-parse testos/buildmaster/x86_64-runtime) +${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull testos ${ref} +rev=$(${CMD_PREFIX} ostree --repo=sysroot/ostree/repo rev-parse ${ref}) export rev echo "rev=${rev}" -# This initial deployment gets kicked off with some kernel arguments -${CMD_PREFIX} ostree admin deploy --karg=root=LABEL=MOO --karg=quiet --os=testos testos:testos/buildmaster/x86_64-runtime +# This initial deployment gets kicked off with some kernel arguments +${CMD_PREFIX} ostree admin deploy --karg=root=LABEL=MOO --karg=quiet --os=testos testos:${ref} assert_has_dir sysroot/boot/ostree/testos-${bootcsum} # This should be a no-op ${CMD_PREFIX} ostree admin upgrade --os=testos -# Now reset to an older revision -${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo reset testos/buildmaster/x86_64-runtime{,^} - +# Generate a new commit with an older timestamp that also has +# some new content, so we test timestamp checking during pull +# +origrev=$(ostree --repo=${test_tmpdir}/sysroot/ostree/repo rev-parse testos:${ref}) +cd ${test_tmpdir}/osdata +echo "new content for pull timestamp checking" > usr/share/test-pull-ts-check.txt +${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo commit --add-metadata-string "version=tscheck" \ + -b ${ref} --timestamp='October 25 1985' +newrev=$(ostree --repo=${test_tmpdir}/testos-repo rev-parse ${ref}) +assert_not_streq ${origrev} ${newrev} +cd ${test_tmpdir} +tscheck_checksum=$(ostree_file_path_to_checksum testos-repo ${ref} /usr/share/test-pull-ts-check.txt) +tscheck_fileobjpath=$(ostree_checksum_to_relative_object_path testos-repo ${tscheck_checksum}) +assert_has_file testos-repo/${tscheck_fileobjpath} if ${CMD_PREFIX} ostree admin upgrade --os=testos 2>upgrade-err.txt; then assert_not_reached 'upgrade unexpectedly succeeded' fi assert_file_has_content upgrade-err.txt 'chronologically older' +currev=$(ostree --repo=sysroot/ostree/repo rev-parse testos:${ref}) +assert_not_streq ${newrev} ${currev} +assert_streq ${origrev} ${currev} +assert_not_has_file sysroot/ostree/repo/$tscheck_fileobjpath echo 'ok upgrade will not go backwards'