lib/pull: Default checksum for archive mirror, add TRUSTED_HTTP flag

src/libostree/ostree-repo-pull.c

@@ -142,8 +142,7 @@ typedef struct {
 
   gboolean          is_mirror;
   gboolean          is_commit_only;
-  gboolean          is_untrusted;
-  gboolean          is_bareuseronly_files;
+  OstreeRepoImportFlags importflags;
 
   GPtrArray        *dirs;
 
@@ -622,49 +621,28 @@ pull_matches_subdir (OtPullData *pull_data,
   return FALSE;
 }
 
-/* Synchronously import a single content object; this is used async for content,
- * or synchronously for metadata. @src_repo is either
- * pull_data->remote_repo_local or one of pull_data->localcache_repos.
- *
- * One important special case here is handling the
- * OSTREE_REPO_PULL_FLAGS_BAREUSERONLY_FILES flag.
- */
-static gboolean
-import_one_local_content_object_sync (OtPullData *pull_data,
-                                      OstreeRepo *src_repo,
-                                      const char *checksum,
-                                      GCancellable *cancellable,
-                                      GError    **error)
-{
-  OstreeRepoImportFlags flags = _OSTREE_REPO_IMPORT_FLAGS_NONE;
-  if (!pull_data->is_untrusted)
-    flags |= _OSTREE_REPO_IMPORT_FLAGS_TRUSTED;
-  if (pull_data->is_bareuseronly_files)
-    flags |= _OSTREE_REPO_IMPORT_FLAGS_VERIFY_BAREUSERONLY;
-  return _ostree_repo_import_object (pull_data->repo, src_repo,
-                                     OSTREE_OBJECT_TYPE_FILE, checksum,
-                                     flags, cancellable, error);
-}
-
 typedef struct {
   OtPullData *pull_data;
   OstreeRepo *src_repo;
   char checksum[OSTREE_SHA256_STRING_LEN+1];
 } ImportLocalAsyncData;
 
+/* Asynchronously import a single content object. @src_repo is either
+ * pull_data->remote_repo_local or one of pull_data->localcache_repos.
+ */
 static void
 async_import_in_thread (GTask *task,
                         gpointer source,
                         gpointer task_data,
                         GCancellable *cancellable)
 {
   ImportLocalAsyncData *iataskdata = task_data;
+  OtPullData *pull_data = iataskdata->pull_data;
   g_autoptr(GError) local_error = NULL;
-  if (!import_one_local_content_object_sync (iataskdata->pull_data,
-                                             iataskdata->src_repo,
-                                             iataskdata->checksum,
-                                             cancellable,
-                                             &local_error))
+  /* pull_data->importflags was set up in the pull option processing */
+  if (!_ostree_repo_import_object (pull_data->repo, iataskdata->src_repo,
+                                   OSTREE_OBJECT_TYPE_FILE, iataskdata->checksum,
+                                   pull_data->importflags, cancellable, &local_error))
     g_task_return_error (task, g_steal_pointer (&local_error));
   else
     g_task_return_boolean (task, TRUE);
@@ -1025,12 +1003,18 @@ content_fetch_on_complete (GObject        *object,
   checksum_obj = ostree_object_to_string (checksum, objtype);
   g_debug ("fetch of %s complete", checksum_obj);
 
-  /* If we're mirroring and writing into an archive repo, we can directly copy
-   * the content rather than paying the cost of exploding it, checksumming, and
-   * re-gzip.
+  const gboolean verifying_bareuseronly =
+    (pull_data->importflags & _OSTREE_REPO_IMPORT_FLAGS_VERIFY_BAREUSERONLY) > 0;
+
+  /* If we're mirroring and writing into an archive repo, and both checksum and
+   * bareuseronly are turned off, we can directly copy the content rather than
+   * paying the cost of exploding it, checksumming, and re-gzip.
    */
-  if (pull_data->is_mirror && pull_data->repo->mode == OSTREE_REPO_MODE_ARCHIVE
-      && !pull_data->is_bareuseronly_files)
+  const gboolean mirroring_into_archive =
+    pull_data->is_mirror && pull_data->repo->mode == OSTREE_REPO_MODE_ARCHIVE;
+  const gboolean import_trusted = !verifying_bareuseronly &&
+    (pull_data->importflags & _OSTREE_REPO_IMPORT_FLAGS_TRUSTED) > 0;
+  if (mirroring_into_archive && import_trusted)
     {
       gboolean have_object;
       if (!ostree_repo_has_object (pull_data->repo, OSTREE_OBJECT_TYPE_FILE, checksum,
@@ -1064,7 +1048,7 @@ content_fetch_on_complete (GObject        *object,
        */
       ot_cleanup_unlinkat (&tmp_unlinker);
 
-      if (pull_data->is_bareuseronly_files)
+      if (verifying_bareuseronly)
         {
           if (!_ostree_validate_bareuseronly_mode_finfo (file_info, checksum, error))
             goto out;
@@ -1749,35 +1733,37 @@ scan_one_metadata_object_c (OtPullData                 *pull_data,
   g_autofree char *tmp_checksum = ostree_checksum_from_bytes (csum);
   g_autoptr(GVariant) object = ostree_object_name_serialize (tmp_checksum, objtype);
 
+  /* It may happen that we've already looked at this object (think shared
+   * dirtree subtrees), if that's the case, we're done */
   if (g_hash_table_lookup (pull_data->scanned_metadata, object))
     return TRUE;
 
   gboolean is_requested = g_hash_table_lookup (pull_data->requested_metadata, object) != NULL;
+  /* Determine if we already have the object */
   gboolean is_stored;
   if (!ostree_repo_has_object (pull_data->repo, objtype, tmp_checksum, &is_stored,
                                cancellable, error))
     return FALSE;
 
-  if (pull_data->remote_repo_local)
+  /* Are we pulling an object we don't have from a local repo? */
+  if (!is_stored && pull_data->remote_repo_local)
     {
-      if (!is_stored)
+      if (objtype == OSTREE_OBJECT_TYPE_COMMIT)
         {
-          if (objtype == OSTREE_OBJECT_TYPE_COMMIT)
-            {
-              /* mark as partial to ensure we scan the commit below */
-              if (!write_commitpartial_for (pull_data, tmp_checksum, error))
-                return FALSE;
-            }
-          if (!ostree_repo_import_object_from_with_trust (pull_data->repo, pull_data->remote_repo_local,
-                                                          objtype, tmp_checksum, !pull_data->is_untrusted,
-                                                          cancellable, error))
+          /* mark as partial to ensure we scan the commit below */
+          if (!write_commitpartial_for (pull_data, tmp_checksum, error))
             return FALSE;
-          /* The import API will fetch both the commit and detached metadata, so
-           * add it to the hash to avoid re-fetching it below.
-           */
-          if (objtype == OSTREE_OBJECT_TYPE_COMMIT)
-            g_hash_table_add (pull_data->fetched_detached_metadata, g_strdup (tmp_checksum));
         }
+
+      if (!_ostree_repo_import_object (pull_data->repo, pull_data->remote_repo_local,
+                                       objtype, tmp_checksum, pull_data->importflags,
+                                       cancellable, error))
+        return FALSE;
+      /* The import API will fetch both the commit and detached metadata, so
+       * add it to the hash to avoid re-fetching it below.
+       */
+      if (objtype == OSTREE_OBJECT_TYPE_COMMIT)
+        g_hash_table_add (pull_data->fetched_detached_metadata, g_strdup (tmp_checksum));
       is_stored = TRUE;
       is_requested = TRUE;
     }
@@ -1800,10 +1786,9 @@ scan_one_metadata_object_c (OtPullData                 *pull_data,
               if (!write_commitpartial_for (pull_data, tmp_checksum, error))
                 return FALSE;
             }
-          if (!ostree_repo_import_object_from_with_trust (pull_data->repo, refd_repo,
-                                                          objtype, tmp_checksum,
-                                                          !pull_data->is_untrusted,
-                                                          cancellable, error))
+          if (!_ostree_repo_import_object (pull_data->repo, refd_repo,
+                                           objtype, tmp_checksum, pull_data->importflags,
+                                           cancellable, error))
             return FALSE;
           /* See comment above */
           if (objtype == OSTREE_OBJECT_TYPE_COMMIT)
@@ -3249,8 +3234,9 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
 
   pull_data->is_mirror = (flags & OSTREE_REPO_PULL_FLAGS_MIRROR) > 0;
   pull_data->is_commit_only = (flags & OSTREE_REPO_PULL_FLAGS_COMMIT_ONLY) > 0;
-  pull_data->is_untrusted = (flags & OSTREE_REPO_PULL_FLAGS_UNTRUSTED) > 0;
-  pull_data->is_bareuseronly_files = (flags & OSTREE_REPO_PULL_FLAGS_BAREUSERONLY_FILES) > 0;
+  /* See our processing of OSTREE_REPO_PULL_FLAGS_UNTRUSTED below */
+  if ((flags & OSTREE_REPO_PULL_FLAGS_BAREUSERONLY_FILES) > 0)
+    pull_data->importflags |= _OSTREE_REPO_IMPORT_FLAGS_VERIFY_BAREUSERONLY;
   pull_data->cancellable = cancellable ? g_object_ref (cancellable) : NULL;
 
   if (error)
@@ -3539,6 +3525,9 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
     }
   }
 
+  /* Change some option defaults if we're actually pulling from a local
+   * (filesystem accessible) repo.
+   */
   if (pull_data->remote_repo_local)
     {
       /* For local pulls, default to disabling static deltas so that the
@@ -3547,6 +3536,21 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
       if (!pull_data->require_static_deltas)
         pull_data->disable_static_deltas = TRUE;
 
+      /* Note the inversion here; PULL_FLAGS_UNTRUSTED is converted to
+       * IMPORT_FLAGS_TRUSTED only if it's unset (and just for local repos).
+       */
+      if ((flags & OSTREE_REPO_PULL_FLAGS_UNTRUSTED) == 0)
+        pull_data->importflags |= _OSTREE_REPO_IMPORT_FLAGS_TRUSTED;
+    }
+  else
+    {
+      /* For non-local repos, we require the TRUSTED_HTTP pull flag to map to
+       * the TRUSTED object import flag. In practice we don't do object imports
+       * for HTTP, but it's easiest to use one set of flags between HTTP and
+       * local imports.
+       */
+      if (flags & OSTREE_REPO_PULL_FLAGS_TRUSTED_HTTP)
+        pull_data->importflags |= _OSTREE_REPO_IMPORT_FLAGS_TRUSTED;
     }
 
   /* We can't use static deltas if pulling into an archive repo. */

src/libostree/ostree-repo.h

@@ -1126,15 +1126,17 @@ gboolean ostree_repo_prune_from_reachable (OstreeRepo             *self,
  * @OSTREE_REPO_PULL_FLAGS_NONE: No special options for pull
  * @OSTREE_REPO_PULL_FLAGS_MIRROR: Write out refs suitable for mirrors and fetch all refs if none requested
  * @OSTREE_REPO_PULL_FLAGS_COMMIT_ONLY: Fetch only the commit metadata
- * @OSTREE_REPO_PULL_FLAGS_UNTRUSTED: Don't trust local remote
+ * @OSTREE_REPO_PULL_FLAGS_UNTRUSTED: Do verify checksums of local (filesystem-accessible) repositories (defaults on for HTTP)
  * @OSTREE_REPO_PULL_FLAGS_BAREUSERONLY_FILES: Since 2017.7.  Reject writes of content objects with modes outside of 0775.
+ * @OSTREE_REPO_PULL_FLAGS_TRUSTED_HTTP: Don't verify checksums of objects HTTP repositories (Since: 2017.12)
  */
 typedef enum {
   OSTREE_REPO_PULL_FLAGS_NONE,
   OSTREE_REPO_PULL_FLAGS_MIRROR = (1 << 0),
   OSTREE_REPO_PULL_FLAGS_COMMIT_ONLY = (1 << 1),
   OSTREE_REPO_PULL_FLAGS_UNTRUSTED = (1 << 2),
-  OSTREE_REPO_PULL_FLAGS_BAREUSERONLY_FILES = (1 << 3)
+  OSTREE_REPO_PULL_FLAGS_BAREUSERONLY_FILES = (1 << 3),
+  OSTREE_REPO_PULL_FLAGS_TRUSTED_HTTP = (1 << 4),
 } OstreeRepoPullFlags;
 
 _OSTREE_PUBLIC

src/ostree/ot-builtin-pull-local.c

@@ -46,7 +46,7 @@ static int opt_depth = 0;
 static GOptionEntry options[] = {
   { "remote", 0, 0, G_OPTION_ARG_STRING, &opt_remote, "Add REMOTE to refspec", "REMOTE" },
   { "disable-fsync", 0, 0, G_OPTION_ARG_NONE, &opt_disable_fsync, "Do not invoke fsync()", NULL },
-  { "untrusted", 0, 0, G_OPTION_ARG_NONE, &opt_untrusted, "Do not verify checksums of local sources (always enabled for HTTP pulls)", NULL },
+  { "untrusted", 0, 0, G_OPTION_ARG_NONE, &opt_untrusted, "Verify checksums of local sources (always enabled for HTTP pulls)", NULL },
   { "bareuseronly-files", 0, 0, G_OPTION_ARG_NONE, &opt_bareuseronly_files, "Reject regular files with mode outside of 0775 (world writable, suid, etc.)", NULL },
   { "require-static-deltas", 0, 0, G_OPTION_ARG_NONE, &opt_require_static_deltas, "Require static deltas", NULL },
   { "gpg-verify", 0, 0, G_OPTION_ARG_NONE, &opt_gpg_verify, "GPG verify commits (must specify --remote)", NULL },

src/ostree/ot-builtin-pull.c

@@ -33,6 +33,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_http_trusted;
 static gboolean opt_timestamp_check;
 static gboolean opt_bareuseronly_files;
 static char** opt_subpaths;
@@ -56,7 +57,8 @@ static GOptionEntry options[] = {
    { "require-static-deltas", 0, 0, G_OPTION_ARG_NONE, &opt_require_static_deltas, "Require static deltas", NULL },
    { "mirror", 0, 0, G_OPTION_ARG_NONE, &opt_mirror, "Write refs suitable for a mirror and fetches all refs if none provided", NULL },
    { "subpath", 0, 0, G_OPTION_ARG_FILENAME_ARRAY, &opt_subpaths, "Only pull the provided subpath(s)", NULL },
-   { "untrusted", 0, 0, G_OPTION_ARG_NONE, &opt_untrusted, "Do not verify checksums of local sources (always enabled for HTTP pulls)", NULL },
+   { "untrusted", 0, 0, G_OPTION_ARG_NONE, &opt_untrusted, "Verify checksums of local sources (always enabled for HTTP pulls)", NULL },
+   { "http-trusted", 0, 0, G_OPTION_ARG_NONE, &opt_http_trusted, "Do not verify checksums of HTTP sources (mostly useful when mirroring)", NULL },
    { "bareuseronly-files", 0, 0, G_OPTION_ARG_NONE, &opt_bareuseronly_files, "Reject regular files with mode outside of 0775 (world writable, suid, etc.)", NULL },
    { "dry-run", 0, 0, G_OPTION_ARG_NONE, &opt_dry_run, "Only print information on what will be downloaded (requires static deltas)", NULL },
    { "depth", 0, 0, G_OPTION_ARG_INT, &opt_depth, "Traverse DEPTH parents (-1=infinite) (default: 0)", "DEPTH" },
@@ -182,8 +184,14 @@ ostree_builtin_pull (int argc, char **argv, GCancellable *cancellable, GError **
   if (opt_commit_only)
     pullflags |= OSTREE_REPO_PULL_FLAGS_COMMIT_ONLY;
 
+  if (opt_http_trusted)
+    pullflags |= OSTREE_REPO_PULL_FLAGS_TRUSTED_HTTP;
   if (opt_untrusted)
-    pullflags |= OSTREE_REPO_PULL_FLAGS_UNTRUSTED;
+    {
+      pullflags |= OSTREE_REPO_PULL_FLAGS_UNTRUSTED;
+      /* If the user specifies both, assume they really mean untrusted */
+      pullflags &= ~OSTREE_REPO_PULL_FLAGS_TRUSTED_HTTP;
+    }
   if (opt_bareuseronly_files)
     pullflags |= OSTREE_REPO_PULL_FLAGS_BAREUSERONLY_FILES;
 

tests/pull-test.sh

@@ -35,7 +35,7 @@ function verify_initial_contents() {
     assert_file_has_content baz/cow '^moo$'
 }
 
-echo "1..29"
+echo "1..30"
 
 # Try both syntaxes
 repo_init --no-gpg-verify
@@ -137,6 +137,63 @@ ${CMD_PREFIX} ostree --repo=mirrorrepo prune --refs-only
 ${CMD_PREFIX} ostree --repo=mirrorrepo pull --mirror --bareuseronly-files origin main
 echo "ok pull (bareuseronly mirror)"
 
+# Corruption tests <https://github.com/ostreedev/ostree/issues/1211>
+cd ${test_tmpdir}
+repo_init --no-gpg-verify
+if ! is_bare_user_only_repo repo && ! skip_one_without_user_xattrs; then
+    if is_bare_user_only_repo repo; then
+        cacherepomode=bare-user-only
+    else
+        cacherepomode=bare-user
+    fi
+    rm cacherepo -rf
+    ostree_repo_init cacherepo --mode=${cacherepomode}
+    ${CMD_PREFIX} ostree --repo=cacherepo pull-local ostree-srv/gnomerepo main
+    rev=$(ostree --repo=cacherepo rev-parse main)
+    ${CMD_PREFIX} ostree --repo=cacherepo ls -R -C main > ls.txt
+    regfile_hash=$(grep -E -e '^-0' ls.txt | head -1 | awk '{ print $5 }')
+    ${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false corruptrepo $(cat httpd-address)/ostree/corruptrepo
+    # Make this a loop so in the future we can add more object types like commit etc.
+    for object in ${regfile_hash}.file; do
+        checksum=$(echo ${object} | sed -e 's,\(.*\)\.[a-z]*$,\1,')
+        path=cacherepo/objects/${object:0:2}/${object:2}
+        # Preserve user.ostreemeta xattr
+        cp -a ${path}{,.new}
+        (dd if=${path} conv=swab) > ${path}.new
+        mv ${path}{.new,}
+        if ${CMD_PREFIX} ostree --repo=cacherepo fsck 2>err.txt; then
+            fatal "corrupt repo fsck?"
+        fi
+        assert_file_has_content err.txt "corrupted.*${checksum}"
+        rm ostree-srv/corruptrepo -rf
+        ostree_repo_init ostree-srv/corruptrepo --mode=archive
+        ${CMD_PREFIX} ostree --repo=ostree-srv/corruptrepo pull-local cacherepo main
+        # Pulling via HTTP into a non-archive should fail, even with
+        # --http-trusted.
+        if ${CMD_PREFIX} ostree --repo=repo pull --http-trusted corruptrepo main 2>err.txt; then
+            fatal "Pulled from corrupt repo?"
+        fi
+        assert_file_has_content err.txt "Corrupted.*${checksum}"
+        if ${CMD_PREFIX} ostree --repo=repo show corruptrepo:main >/dev/null; then
+            fatal "Pulled from corrupt repo?"
+        fi
+        ${CMD_PREFIX} ostree --repo=repo prune --refs-only
+        rm repo/tmp/* -rf
+        ostree_repo_init corruptmirrorrepo --mode=archive
+        # Pulling via http-trusted should not verify the checksum
+        ${CMD_PREFIX} ostree --repo=corruptmirrorrepo remote add --set=gpg-verify=false corruptrepo $(cat httpd-address)/ostree/corruptrepo
+        ${CMD_PREFIX} ostree --repo=corruptmirrorrepo pull --mirror --http-trusted corruptrepo main
+        # But it should fail to fsck
+        if ${CMD_PREFIX} ostree --repo=corruptmirrorrepo fsck 2>err.txt; then
+            fatal "corrupt mirror repo fsck?"
+        fi
+    done
+
+    # And ensure the repo is reinitialized
+    repo_init --no-gpg-verify
+    echo "ok corruption"
+fi
+
 cd ${test_tmpdir}
 rm mirrorrepo/refs/remotes/* -rf
 ${CMD_PREFIX} ostree --repo=mirrorrepo prune --refs-only
@@ -158,7 +215,7 @@ ostree_repo_init mirrorrepo-local --mode=archive
 ${CMD_PREFIX} ostree --repo=mirrorrepo-local remote add --set=gpg-verify=false origin file://$(pwd)/ostree-srv/gnomerepo
 ${CMD_PREFIX} ostree --repo=mirrorrepo-local pull --mirror origin main
 ${CMD_PREFIX} ostree --repo=mirrorrepo-local fsck
-$OSTREE show main >/dev/null
+${CMD_PREFIX} ostree --repo=mirrorrepo show main >/dev/null
 echo "ok pull local mirror"
 
 cd ${test_tmpdir}