From c8efce06564b7adef83994dddb41cd61a030207d Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 14 May 2020 13:44:32 -0400 Subject: [PATCH 1/2] lib/pull: Add `timestamp-check-from-rev` The way `timestamp-check` works might be too restrictive in some situations. Essentially, we need to support the case where users want to pull an older commit than the current tip, but while still guaranteeing that it is newer than some even older commit. This will be used in Fedora CoreOS. For more information see: https://github.com/coreos/rpm-ostree/pull/2094 https://github.com/coreos/fedora-coreos-tracker/issues/481 --- src/libostree/ostree-repo-pull-private.h | 1 + src/libostree/ostree-repo-pull.c | 29 +++++++++++++++++++++++- src/ostree/ot-builtin-pull.c | 5 ++++ tests/pull-test.sh | 24 +++++++++++++++++++- 4 files changed, 57 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-pull-private.h b/src/libostree/ostree-repo-pull-private.h index 5bc2a33a..86d1ffee 100644 --- a/src/libostree/ostree-repo-pull-private.h +++ b/src/libostree/ostree-repo-pull-private.h @@ -114,6 +114,7 @@ typedef struct { guint n_imported_content; gboolean timestamp_check; /* Verify commit timestamps */ + char *timestamp_check_from_rev; int maxdepth; guint64 max_metadata_size; guint64 start_time; diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 363db9e2..291f3fe6 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1593,6 +1593,7 @@ scan_commit_object (OtPullData *pull_data, commit, error)) return glnx_prefix_error (error, "Commit %s", checksum); + guint64 new_ts = ostree_commit_get_timestamp (commit); if (pull_data->timestamp_check) { /* We don't support timestamp checking while recursing right now */ @@ -1611,11 +1612,22 @@ scan_commit_object (OtPullData *pull_data, return glnx_prefix_error (error, "Reading %s for timestamp-check", ref->ref_name); 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)) return FALSE; } } + if (pull_data->timestamp_check_from_rev) + { + g_autoptr(GVariant) commit = NULL; + if (!ostree_repo_load_commit (pull_data->repo, pull_data->timestamp_check_from_rev, + &commit, NULL, error)) + return glnx_prefix_error (error, "Reading %s for timestamp-check-from-rev", + pull_data->timestamp_check_from_rev); + + guint64 ts = ostree_commit_get_timestamp (commit); + if (!_ostree_compare_timestamps (pull_data->timestamp_check_from_rev, ts, checksum, new_ts, error)) + return FALSE; + } /* If we found a legacy transaction flag, assume all commits are partial */ gboolean is_partial = commitstate_is_partial (pull_data, commitstate); @@ -3270,6 +3282,7 @@ initiate_request (OtPullData *pull_data, * * 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 + * * timestamp-check-from-rev (s): Verify that all fetched commit timestamps are newer than timestamp of given rev; Since: 2020.4 * * metadata-size-restriction (t): Restrict metadata objects to a maximum number of bytes; 0 to disable. Since: 2018.9 * * 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 @@ -3372,6 +3385,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, (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); + (void) g_variant_lookup (options, "timestamp-check-from-rev", "s", &pull_data->timestamp_check_from_rev); (void) g_variant_lookup (options, "max-metadata-size", "t", &pull_data->max_metadata_size); (void) g_variant_lookup (options, "append-user-agent", "s", &pull_data->append_user_agent); opt_n_network_retries_set = @@ -4259,6 +4273,18 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_steal_pointer (&checksum)); } + /* Resolve refs to a checksum if necessary */ + if (pull_data->timestamp_check_from_rev && + !ostree_validate_checksum_string (pull_data->timestamp_check_from_rev, NULL)) + { + g_autofree char *from_rev = NULL; + if (!ostree_repo_resolve_rev (pull_data->repo, pull_data->timestamp_check_from_rev, FALSE, + &from_rev, error)) + goto out; + g_free (pull_data->timestamp_check_from_rev); + pull_data->timestamp_check_from_rev = g_steal_pointer (&from_rev); + } + g_hash_table_unref (requested_refs_to_fetch); requested_refs_to_fetch = g_steal_pointer (&updated_requested_refs_to_fetch); if (g_hash_table_size (requested_refs_to_fetch) == 1) @@ -4604,6 +4630,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, 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_free (pull_data->timestamp_check_from_rev); g_clear_pointer (&pull_data->verified_commits, (GDestroyNotify) g_hash_table_unref); g_clear_pointer (&pull_data->ref_keyring_map, (GDestroyNotify) g_hash_table_unref); g_clear_pointer (&pull_data->requested_content, (GDestroyNotify) g_hash_table_unref); diff --git a/src/ostree/ot-builtin-pull.c b/src/ostree/ot-builtin-pull.c index 1fae0a38..1625ab47 100644 --- a/src/ostree/ot-builtin-pull.c +++ b/src/ostree/ot-builtin-pull.c @@ -37,6 +37,7 @@ static gboolean opt_require_static_deltas; static gboolean opt_untrusted; static gboolean opt_http_trusted; static gboolean opt_timestamp_check; +static char* opt_timestamp_check_from_rev; static gboolean opt_bareuseronly_files; static char** opt_subpaths; static char** opt_http_headers; @@ -72,6 +73,7 @@ static GOptionEntry options[] = { { "network-retries", 0, 0, G_OPTION_ARG_INT, &opt_network_retries, "Specifies how many times each download should be retried upon error (default: 5)", "N"}, { "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 }, + { "timestamp-check-from-rev", 0, 0, G_OPTION_ARG_STRING, &opt_timestamp_check_from_rev, "Require fetched commits to have newer timestamps than given rev", NULL }, /* let's leave this hidden for now; we just need it for tests */ { "append-user-agent", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_STRING, &opt_append_user_agent, "Append string to user agent", NULL }, { NULL } @@ -313,6 +315,9 @@ ostree_builtin_pull (int argc, char **argv, OstreeCommandInvocation *invocation, 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 (opt_timestamp_check_from_rev) + g_variant_builder_add (&builder, "{s@v}", "timestamp-check-from-rev", + g_variant_new_variant (g_variant_new_string (opt_timestamp_check_from_rev))); if (override_commit_ids) g_variant_builder_add (&builder, "{s@v}", "override-commit-ids", diff --git a/tests/pull-test.sh b/tests/pull-test.sh index a52adab9..92bcf112 100644 --- a/tests/pull-test.sh +++ b/tests/pull-test.sh @@ -318,7 +318,7 @@ ${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 +# test pull -T and --timestamp-check-from-rev cd ${test_tmpdir} repo_init --no-sign-verify ${CMD_PREFIX} ostree --repo=repo pull origin main @@ -347,6 +347,28 @@ 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 +# Now test --timestamp-check-from-rev. First, add two new commits with distinct +# but newer timestamps. +oldrev=${newrev2} +middlerev=$(${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo commit ${COMMIT_ARGS} -b main --tree=ref=main) +sleep 1 +latestrev=$(${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo commit ${COMMIT_ARGS} -b main --tree=ref=main) +${CMD_PREFIX} ostree --repo=ostree-srv/gnomerepo summary -u +# OK, let's pull the latest now. +${CMD_PREFIX} ostree --repo=repo pull -T origin main +assert_streq ${latestrev} "$(${CMD_PREFIX} ostree --repo=repo rev-parse main)" +# Check we can't pull the middle commit by overrides with ts checking on +if ${CMD_PREFIX} ostree --repo=repo pull -T origin main@${middlerev} 2>err.txt; then + fatal "pulled older commit override with timestamp checking enabled?" +fi +assert_file_has_content err.txt "Upgrade.*is chronologically older" +# Check we can't pull an older commit by override if it's newer than --timestamp-check-from-rev +if ${CMD_PREFIX} ostree --repo=repo pull --timestamp-check-from-rev=${latestrev} origin main@${middlerev} 2>err.txt; then + fatal "pulled older commit override with timestamp checking enabled?" +fi +assert_file_has_content err.txt "Upgrade.*is chronologically older" +# But we can pull it with --timestamp-check-from-rev when starting from the oldrev +${CMD_PREFIX} ostree --repo=repo pull --timestamp-check-from-rev=${oldrev} origin main@${middlerev} echo "ok pull timestamp checking" cd ${test_tmpdir} From 79079c265759613e254b377a937c1baa34cc109e Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 14 May 2020 15:05:45 -0400 Subject: [PATCH 2/2] lib/upgrader: Pull with `timestamp-check-from-rev` For the same reason as https://github.com/coreos/rpm-ostree/pull/2094. What we care most about is that the new commit we pull is newer than the one we're currently sitting on, not necessarily that it's newer than the branch itself, which it might not be if e.g. we're trying to deploy a commit older than the tip but still newer than the deployment (via `--override-commit`). --- src/libostree/ostree-sysroot-upgrader.c | 4 ++-- tests/admin-test.sh | 20 +++++++++++++++++++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-sysroot-upgrader.c b/src/libostree/ostree-sysroot-upgrader.c index 8fb231a3..46813358 100644 --- a/src/libostree/ostree-sysroot-upgrader.c +++ b/src/libostree/ostree-sysroot-upgrader.c @@ -530,8 +530,8 @@ ostree_sysroot_upgrader_pull_one_dir (OstreeSysrootUpgrader *self, 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}", "timestamp-check-from-rev", + g_variant_new_variant (g_variant_new_string (from_revision))); g_variant_builder_add (optbuilder, "{s@v}", "refs", g_variant_new_variant (g_variant_new_strv ((const char *const*) refs_to_fetch, -1))); diff --git a/tests/admin-test.sh b/tests/admin-test.sh index 11b9ea14..03b455a3 100644 --- a/tests/admin-test.sh +++ b/tests/admin-test.sh @@ -21,7 +21,7 @@ set -euo pipefail -echo "1..$((27 + ${extra_admin_tests:-0}))" +echo "1..$((28 + ${extra_admin_tests:-0}))" mkdir sysrootmin ${CMD_PREFIX} ostree admin init-fs --modern sysrootmin @@ -304,8 +304,14 @@ ${CMD_PREFIX} ostree pull --repo=sysroot/ostree/repo --commit-metadata-only --de head_rev=$(${CMD_PREFIX} ostree rev-parse --repo=sysroot/ostree/repo testos/buildmaster/x86_64-runtime) prev_rev=$(${CMD_PREFIX} ostree rev-parse --repo=sysroot/ostree/repo testos/buildmaster/x86_64-runtime^^^^) assert_not_streq ${head_rev} ${prev_rev} +# check that we can't "upgrade" to an older commit without --allow-downgrade +if ${CMD_PREFIX} ostree admin upgrade --os=testos --override-commit=${prev_rev} 2> err.txt; then + fatal "downgraded without --allow-downgrade?" +fi +assert_file_has_content err.txt "Upgrade.*is chronologically older" ${CMD_PREFIX} ostree admin upgrade --os=testos --override-commit=${prev_rev} --allow-downgrade curr_rev=$(${CMD_PREFIX} ostree rev-parse --repo=sysroot/ostree/repo testos/buildmaster/x86_64-runtime) + assert_streq ${curr_rev} ${prev_rev} ${CMD_PREFIX} ostree admin upgrade --os=testos curr_rev=$(${CMD_PREFIX} ostree rev-parse --repo=sysroot/ostree/repo testos/buildmaster/x86_64-runtime) @@ -313,6 +319,18 @@ assert_streq ${curr_rev} ${head_rev} echo "ok upgrade with and without override-commit" +# check that we can still upgrade to a rev that's not the tip of the branch but +# that's still newer than the deployment +sleep 1 +os_repository_new_commit +sleep 1 +os_repository_new_commit +${CMD_PREFIX} ostree pull --repo=sysroot/ostree/repo --commit-metadata-only --depth=-1 testos:testos/buildmaster/x86_64-runtime +curr_rev=$(${CMD_PREFIX} ostree rev-parse --repo=sysroot/ostree/repo testos/buildmaster/x86_64-runtime) +prev_rev=$(${CMD_PREFIX} ostree rev-parse --repo=sysroot/ostree/repo testos/buildmaster/x86_64-runtime^) +${CMD_PREFIX} ostree admin upgrade --os=testos --override-commit=${prev_rev} +echo "ok upgrade to newer version older than branch tip" + ${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo commit --add-metadata-string "version=${version}" \ --add-metadata-string 'ostree.source-title=libtest os_repository_new_commit()' -b testos/buildmaster/x86_64-runtime \ -s "Build" --tree=dir=${test_tmpdir}/osdata