[Regression] v7.7.0.0 - SCP fails when using bash as alternative shell

[hugginsa]

If it is a terminal issue then please go through wiki
https://github.com/PowerShell/Win32-OpenSSH/wiki/TTY-PTY-support-in-Windows-OpenSSH

"OpenSSH for Windows" version
7.7.0.0

Server OperatingSystem
Windows 10 Pro

Client OperatingSystem
Windows 8 Pro

What is failing
When server uses either Cygwin bash or Git for Windows bash as the alternative shell, scp does not execute. Message in log when debugging is:

debug1: Executing command: "D:\\cygwin64\\bin\\bash.exe" -c "D:\\Apps\\OpenSSH\\OpenSSH-Win64\\scp -t path_of_file_being_copied"

Previous version 7.6.1.0 did not have this issue and successfully copies.

Expected output
A successful copy operation

Actual output

/usr/bin/bash: D:AppsOpenSSHOpenSSH-Win64scp: command not found

[hugginsa]

@manojampalam Just compiled and tested the latest commit. Bash is now failing commands passed to it.
Example command:

ssh user@computer java -version

Debug message:

debug1: Executing command: "D:\\Apps\\Git\\bin\\bash.exe" -c java -version

Command to successfully execute should be

"D:\\Apps\\Git\\bin\\bash.exe" -c "java -version"

Also affects SCP.

debug1: Executing command: "D:\\Apps\\Git\\bin\\bash.exe" -c scp.exe -t D:\\filename.jar

[hugginsa]

Powershell bash is not affected, only appears to be vanilla bash such as with Git for Windows and Cygwin.

[hugginsa]

Fun experiment, won't work in the long term but double-quoting single-quote command passes and executes.

ssh.exe user@computer "'java -version'"

[hugginsa]

@manojampalam I know this is highly irregular, but here is the fix I propose. However, I cannot contribute myself. I suggest adding a generic bash "SH_GEN_BASH" to the enum for command building, or else having "SH_OTHER" get decorated like WSL bash or Cygwin bash.

The following code fixes my issue, and even allows Jenkins to successfully connect an SSH slave. Starts at line 1724 and ends at 1899.

/* builds session commandline. returns NULL with errno set on failure, caller should free returned string */
char* 
build_session_commandline(const char *shell, const char* shell_arg, const char *command, int pty)
{
	enum sh_type { SH_CMD, SH_PS, SH_WSL_BASH, SH_CYGWIN, SH_OTHER, SH_GEN_BASH } shell_type = SH_OTHER;
	enum cmd_type { CMD_OTHER, CMD_SFTP, CMD_SCP } command_type = CMD_OTHER;
	char *progdir = w32_programdir(), *cmd_sp = NULL, *cmdline = NULL, *ret = NULL, *p;
	int len, progdir_len = (int)strlen(progdir);

#define CMDLINE_APPEND(P, S)		\
do {					\
	int _S_len = (int)strlen(S);		\
	memcpy((P), (S), _S_len);	\
	(P) += _S_len;			\
} while(0)

	/* get shell type */
	if (strstr(shell, "system32\\cmd"))
		shell_type = SH_CMD;
	else if (strstr(shell, "powershell"))
		shell_type = SH_PS;
	else if (strstr(shell, "system32\\bash"))
		shell_type = SH_WSL_BASH;
	else if (strstr(shell, "cygwin"))
		shell_type = SH_CYGWIN;
	else if (strstr(shell, "\\bash"))
		shell_type = SH_GEN_BASH;

	/* special case where incoming command needs to be adjusted */
	do {
		/*
		* identify scp and sftp sessions
		* we want to launch scp and sftp executables from the same binary directory
		* that sshd is hosted in. This will facilitate hosting and evaluating
		* multiple versions of OpenSSH at the same time.
		*
		* currently we can only accomodate this for cmd.exe, since cmd.exe simply executes 
		* its commandline without applying CRT or shell specific rules
		*
		* Ex.
		* this works
		*	cmd /c "c:\program files\sftp" -d
		* this following wouldn't work in powershell, cygwin's or WSL bash unless we put
		* some shell specific rules 
		*	powershell -c "c:\program files\scp" -t
		*	cygwin\bash -c "c:\program files\scp" -t
		*	bash -c "c:\program files\scp" -t
		* 
		* for now, for all non-cmd shells, we launch scp and sftp-server directly -
		*	shell -c "scp.exe -t"
		* note that .exe extension and case matters for WSL bash
		* note that double quotes matter for WSL and Cygwin bash, they dont matter for PS
		*
		* consequence - 
		* for non-cmd shells - sftp and scp installation path is expected to be in machine wide PATH
		* 
		*/

		int command_len;
		const char *command_args = NULL;

		if (!command)
			break;
		command_len = (int)strlen(command);

		if (command_len >= 13 && _memicmp(command, "internal-sftp", 13) == 0) {
			command_type = CMD_SFTP;
			command_args = command + 13;
		} else if (command_len >= 11 && _memicmp(command, "sftp-server", 11) == 0) {
			command_type = CMD_SFTP;

			/* account for possible .exe extension */
			if (command_len >= 15 && _memicmp(command + 11, ".exe", 4) == 0)
				command_args = command + 15;
			else
				command_args = command + 11;
		} else if (command_len >= 3 && _memicmp(command, "scp", 3) == 0) {
			command_type = CMD_SCP;

			/* account for possible .exe extension */
			if (command_len >= 7 && _memicmp(command + 3, ".exe", 4) == 0)
				command_args = command + 7;
			else
				command_args = command + 3;
		}

		if (command_type == CMD_OTHER)
			break;

		len = 0;
		len += progdir_len + 4; /* account for " around */
		len += command_len + 4; /* account for possible .exe addition */

		if ((cmd_sp = malloc(len)) == NULL) {
			errno = ENOMEM;
			goto done;
		}

		p = cmd_sp;

		if (shell_type == SH_CMD) {

			CMDLINE_APPEND(p, "\"");
			CMDLINE_APPEND(p, progdir);

			if (command_type == CMD_SCP)
				CMDLINE_APPEND(p, "\\scp.exe\"");
			else
				CMDLINE_APPEND(p, "\\sftp-server.exe\"");

		} else {
			if (command_type == CMD_SCP)
				CMDLINE_APPEND(p, "scp.exe");
			else
				CMDLINE_APPEND(p, "sftp-server.exe");
		}

		CMDLINE_APPEND(p, command_args);
		*p = '\0';
		command = cmd_sp;
	} while (0);

	len = 0;
	if (pty)
		len += progdir_len + (int)strlen("ssh-shellhost.exe") + 5;
	len +=(int) strlen(shell) + 3;/* 3 for " around shell path and trailing space */
	if (command) {
		len += 15; /* for shell command argument, typically -c or /c */
		len += (int)strlen(command) + 5; /* 5 for possible " around command and null term*/
	}
	
	if ((cmdline = malloc(len)) == NULL) {
		errno = ENOMEM;
		goto done;
	}

	p = cmdline;
	if (pty) {
		CMDLINE_APPEND(p, "\"");
		CMDLINE_APPEND(p, progdir);
		CMDLINE_APPEND(p, "\\ssh-shellhost.exe\" ");
	}
	CMDLINE_APPEND(p, "\"");
	CMDLINE_APPEND(p, shell);
	CMDLINE_APPEND(p, "\"");
	if (command) {
		if (shell_arg) {
			CMDLINE_APPEND(p, " ");
			CMDLINE_APPEND(p, shell_arg);
			CMDLINE_APPEND(p, " ");
		}
		else if (shell_type == SH_CMD)
			CMDLINE_APPEND(p, " /c ");
		else
			CMDLINE_APPEND(p, " -c ");

		/* bash type shells require " decoration around command*/
		if (shell_type == SH_WSL_BASH || shell_type == SH_CYGWIN || shell_type == SH_GEN_BASH)
			CMDLINE_APPEND(p, "\"");

		CMDLINE_APPEND(p, command);

		if (shell_type == SH_WSL_BASH || shell_type == SH_CYGWIN || shell_type == SH_GEN_BASH)
			CMDLINE_APPEND(p, "\"");
	}
	*p = '\0';
	ret = cmdline;
	cmdline = NULL;
done:
	if (cmd_sp)
		free(cmd_sp);
	if (cmdline)
		free(cmdline);

	return ret;
}

[manojampalam]

Thanks @hugginsa I've create one more PR, to apply same rules to all possible bash shells. Can you try it out?

[hugginsa]

Testing now, but looking at the code I believe this solves my issue. Will request that it re-open if this does not give the expected behavior with bash.