pull: Add support for sign-verify=<list>

The goal here is to move the code towards a model
where the *client* can explicitly specify which signature types
are acceptable.

We retain support for `sign-verify=true` for backwards compatibility.
But in that configuration, a missing public key is just "no signatures found".

With `sign-verify=ed25519` and no key configured, we can
explicitly say `No keys found for required signapi type ed25519`
which is much, much clearer.

Implementation side, rather than maintaining `gboolean sign_verify` *and*
`GPtrArray sign_verifiers`, just have the array.  If it's `NULL` that means
not to verify.

Note that currently, an explicit list is an OR of signatures, not AND.
In practice...I think most people are going to be using a single entry
anyways.

src/libostree/ostree-repo-pull-private.h

@@ -67,8 +67,6 @@ typedef struct {
 
   gboolean          gpg_verify;
   gboolean          gpg_verify_summary;
-  gboolean          sign_verify;
-  gboolean          sign_verify_summary;
   gboolean          require_static_deltas;
   gboolean          disable_static_deltas;
   gboolean          has_tombstone_commits;
@@ -124,7 +122,8 @@ typedef struct {
   gboolean          is_commit_only;
   OstreeRepoImportFlags importflags;
 
-  GPtrArray        *signapi_verifiers;
+  GPtrArray        *signapi_commit_verifiers;
+  GPtrArray        *signapi_summary_verifiers;
 
   GPtrArray        *dirs;
 
@@ -140,11 +139,12 @@ typedef struct {
   GSource *idle_src;
 } OtPullData;
 
-GPtrArray *
+gboolean
-_signapi_verifiers_for_remote (OstreeRepo *repo,
+_signapi_init_for_remote (OstreeRepo *repo,
-                               const char *remote_name,
+                          const char *remote_name,
-                               GError    **error);
+                          GPtrArray **out_commit_verifiers,
-
+                          GPtrArray **out_summary_verifiers,
+                          GError    **error);
 gboolean
 _sign_verify_for_remote (GPtrArray *signers,
                          GBytes *signed_data,

src/libostree/ostree-repo-pull-verify.c

@@ -71,6 +71,7 @@ static gboolean
 _signapi_load_public_keys (OstreeSign *sign,
                            OstreeRepo *repo,
                            const gchar *remote_name,
+                           gboolean required,
                            GError **error)
 {
   g_autofree gchar *pk_ascii = NULL;
@@ -95,6 +96,8 @@ _signapi_load_public_keys (OstreeSign *sign,
        * and call it here for loading with method and file structure
        * specific for signature type.
        */
+      if (required)
+        return glnx_throw (error, "No keys found for required signapi type %s", ostree_sign_get_name (sign));
       return TRUE;
     }
 
@@ -142,31 +145,120 @@ _signapi_load_public_keys (OstreeSign *sign,
   return TRUE;
 }
 
-/* Create a new array of OstreeSign objects and load the public
+static gboolean
- * keys as described by the remote configuration.
+string_is_gkeyfile_truthy (const char *value,
- */
+                           gboolean   *out_truth)
-GPtrArray *
-_signapi_verifiers_for_remote (OstreeRepo *repo,
-                               const char *remote_name,
-                               GError    **error)
 {
-  g_autoptr(GPtrArray) signers = ostree_sign_get_all ();
+  /* See https://gitlab.gnome.org/GNOME/glib/-/blob/20fb5bf868added5aec53c013ae85ec78ba2eedc/glib/gkeyfile.c#L4528 */
-  g_assert_cmpuint (signers->len, >=, 1);
+  if (g_str_equal (value, "true") || g_str_equal (value, "1"))
-  for (guint i = 0; i < signers->len; i++)
     {
-      OstreeSign *sign = signers->pdata[i];
+      *out_truth = TRUE;
-      /* Try to load public key(s) according remote's configuration */
+      return TRUE;
-      if (!_signapi_load_public_keys (sign, repo, remote_name, error))
-        return FALSE;
     }
-  return g_steal_pointer (&signers);
+  else if (g_str_equal (value, "false") || g_str_equal (value, "0"))
+    {
+      *out_truth = FALSE;
+      return TRUE;
+    }
+  return FALSE;
 }
 
-/* Iterate over the configured signers, and require the commit is signed
+static gboolean
+verifiers_from_config (OstreeRepo *repo,
+                       const char *remote_name,
+                       const char *key,
+                       GPtrArray **out_verifiers,
+                       GError    **error)
+{
+  g_autoptr(GPtrArray) verifiers = NULL;
+
+  g_autofree char *raw_value = NULL;
+  if (!ostree_repo_get_remote_option (repo, remote_name,
+                                      key, NULL,
+                                      &raw_value, error))
+    return FALSE;
+  if (raw_value == NULL || g_str_equal (raw_value, ""))
+    {
+      *out_verifiers = NULL;
+      return TRUE;
+    }
+  gboolean sign_verify_bool = FALSE;
+  /* Is the value "truthy" according to GKeyFile's rules?  If so,
+   * then we take this to be "accept signatures from any compiled
+   * type that happens to have keys configured".
+   */
+  if (string_is_gkeyfile_truthy (raw_value, &sign_verify_bool))
+    {
+      if (sign_verify_bool)
+        {
+          verifiers = ostree_sign_get_all ();
+          for (guint i = 0; i < verifiers->len; i++)
+            {
+              OstreeSign *sign = verifiers->pdata[i];
+              /* Try to load public key(s) according remote's configuration;
+               * this one is optional.
+               */
+              if (!_signapi_load_public_keys (sign, repo, remote_name, FALSE, error))
+                return FALSE;
+            }
+        }
+    }
+  else
+    {
+      /* If the value isn't "truthy", then it must be an explicit list */
+      g_auto(GStrv) sign_types = NULL;
+      if (!ostree_repo_get_remote_list_option (repo, remote_name,
+                                               key, &sign_types,
+                                               error))
+        return FALSE;
+      verifiers = g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref);
+      for (char **iter = sign_types; iter && *iter; iter++)
+        {
+          const char *sign_type = *iter;
+          OstreeSign *verifier = ostree_sign_get_by_name (sign_type, error);
+          if (!verifier)
+            return FALSE;
+          if (!_signapi_load_public_keys (verifier, repo, remote_name, TRUE, error))
+            return FALSE;
+          g_ptr_array_add (verifiers, verifier);
+        }
+      g_assert_cmpuint (verifiers->len, >=, 1);
+    }
+
+  *out_verifiers = g_steal_pointer (&verifiers);
+  return TRUE;
+}
+
+/* Create a new array of OstreeSign objects and load the public
+ * keys as described by the remote configuration.  If the
+ * remote does not have signing verification enabled, then
+ * the resulting verifier list will be NULL.
+ */
+gboolean
+_signapi_init_for_remote (OstreeRepo *repo,
+                          const char *remote_name,
+                          GPtrArray **out_commit_verifiers,
+                          GPtrArray **out_summary_verifiers,
+                          GError    **error)
+{
+  g_autoptr(GPtrArray) commit_verifiers = NULL;
+  g_autoptr(GPtrArray) summary_verifiers = NULL;
+
+  if (!verifiers_from_config (repo, remote_name, "sign-verify", &commit_verifiers, error))
+    return FALSE;
+  if (!verifiers_from_config (repo, remote_name, "sign-verify-summary", &summary_verifiers, error))
+    return FALSE;
+
+  ot_transfer_out_value (out_commit_verifiers, &commit_verifiers);
+  ot_transfer_out_value (out_summary_verifiers, &summary_verifiers);
+  return TRUE;
+}
+
+/* Iterate over the configured verifiers, and require the commit is signed
  * by at least one.
  */
 gboolean
-_sign_verify_for_remote (GPtrArray *signers,
+_sign_verify_for_remote (GPtrArray *verifiers,
                          GBytes *signed_data,
                          GVariant *metadata,
                          GError **error)
@@ -175,10 +267,10 @@ _sign_verify_for_remote (GPtrArray *signers,
   g_autoptr (GError) last_sig_error = NULL;
   gboolean found_sig = FALSE;
 
-  g_assert_cmpuint (signers->len, >=, 1);
+  g_assert_cmpuint (verifiers->len, >=, 1);
-  for (guint i = 0; i < signers->len; i++)
+  for (guint i = 0; i < verifiers->len; i++)
     {
-      OstreeSign *sign = signers->pdata[i];
+      OstreeSign *sign = verifiers->pdata[i];
       const gchar *signature_key = ostree_sign_metadata_key (sign);
       GVariantType *signature_format = (GVariantType *) ostree_sign_metadata_format (sign);
       g_autoptr (GVariant) signatures =
@@ -257,7 +349,7 @@ _verify_unwritten_commit (OtPullData                 *pull_data,
                           GError                    **error)
 {
 
-  if (pull_data->gpg_verify || pull_data->sign_verify)
+  if (pull_data->gpg_verify || pull_data->signapi_commit_verifiers)
     /* Shouldn't happen, but see comment in process_gpg_verify_result() */
     if (g_hash_table_contains (pull_data->verified_commits, checksum))
       return TRUE;
@@ -284,13 +376,13 @@ _verify_unwritten_commit (OtPullData                 *pull_data,
     }
 #endif /* OSTREE_DISABLE_GPGME */
 
-  if (pull_data->sign_verify)
+  if (pull_data->signapi_commit_verifiers)
     {
       /* Nothing to check if detached metadata is absent */
       if (detached_metadata == NULL)
         return glnx_throw (error, "Can't verify commit without detached metadata");
 
-      if (!_sign_verify_for_remote (pull_data->signapi_verifiers, signed_data, detached_metadata, error))
+      if (!_sign_verify_for_remote (pull_data->signapi_commit_verifiers, signed_data, detached_metadata, error))
         return glnx_prefix_error (error, "Can't verify commit");
 
       /* Mark the commit as verified to avoid double verification

src/libostree/ostree-repo-pull.c

@@ -1537,17 +1537,16 @@ scan_commit_object (OtPullData                 *pull_data,
     }
 #endif /* OSTREE_DISABLE_GPGME */
 
-  if (pull_data->sign_verify &&
+  if (pull_data->signapi_commit_verifiers &&
       !g_hash_table_contains (pull_data->verified_commits, checksum))
     {
       g_autoptr(GError) last_verification_error = NULL;
       gboolean found_any_signature = FALSE;
       gboolean found_valid_signature = FALSE;
 
-      g_assert (pull_data->signapi_verifiers);
+      for (guint i = 0; i < pull_data->signapi_commit_verifiers->len; i++)
-      for (guint i = 0; i < pull_data->signapi_verifiers->len; i++)
         {
-          OstreeSign *sign = pull_data->signapi_verifiers->pdata[i];
+          OstreeSign *sign = pull_data->signapi_commit_verifiers->pdata[i];
 
           found_any_signature = TRUE;
 
@@ -3552,31 +3551,6 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
         if (!ostree_repo_remote_get_gpg_verify_summary (self, pull_data->remote_name,
                                                         &pull_data->gpg_verify_summary, error))
           goto out;
-      /* signapi differs from GPG in that it can only be explicitly *disabled*
-       * transiently during pulls, not enabled.
-       */
-      if (disable_sign_verify)
-        {
-          pull_data->sign_verify = FALSE;
-        }
-      else
-        {
-          if (!ostree_repo_get_remote_boolean_option (self, pull_data->remote_name,
-                                                      "sign-verify", FALSE,
-                                                      &pull_data->sign_verify, error))
-            goto out;
-        }
-      if (disable_sign_verify_summary)
-        {
-          pull_data->sign_verify_summary = FALSE;
-        }
-      else
-        {
-          if (!ostree_repo_get_remote_boolean_option (self, pull_data->remote_name,
-                                                      "sign-verify-summary", FALSE,
-                                                      &pull_data->sign_verify_summary, error))
-            goto out;
-        }
 
       /* NOTE: If changing this, see the matching implementation in
        * ostree-sysroot-upgrader.c
@@ -3595,13 +3569,13 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
         }
     }
 
-  if (pull_data->sign_verify || pull_data->sign_verify_summary)
+  if (pull_data->remote_name && !(disable_sign_verify && disable_sign_verify_summary))
     {
-      g_assert (pull_data->remote_name != NULL);
+      if (!_signapi_init_for_remote (pull_data->repo, pull_data->remote_name,
-      pull_data->signapi_verifiers = _signapi_verifiers_for_remote (pull_data->repo, pull_data->remote_name, error);
+                                     &pull_data->signapi_commit_verifiers,
-      if (!pull_data->signapi_verifiers)
+                                     &pull_data->signapi_summary_verifiers,
-        goto out;
+                                     error))
-      g_assert_cmpint (pull_data->signapi_verifiers->len, >=, 1);
+        return FALSE;
     }
 
   pull_data->phase = OSTREE_PULL_PHASE_FETCHING_REFS;
@@ -3967,9 +3941,9 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
       }
 #endif /* OSTREE_DISABLE_GPGME */
 
-    if (pull_data->sign_verify_summary)
+    if (pull_data->signapi_summary_verifiers)
       {
-        if (!bytes_sig && pull_data->sign_verify_summary)
+        if (!bytes_sig && pull_data->signapi_summary_verifiers)
           {
             g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
                          "Signatures verification enabled, but no summary.sig found (use sign-verify-summary=false in remote config to disable)");
@@ -3984,8 +3958,8 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
                                                    bytes_sig, FALSE);
 
 
-            g_assert (pull_data->signapi_verifiers);
+            g_assert (pull_data->signapi_summary_verifiers);
-            if (!_sign_verify_for_remote (pull_data->signapi_verifiers, bytes_summary, signatures, &temp_error))
+            if (!_sign_verify_for_remote (pull_data->signapi_summary_verifiers, bytes_summary, signatures, &temp_error))
               {
                 if (summary_from_cache)
                   {
@@ -4014,7 +3988,7 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
                                                                      cancellable, error))
                       goto out;
 
-                    if (!_sign_verify_for_remote (pull_data->signapi_verifiers, bytes_summary, signatures, error))
+                    if (!_sign_verify_for_remote (pull_data->signapi_summary_verifiers, bytes_summary, signatures, error))
                         goto out;
                   }
                 else
@@ -4536,7 +4510,7 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
       g_string_append_printf (msg, "\nsecurity: GPG: %s ", gpg_verify_state);
 
       const char *sign_verify_state;
-      sign_verify_state = (pull_data->sign_verify ? "commit" : "disabled");
+      sign_verify_state = (pull_data->signapi_commit_verifiers ? "commit" : "disabled");
       g_string_append_printf (msg, "\nsecurity: SIGN: %s ", sign_verify_state);
 
       OstreeFetcherURI *first_uri = pull_data->meta_mirrorlist->pdata[0];
@@ -4629,7 +4603,8 @@ 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->signapi_commit_verifiers, (GDestroyNotify) g_ptr_array_unref);
+  g_clear_pointer (&pull_data->signapi_summary_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);
@@ -6050,7 +6025,7 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo    *self,
   g_autoptr(GBytes) summary = NULL;
   g_autoptr(GBytes) signatures = NULL;
   gboolean gpg_verify_summary;
-  gboolean sign_verify_summary;
+  g_autoptr(GPtrArray) signapi_summary_verifiers = NULL;
   gboolean ret = FALSE;
   gboolean summary_is_from_cache;
 
@@ -6107,11 +6082,12 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo    *self,
         }
     }
 
-  if (!ostree_repo_get_remote_boolean_option (self, name, "sign-verify-summary",
+  if (!_signapi_init_for_remote (self, name, NULL,
-                                              FALSE, &sign_verify_summary, error))
+                                 &signapi_summary_verifiers,
-      goto out;
+                                 error))
+    goto out;
 
-  if (sign_verify_summary)
+  if (signapi_summary_verifiers)
     {
       if (summary == NULL)
         {
@@ -6134,10 +6110,8 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo    *self,
 
           sig_variant = g_variant_new_from_bytes (OSTREE_SUMMARY_SIG_GVARIANT_FORMAT,
                                                   signatures, FALSE);
-          g_autoptr(GPtrArray) signapi_verifiers = _signapi_verifiers_for_remote (self, name, error);
+
-          if (!signapi_verifiers)
+          if (!_sign_verify_for_remote (signapi_summary_verifiers, summary, sig_variant, error))
-            goto out;
-          if (!_sign_verify_for_remote (signapi_verifiers, summary, sig_variant, error))
             goto out;
         }
     }

tests/test-signed-pull.sh

@@ -23,7 +23,7 @@ set -euo pipefail
 
 . $(dirname $0)/libtest.sh
 
-echo "1..16"
+echo "1..20"
 
 # This is explicitly opt in for testing
 export OSTREE_DUMMY_SIGN_ENABLED=1
@@ -102,6 +102,31 @@ test_signed_pull "dummy" ""
 repo_init --sign-verify=dummy=inline:${DUMMYSIGN}
 test_signed_pull "dummy" "from remote opt"
 
+# And now explicitly limit it to dummy
+repo_init
+${CMD_PREFIX} ostree --repo=repo config set 'remote "origin"'.sign-verify dummy
+${CMD_PREFIX} ostree --repo=repo config set 'remote "origin"'.verification-dummy-key "${DUMMYSIGN}"
+test_signed_pull "dummy" "explicit value"
+
+# dummy, but no key configured
+repo_init
+${CMD_PREFIX} ostree --repo=repo config set 'remote "origin"'.sign-verify dummy
+if ${CMD_PREFIX} ostree --repo=repo pull origin main 2>err.txt; then
+    assert_not_reached "pull with nosuchsystem succeeded"
+fi
+assert_file_has_content err.txt 'No keys found for required signapi type dummy'
+echo "ok explicit dummy but unconfigured"
+
+# Set it to an unknown explicit value
+repo_init
+${CMD_PREFIX} ostree --repo=repo config set 'remote "origin"'.sign-verify nosuchsystem;
+${CMD_PREFIX} ostree --repo=repo config set 'remote "origin"'.verification-dummy-key "${DUMMYSIGN}"
+if ${CMD_PREFIX} ostree --repo=repo pull origin main 2>err.txt; then
+    assert_not_reached "pull with nosuchsystem succeeded"
+fi
+assert_file_has_content err.txt 'Requested signature type is not implemented'
+echo "ok pull failure for unknown system"
+
 repo_init
 if ${CMD_PREFIX} ostree --repo=repo remote add other --sign-verify=trustme=inline:ok http://localhost 2>err.txt; then
     assert_not_reached "remote add with invalid keytype succeeded"