diff options
author | Hans Jerry Illikainen <hji@dyntopia.com> | 2019-11-27 20:48:21 +0300 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2019-12-01 00:52:35 +0300 |
commit | 72b006f4bfd30b7c5037c163efaf279ab65bea9c (patch) | |
tree | 50fdf5a566469f65c1b496efaaee77961775322e /log-tree.c | |
parent | 67a6ea63008bcee32a239934ad29eb5c5a554509 (diff) |
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.c | 30 |
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 */ } |