aboutsummaryrefslogtreecommitdiffstats
path: root/log-tree.c
diff options
context:
space:
mode:
authorHans Jerry Illikainen <hji@dyntopia.com>2019-11-27 17:48:21 +0000
committerJunio C Hamano <gitster@pobox.com>2019-11-30 13:52:35 -0800
commit72b006f4bfd30b7c5037c163efaf279ab65bea9c (patch)
tree50fdf5a566469f65c1b496efaaee77961775322e /log-tree.c
parent67a6ea63008bcee32a239934ad29eb5c5a554509 (diff)
downloadgit-72b006f4bfd30b7c5037c163efaf279ab65bea9c.tar.gz
gpg-interface: prefer check_signature() for GPG verification
This commit refactors the use of verify_signed_buffer() outside of gpg-interface.c to use check_signature() instead. It also turns verify_signed_buffer() into a file-local function since it's now only invoked internally by check_signature(). There were previously two globally scoped functions used in different parts of Git to perform GPG signature verification: verify_signed_buffer() and check_signature(). Now only check_signature() is used. The verify_signed_buffer() function doesn't guard against duplicate signatures as described by Michał Górny [1]. Instead it only ensures a non-erroneous exit code from GPG and the presence of at least one GOODSIG status field. This stands in contrast with check_signature() that returns an error if more than one signature is encountered. The lower degree of verification makes the use of verify_signed_buffer() problematic if callers don't parse and validate the various parts of the GPG status message themselves. And processing these messages seems like a task that should be reserved to gpg-interface.c with the function check_signature(). Furthermore, the use of verify_signed_buffer() makes it difficult to introduce new functionality that relies on the content of the GPG status lines. Now all operations that does signature verification share a single entry point to gpg-interface.c. This makes it easier to propagate changed or additional functionality in GPG signature verification to all parts of Git, without having odd edge-cases that don't perform the same degree of verification. [1] https://dev.gentoo.org/~mgorny/articles/attack-on-git-signature-verification.html Signed-off-by: Hans Jerry Illikainen <hji@dyntopia.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'log-tree.c')
-rw-r--r--log-tree.c30
1 files changed, 16 insertions, 14 deletions
diff --git a/log-tree.c b/log-tree.c
index 1e56df62a7..aa6b038adb 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -448,22 +448,22 @@ static void show_signature(struct rev_info *opt, struct commit *commit)
{
struct strbuf payload = STRBUF_INIT;
struct strbuf signature = STRBUF_INIT;
- struct strbuf gpg_output = STRBUF_INIT;
+ struct signature_check sigc = { 0 };
int status;
if (parse_signed_commit(commit, &payload, &signature) <= 0)
goto out;
- status = verify_signed_buffer(payload.buf, payload.len,
- signature.buf, signature.len,
- &gpg_output, NULL);
- if (status && !gpg_output.len)
- strbuf_addstr(&gpg_output, "No signature\n");
-
- show_sig_lines(opt, status, gpg_output.buf);
+ status = check_signature(payload.buf, payload.len, signature.buf,
+ signature.len, &sigc);
+ if (status && sigc.result == 'N')
+ show_sig_lines(opt, status, "No signature\n");
+ else {
+ show_sig_lines(opt, status, sigc.gpg_output);
+ signature_check_clear(&sigc);
+ }
out:
- strbuf_release(&gpg_output);
strbuf_release(&payload);
strbuf_release(&signature);
}
@@ -496,6 +496,7 @@ static int show_one_mergetag(struct commit *commit,
struct object_id oid;
struct tag *tag;
struct strbuf verify_message;
+ struct signature_check sigc = { 0 };
int status, nth;
size_t payload_size, gpg_message_offset;
@@ -524,12 +525,13 @@ static int show_one_mergetag(struct commit *commit,
status = -1;
if (extra->len > payload_size) {
/* could have a good signature */
- if (!verify_signed_buffer(extra->value, payload_size,
- extra->value + payload_size,
- extra->len - payload_size,
- &verify_message, NULL))
+ if (!check_signature(extra->value, payload_size,
+ extra->value + payload_size,
+ extra->len - payload_size, &sigc)) {
+ strbuf_addstr(&verify_message, sigc.gpg_output);
+ signature_check_clear(&sigc);
status = 0; /* good */
- else if (verify_message.len <= gpg_message_offset)
+ } else if (verify_message.len <= gpg_message_offset)
strbuf_addstr(&verify_message, "No signature\n");
/* otherwise we couldn't verify, which is shown as bad */
}