{"thread":{"id":"9162","subject":"[PATCH] Make verify-tag a builtin.","startedAt":"2007-07-23T00:20:12Z","lastAt":"2007-07-23T22:32:36Z","messageCount":6,"participants":["Carlos Rica","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"48248","messageId":"46A3F43C.3030101@gmail.com","threadId":"9162","inReplyTo":null,"subject":"[PATCH] Make verify-tag a builtin.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2007-07-23T00:20:12Z","receivedAt":"2007-07-23T00:20:12Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"This replaces \"git-verify-tag.sh\" with \"builtin-verify-tag.c\".\n\nSignal SIGPIPE is ignored because the program sometimes was\nstopped because that signal when writing the input for gpg.\n\nSigned-off-by: Carlos Rica \n---\n\n Comments are welcomed.\n\n Makefile | 3 +-\n builtin-verify-tag.c | 113 ++++++++++++++++++++\n builtin.h | 1 +\n .../examples/git-verify-tag.sh | 0\n git.c | 1 +\n 5 files changed, 117 insertions(+), 1 deletions(-)\n create mode 100644 builtin-verify-tag.c\n rename git-verify-tag.sh => contrib/examples/git-verify-tag.sh (100%)\n\ndiff --git a/Makefile b/Makefile\nindex 73b487f..c6ed79f 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -206,7 +206,7 @@ SCRIPT_SH = \\\n \tgit-pull.sh git-rebase.sh git-rebase--interactive.sh \\\n \tgit-repack.sh git-request-pull.sh git-reset.sh \\\n \tgit-sh-setup.sh \\\n-\tgit-tag.sh git-verify-tag.sh \\\n+\tgit-tag.sh \\\n \tgit-am.sh \\\n \tgit-merge.sh git-merge-stupid.sh git-merge-octopus.sh \\\n \tgit-merge-resolve.sh git-merge-ours.sh \\\n@@ -367,6 +367,7 @@ BUILTIN_OBJS = \\\n \tbuiltin-update-ref.o \\\n \tbuiltin-upload-archive.o \\\n \tbuiltin-verify-pack.o \\\n+\tbuiltin-verify-tag.o \\\n \tbuiltin-write-tree.o \\\n \tbuiltin-show-ref.o \\\n \tbuiltin-pack-refs.o\ndiff --git a/builtin-verify-tag.c b/builtin-verify-tag.c\nnew file mode 100644\nindex 0000000..586f28a\n--- /dev/null\n+++ b/builtin-verify-tag.c\n@@ -0,0 +1,113 @@\n+/*\n+ * Builtin \"git verify-tag\"\n+ *\n+ * Copyright (c) 2007 Carlos Rica \n+ *\n+ * Based on git-verify-tag.sh\n+ */\n+#include \"cache.h\"\n+#include \"builtin.h\"\n+#include \"tag.h\"\n+#include \"run-command.h\"\n+#include \n+\n+static const char builtin_verify_tag_usage[] =\n+\t\t\"git-verify-tag [-v|--verbose] ...\";\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, *eol;\n+\tsize_t len;\n+\tint fd, ret;\n+\n+\tpath = xstrdup(git_path(\"VTAG_TMP\"));\n+\tfd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0600);\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\")) {\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+\tgpg.out = 1;\n+\targs_gpg[2] = path;\n+\tif (start_command(&gpg))\n+\t\treturn error(\"could not run gpg.\");\n+\n+\twrite_in_full(gpg.in, buf, len);\n+\tclose(gpg.in);\n+\tgpg.close_in = 0;\n+\tret = finish_command(&gpg);\n+\n+\tremove(path);\n+\tfree(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+\t\treturn error(\"tag '%s' not found.\", name);\n+\n+\ttype = sha1_object_info(sha1, NULL);\n+\tif (type <= OBJ_NONE)\n+\t\treturn error(\"%s: bad object type.\", name);\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+\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+\treturn ret;\n+}\n+\n+int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n+{\n+\tint i, verbose = 0, had_error = 0;\n+\n+\tgit_config(git_default_config);\n+\n+\tif (argc == 1)\n+\t\tusage(builtin_verify_tag_usage);\n+\n+\ti = 1;\n+\tif (!strcmp(argv[i], \"-v\") || !strcmp(argv[i], \"--verbose\")) {\n+\t\tverbose = 1;\n+\t\ti++;\n+\t}\n+\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+}\ndiff --git a/builtin.h b/builtin.h\nindex 4cc228d..cb860a0 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -76,6 +76,7 @@ extern int cmd_update_index(int argc, const char **argv, const char *prefix);\n extern int cmd_update_ref(int argc, const char **argv, const char *prefix);\n extern int cmd_upload_archive(int argc, const char **argv, const char *prefix);\n extern int cmd_upload_tar(int argc, const char **argv, const char *prefix);\n+extern int cmd_verify_tag(int argc, const char **argv, const char *prefix);\n extern int cmd_version(int argc, const char **argv, const char *prefix);\n extern int cmd_whatchanged(int argc, const char **argv, const char *prefix);\n extern int cmd_write_tree(int argc, const char **argv, const char *prefix);\ndiff --git a/git-verify-tag.sh b/contrib/examples/git-verify-tag.sh\nsimilarity index 100%\nrename from git-verify-tag.sh\nrename to contrib/examples/git-verify-tag.sh\ndiff --git a/git.c b/git.c\nindex a647f9c..1dfe120 100644\n--- a/git.c\n+++ b/git.c\n@@ -368,6 +368,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"update-index\", cmd_update_index, RUN_SETUP },\n \t\t{ \"update-ref\", cmd_update_ref, RUN_SETUP },\n \t\t{ \"upload-archive\", cmd_upload_archive },\n+\t\t{ \"verify-tag\", cmd_verify_tag, RUN_SETUP },\n \t\t{ \"version\", cmd_version },\n \t\t{ \"whatchanged\", cmd_whatchanged, RUN_SETUP | USE_PAGER },\n \t\t{ \"write-tree\", cmd_write_tree, RUN_SETUP },\n-- \n1.5.0\n"},{"id":"48251","messageId":"Pine.LNX.4.64.0707230141080.14781@racer.site","threadId":"9162","inReplyTo":"46A3F43C.3030101@gmail.com","subject":"Re: [PATCH] Make verify-tag a builtin.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-23T00:52:39Z","receivedAt":"2007-07-23T00:52:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 23 Jul 2007, Carlos Rica wrote:\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, *eol;\n> +\tsize_t len;\n> +\tint fd, ret;\n> +\n> +\tpath = xstrdup(git_path(\"VTAG_TMP\"));\n> +\tfd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0600);\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\nI just tested something like \"gpg --verify - - < \" and it \nworked...\n\n> +\t/* find the length without signature */\n> +\tlen = 0;\n> +\twhile (len < size && prefixcmp(buf + len, PGP_SIGNATURE \"\\n\")) {\n> +\t\teol = memchr(buf + len, '\\n', size - len);\n> +\t\tlen += eol ? eol - (buf + len) + 1 : size - len;\n> +\t}\n\nHow about this instead:\n\n\tconst char *sig = strstr(buf, \"\\n\" PGP_SIGNATURE \"\\n\"));\n\tif (!sig)\n\t\tsig = buf + len;\n\nHmm?\n\nBut of course only if gpg does not grok \"git cat-file tag |\ngpg --verify - -\".\n\n> +\tremove(path);\n\nI think we use unlink() consistently, since we are sure it is no \ndirectory.\n\n> +\tif (type <= OBJ_NONE)\n> +\t\treturn error(\"%s: bad object type.\", name);\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\nThese two can be unified into one, right?\n\n> +int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n> +{\n> +\tint i, verbose = 0, had_error = 0;\n> +\n> +\tgit_config(git_default_config);\n> +\n> +\tif (argc == 1)\n> +\t\tusage(builtin_verify_tag_usage);\n> +\n> +\ti = 1;\n\nYou can initialise i above.\n\n> +\tif (!strcmp(argv[i], \"-v\") || !strcmp(argv[i], \"--verbose\")) {\n> +\t\tverbose = 1;\n> +\t\ti++;\n> +\t}\n> +\n> +\tsignal(SIGPIPE, SIG_IGN);\n> +\twhile (i < argc)\n> +\t\tif (verify_tag(argv[i++], verbose))\n\nHeh. I thought that you only made this a separate function to help \nbuilding it into builtin-tag...\n\nThat was quick!\n\nCiao,\nDscho\n"},{"id":"48359","messageId":"1b46aba20707231421k54ad2796i5c2f21cf11b4bea5@mail.gmail.com","threadId":"9162","inReplyTo":"Pine.LNX.4.64.0707230141080.14781@racer.site","subject":"Re: [PATCH] Make verify-tag a builtin.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2007-07-23T21:21:03Z","receivedAt":"2007-07-23T21:21:03Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"2007/7/23, Johannes Schindelin :\n> On Mon, 23 Jul 2007, Carlos Rica wrote:\n> > +static int run_gpg_verify(const char *buf, unsigned long size, int verbose)\n> > +{\n> > + struct child_process gpg;\n> > + const char *args_gpg[] = {\"gpg\", \"--verify\", \"FILE\", \"-\", NULL};\n> > + char *path, *eol;\n> > + size_t len;\n> > + int fd, ret;\n> > +\n> > + path = xstrdup(git_path(\"VTAG_TMP\"));\n> > + fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0600);\n> > + if (fd < 0)\n> > + return error(\"could not create temporary file '%s': %s\",\n> > + path, strerror(errno));\n> > + if (write_in_full(fd, buf, size) < 0)\n> > + return error(\"failed writing temporary file '%s': %s\",\n> > + path, strerror(errno));\n> > + close(fd);\n>\n> I just tested something like \"gpg --verify - - < \" and it\n> worked...\n\nI couldn't verify tags using \"gpg --verify - - \n\nI even tested to insert the header myself after creating a signed file\nas git does:\n cp file otherfile\n gpg -bsa otherfile\n ...\n cat otherfile.asc >>otherfile\n ( echo -e \"-----BEGIN PGP SIGNED MESSAGE-----\\nHash: SHA1\\n\" ; cat\notherfile ) \\\n | gpg --verify\n\nNote that sending them to \"diff file.asc -\" also shows different signatures,\nbut anyway it also says \"gpg: BAD signature from ...\", the same message as in:\n gpg --verify - - 2007/7/23, Johannes Schindelin :\n> > On Mon, 23 Jul 2007, Carlos Rica wrote:\n> > > +static int run_gpg_verify(const char *buf, unsigned long size, int\n> > verbose)\n> > > +{\n> > > + struct child_process gpg;\n> > > + const char *args_gpg[] = {\"gpg\", \"--verify\", \"FILE\", \"-\", NULL};\n> > > + char *path, *eol;\n> > > + size_t len;\n> > > + int fd, ret;\n> > > +\n> > > + path = xstrdup(git_path(\"VTAG_TMP\"));\n> > > + fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0600);\n> > > + if (fd < 0)\n> > > + return error(\"could not create temporary file '%s': %s\",\n> > > + path, strerror(errno));\n> > > + if (write_in_full(fd, buf, size) < 0)\n> > > + return error(\"failed writing temporary file '%s': %s\",\n> > > + path, strerror(errno));\n> > > + close(fd);\n> > \n> > I just tested something like \"gpg --verify - - < \" and it\n> > worked...\n> \n> I couldn't verify tags using \"gpg --verify - - pointed in your response.\n\nAs discussed on IRC, my test did not use a detached signature, while tags \n_do_ use a detached signature.\n\nOh, well. I retract my suggestion. But I slightly disagree that having \nto write a temporary file is good for security, as the gpg man page \nsuggests:\n\n--verify [[sigfile] [signed-files]]\n [...] To read the signed stuff from stdin, use - as the second \n\tfilename. For security reasons a detached signature cannot read \n\tthe signed material from stdin without denoting it in the above \n\tway.\n\nSo we have to live, or die, with the temporary file.\n\nJust one request: please use lock_file, to prevent multiple instances from \ngetting confused... Even if that cannot be 100% secure: if you write to \nthe file, and only then start gpg, there is still the odd chance for an \nattacker. Even if you created the file with 0600 (very wise, BTW).\n\nCiao,\nDscho\n"},{"id":"48365","messageId":"1b46aba20707231531i4e05858foef3ee81479e559de@mail.gmail.com","threadId":"9162","inReplyTo":"Pine.LNX.4.64.0707232242430.14781@racer.site","subject":"Re: [PATCH] Make verify-tag a builtin.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2007-07-23T22:31:31Z","receivedAt":"2007-07-23T22:31:31Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"2007/7/23, Johannes Schindelin :\n> Just one request: please use lock_file, to prevent multiple instances from\n> getting confused... Even if that cannot be 100% secure: if you write to\n> the file, and only then start gpg, there is still the odd chance for an\n> attacker. Even if you created the file with 0600 (very wise, BTW).\n\nWhy not using also a random name for the temporary file?\nSomething like \"VTAG_TMP30946\".\n"},{"id":"48367","messageId":"Pine.LNX.4.64.0707232332000.14781@racer.site","threadId":"9162","inReplyTo":"1b46aba20707231531i4e05858foef3ee81479e559de@mail.gmail.com","subject":"Re: [PATCH] Make verify-tag a builtin.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-23T22:32:36Z","receivedAt":"2007-07-23T22:32:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 24 Jul 2007, Carlos Rica wrote:\n\n> 2007/7/23, Johannes Schindelin :\n> > Just one request: please use lock_file, to prevent multiple instances from\n> > getting confused... Even if that cannot be 100% secure: if you write to\n> > the file, and only then start gpg, there is still the odd chance for an\n> > attacker. Even if you created the file with 0600 (very wise, BTW).\n> \n> Why not using also a random name for the temporary file?\n> Something like \"VTAG_TMP30946\".\n\nYes, makes sense. One call to mkstemp() away.\n\nCiao,\nDscho\n"}]}