git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: santiago@nyu.edu
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>,
	Eric Sunshine <sunshine@sunshineco.com>,
	Colin Walters <walters@verbum.org>,
	Santiago Torres <santiago@nyu.edu>
Subject: [RFC/PATCH] verify-tag: add --check-name flag
Date: Tue,  7 Jun 2016 15:56:08 -0400	[thread overview]
Message-ID: <20160607195608.16643-1-santiago@nyu.edu> (raw)

From: Santiago Torres <santiago@nyu.edu>

Hello everyone,

In a previous thread [1] we discussed about the possibility of having a
--check-name flag, for the tag-verify command (and possibly git tag -v).
Although many points were in the table, I don't think that it was
conclusive as to whether having it or not. Also, I noticed that the,
refactored interface requires really minmal changes to do this
(see attached patch). 

This is the behavior I had in mind:


    santiago at ~/.../.../git ✗ ./git-verify-tag --check-name v2.7.0-rc20000
    gpg: Signature made Tue 22 Dec 2015 05:46:04 PM EST using RSA ... 
    gpg: Good signature from "Junio C Hamano <gitster@pobox.com>" [...]
    ...
    error: tag name doesn't match tag header!(v2.7.0-rc2)

    santiago at ~/.../.../git ✗ ./git-verify-tag --check-name v2.7.0-rc2
    gpg: Signature made Tue 22 Dec 2015 05:46:04 PM EST using RSA ...
    gpg: Good signature from "Junio C Hamano <gitster@pobox.com>" [...]
    ...

The rationale behind this is the following:

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.

2.- The tuple tagname + signature can uniquely identify a tag. There
    are many tags that can have the same name, but this is mostly due
    to the naming policy. Having a tag-ref pointing to the right tag
    name with an appropriate signature provides tigther guarantees about
    the tag that's being checked-out.

3.- This follows concerns about other people who wish to provide a
    tighter binding between the refs and the tag objects. The git-evtag
    project is an example of this[2].

What I want to prevent is the following: 

    santiago at ~/.../ ✔ pip install -e git+https://.../django/@1.8.3#egg=django
    Obtaining django from git+https://.../django/@1.8.3#egg=django
    [...] 
    Successfully installed django
    santiago at ~/.../ ✔ django-admin.py --version
    1.4.11

In this example, the tag retrieved is an annotated tag, signed by the
right developer. In this case signature verification would pass, and
there are no hints that something *might* have be wrong. Needless to
say, Django 1.4.11 is deprecated and vulnerable to really nasty XSS and
SQLi vectors...

I acknowledge that it is possible to provide this by using the SHA1 of
the tag object instead of the tag-ref, but this provides comparable
guarantees while keeping a readable interface. Of course that the sha1
is the "tightest" binding, so this also allows for developers to
remove tags during quick-fixes (as Junio pointed out in [1]) and other
edge cases in which the SHA1 would break.

Of course that using this flag needs to be integrated by package
managers and other tools; I wouldn't mind reaching out and even
proposing patches for the popular ones.

A stub of the intended patch follows. I'll can make a cleaner patch
(e.g., to include this in git tag -v) based on any feedback provided.

[1] http://thread.gmane.org/gmane.comp.version-control.git/284757
[2] http://thread.gmane.org/gmane.comp.version-control.git/288439

---
 builtin/verify-tag.c | 1 +
 tag.c                | 8 ++++++++
 tag.h                | 1 +
 3 files changed, 10 insertions(+)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 99f8148..947babe 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -33,6 +33,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
 	const struct option verify_tag_options[] = {
 		OPT__VERBOSE(&verbose, N_("print tag contents")),
 		OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW),
+		OPT_BIT(0, "check-name", &flags, N_("Verify the tag name"), TAG_VERIFY_NAME),
 		OPT_END()
 	};
 
diff --git a/tag.c b/tag.c
index d1dcd18..591b31e 100644
--- a/tag.c
+++ b/tag.c
@@ -55,6 +55,14 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,
 
 	ret = run_gpg_verify(buf, size, flags);
 
+	if (flags & TAG_VERIFY_NAME) {
+		struct tag tag_info;
+		ret += parse_tag_buffer(&tag_info, buf, size);
+		if strncmp(tag_info.tag, name_to_report, size)
+			ret += error("tag name doesn't match tag header!(%s)",
+					tag_info.tag);
+	}
+
 	free(buf);
 	return ret;
 }
diff --git a/tag.h b/tag.h
index a5721b6..30c922e 100644
--- a/tag.h
+++ b/tag.h
@@ -2,6 +2,7 @@
 #define TAG_H
 
 #include "object.h"
+#define TAG_VERIFY_NAME 0x10
 
 extern const char *tag_type;
 
-- 
2.8.3

             reply	other threads:[~2016-06-07 19:56 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-07 19:56 santiago [this message]
2016-06-07 21:05 ` Junio C Hamano
2016-06-07 21:17   ` Jeff King
2016-06-07 21:30     ` Santiago Torres
2016-06-07 21:50     ` Junio C Hamano
2016-06-07 21:55       ` Jeff King
2016-06-07 22:05         ` Junio C Hamano
2016-06-07 22:07           ` Jeff King
2016-06-07 22:11             ` Junio C Hamano
2016-06-07 22:13               ` Jeff King
2016-06-07 22:16                 ` Santiago Torres
2016-06-07 22:21                 ` Junio C Hamano
2016-06-07 22:29                   ` Jeff King
2016-06-07 22:35                     ` Junio C Hamano
2016-06-08 14:21                       ` Santiago Torres
2016-06-08 18:43                         ` Junio C Hamano
2016-06-09 11:48                           ` Michael J Gruber
2016-06-07 21:20   ` Santiago Torres
2016-06-07 21:08 ` Jeff King
2016-06-07 21:13   ` Santiago Torres
2016-06-07 21:18     ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160607195608.16643-1-santiago@nyu.edu \
    --to=santiago@nyu.edu \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    --cc=walters@verbum.org \
    --subject='Re: [RFC/PATCH] verify-tag: add --check-name flag' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).