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

I now think commit fab1e113db558cb7d6754e243919558df92d4864 was a mistake;
because it breaks the mental model that at least I'd built up that "local repos
don't have checksums verified, HTTP does".

For example, a problem with this is (with that mental model in place) it's easy
for people who set up mirrors like this to then do local pulls, and at that
point we've done a deployment with no checksum verification.

Further, since then we did PR #671 AKA commit 3d38f03 which is really most of
the speed hit.

So let's switch the default even for this case to doing checksum verification,
and add `ostree pull --http-trusted`. People who are in situations where they
know they want this can find it and turn it on.

Closes: https://github.com/ostreedev/ostree/issues/1211

Closes: #1212
Approved by: jlebon

src/libostree/ostree-repo-pull.c

@@ -1006,12 +1006,15 @@ content_fetch_on_complete (GObject        *object,
   const gboolean verifying_bareuseronly =
     (pull_data->importflags & _OSTREE_REPO_IMPORT_FLAGS_VERIFY_BAREUSERONLY) > 0;
 
-  /* 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.
+  /* 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
-      && !verifying_bareuseronly)
+  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,
@@ -3541,9 +3544,13 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
     }
   else
     {
-      /* We don't add _IMPORT_FLAGS_TRUSTED for http repos;
-       * OSTREE_REPO_PULL_FLAGS_UNTRUSTED only matters for local repos.
+      /* 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.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;
@@ -57,6 +58,7 @@ static GOptionEntry options[] = {
    { "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, "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}