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

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index 91cad430ec61f0606c15caa5c41eb52bd6945014..cd5fe52039ba681d6209cf09368f71a26943a1cd 100644 (file)
--- a/src/libostree/ostree-repo-pull.c
+++ b/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. */

diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h
index 1ba71c12d15984baca7ded7d83a9c116d15d5e55..51b44b3f931ab27a49cd7c5ca6fdacc32381f58f 100644 (file)
--- a/src/libostree/ostree-repo.h
+++ b/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

diff --git a/src/ostree/ot-builtin-pull.c b/src/ostree/ot-builtin-pull.c
index a6fab0e35a21519afb8d45ddd6b2ff862dc5b2ca..e67d59930f4c3c968ec1a7eb0597b2a9a1a55841 100644 (file)
--- a/src/ostree/ot-builtin-pull.c
+++ b/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;
 

diff --git a/tests/pull-test.sh b/tests/pull-test.sh
index 3780d1ad57a92adc02fc49b84ff6c10be3539d24..573f83608ff85d228f08bcf52b7c84d7357c556a 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..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}