Merge pull request #2099 from jlebon/pr/timestamp-check-from-rev

lib/pull: Add `timestamp-check-from-rev`

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;

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);

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)));

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",

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

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}