threads / patch / 29295

patch, 3 partsverify_signed_buffer: fix stale comment

Subject: [PATCH 1/3] verify_signed_buffer: fix stale comment

## tl;dr

7 messages between Jan 4, 2012 and Jan 5, 2012. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

Junio C Hamano· Jan 4, 2012, 22:10 UTC · lore

[PATCH 0/3] More on pulling signed tags

This extends on the earlier topic to pull and merge signed tags. So far, we did record them for later auditing, but did not have an easy way to perform the auditing, other than manually extracting the information from the commit object.

Applies on top of the merge between the current 'master' and the 'jc/signed-commit' topic still in 'next', as the latter topic already has necessary infrastructure to hook into the "log" machinery to show the signatures embedded in commit objects with the "--show-signature" option.

Help on adding tests, docs and tying loose ends are very much appreciated.
A sample workflow with the command may look like this:
  $ git checkout v1.7.8^0
  $ git pull $elsewhere v1.7.8.2
  $ git show -s --show-signature
  commit e00ee29c760ae27c177ec16adb8c94b85909f464
  parent #2, tagged 'v1.7.8.2'
  gpg: Signature made Wed 28 Dec 2011 12:03:39 PM PST using RSA key ID 96AFE6CB
  gpg: Good signature from "Junio C Hamano <gitster@pobox.com>"
  gpg:                 aka "Junio C Hamano <junio@pobox.com>"
  gpg:                 aka "Junio C Hamano <jch@google.com>"
  Merge: 406da78 f3f778d
  Author: Junio C Hamano <gitster@pobox.com>
  Date:   Wed Jan 4 14:05:03 2012 -0800
      Merge tag 'v1.7.8.2' of ../git into HEAD
      Git 1.7.8.2
  $ git commit -S --amend --no-edit
  commit df409f0d0b1f3f521dd0fd2baf6ecf01495c0191
  gpg: Signature made Wed 04 Jan 2012 02:05:26 PM PST using RSA key ID 96AFE6CB
  gpg: Good signature from "Junio C Hamano <gitster@pobox.com>"
  gpg:                 aka "Junio C Hamano <junio@pobox.com>"
  gpg:                 aka "Junio C Hamano <jch@google.com>"
  parent #2, tagged 'v1.7.8.2'
  gpg: Signature made Wed 28 Dec 2011 12:03:39 PM PST using RSA key ID 96AFE6CB
  gpg: Good signature from "Junio C Hamano <gitster@pobox.com>"
  gpg:                 aka "Junio C Hamano <junio@pobox.com>"
  gpg:                 aka "Junio C Hamano <jch@google.com>"
  Merge: 406da78 f3f778d
  Author: Junio C Hamano <gitster@pobox.com>
  Date:   Wed Jan 4 14:05:03 2012 -0800
      Merge tag 'v1.7.8.2' of ../git into HEAD
      Git 1.7.8.2
Junio C Hamano (3):
  verify_signed_buffer: fix stale comment
  log-tree.c: small refactor in show_signature()
  log-tree: show mergetag in log --show-signature output
 gpg-interface.c |    5 +--
 log-tree.c      |  104 ++++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 92 insertions(+), 17 deletions(-)
-- 
1.7.8.2.340.gd18f0f
Junio C Hamano· Jan 4, 2012, 22:10 UTC · re: Junio C Hamano · lore

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 <gitster@pobox.com>
---
 gpg-interface.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)
Show changes to gpg-interface.c +1 −4
diff --git a/gpg-interface.c b/gpg-interface.c
index 18630ff..09ab64a 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,

-- 
1.7.8.2.340.gd18f0f
Junio C Hamano· Jan 4, 2012, 22:10 UTC · re: Junio C Hamano · lore

[PATCH 2/3] 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 <gitster@pobox.com>
---
 log-tree.c |   28 ++++++++++++++++------------
 1 files changed, 16 insertions(+), 12 deletions(-)
Show changes to log-tree.c +16 −12
diff --git a/log-tree.c b/log-tree.c
index 142ba51..005c5a5 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);

-- 
1.7.8.2.340.gd18f0f
Junio C Hamano· Jan 4, 2012, 22:10 UTC · re: Junio C Hamano · lore

[PATCH 3/3] 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 <gitster@pobox.com>
---
 log-tree.c |   76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 75 insertions(+), 1 deletions(-)
Show changes to log-tree.c +75 −1
diff --git a/log-tree.c b/log-tree.c
index 005c5a5..b87dd53 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);
+	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;

-- 
1.7.8.2.340.gd18f0f
Junio C Hamano· Jan 5, 2012, 00:10 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/3] More on pulling signed tags

Junio C Hamano <gitster@pobox.com> writes:
> A sample workflow with the command may look like this:
> ...
>   $ git show -s --show-signature
With this, in the Linux kernel repository, you could do this.
    $ git show -s --show-signature 2240a7bb47
    commit 2240a7bb479c38434bd636da9ce6afbd3f49447a
    parent #2, tagged 'tytso-for-linus-20111214'
    gpg: Signature made Wed 14 Dec 2011 11:15:13 AM PST using RSA key ID C11804F0
    gpg: Good signature from "Theodore Ts'o <tytso@mit.edu>"
    gpg:                 aka "Theodore Ts'o <tytso@debian.org>"
    gpg:                 aka "Theodore Ts'o <tytso@google.com>"
    Merge: 30aaca4 5a0dc73
    Author: Linus Torvalds <torvalds@linux-foundation.org>
    Date:   Wed Dec 14 18:25:58 2011 -0800
        Merge tag 'tytso-for-linus-20111214' of git://git.kernel.org/pub/scm/linu...
        * tag 'tytso-for-linus-20111214' of git://git.kernel.org/pub/scm/linux/ke...
      8< snip 8<
        .. using the new signed tag merge of git that now verifies the gpg
        signature automatically.  Yay.  The branchname was just 'dev', which is
        prettier.  I'll tell Ted to use nicer tag names for future cases.

We might want to reword "parent #2", by the way. The code was designed to apply even to an Octopus, but I suspect 99.9% of the mergetag will be on the second parent of a two-parent commit. You will never see "parent #1" (i.e. direct parent you pulled into) unless you hand-craft the merge commit object, and you are not likely to create an Octopus that pulls multiple signed tags into one commit.

Junio C Hamano· Jan 5, 2012, 00:22 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/3] More on pulling signed tags

Junio C Hamano <gitster@pobox.com> writes:
Show 6 quoted lines
> We might want to reword "parent #2", by the way. The code was designed to
> apply even to an Octopus, but I suspect 99.9% of the mergetag will be on
> the second parent of a two-parent commit. You will never see "parent #1"
> (i.e. direct parent you pulled into) unless you hand-craft the merge
> commit object, and you are not likely to create an Octopus that pulls
> multiple signed tags into one commit.

... and this is a fix-up patch on top of the series. With this, the output becomes like this:

  commit 2240a7bb479c38434bd636da9ce6afbd3f49447a
  merged tag 'tytso-for-linus-20111214'
  gpg: Signature made Wed 14 Dec 2011 11:15:13 AM PST using RSA key ID C11804F0
  gpg: Good signature from "Theodore Ts'o <tytso@mit.edu>"
  gpg:                 aka "Theodore Ts'o <tytso@debian.org>"
  gpg:                 aka "Theodore Ts'o <tytso@google.com>"
  Merge: 30aaca4 5a0dc73
  Author: Linus Torvalds <torvalds@linux-foundation.org>
  Date:   Wed Dec 14 18:25:58 2011 -0800
      Merge tag 'tytso-for-linus-20111214' of git://git.kernel.org/pub/scm/l...
      * tag 'tytso-for-linus-20111214' of git://git.kernel.org/pub/scm/linux...
        ext4: handle EOF correctly in ext4_bio_write_page()
       ...
 log-tree.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)
Show changes to log-tree.c +12 −0
diff --git a/log-tree.c b/log-tree.c
index b87dd53..37510da 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);
Linus Torvalds· Jan 5, 2012, 00:30 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/3] More on pulling signed tags

On Wed, Jan 4, 2012 at 4:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> ... and this is a fix-up patch on top of the series. With this, the output
> becomes like this:

Looking good, this is very useful. The hacks to extract the signatures with 'git cat-file' were pretty ugly.

This is also nice for the case where the person doing the merge doesn't yet have the pgp key something was signed with, so when you do the "git pull", you get the "unknown key" thing instead of a "good signature" message in the editor buffer.

It *used* to be the case that you had to abort the merge, fetch the key, and re-pull in order to verify the key.

Now you can just say "ok, I'll commit the merge" and finish up, then fetch the key, do "git show --show-signature", and if it doesn't show up as good (which would be very rare) you can then 'git reset' instead. Which while less anal, is a lot more convenient.

                    Linus

← back to recent threads