Re: [RFC/PATCH] verify-tag: add --check-name flag

santiago@nyu.edu writes:

1.- Using a tag ref as a check-out mechanism is pretty common by package
    managers and other tools. Verifying the tag signature provides
    authentication guarantees, but there is no feedback that the
    signature being verified belongs to the intended tag.

Very true.

The above means that the existing package managers and other tools
need to be updated with some new code that lets them learn how to
tell if the tagname (in their refs/tags/ namespace) matches the
intended "real" tag name, and your --check-name option could be
that.

But if you are adding new code to the existing package managers and
other tools _anyway_, wouldn't it be a more direct solution to let
them learn how to tell what the intended "real" tag name is with
that new code?

It is true that "git cat-file tag v1.4.11" lets you examine all
lines of a given tag object, but the calling program needs to pick
pieces apart with something like:

	git cat-file tag v1.4.11 | sed -e '/^$/q' -e 's/^tag //p'

which may be cumbersome.  Perhaps, just like "git tag -v v1.4.11" is
a way to see if the contents of the tag is signed properly, if you
add "git tag --show-tagname v1.4.11" that does the above pipeline,
these package managers and other tools can be updated to

	tag="$1"
-	if ! git tag -v "$tag"
+	if ! git tag -v "$tag" ||
+	   test "$tag" != "$(git tag --show-tagname $tag)"
        then
		echo >&2 "Bad tag."
                exit 1
	fi
	make dest=/usr/local/$package/$tag install

Or it could even do this:

	tag="$1"
	if ! git tag -v "$tag"
	if ! git tag -v "$tag"
        then
		echo >&2 "Bad tag."
                exit 1
	fi
+	tag=$(git tag --show-tagname $tag)
	make dest=/usr/local/$package/$tag install

i.e. ignore the refname entirely and use the "real" tagname it reads
after validating the signature as the name of the resulting version
getting installed, distributed and/or used.

On Tue, Jun 07, 2016 at 02:05:20PM -0700, Junio C Hamano wrote:

It is true that "git cat-file tag v1.4.11" lets you examine all
lines of a given tag object, but the calling program needs to pick
pieces apart with something like:

	git cat-file tag v1.4.11 | sed -e '/^$/q' -e 's/^tag //p'

which may be cumbersome.  Perhaps, just like "git tag -v v1.4.11" is
a way to see if the contents of the tag is signed properly, if you
add "git tag --show-tagname v1.4.11" that does the above pipeline,
these package managers and other tools can be updated to

	tag="$1"
-	if ! git tag -v "$tag"
+	if ! git tag -v "$tag" ||
+	   test "$tag" != "$(git tag --show-tagname $tag)"
        then
		echo >&2 "Bad tag."
                exit 1
	fi
	make dest=/usr/local/$package/$tag install

That is much more flexible, as they could even do some more complicated
matching than a single string (though in practice, for security things,
I think simpler is better).

I think this option is going to become a blueprint for other "extended"
checks, too. E.g., you might also want to check that the tagger ident
matches the uid on the signing key.

My main worry is that we'll accrue a whole bunch of such logic. And even
though each one is relatively simple, it would be nice for callers to be
able to ask us to just do the standard safety checks.

If we do go with the "print it out and let the caller do their own
checks" strategy, I think I'd prefer rather than "--show-tagname" to
just respect the "--format" we use for tag-listing. That would let you
do:

  git tag -v --format='%(tag)%n%(tagger)'

or similar. In fact you can already do that with a separate step (modulo
%n, which we do not seem to understand here), but like your example:

Or it could even do this:

	tag="$1"
	if ! git tag -v "$tag"
	if ! git tag -v "$tag"
        then
		echo >&2 "Bad tag."
                exit 1
	fi
+	tag=$(git tag --show-tagname $tag)
	make dest=/usr/local/$package/$tag install

It is racy. That probably doesn't matter for most callers, but it would
be nice to be able to get a custom format out of the "-v" invocation.

-Peff

On Tue, Jun 07, 2016 at 02:05:20PM -0700, Junio C Hamano wrote:

santiago@nyu.edu writes:

quoted

1.- Using a tag ref as a check-out mechanism is pretty common by package
    managers and other tools. Verifying the tag signature provides
    authentication guarantees, but there is no feedback that the
    signature being verified belongs to the intended tag.

Very true.

The above means that the existing package managers and other tools
need to be updated with some new code that lets them learn how to
tell if the tagname (in their refs/tags/ namespace) matches the
intended "real" tag name, and your --check-name option could be
that.

But if you are adding new code to the existing package managers and
other tools _anyway_, wouldn't it be a more direct solution to let
them learn how to tell what the intended "real" tag name is with
that new code?

Yeah, you're right, I didn't consider that. I'm thinking that this kind
of verification could simplify the lives of upstream maintainers if we
do the verification in-house though (i.e., by having them just add the
flag).

which may be cumbersome.  Perhaps, just like "git tag -v v1.4.11" is
a way to see if the contents of the tag is signed properly, if you
add "git tag --show-tagname v1.4.11" that does the above pipeline,
these package managers and other tools can be updated to
...
make dest=/usr/local/$package/$tag install

i.e. ignore the refname entirely and use the "real" tagname it reads
after validating the signature as the name of the resulting version
getting installed, distributed and/or used.

This is also an alternative, that might be cleaner. I'm wondering if
this is easier to implement than having the --check-name flag.
Intuitively, it seems like that's the case. Would you suggest taking
this path instead?

Thanks!
-Santiago.

On Tue, Jun 07, 2016 at 05:17:07PM -0400, Jeff King wrote:

That is much more flexible, as they could even do some more complicated
matching than a single string (though in practice, for security things,
I think simpler is better).

I think this option is going to become a blueprint for other "extended"
checks, too. E.g., you might also want to check that the tagger ident
matches the uid on the signing key.

My main worry is that we'll accrue a whole bunch of such logic. And even
though each one is relatively simple, it would be nice for callers to be
able to ask us to just do the standard safety checks.

I agree with this. I can't think of other checks off the top of my head,
but I wouldn't be surprised if this is the case. 

I think that having custom flags for each check can also derive in each
package manager/user picking each check based on many different
rationales, which might lead to people overcomplicating things?

If we do go with the "print it out and let the caller do their own
checks" strategy, I think I'd prefer rather than "--show-tagname" to
just respect the "--format" we use for tag-listing. That would let you
do:

  git tag -v --format='%(tag)%n%(tagger)'

or similar. In fact you can already do that with a separate step (modulo
%n, which we do not seem to understand here), but like your example:

It worries me that, in this case, the patches for upstream managers
might be harder to integrate/pitch for users.

Also, maybe we could take both strategies? add a --check-name for
verify-tag and a --format for tag -v (I think either change is easy
enough to do).

quoted

Or it could even do this:

	tag="$1"
	if ! git tag -v "$tag"
	if ! git tag -v "$tag"
        then
		echo >&2 "Bad tag."
                exit 1
	fi
+	tag=$(git tag --show-tagname $tag)
	make dest=/usr/local/$package/$tag install

It is racy. That probably doesn't matter for most callers, but it would
be nice to be able to get a custom format out of the "-v" invocation.

Oh yeah, I didn't consider this either. I also don't think it's such an
issue, but it sounds like a good idea not to have these races.

-Peff

Thanks!
-Santiago.