From 36258036ae1388c728eec0237b283e225f64a7b1 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 17 Jun 2020 00:22:49 +0000 Subject: [PATCH] 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. --- src/libostree/ostree-repo-pull-private.h | 2 ++ src/libostree/ostree-repo-pull-verify.c | 21 ++++++++++++++------- src/libostree/ostree-repo-pull.c | 19 +++++++++++++++---- src/libostree/ostree-sign-dummy.c | 7 ++++++- src/libostree/ostree-sign-dummy.h | 1 + src/libostree/ostree-sign-ed25519.c | 9 +++++++-- src/libostree/ostree-sign-ed25519.h | 1 + src/libostree/ostree-sign.c | 5 ++++- src/libostree/ostree-sign.h | 3 +++ src/ostree/ot-builtin-sign.c | 10 +++++++++- tests/test-signed-commit.sh | 9 ++++++--- 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 of commits that have been verified */ + GHashTable *signapi_verified_commits; /* Map 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 05fbe5eb..ed2bf977 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