From e3f55e07076f88ec01a49dcfb7c2ac56658145a4 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 4 Jan 2012 12:43:02 -0800 Subject: [PATCH 1/5] verify_signed_buffer: fix stale comment The function used to take an integer flag to specify where the output should go, but these days we supply a strbuf to receive it. Signed-off-by: Junio C Hamano --- gpg-interface.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/gpg-interface.c b/gpg-interface.c index 18630ff8da..09ab64aa24 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -95,10 +95,7 @@ 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_to tells where the output from "gpg" should go: - * < 0: /dev/null - * = 0: standard error of the calling process - * > 0: the specified file descriptor + * gpg_output, when set, receives the diagnostic output from GPG. */ int verify_signed_buffer(const char *payload, size_t payload_size, const char *signature, size_t signature_size, From c871a1d17b8433d98df59b03da5538f10c4ae52c Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 5 Jan 2012 10:54:14 -0800 Subject: [PATCH 2/5] commit --amend -S: strip existing gpgsig headers Any existing commit signature was made against the contents of the old commit, including its committer date that is about to change, and will become invalid by amending it. Signed-off-by: Junio C Hamano --- builtin/commit.c | 3 ++- commit.c | 26 ++++++++++++++++++++++---- commit.h | 4 ++-- t/t7510-signed-commit.sh | 11 ++++++++++- 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index fa41ec8c87..970a83662a 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1494,7 +1494,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix) } if (amend) { - extra = read_commit_extra_headers(current_head); + const char *exclude_gpgsig[2] = { "gpgsig", NULL }; + extra = read_commit_extra_headers(current_head, exclude_gpgsig); } else { struct commit_extra_header **tail = &extra; append_merge_tag_headers(parents, &tail); diff --git a/commit.c b/commit.c index 27c7226abb..2162a7c572 100644 --- a/commit.c +++ b/commit.c @@ -981,14 +981,15 @@ static void add_extra_header(struct strbuf *buffer, strbuf_addch(buffer, '\n'); } -struct commit_extra_header *read_commit_extra_headers(struct commit *commit) +struct commit_extra_header *read_commit_extra_headers(struct commit *commit, + const char **exclude) { struct commit_extra_header *extra = NULL; unsigned long size; enum object_type type; char *buffer = read_sha1_file(commit->object.sha1, &type, &size); if (buffer && type == OBJ_COMMIT) - extra = read_commit_extra_header_lines(buffer, size); + extra = read_commit_extra_header_lines(buffer, size, exclude); free(buffer); return extra; } @@ -1002,7 +1003,23 @@ static inline int standard_header_field(const char *field, size_t len) (len == 8 && !memcmp(field, "encoding ", 9))); } -struct commit_extra_header *read_commit_extra_header_lines(const char *buffer, size_t size) +static int excluded_header_field(const char *field, size_t len, const char **exclude) +{ + if (!exclude) + return 0; + + while (*exclude) { + size_t xlen = strlen(*exclude); + if (len == xlen && + !memcmp(field, *exclude, xlen) && field[xlen] == ' ') + return 1; + exclude++; + } + return 0; +} + +struct commit_extra_header *read_commit_extra_header_lines(const char *buffer, size_t size, + const char **exclude) { struct commit_extra_header *extra = NULL, **tail = &extra, *it = NULL; const char *line, *next, *eof, *eob; @@ -1028,7 +1045,8 @@ struct commit_extra_header *read_commit_extra_header_lines(const char *buffer, s if (next <= eof) eof = next; - if (standard_header_field(line, eof - line)) + if (standard_header_field(line, eof - line) || + excluded_header_field(line, eof - line, exclude)) continue; it = xcalloc(1, sizeof(*it)); diff --git a/commit.h b/commit.h index 61076486df..123aea3a7c 100644 --- a/commit.h +++ b/commit.h @@ -200,8 +200,8 @@ extern int commit_tree_extended(const char *msg, unsigned char *tree, const char *author, const char *sign_commit, struct commit_extra_header *); -extern struct commit_extra_header *read_commit_extra_headers(struct commit *); -extern struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len); +extern struct commit_extra_header *read_commit_extra_headers(struct commit *, const char **); +extern struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **); extern void free_commit_extra_headers(struct commit_extra_header *extra); diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh index 30401ced07..1d3c56fe61 100755 --- a/t/t7510-signed-commit.sh +++ b/t/t7510-signed-commit.sh @@ -24,7 +24,8 @@ test_expect_success GPG 'create signed commits' ' echo 4 >file && test_tick && git commit -a -m "fourth unsigned" && git tag fourth-unsigned && - test_tick && git commit --amend -S -m "fourth signed" + test_tick && git commit --amend -S -m "fourth signed" && + git tag fourth-signed ' test_expect_success GPG 'show signatures' ' @@ -68,4 +69,12 @@ test_expect_success GPG 'detect fudged signature with NUL' ' ! grep "Good signature from" actual2 ' +test_expect_success GPG 'amending already signed commit' ' + git checkout fourth-signed^0 && + git commit --amend -S --no-edit && + git show -s --show-signature HEAD >actual && + grep "Good signature from" actual && + ! grep "BAD signature from" actual +' + test_done From c6b3ec41e2efdd9e05118c2e99ad70f0e0629873 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 4 Jan 2012 13:48:45 -0800 Subject: [PATCH 3/5] log-tree.c: small refactor in show_signature() The next patch needs to show the result of signature verification on a mergetag extended header in a way similar to how embedded signature for the commit object itself is shown. Separate out the logic to go through the message lines and show them in the "error" color (highlighted) or the "correct" color (dim). Signed-off-by: Junio C Hamano --- log-tree.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/log-tree.c b/log-tree.c index 142ba51427..005c5a51c0 100644 --- a/log-tree.c +++ b/log-tree.c @@ -404,13 +404,27 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit, *extra_headers_p = extra_headers; } +static void show_sig_lines(struct rev_info *opt, int status, const char *bol) +{ + const char *color, *reset, *eol; + + color = diff_get_color_opt(&opt->diffopt, + status ? DIFF_WHITESPACE : DIFF_FRAGINFO); + reset = diff_get_color_opt(&opt->diffopt, DIFF_RESET); + while (*bol) { + eol = strchrnul(bol, '\n'); + printf("%s%.*s%s%s", color, (int)(eol - bol), bol, reset, + *eol ? "\n" : ""); + bol = (*eol) ? (eol + 1) : eol; + } +} + 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; int status; - const char *color, *reset, *bol, *eol; if (parse_signed_commit(commit->object.sha1, &payload, &signature) <= 0) goto out; @@ -421,17 +435,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit) if (status && !gpg_output.len) strbuf_addstr(&gpg_output, "No signature\n"); - color = diff_get_color_opt(&opt->diffopt, - status ? DIFF_WHITESPACE : DIFF_FRAGINFO); - reset = diff_get_color_opt(&opt->diffopt, DIFF_RESET); - - bol = gpg_output.buf; - while (*bol) { - eol = strchrnul(bol, '\n'); - printf("%s%.*s%s%s", color, (int)(eol - bol), bol, reset, - *eol ? "\n" : ""); - bol = (*eol) ? (eol + 1) : eol; - } + show_sig_lines(opt, status, gpg_output.buf); out: strbuf_release(&gpg_output); From 824958e50b250e957998626a21763603ca30e832 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 4 Jan 2012 13:51:28 -0800 Subject: [PATCH 4/5] log-tree: show mergetag in log --show-signature output A commit object that merges a signed tag records the "mergetag" extended header. Check the validity of the GPG signature on it, and show it in a way similar to how "gpgsig" extended header is shown. Signed-off-by: Junio C Hamano --- log-tree.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/log-tree.c b/log-tree.c index 005c5a51c0..61a12a7cb0 100644 --- a/log-tree.c +++ b/log-tree.c @@ -443,6 +443,78 @@ static void show_signature(struct rev_info *opt, struct commit *commit) strbuf_release(&signature); } +static int which_parent(const unsigned char *sha1, const struct commit *commit) +{ + int nth; + const struct commit_list *parent; + + for (nth = 0, parent = commit->parents; parent; parent = parent->next) { + if (!hashcmp(parent->item->object.sha1, sha1)) + return nth; + nth++; + } + return -1; +} + +static void show_one_mergetag(struct rev_info *opt, + struct commit_extra_header *extra, + struct commit *commit) +{ + unsigned char sha1[20]; + struct tag *tag; + struct strbuf verify_message; + int status, nth; + size_t payload_size, gpg_message_offset; + + hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), sha1); + tag = lookup_tag(sha1); + if (!tag) + return; /* error message already given */ + + strbuf_init(&verify_message, 256); + if (parse_tag_buffer(tag, extra->value, extra->len)) + strbuf_addstr(&verify_message, "malformed mergetag\n"); + else if ((nth = which_parent(tag->tagged->sha1, commit)) < 0) + strbuf_addf(&verify_message, "tag %s names a non-parent %s\n", + tag->tag, tag->tagged->sha1); + else + strbuf_addf(&verify_message, + "parent #%d, tagged '%s'\n", nth + 1, tag->tag); + 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; + + show_sig_lines(opt, status, verify_message.buf); + strbuf_release(&verify_message); +} + +static void show_mergetag(struct rev_info *opt, struct commit *commit) +{ + struct commit_extra_header *extra, *to_free; + + to_free = read_commit_extra_headers(commit, NULL); + for (extra = to_free; extra; extra = extra->next) { + if (strcmp(extra->key, "mergetag")) + continue; /* not a merge tag */ + show_one_mergetag(opt, extra, commit); + } + free_commit_extra_headers(to_free); +} + void show_log(struct rev_info *opt) { struct strbuf msgbuf = STRBUF_INIT; @@ -554,8 +626,10 @@ void show_log(struct rev_info *opt) } } - if (opt->show_signature) + if (opt->show_signature) { show_signature(opt, commit); + show_mergetag(opt, commit); + } if (!commit->buffer) return; From d041ffa55a69cb6cdc3b160dc181c7e59b3bd4bb Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 4 Jan 2012 16:23:12 -0800 Subject: [PATCH 5/5] log --show-signature: reword the common two-head merge case While identifying the commit merged to our history as "parent #2" is technically correct, we will never say "parent #1" (as that is the tip of our history before the merge is made), and we rarely would say "parent #3" (which would mean the merge is an octopus), especially when responding to a request to pull a signed tag. Treat the most common case to merge a single commit specially, and just say "merged tag ''" instead. Signed-off-by: Junio C Hamano --- log-tree.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/log-tree.c b/log-tree.c index 61a12a7cb0..3cf569ce7e 100644 --- a/log-tree.c +++ b/log-tree.c @@ -456,6 +456,13 @@ static int which_parent(const unsigned char *sha1, const struct commit *commit) return -1; } +static int is_common_merge(const struct commit *commit) +{ + return (commit->parents + && commit->parents->next + && !commit->parents->next->next); +} + static void show_one_mergetag(struct rev_info *opt, struct commit_extra_header *extra, struct commit *commit) @@ -474,6 +481,11 @@ static void show_one_mergetag(struct rev_info *opt, strbuf_init(&verify_message, 256); if (parse_tag_buffer(tag, extra->value, extra->len)) strbuf_addstr(&verify_message, "malformed mergetag\n"); + else if (is_common_merge(commit) && + !hashcmp(tag->tagged->sha1, + commit->parents->next->item->object.sha1)) + strbuf_addf(&verify_message, + "merged tag '%s'\n", tag->tag); else if ((nth = which_parent(tag->tagged->sha1, commit)) < 0) strbuf_addf(&verify_message, "tag %s names a non-parent %s\n", tag->tag, tag->tagged->sha1);