Skip to content

Commit

Permalink
pull: Allow disabling commit binding verification
Browse files Browse the repository at this point in the history
In some cases such as backups or mirroring you may want to pull commits
from one repo to another even if there commits that have incorrect
bindings. Fixing the commits in the source repository to have correct
bindings may not be feasible, so provide a pull option to disable
verification.

For Endless we have several repositories that predate collection IDs and
ref bindings. Later these repositories gained collection IDs to support
the features they provide and ref bindings as the ostree tooling was
upgraded. These repositories contain released commits that were valid to
the clients they were targeting at the time. Correcting the bindings is
not really an option as it would mean invalidating the repository
history.

(cherry picked from commit 4db2ba0)

https://phabricator.endlessm.com/T20138
  • Loading branch information
dbnicholson committed Mar 19, 2021
1 parent 090932d commit ef2d75c
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 11 deletions.
2 changes: 2 additions & 0 deletions bash/ostree
Original file line number Diff line number Diff line change
Expand Up @@ -849,6 +849,7 @@ _ostree_pull_local() {
--gpg-verify-summary
--require-static-deltas
--untrusted
--disable-verify-bindings
"

local options_with_args="
Expand Down Expand Up @@ -904,6 +905,7 @@ _ostree_pull() {
--untrusted
--bareuseronly-files
--dry-run
--disable-verify-bindings
"

local options_with_args="
Expand Down
8 changes: 8 additions & 0 deletions man/ostree-pull-local.xml
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,14 @@ Boston, MA 02111-1307, USA.
Do not trust source, verify checksums and don't hardlink into source.
</para></listitem>
</varlistentry>

<varlistentry>
<term><option>--disable-verify-bindings</option></term>

<listitem><para>
Disable verification of commit metadata bindings.
</para></listitem>
</varlistentry>
</variablelist>
</refsect1>

Expand Down
8 changes: 8 additions & 0 deletions man/ostree-pull.xml
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,14 @@ Boston, MA 02111-1307, USA.
Specifies how many times each download should be retried upon error (default: 5)
</para></listitem>
</varlistentry>

<varlistentry>
<term><option>--disable-verify-bindings</option></term>

<listitem><para>
Disable verification of commit metadata bindings.
</para></listitem>
</varlistentry>
</variablelist>
</refsect1>

Expand Down
1 change: 1 addition & 0 deletions src/libostree/ostree-repo-pull-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ typedef struct {
gboolean require_static_deltas;
gboolean disable_static_deltas;
gboolean has_tombstone_commits;
gboolean disable_verify_bindings;

GBytes *summary_data;
char *summary_etag;
Expand Down
23 changes: 14 additions & 9 deletions src/libostree/ostree-repo-pull.c
Original file line number Diff line number Diff line change
Expand Up @@ -1620,15 +1620,17 @@ scan_commit_object (OtPullData *pull_data,
if (!ostree_repo_load_commit (pull_data->repo, checksum, &commit, &commitstate, error))
return FALSE;

/* If ref is non-NULL then the commit we fetched was requested through the
* branch, otherwise we requested a commit checksum without specifying a branch.
*/
g_autofree char *remote_collection_id = NULL;
remote_collection_id = get_remote_repo_collection_id (pull_data);
if (!_ostree_repo_verify_bindings (remote_collection_id,
(ref != NULL) ? ref->ref_name : NULL,
commit, error))
return glnx_prefix_error (error, "Commit %s", checksum);
if (!pull_data->disable_verify_bindings) {
/* If ref is non-NULL then the commit we fetched was requested through the
* branch, otherwise we requested a commit checksum without specifying a branch.
*/
g_autofree char *remote_collection_id = NULL;
remote_collection_id = get_remote_repo_collection_id (pull_data);
if (!_ostree_repo_verify_bindings (remote_collection_id,
(ref != NULL) ? ref->ref_name : NULL,
commit, error))
return glnx_prefix_error (error, "Commit %s", checksum);
}

guint64 new_ts = ostree_commit_get_timestamp (commit);
if (pull_data->timestamp_check)
Expand Down Expand Up @@ -3670,6 +3672,8 @@ all_requested_refs_have_commit (GHashTable *requested_refs /* (element-type Ostr
* specified, the `summary` will be downloaded from the remote. Since: 2020.5
* * `summary-sig-bytes` (`ay`): Contents of the `summary.sig` file. If this
* is specified, `summary-bytes` must also be specified. Since: 2020.5
* * `disable-verify-bindings` (`b`): Disable verification of commit bindings.
* Since: 2020.9
*/
static gboolean
ostree_repo_pull_with_options_internal (OstreeRepo *self,
Expand Down Expand Up @@ -3771,6 +3775,7 @@ ostree_repo_pull_with_options_internal (OstreeRepo *self,
g_variant_lookup (options, "ref-keyring-map", "a(sss)", &ref_keyring_map_iter);
(void) g_variant_lookup (options, "summary-bytes", "@ay", &summary_bytes_v);
(void) g_variant_lookup (options, "summary-sig-bytes", "@ay", &summary_sig_bytes_v);
(void) g_variant_lookup (options, "disable-verify-bindings", "b", &pull_data->disable_verify_bindings);

if (pull_data->remote_refspec_name != NULL)
pull_data->remote_name = g_strdup (pull_data->remote_refspec_name);
Expand Down
4 changes: 4 additions & 0 deletions src/ostree/ot-builtin-pull-local.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ static gboolean opt_bareuseronly_files;
static gboolean opt_require_static_deltas;
static gboolean opt_gpg_verify;
static gboolean opt_gpg_verify_summary;
static gboolean opt_disable_verify_bindings;
static int opt_depth = 0;

/* ATTENTION:
Expand All @@ -57,6 +58,7 @@ static GOptionEntry options[] = {
{ "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 },
{ "gpg-verify-summary", 0, 0, G_OPTION_ARG_NONE, &opt_gpg_verify_summary, "GPG verify summary (must specify --remote)", NULL },
{ "disable-verify-bindings", 0, 0, G_OPTION_ARG_NONE, &opt_disable_verify_bindings, "Do not verify commit bindings", NULL },
{ "depth", 0, 0, G_OPTION_ARG_INT, &opt_depth, "Traverse DEPTH parents (-1=infinite) (default: 0)", "DEPTH" },
{ NULL }
};
Expand Down Expand Up @@ -181,6 +183,8 @@ ostree_builtin_pull_local (int argc, char **argv, OstreeCommandInvocation *invoc
if (opt_gpg_verify_summary)
g_variant_builder_add (&builder, "{s@v}", "gpg-verify-summary",
g_variant_new_variant (g_variant_new_boolean (TRUE)));
g_variant_builder_add (&builder, "{s@v}", "disable-verify-bindings",
g_variant_new_variant (g_variant_new_boolean (opt_disable_verify_bindings)));
g_variant_builder_add (&builder, "{s@v}", "depth",
g_variant_new_variant (g_variant_new_int32 (opt_depth)));
/* local pulls always disable signapi verification. If you don't want this, use
Expand Down
4 changes: 4 additions & 0 deletions src/ostree/ot-builtin-pull.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ static gboolean opt_require_static_deltas;
static gboolean opt_untrusted;
static gboolean opt_http_trusted;
static gboolean opt_timestamp_check;
static gboolean opt_disable_verify_bindings;
static char* opt_timestamp_check_from_rev;
static gboolean opt_bareuseronly_files;
static char** opt_subpaths;
Expand Down Expand Up @@ -76,6 +77,7 @@ static GOptionEntry options[] = {
{ "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 },
{ "timestamp-check-from-rev", 0, 0, G_OPTION_ARG_STRING, &opt_timestamp_check_from_rev, "Require fetched commits to have newer timestamps than given rev", NULL },
{ "disable-verify-bindings", 0, 0, G_OPTION_ARG_NONE, &opt_disable_verify_bindings, "Do not verify commit bindings", NULL },
/* let's leave this hidden for now; we just need it for tests */
{ "append-user-agent", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_STRING, &opt_append_user_agent, "Append string to user agent", NULL },
{ NULL }
Expand Down Expand Up @@ -330,6 +332,8 @@ ostree_builtin_pull (int argc, char **argv, OstreeCommandInvocation *invocation,
if (opt_per_object_fsync)
g_variant_builder_add (&builder, "{s@v}", "per-object-fsync",
g_variant_new_variant (g_variant_new_boolean (TRUE)));
g_variant_builder_add (&builder, "{s@v}", "disable-verify-bindings",
g_variant_new_variant (g_variant_new_boolean (opt_disable_verify_bindings)));
if (opt_http_headers)
{
GVariantBuilder hdr_builder;
Expand Down
12 changes: 10 additions & 2 deletions tests/test-pull-collections.sh
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ do_pull() {
local branch=$3
shift 3

if ${CMD_PREFIX} ostree "--repo=${repo}" pull "${remote_repo}-remote" "${branch}"
if ${CMD_PREFIX} ostree "--repo=${repo}" pull "$@" "${remote_repo}-remote" "${branch}"
then return 0
else return 1
fi
Expand All @@ -129,7 +129,7 @@ do_local_pull() {
local branch=$3
shift 3

if ${CMD_PREFIX} ostree "--repo=${repo}" pull-local "${remote_repo}" "${branch}"
if ${CMD_PREFIX} ostree "--repo=${repo}" pull-local "$@" "${remote_repo}" "${branch}"
then return 0
else return 1
fi
Expand Down Expand Up @@ -221,19 +221,23 @@ if do_pull local collection-repo badcref1
then
assert_not_reached "pulling a commit without collection ID from a repo with collection ID should fail"
fi
do_pull local collection-repo badcref1 --disable-verify-bindings
if do_pull local collection-repo badcref2
then
assert_not_reached "pulling a commit with a mismatched collection ID from a repo with collection ID should fail"
fi
do_pull local collection-repo badcref2 --disable-verify-bindings
if do_pull local collection-repo badcref3
then
assert_not_reached "pulling a commit with empty collection ID from repo with collection ID should fail"
fi
do_pull local collection-repo badcref3 --disable-verify-bindings
do_pull local collection-repo goodcref1
if do_pull local collection-repo badcref4
then
assert_not_reached "pulling a commit that was not requested from repo with collection ID should fail"
fi
do_pull local collection-repo badcref4 --disable-verify-bindings

echo "ok 5 pull refs from remote repos"

Expand All @@ -243,19 +247,23 @@ if do_local_pull local collection-local-repo badclref1
then
assert_not_reached "pulling a commit without collection ID from a repo with collection ID should fail"
fi
do_local_pull local collection-local-repo badclref1 --disable-verify-bindings
if do_local_pull local collection-local-repo badclref2
then
assert_not_reached "pulling a commit with a mismatched collection ID from a repo with collection ID should fail"
fi
do_local_pull local collection-local-repo badclref2 --disable-verify-bindings
if do_local_pull local collection-local-repo badclref3
then
assert_not_reached "pulling a commit with empty collection ID from repo with collection ID should fail"
fi
do_local_pull local collection-local-repo badclref3 --disable-verify-bindings
do_local_pull local collection-local-repo goodclref1
if do_local_pull local collection-local-repo badclref4
then
assert_not_reached "pulling a commit that was not requested from repo with collection ID should fail"
fi
do_local_pull local collection-local-repo badclref4 --disable-verify-bindings

echo "ok 6 pull refs from local repos"

Expand Down

0 comments on commit ef2d75c

Please sign in to comment.