{"thread":{"id":"14535","subject":"[PATCH] Ensure that SSH runs in non-interactive mode","startedAt":"2008-07-19T17:06:55Z","lastAt":"2008-07-21T07:05:17Z","messageCount":16,"participants":["Fredrik Tolf","Mike Hommey","Keith Packard","Johannes Schindelin","Junio C Hamano","Jeff King","Steffen Prohaska"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"83952","messageId":"1216487215-6927-1-git-send-email-fredrik@dolda2000.com","threadId":"14535","inReplyTo":null,"subject":"[PATCH] Ensure that SSH runs in non-interactive mode","fromName":"Fredrik Tolf","fromEmail":"fredrik@dolda2000.com","sentAt":"2008-07-19T17:06:55Z","receivedAt":"2008-07-19T17:06:55Z","isPatch":true,"sender":{"key":"fredrik@dolda2000.com","avatar":null},"body":"OpenSSH has the nice feature that it sets the IP TOS value of its\nconnection depending on usage. When used in interactive mode, it\nis set to Minimize-Delay, and other wise to Maximize-Throughput. Its\nusage by Git is best served by Maximize-Throughput, for obvious\nreasons.\n\nHowever, it seems to use a DWIM heuristic for detecting interactive\nmode. The current implementation enters interactive mode if either\na PTY is allocated or X11 forwarding is enabled, and even though Git\nSSH:ing does not allocate a PTY, X11 forwarding is often turned on\nby default. By removing the DISPLAY env variable before forking, SSH\ncan thus be forced into non-interactive mode, without any obvious\nill effects.\n\nSigned-off-by: Fredrik Tolf <fredrik@dolda2000.com>\n---\n connect.c |    7 +++++++\n 1 files changed, 7 insertions(+), 0 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex 574f42f..54888d3 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -607,6 +607,13 @@ struct child_process *git_connect(int fd[2], const char *url_orig,\n \t\t\t*arg++ = port;\n \t\t}\n \t\t*arg++ = host;\n+\t\t/* Remove the X11 DISPLAY from the environment, to\n+\t\t * make SSH run non-interactively */\n+\t\tconst char *env[] = {\n+\t\t\t\"DISPLAY\",\n+\t\t\tNULL\n+\t\t};\n+\t\tconn->env = env;\n \t}\n \telse {\n \t\t/* remove these from the environment */\n-- \n1.5.6.2\n"},{"id":"83954","messageId":"20080719175210.GA7835@glandium.org","threadId":"14535","inReplyTo":"1216487215-6927-1-git-send-email-fredrik@dolda2000.com","subject":"Re: [PATCH] Ensure that SSH runs in non-interactive mode","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-07-19T17:52:10Z","receivedAt":"2008-07-19T17:52:10Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sat, Jul 19, 2008 at 07:06:55PM +0200, Fredrik Tolf wrote:\n> OpenSSH has the nice feature that it sets the IP TOS value of its\n> connection depending on usage. When used in interactive mode, it\n> is set to Minimize-Delay, and other wise to Maximize-Throughput. Its\n> usage by Git is best served by Maximize-Throughput, for obvious\n> reasons.\n> \n> However, it seems to use a DWIM heuristic for detecting interactive\n> mode. The current implementation enters interactive mode if either\n> a PTY is allocated or X11 forwarding is enabled, and even though Git\n> SSH:ing does not allocate a PTY, X11 forwarding is often turned on\n> by default. By removing the DISPLAY env variable before forking, SSH\n> can thus be forced into non-interactive mode, without any obvious\n> ill effects.\n\nWouldn't adding the -x option be better ? Also adding -T could be a good\nidea.\n\nMike\n"},{"id":"83956","messageId":"1216490252.10694.58.camel@koto.keithp.com","threadId":"14535","inReplyTo":"1216487215-6927-1-git-send-email-fredrik@dolda2000.com","subject":"Re: [PATCH] Ensure that SSH runs in non-interactive mode","fromName":"Keith Packard","fromEmail":"keithp@keithp.com","sentAt":"2008-07-19T17:57:32Z","receivedAt":"2008-07-19T17:57:32Z","isPatch":true,"sender":{"key":"keithp@keithp.com","avatar":"https://gravatar.com/avatar/fa1f479cdd51322fe86215c955a81d296bbf66a1fe625f8a12d87a8ec7faf648?d=mp&s=160"},"body":"On Sat, 2008-07-19 at 19:06 +0200, Fredrik Tolf wrote:\n>  By removing the DISPLAY env variable before forking, SSH\n> can thus be forced into non-interactive mode, without any obvious\n> ill effects.\n\nThis will keep ssh-askpass from using any X-based password input\nprogram.\n\n-- \nkeith.packard@intel.com\n"},{"id":"83962","messageId":"1216491512.3911.9.camel@pc7.dolda2000.com","threadId":"14535","inReplyTo":"1216490252.10694.58.camel@koto.keithp.com","subject":"Re: [PATCH] Ensure that SSH runs in non-interactive mode","fromName":"Fredrik Tolf","fromEmail":"fredrik@dolda2000.com","sentAt":"2008-07-19T18:18:32Z","receivedAt":"2008-07-19T18:18:32Z","isPatch":true,"sender":{"key":"fredrik@dolda2000.com","avatar":null},"body":"On Sat, 2008-07-19 at 10:57 -0700, Keith Packard wrote:\n> On Sat, 2008-07-19 at 19:06 +0200, Fredrik Tolf wrote:\n> >  By removing the DISPLAY env variable before forking, SSH\n> > can thus be forced into non-interactive mode, without any obvious\n> > ill effects.\n> \n> This will keep ssh-askpass from using any X-based password input\n> program.\n\nAh, right. Would it be OK to add the `-x' flag to ssh instead? I imagine\nthat that might make git less portable to SSH implementations other than\nOpenSSH, but I don't know if that is considered a problem.\n\nFredrik Tolf\n"},{"id":"84015","messageId":"alpine.DEB.1.00.0807201214060.3305@eeepc-johanness","threadId":"14535","inReplyTo":"1216491512.3911.9.camel@pc7.dolda2000.com","subject":"Re: [PATCH] Ensure that SSH runs in non-interactive mode","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-20T10:27:38Z","receivedAt":"2008-07-20T10:27:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 19 Jul 2008, Fredrik Tolf wrote:\n\n> On Sat, 2008-07-19 at 10:57 -0700, Keith Packard wrote:\n> > On Sat, 2008-07-19 at 19:06 +0200, Fredrik Tolf wrote:\n> > >  By removing the DISPLAY env variable before forking, SSH\n> > > can thus be forced into non-interactive mode, without any obvious\n> > > ill effects.\n> > \n> > This will keep ssh-askpass from using any X-based password input\n> > program.\n> \n> Ah, right. Would it be OK to add the `-x' flag to ssh instead?\n\nI think this would be the correct way, together with \"-T\".\n\n> I imagine that that might make git less portable to SSH implementations \n> other than OpenSSH, but I don't know if that is considered a problem.\n\nWell, this was to be expected, after what I wrote in response to 3. in\nhttp://thread.gmane.org/gmane.comp.version-control.git/76650/focus=2598\n\nReality always catches up with you, and here again we see that plink and \nother siblings of OpenSSH should be best handled with scripts, preferably \nones that strip out options they do not recognize.\n\nIOW something like\n\n-- snip --\n#!/bin/bash\n\nplinkopt=\nwhile test $# != 0\ndo\n\tcase \"$1\" in\n\t-p)\n\t\tplinkopt=\"$plinkopt -P $2\"\n\t\tshift\n\t;;\n\t-*)\n\t\t# unrecognized; strip out\n\t;;\n\t*)\n\t\tbreak\n\t;;\n\tesac\n\tshift\ndone\n\nexec plink $plinkopt \"$@\"\n-- snap --\n\nCiao,\nDscho\n"},{"id":"84039","messageId":"1216576183.3673.2.camel@pc7.dolda2000.com","threadId":"14535","inReplyTo":"alpine.DEB.1.00.0807201214060.3305@eeepc-johanness","subject":"Re: [PATCH] Ensure that SSH runs in non-interactive mode","fromName":"Fredrik Tolf","fromEmail":"fredrik@dolda2000.com","sentAt":"2008-07-20T17:49:43Z","receivedAt":"2008-07-20T17:49:43Z","isPatch":true,"sender":{"key":"fredrik@dolda2000.com","avatar":null},"body":"On Sun, 2008-07-20 at 12:27 +0200, Johannes Schindelin wrote:\n> Well, this was to be expected, after what I wrote in response to 3. in\n> http://thread.gmane.org/gmane.comp.version-control.git/76650/focus=2598\n> \n> Reality always catches up with you, and here again we see that plink and \n> other siblings of OpenSSH should be best handled with scripts, preferably \n> ones that strip out options they do not recognize.\n\nOtherwise, an alternative may be to always install a script, say\n`git-ssh', that would invoke the real SSH in a manner specific for the\nplatform. The exact script installed could even be parametrized by the\nMakefile. For systems using OpenSSH, it would probably just consist of\n`ssh -xT \"$@\"'.\n\nWhat do you think?\n\nFredrik Tolf\n"},{"id":"84042","messageId":"7v63r0bejy.fsf@gitster.siamese.dyndns.org","threadId":"14535","inReplyTo":"alpine.DEB.1.00.0807201214060.3305@eeepc-johanness","subject":"Re: [PATCH] Ensure that SSH runs in non-interactive mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-20T18:23:13Z","receivedAt":"2008-07-20T18:23:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> Ah, right. Would it be OK to add the `-x' flag to ssh instead?\n>\n> I think this would be the correct way, together with \"-T\".\n>\n>> I imagine that that might make git less portable to SSH implementations \n>> other than OpenSSH, but I don't know if that is considered a problem.\n>\n> Well, this was to be expected, after what I wrote in response to 3. in\n> http://thread.gmane.org/gmane.comp.version-control.git/76650/focus=2598\n>\n> Reality always catches up with you, and here again we see that plink and \n> other siblings of OpenSSH should be best handled with scripts, preferably \n> ones that strip out options they do not recognize.\n>\n> IOW something like\n>\n> -- snip --\n> #!/bin/bash\n>\n> plinkopt=\n> while test $# != 0\n> do\n> \tcase \"$1\" in\n> \t-p)\n> \t\tplinkopt=\"$plinkopt -P $2\"\n> \t\tshift\n> \t;;\n> \t-*)\n> \t\t# unrecognized; strip out\n> \t;;\n> \t*)\n> \t\tbreak\n> \t;;\n> \tesac\n> \tshift\n> done\n>\n> exec plink $plinkopt \"$@\"\n> -- snap --\n\nI think that is a very sensible approach, but just like we have a few\n\"built-in\" function-header regexps with customization possibilities for\nthe user, we might want to:\n\n * Have that \"-x\", \"-T\" in the command line we generate for OpenSSH;\n\n * Allow users to specify OpenSSH substitute via a configuration and/or\n   environment variable, and have them use your script; and\n\n * Have a built-in logic for selected and common \"OpenSSH substitute\",\n   e.g. plink.\n\nThere is no reason to make users suffer an extra redirection for common\nenough alternatives.\n\nHere is to get it started...\n\n connect.c |   30 +++++++++++++++++++++++++++---\n 1 files changed, 27 insertions(+), 3 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex 574f42f..c72dd9e 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -599,12 +599,36 @@ struct child_process *git_connect(int fd[2], const char *url_orig,\n \tconn->argv = arg = xcalloc(6, sizeof(*arg));\n \tif (protocol == PROTO_SSH) {\n \t\tconst char *ssh = getenv(\"GIT_SSH\");\n+\t\tconst char *ssh_basename;\n \t\tif (!ssh) ssh = \"ssh\";\n \n+\t\tssh_basename = strrchr(ssh, '/');\n+\t\tssh_basename = ssh_basename ? (ssh_basename + 1) : ssh;\n+\n \t\t*arg++ = ssh;\n-\t\tif (port) {\n-\t\t\t*arg++ = \"-p\";\n-\t\t\t*arg++ = port;\n+\t\t/*\n+\t\t * Make sure to enlarge conn->argv if you add more\n+\t\t * paremeters here.\n+\t\t *\n+\t\t * We know how to invoke a few ssh implementations\n+\t\t * ourselves.\n+\t\t */\n+\t\tif (!strcmp(ssh_basename, \"plink\")) {\n+\t\t\tif (port) {\n+\t\t\t\t*arg++ = \"-P\";\n+\t\t\t\t*arg++ = port;\n+\t\t\t}\n+\t\t} else {\n+\t\t\t/*\n+\t\t\t * This is for stock OpenSSH, but you can have\n+\t\t\t * your custom wrapper script to parse this\n+\t\t\t * and invoke other ssh implementations after\n+\t\t\t * rearranging parameters as well.\n+\t\t\t */\n+\t\t\tif (port) {\n+\t\t\t\t*arg++ = \"-p\";\n+\t\t\t\t*arg++ = port;\n+\t\t\t}\n \t\t}\n \t\t*arg++ = host;\n \t}\n"},{"id":"84047","messageId":"alpine.DEB.1.00.0807202029310.3305@eeepc-johanness","threadId":"14535","inReplyTo":"1216576183.3673.2.camel@pc7.dolda2000.com","subject":"Re: [PATCH] Ensure that SSH runs in non-interactive mode","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-20T18:33:30Z","receivedAt":"2008-07-20T18:33:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 20 Jul 2008, Fredrik Tolf wrote:\n\n> On Sun, 2008-07-20 at 12:27 +0200, Johannes Schindelin wrote:\n>\n> > Well, this was to be expected, after what I wrote in response to 3. in \n> > http://thread.gmane.org/gmane.comp.version-control.git/76650/focus=2598\n> > \n> > Reality always catches up with you, and here again we see that plink \n> > and other siblings of OpenSSH should be best handled with scripts, \n> > preferably ones that strip out options they do not recognize.\n> \n> Otherwise, an alternative may be to always install a script, say \n> `git-ssh', that would invoke the real SSH in a manner specific for the \n> platform. The exact script installed could even be parametrized by the \n> Makefile. For systems using OpenSSH, it would probably just consist of \n> `ssh -xT \"$@\"'.\n> \n> What do you think?\n\nUmm, why?  I fully expect OpenSSH to be the most common ssh helper.  I \nfail to see why we should optimize for something else.\n\nThe GIT_SSH solution works.  Why not just leave things like they are?\n\nCiao,\nDscho\n"},{"id":"84049","messageId":"1216579329.3673.5.camel@pc7.dolda2000.com","threadId":"14535","inReplyTo":"alpine.DEB.1.00.0807202029310.3305@eeepc-johanness","subject":"Re: [PATCH] Ensure that SSH runs in non-interactive mode","fromName":"Fredrik Tolf","fromEmail":"fredrik@dolda2000.com","sentAt":"2008-07-20T18:42:09Z","receivedAt":"2008-07-20T18:42:09Z","isPatch":true,"sender":{"key":"fredrik@dolda2000.com","avatar":null},"body":"On Sun, 2008-07-20 at 20:33 +0200, Johannes Schindelin wrote:\n> > Otherwise, an alternative may be to always install a script, say \n> > `git-ssh', that would invoke the real SSH in a manner specific for the \n> > platform. The exact script installed could even be parametrized by the \n> > Makefile. For systems using OpenSSH, it would probably just consist of \n> > `ssh -xT \"$@\"'.\n> > \n> > What do you think?\n> \n> Umm, why?  I fully expect OpenSSH to be the most common ssh helper.  I \n> fail to see why we should optimize for something else.\n\nI guess I just thought the guys trying to port Git to Windows might find\nit helpful as well. I don't really care myself.\n\n> The GIT_SSH solution works.  Why not just leave things like they are?\n\nI don't think it is a good idea to leave it like it is, because I think\nit is unreasonable to run Git traffic with Minimize-Delay TOS by\ndefault. If for no other reason, changing the TOS would at least make\nmany networks happier.\n\nFredrik Tolf\n"},{"id":"84051","messageId":"alpine.DEB.1.00.0807202035090.3305@eeepc-johanness","threadId":"14535","inReplyTo":"7v63r0bejy.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Ensure that SSH runs in non-interactive mode","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-20T18:57:48Z","receivedAt":"2008-07-20T18:57:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 20 Jul 2008, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> >> Ah, right. Would it be OK to add the `-x' flag to ssh instead?\n> >\n> > I think this would be the correct way, together with \"-T\".\n> >\n> >> I imagine that that might make git less portable to SSH implementations \n> >> other than OpenSSH, but I don't know if that is considered a problem.\n> >\n> > Well, this was to be expected, after what I wrote in response to 3. in\n> > http://thread.gmane.org/gmane.comp.version-control.git/76650/focus=2598\n> >\n> > Reality always catches up with you, and here again we see that plink and \n> > other siblings of OpenSSH should be best handled with scripts, preferably \n> > ones that strip out options they do not recognize.\n> >\n> > IOW something like\n> >\n> > -- snip --\n> > #!/bin/bash\n> >\n> > plinkopt=\n> > while test $# != 0\n> > do\n> > \tcase \"$1\" in\n> > \t-p)\n> > \t\tplinkopt=\"$plinkopt -P $2\"\n> > \t\tshift\n> > \t;;\n> > \t-*)\n> > \t\t# unrecognized; strip out\n> > \t;;\n> > \t*)\n> > \t\tbreak\n> > \t;;\n> > \tesac\n> > \tshift\n> > done\n> >\n> > exec plink $plinkopt \"$@\"\n> > -- snap --\n> \n> I think that is a very sensible approach, but just like we have a few\n> \"built-in\" function-header regexps with customization possibilities for\n> the user, we might want to:\n> \n>  * Have that \"-x\", \"-T\" in the command line we generate for OpenSSH;\n> \n>  * Allow users to specify OpenSSH substitute via a configuration and/or\n>    environment variable, and have them use your script; and\n> \n>  * Have a built-in logic for selected and common \"OpenSSH substitute\",\n>    e.g. plink.\n> \n> There is no reason to make users suffer an extra redirection for common\n> enough alternatives.\n> \n> Here is to get it started...\n\nHow about this instead?\n\n-- snipsnap --\ndiff --git a/connect.c b/connect.c\nindex 574f42f..7e7f4d3 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -603,7 +603,8 @@ struct child_process *git_connect(int fd[2], const char *url\n \n \t\t*arg++ = ssh;\n \t\tif (port) {\n-\t\t\t*arg++ = \"-p\";\n+\t\t\tconst char *opt = getenv(\"GIT_SSH_PORT_OPTION\");\n+\t\t\t*arg++ = opt ? opt : \"-p\";\n \t\t\t*arg++ = port;\n \t\t}\n \t\t*arg++ = host;\n"},{"id":"84056","messageId":"7vhcak5o6n.fsf@gitster.siamese.dyndns.org","threadId":"14535","inReplyTo":"alpine.DEB.1.00.0807202035090.3305@eeepc-johanness","subject":"Re: [PATCH] Ensure that SSH runs in non-interactive mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-20T19:51:44Z","receivedAt":"2008-07-20T19:51:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> How about this instead?\n>\n> -- snipsnap --\n> diff --git a/connect.c b/connect.c\n> index 574f42f..7e7f4d3 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -603,7 +603,8 @@ struct child_process *git_connect(int fd[2], const char *url\n>  \n>  \t\t*arg++ = ssh;\n>  \t\tif (port) {\n> -\t\t\t*arg++ = \"-p\";\n> +\t\t\tconst char *opt = getenv(\"GIT_SSH_PORT_OPTION\");\n> +\t\t\t*arg++ = opt ? opt : \"-p\";\n>  \t\t\t*arg++ = port;\n>  \t\t}\n>  \t\t*arg++ = host;\n\nIf you only care only about the ones we currently want to support, I do\nnot htink it makes any difference either way, but if we are shooting for\nhaving a minimum-but-reasonable framework to make it easy to support other\nones that we haven't seen, it feels very much like an inadequate hack to\nwaste an envirnoment variable for such a narrow special case.  With this,\nwhat you really mean is \"Plink uses -P instead of -p\", right?\n\nI do not know if \"plink\" is used widely enough to be special cased, but if\nso, I think we would better have an explicit support for it.  Will we add\nGIT_SSH_FORBID_X11_FORWARDING_OPTION environment variable and friends,\ntoo?\n\nThe extra environment would not help dealing with an implementation that\nwants --port=90222 (i.e. not as two separate arguments but a single one),\nfor example.  You would need the extra wrapper support for that kind of\nthing anyway.  That extra environment _solution_ will need to make an\nassuption that any reasonable implementation would have an option string\nto specify port which may not be \"-p\" and that is to be followed by a\nseparate argument that is a decimal port number, which probably is\nreasonable for this particular \"port\" thing, but as a general design\nprinciple I do not think it is a good direction to go.\n"},{"id":"84082","messageId":"alpine.DEB.1.00.0807210012480.3305@eeepc-johanness","threadId":"14535","inReplyTo":"7vhcak5o6n.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Ensure that SSH runs in non-interactive mode","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-20T22:17:47Z","receivedAt":"2008-07-20T22:17:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 20 Jul 2008, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > How about this instead?\n> >\n> > -- snipsnap --\n> > diff --git a/connect.c b/connect.c\n> > index 574f42f..7e7f4d3 100644\n> > --- a/connect.c\n> > +++ b/connect.c\n> > @@ -603,7 +603,8 @@ struct child_process *git_connect(int fd[2], const char *url\n> >  \n> >  \t\t*arg++ = ssh;\n> >  \t\tif (port) {\n> > -\t\t\t*arg++ = \"-p\";\n> > +\t\t\tconst char *opt = getenv(\"GIT_SSH_PORT_OPTION\");\n> > +\t\t\t*arg++ = opt ? opt : \"-p\";\n> >  \t\t\t*arg++ = port;\n> >  \t\t}\n> >  \t\t*arg++ = host;\n> \n> If you only care only about the ones we currently want to support, I do\n> not htink it makes any difference either way, but if we are shooting for\n> having a minimum-but-reasonable framework to make it easy to support other\n> ones that we haven't seen, it feels very much like an inadequate hack to\n> waste an envirnoment variable for such a narrow special case.  With this,\n> what you really mean is \"Plink uses -P instead of -p\", right?\n\nYeah.  My first attempt was to allow \"GIT_SSH='plink.exe -P %p %h'\" to \nwork, and for that matter, \"git config --global transport.ssh 'plink.exe \n-P %p %h'\", but I decided that it would be easier to do the patch I \nposted.\n\nAnyway, I think that this issue wasted enough of my time, as I will never \nuse plink anyway.  As long as the patch does not have an adverse effect on \nmy use case, which happens to be the default case, I will just not bother \nanymore, even if I think that GIT_SSH=wrapper would be better than special \ncase rarely exercized ssh programs in the source code.\n\nCiao,\nDscho\n"},{"id":"84110","messageId":"20080721001422.GB12454@sigill.intra.peff.net","threadId":"14535","inReplyTo":"7v63r0bejy.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Ensure that SSH runs in non-interactive mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-07-21T00:14:22Z","receivedAt":"2008-07-21T00:14:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jul 20, 2008 at 11:23:13AM -0700, Junio C Hamano wrote:\n\n> I think that is a very sensible approach, but just like we have a few\n> \"built-in\" function-header regexps with customization possibilities for\n> the user, we might want to:\n> \n>  * Have that \"-x\", \"-T\" in the command line we generate for OpenSSH;\n\nI am slightly negative on this, because we are setting OpenSSH\npreferences behind the user's back that they would not normally expect\ngit to be tampering with.\n\nI think the expectation for this is that it impacts only the ssh session\nused by git.  But because OpenSSH supports the concept of \"master\" and\n\"slave\" sessions (i.e., it can multiplex many sessions over a single ssh\nsession, avoiding authentication and thus reducing latency until the\nstart of the session), what you do in one session can impact other\nsessions. In particular, if the 'master' does not have x11 forwarding\n(because it happens to be started by git), then slave connections do not\nget it. So a user with X11Forwarding and ControlMaster set in his config\nwould usually have everything work, but bad timing with the\ngit-initiated session as the master would unexpectedly break his\nX11Forwarding for other sessions.\n\nI don't know how commonly the ControlMaster option for openssh is used.\nI also don't know if this should simply be considered a bug in openssh,\nsince it silently ignores the request for X forwarding.  Personally, I\nwill not be affected because I don't do X forwarding by default, anyway.\nBut I thought I would raise the point.\n\n-Peff\n"},{"id":"84137","messageId":"5226AAB4-379A-4C44-870A-6040A61C66C1@zib.de","threadId":"14535","inReplyTo":"7vhcak5o6n.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Ensure that SSH runs in non-interactive mode","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2008-07-21T05:07:15Z","receivedAt":"2008-07-21T05:07:15Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Jul 20, 2008, at 9:51 PM, Junio C Hamano wrote:\n\n> I do not know if \"plink\" is used widely enough to be special cased,  \n> but if\n> so, I think we would better have an explicit support for it.\n\nOur installer on Windows explicitly supports plink as an alternative to\nOpenSSH.  Putty has a GUI for managing your ssh keys (Pageant).  You  \nneed\nto type your password only once to unlock a key and make it available to\nall connections that you start afterwards.\n\nI think it should be special cased.  I use plink myself.\n\n\tSteffen\n"},{"id":"84146","messageId":"20080721065348.GB24608@glandium.org","threadId":"14535","inReplyTo":"20080721001422.GB12454@sigill.intra.peff.net","subject":"Re: [PATCH] Ensure that SSH runs in non-interactive mode","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-07-21T06:53:48Z","receivedAt":"2008-07-21T06:53:48Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sun, Jul 20, 2008 at 08:14:22PM -0400, Jeff King wrote:\n> On Sun, Jul 20, 2008 at 11:23:13AM -0700, Junio C Hamano wrote:\n> \n> > I think that is a very sensible approach, but just like we have a few\n> > \"built-in\" function-header regexps with customization possibilities for\n> > the user, we might want to:\n> > \n> >  * Have that \"-x\", \"-T\" in the command line we generate for OpenSSH;\n> \n> I am slightly negative on this, because we are setting OpenSSH\n> preferences behind the user's back that they would not normally expect\n> git to be tampering with.\n> \n> I think the expectation for this is that it impacts only the ssh session\n> used by git.  But because OpenSSH supports the concept of \"master\" and\n> \"slave\" sessions (i.e., it can multiplex many sessions over a single ssh\n> session, avoiding authentication and thus reducing latency until the\n> start of the session), what you do in one session can impact other\n> sessions. In particular, if the 'master' does not have x11 forwarding\n> (because it happens to be started by git), then slave connections do not\n> get it. So a user with X11Forwarding and ControlMaster set in his config\n> would usually have everything work, but bad timing with the\n> git-initiated session as the master would unexpectedly break his\n> X11Forwarding for other sessions.\n> \n> I don't know how commonly the ControlMaster option for openssh is used.\n> I also don't know if this should simply be considered a bug in openssh,\n> since it silently ignores the request for X forwarding.  Personally, I\n> will not be affected because I don't do X forwarding by default, anyway.\n> But I thought I would raise the point.\n\nI'm not sure the ControlMaster option is still followed when using -T. \nAlso, IIRC, ControlMaster doesn't exit until slave connections are\ndone, so git ssh sessions granted the master control would stall until\nthen if they happen to have slaves launched. i.e. It can *already* have\nbad side effects.\n\nAdding '-S none' would ensure ControlMaster would not take effect; on\nthe other hand, it would not allow git's ssh connection to be a slave\neither. '-o ControlMaster no' could be a solution.\n\nAll these need to be tested, obviously.\n\nMike\n"},{"id":"84147","messageId":"20080721070517.GB2080@sigill.intra.peff.net","threadId":"14535","inReplyTo":"20080721065348.GB24608@glandium.org","subject":"Re: [PATCH] Ensure that SSH runs in non-interactive mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-07-21T07:05:17Z","receivedAt":"2008-07-21T07:05:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 21, 2008 at 08:53:48AM +0200, Mike Hommey wrote:\n\n> I'm not sure the ControlMaster option is still followed when using -T. \n\nIt is still followed.\n\n> Also, IIRC, ControlMaster doesn't exit until slave connections are\n> done, so git ssh sessions granted the master control would stall until\n> then if they happen to have slaves launched. i.e. It can *already* have\n> bad side effects.\n\nYes, that is a problem (and IMHO a weakness in the implementation, but\nobviously not git's problem at all).\n\n> Adding '-S none' would ensure ControlMaster would not take effect; on\n\nI think that is definitely a mistake; git is one of the main reasons I\nuse ControlMaster in the first place.\n\n> the other hand, it would not allow git's ssh connection to be a slave\n> either. '-o ControlMaster no' could be a solution.\n\nThat is actually quite sensible, and would make this a non-issue, as\nfar as I can see.\n\n> All these need to be tested, obviously.\n\nI tested, and doing \"ssh -Tx -o 'ControlMaster no'\" does the right thing\n(reuse existing session if possible, create a new one with -Tx\notherwise, and never create a control socket for slaves).\n\n-Peff\n"}]}