{"thread":{"id":"16436","subject":"[RFC PATCH 2/4] verify-tag.c: ignore SIGPIPE around gpg invocation","startedAt":"2008-11-24T03:23:16Z","lastAt":"2008-11-28T01:43:00Z","messageCount":16,"participants":["Deskin Miller","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"96396","messageId":"1227497000-8684-1-git-send-email-deskinm@umich.edu","threadId":"16436","inReplyTo":null,"subject":"[RFC PATCH 0/4] Teach git fetch to verify signed tags automatically","fromName":"Deskin Miller","fromEmail":"deskinm@umich.edu","sentAt":"2008-11-24T03:23:16Z","receivedAt":"2008-11-24T03:23:16Z","isPatch":true,"sender":{"key":"deskinm@umich.edu","avatar":"https://gravatar.com/avatar/d340a0e612cdf0a79535c71863c0b4c535e9aba63b42032226ae903e638b64f9?d=mp&s=160"},"body":"It struck me a while back when I fetched a new tagged release from git.git that\nif I wanted to verify the tag's signature, I'd have to issue another command to\ndo so.  Shouldn't git be able to do that for me automatically, when it fetches\nsigned tags?  Now it does.  Also, 'git remote update' gets this for free.\n\nIndividual commit messages explain things reasonably well, I hope; here are a\nfew points for discussion:\n\n-Is refactoring builtin-verify-tag.c the right thing to do?\n-Now that the SIGPIPE ignoring is occurring at a lower level, should it be\n removed from cmd_verify_tag?\n-Output format: good, bad, ugly?\n-What to do if a tag is found to have a bad signature?\n\nDeskin Miller (4):\n  Refactor builtin-verify-tag.c\n  verify-tag.c: ignore SIGPIPE around gpg invocation\n  verify-tag.c: suppress gpg output if asked\n  Make git fetch verify signed tags\n\n Makefile             |    2 +\n builtin-fetch.c      |   25 +++++++++++----\n builtin-verify-tag.c |   61 ++----------------------------------\n t/t7004-tag.sh       |   37 ++++++++++++++++++++++\n verify-tag.c         |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++\n verify-tag.h         |   10 ++++++\n 6 files changed, 155 insertions(+), 64 deletions(-)\n create mode 100644 verify-tag.c\n create mode 100644 verify-tag.h\n"},{"id":"96395","messageId":"1227497000-8684-2-git-send-email-deskinm@umich.edu","threadId":"16436","inReplyTo":"1227497000-8684-1-git-send-email-deskinm@umich.edu","subject":"[RFC PATCH 1/4] Refactor builtin-verify-tag.c","fromName":"Deskin Miller","fromEmail":"deskinm@umich.edu","sentAt":"2008-11-24T03:23:17Z","receivedAt":"2008-11-24T03:23:17Z","isPatch":true,"sender":{"key":"deskinm@umich.edu","avatar":"https://gravatar.com/avatar/d340a0e612cdf0a79535c71863c0b4c535e9aba63b42032226ae903e638b64f9?d=mp&s=160"},"body":"builtin-verify-tag.c didn't expose any of its functionality to be used\ninternally.  Refactor some of it into new verify-tag.c and expose\nverify_tag_sha1 able to be called from elsewhere in git.\n\nSigned-off-by: Deskin Miller <deskinm@umich.edu>\n---\n Makefile             |    2 +\n builtin-verify-tag.c |   61 ++-------------------------------------\n verify-tag.c         |   77 ++++++++++++++++++++++++++++++++++++++++++++++++++\n verify-tag.h         |   10 ++++++\n 4 files changed, 93 insertions(+), 57 deletions(-)\n create mode 100644 verify-tag.c\n create mode 100644 verify-tag.h\n\ndiff --git a/Makefile b/Makefile\nindex 35adafa..b372aa4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -392,6 +392,7 @@ LIB_H += tree-walk.h\n LIB_H += unpack-trees.h\n LIB_H += userdiff.h\n LIB_H += utf8.h\n+LIB_H += verify-tag.h\n LIB_H += wt-status.h\n \n LIB_OBJS += abspath.o\n@@ -490,6 +491,7 @@ LIB_OBJS += unpack-trees.o\n LIB_OBJS += userdiff.o\n LIB_OBJS += usage.o\n LIB_OBJS += utf8.o\n+LIB_OBJS += verify-tag.o\n LIB_OBJS += walker.o\n LIB_OBJS += wrapper.o\n LIB_OBJS += write_or_die.o\ndiff --git a/builtin-verify-tag.c b/builtin-verify-tag.c\nindex 729a159..dd350e8 100644\n--- a/builtin-verify-tag.c\n+++ b/builtin-verify-tag.c\n@@ -7,65 +7,16 @@\n  */\n #include \"cache.h\"\n #include \"builtin.h\"\n-#include \"tag.h\"\n-#include \"run-command.h\"\n+#include \"verify-tag.h\"\n #include <signal.h>\n \n static const char builtin_verify_tag_usage[] =\n \t\t\"git verify-tag [-v|--verbose] <tag>...\";\n \n-#define PGP_SIGNATURE \"-----BEGIN PGP SIGNATURE-----\"\n-\n-static int run_gpg_verify(const char *buf, unsigned long size, int verbose)\n-{\n-\tstruct child_process gpg;\n-\tconst char *args_gpg[] = {\"gpg\", \"--verify\", \"FILE\", \"-\", NULL};\n-\tchar path[PATH_MAX], *eol;\n-\tsize_t len;\n-\tint fd, ret;\n-\n-\tfd = git_mkstemp(path, PATH_MAX, \".git_vtag_tmpXXXXXX\");\n-\tif (fd < 0)\n-\t\treturn error(\"could not create temporary file '%s': %s\",\n-\t\t\t\t\t\tpath, strerror(errno));\n-\tif (write_in_full(fd, buf, size) < 0)\n-\t\treturn error(\"failed writing temporary file '%s': %s\",\n-\t\t\t\t\t\tpath, strerror(errno));\n-\tclose(fd);\n-\n-\t/* find the length without signature */\n-\tlen = 0;\n-\twhile (len < size && prefixcmp(buf + len, PGP_SIGNATURE)) {\n-\t\teol = memchr(buf + len, '\\n', size - len);\n-\t\tlen += eol ? eol - (buf + len) + 1 : size - len;\n-\t}\n-\tif (verbose)\n-\t\twrite_in_full(1, buf, len);\n-\n-\tmemset(&gpg, 0, sizeof(gpg));\n-\tgpg.argv = args_gpg;\n-\tgpg.in = -1;\n-\targs_gpg[2] = path;\n-\tif (start_command(&gpg)) {\n-\t\tunlink(path);\n-\t\treturn error(\"could not run gpg.\");\n-\t}\n-\n-\twrite_in_full(gpg.in, buf, len);\n-\tclose(gpg.in);\n-\tret = finish_command(&gpg);\n-\n-\tunlink(path);\n-\n-\treturn ret;\n-}\n-\n static int verify_tag(const char *name, int verbose)\n {\n \tenum object_type type;\n \tunsigned char sha1[20];\n-\tchar *buf;\n-\tunsigned long size;\n \tint ret;\n \n \tif (get_sha1(name, sha1))\n@@ -76,13 +27,9 @@ static int verify_tag(const char *name, int verbose)\n \t\treturn error(\"%s: cannot verify a non-tag object of type %s.\",\n \t\t\t\tname, typename(type));\n \n-\tbuf = read_sha1_file(sha1, &type, &size);\n-\tif (!buf)\n-\t\treturn error(\"%s: unable to read file.\", name);\n-\n-\tret = run_gpg_verify(buf, size, verbose);\n-\n-\tfree(buf);\n+\tret = verify_tag_sha1(sha1, verbose);\n+\tif (ret)\n+\t\terror(\"Failed to verify %s.\", name);\n \treturn ret;\n }\n \ndiff --git a/verify-tag.c b/verify-tag.c\nnew file mode 100644\nindex 0000000..c9be331\n--- /dev/null\n+++ b/verify-tag.c\n@@ -0,0 +1,77 @@\n+/*\n+ * Internals for \"git verify-tag\"\n+ *\n+ * Copyright (c) 2008 Deskin Miller <deskinm@umich.edu>\n+ *\n+ */\n+#include \"cache.h\"\n+#include \"object.h\"\n+#include \"run-command.h\"\n+\n+#define PGP_SIGNATURE \"-----BEGIN PGP SIGNATURE-----\"\n+\n+static int run_gpg_verify(const char *buf, unsigned long size, int verbose)\n+{\n+\tstruct child_process gpg;\n+\tconst char *args_gpg[] = {\"gpg\", \"--verify\", \"FILE\", \"-\", NULL};\n+\tchar path[PATH_MAX], *eol;\n+\tsize_t len;\n+\tint fd, ret;\n+\n+\tfd = git_mkstemp(path, PATH_MAX, \".git_vtag_tmpXXXXXX\");\n+\tif (fd < 0)\n+\t\treturn error(\"could not create temporary file '%s': %s\",\n+\t\t\t\t\t\tpath, strerror(errno));\n+\tif (write_in_full(fd, buf, size) < 0)\n+\t\treturn error(\"failed writing temporary file '%s': %s\",\n+\t\t\t\t\t\tpath, strerror(errno));\n+\tclose(fd);\n+\n+\t/* find the length without signature */\n+\tlen = 0;\n+\twhile (len < size && prefixcmp(buf + len, PGP_SIGNATURE)) {\n+\t\teol = memchr(buf + len, '\\n', size - len);\n+\t\tlen += eol ? eol - (buf + len) + 1 : size - len;\n+\t}\n+\tif (verbose)\n+\t\twrite_in_full(1, buf, len);\n+\n+\tmemset(&gpg, 0, sizeof(gpg));\n+\tgpg.argv = args_gpg;\n+\tgpg.in = -1;\n+\targs_gpg[2] = path;\n+\tif (start_command(&gpg)) {\n+\t\tunlink(path);\n+\t\treturn error(\"could not run gpg.\");\n+\t}\n+\n+\twrite_in_full(gpg.in, buf, len);\n+\tclose(gpg.in);\n+\tret = finish_command(&gpg);\n+\n+\tunlink(path);\n+\n+\treturn ret;\n+}\n+\n+int verify_tag_sha1(const unsigned char *sha1, int verbose)\n+{\n+\tenum object_type type;\n+\tchar *buf;\n+\tunsigned long size;\n+\tint ret;\n+\n+\ttype = sha1_object_info(sha1, NULL);\n+\tif (type != OBJ_TAG)\n+\t\treturn error(\"Cannot verify a non-tag object of type %s.\",\n+\t\t\t\ttypename(type));\n+\n+\tbuf = read_sha1_file(sha1, &type, &size);\n+\tif (!buf)\n+\t\treturn error(\"Cnable to read file.\");\n+\n+\tret = run_gpg_verify(buf, size, verbose);\n+\n+\tfree(buf);\n+\treturn ret;\n+}\ndiff --git a/verify-tag.h b/verify-tag.h\nnew file mode 100644\nindex 0000000..45bdca7\n--- /dev/null\n+++ b/verify-tag.h\n@@ -0,0 +1,10 @@\n+#ifndef VERIFY_TAG_H\n+#define VERIFY_TAG_H\n+/*\n+ * Internals for \"git verify-tag\"\n+ *\n+ * Copyright (c) 2008 Deskin Miller <deskinm@umich.edu>\n+ */\n+extern int verify_tag_sha1(const unsigned char *sha1, int verbose);\n+\n+#endif /* VERIFY_TAG_H */\n-- \n1.6.0.4.770.ga8394\n"},{"id":"96394","messageId":"1227497000-8684-3-git-send-email-deskinm@umich.edu","threadId":"16436","inReplyTo":"1227497000-8684-2-git-send-email-deskinm@umich.edu","subject":"[RFC PATCH 2/4] verify-tag.c: ignore SIGPIPE around gpg invocation","fromName":"Deskin Miller","fromEmail":"deskinm@umich.edu","sentAt":"2008-11-24T03:23:18Z","receivedAt":"2008-11-24T03:23:18Z","isPatch":true,"sender":{"key":"deskinm@umich.edu","avatar":"https://gravatar.com/avatar/d340a0e612cdf0a79535c71863c0b4c535e9aba63b42032226ae903e638b64f9?d=mp&s=160"},"body":"builtin-verify-tag.c already sets SIG_IGN for SIGPIPE before calling\nverify_tag, but new callers of verify_tag_sha1 may not have modified the\nsignal handler, and shouldn't have to.  Save and restore the signal\nhandler for SIGPIPE around the invocation of gpg.\n\nSigned-off-by: Deskin Miller <deskinm@umich.edu>\n---\n verify-tag.c |    5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/verify-tag.c b/verify-tag.c\nindex c9be331..c3e35f3 100644\n--- a/verify-tag.c\n+++ b/verify-tag.c\n@@ -7,6 +7,7 @@\n #include \"cache.h\"\n #include \"object.h\"\n #include \"run-command.h\"\n+#include <signal.h>\n \n #define PGP_SIGNATURE \"-----BEGIN PGP SIGNATURE-----\"\n \n@@ -17,6 +18,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, int verbose)\n \tchar path[PATH_MAX], *eol;\n \tsize_t len;\n \tint fd, ret;\n+\tsighandler_t save_handle;\n \n \tfd = git_mkstemp(path, PATH_MAX, \".git_vtag_tmpXXXXXX\");\n \tif (fd < 0)\n@@ -40,8 +42,10 @@ static int run_gpg_verify(const char *buf, unsigned long size, int verbose)\n \tgpg.argv = args_gpg;\n \tgpg.in = -1;\n \targs_gpg[2] = path;\n+\tsave_handle = signal(SIGPIPE, SIG_IGN);\n \tif (start_command(&gpg)) {\n \t\tunlink(path);\n+\t\tsignal(SIGPIPE, save_handle);\n \t\treturn error(\"could not run gpg.\");\n \t}\n \n@@ -50,6 +54,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, int verbose)\n \tret = finish_command(&gpg);\n \n \tunlink(path);\n+\tsignal(SIGPIPE, save_handle);\n \n \treturn ret;\n }\n-- \n1.6.0.4.770.ga8394\n"},{"id":"96397","messageId":"1227497000-8684-4-git-send-email-deskinm@umich.edu","threadId":"16436","inReplyTo":"1227497000-8684-3-git-send-email-deskinm@umich.edu","subject":"[RFC PATCH 3/4] verify-tag.c: suppress gpg output if asked","fromName":"Deskin Miller","fromEmail":"deskinm@umich.edu","sentAt":"2008-11-24T03:23:19Z","receivedAt":"2008-11-24T03:23:19Z","isPatch":true,"sender":{"key":"deskinm@umich.edu","avatar":"https://gravatar.com/avatar/d340a0e612cdf0a79535c71863c0b4c535e9aba63b42032226ae903e638b64f9?d=mp&s=160"},"body":"Previously, tag verification would output messages from gpg on standard\nerror.  Allow this to be controlled by a parameter to verify_tag_sha1.\n\nSigned-off-by: Deskin Miller <deskinm@umich.edu>\n---\n verify-tag.c |    4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/verify-tag.c b/verify-tag.c\nindex c3e35f3..b47acc9 100644\n--- a/verify-tag.c\n+++ b/verify-tag.c\n@@ -35,13 +35,15 @@ static int run_gpg_verify(const char *buf, unsigned long size, int verbose)\n \t\teol = memchr(buf + len, '\\n', size - len);\n \t\tlen += eol ? eol - (buf + len) + 1 : size - len;\n \t}\n-\tif (verbose)\n+\tif (verbose == 1)\n \t\twrite_in_full(1, buf, len);\n \n \tmemset(&gpg, 0, sizeof(gpg));\n \tgpg.argv = args_gpg;\n \tgpg.in = -1;\n \targs_gpg[2] = path;\n+\tif (verbose == -1)\n+\t\tgpg.no_stderr = 1;\n \tsave_handle = signal(SIGPIPE, SIG_IGN);\n \tif (start_command(&gpg)) {\n \t\tunlink(path);\n-- \n1.6.0.4.770.ga8394\n"},{"id":"96398","messageId":"1227497000-8684-5-git-send-email-deskinm@umich.edu","threadId":"16436","inReplyTo":"1227497000-8684-4-git-send-email-deskinm@umich.edu","subject":"[RFC PATCH 4/4] Make git fetch verify signed tags","fromName":"Deskin Miller","fromEmail":"deskinm@umich.edu","sentAt":"2008-11-24T03:23:20Z","receivedAt":"2008-11-24T03:23:20Z","isPatch":true,"sender":{"key":"deskinm@umich.edu","avatar":"https://gravatar.com/avatar/d340a0e612cdf0a79535c71863c0b4c535e9aba63b42032226ae903e638b64f9?d=mp&s=160"},"body":"When git fetch downloads signed tag objects, make it verify them right\nthen.  This extends the output summary of fetch to include \"(good\nsignature)\" for valid tags and \"(BAD SIGNATURE)\" for invalid tags.  If\nthe user does not have the correct key in the gpg keyring, gpg returns\n2, verify_tag_sha1 returns -2 and nothing additional is output about\nthe tag's validity.\n\nAlternate fetch method 'git remote update' gets this check as well due\nto the use of the fetch routines.\n\nSigned-off-by: Deskin Miller <deskinm@umich.edu>\n---\n builtin-fetch.c |   25 ++++++++++++++++++-------\n t/t7004-tag.sh  |   37 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 55 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex f151cfa..f7a50b7 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -10,6 +10,7 @@\n #include \"transport.h\"\n #include \"run-command.h\"\n #include \"parse-options.h\"\n+#include \"verify-tag.h\"\n \n static const char * const builtin_fetch_usage[] = {\n \t\"git fetch [options] [<repository> <refspec>...]\",\n@@ -233,11 +234,16 @@ static int update_local_ref(struct ref *ref,\n \n \tif (!is_null_sha1(ref->old_sha1) &&\n \t    !prefixcmp(ref->name, \"refs/tags/\")) {\n-\t\tint r;\n+\t\tint r, v;\n \t\tr = s_update_ref(\"updating tag\", ref, 0);\n-\t\tsprintf(display, \"%c %-*s %-*s -> %s%s\", r ? '!' : '-',\n+\t\tif (type == OBJ_TAG)\n+\t\t\tv = verify_tag_sha1(ref->new_sha1, -1);\n+\t\tsprintf(display, \"%c %-*s %-*s -> %s%s\", r ? '!' :\n+\t\t\t(type == OBJ_TAG ? (v == -1 ? '!' : '-') : '-'),\n \t\t\tSUMMARY_WIDTH, \"[tag update]\", REFCOL_WIDTH, remote,\n-\t\t\tpretty_ref, r ? \"  (unable to update local ref)\" : \"\");\n+\t\t\tpretty_ref, r ? \"  (unable to update local ref)\" :\n+\t\t\t(type == OBJ_TAG ? (v == 0 ? \" (good signature)\" :\n+\t\t\t(v == -1 ? \" (BAD SIGNATURE)\" : \"\")) : \"\"));\n \t\treturn r;\n \t}\n \n@@ -246,7 +252,7 @@ static int update_local_ref(struct ref *ref,\n \tif (!current || !updated) {\n \t\tconst char *msg;\n \t\tconst char *what;\n-\t\tint r;\n+\t\tint r, v;\n \t\tif (!strncmp(ref->name, \"refs/tags/\", 10)) {\n \t\t\tmsg = \"storing tag\";\n \t\t\twhat = \"[new tag]\";\n@@ -257,9 +263,14 @@ static int update_local_ref(struct ref *ref,\n \t\t}\n \n \t\tr = s_update_ref(msg, ref, 0);\n-\t\tsprintf(display, \"%c %-*s %-*s -> %s%s\", r ? '!' : '*',\n-\t\t\tSUMMARY_WIDTH, what, REFCOL_WIDTH, remote, pretty_ref,\n-\t\t\tr ? \"  (unable to update local ref)\" : \"\");\n+\t\tif (type == OBJ_TAG)\n+\t\t\tv = verify_tag_sha1(ref->new_sha1, -1);\n+\t\tsprintf(display, \"%c %-*s %-*s -> %s%s\", r ? '!' :\n+\t\t\t(type == OBJ_TAG ? (v == -1 ? '!' : '*') : '*'),\n+\t\t\tSUMMARY_WIDTH, what, REFCOL_WIDTH, remote,\n+\t\t\tpretty_ref, r ? \"  (unable to update local ref)\" :\n+\t\t\t(type == OBJ_TAG ? (v == 0 ? \" (good signature)\" :\n+\t\t\t(v == -1 ? \" (BAD SIGNATURE)\" : \"\")) : \"\"));\n \t\treturn r;\n \t}\n \ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex f377fea..00327cc 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1037,6 +1037,43 @@ test_expect_success \\\n \t'test_must_fail git tag -s -m tail tag-gpg-failure'\n git config --unset user.signingkey\n \n+git tag -s -m 'good tag' good-tag HEAD\n+bad=$(git cat-file tag good-tag | sed -e 's/good-tag/bad-tag/' | git mktag)\n+git tag bad-tag $bad\n+head=$(git rev-parse HEAD)\n+nonkey=$(cat <<EOF | git mktag\n+object $head\n+type commit\n+tag v1.6.0.4\n+tagger Junio C Hamano <gitster@pobox.com> 1226208581 -0800\n+\n+GIT 1.6.0.4\n+-----BEGIN PGP SIGNATURE-----\n+Version: GnuPG v1.4.9 (GNU/Linux)\n+\n+iEYEABECAAYFAkkWdUUACgkQwMbZpPMRm5rSmwCfWu+K/hXyLUnEWoOMYy1eKuMK\n+KcoAnjB2qir794ibWPy6cn11uUbk7AlC\n+=eaFZ\n+-----END PGP SIGNATURE-----\n+EOF\n+)\n+git tag nonkey-tag $nonkey\n+\n+echo 'bad-tag (BAD SIGNATURE)' > expect\n+echo 'good-tag (good signature)' >> expect\n+echo 'nonkey-tag' >> expect\n+\n+test_expect_success \\\n+\t'git fetch verifies tags' \\\n+\t'mkdir clone &&\n+\t(cd clone &&\n+\tgit init &&\n+\tgit remote add origin file://\"$(cd .. && pwd)\" &&\n+\tgit fetch origin 2>../actual) &&\n+\tsed -i -ne \"/ \\(bad\\|good\\|nonkey\\)-tag/s/^.*->[^a-z]*//p\" actual &&\n+\ttest_cmp expect actual\n+\t'\n+\n # try to verify without gpg:\n \n rm -rf gpghome\n-- \n1.6.0.4.770.ga8394\n"},{"id":"96400","messageId":"7v4p1xohbw.fsf@gitster.siamese.dyndns.org","threadId":"16436","inReplyTo":"1227497000-8684-1-git-send-email-deskinm@umich.edu","subject":"Re: [RFC PATCH 0/4] Teach git fetch to verify signed tags automatically","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-24T04:53:23Z","receivedAt":"2008-11-24T04:53:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Deskin Miller <deskinm@umich.edu> writes:\n\n> It struck me a while back when I fetched a new tagged release from git.git that\n> if I wanted to verify the tag's signature, I'd have to issue another command to\n> do so.  Shouldn't git be able to do that for me automatically, when it fetches\n> signed tags?  Now it does.  Also, 'git remote update' gets this for free.\n\nI think this should be done inside your own hook.  Not interested at all\nin a solution to touch builtin-fetch.c, unless if the patch is about\nadding a new hook so that people with other needs can use it as well.\n"},{"id":"96401","messageId":"7vmyfpn10v.fsf@gitster.siamese.dyndns.org","threadId":"16436","inReplyTo":"7v4p1xohbw.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC PATCH 0/4] Teach git fetch to verify signed tags automatically","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-24T05:30:56Z","receivedAt":"2008-11-24T05:30:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Deskin Miller <deskinm@umich.edu> writes:\n>\n>> It struck me a while back when I fetched a new tagged release from git.git that\n>> if I wanted to verify the tag's signature, I'd have to issue another command to\n>> do so.  Shouldn't git be able to do that for me automatically, when it fetches\n>> signed tags?  Now it does.  Also, 'git remote update' gets this for free.\n>\n> I think this should be done inside your own hook.  Not interested at all\n> in a solution to touch builtin-fetch.c, unless if the patch is about\n> adding a new hook so that people with other needs can use it as well.\n\n... or a much stronger case can be made why this shouldn't be done in a\nhook.\n\nI realize \"not interested at all\" was a bit too strong, so I am trying to\nrephrase it here.  The cycle that begins with an RFC that leads to\ndiscussion and review is about clarifying the rationale and design\nincrementally, so please do not get offended by my no, and sorry for using\nunnecessarily strong wording.\n\nWhat I meant was more like \"The justification as given in the message does\nnot interest me in the patch at all as it stands.  I do not understand why\nthis has to be done as a patch to git-fetch itself, not in a hook script,\nor why doing it inside git-fetch is a better approach than doing it in a\nhook (if there already is a hook mechanism to do this)\".\n"},{"id":"96407","messageId":"alpine.DEB.1.00.0811241140280.30769@pacific.mpi-cbg.de","threadId":"16436","inReplyTo":"1227497000-8684-1-git-send-email-deskinm@umich.edu","subject":"Re: [RFC PATCH 0/4] Teach git fetch to verify signed tags automatically","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-11-24T10:41:27Z","receivedAt":"2008-11-24T10:41:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 23 Nov 2008, Deskin Miller wrote:\n\n> -What to do if a tag is found to have a bad signature?\n\nOr even worse: if the public key was not found?  In dubio pro reo, they \nsay, but OTOH you asked to verify the signatures...\n\nCiao,\nDscho\n"},{"id":"96408","messageId":"alpine.DEB.1.00.0811241143410.30769@pacific.mpi-cbg.de","threadId":"16436","inReplyTo":"1227497000-8684-5-git-send-email-deskinm@umich.edu","subject":"Re: [RFC PATCH 4/4] Make git fetch verify signed tags","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-11-24T10:44:40Z","receivedAt":"2008-11-24T10:44:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 23 Nov 2008, Deskin Miller wrote:\n\n> When git fetch downloads signed tag objects, make it verify them right \n> then.  This extends the output summary of fetch to include \"(good \n> signature)\" for valid tags and \"(BAD SIGNATURE)\" for invalid tags.  If \n> the user does not have the correct key in the gpg keyring, gpg returns \n> 2, verify_tag_sha1 returns -2 and nothing additional is output about the \n> tag's validity.\n\nThis must be turned off by default, IMO.  You cannot expect each and every \ndeveloper to have gpg _and_ all those public keys installed.\n\nCiao,\nDscho\n"},{"id":"96410","messageId":"alpine.DEB.1.00.0811241147490.30769@pacific.mpi-cbg.de","threadId":"16436","inReplyTo":"1227497000-8684-2-git-send-email-deskinm@umich.edu","subject":"Re: [RFC PATCH 1/4] Refactor builtin-verify-tag.c","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-11-24T11:04:59Z","receivedAt":"2008-11-24T11:04:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 23 Nov 2008, Deskin Miller wrote:\n\n> builtin-verify-tag.c didn't expose any of its functionality to be used\n> internally.  Refactor some of it into new verify-tag.c and expose\n> verify_tag_sha1 able to be called from elsewhere in git.\n> \n> Signed-off-by: Deskin Miller <deskinm@umich.edu>\n> ---\n>  Makefile             |    2 +\n>  builtin-verify-tag.c |   61 ++-------------------------------------\n>  verify-tag.c         |   77 ++++++++++++++++++++++++++++++++++++++++++++++++++\n>  verify-tag.h         |   10 ++++++\n>  4 files changed, 93 insertions(+), 57 deletions(-)\n>  create mode 100644 verify-tag.c\n>  create mode 100644 verify-tag.h\n\nI'll comment on the output of \"format-patch -n -C -C\" instead, as that \nmakes it much easier to see what you actually did:\n\n>  Makefile                             |    2 +\n>  builtin-verify-tag.c                 |   61 ++-------------------------------\n>  builtin-verify-tag.c => verify-tag.c |   48 ++++-----------------------\n>  verify-tag.h                         |   10 +++++\n>  4 files changed, 23 insertions(+), 98 deletions(-)\n>  copy builtin-verify-tag.c => verify-tag.c (56%)\n>  create mode 100644 verify-tag.h\n>\n> [...] \n> diff --git a/builtin-verify-tag.c b/verify-tag.c\n> similarity index 56%\n> copy from builtin-verify-tag.c\n> copy to verify-tag.c\n> index 729a159..c9be331 100644\n> --- a/builtin-verify-tag.c\n> +++ b/verify-tag.c\n> @@ -1,18 +1,12 @@\n>  /*\n> - * Builtin \"git verify-tag\"\n> + * Internals for \"git verify-tag\"\n\nAgree.\n\n>   *\n> - * Copyright (c) 2007 Carlos Rica <jasampler@gmail.com>\n> + * Copyright (c) 2008 Deskin Miller <deskinm@umich.edu>\n\nDisagree.\n\nEven if Carlos seemed to stop his work on Git entirely, which I find \ndisappointing, you are _not_ free to pretend his work is yours.  And given \nthis diff:\n\n>   *\n> - * Based on git-verify-tag.sh\n>   */\n>  #include \"cache.h\"\n> -#include \"builtin.h\"\n> -#include \"tag.h\"\n> +#include \"object.h\"\n>  #include \"run-command.h\"\n> -#include <signal.h>\n> -\n> -static const char builtin_verify_tag_usage[] =\n> -\t\t\"git verify-tag [-v|--verbose] <tag>...\";\n>  \n>  #define PGP_SIGNATURE \"-----BEGIN PGP SIGNATURE-----\"\n>  \n> @@ -60,52 +54,24 @@ static int run_gpg_verify(const char *buf, unsigned long size, int verbose)\n>  \treturn ret;\n>  }\n>  \n> -static int verify_tag(const char *name, int verbose)\n> +int verify_tag_sha1(const unsigned char *sha1, int verbose)\n>  {\n>  \tenum object_type type;\n> -\tunsigned char sha1[20];\n>  \tchar *buf;\n>  \tunsigned long size;\n>  \tint ret;\n>  \n> -\tif (get_sha1(name, sha1))\n> -\t\treturn error(\"tag '%s' not found.\", name);\n> -\n>  \ttype = sha1_object_info(sha1, NULL);\n>  \tif (type != OBJ_TAG)\n> -\t\treturn error(\"%s: cannot verify a non-tag object of type %s.\",\n> -\t\t\t\tname, typename(type));\n> +\t\treturn error(\"Cannot verify a non-tag object of type %s.\",\n> +\t\t\t\ttypename(type));\n>  \n>  \tbuf = read_sha1_file(sha1, &type, &size);\n>  \tif (!buf)\n> -\t\treturn error(\"%s: unable to read file.\", name);\n> +\t\treturn error(\"Cnable to read file.\");\n>  \n>  \tret = run_gpg_verify(buf, size, verbose);\n>  \n>  \tfree(buf);\n>  \treturn ret;\n>  }\n> -\n> -int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n> -{\n> -\tint i = 1, verbose = 0, had_error = 0;\n> -\n> -\tgit_config(git_default_config, NULL);\n> -\n> -\tif (argc > 1 &&\n> -\t    (!strcmp(argv[i], \"-v\") || !strcmp(argv[i], \"--verbose\"))) {\n> -\t\tverbose = 1;\n> -\t\ti++;\n> -\t}\n> -\n> -\tif (argc <= i)\n> -\t\tusage(builtin_verify_tag_usage);\n> -\n> -\t/* sometimes the program was terminated because this signal\n> -\t * was received in the process of writing the gpg input: */\n> -\tsignal(SIGPIPE, SIG_IGN);\n> -\twhile (i < argc)\n> -\t\tif (verify_tag(argv[i++], verbose))\n> -\t\t\thad_error = 1;\n> -\treturn had_error;\n> -}\n\nI think pretty much all you did was deleting (and thereby you do not gain \nany copyright).\n\nExcept for one change: why on earth did you think it a good idea to \nsuppress telling the user the _name_ of the tag when an error occurs?\n\nI, for one, would find it way less than helpful to read\n\n\tCannot verify a non-tag object of type blob.\n\nthan to read\n\n\trefs/tags/dscho-key: cannot verify a non-tag object of type blob.\n\nBesides, I do not see where you warn that \"tag <name> not found.\"  Changes \nlike this one need to be justified (by saying in the commit message where \nthe warning is already issued, and not letting the reviewer/reader leave \nwondering).\n\nPlease, next time you submit a patch like this, do the -C -C yourself.  \nLetting all the reviewers do it looks lousy on the overall time balance \nsheet, and it may also lead to a potential reviewer preferring to do \nsomething else instead.\n\nNow, Junio already said that he is not (yet) convinced that this change \nshould be in Git proper, rather than a hook, so it is up to you to decide \nif you deem it important enough to try harder to convince people.\n\nI, for one, would think that it may be a good change: AFAIK only hard-core \ngits use hooks, everybody else avoids them.  So if we deem verifying \nsignatures important enough, we might want to have better support for it \nthan some example hooks.\n\nSo color me half-convinced.\n\nCiao,\nDscho\n"},{"id":"96656","messageId":"20081128000606.GB2759@euler","threadId":"16436","inReplyTo":"7vmyfpn10v.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC PATCH 0/4] Teach git fetch to verify signed tags automatically","fromName":"Deskin Miller","fromEmail":"deskinm@umich.edu","sentAt":"2008-11-28T00:09:11Z","receivedAt":"2008-11-28T00:09:11Z","isPatch":true,"sender":{"key":"deskinm@umich.edu","avatar":"https://gravatar.com/avatar/d340a0e612cdf0a79535c71863c0b4c535e9aba63b42032226ae903e638b64f9?d=mp&s=160"},"body":"On Sun, Nov 23, 2008 at 09:30:56PM -0800, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > Deskin Miller <deskinm@umich.edu> writes:\n> >\n> >> It struck me a while back when I fetched a new tagged release from git.git that\n> >> if I wanted to verify the tag's signature, I'd have to issue another command to\n> >> do so.  Shouldn't git be able to do that for me automatically, when it fetches\n> >> signed tags?  Now it does.  Also, 'git remote update' gets this for free.\n> >\n> > I think this should be done inside your own hook.  Not interested at all\n> > in a solution to touch builtin-fetch.c, unless if the patch is about\n> > adding a new hook so that people with other needs can use it as well.\n> \n> ... or a much stronger case can be made why this shouldn't be done in a\n> hook.\n> \n> I realize \"not interested at all\" was a bit too strong, so I am trying to\n> rephrase it here.  The cycle that begins with an RFC that leads to\n> discussion and review is about clarifying the rationale and design\n> incrementally, so please do not get offended by my no, and sorry for using\n> unnecessarily strong wording.\n> \n> What I meant was more like \"The justification as given in the message does\n> not interest me in the patch at all as it stands.  I do not understand why\n> this has to be done as a patch to git-fetch itself, not in a hook script,\n> or why doing it inside git-fetch is a better approach than doing it in a\n> hook (if there already is a hook mechanism to do this)\".\n\nLet's try this then:\n\n-----\nDespite core git's built-in support of cryptographic authentication and\nintegrity verification through the use of signed tags, git still fails\nto provide a good first line of defense against malicious entities\nhijacking repositories and disseminating arbitrary code, since git does\nnot try to verify signed tags at the time they are fetched.  If such a\ncompromise occurred, the prospect of even one individual who did not\nverify the newly-fetched tag prior to use gives this a large potential\nvalue to attackers, and represents a commensurate risk to the git-using\ncommunity.\n\nThis patch series mitigates this risk by trying to verify each signed\ntag when it is first fetched.  Since, however, not everyone is concerned\nwith the security of signed tags, this feature tries to be conservative\ninsofar as signatures with public keys which are missing from the user's\nkeyring do not cause anything to be said about the tag's validity;\nfurthermore, a configuration variable exists to disable these checks\nentirely, if desired.\n-----\n*the RFC patch series v1 does not include such a configuration variable.\n\nI appreciate that such verification could be accomplished by the\nas-yet-nonexistent post-fetch hook, and if that hook existed, I probably\nwould have done this in that hook.  With that said, I do feel like this\nfeature merits consideration for inclusion in the builtin fetch code.\nFirst, I very much agree with what Dscho said in his review of patch 1,\nthat hooks represent a rather more advanced feature of git than most\nusers are willing to investigate.\n\nSo the question, then, is whether this feature is important enough to\ninclude in core git.  I of course think that it is important enough.\nGiven that we have git-tag installed by default when one builds git,\nthere is a certain commitment to supporting the use of signed tags; and\nI see verifying them when first fetched as a logical continuation of\nthis support.  As such, a hook seems an unsuitable approach to provide\nsupport for this new use of signed tags.\n\nI'm happy to ask what suggestions you have for someone intending to\nimplement hooks around fetch; I've not looked at the code in this light,\nbut someone's got to do it sooner or later.\n\nDeskin Miller\n"},{"id":"96657","messageId":"20081128001825.GA29662@euler","threadId":"16436","inReplyTo":"alpine.DEB.1.00.0811241140280.30769@pacific.mpi-cbg.de","subject":"Re: [RFC PATCH 0/4] Teach git fetch to verify signed tags automatically","fromName":"Deskin Miller","fromEmail":"deskinm@umich.edu","sentAt":"2008-11-28T00:18:25Z","receivedAt":"2008-11-28T00:18:25Z","isPatch":true,"sender":{"key":"deskinm@umich.edu","avatar":"https://gravatar.com/avatar/d340a0e612cdf0a79535c71863c0b4c535e9aba63b42032226ae903e638b64f9?d=mp&s=160"},"body":"On Mon, Nov 24, 2008 at 11:41:27AM +0100, Johannes Schindelin wrote:\n> On Sun, 23 Nov 2008, Deskin Miller wrote:\n> \n> > -What to do if a tag is found to have a bad signature?\n> \n> Or even worse: if the public key was not found?  In dubio pro reo, they \n> say, but OTOH you asked to verify the signatures...\n\nI don't see how not finding the public key is `worse' than a bad\nsignature.  Compared to what the user learns currently when they run git\nfetch and receive new signed tags, the case of not having the required\npublic key leaves them in exactly the same state: the user does not know\nwhether the signature is valid or not.\n\nThe user didn't ask to verify, as I see it; rather, they asked git to\n*try* to verify.  If that fails in a way they don't expect, they're free\nto investigate further with git tag -v for situations like not having\nthe right public key.\n\nDeskin Miller \n"},{"id":"96658","messageId":"20081128001847.GB29662@euler","threadId":"16436","inReplyTo":"alpine.DEB.1.00.0811241147490.30769@pacific.mpi-cbg.de","subject":"Re: [RFC PATCH 1/4] Refactor builtin-verify-tag.c","fromName":"Deskin Miller","fromEmail":"deskinm@umich.edu","sentAt":"2008-11-28T00:18:47Z","receivedAt":"2008-11-28T00:18:47Z","isPatch":true,"sender":{"key":"deskinm@umich.edu","avatar":"https://gravatar.com/avatar/d340a0e612cdf0a79535c71863c0b4c535e9aba63b42032226ae903e638b64f9?d=mp&s=160"},"body":"On Mon, Nov 24, 2008 at 12:04:59PM +0100, Johannes Schindelin wrote:\n> Hi,\n> \n> On Sun, 23 Nov 2008, Deskin Miller wrote:\n> \n> > builtin-verify-tag.c didn't expose any of its functionality to be used\n> > internally.  Refactor some of it into new verify-tag.c and expose\n> > verify_tag_sha1 able to be called from elsewhere in git.\n> > \n> > Signed-off-by: Deskin Miller <deskinm@umich.edu>\n> > ---\n> >  Makefile             |    2 +\n> >  builtin-verify-tag.c |   61 ++-------------------------------------\n> >  verify-tag.c         |   77 ++++++++++++++++++++++++++++++++++++++++++++++++++\n> >  verify-tag.h         |   10 ++++++\n> >  4 files changed, 93 insertions(+), 57 deletions(-)\n> >  create mode 100644 verify-tag.c\n> >  create mode 100644 verify-tag.h\n> \n> I'll comment on the output of \"format-patch -n -C -C\" instead, as that \n> makes it much easier to see what you actually did:\n\nDidn't realise -C -C was the magic incantation; I'll remember it for the\nfuture.\n\n> >  Makefile                             |    2 +\n> >  builtin-verify-tag.c                 |   61 ++-------------------------------\n> >  builtin-verify-tag.c => verify-tag.c |   48 ++++-----------------------\n> >  verify-tag.h                         |   10 +++++\n> >  4 files changed, 23 insertions(+), 98 deletions(-)\n> >  copy builtin-verify-tag.c => verify-tag.c (56%)\n> >  create mode 100644 verify-tag.h\n> >\n> > [...] \n> > diff --git a/builtin-verify-tag.c b/verify-tag.c\n> > similarity index 56%\n> > copy from builtin-verify-tag.c\n> > copy to verify-tag.c\n> > index 729a159..c9be331 100644\n> > --- a/builtin-verify-tag.c\n> > +++ b/verify-tag.c\n> > @@ -1,18 +1,12 @@\n> >  /*\n> > - * Builtin \"git verify-tag\"\n> > + * Internals for \"git verify-tag\"\n> \n> Agree.\n> \n> >   *\n> > - * Copyright (c) 2007 Carlos Rica <jasampler@gmail.com>\n> > + * Copyright (c) 2008 Deskin Miller <deskinm@umich.edu>\n> \n> Disagree.\n> \n> Even if Carlos seemed to stop his work on Git entirely, which I find \n> disappointing, you are _not_ free to pretend his work is yours.  And given \n> this diff:\n> [...] \n> I think pretty much all you did was deleting (and thereby you do not gain \n> any copyright).\n\nI realised my mistake in altering the copyright information just after\nsending out these patches.  I think I'd written the header first in\nverify-tag.c before copying the code in; though I couldn't say what I\nthought I'd be writing that would end up protected by copyright.  At any\nrate, it was an honest mistake, and I apologise, Carlos, for my\nunintended plagarism; I'll be sure to restore the proper copyright\nnotice for any subsequent versions.\n\n> Except for one change: why on earth did you think it a good idea to \n> suppress telling the user the _name_ of the tag when an error occurs?\n> \n> I, for one, would find it way less than helpful to read\n> \n> \tCannot verify a non-tag object of type blob.\n> \n> than to read\n> \n> \trefs/tags/dscho-key: cannot verify a non-tag object of type blob.\n>\n> Besides, I do not see where you warn that \"tag <name> not found.\"  Changes \n> like this one need to be justified (by saying in the commit message where \n> the warning is already issued, and not letting the reviewer/reader leave \n> wondering).\n\nThe verify_tag_sha1 function is newly exposed to the rest of git, and\nhas a different signature from verify_tag, which could take a ref while\nverify_tag_sha1 takes a sha1.  verify_tag still includes both the checks\nyou refer to before calling verify_tag_sha1, so the error output is\nidentical in all cases before and after applying this patch.\n\nThe OBJ_TAG check, however, is duplicated so that internal git calls to\nverify_tag_sha1 can't pass in e.g. a blob sha1 which just happens to\ncontain the same contents as a signed tag.\n\nActually, I initially did not leave the OBJ_TAG check in verify_tag, but\nrelied on it checking the return value of verify_tag_sha1 to see if an\nerror occurred, and printing 'Failed to verify <name>' in that case, for\nprecisely the reason you point out, that the ref name is very useful in\nthis failure case.  However, I ultimately decided to duplicate the check\nso that the error output would match up exactly.\n \n> Please, next time you submit a patch like this, do the -C -C yourself.  \n> Letting all the reviewers do it looks lousy on the overall time balance \n> sheet, and it may also lead to a potential reviewer preferring to do \n> something else instead.\n\nWill do; thanks for reviewing in spite of my shortcomings.\n\n> Now, Junio already said that he is not (yet) convinced that this change \n> should be in Git proper, rather than a hook, so it is up to you to decide \n> if you deem it important enough to try harder to convince people.\n> \n> I, for one, would think that it may be a good change: AFAIK only hard-core \n> gits use hooks, everybody else avoids them.  So if we deem verifying \n> signatures important enough, we might want to have better support for it \n> than some example hooks.\n> \n> So color me half-convinced.\n> \n> Ciao,\n> Dscho\n\nDeskin Miller \n"},{"id":"96659","messageId":"20081128001901.GC29662@euler","threadId":"16436","inReplyTo":"alpine.DEB.1.00.0811241143410.30769@pacific.mpi-cbg.de","subject":"Re: [RFC PATCH 4/4] Make git fetch verify signed tags","fromName":"Deskin Miller","fromEmail":"deskinm@umich.edu","sentAt":"2008-11-28T00:19:01Z","receivedAt":"2008-11-28T00:19:01Z","isPatch":true,"sender":{"key":"deskinm@umich.edu","avatar":"https://gravatar.com/avatar/d340a0e612cdf0a79535c71863c0b4c535e9aba63b42032226ae903e638b64f9?d=mp&s=160"},"body":"On Mon, Nov 24, 2008 at 11:44:40AM +0100, Johannes Schindelin wrote:\n> Hi,\n> \n> On Sun, 23 Nov 2008, Deskin Miller wrote:\n> \n> > When git fetch downloads signed tag objects, make it verify them right \n> > then.  This extends the output summary of fetch to include \"(good \n> > signature)\" for valid tags and \"(BAD SIGNATURE)\" for invalid tags.  If \n> > the user does not have the correct key in the gpg keyring, gpg returns \n> > 2, verify_tag_sha1 returns -2 and nothing additional is output about the \n> > tag's validity.\n> \n> This must be turned off by default, IMO.  You cannot expect each and every \n> developer to have gpg _and_ all those public keys installed.\n\nAdding a configuration variable to control this makes sense, and is on\nmy TODO list for v2 (core.autoVerifyTags?).  However, I don't see a\ncompelling reason to make it off by default, as if gpg isn't found, or a\nparticular public key isn't in the keyring, the output is no different\nfrom what fetch prints now.\n\nDeskin Miller\n"},{"id":"96663","messageId":"alpine.DEB.1.00.0811280213140.30769@pacific.mpi-cbg.de","threadId":"16436","inReplyTo":"20081128000606.GB2759@euler","subject":"Re: [RFC PATCH 0/4] Teach git fetch to verify signed tags automatically","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-11-28T01:18:55Z","receivedAt":"2008-11-28T01:18:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 27 Nov 2008, Deskin Miller wrote:\n\n> This patch series mitigates this risk by trying to verify each signed \n> tag when it is first fetched.  Since, however, not everyone is concerned \n> with the security of signed tags, this feature tries to be conservative \n> insofar as signatures with public keys which are missing from the user's \n> keyring do not cause anything to be said about the tag's validity;\n\nNow, in the context of security, this is not conservative.  Conservative \nwould be to fail as soon as a signature could not be verified, be it that \nthere is no key to match against, or that the signature is corrupt.\n\nYour notion to fail silently if the necessary keys were not found makes \nyour patch series rather useless, no?\n\nAfter all, the whole idea is to let Git check if every signature is \ncorrect, and when Git does not fail, rely on them being valid.\n\nSo I think that the _only_ thing that would make sense is to fail _unless_ \nall the signatures were verified to be correct.\n\n_That_ is why I want this feature to be off by default.\n\nCiao,\nDscho\n"},{"id":"96666","messageId":"7v63m8aamz.fsf@gitster.siamese.dyndns.org","threadId":"16436","inReplyTo":"20081128001825.GA29662@euler","subject":"Re: [RFC PATCH 0/4] Teach git fetch to verify signed tags automatically","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-11-28T01:43:00Z","receivedAt":"2008-11-28T01:43:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Deskin Miller <deskinm@umich.edu> writes:\n\n> The user didn't ask to verify, as I see it; rather, they asked git to\n> *try* to verify.\n\nIf that is your argument, I really do not see any point in your patch.\nThey asked git to fetch, and did not say anything about trying anything\nelse.\n"}]}