From b60b7566c04e5f54c0e40229c1716d99d834ab68 Mon Sep 17 00:00:00 2001 From: Michael J Gruber Date: Thu, 14 Feb 2013 17:04:42 +0100 Subject: gpg-interface: check good signature in a reliable way Currently, verify_signed_buffer() only checks the return code of gpg, and some callers implement additional unreliable checks for "Good signature" in the gpg output meant for the user. Use the status output instead and parse for a line beinning with "[GNUPG:] GOODSIG ". This is the only reliable way of checking for a good gpg signature. If needed we can change this easily to "[GNUPG:] VALIDSIG " if we want to take into account the trust model. Signed-off-by: Michael J Gruber Signed-off-by: Junio C Hamano --- gpg-interface.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/gpg-interface.c b/gpg-interface.c index 5f142f6198..f700b4c30d 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -96,15 +96,17 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig /* * Run "gpg" to see if the payload matches the detached signature. * gpg_output, when set, receives the diagnostic output from GPG. + * gpg_status, when set, receives the status output from GPG. */ int verify_signed_buffer(const char *payload, size_t payload_size, const char *signature, size_t signature_size, struct strbuf *gpg_output) { struct child_process gpg; - const char *args_gpg[] = {NULL, "--verify", "FILE", "-", NULL}; + const char *args_gpg[] = {NULL, "--status-fd=1", "--verify", "FILE", "-", NULL}; char path[PATH_MAX]; int fd, ret; + struct strbuf buf = STRBUF_INIT; args_gpg[0] = gpg_program; fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX"); @@ -119,9 +121,10 @@ int verify_signed_buffer(const char *payload, size_t payload_size, memset(&gpg, 0, sizeof(gpg)); gpg.argv = args_gpg; gpg.in = -1; + gpg.out = -1; if (gpg_output) gpg.err = -1; - args_gpg[2] = path; + args_gpg[3] = path; if (start_command(&gpg)) { unlink(path); return error("could not run gpg."); @@ -134,9 +137,15 @@ int verify_signed_buffer(const char *payload, size_t payload_size, strbuf_read(gpg_output, gpg.err, 0); close(gpg.err); } + strbuf_read(&buf, gpg.out, 0); + close(gpg.out); + ret = finish_command(&gpg); unlink_or_warn(path); + ret |= !strstr(buf.buf, "\n[GNUPG:] GOODSIG "); + strbuf_release(&buf); + return ret; } -- cgit v1.2.1 From 1315093f99f327ff498ae6c8afcc42651bbddebc Mon Sep 17 00:00:00 2001 From: Michael J Gruber Date: Thu, 14 Feb 2013 17:04:43 +0100 Subject: log-tree: rely upon the check in the gpg_interface It's just so much clearer. Signed-off-by: Michael J Gruber Signed-off-by: Junio C Hamano --- log-tree.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/log-tree.c b/log-tree.c index 4f86defe32..ff9522f3d4 100644 --- a/log-tree.c +++ b/log-tree.c @@ -498,20 +498,17 @@ static void show_one_mergetag(struct rev_info *opt, gpg_message_offset = verify_message.len; payload_size = parse_signature(extra->value, extra->len); - if ((extra->len <= payload_size) || - (verify_signed_buffer(extra->value, payload_size, - extra->value + payload_size, - extra->len - payload_size, - &verify_message) && - verify_message.len <= gpg_message_offset)) { - strbuf_addstr(&verify_message, "No signature\n"); - status = -1; - } - else if (strstr(verify_message.buf + gpg_message_offset, - ": Good signature from ")) - status = 0; - else - status = -1; + status = -1; + if (extra->len > payload_size) + if (verify_signed_buffer(extra->value, payload_size, + extra->value + payload_size, + extra->len - payload_size, + &verify_message)) { + if (verify_message.len <= gpg_message_offset) + strbuf_addstr(&verify_message, "No signature\n"); + else + status = 0; + } show_sig_lines(opt, status, verify_message.buf); strbuf_release(&verify_message); -- cgit v1.2.1 From 9cc4ac8ff1ae84f9435f2c7de3f7ab796103adba Mon Sep 17 00:00:00 2001 From: Michael J Gruber Date: Thu, 14 Feb 2013 17:04:44 +0100 Subject: gpg_interface: allow to request status return Currently, verify_signed_buffer() returns the user facing output only. Allow callers to request the status output also. Signed-off-by: Michael J Gruber Signed-off-by: Junio C Hamano --- builtin/fmt-merge-msg.c | 2 +- builtin/verify-tag.c | 2 +- gpg-interface.c | 11 +++++++---- gpg-interface.h | 2 +- log-tree.c | 4 ++-- pretty.c | 2 +- 6 files changed, 13 insertions(+), 10 deletions(-) diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index d9af43c257..69bf15a981 100644 --- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c @@ -492,7 +492,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out) if (size == len) ; /* merely annotated */ - else if (verify_signed_buffer(buf, len, buf + len, size - len, &sig)) { + else if (verify_signed_buffer(buf, len, buf + len, size - len, &sig, NULL)) { if (!sig.len) strbuf_addstr(&sig, "gpg verification failed.\n"); } diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index a8eee886a5..9cdf332333 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -29,7 +29,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, int verbose) if (size == len) return error("no signature found"); - return verify_signed_buffer(buf, len, buf + len, size - len, NULL); + return verify_signed_buffer(buf, len, buf + len, size - len, NULL, NULL); } static int verify_tag(const char *name, int verbose) diff --git a/gpg-interface.c b/gpg-interface.c index f700b4c30d..ce07cd5cbb 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -100,13 +100,14 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig */ int verify_signed_buffer(const char *payload, size_t payload_size, const char *signature, size_t signature_size, - struct strbuf *gpg_output) + struct strbuf *gpg_output, struct strbuf *gpg_status) { struct child_process gpg; const char *args_gpg[] = {NULL, "--status-fd=1", "--verify", "FILE", "-", NULL}; char path[PATH_MAX]; int fd, ret; struct strbuf buf = STRBUF_INIT; + struct strbuf *pbuf = &buf; args_gpg[0] = gpg_program; fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX"); @@ -137,15 +138,17 @@ int verify_signed_buffer(const char *payload, size_t payload_size, strbuf_read(gpg_output, gpg.err, 0); close(gpg.err); } - strbuf_read(&buf, gpg.out, 0); + if (gpg_status) + pbuf = gpg_status; + strbuf_read(pbuf, gpg.out, 0); close(gpg.out); ret = finish_command(&gpg); unlink_or_warn(path); - ret |= !strstr(buf.buf, "\n[GNUPG:] GOODSIG "); - strbuf_release(&buf); + ret |= !strstr(pbuf->buf, "\n[GNUPG:] GOODSIG "); + strbuf_release(&buf); /* no matter it was used or not */ return ret; } diff --git a/gpg-interface.h b/gpg-interface.h index b9c36088ce..cf99021842 100644 --- a/gpg-interface.h +++ b/gpg-interface.h @@ -2,7 +2,7 @@ #define GPG_INTERFACE_H extern int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key); -extern int verify_signed_buffer(const char *payload, size_t payload_size, const char *signature, size_t signature_size, struct strbuf *gpg_output); +extern int verify_signed_buffer(const char *payload, size_t payload_size, const char *signature, size_t signature_size, struct strbuf *gpg_output, struct strbuf *gpg_status); extern int git_gpg_config(const char *, const char *, void *); extern void set_signing_key(const char *); extern const char *get_signing_key(void); diff --git a/log-tree.c b/log-tree.c index ff9522f3d4..9cb78d195e 100644 --- a/log-tree.c +++ b/log-tree.c @@ -434,7 +434,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit) status = verify_signed_buffer(payload.buf, payload.len, signature.buf, signature.len, - &gpg_output); + &gpg_output, NULL); if (status && !gpg_output.len) strbuf_addstr(&gpg_output, "No signature\n"); @@ -503,7 +503,7 @@ static void show_one_mergetag(struct rev_info *opt, if (verify_signed_buffer(extra->value, payload_size, extra->value + payload_size, extra->len - payload_size, - &verify_message)) { + &verify_message, NULL)) { if (verify_message.len <= gpg_message_offset) strbuf_addstr(&verify_message, "No signature\n"); else diff --git a/pretty.c b/pretty.c index 91bb2d3ef6..1ca86dc7fd 100644 --- a/pretty.c +++ b/pretty.c @@ -917,7 +917,7 @@ static void parse_commit_signature(struct format_commit_context *ctx) goto out; status = verify_signed_buffer(payload.buf, payload.len, signature.buf, signature.len, - &gpg_output); + &gpg_output, NULL); if (status && !gpg_output.len) goto out; ctx->signature.gpg_output = strbuf_detach(&gpg_output, NULL); -- cgit v1.2.1 From 4a868fd655a72c2663d9519363162db467baccb2 Mon Sep 17 00:00:00 2001 From: Michael J Gruber Date: Thu, 14 Feb 2013 17:04:45 +0100 Subject: pretty: parse the gpg status lines rather than the output Currently, parse_signature_lines() parses the gpg output for strings which depend on LANG so it fails to recognize good commit signatures (and thus does not fill in %G? and the like) in most locales. Make it parse the status lines from gpg instead, which are the proper machine interface. This fixes the problem described above. There is a change in behavior for "%GS" which we intentionally do not work around: "%GS" used to put quotes around the signer's uid (or rather: it inherited from the gpg user output). We output the uid without quotes now, just like author and committer names. Signed-off-by: Michael J Gruber Signed-off-by: Junio C Hamano --- pretty.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/pretty.c b/pretty.c index 1ca86dc7fd..9ffe7bcebb 100644 --- a/pretty.c +++ b/pretty.c @@ -692,6 +692,7 @@ struct format_commit_context { unsigned commit_signature_parsed:1; struct { char *gpg_output; + char *gpg_status; char good_bad; char *signer; } signature; @@ -881,13 +882,13 @@ static struct { char result; const char *check; } signature_check[] = { - { 'G', ": Good signature from " }, - { 'B', ": BAD signature from " }, + { 'G', "\n[GNUPG:] GOODSIG " }, + { 'B', "\n[GNUPG:] BADSIG " }, }; static void parse_signature_lines(struct format_commit_context *ctx) { - const char *buf = ctx->signature.gpg_output; + const char *buf = ctx->signature.gpg_status; int i; for (i = 0; i < ARRAY_SIZE(signature_check); i++) { @@ -896,7 +897,7 @@ static void parse_signature_lines(struct format_commit_context *ctx) if (!found) continue; ctx->signature.good_bad = signature_check[i].result; - found += strlen(signature_check[i].check); + found += strlen(signature_check[i].check)+17; next = strchrnul(found, '\n'); ctx->signature.signer = xmemdupz(found, next - found); break; @@ -908,6 +909,7 @@ static void parse_commit_signature(struct format_commit_context *ctx) struct strbuf payload = STRBUF_INIT; struct strbuf signature = STRBUF_INIT; struct strbuf gpg_output = STRBUF_INIT; + struct strbuf gpg_status = STRBUF_INIT; int status; ctx->commit_signature_parsed = 1; @@ -917,13 +919,15 @@ static void parse_commit_signature(struct format_commit_context *ctx) goto out; status = verify_signed_buffer(payload.buf, payload.len, signature.buf, signature.len, - &gpg_output, NULL); + &gpg_output, &gpg_status); if (status && !gpg_output.len) goto out; ctx->signature.gpg_output = strbuf_detach(&gpg_output, NULL); + ctx->signature.gpg_status = strbuf_detach(&gpg_status, NULL); parse_signature_lines(ctx); out: + strbuf_release(&gpg_status); strbuf_release(&gpg_output); strbuf_release(&payload); strbuf_release(&signature); -- cgit v1.2.1 From 0174eeaa736226a0bea19e9bf88c270d61aa9cce Mon Sep 17 00:00:00 2001 From: Michael J Gruber Date: Thu, 14 Feb 2013 17:04:46 +0100 Subject: pretty: make %GK output the signing key for signed commits In order to employ signed keys in an automated way it is absolutely necessary to check which keys the signatures come from. Signed-off-by: Michael J Gruber Signed-off-by: Junio C Hamano --- Documentation/pretty-formats.txt | 1 + pretty.c | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt index d9eddedc72..443381817a 100644 --- a/Documentation/pretty-formats.txt +++ b/Documentation/pretty-formats.txt @@ -133,6 +133,7 @@ The placeholders are: - '%GG': raw verification message from GPG for a signed commit - '%G?': show either "G" for Good or "B" for Bad for a signed commit - '%GS': show the name of the signer for a signed commit +- '%GK': show the key used to sign a signed commit - '%gD': reflog selector, e.g., `refs/stash@{1}` - '%gd': shortened reflog selector, e.g., `stash@{1}` - '%gn': reflog identity name diff --git a/pretty.c b/pretty.c index 9ffe7bcebb..9119fe7728 100644 --- a/pretty.c +++ b/pretty.c @@ -695,6 +695,7 @@ struct format_commit_context { char *gpg_status; char good_bad; char *signer; + char *key; } signature; char *message; size_t width, indent1, indent2; @@ -897,7 +898,9 @@ static void parse_signature_lines(struct format_commit_context *ctx) if (!found) continue; ctx->signature.good_bad = signature_check[i].result; - found += strlen(signature_check[i].check)+17; + found += strlen(signature_check[i].check); + ctx->signature.key = xmemdupz(found, 16); + found += 17; next = strchrnul(found, '\n'); ctx->signature.signer = xmemdupz(found, next - found); break; @@ -1130,6 +1133,10 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder, if (c->signature.signer) strbuf_addstr(sb, c->signature.signer); break; + case 'K': + if (c->signature.key) + strbuf_addstr(sb, c->signature.key); + break; } return 2; } -- cgit v1.2.1