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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Santiago Torres <santiago@xxxxxxx>

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@xxxxxxxxx>" [...]
    ...
    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@xxxxxxxxx>" [...]
    ...

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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]