From 9509a4bc948672b6bdf3ccdd0ea882a141c78974 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 12 May 2020 01:26:00 +0000 Subject: [PATCH] pull: Further cleanup signapi verification Previously in the pull code, every time we went to verify a commit we would re-initialize an `OstreeSign` instance of each time, re-parse the remote configuration and re-load its public keys etc. In most cases this doesn't matter really because we're pulling one commit, but if e.g. pulling a commit with history would get a bit silly. This changes things so that the pull code initializes the verifiers once, and reuses them thereafter. This is continuing towards changing the code to support explicitly configured verifiers, xref https://github.com/ostreedev/ostree/issues/2080 --- src/libostree/ostree-repo-pull-private.h | 20 ++++++------ src/libostree/ostree-repo-pull-verify.c | 41 ++++++++++++++++-------- src/libostree/ostree-repo-pull.c | 32 +++++++++++------- 3 files changed, 58 insertions(+), 35 deletions(-) diff --git a/src/libostree/ostree-repo-pull-private.h b/src/libostree/ostree-repo-pull-private.h index b9eb2342..5bc2a33a 100644 --- a/src/libostree/ostree-repo-pull-private.h +++ b/src/libostree/ostree-repo-pull-private.h @@ -123,6 +123,8 @@ typedef struct { gboolean is_commit_only; OstreeRepoImportFlags importflags; + GPtrArray *signapi_verifiers; + GPtrArray *dirs; gboolean have_previous_bytes; @@ -137,18 +139,16 @@ typedef struct { GSource *idle_src; } OtPullData; -gboolean -_sign_verify_for_remote (OstreeRepo *repo, - const gchar *remote_name, - GBytes *signed_data, - GVariant *metadata, - GError **error); +GPtrArray * +_signapi_verifiers_for_remote (OstreeRepo *repo, + const char *remote_name, + GError **error); gboolean -_signapi_load_public_keys (OstreeSign *sign, - OstreeRepo *repo, - const gchar *remote_name, - GError **error); +_sign_verify_for_remote (GPtrArray *signers, + GBytes *signed_data, + GVariant *metadata, + GError **error); gboolean _verify_unwritten_commit (OtPullData *pull_data, diff --git a/src/libostree/ostree-repo-pull-verify.c b/src/libostree/ostree-repo-pull-verify.c index 84f7623b..36d877ac 100644 --- a/src/libostree/ostree-repo-pull-verify.c +++ b/src/libostree/ostree-repo-pull-verify.c @@ -67,7 +67,7 @@ get_signapi_remote_option (OstreeRepo *repo, * Returns: %FALSE if any source is configured but nothing has been loaded. * Returns: %TRUE if no configuration or any key loaded. * */ -gboolean +static gboolean _signapi_load_public_keys (OstreeSign *sign, OstreeRepo *repo, const gchar *remote_name, @@ -142,21 +142,40 @@ _signapi_load_public_keys (OstreeSign *sign, return TRUE; } -/* Iterate over all known signing types, and check if the commit is signed +/* Create a new array of OstreeSign objects and load the public + * keys as described by the remote configuration. + */ +GPtrArray * +_signapi_verifiers_for_remote (OstreeRepo *repo, + const char *remote_name, + GError **error) +{ + g_autoptr(GPtrArray) signers = ostree_sign_get_all (); + g_assert_cmpuint (signers->len, >=, 1); + for (guint i = 0; i < signers->len; i++) + { + OstreeSign *sign = signers->pdata[i]; + /* Try to load public key(s) according remote's configuration */ + if (!_signapi_load_public_keys (sign, repo, remote_name, error)) + return FALSE; + } + return g_steal_pointer (&signers); +} + +/* Iterate over the configured signers, and require the commit is signed * by at least one. */ gboolean -_sign_verify_for_remote (OstreeRepo *repo, - const gchar *remote_name, - GBytes *signed_data, - GVariant *metadata, - GError **error) +_sign_verify_for_remote (GPtrArray *signers, + GBytes *signed_data, + GVariant *metadata, + GError **error) { guint n_invalid_signatures = 0; g_autoptr (GError) last_sig_error = NULL; gboolean found_sig = FALSE; - g_autoptr(GPtrArray) signers = ostree_sign_get_all (); + g_assert_cmpuint (signers->len, >=, 1); for (guint i = 0; i < signers->len; i++) { OstreeSign *sign = signers->pdata[i]; @@ -169,10 +188,6 @@ _sign_verify_for_remote (OstreeRepo *repo, if (!signatures) continue; - /* Try to load public key(s) according remote's configuration */ - if (!_signapi_load_public_keys (sign, repo, remote_name, error)) - return FALSE; - found_sig = TRUE; /* Return true if any signature fit to pre-loaded public keys. @@ -275,7 +290,7 @@ _verify_unwritten_commit (OtPullData *pull_data, if (detached_metadata == NULL) return glnx_throw (error, "Can't verify commit without detached metadata"); - if (!_sign_verify_for_remote (pull_data->repo, pull_data->remote_name, signed_data, detached_metadata, error)) + if (!_sign_verify_for_remote (pull_data->signapi_verifiers, signed_data, detached_metadata, error)) return glnx_prefix_error (error, "Can't verify commit"); /* Mark the commit as verified to avoid double verification diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 4d617c96..363db9e2 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1544,15 +1544,10 @@ scan_commit_object (OtPullData *pull_data, gboolean found_any_signature = FALSE; gboolean found_valid_signature = FALSE; - /* FIXME - dedup this with _sign_verify_for_remote() */ - g_autoptr(GPtrArray) signers = ostree_sign_get_all (); - for (guint i = 0; i < signers->len; i++) + g_assert (pull_data->signapi_verifiers); + for (guint i = 0; i < pull_data->signapi_verifiers->len; i++) { - OstreeSign *sign = signers->pdata[i]; - - /* Try to load public key(s) according remote's configuration */ - if (!_signapi_load_public_keys (sign, pull_data->repo, pull_data->remote_name, error)) - return FALSE; + OstreeSign *sign = pull_data->signapi_verifiers->pdata[i]; found_any_signature = TRUE; @@ -3574,6 +3569,15 @@ ostree_repo_pull_with_options (OstreeRepo *self, } } + if (pull_data->sign_verify || pull_data->sign_verify_summary) + { + g_assert (pull_data->remote_name != NULL); + pull_data->signapi_verifiers = _signapi_verifiers_for_remote (pull_data->repo, pull_data->remote_name, error); + if (!pull_data->signapi_verifiers) + goto out; + g_assert_cmpint (pull_data->signapi_verifiers->len, >=, 1); + } + pull_data->phase = OSTREE_PULL_PHASE_FETCHING_REFS; if (!reinitialize_fetcher (pull_data, remote_name_or_baseurl, error)) @@ -3954,7 +3958,8 @@ ostree_repo_pull_with_options (OstreeRepo *self, bytes_sig, FALSE); - if (!_sign_verify_for_remote (pull_data->repo, pull_data->remote_name, bytes_summary, signatures, &temp_error)) + g_assert (pull_data->signapi_verifiers); + if (!_sign_verify_for_remote (pull_data->signapi_verifiers, bytes_summary, signatures, &temp_error)) { if (summary_from_cache) { @@ -3983,7 +3988,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, cancellable, error)) goto out; - if (!_sign_verify_for_remote (pull_data->repo, pull_data->remote_name, bytes_summary, signatures, error)) + if (!_sign_verify_for_remote (pull_data->signapi_verifiers, bytes_summary, signatures, error)) goto out; } else @@ -4586,6 +4591,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_free (pull_data->remote_refspec_name); g_free (pull_data->remote_name); g_free (pull_data->append_user_agent); + g_clear_pointer (&pull_data->signapi_verifiers, (GDestroyNotify) g_ptr_array_unref); g_clear_pointer (&pull_data->meta_mirrorlist, (GDestroyNotify) g_ptr_array_unref); g_clear_pointer (&pull_data->content_mirrorlist, (GDestroyNotify) g_ptr_array_unref); g_clear_pointer (&pull_data->summary_data, (GDestroyNotify) g_bytes_unref); @@ -6089,8 +6095,10 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo *self, sig_variant = g_variant_new_from_bytes (OSTREE_SUMMARY_SIG_GVARIANT_FORMAT, signatures, FALSE); - - if (!_sign_verify_for_remote (self, name, summary, sig_variant, error)) + g_autoptr(GPtrArray) signapi_verifiers = _signapi_verifiers_for_remote (self, name, error); + if (!signapi_verifiers) + goto out; + if (!_sign_verify_for_remote (signapi_verifiers, summary, sig_variant, error)) goto out; } }