git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* Bug: send-pack does not respect http.signingkey
@ 2015-07-16 19:45 Dave Borowitz
  2015-07-16 20:06 ` Junio C Hamano
  0 siblings, 1 reply; 9+ messages in thread
From: Dave Borowitz @ 2015-07-16 19:45 UTC (permalink / raw)
  To: Junio C Hamano, git

When git-send-pack is exec'ed, as is done by git-remote-http, it does
not reread the config, so it does not respect the configured
http.signingkey, either from the config file or -c on the command
line. Thus it is currently impossible to specify a signing key over
HTTP, other than the default one matching the "Name <email>" format in
the keyring.

This is not an issue for git:// as send-pack is executed directly in
the same process that reads the config.

---
 gpg-interface.c | 1 +
 run-command.c   | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 68b0c81..e0ffcb0 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -87,6 +87,7 @@ void set_signing_key(const char *key)
 int git_gpg_config(const char *var, const char *value, void *cb)
 {
  if (!strcmp(var, "user.signingkey")) {
+ fprintf(stderr, "setting %s\n", value);
  set_signing_key(value);
  }
  if (!strcmp(var, "gpg.program")) {
diff --git a/run-command.c b/run-command.c
index 4d73e90..39ae8d5 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "run-command.h"
+#include "gpg-interface.h"
 #include "exec_cmd.h"
 #include "sigchain.h"
 #include "argv-array.h"
@@ -344,7 +345,7 @@ fail_pipe:
  cmd->err = fderr[0];
  }

- trace_argv_printf(cmd->argv, "trace: run_command:");
+ trace_argv_printf(cmd->argv, "trace: run_command (%s):", get_signing_key());
  fflush(NULL);

 #ifndef GIT_WINDOWS_NATIVE
-- 
2.4.3.573.g4eafbef

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Bug: send-pack does not respect http.signingkey
  2015-07-16 19:45 Bug: send-pack does not respect http.signingkey Dave Borowitz
@ 2015-07-16 20:06 ` Junio C Hamano
  2015-07-16 20:08   ` Dave Borowitz
  0 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2015-07-16 20:06 UTC (permalink / raw)
  To: Dave Borowitz; +Cc: git

Dave Borowitz <dborowitz@google.com> writes:

> When git-send-pack is exec'ed, as is done by git-remote-http, it does
> not reread the config, so it does not respect the configured
> http.signingkey, either from the config file or -c on the command
> line. Thus it is currently impossible to specify a signing key over
> HTTP, other than the default one matching the "Name <email>" format in
> the keyring.
>
> This is not an issue for git:// as send-pack is executed directly in
> the same process that reads the config.

Interesting.  I agree that it would be a problem not to be able to
specify which signing key to use.

Perhaps something like this?

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index b564a77..57c3a9c 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -11,6 +11,7 @@
 #include "transport.h"
 #include "version.h"
 #include "sha1-array.h"
+#include "gpg-interface.h"
 
 static const char send_pack_usage[] =
 "git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
@@ -113,6 +114,8 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 	int from_stdin = 0;
 	struct push_cas_option cas = {0};
 
+	git_config(git_gpg_config, NULL);
+
 	argv++;
 	for (i = 1; i < argc; i++, argv++) {
 		const char *arg = *argv;

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Bug: send-pack does not respect http.signingkey
  2015-07-16 20:06 ` Junio C Hamano
@ 2015-07-16 20:08   ` Dave Borowitz
  2015-07-16 20:12     ` Junio C Hamano
  0 siblings, 1 reply; 9+ messages in thread
From: Dave Borowitz @ 2015-07-16 20:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Dave Borowitz <dborowitz@google.com> writes:
>
>> When git-send-pack is exec'ed, as is done by git-remote-http, it does
>> not reread the config, so it does not respect the configured
>> http.signingkey, either from the config file or -c on the command
>> line. Thus it is currently impossible to specify a signing key over
>> HTTP, other than the default one matching the "Name <email>" format in
>> the keyring.
>>
>> This is not an issue for git:// as send-pack is executed directly in
>> the same process that reads the config.
>
> Interesting.  I agree that it would be a problem not to be able to
> specify which signing key to use.
>
> Perhaps something like this?

Seems like it should work.

Jonathan had suggested there might be some principled reason why
send-pack does not respect config options, and suggested passing it in
as a flag. But that would be more work, certainly, as it would also
have to get passed through git-remote-http somehow.

> diff --git a/builtin/send-pack.c b/builtin/send-pack.c
> index b564a77..57c3a9c 100644
> --- a/builtin/send-pack.c
> +++ b/builtin/send-pack.c
> @@ -11,6 +11,7 @@
>  #include "transport.h"
>  #include "version.h"
>  #include "sha1-array.h"
> +#include "gpg-interface.h"
>
>  static const char send_pack_usage[] =
>  "git send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
> @@ -113,6 +114,8 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
>         int from_stdin = 0;
>         struct push_cas_option cas = {0};
>
> +       git_config(git_gpg_config, NULL);
> +
>         argv++;
>         for (i = 1; i < argc; i++, argv++) {
>                 const char *arg = *argv;

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Bug: send-pack does not respect http.signingkey
  2015-07-16 20:08   ` Dave Borowitz
@ 2015-07-16 20:12     ` Junio C Hamano
  2015-07-16 20:31       ` Dave Borowitz
  0 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2015-07-16 20:12 UTC (permalink / raw)
  To: Dave Borowitz; +Cc: git

Dave Borowitz <dborowitz@google.com> writes:

> On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> Perhaps something like this?
>
> Seems like it should work.
>
> Jonathan had suggested there might be some principled reason why
> send-pack does not respect config options, and suggested passing it in
> as a flag. But that would be more work, certainly, as it would also
> have to get passed through git-remote-http somehow.

I actually was wondering about exactly the same thing as Jonathan,
and that is where my "Perhaps" came from.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Bug: send-pack does not respect http.signingkey
  2015-07-16 20:12     ` Junio C Hamano
@ 2015-07-16 20:31       ` Dave Borowitz
  2015-07-16 21:10         ` Junio C Hamano
  0 siblings, 1 reply; 9+ messages in thread
From: Dave Borowitz @ 2015-07-16 20:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Thu, Jul 16, 2015 at 1:12 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Dave Borowitz <dborowitz@google.com> writes:
>
>> On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>>> Perhaps something like this?
>>
>> Seems like it should work.
>>
>> Jonathan had suggested there might be some principled reason why
>> send-pack does not respect config options, and suggested passing it in
>> as a flag. But that would be more work, certainly, as it would also
>> have to get passed through git-remote-http somehow.
>
> I actually was wondering about exactly the same thing as Jonathan,
> and that is where my "Perhaps" came from.

I will say, though, as the maintainer of a handful of custom remote
helpers, I would prefer a solution that does not involve changing the
implementation of those just to pass this configuration through. So my
vote would be for send-pack to respect the normal config options.

Not sure what that would mean for -c on the command line, though.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Bug: send-pack does not respect http.signingkey
  2015-07-16 20:31       ` Dave Borowitz
@ 2015-07-16 21:10         ` Junio C Hamano
  2015-07-16 22:08           ` Dave Borowitz
  0 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2015-07-16 21:10 UTC (permalink / raw)
  To: Dave Borowitz; +Cc: git

Dave Borowitz <dborowitz@google.com> writes:

> On Thu, Jul 16, 2015 at 1:12 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Dave Borowitz <dborowitz@google.com> writes:
>>
>>> On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>
>>>> Perhaps something like this?
>>>
>>> Seems like it should work.
>>>
>>> Jonathan had suggested there might be some principled reason why
>>> send-pack does not respect config options, and suggested passing it in
>>> as a flag. But that would be more work, certainly, as it would also
>>> have to get passed through git-remote-http somehow.
>>
>> I actually was wondering about exactly the same thing as Jonathan,
>> and that is where my "Perhaps" came from.
>
> I will say, though, as the maintainer of a handful of custom remote
> helpers, I would prefer a solution that does not involve changing the
> implementation of those just to pass this configuration through. 

That is not a controversial part ;-)

> So my
> vote would be for send-pack to respect the normal config options.

The thing is what should be included in the "normal" config options.

The "something like this?" patch was deliberately narrow, including
only the GPG thing and nothing else.  But anticipating that the ref
backend would be per repo configuration, and send-pack would want to
read from refs (and possibly write back tracking?), we may want to
prepare ourselves by reading a bit wider than "GPG thing and nothing
else", e.g. git_default_config() or something like that.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Bug: send-pack does not respect http.signingkey
  2015-07-16 21:10         ` Junio C Hamano
@ 2015-07-16 22:08           ` Dave Borowitz
  2015-07-21 19:25             ` Dave Borowitz
  0 siblings, 1 reply; 9+ messages in thread
From: Dave Borowitz @ 2015-07-16 22:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Thu, Jul 16, 2015 at 2:10 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Dave Borowitz <dborowitz@google.com> writes:
>
>> On Thu, Jul 16, 2015 at 1:12 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Dave Borowitz <dborowitz@google.com> writes:
>>>
>>>> On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>>
>>>>> Perhaps something like this?
>>>>
>>>> Seems like it should work.
>>>>
>>>> Jonathan had suggested there might be some principled reason why
>>>> send-pack does not respect config options, and suggested passing it in
>>>> as a flag. But that would be more work, certainly, as it would also
>>>> have to get passed through git-remote-http somehow.
>>>
>>> I actually was wondering about exactly the same thing as Jonathan,
>>> and that is where my "Perhaps" came from.
>>
>> I will say, though, as the maintainer of a handful of custom remote
>> helpers, I would prefer a solution that does not involve changing the
>> implementation of those just to pass this configuration through.
>
> That is not a controversial part ;-)
>
>> So my
>> vote would be for send-pack to respect the normal config options.
>
> The thing is what should be included in the "normal" config options.
>
> The "something like this?" patch was deliberately narrow, including
> only the GPG thing and nothing else.  But anticipating that the ref
> backend would be per repo configuration, and send-pack would want to
> read from refs (and possibly write back tracking?), we may want to
> prepare ourselves by reading a bit wider than "GPG thing and nothing
> else", e.g. git_default_config() or something like that.

Ah, now I understand the question. I have no opinion other than that
we shouldn't let discussion about future features prevent us from
fixing this obvious signed push bug :)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Bug: send-pack does not respect http.signingkey
  2015-07-16 22:08           ` Dave Borowitz
@ 2015-07-21 19:25             ` Dave Borowitz
  2015-07-21 19:33               ` Junio C Hamano
  0 siblings, 1 reply; 9+ messages in thread
From: Dave Borowitz @ 2015-07-21 19:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Thu, Jul 16, 2015 at 3:08 PM, Dave Borowitz <dborowitz@google.com> wrote:
> On Thu, Jul 16, 2015 at 2:10 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Dave Borowitz <dborowitz@google.com> writes:
>>
>>> On Thu, Jul 16, 2015 at 1:12 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> Dave Borowitz <dborowitz@google.com> writes:
>>>>
>>>>> On Thu, Jul 16, 2015 at 1:06 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>>>
>>>>>> Perhaps something like this?
>>>>>
>>>>> Seems like it should work.
>>>>>
>>>>> Jonathan had suggested there might be some principled reason why
>>>>> send-pack does not respect config options, and suggested passing it in
>>>>> as a flag. But that would be more work, certainly, as it would also
>>>>> have to get passed through git-remote-http somehow.
>>>>
>>>> I actually was wondering about exactly the same thing as Jonathan,
>>>> and that is where my "Perhaps" came from.
>>>
>>> I will say, though, as the maintainer of a handful of custom remote
>>> helpers, I would prefer a solution that does not involve changing the
>>> implementation of those just to pass this configuration through.
>>
>> That is not a controversial part ;-)
>>
>>> So my
>>> vote would be for send-pack to respect the normal config options.
>>
>> The thing is what should be included in the "normal" config options.
>>
>> The "something like this?" patch was deliberately narrow, including
>> only the GPG thing and nothing else.  But anticipating that the ref
>> backend would be per repo configuration, and send-pack would want to
>> read from refs (and possibly write back tracking?), we may want to
>> prepare ourselves by reading a bit wider than "GPG thing and nothing
>> else", e.g. git_default_config() or something like that.
>
> Ah, now I understand the question. I have no opinion other than that
> we shouldn't let discussion about future features prevent us from
> fixing this obvious signed push bug :)

Should I formally send a patch with your configuration one-liner?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Bug: send-pack does not respect http.signingkey
  2015-07-21 19:25             ` Dave Borowitz
@ 2015-07-21 19:33               ` Junio C Hamano
  0 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2015-07-21 19:33 UTC (permalink / raw)
  To: Dave Borowitz; +Cc: git

Dave Borowitz <dborowitz@google.com> writes:

> Should I formally send a patch with your configuration one-liner?

Yeah, the log message, that explains the motivation of the change
and the decision to read which part of the configuration, is much
more important than the actual patch, so please do so ;-)

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-07-21 19:33 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-16 19:45 Bug: send-pack does not respect http.signingkey Dave Borowitz
2015-07-16 20:06 ` Junio C Hamano
2015-07-16 20:08   ` Dave Borowitz
2015-07-16 20:12     ` Junio C Hamano
2015-07-16 20:31       ` Dave Borowitz
2015-07-16 21:10         ` Junio C Hamano
2015-07-16 22:08           ` Dave Borowitz
2015-07-21 19:25             ` Dave Borowitz
2015-07-21 19:33               ` Junio C Hamano

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).