{"thread":{"id":"32902","subject":"[BUG] Veryfing signatures in git log fails when language is not english","startedAt":"2013-02-14T00:18:37Z","lastAt":"2013-02-15T16:08:54Z","messageCount":14,"participants":["XANi","Michael J Gruber","Mariusz Gronczewski","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"209503","messageId":"20130214011837.04880b3e@hydra.devrandom.pl","threadId":"32902","inReplyTo":null,"subject":"[BUG] Veryfing signatures in git log fails when language is not english","fromName":"XANi","fromEmail":"xani666@gmail.com","sentAt":"2013-02-14T00:18:37Z","receivedAt":"2013-02-14T00:18:37Z","isPatch":false,"sender":{"key":"xani666@gmail.com","avatar":"https://gravatar.com/avatar/078387db945420fb3089b7094b28d7cca5148dbac5494b40432eee39e5620e53?d=mp&s=160"},"body":"Hi,\n\nany functionality that depends on exact exit msg of program\n can potentially fail because of that\nᛯ export |grep LANG\ndeclare -x LANG=\"pl_PL.UTF-8\"\n\nᛯ ~/src/os/git/git log --format=\"%G? %h\" |head -2 \n 0d19377\n 5b9d7f8\n\nᛯ unset LANG\nᛯ ~/src/os/git/git log --format=\"%G? %h\" |head -2\nG 0d19377\nG 5b9d7f8\n\ntested against maint (d32805d) and master (5bf72ed)\n\nmaybe git should set up some output-changing variables before calling\nexternal programs? I think setting LC_ALL=C should be enougth.\n\n-- \nMariusz Gronczewski (XANi) \nGnuPG: 0xEA8ACE64\n\n\n"},{"id":"209512","messageId":"511CC288.30607@drmicha.warpmail.net","threadId":"32902","inReplyTo":"20130214011837.04880b3e@hydra.devrandom.pl","subject":"Re: [BUG] Veryfing signatures in git log fails when language is not english","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2013-02-14T10:55:04Z","receivedAt":"2013-02-14T10:55:04Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"XANi venit, vidit, dixit 14.02.2013 01:18:\n> Hi,\n> \n> any functionality that depends on exact exit msg of program\n> can potentially fail because of that\n> ᛯ export |grep LANG\n> declare -x LANG=\"pl_PL.UTF-8\"\n> \n> ᛯ ~/src/os/git/git log --format=\"%G? %h\" |head -2 \n> 0d19377\n> 5b9d7f8\n> \n> ᛯ unset LANG\n> ᛯ ~/src/os/git/git log --format=\"%G? %h\" |head -2\n> G 0d19377\n> G 5b9d7f8\n> \n> tested against maint (d32805d) and master (5bf72ed)\n> \n> maybe git should set up some output-changing variables before calling\n> external programs? I think setting LC_ALL=C should be enougth.\n> \n\nThere are really multiple problems here:\n\n1. git calls gpg without setting LANG but expects output in LANG=C\n\n2. git looks at the textual output from gpg to check the validity.\n\n3. In fact, it does so only for %G and the display of signed merge\ncommits, in all other cases it checks the return code only.\n\ngpg is not supposed to be used like that.\n\nSince the callers of verify_signed_buffer do that craziness there is\nsome refactoring to be done.\n\nA false hotfix would be to set LANG=C when calling gpg from git, but\nthat wouldn't solve the real problem. Besides, we do want LANG dependent\noutput for the user.\n\nI'll have a closer look.\n\nBTW: Thanks for the clear report :)\n\nMichael\n"},{"id":"209513","messageId":"CAJ9Ak2pLqUesOWKNtjwG7P_BW7-EqJP4Kaj0FF_+72Wn=oGoew@mail.gmail.com","threadId":"32902","inReplyTo":"511CC288.30607@drmicha.warpmail.net","subject":"Re: [BUG] Veryfing signatures in git log fails when language is not english","fromName":"Mariusz Gronczewski","fromEmail":"xani666@gmail.com","sentAt":"2013-02-14T12:42:41Z","receivedAt":"2013-02-14T12:42:41Z","isPatch":false,"sender":{"key":"xani666@gmail.com","avatar":"https://gravatar.com/avatar/078387db945420fb3089b7094b28d7cca5148dbac5494b40432eee39e5620e53?d=mp&s=160"},"body":"2013/2/14 Michael J Gruber :\n> XANi venit, vidit, dixit 14.02.2013 01:18:\n>> Hi,\n>>\n>> any functionality that depends on exact exit msg of program\n>> can potentially fail because of that\n>> ᛯ export |grep LANG\n>> declare -x LANG=\"pl_PL.UTF-8\"\n>>\n>> ᛯ ~/src/os/git/git log --format=\"%G? %h\" |head -2\n>> 0d19377\n>> 5b9d7f8\n>>\n>> ᛯ unset LANG\n>> ᛯ ~/src/os/git/git log --format=\"%G? %h\" |head -2\n>> G 0d19377\n>> G 5b9d7f8\n>>\n>> tested against maint (d32805d) and master (5bf72ed)\n>>\n>> maybe git should set up some output-changing variables before calling\n>> external programs? I think setting LC_ALL=C should be enougth.\n>>\n>\n> There are really multiple problems here:\n>\n> 1. git calls gpg without setting LANG but expects output in LANG=C\n>\n> 2. git looks at the textual output from gpg to check the validity.\n>\n> 3. In fact, it does so only for %G and the display of signed merge\n> commits, in all other cases it checks the return code only.\n>\n> gpg is not supposed to be used like that.\n>\n> Since the callers of verify_signed_buffer do that craziness there is\n> some refactoring to be done.\n>\n> A false hotfix would be to set LANG=C when calling gpg from git, but\n> that wouldn't solve the real problem. Besides, we do want LANG dependent\n> output for the user.\n>\n> I'll have a closer look.\n>\n> BTW: Thanks for the clear report :)\n>\n> Michael\n\nWhat is really missing is an ability to display used key ID without\nhammering git log output with regexps, it would be much easier to\nvalidate incoming commits if there was format option to just display\nkey ID instead of signer name. %GS isn't really good solution for that\nbecause it will show only one of email addresses used in the key and\nscript checking signatures would have to always pick \"right\" one.\n\n-- \nMariusz Gronczewski (XANi) \nGnuPG: 0xEA8ACE64\n"},{"id":"209519","messageId":"cover.1360857415.git.git@drmicha.warpmail.net","threadId":"32902","inReplyTo":"CAJ9Ak2pLqUesOWKNtjwG7P_BW7-EqJP4Kaj0FF_+72Wn=oGoew@mail.gmail.com","subject":"[PATCH 0/5] gpg_interface: use the status","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2013-02-14T16:04:41Z","receivedAt":"2013-02-14T16:04:41Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, we look at the user facing output of gpg, which is LANG dependent as\nwell as insecure.\n\nAfter this series, we look at the status output (--status-fd) which is designed\nfor that purpose. As an additional benefit, we can read off the key which was used\nfor the signature, which is important for assigning trust.\n\nAll existing tests pass with this.\n\nBTW: git branch --set-upstream-to= coredumps when on a detached head.\n\nMichael J Gruber (5):\n gpg-interface: check good signature in a reliable way\n log-tree: rely upon the check in the gpg_interface\n gpg_interface: allow to request status return\n pretty: parse the gpg status lines rather than the output\n pretty: make %GK output the signing key for signed commits\n\n Documentation/pretty-formats.txt | 1 +\n builtin/fmt-merge-msg.c | 2 +-\n builtin/verify-tag.c | 2 +-\n gpg-interface.c | 18 +++++++++++++++---\n gpg-interface.h | 2 +-\n log-tree.c | 27 ++++++++++++---------------\n pretty.c | 19 +++++++++++++++----\n 7 files changed, 46 insertions(+), 25 deletions(-)\n\n-- \n1.8.1.3.797.ge0260c7\n"},{"id":"209523","messageId":"e0f3c48e474c64e33c67c772468bbf7bba0169af.1360857415.git.git@drmicha.warpmail.net","threadId":"32902","inReplyTo":"CAJ9Ak2pLqUesOWKNtjwG7P_BW7-EqJP4Kaj0FF_+72Wn=oGoew@mail.gmail.com","subject":"[PATCH 1/5] gpg-interface: check good signature in a reliable way","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2013-02-14T16:04:42Z","receivedAt":"2013-02-14T16:04:42Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, verify_signed_buffer() only checks the return code of gpg,\nand some callers implement additional unreliable checks for \"Good\nsignature\" in the gpg output meant for the user.\n\nUse the status output instead and parse for a line beinning with\n\"[GNUPG:] GOODSIG \". This is the only reliable way of checking for a\ngood gpg signature.\n\nIf needed we can change this easily to \"[GNUPG:] VALIDSIG \" if we want\nto take into account the trust model.\n\nSigned-off-by: Michael J Gruber \n---\n gpg-interface.c | 13 +++++++++++--\n 1 file changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 4559033..c582b2e 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -96,15 +96,17 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n /*\n * Run \"gpg\" to see if the payload matches the detached signature.\n * gpg_output, when set, receives the diagnostic output from GPG.\n+ * gpg_status, when set, receives the status output from GPG.\n */\n int verify_signed_buffer(const char *payload, size_t payload_size,\n \t\t\t const char *signature, size_t signature_size,\n \t\t\t struct strbuf *gpg_output)\n {\n \tstruct child_process gpg;\n-\tconst char *args_gpg[] = {NULL, \"--verify\", \"FILE\", \"-\", NULL};\n+\tconst char *args_gpg[] = {NULL, \"--status-fd=1\", \"--verify\", \"FILE\", \"-\", NULL};\n \tchar path[PATH_MAX];\n \tint fd, ret;\n+\tstruct strbuf buf = STRBUF_INIT;\n \n \targs_gpg[0] = gpg_program;\n \tfd = git_mkstemp(path, PATH_MAX, \".git_vtag_tmpXXXXXX\");\n@@ -119,9 +121,10 @@ int verify_signed_buffer(const char *payload, size_t payload_size,\n \tmemset(&gpg, 0, sizeof(gpg));\n \tgpg.argv = args_gpg;\n \tgpg.in = -1;\n+\tgpg.out = -1;\n \tif (gpg_output)\n \t\tgpg.err = -1;\n-\targs_gpg[2] = path;\n+\targs_gpg[3] = path;\n \tif (start_command(&gpg)) {\n \t\tunlink(path);\n \t\treturn error(_(\"could not run gpg.\"));\n@@ -134,9 +137,15 @@ int verify_signed_buffer(const char *payload, size_t payload_size,\n \t\tstrbuf_read(gpg_output, gpg.err, 0);\n \t\tclose(gpg.err);\n \t}\n+\tstrbuf_read(&buf, gpg.out, 0);\n+\tclose(gpg.out);\n+\n \tret = finish_command(&gpg);\n \n \tunlink_or_warn(path);\n \n+\tret |= !strstr(buf.buf, \"\\n[GNUPG:] GOODSIG \");\n+\tstrbuf_release(&buf);\n+\n \treturn ret;\n }\n-- \n1.8.1.3.797.ge0260c7\n"},{"id":"209522","messageId":"a266fc66b482fa63dd2dd4db4862a073a0c83296.1360857415.git.git@drmicha.warpmail.net","threadId":"32902","inReplyTo":"CAJ9Ak2pLqUesOWKNtjwG7P_BW7-EqJP4Kaj0FF_+72Wn=oGoew@mail.gmail.com","subject":"[PATCH 2/5] log-tree: rely upon the check in the gpg_interface","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2013-02-14T16:04:43Z","receivedAt":"2013-02-14T16:04:43Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"It's just so much betterer.\n\nSigned-off-by: Michael J Gruber \n---\n log-tree.c | 25 +++++++++++--------------\n 1 file changed, 11 insertions(+), 14 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 5dc45c4..912fe08 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -508,20 +508,17 @@ static void show_one_mergetag(struct rev_info *opt,\n \tgpg_message_offset = verify_message.len;\n \n \tpayload_size = parse_signature(extra->value, extra->len);\n-\tif ((extra->len <= payload_size) ||\n-\t (verify_signed_buffer(extra->value, payload_size,\n-\t\t\t\t extra->value + payload_size,\n-\t\t\t\t extra->len - payload_size,\n-\t\t\t\t &verify_message) &&\n-\t verify_message.len <= gpg_message_offset)) {\n-\t\tstrbuf_addstr(&verify_message, \"No signature\\n\");\n-\t\tstatus = -1;\n-\t}\n-\telse if (strstr(verify_message.buf + gpg_message_offset,\n-\t\t\t\": Good signature from \"))\n-\t\tstatus = 0;\n-\telse\n-\t\tstatus = -1;\n+\tstatus = -1;\n+\tif (extra->len > payload_size)\n+\t\tif (verify_signed_buffer(extra->value, payload_size,\n+\t\t\t\t\t extra->value + payload_size,\n+\t\t\t\t\t extra->len - payload_size,\n+\t\t\t\t\t &verify_message)) {\n+\t\t\tif (verify_message.len <= gpg_message_offset)\n+\t\t\t\tstrbuf_addstr(&verify_message, \"No signature\\n\");\n+\t\t\telse\n+\t\t\t\tstatus = 0;\n+\t\t}\n \n \tshow_sig_lines(opt, status, verify_message.buf);\n \tstrbuf_release(&verify_message);\n-- \n1.8.1.3.797.ge0260c7\n"},{"id":"209521","messageId":"1d89315f4b9ef179cb7944582c22948e05e78f33.1360857415.git.git@drmicha.warpmail.net","threadId":"32902","inReplyTo":"CAJ9Ak2pLqUesOWKNtjwG7P_BW7-EqJP4Kaj0FF_+72Wn=oGoew@mail.gmail.com","subject":"[PATCH 3/5] gpg_interface: allow to request status return","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2013-02-14T16:04:44Z","receivedAt":"2013-02-14T16:04:44Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, verify_signed_buffer() returns the user facing output only.\n\nAllow callers to request the status output also.\n\nSigned-off-by: Michael J Gruber \n---\n builtin/fmt-merge-msg.c | 2 +-\n builtin/verify-tag.c | 2 +-\n gpg-interface.c | 11 +++++++----\n gpg-interface.h | 2 +-\n log-tree.c | 4 ++--\n pretty.c | 2 +-\n 6 files changed, 13 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex b49612f..265a925 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -492,7 +492,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \n \t\tif (size == len)\n \t\t\t; /* merely annotated */\n-\t\telse if (verify_signed_buffer(buf, len, buf + len, size - len, &sig)) {\n+\t\telse if (verify_signed_buffer(buf, len, buf + len, size - len, &sig, NULL)) {\n \t\t\tif (!sig.len)\n \t\t\t\tstrbuf_addstr(&sig, \"gpg verification failed.\\n\");\n \t\t}\ndiff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\nindex a8eee88..9cdf332 100644\n--- a/builtin/verify-tag.c\n+++ b/builtin/verify-tag.c\n@@ -29,7 +29,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, int verbose)\n \tif (size == len)\n \t\treturn error(\"no signature found\");\n \n-\treturn verify_signed_buffer(buf, len, buf + len, size - len, NULL);\n+\treturn verify_signed_buffer(buf, len, buf + len, size - len, NULL, NULL);\n }\n \n static int verify_tag(const char *name, int verbose)\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex c582b2e..8b0e874 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -100,13 +100,14 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n */\n int verify_signed_buffer(const char *payload, size_t payload_size,\n \t\t\t const char *signature, size_t signature_size,\n-\t\t\t struct strbuf *gpg_output)\n+\t\t\t struct strbuf *gpg_output, struct strbuf *gpg_status)\n {\n \tstruct child_process gpg;\n \tconst char *args_gpg[] = {NULL, \"--status-fd=1\", \"--verify\", \"FILE\", \"-\", NULL};\n \tchar path[PATH_MAX];\n \tint fd, ret;\n \tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strbuf *pbuf = &buf;\n \n \targs_gpg[0] = gpg_program;\n \tfd = git_mkstemp(path, PATH_MAX, \".git_vtag_tmpXXXXXX\");\n@@ -137,15 +138,17 @@ int verify_signed_buffer(const char *payload, size_t payload_size,\n \t\tstrbuf_read(gpg_output, gpg.err, 0);\n \t\tclose(gpg.err);\n \t}\n-\tstrbuf_read(&buf, gpg.out, 0);\n+\tif (gpg_status)\n+\t\tpbuf = gpg_status;\n+\tstrbuf_read(pbuf, gpg.out, 0);\n \tclose(gpg.out);\n \n \tret = finish_command(&gpg);\n \n \tunlink_or_warn(path);\n \n-\tret |= !strstr(buf.buf, \"\\n[GNUPG:] GOODSIG \");\n-\tstrbuf_release(&buf);\n+\tret |= !strstr(pbuf->buf, \"\\n[GNUPG:] GOODSIG \");\n+\tstrbuf_release(&buf); /* no matter it was used or not */\n \n \treturn ret;\n }\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex b9c3608..cf99021 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -2,7 +2,7 @@\n #define GPG_INTERFACE_H\n \n extern int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key);\n-extern int verify_signed_buffer(const char *payload, size_t payload_size, const char *signature, size_t signature_size, struct strbuf *gpg_output);\n+extern int verify_signed_buffer(const char *payload, size_t payload_size, const char *signature, size_t signature_size, struct strbuf *gpg_output, struct strbuf *gpg_status);\n extern int git_gpg_config(const char *, const char *, void *);\n extern void set_signing_key(const char *);\n extern const char *get_signing_key(void);\ndiff --git a/log-tree.c b/log-tree.c\nindex 912fe08..3d88823 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -444,7 +444,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit)\n \n \tstatus = verify_signed_buffer(payload.buf, payload.len,\n \t\t\t\t signature.buf, signature.len,\n-\t\t\t\t &gpg_output);\n+\t\t\t\t &gpg_output, NULL);\n \tif (status && !gpg_output.len)\n \t\tstrbuf_addstr(&gpg_output, \"No signature\\n\");\n \n@@ -513,7 +513,7 @@ static void show_one_mergetag(struct rev_info *opt,\n \t\tif (verify_signed_buffer(extra->value, payload_size,\n \t\t\t\t\t extra->value + payload_size,\n \t\t\t\t\t extra->len - payload_size,\n-\t\t\t\t\t &verify_message)) {\n+\t\t\t\t\t &verify_message, NULL)) {\n \t\t\tif (verify_message.len <= gpg_message_offset)\n \t\t\t\tstrbuf_addstr(&verify_message, \"No signature\\n\");\n \t\t\telse\ndiff --git a/pretty.c b/pretty.c\nindex eae57ad..2a1e174 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -984,7 +984,7 @@ static void parse_commit_signature(struct format_commit_context *ctx)\n \t\tgoto out;\n \tstatus = verify_signed_buffer(payload.buf, payload.len,\n \t\t\t\t signature.buf, signature.len,\n-\t\t\t\t &gpg_output);\n+\t\t\t\t &gpg_output, NULL);\n \tif (status && !gpg_output.len)\n \t\tgoto out;\n \tctx->signature.gpg_output = strbuf_detach(&gpg_output, NULL);\n-- \n1.8.1.3.797.ge0260c7\n"},{"id":"209520","messageId":"0e78892fa82498b67ef6cf9316f2dcc73d42c892.1360857415.git.git@drmicha.warpmail.net","threadId":"32902","inReplyTo":"CAJ9Ak2pLqUesOWKNtjwG7P_BW7-EqJP4Kaj0FF_+72Wn=oGoew@mail.gmail.com","subject":"[PATCH 4/5] pretty: parse the gpg status lines rather than the output","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2013-02-14T16:04:45Z","receivedAt":"2013-02-14T16:04:45Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, parse_signature_lines() parses the gpg output for strings\nwhich depend on LANG so it fails to recognize good commit signatures\n(and thus does not fill in %G? and the like) in most locales.\n\nMake it parse the status lines from gpg instead, which are the proper\nmachine interface. This fixes the problem described above.\n\nThere is a change in behavior for \"%GS\" which we intentionally do not\nwork around: \"%GS\" used to put quotes around the signer's uid (or\nrather: it inherited from the gpg user output). We output the uid\nwithout quotes now, just like author and committer names.\n\nSigned-off-by: Michael J Gruber \n---\n pretty.c | 14 +++++++++-----\n 1 file changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 2a1e174..973b912 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -759,6 +759,7 @@ struct format_commit_context {\n \tunsigned commit_signature_parsed:1;\n \tstruct {\n \t\tchar *gpg_output;\n+\t\tchar *gpg_status;\n \t\tchar good_bad;\n \t\tchar *signer;\n \t} signature;\n@@ -948,13 +949,13 @@ static struct {\n \tchar result;\n \tconst char *check;\n } signature_check[] = {\n-\t{ 'G', \": Good signature from \" },\n-\t{ 'B', \": BAD signature from \" },\n+\t{ 'G', \"\\n[GNUPG:] GOODSIG \" },\n+\t{ 'B', \"\\n[GNUPG:] BADSIG \" },\n };\n \n static void parse_signature_lines(struct format_commit_context *ctx)\n {\n-\tconst char *buf = ctx->signature.gpg_output;\n+\tconst char *buf = ctx->signature.gpg_status;\n \tint i;\n \n \tfor (i = 0; i < ARRAY_SIZE(signature_check); i++) {\n@@ -963,7 +964,7 @@ static void parse_signature_lines(struct format_commit_context *ctx)\n \t\tif (!found)\n \t\t\tcontinue;\n \t\tctx->signature.good_bad = signature_check[i].result;\n-\t\tfound += strlen(signature_check[i].check);\n+\t\tfound += strlen(signature_check[i].check)+17;\n \t\tnext = strchrnul(found, '\\n');\n \t\tctx->signature.signer = xmemdupz(found, next - found);\n \t\tbreak;\n@@ -975,6 +976,7 @@ static void parse_commit_signature(struct format_commit_context *ctx)\n \tstruct strbuf payload = STRBUF_INIT;\n \tstruct strbuf signature = STRBUF_INIT;\n \tstruct strbuf gpg_output = STRBUF_INIT;\n+\tstruct strbuf gpg_status = STRBUF_INIT;\n \tint status;\n \n \tctx->commit_signature_parsed = 1;\n@@ -984,13 +986,15 @@ static void parse_commit_signature(struct format_commit_context *ctx)\n \t\tgoto out;\n \tstatus = verify_signed_buffer(payload.buf, payload.len,\n \t\t\t\t signature.buf, signature.len,\n-\t\t\t\t &gpg_output, NULL);\n+\t\t\t\t &gpg_output, &gpg_status);\n \tif (status && !gpg_output.len)\n \t\tgoto out;\n \tctx->signature.gpg_output = strbuf_detach(&gpg_output, NULL);\n+\tctx->signature.gpg_status = strbuf_detach(&gpg_status, NULL);\n \tparse_signature_lines(ctx);\n \n out:\n+\tstrbuf_release(&gpg_status);\n \tstrbuf_release(&gpg_output);\n \tstrbuf_release(&payload);\n \tstrbuf_release(&signature);\n-- \n1.8.1.3.797.ge0260c7\n"},{"id":"209524","messageId":"6787e82ff88f641eeb17245e93e9240e61270842.1360857415.git.git@drmicha.warpmail.net","threadId":"32902","inReplyTo":"CAJ9Ak2pLqUesOWKNtjwG7P_BW7-EqJP4Kaj0FF_+72Wn=oGoew@mail.gmail.com","subject":"[PATCH 5/5] pretty: make %GK output the signing key for signed commits","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2013-02-14T16:04:46Z","receivedAt":"2013-02-14T16:04:46Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Because we can.\n\nNo, really: In order to employ signed keys in an automated way it is\nabsolutely necessary to check which keys the signatures come from. Now\nyou can.\n\nSigned-off-by: Michael J Gruber \n---\n Documentation/pretty-formats.txt | 1 +\n pretty.c | 9 ++++++++-\n 2 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt\nindex 105f18a..2939655 100644\n--- a/Documentation/pretty-formats.txt\n+++ b/Documentation/pretty-formats.txt\n@@ -133,6 +133,7 @@ The placeholders are:\n - '%GG': raw verification message from GPG for a signed commit\n - '%G?': show either \"G\" for Good or \"B\" for Bad for a signed commit\n - '%GS': show the name of the signer for a signed commit\n+- '%GK': show the key used to sign a signed commit\n - '%gD': reflog selector, e.g., `refs/stash@{1}`\n - '%gd': shortened reflog selector, e.g., `stash@{1}`\n - '%gn': reflog identity name\ndiff --git a/pretty.c b/pretty.c\nindex 973b912..b57adef 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -762,6 +762,7 @@ struct format_commit_context {\n \t\tchar *gpg_status;\n \t\tchar good_bad;\n \t\tchar *signer;\n+\t\tchar *key;\n \t} signature;\n \tchar *message;\n \tsize_t width, indent1, indent2;\n@@ -964,7 +965,9 @@ static void parse_signature_lines(struct format_commit_context *ctx)\n \t\tif (!found)\n \t\t\tcontinue;\n \t\tctx->signature.good_bad = signature_check[i].result;\n-\t\tfound += strlen(signature_check[i].check)+17;\n+\t\tfound += strlen(signature_check[i].check);\n+\t\tctx->signature.key = xmemdupz(found, 16);\n+\t\tfound += 17;\n \t\tnext = strchrnul(found, '\\n');\n \t\tctx->signature.signer = xmemdupz(found, next - found);\n \t\tbreak;\n@@ -1204,6 +1207,10 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,\n \t\t\tif (c->signature.signer)\n \t\t\t\tstrbuf_addstr(sb, c->signature.signer);\n \t\t\tbreak;\n+\t\tcase 'K':\n+\t\t\tif (c->signature.key)\n+\t\t\t\tstrbuf_addstr(sb, c->signature.key);\n+\t\t\tbreak;\n \t\t}\n \t\treturn 2;\n \t}\n-- \n1.8.1.3.797.ge0260c7\n"},{"id":"209528","messageId":"7vliaq3kr0.fsf@alter.siamese.dyndns.org","threadId":"32902","inReplyTo":"CAJ9Ak2pLqUesOWKNtjwG7P_BW7-EqJP4Kaj0FF_+72Wn=oGoew@mail.gmail.com","subject":"Re: [BUG] Veryfing signatures in git log fails when language is not english","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-14T16:47:47Z","receivedAt":"2013-02-14T16:47:47Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mariusz Gronczewski writes:\n\n> What is really missing is an ability to display used key ID without\n> hammering git log output with regexps, it would be much easier to\n> validate incoming commits if there was format option to just display\n> key ID instead of signer name. %GS isn't really good solution for that\n> because it will show only one of email addresses used in the key and\n> script checking signatures would have to always pick \"right\" one.\n\nThe %G pretty modifiers other than %GG were done mostly as\nplaceholders.\n\nI think the following would be a good way to refine them:\n\n - %GG, and possibly \"log --show-signature\" should run GPG under\n the user's LANG.\n\n - %G? is mostly useless, unless it is made to always mean \"does\n it verify crypto-wise\" and nothing else. One bit is simply\n too small to represent all the cases where you may or may not\n have the signer's key, or you may have the key but you do not\n have enough trust in it (e.g. the key may be expired, revoked,\n or not enough confidence in your web of trust).\n\n - The \"right\" one you mention for %GS is easier than you might\n think. If you just verify against the accompanying \"tagger\"\n identity, that should be sufficient. It of course cannot be\n generally solved, as you could tag as person A while signing\n with key for person B, but a simple social convention would\n help us out there: if you tag as Mariusz Gronczewski, your\n signature should also say so.\n"},{"id":"209533","messageId":"7vzjz624kj.fsf@alter.siamese.dyndns.org","threadId":"32902","inReplyTo":"e0f3c48e474c64e33c67c772468bbf7bba0169af.1360857415.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 1/5] gpg-interface: check good signature in a reliable way","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-14T17:22:36Z","receivedAt":"2013-02-14T17:22:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Currently, verify_signed_buffer() only checks the return code of gpg,\n> and some callers implement additional unreliable checks for \"Good\n> signature\" in the gpg output meant for the user.\n>\n> Use the status output instead and parse for a line beinning with\n> \"[GNUPG:] GOODSIG \". This is the only reliable way of checking for a\n> good gpg signature.\n>\n> If needed we can change this easily to \"[GNUPG:] VALIDSIG \" if we want\n> to take into account the trust model.\n\nThanks. I didn't look beyond \"man gpg\" nor bother looking at\nDETAILS file in its source, which the manpage refers to.\n\nI think GOODSIG is a good starting point. Depending on the context\n(e.g. \"%G?\") we may also want to consider EXPSIG (but not EXPKEYSIG\nor REVKEYSIG) acceptable, while reading \"log --show-signature\" on\nancient part of the history, no?\n\n> Signed-off-by: Michael J Gruber \n> ---\n> gpg-interface.c | 13 +++++++++++--\n> 1 file changed, 11 insertions(+), 2 deletions(-)\n>\n> diff --git a/gpg-interface.c b/gpg-interface.c\n> index 4559033..c582b2e 100644\n> --- a/gpg-interface.c\n> +++ b/gpg-interface.c\n> @@ -96,15 +96,17 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n> /*\n> * Run \"gpg\" to see if the payload matches the detached signature.\n> * gpg_output, when set, receives the diagnostic output from GPG.\n> + * gpg_status, when set, receives the status output from GPG.\n> */\n> int verify_signed_buffer(const char *payload, size_t payload_size,\n> \t\t\t const char *signature, size_t signature_size,\n> \t\t\t struct strbuf *gpg_output)\n> {\n> \tstruct child_process gpg;\n> -\tconst char *args_gpg[] = {NULL, \"--verify\", \"FILE\", \"-\", NULL};\n> +\tconst char *args_gpg[] = {NULL, \"--status-fd=1\", \"--verify\", \"FILE\", \"-\", NULL};\n> \tchar path[PATH_MAX];\n> \tint fd, ret;\n> +\tstruct strbuf buf = STRBUF_INIT;\n> \n> \targs_gpg[0] = gpg_program;\n> \tfd = git_mkstemp(path, PATH_MAX, \".git_vtag_tmpXXXXXX\");\n> @@ -119,9 +121,10 @@ int verify_signed_buffer(const char *payload, size_t payload_size,\n> \tmemset(&gpg, 0, sizeof(gpg));\n> \tgpg.argv = args_gpg;\n> \tgpg.in = -1;\n> +\tgpg.out = -1;\n> \tif (gpg_output)\n> \t\tgpg.err = -1;\n> -\targs_gpg[2] = path;\n> +\targs_gpg[3] = path;\n> \tif (start_command(&gpg)) {\n> \t\tunlink(path);\n> \t\treturn error(_(\"could not run gpg.\"));\n> @@ -134,9 +137,15 @@ int verify_signed_buffer(const char *payload, size_t payload_size,\n> \t\tstrbuf_read(gpg_output, gpg.err, 0);\n> \t\tclose(gpg.err);\n> \t}\n> +\tstrbuf_read(&buf, gpg.out, 0);\n> +\tclose(gpg.out);\n> +\n> \tret = finish_command(&gpg);\n> \n> \tunlink_or_warn(path);\n> \n> +\tret |= !strstr(buf.buf, \"\\n[GNUPG:] GOODSIG \");\n> +\tstrbuf_release(&buf);\n> +\n> \treturn ret;\n> }\n"},{"id":"209549","messageId":"511DF174.4090301@drmicha.warpmail.net","threadId":"32902","inReplyTo":"7vzjz624kj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/5] gpg-interface: check good signature in a reliable way","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2013-02-15T08:27:32Z","receivedAt":"2013-02-15T08:27:32Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 14.02.2013 18:22:\n> Michael J Gruber writes:\n> \n>> Currently, verify_signed_buffer() only checks the return code of gpg,\n>> and some callers implement additional unreliable checks for \"Good\n>> signature\" in the gpg output meant for the user.\n>>\n>> Use the status output instead and parse for a line beinning with\n>> \"[GNUPG:] GOODSIG \". This is the only reliable way of checking for a\n>> good gpg signature.\n>>\n>> If needed we can change this easily to \"[GNUPG:] VALIDSIG \" if we want\n>> to take into account the trust model.\n> \n> Thanks. I didn't look beyond \"man gpg\" nor bother looking at\n> DETAILS file in its source, which the manpage refers to.\n> \n> I think GOODSIG is a good starting point. Depending on the context\n> (e.g. \"%G?\") we may also want to consider EXPSIG (but not EXPKEYSIG\n> or REVKEYSIG) acceptable, while reading \"log --show-signature\" on\n> ancient part of the history, no?\n\nYes, we could certainly return a more detailed status to the callers.\nCurrently, \"0\" is OK (GOODSIG) and everything else is a fail. We would\nneed to change the callers to allow more details on the \"fail\" as well\nas the \"OK\" so that they can decide what is good enough, say:\n\n-1: fail for technical reasons (no sig, can't run gpg etc.)\n0: sig present bad (cryptographically) BAD\n1: REVKEYSIG\n2: EXPKEYSIG\n3: EXPSIG\n4: GOODSIG\n5: VALIDSIG\n\nI'd have to recheck whether a bitmask or ordered values make more sense.\n\n>> Signed-off-by: Michael J Gruber \n>> ---\n>> gpg-interface.c | 13 +++++++++++--\n>> 1 file changed, 11 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/gpg-interface.c b/gpg-interface.c\n>> index 4559033..c582b2e 100644\n>> --- a/gpg-interface.c\n>> +++ b/gpg-interface.c\n>> @@ -96,15 +96,17 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n>> /*\n>> * Run \"gpg\" to see if the payload matches the detached signature.\n>> * gpg_output, when set, receives the diagnostic output from GPG.\n>> + * gpg_status, when set, receives the status output from GPG.\n>> */\n>> int verify_signed_buffer(const char *payload, size_t payload_size,\n>> \t\t\t const char *signature, size_t signature_size,\n>> \t\t\t struct strbuf *gpg_output)\n>> {\n>> \tstruct child_process gpg;\n>> -\tconst char *args_gpg[] = {NULL, \"--verify\", \"FILE\", \"-\", NULL};\n>> +\tconst char *args_gpg[] = {NULL, \"--status-fd=1\", \"--verify\", \"FILE\", \"-\", NULL};\n>> \tchar path[PATH_MAX];\n>> \tint fd, ret;\n>> +\tstruct strbuf buf = STRBUF_INIT;\n>> \n>> \targs_gpg[0] = gpg_program;\n>> \tfd = git_mkstemp(path, PATH_MAX, \".git_vtag_tmpXXXXXX\");\n>> @@ -119,9 +121,10 @@ int verify_signed_buffer(const char *payload, size_t payload_size,\n>> \tmemset(&gpg, 0, sizeof(gpg));\n>> \tgpg.argv = args_gpg;\n>> \tgpg.in = -1;\n>> +\tgpg.out = -1;\n>> \tif (gpg_output)\n>> \t\tgpg.err = -1;\n>> -\targs_gpg[2] = path;\n>> +\targs_gpg[3] = path;\n>> \tif (start_command(&gpg)) {\n>> \t\tunlink(path);\n>> \t\treturn error(_(\"could not run gpg.\"));\n>> @@ -134,9 +137,15 @@ int verify_signed_buffer(const char *payload, size_t payload_size,\n>> \t\tstrbuf_read(gpg_output, gpg.err, 0);\n>> \t\tclose(gpg.err);\n>> \t}\n>> +\tstrbuf_read(&buf, gpg.out, 0);\n>> +\tclose(gpg.out);\n>> +\n>> \tret = finish_command(&gpg);\n>> \n>> \tunlink_or_warn(path);\n>> \n>> +\tret |= !strstr(buf.buf, \"\\n[GNUPG:] GOODSIG \");\n>> +\tstrbuf_release(&buf);\n>> +\n>> \treturn ret;\n>> }\n"},{"id":"209552","messageId":"CAJ9Ak2oF+SiYjStJndRZqLfnzBisn3TyXr3U_E_42BHrKbKPfQ@mail.gmail.com","threadId":"32902","inReplyTo":"7vliaq3kr0.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] Veryfing signatures in git log fails when language is not english","fromName":"Mariusz Gronczewski","fromEmail":"xani666@gmail.com","sentAt":"2013-02-15T14:14:47Z","receivedAt":"2013-02-15T14:14:47Z","isPatch":false,"sender":{"key":"xani666@gmail.com","avatar":"https://gravatar.com/avatar/078387db945420fb3089b7094b28d7cca5148dbac5494b40432eee39e5620e53?d=mp&s=160"},"body":"2013/2/14 Junio C Hamano :\n>\n> - The \"right\" one you mention for %GS is easier than you might\n> think. If you just verify against the accompanying \"tagger\"\n> identity, that should be sufficient. It of course cannot be\n> generally solved, as you could tag as person A while signing\n> with key for person B, but a simple social convention would\n> help us out there: if you tag as Mariusz Gronczewski, your\n> signature should also say so.\nunless there is someone else with same name, which happens more often\n(so far i've seen it happen twice) than same GPG IDs. It's all fine if\nyou just have one keyring that you can use to validate against all\nrepos but when there are multiple projects each with different persons\nresponsible for deploying it can get messy ;].\n\nmy use-case is basically \"allow only commits signed by person X Y or Z\nto be deployed on production\" and \"allow only persons A, B, C, X, Y,\nZ to commit\", while latter case can be solved by software like\ngitolite, credential validation is messy at best as you have to\nvalidate:\n- ssh key\n- if ssh key owner matches commiter name\n- if commiter name =! author name, if a given person can do that\n(project architect or some other person accepting patches) or can't\nand I'm trying to implement GPG signing so if someone does something\nmalicious i can say \"OK that commit was signed by your key ID, why you\ndid it?\"\n\n\n-- \nMariusz Gronczewski (XANi) \nGnuPG: 0xEA8ACE64\n"},{"id":"209554","messageId":"7vip5twodl.fsf@alter.siamese.dyndns.org","threadId":"32902","inReplyTo":"CAJ9Ak2oF+SiYjStJndRZqLfnzBisn3TyXr3U_E_42BHrKbKPfQ@mail.gmail.com","subject":"Re: [BUG] Veryfing signatures in git log fails when language is not english","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-15T16:08:54Z","receivedAt":"2013-02-15T16:08:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mariusz Gronczewski writes:\n\n> 2013/2/14 Junio C Hamano :\n>>\n>> - The \"right\" one you mention for %GS is easier than you might\n>> think. If you just verify against the accompanying \"tagger\"\n>> identity, that should be sufficient. It of course cannot be\n>> generally solved, as you could tag as person A while signing\n>> with key for person B, but a simple social convention would\n>> help us out there: if you tag as Mariusz Gronczewski, your\n>> signature should also say so.\n> unless there is someone else with same name, which happens more often\n> (so far i've seen it happen twice) than same GPG IDs.\n\nOh, I didn't mean to say \"ignore email part\", which of course will\nmake the result more likely to be ambiguous.\n\nI thought you meant by \"have to show right one\" the following\nscenario:\n\n The tag v1.8.1 has a GPG signature. The key 96AFE6CB was used\n to sign it. The key is associated with more than one identities.\n One of them is \"Junio C Hamano \", but that is\n not the only one. I also have combinations of other e-mail\n addresses and names spelled differently (e.g. \"Junio Hamano\n \") that are _not_ associated with that key.\n\n GPG may say \"good signature from A aka B aka C\"; which one of A,\n B, or C should we choose?\n\nI was suggesting that among the identities associated with the key\nused to sign the tag, we should show the one that matches the\nidentity on the tagger field.\n\n object 5d417842efeafb6e109db7574196901c4e95d273\n type commit\n tag v1.8.1\n tagger Junio C Hamano 1356992771 -0800\n\n Git 1.8.1\n -----BEGIN PGP SIGNATURE-----\n Version: GnuPG v1.4.10 (GNU/Linux)\n\n iQIc...\n =v706\n -----END PGP SIGNATURE-----\n\nBecause it is clear from the context where the signature appears\nthat that identity is what matters for me as a signer in the project\nthe tag appears in.\n\nI may have other e-mail addresses that are not associated with that\nkey, but it would be insane to put that on the tagger field of the\ntag, while GPG-signing with that key.\n"}]}