{"thread":{"id":"42600","subject":"I lost my commit signature","startedAt":"2016-06-16T06:55:52Z","lastAt":"2016-06-17T16:40:20Z","messageCount":31,"participants":["ZhenTian","Jeff King","Michael J Gruber","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"289190","messageId":"CAGrdoOrrpc7OiU74fa-qpjLNxOtm4upAOAfWPG6VbWUjyE0C2Q@mail.gmail.com","threadId":"42600","inReplyTo":null,"subject":"I lost my commit signature","fromName":"ZhenTian","fromEmail":"loooseleaves@gmail.com","sentAt":"2016-06-14T07:50:43Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":false,"sender":{"key":"loooseleaves@gmail.com","avatar":"https://gravatar.com/avatar/bc445fa477ccc779cf8752086b2e56f6c8fc776d4a3b19d8cd318dbb0ea895b5?d=mp&s=160"},"body":"Hi git developers,\n\nI commit with -S argument, and I got some output like this:\n\nYou need a passphrase to unlock the secret key for\nuser: \"Tian Zhen <tianzhen@honovation.com>\"\n4096-bit RSA key, ID 2EF2AD6E, created 2016-05-21\n\n[master d107770] feat: mobile support free freight hint, closed #1417\n 8 files changed, 58 insertions(+), 29 deletions(-)\n rewrite static/css/mobile.min.css (64%)\n\n\nbut when I check git log with --show-signature, I can't find my sign.\n\nmy git is 2.4.8, and OS is Ubuntu 14.04.4\n\nSincerely,\ndawncold\n"},{"id":"289192","messageId":"20160614075824.GB12563@sigill.intra.peff.net","threadId":"42600","inReplyTo":"CAGrdoOrrpc7OiU74fa-qpjLNxOtm4upAOAfWPG6VbWUjyE0C2Q@mail.gmail.com","subject":"Re: I lost my commit signature","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-14T07:58:24Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 14, 2016 at 03:50:43PM +0800, ZhenTian wrote:\n\n> I commit with -S argument, and I got some output like this:\n> \n> You need a passphrase to unlock the secret key for\n> user: \"Tian Zhen <tianzhen@honovation.com>\"\n> 4096-bit RSA key, ID 2EF2AD6E, created 2016-05-21\n> \n> [master d107770] feat: mobile support free freight hint, closed #1417\n>  8 files changed, 58 insertions(+), 29 deletions(-)\n>  rewrite static/css/mobile.min.css (64%)\n>\n> but when I check git log with --show-signature, I can't find my sign.\n> \n> my git is 2.4.8, and OS is Ubuntu 14.04.4\n\nHere's a reproduction which should work (and does for me):\n\n  $ git init\n  $ echo content >file\n  $ git add file\n  $ git commit -m foo -S\n  You need a passphrase to unlock the secret key for\n  user: \"Jeff King <peff@peff.net>\"\n  4096-bit RSA key, ID F9430ED9, created 2016-02-03 (main key ID D7B337A8)\n\n  [master (root-commit) 6b0b230] foo\n   1 file changed, 1 insertion(+)\n   create mode 100644 file\n\n  $ git log --show-signature\n  commit 6b0b230c79f8912bf8b21afc0d12d2cbf54cc74d (HEAD -> master)\n  gpg: Signature made Tue 14 Jun 2016 03:55:11 AM EDT using RSA key ID F9430ED9\n  gpg: Good signature from \"Jeff King <peff@peff.net>\"\n  gpg:                 aka \"Jeff King <peff@github.com>\"\n  Author: Jeff King <peff@peff.net>\n  Date:   Tue Jun 14 03:55:11 2016 -0400\n\n      foo\n\nDoes something similar work for you? If so, then we need to figure out\nwhat happened in your original case. Can you show the exact commands you\nran, and what they did output?\n\nIf the simple case above doesn't work, then we need to figure out\nwhether the commit doesn't get a signature, or whether \"log\n--show-signature\" is not working on your system. For the former, I'd try\n\"git cat-file commit HEAD\", which should show the encoded signature\nblock. If it's there, then presumably something is not working in\ncalling gpg.\n\n-Peff\n"},{"id":"289193","messageId":"CAGrdoOp=dDkiTr+Sb-uZWx66b4hoZCYAiRjfgYqoE8H4-kXJvg@mail.gmail.com","threadId":"42600","inReplyTo":"20160614075824.GB12563@sigill.intra.peff.net","subject":"Re: I lost my commit signature","fromName":"ZhenTian","fromEmail":"loooseleaves@gmail.com","sentAt":"2016-06-14T08:09:52Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":false,"sender":{"key":"loooseleaves@gmail.com","avatar":"https://gravatar.com/avatar/bc445fa477ccc779cf8752086b2e56f6c8fc776d4a3b19d8cd318dbb0ea895b5?d=mp&s=160"},"body":"Hi Peff,\n\nI commit via this command: gcs -nm \"feat: mobile support free freight\nhint, closed #1417\"\n\ngcs is an alias in zsh, which is: git commit -S\n\nI have tested sign my work in another project, it works fine, I have\ncommitted five times, all commits are signed.\n\nI can't find encoded signature block in the output of \"git cat-file\ncommit HEAD\", only these:\n```\ntree 17a572e349ce2fda47470951b5011b9c2f6533b7\nparent 2c35701725d34325520acb9b45daf42f64adc536\nauthor TianZhen <tianzhen@honovation.com> 1465887785 +0800\ncommitter TianZhen <tianzhen@honovation.com> 1465887791 +0800\n\nfeat: mobile support free freight hint, closed #1417\n```\n\nSome of my commits are signed, for example I have committed four times\ntoday, only first commit is signed. Is it possible some issue with\ngpg-agent? I can't find it via `ps -Af | grep gpg`.\n\n-Dawncold\nSincerely,\n田震\n\n\nOn Tue, Jun 14, 2016 at 3:58 PM, Jeff King <peff@peff.net> wrote:\n> On Tue, Jun 14, 2016 at 03:50:43PM +0800, ZhenTian wrote:\n>\n>> I commit with -S argument, and I got some output like this:\n>>\n>> You need a passphrase to unlock the secret key for\n>> user: \"Tian Zhen <tianzhen@honovation.com>\"\n>> 4096-bit RSA key, ID 2EF2AD6E, created 2016-05-21\n>>\n>> [master d107770] feat: mobile support free freight hint, closed #1417\n>>  8 files changed, 58 insertions(+), 29 deletions(-)\n>>  rewrite static/css/mobile.min.css (64%)\n>>\n>> but when I check git log with --show-signature, I can't find my sign.\n>>\n>> my git is 2.4.8, and OS is Ubuntu 14.04.4\n>\n> Here's a reproduction which should work (and does for me):\n>\n>   $ git init\n>   $ echo content >file\n>   $ git add file\n>   $ git commit -m foo -S\n>   You need a passphrase to unlock the secret key for\n>   user: \"Jeff King <peff@peff.net>\"\n>   4096-bit RSA key, ID F9430ED9, created 2016-02-03 (main key ID D7B337A8)\n>\n>   [master (root-commit) 6b0b230] foo\n>    1 file changed, 1 insertion(+)\n>    create mode 100644 file\n>\n>   $ git log --show-signature\n>   commit 6b0b230c79f8912bf8b21afc0d12d2cbf54cc74d (HEAD -> master)\n>   gpg: Signature made Tue 14 Jun 2016 03:55:11 AM EDT using RSA key ID F9430ED9\n>   gpg: Good signature from \"Jeff King <peff@peff.net>\"\n>   gpg:                 aka \"Jeff King <peff@github.com>\"\n>   Author: Jeff King <peff@peff.net>\n>   Date:   Tue Jun 14 03:55:11 2016 -0400\n>\n>       foo\n>\n> Does something similar work for you? If so, then we need to figure out\n> what happened in your original case. Can you show the exact commands you\n> ran, and what they did output?\n>\n> If the simple case above doesn't work, then we need to figure out\n> whether the commit doesn't get a signature, or whether \"log\n> --show-signature\" is not working on your system. For the former, I'd try\n> \"git cat-file commit HEAD\", which should show the encoded signature\n> block. If it's there, then presumably something is not working in\n> calling gpg.\n>\n> -Peff\n"},{"id":"289194","messageId":"20160614081854.GA13457@sigill.intra.peff.net","threadId":"42600","inReplyTo":"CAGrdoOp=dDkiTr+Sb-uZWx66b4hoZCYAiRjfgYqoE8H4-kXJvg@mail.gmail.com","subject":"Re: I lost my commit signature","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-14T08:18:55Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 14, 2016 at 04:09:52PM +0800, ZhenTian wrote:\n\n> I have tested sign my work in another project, it works fine, I have\n> committed five times, all commits are signed.\n> \n> I can't find encoded signature block in the output of \"git cat-file\n> commit HEAD\", only these:\n> ```\n> tree 17a572e349ce2fda47470951b5011b9c2f6533b7\n> parent 2c35701725d34325520acb9b45daf42f64adc536\n> author TianZhen <tianzhen@honovation.com> 1465887785 +0800\n> committer TianZhen <tianzhen@honovation.com> 1465887791 +0800\n> \n> feat: mobile support free freight hint, closed #1417\n> ```\n> \n> Some of my commits are signed, for example I have committed four times\n> today, only first commit is signed. Is it possible some issue with\n> gpg-agent? I can't find it via `ps -Af | grep gpg`.\n\nPossibly. If you set gpg.program \"gpg -v\", does it help? You could also\ntry setting it to \"gpg | /tmp/log\" to see what gpg is passing back to\ngit.\n\n-Peff\n"},{"id":"289196","messageId":"CAGrdoOqfcacG488u-MA7UiapgvJEGNX2QaRq8BMmycEWg-BGWg@mail.gmail.com","threadId":"42600","inReplyTo":"20160614081854.GA13457@sigill.intra.peff.net","subject":"Re: I lost my commit signature","fromName":"ZhenTian","fromEmail":"loooseleaves@gmail.com","sentAt":"2016-06-14T08:39:38Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":false,"sender":{"key":"loooseleaves@gmail.com","avatar":"https://gravatar.com/avatar/bc445fa477ccc779cf8752086b2e56f6c8fc776d4a3b19d8cd318dbb0ea895b5?d=mp&s=160"},"body":"Hi Peff,\n\nI want to set gpg -v to pgp.program, but if I set it, it can't call gpg:\n```\nerror: cannot run gpg -v: No such file or directory\nerror: could not run gpg.\nfatal: failed to write commit object\n```\n\nI have tried set gpg.program value to `gpg|/tmp/log`, `/usr/bin/gpg\n-v`, `gpg -v`, `\"/usr/bin/gpg -v\"`\n\nonly after I set to `gpg` or `/usr/bin/gpg` without any argument, it will work.\nSincerely,\n田震\n\n\nOn Tue, Jun 14, 2016 at 4:18 PM, Jeff King <peff@peff.net> wrote:\n> On Tue, Jun 14, 2016 at 04:09:52PM +0800, ZhenTian wrote:\n>\n>> I have tested sign my work in another project, it works fine, I have\n>> committed five times, all commits are signed.\n>>\n>> I can't find encoded signature block in the output of \"git cat-file\n>> commit HEAD\", only these:\n>> ```\n>> tree 17a572e349ce2fda47470951b5011b9c2f6533b7\n>> parent 2c35701725d34325520acb9b45daf42f64adc536\n>> author TianZhen <tianzhen@honovation.com> 1465887785 +0800\n>> committer TianZhen <tianzhen@honovation.com> 1465887791 +0800\n>>\n>> feat: mobile support free freight hint, closed #1417\n>> ```\n>>\n>> Some of my commits are signed, for example I have committed four times\n>> today, only first commit is signed. Is it possible some issue with\n>> gpg-agent? I can't find it via `ps -Af | grep gpg`.\n>\n> Possibly. If you set gpg.program \"gpg -v\", does it help? You could also\n> try setting it to \"gpg | /tmp/log\" to see what gpg is passing back to\n> git.\n>\n> -Peff\n"},{"id":"289199","messageId":"20160614094121.GA13971@sigill.intra.peff.net","threadId":"42600","inReplyTo":"CAGrdoOqfcacG488u-MA7UiapgvJEGNX2QaRq8BMmycEWg-BGWg@mail.gmail.com","subject":"Re: I lost my commit signature","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-14T09:41:21Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 14, 2016 at 04:39:38PM +0800, ZhenTian wrote:\n\n> I want to set gpg -v to pgp.program, but if I set it, it can't call gpg:\n> ```\n> error: cannot run gpg -v: No such file or directory\n> error: could not run gpg.\n> fatal: failed to write commit object\n> ```\n> \n> I have tried set gpg.program value to `gpg|/tmp/log`, `/usr/bin/gpg\n> -v`, `gpg -v`, `\"/usr/bin/gpg -v\"`\n> \n> only after I set to `gpg` or `/usr/bin/gpg` without any argument, it will work.\n\nAh, right. Most of the time we run such programs as shell commands, but\nit looks like we do not. So you'd have to do something like:\n\n\tcat >/tmp/fake-gpg <<-\\EOF\n\t#!/bin/sh\n\tgpg -v \"$@\"\n\tEOF\n\tchmod +x /tmp/fake-gpg\n\tgit config gpg.program /tmp/fake-gpg\n\n-Peff\n"},{"id":"289200","messageId":"CAGrdoOoadz_1OV4eB1+yjCiDpKRe1T1CtCTu7cq1g+9CEc2DxQ@mail.gmail.com","threadId":"42600","inReplyTo":"20160614094121.GA13971@sigill.intra.peff.net","subject":"Re: I lost my commit signature","fromName":"ZhenTian","fromEmail":"loooseleaves@gmail.com","sentAt":"2016-06-14T09:56:36Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":false,"sender":{"key":"loooseleaves@gmail.com","avatar":"https://gravatar.com/avatar/bc445fa477ccc779cf8752086b2e56f6c8fc776d4a3b19d8cd318dbb0ea895b5?d=mp&s=160"},"body":"Thanks Peff, I have setup to gpg.program, if any progress I will reply.\n\nOver.\nSincerely,\n田震\n\n\nOn Tue, Jun 14, 2016 at 5:41 PM, Jeff King <peff@peff.net> wrote:\n> On Tue, Jun 14, 2016 at 04:39:38PM +0800, ZhenTian wrote:\n>\n>> I want to set gpg -v to pgp.program, but if I set it, it can't call gpg:\n>> ```\n>> error: cannot run gpg -v: No such file or directory\n>> error: could not run gpg.\n>> fatal: failed to write commit object\n>> ```\n>>\n>> I have tried set gpg.program value to `gpg|/tmp/log`, `/usr/bin/gpg\n>> -v`, `gpg -v`, `\"/usr/bin/gpg -v\"`\n>>\n>> only after I set to `gpg` or `/usr/bin/gpg` without any argument, it will work.\n>\n> Ah, right. Most of the time we run such programs as shell commands, but\n> it looks like we do not. So you'd have to do something like:\n>\n>         cat >/tmp/fake-gpg <<-\\EOF\n>         #!/bin/sh\n>         gpg -v \"$@\"\n>         EOF\n>         chmod +x /tmp/fake-gpg\n>         git config gpg.program /tmp/fake-gpg\n>\n> -Peff\n"},{"id":"289204","messageId":"e129da8e-4e1d-c535-ca62-d2a4c2e23799@drmicha.warpmail.net","threadId":"42600","inReplyTo":"20160614094121.GA13971@sigill.intra.peff.net","subject":"Re: I lost my commit signature","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-06-14T10:57:34Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 14.06.2016 11:41:\n> On Tue, Jun 14, 2016 at 04:39:38PM +0800, ZhenTian wrote:\n> \n>> I want to set gpg -v to pgp.program, but if I set it, it can't call gpg:\n>> ```\n>> error: cannot run gpg -v: No such file or directory\n>> error: could not run gpg.\n>> fatal: failed to write commit object\n>> ```\n>>\n>> I have tried set gpg.program value to `gpg|/tmp/log`, `/usr/bin/gpg\n>> -v`, `gpg -v`, `\"/usr/bin/gpg -v\"`\n>>\n>> only after I set to `gpg` or `/usr/bin/gpg` without any argument, it will work.\n> \n> Ah, right. Most of the time we run such programs as shell commands, but\n> it looks like we do not. So you'd have to do something like:\n> \n> \tcat >/tmp/fake-gpg <<-\\EOF\n> \t#!/bin/sh\n> \tgpg -v \"$@\"\n> \tEOF\n> \tchmod +x /tmp/fake-gpg\n> \tgit config gpg.program /tmp/fake-gpg\n> \n> -Peff\n> \n\nThe content of \"gpg.program\" is used as argv[0] when we build up various\ncommands to be run; we expect it to heed standard gpg options.\n\nOn the other hand:\n\ngit -c gpg.program=echo commit -S\n\n'successfully' creates a commit that has\n\ngpgsig -bsau Michael J Gruber <mickey@mouse.dis>\n\nas the last header line. gpg.program=true fails (as does cat, unhappy\nwith the options), so apparently we do some error checking but not enough.\n\nMichael\n"},{"id":"289207","messageId":"2f473a993b6bc951dec76d38c11d0e600b59b8d3.1465902530.git.git@drmicha.warpmail.net","threadId":"42600","inReplyTo":"e129da8e-4e1d-c535-ca62-d2a4c2e23799@drmicha.warpmail.net","subject":"[PATCH] gpg-interface: check gpg signature for correct header","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-06-14T11:11:19Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"When we create a signature, it may happen that gpg returns with\n\"success\" but not with an actual detached signature on stdout.\n\nCheck for the correct header to catch these cases better.\n\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\nThis catches at least my echo example.\n\nWe could do a full blown gpg signature check, of course.\n\n gpg-interface.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex c4b1e8c..664796f 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -185,7 +185,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n \n \tsigchain_pop(SIGPIPE);\n \n-\tif (finish_command(&gpg) || !len || len < 0)\n+\tif (finish_command(&gpg) || !len || len < 0 || strncmp(signature->buf, PGP_SIGNATURE, strlen(PGP_SIGNATURE)))\n \t\treturn error(_(\"gpg failed to sign the data\"));\n \n \t/* Strip CR from the line endings, in case we are on Windows. */\n-- \n2.9.0.382.g87fd384\n"},{"id":"289209","messageId":"20160614112006.GA15889@sigill.intra.peff.net","threadId":"42600","inReplyTo":"2f473a993b6bc951dec76d38c11d0e600b59b8d3.1465902530.git.git@drmicha.warpmail.net","subject":"Re: [PATCH] gpg-interface: check gpg signature for correct header","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-14T11:20:06Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 14, 2016 at 01:11:19PM +0200, Michael J Gruber wrote:\n\n> When we create a signature, it may happen that gpg returns with\n> \"success\" but not with an actual detached signature on stdout.\n> \n> Check for the correct header to catch these cases better.\n\nSeems like a reasonable idea.\n\nI do worry that checking for PGP_SIGNATURE is a little fragile, though.\nWe currently let you sign with gpgsm, for example, and I think this\nwould break it (the verification side is not great because we don't\nrecognize gpgsm headers, but this feels like a step backwards).\n\nThat wouldn't be too hard to work around with a \"is this a signature\"\nfunction that checks both types.\n\n> diff --git a/gpg-interface.c b/gpg-interface.c\n> index c4b1e8c..664796f 100644\n> --- a/gpg-interface.c\n> +++ b/gpg-interface.c\n> @@ -185,7 +185,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n>  \n>  \tsigchain_pop(SIGPIPE);\n>  \n> -\tif (finish_command(&gpg) || !len || len < 0)\n> +\tif (finish_command(&gpg) || !len || len < 0 || strncmp(signature->buf, PGP_SIGNATURE, strlen(PGP_SIGNATURE)))\n>  \t\treturn error(_(\"gpg failed to sign the data\"));\n\nI think your strncmp is better spelled:\n\n  starts_with(signature->buf, PGP_SIGNATURE);\n\nThe check for \"!len\" is redundant now. I think you could drop \"len < 0\"\nas well (and in fact, drop the \"len\" variable entirely), as in the error\ncase we'd simply have an empty signature->len.\n\nYour patch effectively swaps out \"did we get any data\" for \"did we get\nthe data we expect\", which is what those \"len\" checks were doing.\n\n-Peff\n"},{"id":"289210","messageId":"3ccf7649-be0e-dcb8-c9a7-cdc36dc85d16@drmicha.warpmail.net","threadId":"42600","inReplyTo":"20160614112006.GA15889@sigill.intra.peff.net","subject":"Re: [PATCH] gpg-interface: check gpg signature for correct header","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-06-14T11:34:20Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 14.06.2016 13:20:\n> On Tue, Jun 14, 2016 at 01:11:19PM +0200, Michael J Gruber wrote:\n> \n>> When we create a signature, it may happen that gpg returns with\n>> \"success\" but not with an actual detached signature on stdout.\n>>\n>> Check for the correct header to catch these cases better.\n> \n> Seems like a reasonable idea.\n> \n> I do worry that checking for PGP_SIGNATURE is a little fragile, though.\n> We currently let you sign with gpgsm, for example, and I think this\n> would break it (the verification side is not great because we don't\n> recognize gpgsm headers, but this feels like a step backwards).\n> \n> That wouldn't be too hard to work around with a \"is this a signature\"\n> function that checks both types.\n> \n>> diff --git a/gpg-interface.c b/gpg-interface.c\n>> index c4b1e8c..664796f 100644\n>> --- a/gpg-interface.c\n>> +++ b/gpg-interface.c\n>> @@ -185,7 +185,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n>>  \n>>  \tsigchain_pop(SIGPIPE);\n>>  \n>> -\tif (finish_command(&gpg) || !len || len < 0)\n>> +\tif (finish_command(&gpg) || !len || len < 0 || strncmp(signature->buf, PGP_SIGNATURE, strlen(PGP_SIGNATURE)))\n>>  \t\treturn error(_(\"gpg failed to sign the data\"));\n> \n> I think your strncmp is better spelled:\n> \n>   starts_with(signature->buf, PGP_SIGNATURE);\n> \n> The check for \"!len\" is redundant now. I think you could drop \"len < 0\"\n> as well (and in fact, drop the \"len\" variable entirely), as in the error\n> case we'd simply have an empty signature->len.\n> \n> Your patch effectively swaps out \"did we get any data\" for \"did we get\n> the data we expect\", which is what those \"len\" checks were doing.\n> \n> -Peff\n> \n\nMy patch actually makes several tests fail, sorry. (I did check before\nthat I can still create signatures...) Maybe my offset in buf is wrong.\n\nstarts_with, yes.\n\nCan't check any further now, sorry. But we do check for the\nPGP_SIGNATURE in our signed objects anyways. So I feel that we can either\n\n- tighthen the check for valid gpg signatures\n\nor\n\n- make our signature interface completely pluggable.\n\nWe can't have it both ways, but at least things are localised in\ngpg-interface.c now.\n\nThe proposed patch is just some consistency check that does not rely on\ngpg.program itself(!), or else we could simply call verify.\n\nMichael\n"},{"id":"289212","messageId":"26353a3d-e495-075f-4f84-b34a2420a6cf@drmicha.warpmail.net","threadId":"42600","inReplyTo":"3ccf7649-be0e-dcb8-c9a7-cdc36dc85d16@drmicha.warpmail.net","subject":"Re: [PATCH] gpg-interface: check gpg signature for correct header","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-06-14T11:58:18Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Michael J Gruber venit, vidit, dixit 14.06.2016 13:34:\n> Jeff King venit, vidit, dixit 14.06.2016 13:20:\n>> On Tue, Jun 14, 2016 at 01:11:19PM +0200, Michael J Gruber wrote:\n>>\n>>> When we create a signature, it may happen that gpg returns with\n>>> \"success\" but not with an actual detached signature on stdout.\n>>>\n>>> Check for the correct header to catch these cases better.\n>>\n>> Seems like a reasonable idea.\n>>\n>> I do worry that checking for PGP_SIGNATURE is a little fragile, though.\n>> We currently let you sign with gpgsm, for example, and I think this\n>> would break it (the verification side is not great because we don't\n>> recognize gpgsm headers, but this feels like a step backwards).\n>>\n>> That wouldn't be too hard to work around with a \"is this a signature\"\n>> function that checks both types.\n>>\n>>> diff --git a/gpg-interface.c b/gpg-interface.c\n>>> index c4b1e8c..664796f 100644\n>>> --- a/gpg-interface.c\n>>> +++ b/gpg-interface.c\n>>> @@ -185,7 +185,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n>>>  \n>>>  \tsigchain_pop(SIGPIPE);\n>>>  \n>>> -\tif (finish_command(&gpg) || !len || len < 0)\n>>> +\tif (finish_command(&gpg) || !len || len < 0 || strncmp(signature->buf, PGP_SIGNATURE, strlen(PGP_SIGNATURE)))\n>>>  \t\treturn error(_(\"gpg failed to sign the data\"));\n>>\n>> I think your strncmp is better spelled:\n>>\n>>   starts_with(signature->buf, PGP_SIGNATURE);\n>>\n>> The check for \"!len\" is redundant now. I think you could drop \"len < 0\"\n>> as well (and in fact, drop the \"len\" variable entirely), as in the error\n>> case we'd simply have an empty signature->len.\n>>\n>> Your patch effectively swaps out \"did we get any data\" for \"did we get\n>> the data we expect\", which is what those \"len\" checks were doing.\n>>\n>> -Peff\n>>\n> \n> My patch actually makes several tests fail, sorry. (I did check before\n> that I can still create signatures...) Maybe my offset in buf is wrong.\n> \n> starts_with, yes.\n> \n> Can't check any further now, sorry. But we do check for the\n> PGP_SIGNATURE in our signed objects anyways. So I feel that we can either\n> \n> - tighthen the check for valid gpg signatures\n> \n> or\n> \n> - make our signature interface completely pluggable.\n> \n> We can't have it both ways, but at least things are localised in\n> gpg-interface.c now.\n> \n> The proposed patch is just some consistency check that does not rely on\n> gpg.program itself(!), or else we could simply call verify.\n> \n> Michael\n\nSo, with\n\n!starts_with(signature->buf+bottom, PGP_SIGNATURE)\n\neverything is fine except our tests for RFC1991 signatures.\nSigh...\n\nI'll resend a patch that uses parse_signature so that all gpg specifics\nare localised there.\n\nMichael\n"},{"id":"289213","messageId":"b057610446674c3323fbba09494229621ec4c032.1465905749.git.git@drmicha.warpmail.net","threadId":"42600","inReplyTo":"26353a3d-e495-075f-4f84-b34a2420a6cf@drmicha.warpmail.net","subject":"[PATCHv2] gpg-interface: check gpg signature for correct header","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-06-14T12:05:05Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"When we create a signature, it may happen that gpg returns with\n\"success\" but not with an actual detached signature on stdout.\n\nCheck for the correct header to catch these cases better. We use the\nsame parse_signature function for that that we use otherwise, so that\ngpg specifics are localised there.\n\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\nSo, this is the real thing.\n\nBetween you and me: parse_signature in fact is more lenient, but hey - it's\nexactly as lenient as we are otherwise, bar running gpg --verify.\n\n gpg-interface.c |  2 +-\n t/t7004-tag.sh  | 10 +++++++++-\n 2 files changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex c4b1e8c..784953c 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -185,7 +185,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n \n \tsigchain_pop(SIGPIPE);\n \n-\tif (finish_command(&gpg) || !len || len < 0)\n+\tif (finish_command(&gpg) || !len || len < 0 || parse_signature(signature->buf, signature->len) == signature->len)\n \t\treturn error(_(\"gpg failed to sign the data\"));\n \n \t/* Strip CR from the line endings, in case we are on Windows. */\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex f9b7d79..467e968 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1202,10 +1202,18 @@ test_expect_success GPG,RFC1991 \\\n # try to sign with bad user.signingkey\n git config user.signingkey BobTheMouse\n test_expect_success GPG \\\n-\t'git tag -s fails if gpg is misconfigured' \\\n+\t'git tag -s fails if gpg is misconfigured (bad key)' \\\n \t'test_must_fail git tag -s -m tail tag-gpg-failure'\n git config --unset user.signingkey\n \n+# try to produce invalid signature\n+git config gpg.program echo\n+test_expect_success GPG \\\n+\t'git tag -s fails if gpg is misconfigured (bad signature format)' \\\n+\t'test_must_fail git tag -s -m tail tag-gpg-failure'\n+git config --unset gpg.program\n+\n+\n # try to verify without gpg:\n \n rm -rf gpghome\n-- \n2.9.0.382.g87fd384\n"},{"id":"289216","messageId":"8e08b63b58302b6e7fe91f0dfb5b476781bfd37d.1465915311.git.git@drmicha.warpmail.net","threadId":"42600","inReplyTo":"26353a3d-e495-075f-4f84-b34a2420a6cf@drmicha.warpmail.net","subject":"[PATCHv3] gpg-interface: check gpg signature creation status","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-06-14T14:44:18Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"When we create a signature, it may happen that gpg returns with\n\"success\" but not with an actual detached signature on stdout.\n\nCheck for the correct signature creation status to catch these cases\nbetter. Really, --status-fd parsing is the only way to check gpg status\nreliably. We do the same for verify already.\n\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\nThat must be the real real thing now...\n\n gpg-interface.c | 22 +++++++++++++++-------\n t/t7004-tag.sh  | 10 +++++++++-\n 2 files changed, 24 insertions(+), 8 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex c4b1e8c..850dc81 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -150,17 +150,19 @@ const char *get_signing_key(void)\n int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)\n {\n \tstruct child_process gpg = CHILD_PROCESS_INIT;\n-\tconst char *args[4];\n-\tssize_t len;\n+\tconst char *args[5];\n \tsize_t i, j, bottom;\n+\tstruct strbuf err = STRBUF_INIT;\n \n \tgpg.argv = args;\n \tgpg.in = -1;\n \tgpg.out = -1;\n+\tgpg.err = -1;\n \targs[0] = gpg_program;\n-\targs[1] = \"-bsau\";\n-\targs[2] = signing_key;\n-\targs[3] = NULL;\n+\targs[1] = \"--status-fd=2\";\n+\targs[2] = \"-bsau\";\n+\targs[3] = signing_key;\n+\targs[4] = NULL;\n \n \tif (start_command(&gpg))\n \t\treturn error(_(\"could not run gpg.\"));\n@@ -174,19 +176,25 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n \tif (write_in_full(gpg.in, buffer->buf, buffer->len) != buffer->len) {\n \t\tclose(gpg.in);\n \t\tclose(gpg.out);\n+\t\tclose(gpg.err);\n \t\tfinish_command(&gpg);\n \t\treturn error(_(\"gpg did not accept the data\"));\n \t}\n \tclose(gpg.in);\n \n \tbottom = signature->len;\n-\tlen = strbuf_read(signature, gpg.out, 1024);\n+\tstrbuf_read(signature, gpg.out, 1024);\n+\tstrbuf_read(&err, gpg.err, 0);\n \tclose(gpg.out);\n+\tclose(gpg.err);\n \n \tsigchain_pop(SIGPIPE);\n \n-\tif (finish_command(&gpg) || !len || len < 0)\n+\tif (finish_command(&gpg) || !strstr(err.buf, \"\\n[GNUPG:] SIG_CREATED \")) {\n+\t\tstrbuf_release(&err);\n \t\treturn error(_(\"gpg failed to sign the data\"));\n+\t}\n+\tstrbuf_release(&err);\n \n \t/* Strip CR from the line endings, in case we are on Windows. */\n \tfor (i = j = bottom; i < signature->len; i++)\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex f9b7d79..467e968 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1202,10 +1202,18 @@ test_expect_success GPG,RFC1991 \\\n # try to sign with bad user.signingkey\n git config user.signingkey BobTheMouse\n test_expect_success GPG \\\n-\t'git tag -s fails if gpg is misconfigured' \\\n+\t'git tag -s fails if gpg is misconfigured (bad key)' \\\n \t'test_must_fail git tag -s -m tail tag-gpg-failure'\n git config --unset user.signingkey\n \n+# try to produce invalid signature\n+git config gpg.program echo\n+test_expect_success GPG \\\n+\t'git tag -s fails if gpg is misconfigured (bad signature format)' \\\n+\t'test_must_fail git tag -s -m tail tag-gpg-failure'\n+git config --unset gpg.program\n+\n+\n # try to verify without gpg:\n \n rm -rf gpghome\n-- \n2.9.0.382.g87fd384\n"},{"id":"289237","messageId":"xmqq60tbaba5.fsf@gitster.mtv.corp.google.com","threadId":"42600","inReplyTo":"8e08b63b58302b6e7fe91f0dfb5b476781bfd37d.1465915311.git.git@drmicha.warpmail.net","subject":"Re: [PATCHv3] gpg-interface: check gpg signature creation status","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-14T18:13:54Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@drmicha.warpmail.net> writes:\n\n>  \tbottom = signature->len;\n> -\tlen = strbuf_read(signature, gpg.out, 1024);\n> +\tstrbuf_read(signature, gpg.out, 1024);\n> +\tstrbuf_read(&err, gpg.err, 0);\n\nHmmmm, isn't this asking for a deadlock?  When GPG spews more than\nwhat would fit in a pipe buffer to its standard error (hence gets\nblocked), its standard output may not complete, and the we would get\nstuck by attempting to read from gpg.out, failing to reach the other\nstrbuf_read() that would unblock GPG by reading from gpg.err?\n"},{"id":"289260","messageId":"20160614215019.GB22334@sigill.intra.peff.net","threadId":"42600","inReplyTo":"xmqq60tbaba5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv3] gpg-interface: check gpg signature creation status","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-14T21:50:19Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 14, 2016 at 11:13:54AM -0700, Junio C Hamano wrote:\n\n> Michael J Gruber <git@drmicha.warpmail.net> writes:\n> \n> >  \tbottom = signature->len;\n> > -\tlen = strbuf_read(signature, gpg.out, 1024);\n> > +\tstrbuf_read(signature, gpg.out, 1024);\n> > +\tstrbuf_read(&err, gpg.err, 0);\n> \n> Hmmmm, isn't this asking for a deadlock?  When GPG spews more than\n> what would fit in a pipe buffer to its standard error (hence gets\n> blocked), its standard output may not complete, and the we would get\n> stuck by attempting to read from gpg.out, failing to reach the other\n> strbuf_read() that would unblock GPG by reading from gpg.err?\n\nYeah, it definitely is a deadlock. I think we'd need a select loop to\nread into multiple strbufs at once (we can't use \"struct async\" because\nthat might happen in another process).\n\n-Peff\n"},{"id":"289265","messageId":"20160614222633.GA32020@sigill.intra.peff.net","threadId":"42600","inReplyTo":"20160614215019.GB22334@sigill.intra.peff.net","subject":"Re: [PATCHv3] gpg-interface: check gpg signature creation status","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-14T22:26:33Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 14, 2016 at 05:50:19PM -0400, Jeff King wrote:\n\n> On Tue, Jun 14, 2016 at 11:13:54AM -0700, Junio C Hamano wrote:\n> \n> > Michael J Gruber <git@drmicha.warpmail.net> writes:\n> > \n> > >  \tbottom = signature->len;\n> > > -\tlen = strbuf_read(signature, gpg.out, 1024);\n> > > +\tstrbuf_read(signature, gpg.out, 1024);\n> > > +\tstrbuf_read(&err, gpg.err, 0);\n> > \n> > Hmmmm, isn't this asking for a deadlock?  When GPG spews more than\n> > what would fit in a pipe buffer to its standard error (hence gets\n> > blocked), its standard output may not complete, and the we would get\n> > stuck by attempting to read from gpg.out, failing to reach the other\n> > strbuf_read() that would unblock GPG by reading from gpg.err?\n> \n> Yeah, it definitely is a deadlock. I think we'd need a select loop to\n> read into multiple strbufs at once (we can't use \"struct async\" because\n> that might happen in another process).\n\nSomething like this on top of Michael's patch (only lightly tested).\n\nI'm still undecided on whether it is a better approach than making sure\nthe stdout we got looks sane. In particular I'd worry that it would make\nthings harder for somebody trying to plug in something gpg-like (e.g.,\nif you wanted to do something exotic like call a program which fetched\nthe signature from a remote device or something). But it's probably not\n_that_ hard for such a script to emulate --status-fd.\n\n---\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 850dc81..576e462 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -153,6 +153,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n \tconst char *args[5];\n \tsize_t i, j, bottom;\n \tstruct strbuf err = STRBUF_INIT;\n+\tstruct strbuf_reader readers[2];\n \n \tgpg.argv = args;\n \tgpg.in = -1;\n@@ -183,8 +184,15 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n \tclose(gpg.in);\n \n \tbottom = signature->len;\n-\tstrbuf_read(signature, gpg.out, 1024);\n-\tstrbuf_read(&err, gpg.err, 0);\n+\n+\treaders[0].buf = signature;\n+\treaders[0].fd = gpg.out;\n+\treaders[0].hint = 1024;\n+\treaders[1].buf = &err;\n+\treaders[1].fd = gpg.err;\n+\treaders[1].hint = 1024;\n+\tstrbuf_read_parallel(readers, 2);\n+\n \tclose(gpg.out);\n \tclose(gpg.err);\n \ndiff --git a/strbuf.c b/strbuf.c\nindex 1ba600b..f674b23 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -395,6 +395,58 @@ ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint)\n \treturn cnt;\n }\n \n+void strbuf_read_parallel(struct strbuf_reader *readers, int nr)\n+{\n+\tint i;\n+\tstruct pollfd *pfd;\n+\n+\tALLOC_ARRAY(pfd, nr);\n+\tfor (i = 0; i < nr; i++)\n+\t\treaders[i].error = 0;\n+\n+\twhile (1) {\n+\t\tint pollsize = 0;\n+\t\tint ret;\n+\n+\t\tfor (i = 0; i < nr; i++) {\n+\t\t\tif (readers[i].fd < 0)\n+\t\t\t\tcontinue;\n+\t\t\tpfd[pollsize].fd = readers[i].fd;\n+\t\t\tpfd[pollsize].events = POLLIN;\n+\t\t\treaders[i].pfd = &pfd[pollsize];\n+\t\t\tpollsize++;\n+\t\t}\n+\n+\t\tif (!pollsize)\n+\t\t\tbreak;\n+\n+\t\tret = poll(pfd, pollsize, -1);\n+\t\tif (ret < 0) {\n+\t\t\tif (errno == EINTR)\n+\t\t\t\tcontinue;\n+\t\t\t/* should never happen? */\n+\t\t\tdie_errno(\"poll failed\");\n+\t\t}\n+\n+\t\tfor (i = 0; i < nr; i++) {\n+\t\t\tif (readers[i].fd < 0)\n+\t\t\t\tcontinue;\n+\t\t\tif (readers[i].pfd->revents &\n+\t\t\t    (POLLIN|POLLHUP|POLLERR|POLLNVAL)) {\n+\t\t\t\tret = strbuf_read_once(readers[i].buf,\n+\t\t\t\t\t\t       readers[i].fd,\n+\t\t\t\t\t\t       readers[i].hint);\n+\t\t\t\tif (ret < 0)\n+\t\t\t\t\treaders[i].error = errno;\n+\t\t\t\tif (ret <= 0)\n+\t\t\t\t\treaders[i].fd = -1;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tfree(pfd);\n+}\n+\n ssize_t strbuf_write(struct strbuf *sb, FILE *f)\n {\n \treturn sb->len ? fwrite(sb->buf, 1, sb->len, f) : 0;\ndiff --git a/strbuf.h b/strbuf.h\nindex 7987405..b93822e 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -374,6 +374,25 @@ extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint);\n  */\n extern ssize_t strbuf_read_once(struct strbuf *, int fd, size_t hint);\n \n+/**\n+ * Read from several descriptors in parallel, each into its own strbuf.\n+ * This can be used, for example, to capture stdout and stderr from a\n+ * sub-process without worrying about deadlocks.\n+ */\n+struct strbuf_reader {\n+\t/* Initialized by caller */\n+\tstruct strbuf *buf;\n+\tint fd;\n+\tsize_t hint;\n+\n+\t/* Returned by strbuf_read_parallel */\n+\tint error; /* 0 for success, otherwise errno */\n+\n+\t/* Internal use */\n+\tstruct pollfd *pfd;\n+};\n+void strbuf_read_parallel(struct strbuf_reader *readers, int nr);\n+\n /**\n  * Read the contents of a file, specified by its path. The third argument\n  * can be used to give a hint about the file size, to avoid reallocs.\n"},{"id":"289268","messageId":"xmqqporj72p4.fsf@gitster.mtv.corp.google.com","threadId":"42600","inReplyTo":"20160614222633.GA32020@sigill.intra.peff.net","subject":"Re: [PATCHv3] gpg-interface: check gpg signature creation status","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-14T23:47:35Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I'm still undecided on whether it is a better approach than making\n> sure the stdout we got looks sane. In particular I'd worry that it\n> would make things harder for somebody trying to plug in something\n> gpg-like (e.g., if you wanted to do something exotic like call a\n> program which fetched the signature from a remote device or\n> something).  But it's probably not _that_ hard for such a script\n> to emulate --status-fd.\n\nI share the same thinking, but at the same time, it already is a\nrequirement to give --status-fd output that is close enough on the\nsignature verification side, isn't it?\n"},{"id":"289269","messageId":"20160615005614.GB32601@sigill.intra.peff.net","threadId":"42600","inReplyTo":"xmqqporj72p4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv3] gpg-interface: check gpg signature creation status","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-15T00:56:14Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 14, 2016 at 04:47:35PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > I'm still undecided on whether it is a better approach than making\n> > sure the stdout we got looks sane. In particular I'd worry that it\n> > would make things harder for somebody trying to plug in something\n> > gpg-like (e.g., if you wanted to do something exotic like call a\n> > program which fetched the signature from a remote device or\n> > something).  But it's probably not _that_ hard for such a script\n> > to emulate --status-fd.\n> \n> I share the same thinking, but at the same time, it already is a\n> requirement to give --status-fd output that is close enough on the\n> signature verification side, isn't it?\n\nYeah, though I could see somebody wanting to sit amidst the signing side\nbut not verification (e.g., if your keys are elsewhere from the machine\nrunning git). Of course such a case could probably ferry --status-fd\nfrom the other side anyway.\n\nI admit I don't know of such a case in practice, though, and\nimplementing a rudimentary --status-fd to say \"SIG OK\" or whatever on\nthe signing side is not _that_ big a deal. So if we think this approach\nis a more robust solution in the normal case, let's not hold it up over\nwhat-ifs.\n\n-Peff\n"},{"id":"289271","messageId":"20160615032804.GA2249@sigill.intra.peff.net","threadId":"42600","inReplyTo":"20160614222633.GA32020@sigill.intra.peff.net","subject":"Re: [PATCHv3] gpg-interface: check gpg signature creation status","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-15T03:28:04Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 14, 2016 at 06:26:33PM -0400, Jeff King wrote:\n\n> > > >  \tbottom = signature->len;\n> > > > -\tlen = strbuf_read(signature, gpg.out, 1024);\n> > > > +\tstrbuf_read(signature, gpg.out, 1024);\n> > > > +\tstrbuf_read(&err, gpg.err, 0);\n> > > \n> > > Hmmmm, isn't this asking for a deadlock?  When GPG spews more than\n> > > what would fit in a pipe buffer to its standard error (hence gets\n> > > blocked), its standard output may not complete, and the we would get\n> > > stuck by attempting to read from gpg.out, failing to reach the other\n> > > strbuf_read() that would unblock GPG by reading from gpg.err?\n> > \n> > Yeah, it definitely is a deadlock. I think we'd need a select loop to\n> > read into multiple strbufs at once (we can't use \"struct async\" because\n> > that might happen in another process).\n> \n> Something like this on top of Michael's patch (only lightly tested).\n\nI wondered if this is something we might run into in other places, but\njust haven't in practice. After grepping existing calls to strbuf_read(),\nI think the only other case is the one in verify_signed_buffer(), which\nreads all of stderr before trying to read stdout. I suspect that _could_\ndeadlock but doesn't tend to in practice.\n\nInterestingly, it also writes in full to gpg's stdin before reading\nanything. If gpg buffers all of its input before writing, that's fine,\nbut in theory it does not have to. I have no idea if it's possible for\nit to be a problem in practice.\n\nThat can be solved in the same select loop, though obviously it's not\njust strbuf_read_parallel() at that point, but rather a kind of a\nfeed_and_capture_command().\n\n-Peff\n"},{"id":"289274","messageId":"CAGrdoOoxphU0tRjV22yduXrhmOSyCc_zVnhuO9fFA6UPRd9WiA@mail.gmail.com","threadId":"42600","inReplyTo":"e129da8e-4e1d-c535-ca62-d2a4c2e23799@drmicha.warpmail.net","subject":"Re: I lost my commit signature","fromName":"ZhenTian","fromEmail":"loooseleaves@gmail.com","sentAt":"2016-06-15T04:27:15Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":false,"sender":{"key":"loooseleaves@gmail.com","avatar":"https://gravatar.com/avatar/bc445fa477ccc779cf8752086b2e56f6c8fc776d4a3b19d8cd318dbb0ea895b5?d=mp&s=160"},"body":"Hi Michael and Peff,\n\nI got two more lines from gpg -v during commit with -S:\n```\ngpg: writing to stdout\ngpg: RSA/SHA1 signature from: \"2EF2AD6E Tian Zhen <tianzhen@honovation.com>\"\n```\n\nafter I commit, I push it to remote, but someone had pushed before to\nmaster branch, so I pull on master branch(`git pull --rebase`), then I\ncheck my commit via `git log --show-signature`, there is no signature\nin it, so I commit it with --ament and -S again, the signature is come\nback.\n\nI haven't check signature before push, because I have checked four\ncommits before, every commit is fine.\n\nI don't know whether the `git pull` influenced signature or not.\n\nMy signature is just like Schrodinger's cat, when I check it, it lost :)\n-Schrödinger\n\n\nOn Tue, Jun 14, 2016 at 6:57 PM, Michael J Gruber\n<git@drmicha.warpmail.net> wrote:\n> Jeff King venit, vidit, dixit 14.06.2016 11:41:\n>> On Tue, Jun 14, 2016 at 04:39:38PM +0800, ZhenTian wrote:\n>>\n>>> I want to set gpg -v to pgp.program, but if I set it, it can't call gpg:\n>>> ```\n>>> error: cannot run gpg -v: No such file or directory\n>>> error: could not run gpg.\n>>> fatal: failed to write commit object\n>>> ```\n>>>\n>>> I have tried set gpg.program value to `gpg|/tmp/log`, `/usr/bin/gpg\n>>> -v`, `gpg -v`, `\"/usr/bin/gpg -v\"`\n>>>\n>>> only after I set to `gpg` or `/usr/bin/gpg` without any argument, it will work.\n>>\n>> Ah, right. Most of the time we run such programs as shell commands, but\n>> it looks like we do not. So you'd have to do something like:\n>>\n>>       cat >/tmp/fake-gpg <<-\\EOF\n>>       #!/bin/sh\n>>       gpg -v \"$@\"\n>>       EOF\n>>       chmod +x /tmp/fake-gpg\n>>       git config gpg.program /tmp/fake-gpg\n>>\n>> -Peff\n>>\n>\n> The content of \"gpg.program\" is used as argv[0] when we build up various\n> commands to be run; we expect it to heed standard gpg options.\n>\n> On the other hand:\n>\n> git -c gpg.program=echo commit -S\n>\n> 'successfully' creates a commit that has\n>\n> gpgsig -bsau Michael J Gruber <mickey@mouse.dis>\n>\n> as the last header line. gpg.program=true fails (as does cat, unhappy\n> with the options), so apparently we do some error checking but not enough.\n>\n> Michael\n"},{"id":"289275","messageId":"20160615043450.GA3975@sigill.intra.peff.net","threadId":"42600","inReplyTo":"CAGrdoOoxphU0tRjV22yduXrhmOSyCc_zVnhuO9fFA6UPRd9WiA@mail.gmail.com","subject":"Re: I lost my commit signature","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-15T04:34:50Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 15, 2016 at 12:27:15PM +0800, ZhenTian wrote:\n\n> I got two more lines from gpg -v during commit with -S:\n> ```\n> gpg: writing to stdout\n> gpg: RSA/SHA1 signature from: \"2EF2AD6E Tian Zhen <tianzhen@honovation.com>\"\n> ```\n> \n> after I commit, I push it to remote, but someone had pushed before to\n> master branch, so I pull on master branch(`git pull --rebase`), then I\n> check my commit via `git log --show-signature`, there is no signature\n> in it, so I commit it with --ament and -S again, the signature is come\n> back.\n> \n> I haven't check signature before push, because I have checked four\n> commits before, every commit is fine.\n> \n> I don't know whether the `git pull` influenced signature or not.\n\nAh, so the problem is probably that you had a signature _initially_, but\nthat it did not survive the rebase. Which makes sense, as rebase would\nneed to re-sign.  It does not by default, but you can tell it to do so\nwith \"-S\". Or you can set `commit.gpgsign`, which should sign in both\ncases.\n\n-Peff\n"},{"id":"289278","messageId":"be771366-00a7-6c20-2623-5fa54b8e19b7@drmicha.warpmail.net","threadId":"42600","inReplyTo":"20160615043450.GA3975@sigill.intra.peff.net","subject":"Re: I lost my commit signature","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-06-15T07:07:16Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 15.06.2016 06:34:\n> On Wed, Jun 15, 2016 at 12:27:15PM +0800, ZhenTian wrote:\n> \n>> I got two more lines from gpg -v during commit with -S:\n>> ```\n>> gpg: writing to stdout\n>> gpg: RSA/SHA1 signature from: \"2EF2AD6E Tian Zhen <tianzhen@honovation.com>\"\n>> ```\n>>\n>> after I commit, I push it to remote, but someone had pushed before to\n>> master branch, so I pull on master branch(`git pull --rebase`), then I\n>> check my commit via `git log --show-signature`, there is no signature\n>> in it, so I commit it with --ament and -S again, the signature is come\n>> back.\n>>\n>> I haven't check signature before push, because I have checked four\n>> commits before, every commit is fine.\n>>\n>> I don't know whether the `git pull` influenced signature or not.\n> \n> Ah, so the problem is probably that you had a signature _initially_, but\n> that it did not survive the rebase. Which makes sense, as rebase would\n> need to re-sign.  It does not by default, but you can tell it to do so\n> with \"-S\". Or you can set `commit.gpgsign`, which should sign in both\n> cases.\n\nWhile it's clear that a rebase invalidates the signature, we could try\nto be more helpful here, especially given the fact that (with our model)\nyou can't sign a commit afterwards any more.\n\ncommit.gpgsign signs everything, but there should be a mode for\nre-signing signed commits, or at least a warning that rebase dropped a\nsignature so that you can --amend -S the last commit.\n\nMichael\n"},{"id":"289279","messageId":"75709ea6-b2bb-fddf-1175-cb8aae66629a@drmicha.warpmail.net","threadId":"42600","inReplyTo":"20160615005614.GB32601@sigill.intra.peff.net","subject":"Re: [PATCHv3] gpg-interface: check gpg signature creation status","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-06-15T07:17:54Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 15.06.2016 02:56:\n> On Tue, Jun 14, 2016 at 04:47:35PM -0700, Junio C Hamano wrote:\n> \n>> Jeff King <peff@peff.net> writes:\n>>\n>>> I'm still undecided on whether it is a better approach than making\n>>> sure the stdout we got looks sane. In particular I'd worry that it\n>>> would make things harder for somebody trying to plug in something\n>>> gpg-like (e.g., if you wanted to do something exotic like call a\n>>> program which fetched the signature from a remote device or\n>>> something).  But it's probably not _that_ hard for such a script\n>>> to emulate --status-fd.\n>>\n>> I share the same thinking, but at the same time, it already is a\n>> requirement to give --status-fd output that is close enough on the\n>> signature verification side, isn't it?\n> \n> Yeah, though I could see somebody wanting to sit amidst the signing side\n> but not verification (e.g., if your keys are elsewhere from the machine\n> running git). Of course such a case could probably ferry --status-fd\n> from the other side anyway.\n> \n> I admit I don't know of such a case in practice, though, and\n> implementing a rudimentary --status-fd to say \"SIG OK\" or whatever on\n> the signing side is not _that_ big a deal. So if we think this approach\n> is a more robust solution in the normal case, let's not hold it up over\n> what-ifs.\n> \n> -Peff\n\nAs for the flexibility:\nWe do code specifically for gpg, which happens to work for gpg2 also.\nThe patch doesn't add any gpg ui requirements that we don't require\nelsewhere already.\nMore flexibility requires a completely pluggable approach - gpgsm\nalready fails to meet the gpg command line ui.\n\nIn any case, \"status-fd\" is *the* way to interface with gpg reliably\njust like plumbing commands are *the* way to interface with git reliably.\n\nAs for the read locking:\nI'm sorry I have no idea about that area at all. I thought that I'm\ndoing the same that we do for verify, but apparently not. My\nstrbuf_read-fu is not that strong either (read: copy&paste). I trust\nyour assessment completely, though.\n\nAs for the original problem:\nThat had a different cause, as we know now (rebase dropping signatures\nwithout hint). I still think we should check gpg status codes properly.\n\nMichael\n"},{"id":"289284","messageId":"CAGrdoOrJEVtgmUQNYQh3MKMyBYGE6otSyHDJORfXMy0G-Ctjkg@mail.gmail.com","threadId":"42600","inReplyTo":"be771366-00a7-6c20-2623-5fa54b8e19b7@drmicha.warpmail.net","subject":"Re: I lost my commit signature","fromName":"ZhenTian","fromEmail":"loooseleaves@gmail.com","sentAt":"2016-06-15T10:36:42Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":false,"sender":{"key":"loooseleaves@gmail.com","avatar":"https://gravatar.com/avatar/bc445fa477ccc779cf8752086b2e56f6c8fc776d4a3b19d8cd318dbb0ea895b5?d=mp&s=160"},"body":"Thank you all very much!\n-Schrödinger\n\n\nOn Wed, Jun 15, 2016 at 3:07 PM, Michael J Gruber\n<git@drmicha.warpmail.net> wrote:\n> Jeff King venit, vidit, dixit 15.06.2016 06:34:\n>> On Wed, Jun 15, 2016 at 12:27:15PM +0800, ZhenTian wrote:\n>>\n>>> I got two more lines from gpg -v during commit with -S:\n>>> ```\n>>> gpg: writing to stdout\n>>> gpg: RSA/SHA1 signature from: \"2EF2AD6E Tian Zhen <tianzhen@honovation.com>\"\n>>> ```\n>>>\n>>> after I commit, I push it to remote, but someone had pushed before to\n>>> master branch, so I pull on master branch(`git pull --rebase`), then I\n>>> check my commit via `git log --show-signature`, there is no signature\n>>> in it, so I commit it with --ament and -S again, the signature is come\n>>> back.\n>>>\n>>> I haven't check signature before push, because I have checked four\n>>> commits before, every commit is fine.\n>>>\n>>> I don't know whether the `git pull` influenced signature or not.\n>>\n>> Ah, so the problem is probably that you had a signature _initially_, but\n>> that it did not survive the rebase. Which makes sense, as rebase would\n>> need to re-sign.  It does not by default, but you can tell it to do so\n>> with \"-S\". Or you can set `commit.gpgsign`, which should sign in both\n>> cases.\n>\n> While it's clear that a rebase invalidates the signature, we could try\n> to be more helpful here, especially given the fact that (with our model)\n> you can't sign a commit afterwards any more.\n>\n> commit.gpgsign signs everything, but there should be a mode for\n> re-signing signed commits, or at least a warning that rebase dropped a\n> signature so that you can --amend -S the last commit.\n>\n> Michael\n"},{"id":"289337","messageId":"20160616092535.GA29954@sigill.intra.peff.net","threadId":"42600","inReplyTo":"75709ea6-b2bb-fddf-1175-cb8aae66629a@drmicha.warpmail.net","subject":"Re: [PATCHv3] gpg-interface: check gpg signature creation status","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-16T09:25:36Z","receivedAt":"2016-06-16T09:25:46Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 15, 2016 at 09:17:54AM +0200, Michael J Gruber wrote:\n\n> As for the flexibility:\n> We do code specifically for gpg, which happens to work for gpg2 also.\n> The patch doesn't add any gpg ui requirements that we don't require\n> elsewhere already.\n> More flexibility requires a completely pluggable approach - gpgsm\n> already fails to meet the gpg command line ui.\n\nDoes it? I haven't run it in production, but I did some tests with gpgsm\na few months ago and found it to be a drop-in replacement, at least with\nrespect to git. Though I don't think that matters one way or the other\nfor the current discussion; it _does_ do --status-fd.\n\n> In any case, \"status-fd\" is *the* way to interface with gpg reliably\n> just like plumbing commands are *the* way to interface with git reliably.\n\nFair enough. I've generally found its exit code pretty reliable. It's\nunclear to me if the problem you saw was because gpg was exiting 0 but\nproducing no signature, or if your debugging was masking its exit code.\n\nEither way, I do not mind using --status-fd; it seems like it should be\nmore robust in general. It's the tip commit in the series I'm about to\npost.\n\n> As for the read locking:\n> I'm sorry I have no idea about that area at all. I thought that I'm\n> doing the same that we do for verify, but apparently not. My\n> strbuf_read-fu is not that strong either (read: copy&paste). I trust\n> your assessment completely, though.\n\nYeah, you're right that the deadlock thing is also a possibility on the\nverification side. I'm not sure whether it's possible to trigger in\npractice or not. See the analysis in the series.\n\n> As for the original problem:\n> That had a different cause, as we know now (rebase dropping signatures\n> without hint). I still think we should check gpg status codes properly.\n\nYep, I agree.\n\n-Peff\n"},{"id":"289354","messageId":"0a9d4b3b-6fc9-a539-0256-3eb09c4791c0@drmicha.warpmail.net","threadId":"42600","inReplyTo":"20160616092535.GA29954@sigill.intra.peff.net","subject":"Re: [PATCHv3] gpg-interface: check gpg signature creation status","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-06-16T11:30:57Z","receivedAt":"2016-06-16T11:31:04Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 16.06.2016 11:25:\n> On Wed, Jun 15, 2016 at 09:17:54AM +0200, Michael J Gruber wrote:\n> \n>> As for the flexibility:\n>> We do code specifically for gpg, which happens to work for gpg2 also.\n>> The patch doesn't add any gpg ui requirements that we don't require\n>> elsewhere already.\n>> More flexibility requires a completely pluggable approach - gpgsm\n>> already fails to meet the gpg command line ui.\n> \n> Does it? I haven't run it in production, but I did some tests with gpgsm\n> a few months ago and found it to be a drop-in replacement, at least with\n> respect to git. Though I don't think that matters one way or the other\n> for the current discussion; it _does_ do --status-fd.\n\nYou are right!\n\nI solely trusted in the documentation rather than simply trying it out.\ngpgsm even supports the undocumented short options.\n\nOnly the resulting header is different (plus the fact that I have to\nturn of crl checks for whatever reason).\n\n>> In any case, \"status-fd\" is *the* way to interface with gpg reliably\n>> just like plumbing commands are *the* way to interface with git reliably.\n> \n> Fair enough. I've generally found its exit code pretty reliable. It's\n> unclear to me if the problem you saw was because gpg was exiting 0 but\n> producing no signature, or if your debugging was masking its exit code.\n> \n> Either way, I do not mind using --status-fd; it seems like it should be\n> more robust in general. It's the tip commit in the series I'm about to\n> post.\n> \n>> As for the read locking:\n>> I'm sorry I have no idea about that area at all. I thought that I'm\n>> doing the same that we do for verify, but apparently not. My\n>> strbuf_read-fu is not that strong either (read: copy&paste). I trust\n>> your assessment completely, though.\n> \n> Yeah, you're right that the deadlock thing is also a possibility on the\n> verification side. I'm not sure whether it's possible to trigger in\n> practice or not. See the analysis in the series.\n\nWith great admiration I've looked at your series which solves this\nproblem at the root. Way above my head (the solution, not the root,\nfortunately).\n\n>> As for the original problem:\n>> That had a different cause, as we know now (rebase dropping signatures\n>> without hint). I still think we should check gpg status codes properly.\n> \n> Yep, I agree.\n> \n> -Peff\n> \n\n"},{"id":"289372","messageId":"xmqqr3bx5ahp.fsf@gitster.mtv.corp.google.com","threadId":"42600","inReplyTo":"20160616073410.GA651@sigill.intra.peff.net","subject":"Re: I lost my commit signature","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-16T17:06:42Z","receivedAt":"2016-06-16T17:06:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> But why does somebody run \"commit -S\" for a single commit, but not all\n> the time? Is it because that commit is special? Or is that particular\n> moment special? One implies that it's important for the signature to be\n> retained during a rebase, and one does not.\n>\n> So I dunno. I would not be opposed to such a feature, but I'm having\n> trouble figuring out why it would be useful (though for the most part, I\n> do not see why anything but per-project commit.gpgSign config is\n> particularly useful. Maybe I just lack imagination).\n\nI am not so imaginative, either. One remotely plausible use case may\nbe a project that has two classes of paths (let's call these classes\nsensitive and others), and requires its participants to sign commits\nthat touch sensitive paths.  The user needs something finter grained\nthan per-project commit.gpgSign there.\n\nBut even in such a case, the fact that an original commit is with a\nsignature should not be a good indication that the rewritten version\nof that commit in the new history still touches the sensitive paths\nthat required the original to be signed (i.e. the history the user\nis rebasing onto may already have the necessary changes to these\npaths).\n\nSo, I dunno, either.\n"},{"id":"289393","messageId":"20160616073410.GA651@sigill.intra.peff.net","threadId":"42600","inReplyTo":"be771366-00a7-6c20-2623-5fa54b8e19b7@drmicha.warpmail.net","subject":"Re: I lost my commit signature","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-16T07:34:10Z","receivedAt":"2016-06-16T19:57:55Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 15, 2016 at 09:07:16AM +0200, Michael J Gruber wrote:\n\n> > Ah, so the problem is probably that you had a signature _initially_, but\n> > that it did not survive the rebase. Which makes sense, as rebase would\n> > need to re-sign.  It does not by default, but you can tell it to do so\n> > with \"-S\". Or you can set `commit.gpgsign`, which should sign in both\n> > cases.\n> \n> While it's clear that a rebase invalidates the signature, we could try\n> to be more helpful here, especially given the fact that (with our model)\n> you can't sign a commit afterwards any more.\n> \n> commit.gpgsign signs everything, but there should be a mode for\n> re-signing signed commits, or at least a warning that rebase dropped a\n> signature so that you can --amend -S the last commit.\n\nI had a similar thought, though I'm not sure how useful a \"re-sign\nsigned commits\" mode would be in practice. Mostly because I'm not sure\nwhy signing would be more important for one commit versus another.\n\nThat is, I can see why somebody would set \"commit.gpgSign\"; their\npreference (or that of their project) is to sign commits, and they've\nset up gpg, etc, to make it relatively painless.\n\nBut why does somebody run \"commit -S\" for a single commit, but not all\nthe time? Is it because that commit is special? Or is that particular\nmoment special? One implies that it's important for the signature to be\nretained during a rebase, and one does not.\n\nSo I dunno. I would not be opposed to such a feature, but I'm having\ntrouble figuring out why it would be useful (though for the most part, I\ndo not see why anything but per-project commit.gpgSign config is\nparticularly useful. Maybe I just lack imagination).\n\n-Peff\n"},{"id":"289429","messageId":"171543e4-a343-c6d2-aa9a-2098db5b42ec@drmicha.warpmail.net","threadId":"42600","inReplyTo":"xmqqr3bx5ahp.fsf@gitster.mtv.corp.google.com","subject":"Re: I lost my commit signature","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-06-17T08:18:04Z","receivedAt":"2016-06-17T08:18:15Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 16.06.2016 19:06:\n> Jeff King <peff@peff.net> writes:\n> \n>> But why does somebody run \"commit -S\" for a single commit, but not all\n>> the time? Is it because that commit is special? Or is that particular\n>> moment special? One implies that it's important for the signature to be\n>> retained during a rebase, and one does not.\n>>\n>> So I dunno. I would not be opposed to such a feature, but I'm having\n>> trouble figuring out why it would be useful (though for the most part, I\n>> do not see why anything but per-project commit.gpgSign config is\n>> particularly useful. Maybe I just lack imagination).\n> \n> I am not so imaginative, either. One remotely plausible use case may\n> be a project that has two classes of paths (let's call these classes\n> sensitive and others), and requires its participants to sign commits\n> that touch sensitive paths.  The user needs something finter grained\n> than per-project commit.gpgSign there.\n> \n> But even in such a case, the fact that an original commit is with a\n> signature should not be a good indication that the rewritten version\n> of that commit in the new history still touches the sensitive paths\n> that required the original to be signed (i.e. the history the user\n> is rebasing onto may already have the necessary changes to these\n> paths).\n> \n> So, I dunno, either.\n> \n\nWhile I follow both of your lines of argumentation, I tend to claim that\nthey imply: there is no reason to blindly sign any commit... We should\ndump that config :)\n\nSince it's not possible to sign commits after the fact without rebasing\n(they are not \"notes\" attached to a commit but part of the commit) it is\nvery conceivable to me that you build up your work with fine-grained\ncommits and then, at some point where everything is ready and carefully\ninspected, you sign it. There are various possible reasons why you may\nnot be able to rebase at that point. (I don't know why one wouldn't want\nto use signed tags here, but I never understood the need for signed\ncommits in the first place.)\n\nI guess users of signed commits with rebase should speak up so that we\ncan serve them well.\n\nMichael\n"},{"id":"289454","messageId":"xmqq1t3v3h2a.fsf@gitster.mtv.corp.google.com","threadId":"42600","inReplyTo":"171543e4-a343-c6d2-aa9a-2098db5b42ec@drmicha.warpmail.net","subject":"Re: I lost my commit signature","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-17T16:39:57Z","receivedAt":"2016-06-17T16:40:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@drmicha.warpmail.net> writes:\n\n> Junio C Hamano venit, vidit, dixit 16.06.2016 19:06:\n>\n>> I am not so imaginative, either. One remotely plausible use case may\n>> be a project that has two classes of paths (let's call these classes\n>> sensitive and others), and requires its participants to sign commits\n>> that touch sensitive paths.  The user needs something finter grained\n>> than per-project commit.gpgSign there.\n>>  ...\n>> So, I dunno, either.\n>\n> While I follow both of your lines of argumentation, I tend to claim that\n> they imply: there is no reason to blindly sign any commit... We should\n> dump that config :)\n\n... no reason to blindly sign any commit IN SUCH A PROJECT that\nwants you to selectively sign commits.\n\nThat does not lead to \"we should dump that config\" that all, does\nit?\n"}]}