git calls SSH_ASKPASS even if DISPLAY is not set

git calls SSH_ASKPASS even if DISPLAY is not set

[Xin Wang]

Hi all,

I'm using git 1.7.3.2 in Fedora 14. In Fedora, SSH_ASKPASS will set to
be /usr/libexec/openssh/gnome-ssh-askpass in
/etc/profile.d/gnome-ssh-askpass.sh, so this environment is set by
login shell, and it will still be set even when X11 is not inuse.

According to ssh's manpage: "If ssh does not have a terminal
associated with it but DISPLAY and SSH_ASKPASS are set, it will
execute the program specified by SSH_ASKPASS and open an X11 window to
read the passphrase." But git will call SSH_ASKPASS even if there is a
terminal associated with it and DISPLAY is not set, then following
warning is displayed and git failed to go through.

$ git fetch

(gnome-ssh-askpass:1487): Gtk-WARNING **: cannot open display:

I think it‘s better if git could implement behavior conforming to ssh.


Thanks,
Xin Wang

[RFC/PATCH] RE: git calls SSH_ASKPASS even if DISPLAY is not set

[Alexander Sulfrian]

Hi,
I prepared a patch to fix this behaviour. It is a simple patch that
adds another check for the DISPLAY environment variable.

But I do not know, if this behaviour breaks something...


Thanks,
Alex

[PATCH] git_getpass: fix ssh-askpass behaviour

[Alexander Sulfrian]

call ssh-askpass only if the display environment variable is also set
---
 connect.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/connect.c b/connect.c
index 57dc20c..2810e3b 100644
--- a/connect.c
+++ b/connect.c
@@ -621,7 +621,7 @@ int finish_connect(struct child_process *conn)
 
 char *git_getpass(const char *prompt)
 {
-	const char *askpass;
+	const char *askpass, *display;
 	struct child_process pass;
 	const char *args[3];
 	static struct strbuf buffer = STRBUF_INIT;
@@ -631,7 +631,10 @@ char *git_getpass(const char *prompt)
 		askpass = askpass_program;
 	if (!askpass)
 		askpass = getenv("SSH_ASKPASS");
-	if (!askpass || !(*askpass)) {
+
+	/* only call askpass if display is set */
+	display = getenv("DISPLAY");
+	if (!display || !(*display) || !askpass || !(*askpass))
 		char *result = getpass(prompt);
 		if (!result)
 			die_errno("Could not read password");
-- 
1.7.2.2

Re: [PATCH] git_getpass: fix ssh-askpass behaviour

[Alexander Sulfrian]

[-- Attachment #1: Type: text/plain, Size: 267 bytes --]

On Sun, 12 Dec 2010 13:32:54 +0100
Alexander Sulfrian <alexander@sulfrian.net> wrote:

> call ssh-askpass only if the display environment variable is also set

Oh forgot to sign-off... I'll wait if there are other comments and
resend it in a few days.

Alex

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] git_getpass: fix ssh-askpass behaviour

[Junio C Hamano]

Alexander Sulfrian <alexander@sulfrian.net> writes:

> call ssh-askpass only if the display environment variable is also set
> ---

I do not use it at all so I don't know for sure, but doesn't this break
OSX?

  20f3490 (web--browse: fix Mac OS X GUI detection for 10.6, 2009-09-14)

is an example that you can be fully graphical without having DISPLAY set
in some environment.  MinGW folks may want to chime in as well.

>  connect.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/connect.c b/connect.c
> index 57dc20c..2810e3b 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -621,7 +621,7 @@ int finish_connect(struct child_process *conn)
>  
>  char *git_getpass(const char *prompt)
>  {
> -	const char *askpass;
> +	const char *askpass, *display;
>  	struct child_process pass;
>  	const char *args[3];
>  	static struct strbuf buffer = STRBUF_INIT;
> @@ -631,7 +631,10 @@ char *git_getpass(const char *prompt)
>  		askpass = askpass_program;
>  	if (!askpass)
>  		askpass = getenv("SSH_ASKPASS");
> -	if (!askpass || !(*askpass)) {
> +
> +	/* only call askpass if display is set */
> +	display = getenv("DISPLAY");
> +	if (!display || !(*display) || !askpass || !(*askpass))
>  		char *result = getpass(prompt);
>  		if (!result)
>  			die_errno("Could not read password");

Re: [PATCH] git_getpass: fix ssh-askpass behaviour

[Xin Wang]

Junio C Hamano <gitster <at> pobox.com> writes:

>
> Alexander Sulfrian <alexander <at> sulfrian.net> writes:
>
> > call ssh-askpass only if the display environment variable is also set
> > ---
>
> I do not use it at all so I don't know for sure, but doesn't this break
> OSX?
>
>   20f3490 (web--browse: fix Mac OS X GUI detection for 10.6, 2009-09-14)
>
> is an example that you can be fully graphical without having DISPLAY set
> in some environment.  MinGW folks may want to chime in as well.
>

How about test whether git is associated with a terminal or not?

> >  connect.c |    7 +++++--
> >  1 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/connect.c b/connect.c
> > index 57dc20c..2810e3b 100644
> > --- a/connect.c
> > +++ b/connect.c
> > @@ -621,7 +621,7 @@ int finish_connect(struct child_process *conn)
> >
> >  char *git_getpass(const char *prompt)
> >  {
> > -	const char *askpass;
> > +	const char *askpass, *display;
> >  	struct child_process pass;
> >  	const char *args[3];
> >  	static struct strbuf buffer = STRBUF_INIT;
> > @@ -631,7 +631,10 @@ char *git_getpass(const char *prompt)
> >  		askpass = askpass_program;
> >  	if (!askpass)
> >  		askpass = getenv("SSH_ASKPASS");
> > -	if (!askpass || !(*askpass)) {
> > +
> > +	/* only call askpass if display is set */
> > +	display = getenv("DISPLAY");
> > +	if (!display || !(*display) || !askpass || !(*askpass))
> >  		char *result = getpass(prompt);
> >  		if (!result)
> >  			die_errno("Could not read password");
>
>

Xin Wang

Re: [PATCH] git_getpass: fix ssh-askpass behaviour

[Johannes Sixt]

Am 12/12/2010 13:32, schrieb Alexander Sulfrian:
> call ssh-askpass only if the display environment variable is also set

Not good: On Windows, we want to call out to SSH_ASKPASS even if DISPLAY
is not set (it almost never is).

-- Hannes

Re: Re: [PATCH] git_getpass: fix ssh-askpass behaviour

[hvoigt]

Hi,

On Sun, Dec 12, 2010 at 04:41:00PM -0800, Junio C Hamano wrote:
> Alexander Sulfrian <alexander@sulfrian.net> writes:
> 
> > call ssh-askpass only if the display environment variable is also set
> > ---
> 
> I do not use it at all so I don't know for sure, but doesn't this break
> OSX?
> 
>   20f3490 (web--browse: fix Mac OS X GUI detection for 10.6, 2009-09-14)
> 
> is an example that you can be fully graphical without having DISPLAY set
> in some environment.  MinGW folks may want to chime in as well.

I am not sure about OSX because I just checked and there seems to be a
DISPLAY variable set which seems to be used to start a X session on
demand. But MinGW definitely has no DISPLAY variable set by default.

Additionally GIT_ASKPASS/SSH_ASKPASS does not have to be a graphical
tool does it? It could also be some program that looks up the password
from some secure database.

Cheers Heiko

Re: git calls SSH_ASKPASS even if DISPLAY is not set

[LarryMartell]

Xin Wang wrote
> 
> Hi all,
> 
> I'm using git 1.7.3.2 in Fedora 14. In Fedora, SSH_ASKPASS will set to
> be /usr/libexec/openssh/gnome-ssh-askpass in
> /etc/profile.d/gnome-ssh-askpass.sh, so this environment is set by
> login shell, and it will still be set even when X11 is not inuse.
> 
> According to ssh's manpage: "If ssh does not have a terminal
> associated with it but DISPLAY and SSH_ASKPASS are set, it will
> execute the program specified by SSH_ASKPASS and open an X11 window to
> read the passphrase." But git will call SSH_ASKPASS even if there is a
> terminal associated with it and DISPLAY is not set, then following
> warning is displayed and git failed to go through.
> 
> $ git fetch
> 
> (gnome-ssh-askpass:1487): Gtk-WARNING **: cannot open display:
> 
> I think it‘s better if git could implement behavior conforming to ssh.
> 
> 


We are getting this error on a new CentOS system we just set up. Was there
ever a fix or workaround for this?

-larry


--
View this message in context: http://git.661346.n2.nabble.com/git-calls-SSH-ASKPASS-even-if-DISPLAY-is-not-set-tp5825303p7537044.html
Sent from the git mailing list archive at Nabble.com.