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

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

[Tom Grennan]

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(-)

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

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

[Junio C Hamano]

Tom Grennan <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>

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".

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

[Tom Grennan]

Junio C Hamano <gitster@pobox.com<mailto:gitster@pobox.com>> writes:
>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

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

[Junio C Hamano]

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".

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

[Tom Grennan]

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.

-- 
TomG

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

[Junio C Hamano]

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?

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

[Tom Grennan]

On Fri, Feb 03, 2012 at 10:20:14PM -0800, Junio C Hamano wrote:
>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

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

[Junio C Hamano]

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?