WolfSSL
/
wolfssh
mirror of https://github.com/wolfSSL/wolfssh.git
1
0
Fork 0
Code Issues Releases Wiki Activity

wolfsshd: bound the shell child reap and flush the held backlog 

- The child's stdin pipe and the pty master are closed before the
  reap. The reap polls waitpid() with WNOHANG for
  WOLFSSHD_CHILD_REAP_TRIES x WOLFSSHD_CHILD_REAP_WAIT_US, then
  sends SIGKILL; a failed kill() is logged and ends the wait.
- ChildSig is installed before forkpty() instead of after it.
- An EIO read from the pty master ends the child's output
  instead of the loop.
- A held backlog is sent with SHELL_FlushOut() before the pipe
  drain; the drain is skipped when the backlog is not sent.
- New sshd_channel_close_test.sh (paramiko) closes the channel
  while output is held, for an exec command and a pty shell, and
  checks that the command and the connection both end; it also
  checks exit status 3 from an exec and an interactive shell.
  run_all runs it.
- sshd-test.yml and code-coverage.yml install python3-paramiko
  through install-apt-deps, and the ubuntu-24.04-tests bundle
  lists it.
pull/1307/head
Yosuke Shimizu 2026-10-09 09:07:18 +09:00 committed by John Safranek
parent 96c2c14ae1
commit 4dc6e86cbb
6 changed files with 241 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
.github/ci-deps/packages-ubuntu-24.04-tests.txt vendored
View File

@ -16,6 +16,7 @@ netcat-traditional
openssh-client
openssh-server
pkg-config
python3-paramiko
python3-pip
sshpass
tpm2-tools

8
.github/workflows/code-coverage.yml vendored
View File

@ -62,6 +62,14 @@ jobs:
ghcr-debs-tag: ubuntu-24.04-compilers
budget-seconds: 300
# For sshd_channel_close_test.sh. The suite runs as root, which does not
# see a per-user pip install, so use the distro package.
- name: Install paramiko
uses: ./.github/actions/install-apt-deps
with:
packages: python3-paramiko
ghcr-debs-tag: ubuntu-24.04-tests
- name: Download wolfSSL
uses: actions/download-artifact@v8
with:

8
.github/workflows/sshd-test.yml vendored
View File

@ -118,6 +118,14 @@ jobs:
working-directory: ./wolfssh/
run: make check
# For sshd_channel_close_test.sh. The suite runs as root, which does not
# see a per-user pip install, so use the distro package.
- name: Install paramiko
uses: ./wolfssh/.github/actions/install-apt-deps
with:
packages: python3-paramiko
ghcr-debs-tag: ubuntu-24.04-tests
- name: Run wolfSSHd tests
working-directory: ./wolfssh/apps/wolfsshd/test
run: |

1
apps/wolfsshd/test/run_all_sshd_tests.sh
View File

@ -15,6 +15,7 @@ test_cases=(
"sshd_term_close_test.sh"
"sshd_stdin_eof_test.sh"
"sshd_stdin_stall_test.sh"
"sshd_channel_close_test.sh"
"ssh_kex_algos.sh"
)

163
apps/wolfsshd/test/sshd_channel_close_test.sh 100755
View File

@ -0,0 +1,163 @@
#!/bin/bash
# Closing a session channel while wolfsshd holds its output must not leave the
# connection process waiting on the command. The exec command traps INT and
# HUP, so only the reap's SIGKILL ends it; a pty shell must exit on the hangup.
# The exit status must still reach the client, from an exec and from a shell.
#
# Needs paramiko: the OpenSSH client never closes the channel early; it keeps
# reading and discards what it cannot write.
HOST="$1"
PORT="$2"
USER_NAME="${3:-`whoami`}"
command -v python3 >/dev/null 2>&1 || exit 77
python3 -c "import paramiko" >/dev/null 2>&1 || exit 77
command -v timeout >/dev/null 2>&1 || exit 77
# The command's pid is checked on this machine.
case "$HOST" in
127.0.0.1|localhost|::1) ;;
*) exit 77 ;;
esac
KEYDIR=`mktemp -d` || exit 1
MARKDIR=`mktemp -d` || exit 1
trap 'rm -rf "$KEYDIR" "$MARKDIR"' EXIT
# Private to the login user, who writes the marker.
chown "$USER_NAME" "$MARKDIR" || exit 77
cp ../../../keys/hansel-key-ecc.pem "$KEYDIR/id_ecdsa" || exit 1
chmod 600 "$KEYDIR/id_ecdsa"
timeout 60 python3 -u - "$HOST" "$PORT" "$USER_NAME" "$KEYDIR/id_ecdsa" \
"$MARKDIR/exited" <<'EOF'
import os
import re
import sys
import time
import paramiko
host, port, user, key = sys.argv[1], int(sys.argv[2]), sys.argv[3], sys.argv[4]
marker = sys.argv[5]
LIMIT = 5
WAIT = 20 # for any one read or exit status
def alive(pid):
try:
os.kill(pid, 0)
except ProcessLookupError:
return False
except PermissionError:
pass
return True
def connect():
client = paramiko.SSHClient()
client.set_missing_host_key_policy(paramiko.AutoAddPolicy())
client.connect(host, port=port, username=user,
pkey=paramiko.ECDSAKey.from_private_key_file(key),
look_for_keys=False, allow_agent=False, timeout=10)
return client
def run_case(name, shell):
client = connect()
chan = client.get_transport().open_session()
chan.settimeout(WAIT)
if shell:
chan.get_pty()
chan.invoke_shell()
# The marker is a builtin redirection on the exit path only.
chan.send("trap 'exit' HUP; trap ': > %s' EXIT; echo PID=$$\n"
% marker)
else:
chan.exec_command('echo PID=$$; trap "" INT HUP; '
'seq 1 10000000; sleep 30')
# The pty echoes the typed line with a literal $$, which does not match,
# and the newline keeps a pid split across two reads from matching.
out = b""
match = None
while match is None:
data = chan.recv(4096)
if not data:
break
out += data
match = re.search(rb"PID=(\d+)\r?\n", out)
if match is None:
print("%s: no pid from the command" % name)
client.close()
return False
pid = int(match.group(1))
if shell:
chan.send("seq 1 10000000\n")
# Stop reading until the window fills and wolfsshd holds output, then
# close the channel with the transport still up.
time.sleep(1)
chan.close()
start = time.time()
while alive(pid) and time.time() - start < LIMIT:
time.sleep(0.1)
# The connection process ends too, and closes the connection.
transport = client.get_transport()
while transport.is_active() and time.time() - start < LIMIT:
time.sleep(0.1)
closed = not transport.is_active()
client.close()
if alive(pid):
print("%s: command %d still running %ds after the channel close" %
(name, pid, LIMIT))
return False
print("%s: command %d gone %.1fs after the channel close" %
(name, pid, time.time() - start))
if not closed:
print("%s: connection still open %ds after the channel close" %
(name, LIMIT))
return False
# SIGKILL cannot run the trap, so the marker means the shell exited itself.
if shell and not os.path.exists(marker):
print("%s: killed instead of exiting on the hangup" % name)
return False
return True
def run_status(name, shell):
client = connect()
chan = client.get_transport().open_session()
if shell:
# An exec takes the pipe path even with a pty; a shell reads the pty.
chan.get_pty()
chan.invoke_shell()
chan.send("exit 3\n")
else:
chan.exec_command("exit 3")
start = time.time()
while not chan.exit_status_ready() and time.time() - start < WAIT:
time.sleep(0.05)
if not chan.exit_status_ready():
print("%s: no exit status within %ds" % (name, WAIT))
client.close()
return False
status = chan.recv_exit_status()
client.close()
print("%s: exit status %d, expected 3" % (name, status))
return status == 3
try:
connect().close()
except (paramiko.SSHException, OSError) as e:
print("cannot connect with paramiko: %s" % e)
sys.exit(77)
ok = run_case("exec", False)
ok = run_case("pty shell", True) and ok
ok = run_status("exec status", False) and ok
ok = run_status("shell status", True) and ok
sys.exit(0 if ok else 1)
EOF

66
apps/wolfsshd/wolfsshd.c
View File

@ -2672,6 +2672,12 @@ static int SHELL_SetNonBlocking(int fd)
#ifndef WOLFSSHD_SHELL_FLUSH_WAIT_US
#define WOLFSSHD_SHELL_FLUSH_WAIT_US 50000
#endif
#ifndef WOLFSSHD_CHILD_REAP_TRIES
#define WOLFSSHD_CHILD_REAP_TRIES 10
#endif
#ifndef WOLFSSHD_CHILD_REAP_WAIT_US
#define WOLFSSHD_CHILD_REAP_WAIT_US 50000
#endif
/* Send out the last of a shell's output once the child has been reaped. The
* SSH socket is non blocking, so retry a bounded number of times on a full
@ -2784,6 +2790,7 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
* destructive, so what a short write leaves is
* carried to the next pass. */
int childInIdx = 0; /* How much of those the child has taken. */
int drainPipes = 1;
backlog.len = 0;
backlog.state = SHELL_SEND_READY;
@ -2862,6 +2869,8 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
}
}
/* Before the fork, so a child that exits at once clears ChildRunning */
signal(SIGCHLD, ChildSig);
ChildRunning = 1;
childPid = forkpty(&childFd, NULL, NULL, NULL);
if (childPid < 0) {
@ -3039,7 +3048,6 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
return WS_FATAL_ERROR;
}
signal(SIGCHLD, ChildSig);
signal(SIGINT, SIG_DFL);
rc = tcgetattr(childFd, &tios);
@ -3487,7 +3495,10 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
/* Treat a 0 return as EOF so the loop can shut down. */
if (cnt_r < 0) {
int err = errno;
if (err != EINTR && err != EAGAIN
if (err == EIO) {
stdoutEmpty = 1; /* no process has the terminal open */
}
else if (err != EINTR && err != EAGAIN
&& err != EWOULDBLOCK) {
break;
}
@ -3535,14 +3546,45 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
}
}
/* Nothing more reaches the child's stdin, so let a reader see EOF */
if (stdinPipe[1] != -1 && (!ptyReq || forcedCmd)) {
close(stdinPipe[1]);
stdinPipe[1] = -1;
}
/* Close the pty master so the shell gets SIGHUP and can exit on its own */
if (childFd >= 0) {
close(childFd);
childFd = -1;
}
/* get return value of child process */
{
int waitStatus;
int tries = 0;
do {
rc = waitpid(childPid, &waitStatus, 0);
rc = waitpid(childPid, &waitStatus, WNOHANG);
if (rc == 0 && tries < WOLFSSHD_CHILD_REAP_TRIES) {
usleep(WOLFSSHD_CHILD_REAP_WAIT_US);
tries++;
}
else if (rc == 0) {
/* Child did not exit in time; force it with SIGKILL. */
if (kill(childPid, SIGKILL) == 0) {
rc = waitpid(childPid, &waitStatus, 0);
}
else {
int err = errno;
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Unable to kill child "
"process %d, errno %d", (int)childPid, err);
errno = err;
rc = -1;
}
}
/* if the waitpid experienced an interrupt then try again */
} while (rc < 0 && errno == EINTR);
} while (rc == 0 || (rc < 0 && errno == EINTR));
if (rc < 0) {
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Issue waiting for child's exit "
@ -3557,13 +3599,25 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
}
}
/* A break can leave unsent shell output in shellBuffer; send it before
* the pipe drain reads into shellBuffer. */
if (backlog.len > 0) {
if (backlog.state == SHELL_SEND_NEVER
|| SHELL_FlushOut(ssh, sshFd, shellChannelId, shellBuffer,
backlog.len, backlog.ext) != 0) {
/* later pipe output would leave a gap in the stream */
drainPipes = 0;
}
backlog.len = 0;
}
/* check for any left over data in pipes then close them */
if (!ptyReq || forcedCmd) {
int readSz;
/* when the pipe can not be made non blocking skip the drain, a
* blocking read here could hang the connection process */
if (SHELL_SetNonBlocking(stdoutPipe[0]) == 0) {
if (drainPipes && SHELL_SetNonBlocking(stdoutPipe[0]) == 0) {
readSz = (int)read(stdoutPipe[0], shellBuffer, sizeof shellBuffer);
if (readSz > 0) {
SHELL_FlushOut(ssh, sshFd, shellChannelId, shellBuffer, readSz,
@ -3571,7 +3625,7 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
}
}
if (SHELL_SetNonBlocking(stderrPipe[0]) == 0) {
if (drainPipes && SHELL_SetNonBlocking(stderrPipe[0]) == 0) {
readSz = (int)read(stderrPipe[0], shellBuffer, sizeof shellBuffer);
if (readSz > 0) {
SHELL_FlushOut(ssh, sshFd, shellChannelId, shellBuffer, readSz,