From f8de131e81893768b4528ebccf5767934e7d60a1 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Fri, 13 Oct 2023 16:56:17 -0600 Subject: [PATCH 01/47] fix for shared socket ID between connections --- apps/wolfsshd/auth.c | 2 +- apps/wolfsshd/wolfsshd.c | 38 +++++++++++++++++++++++++------------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index 4da1fb8d8..136e75cd8 100644 --- a/apps/wolfsshd/auth.c +++ b/apps/wolfsshd/auth.c @@ -762,7 +762,7 @@ static int CheckPasswordWIN(const char* usr, const byte* pw, word32 pwSz, WOLFSS usrWSz = WSTRLEN(usr) * sizeof(WCHAR); - usrW = (WCHAR*)WMALLOC(usrWSz + 1, authCtx->heap, DYNTYPE_SSHD); + usrW = (WCHAR*)WMALLOC((usrWSz * sizeof(WCHAR)) + sizeof(WCHAR), authCtx->heap, DYNTYPE_SSHD); if (usrW == NULL) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Ran out of memory"); ret = WSSHD_AUTH_FAILURE; diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index db48ea8d5..e61ac30ed 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -1671,6 +1671,7 @@ static void* HandleConnection(void* arg) WCLOSESOCKET(conn->fd); } wolfSSH_Log(WS_LOG_INFO, "[SSHD] Return from closing connection = %d", ret); + WFREE(conn, NULL, DYNTYPE_SSHD); #ifdef _WIN32 return 0; @@ -1966,7 +1967,11 @@ static int StartSSHD(int argc, char** argv) break; #else ShowUsage(); + #ifndef _WIN32 return WS_FATAL_ERROR; + #else + return; + #endif #endif case 't': @@ -2144,27 +2149,34 @@ static int StartSSHD(int argc, char** argv) #endif /* wait for incoming connections and fork them off */ while (ret == WS_SUCCESS && quit == 0) { - WOLFSSHD_CONNECTION conn; + WOLFSSHD_CONNECTION* conn; #ifdef WOLFSSL_NUCLEUS struct addr_struct clientAddr; #else SOCKADDR_IN_T clientAddr; socklen_t clientAddrSz = sizeof(clientAddr); #endif - conn.auth = auth; - conn.listenFd = (int)listenFd; - conn.isThreaded = isDaemon; + conn = (WOLFSSHD_CONNECTION*)WMALLOC(sizeof(WOLFSSHD_CONNECTION), NULL, DYNTYPE_SSHD); + if (conn == NULL) { + wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Failed to malloc memory for connection"); + ret = WS_MEMORY_E; + break; + } + + conn->auth = auth; + conn->listenFd = (int)listenFd; + conn->isThreaded = isDaemon; /* wait for a connection */ if (PendingConnection(listenFd)) { - conn.ctx = ctx; + conn->ctx = ctx; #ifdef WOLFSSL_NUCLEUS - conn.fd = NU_Accept(listenFd, &clientAddr, 0); + conn->fd = NU_Accept(listenFd, &clientAddr, 0); #else - conn.fd = (int)accept(listenFd, (struct sockaddr*)&clientAddr, + conn->fd = (int)accept(listenFd, (struct sockaddr*)&clientAddr, &clientAddrSz); - if (conn.fd >= 0) { - inet_ntop(AF_INET, &clientAddr.sin_addr, conn.ip, + if (conn->fd >= 0) { + inet_ntop(AF_INET, &clientAddr.sin_addr, conn->ip, INET_ADDRSTRLEN); } #endif @@ -2172,7 +2184,7 @@ static int StartSSHD(int argc, char** argv) { #ifdef USE_WINDOWS_API unsigned long blocking = 1; - if (ioctlsocket(conn.fd, FIONBIO, &blocking) + if (ioctlsocket(conn->fd, FIONBIO, &blocking) == SOCKET_ERROR) err_sys("ioctlsocket failed"); #elif defined(WOLFSSL_MDK_ARM) || defined(WOLFSSL_KEIL_TCP_NET) \ @@ -2180,7 +2192,7 @@ static int StartSSHD(int argc, char** argv) defined(WOLFSSL_NUCLEUS) /* non blocking not supported, for now */ #else - int flags = fcntl(conn.fd, F_GETFL, 0); + int flags = fcntl(conn->fd, F_GETFL, 0); if (flags < 0) err_sys("fcntl get failed"); flags = fcntl(conn.fd, F_SETFL, flags | O_NONBLOCK); @@ -2188,7 +2200,7 @@ static int StartSSHD(int argc, char** argv) err_sys("fcntl set failed"); #endif } - ret = NewConnection(&conn); + ret = NewConnection(conn); } #ifdef _WIN32 /* check if service has been shutdown */ @@ -2258,7 +2270,7 @@ int main(int argc, char** argv) } } else { - StartSSHD(argc, (LPSTR*)argv); + StartSSHD(argc, (LPTSTR*)argv); } return 0; #else From 46d2017d7630b3acdc56ca1ba7ac096854bc0f61 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Mon, 16 Oct 2023 07:57:50 -0600 Subject: [PATCH 02/47] fix reference to struct --- apps/wolfsshd/wolfsshd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index e61ac30ed..bf3ef8c3e 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -2195,7 +2195,7 @@ static int StartSSHD(int argc, char** argv) int flags = fcntl(conn->fd, F_GETFL, 0); if (flags < 0) err_sys("fcntl get failed"); - flags = fcntl(conn.fd, F_SETFL, flags | O_NONBLOCK); + flags = fcntl(conn->fd, F_SETFL, flags | O_NONBLOCK); if (flags < 0) err_sys("fcntl set failed"); #endif From e3637574dd6e6000cee9da5d69d444e35a2b04eb Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Mon, 16 Oct 2023 09:16:34 -0600 Subject: [PATCH 03/47] remove unused ret value --- apps/wolfsshd/wolfsshd.c | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index bf3ef8c3e..003d112be 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -2159,7 +2159,6 @@ static int StartSSHD(int argc, char** argv) conn = (WOLFSSHD_CONNECTION*)WMALLOC(sizeof(WOLFSSHD_CONNECTION), NULL, DYNTYPE_SSHD); if (conn == NULL) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Failed to malloc memory for connection"); - ret = WS_MEMORY_E; break; } From b4858be16b31420eac5d3c9042428eb5ea39d3c7 Mon Sep 17 00:00:00 2001 From: Fabio <507164+falemagn@users.noreply.github.com> Date: Fri, 20 Oct 2023 14:06:39 +0200 Subject: [PATCH 04/47] Fix for #605: check for WS_WANT_READ too. --- src/internal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index 098f51825..4d4acee9c 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7844,7 +7844,7 @@ int DoReceive(WOLFSSH* ssh) ssh->error = ret; if (ret < 0 && !(ret == WS_CHAN_RXD || ret == WS_EXTDATA || ret == WS_CHANNEL_CLOSED || ret == WS_WANT_WRITE || - ret == WS_REKEYING)) { + ret == WS_REKEYING || ret == WS_WANT_READ)) { return WS_FATAL_ERROR; } WLOG(WS_LOG_DEBUG, "PR3: peerMacSz = %u", peerMacSz); From 92669d0f1a1321aa28ed81dead5750c4f3ddfe24 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 23 Oct 2023 09:12:07 -0700 Subject: [PATCH 05/47] Threading Fix 1. The wolfSSL version number to check for the old threading code was v5.5.2, not v5.5.1. 2. A new wrapper was introduced around return for threading in what will be wolfSSL v5.6.4. Added that wrapper if it didn't exist. 3. Some other types for threading are added in v5.6.4, wolfSSH needs to use the old threading model if using wolfSSL v5.6.3. --- wolfssh/test.h | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/wolfssh/test.h b/wolfssh/test.h index b4db3b4d5..e584018d7 100644 --- a/wolfssh/test.h +++ b/wolfssh/test.h @@ -897,12 +897,14 @@ static INLINE void WaitTcpReady(func_args* args) * The tag WOLFSSL_THREAD is defined as a part of this compatibility, and * will also be checked for. Note that the following types and defines are * used by the examples to define themselves for use as threads by the test - * tools, but they themselves do not use threading. + * tools, but they themselves do not use threading. Before v5.6.4, a new + * macro for return from threads was added. */ -#define WOLFSSL_V5_5_1 0x05005001 +#define WOLFSSL_V5_5_2 0x05005002 +#define WOLFSSL_V5_6_4 0x05006004 -#if (LIBWOLFSSL_VERSION_HEX < WOLFSSL_V5_5_1) && !defined(WOLFSSL_THREAD) - #define WOLFSSH_OLD_THREADING +#if (LIBWOLFSSL_VERSION_HEX < WOLFSSL_V5_5_2) && !defined(WOLFSSL_THREAD) + #define WOLFSSH_OLDER_THREADING #ifdef SINGLE_THREADED typedef unsigned int THREAD_RETURN; typedef void* THREAD_TYPE; @@ -927,11 +929,17 @@ static INLINE void WaitTcpReady(func_args* args) #define WOLFSSH_THREAD WOLFSSL_THREAD #endif +#if (LIBWOLFSSL_VERSION_HEX < WOLFSSL_V5_6_4) \ + && !defined(WOLFSSL_RETURN_FROM_THREAD) + #define WOLFSSL_RETURN_FROM_THREAD(x) return (THREAD_RETURN)(x) + #define WOLFSSH_OLD_THREADING +#endif + #ifdef WOLFSSH_TEST_THREADING -#ifndef WOLFSSH_OLD_THREADING +#if !defined(WOLFSSH_OLD_THREADING) && !defined(WOLFSSH_OLDER_THREADING) static INLINE void ThreadStart(THREAD_CB fun, void* args, THREAD_TYPE* thread) { @@ -1056,7 +1064,7 @@ static INLINE void ThreadStartNoJoin(THREAD_FUNC fun, void* args) ThreadDetach(thread); } -#endif /* WOLFSSH_OLD_THREADING */ +#endif /* !WOLFSSH_OLD_THREADING && !WOLFSSH_OLDER_THREADING */ #endif /* WOLFSSH_TEST_THREADING */ From ecae1d1519d2c639b4395f15bd3b21678ce90cf5 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Mon, 23 Oct 2023 09:47:52 -0700 Subject: [PATCH 06/47] make channel name size overridable and increase default to 4096 --- wolfssh/internal.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/wolfssh/internal.h b/wolfssh/internal.h index f9b5dcc5a..b7650cbeb 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -352,7 +352,11 @@ enum { #define WOLFSSH_MAX_NAMESZ 32 -#define WOLFSSH_MAX_CHN_NAMESZ 256 + +#ifndef WOLFSSH_MAX_CHN_NAMESZ + #define WOLFSSH_MAX_CHN_NAMESZ 4096 +#endif + #define MAX_ENCRYPTION 3 #define MAX_INTEGRITY 2 #define MAX_KEY_EXCHANGE 2 From 3b50a89b2f25f669477a14272e61b15eda08904e Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 24 Oct 2023 15:23:01 -0700 Subject: [PATCH 07/47] Configure Type Checking Add checks to configure.ac for the types __uint128_t, size_t, uint8_t, and uintptr_t. --- configure.ac | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/configure.ac b/configure.ac index 3d29d3aa8..ffb0378c5 100644 --- a/configure.ac +++ b/configure.ac @@ -50,6 +50,10 @@ AC_PROG_INSTALL AC_CHECK_SIZEOF([long long]) AC_CHECK_SIZEOF([long]) AC_CHECK_SIZEOF([off_t]) +AC_CHECK_TYPES([__uint128_t]) +AC_TYPE_SIZE_T +AC_TYPE_UINT8_T +AC_TYPE_UINTPTR_T # Check headers/libs AC_CHECK_HEADERS([sys/select.h sys/time.h sys/ioctl.h pty.h util.h termios.h]) From fad4d71fa39bfd8729abd08b5e91d658059a69a4 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Tue, 31 Oct 2023 07:14:02 -0600 Subject: [PATCH 08/47] truncate vsnprintf --- src/log.c | 2 +- wolfssh/port.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/log.c b/src/log.c index fdd541be3..2f66d5a9c 100644 --- a/src/log.c +++ b/src/log.c @@ -173,7 +173,7 @@ void wolfSSH_Log(enum wolfSSH_LogLevel level, const char *const fmt, ...) /* format msg */ va_start(vlist, fmt); - WVSNPRINTF(msgStr, sizeof(msgStr), fmt, vlist); + WVSNPRINTF(msgStr, sizeof(msgStr)-1, fmt, vlist); va_end(vlist); if (logFunction) diff --git a/wolfssh/port.h b/wolfssh/port.h index ce2503c5f..241fa5c5f 100644 --- a/wolfssh/port.h +++ b/wolfssh/port.h @@ -499,7 +499,7 @@ extern "C" { #define WSTRNCPY(s1,s2,n) strncpy_s((s1),(n),(s2),(n)) #define WSTRNCASECMP(s1,s2,n) _strnicmp((s1),(s2),(n)) #define WSNPRINTF(s,n,f,...) _snprintf_s((s),(n),_TRUNCATE,(f),##__VA_ARGS__) - #define WVSNPRINTF(s,n,f,...) _vsnprintf_s((s),(n),(n),(f),##__VA_ARGS__) + #define WVSNPRINTF(s,n,f,...) _vsnprintf_s((s),(n),_TRUNCATE,(f),##__VA_ARGS__) #define WSTRTOK(s1,s2,s3) strtok_s((s1),(s2),(s3)) #elif defined(MICROCHIP_MPLAB_HARMONY) || defined(MICROCHIP_PIC32) #include From 66dc60853c959a35febfe00cbc8d79a20f191200 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 31 Oct 2023 10:21:44 -0700 Subject: [PATCH 09/47] Echoserver Select Timeouts A couple changes to keep the echoserver from spin-locking. 1. The SFTP worker should wait for data if the channel window is full. If the client isn't sending a window adjust, it might be because of a human time scale issue. New timeout is 60 seconds. 2. When the echoserver is waiting for something from the user and the select times out, wait a second instead. --- examples/echoserver/echoserver.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index 3d141bf4c..63409c2f6 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -1142,8 +1142,9 @@ static int ssh_worker(thread_ctx_t* threadCtx) #ifdef WOLFSSH_SFTP -#define TEST_SFTP_TIMEOUT_NONE 0 +#define TEST_SFTP_TIMEOUT_SHORT 0 #define TEST_SFTP_TIMEOUT 1 +#define TEST_SFTP_TIMEOUT_LONG 60 /* handle SFTP operations * returns 0 on success @@ -1165,8 +1166,17 @@ static int sftp_worker(thread_ctx_t* threadCtx) /* Yes, process the SFTP data. */ ret = wolfSSH_SFTP_read(ssh); error = wolfSSH_get_error(ssh); - timeout = (ret == WS_REKEYING) ? - TEST_SFTP_TIMEOUT : TEST_SFTP_TIMEOUT_NONE; + + if (ret == WS_REKEYING) { + timeout = TEST_SFTP_TIMEOUT; + } + else if (error == WS_WINDOW_FULL) { + timeout = TEST_SFTP_TIMEOUT_LONG; + } + else { + timeout = TEST_SFTP_TIMEOUT_SHORT; + } + if (error == WS_WANT_READ || error == WS_WANT_WRITE || error == WS_CHAN_RXD || error == WS_REKEYING || error == WS_WINDOW_FULL) @@ -1184,6 +1194,10 @@ static int sftp_worker(thread_ctx_t* threadCtx) if (selected == WS_SELECT_ERROR_READY) { break; } + else if (selected == WS_SELECT_TIMEOUT) { + timeout = TEST_SFTP_TIMEOUT_LONG; + continue; + } if (ret == WS_WANT_READ || ret == WS_WANT_WRITE || selected == WS_SELECT_RECV_READY) { @@ -1217,7 +1231,7 @@ static int sftp_worker(thread_ctx_t* threadCtx) ret = wolfSSH_SFTP_read(ssh); error = wolfSSH_get_error(ssh); timeout = (ret == WS_REKEYING) ? - TEST_SFTP_TIMEOUT : TEST_SFTP_TIMEOUT_NONE; + TEST_SFTP_TIMEOUT : TEST_SFTP_TIMEOUT_SHORT; if (error == WS_WANT_READ || error == WS_WANT_WRITE || error == WS_CHAN_RXD || error == WS_REKEYING || error == WS_WINDOW_FULL) From 13c86631ee1e4734b06cbf2ee5f20ace208bf1d7 Mon Sep 17 00:00:00 2001 From: TakayukiMatsuo Date: Wed, 1 Nov 2023 17:09:40 +0900 Subject: [PATCH 10/47] Fix index of signature part in case of RSA --- src/internal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index 4d4acee9c..f8732546e 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4303,7 +4303,7 @@ static int DoKexDhReply(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) /* Verify h with the server's public key. */ if (ret == WS_SUCCESS) { #ifndef WOLFSSH_NO_RSA - int tmpIdx = begin; + int tmpIdx = begin - sigSz; #endif /* Skip past the sig name. Check it, though. Other SSH * implementations do the verify based on the name, despite what From 70aa287d78c9804abe7fe2e0f208b918ba33d35f Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 13 Oct 2023 14:11:13 -0700 Subject: [PATCH 11/47] wolfSSH Client with OpenSSH-format Keys 1. Move the KeySignature struct around in internal.c so it can be used for a couple tasks. 2. Add decoder for the OpenSSH-format keys. 3. Add an identification function for the new key. 4. Update ReadKey to handle the new format. --- apps/wolfssh/common.c | 2 +- src/internal.c | 428 ++++++++++++++++++++++++++++++++++-------- src/ssh.c | 298 +++++++++++++++++++---------- wolfssh/internal.h | 8 +- wolfssh/ssh.h | 1 + 5 files changed, 556 insertions(+), 181 deletions(-) diff --git a/apps/wolfssh/common.c b/apps/wolfssh/common.c index 3945866c2..7d0ccb44d 100644 --- a/apps/wolfssh/common.c +++ b/apps/wolfssh/common.c @@ -379,7 +379,7 @@ int ClientSetEcho(int type) newTerm.c_lflag &= ~(ICANON | ECHOE | ECHOK | ECHONL | ISIG); } else { - newTerm.c_lflag |= (ICANON | ECHONL); + newTerm.c_lflag |= ICANON; } if (tcsetattr(STDIN_FILENO, TCSANOW, &newTerm) != 0) { diff --git a/src/internal.c b/src/internal.c index f8732546e..0f4407d01 100644 --- a/src/internal.c +++ b/src/internal.c @@ -845,21 +845,100 @@ void SshResourceFree(WOLFSSH* ssh, void* heap) #endif } -union wolfSSH_key { + +typedef struct WS_KeySignature { + byte keySigId; + word32 sigSz; + const char *name; + word32 nameSz; + union { #ifndef WOLFSSH_NO_RSA - RsaKey rsa; + struct { + RsaKey key; + byte e[256]; + word32 eSz; + byte ePad; + byte n[256]; + word32 nSz; + byte nPad; + } rsa; #endif #ifndef WOLFSSH_NO_ECDSA - ecc_key ecc; + struct { + ecc_key key; + word32 keyBlobSz; + const char *keyBlobName; + word32 keyBlobNameSz; + byte q[256]; + word32 qSz; + byte qPad; + const char *primeName; + word32 primeNameSz; + } ecc; #endif -}; + } ks; +} WS_KeySignature; + + +static int wolfSSH_KEY_init(WS_KeySignature* key, byte keyId, void* heap) +{ + int ret = WS_SUCCESS; + + if (key != NULL) { + key->keySigId = keyId; + + switch (keyId) { +#ifndef WOLFSSH_NO_RSA + case ID_SSH_RSA: + ret = wc_InitRsaKey(&key->ks.rsa.key, NULL); + break; +#endif +#ifndef WOLFSSH_NO_ECDSA + case ID_ECDSA_SHA2_NISTP256: + ret = wc_ecc_init_ex(&key->ks.ecc.key, heap, INVALID_DEVID); + break; +#endif + default: + ret = WS_INVALID_ALGO_ID; + } + } + + return ret; +} + + +static int wolfSSH_KEY_clean(WS_KeySignature* key) +{ + int ret = WS_SUCCESS; + + if (key != NULL) { + switch (key->keySigId) { +#ifndef WOLFSSH_NO_RSA + case ID_SSH_RSA: + wc_FreeRsaKey(&key->ks.rsa.key); + break; +#endif +#ifndef WOLFSSH_NO_ECDSA + case ID_ECDSA_SHA2_NISTP256: + case ID_ECDSA_SHA2_NISTP384: + case ID_ECDSA_SHA2_NISTP521: + wc_ecc_free(&key->ks.ecc.key); + break; +#endif + default: + ret = WS_INVALID_ALGO_ID; + } + } + + return ret; +} /* - * Identifies the flavor of a key, RSA or ECDSA, and returns the key type ID. - * The process is to decode the key as if it was RSA and if that fails try - * to load it as if ECDSA. Both public and private keys can be decoded. - * For RSA keys, the key format is described as "ssh-rsa". + * Identifies the flavor of an ASN.1 key, RSA or ECDSA, and returns the key + * type ID. The process is to decode the key as if it was RSA and if that + * fails try to load it as if ECDSA. Both public and private keys can be + * decoded. For RSA keys, the key format is described as "ssh-rsa". * * @param in key to identify * @param inSz size of key @@ -867,88 +946,271 @@ union wolfSSH_key { * @param heap heap to use for memory allocation * @return keyId as int, WS_MEMORY_E, WS_UNIMPLEMENTED_E */ -int IdentifyKey(const byte* in, word32 inSz, int isPrivate, void* heap) +int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap) { - union wolfSSH_key *key = NULL; - int keyId = ID_UNKNOWN; + WS_KeySignature *key = NULL; word32 idx; int ret; int dynType = isPrivate ? DYNTYPE_PRIVKEY : DYNTYPE_PUBKEY; WOLFSSH_UNUSED(dynType); - key = (union wolfSSH_key*)WMALLOC(sizeof(union wolfSSH_key), heap, dynType); + key = (WS_KeySignature*)WMALLOC(sizeof(WS_KeySignature), heap, dynType); + + if (key == NULL) { + ret = WS_MEMORY_E; + } + else { + WMEMSET(key, 0, sizeof(*key)); + key->keySigId = ID_UNKNOWN; #ifndef WOLFSSH_NO_RSA - if (key != NULL) { /* Check RSA key */ - if (keyId == ID_UNKNOWN) { + if (key->keySigId == ID_UNKNOWN) { idx = 0; - ret = wc_InitRsaKey(&key->rsa, NULL); + ret = wc_InitRsaKey(&key->ks.rsa.key, NULL); if (ret == 0) { if (isPrivate) { - ret = wc_RsaPrivateKeyDecode(in, &idx, &key->rsa, inSz); + ret = wc_RsaPrivateKeyDecode(in, &idx, + &key->ks.rsa.key, inSz); } else { - ret = wc_RsaPublicKeyDecode(in, &idx, &key->rsa, inSz); + ret = wc_RsaPublicKeyDecode(in, &idx, + &key->ks.rsa.key, inSz); } /* If decode was successful, this is an RSA key. */ if (ret == 0) { - keyId = ID_SSH_RSA; + key->keySigId = ID_SSH_RSA; } } - wc_FreeRsaKey(&key->rsa); + wc_FreeRsaKey(&key->ks.rsa.key); } - } #endif /* WOLFSSH_NO_RSA */ #ifndef WOLFSSH_NO_ECDSA - if (key != NULL) { /* Check ECDSA key */ - if (keyId == ID_UNKNOWN) { + if (key->keySigId == ID_UNKNOWN) { idx = 0; - ret = wc_ecc_init_ex(&key->ecc, heap, INVALID_DEVID); + ret = wc_ecc_init_ex(&key->ks.ecc.key, heap, INVALID_DEVID); if (ret == 0) { if (isPrivate) { - ret = wc_EccPrivateKeyDecode(in, &idx, &key->ecc, inSz); + ret = wc_EccPrivateKeyDecode(in, &idx, + &key->ks.ecc.key, inSz); } else { - ret = wc_EccPublicKeyDecode(in, &idx, &key->ecc, inSz); + ret = wc_EccPublicKeyDecode(in, &idx, + &key->ks.ecc.key, inSz); } /* If decode was successful, this is an ECDSA key. */ if (ret == 0) { - switch (wc_ecc_get_curve_id(key->ecc.idx)) { + switch (wc_ecc_get_curve_id(key->ks.ecc.key.idx)) { case ECC_SECP256R1: - keyId = ID_ECDSA_SHA2_NISTP256; + key->keySigId = ID_ECDSA_SHA2_NISTP256; break; case ECC_SECP384R1: - keyId = ID_ECDSA_SHA2_NISTP384; + key->keySigId = ID_ECDSA_SHA2_NISTP384; break; case ECC_SECP521R1: - keyId = ID_ECDSA_SHA2_NISTP521; + key->keySigId = ID_ECDSA_SHA2_NISTP521; break; } } } - wc_ecc_free(&key->ecc); + wc_ecc_free(&key->ks.ecc.key); } - } #endif /* WOLFSSH_NO_ECDSA */ + if (key->keySigId == ID_UNKNOWN) { + ret = WS_UNIMPLEMENTED_E; + } + else { + ret = key->keySigId; + } + + WFREE(key, heap, dynType); + } + + return ret; +} + + +static int GetOpenSshKeyRsa(RsaKey* key, + const byte* buf, word32 len, word32* idx) +{ + const byte* val; + word32 valSz; + mp_int m; + + GetMpint(&valSz, &val, buf, len, idx); /* n */ + mp_read_unsigned_bin(&key->n, val, valSz); + GetMpint(&valSz, &val, buf, len, idx); /* e */ + mp_read_unsigned_bin(&key->e, val, valSz); + GetMpint(&valSz, &val, buf, len, idx); /* d */ + mp_read_unsigned_bin(&key->d, val, valSz); + GetMpint(&valSz, &val, buf, len, idx); /* iqmp */ + mp_read_unsigned_bin(&key->u, val, valSz); + GetMpint(&valSz, &val, buf, len, idx); /* p */ + mp_read_unsigned_bin(&key->p, val, valSz); + GetMpint(&valSz, &val, buf, len, idx); /* q */ + mp_read_unsigned_bin(&key->q, val, valSz); + + /* Calculate dP and dQ for wolfCrypt. */ + mp_init(&m); + mp_sub_d(&key->p, 1, &m); + mp_mod(&key->d, &m, &key->dP); + mp_sub_d(&key->q, 1, &m); + mp_mod(&key->d, &m, &key->dQ); + mp_forcezero(&m); + mp_free(&m); + + return 0; +} + + +static int GetOpenSshKeyEcc(ecc_key* key, + const byte* buf, word32 len, word32* idx) +{ + const byte *name, *priv, *pub; + word32 nameSz, privSz, pubSz; + int ret; + + GetStringRef(&nameSz, &name, buf, len, idx); /* curve name */ + GetStringRef(&pubSz, &pub, buf, len, idx); /* Q */ + GetMpint(&privSz, &priv, buf, len, idx); /* d */ + + ret = wc_ecc_import_private_key_ex(priv, privSz, pub, pubSz, + key, ECC_CURVE_DEF); + + return ret != 0; +} + + + +static int GetOpenSshKey(WS_KeySignature *key, + const byte* buf, word32 len, word32* idx) +{ + const char AuthMagic[] = "openssh-key-v1"; + const byte* str; + int ret = WS_SUCCESS; + word32 keyCount, i, strSz; + + if (strcmp(AuthMagic, (const char*)buf) != 0) { + ret = -1; + } + strSz = (word32)strlen(AuthMagic); + *idx += strSz + 1; + + GetSkip(buf, len, idx); /* ciphername */ + GetSkip(buf, len, idx); /* kdfname */ + GetSkip(buf, len, idx); /* kdfoptions */ + + GetUint32(&keyCount, buf, len, idx); /* key count */ + + for (i = 0; i < keyCount; i++) { + GetStringRef(&strSz, &str, buf, len, idx); /* public buf */ + } + + GetStringRef(&strSz, &str, buf, len, idx); /* list of private keys */ + + if (strSz > 0) { + const byte* subStr; + word32 subStrSz, subIdx = 0, check1 = 0, check2 = ~0; + byte keyId; + + idx = 0; + GetUint32(&check1, str, strSz, &subIdx); /* checkint 1 */ + GetUint32(&check2, str, strSz, &subIdx); /* checkint 2 */ + if (check1 == check2) { + for (i = 0; i < keyCount; i++) { + GetStringRef(&subStrSz, &subStr, str, strSz, &subIdx); + keyId = NameToId((const char*)subStr, subStrSz); + wolfSSH_KEY_init(key, keyId, NULL); + switch (keyId) { + #ifndef WOLFSSH_NO_RSA + case ID_SSH_RSA: + GetOpenSshKeyRsa(&key->ks.rsa.key, + str, strSz, &subIdx); + break; + #endif + #ifndef WOLFSSH_NO_ECDSA + case ID_ECDSA_SHA2_NISTP256: + GetOpenSshKeyEcc(&key->ks.ecc.key, + str, strSz, &subIdx); + break; + #endif + default: + ret = WS_UNIMPLEMENTED_E; + break; + } + GetSkip(str, strSz, &subIdx); /* comment */ + } + /* Padding: Add increasing digits to pad to the nearest block + * size. Default block size is 8, but depends on the encryption + * algo. The private key chunk's length, and the length of the + * comment delimit the end of the encrypted blob. No added + * padding required. */ + if (strSz % 8 == 0) { + if (strSz - subIdx > 0) { + /* The padding starts at 1. */ + check2 = strSz - subIdx; + for (check1 = 1; check1 <= check2; check1++, subIdx++) { + if (check1 != str[subIdx]) { + /* Bad pad value. */ + } + } + } + } + } + } + + return ret; +} + + +/* + * Identifies the flavor of an OpenSSH key, RSA or ECDSA, and returns the + * key type ID. The process is to decode the key extracting the identifiers, + * and try to decode the key as the type indicated type. For RSA keys, the + * key format is described as "ssh-rsa". + * + * @param in key to identify + * @param inSz size of key + * @param heap heap to use for memory allocation + * @return keyId as int, WS_MEMORY_E, WS_UNIMPLEMENTED_E, + * WS_INVALID_ALGO_ID + */ +int IdentifyOpenSshKey(const byte* in, word32 inSz, void* heap) +{ + WS_KeySignature *key = NULL; + word32 idx = 0; + int ret; + + key = (WS_KeySignature*)WMALLOC(sizeof(WS_KeySignature), + heap, DYNTYPE_PRIVKEY); + if (key == NULL) { ret = WS_MEMORY_E; } - else if (keyId == ID_UNKNOWN) { - ret = WS_UNIMPLEMENTED_E; - } else { - ret = keyId; + WMEMSET(key, 0, sizeof(*key)); + key->keySigId = ID_UNKNOWN; + + ret = GetOpenSshKey(key, in, inSz, &idx); + + if (ret == WS_SUCCESS) { + ret = key->keySigId; + } + else if (key->keySigId == ID_UNKNOWN) { + ret = WS_UNIMPLEMENTED_E; + } + + wolfSSH_KEY_clean(key); + WFREE(key, heap, DYNTYPE_PRIVKEY); } - WFREE(key, heap, dynType); return ret; } @@ -958,7 +1220,7 @@ int IdentifyKey(const byte* in, word32 inSz, int isPrivate, void* heap) /* * Identifies the flavor of an X.509 certificate, RSA or ECDSA, and returns * the key type ID. The process is to decode the certificate and pass the - * public key to IdentifyKey. + * public key to IdentifyAsn1Key. * * @param in certificate to identify * @param inSz size of certificate @@ -1005,7 +1267,7 @@ static int IdentifyCert(const byte* in, word32 inSz, void* heap) } if (ret == 0) { - ret = IdentifyKey(key, keySz, 0, heap); + ret = IdentifyAsn1Key(key, keySz, 0, heap); } WFREE(key, heap, DYNTYPE_PUBKEY); @@ -1288,9 +1550,12 @@ int wolfSSH_ProcessBuffer(WOLFSSH_CTX* ctx, if (ctx == NULL || in == NULL || inSz == 0) return WS_BAD_ARGUMENT; - if (format != WOLFSSH_FORMAT_ASN1 && format != WOLFSSH_FORMAT_PEM && - format != WOLFSSH_FORMAT_RAW) + if (format != WOLFSSH_FORMAT_ASN1 + && format != WOLFSSH_FORMAT_PEM + && format != WOLFSSH_FORMAT_RAW + && format != WOLFSSH_FORMAT_OPENSSH) { return WS_BAD_FILETYPE_E; + } if (type == BUFTYPE_CA) { dynamicType = DYNTYPE_CA; @@ -1334,6 +1599,9 @@ int wolfSSH_ProcessBuffer(WOLFSSH_CTX* ctx, derSz = (word32)ret; } #endif /* WOLFSSH_CERTS */ + else if (format == WOLFSSH_FORMAT_OPENSSH) { + /* TODO */ + } else { return WS_UNIMPLEMENTED_E; } @@ -1341,7 +1609,7 @@ int wolfSSH_ProcessBuffer(WOLFSSH_CTX* ctx, /* Maybe decrypt */ if (type == BUFTYPE_PRIVKEY) { - ret = IdentifyKey(der, derSz, 1, ctx->heap); + ret = IdentifyAsn1Key(der, derSz, 1, ctx->heap); if (ret < 0) { WFREE(der, heap, dynamicType); return ret; @@ -1713,6 +1981,11 @@ static const NameIdPair NameIdMap[] = { /* Ext Info IDs */ { ID_EXTINFO_SERVER_SIG_ALGS, "server-sig-algs" }, + + /* Curve Name IDs */ + { ID_CURVE_NISTP256, "nistp256" }, + { ID_CURVE_NISTP384, "nistp384" }, + { ID_CURVE_NISTP521, "nistp521" }, }; @@ -2397,6 +2670,26 @@ int GetSize(word32* v, const byte* buf, word32 len, word32* idx) } +int GetSkip(const byte* buf, word32 len, word32* idx) +{ + int result; + word32 sz; + + result = GetUint32(&sz, buf, len, idx); + + if (result == WS_SUCCESS) { + result = WS_BUFFER_E; + + if (*idx < len && sz <= len - *idx) { + *idx += sz; + result = WS_SUCCESS; + } + } + + return result; +} + + /* Gets the size of the mpint, and puts the pointer to the start of * buf's number into *mpint. This function does not copy. */ int GetMpint(word32* mpintSz, const byte** mpint, @@ -10556,40 +10849,6 @@ int SendExtInfo(WOLFSSH* ssh) } -typedef struct WS_KeySignature { - byte keySigId; - word32 sigSz; - const char *name; - word32 nameSz; - union { -#ifndef WOLFSSH_NO_RSA - struct { - RsaKey key; - byte e[256]; - word32 eSz; - byte ePad; - byte n[256]; - word32 nSz; - byte nPad; - } rsa; -#endif -#ifndef WOLFSSH_NO_ECDSA - struct { - ecc_key key; - word32 keyBlobSz; - const char *keyBlobName; - word32 keyBlobNameSz; - byte q[256]; - word32 qSz; - byte qPad; - const char *primeName; - word32 primeNameSz; - } ecc; -#endif - } ks; -} WS_KeySignature; - - /* Updates the payload size, and maybe loads keys. */ static int PrepareUserAuthRequestPassword(WOLFSSH* ssh, word32* payloadSz, const WS_UserAuthData* authData) @@ -10647,15 +10906,24 @@ static int PrepareUserAuthRequestRsa(WOLFSSH* ssh, word32* payloadSz, if (ret == WS_SUCCESS) { word32 idx = 0; #ifdef WOLFSSH_AGENT - if (ssh->agentEnabled) + if (ssh->agentEnabled) { ret = wc_RsaPublicKeyDecode(authData->sf.publicKey.publicKey, &idx, &keySig->ks.rsa.key, authData->sf.publicKey.publicKeySz); + } else #endif + { ret = wc_RsaPrivateKeyDecode(authData->sf.publicKey.privateKey, &idx, &keySig->ks.rsa.key, authData->sf.publicKey.privateKeySz); + if (ret != 0) { + idx = 0; + ret = GetOpenSshKey(keySig, + authData->sf.publicKey.privateKey, + authData->sf.publicKey.privateKeySz, &idx); + } + } } if (ret == WS_SUCCESS) { @@ -11040,6 +11308,12 @@ static int PrepareUserAuthRequestEcc(WOLFSSH* ssh, word32* payloadSz, ret = wc_EccPrivateKeyDecode(authData->sf.publicKey.privateKey, &idx, &keySig->ks.ecc.key, authData->sf.publicKey.privateKeySz); + if (ret != 0) { + idx = 0; + ret = GetOpenSshKey(keySig, + authData->sf.publicKey.privateKey, + authData->sf.publicKey.privateKeySz, &idx); + } } if (ret == WS_SUCCESS) { @@ -13491,7 +13765,6 @@ int wolfSSH_CleanPath(WOLFSSH* ssh, char* in) #endif /* WOLFSSH_SFTP || WOLFSSH_SCP */ -#ifdef DEBUG_WOLFSSH #define LINE_WIDTH 16 void DumpOctetString(const byte* input, word32 inputSz) @@ -13527,7 +13800,6 @@ void DumpOctetString(const byte* input, word32 inputSz) } } -#endif #ifdef WOLFSSH_SFTP diff --git a/src/ssh.c b/src/ssh.c index 93a5df572..738d54d1f 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1323,7 +1323,6 @@ void* wolfSSH_GetPublicKeyCheckCtx(WOLFSSH* ssh) return NULL; } -#ifdef WOLFSSH_TERM #if defined(WOLFSSH_TERM) && !defined(NO_FILESYSTEM) /* Used to resize terminal window with shell connections @@ -1361,8 +1360,6 @@ void wolfSSH_SetTerminalResizeCtx(WOLFSSH* ssh, void* usrCtx) } #endif -#endif - /* Used to set the channel request type sent in wolfSSH connect. The default * type set is shell if this function is not called. @@ -1489,137 +1486,232 @@ union wolfSSH_key { #endif }; -/* Reads a key from the buffer in to out. If the out buffer doesn't exist - it is created. The type of key is stored in outType. It'll be a pointer - to a constant string. Format indicates the format of the key, currently - either SSH format (a public key) or ASN.1 in DER or PEM format (a - private key). */ -int wolfSSH_ReadKey_buffer(const byte* in, word32 inSz, int format, - byte** out, word32* outSz, const byte** outType, word32* outTypeSz, +static const char* PrivBeginOpenSSH = "-----BEGIN OPENSSH PRIVATE KEY-----"; +static const char* PrivEndOpenSSH = "-----END OPENSSH PRIVATE KEY-----"; +static const char* PrivBeginPrefix = "-----BEGIN "; +/* static const char* PrivEndPrefix = "-----END "; */ +static const char* PrivSuffix = " PRIVATE KEY-----"; + + +static int DoSshPubKey(const byte* in, word32 inSz, byte** out, + word32* outSz, const byte** outType, word32* outTypeSz, void* heap) { int ret = WS_SUCCESS; byte* newKey = NULL; + char* c; + char* last; + char* type = NULL; + char* key = NULL; + WOLFSSH_UNUSED(inSz); WOLFSSH_UNUSED(heap); - if (in == NULL || inSz == 0 || out == NULL || outSz == NULL || - outType == NULL || outTypeSz == NULL) - return WS_BAD_ARGUMENT; + /* + SSH format is: + type AAAABASE64ENCODEDKEYDATA comment + */ + c = WSTRDUP((const char*)in, heap, DYNTYPE_STRING); + type = WSTRTOK(c, " \n", &last); + key = WSTRTOK(NULL, " \n", &last); - if (format == WOLFSSH_FORMAT_SSH) { - char* c; - char* last; - char* type = NULL; - char* key = NULL; - - /* - SSH format is: - type AAAABASE64ENCODEDKEYDATA comment - */ - c = WSTRDUP((const char*)in, heap, DYNTYPE_STRING); - type = WSTRTOK(c, " \n", &last); - key = WSTRTOK(NULL, " \n", &last); - - if (type != NULL && key != NULL) { - const char* name; - word32 typeSz; - byte nameId; - - typeSz = (word32)WSTRLEN(type); - - nameId = NameToId(type, typeSz); - name = IdToName(nameId); - *outType = (const byte*)name; - *outTypeSz = typeSz; - - if (*out == NULL) { - /* set size based on sanity check in wolfSSL base64 decode - * function */ - *outSz = ((word32)WSTRLEN(key) * 3 + 3) / 4; - newKey = (byte*)WMALLOC(*outSz, heap, DYNTYPE_PRIVKEY); - if (newKey == NULL) { - return WS_MEMORY_E; - } - *out = newKey; - } + if (type != NULL && key != NULL) { + const char* name; + word32 typeSz; + byte nameId; - ret = Base64_Decode((byte*)key, (word32)WSTRLEN(key), *out, outSz); - } + typeSz = (word32)WSTRLEN(type); - if (ret != 0) { - WLOG(WS_LOG_DEBUG, "Base64 decode of public key failed."); - ret = WS_PARSE_E; - } + nameId = NameToId(type, typeSz); + name = IdToName(nameId); + *outType = (const byte*)name; + *outTypeSz = typeSz; - WFREE(c, heap, DYNTYPE_STRING); - } - else if (format == WOLFSSH_FORMAT_ASN1) { if (*out == NULL) { - newKey = (byte*)WMALLOC(inSz, heap, DYNTYPE_PRIVKEY); + /* set size based on sanity check in wolfSSL base64 decode + * function */ + *outSz = ((word32)WSTRLEN(key) * 3 + 3) / 4; + newKey = (byte*)WMALLOC(*outSz, heap, DYNTYPE_PRIVKEY); if (newKey == NULL) { return WS_MEMORY_E; } *out = newKey; } - else { - if (*outSz < inSz) { - WLOG(WS_LOG_DEBUG, "DER private key output size too small"); - return WS_BUFFER_E; - } - newKey = *out; + + ret = Base64_Decode((byte*)key, (word32)WSTRLEN(key), *out, outSz); + } + + if (ret != 0) { + WLOG(WS_LOG_DEBUG, "Base64 decode of public key failed."); + ret = WS_PARSE_E; + } + + WFREE(c, heap, DYNTYPE_STRING); + return ret; +} + + +static int DoAsn1Key(const byte* in, word32 inSz, byte** out, + word32* outSz, const byte** outType, word32* outTypeSz, + void* heap) +{ + int ret = WS_SUCCESS; + byte* newKey = NULL; + + WOLFSSH_UNUSED(heap); + + if (*out == NULL) { + newKey = (byte*)WMALLOC(inSz, heap, DYNTYPE_PRIVKEY); + if (newKey == NULL) { + return WS_MEMORY_E; + } + *out = newKey; + } + else { + if (*outSz < inSz) { + WLOG(WS_LOG_DEBUG, "DER private key output size too small"); + return WS_BUFFER_E; + } + newKey = *out; + } + *outSz = inSz; + WMEMCPY(newKey, in, inSz); + + ret = IdentifyAsn1Key(in, inSz, 1, heap); + if (ret > 0) { + *outType = (const byte*)IdToName(ret); + *outTypeSz = (word32)WSTRLEN((const char*)*outType); + ret = WS_SUCCESS; + } + + return ret; +} + + +static int DoPemKey(const byte* in, word32 inSz, byte** out, + word32* outSz, const byte** outType, word32* outTypeSz, + void* heap) +{ + int ret = WS_SUCCESS; + byte* newKey = NULL; + + WOLFSSH_UNUSED(heap); + + word32 newKeySz = inSz; /* binary will be smaller than PEM */ + + if (*out == NULL) { + newKey = (byte*)WMALLOC(newKeySz, heap, DYNTYPE_PRIVKEY); + if (newKey == NULL) { + return WS_MEMORY_E; + } + *out = newKey; + } + else { + if (*outSz < inSz) { + WLOG(WS_LOG_DEBUG, "PEM private key output size too small"); + return WS_BUFFER_E; } - *outSz = inSz; - WMEMCPY(newKey, in, inSz); + newKey = *out; + } + + /* If it is PEM, convert to ASN1 then process. */ + ret = wc_KeyPemToDer(in, inSz, newKey, newKeySz, NULL); + if (ret > 0) { + newKeySz = (word32)ret; + ret = WS_SUCCESS; + } + else { + WLOG(WS_LOG_DEBUG, "Base64 decode of public key failed."); + ret = WS_PARSE_E; + } - ret = IdentifyKey(in, inSz, 1, heap); + if (ret == WS_SUCCESS) { + ret = IdentifyAsn1Key(newKey, newKeySz, 1, heap); if (ret > 0) { + *outSz = newKeySz; *outType = (const byte*)IdToName(ret); *outTypeSz = (word32)WSTRLEN((const char*)*outType); ret = WS_SUCCESS; } + else { + WLOG(WS_LOG_DEBUG, "unable to identify key"); + } } - else if (format == WOLFSSH_FORMAT_PEM) { - word32 newKeySz = inSz; /* binary will be smaller than PEM */ - if (*out == NULL) { - newKey = (byte*)WMALLOC(newKeySz, heap, DYNTYPE_PRIVKEY); - if (newKey == NULL) { - return WS_MEMORY_E; - } - *out = newKey; + return ret; +} + + +static int DoOpenSshKey(const byte* in, word32 inSz, byte** out, + word32* outSz, const byte** outType, word32* outTypeSz, + void* heap) +{ + int ret = WS_SUCCESS; + byte* newKey = NULL; + word32 newKeySz = inSz; /* binary will be smaller than PEM */ + + if (*out == NULL) { + newKey = (byte*)WMALLOC(newKeySz, heap, DYNTYPE_PRIVKEY); + if (newKey == NULL) { + return WS_MEMORY_E; } - else { - if (*outSz < inSz) { - WLOG(WS_LOG_DEBUG, "PEM private key output size too small"); - return WS_BUFFER_E; - } - newKey = *out; + *out = newKey; + } + else { + if (*outSz < inSz) { + WLOG(WS_LOG_DEBUG, "PEM private key output size too small"); + return WS_BUFFER_E; } + newKey = *out; + } + + in += strlen(PrivBeginOpenSSH); + inSz -= (strlen(PrivBeginOpenSSH) + strlen(PrivEndOpenSSH) + 2); - /* If it is PEM, convert to ASN1 then process. */ - ret = wc_KeyPemToDer(in, inSz, newKey, newKeySz, NULL); + ret = Base64_Decode((byte*)in, inSz, newKey, &newKeySz); + if (ret == 0) { + ret = IdentifyOpenSshKey(newKey, newKeySz, heap); if (ret > 0) { - newKeySz = (word32)ret; + *outSz = newKeySz; + *outType = (const byte*)IdToName(ret); + *outTypeSz = (word32)WSTRLEN((const char*)*outType); ret = WS_SUCCESS; } else { - WLOG(WS_LOG_DEBUG, "Base64 decode of public key failed."); - ret = WS_PARSE_E; + WLOG(WS_LOG_DEBUG, "unable to identify key"); } + } - if (ret == WS_SUCCESS) { - ret = IdentifyKey(newKey, newKeySz, 1, heap); - if (ret > 0) { - *outSz = newKeySz; - *outType = (const byte*)IdToName(ret); - *outTypeSz = (word32)WSTRLEN((const char*)*outType); - ret = WS_SUCCESS; - } - else { - WLOG(WS_LOG_DEBUG, "unable to identify key"); - } - } + return ret; +} + + +/* Reads a key from the buffer in to out. If the out buffer doesn't exist + it is created. The type of key is stored in outType. It'll be a pointer + to a constant string. Format indicates the format of the key, currently + either SSH format (a public key) or ASN.1 in DER or PEM format (a + private key). */ +int wolfSSH_ReadKey_buffer(const byte* in, word32 inSz, int format, + byte** out, word32* outSz, const byte** outType, word32* outTypeSz, + void* heap) +{ + int ret = WS_SUCCESS; + + if (in == NULL || inSz == 0 || out == NULL || outSz == NULL || + outType == NULL || outTypeSz == NULL) + return WS_BAD_ARGUMENT; + + if (format == WOLFSSH_FORMAT_SSH) { + ret = DoSshPubKey(in, inSz, out, outSz, outType, outTypeSz, heap); + } + else if (format == WOLFSSH_FORMAT_ASN1) { + ret = DoAsn1Key(in, inSz, out, outSz, outType, outTypeSz, heap); + } + else if (format == WOLFSSH_FORMAT_PEM) { + ret = DoPemKey(in, inSz, out, outSz, outType, outTypeSz, heap); + } + else if (format == WOLFSSH_FORMAT_OPENSSH) { + ret = DoOpenSshKey(in, inSz, out, outSz, outType, outTypeSz, heap); } else { WLOG(WS_LOG_DEBUG, "Invalid key format"); @@ -1687,9 +1779,13 @@ int wolfSSH_ReadKey_file(const char* name, format = WOLFSSH_FORMAT_SSH; in[inSz] = 0; } - else if ((WSTRNSTR((const char*)in, "-----BEGIN ", inSz) + else if (WSTRNSTR((const char*)in, PrivBeginOpenSSH, inSz) != NULL) { + *isPrivate = 1; + format = WOLFSSH_FORMAT_OPENSSH; + } + else if ((WSTRNSTR((const char*)in, PrivBeginPrefix, inSz) == (const char*)in) - && (WSTRNSTR((const char*)in, "PRIVATE KEY-----", inSz) + && (WSTRNSTR((const char*)in, PrivSuffix, inSz) != NULL)) { *isPrivate = 1; format = WOLFSSH_FORMAT_PEM; diff --git a/wolfssh/internal.h b/wolfssh/internal.h index b7650cbeb..2c411c076 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -347,6 +347,10 @@ enum { ID_EXTINFO_SERVER_SIG_ALGS, + ID_CURVE_NISTP256, + ID_CURVE_NISTP384, + ID_CURVE_NISTP521, + ID_UNKNOWN }; @@ -855,8 +859,9 @@ WOLFSSH_LOCAL void ChannelDelete(WOLFSSH_CHANNEL*, void*); WOLFSSH_LOCAL WOLFSSH_CHANNEL* ChannelFind(WOLFSSH*, word32, byte); WOLFSSH_LOCAL int ChannelRemove(WOLFSSH*, word32, byte); WOLFSSH_LOCAL int ChannelPutData(WOLFSSH_CHANNEL*, byte*, word32); -WOLFSSH_LOCAL int IdentifyKey(const byte* in, word32 inSz, +WOLFSSH_LOCAL int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap); +WOLFSSH_LOCAL int IdentifyOpenSshKey(const byte* in, word32 inSz, void* heap); WOLFSSH_LOCAL int wolfSSH_ProcessBuffer(WOLFSSH_CTX*, const byte*, word32, int, int); @@ -869,6 +874,7 @@ WOLFSSH_LOCAL int GetUint32(word32* v, const byte* buf, word32 len, word32* idx); WOLFSSH_LOCAL int GetSize(word32* v, const byte* buf, word32 len, word32* idx); +WOLFSSH_LOCAL int GetSkip(const byte* buf, word32 len, word32* idx); WOLFSSH_LOCAL int GetMpint(word32* mpintSz, const byte** mpint, const byte* buf, word32 len, word32* idx); WOLFSSH_LOCAL int GetString(char* s, word32* sSz, diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index 492ab5dd4..ab315daad 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -311,6 +311,7 @@ enum WS_FormatTypes { WOLFSSH_FORMAT_PEM, WOLFSSH_FORMAT_RAW, WOLFSSH_FORMAT_SSH, + WOLFSSH_FORMAT_OPENSSH }; From 732aba4bc6a0d7d694f01758b2086749d688a715 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 16 Oct 2023 16:29:17 -0700 Subject: [PATCH 12/47] wolfSSH Client with OpenSSH-format Keys 1. Add two error codes for the new key format decoding. 2. Add in some better error and bound checking. 3. Fix ordering on a WOLFSSH_UNUSED and variable declaration. 4. Remove redundant ; from WOLFSSH_UNUSED function-like macro. --- src/internal.c | 155 +++++++++++++++++++++++++++++------------------- src/ssh.c | 5 +- wolfssh/error.h | 4 +- wolfssh/port.h | 2 +- 4 files changed, 100 insertions(+), 66 deletions(-) diff --git a/src/internal.c b/src/internal.c index 0f4407d01..00f4eb498 100644 --- a/src/internal.c +++ b/src/internal.c @@ -419,6 +419,12 @@ const char* GetErrorString(int err) case WS_MATCH_UA_KEY_ID_E: return "unable to match user auth key type"; + case WS_KEY_AUTH_MAGIC_E: + return "key auth magic check error"; + + case WS_KEY_CHECK_VAL_E: + return "key check value error"; + default: return "Unknown error code"; } @@ -1041,8 +1047,8 @@ int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap) static int GetOpenSshKeyRsa(RsaKey* key, const byte* buf, word32 len, word32* idx) { - const byte* val; - word32 valSz; + const byte* val = NULL; + word32 valSz = 0; mp_int m; GetMpint(&valSz, &val, buf, len, idx); /* n */ @@ -1074,8 +1080,8 @@ static int GetOpenSshKeyRsa(RsaKey* key, static int GetOpenSshKeyEcc(ecc_key* key, const byte* buf, word32 len, word32* idx) { - const byte *name, *priv, *pub; - word32 nameSz, privSz, pubSz; + const byte *name = NULL, *priv = NULL, *pub = NULL; + word32 nameSz = 0, privSz = 0, pubSz = 0; int ret; GetStringRef(&nameSz, &name, buf, len, idx); /* curve name */ @@ -1094,72 +1100,100 @@ static int GetOpenSshKey(WS_KeySignature *key, const byte* buf, word32 len, word32* idx) { const char AuthMagic[] = "openssh-key-v1"; - const byte* str; + const byte* str = NULL; + word32 keyCount = 0, strSz = 0, i; int ret = WS_SUCCESS; - word32 keyCount, i, strSz; if (strcmp(AuthMagic, (const char*)buf) != 0) { - ret = -1; + ret = WS_KEY_AUTH_MAGIC_E; } - strSz = (word32)strlen(AuthMagic); - *idx += strSz + 1; - GetSkip(buf, len, idx); /* ciphername */ - GetSkip(buf, len, idx); /* kdfname */ - GetSkip(buf, len, idx); /* kdfoptions */ + if (ret == WS_SUCCESS) { + *idx += (word32)strlen(AuthMagic) + 1; + ret = GetSkip(buf, len, idx); /* ciphername */ + } - GetUint32(&keyCount, buf, len, idx); /* key count */ + if (ret == WS_SUCCESS) + ret = GetSkip(buf, len, idx); /* kdfname */ + + if (ret == WS_SUCCESS) + ret = GetSkip(buf, len, idx); /* kdfoptions */ + + if (ret == WS_SUCCESS) + ret = GetUint32(&keyCount, buf, len, idx); /* key count */ for (i = 0; i < keyCount; i++) { - GetStringRef(&strSz, &str, buf, len, idx); /* public buf */ + if (ret == WS_SUCCESS) + ret = GetStringRef(&strSz, &str, buf, len, idx); /* public buf */ } - GetStringRef(&strSz, &str, buf, len, idx); /* list of private keys */ + if (keyCount > 0) { + if (ret == WS_SUCCESS) { + ret = GetStringRef(&strSz, &str, buf, len, idx); + /* list of private keys */ + } - if (strSz > 0) { - const byte* subStr; - word32 subStrSz, subIdx = 0, check1 = 0, check2 = ~0; - byte keyId; - - idx = 0; - GetUint32(&check1, str, strSz, &subIdx); /* checkint 1 */ - GetUint32(&check2, str, strSz, &subIdx); /* checkint 2 */ - if (check1 == check2) { - for (i = 0; i < keyCount; i++) { - GetStringRef(&subStrSz, &subStr, str, strSz, &subIdx); - keyId = NameToId((const char*)subStr, subStrSz); - wolfSSH_KEY_init(key, keyId, NULL); - switch (keyId) { - #ifndef WOLFSSH_NO_RSA - case ID_SSH_RSA: - GetOpenSshKeyRsa(&key->ks.rsa.key, - str, strSz, &subIdx); - break; - #endif - #ifndef WOLFSSH_NO_ECDSA - case ID_ECDSA_SHA2_NISTP256: - GetOpenSshKeyEcc(&key->ks.ecc.key, - str, strSz, &subIdx); - break; - #endif - default: - ret = WS_UNIMPLEMENTED_E; - break; + if (strSz > 0) { + const byte* subStr = NULL; + word32 subStrSz = 0, subIdx = 0, check1 = 0, check2 = ~0; + byte keyId; + + idx = 0; + if (ret == WS_SUCCESS) + ret = GetUint32(&check1, str, strSz, &subIdx); /* checkint 1 */ + if (ret == WS_SUCCESS) + ret = GetUint32(&check2, str, strSz, &subIdx); /* checkint 2 */ + if (ret == WS_SUCCESS) { + if (check1 != check2) { + ret = WS_KEY_CHECK_VAL_E; } - GetSkip(str, strSz, &subIdx); /* comment */ - } - /* Padding: Add increasing digits to pad to the nearest block - * size. Default block size is 8, but depends on the encryption - * algo. The private key chunk's length, and the length of the - * comment delimit the end of the encrypted blob. No added - * padding required. */ - if (strSz % 8 == 0) { - if (strSz - subIdx > 0) { - /* The padding starts at 1. */ - check2 = strSz - subIdx; - for (check1 = 1; check1 <= check2; check1++, subIdx++) { - if (check1 != str[subIdx]) { - /* Bad pad value. */ + } + if (ret == WS_SUCCESS) { + for (i = 0; i < keyCount; i++) { + ret = GetStringRef(&subStrSz, &subStr, + str, strSz, &subIdx); + if (ret == WS_SUCCESS) { + keyId = NameToId((const char*)subStr, subStrSz); + wolfSSH_KEY_init(key, keyId, NULL); + switch (keyId) { + #ifndef WOLFSSH_NO_RSA + case ID_SSH_RSA: + GetOpenSshKeyRsa(&key->ks.rsa.key, + str, strSz, &subIdx); + break; + #endif + #ifndef WOLFSSH_NO_ECDSA + case ID_ECDSA_SHA2_NISTP256: + GetOpenSshKeyEcc(&key->ks.ecc.key, + str, strSz, &subIdx); + break; + #endif + default: + ret = WS_UNIMPLEMENTED_E; + break; + } + if (ret == WS_SUCCESS) + ret = GetSkip(str, strSz, &subIdx); + /* key comment */ + } + } + /* Padding: Add increasing digits to pad to the nearest + * block size. Default block size is 8, but depends on + * the encryption algo. The private key chunk's length, + * and the length of the comment delimit the end of the + * encrypted blob. No added padding required. */ + if (ret == WS_SUCCESS) { + if (strSz % 8 == 0) { + if (strSz - subIdx > 0) { + /* The padding starts at 1. */ + check2 = strSz - subIdx; + for (check1 = 1; + check1 <= check2; + check1++, subIdx++) { + if (check1 != str[subIdx]) { + /* Bad pad value. */ + } + } } } } @@ -1599,9 +1633,6 @@ int wolfSSH_ProcessBuffer(WOLFSSH_CTX* ctx, derSz = (word32)ret; } #endif /* WOLFSSH_CERTS */ - else if (format == WOLFSSH_FORMAT_OPENSSH) { - /* TODO */ - } else { return WS_UNIMPLEMENTED_E; } @@ -11305,6 +11336,7 @@ static int PrepareUserAuthRequestEcc(WOLFSSH* ssh, word32* payloadSz, } else #endif + { ret = wc_EccPrivateKeyDecode(authData->sf.publicKey.privateKey, &idx, &keySig->ks.ecc.key, authData->sf.publicKey.privateKeySz); @@ -11314,6 +11346,7 @@ static int PrepareUserAuthRequestEcc(WOLFSSH* ssh, word32* payloadSz, authData->sf.publicKey.privateKey, authData->sf.publicKey.privateKeySz, &idx); } + } } if (ret == WS_SUCCESS) { diff --git a/src/ssh.c b/src/ssh.c index 738d54d1f..80535ddc5 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1594,11 +1594,10 @@ static int DoPemKey(const byte* in, word32 inSz, byte** out, { int ret = WS_SUCCESS; byte* newKey = NULL; + word32 newKeySz = inSz; /* binary will be smaller than PEM */ WOLFSSH_UNUSED(heap); - word32 newKeySz = inSz; /* binary will be smaller than PEM */ - if (*out == NULL) { newKey = (byte*)WMALLOC(newKeySz, heap, DYNTYPE_PRIVKEY); if (newKey == NULL) { @@ -1666,7 +1665,7 @@ static int DoOpenSshKey(const byte* in, word32 inSz, byte** out, } in += strlen(PrivBeginOpenSSH); - inSz -= (strlen(PrivBeginOpenSSH) + strlen(PrivEndOpenSSH) + 2); + inSz -= (word32)(strlen(PrivBeginOpenSSH) + strlen(PrivEndOpenSSH) + 2); ret = Base64_Decode((byte*)in, inSz, newKey, &newKeySz); if (ret == 0) { diff --git a/wolfssh/error.h b/wolfssh/error.h index d964b7ac3..10ed54d3e 100644 --- a/wolfssh/error.h +++ b/wolfssh/error.h @@ -128,8 +128,10 @@ enum WS_ErrorCodes { WS_CERT_KEY_SIZE_E = -1087, /* Key size error */ WS_CTX_KEY_COUNT_E = -1088, /* Adding too many private keys */ WS_MATCH_UA_KEY_ID_E = -1089, /* Match user auth key key fail */ + WS_KEY_AUTH_MAGIC_E = -1090, /* OpenSSH key auth magic check fail */ + WS_KEY_CHECK_VAL_E = -1091, /* OpenSSH key check value fail */ - WS_LAST_E = -1089 /* Update this to indicate last error */ + WS_LAST_E = -1091 /* Update this to indicate last error */ }; diff --git a/wolfssh/port.h b/wolfssh/port.h index 241fa5c5f..a0bfbc282 100644 --- a/wolfssh/port.h +++ b/wolfssh/port.h @@ -1390,7 +1390,7 @@ extern "C" { #ifndef WOLFSSH_UNUSED - #define WOLFSSH_UNUSED(arg) (void)(arg); + #define WOLFSSH_UNUSED(arg) (void)(arg) #endif From 9bbfcf1a96c15c576416a61d5cc5388ca96c1ebc Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 1 Nov 2023 17:07:43 -0700 Subject: [PATCH 13/47] PR review: Update some std-C functions with the proper porting wrappers. --- src/internal.c | 11 ++++++----- src/ssh.c | 4 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/internal.c b/src/internal.c index 00f4eb498..863847f14 100644 --- a/src/internal.c +++ b/src/internal.c @@ -891,6 +891,7 @@ static int wolfSSH_KEY_init(WS_KeySignature* key, byte keyId, void* heap) int ret = WS_SUCCESS; if (key != NULL) { + WMEMSET(key, 0, sizeof(*key)); key->keySigId = keyId; switch (keyId) { @@ -1104,12 +1105,12 @@ static int GetOpenSshKey(WS_KeySignature *key, word32 keyCount = 0, strSz = 0, i; int ret = WS_SUCCESS; - if (strcmp(AuthMagic, (const char*)buf) != 0) { + if (WSTRCMP(AuthMagic, (const char*)buf) != 0) { ret = WS_KEY_AUTH_MAGIC_E; } if (ret == WS_SUCCESS) { - *idx += (word32)strlen(AuthMagic) + 1; + *idx += (word32)WSTRLEN(AuthMagic) + 1; ret = GetSkip(buf, len, idx); /* ciphername */ } @@ -9138,7 +9139,7 @@ static int SendKexGetSigningKey(WOLFSSH* ssh, sigKeyBlock_ptr->sk.ecc.primeName = PrimeNameForId(ssh->handshake->pubKeyId); sigKeyBlock_ptr->sk.ecc.primeNameSz = - (word32)strlen(sigKeyBlock_ptr->sk.ecc.primeName); + (word32)WSTRLEN(sigKeyBlock_ptr->sk.ecc.primeName); /* Decode the user-configured ECDSA private key. */ sigKeyBlock_ptr->sk.ecc.qSz = @@ -9428,7 +9429,7 @@ int SendKexDhReply(WOLFSSH* ssh) sigKeyBlock_ptr->pubKeyName = IdToName(SigTypeForId(sigKeyBlock_ptr->pubKeyId)); sigKeyBlock_ptr->pubKeyNameSz = - (word32)strlen(sigKeyBlock_ptr->pubKeyName); + (word32)WSTRLEN(sigKeyBlock_ptr->pubKeyName); sigKeyBlock_ptr->pubKeyFmtId = sigKeyBlock_ptr->pubKeyId; if (sigKeyBlock_ptr->pubKeyId == ID_RSA_SHA2_256 || sigKeyBlock_ptr->pubKeyId == ID_RSA_SHA2_512) { @@ -9437,7 +9438,7 @@ int SendKexDhReply(WOLFSSH* ssh) sigKeyBlock_ptr->pubKeyFmtName = IdToName(sigKeyBlock_ptr->pubKeyFmtId); sigKeyBlock_ptr->pubKeyFmtNameSz = - (word32)strlen(sigKeyBlock_ptr->pubKeyFmtName); + (word32)WSTRLEN(sigKeyBlock_ptr->pubKeyFmtName); switch (ssh->handshake->kexId) { #ifndef WOLFSSH_NO_ECDH_SHA2_NISTP256 diff --git a/src/ssh.c b/src/ssh.c index 80535ddc5..b5b6171a8 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1664,8 +1664,8 @@ static int DoOpenSshKey(const byte* in, word32 inSz, byte** out, newKey = *out; } - in += strlen(PrivBeginOpenSSH); - inSz -= (word32)(strlen(PrivBeginOpenSSH) + strlen(PrivEndOpenSSH) + 2); + in += WSTRLEN(PrivBeginOpenSSH); + inSz -= (word32)(WSTRLEN(PrivBeginOpenSSH) + WSTRLEN(PrivEndOpenSSH) + 2); ret = Base64_Decode((byte*)in, inSz, newKey, &newKeySz); if (ret == 0) { From 8049606f2e6e774c2aac3ba10e75a4cfd475b873 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 2 Nov 2023 15:57:11 -0700 Subject: [PATCH 14/47] PR review: Fix potential memory leak when failing to parse a key of any type. --- src/ssh.c | 136 ++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 91 insertions(+), 45 deletions(-) diff --git a/src/ssh.c b/src/ssh.c index b5b6171a8..1ad8c5c6f 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1497,12 +1497,13 @@ static int DoSshPubKey(const byte* in, word32 inSz, byte** out, word32* outSz, const byte** outType, word32* outTypeSz, void* heap) { - int ret = WS_SUCCESS; byte* newKey = NULL; char* c; char* last; char* type = NULL; char* key = NULL; + int ret = WS_SUCCESS; + word32 newKeySz, typeSz; WOLFSSH_UNUSED(inSz); WOLFSSH_UNUSED(heap); @@ -1512,38 +1513,60 @@ static int DoSshPubKey(const byte* in, word32 inSz, byte** out, type AAAABASE64ENCODEDKEYDATA comment */ c = WSTRDUP((const char*)in, heap, DYNTYPE_STRING); - type = WSTRTOK(c, " \n", &last); - key = WSTRTOK(NULL, " \n", &last); + if (c != NULL) { + type = WSTRTOK(c, " \n", &last); + key = WSTRTOK(NULL, " \n", &last); + } + else { + ret = WS_MEMORY_E; + } - if (type != NULL && key != NULL) { - const char* name; - word32 typeSz; - byte nameId; + if (ret == WS_SUCCESS) { + if (type == NULL || key == NULL) { + ret = WS_PARSE_E; + } + } + if (ret == WS_SUCCESS) { typeSz = (word32)WSTRLEN(type); - - nameId = NameToId(type, typeSz); - name = IdToName(nameId); - *outType = (const byte*)name; - *outTypeSz = typeSz; - + /* set size based on sanity check in wolfSSL base64 decode + * function */ + newKeySz = ((word32)WSTRLEN(key) * 3 + 3) / 4; if (*out == NULL) { - /* set size based on sanity check in wolfSSL base64 decode - * function */ - *outSz = ((word32)WSTRLEN(key) * 3 + 3) / 4; newKey = (byte*)WMALLOC(*outSz, heap, DYNTYPE_PRIVKEY); if (newKey == NULL) { - return WS_MEMORY_E; + ret = WS_MEMORY_E; + } + } + else { + if (*outSz < newKeySz) { + WLOG(WS_LOG_DEBUG, "PEM private key output size too small"); + ret = WS_BUFFER_E; + } + else { + newKey = *out; } - *out = newKey; } - - ret = Base64_Decode((byte*)key, (word32)WSTRLEN(key), *out, outSz); } - if (ret != 0) { - WLOG(WS_LOG_DEBUG, "Base64 decode of public key failed."); - ret = WS_PARSE_E; + if (ret == WS_SUCCESS) { + ret = Base64_Decode((byte*)key, (word32)WSTRLEN(key), + newKey, &newKeySz); + + if (ret == 0) { + *out = newKey; + *outSz = newKeySz; + *outType = (const byte *)IdToName(NameToId(type, typeSz)); + *outTypeSz = (word32)WSTRLEN((const char*)*outType); + ret = WS_SUCCESS; + } + else { + WLOG(WS_LOG_DEBUG, "Base64 decode of public key failed."); + if (*out == NULL) { + WFREE(newKey, heap, DYNTYPE_PRIVKEY); + } + ret = WS_PARSE_E; + } } WFREE(c, heap, DYNTYPE_STRING); @@ -1565,7 +1588,6 @@ static int DoAsn1Key(const byte* in, word32 inSz, byte** out, if (newKey == NULL) { return WS_MEMORY_E; } - *out = newKey; } else { if (*outSz < inSz) { @@ -1574,15 +1596,23 @@ static int DoAsn1Key(const byte* in, word32 inSz, byte** out, } newKey = *out; } - *outSz = inSz; - WMEMCPY(newKey, in, inSz); ret = IdentifyAsn1Key(in, inSz, 1, heap); + if (ret > 0) { + *out = newKey; + *outSz = inSz; + WMEMCPY(newKey, in, inSz); *outType = (const byte*)IdToName(ret); *outTypeSz = (word32)WSTRLEN((const char*)*outType); ret = WS_SUCCESS; } + else { + WLOG(WS_LOG_DEBUG, "unable to identify key"); + if (*out == NULL) { + WFREE(newKey, heap, DYNTYPE_PRIVKEY); + } + } return ret; } @@ -1599,11 +1629,10 @@ static int DoPemKey(const byte* in, word32 inSz, byte** out, WOLFSSH_UNUSED(heap); if (*out == NULL) { - newKey = (byte*)WMALLOC(newKeySz, heap, DYNTYPE_PRIVKEY); + newKey = (byte*)WMALLOC(inSz, heap, DYNTYPE_PRIVKEY); if (newKey == NULL) { return WS_MEMORY_E; } - *out = newKey; } else { if (*outSz < inSz) { @@ -1626,14 +1655,19 @@ static int DoPemKey(const byte* in, word32 inSz, byte** out, if (ret == WS_SUCCESS) { ret = IdentifyAsn1Key(newKey, newKeySz, 1, heap); - if (ret > 0) { - *outSz = newKeySz; - *outType = (const byte*)IdToName(ret); - *outTypeSz = (word32)WSTRLEN((const char*)*outType); - ret = WS_SUCCESS; - } - else { - WLOG(WS_LOG_DEBUG, "unable to identify key"); + } + + if (ret > 0) { + *out = newKey; + *outSz = newKeySz; + *outType = (const byte*)IdToName(ret); + *outTypeSz = (word32)WSTRLEN((const char*)*outType); + ret = WS_SUCCESS; + } + else { + WLOG(WS_LOG_DEBUG, "unable to identify key"); + if (*out == NULL) { + WFREE(newKey, heap, DYNTYPE_PRIVKEY); } } @@ -1654,7 +1688,6 @@ static int DoOpenSshKey(const byte* in, word32 inSz, byte** out, if (newKey == NULL) { return WS_MEMORY_E; } - *out = newKey; } else { if (*outSz < inSz) { @@ -1669,15 +1702,28 @@ static int DoOpenSshKey(const byte* in, word32 inSz, byte** out, ret = Base64_Decode((byte*)in, inSz, newKey, &newKeySz); if (ret == 0) { + ret = WS_SUCCESS; + } + else { + WLOG(WS_LOG_DEBUG, "Base64 decode of public key failed."); + ret = WS_PARSE_E; + } + + if (ret == WS_SUCCESS) { ret = IdentifyOpenSshKey(newKey, newKeySz, heap); - if (ret > 0) { - *outSz = newKeySz; - *outType = (const byte*)IdToName(ret); - *outTypeSz = (word32)WSTRLEN((const char*)*outType); - ret = WS_SUCCESS; - } - else { - WLOG(WS_LOG_DEBUG, "unable to identify key"); + } + + if (ret > 0) { + *out = newKey; + *outSz = newKeySz; + *outType = (const byte*)IdToName(ret); + *outTypeSz = (word32)WSTRLEN((const char*)*outType); + ret = WS_SUCCESS; + } + else { + WLOG(WS_LOG_DEBUG, "unable to identify key"); + if (*out == NULL) { + WFREE(newKey, heap, DYNTYPE_PRIVKEY); } } From 3b443c01fc8abb58c45af4f11e46135e8e7b716d Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 3 Nov 2023 10:19:07 -0700 Subject: [PATCH 15/47] PR Review 1. Add better error checking to the OpenSSH key code. 2. Add a couple heaps that were missing. --- src/internal.c | 119 ++++++++++++++++++++++++++++++++++++------------- 1 file changed, 87 insertions(+), 32 deletions(-) diff --git a/src/internal.c b/src/internal.c index 863847f14..b9e33888b 100644 --- a/src/internal.c +++ b/src/internal.c @@ -897,7 +897,7 @@ static int wolfSSH_KEY_init(WS_KeySignature* key, byte keyId, void* heap) switch (keyId) { #ifndef WOLFSSH_NO_RSA case ID_SSH_RSA: - ret = wc_InitRsaKey(&key->ks.rsa.key, NULL); + ret = wc_InitRsaKey(&key->ks.rsa.key, heap); break; #endif #ifndef WOLFSSH_NO_ECDSA @@ -1045,39 +1045,84 @@ int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap) } -static int GetOpenSshKeyRsa(RsaKey* key, +/* + * Utility function to read an Mpint from the stream directly into a mp_int. + */ +static INLINE int GetMpintToMp(mp_int* mp, const byte* buf, word32 len, word32* idx) { const byte* val = NULL; word32 valSz = 0; + int ret; + + ret = GetMpint(&valSz, &val, buf, len, idx); + if (ret == WS_SUCCESS) + ret = mp_read_unsigned_bin(mp, val, valSz); + + return ret; +} + + +/* + * For the given RSA key, calculate p^-1 and q^-1. wolfCrypt's RSA + * code expects them, but the OpenSSH format key doesn't store them. + * TODO: Add a RSA read function to wolfCrypt to handle this condition. + */ +static INLINE int CalcRsaInverses(RsaKey* key) +{ mp_int m; + int ret; + + ret = mp_init(&m); + if (ret == MP_OKAY) { + ret = mp_sub_d(&key->p, 1, &m); + if (ret == MP_OKAY) + ret = mp_mod(&key->d, &m, &key->dP); + if (ret == MP_OKAY) + ret = mp_sub_d(&key->q, 1, &m); + if (ret == MP_OKAY) + ret = mp_mod(&key->d, &m, &key->dQ); + mp_forcezero(&m); + } + + return ret; +} + + +/* + * Utility for GetOpenSshKey() to read in RSA keys. + */ +static int GetOpenSshKeyRsa(RsaKey* key, + const byte* buf, word32 len, word32* idx) +{ + int ret; - GetMpint(&valSz, &val, buf, len, idx); /* n */ - mp_read_unsigned_bin(&key->n, val, valSz); - GetMpint(&valSz, &val, buf, len, idx); /* e */ - mp_read_unsigned_bin(&key->e, val, valSz); - GetMpint(&valSz, &val, buf, len, idx); /* d */ - mp_read_unsigned_bin(&key->d, val, valSz); - GetMpint(&valSz, &val, buf, len, idx); /* iqmp */ - mp_read_unsigned_bin(&key->u, val, valSz); - GetMpint(&valSz, &val, buf, len, idx); /* p */ - mp_read_unsigned_bin(&key->p, val, valSz); - GetMpint(&valSz, &val, buf, len, idx); /* q */ - mp_read_unsigned_bin(&key->q, val, valSz); + ret = GetMpintToMp(&key->n, buf, len, idx); + if (ret == WS_SUCCESS) + ret = GetMpintToMp(&key->e, buf, len, idx); + if (ret == WS_SUCCESS) + ret = GetMpintToMp(&key->d, buf, len, idx); + if (ret == WS_SUCCESS) + ret = GetMpintToMp(&key->u, buf, len, idx); + if (ret == WS_SUCCESS) + ret = GetMpintToMp(&key->p, buf, len, idx); + if (ret == WS_SUCCESS) + ret = GetMpintToMp(&key->q, buf, len, idx); /* Calculate dP and dQ for wolfCrypt. */ - mp_init(&m); - mp_sub_d(&key->p, 1, &m); - mp_mod(&key->d, &m, &key->dP); - mp_sub_d(&key->q, 1, &m); - mp_mod(&key->d, &m, &key->dQ); - mp_forcezero(&m); - mp_free(&m); + if (ret == WS_SUCCESS) + ret = CalcRsaInverses(key); + + if (ret != WS_SUCCESS) + ret = WS_RSA_E; - return 0; + return ret; } +/* + * Utility for GetOpenSshKey() to read in ECDSA keys. + */ static int GetOpenSshKeyEcc(ecc_key* key, const byte* buf, word32 len, word32* idx) { @@ -1085,18 +1130,26 @@ static int GetOpenSshKeyEcc(ecc_key* key, word32 nameSz = 0, privSz = 0, pubSz = 0; int ret; - GetStringRef(&nameSz, &name, buf, len, idx); /* curve name */ - GetStringRef(&pubSz, &pub, buf, len, idx); /* Q */ - GetMpint(&privSz, &priv, buf, len, idx); /* d */ + ret = GetStringRef(&nameSz, &name, buf, len, idx); /* curve name */ + if (ret == WS_SUCCESS) + ret = GetStringRef(&pubSz, &pub, buf, len, idx); /* Q */ + if (ret == WS_SUCCESS) + ret = GetMpint(&privSz, &priv, buf, len, idx); /* d */ - ret = wc_ecc_import_private_key_ex(priv, privSz, pub, pubSz, - key, ECC_CURVE_DEF); + if (ret == WS_SUCCESS) + ret = wc_ecc_import_private_key_ex(priv, privSz, pub, pubSz, + key, ECC_CURVE_DEF); - return ret != 0; -} + if (ret != WS_SUCCESS) + ret = WS_ECC_E; + return ret; +} +/* + * Decodes an OpenSSH format key. + */ static int GetOpenSshKey(WS_KeySignature *key, const byte* buf, word32 len, word32* idx) { @@ -1155,17 +1208,19 @@ static int GetOpenSshKey(WS_KeySignature *key, str, strSz, &subIdx); if (ret == WS_SUCCESS) { keyId = NameToId((const char*)subStr, subStrSz); - wolfSSH_KEY_init(key, keyId, NULL); + ret = wolfSSH_KEY_init(key, keyId, NULL); + } + if (ret == WS_SUCCESS) { switch (keyId) { #ifndef WOLFSSH_NO_RSA case ID_SSH_RSA: - GetOpenSshKeyRsa(&key->ks.rsa.key, + ret = GetOpenSshKeyRsa(&key->ks.rsa.key, str, strSz, &subIdx); break; #endif #ifndef WOLFSSH_NO_ECDSA case ID_ECDSA_SHA2_NISTP256: - GetOpenSshKeyEcc(&key->ks.ecc.key, + ret = GetOpenSshKeyEcc(&key->ks.ecc.key, str, strSz, &subIdx); break; #endif From 23a9bb82a0518037c6d84d9a1f3dce7859d38f69 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 3 Nov 2023 14:24:28 -0700 Subject: [PATCH 16/47] PR Review 1. Change the block size in the key decoding to a named constant rather than a bare number. 2. Change the comparison from a difference of two unsigned values against zero to comparing them directly. --- src/internal.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/internal.c b/src/internal.c index b9e33888b..00ccf8779 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1239,8 +1239,8 @@ static int GetOpenSshKey(WS_KeySignature *key, * and the length of the comment delimit the end of the * encrypted blob. No added padding required. */ if (ret == WS_SUCCESS) { - if (strSz % 8 == 0) { - if (strSz - subIdx > 0) { + if (strSz % MIN_BLOCK_SZ == 0) { + if (strSz > subIdx) { /* The padding starts at 1. */ check2 = strSz - subIdx; for (check1 = 1; From 707104ee3a290bcfec89d041e8e34f006a12167e Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 3 Nov 2023 14:58:54 -0700 Subject: [PATCH 17/47] PR Review: Remove unused struct members from WS_KeySignature. --- src/internal.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/internal.c b/src/internal.c index 00ccf8779..04fac03bb 100644 --- a/src/internal.c +++ b/src/internal.c @@ -861,25 +861,11 @@ typedef struct WS_KeySignature { #ifndef WOLFSSH_NO_RSA struct { RsaKey key; - byte e[256]; - word32 eSz; - byte ePad; - byte n[256]; - word32 nSz; - byte nPad; } rsa; #endif #ifndef WOLFSSH_NO_ECDSA struct { ecc_key key; - word32 keyBlobSz; - const char *keyBlobName; - word32 keyBlobNameSz; - byte q[256]; - word32 qSz; - byte qPad; - const char *primeName; - word32 primeNameSz; } ecc; #endif } ks; From dba9a6c36c200ddffab8e6ec6f41801369eba86e Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 6 Nov 2023 15:16:39 -0800 Subject: [PATCH 18/47] PR Review 1. Add test keys. 2. Add API test for wolfSSH_ReadKey_buffer(). 3. Fix allocation issue found using the API test. --- keys/id_ecdsa | 9 +++ keys/id_ecdsa.pub | 1 + keys/id_rsa | 27 +++++++ keys/id_rsa.pub | 1 + keys/include.am | 1 + src/ssh.c | 2 +- tests/api.c | 198 ++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 238 insertions(+), 1 deletion(-) create mode 100644 keys/id_ecdsa create mode 100644 keys/id_ecdsa.pub create mode 100644 keys/id_rsa create mode 100644 keys/id_rsa.pub diff --git a/keys/id_ecdsa b/keys/id_ecdsa new file mode 100644 index 000000000..a73f1d4fe --- /dev/null +++ b/keys/id_ecdsa @@ -0,0 +1,9 @@ +-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAaAAAABNlY2RzYS +1zaGEyLW5pc3RwMjU2AAAACG5pc3RwMjU2AAAAQQTAqdBgCp8bYSq2kQQ48/Ud8Iy6Mjnb +/fpB3LfSE/1kx9VaaE4FL3i9Gg2vDV0eLGM3PWksFNPhULxtcYJyjaBjAAAAqJAeleSQHp +XkAAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBMCp0GAKnxthKraR +BDjz9R3wjLoyOdv9+kHct9IT/WTH1VpoTgUveL0aDa8NXR4sYzc9aSwU0+FQvG1xgnKNoG +MAAAAgPrOgktioNqad/wHNC/rt/zVrpNqDnOwg9tNDFMOTwo8AAAANYm9iQGxvY2FsaG9z +dAECAw== +-----END OPENSSH PRIVATE KEY----- diff --git a/keys/id_ecdsa.pub b/keys/id_ecdsa.pub new file mode 100644 index 000000000..22fb1dc32 --- /dev/null +++ b/keys/id_ecdsa.pub @@ -0,0 +1 @@ +ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBMCp0GAKnxthKraRBDjz9R3wjLoyOdv9+kHct9IT/WTH1VpoTgUveL0aDa8NXR4sYzc9aSwU0+FQvG1xgnKNoGM= bob@localhost diff --git a/keys/id_rsa b/keys/id_rsa new file mode 100644 index 000000000..b1cd15885 --- /dev/null +++ b/keys/id_rsa @@ -0,0 +1,27 @@ +-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAABFwAAAAdzc2gtcn +NhAAAAAwEAAQAAAQEAy2cigZDlpBT+X2MJHAoHnfeFf6+LHm6BDkAT8V9ejHA4dY0Aepb6 +NbV6u/oYZlueKPeAZ3GNztR9szoL6FSlMvkd9oqvfoxjTGu71T0981ybJelqqGATGtevHU +6Jko/I0+lgSQFKWQJ7D3Dj2zlZpIXB2Q7xl/i9kFZgaIqFhUHdWO9JMOwCFwoDrhd8v5xk +y1v3OIIZDxiYxVIKbf2J07WbwiSFAxXfiX8TjUBDLFmtqt1AF6LjAyGyaRICXkaGJQ/QJ9 +sX85h9bkiPlGNAtQGQtNUg3tC9GqOkZ9tCKY1Efh/r0zosOA7ufxg6ymLpq1C4LU/4ENGH +kuRPAKvu8wAAA8gztJfmM7SX5gAAAAdzc2gtcnNhAAABAQDLZyKBkOWkFP5fYwkcCged94 +V/r4seboEOQBPxX16McDh1jQB6lvo1tXq7+hhmW54o94BncY3O1H2zOgvoVKUy+R32iq9+ +jGNMa7vVPT3zXJsl6WqoYBMa168dTomSj8jT6WBJAUpZAnsPcOPbOVmkhcHZDvGX+L2QVm +BoioWFQd1Y70kw7AIXCgOuF3y/nGTLW/c4ghkPGJjFUgpt/YnTtZvCJIUDFd+JfxONQEMs +Wa2q3UAXouMDIbJpEgJeRoYlD9An2xfzmH1uSI+UY0C1AZC01SDe0L0ao6Rn20IpjUR+H+ +vTOiw4Du5/GDrKYumrULgtT/gQ0YeS5E8Aq+7zAAAAAwEAAQAAAQEAvbdBiQXkGyn1pHST +/5IfTqia3OCX6td5ChicQUsJvgXBs2rDopQFZmkRxBjd/0K+/0jyfAl/EgZCBBRFHPsuZp +/S4ayzSV6aE6J8vMT1bnLWxwKyl7+csjGwRK6HRKtVzsnjI9TPSrw0mc9ax5PzV6/mgZUd +o/i+nszh+UASj5mYrBGqMiINspzX6YC+qoUHor3rEJOd9p1aO+N5+1fDKiDnlkM5IO0Qsz +GktuwL0fzv9zBnGfnWVJz3CorfP1OW5KCtrDn7BnkQf1eBeVLzq/uoglUjS4DNnVfLA67D +O4ZfwtnoW8Gr2R+KdvnypvHnDeY5X51r5PDgL4+7z47pWQAAAIBNFcAzHHE19ISGN8YRHk +23/r/3zfvzHU68GSKR1Xj/Y4LSdRTpSm3wBrdQ17f5B4V7RVl2CJvoPekTggnBDQlLJ7fU +NU93/nZrY9teYdrNh03buL54VVb5tUM+KN+27zERlTj0/LmYJupN97sZXmlgKsvLbcsnM2 +i7HuQQaFnsIQAAAIEA5wqFVatT9yovt8pS7rAyYUL/cqc50TZ/5Nwfy5uasRyf1BphHwEW +LEimBemVc+VrNwAkt6MFWuloK5ssqb1ubvtRI8Mntd15rRfZtq/foS3J8FJxueXLDWlECy +PmVyfVN1Vv4ZeirBy9BTYLiSuxMes+HYks3HucQhxIN1j8SA0AAACBAOFgRjfWXv1/93Jp +6CCJ5c98MWP+zu1FbLIlklxPb85osZqlazXHNPPEtblC4z+OqRGMCsv2683anU4ZzcTFIk +JS3lzeJ3tdAH4osQ5etKkV4mcdCmeRpjudB9VbaziVhPX02qkPWpM0ckPrgB3hVNUDPz89 +GtJd3mlhyY5IfFL/AAAADWJvYkBsb2NhbGhvc3QBAgMEBQ== +-----END OPENSSH PRIVATE KEY----- diff --git a/keys/id_rsa.pub b/keys/id_rsa.pub new file mode 100644 index 000000000..9cf541905 --- /dev/null +++ b/keys/id_rsa.pub @@ -0,0 +1 @@ +ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDLZyKBkOWkFP5fYwkcCged94V/r4seboEOQBPxX16McDh1jQB6lvo1tXq7+hhmW54o94BncY3O1H2zOgvoVKUy+R32iq9+jGNMa7vVPT3zXJsl6WqoYBMa168dTomSj8jT6WBJAUpZAnsPcOPbOVmkhcHZDvGX+L2QVmBoioWFQd1Y70kw7AIXCgOuF3y/nGTLW/c4ghkPGJjFUgpt/YnTtZvCJIUDFd+JfxONQEMsWa2q3UAXouMDIbJpEgJeRoYlD9An2xfzmH1uSI+UY0C1AZC01SDe0L0ao6Rn20IpjUR+H+vTOiw4Du5/GDrKYumrULgtT/gQ0YeS5E8Aq+7z bob@localhost diff --git a/keys/include.am b/keys/include.am index 5f314bb05..cc2aa720f 100644 --- a/keys/include.am +++ b/keys/include.am @@ -22,5 +22,6 @@ EXTRA_DIST+= \ keys/server-cert.der keys/server-cert.pem \ keys/fred-cert.der keys/fred-cert.pem \ keys/server-key.pem keys/fred-key.der keys/fred-key.pem \ + keys/id_ecdsa keys/id_ecdsa.pub keys/id_rsa keys/id_rsa.pub \ keys/renewcerts.sh keys/renewcerts.cnf diff --git a/src/ssh.c b/src/ssh.c index 1ad8c5c6f..70b5d53d5 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1533,7 +1533,7 @@ static int DoSshPubKey(const byte* in, word32 inSz, byte** out, * function */ newKeySz = ((word32)WSTRLEN(key) * 3 + 3) / 4; if (*out == NULL) { - newKey = (byte*)WMALLOC(*outSz, heap, DYNTYPE_PRIVKEY); + newKey = (byte*)WMALLOC(newKeySz, heap, DYNTYPE_PRIVKEY); if (newKey == NULL) { ret = WS_MEMORY_E; } diff --git a/tests/api.c b/tests/api.c index 1e098fdfb..9353c4d2b 100644 --- a/tests/api.c +++ b/tests/api.c @@ -602,6 +602,203 @@ static void test_wolfSSH_CertMan(void) } +#define KEY_BUF_SZ 2048 + +#ifndef WOLFSSH_NO_RSA + +const char id_rsa[] = + "-----BEGIN OPENSSH PRIVATE KEY-----\n" + "b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAABFwAAAAdzc2gtcn\n" + "NhAAAAAwEAAQAAAQEAy2cigZDlpBT+X2MJHAoHnfeFf6+LHm6BDkAT8V9ejHA4dY0Aepb6\n" + "NbV6u/oYZlueKPeAZ3GNztR9szoL6FSlMvkd9oqvfoxjTGu71T0981ybJelqqGATGtevHU\n" + "6Jko/I0+lgSQFKWQJ7D3Dj2zlZpIXB2Q7xl/i9kFZgaIqFhUHdWO9JMOwCFwoDrhd8v5xk\n" + "y1v3OIIZDxiYxVIKbf2J07WbwiSFAxXfiX8TjUBDLFmtqt1AF6LjAyGyaRICXkaGJQ/QJ9\n" + "sX85h9bkiPlGNAtQGQtNUg3tC9GqOkZ9tCKY1Efh/r0zosOA7ufxg6ymLpq1C4LU/4ENGH\n" + "kuRPAKvu8wAAA8gztJfmM7SX5gAAAAdzc2gtcnNhAAABAQDLZyKBkOWkFP5fYwkcCged94\n" + "V/r4seboEOQBPxX16McDh1jQB6lvo1tXq7+hhmW54o94BncY3O1H2zOgvoVKUy+R32iq9+\n" + "jGNMa7vVPT3zXJsl6WqoYBMa168dTomSj8jT6WBJAUpZAnsPcOPbOVmkhcHZDvGX+L2QVm\n" + "BoioWFQd1Y70kw7AIXCgOuF3y/nGTLW/c4ghkPGJjFUgpt/YnTtZvCJIUDFd+JfxONQEMs\n" + "Wa2q3UAXouMDIbJpEgJeRoYlD9An2xfzmH1uSI+UY0C1AZC01SDe0L0ao6Rn20IpjUR+H+\n" + "vTOiw4Du5/GDrKYumrULgtT/gQ0YeS5E8Aq+7zAAAAAwEAAQAAAQEAvbdBiQXkGyn1pHST\n" + "/5IfTqia3OCX6td5ChicQUsJvgXBs2rDopQFZmkRxBjd/0K+/0jyfAl/EgZCBBRFHPsuZp\n" + "/S4ayzSV6aE6J8vMT1bnLWxwKyl7+csjGwRK6HRKtVzsnjI9TPSrw0mc9ax5PzV6/mgZUd\n" + "o/i+nszh+UASj5mYrBGqMiINspzX6YC+qoUHor3rEJOd9p1aO+N5+1fDKiDnlkM5IO0Qsz\n" + "GktuwL0fzv9zBnGfnWVJz3CorfP1OW5KCtrDn7BnkQf1eBeVLzq/uoglUjS4DNnVfLA67D\n" + "O4ZfwtnoW8Gr2R+KdvnypvHnDeY5X51r5PDgL4+7z47pWQAAAIBNFcAzHHE19ISGN8YRHk\n" + "23/r/3zfvzHU68GSKR1Xj/Y4LSdRTpSm3wBrdQ17f5B4V7RVl2CJvoPekTggnBDQlLJ7fU\n" + "NU93/nZrY9teYdrNh03buL54VVb5tUM+KN+27zERlTj0/LmYJupN97sZXmlgKsvLbcsnM2\n" + "i7HuQQaFnsIQAAAIEA5wqFVatT9yovt8pS7rAyYUL/cqc50TZ/5Nwfy5uasRyf1BphHwEW\n" + "LEimBemVc+VrNwAkt6MFWuloK5ssqb1ubvtRI8Mntd15rRfZtq/foS3J8FJxueXLDWlECy\n" + "PmVyfVN1Vv4ZeirBy9BTYLiSuxMes+HYks3HucQhxIN1j8SA0AAACBAOFgRjfWXv1/93Jp\n" + "6CCJ5c98MWP+zu1FbLIlklxPb85osZqlazXHNPPEtblC4z+OqRGMCsv2683anU4ZzcTFIk\n" + "JS3lzeJ3tdAH4osQ5etKkV4mcdCmeRpjudB9VbaziVhPX02qkPWpM0ckPrgB3hVNUDPz89\n" + "GtJd3mlhyY5IfFL/AAAADWJvYkBsb2NhbGhvc3QBAgMEBQ==\n" + "-----END OPENSSH PRIVATE KEY-----\n"; + +const char id_rsa_pub[] = + "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDLZyKBkOWkFP5fYwkcCged94V/r4seboEO" + "QBPxX16McDh1jQB6lvo1tXq7+hhmW54o94BncY3O1H2zOgvoVKUy+R32iq9+jGNMa7vVPT3z" + "XJsl6WqoYBMa168dTomSj8jT6WBJAUpZAnsPcOPbOVmkhcHZDvGX+L2QVmBoioWFQd1Y70kw" + "7AIXCgOuF3y/nGTLW/c4ghkPGJjFUgpt/YnTtZvCJIUDFd+JfxONQEMsWa2q3UAXouMDIbJp" + "EgJeRoYlD9An2xfzmH1uSI+UY0C1AZC01SDe0L0ao6Rn20IpjUR+H+vTOiw4Du5/GDrKYumr" + "ULgtT/gQ0YeS5E8Aq+7z bob@localhost\n"; + +#endif /* WOLFSSH_NO_RSA */ + +#ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP256 + +const char id_ecdsa[] = + "-----BEGIN OPENSSH PRIVATE KEY-----\n" + "b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAaAAAABNlY2RzYS\n" + "1zaGEyLW5pc3RwMjU2AAAACG5pc3RwMjU2AAAAQQTAqdBgCp8bYSq2kQQ48/Ud8Iy6Mjnb\n" + "/fpB3LfSE/1kx9VaaE4FL3i9Gg2vDV0eLGM3PWksFNPhULxtcYJyjaBjAAAAqJAeleSQHp\n" + "XkAAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBMCp0GAKnxthKraR\n" + "BDjz9R3wjLoyOdv9+kHct9IT/WTH1VpoTgUveL0aDa8NXR4sYzc9aSwU0+FQvG1xgnKNoG\n" + "MAAAAgPrOgktioNqad/wHNC/rt/zVrpNqDnOwg9tNDFMOTwo8AAAANYm9iQGxvY2FsaG9z\n" + "dAECAw==\n" + "-----END OPENSSH PRIVATE KEY-----\n"; + +const char id_ecdsa_pub[] = + "ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABB" + "BMCp0GAKnxthKraRBDjz9R3wjLoyOdv9+kHct9IT/WTH1VpoTgUveL0aDa8NXR4sYzc9aSwU" + "0+FQvG1xgnKNoGM= bob@localhost\n"; + +#endif /* WOLFSSH_NO_ECDSA_SHA2_NISTP256 */ + +static void test_wolfSSH_ReadKey(void) +{ +#if !defined(WOLFSSH_NO_RSA) || !defined(WOLFSSH_NO_ECDSA_SHA2_NISTP256) + byte *key, *keyCheck, *derKey; + const byte* keyType; + word32 keySz, keyTypeSz, derKeySz; + int ret; +#endif + +#ifndef WOLFSSH_NO_RSA + + /* OpenSSH Format, ssh-rsa, private, need alloc */ + key = NULL; + keySz = 0; + keyType = NULL; + keyTypeSz = 0; + ret = wolfSSH_ReadKey_buffer((const byte*)id_rsa, (word32)WSTRLEN(id_rsa), + WOLFSSH_FORMAT_OPENSSH, &key, &keySz, &keyType, &keyTypeSz, NULL); + AssertIntEQ(ret, WS_SUCCESS); + AssertNotNull(key); + AssertIntGT(keySz, 0); + AssertStrEQ(keyType, "ssh-rsa"); + AssertIntEQ(keyTypeSz, (word32)WSTRLEN("ssh-rsa")); + WFREE(key, NULL, DYNTYPE_FILE); + + /* SSL PEM Format, ssh-rsa, private, need alloc */ + derKey = NULL; + derKeySz = 0; + key = NULL; + keySz = 0; + keyType = NULL; + keyTypeSz = 0; + ret = ConvertHexToBin(serverKeyRsaDer, &derKey, &derKeySz, + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL); + AssertIntEQ(ret, 0); + ret = wolfSSH_ReadKey_buffer(derKey, derKeySz, WOLFSSH_FORMAT_ASN1, + &key, &keySz, &keyType, &keyTypeSz, NULL); + AssertIntEQ(ret, WS_SUCCESS); + AssertNotNull(key); + AssertIntGT(keySz, 0); + AssertStrEQ(keyType, "ssh-rsa"); + AssertIntEQ(keyTypeSz, (word32)WSTRLEN("ssh-rsa")); + WFREE(key, NULL, DYNTYPE_FILE); + WFREE(derKey, NULL, 0); + + /* OpenSSH Format, ssh-rsa, public, need alloc */ + key = NULL; + keySz = 0; + keyType = NULL; + keyTypeSz = 0; + ret = wolfSSH_ReadKey_buffer((const byte*)id_rsa_pub, + (word32)WSTRLEN(id_rsa_pub), WOLFSSH_FORMAT_SSH, + &key, &keySz, &keyType, &keyTypeSz, NULL); + AssertIntEQ(ret, WS_SUCCESS); + AssertNotNull(key); + AssertIntGT(keySz, 0); + AssertStrEQ(keyType, "ssh-rsa"); + AssertIntEQ(keyTypeSz, (word32)WSTRLEN("ssh-rsa")); + WFREE(key, NULL, DYNTYPE_FILE); + + /* OpenSSH Format, ssh-rsa, private, no alloc */ + keyCheck = (byte*)WMALLOC(KEY_BUF_SZ, NULL, DYNTYPE_FILE); + AssertNotNull(keyCheck); + key = keyCheck; + keySz = KEY_BUF_SZ; + keyType = NULL; + keyTypeSz = 0; + ret = wolfSSH_ReadKey_buffer((const byte*)id_rsa, (word32)WSTRLEN(id_rsa), + WOLFSSH_FORMAT_OPENSSH, &key, &keySz, &keyType, &keyTypeSz, NULL); + AssertIntEQ(ret, WS_SUCCESS); + AssertTrue(key == keyCheck); + AssertIntGT(keySz, 0); + AssertStrEQ(keyType, "ssh-rsa"); + AssertIntEQ(keyTypeSz, (word32)WSTRLEN("ssh-rsa")); + WFREE(keyCheck, NULL, DYNTYPE_FILE); + +#endif /* WOLFSSH_NO_RSA */ + +#ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP256 + + /* OpenSSH Format, ecdsa-sha2-nistp256, private, need alloc */ + key = NULL; + keySz = 0; + keyType = NULL; + keyTypeSz = 0; + ret = wolfSSH_ReadKey_buffer((const byte*)id_ecdsa, + (word32)WSTRLEN(id_ecdsa), WOLFSSH_FORMAT_OPENSSH, + &key, &keySz, &keyType, &keyTypeSz, NULL); + AssertIntEQ(ret, WS_SUCCESS); + AssertNotNull(key); + AssertIntGT(keySz, 0); + AssertStrEQ(keyType, "ecdsa-sha2-nistp256"); + AssertIntEQ(keyTypeSz, (word32)WSTRLEN("ecdsa-sha2-nistp256")); + WFREE(key, NULL, DYNTYPE_FILE); + + /* SSL DER Format, ecdsa-sha2-nistp256, private, need alloc */ + derKey = NULL; + derKeySz = 0; + key = NULL; + keySz = 0; + keyType = NULL; + keyTypeSz = 0; + ret = ConvertHexToBin(serverKeyEccDer, &derKey, &derKeySz, + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL); + AssertIntEQ(ret, WS_SUCCESS); + ret = wolfSSH_ReadKey_buffer(derKey, derKeySz, WOLFSSH_FORMAT_ASN1, + &key, &keySz, &keyType, &keyTypeSz, NULL); + AssertIntEQ(ret, WS_SUCCESS); + AssertNotNull(key); + AssertIntGT(keySz, 0); + AssertStrEQ(keyType, "ecdsa-sha2-nistp256"); + AssertIntEQ(keyTypeSz, (word32)WSTRLEN("ecdsa-sha2-nistp256")); + WFREE(key, NULL, DYNTYPE_FILE); + WFREE(derKey, NULL, 0); + + /* OpenSSH Format, ecdsa-sha2-nistp256, public, need alloc */ + key = NULL; + keySz = 0; + keyType = NULL; + keyTypeSz = 0; + ret = wolfSSH_ReadKey_buffer((const byte*)id_ecdsa_pub, + (word32)WSTRLEN(id_ecdsa_pub), WOLFSSH_FORMAT_SSH, + &key, &keySz, &keyType, &keyTypeSz, NULL); + AssertIntEQ(ret, WS_SUCCESS); + AssertNotNull(key); + AssertIntGT(keySz, 0); + AssertStrEQ(keyType, "ecdsa-sha2-nistp256"); + AssertIntEQ(keyTypeSz, (word32)WSTRLEN("ecdsa-sha2-nistp256")); + WFREE(key, NULL, DYNTYPE_FILE); + +#endif /* WOLFSSH_NO_ECDSA_SHA2_NISTP256 */ +} + + #ifdef WOLFSSH_SCP static int my_ScpRecv(WOLFSSH* ssh, int state, const char* basePath, @@ -1125,6 +1322,7 @@ int wolfSSH_ApiTest(int argc, char** argv) test_wolfSSH_CTX_UsePrivateKey_buffer(); test_wolfSSH_CTX_UseCert_buffer(); test_wolfSSH_CertMan(); + test_wolfSSH_ReadKey(); /* SCP tests */ test_wolfSSH_SCP_CB(); From 03f82ab744bc919196809708efe405bf37172bbd Mon Sep 17 00:00:00 2001 From: Fabio <507164+falemagn@users.noreply.github.com> Date: Thu, 9 Nov 2023 12:00:49 +0100 Subject: [PATCH 19/47] Made setting file modes portable, by means of defining the WOLFSSH_SFTP_SETFILEMODE and WOLFSSH_SFTP_SETMODE macros, in which case the functions SFTP_SetMode and SFTP_SetFileMode will have to be externally defined for the platform the library is being ported to. --- src/wolfsftp.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/wolfsftp.c b/src/wolfsftp.c index d50792fd3..f7210cae3 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -5087,19 +5087,29 @@ int wolfSSH_SFTP_RecvLSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) return ret; } - -#if !defined(USE_WINDOWS_API) && !defined(WOLFSSH_ZEPHYR) +#if !defined(USE_WINDOWS_API) && !defined(WOLFSSH_ZEPHYR) && !defined(WOLFSSH_SFTP_SETMODE) /* Set the files mode * return WS_SUCCESS on success */ -static int SFTP_SetMode(WOLFSSH* ssh, char* name, word32 mode) { - WOLFSSH_UNUSED(ssh); - if (WCHMOD(ssh->fs, name, mode) != 0) { +static int SFTP_SetMode(void* fs, char* name, word32 mode) { + WOLFSSH_UNUSED(fs) + if (WCHMOD(fs, name, mode) != 0) { return WS_BAD_FILE_E; } return WS_SUCCESS; } #endif +#if !defined(USE_WINDOWS_API) && !defined(WOLFSSH_SFTP_SETMODEHANDLE) +/* Set the files mode + * return WS_SUCCESS on success */ +static int SFTP_SetModeHandle(void* fs, WFD handle, word32 mode) { + WOLFSSH_UNUSED(fs) + if (WFCHMOD(fs, handle, mode) != 0) { + return WS_BAD_FILE_E; + } + return WS_SUCCESS; +} +#endif #if !defined(_WIN32_WCE) && !defined(WOLFSSH_ZEPHYR) @@ -5122,7 +5132,7 @@ static int SFTP_SetFileAttributes(WOLFSSH* ssh, char* name, WS_SFTP_FILEATRB* at #if !defined(USE_WINDOWS_API) && !defined(WOLFSSH_ZEPHYR) /* check if permissions attribute present */ if (atr->flags & WOLFSSH_FILEATRB_PERM) { - ret = SFTP_SetMode(ssh, name, atr->per); + ret = SFTP_SetMode(ssh->fs, name, atr->per); } #endif @@ -5162,9 +5172,7 @@ static int SFTP_SetFileAttributesHandle(WOLFSSH* ssh, WFD handle, WS_SFTP_FILEAT #ifndef USE_WINDOWS_API /* check if permissions attribute present */ if (atr->flags & WOLFSSH_FILEATRB_PERM) { - if (WFCHMOD(ssh->fs, handle, atr->per) != 0) { - ret = WS_BAD_FILE_E; - } + ret = SFTP_SetModeHandle(ssh->fs, handle, atr->per); } #endif From 74c15231d977ad85ad1f8c599598ae529453d120 Mon Sep 17 00:00:00 2001 From: Fabio <507164+falemagn@users.noreply.github.com> Date: Thu, 9 Nov 2023 15:45:18 +0100 Subject: [PATCH 20/47] Don't compile SFTP_SetModeHandle on Zephyr. --- src/wolfsftp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wolfsftp.c b/src/wolfsftp.c index f7210cae3..a7d26fb99 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -5099,7 +5099,7 @@ static int SFTP_SetMode(void* fs, char* name, word32 mode) { } #endif -#if !defined(USE_WINDOWS_API) && !defined(WOLFSSH_SFTP_SETMODEHANDLE) +#if !defined(USE_WINDOWS_API) && !defined(WOLFSSH_ZEPHYR) && !defined(WOLFSSH_SFTP_SETMODEHANDLE) /* Set the files mode * return WS_SUCCESS on success */ static int SFTP_SetModeHandle(void* fs, WFD handle, word32 mode) { From 2d3956e0cb165a869e612cad995b2e7986807169 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Mon, 13 Nov 2023 15:59:55 -0800 Subject: [PATCH 21/47] do not require locking file with SFTP on Windows --- src/wolfsftp.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/wolfsftp.c b/src/wolfsftp.c index d50792fd3..a2619596e 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -2171,8 +2171,9 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) } #endif - fileHandle = WS_CreateFileA(dir, desiredAccess, 0, creationDisp, - FILE_ATTRIBUTE_NORMAL, ssh->ctx->heap); + fileHandle = WS_CreateFileA(dir, desiredAccess, + (FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE), + creationDisp, FILE_ATTRIBUTE_NORMAL, ssh->ctx->heap); if (fileHandle == INVALID_HANDLE_VALUE) { WLOG(WS_LOG_SFTP, "Error opening file %s", dir); res = oer; @@ -8512,8 +8513,9 @@ int wolfSSH_SFTP_Get(WOLFSSH* ssh, char* from, if (state->gOfst > 0) desiredAccess |= FILE_APPEND_DATA; state->fileHandle = WS_CreateFileA(to, desiredAccess, - 0, CREATE_ALWAYS, - FILE_ATTRIBUTE_NORMAL, ssh->ctx->heap); + (FILE_SHARE_DELETE | FILE_SHARE_READ | + FILE_SHARE_WRITE), CREATE_ALWAYS, + FILE_ATTRIBUTE_NORMAL, ssh->ctx->heap); } if (resume) { WMEMSET(&state->offset, 0, sizeof(OVERLAPPED)); From 68cf698321289a85a1dd67f3b0e1944e166e30a6 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Tue, 14 Nov 2023 15:52:44 -0800 Subject: [PATCH 22/47] when getting failure status packets return failure error code --- src/wolfsftp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/wolfsftp.c b/src/wolfsftp.c index a2619596e..0d3cf44c1 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -7475,6 +7475,9 @@ int wolfSSH_SFTP_SendReadPacket(WOLFSSH* ssh, byte* handle, word32 handleSz, WLOG(WS_LOG_SFTP, "OK or EOF found"); ret = 0; /* nothing was read */ } + else { + ret = WS_FATAL_ERROR; + } } state->state = STATE_SEND_READ_CLEANUP; NO_BREAK; From ec1964dd1c7932944ec7fcfc6e0a821a86296a99 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 16 Nov 2023 16:33:28 -0800 Subject: [PATCH 23/47] PR Review 1. Rename and move CleanupUserAuthRequestPublicKey() as wolfSSH_KEY_clean(). 2. Restrict the number of keys allowed in an OpenSSH key package to 1. 3. Initialize the Rsa and Ecc keys right before getting them. 4. New error code for an OpenSSH key format error when decoding. --- src/internal.c | 115 ++++++++++++++++----------------------------- src/ssh.c | 4 ++ wolfssh/error.h | 3 +- wolfssh/internal.h | 3 ++ 4 files changed, 50 insertions(+), 75 deletions(-) diff --git a/src/internal.c b/src/internal.c index 04fac03bb..2b722f7d8 100644 --- a/src/internal.c +++ b/src/internal.c @@ -137,6 +137,8 @@ Set when all ECDH algorithms are disabled. Set to disable use of all ECDH algorithms for key agreement. Setting this will force all ECDH key agreement algorithms off. + WOLFSSH_KEY_QUANTITY_REQ + Number of keys required to be in an OpenSSH-style key wrapper. */ static const char sshProtoIdStr[] = "SSH-2.0-wolfSSHv" @@ -425,6 +427,9 @@ const char* GetErrorString(int err) case WS_KEY_CHECK_VAL_E: return "key check value error"; + case WS_KEY_FORMAT_E: + return "key format wrong error"; + default: return "Unknown error code"; } @@ -872,58 +877,22 @@ typedef struct WS_KeySignature { } WS_KeySignature; -static int wolfSSH_KEY_init(WS_KeySignature* key, byte keyId, void* heap) +static void wolfSSH_KEY_clean(WS_KeySignature* key) { - int ret = WS_SUCCESS; - if (key != NULL) { - WMEMSET(key, 0, sizeof(*key)); - key->keySigId = keyId; - - switch (keyId) { + if (key->keySigId == ID_SSH_RSA) { #ifndef WOLFSSH_NO_RSA - case ID_SSH_RSA: - ret = wc_InitRsaKey(&key->ks.rsa.key, heap); - break; -#endif -#ifndef WOLFSSH_NO_ECDSA - case ID_ECDSA_SHA2_NISTP256: - ret = wc_ecc_init_ex(&key->ks.ecc.key, heap, INVALID_DEVID); - break; + wc_FreeRsaKey(&key->ks.rsa.key); #endif - default: - ret = WS_INVALID_ALGO_ID; } - } - - return ret; -} - - -static int wolfSSH_KEY_clean(WS_KeySignature* key) -{ - int ret = WS_SUCCESS; - - if (key != NULL) { - switch (key->keySigId) { -#ifndef WOLFSSH_NO_RSA - case ID_SSH_RSA: - wc_FreeRsaKey(&key->ks.rsa.key); - break; -#endif + else if (key->keySigId == ID_ECDSA_SHA2_NISTP256 || + key->keySigId == ID_ECDSA_SHA2_NISTP384 || + key->keySigId == ID_ECDSA_SHA2_NISTP521) { #ifndef WOLFSSH_NO_ECDSA - case ID_ECDSA_SHA2_NISTP256: - case ID_ECDSA_SHA2_NISTP384: - case ID_ECDSA_SHA2_NISTP521: - wc_ecc_free(&key->ks.ecc.key); - break; + wc_ecc_free(&key->ks.ecc.key); #endif - default: - ret = WS_INVALID_ALGO_ID; } } - - return ret; } @@ -1083,7 +1052,9 @@ static int GetOpenSshKeyRsa(RsaKey* key, { int ret; - ret = GetMpintToMp(&key->n, buf, len, idx); + ret = wc_InitRsaKey(key, NULL); + if (ret == WS_SUCCESS) + ret = GetMpintToMp(&key->n, buf, len, idx); if (ret == WS_SUCCESS) ret = GetMpintToMp(&key->e, buf, len, idx); if (ret == WS_SUCCESS) @@ -1116,7 +1087,9 @@ static int GetOpenSshKeyEcc(ecc_key* key, word32 nameSz = 0, privSz = 0, pubSz = 0; int ret; - ret = GetStringRef(&nameSz, &name, buf, len, idx); /* curve name */ + ret = wc_ecc_init(key); + if (ret == WS_SUCCESS) + ret = GetStringRef(&nameSz, &name, buf, len, idx); /* curve name */ if (ret == WS_SUCCESS) ret = GetStringRef(&pubSz, &pub, buf, len, idx); /* Q */ if (ret == WS_SUCCESS) @@ -1141,7 +1114,7 @@ static int GetOpenSshKey(WS_KeySignature *key, { const char AuthMagic[] = "openssh-key-v1"; const byte* str = NULL; - word32 keyCount = 0, strSz = 0, i; + word32 keyCount = 0, strSz, i; int ret = WS_SUCCESS; if (WSTRCMP(AuthMagic, (const char*)buf) != 0) { @@ -1162,18 +1135,29 @@ static int GetOpenSshKey(WS_KeySignature *key, if (ret == WS_SUCCESS) ret = GetUint32(&keyCount, buf, len, idx); /* key count */ - for (i = 0; i < keyCount; i++) { - if (ret == WS_SUCCESS) - ret = GetStringRef(&strSz, &str, buf, len, idx); /* public buf */ + if (ret == WS_SUCCESS) { + if (keyCount != WOLFSSH_KEY_QUANTITY_REQ) { + ret = WS_KEY_FORMAT_E; + } } - if (keyCount > 0) { - if (ret == WS_SUCCESS) { - ret = GetStringRef(&strSz, &str, buf, len, idx); - /* list of private keys */ + if (ret == WS_SUCCESS) { + strSz = 0; + ret = GetStringRef(&strSz, &str, buf, len, idx); + /* public buf */ + } + + if (ret == WS_SUCCESS) { + strSz = 0; + ret = GetStringRef(&strSz, &str, buf, len, idx); + /* list of private keys */ + + /* If there isn't a private key, the key file is bad. */ + if (ret == WS_SUCCESS && strSz == 0) { + ret = WS_KEY_FORMAT_E; } - if (strSz > 0) { + if (ret == WS_SUCCESS) { const byte* subStr = NULL; word32 subStrSz = 0, subIdx = 0, check1 = 0, check2 = ~0; byte keyId; @@ -1194,7 +1178,7 @@ static int GetOpenSshKey(WS_KeySignature *key, str, strSz, &subIdx); if (ret == WS_SUCCESS) { keyId = NameToId((const char*)subStr, subStrSz); - ret = wolfSSH_KEY_init(key, keyId, NULL); + key->keySigId = keyId; } if (ret == WS_SUCCESS) { switch (keyId) { @@ -1273,7 +1257,7 @@ int IdentifyOpenSshKey(const byte* in, word32 inSz, void* heap) } else { WMEMSET(key, 0, sizeof(*key)); - key->keySigId = ID_UNKNOWN; + key->keySigId = ID_NONE; ret = GetOpenSshKey(key, in, inSz, &idx); @@ -12068,23 +12052,6 @@ static int BuildUserAuthRequestPublicKey(WOLFSSH* ssh, } -static void CleanupUserAuthRequestPublicKey(WS_KeySignature* keySig) -{ - if (keySig != NULL) { - if (keySig->keySigId == ID_SSH_RSA) { -#ifndef WOLFSSH_NO_RSA - wc_FreeRsaKey(&keySig->ks.rsa.key); -#endif - } - else if (keySig->keySigId == ID_ECDSA_SHA2_NISTP256 || - keySig->keySigId == ID_ECDSA_SHA2_NISTP384 || - keySig->keySigId == ID_ECDSA_SHA2_NISTP521) { -#ifndef WOLFSSH_NO_ECDSA - wc_ecc_free(&keySig->ks.ecc.key); -#endif - } - } -} #endif @@ -12234,7 +12201,7 @@ int SendUserAuthRequest(WOLFSSH* ssh, byte authType, int addSig) } if (authId == ID_USERAUTH_PUBLICKEY) - CleanupUserAuthRequestPublicKey(keySig_ptr); + wolfSSH_KEY_clean(keySig_ptr); if (ret == WS_SUCCESS) { ret = wolfSSH_SendPacket(ssh); diff --git a/src/ssh.c b/src/ssh.c index 70b5d53d5..4e3758d72 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1824,6 +1824,10 @@ int wolfSSH_ReadKey_file(const char* name, format = WOLFSSH_FORMAT_SSH; in[inSz] = 0; } +#if 0 + else if (WSTRNSTR((const char*)in, PrivBeginOpenSSH, inSz) != NULL && + WSTRNSTR((const char*)in, PrivEndOpenSSH, inSz) != NULL) { +#endif else if (WSTRNSTR((const char*)in, PrivBeginOpenSSH, inSz) != NULL) { *isPrivate = 1; format = WOLFSSH_FORMAT_OPENSSH; diff --git a/wolfssh/error.h b/wolfssh/error.h index 10ed54d3e..598ec1a48 100644 --- a/wolfssh/error.h +++ b/wolfssh/error.h @@ -130,8 +130,9 @@ enum WS_ErrorCodes { WS_MATCH_UA_KEY_ID_E = -1089, /* Match user auth key key fail */ WS_KEY_AUTH_MAGIC_E = -1090, /* OpenSSH key auth magic check fail */ WS_KEY_CHECK_VAL_E = -1091, /* OpenSSH key check value fail */ + WS_KEY_FORMAT_E = -1092, /* OpenSSH key format fail */ - WS_LAST_E = -1091 /* Update this to indicate last error */ + WS_LAST_E = -1092 /* Update this to indicate last error */ }; diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 2c411c076..674f93de2 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -424,6 +424,9 @@ enum { #ifndef WOLFSSH_MAX_PUB_KEY_ALGO #define WOLFSSH_MAX_PUB_KEY_ALGO (WOLFSSH_MAX_PVT_KEYS + 2) #endif +#ifndef WOLFSSH_KEY_QUANTITY_REQ + #define WOLFSSH_KEY_QUANTITY_REQ 1 +#endif WOLFSSH_LOCAL byte NameToId(const char*, word32); WOLFSSH_LOCAL const char* IdToName(byte); From a60df27e51df9d2a5d85df102d809d31d8d58ac6 Mon Sep 17 00:00:00 2001 From: Fabio <507164+falemagn@users.noreply.github.com> Date: Fri, 17 Nov 2023 17:34:22 +0100 Subject: [PATCH 24/47] Added missing semicolons. --- src/wolfsftp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wolfsftp.c b/src/wolfsftp.c index a7d26fb99..a3e933e6f 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -5091,7 +5091,7 @@ int wolfSSH_SFTP_RecvLSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) /* Set the files mode * return WS_SUCCESS on success */ static int SFTP_SetMode(void* fs, char* name, word32 mode) { - WOLFSSH_UNUSED(fs) + WOLFSSH_UNUSED(fs); if (WCHMOD(fs, name, mode) != 0) { return WS_BAD_FILE_E; } @@ -5103,7 +5103,7 @@ static int SFTP_SetMode(void* fs, char* name, word32 mode) { /* Set the files mode * return WS_SUCCESS on success */ static int SFTP_SetModeHandle(void* fs, WFD handle, word32 mode) { - WOLFSSH_UNUSED(fs) + WOLFSSH_UNUSED(fs); if (WFCHMOD(fs, handle, mode) != 0) { return WS_BAD_FILE_E; } From 19a60a06c695c3c89b957035add1716ecbbd1b61 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Mon, 20 Nov 2023 10:43:34 -0800 Subject: [PATCH 25/47] add include of select.h --- wolfssh/test.h | 1 + 1 file changed, 1 insertion(+) diff --git a/wolfssh/test.h b/wolfssh/test.h index e584018d7..9b767f08c 100644 --- a/wolfssh/test.h +++ b/wolfssh/test.h @@ -176,6 +176,7 @@ #else /* USE_WINDOWS_API */ #include #include + #include #include #include #include From e16c247a4b9ccc5c54c676169b413fec95bed751 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 18 Oct 2023 17:04:43 -0700 Subject: [PATCH 26/47] Known Hosts 1. Added support to the wolfSSH client for a known hosts file. 2. In the client's common.c file, moved the functions ato32() and load_der_file() around so the known hosts callback can uses them. --- apps/wolfssh/common.c | 102 +++++++++++++++++++++++++++++++++++------- 1 file changed, 85 insertions(+), 17 deletions(-) diff --git a/apps/wolfssh/common.c b/apps/wolfssh/common.c index 7d0ccb44d..79eaf3c04 100644 --- a/apps/wolfssh/common.c +++ b/apps/wolfssh/common.c @@ -66,7 +66,11 @@ static const byte publicKeyType[] = "x509v3-ecdsa-sha2-nistp256"; #endif -#if defined(WOLFSSH_CERTS) +static inline void ato32(const byte* c, word32* u32) +{ + *u32 = (c[0] << 24) | (c[1] << 16) | (c[2] << 8) | c[3]; +} + static int load_der_file(const char* filename, byte** out, word32* outSz) { @@ -119,12 +123,9 @@ static int load_der_file(const char* filename, byte** out, word32* outSz) } -#if (defined(OPENSSL_ALL) || defined(WOLFSSL_IP_ALT_NAME)) -static inline void ato32(const byte* c, word32* u32) -{ - *u32 = (c[0] << 24) | (c[1] << 16) | (c[2] << 8) | c[3]; -} +#if defined(WOLFSSH_CERTS) +#if (defined(OPENSSL_ALL) || defined(WOLFSSL_IP_ALT_NAME)) /* when set as true then ignore miss matching IP addresses */ static int IPOverride = 0; @@ -186,20 +187,87 @@ void ClientIPOverride(int flag) #endif /* WOLFSSH_CERTS */ +static int AppendKeyToFile(const char* filename, const char* name, + const char* type, const char* key) +{ + FILE *f; + + f = fopen(filename, "a"); + fprintf(f, "%s %s %s\n", name, type, key); + fclose(f); + + return 0; +} + + int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) { - int ret = 0; + char *cursor; + char *line; + const char *targetName = (const char*)ctx; + char *name; + char *keyType; + char *key; + char *knownHosts = NULL; + char *knownHostsName = NULL; + int ret = 0, found = 0, badMatch = 0; + word32 sz; + char encodedKey[1200]; + char pubKeyType[54]; + + { + const char *defaultName = "/.ssh/known_hosts"; + char *env; + + env = getenv("HOME"); + sz = (word32)(WSTRLEN(env) + WSTRLEN(defaultName) + 1); + knownHostsName = (char*)WMALLOC(sz, NULL, 0); + if (knownHostsName != NULL) { + strcpy(knownHostsName, env); + strcat(knownHostsName, defaultName); + } + } - #ifdef DEBUG_WOLFSSH - printf("Sample public key check callback\n" - " public key = %p\n" - " public key size = %u\n" - " ctx = %s\n", pubKey, pubKeySz, (const char*)ctx); - #else - (void)pubKey; - (void)pubKeySz; - (void)ctx; - #endif + sz = 0; + ret = load_der_file(knownHostsName, (byte**)&knownHosts, &sz); + + /* Get the key type out of the key. */ + ato32(pubKey, &sz); + memcpy(pubKeyType, pubKey + LENGTH_SZ, sz); + pubKeyType[sz] = 0; + + sz = (word32)sizeof(encodedKey); + ret = Base64_Encode_NoNl(pubKey, pubKeySz, (byte*)encodedKey, &sz); + + cursor = knownHosts; + while (cursor) { + line = strsep(&cursor, "\n"); + if (line != NULL && *line) { + name = strsep(&line, " "); + keyType = strsep(&line, " "); + key = strsep(&line, " "); + if (name && keyType && key) { + if (strcmp(targetName, name) == 0 + && strcmp(pubKeyType, keyType) == 0) { + if (strcmp(encodedKey, key) == 0) { + found = 1; + } + else { + badMatch = 1; + } + } + } + } + } + WFREE(knownHosts, NULL, 0); + + if (badMatch) { + ret = -1; + } + else if (!found) { + ret = AppendKeyToFile(knownHostsName, + targetName, pubKeyType, encodedKey); + } #ifdef WOLFSSH_CERTS #if defined(OPENSSL_ALL) || defined(WOLFSSL_IP_ALT_NAME) From 51f4f4890183dc85eb83d9116b6e81e386307655 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 23 Oct 2023 11:41:55 -0700 Subject: [PATCH 27/47] Known Hosts 1. Comment out the original known hosts check for another method. 2. Implement new key checking method. --- apps/wolfssh/common.c | 44 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/apps/wolfssh/common.c b/apps/wolfssh/common.c index 79eaf3c04..898d605a0 100644 --- a/apps/wolfssh/common.c +++ b/apps/wolfssh/common.c @@ -210,7 +210,7 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) char *key; char *knownHosts = NULL; char *knownHostsName = NULL; - int ret = 0, found = 0, badMatch = 0; + int ret = 0, found = 0, badMatch = 0, requestAdd = 0; word32 sz; char encodedKey[1200]; char pubKeyType[54]; @@ -247,6 +247,7 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) keyType = strsep(&line, " "); key = strsep(&line, " "); if (name && keyType && key) { +#if 0 if (strcmp(targetName, name) == 0 && strcmp(pubKeyType, keyType) == 0) { if (strcmp(encodedKey, key) == 0) { @@ -256,15 +257,56 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) badMatch = 1; } } + else { + if (strcmp(encodedKey, key) == 0) { + printf("Found key, but name is wrong.\n"); + } + } +#endif + int nameMatch, keyTypeMatch, keyMatch; + + nameMatch = strcmp(targetName , name) == 0; + keyTypeMatch = strcmp(pubKeyType, keyType) == 0; + keyMatch = strcmp(encodedKey, key) == 0; + + if (nameMatch) { + if (keyTypeMatch) { + if (keyMatch) { + found = 1; + printf("key found/matched\n"); + } + else { + /* report mismatch */ + badMatch = 1; + printf("key found/not matched\n"); + } + break; + } + } + else { + if (keyTypeMatch) { + if (keyMatch) { + /* report key used on different address */ + requestAdd = 1; + printf("key found/wrong name\n"); + } + } + } } } } WFREE(knownHosts, NULL, 0); if (badMatch) { + printf("rejecting\n"); + ret = -1; + } + else if (requestAdd) { + printf("requesting to add\n"); ret = -1; } else if (!found) { + printf("adding\n"); ret = AppendKeyToFile(knownHostsName, targetName, pubKeyType, encodedKey); } From efd7ed01da5fd840f83449fb2c3bbe5f01f9da8a Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 21 Nov 2023 16:29:11 -0800 Subject: [PATCH 28/47] Known Hosts 1. Update the logic for checking the key name and key against the known hosts file. 2. Key fingerprinting for user confirmation. 3. Add user confirmation of host key additions. 4. Remove old debugging for known hosts. --- apps/wolfssh/common.c | 120 ++++++++++++++++++++++++++++++------------ 1 file changed, 85 insertions(+), 35 deletions(-) diff --git a/apps/wolfssh/common.c b/apps/wolfssh/common.c index 898d605a0..613e58244 100644 --- a/apps/wolfssh/common.c +++ b/apps/wolfssh/common.c @@ -200,6 +200,45 @@ static int AppendKeyToFile(const char* filename, const char* name, } +static int FingerprintKey(const byte* pubKey, word32 pubKeySz, char* out) +{ + wc_Sha256 sha; + byte digest[WC_SHA256_DIGEST_SIZE]; + char fp[48] = { 0 }; + word32 fpSz = sizeof(fp); + + wc_InitSha256(&sha); + wc_Sha256Update(&sha, pubKey, pubKeySz); + wc_Sha256Final(&sha, digest); + + Base64_Encode_NoNl(digest, sizeof(digest), (byte*)fp, &fpSz); + + if (fp[fpSz] == '=') { + fp[fpSz] = 0; + } + + strcat(out, "SHA256:"); + strcat(out, fp); + + return 0; +} + + +static int GetConfirmation(void) +{ + int c, confirmed = 0; + + printf("Y/n: "); + c = getchar(); + + if (c == 'Y') { + confirmed = 1; + } + + return confirmed; +} + + int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) { char *cursor; @@ -210,10 +249,11 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) char *key; char *knownHosts = NULL; char *knownHostsName = NULL; - int ret = 0, found = 0, badMatch = 0, requestAdd = 0; - word32 sz; - char encodedKey[1200]; - char pubKeyType[54]; + int ret = 0, found = 0, badMatch = 0, otherMatch = 0; + word32 sz, lineCount = 0; + char encodedKey[1200] = { 0 }; + char pubKeyType[54] = { 0 }; + char fp[56] = { 0 }; { const char *defaultName = "/.ssh/known_hosts"; @@ -238,31 +278,17 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) sz = (word32)sizeof(encodedKey); ret = Base64_Encode_NoNl(pubKey, pubKeySz, (byte*)encodedKey, &sz); + FingerprintKey(pubKey, pubKeySz, fp); cursor = knownHosts; while (cursor) { + lineCount++; line = strsep(&cursor, "\n"); if (line != NULL && *line) { name = strsep(&line, " "); keyType = strsep(&line, " "); key = strsep(&line, " "); if (name && keyType && key) { -#if 0 - if (strcmp(targetName, name) == 0 - && strcmp(pubKeyType, keyType) == 0) { - if (strcmp(encodedKey, key) == 0) { - found = 1; - } - else { - badMatch = 1; - } - } - else { - if (strcmp(encodedKey, key) == 0) { - printf("Found key, but name is wrong.\n"); - } - } -#endif int nameMatch, keyTypeMatch, keyMatch; nameMatch = strcmp(targetName , name) == 0; @@ -273,22 +299,24 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) if (keyTypeMatch) { if (keyMatch) { found = 1; - printf("key found/matched\n"); } else { - /* report mismatch */ badMatch = 1; - printf("key found/not matched\n"); } break; } } else { - if (keyTypeMatch) { - if (keyMatch) { - /* report key used on different address */ - requestAdd = 1; - printf("key found/wrong name\n"); + if (keyTypeMatch && keyMatch) { + /* report key used on different address */ + if (!otherMatch) { + printf("Key fingerprint is %s.\n", fp); + printf("This key matches other servers:\n"); + otherMatch = 1; + } + if (otherMatch) { + printf("\t%s:%u: %s\n", + knownHostsName, lineCount, name); } } } @@ -298,17 +326,39 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) WFREE(knownHosts, NULL, 0); if (badMatch) { - printf("rejecting\n"); + printf("That server is known, but that key is not.\n"); + printf("Rejecting connection and closing.\n"); ret = -1; } - else if (requestAdd) { - printf("requesting to add\n"); - ret = -1; + else if (otherMatch) { + if (!found) { + printf("Does this key look familiar to you?\n"); + printf("Fingerprint: %s\n", fp); + printf("Shall I add it to the known hosts?\n"); + /* Query. */ + if (GetConfirmation()) { + ret = AppendKeyToFile(knownHostsName, + targetName, pubKeyType, encodedKey); + } + else { + ret = -1; + } + } + else { + printf("This key matched multiple servers including %s.\n", + targetName); + printf("Attempting to connect.\n"); + } } else if (!found) { - printf("adding\n"); - ret = AppendKeyToFile(knownHostsName, - targetName, pubKeyType, encodedKey); + printf("The server is unknown and the key is unknown.\n"); + printf("Fingerprint: %s\n", fp); + printf("Shall I add it to the known hosts?\n"); + /* Query. */ + if (GetConfirmation()) { + ret = AppendKeyToFile(knownHostsName, + targetName, pubKeyType, encodedKey); + } } #ifdef WOLFSSH_CERTS From 0074ead857fd6382704efe0f79cbc2aa8cd180c2 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 21 Nov 2023 16:47:21 -0800 Subject: [PATCH 29/47] Known Hosts 1. Add porting layer wrapper macros for strcat, strcpy, and strsep. 2. Use port layer macros for the known hosts additions. --- apps/wolfssh/common.c | 22 +++++++++++----------- wolfssh/port.h | 3 +++ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/apps/wolfssh/common.c b/apps/wolfssh/common.c index 613e58244..233900fea 100644 --- a/apps/wolfssh/common.c +++ b/apps/wolfssh/common.c @@ -263,8 +263,8 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) sz = (word32)(WSTRLEN(env) + WSTRLEN(defaultName) + 1); knownHostsName = (char*)WMALLOC(sz, NULL, 0); if (knownHostsName != NULL) { - strcpy(knownHostsName, env); - strcat(knownHostsName, defaultName); + WSTRCPY(knownHostsName, env); + WSTRCAT(knownHostsName, defaultName); } } @@ -273,7 +273,7 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) /* Get the key type out of the key. */ ato32(pubKey, &sz); - memcpy(pubKeyType, pubKey + LENGTH_SZ, sz); + WMEMCPY(pubKeyType, pubKey + LENGTH_SZ, sz); pubKeyType[sz] = 0; sz = (word32)sizeof(encodedKey); @@ -283,17 +283,17 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) cursor = knownHosts; while (cursor) { lineCount++; - line = strsep(&cursor, "\n"); + line = WSTRSEP(&cursor, "\n"); if (line != NULL && *line) { - name = strsep(&line, " "); - keyType = strsep(&line, " "); - key = strsep(&line, " "); + name = WSTRSEP(&line, " "); + keyType = WSTRSEP(&line, " "); + key = WSTRSEP(&line, " "); if (name && keyType && key) { int nameMatch, keyTypeMatch, keyMatch; - nameMatch = strcmp(targetName , name) == 0; - keyTypeMatch = strcmp(pubKeyType, keyType) == 0; - keyMatch = strcmp(encodedKey, key) == 0; + nameMatch = WSTRCMP(targetName , name) == 0; + keyTypeMatch = WSTRCMP(pubKeyType, keyType) == 0; + keyMatch = WSTRCMP(encodedKey, key) == 0; if (nameMatch) { if (keyTypeMatch) { @@ -474,7 +474,7 @@ int ClientUserAuth(byte authType, if (defaultPassword != NULL) { passwordSz = (word32)strlen(defaultPassword); - memcpy(userPassword, defaultPassword, passwordSz); + WMEMCPY(userPassword, defaultPassword, passwordSz); } else { printf("Password: "); diff --git a/wolfssh/port.h b/wolfssh/port.h index a0bfbc282..189109c63 100644 --- a/wolfssh/port.h +++ b/wolfssh/port.h @@ -485,6 +485,9 @@ extern "C" { #define WSTRNCMP(s1,s2,n) strncmp((s1),(s2),(n)) #define WSTRSPN(s1,s2) strspn((s1),(s2)) #define WSTRCSPN(s1,s2) strcspn((s1),(s2)) + #define WSTRSEP(s,d) strsep((s),(d)) + #define WSTRCAT(s1,s2) strcat((s1),(s2)) + #define WSTRCPY(s1,s2) strcpy((s1),(s2)) /* for these string functions use internal versions */ WOLFSSH_API char* wstrnstr(const char*, const char*, unsigned int); From 205cec1f3c8d7bcd57507c4440cab1f6eb2e6d34 Mon Sep 17 00:00:00 2001 From: Fabio <507164+falemagn@users.noreply.github.com> Date: Wed, 22 Nov 2023 13:33:29 +0100 Subject: [PATCH 30/47] Update the buffer's index in DoReceive() any time the buffer's index is updated in DoPacket(). Fixes #621 --- src/internal.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/internal.c b/src/internal.c index 2b722f7d8..99bbd9762 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7456,7 +7456,7 @@ static int DoChannelExtendedData(WOLFSSH* ssh, } -static int DoPacket(WOLFSSH* ssh) +static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed) { byte* buf = (byte*)ssh->inputBuffer.buffer; word32 idx = ssh->inputBuffer.idx; @@ -7469,6 +7469,8 @@ static int DoPacket(WOLFSSH* ssh) WLOG(WS_LOG_DEBUG, "DoPacket sequence number: %d", ssh->peerSeq); + *bufferConsumed = 0; + idx += UINT32_SZ; padSz = buf[idx++]; @@ -7696,6 +7698,7 @@ static int DoPacket(WOLFSSH* ssh) idx += padSz; ssh->inputBuffer.idx = idx; ssh->peerSeq++; + *bufferConsumed = 1; return ret; } @@ -8084,6 +8087,7 @@ int DoReceive(WOLFSSH* ssh) byte peerBlockSz = ssh->peerBlockSz; byte peerMacSz = ssh->peerMacSz; byte aeadMode = ssh->peerAeadMode; + byte bufferConsumed = 0; switch (ssh->processReplyState) { case PROCESS_INIT: @@ -8190,15 +8194,13 @@ int DoReceive(WOLFSSH* ssh) NO_BREAK; case PROCESS_PACKET: - ret = DoPacket(ssh); + ret = DoPacket(ssh, &bufferConsumed); ssh->error = ret; if (ret < 0 && !(ret == WS_CHAN_RXD || ret == WS_EXTDATA || ret == WS_CHANNEL_CLOSED || ret == WS_WANT_WRITE || ret == WS_REKEYING || ret == WS_WANT_READ)) { - return WS_FATAL_ERROR; + ret = WS_FATAL_ERROR; } - WLOG(WS_LOG_DEBUG, "PR3: peerMacSz = %u", peerMacSz); - ssh->inputBuffer.idx += peerMacSz; break; default: @@ -8206,9 +8208,15 @@ int DoReceive(WOLFSSH* ssh) ssh->error = WS_INPUT_CASE_E; return WS_FATAL_ERROR; } - WLOG(WS_LOG_DEBUG, "PR4: Shrinking input buffer"); - ShrinkBuffer(&ssh->inputBuffer, 1); - ssh->processReplyState = PROCESS_INIT; + + if (bufferConsumed) { + WLOG(WS_LOG_DEBUG, "PR3: peerMacSz = %u", peerMacSz); + ssh->inputBuffer.idx += peerMacSz; + + WLOG(WS_LOG_DEBUG, "PR4: Shrinking input buffer"); + ShrinkBuffer(&ssh->inputBuffer, 1); + ssh->processReplyState = PROCESS_INIT; + } WLOG(WS_LOG_DEBUG, "PR5: txCount = %u, rxCount = %u", ssh->txCount, ssh->rxCount); From 8bcb466f92d4ae7dd5d0d696fbf9ba1e46aa2182 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 22 Nov 2023 16:46:31 -0800 Subject: [PATCH 31/47] Known Hosts: PR Review 1. Fix a few stdC function calls to use the porting wrappers. 2. Missing some error checking, added it. 3. Add some bounds checking to the sz usage when checking for the server's key in the known hosts file. 4. Move the temp buffers from the stack to the heap. 5. Make sure to nul terminate the read known_hosts file. --- apps/wolfssh/common.c | 166 ++++++++++++++++++++++++++++-------------- 1 file changed, 113 insertions(+), 53 deletions(-) diff --git a/apps/wolfssh/common.c b/apps/wolfssh/common.c index 233900fea..881deb296 100644 --- a/apps/wolfssh/common.c +++ b/apps/wolfssh/common.c @@ -190,13 +190,16 @@ void ClientIPOverride(int flag) static int AppendKeyToFile(const char* filename, const char* name, const char* type, const char* key) { - FILE *f; + WFILE *f; + int ret; - f = fopen(filename, "a"); - fprintf(f, "%s %s %s\n", name, type, key); - fclose(f); + ret = WFOPEN(NULL, &f, filename, "a"); + if (ret == 0 && f != WBADFILE) { + fprintf(f, "%s %s %s\n", name, type, key); + WFCLOSE(NULL, f); + } - return 0; + return ret; } @@ -206,21 +209,27 @@ static int FingerprintKey(const byte* pubKey, word32 pubKeySz, char* out) byte digest[WC_SHA256_DIGEST_SIZE]; char fp[48] = { 0 }; word32 fpSz = sizeof(fp); + int ret; - wc_InitSha256(&sha); - wc_Sha256Update(&sha, pubKey, pubKeySz); - wc_Sha256Final(&sha, digest); + ret = wc_InitSha256(&sha); + if (ret == 0) + ret = wc_Sha256Update(&sha, pubKey, pubKeySz); + if (ret == 0) + ret = wc_Sha256Final(&sha, digest); - Base64_Encode_NoNl(digest, sizeof(digest), (byte*)fp, &fpSz); + if (ret == 0) + ret = Base64_Encode_NoNl(digest, sizeof(digest), (byte*)fp, &fpSz); - if (fp[fpSz] == '=') { - fp[fpSz] = 0; - } + if (ret == 0) { + if (fp[fpSz] == '=') { + fp[fpSz] = 0; + } - strcat(out, "SHA256:"); - strcat(out, fp); + WSTRCAT(out, "SHA256:"); + WSTRCAT(out, fp); + } - return 0; + return ret; } @@ -239,6 +248,10 @@ static int GetConfirmation(void) } +#define WOLFSSH_CLIENT_ENCKEY_SIZE_ESTIMATE 1200 +#define WOLFSSH_CLIENT_PUBKEYTYPE_SIZE_ESTIMATE 54 +#define WOLFSSH_CLIENT_FINGERPRINT_SIZE_ESTIMATE 56 + int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) { char *cursor; @@ -249,11 +262,11 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) char *key; char *knownHosts = NULL; char *knownHostsName = NULL; + char *encodedKey = NULL; + char *pubKeyType = NULL; + char *fp = NULL; int ret = 0, found = 0, badMatch = 0, otherMatch = 0; word32 sz, lineCount = 0; - char encodedKey[1200] = { 0 }; - char pubKeyType[54] = { 0 }; - char fp[56] = { 0 }; { const char *defaultName = "/.ssh/known_hosts"; @@ -270,17 +283,58 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) sz = 0; ret = load_der_file(knownHostsName, (byte**)&knownHosts, &sz); + /* load_der_file() loads exactly what's in the file. Since it is + * NL terminated lines of known host data, and the last line ends + * in a NL, overwrite that with a nul to terminate the new string. */ + knownHosts[sz - 1] = 0; + + if (ret == 0) { + if (sz < sizeof(word32)) { + /* This file is too small. There must be at least a word32 + * length size. */ + ret = -1; + } + } + + if (ret == 0) { + encodedKey = (char*)WMALLOC(WOLFSSH_CLIENT_ENCKEY_SIZE_ESTIMATE + + WOLFSSH_CLIENT_PUBKEYTYPE_SIZE_ESTIMATE + + WOLFSSH_CLIENT_FINGERPRINT_SIZE_ESTIMATE, NULL, 0); + if (encodedKey == NULL) { + ret = -1; + } + } + + if (ret == 0) { + word32 keySz; + + pubKeyType = encodedKey + WOLFSSH_CLIENT_ENCKEY_SIZE_ESTIMATE; + fp = pubKeyType + WOLFSSH_CLIENT_PUBKEYTYPE_SIZE_ESTIMATE; + + encodedKey[0] = 0; + pubKeyType[0] = 0; + fp[0] = 0; + + /* Get the key type out of the key. */ + ato32(pubKey, &keySz); + if (keySz > sz - sizeof(word32)) { + ret = -1; + } + } - /* Get the key type out of the key. */ - ato32(pubKey, &sz); - WMEMCPY(pubKeyType, pubKey + LENGTH_SZ, sz); - pubKeyType[sz] = 0; + if (ret == 0) { + WMEMCPY(pubKeyType, pubKey + LENGTH_SZ, sz); + pubKeyType[sz] = 0; - sz = (word32)sizeof(encodedKey); - ret = Base64_Encode_NoNl(pubKey, pubKeySz, (byte*)encodedKey, &sz); - FingerprintKey(pubKey, pubKeySz, fp); + sz = WOLFSSH_CLIENT_ENCKEY_SIZE_ESTIMATE; + ret = Base64_Encode_NoNl(pubKey, pubKeySz, (byte*)encodedKey, &sz); + } + + if (ret == 0) + ret = FingerprintKey(pubKey, pubKeySz, fp); + + cursor = (ret == 0) ? knownHosts : NULL; - cursor = knownHosts; while (cursor) { lineCount++; line = WSTRSEP(&cursor, "\n"); @@ -323,16 +377,35 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) } } } - WFREE(knownHosts, NULL, 0); - if (badMatch) { - printf("That server is known, but that key is not.\n"); - printf("Rejecting connection and closing.\n"); - ret = -1; - } - else if (otherMatch) { - if (!found) { - printf("Does this key look familiar to you?\n"); + if (ret == 0) { + if (badMatch) { + printf("That server is known, but that key is not.\n"); + printf("Rejecting connection and closing.\n"); + ret = -1; + } + else if (otherMatch) { + if (!found) { + printf("Does this key look familiar to you?\n"); + printf("Fingerprint: %s\n", fp); + printf("Shall I add it to the known hosts?\n"); + /* Query. */ + if (GetConfirmation()) { + ret = AppendKeyToFile(knownHostsName, + targetName, pubKeyType, encodedKey); + } + else { + ret = -1; + } + } + else { + printf("This key matched multiple servers including %s.\n", + targetName); + printf("Attempting to connect.\n"); + } + } + else if (!found) { + printf("The server is unknown and the key is unknown.\n"); printf("Fingerprint: %s\n", fp); printf("Shall I add it to the known hosts?\n"); /* Query. */ @@ -340,24 +413,6 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) ret = AppendKeyToFile(knownHostsName, targetName, pubKeyType, encodedKey); } - else { - ret = -1; - } - } - else { - printf("This key matched multiple servers including %s.\n", - targetName); - printf("Attempting to connect.\n"); - } - } - else if (!found) { - printf("The server is unknown and the key is unknown.\n"); - printf("Fingerprint: %s\n", fp); - printf("Shall I add it to the known hosts?\n"); - /* Query. */ - if (GetConfirmation()) { - ret = AppendKeyToFile(knownHostsName, - targetName, pubKeyType, encodedKey); } } @@ -420,6 +475,11 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) #endif #endif + if (encodedKey) + WFREE(encodedKey, NULL, 0); + if (knownHosts) + WFREE(knownHosts, NULL, 0); + return ret; } From 82323db6f87ad896b886bf2c4e2ef366ca78bc4b Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Tue, 28 Nov 2023 08:01:05 -0700 Subject: [PATCH 32/47] cancel alarm timer after connected --- apps/wolfsshd/wolfsshd.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index 003d112be..bf6164bd9 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -1509,6 +1509,14 @@ static void* HandleConnection(void* arg) error = WS_FATAL_ERROR; } + if (graceTime > 0) { + #ifdef WIN32 + /* @TODO SetTimer(NULL, NULL, graceTime, alarmCatch); */ + #else + alarm(0); /* cancel any alarm */ + #endif + } + if (ret != WS_SUCCESS && ret != WS_SFTP_COMPLETE && ret != WS_SCP_INIT) { wolfSSH_Log(WS_LOG_ERROR, From 9a33bed122c5acd6e8ecff49aa8398280e6c8eab Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Tue, 28 Nov 2023 10:04:11 -0700 Subject: [PATCH 33/47] add regression test for grace period login --- apps/wolfsshd/test/run_all_sshd_tests.sh | 1 + apps/wolfsshd/test/sshd_login_grace_test.sh | 61 +++++++++++++++++++++ apps/wolfsshd/wolfsshd.c | 4 +- 3 files changed, 65 insertions(+), 1 deletion(-) create mode 100755 apps/wolfsshd/test/sshd_login_grace_test.sh diff --git a/apps/wolfsshd/test/run_all_sshd_tests.sh b/apps/wolfsshd/test/run_all_sshd_tests.sh index 638391d48..e0b33181c 100755 --- a/apps/wolfsshd/test/run_all_sshd_tests.sh +++ b/apps/wolfsshd/test/run_all_sshd_tests.sh @@ -63,6 +63,7 @@ fi # these tests require setting up an sshd if [ "$USING_LOCAL_HOST" == 1 ]; then run_test "sshd_forcedcmd_test.sh" + run_test "sshd_login_grace_test.sh" else printf "Skipping tests that need to setup local SSHD\n" SKIPPED=$((SKIPPED+1)) diff --git a/apps/wolfsshd/test/sshd_login_grace_test.sh b/apps/wolfsshd/test/sshd_login_grace_test.sh new file mode 100755 index 000000000..f273ad3e1 --- /dev/null +++ b/apps/wolfsshd/test/sshd_login_grace_test.sh @@ -0,0 +1,61 @@ +#!/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` +USER=`whoami` +TEST_PORT="$2" +TEST_HOST="$1" + +sudo rm log.txt +touch log.txt + +source ./start_sshd.sh +cat < sshd_config_test_login_grace +Port $TEST_PORT +Protocol 2 +LoginGraceTime 5 +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_login_grace" +cd ../../.. + +TEST_CLIENT="./examples/client/client" +PRIVATE_KEY="./keys/hansel-key-ecc.der" +PUBLIC_KEY="./keys/hansel-key-ecc.pub" + +RESULT=`$TEST_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h $TEST_HOST -p $TEST_PORT -c 'sleep 6 && echo still connected && exit'` +echo $RESULT | grep "still connected" +RESULT=$? +if [ "$RESULT" != 0 ]; then + echo "Connection was not held open" + exit -1 +fi + +# test grace login timeout by stalling on password prompt +timeout 7 $TEST_CLIENT -u $USER -h $TEST_HOST -p $TEST_PORT -t +cat ./log.txt | grep "Failed login within grace period" +RESULT=$? +if [ "$RESULT" != 0 ]; then + echo "Grace period not hit" + exit -1 +fi + +cd $PWD +#stop_wolfsshd +exit 0 + + diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index bf6164bd9..38ac2fb57 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -2046,7 +2046,9 @@ static int StartSSHD(int argc, char** argv) #ifdef __unix__ /* Daemonizing in POSIX, so set a syslog based log */ - wolfSSH_SetLoggingCb(SyslogCb); + if (logFile == stderr) { + wolfSSH_SetLoggingCb(SyslogCb); + } #endif p = fork(); if (p < 0) { From c6bc8992e67b824d942205c0b86a6d594a95771b Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Tue, 28 Nov 2023 10:15:38 -0700 Subject: [PATCH 34/47] add sshd test run --- .github/workflows/sshd-test.yml | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 .github/workflows/sshd-test.yml diff --git a/.github/workflows/sshd-test.yml b/.github/workflows/sshd-test.yml new file mode 100644 index 000000000..42287a66f --- /dev/null +++ b/.github/workflows/sshd-test.yml @@ -0,0 +1,32 @@ +name: wolfSSHd Test + +on: + push: + branches: [ '*' ] + pull_request: + branches: [ '*' ] + +jobs: + build: + + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v2 + with: + repository: wolfSSL/wolfssl.git + ref: master + - name: build wolfSSL + run: ./autogen.sh && ./configure --enable-all --prefix=/usr && make && sudo make install + - uses: actions/checkout@v2 + - name: autogen + run: ./autogen.sh + - name: configure + run: ./configure --enable-all CPPFLAGS=-DWOLFSSH_NO_FPKI + - name: make + run: make + - name: make check + run: make check + - name: run wolfSSHd tests + run: sudo ./run_all_sshd_tests.sh + working-directory: ./apps/wolfsshd/test From 7569d45b699caac47f1edf3bf0b0fa1c11ed3cf4 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Tue, 28 Nov 2023 13:22:18 -0700 Subject: [PATCH 35/47] remove debugging comment in test --- apps/wolfsshd/test/sshd_login_grace_test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/wolfsshd/test/sshd_login_grace_test.sh b/apps/wolfsshd/test/sshd_login_grace_test.sh index f273ad3e1..d2f559fb6 100755 --- a/apps/wolfsshd/test/sshd_login_grace_test.sh +++ b/apps/wolfsshd/test/sshd_login_grace_test.sh @@ -55,7 +55,7 @@ if [ "$RESULT" != 0 ]; then fi cd $PWD -#stop_wolfsshd +stop_wolfsshd exit 0 From 979801a05dd0d1b4f9bc5da022b80735ac77a335 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 30 Nov 2023 13:12:33 -0800 Subject: [PATCH 36/47] Known Hosts Update 1. Move setting a nul termination on the knownHosts data until after checking the size is reasonable. 2. A temporary keySz variable was getting used to get the length of the key type value, but it wasn't used to copy the value. Deleted it and used the other sz value. 3. Fix the leaking of the known hosts filename. --- apps/wolfssh/common.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/apps/wolfssh/common.c b/apps/wolfssh/common.c index 881deb296..28376cf7c 100644 --- a/apps/wolfssh/common.c +++ b/apps/wolfssh/common.c @@ -283,11 +283,6 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) sz = 0; ret = load_der_file(knownHostsName, (byte**)&knownHosts, &sz); - /* load_der_file() loads exactly what's in the file. Since it is - * NL terminated lines of known host data, and the last line ends - * in a NL, overwrite that with a nul to terminate the new string. */ - knownHosts[sz - 1] = 0; - if (ret == 0) { if (sz < sizeof(word32)) { /* This file is too small. There must be at least a word32 @@ -297,6 +292,11 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) } if (ret == 0) { + /* load_der_file() loads exactly what's in the file. Since it is + * NL terminated lines of known host data, and the last line ends + * in a NL, overwrite that with a nul to terminate the new string. */ + knownHosts[sz - 1] = 0; + encodedKey = (char*)WMALLOC(WOLFSSH_CLIENT_ENCKEY_SIZE_ESTIMATE + WOLFSSH_CLIENT_PUBKEYTYPE_SIZE_ESTIMATE + WOLFSSH_CLIENT_FINGERPRINT_SIZE_ESTIMATE, NULL, 0); @@ -306,8 +306,6 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) } if (ret == 0) { - word32 keySz; - pubKeyType = encodedKey + WOLFSSH_CLIENT_ENCKEY_SIZE_ESTIMATE; fp = pubKeyType + WOLFSSH_CLIENT_PUBKEYTYPE_SIZE_ESTIMATE; @@ -316,8 +314,9 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) fp[0] = 0; /* Get the key type out of the key. */ - ato32(pubKey, &keySz); - if (keySz > sz - sizeof(word32)) { + ato32(pubKey, &sz); + if ((sz > pubKeySz - sizeof(word32)) + || (sz > WOLFSSH_CLIENT_PUBKEYTYPE_SIZE_ESTIMATE - 1)) { ret = -1; } } @@ -479,6 +478,8 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) WFREE(encodedKey, NULL, 0); if (knownHosts) WFREE(knownHosts, NULL, 0); + if (knownHostsName) + WFREE(knownHostsName, NULL, 0); return ret; } From 6d22e040d7680859899880aa3502db2866c6a8e1 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Thu, 30 Nov 2023 15:40:29 -0700 Subject: [PATCH 37/47] update to test case --- apps/wolfsshd/test/sshd_login_grace_test.sh | 23 ++++++++++++--------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/apps/wolfsshd/test/sshd_login_grace_test.sh b/apps/wolfsshd/test/sshd_login_grace_test.sh index d2f559fb6..9e498d8cd 100755 --- a/apps/wolfsshd/test/sshd_login_grace_test.sh +++ b/apps/wolfsshd/test/sshd_login_grace_test.sh @@ -5,7 +5,7 @@ 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 + exit 1 fi PWD=`pwd` @@ -13,7 +13,9 @@ USER=`whoami` TEST_PORT="$2" TEST_HOST="$1" -sudo rm log.txt +if [ -f ./log.txt ]; then + sudo rm -rf log.txt +fi touch log.txt source ./start_sshd.sh @@ -31,30 +33,31 @@ AuthorizedKeysFile $PWD/authorized_keys_test EOF start_wolfsshd "sshd_config_test_login_grace" -cd ../../.. +pushd ../../.. TEST_CLIENT="./examples/client/client" PRIVATE_KEY="./keys/hansel-key-ecc.der" PUBLIC_KEY="./keys/hansel-key-ecc.pub" RESULT=`$TEST_CLIENT -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h $TEST_HOST -p $TEST_PORT -c 'sleep 6 && echo still connected && exit'` -echo $RESULT | grep "still connected" +echo "$RESULT" | grep "still connected" RESULT=$? if [ "$RESULT" != 0 ]; then - echo "Connection was not held open" - exit -1 + echo "FAIL: Connection was not held open" + exit 1 fi # test grace login timeout by stalling on password prompt -timeout 7 $TEST_CLIENT -u $USER -h $TEST_HOST -p $TEST_PORT -t +timeout 7 "$TEST_CLIENT" -u "$USER" -h "$TEST_HOST" -p "$TEST_PORT" -t + +popd cat ./log.txt | grep "Failed login within grace period" RESULT=$? if [ "$RESULT" != 0 ]; then - echo "Grace period not hit" - exit -1 + echo "FAIL: Grace period not hit" + exit 1 fi -cd $PWD stop_wolfsshd exit 0 From 72888da82b25fd6507bcd75b4d89e8bccba80439 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 30 Nov 2023 14:35:04 -0800 Subject: [PATCH 38/47] Unused Heap 1. Moved the heap variable declaration to the top of the function ChannelUpdateForward(). 2. Only set heap if channel is non-null. 3. Tag heap as unused. Depending on the build environment, it might be left out by the preprocessor. 4. Fix a bad memory type name used in a malloc in the agent. 5. Fix a use of heap when ctx->heap was intended. 6. Fix a typo where ssh->ctx->heap was intended. --- src/agent.c | 2 +- src/internal.c | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/agent.c b/src/agent.c index 9706706f0..f72b7185e 100644 --- a/src/agent.c +++ b/src/agent.c @@ -1553,7 +1553,7 @@ void wolfSSH_AGENT_ID_free(WOLFSSH_AGENT_ID* id, void* heap) WFREE(id->keyBuffer, heap, DYNTYPE_STRING); } WMEMSET(id, 0, sizeof(WOLFSSH_AGENT_ID)); - WFREE(id, heap, DYNATYPE_AGENT_ID); + WFREE(id, heap, DYNTYPE_AGENT_ID); } WLOG(WS_LOG_AGENT, "Leaving wolfSSH_AGENT_ID_free()"); diff --git a/src/internal.c b/src/internal.c index 2b722f7d8..ccad259d4 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1516,7 +1516,7 @@ static int SetHostCertificate(WOLFSSH_CTX* ctx, if (pvtKey->publicKeyFmt == certId) { if (pvtKey->cert != NULL) { - WFREE(pvtKey->cert, heap, dynamicType); + WFREE(pvtKey->cert, ctx->heap, dynamicType); } } else { @@ -2210,17 +2210,19 @@ int ChannelUpdateForward(WOLFSSH_CHANNEL* channel, const char* origin, word32 originPort, int isDirect) { + void* heap = NULL; int ret = WS_SUCCESS; char* hostCopy = NULL; char* originCopy = NULL; word32 hostSz; word32 originSz; + WOLFSSH_UNUSED(heap); + if (channel == NULL || host == NULL || origin == NULL) ret = WS_BAD_ARGUMENT; else { - void* heap = channel->ssh->ctx->heap; - + heap = channel->ssh->ctx->heap; hostSz = (word32)WSTRLEN(host) + 1; originSz = (word32)WSTRLEN(origin) + 1; hostCopy = (char*)WMALLOC(hostSz, heap, DYNTYPE_STRING); @@ -5614,7 +5616,7 @@ static int DoUserAuthRequestRsa(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk, if (checkDigest) WFREE(checkDigest, ssh->ctx->heap, DYNTYPE_BUFFER); if (encDigest) - WFREE(encDigest, ssh->ctx_heap, DYNTYPE_BUFFER); + WFREE(encDigest, ssh->ctx->heap, DYNTYPE_BUFFER); #endif WLOG(WS_LOG_DEBUG, "Leaving DoUserAuthRequestRsa(), ret = %d", ret); return ret; From a5103bf885b7fb3beb1783749f34a8c7b4874869 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Wed, 29 Nov 2023 16:20:15 -0800 Subject: [PATCH 39/47] add terminal resize callback for unix like use --- apps/wolfsshd/wolfsshd.c | 18 ++++++++++++++++++ src/internal.c | 41 ++++++++++++++++++++++++++++++++++------ wolfssh/internal.h | 2 ++ 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index 003d112be..72aedb611 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -1148,6 +1148,9 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, return ret; } #else +#if defined(HAVE_SYS_IOCTL_H) + #include +#endif /* handles creating a new shell env. and maintains SSH connection for incoming * user input as well as output of the shell. @@ -1314,6 +1317,21 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, return WS_FATAL_ERROR; } + /* set initial size of terminal based on saved size */ +#if defined(HAVE_SYS_IOCTL_H) + { + struct winsize s; + + WMEMSET(&s, 0, sizeof s); + s.ws_col = ssh->curX; + s.ws_row = ssh->curY; + s.ws_xpixel = ssh->curXP; + s.ws_ypixel = ssh->curYP; + ioctl(childFd, TIOCSWINSZ, &s); + } +#endif + + wolfSSH_SetTerminalResizeCtx(ssh, (void*)&childFd); while (ChildRunning) { byte tmp[2]; fd_set readFds; diff --git a/src/internal.c b/src/internal.c index 2b722f7d8..27f308c85 100644 --- a/src/internal.c +++ b/src/internal.c @@ -645,8 +645,9 @@ void CtxResourceFree(WOLFSSH_CTX* ctx) #ifdef WOLFSSH_TERM /* default terminal resize handling callbacks */ -#if defined(USE_WINDOWS_API) && defined(WOLFSSH_SSHD) -static int WS_WindowsTermResize(WOLFSSH* ssh, word32 col, word32 row, word32 colP, +#if defined(WOLFSSH_SSHD) && !defined(WOLFSSH_RESIZE_NO_DEFUALT) +#if defined(USE_WINDOWS_API) +static int WS_TermResize(WOLFSSH* ssh, word32 col, word32 row, word32 colP, word32 rowP, void* usrCtx) { HPCON* term = (HPCON*)usrCtx; @@ -667,7 +668,33 @@ static int WS_WindowsTermResize(WOLFSSH* ssh, word32 col, word32 row, word32 col return ret; } +#elif defined(HAVE_SYS_IOCTL_H) + +#include +static int WS_TermResize(WOLFSSH* ssh, word32 col, word32 row, word32 colP, + word32 rowP, void* usrCtx) +{ + struct winsize s; + int ret = WS_SUCCESS; + int* fd = (int*)usrCtx; + + if (fd != NULL) { + WMEMSET(&s, 0, sizeof s); + s.ws_row = row; + s.ws_col = col; + s.ws_xpixel = colP; + s.ws_ypixel = rowP; + + ioctl(*fd, TIOCSWINSZ, &s); + } + + (void)ssh; + return ret; +} +#else + #define WOLFSSH_RESIZE_NO_DEFUALT #endif +#endif /* WOLFSSH_SSHD */ #endif /* WOLFSSH_TERM */ @@ -763,10 +790,10 @@ WOLFSSH* SshInit(WOLFSSH* ssh, WOLFSSH_CTX* ctx) ssh->agentEnabled = ctx->agentEnabled; #endif -#ifdef WOLFSSH_TERM - #if defined(USE_WINDOWS_API) && defined(WOLFSSH_SSHD) - ssh->termResizeCb = WS_WindowsTermResize; - #endif +#if defined(WOLFSSH_TERM) && defined(WOLFSSH_SSHD) +#ifndef WOLFSSH_RESIZE_NO_DEFUALT + ssh->termResizeCb = WS_TermResize; +#endif #endif if (BufferInit(&ssh->inputBuffer, 0, ctx->heap) != WS_SUCCESS || @@ -7169,6 +7196,8 @@ static int DoChannelRequest(WOLFSSH* ssh, WLOG(WS_LOG_DEBUG, " heightPixels = %u", heightPixels); ssh->curX = widthChar; ssh->curY = heightRows; + ssh->curXP = widthPixels; + ssh->curYP = heightPixels; if (ssh->termResizeCb) { if (ssh->termResizeCb(ssh, widthChar, heightRows, widthPixels, heightPixels, ssh->termCtx) diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 674f93de2..cb2a4dce3 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -815,6 +815,8 @@ struct WOLFSSH { void* termCtx; word32 curX; /* current terminal width */ word32 curY; /* current terminal height */ + word32 curXP; /* pixel width */ + word32 curYP; /* pixel height */ #endif }; From d004536aa73bf25c777d09128950139bb9832530 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 30 Nov 2023 16:53:09 -0800 Subject: [PATCH 40/47] Known Hosts Fix 1. Didn't take into account of getenv() returning NULL. Fixed. --- apps/wolfssh/common.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/apps/wolfssh/common.c b/apps/wolfssh/common.c index 28376cf7c..f83d135f7 100644 --- a/apps/wolfssh/common.c +++ b/apps/wolfssh/common.c @@ -273,16 +273,23 @@ int ClientPublicKeyCheck(const byte* pubKey, word32 pubKeySz, void* ctx) char *env; env = getenv("HOME"); - sz = (word32)(WSTRLEN(env) + WSTRLEN(defaultName) + 1); - knownHostsName = (char*)WMALLOC(sz, NULL, 0); - if (knownHostsName != NULL) { - WSTRCPY(knownHostsName, env); - WSTRCAT(knownHostsName, defaultName); + if (env != NULL) { + sz = (word32)(WSTRLEN(env) + WSTRLEN(defaultName) + 1); + knownHostsName = (char*)WMALLOC(sz, NULL, 0); + if (knownHostsName != NULL) { + WSTRCPY(knownHostsName, env); + WSTRCAT(knownHostsName, defaultName); + } } + else + ret = -1; + } + + if (ret == 0) { + sz = 0; + ret = load_der_file(knownHostsName, (byte**)&knownHosts, &sz); } - sz = 0; - ret = load_der_file(knownHostsName, (byte**)&knownHosts, &sz); if (ret == 0) { if (sz < sizeof(word32)) { /* This file is too small. There must be at least a word32 From 7e300b5109e446eb94b8f480a89391ec676d6ed0 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Fri, 1 Dec 2023 13:38:24 -0800 Subject: [PATCH 41/47] add terminal size test case --- apps/wolfsshd/test/run_all_sshd_tests.sh | 5 +- apps/wolfsshd/test/sshd_term_size_test.sh | 98 +++++++++++++++++++++++ 2 files changed, 101 insertions(+), 2 deletions(-) create mode 100755 apps/wolfsshd/test/sshd_term_size_test.sh diff --git a/apps/wolfsshd/test/run_all_sshd_tests.sh b/apps/wolfsshd/test/run_all_sshd_tests.sh index 638391d48..4c082d2cd 100755 --- a/apps/wolfsshd/test/run_all_sshd_tests.sh +++ b/apps/wolfsshd/test/run_all_sshd_tests.sh @@ -25,7 +25,7 @@ else start_wolfsshd "sshd_config_test" if [ -z "$PID" ]; then echo "Issue starting up wolfSSHd" - exit -1 + exit 1 fi fi @@ -46,11 +46,12 @@ run_test() { printf "Shutting down test wolfSSHd\n" stop_wolfsshd fi - exit -1 + exit 1 fi } run_test "sshd_exec_test.sh" +run_test "sshd_term_size_test.sh" # add aditional tests here, check on var USING_LOCAL_HOST if can make sshd # server start/restart with changes diff --git a/apps/wolfsshd/test/sshd_term_size_test.sh b/apps/wolfsshd/test/sshd_term_size_test.sh new file mode 100755 index 000000000..22d2f3a33 --- /dev/null +++ b/apps/wolfsshd/test/sshd_term_size_test.sh @@ -0,0 +1,98 @@ +#!/bin/bash + +# sshd local test + +pushd ../../.. + +TEST_CLIENT="./examples/client/client" +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 + +set -e +echo "Creating tmux session at $PWD with command :" +tmux new-session -d -s test "$TEST_CLIENT -t -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h \"$1\" -p \"$2\"" + +# give the command a second to establish SSH connection +sleep 0.5 + +COL=`tmux display -p -t test '#{pane_width}'` +ROW=`tmux display -p -t test '#{pane_height}'` + +# get the terminals columns and lines +tmux send-keys -t test 'echo col=$COLUMNS row=$LINES' +tmux send-keys -t test 'ENTER' +tmux capture-pane -t test +RESULT=`tmux show-buffer | grep -v echo | grep -v rejecting | grep "col="` + +echo "$RESULT" +echo "" +echo "" +ROW_FOUND=`echo "$RESULT" | sed -e 's/.*[^0-9]\([0-9]\+\)[^0-9]*$/\1/'` +COL_FOUND=`echo "$RESULT" | sed -r 's/^[^0-9]*([0-9]+).*$/\1/'` + +if [ "$COL" != "$COL_FOUND" ]; then + echo "Col found was $COL_FOUND which does not match expected $COL" + exit 1 +fi + +if [ "$ROW" != "$ROW_FOUND" ]; then + echo "Row found was $ROW_FOUND which does not match expected $ROW" + exit 1 +fi + +# resize tmux after connection is open is not working @TODO +#tmux set-window-option -g aggressive-resize +#printf '\e[8;50;100t' +#tmux resize-pane -x 50 -y 10 -t test + +# close down the SSH session +tmux send-keys -t test 'exit' +tmux send-keys -t test 'ENTER' +set +e + +# kill off the session if it's still running, but don't error out if the session +# has already closed down +tmux kill-session -t test +set -e + +tmux new-session -d -x 50 -y 10 -s test "$TEST_CLIENT -t -u $USER -i $PRIVATE_KEY -j $PUBLIC_KEY -h \"$1\" -p \"$2\"" + +# give the command a second to establish SSH connection +sleep 0.5 + +echo "New COL=$COL ROW=$ROW" + +tmux send-keys -t test 'echo col=$COLUMNS row=$LINES' +tmux send-keys -t test 'ENTER' +tmux capture-pane -t test +RESULT=`tmux show-buffer | grep -v echo | grep -v rejecting | grep "col="` + +ROW_FOUND=`echo "$RESULT" | sed -e 's/.*[^0-9]\([0-9]\+\)[^0-9]*$/\1/'` +COL_FOUND=`echo "$RESULT" | sed -r 's/^[^0-9]*([0-9]+).*$/\1/'` + +if [ "50" != "$COL_FOUND" ]; then + echo "Col found was $COL_FOUND which does not match expected 50" + exit 1 +fi + +if [ "10" != "$ROW_FOUND" ]; then + echo "Row found was $ROW_FOUND which does not match expected 10" + exit 1 +fi + +# close down the SSH session +tmux send-keys -t test 'exit' +tmux send-keys -t test 'ENTER' +set +e +tmux kill-session -t test + +popd +exit 0 + From 47ce821b33d71bd565d0cdd4cddc1cd5be2c33af Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Mon, 4 Dec 2023 00:13:39 -0700 Subject: [PATCH 42/47] avoid zombie processes --- apps/wolfsshd/wolfsshd.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index d1158f0dc..5a98e5cf4 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -1291,6 +1291,10 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, exit(0); /* exit child process and close down SSH connection */ } + /* do not wait for status of child process, and signal that the child can + * be reaped to avoid zombie processes when running in the foreground */ + signal(SIGCHLD, SIG_IGN); + if (wolfSSHD_AuthReducePermissionsUser(conn->auth, pPasswd->pw_uid, pPasswd->pw_gid) != WS_SUCCESS) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error setting user ID"); @@ -1730,6 +1734,11 @@ static int NewConnection(WOLFSSHD_CONNECTION* conn) exit(0); } else { + /* do not wait for status of child process, and signal that the + child can be reaped to avoid zombie processes when running in + the foreground */ + signal(SIGCHLD, SIG_IGN); + wolfSSH_Log(WS_LOG_INFO, "[SSHD] Spawned new process %d\n", pd); WCLOSESOCKET(conn->fd); } From 9cc9711dc8a6a5575003494e8995c8437c35bfe0 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Tue, 5 Dec 2023 10:39:52 -0700 Subject: [PATCH 43/47] add override for default sshd user --- apps/wolfsshd/auth.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index 136e75cd8..53722ee20 100644 --- a/apps/wolfsshd/auth.c +++ b/apps/wolfsshd/auth.c @@ -1363,6 +1363,12 @@ static int SetDefaultPublicKeyCheck(WOLFSSHD_AUTH* auth) return ret; } +#ifndef WOLFSSH_SSHD_USER + #define WOLFSSH_SSHD_USER sshd +#endif +#define WOLFSSH_USER_GET_STRING(x) #x +#define WOLFSSH_USER_STRING(x) WOLFSSH_USER_GET_STRING(x) + static int SetDefualtUserID(WOLFSSHD_AUTH* auth) { #ifdef _WIN32 @@ -1370,13 +1376,13 @@ static int SetDefualtUserID(WOLFSSHD_AUTH* auth) return 0; #else struct passwd* pwInfo; - const char* usr = "sshd"; int ret = WS_SUCCESS; - pwInfo = getpwnam(usr); + pwInfo = getpwnam(WOLFSSH_USER_STRING(WOLFSSH_SSHD_USER)); if (pwInfo == NULL) { /* user name not found on system */ - wolfSSH_Log(WS_LOG_INFO, "[SSHD] No sshd user found to use"); + wolfSSH_Log(WS_LOG_INFO, "[SSHD] No %s user found to use", + WOLFSSH_USER_STRING(WOLFSSH_SSHD_USER)); ret = WS_FATAL_ERROR; } From 7e17dc9d393f01a55b66e2668880efd8622df493 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 6 Dec 2023 12:08:48 -0800 Subject: [PATCH 44/47] Test Fixes 1. Remove the grace period wolfSSHd test. It asks for a password, and when running as an action it doesn't have stdin, so it fails. The test works when stdin is available. 2. Remove the check for __uint128_t from configure. wolfSSL exports this value, if present, in its options.h. After wolfSSL v5.6.4, wolfSSL also exports this to options.h when building it with CMake. The check in wolfSSH isn't necessary now. --- apps/wolfsshd/test/run_all_sshd_tests.sh | 1 - configure.ac | 1 - 2 files changed, 2 deletions(-) diff --git a/apps/wolfsshd/test/run_all_sshd_tests.sh b/apps/wolfsshd/test/run_all_sshd_tests.sh index 7e4fb3372..4c082d2cd 100755 --- a/apps/wolfsshd/test/run_all_sshd_tests.sh +++ b/apps/wolfsshd/test/run_all_sshd_tests.sh @@ -64,7 +64,6 @@ fi # these tests require setting up an sshd if [ "$USING_LOCAL_HOST" == 1 ]; then run_test "sshd_forcedcmd_test.sh" - run_test "sshd_login_grace_test.sh" else printf "Skipping tests that need to setup local SSHD\n" SKIPPED=$((SKIPPED+1)) diff --git a/configure.ac b/configure.ac index ffb0378c5..280451af5 100644 --- a/configure.ac +++ b/configure.ac @@ -50,7 +50,6 @@ AC_PROG_INSTALL AC_CHECK_SIZEOF([long long]) AC_CHECK_SIZEOF([long]) AC_CHECK_SIZEOF([off_t]) -AC_CHECK_TYPES([__uint128_t]) AC_TYPE_SIZE_T AC_TYPE_UINT8_T AC_TYPE_UINTPTR_T From 9d9267293124d766c70955ad0dc9aa04bea1e392 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 25 Oct 2023 16:55:45 -0700 Subject: [PATCH 45/47] SFTP Zero Byte Files 1. When getting a file with SFTP, the client should check that the requested file is a regular file based on its attributes. 2. Add the attributes to check in the permissions. 3. Add a new error for a non-regular file. --- examples/sftpclient/sftpclient.c | 6 ++++++ src/internal.c | 3 +++ src/wolfsftp.c | 8 ++++++++ wolfssh/error.h | 3 ++- wolfssh/wolfsftp.h | 7 +++++++ 5 files changed, 26 insertions(+), 1 deletion(-) diff --git a/examples/sftpclient/sftpclient.c b/examples/sftpclient/sftpclient.c index adbb5df53..7a3cb34b5 100644 --- a/examples/sftpclient/sftpclient.c +++ b/examples/sftpclient/sftpclient.c @@ -523,6 +523,12 @@ static int doCmds(func_args* args) #endif if (ret != WS_SUCCESS) { + if (wolfSSH_get_error(ssh) == WS_SFTP_NOT_FILE_E) { + if (SFTP_FPUTS(args, "Not a regular file\n") < 0) { + err_msg("fputs error"); + return -1; + } + } if (SFTP_FPUTS(args, "Error getting file\n") < 0) { err_msg("fputs error"); return -1; diff --git a/src/internal.c b/src/internal.c index ea6bce4d7..c68e136c0 100644 --- a/src/internal.c +++ b/src/internal.c @@ -430,6 +430,9 @@ const char* GetErrorString(int err) case WS_KEY_FORMAT_E: return "key format wrong error"; + case WS_SFTP_NOT_FILE_E: + return "not a regular file"; + default: return "Unknown error code"; } diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 5ac05dd05..82975c6dd 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -8481,6 +8481,14 @@ int wolfSSH_SFTP_Get(WOLFSSH* ssh, char* from, state->state = STATE_GET_CLEANUP; continue; } + if ((state->attrib.per & FILEATRB_PER_MASK_TYPE) + != FILEATRB_PER_FILE) { + WLOG(WS_LOG_SFTP, "Not a file"); + ssh->error = WS_SFTP_NOT_FILE_E; + ret = WS_FATAL_ERROR; + state->state = STATE_GET_CLEANUP; + continue; + } state->handleSz = WOLFSSH_MAX_HANDLE; state->state = STATE_GET_OPEN_REMOTE; NO_BREAK; diff --git a/wolfssh/error.h b/wolfssh/error.h index 598ec1a48..d1fdfdd27 100644 --- a/wolfssh/error.h +++ b/wolfssh/error.h @@ -131,8 +131,9 @@ enum WS_ErrorCodes { WS_KEY_AUTH_MAGIC_E = -1090, /* OpenSSH key auth magic check fail */ WS_KEY_CHECK_VAL_E = -1091, /* OpenSSH key check value fail */ WS_KEY_FORMAT_E = -1092, /* OpenSSH key format fail */ + WS_SFTP_NOT_FILE_E = -1093, /* Not a regular file */ - WS_LAST_E = -1092 /* Update this to indicate last error */ + WS_LAST_E = -1093 /* Update this to indicate last error */ }; diff --git a/wolfssh/wolfsftp.h b/wolfssh/wolfsftp.h index b0272b2d6..8fbbdbb77 100644 --- a/wolfssh/wolfsftp.h +++ b/wolfssh/wolfsftp.h @@ -126,6 +126,13 @@ struct WS_SFTP_FILEATRB_EX { WS_SFTP_FILEATRB_EX* next; }; +#define FILEATRB_PER_MASK_TYPE 0770000 +#define FILEATRB_PER_FILE 0100000 +#define FILEATRB_PER_DEV_CHAR 0020000 +#define FILEATRB_PER_DIR 0040000 +#define FILEATRB_PER_DEV_BLOCK 0060000 +#define FILEATRB_PER_MASK_PERM 0000777 + typedef struct WS_SFTP_FILEATRB { word32 flags; word32 sz[2]; /* sz[0] being the lower and sz[1] being the upper */ From f35cab9e866498ef1525c4b40984dcb17a75246d Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 5 Dec 2023 15:07:38 -0800 Subject: [PATCH 46/47] SFTP Zero Byte Files 1. When putting a file with SFTP, the client should check that the requested file is a regular file based on its attributes. 2. Add the attributes to check in the permissions. 3. Add server checking for non-regular files and not allowing them to be opened for reading or writing. --- examples/sftpclient/sftpclient.c | 6 ++++ src/wolfsftp.c | 58 ++++++++++++++++++++++++++------ 2 files changed, 53 insertions(+), 11 deletions(-) diff --git a/examples/sftpclient/sftpclient.c b/examples/sftpclient/sftpclient.c index 7a3cb34b5..9e1a7d4d7 100644 --- a/examples/sftpclient/sftpclient.c +++ b/examples/sftpclient/sftpclient.c @@ -634,6 +634,12 @@ static int doCmds(func_args* args) #endif if (ret != WS_SUCCESS) { + if (wolfSSH_get_error(ssh) == WS_SFTP_NOT_FILE_E) { + if (SFTP_FPUTS(args, "Not a regular file\n") < 0) { + err_msg("fputs error"); + return -1; + } + } if (SFTP_FPUTS(args, "Error pushing file\n") < 0) { err_msg("fputs error"); return -1; diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 82975c6dd..530b340c6 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -1977,6 +1977,7 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) char* res = NULL; char ier[] = "Internal Failure"; char oer[] = "Open File Error"; + char naf[] = "Not A File"; if (ssh == NULL) { return WS_BAD_ARGUMENT; @@ -2036,20 +2037,40 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) m |= WOLFSSH_O_EXCL; } - /* if file permissions not set then use default */ - if (!(atr.flags & WOLFSSH_FILEATRB_PERM)) { - atr.per = 0644; + { + WS_SFTP_FILEATRB fileAtr = { 0 }; + if (SFTP_GetAttributes(ssh->fs, + dir, &fileAtr, 1, ssh->ctx->heap) == WS_SUCCESS) { + if ((fileAtr.per & FILEATRB_PER_MASK_TYPE) != FILEATRB_PER_FILE) { + WLOG(WS_LOG_SFTP, "Not a file"); + ssh->error = WS_SFTP_NOT_FILE_E; + + res = naf; + if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, + res, "English", NULL, &outSz) != WS_SIZE_ONLY) { + return WS_FATAL_ERROR; + } + ret = WS_FATAL_ERROR; + } + } } - fd = WOPEN(ssh->fs, dir, m, atr.per); - if (fd < 0) { - WLOG(WS_LOG_SFTP, "Error opening file %s", dir); - res = oer; - if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res, - "English", NULL, &outSz) != WS_SIZE_ONLY) { - return WS_FATAL_ERROR; + if (ret == WS_SUCCESS) { + /* if file permissions not set then use default */ + if (!(atr.flags & WOLFSSH_FILEATRB_PERM)) { + atr.per = 0644; + } + + fd = WOPEN(ssh->fs, dir, m, atr.per); + if (fd < 0) { + WLOG(WS_LOG_SFTP, "Error opening file %s", dir); + res = oer; + if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res, + "English", NULL, &outSz) != WS_SIZE_ONLY) { + return WS_FATAL_ERROR; + } + ret = WS_BAD_FILE_E; } - ret = WS_BAD_FILE_E; } #ifdef WOLFSSH_STOREHANDLE @@ -8722,6 +8743,21 @@ int wolfSSH_SFTP_Put(WOLFSSH* ssh, char* from, char* to, byte resume, case STATE_PUT_OPEN_LOCAL: WLOG(WS_LOG_SFTP, "SFTP PUT STATE: OPEN LOCAL"); #ifndef USE_WINDOWS_API + { + WS_SFTP_FILEATRB fileAtr = { 0 }; + if (SFTP_GetAttributes(ssh->fs, + from, &fileAtr, 1, ssh->ctx->heap) + == WS_SUCCESS) { + if ((fileAtr.per & FILEATRB_PER_MASK_TYPE) + != FILEATRB_PER_FILE) { + WLOG(WS_LOG_SFTP, "Not a file"); + ssh->error = WS_SFTP_NOT_FILE_E; + ret = WS_FATAL_ERROR; + state->state = STATE_PUT_CLEANUP; + continue; + } + } + } ret = WFOPEN(ssh->fs, &state->fl, from, "rb"); if (ret != 0) { WLOG(WS_LOG_SFTP, "Unable to open input file"); From 59e7b371366ba3a206061c549060911cbdaca9ad Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 6 Dec 2023 10:41:18 -0800 Subject: [PATCH 47/47] Zephyr File Permissions When populating the file permission status for a file vs directory in the zephyr porting, use the new constants for the file permissions being a regular file or directory. Not (the wrong) bare numbers. --- src/wolfsftp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 530b340c6..3bc55c09e 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -4729,9 +4729,9 @@ static int PopulateAttributes(WS_SFTP_FILEATRB* atr, WSTAT_T* stats) atr->per = 0755; /* Mimic S_IFMT */ if (stats->type == FS_DIR_ENTRY_FILE) - atr->per |= 0040000; + atr->per |= FILEATRB_PER_FILE; else if (stats->type == FS_DIR_ENTRY_DIR) - atr->per |= 0100000; + atr->per |= FILEATRB_PER_DIR; else return WS_BAD_FILE_E;