diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index d8048e757e..cd5fe52039 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/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,36 +621,15 @@ 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, @@ -659,12 +637,12 @@ async_import_in_thread (GTask *task, 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. */ diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index 1ba71c12d1..51b44b3f93 100644 --- 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-local.c b/src/ostree/ot-builtin-pull-local.c index d80c5a257f..c3dc50c9c3 100644 --- a/src/ostree/ot-builtin-pull-local.c +++ b/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 }, diff --git a/src/ostree/ot-builtin-pull.c b/src/ostree/ot-builtin-pull.c index 47f9d8be5a..e67d59930f 100644 --- 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; @@ -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; diff --git a/tests/pull-test.sh b/tests/pull-test.sh index 3780d1ad57..573f83608f 100644 --- 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 +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}