Skip to content

Commit 055d024

Browse files
dgarskedgarske
authored
Merge pull request #776 from JacobBarthelmeh/wolfsshd
fix for FD_SET call on pipes and handling of channel EOF
2 parents b8b72b6 + 3859213 commit 055d024

2 files changed

Lines changed: 104 additions & 27 deletions

File tree

‎.github/workflows/sshd-test.yml‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,25 @@ jobs:
108108
./configure --enable-all LDFLAGS="-L${{ github.workspace }}/build-dir/lib" CPPFLAGS="-I${{ github.workspace }}/build-dir/include -DWOLFSSH_NO_FPKI -DWOLFSSH_NO_SFTP_TIMEOUT -DWOLFSSH_MAX_SFTP_RW=4000000 -DMAX_PATH_SZ=120" --enable-static --disable-shared && make
109109
sudo timeout --preserve-status -s 2 5 valgrind --error-exitcode=1 --leak-check=full ./apps/wolfsshd/wolfsshd -D -f sshd_config -h ./keys/server-key.pem -d -p 22222
110110
111+
# regression test, check that cat command does not hang
112+
- name: Test cat command for hanging
113+
working-directory: ./wolfssh/
114+
timeout-minutes: 1
115+
run: |
116+
touch sshd_config.txt
117+
echo "AuthorizedKeysFile $PWD/authorized_keys_test" >> sshd_config.txt
118+
cat ./keys/hansel-*.pub > authorized_keys_test
119+
sed -i.bak "s/hansel/$USER/" ./authorized_keys_test
120+
./configure --enable-all LDFLAGS="-L${{ github.workspace }}/build-dir/lib" CPPFLAGS="-I${{ github.workspace }}/build-dir/include -DWOLFSSH_NO_FPKI -DWOLFSSH_NO_SFTP_TIMEOUT -DWOLFSSH_MAX_SFTP_RW=4000000 -DMAX_PATH_SZ=120" --enable-static --disable-shared && make
121+
sudo ./apps/wolfsshd/wolfsshd -f sshd_config.txt -h ./keys/server-key.pem -p 22225
122+
chmod 600 ./keys/hansel-key-rsa.pem
123+
tail -c 50000 /dev/urandom > test
124+
while ! nc -z 127.0.0.1 22225; do echo "waiting for wolfSSHd"; sleep 0.2; done
125+
cat test | ssh -vvv -T -i ./keys/hansel-key-rsa.pem -oStrictHostKeyChecking=no 127.0.0.1 -p 22225 'cat > test-file'
126+
diff test ~/test-file
127+
sudo pkill wolfsshd
128+
129+
111130
- name: configure with debug
112131
working-directory: ./wolfssh/
113132
run : |

‎apps/wolfsshd/wolfsshd.c‎

Lines changed: 85 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@
5757
#define WOLFSSHD_TIMEOUT 1
5858
#endif
5959

60+
#ifdef EXAMPLE_BUFFER_SZ
61+
#warning use WOLFSSHD_SHELL_BUFFER_SZ instead of EXAMPLE_BUFFER_SZ
62+
#define WOLFSSHD_SHELL_BUFFER_SZ EXAMPLE_BUFFER_SZ
63+
#endif
64+
6065
#if defined(WOLFSSH_SHELL) && !defined(_WIN32)
6166
#ifdef HAVE_PTY_H
6267
#include <pty.h>
@@ -815,10 +820,11 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
815820
{
816821
BOOL ret;
817822
word32 shellChannelId = 0;
818-
#ifndef EXAMPLE_BUFFER_SZ
819-
#define EXAMPLE_BUFFER_SZ 4096
823+
#ifndef WOLFSSHD_SHELL_BUFFER_SZ
824+
/* default to try and read max packet size */
825+
#define WOLFSSHD_SHELL_BUFFER_SZ 32768
820826
#endif
821-
byte shellBuffer[EXAMPLE_BUFFER_SZ];
827+
byte shellBuffer[WOLFSSHD_SHELL_BUFFER_SZ];
822828
int cnt_r, cnt_w;
823829
HANDLE ptyIn = NULL, ptyOut = NULL;
824830
HANDLE cnslIn = NULL, cnslOut = NULL;
@@ -1105,9 +1111,9 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
11051111
}
11061112

11071113
if (readPending) {
1108-
WMEMSET(shellBuffer, 0, EXAMPLE_BUFFER_SZ);
1114+
WMEMSET(shellBuffer, 0, WOLFSSHD_SHELL_BUFFER_SZ);
11091115

1110-
if (ReadFile(ptyOut, shellBuffer, EXAMPLE_BUFFER_SZ, &cnt_r,
1116+
if (ReadFile(ptyOut, shellBuffer, WOLFSSHD_SHELL_BUFFER_SZ, &cnt_r,
11111117
NULL) != TRUE) {
11121118
wolfSSH_Log(WS_LOG_INFO,
11131119
"[SSHD] Error reading from pipe for console");
@@ -1163,16 +1169,18 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
11631169
int rc;
11641170
WS_SOCKET_T childFd = 0;
11651171
int stdoutPipe[2], stderrPipe[2];
1172+
int stdinPipe[2];
11661173
pid_t childPid;
11671174

1168-
#ifndef EXAMPLE_BUFFER_SZ
1169-
#define EXAMPLE_BUFFER_SZ 4096
1175+
#ifndef WOLFSSHD_SHELL_BUFFER_SZ
1176+
/* default to try and read max packet size */
1177+
#define WOLFSSHD_SHELL_BUFFER_SZ 32768
11701178
#endif
11711179
#ifndef MAX_IDLE_COUNT
11721180
#define MAX_IDLE_COUNT 2
11731181
#endif
1174-
byte shellBuffer[EXAMPLE_BUFFER_SZ];
1175-
byte channelBuffer[EXAMPLE_BUFFER_SZ];
1182+
byte shellBuffer[WOLFSSHD_SHELL_BUFFER_SZ];
1183+
byte channelBuffer[WOLFSSHD_SHELL_BUFFER_SZ];
11761184
char* forcedCmd;
11771185
int windowFull = 0; /* Contains size of bytes from shellBuffer that did
11781186
* not get passed on to wolfSSH yet. This happens
@@ -1186,6 +1194,8 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
11861194
stdoutPipe[1] = -1;
11871195
stderrPipe[0] = -1;
11881196
stderrPipe[1] = -1;
1197+
stdinPipe[0] = -1;
1198+
stdinPipe[1] = -1;
11891199

11901200
forcedCmd = wolfSSHD_ConfigGetForcedCmd(usrConf);
11911201

@@ -1216,10 +1226,18 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
12161226
}
12171227
if (pipe(stderrPipe) != 0) {
12181228
close(stdoutPipe[0]);
1219-
close(stderrPipe[1]);
1229+
close(stdoutPipe[1]);
12201230
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Issue creating stderr pipe");
12211231
return WS_FATAL_ERROR;
12221232
}
1233+
if (pipe(stdinPipe) != 0) {
1234+
close(stdoutPipe[0]);
1235+
close(stdoutPipe[1]);
1236+
close(stderrPipe[0]);
1237+
close(stderrPipe[1]);
1238+
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Issue creating stdin pipe");
1239+
return WS_FATAL_ERROR;
1240+
}
12231241
}
12241242

12251243
ChildRunning = 1;
@@ -1243,8 +1261,20 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
12431261
if (forcedCmd) {
12441262
close(stdoutPipe[0]);
12451263
close(stderrPipe[0]);
1264+
close(stdinPipe[1]);
12461265
stdoutPipe[0] = -1;
12471266
stderrPipe[0] = -1;
1267+
stdinPipe[1] = -1;
1268+
1269+
if (dup2(stdinPipe[0], STDIN_FILENO) == -1) {
1270+
wolfSSH_Log(WS_LOG_ERROR,
1271+
"[SSHD] Error redirecting stdin pipe");
1272+
if (wolfSSHD_AuthReducePermissions(conn->auth) != WS_SUCCESS) {
1273+
exit(1);
1274+
}
1275+
1276+
return WS_FATAL_ERROR;
1277+
}
12481278
if (dup2(stdoutPipe[1], STDOUT_FILENO) == -1) {
12491279
wolfSSH_Log(WS_LOG_ERROR,
12501280
"[SSHD] Error redirecting stdout pipe");
@@ -1354,6 +1384,7 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
13541384
ret = execv(cmd, (char**)args);
13551385
close(stdoutPipe[1]);
13561386
close(stderrPipe[1]);
1387+
close(stdinPipe[1]);
13571388
}
13581389
else {
13591390
ret = execv(cmd, (char**)args);
@@ -1411,6 +1442,7 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
14111442
if (forcedCmd) {
14121443
close(stdoutPipe[1]);
14131444
close(stderrPipe[1]);
1445+
close(stdinPipe[0]);
14141446
}
14151447

14161448
while (ChildRunning || windowFull || !stdoutEmpty || peerConnected) {
@@ -1431,23 +1463,23 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
14311463
FD_SET(sshFd, &writeFds);
14321464
}
14331465

1434-
/* select on stdout/stderr pipes with forced commands */
1435-
if (forcedCmd) {
1436-
FD_SET(stdoutPipe[0], &readFds);
1437-
if (stdoutPipe[0] > maxFd)
1438-
maxFd = stdoutPipe[0];
1466+
if (wolfSSH_stream_peek(ssh, tmp, 1) <= 0) {
1467+
/* select on stdout/stderr pipes with forced commands */
1468+
if (forcedCmd) {
1469+
FD_SET(stdoutPipe[0], &readFds);
1470+
if (stdoutPipe[0] > maxFd)
1471+
maxFd = stdoutPipe[0];
14391472

1440-
FD_SET(stderrPipe[0], &readFds);
1441-
if (stderrPipe[0] > maxFd)
1442-
maxFd = stderrPipe[0];
1443-
}
1444-
else {
1445-
FD_SET(childFd, &readFds);
1446-
if (childFd > maxFd)
1447-
maxFd = childFd;
1448-
}
1473+
FD_SET(stderrPipe[0], &readFds);
1474+
if (stderrPipe[0] > maxFd)
1475+
maxFd = stderrPipe[0];
1476+
}
1477+
else {
1478+
FD_SET(childFd, &readFds);
1479+
if (childFd > maxFd)
1480+
maxFd = childFd;
1481+
}
14491482

1450-
if (wolfSSH_stream_peek(ssh, tmp, 1) <= 0) {
14511483
rc = select((int)maxFd + 1, &readFds, &writeFds, NULL, NULL);
14521484
if (rc == -1)
14531485
break;
@@ -1478,8 +1510,15 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
14781510
sizeof channelBuffer);
14791511
if (cnt_r <= 0)
14801512
break;
1481-
cnt_w = (int)write(childFd,
1482-
channelBuffer, cnt_r);
1513+
1514+
if (forcedCmd) {
1515+
cnt_w = (int)write(stdinPipe[1], channelBuffer,
1516+
cnt_r);
1517+
}
1518+
else {
1519+
cnt_w = (int)write(childFd, channelBuffer,
1520+
cnt_r);
1521+
}
14831522
if (cnt_w <= 0)
14841523
break;
14851524
}
@@ -1503,6 +1542,21 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
15031542
break;
15041543
}
15051544
}
1545+
1546+
/* did the channel just receive an EOF? */
1547+
if (cnt_r == 0) {
1548+
int eof;
1549+
WOLFSSH_CHANNEL* current;
1550+
1551+
current = wolfSSH_ChannelFind(ssh, lastChannel,
1552+
WS_CHANNEL_ID_SELF);
1553+
eof = wolfSSH_ChannelGetEof(current);
1554+
if (eof && forcedCmd) {
1555+
/* SSH is done, close stdin pipe to child process */
1556+
close(stdinPipe[1]);
1557+
stdinPipe[1] = -1;
1558+
}
1559+
}
15061560
}
15071561

15081562
/* if the window was previously full, try resending the data */
@@ -1683,8 +1737,12 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
16831737
if (readSz > 0) {
16841738
wolfSSH_extended_data_send(ssh, shellBuffer, readSz);
16851739
}
1740+
16861741
close(stdoutPipe[0]);
16871742
close(stderrPipe[0]);
1743+
if (stdinPipe[1] != -1) {
1744+
close(stdinPipe[1]);
1745+
}
16881746
}
16891747

16901748
(void)conn;

0 commit comments

Comments
 (0)