signapi: Change API to also return a success message

This is the dual of https://github.com/ostreedev/ostree/pull/2129/commits/1f3c8c5b3de978f6e069c24938967f823cce7ee8
where we output more detail when signapi fails to validate.

Extend the API to return a string for success, which we output
to stdout.

This will help the test suite *and* end users validate that the expected
thing is happening.

In order to make this cleaner, split the "verified commit" set
in the pull code into GPG and signapi verified sets, and have
the signapi verified set contain the verification string.

We're not doing anything with the verification string in the
pull code *yet* but I plan to add something like
`ostree pull --verbose` which would finally print this.

diff --git a/src/libostree/ostree-repo-pull-private.h b/src/libostree/ostree-repo-pull-private.h
index fd17baeec1d48d400bb96a02cffe53b20addb51e..689118be69e164970c66cd75cd28a7d88ede8fcc 100644 (file)
--- a/src/libostree/ostree-repo-pull-private.h
+++ b/src/libostree/ostree-repo-pull-private.h
@@ -77,6 +77,7 @@ typedef struct {
   GHashTable       *summary_deltas_checksums;
   GHashTable       *ref_original_commits; /* Maps checksum to commit, used by timestamp checks */
   GHashTable       *verified_commits; /* Set<checksum> of commits that have been verified */
+  GHashTable       *signapi_verified_commits; /* Map<checksum,verification> of commits that have been signapi verified */
   GHashTable       *ref_keyring_map; /* Maps OstreeCollectionRef to keyring remote name */
   GPtrArray        *static_delta_superblocks;
   GHashTable       *expected_commit_sizes; /* Maps commit checksum to known size */
@@ -149,6 +150,7 @@ gboolean
 _sign_verify_for_remote (GPtrArray *signers,
                          GBytes *signed_data,
                          GVariant *metadata,
+                         char    **out_success_message,
                          GError **error);
 
 gboolean

diff --git a/src/libostree/ostree-repo-pull-verify.c b/src/libostree/ostree-repo-pull-verify.c
index ab680daf06135cfefdd4389d906795a5a9996eab..fa170f941ce41ffb56378dcc932d868b22e9710e 100644 (file)
--- a/src/libostree/ostree-repo-pull-verify.c
+++ b/src/libostree/ostree-repo-pull-verify.c
@@ -261,12 +261,15 @@ gboolean
 _sign_verify_for_remote (GPtrArray *verifiers,
                          GBytes *signed_data,
                          GVariant *metadata,
+                         char    **out_success_message,
                          GError **error)
 {
   guint n_invalid_signatures = 0;
   g_autoptr (GError) last_sig_error = NULL;
   gboolean found_sig = FALSE;
 
+  g_assert (out_success_message == NULL || *out_success_message == NULL);
+
   g_assert_cmpuint (verifiers->len, >=, 1);
   for (guint i = 0; i < verifiers->len; i++)
     {
@@ -282,17 +285,21 @@ _sign_verify_for_remote (GPtrArray *verifiers,
 
       found_sig = TRUE;
 
+      g_autofree char *success_message = NULL;
         /* Return true if any signature fit to pre-loaded public keys.
           * If no keys configured -- then system configuration will be used */
       if (!ostree_sign_data_verify (sign,
                                     signed_data,
                                     signatures,
+                                    &success_message,
                                     last_sig_error ? NULL : &last_sig_error))
         {
           n_invalid_signatures++;
           continue;
         }
       /* Accept the first valid signature */
+      if (out_success_message)
+        *out_success_message = g_steal_pointer (&success_message);
       return TRUE;
     }
 
@@ -348,11 +355,10 @@ _verify_unwritten_commit (OtPullData                 *pull_data,
                           GCancellable               *cancellable,
                           GError                    **error)
 {
-
-  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;
+  /* Shouldn't happen, but see comment in process_gpg_verify_result() */
+  if ((!pull_data->gpg_verify || g_hash_table_contains (pull_data->verified_commits, checksum))
+      && (!pull_data->signapi_commit_verifiers || g_hash_table_contains (pull_data->signapi_verified_commits, checksum)))
+    return TRUE;
 
   g_autoptr(GBytes) signed_data = g_variant_get_data_as_bytes (commit);
 
@@ -382,12 +388,13 @@ _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->signapi_commit_verifiers, signed_data, detached_metadata, error))
+      g_autofree char *success_message = NULL;
+      if (!_sign_verify_for_remote (pull_data->signapi_commit_verifiers, signed_data, detached_metadata, &success_message, error))
         return glnx_prefix_error (error, "Can't verify commit");
 
       /* Mark the commit as verified to avoid double verification
        * see process_verify_result () for rationale */
-      g_hash_table_add (pull_data->verified_commits, g_strdup (checksum));
+      g_hash_table_insert (pull_data->signapi_verified_commits, g_strdup (checksum), g_steal_pointer (&success_message));
     }
 
   return TRUE;

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index fbcfc8a671bdbdf26327d1c09c65cdff3451512a..5a276e62c7ba033e45f378379ad99cb323a2c727 100644 (file)
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -1541,11 +1541,12 @@ scan_commit_object (OtPullData                 *pull_data,
 #endif /* OSTREE_DISABLE_GPGME */
 
   if (pull_data->signapi_commit_verifiers &&
-      !g_hash_table_contains (pull_data->verified_commits, checksum))
+      !g_hash_table_contains (pull_data->signapi_verified_commits, checksum))
     {
       g_autoptr(GError) last_verification_error = NULL;
       gboolean found_any_signature = FALSE;
       gboolean found_valid_signature = FALSE;
+      g_autofree char *success_message = NULL;
 
       for (guint i = 0; i < pull_data->signapi_commit_verifiers->len; i++)
         {
@@ -1557,6 +1558,7 @@ scan_commit_object (OtPullData                 *pull_data,
           if (ostree_sign_commit_verify (sign,
                                           pull_data->repo,
                                           checksum,
+                                          &success_message,
                                           cancellable,
                                           last_verification_error ? NULL : &last_verification_error))
             {
@@ -1574,6 +1576,8 @@ scan_commit_object (OtPullData                 *pull_data,
           g_propagate_error (error, g_steal_pointer (&last_verification_error));
           return glnx_prefix_error (error, "Can't verify commit %s", checksum);
         }
+      g_assert (success_message);
+      g_hash_table_insert (pull_data->signapi_verified_commits, g_strdup (checksum), g_steal_pointer (&success_message));
     }
 
   /* If we found a legacy transaction flag, assume we have to scan.
@@ -3469,6 +3473,8 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
                                                            (GDestroyNotify)g_free);
   pull_data->verified_commits = g_hash_table_new_full (g_str_hash, g_str_equal,
                                                        (GDestroyNotify)g_free, NULL);
+  pull_data->signapi_verified_commits = g_hash_table_new_full (g_str_hash, g_str_equal,
+                                                               (GDestroyNotify)g_free, NULL);
   pull_data->ref_keyring_map = g_hash_table_new_full (ostree_collection_ref_hash, ostree_collection_ref_equal,
                                                       (GDestroyNotify)ostree_collection_ref_free, (GDestroyNotify)g_free);
   pull_data->scanned_metadata = g_hash_table_new_full (ostree_hash_object_name, g_variant_equal,
@@ -3962,7 +3968,7 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
 
 
             g_assert (pull_data->signapi_summary_verifiers);
-            if (!_sign_verify_for_remote (pull_data->signapi_summary_verifiers, bytes_summary, signatures, &temp_error))
+            if (!_sign_verify_for_remote (pull_data->signapi_summary_verifiers, bytes_summary, signatures, NULL, &temp_error))
               {
                 if (summary_from_cache)
                   {
@@ -3991,7 +3997,7 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
                                                                      cancellable, error))
                       goto out;
 
-                    if (!_sign_verify_for_remote (pull_data->signapi_summary_verifiers, bytes_summary, signatures, error))
+                    if (!_sign_verify_for_remote (pull_data->signapi_summary_verifiers, bytes_summary, signatures, NULL, error))
                         goto out;
                   }
                 else
@@ -4546,6 +4552,10 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
       const guint n_seconds = (guint) ((end_time - pull_data->start_time) / G_USEC_PER_SEC);
       g_autofree char *formatted_xferred = g_format_size (bytes_transferred);
       g_string_append_printf (msg, "\ntransfer: secs: %u size: %s", n_seconds, formatted_xferred);
+      if (pull_data->signapi_commit_verifiers)
+        {
+          g_assert_cmpuint (g_hash_table_size (pull_data->signapi_verified_commits), >, 0);
+        }
 
       ot_journal_send ("MESSAGE=%s", msg->str,
                        "MESSAGE_ID=" SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(OSTREE_MESSAGE_FETCH_COMPLETE_ID),
@@ -4622,6 +4632,7 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
   g_clear_pointer (&pull_data->ref_original_commits, (GDestroyNotify) g_hash_table_unref);
   g_free (pull_data->timestamp_check_from_rev);
   g_clear_pointer (&pull_data->verified_commits, (GDestroyNotify) g_hash_table_unref);
+  g_clear_pointer (&pull_data->signapi_verified_commits, (GDestroyNotify) g_hash_table_unref);
   g_clear_pointer (&pull_data->ref_keyring_map, (GDestroyNotify) g_hash_table_unref);
   g_clear_pointer (&pull_data->requested_content, (GDestroyNotify) g_hash_table_unref);
   g_clear_pointer (&pull_data->requested_fallback_content, (GDestroyNotify) g_hash_table_unref);
@@ -6114,7 +6125,7 @@ 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 (signapi_summary_verifiers, summary, sig_variant, error))
+          if (!_sign_verify_for_remote (signapi_summary_verifiers, summary, sig_variant, NULL, error))
             goto out;
         }
     }

diff --git a/src/libostree/ostree-sign-dummy.c b/src/libostree/ostree-sign-dummy.c
index 82575dc556bb3c94b619593c782cdfbfeae9015d..56f10d6e7e15885beb0ad63a132824bfbfd05723 100644 (file)
--- a/src/libostree/ostree-sign-dummy.c
+++ b/src/libostree/ostree-sign-dummy.c
@@ -154,6 +154,7 @@ const gchar * ostree_sign_dummy_metadata_format (OstreeSign *self)
 gboolean ostree_sign_dummy_data_verify (OstreeSign *self,
                                             GBytes     *data,
                                             GVariant   *signatures,
+                                            char       **out_success_message,
                                             GError     **error)
 {
   if (!check_dummy_sign_enabled (error))
@@ -182,7 +183,11 @@ gboolean ostree_sign_dummy_data_verify (OstreeSign *self,
       g_debug("Stored signature %d: %s", (gint)i, sign->pk_ascii);
 
       if (!g_strcmp0(sign_ascii, sign->pk_ascii))
-        return TRUE;
+        {
+          if (out_success_message)
+            *out_success_message = g_strdup ("dummy: Signature verified");
+          return TRUE;
+        }
       else
         return glnx_throw (error, "signature: dummy: incorrect signature %" G_GSIZE_FORMAT, i);
     }

diff --git a/src/libostree/ostree-sign-dummy.h b/src/libostree/ostree-sign-dummy.h
index c37bcdfaf96b66b9a60db8569d789cac9fc295a8..bf5d63a1db2480f885db637cfcffff7aff2b7eca 100644 (file)
--- a/src/libostree/ostree-sign-dummy.h
+++ b/src/libostree/ostree-sign-dummy.h
@@ -63,6 +63,7 @@ gboolean ostree_sign_dummy_data (OstreeSign *self,
 gboolean ostree_sign_dummy_data_verify (OstreeSign *self,
                                         GBytes     *data,
                                         GVariant   *signatures,
+                                        char       **success_message,
                                         GError     **error);
 
 const gchar * ostree_sign_dummy_metadata_key (OstreeSign *self);

diff --git a/src/libostree/ostree-sign-ed25519.c b/src/libostree/ostree-sign-ed25519.c
index 05fbe5ebfd710e50f6a046095fd0b2efb431f71b..ed2bf9777f6bb63bd79b9c2216d7917c87bb991d 100644 (file)
--- a/src/libostree/ostree-sign-ed25519.c
+++ b/src/libostree/ostree-sign-ed25519.c
@@ -169,6 +169,7 @@ _compare_ed25519_keys(gconstpointer a, gconstpointer b) {
 gboolean ostree_sign_ed25519_data_verify (OstreeSign *self,
                                           GBytes     *data,
                                           GVariant   *signatures,
+                                          char      **out_success_message,
                                           GError     **error)
 {
   g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE);
@@ -243,8 +244,12 @@ gboolean ostree_sign_ed25519_data_verify (OstreeSign *self,
             }
           else
             {
-              g_debug ("Signature verified successfully with key '%s'",
-                       sodium_bin2hex (hex, crypto_sign_PUBLICKEYBYTES*2+1, public_key->data, crypto_sign_PUBLICKEYBYTES));
+              if (out_success_message)
+                {
+                  *out_success_message =
+                    g_strdup_printf ("ed25519: Signature verified successfully with key '%s'",
+                                     sodium_bin2hex (hex, crypto_sign_PUBLICKEYBYTES*2+1, public_key->data, crypto_sign_PUBLICKEYBYTES));
+                }
               return TRUE;
             }
         }

diff --git a/src/libostree/ostree-sign-ed25519.h b/src/libostree/ostree-sign-ed25519.h
index 76c7e14d426c0e7e0b835ea60f3a31ca1df19957..72152eab07b7332c443230855b980242aae3d21c 100644 (file)
--- a/src/libostree/ostree-sign-ed25519.h
+++ b/src/libostree/ostree-sign-ed25519.h
@@ -61,6 +61,7 @@ gboolean ostree_sign_ed25519_data (OstreeSign *self,
 gboolean ostree_sign_ed25519_data_verify (OstreeSign *self,
                                           GBytes     *data,
                                           GVariant   *signatures,
+                                          char      **out_success_message,
                                           GError     **error);
 
 const gchar * ostree_sign_ed25519_get_name (OstreeSign *self);

diff --git a/src/libostree/ostree-sign.c b/src/libostree/ostree-sign.c
index f399248099028d14a742b43bb111b5142ee17b16..bcb5d0a6da047091b40477e10810afc605a1f29b 100644 (file)
--- a/src/libostree/ostree-sign.c
+++ b/src/libostree/ostree-sign.c
@@ -322,13 +322,14 @@ gboolean
 ostree_sign_data_verify (OstreeSign *self,
                          GBytes     *data,
                          GVariant   *signatures,
+                         char      **out_success_message,
                          GError     **error)
 {
   g_return_val_if_fail (OSTREE_IS_SIGN (self), FALSE);
   if (OSTREE_SIGN_GET_IFACE (self)->data_verify == NULL)
     return glnx_throw (error, "not implemented");
 
-  return OSTREE_SIGN_GET_IFACE (self)->data_verify(self, data, signatures, error);
+  return OSTREE_SIGN_GET_IFACE (self)->data_verify(self, data, signatures, out_success_message, error);
 }
 
 /*
@@ -389,6 +390,7 @@ gboolean
 ostree_sign_commit_verify (OstreeSign     *self,
                            OstreeRepo     *repo,
                            const gchar    *commit_checksum,
+                           char          **out_success_message,
                            GCancellable   *cancellable,
                            GError         **error)
 
@@ -427,6 +429,7 @@ ostree_sign_commit_verify (OstreeSign     *self,
   return ostree_sign_data_verify (self,
                                   signed_data,
                                   signatures,
+                                  out_success_message,
                                   error);
 }
 

diff --git a/src/libostree/ostree-sign.h b/src/libostree/ostree-sign.h
index 588ace535d025c38f666dc5b0ed6a1f28314e4e5..0d0690599014eaddb39177e6df6d016062c79e35 100644 (file)
--- a/src/libostree/ostree-sign.h
+++ b/src/libostree/ostree-sign.h
@@ -72,6 +72,7 @@ struct _OstreeSignInterface
   gboolean (* data_verify) (OstreeSign *self,
                             GBytes *data,
                             GVariant   *signatures,
+                            char      **out_success_message,
                             GError **error);
   const gchar *(* metadata_key) (OstreeSign *self);
   const gchar *(* metadata_format) (OstreeSign *self);
@@ -105,6 +106,7 @@ _OSTREE_PUBLIC
 gboolean ostree_sign_data_verify (OstreeSign *self,
                                   GBytes     *data,
                                   GVariant   *signatures,
+                                  char      **out_success_message,
                                   GError     **error);
 
 _OSTREE_PUBLIC
@@ -124,6 +126,7 @@ _OSTREE_PUBLIC
 gboolean ostree_sign_commit_verify (OstreeSign *self,
                                     OstreeRepo     *repo,
                                     const gchar    *commit_checksum,
+                                    char          **out_success_message,
                                     GCancellable   *cancellable,
                                     GError         **error);
 

diff --git a/src/ostree/ot-builtin-sign.c b/src/ostree/ot-builtin-sign.c
index d6cc167ada46e7041cbd73851086775d369ea68b..c77774891534d651c4e46f0d1772c6190e892e4f 100644 (file)
--- a/src/ostree/ot-builtin-sign.c
+++ b/src/ostree/ot-builtin-sign.c
@@ -70,6 +70,7 @@ ostree_builtin_sign (int argc, char **argv, OstreeCommandInvocation *invocation,
   g_autoptr (OstreeRepo) repo = NULL;
   g_autoptr (OstreeSign) sign = NULL;
   g_autofree char *resolved_commit = NULL;
+  g_autofree char *success_message = NULL;
   const char *commit;
   char **key_ids;
   int n_key_ids, ii;
@@ -130,9 +131,12 @@ ostree_builtin_sign (int argc, char **argv, OstreeCommandInvocation *invocation,
           if (ostree_sign_commit_verify (sign,
                                          repo,
                                          resolved_commit,
+                                         &success_message,
                                          cancellable,
                                          &local_error))
             {
+              g_assert (success_message);
+              g_print ("%s\n", success_message);
               ret = TRUE;
               goto out;
             }
@@ -180,9 +184,13 @@ ostree_builtin_sign (int argc, char **argv, OstreeCommandInvocation *invocation,
           if (ostree_sign_commit_verify (sign,
                                          repo,
                                          resolved_commit,
+                                         &success_message,
                                          cancellable,
                                          error))
-            ret = TRUE;
+            {
+              g_print ("%s\n", success_message);
+              ret = TRUE;
+            }
         } /* Check via file */
     }
   else

diff --git a/tests/test-signed-commit.sh b/tests/test-signed-commit.sh
index 6bdbfdd60927efc9f74ce12566d074a4463440d4..d43efef7cc3dde17c8038a5024b6024a03ce3084 100755 (executable)
--- a/tests/test-signed-commit.sh
+++ b/tests/test-signed-commit.sh
@@ -121,8 +121,10 @@ done
 ${CMD_PREFIX} ostree --repo=${test_tmpdir}/repo sign --sign-type=dummy ${COMMIT} ${DUMMYSIGN}
 ${CMD_PREFIX} ostree --repo=${test_tmpdir}/repo sign --sign-type=ed25519 ${COMMIT} ${SECRET}
 # and verify
-${CMD_PREFIX} ostree --repo=${test_tmpdir}/repo sign --verify --sign-type=ed25519 ${COMMIT} ${PUBLIC}
-${CMD_PREFIX} ostree --repo=${test_tmpdir}/repo sign --sign-type=dummy --verify ${COMMIT} ${DUMMYSIGN}
+${CMD_PREFIX} ostree --repo=${test_tmpdir}/repo sign --verify --sign-type=ed25519 ${COMMIT} ${PUBLIC} >out.txt
+assert_file_has_content out.txt "ed25519: Signature verified successfully with key"
+${CMD_PREFIX} ostree --repo=${test_tmpdir}/repo sign --sign-type=dummy --verify ${COMMIT} ${DUMMYSIGN} >out.txt
+assert_file_has_content out.txt "dummy: Signature verified"
 echo "ok multiple signing "
 
 # Prepare files with public ed25519 signatures
@@ -140,7 +142,8 @@ fi
 
 # Test with single key in list
 echo ${PUBLIC} > ${PUBKEYS}
-${CMD_PREFIX} ostree --repo=${test_tmpdir}/repo sign --verify --sign-type=ed25519 --keys-file=${PUBKEYS} ${COMMIT}
+${CMD_PREFIX} ostree --repo=${test_tmpdir}/repo sign --verify --sign-type=ed25519 --keys-file=${PUBKEYS} ${COMMIT} >out.txt
+assert_file_has_content out.txt 'ed25519: Signature verified successfully'
 
 # Test the file with multiple keys without a valid public key
 for((i=0;i<100;i++)); do