ProxyCommand not working if $SHELL not defined

Antonio Mignolli antonio.mignolli at yahoo.it
Thu Sep 17 20:16:44 EST 2009


Maybe the mailing list cuts the attachments.

patch.openssh-5.2p1.SHELLfix:

8<-----------------------------------------------------------------
Common subdirectories: openssh-5.2p1/contrib and openssh-5.2p1.new/contrib
diff -NupwB openssh-5.2p1/misc.c openssh-5.2p1.new/misc.c
--- openssh-5.2p1/misc.c	2009-02-21 22:47:02.000000000 +0100
+++ openssh-5.2p1.new/misc.c	2009-09-16 02:21:11.000000000 +0200
@@ -849,3 +849,29 @@ ms_to_timeval(struct timeval *tv, int ms
 	tv->tv_usec = (ms % 1000) * 1000;
 }

+/*
+ * Get shell from env or use default '/bin/sh'
+ */
+char *
+get_shell_from_env()
+{
+	struct stat st;
+	char *shell;
+
+	shell = getenv("SHELL");
+
+	/* If shell does not exist or it is not executable switch to '/bin/sh' */
+
+	if (stat(shell, &st) == 0) {
+		if (S_ISREG(st.st_mode) == 0 ||
+			(st.st_mode & (S_IXOTH|S_IXUSR|S_IXGRP)) == 0) {
+			debug("Warning, environment variable $SHELL='%s' points to a
non-executable, using default: %s",shell,_PATH_BSHELL);
+			shell = _PATH_BSHELL;
+		}
+	}
+	else {
+        debug("Warning, shell not found (environment $SHELL='%s'),
using default: %s",shell,_PATH_BSHELL);
+		shell = _PATH_BSHELL;
+	}
+	return shell;
+}
diff -NupwB openssh-5.2p1/misc.h openssh-5.2p1.new/misc.h
--- openssh-5.2p1/misc.h	2008-06-12 22:42:45.000000000 +0200
+++ openssh-5.2p1.new/misc.h	2009-09-15 23:30:08.000000000 +0200
@@ -53,6 +53,8 @@ void	 freeargs(arglist *);

 int	 tun_open(int, int);

+char *get_shell_from_env();
+
 /* Common definitions for ssh tunnel device forwarding */
 #define SSH_TUNMODE_NO		0x00
 #define SSH_TUNMODE_POINTOPOINT	0x01
Common subdirectories: openssh-5.2p1/openbsd-compat and
openssh-5.2p1.new/openbsd-compat
Common subdirectories: openssh-5.2p1/regress and openssh-5.2p1.new/regress
Common subdirectories: openssh-5.2p1/scard and openssh-5.2p1.new/scard
diff -NupwB openssh-5.2p1/sftp.c openssh-5.2p1.new/sftp.c
--- openssh-5.2p1/sftp.c	2009-02-14 06:26:19.000000000 +0100
+++ openssh-5.2p1.new/sftp.c	2009-09-15 18:01:12.000000000 +0200
@@ -250,8 +250,7 @@ local_do_shell(const char *args)
 	if (!*args)
 		args = NULL;

-	if ((shell = getenv("SHELL")) == NULL)
-		shell = _PATH_BSHELL;
+	shell = get_shell_from_env();

 	if ((pid = fork()) == -1)
 		fatal("Couldn't fork: %s", strerror(errno));
diff -NupwB openssh-5.2p1/ssh-agent.c openssh-5.2p1.new/ssh-agent.c
--- openssh-5.2p1/ssh-agent.c	2008-07-04 15:10:49.000000000 +0200
+++ openssh-5.2p1.new/ssh-agent.c	2009-09-15 22:57:17.000000000 +0200
@@ -1120,9 +1120,8 @@ main(int ac, char **av)
 		usage();

 	if (ac == 0 && !c_flag && !s_flag) {
-		shell = getenv("SHELL");
-		if (shell != NULL &&
-		    strncmp(shell + strlen(shell) - 3, "csh", 3) == 0)
+		shell = get_shell_from_env(); /* shell can never be NULL */
+		if (strncmp(shell + strlen(shell) - 3, "csh", 3) == 0)
 			c_flag = 1;
 	}
 	if (k_flag) {
diff -NupwB openssh-5.2p1/sshconnect.c openssh-5.2p1.new/sshconnect.c
--- openssh-5.2p1/sshconnect.c	2009-02-01 12:19:54.000000000 +0100
+++ openssh-5.2p1.new/sshconnect.c	2009-09-16 02:14:19.000000000 +0200
@@ -84,8 +84,7 @@ ssh_proxy_connect(const char *host, u_sh
 	pid_t pid;
 	char *shell, strport[NI_MAXSERV];

-	if ((shell = getenv("SHELL")) == NULL)
-		shell = _PATH_BSHELL;
+	shell = get_shell_from_env();

 	/* Convert the port number into a string. */
 	snprintf(strport, sizeof strport, "%hu", port);
@@ -1159,8 +1158,7 @@ ssh_local_cmd(const char *args)
 	    args == NULL || !*args)
 		return (1);

-	if ((shell = getenv("SHELL")) == NULL)
-		shell = _PATH_BSHELL;
+	shell = get_shell_from_env();

 	pid = fork();
 	if (pid == 0) {
----------------------------------------------------------------->8

2009/9/17 Antonio Mignolli <antonio.mignolli at yahoo.it>:
> Someone told me the patch was not attached.
> I see it as correctly attached in my sent mail,
> now I retry changing the file name.
>
> 2009/9/16 Antonio Mignolli <antonio.mignolli at yahoo.it>:
>> Here is the suggested patch fixing the SHELL env var issues.
>>
>> 2009/9/16 Antonio Mignolli <antoniomignolli at gmail.com>
>>>
>>> I have a suggested patch for the discussed SHELL var issue.
>>> With the help of a function get_shell_from_env(), in misc.c,
>>> it is now able to set always a good shell, falling back
>>> to default if SHELL is not defined or it is not a valid executable,
>>> hence including SHELL="" (empty string).
>>>
>>> Tested in all cases:
>>> . ssh with proxycommand
>>> . ssh with local command execution
>>> . sftp with local command execution (ex. lls)
>>>
>>> (The patch also changes getenv("SHELL") with get_shell_from_env()
>>>   in ssh-agent.c, where there is a check whether csh is specified).
>>>
>>> The patch is made (and inspired) also by my friend Daniele Calore ( orkaan () orkaan dot org ).
>>>
>>> 2009/9/12 Antonio Mignolli <antoniomignolli at gmail.com>
>>>>
>>>> I forgot, same issue with sftp, when executing local commands.
>>>>
>>>>
>>>>
>>>> 2009/9/12 Antonio Mignolli <antoniomignolli at gmail.com>
>>>>>
>>>>> Now I got it.
>>>>> It happens when SHELL is defined but as empty string,
>>>>> so I don't know if is to be considered an openssh issue.
>>>>>
>>>>> My user had a blank field in /etc/passwd, and logging in
>>>>> in a icewm environment will give me shells with
>>>>> an empty SHELL variable, causing getenv return a not NULL value.
>>>>> Same issue when executing ssh_local_cmd, for the same reason,
>>>>> there is another check on SHELL not null.
>>>>> I have tested that, and with SHELL="" I got "Couldn't execute ..".
>>>>>
>>>>> Thanks for your attention.
>>>>>
>>>>> 2009/9/11 Darren Tucker <dtucker at zip.com.au>
>>>>>>
>>>>>> Antonio Mignolli wrote:
>>>>>>>
>>>>>>> Probably is not a real issue, because everyone should have its SHELL var
>>>>>>> defined,
>>>>>>> but as said above, when it's not, ssh with ProxyCommand will fail.
>>>>>>> I use connect.c, but with no SHELL var defined is not executed, ssh -v
>>>>>>> will give "No such file", and I'm pretty sure it refers to the shell,
>>>>>>> I read in ChangeLog that now ProxyCommand will use $SHELL instead
>>>>>>> of /bin/sh.
>>>>>>> Maybe consider using /bin/sh as default when SHELL is not defined?
>>>>>>
>>>>>> It already does that.  From sshconnect.c:ssh_proxy_connect()
>>>>>>
>>>>>>        if ((shell = getenv("SHELL")) == NULL)
>>>>>>                shell = _PATH_BSHELL;
>>>>>>
>>>>>> where _PATH_BSHELL is defined in defines.h as:
>>>>>>
>>>>>> #ifndef _PATH_BSHELL
>>>>>> # define _PATH_BSHELL "/bin/sh"
>>>>>>
>>>>>> The only thing I can think of is _PATH_BSHELL being defined in the system headers and pointing somewhere else.  If you strace/truss ssh can you see what it's trying to open immediately before the "No such file"?
>>>>>>
>>>>>> --
>>>>>> Darren Tucker (dtucker at zip.com.au)
>>>>>> GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
>>>>>>    Good judgement comes with experience. Unfortunately, the experience
>>>>>> usually comes from bad judgement.
>>>>>
>>>>
>>>
>>
>


More information about the openssh-unix-dev mailing list