Skip to content

Commit

Permalink
Fix SCP server side
Browse files Browse the repository at this point in the history
SCP on the server side would get an EAGAIN around the 128KB mark, which
would trigger an error. That error in-turn would cause two attempts to
close the file, which would segfault.

Also fix inverted error return status on scpclient.
  • Loading branch information
LinuxJedi committed Feb 19, 2025
1 parent 697f54a commit 89c793d
Show file tree
Hide file tree
Showing 9 changed files with 98 additions and 36 deletions.
1 change: 1 addition & 0 deletions apps/wolfsshd/test/run_all_sshd_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ test_cases=(
"sshd_term_size_test.sh"
"sshd_large_sftp_test.sh"
"sshd_bad_sftp_test.sh"
"sshd_scp_fail.sh"
"sshd_term_close_test.sh"
"ssh_kex_algos.sh"
)
Expand Down
47 changes: 47 additions & 0 deletions apps/wolfsshd/test/sshd_scp_fail.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#!/bin/sh

# sshd local test

PWD=`pwd`
cd ../../..

TEST_SCP_CLIENT="./examples/scpclient/wolfscp"
USER=`whoami`
PRIVATE_KEY="./keys/hansel-key-ecc.der"
PUBLIC_KEY="./keys/hansel-key-ecc.pub"

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

mkdir test-$$

OUTDIR="`pwd`/test-$$"

dd if=/dev/random of=$OUTDIR/test.dat bs=1024 count=512

echo "$TEST_SCP_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -S$OUTDIR/test.dat:. -H $1 -p $2"
$TEST_SCP_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -S$OUTDIR/test.dat:. -H $1 -p $2

RESULT=$?
if [ "$RESULT" != "0" ]; then
echo "Expecting to pass transfer"
exit 1
fi

MD5SOURCE=`md5sum $OUTDIR/test.dat | awk '{ print $1 }'`
MD5DEST=`md5sum test.dat | awk '{ print $1 }'`

if [ "$MD5SOURCE" != "$MD5DEST" ]; then
echo "Files do not match $MD5SOURCE != $MD5DEST"
exit 1
fi

rm -rf test-$$
rm testout.dat

cd $PWD
exit 0

2 changes: 1 addition & 1 deletion examples/scpclient/scpclient.c
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ THREAD_RETURN WOLFSSH_THREAD scp_client(void* args)
wc_ecc_fp_free(); /* free per thread cache */
#endif

if (ret != WS_SUCCESS)
if ((ret != WS_SUCCESS) && (ret != WS_CHANNEL_CLOSED))
((func_args*)args)->return_code = 1;
return 0;
}
Expand Down
Binary file modified keys/ca-cert-ecc.der
Binary file not shown.
28 changes: 14 additions & 14 deletions keys/ca-cert-ecc.pem
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ Certificate:
Version: 3 (0x2)
Serial Number: 6 (0x6)
Signature Algorithm: ecdsa-with-SHA256
Issuer: C = US, ST = Washington, L = Seattle, O = wolfSSL, OU = Development, CN = www.wolfssl.com, emailAddress = [email protected]
Issuer: C=US, ST=Washington, L=Seattle, O=wolfSSL, OU=Development, CN=www.wolfssl.com, emailAddress=[email protected]
Validity
Not Before: Oct 1 05:54:44 2022 GMT
Not After : Sep 28 05:54:44 2032 GMT
Subject: C = US, ST = Washington, L = Seattle, O = wolfSSL, OU = Development, CN = www.wolfssl.com, emailAddress = [email protected]
Not Before: Feb 19 10:16:58 2025 GMT
Not After : Feb 17 10:16:58 2035 GMT
Subject: C=US, ST=Washington, L=Seattle, O=wolfSSL, OU=Development, CN=www.wolfssl.com, emailAddress=[email protected]
Subject Public Key Info:
Public Key Algorithm: id-ecPublicKey
Public-Key: (256 bit)
Expand All @@ -26,22 +26,22 @@ Certificate:
keyid:56:8E:9A:C3:F0:42:DE:18:B9:45:55:6E:F9:93:CF:EA:C3:F3:A5:21
DirName:/C=US/ST=Washington/L=Seattle/O=wolfSSL/OU=Development/CN=www.wolfssl.com/[email protected]
serial:06

X509v3 Basic Constraints: critical
CA:TRUE
X509v3 Key Usage: critical
Digital Signature, Certificate Sign, CRL Sign
Signature Algorithm: ecdsa-with-SHA256
30:45:02:20:18:bc:74:fd:d9:26:f2:f5:c2:f3:f5:cd:99:38:
9d:85:7d:8b:67:c8:f5:51:4a:5a:88:b6:3f:61:38:6b:9f:11:
02:21:00:f1:95:08:34:2b:47:32:93:8c:10:4b:4b:fd:6e:22:
f2:48:3b:5d:8a:74:46:24:7d:30:eb:65:15:06:e4:38:e0
Signature Value:
30:46:02:21:00:89:3c:83:7d:39:e1:f5:dd:48:f4:c0:f7:16:
ba:64:28:8d:9c:1f:f2:96:97:48:c9:31:ca:75:c0:13:d4:3d:
0c:02:21:00:f2:21:8a:3d:45:5d:cc:2a:22:11:e0:11:b3:54:
3f:7f:99:ea:ab:85:28:31:7c:c2:3f:50:d8:42:f2:23:e6:d2
-----BEGIN CERTIFICATE-----
MIIDJjCCAsygAwIBAgIBBjAKBggqhkjOPQQDAjCBlTELMAkGA1UEBhMCVVMxEzAR
MIIDJzCCAsygAwIBAgIBBjAKBggqhkjOPQQDAjCBlTELMAkGA1UEBhMCVVMxEzAR
BgNVBAgMCldhc2hpbmd0b24xEDAOBgNVBAcMB1NlYXR0bGUxEDAOBgNVBAoMB3dv
bGZTU0wxFDASBgNVBAsMC0RldmVsb3BtZW50MRgwFgYDVQQDDA93d3cud29sZnNz
bC5jb20xHTAbBgkqhkiG9w0BCQEWDmNhQGV4YW1wbGUuY29tMB4XDTIyMTAwMTA1
NTQ0NFoXDTMyMDkyODA1NTQ0NFowgZUxCzAJBgNVBAYTAlVTMRMwEQYDVQQIDApX
bC5jb20xHTAbBgkqhkiG9w0BCQEWDmNhQGV4YW1wbGUuY29tMB4XDTI1MDIxOTEw
MTY1OFoXDTM1MDIxNzEwMTY1OFowgZUxCzAJBgNVBAYTAlVTMRMwEQYDVQQIDApX
YXNoaW5ndG9uMRAwDgYDVQQHDAdTZWF0dGxlMRAwDgYDVQQKDAd3b2xmU1NMMRQw
EgYDVQQLDAtEZXZlbG9wbWVudDEYMBYGA1UEAwwPd3d3LndvbGZzc2wuY29tMR0w
GwYJKoZIhvcNAQkBFg5jYUBleGFtcGxlLmNvbTBZMBMGByqGSM49AgEGCCqGSM49
Expand All @@ -52,6 +52,6 @@ pSGhgZukgZgwgZUxCzAJBgNVBAYTAlVTMRMwEQYDVQQIDApXYXNoaW5ndG9uMRAw
DgYDVQQHDAdTZWF0dGxlMRAwDgYDVQQKDAd3b2xmU1NMMRQwEgYDVQQLDAtEZXZl
bG9wbWVudDEYMBYGA1UEAwwPd3d3LndvbGZzc2wuY29tMR0wGwYJKoZIhvcNAQkB
Fg5jYUBleGFtcGxlLmNvbYIBBjAPBgNVHRMBAf8EBTADAQH/MA4GA1UdDwEB/wQE
AwIBhjAKBggqhkjOPQQDAgNIADBFAiAYvHT92Sby9cLz9c2ZOJ2FfYtnyPVRSlqI
tj9hOGufEQIhAPGVCDQrRzKTjBBLS/1uIvJIO12KdEYkfTDrZRUG5Djg
AwIBhjAKBggqhkjOPQQDAgNJADBGAiEAiTyDfTnh9d1I9MD3FrpkKI2cH/KWl0jJ
Mcp1wBPUPQwCIQDyIYo9RV3MKiIR4BGzVD9/meqrhSgxfMI/UNhC8iPm0g==
-----END CERTIFICATE-----
10 changes: 5 additions & 5 deletions keys/renewcerts.cnf
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,14 @@ authorityKeyIdentifier=keyid:always,issuer:always
basicConstraints = critical,CA:true
keyUsage = critical, digitalSignature, keyCertSign, cRLSign

# Extensions for fred cert
[ v3_fred ]
# Extensions for root cert
[ v3_root ]
subjectKeyIdentifier=hash
authorityKeyIdentifier=keyid:always,issuer:always
subjectAltName = @fred_altnames
subjectAltName = @root_altnames

[ fred_altnames ]
otherName = msUPN;UTF8:fred@example
[ root_altnames ]
otherName = msUPN;UTF8:root@example

# Extensions for server cert
[ v3_server ]
Expand Down
Binary file modified keys/server-cert.der
Binary file not shown.
28 changes: 14 additions & 14 deletions keys/server-cert.pem
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ Certificate:
Version: 3 (0x2)
Serial Number: 8 (0x8)
Signature Algorithm: ecdsa-with-SHA256
Issuer: C = US, ST = Washington, L = Seattle, O = wolfSSL, OU = Development, CN = www.wolfssl.com, emailAddress = [email protected]
Issuer: C=US, ST=Washington, L=Seattle, O=wolfSSL, OU=Development, CN=www.wolfssl.com, emailAddress=[email protected]
Validity
Not Before: Oct 1 05:54:44 2022 GMT
Not After : Sep 28 05:54:44 2032 GMT
Subject: C = US, ST = Washington, L = Seattle, O = Eliptic, OU = ECC, CN = www.wolfssl.com, emailAddress = [email protected]
Not Before: Feb 19 10:16:59 2025 GMT
Not After : Feb 17 10:16:59 2035 GMT
Subject: C=US, ST=Washington, L=Seattle, O=Eliptic, OU=ECC, CN=www.wolfssl.com, emailAddress=[email protected]
Subject Public Key Info:
Public Key Algorithm: id-ecPublicKey
Public-Key: (256 bit)
Expand All @@ -26,20 +26,20 @@ Certificate:
keyid:56:8E:9A:C3:F0:42:DE:18:B9:45:55:6E:F9:93:CF:EA:C3:F3:A5:21
DirName:/C=US/ST=Washington/L=Seattle/O=wolfSSL/OU=Development/CN=www.wolfssl.com/[email protected]
serial:06

X509v3 Subject Alternative Name:
DNS:example, IP Address:127.0.0.1
Signature Algorithm: ecdsa-with-SHA256
30:45:02:20:42:d8:a0:95:e7:aa:4e:63:fd:50:6e:6b:f9:98:
90:be:3d:44:53:68:1b:66:dd:22:a3:12:77:70:94:56:db:82:
02:21:00:ce:18:b2:10:b2:2d:2a:b9:79:d4:76:64:df:28:91:
23:8d:93:22:e9:4b:ea:7f:49:4e:eb:65:ce:c8:86:ba:fb
Signature Value:
30:44:02:20:75:7f:24:0a:80:3c:90:38:3a:a1:16:86:ba:44:
43:0a:75:34:52:a0:d0:2e:29:b1:a1:92:e3:85:d2:b4:24:be:
02:20:63:d5:b8:eb:3c:cf:19:3e:60:78:7b:01:f1:e4:94:d6:
fd:3a:73:f1:05:a1:74:07:fd:cb:55:59:fc:2c:19:72
-----BEGIN CERTIFICATE-----
MIIDGjCCAsCgAwIBAgIBCDAKBggqhkjOPQQDAjCBlTELMAkGA1UEBhMCVVMxEzAR
MIIDGTCCAsCgAwIBAgIBCDAKBggqhkjOPQQDAjCBlTELMAkGA1UEBhMCVVMxEzAR
BgNVBAgMCldhc2hpbmd0b24xEDAOBgNVBAcMB1NlYXR0bGUxEDAOBgNVBAoMB3dv
bGZTU0wxFDASBgNVBAsMC0RldmVsb3BtZW50MRgwFgYDVQQDDA93d3cud29sZnNz
bC5jb20xHTAbBgkqhkiG9w0BCQEWDmNhQGV4YW1wbGUuY29tMB4XDTIyMTAwMTA1
NTQ0NFoXDTMyMDkyODA1NTQ0NFowgZExCzAJBgNVBAYTAlVTMRMwEQYDVQQIDApX
bC5jb20xHTAbBgkqhkiG9w0BCQEWDmNhQGV4YW1wbGUuY29tMB4XDTI1MDIxOTEw
MTY1OVoXDTM1MDIxNzEwMTY1OVowgZExCzAJBgNVBAYTAlVTMRMwEQYDVQQIDApX
YXNoaW5ndG9uMRAwDgYDVQQHDAdTZWF0dGxlMRAwDgYDVQQKDAdFbGlwdGljMQww
CgYDVQQLDANFQ0MxGDAWBgNVBAMMD3d3dy53b2xmc3NsLmNvbTEhMB8GCSqGSIb3
DQEJARYSc2VydmVyQGV4YW1wbGUuY29tMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcD
Expand All @@ -50,6 +50,6 @@ pIGYMIGVMQswCQYDVQQGEwJVUzETMBEGA1UECAwKV2FzaGluZ3RvbjEQMA4GA1UE
BwwHU2VhdHRsZTEQMA4GA1UECgwHd29sZlNTTDEUMBIGA1UECwwLRGV2ZWxvcG1l
bnQxGDAWBgNVBAMMD3d3dy53b2xmc3NsLmNvbTEdMBsGCSqGSIb3DQEJARYOY2FA
ZXhhbXBsZS5jb22CAQYwGAYDVR0RBBEwD4IHZXhhbXBsZYcEfwAAATAKBggqhkjO
PQQDAgNIADBFAiBC2KCV56pOY/1Qbmv5mJC+PURTaBtm3SKjEndwlFbbggIhAM4Y
shCyLSq5edR2ZN8okSONkyLpS+p/SU7rZc7Ihrr7
PQQDAgNHADBEAiB1fyQKgDyQODqhFoa6REMKdTRSoNAuKbGhkuOF0rQkvgIgY9W4
6zzPGT5geHsB8eSU1v06c/EFoXQH/ctVWfwsGXI=
-----END CERTIFICATE-----
18 changes: 16 additions & 2 deletions src/wolfscp.c
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ int DoScpSource(WOLFSSH* ssh)
ssh->scpBufferedSz);
if (ret == WS_WINDOW_FULL || ret == WS_REKEYING) {
ret = wolfSSH_worker(ssh, NULL);
if (ret == WS_SUCCESS)
if (ret == WS_SUCCESS || ssh->error == WS_WANT_READ)
continue;
}
if (ret == WS_EXTDATA) {
Expand All @@ -616,8 +616,10 @@ int DoScpSource(WOLFSSH* ssh)
* open file descriptor before exit */
ScpSendCtx* sendCtx = NULL;
sendCtx = (ScpSendCtx*)wolfSSH_GetScpSendCtx(ssh);
if (sendCtx != NULL)
if (sendCtx != NULL) {
WFCLOSE(ssh->fs, sendCtx->fp);
sendCtx->fp = NULL;
}
#endif
WLOG(WS_LOG_ERROR, scpError, "failed to send file", ret);
break;
Expand Down Expand Up @@ -1181,6 +1183,7 @@ static int ParseBasePathHelper(WOLFSSH* ssh, int cmdSz)

if (ScpPushDir(ssh->fs, &ctx, ssh->scpBasePath, ssh->ctx->heap) != WS_SUCCESS) {
WLOG(WS_LOG_DEBUG, "scp : issue opening base dir");
ssh->error = WS_INVALID_PATH_E;
ret = WS_FATAL_ERROR;
}
else {
Expand Down Expand Up @@ -2021,6 +2024,7 @@ int wsScpRecvCallback(WOLFSSH* ssh, int state, const char* basePath,
WLOG(WS_LOG_ERROR, scpError, "scp receive callback unable "
"to write requested size to file", bytes);
WFCLOSE(ssh->fs, fp);
fp = NULL;
ret = WS_SCP_ABORT;
} else {
#ifdef WOLFSCP_FLUSH
Expand All @@ -2047,6 +2051,7 @@ int wsScpRecvCallback(WOLFSSH* ssh, int state, const char* basePath,
flush_bytes = 0;
#endif
WFCLOSE(ssh->fs, fp);
fp = NULL;
}

/* set timestamp info */
Expand Down Expand Up @@ -2587,6 +2592,7 @@ static int ScpProcessEntry(WOLFSSH* ssh, char* fileName, word64* mTime,
if ((sendCtx->fp != NULL) &&
((ret < 0) || (*totalFileSz == (word32)ret))) {
WFCLOSE(ssh->fs, sendCtx->fp);
sendCtx->fp = NULL;
}
}

Expand Down Expand Up @@ -2758,6 +2764,7 @@ int wsScpSendCallback(WOLFSSH* ssh, int state, const char* peerRequest,
if ((sendCtx != NULL) && (sendCtx->fp != NULL) &&
((ret < 0) || (*totalFileSz == (word32)ret))) {
WFCLOSE(ssh->fs, sendCtx->fp);
sendCtx->fp = NULL;
}

break;
Expand Down Expand Up @@ -2840,13 +2847,20 @@ int wsScpSendCallback(WOLFSSH* ssh, int state, const char* peerRequest,
break;
}

if (sendCtx->fp == NULL) {
WLOG(WS_LOG_ERROR, "scp: file has been closed, abort");
ret = WS_SCP_ABORT;
break;
}

ret = (word32)WFREAD(ssh->fs, buf, 1, bufSz, sendCtx->fp);
if (ret == 0) { /* handle case of EOF */
ret = WS_EOF;
}

if ((ret <= 0) || (fileOffset + ret == *totalFileSz)) {
WFCLOSE(ssh->fs, sendCtx->fp);
sendCtx->fp = NULL;
}

break;
Expand Down

0 comments on commit 89c793d

Please sign in to comment.