{"thread":{"id":"39862","subject":"Bug: send-pack does not respect http.signingkey","startedAt":"2015-07-16T19:45:16Z","lastAt":"2015-07-21T19:33:22Z","messageCount":9,"participants":["Dave Borowitz","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"266203","messageId":"CAD0k6qQ=ovEBZn_wje-exBhvW8brRbTgULDr68rmeiw-ZdsozQ@mail.gmail.com","threadId":"39862","inReplyTo":null,"subject":"Bug: send-pack does not respect http.signingkey","fromName":"Dave Borowitz","fromEmail":"dborowitz@google.com","sentAt":"2015-07-16T19:45:16Z","receivedAt":"2015-07-16T19:45:16Z","isPatch":false,"sender":{"key":"dborowitz@google.com","avatar":"https://avatars.githubusercontent.com/u/194927?v=4"},"body":"When git-send-pack is exec'ed, as is done by git-remote-http, it does\nnot reread the config, so it does not respect the configured\nhttp.signingkey, either from the config file or -c on the command\nline. Thus it is currently impossible to specify a signing key over\nHTTP, other than the default one matching the \"Name \" format in\nthe keyring.\n\nThis is not an issue for git:// as send-pack is executed directly in\nthe same process that reads the config.\n\n---\n gpg-interface.c | 1 +\n run-command.c | 3 ++-\n 2 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 68b0c81..e0ffcb0 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -87,6 +87,7 @@ void set_signing_key(const char *key)\n int git_gpg_config(const char *var, const char *value, void *cb)\n {\n if (!strcmp(var, \"user.signingkey\")) {\n+ fprintf(stderr, \"setting %s\\n\", value);\n set_signing_key(value);\n }\n if (!strcmp(var, \"gpg.program\")) {\ndiff --git a/run-command.c b/run-command.c\nindex 4d73e90..39ae8d5 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1,5 +1,6 @@\n #include \"cache.h\"\n #include \"run-command.h\"\n+#include \"gpg-interface.h\"\n #include \"exec_cmd.h\"\n #include \"sigchain.h\"\n #include \"argv-array.h\"\n@@ -344,7 +345,7 @@ fail_pipe:\n cmd->err = fderr[0];\n }\n\n- trace_argv_printf(cmd->argv, \"trace: run_command:\");\n+ trace_argv_printf(cmd->argv, \"trace: run_command (%s):\", get_signing_key());\n fflush(NULL);\n\n #ifndef GIT_WINDOWS_NATIVE\n-- \n2.4.3.573.g4eafbef\n"},{"id":"266204","messageId":"xmqqlhef50kz.fsf@gitster.dls.corp.google.com","threadId":"39862","inReplyTo":"CAD0k6qQ=ovEBZn_wje-exBhvW8brRbTgULDr68rmeiw-ZdsozQ@mail.gmail.com","subject":"Re: Bug: send-pack does not respect http.signingkey","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-16T20:06:36Z","receivedAt":"2015-07-16T20:06:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dave Borowitz writes:\n\n> When git-send-pack is exec'ed, as is done by git-remote-http, it does\n> not reread the config, so it does not respect the configured\n> http.signingkey, either from the config file or -c on the command\n> line. Thus it is currently impossible to specify a signing key over\n> HTTP, other than the default one matching the \"Name \" format in\n> the keyring.\n>\n> This is not an issue for git:// as send-pack is executed directly in\n> the same process that reads the config.\n\nInteresting. I agree that it would be a problem not to be able to\nspecify which signing key to use.\n\nPerhaps something like this?\n\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex b564a77..57c3a9c 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -11,6 +11,7 @@\n #include \"transport.h\"\n #include \"version.h\"\n #include \"sha1-array.h\"\n+#include \"gpg-interface.h\"\n \n static const char send_pack_usage[] =\n \"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=] [--verbose] [--thin] [:] [...]\\n\"\n@@ -113,6 +114,8 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \tint from_stdin = 0;\n \tstruct push_cas_option cas = {0};\n \n+\tgit_config(git_gpg_config, NULL);\n+\n \targv++;\n \tfor (i = 1; i < argc; i++, argv++) {\n \t\tconst char *arg = *argv;\n"},{"id":"266205","messageId":"CAD0k6qS_uQk5KZVJYA7BVZ9sRPRj=cZ3rbYRfJ3vhgSh2drEMA@mail.gmail.com","threadId":"39862","inReplyTo":"xmqqlhef50kz.fsf@gitster.dls.corp.google.com","subject":"Re: Bug: send-pack does not respect http.signingkey","fromName":"Dave Borowitz","fromEmail":"dborowitz@google.com","sentAt":"2015-07-16T20:08:40Z","receivedAt":"2015-07-16T20:08:40Z","isPatch":false,"sender":{"key":"dborowitz@google.com","avatar":"https://avatars.githubusercontent.com/u/194927?v=4"},"body":"On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano wrote:\n> Dave Borowitz writes:\n>\n>> When git-send-pack is exec'ed, as is done by git-remote-http, it does\n>> not reread the config, so it does not respect the configured\n>> http.signingkey, either from the config file or -c on the command\n>> line. Thus it is currently impossible to specify a signing key over\n>> HTTP, other than the default one matching the \"Name \" format in\n>> the keyring.\n>>\n>> This is not an issue for git:// as send-pack is executed directly in\n>> the same process that reads the config.\n>\n> Interesting. I agree that it would be a problem not to be able to\n> specify which signing key to use.\n>\n> Perhaps something like this?\n\nSeems like it should work.\n\nJonathan had suggested there might be some principled reason why\nsend-pack does not respect config options, and suggested passing it in\nas a flag. But that would be more work, certainly, as it would also\nhave to get passed through git-remote-http somehow.\n\n> diff --git a/builtin/send-pack.c b/builtin/send-pack.c\n> index b564a77..57c3a9c 100644\n> --- a/builtin/send-pack.c\n> +++ b/builtin/send-pack.c\n> @@ -11,6 +11,7 @@\n> #include \"transport.h\"\n> #include \"version.h\"\n> #include \"sha1-array.h\"\n> +#include \"gpg-interface.h\"\n>\n> static const char send_pack_usage[] =\n> \"git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=] [--verbose] [--thin] [:] [...]\\n\"\n> @@ -113,6 +114,8 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n> int from_stdin = 0;\n> struct push_cas_option cas = {0};\n>\n> + git_config(git_gpg_config, NULL);\n> +\n> argv++;\n> for (i = 1; i < argc; i++, argv++) {\n> const char *arg = *argv;\n"},{"id":"266206","messageId":"xmqqh9p350ad.fsf@gitster.dls.corp.google.com","threadId":"39862","inReplyTo":"CAD0k6qS_uQk5KZVJYA7BVZ9sRPRj=cZ3rbYRfJ3vhgSh2drEMA@mail.gmail.com","subject":"Re: Bug: send-pack does not respect http.signingkey","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-16T20:12:58Z","receivedAt":"2015-07-16T20:12:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dave Borowitz writes:\n\n> On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano wrote:\n>\n>> Perhaps something like this?\n>\n> Seems like it should work.\n>\n> Jonathan had suggested there might be some principled reason why\n> send-pack does not respect config options, and suggested passing it in\n> as a flag. But that would be more work, certainly, as it would also\n> have to get passed through git-remote-http somehow.\n\nI actually was wondering about exactly the same thing as Jonathan,\nand that is where my \"Perhaps\" came from.\n"},{"id":"266207","messageId":"CAD0k6qSZtZN60ExQQfH0mq_rZrOA4Fw0ZeWfX_gr=2btbSYHQw@mail.gmail.com","threadId":"39862","inReplyTo":"xmqqh9p350ad.fsf@gitster.dls.corp.google.com","subject":"Re: Bug: send-pack does not respect http.signingkey","fromName":"Dave Borowitz","fromEmail":"dborowitz@google.com","sentAt":"2015-07-16T20:31:55Z","receivedAt":"2015-07-16T20:31:55Z","isPatch":false,"sender":{"key":"dborowitz@google.com","avatar":"https://avatars.githubusercontent.com/u/194927?v=4"},"body":"On Thu, Jul 16, 2015 at 1:12 PM, Junio C Hamano wrote:\n> Dave Borowitz writes:\n>\n>> On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano wrote:\n>>\n>>> Perhaps something like this?\n>>\n>> Seems like it should work.\n>>\n>> Jonathan had suggested there might be some principled reason why\n>> send-pack does not respect config options, and suggested passing it in\n>> as a flag. But that would be more work, certainly, as it would also\n>> have to get passed through git-remote-http somehow.\n>\n> I actually was wondering about exactly the same thing as Jonathan,\n> and that is where my \"Perhaps\" came from.\n\nI will say, though, as the maintainer of a handful of custom remote\nhelpers, I would prefer a solution that does not involve changing the\nimplementation of those just to pass this configuration through. So my\nvote would be for send-pack to respect the normal config options.\n\nNot sure what that would mean for -c on the command line, though.\n"},{"id":"266210","messageId":"xmqq8uaf4xmo.fsf@gitster.dls.corp.google.com","threadId":"39862","inReplyTo":"CAD0k6qSZtZN60ExQQfH0mq_rZrOA4Fw0ZeWfX_gr=2btbSYHQw@mail.gmail.com","subject":"Re: Bug: send-pack does not respect http.signingkey","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-16T21:10:23Z","receivedAt":"2015-07-16T21:10:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dave Borowitz writes:\n\n> On Thu, Jul 16, 2015 at 1:12 PM, Junio C Hamano wrote:\n>> Dave Borowitz writes:\n>>\n>>> On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano wrote:\n>>>\n>>>> Perhaps something like this?\n>>>\n>>> Seems like it should work.\n>>>\n>>> Jonathan had suggested there might be some principled reason why\n>>> send-pack does not respect config options, and suggested passing it in\n>>> as a flag. But that would be more work, certainly, as it would also\n>>> have to get passed through git-remote-http somehow.\n>>\n>> I actually was wondering about exactly the same thing as Jonathan,\n>> and that is where my \"Perhaps\" came from.\n>\n> I will say, though, as the maintainer of a handful of custom remote\n> helpers, I would prefer a solution that does not involve changing the\n> implementation of those just to pass this configuration through. \n\nThat is not a controversial part ;-)\n\n> So my\n> vote would be for send-pack to respect the normal config options.\n\nThe thing is what should be included in the \"normal\" config options.\n\nThe \"something like this?\" patch was deliberately narrow, including\nonly the GPG thing and nothing else. But anticipating that the ref\nbackend would be per repo configuration, and send-pack would want to\nread from refs (and possibly write back tracking?), we may want to\nprepare ourselves by reading a bit wider than \"GPG thing and nothing\nelse\", e.g. git_default_config() or something like that.\n"},{"id":"266212","messageId":"CAD0k6qSq8+JMFZgvQuVptCxUknYtMa7xrojABEDYLQAw015qvg@mail.gmail.com","threadId":"39862","inReplyTo":"xmqq8uaf4xmo.fsf@gitster.dls.corp.google.com","subject":"Re: Bug: send-pack does not respect http.signingkey","fromName":"Dave Borowitz","fromEmail":"dborowitz@google.com","sentAt":"2015-07-16T22:08:48Z","receivedAt":"2015-07-16T22:08:48Z","isPatch":false,"sender":{"key":"dborowitz@google.com","avatar":"https://avatars.githubusercontent.com/u/194927?v=4"},"body":"On Thu, Jul 16, 2015 at 2:10 PM, Junio C Hamano wrote:\n> Dave Borowitz writes:\n>\n>> On Thu, Jul 16, 2015 at 1:12 PM, Junio C Hamano wrote:\n>>> Dave Borowitz writes:\n>>>\n>>>> On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano wrote:\n>>>>\n>>>>> Perhaps something like this?\n>>>>\n>>>> Seems like it should work.\n>>>>\n>>>> Jonathan had suggested there might be some principled reason why\n>>>> send-pack does not respect config options, and suggested passing it in\n>>>> as a flag. But that would be more work, certainly, as it would also\n>>>> have to get passed through git-remote-http somehow.\n>>>\n>>> I actually was wondering about exactly the same thing as Jonathan,\n>>> and that is where my \"Perhaps\" came from.\n>>\n>> I will say, though, as the maintainer of a handful of custom remote\n>> helpers, I would prefer a solution that does not involve changing the\n>> implementation of those just to pass this configuration through.\n>\n> That is not a controversial part ;-)\n>\n>> So my\n>> vote would be for send-pack to respect the normal config options.\n>\n> The thing is what should be included in the \"normal\" config options.\n>\n> The \"something like this?\" patch was deliberately narrow, including\n> only the GPG thing and nothing else. But anticipating that the ref\n> backend would be per repo configuration, and send-pack would want to\n> read from refs (and possibly write back tracking?), we may want to\n> prepare ourselves by reading a bit wider than \"GPG thing and nothing\n> else\", e.g. git_default_config() or something like that.\n\nAh, now I understand the question. I have no opinion other than that\nwe shouldn't let discussion about future features prevent us from\nfixing this obvious signed push bug :)\n"},{"id":"266563","messageId":"CAD0k6qSjKnHMHXAQxJ97-dLcWVz+_ay+j6=X9j_yjx25sCwFrw@mail.gmail.com","threadId":"39862","inReplyTo":"CAD0k6qSq8+JMFZgvQuVptCxUknYtMa7xrojABEDYLQAw015qvg@mail.gmail.com","subject":"Re: Bug: send-pack does not respect http.signingkey","fromName":"Dave Borowitz","fromEmail":"dborowitz@google.com","sentAt":"2015-07-21T19:25:09Z","receivedAt":"2015-07-21T19:25:09Z","isPatch":false,"sender":{"key":"dborowitz@google.com","avatar":"https://avatars.githubusercontent.com/u/194927?v=4"},"body":"On Thu, Jul 16, 2015 at 3:08 PM, Dave Borowitz wrote:\n> On Thu, Jul 16, 2015 at 2:10 PM, Junio C Hamano wrote:\n>> Dave Borowitz writes:\n>>\n>>> On Thu, Jul 16, 2015 at 1:12 PM, Junio C Hamano wrote:\n>>>> Dave Borowitz writes:\n>>>>\n>>>>> On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano wrote:\n>>>>>\n>>>>>> Perhaps something like this?\n>>>>>\n>>>>> Seems like it should work.\n>>>>>\n>>>>> Jonathan had suggested there might be some principled reason why\n>>>>> send-pack does not respect config options, and suggested passing it in\n>>>>> as a flag. But that would be more work, certainly, as it would also\n>>>>> have to get passed through git-remote-http somehow.\n>>>>\n>>>> I actually was wondering about exactly the same thing as Jonathan,\n>>>> and that is where my \"Perhaps\" came from.\n>>>\n>>> I will say, though, as the maintainer of a handful of custom remote\n>>> helpers, I would prefer a solution that does not involve changing the\n>>> implementation of those just to pass this configuration through.\n>>\n>> That is not a controversial part ;-)\n>>\n>>> So my\n>>> vote would be for send-pack to respect the normal config options.\n>>\n>> The thing is what should be included in the \"normal\" config options.\n>>\n>> The \"something like this?\" patch was deliberately narrow, including\n>> only the GPG thing and nothing else. But anticipating that the ref\n>> backend would be per repo configuration, and send-pack would want to\n>> read from refs (and possibly write back tracking?), we may want to\n>> prepare ourselves by reading a bit wider than \"GPG thing and nothing\n>> else\", e.g. git_default_config() or something like that.\n>\n> Ah, now I understand the question. I have no opinion other than that\n> we shouldn't let discussion about future features prevent us from\n> fixing this obvious signed push bug :)\n\nShould I formally send a patch with your configuration one-liner?\n"},{"id":"266566","messageId":"xmqqr3o1uwzh.fsf@gitster.dls.corp.google.com","threadId":"39862","inReplyTo":"CAD0k6qSjKnHMHXAQxJ97-dLcWVz+_ay+j6=X9j_yjx25sCwFrw@mail.gmail.com","subject":"Re: Bug: send-pack does not respect http.signingkey","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-21T19:33:22Z","receivedAt":"2015-07-21T19:33:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dave Borowitz writes:\n\n> Should I formally send a patch with your configuration one-liner?\n\nYeah, the log message, that explains the motivation of the change\nand the decision to read which part of the configuration, is much\nmore important than the actual patch, so please do so ;-)\n"}]}