threads / rfc / 29539

RFC patchverify-tag: check sig of all tags to given object

Subject: [RFC/PATCH] verify-tag: check sig of all tags to given object

## tl;dr

8 messages between Feb 4, 2012 and Feb 4, 2012. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Tom Grennan· Feb 4, 2012, 01:25 UTC · lore

If the command argument is a non-tag object, scan and verify all tags to the given object; for example:

john$ git tag -s -m "I approve" john-README master:README ... john$ git tag -s -m "I recommend" john-HEAD HEAD ... john$ git push <url> tag john-README john$ git push <url> tag john-HEAD

jane$ git fetch --tags <url> jane$ git tag -s -m "I also approve" jane-README master:README ... jane$ git push <url> tag jane-README

jeff$ git fetch --tags <url> jeff$ git verify-tag master:README tag john-README: OK tag jane-README: OK jeff$ git verify-tag HEAD tag john-HEAD: OK

Signed-off-by: Tom Grennan <tom.grennan@ericsson.com>
---
 Documentation/git-verify-tag.txt |    6 +++-
 builtin/verify-tag.c             |   53 +++++++++++++++++++++++++++++++++++---
 2 files changed, 53 insertions(+), 6 deletions(-)
Show changes to 2 files +53 −6

Documentation/git-verify-tag.txt, builtin/verify-tag.c

diff --git a/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt
index 5ff76e8..ce47f95 100644
--- a/Documentation/git-verify-tag.txt
+++ b/Documentation/git-verify-tag.txt
@@ -8,7 +8,7 @@ git-verify-tag - Check the GPG signature of tags
 SYNOPSIS
 --------
 [verse]
-'git verify-tag' <tag>...
+'git verify-tag' <object>...
 
 DESCRIPTION
 -----------
@@ -20,8 +20,10 @@ OPTIONS
 --verbose::
 	Print the contents of the tag object before validating it.
 
-<tag>...::
+<object>...::
 	SHA1 identifiers of git tag objects.
+	For non-tag objects, scan and verify all tags to the given
+	object.
 
 GIT
 ---
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 28c2174..df9e93c 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -7,6 +7,7 @@
  */
 #include "cache.h"
 #include "builtin.h"
+#include "refs.h"
 #include "tag.h"
 #include "run-command.h"
 #include <signal.h>
@@ -14,7 +15,7 @@
 #include "gpg-interface.h"
 
 static const char * const verify_tag_usage[] = {
-		"git verify-tag [-v|--verbose] <tag>...",
+		"git verify-tag [-v|--verbose] <object>...",
 		NULL
 };
 
@@ -32,6 +33,46 @@ static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
 	return verify_signed_buffer(buf, len, buf + len, size - len, NULL);
 }
 
+struct obj_filter {
+	const unsigned char *sha1;
+	int verbose;
+	struct strbuf sb;
+};
+
+static int verify_tag_of_obj(const char *refname, const unsigned char *sha1,
+			     int flag, void *cb_data)
+{
+	struct obj_filter *obj = cb_data;
+	enum object_type type;
+	unsigned long size;
+	int len, ret;
+	char *buf = NULL;
+	unsigned char tagged_sha1[20];
+
+	if ((type = sha1_object_info(sha1, NULL), type == OBJ_TAG) \
+	    && (buf = read_sha1_file(sha1, &type, &size), buf) \
+	    && !memcmp("object ", buf, 7) \
+	    && !get_sha1_hex(buf + 7, tagged_sha1) \
+	    && buf[47] == '\n' \
+	    && !memcmp(obj->sha1, tagged_sha1, 20) \
+	    && (len = parse_signature(buf, size), len != size)) {
+		strbuf_reset(&obj->sb);
+		ret = verify_signed_buffer(buf, len, buf + len, size - len,
+					   &obj->sb);
+		if (obj->verbose) {
+			write_in_full(1, buf, len);
+			write_in_full(1, obj->sb.buf, obj->sb.len);
+		} else if (ret) {
+			printf("tag %s: FAILED\n", refname);
+			write_in_full(1, obj->sb.buf, obj->sb.len);
+		} else
+			printf("tag %s: OK\n", refname);
+	}
+	if (buf)
+		free(buf);
+	return 0;
+}
+
 static int verify_tag(const char *name, int verbose)
 {
 	enum object_type type;
@@ -44,9 +85,13 @@ static int verify_tag(const char *name, int verbose)
 		return error("tag '%s' not found.", name);
 
 	type = sha1_object_info(sha1, NULL);
-	if (type != OBJ_TAG)
-		return error("%s: cannot verify a non-tag object of type %s.",
-				name, typename(type));
+	if (type != OBJ_TAG) {
+		struct obj_filter obj = { sha1, verbose };
+		strbuf_init(&obj.sb, 4096);
+		for_each_tag_ref(verify_tag_of_obj, (void *) &obj);
+		strbuf_release(&obj.sb);
+		return 0;
+	}
 
 	buf = read_sha1_file(sha1, &type, &size);
 	if (!buf)
-- 
1.7.9.dirty
Junio C Hamano· Feb 4, 2012, 03:16 UTC · re: Tom Grennan · lore

Re: [RFC/PATCH] verify-tag: check sig of all tags to given object

Tom Grennan <tom.grennan@ericsson.com> writes:
Show 23 quoted lines
> If the command argument is a non-tag object, scan and verify all tags to
> the given object; for example:
>
> john$ git tag -s -m "I approve" john-README master:README
> ...
> john$ git tag -s -m "I recommend" john-HEAD HEAD
> ...
> john$ git push <url> tag john-README
> john$ git push <url> tag john-HEAD
>
> jane$ git fetch --tags <url>
> jane$ git tag -s -m "I also approve" jane-README master:README
> ...
> jane$ git push <url> tag jane-README
>
> jeff$ git fetch --tags <url>
> jeff$ git verify-tag master:README
> tag john-README: OK
> tag jane-README: OK
> jeff$ git verify-tag HEAD
> tag john-HEAD: OK
>
> Signed-off-by: Tom Grennan <tom.grennan@ericsson.com>

You did not describe what problem you are trying to solve, but the above tells me that the design of this feature has a lot of room to be improved to be useful for even a single trivial use scenario I can think of off the top of my head.

Let's say after tagging v1.7.10, for some reason (as I do not know what problem you are trying to solve), I decided to ask my back-up maintainers, let's call them Shawn and Jeff, to sign that tag. Shawn is expected to do this:

    spearce$ git fetch tag v1.7.10
    spearce$ git tag -s -m "This tag is Gitster's" v1.7.10-spearce v1.7.10
    spearce$ git push http://example.com/spearce/git tags/v1.7.10-spearce

Jeff will do the same, and I'll fetch v1.7.10-spearce and v1.7.10-peff tags from them.

It is natural for me to be able to ask "I want to verify all tags that point at the object I asked to be signed, namely, v1.7.10" from this feature.

But
    gitster$ git verify-tag v1.7.10

would not be a way to do so, as that would check my signature in v1.7.10 tag itself.

It gets even worse.  Suppose Jeff does this instead by mistake:
    peff$ git fetch v1.7.10
    peff$ git tag v1.7.10-peff v1.7.10
    peff$ git push http://example.com/peff/git tags/v1.7.10-peff

Even if you added "git verify-tag --pointed v1.7.10" to disambiguate the request to use the new feature, the result is unusable, as I would see:

    gitster$ git verify-tag --pointed v1.7.10
    v1.7.10-spearce: OK
    v1.7.10-peff: OK

v1.7.10-spearce and v1.7.10-peff both resolve to my v1.7.10, and they both are signed by known key, but v1.7.10-peff is a lightweight tag that points directly at my v1.7.10 and I would be seeing a signature of my own as "OK".

Junio C Hamano· Feb 4, 2012, 05:16 UTC · re: Junio C Hamano · lore

Re: [RFC/PATCH] verify-tag: check sig of all tags to given object

Junio C Hamano <gitster@pobox.com> writes:
> You did not describe what problem you are trying to solve, but the above
> tells me that the design of this feature has a lot of room to be improved
> to be useful for even a single trivial use scenario I can think of off the
> top of my head.

Having said all that, the biggest problem I have with the approach of this patch is that it does not decompose the necessary functionalities in a way that allows combinations of options people naturally would expect from the description of the feature.

"git tag" can be used to create (-a), delete (-d), show (-l) and verify (-v). Among these modes, creation has to work on a single tag, so it is a bit special. But the other three modes could also use the filtering feature of what "show" supports among themselves.

The mode to show tags (-l) starts from _all_ tags, but limits the output to those that match given patterns, and the output can further be limited to the ones that contain a given commit. The verify mode and the delete mode do not offer this behaviour and require the user to specify exact tag names. If you are adding "I want to do X on all tags that point at this object" feature, in which X happens to be "verification" in your case, don't you think other people would naturally want to use the feature with X=show instead?

The right first step to do this would be to enhance the "filter" logic so that it not just lets "--contains", but also "--points-at", be specified. Once that is done, you could just say:

	$ git tag -l --points-at master:README

to list all the tags that point at the blob, and then your original example becomes:

	$ git tag --points-at master:README | xargs git tag -v
It even allows something like this:
	$ git tag --points-at master:README 'v1.[0-4].?' |
          xargs git tag -v

to work on tags that point at the blob but only those whose names match the given pattern, i.e. versions from v1.0 series up to v1.4 series but not later.

After that is done, you could teach the --verify mode to also accept the patterns, not exact names, so that the above could become:

	$ git tag --verify --points-at master:README
        $ git tag --verify --points-at master:README 'v1.[0-4].?'

Theoretically, the "start from all and then filter" feature could be shared also with "delete" mode, but I would not recommend doing so for safety reasons (i.e. "delete" is destructive). Even so, if somebody does want to do a bulk delete, it is just a simple matter of:

	$ git tag --points-at master:README 'v1.[0-4].?' |
          xargs git tag -d

So it is not a big deal if you did not to teach --delete to share the filtering logic with "show".

For exactly the same reason, --verify mode does not have to share the logic, either. Not having to use "| xargs" is merely an icing on the cake.

Hmm?
Tom Grennan· Feb 4, 2012, 05:08 UTC · lore

Re: [RFC/PATCH] verify-tag: check sig of all tags to given object

Junio C Hamano <gitster@pobox.com<mailto:gitster@pobox.com>> writes:
Show 70 quoted lines
>Tom Grennan <tom.grennan@ericsson.com<mailto:tom.grennan@ericsson.com>> writes:
>>
>>If the command argument is a non-tag object, scan and verify all tags to
>>the given object; for example:
>>
>>john$ git tag -s -m "I approve" john-README master:README
>>...
>>john$ git tag -s -m "I recommend" john-HEAD HEAD
>>...
>>john$ git push <url> tag john-README
>>john$ git push <url> tag john-HEAD
>>
>>jane$ git fetch --tags <url>
>>jane$ git tag -s -m "I also approve" jane-README master:README
>>...
>>jane$ git push <url> tag jane-README
>>
>>jeff$ git fetch --tags <url>
>>jeff$ git verify-tag master:README
>>tag john-README: OK
>>tag jane-README: OK
>>jeff$ git verify-tag HEAD
>>tag john-HEAD: OK
>>
>>Signed-off-by: Tom Grennan <tom.grennan@ericsson.com<mailto:tom.grennan@ericsson.com>>
>
>You did not describe what problem you are trying to solve, but the above
>tells me that the design of this feature has a lot of room to be improved
>to be useful for even a single trivial use scenario I can think of off the
>top of my head.
>
>Let's say after tagging v1.7.10, for some reason (as I do not know what
>problem you are trying to solve), I decided to ask my back-up maintainers,
>let's call them Shawn and Jeff, to sign that tag.  Shawn is expected to do
>this:
>
>   spearce$ git fetch tag v1.7.10
>   spearce$ git tag -s -m "This tag is Gitster's" v1.7.10-spearce v1.7.10
>   spearce$ git push http://example.com/spearce/git tags/v1.7.10-spearce
>
>Jeff will do the same, and I'll fetch v1.7.10-spearce and v1.7.10-peff
>tags from them.
>
>It is natural for me to be able to ask "I want to verify all tags that
>point at the object I asked to be signed, namely, v1.7.10" from this
>feature.
>
>But
>
>   gitster$ git verify-tag v1.7.10
>
>would not be a way to do so, as that would check my signature in v1.7.10
>tag itself.
>
>It gets even worse.  Suppose Jeff does this instead by mistake:
>
>   peff$ git fetch v1.7.10
>   peff$ git tag v1.7.10-peff v1.7.10
>   peff$ git push http://example.com/peff/git tags/v1.7.10-peff
>
>Even if you added "git verify-tag --pointed v1.7.10" to disambiguate the
>request to use the new feature, the result is unusable, as I would see:
>
>   gitster$ git verify-tag --pointed v1.7.10
>   v1.7.10-spearce: OK
>   v1.7.10-peff: OK
>
>v1.7.10-spearce and v1.7.10-peff both resolve to my v1.7.10, and they both
>are signed by known key, but v1.7.10-peff is a lightweight tag that points
>directly at my v1.7.10 and I would be seeing a signature of my own as "OK".
Sorry for messing up this thread. I had remote access trouble with work.

Wouldn't you want Shawn and Jeff to tag the object (commit, tree, or blob) that you had tagged?

   spearce$ git fetch tag v1.7.10
   spearce$ eval $(git verify-tag -v v1.7.10 | sed -n '1s/ /=/p')
   spearce$ git tag -s -m "This tag is Gitster's" v1.7.10-spearce $object
   spearce$ git push http://example.com/spearce/git tags/v1.7.10-spearce
Or,
   spearce$ git fetch tag v1.7.10
   spearce$ git tag -s -m "This tag is Gitster's" v1.7.10-spearce --pointed v1.7.10
   spearce$ git push http://example.com/spearce/git tags/v1.7.10-spearce
Then,
   gitster$ git verify-tag $object
   tag v1.7.10: OK
   tag v1.7.10-spearce: OK
Or,
   gitster$ git verify-tag --pointed v1.7.10
   tag v1.7.10: OK
   tag v1.7.10-spearce: OK

I hadn't thought of tagging a tag. As you indicate, this would be tricky to disambiguate.

So, I intended this to verify all tags of the non-tag object that one asked to be signed; no sign, no record.

BTW, it may also be convenient to use auto-generated tag names for the secondary sign-offs, perhaps using the tag SHA as it's refs/tags/SHA.

   spearce$ git tag -s -m "This is Gitster's v1.7.10" -- --pointed v1.7.10
   peff$ git tag -s -m "This is Gitster's v1.7.10" -- --pointed v1.7.10
   gitster$ git verify-tag --pointed v1.7.10
   tag v1.7.10: OK
   tag XXX....XXX: OK
   tag YYY....YYY: OK
`git tag -l` could then ignore refs/tags/SHA with content SHA.
-- 
TomG
Junio C Hamano· Feb 4, 2012, 05:22 UTC · re: Tom Grennan · lore

Re: [RFC/PATCH] verify-tag: check sig of all tags to given object

Tom Grennan <tmgrennan@gmail.com> writes:
> Wouldn't you want Shawn and Jeff to tag the object (commit, tree, or
> blob) that you had tagged?
No.

We _designed_ our tag objects so that they are capable of pointing at another tag, not the object that is pointed at that other tag. And that is the example usage I gave you.

The statement by Shawn and Jeff, "This tag is Gitster's" is exactly that. It was not about asserting the authenticity of the commit. It was about the tag object I created.

>    gitster$ git verify-tag --pointed v1.7.10
>    tag v1.7.10: OK

Just saying "$name: OK" will *never* be acceptable. "A signature made by any key in my keychain is fine" is not the usual use case. At least the output needs to be "Good signature from X".

Tom Grennan· Feb 4, 2012, 05:56 UTC · re: Junio C Hamano · lore

Re: [RFC/PATCH] verify-tag: check sig of all tags to given object

On Fri, Feb 03, 2012 at 09:22:44PM -0800, Junio C Hamano wrote:
Show 14 quoted lines
>Tom Grennan <tmgrennan@gmail.com> writes:
>
>> Wouldn't you want Shawn and Jeff to tag the object (commit, tree, or
>> blob) that you had tagged?
>
>No.
>
>We _designed_ our tag objects so that they are capable of pointing at
>another tag, not the object that is pointed at that other tag.  And that
>is the example usage I gave you.
>
>The statement by Shawn and Jeff, "This tag is Gitster's" is exactly that.
>It was not about asserting the authenticity of the commit. It was about
>the tag object I created.

Hmm, how about "git verify-tag [[-v] [--to]] <tag|object>"? With "--to", all tags to the given tag (or object) are verified. Without "--to" just the given <tag> is verified.

Show 6 quoted lines
>>    gitster$ git verify-tag --pointed v1.7.10
>>    tag v1.7.10: OK
>
>Just saying "$name: OK" will *never* be acceptable. "A signature made by
>any key in my keychain is fine" is not the usual use case. At least the
>output needs to be "Good signature from X".
OK, I'll have to play with the gpg --verify-options.
-- 
TomG
Junio C Hamano· Feb 4, 2012, 06:20 UTC · re: Tom Grennan · lore

Re: [RFC/PATCH] verify-tag: check sig of all tags to given object

Tom Grennan <tmgrennan@gmail.com> writes:
Show 28 quoted lines
> On Fri, Feb 03, 2012 at 09:22:44PM -0800, Junio C Hamano wrote:
>>Tom Grennan <tmgrennan@gmail.com> writes:
>>
>>> Wouldn't you want Shawn and Jeff to tag the object (commit, tree, or
>>> blob) that you had tagged?
>>
>>No.
>>
>>We _designed_ our tag objects so that they are capable of pointing at
>>another tag, not the object that is pointed at that other tag.  And that
>>is the example usage I gave you.
>>
>>The statement by Shawn and Jeff, "This tag is Gitster's" is exactly that.
>>It was not about asserting the authenticity of the commit. It was about
>>the tag object I created.
>
> Hmm, how about "git verify-tag [[-v] [--to]] <tag|object>"?
> With "--to", all tags to the given tag (or object) are verified.
> Without "--to" just the given <tag> is verified.
>
>>>    gitster$ git verify-tag --pointed v1.7.10
>>>    tag v1.7.10: OK
>>
>>Just saying "$name: OK" will *never* be acceptable. "A signature made by
>>any key in my keychain is fine" is not the usual use case. At least the
>>output needs to be "Good signature from X".
>
> OK, I'll have to play with the gpg --verify-options.

If it wasn't clear enough from my other message, I would rather not to see any change to --verify codepath as the first step. Don't you think that the simplest and cleanest first step is to add --points-at to the list mode, so that with help from "| xargs git tag -v" you can bulk-verify without any other change?

Tom Grennan· Feb 4, 2012, 06:49 UTC · re: Junio C Hamano · lore

Re: [RFC/PATCH] verify-tag: check sig of all tags to given object

On Fri, Feb 03, 2012 at 10:20:14PM -0800, Junio C Hamano wrote:
Show 36 quoted lines
>Tom Grennan <tmgrennan@gmail.com> writes:
>
>> On Fri, Feb 03, 2012 at 09:22:44PM -0800, Junio C Hamano wrote:
>>>Tom Grennan <tmgrennan@gmail.com> writes:
>>>
>>>> Wouldn't you want Shawn and Jeff to tag the object (commit, tree, or
>>>> blob) that you had tagged?
>>>
>>>No.
>>>
>>>We _designed_ our tag objects so that they are capable of pointing at
>>>another tag, not the object that is pointed at that other tag.  And that
>>>is the example usage I gave you.
>>>
>>>The statement by Shawn and Jeff, "This tag is Gitster's" is exactly that.
>>>It was not about asserting the authenticity of the commit. It was about
>>>the tag object I created.
>>
>> Hmm, how about "git verify-tag [[-v] [--to]] <tag|object>"?
>> With "--to", all tags to the given tag (or object) are verified.
>> Without "--to" just the given <tag> is verified.
>>
>>>>    gitster$ git verify-tag --pointed v1.7.10
>>>>    tag v1.7.10: OK
>>>
>>>Just saying "$name: OK" will *never* be acceptable. "A signature made by
>>>any key in my keychain is fine" is not the usual use case. At least the
>>>output needs to be "Good signature from X".
>>
>> OK, I'll have to play with the gpg --verify-options.
>
>If it wasn't clear enough from my other message, I would rather not to see
>any change to --verify codepath as the first step.  Don't you think that
>the simplest and cleanest first step is to add --points-at to the list
>mode, so that with help from "| xargs git tag -v" you can bulk-verify
>without any other change?

No, I missed that. So you're suggesting, git tag [-n[<num>]] -l [--contains <commit>] [--points-at <object>] [<pattern>...]

If so, I'll look into it tomorrow.

thanks, TomG

← back to recent threads