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

diff --git a/src/libostree/ostree-core-private.h b/src/libostree/ostree-core-private.h
index 799bd228e4900e5c2069376afe73271f0ea51b46..5a2835d5ced3607835f3d1d3bc4fb808a4898daf 100644 (file)
--- 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 <libglnx.h>
 #include "ostree-ref.h"

diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c
index c13d2f2eef3eccc6fbdbdcb58faac057c16972d2..4118cf7e00ef4738368d7c6e308baf73c34f3882 100644 (file)
--- 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 e21342fe5aeafd79ebde3e7e5da58b1c122999b0..0bb1bc04ca5926fbe1732904848d2541af1419e5 100644 (file)
--- 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 8afea3a62a7eefb6f119d685c2a5f486b8449d30..f028fa7cc473a1afcae44d8b2266f84ce287139d 100644 (file)
--- 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 37cfd1431a2d0ff08df8294ec113aa1d9f6476cc..eceddb0f3293be255f80495e3a923ae31bb78e0c 100644 (file)
--- 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 1381a69e6785229fce15d274ea7ad2470e2c087b..4db8b730ea6d054237aca316e8ac71a103515f61 100755 (executable)
--- 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 f51d4445a3efa6e8cb724a8db32e0e591ebc29ce..f44c2cedd9427ad144f04f9cf343355dece1f5f2 100644 (file)
--- 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 fd1e1108940496afa722bb5478126e50643ae7a8..67d51737bdcffbbd77ca3da543286670754948db 100755 (executable)
--- 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
+# <https://github.com/ostreedev/ostree/pull/1055>
+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'