From 2722cb9da8a07ef86547fbf4615cdc51cd7af108 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Mon, 24 Jun 2024 15:11:22 -0600 Subject: [PATCH 1/5] handle send retry when SSH window is full --- apps/wolfsshd/wolfsshd.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index ce0567595..6558bfc48 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -1495,17 +1495,19 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, /* if the window was previously full, try resending the data */ if (windowFull) { cnt_w = wolfSSH_ChannelIdSend(ssh, shellChannelId, - shellBuffer, cnt_r); + shellBuffer, windowFull); if (cnt_w == WS_WINDOW_FULL) { - windowFull = 1; continue; } else if (cnt_w == WS_WANT_WRITE) { - windowFull = 1; continue; } else { - windowFull = 0; + windowFull -= cnt_w; + if (windowFull > 0) + continue; + if (windowFull < 0) + windowFull = 0; } } @@ -1525,7 +1527,7 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, cnt_w = wolfSSH_extended_data_send(ssh, shellBuffer, cnt_r); if (cnt_w == WS_WINDOW_FULL) { - windowFull = 1; + windowFull = cnt_r; /* save amount to be sent */ continue; } else if (cnt_w == WS_WANT_WRITE) { @@ -1557,7 +1559,7 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, cnt_w = wolfSSH_ChannelIdSend(ssh, shellChannelId, shellBuffer, cnt_r); if (cnt_w == WS_WINDOW_FULL) { - windowFull = 1; + windowFull = cnt_r; /* save amount to be sent */ continue; } else if (cnt_w == WS_WANT_WRITE) { From 46832e46b441cb74ff8c27bc7824bd17562fd3de Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Mon, 1 Jul 2024 11:18:32 -0600 Subject: [PATCH 2/5] add test case --- apps/wolfsshd/test/README.md | 2 +- apps/wolfsshd/test/run_all_sshd_tests.sh | 3 +- apps/wolfsshd/test/sshd_window_full_test.sh | 56 +++++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) create mode 100755 apps/wolfsshd/test/sshd_window_full_test.sh diff --git a/apps/wolfsshd/test/README.md b/apps/wolfsshd/test/README.md index eba11ad56..1bd285b0c 100644 --- a/apps/wolfsshd/test/README.md +++ b/apps/wolfsshd/test/README.md @@ -10,7 +10,7 @@ To run all tests do: ``` $ cd apps/wolfsshd/test/ -$ sudo ./run_all_sshd_tests.sh +$ sudo ./run_all_sshd_tests.sh Running all wolfSSHd tests Starting up local wolfSSHd for tests on 127.0.0.1:22222 SSHD running on PID 7979 diff --git a/apps/wolfsshd/test/run_all_sshd_tests.sh b/apps/wolfsshd/test/run_all_sshd_tests.sh index d103939c4..b851963ef 100755 --- a/apps/wolfsshd/test/run_all_sshd_tests.sh +++ b/apps/wolfsshd/test/run_all_sshd_tests.sh @@ -76,9 +76,10 @@ fi # these tests require setting up an sshd if [ "$USING_LOCAL_HOST" == 1 ]; then run_test "sshd_forcedcmd_test.sh" + run_test "sshd_window_full_test.sh" else printf "Skipping tests that need to setup local SSHD\n" - SKIPPED=$((SKIPPED+1)) + SKIPPED=$((SKIPPED+2)) fi # these tests run with X509 sshd-config loaded diff --git a/apps/wolfsshd/test/sshd_window_full_test.sh b/apps/wolfsshd/test/sshd_window_full_test.sh new file mode 100755 index 000000000..ae49d1cab --- /dev/null +++ b/apps/wolfsshd/test/sshd_window_full_test.sh @@ -0,0 +1,56 @@ +#!/bin/bash + +# sshd local test + +if [ -z "$1" ] || [ -z "$2" ]; then + echo "expecting host and port as arguments" + echo "./sshd_exec_test.sh 127.0.0.1 22222" + exit 1 +fi + +PWD=`pwd` + +if [ ! -z "$3" ]; then + USER="$3" +else + USER=`whoami` +fi +TEST_PORT="$2" +TEST_HOST="$1" +source ./start_sshd.sh +cat < sshd_config_test_window +Port $TEST_PORT +Protocol 2 +LoginGraceTime 600 +PermitRootLogin yes +PasswordAuthentication yes +PermitEmptyPasswords no +UsePrivilegeSeparation no +UseDNS no +HostKey $PWD/../../../keys/server-key.pem +AuthorizedKeysFile $PWD/authorized_keys_test +EOF + +start_wolfsshd "sshd_config_test_forcedcmd" +cd ../../.. + +TEST_CLIENT="./examples/client/client" +TEST_SFTP="./examples/sftpclient/wolfsftp" +PRIVATE_KEY="./keys/hansel-key-ecc.der" +PUBLIC_KEY="./keys/hansel-key-ecc.pub" + +head -c 1G /dev/urandom > random-test.txt + +PWD=`pwd` +$TEST_CLIENT -c "cd $PWD; $TEST_CLIENT -c \"cat $PWD/random-test.txt\" -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h $TEST_HOST -p $TEST_PORT" -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h $TEST_HOST -p $TEST_PORT > random-test-result.txt + +diff random-test.txt random-test-result.txt +RESULT=$? +if [ "$RESULT" != 0 ]; then + echo "cat did not pass through all expected data" + exit 1 +fi + +stop_wolfsshd +exit 0 + From 781aa27582cf73246e5d61658861da7f5336e367 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Wed, 17 Jul 2024 16:46:07 -0600 Subject: [PATCH 3/5] touch up after rebase --- apps/wolfsshd/wolfsshd.c | 53 ++++++++++++++++++++++++---------------- examples/client/common.c | 2 ++ 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index 6558bfc48..36206a663 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -1173,7 +1173,10 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, byte shellBuffer[EXAMPLE_BUFFER_SZ]; byte channelBuffer[EXAMPLE_BUFFER_SZ]; char* forcedCmd; - int windowFull = 0; + int windowFull = 0; /* Contains size of bytes from shellBuffer that did + * not get passed on to wolfSSH yet. This happens + * with window full errors or when rekeying. */ + int wantWrite = 0; int peerConnected = 1; int stdoutEmpty = 0; @@ -1423,7 +1426,7 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, maxFd = sshFd; FD_ZERO(&writeFds); - if (windowFull) { + if (windowFull || wantWrite) { FD_SET(sshFd, &writeFds); } @@ -1452,10 +1455,10 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, pending = 1; /* found some pending SSH data */ } - if (windowFull || pending || FD_ISSET(sshFd, &readFds)) { + if (wantWrite || windowFull || pending || FD_ISSET(sshFd, &readFds)) { word32 lastChannel = 0; - windowFull = 0; + wantWrite = 0; /* The following tries to read from the first channel inside the stream. If the pending data in the socket is for another channel, this will return an error with id @@ -1466,16 +1469,19 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, if (cnt_r < 0) { rc = wolfSSH_get_error(ssh); if (rc == WS_CHAN_RXD) { - if (lastChannel == shellChannelId) { - cnt_r = wolfSSH_ChannelIdRead(ssh, shellChannelId, + if (!windowFull) { /* don't rewrite channeldBuffer if full + * of windowFull left overs */ + if (lastChannel == shellChannelId) { + cnt_r = wolfSSH_ChannelIdRead(ssh, shellChannelId, channelBuffer, sizeof channelBuffer); - if (cnt_r <= 0) - break; - cnt_w = (int)write(childFd, - channelBuffer, cnt_r); - if (cnt_w <= 0) - break; + if (cnt_r <= 0) + break; + cnt_w = (int)write(childFd, + channelBuffer, cnt_r); + if (cnt_w <= 0) + break; + } } } else if (rc == WS_CHANNEL_CLOSED) { @@ -1483,7 +1489,11 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, continue; } else if (rc == WS_WANT_WRITE) { - windowFull = 1; + wantWrite = 1; + continue; + } + else if (rc == WS_REKEYING) { + wantWrite = 1; continue; } else if (rc != WS_WANT_READ) { @@ -1496,10 +1506,11 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, if (windowFull) { cnt_w = wolfSSH_ChannelIdSend(ssh, shellChannelId, shellBuffer, windowFull); - if (cnt_w == WS_WINDOW_FULL) { + if (cnt_w == WS_WINDOW_FULL || cnt_w == WS_REKEYING) { continue; } else if (cnt_w == WS_WANT_WRITE) { + wantWrite = 1; continue; } else { @@ -1526,12 +1537,12 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, if (cnt_r > 0) { cnt_w = wolfSSH_extended_data_send(ssh, shellBuffer, cnt_r); - if (cnt_w == WS_WINDOW_FULL) { + if (cnt_w == WS_WINDOW_FULL || cnt_w == WS_REKEYING) { windowFull = cnt_r; /* save amount to be sent */ continue; } else if (cnt_w == WS_WANT_WRITE) { - windowFull = 1; + wantWrite = 1; continue; } else if (cnt_w < 0) @@ -1558,12 +1569,12 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, if (cnt_r > 0) { cnt_w = wolfSSH_ChannelIdSend(ssh, shellChannelId, shellBuffer, cnt_r); - if (cnt_w == WS_WINDOW_FULL) { + if (cnt_w == WS_WINDOW_FULL || cnt_w == WS_REKEYING) { windowFull = cnt_r; /* save amount to be sent */ continue; } else if (cnt_w == WS_WANT_WRITE) { - windowFull = 1; + wantWrite = 1; continue; } else if (cnt_w < 0) { @@ -1588,12 +1599,12 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, if (cnt_r > 0) { cnt_w = wolfSSH_ChannelIdSend(ssh, shellChannelId, shellBuffer, cnt_r); - if (cnt_w == WS_WINDOW_FULL) { - windowFull = 1; + if (cnt_w == WS_WINDOW_FULL || cnt_w == WS_REKEYING) { + windowFull = cnt_r; continue; } else if (cnt_w == WS_WANT_WRITE) { - windowFull = 1; + wantWrite = 1; continue; } else if (cnt_w < 0) { diff --git a/examples/client/common.c b/examples/client/common.c index c4281ab4f..db27d0f96 100644 --- a/examples/client/common.c +++ b/examples/client/common.c @@ -468,8 +468,10 @@ int ClientUserAuth(byte authType, * passed in a public key file, use public key auth */ if (pubKeyLoaded == 1) { if (authType == WOLFSSH_USERAUTH_PASSWORD) { + #ifdef WOLFSSH_DEBUG printf("rejecting password type with %s in favor of pub key\n", (char*)authData->username); + #endif return WOLFSSH_USERAUTH_FAILURE; } } From 15f4dc9d0fb3453dff581c113ddfe5a0aaacb2ae Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Wed, 17 Jul 2024 17:10:03 -0600 Subject: [PATCH 4/5] adjustments to test case --- apps/wolfsshd/test/sshd_window_full_test.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/wolfsshd/test/sshd_window_full_test.sh b/apps/wolfsshd/test/sshd_window_full_test.sh index ae49d1cab..3262c2ecb 100755 --- a/apps/wolfsshd/test/sshd_window_full_test.sh +++ b/apps/wolfsshd/test/sshd_window_full_test.sh @@ -31,7 +31,7 @@ HostKey $PWD/../../../keys/server-key.pem AuthorizedKeysFile $PWD/authorized_keys_test EOF -start_wolfsshd "sshd_config_test_forcedcmd" +start_wolfsshd "sshd_config_test_window" cd ../../.. TEST_CLIENT="./examples/client/client" @@ -48,6 +48,8 @@ diff random-test.txt random-test-result.txt RESULT=$? if [ "$RESULT" != 0 ]; then echo "cat did not pass through all expected data" + ls -la random-test.txt + ls -la random-test-result.txt exit 1 fi From 2fbe01092d88e37bbd598c3eba26ff3149a08e06 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Mon, 5 Aug 2024 14:48:51 -0600 Subject: [PATCH 5/5] account for partial sends --- apps/wolfsshd/wolfsshd.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index 36206a663..31adce293 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -1515,8 +1515,10 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, } else { windowFull -= cnt_w; - if (windowFull > 0) + if (windowFull > 0) { + WMEMMOVE(shellBuffer, shellBuffer + cnt_w, windowFull); continue; + } if (windowFull < 0) windowFull = 0; } @@ -1537,7 +1539,13 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, if (cnt_r > 0) { cnt_w = wolfSSH_extended_data_send(ssh, shellBuffer, cnt_r); - if (cnt_w == WS_WINDOW_FULL || cnt_w == WS_REKEYING) { + if (cnt_w > 0 && cnt_w < cnt_r) { /* partial send */ + windowFull = cnt_r - cnt_w; + WMEMMOVE(shellBuffer, shellBuffer + cnt_w, + windowFull); + } + else if (cnt_w == WS_WINDOW_FULL || + cnt_w == WS_REKEYING) { windowFull = cnt_r; /* save amount to be sent */ continue; } @@ -1569,7 +1577,13 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, if (cnt_r > 0) { cnt_w = wolfSSH_ChannelIdSend(ssh, shellChannelId, shellBuffer, cnt_r); - if (cnt_w == WS_WINDOW_FULL || cnt_w == WS_REKEYING) { + if (cnt_w > 0 && cnt_w < cnt_r) { /* partial send */ + windowFull = cnt_r - cnt_w; + WMEMMOVE(shellBuffer, shellBuffer + cnt_w, + windowFull); + } + else if (cnt_w == WS_WINDOW_FULL || + cnt_w == WS_REKEYING) { windowFull = cnt_r; /* save amount to be sent */ continue; } @@ -1599,7 +1613,13 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, if (cnt_r > 0) { cnt_w = wolfSSH_ChannelIdSend(ssh, shellChannelId, shellBuffer, cnt_r); - if (cnt_w == WS_WINDOW_FULL || cnt_w == WS_REKEYING) { + if (cnt_w > 0 && cnt_w < cnt_r) { /* partial send */ + windowFull = cnt_r - cnt_w; + WMEMMOVE(shellBuffer, shellBuffer + cnt_w, + windowFull); + } + else if (cnt_w == WS_WINDOW_FULL || + cnt_w == WS_REKEYING) { windowFull = cnt_r; continue; }