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

diff --git a/src/libostree/ostree-repo-pull-private.h b/src/libostree/ostree-repo-pull-private.h
index b9eb234201b115cb02c8db3565d94dcaeba0b6a1..5bc2a33a55f5f30d25bb87c6dd18e38f76b0be26 100644 (file)
--- 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 84f7623b7adb5f497fa503f129eb4fbd05244c94..36d877ac599401edd5c8882847dd0dc68add9b50 100644 (file)
--- 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 4d617c96638ddeb155796271e3535b3b9f14eae1..363db9e2285338e68c0f51d6c6baa7d1f015884c 100644 (file)
--- 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;
         }
     }