Merge pull request #2131 from cgwalters/sign-success

signapi: Change API to also return a success message

11 files changed, 68 insertions, 19 deletions

diff --git a/src/libostree/ostree-repo-pull-private.h b/src/libostree/ostree-repo-pull-private.h
index fd17baee..689118be 100644
--- 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 ab680daf..fa170f94 100644
--- 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 fbcfc8a6..5a276e62 100644
--- 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 82575dc5..56f10d6e 100644
--- 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 c37bcdfa..bf5d63a1 100644
--- 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 ed6d0c23..d728afde 100644
--- 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 76c7e14d..72152eab 100644
--- 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 f3992480..bcb5d0a6 100644
--- 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 588ace53..0d069059 100644
--- 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 d6cc167a..c7777489 100644
--- 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 6bdbfdd6..d43efef7 100755
--- 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