From d9808cec31ad6827fc1c5991d67c5056f6689f45 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 5 Oct 2016 21:12:06 -0700 Subject: [PATCH 01/17] tweak the peerWindowSz updating and logging --- src/internal.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/internal.c b/src/internal.c index f0fca6c34..60ada4f72 100644 --- a/src/internal.c +++ b/src/internal.c @@ -465,6 +465,11 @@ int ChannelPutData(WOLFSSH_CHANNEL* channel, uint8_t* data, uint32_t dataSz) WMEMCPY(inBuf->buffer + inBuf->length, data, dataSz); inBuf->length += dataSz; + + WLOG(WS_LOG_INFO, " dataSz = %u", dataSz); + WLOG(WS_LOG_INFO, " windowSz = %u", channel->windowSz); + channel->windowSz -= dataSz; + WLOG(WS_LOG_INFO, " windowSz = %u", channel->windowSz); } else { return WS_RECV_OVERFLOW_E; @@ -2214,10 +2219,12 @@ static int DoChannelWindowAdjust(WOLFSSH* ssh, WLOG(WS_LOG_INFO, " bytesToAdd = %u", bytesToAdd); WLOG(WS_LOG_INFO, " peerWindowSz = %u", channel->peerWindowSz); - WLOG(WS_LOG_INFO, " update peerWindowSz = %u", - channel->peerWindowSz + bytesToAdd); channel->peerWindowSz += bytesToAdd; + + WLOG(WS_LOG_INFO, " update peerWindowSz = %u", + channel->peerWindowSz); + } } @@ -3642,9 +3649,8 @@ int SendChannelData(WOLFSSH* ssh, uint32_t peerChannel, WLOG(WS_LOG_INFO, " dataSz = %u", dataSz); WLOG(WS_LOG_INFO, " peerWindowSz = %u", channel->peerWindowSz); - WLOG(WS_LOG_INFO, " update peerWindowSz = %u", - channel->peerWindowSz - dataSz); channel->peerWindowSz -= dataSz; + WLOG(WS_LOG_INFO, " update peerWindowSz = %u", channel->peerWindowSz); } WLOG(WS_LOG_DEBUG, "Leaving SendChannelData(), ret = %d", ret); From a69b7fba2661ccd189c214c512336867b12b55db Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 5 Oct 2016 21:14:15 -0700 Subject: [PATCH 02/17] More Tweaks 1. Made default window size updatable at configure time. 2. Lowered the default highwater mark by 32k. 3. Removed a parameter name from a couple function protypes. --- wolfssh/internal.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/wolfssh/internal.h b/wolfssh/internal.h index a84b843d4..b6e6276ee 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -101,8 +101,10 @@ enum { #define MSG_ID_SZ 1 #define SHA1_96_SZ 12 #define UINT32_SZ 4 -#define DEFAULT_COUNT_HIGHWATER (1024 * 1024 * 1024) -#define DEFAULT_WINDOW_SZ (1024 * 1024) +#define DEFAULT_COUNT_HIGHWATER ((1024 * 1024 * 1024) - (32 * 1024)) +#ifndef DEFAULT_WINDOW_SZ + #define DEFAULT_WINDOW_SZ (1024 * 1024) +#endif #define DEFAULT_MAX_PACKET_SZ (16 * 1024) #define DEFAULT_NEXT_CHANNEL 13013 @@ -299,8 +301,8 @@ WOLFSSH_LOCAL int SendUserAuthFailure(WOLFSSH*, uint8_t); WOLFSSH_LOCAL int SendUserAuthBanner(WOLFSSH*); WOLFSSH_LOCAL int SendUserAuthPkOk(WOLFSSH*, const uint8_t*, uint32_t, const uint8_t*, uint32_t); -WOLFSSH_LOCAL int SendChannelOpenConf(WOLFSSH* ssh); -WOLFSSH_LOCAL int SendChannelData(WOLFSSH* ssh, uint32_t, uint8_t*, uint32_t); +WOLFSSH_LOCAL int SendChannelOpenConf(WOLFSSH*); +WOLFSSH_LOCAL int SendChannelData(WOLFSSH*, uint32_t, uint8_t*, uint32_t); WOLFSSH_LOCAL int GenerateKey(uint8_t, uint8_t, uint8_t*, uint32_t, const uint8_t*, uint32_t, const uint8_t*, uint32_t, From b6cda842e81fbeb6596c34e5c4cb6d92ef1aa630 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 7 Oct 2016 15:02:41 -0700 Subject: [PATCH 03/17] add function to notify peer of window size adjustment --- src/internal.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++ wolfssh/internal.h | 1 + 2 files changed, 55 insertions(+) diff --git a/src/internal.c b/src/internal.c index 60ada4f72..dc82b0027 100644 --- a/src/internal.c +++ b/src/internal.c @@ -3658,6 +3658,60 @@ int SendChannelData(WOLFSSH* ssh, uint32_t peerChannel, } +int SendChannelWindowAdjust(WOLFSSH* ssh, uint32_t peerChannel, + uint32_t bytesToAdd) +{ + uint8_t* output; + uint32_t idx; + int ret = WS_SUCCESS; + WOLFSSH_CHANNEL* channel; + + WLOG(WS_LOG_DEBUG, "Entering SendChannelWindowAdjust()"); + + if (ssh == NULL) + ret = WS_BAD_ARGUMENT; + + channel = ChannelFind(ssh, peerChannel, FIND_PEER); + if (channel == NULL) { + WLOG(WS_LOG_DEBUG, "Invalid peer channel"); + ret = WS_INVALID_CHANID; + } +#if 0 + /* Check bytesToAdd against the buffer size */ + if (bytesToAdd > ssh->inputBuffer.bufferSz - ssh->inputBuffer.length) + ret = WS_BAD_ARGUMENT; +#endif + if (ret == WS_SUCCESS) + ret = PreparePacket(ssh, MSG_ID_SZ + (UINT32_SZ * 2)); + + if (ret == WS_SUCCESS) { + output = ssh->outputBuffer.buffer; + idx = ssh->outputBuffer.length; + + output[idx++] = MSGID_CHANNEL_WINDOW_ADJUST; + c32toa(channel->peerChannel, output + idx); + idx += UINT32_SZ; + c32toa(bytesToAdd, output + idx); + idx += UINT32_SZ; + + ssh->outputBuffer.length = idx; + + ret = BundlePacket(ssh); + } + + if (ret == WS_SUCCESS) { + ret = SendBuffered(ssh); + + WLOG(WS_LOG_INFO, " bytesToAdd = %u", bytesToAdd); + WLOG(WS_LOG_INFO, " windowSz = %u", channel->windowSz); + channel->windowSz += bytesToAdd; + WLOG(WS_LOG_INFO, " update windowSz = %u", channel->windowSz); + } + WLOG(WS_LOG_DEBUG, "Leaving SendChannelWindowAdjust(), ret = %d", ret); + return ret; +} + + #define LINE_WIDTH 16 void DumpOctetString(const uint8_t* input, uint32_t inputSz) { diff --git a/wolfssh/internal.h b/wolfssh/internal.h index b6e6276ee..00935d0f4 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -303,6 +303,7 @@ WOLFSSH_LOCAL int SendUserAuthPkOk(WOLFSSH*, const uint8_t*, uint32_t, const uint8_t*, uint32_t); WOLFSSH_LOCAL int SendChannelOpenConf(WOLFSSH*); WOLFSSH_LOCAL int SendChannelData(WOLFSSH*, uint32_t, uint8_t*, uint32_t); +WOLFSSH_LOCAL int SendChannelWindowAdjust(WOLFSSH*, uint32_t, uint32_t); WOLFSSH_LOCAL int GenerateKey(uint8_t, uint8_t, uint8_t*, uint32_t, const uint8_t*, uint32_t, const uint8_t*, uint32_t, From 919ed1f9448719c290428d414fa27002c1de4825 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 14 Oct 2016 13:16:07 -0700 Subject: [PATCH 04/17] Optionally compile keygen.c. --- configure.ac | 2 ++ src/include.am | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index e4636a0cc..abadcbff0 100644 --- a/configure.ac +++ b/configure.ac @@ -105,6 +105,8 @@ AC_ARG_ENABLE([keygen], AS_IF([test "x$ENABLED_KEYGEN" = "xyes"], [AM_CFLAGS="$AM_CFLAGS -DWOLFSSH_KEYGEN"]) +AM_CONDITIONAL([BUILD_KEYGEN], [test "x$ENABLED_KEYGEN" = "xyes"]) + # Checks for typedefs, structures, and compiler characteristics. if test "$ac_cv_sizeof_long" = "8"; then diff --git a/src/include.am b/src/include.am index ea8a167f5..ce1ea85a5 100644 --- a/src/include.am +++ b/src/include.am @@ -6,7 +6,6 @@ lib_LTLIBRARIES+= src/libwolfssh.la src_libwolfssh_la_SOURCES = src/ssh.c \ src/internal.c \ - src/keygen.c \ src/memory.c \ src/log.c \ src/io.c \ @@ -20,3 +19,7 @@ EXTRA_DIST += if !BUILD_INLINE src_libwolfssh_la_SOURCES += src/misc.c endif + +if BUILD_KEYGEN +src_libwolfssh_la_SOURCES += src/keygen.c +endif From b3ee5cd381b679876f4e8f0f06b996cd0e5ef617 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Sun, 16 Oct 2016 12:11:50 -0700 Subject: [PATCH 05/17] Send channel window adjust update when the receive buffer has processed at least half of its available space. By default, the receive window is 1MB, and the window size is increased every 512kB. --- src/internal.c | 12 +----------- src/ssh.c | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/internal.c b/src/internal.c index dc82b0027..2ce8f19e7 100644 --- a/src/internal.c +++ b/src/internal.c @@ -3676,11 +3676,6 @@ int SendChannelWindowAdjust(WOLFSSH* ssh, uint32_t peerChannel, WLOG(WS_LOG_DEBUG, "Invalid peer channel"); ret = WS_INVALID_CHANID; } -#if 0 - /* Check bytesToAdd against the buffer size */ - if (bytesToAdd > ssh->inputBuffer.bufferSz - ssh->inputBuffer.length) - ret = WS_BAD_ARGUMENT; -#endif if (ret == WS_SUCCESS) ret = PreparePacket(ssh, MSG_ID_SZ + (UINT32_SZ * 2)); @@ -3699,14 +3694,9 @@ int SendChannelWindowAdjust(WOLFSSH* ssh, uint32_t peerChannel, ret = BundlePacket(ssh); } - if (ret == WS_SUCCESS) { + if (ret == WS_SUCCESS) ret = SendBuffered(ssh); - WLOG(WS_LOG_INFO, " bytesToAdd = %u", bytesToAdd); - WLOG(WS_LOG_INFO, " windowSz = %u", channel->windowSz); - channel->windowSz += bytesToAdd; - WLOG(WS_LOG_INFO, " update windowSz = %u", channel->windowSz); - } WLOG(WS_LOG_DEBUG, "Leaving SendChannelWindowAdjust(), ret = %d", ret); return ret; } diff --git a/src/ssh.c b/src/ssh.c index bb186c23c..b751073fb 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -408,6 +408,33 @@ int wolfSSH_stream_read(WOLFSSH* ssh, uint8_t* buf, uint32_t bufSz) WMEMCPY(buf, inputBuffer->buffer + inputBuffer->idx, bufSz); inputBuffer->idx += bufSz; + if (inputBuffer->length > inputBuffer->bufferSz / 2) { + uint32_t usedSz = inputBuffer->length - inputBuffer->idx; + uint32_t bytesToAdd = inputBuffer->idx; + int sendResult; + + WLOG(WS_LOG_DEBUG, "Making more room: %u", usedSz); + if (usedSz) { + WLOG(WS_LOG_DEBUG, " ...moving data down"); + WMEMMOVE(inputBuffer->buffer, + inputBuffer->buffer + bytesToAdd, usedSz); + } + + sendResult = SendChannelWindowAdjust(ssh, + ssh->channelList->peerChannel, + bytesToAdd); + if (sendResult != WS_SUCCESS) + bufSz = sendResult; + + WLOG(WS_LOG_INFO, " bytesToAdd = %u", bytesToAdd); + WLOG(WS_LOG_INFO, " windowSz = %u", ssh->channelList->windowSz); + ssh->channelList->windowSz += bytesToAdd; + WLOG(WS_LOG_INFO, " update windowSz = %u", ssh->channelList->windowSz); + + inputBuffer->length = usedSz; + inputBuffer->idx = 0; + } + WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_stream_read(), rxd = %u", bufSz); return bufSz; } From c564550999160826b99a9901fbcd8855e7e95b1c Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 13 Oct 2016 15:48:55 -0700 Subject: [PATCH 06/17] Automatic Session Rekeying 1. Add stub rekey trigger function. 2. Add new default highwater callback that calls the rekey trigger function. 3. Rename the highwater level as "mark" rather than "count". 4. Add a flag to call the highwater callback once, cleared when the txCount is cleared when sending new keys message to peer. 5. Add new state machine for key exchange. 6. Start massaging the accept state machine for new KEX machine. 7. Update some default sizes, and replace magic numbers with named constants. 8. Scale back the accept state machine to add in the KEX state machine. 9. Capture the client version string and the server's KEX init message for rekeying. 10. Add compiler flag to allow "none" as a user auth method. --- src/internal.c | 111 +++++++++++++++++++++++++++++++++++---------- src/ssh.c | 63 ++++++------------------- wolfssh/error.h | 3 +- wolfssh/internal.h | 38 ++++++++++++---- wolfssh/ssh.h | 1 + 5 files changed, 135 insertions(+), 81 deletions(-) diff --git a/src/internal.c b/src/internal.c index 2ce8f19e7..f90755c52 100644 --- a/src/internal.c +++ b/src/internal.c @@ -139,6 +139,9 @@ const char* GetErrorString(int err) case WS_CRYPTO_FAILED: return "crypto action failed"; + case WS_INVALID_STATE_E: + return "invalid state"; + default: return "Unknown error code"; } @@ -146,6 +149,26 @@ const char* GetErrorString(int err) } +static int wsHighwater(byte dir, void* ctx) +{ + int ret = WS_SUCCESS; + + (void)dir; + + if (ctx) { + WOLFSSH* ssh = (WOLFSSH*)ctx; + + WLOG(WS_LOG_DEBUG, "HIGHWATER MARK: (%u) %s\n", + wolfSSH_GetHighwater(ssh), + (dir == WOLFSSH_HWSIDE_RECEIVE) ? "receive" : "transmit"); + + ret = wolfSSH_TriggerKeyExchange(ssh); + } + + return ret; +} + + WOLFSSH_CTX* CtxInit(WOLFSSH_CTX* ctx, void* heap) { WLOG(WS_LOG_DEBUG, "Entering CtxInit()"); @@ -162,7 +185,8 @@ WOLFSSH_CTX* CtxInit(WOLFSSH_CTX* ctx, void* heap) ctx->ioRecvCb = wsEmbedRecv; ctx->ioSendCb = wsEmbedSend; #endif /* WOLFSSH_USER_IO */ - ctx->countHighwater = DEFAULT_COUNT_HIGHWATER; + ctx->highwaterMark = DEFAULT_HIGHWATER_MARK; + ctx->highwaterCb = wsHighwater; return ctx; } @@ -213,9 +237,11 @@ WOLFSSH* SshInit(WOLFSSH* ssh, WOLFSSH_CTX* ctx) ssh->wfd = -1; /* set to invalid */ ssh->ioReadCtx = &ssh->rfd; /* prevent invalid access if not correctly */ ssh->ioWriteCtx = &ssh->wfd; /* set */ - ssh->countHighwater = ctx->countHighwater; + ssh->highwaterMark = ctx->highwaterMark; + ssh->highwaterCtx = (void*)ssh; ssh->acceptState = ACCEPT_BEGIN; ssh->clientState = CLIENT_BEGIN; + ssh->keyingState = KEYING_UNKEYED; ssh->nextChannel = DEFAULT_NEXT_CHANNEL; ssh->blockSz = MIN_BLOCK_SZ; ssh->encryptId = ID_NONE; @@ -265,6 +291,9 @@ void SshResourceFree(WOLFSSH* ssh, void* heap) if (ssh->userName) { WFREE(ssh->userName, heap, DYNTYPE_STRING); } + if (ssh->clientId) { + WFREE(ssh->clientId, heap, DYNTYPE_STRING); + } if (ssh->channelList) { WOLFSSH_CHANNEL* cur = ssh->channelList; WOLFSSH_CHANNEL* next; @@ -1011,6 +1040,9 @@ static int DoKexInit(WOLFSSH* ssh, uint8_t* buf, uint32_t len, uint32_t* idx) if (ssh == NULL || buf == NULL || len == 0 || idx == NULL) ret = WS_BAD_ARGUMENT; + + if (ssh->keyingState != KEYING_UNKEYED && ssh->keyingState != KEYING_KEYED) + ret = WS_INVALID_STATE_E; /* * I don't need to save what the client sends here. I should decode * each list into a local array of IDs, and pick the one the peer is @@ -2035,6 +2067,11 @@ static int DoUserAuthRequest(WOLFSSH* ssh, authData.sf.publicKey.dataToSign = buf + *idx; ret = DoUserAuthRequestPublicKey(ssh, &authData, buf, len, &begin); } +#ifdef WOLFSSH_ALLOW_USERAUTH_NONE + else if (authNameId == ID_NONE) { + ssh->clientState = CLIENT_USERAUTH_DONE; + } +#endif else { WLOG(WS_LOG_DEBUG, "invalid userauth type: %s", IdToName(authNameId)); @@ -2413,10 +2450,13 @@ static INLINE int Encrypt(WOLFSSH* ssh, uint8_t* cipher, const uint8_t* input, } ssh->txCount += sz; - if (ssh->countHighwater && ssh->txCount > ssh->countHighwater) { + if (ssh->highwaterMark && !ssh->highwaterFlag && + ssh->txCount > ssh->highwaterMark) { + WLOG(WS_LOG_DEBUG, "Transmit over high water mark"); if (ssh->ctx->highwaterCb) ssh->ctx->highwaterCb(WOLFSSH_HWSIDE_TRANSMIT, ssh->highwaterCtx); + ssh->highwaterFlag = 1; } return ret; @@ -2450,10 +2490,13 @@ static INLINE int Decrypt(WOLFSSH* ssh, uint8_t* plain, const uint8_t* input, } ssh->rxCount += sz; - if (ssh->countHighwater && ssh->rxCount > ssh->countHighwater) { + if (ssh->highwaterMark && !ssh->highwaterFlag && + ssh->rxCount > ssh->highwaterMark) { + WLOG(WS_LOG_DEBUG, "Receive over high water mark"); if (ssh->ctx->highwaterCb) ssh->ctx->highwaterCb(WOLFSSH_HWSIDE_RECEIVE, ssh->highwaterCtx); + ssh->highwaterFlag = 1; } return ret; @@ -2679,16 +2722,17 @@ static const char sshIdStr[] = "SSH-2.0-wolfSSHv" int ProcessClientVersion(WOLFSSH* ssh) { - int error; - uint32_t protoLen = 7; /* Length of the SSH-2.0 portion of the ID str */ - uint8_t scratch[LENGTH_SZ]; + int ret; + uint32_t clientIdSz; - if ( (error = GetInputText(ssh)) < 0) { + if ( (ret = GetInputText(ssh)) < 0) { WLOG(WS_LOG_DEBUG, "get input text failed"); - return error; + return ret; } - if (WSTRNCASECMP((char*)ssh->inputBuffer.buffer, sshIdStr, protoLen) == 0) { + if (WSTRNCASECMP((char*)ssh->inputBuffer.buffer, + sshIdStr, SSH_PROTO_SZ) == 0) { + ssh->clientState = CLIENT_VERSION_DONE; } else { @@ -2696,38 +2740,58 @@ int ProcessClientVersion(WOLFSSH* ssh) return WS_VERSION_E; } - c32toa(ssh->inputBuffer.length - 2, scratch); - wc_ShaUpdate(&ssh->handshake->hash, scratch, LENGTH_SZ); - wc_ShaUpdate(&ssh->handshake->hash, ssh->inputBuffer.buffer, - ssh->inputBuffer.length - 2); + clientIdSz = ssh->inputBuffer.length - SSH_PROTO_EOL_SZ; + + ssh->clientId = (uint8_t*)WMALLOC(clientIdSz, + ssh->ctx->heap, DYNTYPE_STRING); + if (ssh->clientId == NULL) + ret = WS_MEMORY_E; + else { + uint8_t flatSz[LENGTH_SZ]; + + /* Store the client version string. Will need it later during rekey */ + WMEMCPY(ssh->clientId, ssh->inputBuffer.buffer, clientIdSz); + ssh->clientIdSz = clientIdSz; + c32toa(clientIdSz, flatSz); + ret = wc_ShaUpdate(&ssh->handshake->hash, flatSz, LENGTH_SZ); + } + + if (ret == WS_SUCCESS) + ret = wc_ShaUpdate(&ssh->handshake->hash, ssh->inputBuffer.buffer, + clientIdSz); + ssh->inputBuffer.idx += ssh->inputBuffer.length; - return WS_SUCCESS; + return ret; } int SendServerVersion(WOLFSSH* ssh) { int ret = WS_SUCCESS; - uint32_t sshIdStrSz = (uint32_t)WSTRLEN(sshIdStr); - uint8_t sshIdStrSzFlat[LENGTH_SZ]; + uint32_t sshIdStrSz; if (ssh == NULL) ret = WS_BAD_ARGUMENT; if (ret == WS_SUCCESS) { WLOG(WS_LOG_DEBUG, "%s", sshIdStr); - ret = SendText(ssh, sshIdStr, (uint32_t)WSTRLEN(sshIdStr)); + sshIdStrSz = (uint32_t)WSTRLEN(sshIdStr); + ret = SendText(ssh, sshIdStr, sshIdStrSz); } if (ret == WS_SUCCESS) { - sshIdStrSz -= 2; /* Remove the CRLF */ + uint8_t sshIdStrSzFlat[LENGTH_SZ]; + + sshIdStrSz -= SSH_PROTO_EOL_SZ; c32toa(sshIdStrSz, sshIdStrSzFlat); - wc_ShaUpdate(&ssh->handshake->hash, sshIdStrSzFlat, LENGTH_SZ); - wc_ShaUpdate(&ssh->handshake->hash, - (const uint8_t*)sshIdStr, sshIdStrSz); + ret = wc_ShaUpdate(&ssh->handshake->hash, sshIdStrSzFlat, LENGTH_SZ); } + if (ret == WS_SUCCESS) + ret = wc_ShaUpdate(&ssh->handshake->hash, + (const uint8_t*)sshIdStr, sshIdStrSz); + return ret; } @@ -2748,7 +2812,7 @@ static int PreparePacket(WOLFSSH* ssh, uint32_t payloadSz) /* Minimum value for paddingSz is 4. */ paddingSz = ssh->blockSz - (LENGTH_SZ + PAD_LENGTH_SZ + payloadSz) % ssh->blockSz; - if (paddingSz < 4) + if (paddingSz < MIN_PAD_LENGTH) paddingSz += ssh->blockSz; ssh->paddingSz = paddingSz; packetSz = PAD_LENGTH_SZ + payloadSz + paddingSz; @@ -3186,6 +3250,7 @@ int SendNewKeys(WOLFSSH* ssh) } ssh->txCount = 0; + ssh->highwaterFlag = 0; } return ret; diff --git a/src/ssh.c b/src/ssh.c index b751073fb..8e3a4ecf8 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -165,7 +165,7 @@ int wolfSSH_SetHighwater(WOLFSSH* ssh, uint32_t highwater) WLOG(WS_LOG_DEBUG, "Entering wolfSSH_SetHighwater()"); if (ssh) { - ssh->countHighwater = highwater; + ssh->highwaterMark = highwater; return WS_SUCCESS; } @@ -179,7 +179,7 @@ uint32_t wolfSSH_GetHighwater(WOLFSSH* ssh) WLOG(WS_LOG_DEBUG, "Entering wolfSSH_GetHighwater()"); if (ssh) - return ssh->countHighwater; + return ssh->highwaterMark; return 0; } @@ -191,7 +191,7 @@ void wolfSSH_SetHighwaterCb(WOLFSSH_CTX* ctx, uint32_t highwater, WLOG(WS_LOG_DEBUG, "Entering wolfSSH_SetHighwaterCb()"); if (ctx) { - ctx->countHighwater = highwater; + ctx->highwaterMark = highwater; ctx->highwaterCb = cb; } } @@ -269,61 +269,21 @@ int wolfSSH_accept(WOLFSSH* ssh) WLOG(WS_LOG_DEBUG, acceptState, "SERVER_VERSION_SENT"); case ACCEPT_SERVER_VERSION_SENT: - while (ssh->clientState < CLIENT_KEXINIT_DONE) { + while (ssh->keyingState < KEYING_KEYED) { if ( (ssh->error = ProcessReply(ssh)) < WS_SUCCESS) { WLOG(WS_LOG_DEBUG, acceptError, "SERVER_VERSION_SENT", ssh->error); return WS_FATAL_ERROR; } } - ssh->acceptState = ACCEPT_CLIENT_KEXINIT_DONE; - WLOG(WS_LOG_DEBUG, acceptState, "CLIENT_KEXINIT_DONE"); + ssh->acceptState = ACCEPT_KEYED; + WLOG(WS_LOG_DEBUG, acceptState, "KEYED"); - case ACCEPT_CLIENT_KEXINIT_DONE: - if ( (ssh->error = SendKexInit(ssh)) < WS_SUCCESS) { - WLOG(WS_LOG_DEBUG, acceptError, - "CLIENT_KEXINIT_DONE", ssh->error); - return WS_FATAL_ERROR; - } - ssh->acceptState = ACCEPT_SERVER_KEXINIT_SENT; - WLOG(WS_LOG_DEBUG, acceptState, "SERVER_KEXINIT_SENT"); - - case ACCEPT_SERVER_KEXINIT_SENT: - while (ssh->clientState < CLIENT_KEXDH_INIT_DONE) { - if ( (ssh->error = ProcessReply(ssh)) < 0) { - WLOG(WS_LOG_DEBUG, acceptError, - "SERVER_KEXINIT_SENT", ssh->error); - return WS_FATAL_ERROR; - } - } - ssh->acceptState = ACCEPT_CLIENT_KEXDH_INIT_DONE; - WLOG(WS_LOG_DEBUG, acceptState, "CLIENT_KEXDH_INIT_DONE"); - - case ACCEPT_CLIENT_KEXDH_INIT_DONE: - if ( (ssh->error = SendKexDhReply(ssh)) < WS_SUCCESS) { - WLOG(WS_LOG_DEBUG, acceptError, - "CLIENT_KEXDH_INIT_DONE", ssh->error); - return WS_FATAL_ERROR; - } - ssh->acceptState = ACCEPT_SERVER_KEXDH_REPLY_SENT; - WLOG(WS_LOG_DEBUG, acceptState, "SERVER_KEXDH_REPLY_SENT"); - - case ACCEPT_SERVER_KEXDH_REPLY_SENT: - while (ssh->clientState < CLIENT_USING_KEYS) { - if ( (ssh->error = ProcessReply(ssh)) < 0) { - WLOG(WS_LOG_DEBUG, acceptError, - "SERVER_KEXDH_REPLY_SENT", ssh->error); - return WS_FATAL_ERROR; - } - } - ssh->acceptState = ACCEPT_USING_KEYS; - WLOG(WS_LOG_DEBUG, acceptState, "USING_KEYS"); - - case ACCEPT_USING_KEYS: + case ACCEPT_KEYED: while (ssh->clientState < CLIENT_USERAUTH_REQUEST_DONE) { if ( (ssh->error = ProcessReply(ssh)) < 0) { WLOG(WS_LOG_DEBUG, acceptError, - "USING_KEYS", ssh->error); + "KEYED", ssh->error); return WS_FATAL_ERROR; } } @@ -385,6 +345,13 @@ int wolfSSH_accept(WOLFSSH* ssh) } +int wolfSSH_TriggerKeyExchange(WOLFSSH* ssh) +{ + (void)ssh; + return WS_SUCCESS; +} + + int wolfSSH_stream_read(WOLFSSH* ssh, uint8_t* buf, uint32_t bufSz) { Buffer* inputBuffer; diff --git a/wolfssh/error.h b/wolfssh/error.h index 559290ace..a1105875d 100644 --- a/wolfssh/error.h +++ b/wolfssh/error.h @@ -66,7 +66,8 @@ enum WS_ErrorCodes { WS_INVALID_CHANTYPE = -26, /* invalid channel type */ WS_INVALID_CHANID = -27, WS_INVALID_USERNAME = -28, - WS_CRYPTO_FAILED = -29 /* crypto action failed */ + WS_CRYPTO_FAILED = -29, /* crypto action failed */ + WS_INVALID_STATE_E = -30 }; diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 00935d0f4..f604980ab 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -97,11 +97,14 @@ enum { #define COOKIE_SZ 16 #define LENGTH_SZ 4 #define PAD_LENGTH_SZ 1 +#define MIN_PAD_LENGTH 4 #define BOOLEAN_SZ 1 #define MSG_ID_SZ 1 #define SHA1_96_SZ 12 #define UINT32_SZ 4 -#define DEFAULT_COUNT_HIGHWATER ((1024 * 1024 * 1024) - (32 * 1024)) +#define SSH_PROTO_SZ 7 /* "SSH-2.0" */ +#define SSH_PROTO_EOL_SZ 2 /* Just the CRLF */ +#define DEFAULT_HIGHWATER_MARK ((1024 * 1024 * 1024) - (32 * 1024)) #ifndef DEFAULT_WINDOW_SZ #define DEFAULT_WINDOW_SZ (1024 * 1024) #endif @@ -145,7 +148,7 @@ struct WOLFSSH_CTX { uint8_t* privateKey; /* Owned by CTX */ uint32_t privateKeySz; - uint32_t countHighwater; + uint32_t highwaterMark; }; @@ -179,6 +182,8 @@ typedef struct HandshakeInfo { Sha hash; uint8_t e[257]; /* May have a leading zero, for unsigned. */ uint32_t eSz; + uint8_t* serverKexInit; /* Used for server initiated rekey. */ + uint32_t serverKeyInitSz; } HandshakeInfo; @@ -194,7 +199,8 @@ struct WOLFSSH { int wflags; /* optional write flags */ uint32_t txCount; uint32_t rxCount; - uint32_t countHighwater; + uint32_t highwaterMark; + uint8_t highwaterFlag; /* Set when highwater CB called */ void* highwaterCtx; uint32_t curSz; uint32_t seq; @@ -204,6 +210,7 @@ struct WOLFSSH { uint8_t acceptState; uint8_t clientState; uint8_t processReplyState; + uint8_t keyingState; uint8_t connReset; uint8_t isClosed; @@ -245,6 +252,8 @@ struct WOLFSSH { uint32_t userNameSz; uint8_t* pkBlob; uint32_t pkBlobSz; + uint8_t* clientId; /* Save for rekey */ + uint32_t clientIdSz; }; @@ -252,7 +261,6 @@ struct WOLFSSH_CHANNEL { uint8_t channelType; uint32_t channel; uint32_t windowSz; - uint32_t highwaterMark; uint32_t maxPacketSz; uint32_t peerChannel; uint32_t peerWindowSz; @@ -310,15 +318,27 @@ WOLFSSH_LOCAL int GenerateKey(uint8_t, uint8_t, uint8_t*, uint32_t, const uint8_t*, uint32_t); +enum KeyingStates { + KEYING_UNKEYED = 0, + + KEYING_KEXINIT_SENT, + KEYING_KEXINIT_RECV, + KEYING_KEXINIT_DONE, + + KEYING_KEXDH_INIT_RECV, + KEYING_KEXDH_DONE, + + KEYING_USING_KEYS_SENT, + KEYING_USING_KEYS_RECV, + KEYING_KEYED +}; + + enum AcceptStates { ACCEPT_BEGIN = 0, ACCEPT_CLIENT_VERSION_DONE, ACCEPT_SERVER_VERSION_SENT, - ACCEPT_CLIENT_KEXINIT_DONE, - ACCEPT_SERVER_KEXINIT_SENT, - ACCEPT_CLIENT_KEXDH_INIT_DONE, - ACCEPT_SERVER_KEXDH_REPLY_SENT, - ACCEPT_USING_KEYS, + ACCEPT_KEYED, ACCEPT_CLIENT_USERAUTH_REQUEST_DONE, ACCEPT_SERVER_USERAUTH_ACCEPT_SENT, ACCEPT_CLIENT_USERAUTH_DONE, diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index a6f8827b6..b3368f43c 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -133,6 +133,7 @@ WOLFSSH_API int wolfSSH_stream_send(WOLFSSH*, uint8_t*, uint32_t); WOLFSSH_API int wolfSSH_channel_read(WOLFSSH_CHANNEL*, uint8_t*, uint32_t); WOLFSSH_API int wolfSSH_channel_send(WOLFSSH_CHANNEL*, uint8_t*, uint32_t); WOLFSSH_API int wolfSSH_worker(WOLFSSH*); +WOLFSSH_API int wolfSSH_TriggerKeyExchange(WOLFSSH*); WOLFSSH_API int wolfSSH_KDF(uint8_t, uint8_t, uint8_t*, uint32_t, const uint8_t*, uint32_t, const uint8_t*, uint32_t, From 41ec11e6abeb24329c195ea2b2a9dc1b32507977 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Sun, 16 Oct 2016 20:25:24 -0700 Subject: [PATCH 07/17] Rekeying Update 1. Flushing out the keying state machine, taking it out of the accept state machine. 2. Changed the HandshakeInfo record to be something that can be recreated post-initial connection for rekeying. 3. Fixed the name of a previously unused size variable. 4. Moved some constants around. --- src/internal.c | 157 ++++++++++++++++++++++++++++++++++++--------- wolfssh/internal.h | 2 +- 2 files changed, 127 insertions(+), 32 deletions(-) diff --git a/src/internal.c b/src/internal.c index f90755c52..26c23ddbd 100644 --- a/src/internal.c +++ b/src/internal.c @@ -169,6 +169,42 @@ static int wsHighwater(byte dir, void* ctx) } +static HandshakeInfo* HandshakeInfoNew(void* heap) +{ + HandshakeInfo* newHs; + + newHs = (HandshakeInfo*)WMALLOC(sizeof(HandshakeInfo), + heap, DYNTYPE_HS); + if (newHs != NULL) { + WMEMSET(newHs, 0, sizeof(HandshakeInfo)); + newHs->kexId = ID_NONE; + newHs->pubKeyId = ID_NONE; + newHs->encryptId = ID_NONE; + newHs->macId = ID_NONE; + newHs->blockSz = MIN_BLOCK_SZ; + + if (wc_InitSha(&newHs->hash) != 0) { + WFREE(newHs, heap, DYNTYPE_HS); + newHs = NULL; + } + } + + return newHs; +} + + +static void HandshakeInfoFree(HandshakeInfo* hs, void* heap) +{ + (void)heap; + + if (hs) { + WFREE(hs->serverKexInit, heap, DYNTYPE_STRING); + ForceZero(hs, sizeof(HandshakeInfo)); + WFREE(hs, heap, DYNTYPE_HS); + } +} + + WOLFSSH_CTX* CtxInit(WOLFSSH_CTX* ctx, void* heap) { WLOG(WS_LOG_DEBUG, "Entering CtxInit()"); @@ -205,8 +241,8 @@ void CtxResourceFree(WOLFSSH_CTX* ctx) WOLFSSH* SshInit(WOLFSSH* ssh, WOLFSSH_CTX* ctx) { - HandshakeInfo* handshake = NULL; - RNG* rng = NULL; + HandshakeInfo* handshake; + RNG* rng; void* heap; WLOG(WS_LOG_DEBUG, "Entering SshInit()"); @@ -215,8 +251,7 @@ WOLFSSH* SshInit(WOLFSSH* ssh, WOLFSSH_CTX* ctx) return ssh; heap = ctx->heap; - handshake = (HandshakeInfo*)WMALLOC(sizeof(HandshakeInfo), - heap, DYNTYPE_HS); + handshake = HandshakeInfoNew(heap); rng = (RNG*)WMALLOC(sizeof(RNG), heap, DYNTYPE_RNG); if (handshake == NULL || rng == NULL || wc_InitRng(rng) != 0) { @@ -229,7 +264,6 @@ WOLFSSH* SshInit(WOLFSSH* ssh, WOLFSSH_CTX* ctx) } WMEMSET(ssh, 0, sizeof(WOLFSSH)); /* default init to zeros */ - WMEMSET(handshake, 0, sizeof(HandshakeInfo)); ssh->ctx = ctx; ssh->error = WS_SUCCESS; @@ -250,15 +284,9 @@ WOLFSSH* SshInit(WOLFSSH* ssh, WOLFSSH_CTX* ctx) ssh->rng = rng; ssh->kSz = sizeof(ssh->k); ssh->handshake = handshake; - handshake->kexId = ID_NONE; - handshake->pubKeyId = ID_NONE; - handshake->encryptId = ID_NONE; - handshake->macId = ID_NONE; - handshake->blockSz = MIN_BLOCK_SZ; if (BufferInit(&ssh->inputBuffer, 0, ctx->heap) != WS_SUCCESS || - BufferInit(&ssh->outputBuffer, 0, ctx->heap) != WS_SUCCESS || - wc_InitSha(&ssh->handshake->hash) != 0) { + BufferInit(&ssh->outputBuffer, 0, ctx->heap) != WS_SUCCESS) { wolfSSH_free(ssh); ssh = NULL; @@ -278,10 +306,7 @@ void SshResourceFree(WOLFSSH* ssh, void* heap) ShrinkBuffer(&ssh->inputBuffer, 1); ShrinkBuffer(&ssh->outputBuffer, 1); ForceZero(ssh->k, ssh->kSz); - if (ssh->handshake) { - ForceZero(ssh->handshake, sizeof(HandshakeInfo)); - WFREE(ssh->handshake, heap, DYNTYPE_HS); - } + HandshakeInfoFree(ssh->handshake, heap); ForceZero(&ssh->clientKeys, sizeof(Keys)); ForceZero(&ssh->serverKeys, sizeof(Keys)); if (ssh->rng) { @@ -1229,6 +1254,14 @@ static int DoKexInit(WOLFSSH* ssh, uint8_t* buf, uint32_t len, uint32_t* idx) if (ret == WS_SUCCESS) { *idx = begin; ssh->clientState = CLIENT_KEXINIT_DONE; + + if (ssh->keyingState == KEYING_UNKEYED) { + ssh->keyingState = KEYING_KEXINIT_RECV; + ret = SendKexInit(ssh); + } + else if (KEYING_KEXINIT_SENT) { + ssh->keyingState = KEYING_KEXINIT_DONE; + } } WLOG(WS_LOG_DEBUG, "Leaving DoKexInit(), ret = %d", ret); @@ -1329,7 +1362,10 @@ static int DoKexDhInit(WOLFSSH* ssh, uint8_t* buf, uint32_t len, uint32_t* idx) ssh->clientState = CLIENT_KEXDH_INIT_DONE; *idx = begin; + + ret = SendKexDhReply(ssh); } + return ret; } @@ -1379,6 +1415,7 @@ static int DoNewKeys(WOLFSSH* ssh, uint8_t* buf, uint32_t len, uint32_t* idx) if (ret == WS_SUCCESS) { ssh->rxCount = 0; ssh->clientState = CLIENT_USING_KEYS; + ssh->keyingState = KEYING_KEYED; } return ret; @@ -2300,6 +2337,11 @@ static int DoChannelData(WOLFSSH* ssh, } +static const char sshIdStr[] = "SSH-2.0-wolfSSHv" + LIBWOLFSSH_VERSION_STRING + "\r\n"; + + static int DoPacket(WOLFSSH* ssh) { uint8_t* buf = (uint8_t*)ssh->inputBuffer.buffer; @@ -2347,11 +2389,43 @@ static int DoPacket(WOLFSSH* ssh) uint8_t szFlat[LENGTH_SZ]; WLOG(WS_LOG_DEBUG, "Decoding MSGID_KEXINIT"); - c32toa(payloadSz + sizeof(msg), szFlat); - wc_ShaUpdate(&ssh->handshake->hash, szFlat, LENGTH_SZ); - wc_ShaUpdate(&ssh->handshake->hash, &msg, sizeof(msg)); - wc_ShaUpdate(&ssh->handshake->hash, buf + idx, payloadSz); - ret = DoKexInit(ssh, buf + idx, payloadSz, &payloadIdx); + ret = WS_SUCCESS; + + if (ssh->keyingState == KEYING_KEYED) { + uint32_t idSz; + + ssh->handshake = HandshakeInfoNew(ssh->ctx->heap); + if (ssh->handshake == NULL) { + WLOG(WS_LOG_DEBUG, "Couldn't allocate handshake info"); + ret = WS_MEMORY_E; + } + c32toa(ssh->clientIdSz, szFlat); + wc_ShaUpdate(&ssh->handshake->hash, szFlat, LENGTH_SZ); + wc_ShaUpdate(&ssh->handshake->hash, + ssh->clientId, ssh->clientIdSz); + + idSz = (uint32_t)WSTRLEN(sshIdStr) - SSH_PROTO_EOL_SZ; + c32toa(idSz, szFlat); + wc_ShaUpdate(&ssh->handshake->hash, szFlat, LENGTH_SZ); + wc_ShaUpdate(&ssh->handshake->hash, + (const uint8_t*)sshIdStr, idSz); + } + + if (ret == WS_SUCCESS) { + c32toa(payloadSz + sizeof(msg), szFlat); + wc_ShaUpdate(&ssh->handshake->hash, szFlat, LENGTH_SZ); + wc_ShaUpdate(&ssh->handshake->hash, &msg, sizeof(msg)); + wc_ShaUpdate(&ssh->handshake->hash, buf + idx, payloadSz); + } + + if (ssh->keyingState == KEYING_KEXINIT_SENT) { + wc_ShaUpdate(&ssh->handshake->hash, + ssh->handshake->serverKexInit, + ssh->handshake->serverKexInitSz); + } + + if (ret == WS_SUCCESS) + ret = DoKexInit(ssh, buf + idx, payloadSz, &payloadIdx); } break; @@ -2715,11 +2789,6 @@ int ProcessReply(WOLFSSH* ssh) } -static const char sshIdStr[] = "SSH-2.0-wolfSSHv" - LIBWOLFSSH_VERSION_STRING - "\r\n"; - - int ProcessClientVersion(WOLFSSH* ssh) { int ret; @@ -2975,18 +3044,42 @@ int SendKexInit(WOLFSSH* ssh) ssh->outputBuffer.length = idx; c32toa(payloadSz, payloadSzFlat); - ret = wc_ShaUpdate(&ssh->handshake->hash, payloadSzFlat, LENGTH_SZ); + if (ssh->keyingState == KEYING_KEYED) { + uint8_t* buf; + uint32_t bufSz = payloadSz + LENGTH_SZ; + + buf = (uint8_t*)WMALLOC(bufSz, ssh->ctx->heap, DYNTYPE_STRING); + if (buf == NULL) { + WLOG(WS_LOG_DEBUG, "Cannot allocate storage for KEX Init msg"); + ret = WS_MEMORY_E; + } + else { + WMEMCPY(buf, payloadSzFlat, LENGTH_SZ); + WMEMCPY(buf + LENGTH_SZ, payload, payloadSz); + ssh->handshake->serverKexInit = buf; + ssh->handshake->serverKexInitSz = bufSz; + } + } + else { + ret = wc_ShaUpdate(&ssh->handshake->hash, payloadSzFlat, LENGTH_SZ); + if (ret == WS_SUCCESS) + ret = wc_ShaUpdate(&ssh->handshake->hash, payload, payloadSz); + } } - if (ret == WS_SUCCESS) - ret = wc_ShaUpdate(&ssh->handshake->hash, payload, payloadSz); - if (ret == WS_SUCCESS) ret = BundlePacket(ssh); if (ret == WS_SUCCESS) ret = SendBuffered(ssh); + if (ret == WS_SUCCESS) { + if (ssh->keyingState == KEYING_KEXINIT_RECV) + ssh->keyingState = KEYING_KEXINIT_DONE; + else if (ssh->keyingState == KEYING_KEYED) + ssh->keyingState = KEYING_KEXINIT_SENT; + } + return ret; } @@ -3191,8 +3284,9 @@ int SendKexDhReply(WOLFSSH* ssh) ssh->outputBuffer.length = idx; ret = BundlePacket(ssh); - if (ret == WS_SUCCESS) + if (ret == WS_SUCCESS) { ret = SendNewKeys(ssh); + } return ret; } @@ -3251,6 +3345,7 @@ int SendNewKeys(WOLFSSH* ssh) ssh->txCount = 0; ssh->highwaterFlag = 0; + ssh->keyingState = KEYING_USING_KEYS_SENT; } return ret; diff --git a/wolfssh/internal.h b/wolfssh/internal.h index f604980ab..38d6dde61 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -183,7 +183,7 @@ typedef struct HandshakeInfo { uint8_t e[257]; /* May have a leading zero, for unsigned. */ uint32_t eSz; uint8_t* serverKexInit; /* Used for server initiated rekey. */ - uint32_t serverKeyInitSz; + uint32_t serverKexInitSz; } HandshakeInfo; From 184b2218d495138e95e03769e4bb57485561af65 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Sun, 16 Oct 2016 20:59:53 -0700 Subject: [PATCH 08/17] Rekeying Update 1. Add the Trigger Rekeying function. 2. Fixing the keying state machine. 3. Modify echoserver for rekeying. --- examples/echoserver/echoserver.c | 93 +++++++++------- src/internal.c | 181 +++++++++++++++++++++---------- src/ssh.c | 21 +++- wolfssh/error.h | 3 +- wolfssh/internal.h | 8 +- 5 files changed, 200 insertions(+), 106 deletions(-) diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index 2d46fb45f..12b8e4041 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -92,8 +92,11 @@ typedef struct { } thread_ctx_t; -#ifndef DEFAULT_HIGHWATER_MARK - #define DEFAULT_HIGHWATER_MARK 0 +#ifndef EXAMPLE_HIGHWATER_MARK + #define EXAMPLE_HIGHWATER_MARK 0x3FFF8000 /* 1GB - 32kB */ +#endif +#ifndef EXAMPLE_BUFFER_SZ + #define EXAMPLE_BUFFER_SZ 4096 #endif @@ -284,23 +287,54 @@ static THREAD_RETURN CYASSL_THREAD server_worker(void* vArgs) WOLFSSH* ssh = (WOLFSSH*)vArgs; SOCKET_T clientFd = wolfSSH_get_fd(ssh); - uint8_t buf[4096]; - int bufSz; - if (wolfSSH_accept(ssh) == WS_SUCCESS) { - - while (1) { - bufSz = wolfSSH_stream_read(ssh, buf, sizeof(buf)); - if (bufSz > 0) { - wolfSSH_stream_send(ssh, buf, bufSz); - if (find_char(0x03, buf, bufSz)) - break; - } - else { - printf("wolfSSH_stream_read returned %d\n", bufSz); - break; + uint8_t* buf = NULL; + uint8_t* tmpBuf; + int bufSz, backlogSz = 0, rxSz, txSz, stop = 0, txSum; + + do { + bufSz = EXAMPLE_BUFFER_SZ + backlogSz; + + tmpBuf = realloc(buf, bufSz); + if (tmpBuf == NULL) + stop = 1; + else + buf = tmpBuf; + + if (!stop) { + rxSz = wolfSSH_stream_read(ssh, + buf + backlogSz, + EXAMPLE_BUFFER_SZ); + if (rxSz > 0) { + backlogSz += rxSz; + txSum = 0; + txSz = 0; + + while (backlogSz != txSum && txSz >= 0 && !stop) { + txSz = wolfSSH_stream_send(ssh, + buf + txSum, + backlogSz - txSum); + + if (txSz > 0) { + if (find_char(0x03, buf + txSum, txSz)) + stop = 1; + else + txSum += txSz; + } + else if (txSz != WS_REKEYING) + stop = 1; + } + + if (txSum < backlogSz) + memmove(buf, buf + txSum, backlogSz - txSum); + backlogSz -= txSum; + } + else + stop = 1; } - } + } while (!stop); + + free(buf); } close(clientFd); wolfSSH_free(ssh); @@ -592,29 +626,12 @@ static int wsUserAuth(uint8_t authType, } -static int wsHighwater(uint8_t side, void* ctx) -{ - if (ctx) { - WOLFSSH* ssh = (WOLFSSH*)ctx; - uint32_t highwaterMark = wolfSSH_GetHighwater(ssh); - - printf("HIGHWATER ALERT: (%u) %s\n", highwaterMark, - (side == WOLFSSH_HWSIDE_RECEIVE) ? "receive" : "transmit"); - highwaterMark *= 2; - printf(" Doubling the highwater mark to %u.\n", highwaterMark); - wolfSSH_SetHighwater(ssh, highwaterMark); - } - - return 0; -} - - int main(void) { WOLFSSH_CTX* ctx = NULL; PwMapList pwMapList; SOCKET_T listenFd = 0; - uint32_t defaultHighwater = DEFAULT_HIGHWATER_MARK; + uint32_t defaultHighwater = EXAMPLE_HIGHWATER_MARK; #ifdef DEBUG_WOLFSSH wolfSSH_Debugging_ON(); @@ -633,8 +650,6 @@ int main(void) memset(&pwMapList, 0, sizeof(pwMapList)); wolfSSH_SetUserAuth(ctx, wsUserAuth); - if (defaultHighwater > 0) - wolfSSH_SetHighwaterCb(ctx, defaultHighwater, wsHighwater); { uint8_t buf[SCRATCH_BUFFER_SIZE]; @@ -678,8 +693,10 @@ int main(void) } wolfSSH_SetUserAuthCtx(ssh, &pwMapList); /* Use the session object for its own highwater callback ctx */ - if (defaultHighwater > 0) + if (defaultHighwater > 0) { wolfSSH_SetHighwaterCtx(ssh, (void*)ssh); + wolfSSH_SetHighwater(ssh, defaultHighwater); + } if (listen(listenFd, 5) != 0) err_sys("tcp listen failed"); diff --git a/src/internal.c b/src/internal.c index 26c23ddbd..0879ebb13 100644 --- a/src/internal.c +++ b/src/internal.c @@ -127,21 +127,24 @@ const char* GetErrorString(int err) case WS_RESOURCE_E: return "insufficient resources for new channel"; - case WS_INVALID_USERNAME: - return "invalid user name"; - case WS_INVALID_CHANTYPE: return "peer requested invalid channel type"; case WS_INVALID_CHANID: return "peer requested invalid channel id"; + case WS_INVALID_USERNAME: + return "invalid user name"; + case WS_CRYPTO_FAILED: return "crypto action failed"; case WS_INVALID_STATE_E: return "invalid state"; + case WS_REKEYING: + return "rekeying with peer"; + default: return "Unknown error code"; } @@ -158,7 +161,7 @@ static int wsHighwater(byte dir, void* ctx) if (ctx) { WOLFSSH* ssh = (WOLFSSH*)ctx; - WLOG(WS_LOG_DEBUG, "HIGHWATER MARK: (%u) %s\n", + WLOG(WS_LOG_DEBUG, "HIGHWATER MARK: (%u) %s", wolfSSH_GetHighwater(ssh), (dir == WOLFSSH_HWSIDE_RECEIVE) ? "receive" : "transmit"); @@ -169,10 +172,28 @@ static int wsHighwater(byte dir, void* ctx) } +static INLINE void HighwaterCheck(WOLFSSH* ssh, byte side) +{ + if (!ssh->highwaterFlag && ssh->highwaterMark && + (ssh->txCount >= ssh->highwaterMark || + ssh->rxCount >= ssh->highwaterMark)) { + + WLOG(WS_LOG_DEBUG, "%s over high water mark", + (side == WOLFSSH_HWSIDE_TRANSMIT) ? "Transmit" : "Receive"); + + ssh->highwaterFlag = 1; + + if (ssh->ctx->highwaterCb) + ssh->ctx->highwaterCb(side, ssh->highwaterCtx); + } +} + + static HandshakeInfo* HandshakeInfoNew(void* heap) { HandshakeInfo* newHs; + WLOG(WS_LOG_DEBUG, "Entering HandshakeInfoNew()"); newHs = (HandshakeInfo*)WMALLOC(sizeof(HandshakeInfo), heap, DYNTYPE_HS); if (newHs != NULL) { @@ -197,6 +218,7 @@ static void HandshakeInfoFree(HandshakeInfo* hs, void* heap) { (void)heap; + WLOG(WS_LOG_DEBUG, "Entering HandshakeInfoFree()"); if (hs) { WFREE(hs->serverKexInit, heap, DYNTYPE_STRING); ForceZero(hs, sizeof(HandshakeInfo)); @@ -712,6 +734,8 @@ static int GetInputText(WOLFSSH* ssh) static int SendBuffered(WOLFSSH* ssh) { + WLOG(WS_LOG_DEBUG, "Entering SendBuffered()"); + if (ssh->ctx->ioSendCb == NULL) { WLOG(WS_LOG_DEBUG, "Your IO Send callback is null, please set"); return WS_SOCKET_ERROR_E; @@ -752,6 +776,8 @@ static int SendBuffered(WOLFSSH* ssh) WLOG(WS_LOG_DEBUG, "SB: Shrinking output buffer"); ShrinkBuffer(&ssh->outputBuffer, 0); + HighwaterCheck(ssh, WOLFSSH_HWSIDE_TRANSMIT); + return WS_SUCCESS; } @@ -1066,8 +1092,6 @@ static int DoKexInit(WOLFSSH* ssh, uint8_t* buf, uint32_t len, uint32_t* idx) if (ssh == NULL || buf == NULL || len == 0 || idx == NULL) ret = WS_BAD_ARGUMENT; - if (ssh->keyingState != KEYING_UNKEYED && ssh->keyingState != KEYING_KEYED) - ret = WS_INVALID_STATE_E; /* * I don't need to save what the client sends here. I should decode * each list into a local array of IDs, and pick the one the peer is @@ -1256,10 +1280,12 @@ static int DoKexInit(WOLFSSH* ssh, uint8_t* buf, uint32_t len, uint32_t* idx) ssh->clientState = CLIENT_KEXINIT_DONE; if (ssh->keyingState == KEYING_UNKEYED) { + WLOG(WS_LOG_DEBUG, "KeyingState now KEXINIT_RECV"); ssh->keyingState = KEYING_KEXINIT_RECV; ret = SendKexInit(ssh); } else if (KEYING_KEXINIT_SENT) { + WLOG(WS_LOG_DEBUG, "KeyingState now KEXINIT_DONE"); ssh->keyingState = KEYING_KEXINIT_DONE; } } @@ -1414,8 +1440,18 @@ static int DoNewKeys(WOLFSSH* ssh, uint8_t* buf, uint32_t len, uint32_t* idx) if (ret == WS_SUCCESS) { ssh->rxCount = 0; - ssh->clientState = CLIENT_USING_KEYS; - ssh->keyingState = KEYING_KEYED; + if (ssh->keyingState == KEYING_USING_KEYS_SENT) { + ssh->clientState = CLIENT_USING_KEYS; + ssh->highwaterFlag = 0; + ssh->keyingState = KEYING_KEYED; + HandshakeInfoFree(ssh->handshake, ssh->ctx->heap); + ssh->handshake = NULL; + WLOG(WS_LOG_DEBUG, "KeyingState now KEYED"); + } + else { + ssh->keyingState = KEYING_USING_KEYS_RECV; + WLOG(WS_LOG_DEBUG, "KeyingState now USING_KEYS_RECV"); + } } return ret; @@ -2392,15 +2428,18 @@ static int DoPacket(WOLFSSH* ssh) ret = WS_SUCCESS; if (ssh->keyingState == KEYING_KEYED) { - uint32_t idSz; - ssh->handshake = HandshakeInfoNew(ssh->ctx->heap); if (ssh->handshake == NULL) { WLOG(WS_LOG_DEBUG, "Couldn't allocate handshake info"); ret = WS_MEMORY_E; } - c32toa(ssh->clientIdSz, szFlat); - wc_ShaUpdate(&ssh->handshake->hash, szFlat, LENGTH_SZ); + } + + if (ssh->keyingState == KEYING_KEYED || + ssh->keyingState == KEYING_KEXINIT_SENT) { + + uint32_t idSz; + wc_ShaUpdate(&ssh->handshake->hash, ssh->clientId, ssh->clientIdSz); @@ -2418,14 +2457,15 @@ static int DoPacket(WOLFSSH* ssh) wc_ShaUpdate(&ssh->handshake->hash, buf + idx, payloadSz); } - if (ssh->keyingState == KEYING_KEXINIT_SENT) { + if (ret == WS_SUCCESS) + ret = DoKexInit(ssh, buf + idx, payloadSz, &payloadIdx); + + if (ssh->keyingState == KEYING_KEXINIT_DONE) { wc_ShaUpdate(&ssh->handshake->hash, ssh->handshake->serverKexInit, ssh->handshake->serverKexInitSz); } - if (ret == WS_SUCCESS) - ret = DoKexInit(ssh, buf + idx, payloadSz, &payloadIdx); } break; @@ -2524,14 +2564,6 @@ static INLINE int Encrypt(WOLFSSH* ssh, uint8_t* cipher, const uint8_t* input, } ssh->txCount += sz; - if (ssh->highwaterMark && !ssh->highwaterFlag && - ssh->txCount > ssh->highwaterMark) { - - WLOG(WS_LOG_DEBUG, "Transmit over high water mark"); - if (ssh->ctx->highwaterCb) - ssh->ctx->highwaterCb(WOLFSSH_HWSIDE_TRANSMIT, ssh->highwaterCtx); - ssh->highwaterFlag = 1; - } return ret; } @@ -2564,14 +2596,7 @@ static INLINE int Decrypt(WOLFSSH* ssh, uint8_t* plain, const uint8_t* input, } ssh->rxCount += sz; - if (ssh->highwaterMark && !ssh->highwaterFlag && - ssh->rxCount > ssh->highwaterMark) { - - WLOG(WS_LOG_DEBUG, "Receive over high water mark"); - if (ssh->ctx->highwaterCb) - ssh->ctx->highwaterCb(WOLFSSH_HWSIDE_RECEIVE, ssh->highwaterCtx); - ssh->highwaterFlag = 1; - } + HighwaterCheck(ssh, WOLFSSH_HWSIDE_RECEIVE); return ret; } @@ -2811,24 +2836,18 @@ int ProcessClientVersion(WOLFSSH* ssh) clientIdSz = ssh->inputBuffer.length - SSH_PROTO_EOL_SZ; - ssh->clientId = (uint8_t*)WMALLOC(clientIdSz, + ssh->clientId = (uint8_t*)WMALLOC(clientIdSz + LENGTH_SZ, ssh->ctx->heap, DYNTYPE_STRING); if (ssh->clientId == NULL) ret = WS_MEMORY_E; else { - uint8_t flatSz[LENGTH_SZ]; - - /* Store the client version string. Will need it later during rekey */ - WMEMCPY(ssh->clientId, ssh->inputBuffer.buffer, clientIdSz); - ssh->clientIdSz = clientIdSz; - c32toa(clientIdSz, flatSz); - ret = wc_ShaUpdate(&ssh->handshake->hash, flatSz, LENGTH_SZ); + c32toa(clientIdSz, ssh->clientId); + WMEMCPY(ssh->clientId + LENGTH_SZ, ssh->inputBuffer.buffer, clientIdSz); + ssh->clientIdSz = clientIdSz + LENGTH_SZ; + ret = wc_ShaUpdate(&ssh->handshake->hash, + ssh->clientId, clientIdSz + LENGTH_SZ); } - if (ret == WS_SUCCESS) - ret = wc_ShaUpdate(&ssh->handshake->hash, ssh->inputBuffer.buffer, - clientIdSz); - ssh->inputBuffer.idx += ssh->inputBuffer.length; return ret; @@ -2997,12 +3016,19 @@ int SendKexInit(WOLFSSH* ssh) uint8_t* payload; uint32_t idx = 0; uint32_t payloadSz; - uint8_t payloadSzFlat[LENGTH_SZ]; int ret = WS_SUCCESS; if (ssh == NULL) ret = WS_BAD_ARGUMENT; + if (ssh->keyingState == KEYING_KEYED) { + ssh->handshake = HandshakeInfoNew(ssh->ctx->heap); + if (ssh->handshake == NULL) { + WLOG(WS_LOG_DEBUG, "Couldn't allocate handshake info"); + ret = WS_MEMORY_E; + } + } + if (ret == WS_SUCCESS) { payloadSz = MSG_ID_SZ + COOKIE_SZ + (LENGTH_SZ * 11) + BOOLEAN_SZ + cannedKexAlgoNamesSz + cannedKeyAlgoNamesSz + @@ -3043,7 +3069,6 @@ int SendKexInit(WOLFSSH* ssh) ssh->outputBuffer.length = idx; - c32toa(payloadSz, payloadSzFlat); if (ssh->keyingState == KEYING_KEYED) { uint8_t* buf; uint32_t bufSz = payloadSz + LENGTH_SZ; @@ -3054,14 +3079,16 @@ int SendKexInit(WOLFSSH* ssh) ret = WS_MEMORY_E; } else { - WMEMCPY(buf, payloadSzFlat, LENGTH_SZ); + c32toa(payloadSz, buf); WMEMCPY(buf + LENGTH_SZ, payload, payloadSz); ssh->handshake->serverKexInit = buf; ssh->handshake->serverKexInitSz = bufSz; } } else { - ret = wc_ShaUpdate(&ssh->handshake->hash, payloadSzFlat, LENGTH_SZ); + uint8_t szFlat[LENGTH_SZ]; + c32toa(payloadSz, szFlat); + ret = wc_ShaUpdate(&ssh->handshake->hash, szFlat, LENGTH_SZ); if (ret == WS_SUCCESS) ret = wc_ShaUpdate(&ssh->handshake->hash, payload, payloadSz); } @@ -3074,10 +3101,14 @@ int SendKexInit(WOLFSSH* ssh) ret = SendBuffered(ssh); if (ret == WS_SUCCESS) { - if (ssh->keyingState == KEYING_KEXINIT_RECV) + if (ssh->keyingState == KEYING_KEXINIT_RECV) { + WLOG(WS_LOG_DEBUG, "KeyingState now KEXINIT_DONE"); ssh->keyingState = KEYING_KEXINIT_DONE; - else if (ssh->keyingState == KEYING_KEYED) + } + else if (ssh->keyingState == KEYING_KEYED) { + WLOG(WS_LOG_DEBUG, "KeyingState now KEXINIT_SENT"); ssh->keyingState = KEYING_KEXINIT_SENT; + } } return ret; @@ -3115,6 +3146,7 @@ int SendKexDhReply(WOLFSSH* ssh) uint32_t idx; int ret; + WLOG(WS_LOG_DEBUG, "Entering SendKexDhReply()"); wc_InitDhKey(&dhKey); switch (ssh->handshake->kexId) { @@ -3288,6 +3320,7 @@ int SendKexDhReply(WOLFSSH* ssh) ret = SendNewKeys(ssh); } + WLOG(WS_LOG_DEBUG, "Leaving SendKexDhReply(), ret = %d", ret); return ret; } @@ -3298,6 +3331,7 @@ int SendNewKeys(WOLFSSH* ssh) uint32_t idx = 0; int ret = WS_SUCCESS; + WLOG(WS_LOG_DEBUG, "Entering SendNewKeys()"); if (ssh == NULL) ret = WS_BAD_ARGUMENT; @@ -3342,12 +3376,24 @@ int SendNewKeys(WOLFSSH* ssh) WLOG(WS_LOG_DEBUG, "SNK: using cipher invalid"); ret = WS_INVALID_ALGO_ID; } + } + if (ret == WS_SUCCESS) { ssh->txCount = 0; - ssh->highwaterFlag = 0; - ssh->keyingState = KEYING_USING_KEYS_SENT; + if (ssh->keyingState == KEYING_USING_KEYS_RECV) { + ssh->highwaterFlag = 0; + ssh->keyingState = KEYING_KEYED; + HandshakeInfoFree(ssh->handshake, ssh->ctx->heap); + ssh->handshake = NULL; + WLOG(WS_LOG_DEBUG, "KeyingState now KEYED"); + } + else { + ssh->keyingState = KEYING_USING_KEYS_SENT; + WLOG(WS_LOG_DEBUG, "KeyingState now USING_KEYS_SENT"); + } } + WLOG(WS_LOG_DEBUG, "Leaving SendNewKeys(), ret = %d", ret); return ret; } @@ -3774,18 +3820,31 @@ int SendChannelData(WOLFSSH* ssh, uint32_t peerChannel, if (ssh == NULL) ret = WS_BAD_ARGUMENT; - channel = ChannelFind(ssh, peerChannel, FIND_PEER); - if (channel == NULL) { - WLOG(WS_LOG_DEBUG, "Invalid peer channel"); - ret = WS_INVALID_CHANID; + if (ret == WS_SUCCESS) { + if (ssh->keyingState != KEYING_KEYED) + ret = WS_REKEYING; } - else if (channel->peerWindowSz < dataSz) { - WLOG(WS_LOG_DEBUG, "Peer window too small"); - ret = WS_OVERFLOW_E; + + if (ret == WS_SUCCESS) { + channel = ChannelFind(ssh, peerChannel, FIND_PEER); + if (channel == NULL) { + WLOG(WS_LOG_DEBUG, "Invalid peer channel"); + ret = WS_INVALID_CHANID; + } } - if (ret == WS_SUCCESS) + if (ret == WS_SUCCESS) { + uint32_t bound = min(channel->peerWindowSz, channel->peerMaxPacketSz); + + if (dataSz > bound) { + WLOG(WS_LOG_DEBUG, + "Trying to send %u, client will only accept %u, limiting", + dataSz, bound); + dataSz = bound; + } + ret = PreparePacket(ssh, MSG_ID_SZ + UINT32_SZ + LENGTH_SZ + dataSz); + } if (ret == WS_SUCCESS) { output = ssh->outputBuffer.buffer; @@ -3804,13 +3863,15 @@ int SendChannelData(WOLFSSH* ssh, uint32_t peerChannel, ret = BundlePacket(ssh); } - if (ret == WS_SUCCESS) { + if (ret == WS_SUCCESS) ret = SendBuffered(ssh); + if (ret == WS_SUCCESS) { WLOG(WS_LOG_INFO, " dataSz = %u", dataSz); WLOG(WS_LOG_INFO, " peerWindowSz = %u", channel->peerWindowSz); channel->peerWindowSz -= dataSz; WLOG(WS_LOG_INFO, " update peerWindowSz = %u", channel->peerWindowSz); + ret = dataSz; } WLOG(WS_LOG_DEBUG, "Leaving SendChannelData(), ret = %d", ret); diff --git a/src/ssh.c b/src/ssh.c index 8e3a4ecf8..183851bce 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -347,8 +347,17 @@ int wolfSSH_accept(WOLFSSH* ssh) int wolfSSH_TriggerKeyExchange(WOLFSSH* ssh) { - (void)ssh; - return WS_SUCCESS; + int ret = WS_SUCCESS; + + WLOG(WS_LOG_DEBUG, "Entering wolfSSH_TriggerKeyExchange()"); + if (ssh == NULL) + ret = WS_BAD_ARGUMENT; + + if (ret == WS_SUCCESS) + ret = SendKexInit(ssh); + + WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_TriggerKeyExchange(), ret = %d", ret); + return ret; } @@ -375,7 +384,9 @@ int wolfSSH_stream_read(WOLFSSH* ssh, uint8_t* buf, uint32_t bufSz) WMEMCPY(buf, inputBuffer->buffer + inputBuffer->idx, bufSz); inputBuffer->idx += bufSz; - if (inputBuffer->length > inputBuffer->bufferSz / 2) { + if (ssh->keyingState == KEYING_KEYED && + (inputBuffer->length > inputBuffer->bufferSz / 2)) { + uint32_t usedSz = inputBuffer->length - inputBuffer->idx; uint32_t bytesToAdd = inputBuffer->idx; int sendResult; @@ -402,7 +413,7 @@ int wolfSSH_stream_read(WOLFSSH* ssh, uint8_t* buf, uint32_t bufSz) inputBuffer->idx = 0; } - WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_stream_read(), rxd = %u", bufSz); + WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_stream_read(), rxd = %d", bufSz); return bufSz; } @@ -418,7 +429,7 @@ int wolfSSH_stream_send(WOLFSSH* ssh, uint8_t* buf, uint32_t bufSz) bytesTxd = SendChannelData(ssh, ssh->channelList->peerChannel, buf, bufSz); - WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_stream_send(), txd = %u", bytesTxd); + WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_stream_send(), txd = %d", bytesTxd); return bytesTxd; } diff --git a/wolfssh/error.h b/wolfssh/error.h index a1105875d..7cd7c2c0f 100644 --- a/wolfssh/error.h +++ b/wolfssh/error.h @@ -67,7 +67,8 @@ enum WS_ErrorCodes { WS_INVALID_CHANID = -27, WS_INVALID_USERNAME = -28, WS_CRYPTO_FAILED = -29, /* crypto action failed */ - WS_INVALID_STATE_E = -30 + WS_INVALID_STATE_E = -30, + WS_REKEYING = -31 }; diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 38d6dde61..98c111296 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -104,11 +104,15 @@ enum { #define UINT32_SZ 4 #define SSH_PROTO_SZ 7 /* "SSH-2.0" */ #define SSH_PROTO_EOL_SZ 2 /* Just the CRLF */ -#define DEFAULT_HIGHWATER_MARK ((1024 * 1024 * 1024) - (32 * 1024)) +#ifndef DEFAULT_HIGHWATER_MARK + #define DEFAULT_HIGHWATER_MARK ((1024 * 1024 * 1024) - (32 * 1024)) +#endif #ifndef DEFAULT_WINDOW_SZ #define DEFAULT_WINDOW_SZ (1024 * 1024) #endif -#define DEFAULT_MAX_PACKET_SZ (16 * 1024) +#ifndef DEFAULT_MAX_PACKET_SZ + #define DEFAULT_MAX_PACKET_SZ (16 * 1024) +#endif #define DEFAULT_NEXT_CHANNEL 13013 From 5b07c8cb1d55d4ec459ffadeb42b9e40a735bee0 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Sun, 23 Oct 2016 14:46:34 -0700 Subject: [PATCH 09/17] 1. Parse the Channel EOF message. 2. Parse the Channel Close message, and reply with a Channel Close. --- src/internal.c | 178 +++++++++++++++++++++++++++++++++++++++++++++ wolfssh/internal.h | 8 +- 2 files changed, 184 insertions(+), 2 deletions(-) diff --git a/src/internal.c b/src/internal.c index 0879ebb13..13c021348 100644 --- a/src/internal.c +++ b/src/internal.c @@ -525,6 +525,50 @@ WOLFSSH_CHANNEL* ChannelFind(WOLFSSH* ssh, uint32_t channel, uint8_t peer) } +int ChannelRemove(WOLFSSH* ssh, uint32_t channel, uint8_t peer) +{ + int ret = WS_SUCCESS; + WOLFSSH_CHANNEL* list; + + WLOG(WS_LOG_DEBUG, "Entering ChannelRemove()"); + + if (ssh == NULL) + ret = WS_BAD_ARGUMENT; + + list = ssh->channelList; + if (list == NULL) + ret = WS_INVALID_CHANID; + + if (ret == WS_SUCCESS) { + WOLFSSH_CHANNEL* prev = NULL; + uint32_t listSz = ssh->channelListSz; + + while (list && listSz) { + if (channel == ((peer == FIND_PEER) ? + list->peerChannel : list->channel)) { + if (prev == NULL) + ssh->channelList = list->next; + else + prev->next = list->next; + ChannelDelete(list, ssh->ctx->heap); + ssh->channelListSz--; + + break; + } + prev = list; + list = list->next; + listSz--; + } + + if (listSz == 0) + ret = WS_INVALID_CHANID; + } + + WLOG(WS_LOG_DEBUG, "Leaving ChannelRemove(), ret = %d", ret); + return ret; +} + + int ChannelPutData(WOLFSSH_CHANNEL* channel, uint8_t* data, uint32_t dataSz) { Buffer* inBuf; @@ -2218,6 +2262,38 @@ static int DoChannelOpen(WOLFSSH* ssh, } +static int DoChannelClose(WOLFSSH* ssh, + uint8_t* buf, uint32_t len, uint32_t* idx) +{ + WOLFSSH_CHANNEL* channel = NULL; + uint32_t begin = *idx; + uint32_t channelId; + int ret; + + WLOG(WS_LOG_DEBUG, "Entering DoChannelClose()"); + + ret = GetUint32(&channelId, buf, len, &begin); + + if (ret == WS_SUCCESS) { + *idx = begin; + + channel = ChannelFind(ssh, channelId, FIND_SELF); + if (channel == NULL) + ret = WS_INVALID_CHANID; + } + + if (ret == WS_SUCCESS) + ret = SendChannelClose(ssh, channelId); + + if (ret == WS_SUCCESS) + ret = ChannelRemove(ssh, channelId, FIND_SELF); + + + WLOG(WS_LOG_DEBUG, "Leaving DoChannelClose(), ret = %d", ret); + return ret; +} + + static int DoChannelRequest(WOLFSSH* ssh, uint8_t* buf, uint32_t len, uint32_t* idx) { @@ -2504,6 +2580,17 @@ static int DoPacket(WOLFSSH* ssh) ret = DoChannelData(ssh, buf + idx, payloadSz, &payloadIdx); break; + case MSGID_CHANNEL_EOF: + WLOG(WS_LOG_DEBUG, "Decoding MSGID_CHANNEL_EOF"); + ret = WS_SUCCESS; + break; + + case MSGID_CHANNEL_CLOSE: + WLOG(WS_LOG_DEBUG, "Decoding MSGID_CHANNEL_CLOSE"); + DoChannelClose(ssh, buf + idx, payloadSz, &payloadIdx); + ret = WS_SUCCESS; + break; + case MSGID_CHANNEL_REQUEST: WLOG(WS_LOG_DEBUG, "Decoding MSGID_CHANNEL_REQUEST"); ret = DoChannelRequest(ssh, buf + idx, payloadSz, &payloadIdx); @@ -3404,6 +3491,9 @@ int SendUnimplemented(WOLFSSH* ssh) uint32_t idx = 0; int ret = WS_SUCCESS; + WLOG(WS_LOG_DEBUG, + "Entering SendUnimplemented(), peerSeq = %u", ssh->peerSeq); + if (ssh == NULL) ret = WS_BAD_ARGUMENT; @@ -3426,6 +3516,7 @@ int SendUnimplemented(WOLFSSH* ssh) if (ret == WS_SUCCESS) ret = SendBuffered(ssh); + WLOG(WS_LOG_DEBUG, "Leaving SendUnimplemented(), ret = %d", ret); return ret; } @@ -3807,6 +3898,90 @@ int SendChannelOpenConf(WOLFSSH* ssh) } +int SendChannelEof(WOLFSSH* ssh, uint32_t peerChannelId) +{ + uint8_t* output; + uint32_t idx; + int ret = WS_SUCCESS; + WOLFSSH_CHANNEL* channel; + + WLOG(WS_LOG_DEBUG, "Entering SendChannelEof()"); + + if (ssh == NULL) + ret = WS_BAD_ARGUMENT; + + if (ret == WS_SUCCESS) { + channel = ChannelFind(ssh, peerChannelId, FIND_PEER); + if (channel == NULL) + ret = WS_INVALID_CHANID; + } + + if (ret == WS_SUCCESS) + ret = PreparePacket(ssh, MSG_ID_SZ + UINT32_SZ); + + if (ret == WS_SUCCESS) { + output = ssh->outputBuffer.buffer; + idx = ssh->outputBuffer.length; + + output[idx++] = MSGID_CHANNEL_EOF; + c32toa(channel->peerChannel, output + idx); + idx += UINT32_SZ; + + ssh->outputBuffer.length = idx; + + ret = BundlePacket(ssh); + } + + if (ret == WS_SUCCESS) + ret = SendBuffered(ssh); + + WLOG(WS_LOG_DEBUG, "Leaving SendChannelEof(), ret = %d", ret); + return ret; +} + + +int SendChannelClose(WOLFSSH* ssh, uint32_t peerChannelId) +{ + uint8_t* output; + uint32_t idx; + int ret = WS_SUCCESS; + WOLFSSH_CHANNEL* channel; + + WLOG(WS_LOG_DEBUG, "Entering SendChannelClose()"); + + if (ssh == NULL) + ret = WS_BAD_ARGUMENT; + + if (ret == WS_SUCCESS) { + channel = ChannelFind(ssh, peerChannelId, FIND_PEER); + if (channel == NULL) + ret = WS_INVALID_CHANID; + } + + if (ret == WS_SUCCESS) + ret = PreparePacket(ssh, MSG_ID_SZ + UINT32_SZ); + + if (ret == WS_SUCCESS) { + output = ssh->outputBuffer.buffer; + idx = ssh->outputBuffer.length; + + output[idx++] = MSGID_CHANNEL_CLOSE; + c32toa(channel->peerChannel, output + idx); + idx += UINT32_SZ; + + ssh->outputBuffer.length = idx; + + ret = BundlePacket(ssh); + } + + if (ret == WS_SUCCESS) + ret = SendBuffered(ssh); + + WLOG(WS_LOG_DEBUG, "Leaving SendChannelClose(), ret = %d", ret); + return ret; +} + + int SendChannelData(WOLFSSH* ssh, uint32_t peerChannel, uint8_t* data, uint32_t dataSz) { @@ -3923,6 +4098,8 @@ int SendChannelWindowAdjust(WOLFSSH* ssh, uint32_t peerChannel, } +#ifdef DEBUG_WOLFSSH + #define LINE_WIDTH 16 void DumpOctetString(const uint8_t* input, uint32_t inputSz) { @@ -3946,3 +4123,4 @@ void DumpOctetString(const uint8_t* input, uint32_t inputSz) } } +#endif diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 98c111296..d9e6155c2 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -283,8 +283,8 @@ WOLFSSH_LOCAL void SshResourceFree(WOLFSSH*, void*); WOLFSSH_LOCAL WOLFSSH_CHANNEL* ChannelNew(WOLFSSH*, uint8_t, uint32_t, uint32_t, uint32_t); WOLFSSH_LOCAL void ChannelDelete(WOLFSSH_CHANNEL*, void*); -WOLFSSH_LOCAL WOLFSSH_CHANNEL* ChannelFind(WOLFSSH*, - uint32_t, uint8_t); +WOLFSSH_LOCAL WOLFSSH_CHANNEL* ChannelFind(WOLFSSH*, uint32_t, uint8_t); +WOLFSSH_LOCAL int ChannelRemove(WOLFSSH*, uint32_t, uint8_t); WOLFSSH_LOCAL int ChannelPutData(WOLFSSH_CHANNEL*, uint8_t*, uint32_t); @@ -314,6 +314,8 @@ WOLFSSH_LOCAL int SendUserAuthBanner(WOLFSSH*); WOLFSSH_LOCAL int SendUserAuthPkOk(WOLFSSH*, const uint8_t*, uint32_t, const uint8_t*, uint32_t); WOLFSSH_LOCAL int SendChannelOpenConf(WOLFSSH*); +WOLFSSH_LOCAL int SendChannelEof(WOLFSSH*, uint32_t); +WOLFSSH_LOCAL int SendChannelClose(WOLFSSH*, uint32_t); WOLFSSH_LOCAL int SendChannelData(WOLFSSH*, uint32_t, uint8_t*, uint32_t); WOLFSSH_LOCAL int SendChannelWindowAdjust(WOLFSSH*, uint32_t, uint32_t); WOLFSSH_LOCAL int GenerateKey(uint8_t, uint8_t, uint8_t*, uint32_t, @@ -397,6 +399,8 @@ enum WS_MessageIds { MSGID_CHANNEL_OPEN_CONF = 91, MSGID_CHANNEL_WINDOW_ADJUST = 93, MSGID_CHANNEL_DATA = 94, + MSGID_CHANNEL_EOF = 96, + MSGID_CHANNEL_CLOSE = 97, MSGID_CHANNEL_REQUEST = 98 }; From bc9eff91d6c60053674bac88b1dfeb2e35d5c25e Mon Sep 17 00:00:00 2001 From: John Safranek Date: Sun, 23 Oct 2016 15:43:08 -0700 Subject: [PATCH 10/17] 1. Rename function ProcessReply(), conflicts when linking against wolfSSL not using cryptonly mode. 2. Send server version before expecting client version. --- src/internal.c | 40 ++++++++++++++++++++-------------------- src/ssh.c | 28 ++++++++++++++-------------- wolfssh/internal.h | 2 +- 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/internal.c b/src/internal.c index 13c021348..6aa8cc8c3 100644 --- a/src/internal.c +++ b/src/internal.c @@ -2802,7 +2802,7 @@ static INLINE int VerifyMac(WOLFSSH* ssh, const uint8_t* in, uint32_t inSz, } -int ProcessReply(WOLFSSH* ssh) +int DoReceive(WOLFSSH* ssh) { int ret = WS_FATAL_ERROR; int verifyResult; @@ -2904,7 +2904,7 @@ int ProcessReply(WOLFSSH* ssh) int ProcessClientVersion(WOLFSSH* ssh) { int ret; - uint32_t clientIdSz; + uint32_t idSz; if ( (ret = GetInputText(ssh)) < 0) { WLOG(WS_LOG_DEBUG, "get input text failed"); @@ -2921,20 +2921,32 @@ int ProcessClientVersion(WOLFSSH* ssh) return WS_VERSION_E; } - clientIdSz = ssh->inputBuffer.length - SSH_PROTO_EOL_SZ; + idSz = ssh->inputBuffer.length - SSH_PROTO_EOL_SZ; - ssh->clientId = (uint8_t*)WMALLOC(clientIdSz + LENGTH_SZ, + ssh->clientId = (uint8_t*)WMALLOC(idSz + LENGTH_SZ, ssh->ctx->heap, DYNTYPE_STRING); if (ssh->clientId == NULL) ret = WS_MEMORY_E; else { - c32toa(clientIdSz, ssh->clientId); - WMEMCPY(ssh->clientId + LENGTH_SZ, ssh->inputBuffer.buffer, clientIdSz); - ssh->clientIdSz = clientIdSz + LENGTH_SZ; + c32toa(idSz, ssh->clientId); + WMEMCPY(ssh->clientId + LENGTH_SZ, ssh->inputBuffer.buffer, idSz); + ssh->clientIdSz = idSz + LENGTH_SZ; ret = wc_ShaUpdate(&ssh->handshake->hash, - ssh->clientId, clientIdSz + LENGTH_SZ); + ssh->clientId, idSz + LENGTH_SZ); } + if (ret == WS_SUCCESS) { + uint8_t idSzFlat[LENGTH_SZ]; + + idSz = (uint32_t)WSTRLEN(sshIdStr) - SSH_PROTO_EOL_SZ; + c32toa(idSz, idSzFlat); + ret = wc_ShaUpdate(&ssh->handshake->hash, idSzFlat, LENGTH_SZ); + } + + if (ret == WS_SUCCESS) + ret = wc_ShaUpdate(&ssh->handshake->hash, + (const uint8_t*)sshIdStr, idSz); + ssh->inputBuffer.idx += ssh->inputBuffer.length; return ret; @@ -2955,18 +2967,6 @@ int SendServerVersion(WOLFSSH* ssh) ret = SendText(ssh, sshIdStr, sshIdStrSz); } - if (ret == WS_SUCCESS) { - uint8_t sshIdStrSzFlat[LENGTH_SZ]; - - sshIdStrSz -= SSH_PROTO_EOL_SZ; - c32toa(sshIdStrSz, sshIdStrSzFlat); - ret = wc_ShaUpdate(&ssh->handshake->hash, sshIdStrSzFlat, LENGTH_SZ); - } - - if (ret == WS_SUCCESS) - ret = wc_ShaUpdate(&ssh->handshake->hash, - (const uint8_t*)sshIdStr, sshIdStrSz); - return ret; } diff --git a/src/ssh.c b/src/ssh.c index 183851bce..642a0cac4 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -250,6 +250,15 @@ int wolfSSH_accept(WOLFSSH* ssh) switch (ssh->acceptState) { case ACCEPT_BEGIN: + if ( (ssh->error = SendServerVersion(ssh)) < WS_SUCCESS) { + WLOG(WS_LOG_DEBUG, acceptError, + "CLIENT_VERSION_DONE", ssh->error); + return WS_FATAL_ERROR; + } + ssh->acceptState = ACCEPT_SERVER_VERSION_SENT; + WLOG(WS_LOG_DEBUG, acceptState, "SERVER_VERSION_SENT"); + + case ACCEPT_SERVER_VERSION_SENT: while (ssh->clientState < CLIENT_VERSION_DONE) { if ( (ssh->error = ProcessClientVersion(ssh)) < WS_SUCCESS) { WLOG(WS_LOG_DEBUG, acceptError, "BEGIN", ssh->error); @@ -260,17 +269,8 @@ int wolfSSH_accept(WOLFSSH* ssh) WLOG(WS_LOG_DEBUG, acceptState, "CLIENT_VERSION_DONE"); case ACCEPT_CLIENT_VERSION_DONE: - if ( (ssh->error = SendServerVersion(ssh)) < WS_SUCCESS) { - WLOG(WS_LOG_DEBUG, acceptError, - "CLIENT_VERSION_DONE", ssh->error); - return WS_FATAL_ERROR; - } - ssh->acceptState = ACCEPT_SERVER_VERSION_SENT; - WLOG(WS_LOG_DEBUG, acceptState, "SERVER_VERSION_SENT"); - - case ACCEPT_SERVER_VERSION_SENT: while (ssh->keyingState < KEYING_KEYED) { - if ( (ssh->error = ProcessReply(ssh)) < WS_SUCCESS) { + if ( (ssh->error = DoReceive(ssh)) < WS_SUCCESS) { WLOG(WS_LOG_DEBUG, acceptError, "SERVER_VERSION_SENT", ssh->error); return WS_FATAL_ERROR; @@ -281,7 +281,7 @@ int wolfSSH_accept(WOLFSSH* ssh) case ACCEPT_KEYED: while (ssh->clientState < CLIENT_USERAUTH_REQUEST_DONE) { - if ( (ssh->error = ProcessReply(ssh)) < 0) { + if ( (ssh->error = DoReceive(ssh)) < 0) { WLOG(WS_LOG_DEBUG, acceptError, "KEYED", ssh->error); return WS_FATAL_ERROR; @@ -302,7 +302,7 @@ int wolfSSH_accept(WOLFSSH* ssh) case ACCEPT_SERVER_USERAUTH_ACCEPT_SENT: while (ssh->clientState < CLIENT_USERAUTH_DONE) { - if ( (ssh->error = ProcessReply(ssh)) < 0) { + if ( (ssh->error = DoReceive(ssh)) < 0) { WLOG(WS_LOG_DEBUG, acceptError, "SERVER_USERAUTH_ACCEPT_SENT", ssh->error); return WS_FATAL_ERROR; @@ -322,7 +322,7 @@ int wolfSSH_accept(WOLFSSH* ssh) case ACCEPT_SERVER_USERAUTH_SENT: while (ssh->clientState < CLIENT_DONE) { - if ( (ssh->error = ProcessReply(ssh)) < 0) { + if ( (ssh->error = DoReceive(ssh)) < 0) { WLOG(WS_LOG_DEBUG, acceptError, "SERVER_USERAUTH_SENT", ssh->error); return WS_FATAL_ERROR; @@ -373,7 +373,7 @@ int wolfSSH_stream_read(WOLFSSH* ssh, uint8_t* buf, uint32_t bufSz) inputBuffer = &ssh->channelList->inputBuffer; while (inputBuffer->length - inputBuffer->idx == 0) { - int ret = ProcessReply(ssh); + int ret = DoReceive(ssh); if (ret < 0) { WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_stream_read(), ret = %d", ret); return ret; diff --git a/wolfssh/internal.h b/wolfssh/internal.h index d9e6155c2..7d6901736 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -297,7 +297,7 @@ WOLFSSH_LOCAL int wsEmbedSend(WOLFSSH*, void*, uint32_t, void*); #endif /* WOLFSSH_USER_IO */ -WOLFSSH_LOCAL int ProcessReply(WOLFSSH*); +WOLFSSH_LOCAL int DoReceive(WOLFSSH*); WOLFSSH_LOCAL int ProcessClientVersion(WOLFSSH*); WOLFSSH_LOCAL int SendServerVersion(WOLFSSH*); WOLFSSH_LOCAL int SendKexInit(WOLFSSH*); From 184182d15263b449f1619108177796942e7f21eb Mon Sep 17 00:00:00 2001 From: John Safranek Date: Sun, 23 Oct 2016 16:00:40 -0700 Subject: [PATCH 11/17] Prep for v1.0.0 1. Bump version number. 2. Update readme. 3. Move the coding standard to a notes file. --- README.md | 54 +++++++++++++++++++++-------------------------- configure.ac | 26 +++++++++++------------ notes.md | 35 ++++++++++++++++++++++++++++++ wolfssh/version.h | 4 ++-- 4 files changed, 74 insertions(+), 45 deletions(-) create mode 100644 notes.md diff --git a/README.md b/README.md index 6e7d6e860..2b165d8a6 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,24 @@ wolfssh wolfSSL's Embeddable SSH Server +dependencies +------------ + +wolfSSH is dependent on wolfCrypt. The simplest configuration of wolfSSL +required for wolfSSH is the default build. + + $ cd wolfssl + $ ./configure [OPTIONS] + $ make check + $ sudo make install + +To use the key generation function in wolfSSH, wolfSSL will need to be +configured with keygen: `--enable-keygen`. + +If the bulk of wolfSSL code isn't desired, wolfSSL can be configured with +the crypto only option: `--enable-cryptonly`. + + building -------- @@ -17,6 +35,7 @@ The `autogen.sh` script only has to be run the first time after cloning the repository. If you have already run it or are using code from a source archive, you should skip it. + examples -------- @@ -38,6 +57,7 @@ The server will send a canned banner to the client: Characters typed into the client will be echoed to the screen by the server. If the characters are echoed twice, the client has local echo enabled. + testing notes ------------- @@ -63,35 +83,9 @@ To use public key authentication use the command line: Where the user can be `gretel` or `hansel`. -coding standard ---------------- - -1. Exceptions are allowed with good reason. - -2. Follow the existing style. - -3. Try not to shorthand variables, except for ijk as indicies. - -4. Lengths of arrays should have the array name followed by Sz. - -5. Single return per function. - -6. Check all incoming parameters. - -7. No gotos. - -8. Check all return codes. It feels a little tedious, but the preferred method -is running checks against success. This way if a function returns an error, the -code will drop to the end. - -``` - ret = functionCall(parameter); - if (ret == SUCCESS) - ret = secondFunctionCall(otherParameter); - if (ret == SUCCESS) - ret = thirdFunctionCall(aParameter, anotherParameter); - cleanUp(); - return ret; -``` +release notes +------------- +### wolfSSH v1.0.0 (10/24/2016) +Initial release. diff --git a/configure.ac b/configure.ac index abadcbff0..8b70e30a1 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ # Copyright (C) 2014-2016 wolfSSL Inc. # All right reserved. -AC_INIT([wolfssh], [0.2.0], [http://wolfssl.com], [wolfssh]) +AC_INIT([wolfssh], [1.0.0], [http://wolfssl.com], [wolfssh]) AC_PREREQ([2.63]) AC_CONFIG_AUX_DIR([build-aux]) @@ -17,18 +17,18 @@ AC_ARG_PROGRAM AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_HEADERS([src/config.h]) -WOLFSSH_LIBRARY_VERSION=1:2:0 -# | | | -# +------+ | +---+ -# | | | -# current:revision:age -# | | | -# | | +- increment if interfaces have been added -# | | set to zero if interfaces have been removed -# | | or changed -# | +- increment if source code has changed -# | set to zero if current is incremented -# +- increment if interfaces have been added, removed or changed +WOLFSSH_LIBRARY_VERSION=2:0:1 +# | | | +# +------+ | +---+ +# | | | +# current:revision:age +# | | | +# | | +- increment if interfaces have been added +# | | set to zero if interfaces have been removed +# | | or changed +# | +- increment if source code has changed +# | set to zero if current is incremented +# +- increment if interfaces have been added, removed or changed AC_SUBST([WOLFSSH_LIBRARY_VERSION]) LT_PREREQ([2.2]) diff --git a/notes.md b/notes.md new file mode 100644 index 000000000..61b98d76a --- /dev/null +++ b/notes.md @@ -0,0 +1,35 @@ +wolfssh notes +============= + +coding standard +--------------- + +1. Exceptions are allowed with good reason. + +2. Follow the existing style. + +3. Try not to shorthand variables, except for ijk as indicies. + +4. Lengths of arrays should have the array name followed by Sz. + +5. Single return per function. + +6. Check all incoming parameters. + +7. No gotos. + +8. Check all return codes. It feels a little tedious, but the preferred method +is running checks against success. This way if a function returns an error, the +code will drop to the end. + +``` + ret = functionCall(parameter); + if (ret == SUCCESS) + ret = secondFunctionCall(otherParameter); + if (ret == SUCCESS) + ret = thirdFunctionCall(aParameter, anotherParameter); + cleanUp(); + return ret; +``` + + diff --git a/wolfssh/version.h b/wolfssh/version.h index eba07150d..24216d077 100644 --- a/wolfssh/version.h +++ b/wolfssh/version.h @@ -33,8 +33,8 @@ extern "C" { #endif -#define LIBWOLFSSH_VERSION_STRING "0.2.0" -#define LIBWOLFSSH_VERSION_HEX 0x00002000 +#define LIBWOLFSSH_VERSION_STRING "1.0.0" +#define LIBWOLFSSH_VERSION_HEX 0x01000000 #ifdef __cplusplus } From ccc1101612e7292c9ddaf2be7ec2a9df36aefb56 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Sun, 23 Oct 2016 16:14:17 -0700 Subject: [PATCH 12/17] Fix where the result of sending a channel close was getting replaced with success. --- src/internal.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/internal.c b/src/internal.c index 6aa8cc8c3..8266a8732 100644 --- a/src/internal.c +++ b/src/internal.c @@ -2587,8 +2587,7 @@ static int DoPacket(WOLFSSH* ssh) case MSGID_CHANNEL_CLOSE: WLOG(WS_LOG_DEBUG, "Decoding MSGID_CHANNEL_CLOSE"); - DoChannelClose(ssh, buf + idx, payloadSz, &payloadIdx); - ret = WS_SUCCESS; + ret = DoChannelClose(ssh, buf + idx, payloadSz, &payloadIdx); break; case MSGID_CHANNEL_REQUEST: From 718a4f4b403f4c5baae2dfb41780b4b9cd68ba48 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 24 Oct 2016 11:51:42 -0700 Subject: [PATCH 13/17] cleanup accept state machine --- src/ssh.c | 8 ++++---- wolfssh/internal.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ssh.c b/src/ssh.c index 642a0cac4..56ca0897d 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -251,8 +251,7 @@ int wolfSSH_accept(WOLFSSH* ssh) case ACCEPT_BEGIN: if ( (ssh->error = SendServerVersion(ssh)) < WS_SUCCESS) { - WLOG(WS_LOG_DEBUG, acceptError, - "CLIENT_VERSION_DONE", ssh->error); + WLOG(WS_LOG_DEBUG, acceptError, "BEGIN", ssh->error); return WS_FATAL_ERROR; } ssh->acceptState = ACCEPT_SERVER_VERSION_SENT; @@ -261,7 +260,8 @@ int wolfSSH_accept(WOLFSSH* ssh) case ACCEPT_SERVER_VERSION_SENT: while (ssh->clientState < CLIENT_VERSION_DONE) { if ( (ssh->error = ProcessClientVersion(ssh)) < WS_SUCCESS) { - WLOG(WS_LOG_DEBUG, acceptError, "BEGIN", ssh->error); + WLOG(WS_LOG_DEBUG, acceptError, + "SERVER_VERSION_SENT", ssh->error); return WS_FATAL_ERROR; } } @@ -272,7 +272,7 @@ int wolfSSH_accept(WOLFSSH* ssh) while (ssh->keyingState < KEYING_KEYED) { if ( (ssh->error = DoReceive(ssh)) < WS_SUCCESS) { WLOG(WS_LOG_DEBUG, acceptError, - "SERVER_VERSION_SENT", ssh->error); + "CLIENT_VERSION_DONE", ssh->error); return WS_FATAL_ERROR; } } diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 7d6901736..f96581002 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -342,8 +342,8 @@ enum KeyingStates { enum AcceptStates { ACCEPT_BEGIN = 0, - ACCEPT_CLIENT_VERSION_DONE, ACCEPT_SERVER_VERSION_SENT, + ACCEPT_CLIENT_VERSION_DONE, ACCEPT_KEYED, ACCEPT_CLIENT_USERAUTH_REQUEST_DONE, ACCEPT_SERVER_USERAUTH_ACCEPT_SENT, From c183000b932a28a27935acafbda81e2df28bfece Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 24 Oct 2016 13:42:58 -0700 Subject: [PATCH 14/17] 1. Manage case where Client KEX Init arrives in same recv() as the client version string. 2. Shrink the receive buffer after reading the client version string. 3. Resize the buffer correctly when needed data is already in the input buffer and grab the remainder as expected. --- src/internal.c | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/src/internal.c b/src/internal.c index 8266a8732..5e6172ec7 100644 --- a/src/internal.c +++ b/src/internal.c @@ -645,7 +645,7 @@ int GrowBuffer(Buffer* buf, uint32_t sz, uint32_t usedSz) /*WLOG(WS_LOG_DEBUG, "GB: resizing buffer");*/ if (buf->length > 0) - WMEMCPY(newBuffer, buf->buffer + buf->idx, buf->length); + WMEMCPY(newBuffer, buf->buffer + buf->idx, usedSz); if (!buf->dynamicFlag) buf->dynamicFlag = 1; @@ -738,11 +738,12 @@ static int Receive(WOLFSSH* ssh, uint8_t* buf, uint32_t sz) } -static int GetInputText(WOLFSSH* ssh) +static int GetInputText(WOLFSSH* ssh, uint8_t** pEol) { int gotLine = 0; int inSz = 255; int in; + char *eol; if (GrowBuffer(&ssh->inputBuffer, inSz, 0) < 0) return WS_MEMORY_E; @@ -763,16 +764,18 @@ static int GetInputText(WOLFSSH* ssh) ssh->inputBuffer.length += in; inSz -= in; - if (ssh->inputBuffer.length > 2) { - if (ssh->inputBuffer.buffer[ssh->inputBuffer.length - 2] == '\r' && - ssh->inputBuffer.buffer[ssh->inputBuffer.length - 1] == '\n') { + eol = WSTRNSTR((const char*)ssh->inputBuffer.buffer, "\r\n", + ssh->inputBuffer.length); - gotLine = 1; - } - } - } while (!gotLine); + if (eol) + gotLine = 1; - return WS_SUCCESS; + } while (!gotLine && inSz); + + if (pEol) + *pEol = (uint8_t*)eol; + + return (gotLine ? WS_SUCCESS : WS_VERSION_E); } @@ -871,7 +874,7 @@ static int GetInputData(WOLFSSH* ssh, uint32_t size) */ if (inSz <= 0) - return WS_BUFFER_E; + return WS_SUCCESS; /* * If we need more space than there is left in the buffer grow buffer. @@ -2904,12 +2907,18 @@ int ProcessClientVersion(WOLFSSH* ssh) { int ret; uint32_t idSz; + uint8_t* eol; - if ( (ret = GetInputText(ssh)) < 0) { + if ( (ret = GetInputText(ssh, &eol)) < 0) { WLOG(WS_LOG_DEBUG, "get input text failed"); return ret; } + if (eol == NULL) { + WLOG(WS_LOG_DEBUG, "invalid EOL"); + return WS_VERSION_E; + } + if (WSTRNCASECMP((char*)ssh->inputBuffer.buffer, sshIdStr, SSH_PROTO_SZ) == 0) { @@ -2920,7 +2929,9 @@ int ProcessClientVersion(WOLFSSH* ssh) return WS_VERSION_E; } - idSz = ssh->inputBuffer.length - SSH_PROTO_EOL_SZ; + *eol = 0; + + idSz = (uint32_t)WSTRLEN((char*)ssh->inputBuffer.buffer); ssh->clientId = (uint8_t*)WMALLOC(idSz + LENGTH_SZ, ssh->ctx->heap, DYNTYPE_STRING); @@ -2933,6 +2944,7 @@ int ProcessClientVersion(WOLFSSH* ssh) ret = wc_ShaUpdate(&ssh->handshake->hash, ssh->clientId, idSz + LENGTH_SZ); } + ssh->inputBuffer.idx += idSz + SSH_PROTO_EOL_SZ; if (ret == WS_SUCCESS) { uint8_t idSzFlat[LENGTH_SZ]; @@ -2946,7 +2958,7 @@ int ProcessClientVersion(WOLFSSH* ssh) ret = wc_ShaUpdate(&ssh->handshake->hash, (const uint8_t*)sshIdStr, idSz); - ssh->inputBuffer.idx += ssh->inputBuffer.length; + ShrinkBuffer(&ssh->inputBuffer, 0); return ret; } From c802b331bd3ab3a5d152b8bd02ddcc2d58f47aa3 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 24 Oct 2016 15:03:54 -0700 Subject: [PATCH 15/17] Check all return codes on wolfCrypt functions. --- src/internal.c | 672 +++++++++++++++++++++++++++++-------------------- 1 file changed, 401 insertions(+), 271 deletions(-) diff --git a/src/internal.c b/src/internal.c index 5e6172ec7..40a6b10db 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1516,6 +1516,7 @@ int GenerateKey(uint8_t hashId, uint8_t keyId, uint8_t kPad = 0; uint8_t pad = 0; uint8_t kSzFlat[LENGTH_SZ]; + int ret; (void)hashId; @@ -1534,52 +1535,80 @@ int GenerateKey(uint8_t hashId, uint8_t keyId, blocks = keySz / SHA_DIGEST_SIZE; remainder = keySz % SHA_DIGEST_SIZE; - wc_InitSha(&sha); - wc_ShaUpdate(&sha, kSzFlat, LENGTH_SZ); - if (kPad) wc_ShaUpdate(&sha, &pad, 1); - wc_ShaUpdate(&sha, k, kSz); - wc_ShaUpdate(&sha, h, hSz); - wc_ShaUpdate(&sha, &keyId, sizeof(keyId)); - wc_ShaUpdate(&sha, sessionId, sessionIdSz); - - if (blocks == 0) { - if (remainder > 0) { - uint8_t lastBlock[SHA_DIGEST_SIZE]; - wc_ShaFinal(&sha, lastBlock); - WMEMCPY(key, lastBlock, remainder); - } - } - else { - uint32_t runningKeySz, curBlock; - - wc_ShaFinal(&sha, key); - runningKeySz = SHA_DIGEST_SIZE; - - for (curBlock = 1; curBlock < blocks; curBlock++) { - wc_InitSha(&sha); - wc_ShaUpdate(&sha, kSzFlat, LENGTH_SZ); - if (kPad) wc_ShaUpdate(&sha, &pad, 1); - wc_ShaUpdate(&sha, k, kSz); - wc_ShaUpdate(&sha, h, hSz); - wc_ShaUpdate(&sha, key, runningKeySz); - wc_ShaFinal(&sha, key + runningKeySz); - runningKeySz += SHA_DIGEST_SIZE; + ret = wc_InitSha(&sha); + if (ret == WS_SUCCESS) + ret = wc_ShaUpdate(&sha, kSzFlat, LENGTH_SZ); + if (ret == WS_SUCCESS && kPad) + ret = wc_ShaUpdate(&sha, &pad, 1); + if (ret == WS_SUCCESS) + ret = wc_ShaUpdate(&sha, k, kSz); + if (ret == WS_SUCCESS) + ret = wc_ShaUpdate(&sha, h, hSz); + if (ret == WS_SUCCESS) + ret = wc_ShaUpdate(&sha, &keyId, sizeof(keyId)); + if (ret == WS_SUCCESS) + ret = wc_ShaUpdate(&sha, sessionId, sessionIdSz); + + if (ret == WS_SUCCESS) { + if (blocks == 0) { + if (remainder > 0) { + uint8_t lastBlock[SHA_DIGEST_SIZE]; + ret = wc_ShaFinal(&sha, lastBlock); + if (ret == WS_SUCCESS) + WMEMCPY(key, lastBlock, remainder); + } } + else { + uint32_t runningKeySz, curBlock; + + runningKeySz = SHA_DIGEST_SIZE; + ret = wc_ShaFinal(&sha, key); + + for (curBlock = 1; curBlock < blocks; curBlock++) { + ret = wc_InitSha(&sha); + if (ret != WS_SUCCESS) break; + ret = wc_ShaUpdate(&sha, kSzFlat, LENGTH_SZ); + if (ret != WS_SUCCESS) break; + if (kPad) + ret = wc_ShaUpdate(&sha, &pad, 1); + if (ret != WS_SUCCESS) break; + ret = wc_ShaUpdate(&sha, k, kSz); + if (ret != WS_SUCCESS) break; + ret = wc_ShaUpdate(&sha, h, hSz); + if (ret != WS_SUCCESS) break; + ret = wc_ShaUpdate(&sha, key, runningKeySz); + if (ret != WS_SUCCESS) break; + ret = wc_ShaFinal(&sha, key + runningKeySz); + if (ret != WS_SUCCESS) break; + runningKeySz += SHA_DIGEST_SIZE; + } - if (remainder > 0) { - uint8_t lastBlock[SHA_DIGEST_SIZE]; - wc_InitSha(&sha); - wc_ShaUpdate(&sha, kSzFlat, LENGTH_SZ); - if (kPad) wc_ShaUpdate(&sha, &pad, 1); - wc_ShaUpdate(&sha, k, kSz); - wc_ShaUpdate(&sha, h, hSz); - wc_ShaUpdate(&sha, key, runningKeySz); - wc_ShaFinal(&sha, lastBlock); - WMEMCPY(key + runningKeySz, lastBlock, remainder); + if (remainder > 0) { + uint8_t lastBlock[SHA_DIGEST_SIZE]; + if (ret == WS_SUCCESS) + ret = wc_InitSha(&sha); + if (ret == WS_SUCCESS) + ret = wc_ShaUpdate(&sha, kSzFlat, LENGTH_SZ); + if (ret == WS_SUCCESS && kPad) + ret = wc_ShaUpdate(&sha, &pad, 1); + if (ret == WS_SUCCESS) + ret = wc_ShaUpdate(&sha, k, kSz); + if (ret == WS_SUCCESS) + ret = wc_ShaUpdate(&sha, h, hSz); + if (ret == WS_SUCCESS) + ret = wc_ShaUpdate(&sha, key, runningKeySz); + if (ret == WS_SUCCESS) + ret = wc_ShaFinal(&sha, lastBlock); + if (ret == WS_SUCCESS) + WMEMCPY(key + runningKeySz, lastBlock, remainder); + } } } - return 0; + if (ret != WS_SUCCESS) + ret = WS_CRYPTO_FAILED; + + return ret; } @@ -1587,37 +1616,45 @@ static int GenerateKeys(WOLFSSH* ssh) { Keys* cK; Keys* sK; + int ret = WS_SUCCESS; if (ssh == NULL) - return WS_BAD_ARGUMENT; + ret = WS_BAD_ARGUMENT; + else { + cK = &ssh->handshake->clientKeys; + sK = &ssh->handshake->serverKeys; + } - cK = &ssh->handshake->clientKeys; - sK = &ssh->handshake->serverKeys; - - GenerateKey(0, 'A', - cK->iv, cK->ivSz, - ssh->k, ssh->kSz, ssh->h, ssh->hSz, - ssh->sessionId, ssh->sessionIdSz); - GenerateKey(0, 'B', - sK->iv, sK->ivSz, - ssh->k, ssh->kSz, ssh->h, ssh->hSz, - ssh->sessionId, ssh->sessionIdSz); - GenerateKey(0, 'C', - cK->encKey, cK->encKeySz, - ssh->k, ssh->kSz, ssh->h, ssh->hSz, - ssh->sessionId, ssh->sessionIdSz); - GenerateKey(0, 'D', - sK->encKey, sK->encKeySz, - ssh->k, ssh->kSz, ssh->h, ssh->hSz, - ssh->sessionId, ssh->sessionIdSz); - GenerateKey(0, 'E', - cK->macKey, cK->macKeySz, - ssh->k, ssh->kSz, ssh->h, ssh->hSz, - ssh->sessionId, ssh->sessionIdSz); - GenerateKey(0, 'F', - sK->macKey, sK->macKeySz, - ssh->k, ssh->kSz, ssh->h, ssh->hSz, - ssh->sessionId, ssh->sessionIdSz); + if (ret == WS_SUCCESS) + ret = GenerateKey(0, 'A', + cK->iv, cK->ivSz, + ssh->k, ssh->kSz, ssh->h, ssh->hSz, + ssh->sessionId, ssh->sessionIdSz); + if (ret == WS_SUCCESS) + ret = GenerateKey(0, 'B', + sK->iv, sK->ivSz, + ssh->k, ssh->kSz, ssh->h, ssh->hSz, + ssh->sessionId, ssh->sessionIdSz); + if (ret == WS_SUCCESS) + ret = GenerateKey(0, 'C', + cK->encKey, cK->encKeySz, + ssh->k, ssh->kSz, ssh->h, ssh->hSz, + ssh->sessionId, ssh->sessionIdSz); + if (ret == WS_SUCCESS) + ret = GenerateKey(0, 'D', + sK->encKey, sK->encKeySz, + ssh->k, ssh->kSz, ssh->h, ssh->hSz, + ssh->sessionId, ssh->sessionIdSz); + if (ret == WS_SUCCESS) + ret = GenerateKey(0, 'E', + cK->macKey, cK->macKeySz, + ssh->k, ssh->kSz, ssh->h, ssh->hSz, + ssh->sessionId, ssh->sessionIdSz); + if (ret == WS_SUCCESS) + ret = GenerateKey(0, 'F', + sK->macKey, sK->macKeySz, + ssh->k, ssh->kSz, ssh->h, ssh->hSz, + ssh->sessionId, ssh->sessionIdSz); #ifdef SHOW_SECRETS printf("\n** Showing Secrets **\nK:\n"); @@ -1641,7 +1678,7 @@ static int GenerateKeys(WOLFSSH* ssh) printf("\n"); #endif /* SHOW_SECRETS */ - return 0; + return ret; } @@ -1950,8 +1987,9 @@ static int DoUserAuthRequestRsa(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk, if (ret == WS_SUCCESS) { n = pk->publicKey + i; - wc_InitRsaKey(&key, ssh->ctx->heap); - ret = wc_RsaPublicKeyDecodeRaw(n, nSz, e, eSz, &key); + ret = wc_InitRsaKey(&key, ssh->ctx->heap); + if (ret == 0) + ret = wc_RsaPublicKeyDecodeRaw(n, nSz, e, eSz, &key); if (ret != 0) { WLOG(WS_LOG_DEBUG, "Could not decode public key"); ret = WS_CRYPTO_FAILED; @@ -2103,34 +2141,49 @@ static int DoUserAuthRequestPublicKey(WOLFSSH* ssh, WS_UserAuthData* authData, volatile int compare; volatile int sizeCompare; - wc_InitSha(&sha); - c32toa(ssh->sessionIdSz, digest); - wc_ShaUpdate(&sha, digest, UINT32_SZ); - wc_ShaUpdate(&sha, ssh->sessionId, ssh->sessionIdSz); - digest[0] = MSGID_USERAUTH_REQUEST; - wc_ShaUpdate(&sha, digest, MSG_ID_SZ); + ret = wc_InitSha(&sha); + if (ret == 0) { + c32toa(ssh->sessionIdSz, digest); + ret = wc_ShaUpdate(&sha, digest, UINT32_SZ); + } + if (ret == WS_SUCCESS) + ret = wc_ShaUpdate(&sha, ssh->sessionId, ssh->sessionIdSz); + + if (ret == 0) { + digest[0] = MSGID_USERAUTH_REQUEST; + ret = wc_ShaUpdate(&sha, digest, MSG_ID_SZ); + } /* The rest of the fields in the signature are already * in the buffer. Just need to account for the sizes. */ - wc_ShaUpdate(&sha, pk->dataToSign, - authData->usernameSz + authData->serviceNameSz + - authData->authNameSz + BOOLEAN_SZ + - pk->publicKeyTypeSz + pk->publicKeySz + - (UINT32_SZ * 5)); - wc_ShaFinal(&sha, digest); + if (ret == 0) + ret = wc_ShaUpdate(&sha, pk->dataToSign, + authData->usernameSz + + authData->serviceNameSz + + authData->authNameSz + BOOLEAN_SZ + + pk->publicKeyTypeSz + pk->publicKeySz + + (UINT32_SZ * 5)); + if (ret == 0) + ret = wc_ShaFinal(&sha, digest); encDigestSz = wc_EncodeSignature(encDigest, digest, SHA_DIGEST_SIZE, SHAh); - compare = ConstantCompare(encDigest, checkDigest, encDigestSz); - sizeCompare = encDigestSz != checkDigestSz; + if (ret != 0) + ret = WS_CRYPTO_FAILED; - if (compare || sizeCompare || ret < 0) { - WLOG(WS_LOG_DEBUG, "DUARPK: signature compare failure"); - ret = SendUserAuthFailure(ssh, 0); - } - else { - ssh->clientState = CLIENT_USERAUTH_DONE; + if (ret == WS_SUCCESS) { + compare = ConstantCompare(encDigest, checkDigest, + encDigestSz); + sizeCompare = encDigestSz != checkDigestSz; + + if (compare || sizeCompare || ret < 0) { + WLOG(WS_LOG_DEBUG, "DUARPK: signature compare failure"); + ret = SendUserAuthFailure(ssh, 0); + } + else { + ssh->clientState = CLIENT_USERAUTH_DONE; + } } } } @@ -2514,35 +2567,48 @@ static int DoPacket(WOLFSSH* ssh) } } - if (ssh->keyingState == KEYING_KEYED || - ssh->keyingState == KEYING_KEXINIT_SENT) { + if (ret == WS_SUCCESS && + (ssh->keyingState == KEYING_KEYED || + ssh->keyingState == KEYING_KEXINIT_SENT)) { uint32_t idSz; - wc_ShaUpdate(&ssh->handshake->hash, + ret = wc_ShaUpdate(&ssh->handshake->hash, ssh->clientId, ssh->clientIdSz); - idSz = (uint32_t)WSTRLEN(sshIdStr) - SSH_PROTO_EOL_SZ; - c32toa(idSz, szFlat); - wc_ShaUpdate(&ssh->handshake->hash, szFlat, LENGTH_SZ); - wc_ShaUpdate(&ssh->handshake->hash, - (const uint8_t*)sshIdStr, idSz); + + if (ret == WS_SUCCESS) { + idSz = (uint32_t)WSTRLEN(sshIdStr) - SSH_PROTO_EOL_SZ; + c32toa(idSz, szFlat); + ret = wc_ShaUpdate(&ssh->handshake->hash, szFlat, + LENGTH_SZ); + } + if (ret == WS_SUCCESS) + ret = wc_ShaUpdate(&ssh->handshake->hash, + (const uint8_t*)sshIdStr, idSz); } if (ret == WS_SUCCESS) { c32toa(payloadSz + sizeof(msg), szFlat); - wc_ShaUpdate(&ssh->handshake->hash, szFlat, LENGTH_SZ); - wc_ShaUpdate(&ssh->handshake->hash, &msg, sizeof(msg)); - wc_ShaUpdate(&ssh->handshake->hash, buf + idx, payloadSz); + ret = wc_ShaUpdate(&ssh->handshake->hash, szFlat, + LENGTH_SZ); } + if (ret == WS_SUCCESS) + ret = wc_ShaUpdate(&ssh->handshake->hash, &msg, + sizeof(msg)); + if (ret == WS_SUCCESS) + ret = wc_ShaUpdate(&ssh->handshake->hash, buf + idx, + payloadSz); if (ret == WS_SUCCESS) ret = DoKexInit(ssh, buf + idx, payloadSz, &payloadIdx); - if (ssh->keyingState == KEYING_KEXINIT_DONE) { - wc_ShaUpdate(&ssh->handshake->hash, - ssh->handshake->serverKexInit, - ssh->handshake->serverKexInitSz); + if (ret == WS_SUCCESS && + ssh->keyingState == KEYING_KEXINIT_DONE) { + + ret = wc_ShaUpdate(&ssh->handshake->hash, + ssh->handshake->serverKexInit, + ssh->handshake->serverKexInitSz); } } @@ -2695,7 +2761,7 @@ static INLINE int CreateMac(WOLFSSH* ssh, const uint8_t* in, uint32_t inSz, uint8_t* mac) { uint8_t flatSeq[LENGTH_SZ]; - int ret = WS_SUCCESS; + int ret; c32toa(ssh->seq++, flatSeq); @@ -2704,6 +2770,7 @@ static INLINE int CreateMac(WOLFSSH* ssh, const uint8_t* in, uint32_t inSz, /* Need to MAC the sequence number and the unencrypted packet */ switch (ssh->macId) { case ID_NONE: + ret = WS_SUCCESS; break; case ID_HMAC_SHA1_96: @@ -2711,13 +2778,17 @@ static INLINE int CreateMac(WOLFSSH* ssh, const uint8_t* in, uint32_t inSz, Hmac hmac; uint8_t digest[SHA_DIGEST_SIZE]; - wc_HmacSetKey(&hmac, SHA, - ssh->serverKeys.macKey, - ssh->serverKeys.macKeySz); - wc_HmacUpdate(&hmac, flatSeq, sizeof(flatSeq)); - wc_HmacUpdate(&hmac, in, inSz); - wc_HmacFinal(&hmac, digest); - WMEMCPY(mac, digest, SHA1_96_SZ); + ret = wc_HmacSetKey(&hmac, SHA, + ssh->serverKeys.macKey, + ssh->serverKeys.macKeySz); + if (ret == WS_SUCCESS) + ret = wc_HmacUpdate(&hmac, flatSeq, sizeof(flatSeq)); + if (ret == WS_SUCCESS) + ret = wc_HmacUpdate(&hmac, in, inSz); + if (ret == WS_SUCCESS) + ret = wc_HmacFinal(&hmac, digest); + if (ret == WS_SUCCESS) + WMEMCPY(mac, digest, SHA1_96_SZ); } break; @@ -2725,12 +2796,15 @@ static INLINE int CreateMac(WOLFSSH* ssh, const uint8_t* in, uint32_t inSz, { Hmac hmac; - wc_HmacSetKey(&hmac, SHA, - ssh->serverKeys.macKey, - ssh->serverKeys.macKeySz); - wc_HmacUpdate(&hmac, flatSeq, sizeof(flatSeq)); - wc_HmacUpdate(&hmac, in, inSz); - wc_HmacFinal(&hmac, mac); + ret = wc_HmacSetKey(&hmac, SHA, + ssh->serverKeys.macKey, + ssh->serverKeys.macKeySz); + if (ret == WS_SUCCESS) + ret = wc_HmacUpdate(&hmac, flatSeq, sizeof(flatSeq)); + if (ret == WS_SUCCESS) + ret = wc_HmacUpdate(&hmac, in, inSz); + if (ret == WS_SUCCESS) + ret = wc_HmacFinal(&hmac, mac); } break; @@ -2738,12 +2812,15 @@ static INLINE int CreateMac(WOLFSSH* ssh, const uint8_t* in, uint32_t inSz, { Hmac hmac; - wc_HmacSetKey(&hmac, SHA256, - ssh->serverKeys.macKey, - ssh->serverKeys.macKeySz); - wc_HmacUpdate(&hmac, flatSeq, sizeof(flatSeq)); - wc_HmacUpdate(&hmac, in, inSz); - wc_HmacFinal(&hmac, mac); + ret = wc_HmacSetKey(&hmac, SHA256, + ssh->serverKeys.macKey, + ssh->serverKeys.macKeySz); + if (ret == WS_SUCCESS) + ret = wc_HmacUpdate(&hmac, flatSeq, sizeof(flatSeq)); + if (ret == WS_SUCCESS) + ret = wc_HmacUpdate(&hmac, in, inSz); + if (ret == WS_SUCCESS) + ret = wc_HmacFinal(&hmac, mac); } break; @@ -2759,7 +2836,7 @@ static INLINE int CreateMac(WOLFSSH* ssh, const uint8_t* in, uint32_t inSz, static INLINE int VerifyMac(WOLFSSH* ssh, const uint8_t* in, uint32_t inSz, const uint8_t* mac) { - int ret = WS_SUCCESS; + int ret; uint8_t flatSeq[LENGTH_SZ]; uint8_t checkMac[MAX_HMAC_SZ]; Hmac hmac; @@ -2773,25 +2850,34 @@ static INLINE int VerifyMac(WOLFSSH* ssh, const uint8_t* in, uint32_t inSz, switch (ssh->peerMacId) { case ID_NONE: + ret = WS_SUCCESS; break; case ID_HMAC_SHA1: case ID_HMAC_SHA1_96: - wc_HmacSetKey(&hmac, SHA, - ssh->clientKeys.macKey, ssh->clientKeys.macKeySz); - wc_HmacUpdate(&hmac, flatSeq, sizeof(flatSeq)); - wc_HmacUpdate(&hmac, in, inSz); - wc_HmacFinal(&hmac, checkMac); + ret = wc_HmacSetKey(&hmac, SHA, + ssh->clientKeys.macKey, + ssh->clientKeys.macKeySz); + if (ret == WS_SUCCESS) + ret = wc_HmacUpdate(&hmac, flatSeq, sizeof(flatSeq)); + if (ret == WS_SUCCESS) + ret = wc_HmacUpdate(&hmac, in, inSz); + if (ret == WS_SUCCESS) + ret = wc_HmacFinal(&hmac, checkMac); if (ConstantCompare(checkMac, mac, ssh->peerMacSz) != 0) ret = WS_VERIFY_MAC_E; break; case ID_HMAC_SHA2_256: - wc_HmacSetKey(&hmac, SHA256, - ssh->clientKeys.macKey, ssh->clientKeys.macKeySz); - wc_HmacUpdate(&hmac, flatSeq, sizeof(flatSeq)); - wc_HmacUpdate(&hmac, in, inSz); - wc_HmacFinal(&hmac, checkMac); + ret = wc_HmacSetKey(&hmac, SHA256, + ssh->clientKeys.macKey, + ssh->clientKeys.macKeySz); + if (ret == WS_SUCCESS) + ret = wc_HmacUpdate(&hmac, flatSeq, sizeof(flatSeq)); + if (ret == WS_SUCCESS) + ret = wc_HmacUpdate(&hmac, in, inSz); + if (ret == WS_SUCCESS) + ret = wc_HmacFinal(&hmac, checkMac); if (ConstantCompare(checkMac, mac, ssh->peerMacSz) != 0) ret = WS_VERIFY_MAC_E; break; @@ -3242,182 +3328,226 @@ int SendKexDhReply(WOLFSSH* ssh) uint32_t scratch = 0; uint8_t* output; uint32_t idx; - int ret; + int ret = WS_SUCCESS; WLOG(WS_LOG_DEBUG, "Entering SendKexDhReply()"); wc_InitDhKey(&dhKey); switch (ssh->handshake->kexId) { case ID_DH_GROUP1_SHA1: - wc_DhSetKey(&dhKey, dhPrimeGroup1, dhPrimeGroup1Sz, - dhGenerator, dhGeneratorSz); + if (wc_DhSetKey(&dhKey, dhPrimeGroup1, dhPrimeGroup1Sz, + dhGenerator, dhGeneratorSz) < 0) + ret = WS_CRYPTO_FAILED; break; case ID_DH_GROUP14_SHA1: - wc_DhSetKey(&dhKey, dhPrimeGroup14, dhPrimeGroup14Sz, - dhGenerator, dhGeneratorSz); + if (wc_DhSetKey(&dhKey, dhPrimeGroup14, dhPrimeGroup14Sz, + dhGenerator, dhGeneratorSz) < 0) + ret = WS_CRYPTO_FAILED; break; default: - return -1; + ret = WS_INVALID_ALGO_ID; } /* Hash in the server's RSA key. */ - wc_InitRsaKey(&rsaKey, ssh->ctx->heap); - ret = wc_RsaPrivateKeyDecode(ssh->ctx->privateKey, &scratch, - &rsaKey, (int)ssh->ctx->privateKeySz); - if (ret < 0) - return ret; - wc_RsaFlattenPublicKey(&rsaKey, rsaE, &rsaESz, rsaN, &rsaNSz); - if (rsaE[0] & 0x80) rsaEPad = 1; - if (rsaN[0] & 0x80) rsaNPad = 1; - rsaKeyBlockSz = (LENGTH_SZ * 3) + 7 + rsaESz + rsaEPad + rsaNSz + rsaNPad; - /* The 7 is for the name "ssh-rsa". */ - c32toa(rsaKeyBlockSz, scratchLen); - wc_ShaUpdate(&ssh->handshake->hash, scratchLen, LENGTH_SZ); - c32toa(7, scratchLen); - wc_ShaUpdate(&ssh->handshake->hash, scratchLen, LENGTH_SZ); - wc_ShaUpdate(&ssh->handshake->hash, (const uint8_t*)"ssh-rsa", 7); - c32toa(rsaESz + rsaEPad, scratchLen); - wc_ShaUpdate(&ssh->handshake->hash, scratchLen, LENGTH_SZ); - if (rsaEPad) { - scratchLen[0] = 0; - wc_ShaUpdate(&ssh->handshake->hash, scratchLen, 1); - } - wc_ShaUpdate(&ssh->handshake->hash, rsaE, rsaESz); - c32toa(rsaNSz + rsaNPad, scratchLen); - wc_ShaUpdate(&ssh->handshake->hash, scratchLen, LENGTH_SZ); - if (rsaNPad) { - scratchLen[0] = 0; - wc_ShaUpdate(&ssh->handshake->hash, scratchLen, 1); - } - wc_ShaUpdate(&ssh->handshake->hash, rsaN, rsaNSz); - - /* Hash in the client's DH e-value. */ - c32toa(ssh->handshake->eSz, scratchLen); - wc_ShaUpdate(&ssh->handshake->hash, scratchLen, LENGTH_SZ); - wc_ShaUpdate(&ssh->handshake->hash, ssh->handshake->e, ssh->handshake->eSz); - - /* Make the server's DH f-value, and the shared secret k. */ - wc_DhGenerateKeyPair(&dhKey, ssh->rng, y, &ySz, f, &fSz); - if (f[0] & 0x80) fPad = 1; - wc_DhAgree(&dhKey, ssh->k, &ssh->kSz, y, ySz, - ssh->handshake->e, ssh->handshake->eSz); - if (ssh->k[0] & 0x80) kPad = 1; - wc_FreeDhKey(&dhKey); - - /* Hash in the server's DH f-value. */ - c32toa(fSz + fPad, scratchLen); - wc_ShaUpdate(&ssh->handshake->hash, scratchLen, LENGTH_SZ); - if (fPad) { - scratchLen[0] = 0; - wc_ShaUpdate(&ssh->handshake->hash, scratchLen, 1); - } - wc_ShaUpdate(&ssh->handshake->hash, f, fSz); - - /* Hash in the shared secret k. */ - c32toa(ssh->kSz + kPad, scratchLen); - wc_ShaUpdate(&ssh->handshake->hash, scratchLen, LENGTH_SZ); - if (kPad) { - scratchLen[0] = 0; - wc_ShaUpdate(&ssh->handshake->hash, scratchLen, 1); - } - wc_ShaUpdate(&ssh->handshake->hash, ssh->k, ssh->kSz); - - /* Save the handshake hash value h, and session ID. */ - wc_ShaFinal(&ssh->handshake->hash, ssh->h); - ssh->hSz = SHA_DIGEST_SIZE; - if (ssh->sessionIdSz == 0) { - WMEMCPY(ssh->sessionId, ssh->h, ssh->hSz); - ssh->sessionIdSz = ssh->hSz; + if (ret == WS_SUCCESS) { + ret = wc_InitRsaKey(&rsaKey, ssh->ctx->heap); + if (ret == 0) + ret = wc_RsaPrivateKeyDecode(ssh->ctx->privateKey, &scratch, + &rsaKey, (int)ssh->ctx->privateKeySz); + if (ret == 0) + ret = wc_RsaFlattenPublicKey(&rsaKey, rsaE, &rsaESz, rsaN, &rsaNSz); + if (ret == 0) { + if (rsaE[0] & 0x80) rsaEPad = 1; + if (rsaN[0] & 0x80) rsaNPad = 1; + rsaKeyBlockSz = (LENGTH_SZ * 3) + 7 + rsaESz + rsaEPad + + rsaNSz + rsaNPad; + /* The 7 is for the name "ssh-rsa". */ + c32toa(rsaKeyBlockSz, scratchLen); + ret = wc_ShaUpdate(&ssh->handshake->hash, scratchLen, LENGTH_SZ); + } + if (ret == 0) { + c32toa(7, scratchLen); + ret = wc_ShaUpdate(&ssh->handshake->hash, scratchLen, LENGTH_SZ); + } + if (ret == 0) + ret = wc_ShaUpdate(&ssh->handshake->hash, + (const uint8_t*)"ssh-rsa", 7); + if (ret == 0) { + c32toa(rsaESz + rsaEPad, scratchLen); + ret = wc_ShaUpdate(&ssh->handshake->hash, scratchLen, LENGTH_SZ); + } + if (ret == 0) { + if (rsaEPad) { + scratchLen[0] = 0; + ret = wc_ShaUpdate(&ssh->handshake->hash, scratchLen, 1); + } + } + if (ret == 0) + ret = wc_ShaUpdate(&ssh->handshake->hash, rsaE, rsaESz); + if (ret == 0) { + c32toa(rsaNSz + rsaNPad, scratchLen); + ret = wc_ShaUpdate(&ssh->handshake->hash, scratchLen, LENGTH_SZ); + } + if (ret == 0) { + if (rsaNPad) { + scratchLen[0] = 0; + ret = wc_ShaUpdate(&ssh->handshake->hash, scratchLen, 1); + } + } + if (ret == 0) + ret = wc_ShaUpdate(&ssh->handshake->hash, rsaN, rsaNSz); + + /* Hash in the client's DH e-value. */ + if (ret == 0) { + c32toa(ssh->handshake->eSz, scratchLen); + ret = wc_ShaUpdate(&ssh->handshake->hash, scratchLen, LENGTH_SZ); + } + if (ret == 0) + ret = wc_ShaUpdate(&ssh->handshake->hash, + ssh->handshake->e, ssh->handshake->eSz); + + /* Make the server's DH f-value, and the shared secret k. */ + if (ret == 0) + ret = wc_DhGenerateKeyPair(&dhKey, ssh->rng, y, &ySz, f, &fSz); + if (ret == 0) { + if (f[0] & 0x80) fPad = 1; + ret = wc_DhAgree(&dhKey, ssh->k, &ssh->kSz, y, ySz, + ssh->handshake->e, ssh->handshake->eSz); + } + if (ret == 0) { + if (ssh->k[0] & 0x80) kPad = 1; + } + wc_FreeDhKey(&dhKey); + + /* Hash in the server's DH f-value. */ + c32toa(fSz + fPad, scratchLen); + wc_ShaUpdate(&ssh->handshake->hash, scratchLen, LENGTH_SZ); + if (fPad) { + scratchLen[0] = 0; + wc_ShaUpdate(&ssh->handshake->hash, scratchLen, 1); + } + wc_ShaUpdate(&ssh->handshake->hash, f, fSz); + + /* Hash in the shared secret k. */ + c32toa(ssh->kSz + kPad, scratchLen); + wc_ShaUpdate(&ssh->handshake->hash, scratchLen, LENGTH_SZ); + if (kPad) { + scratchLen[0] = 0; + wc_ShaUpdate(&ssh->handshake->hash, scratchLen, 1); + } + wc_ShaUpdate(&ssh->handshake->hash, ssh->k, ssh->kSz); + + /* Save the handshake hash value h, and session ID. */ + wc_ShaFinal(&ssh->handshake->hash, ssh->h); + ssh->hSz = SHA_DIGEST_SIZE; + if (ssh->sessionIdSz == 0) { + WMEMCPY(ssh->sessionId, ssh->h, ssh->hSz); + ssh->sessionIdSz = ssh->hSz; + } + if (ret != WS_SUCCESS) + ret = WS_CRYPTO_FAILED; } /* Sign h with the server's RSA private key. */ - { + if (ret == WS_SUCCESS) { Sha sha; uint8_t digest[SHA_DIGEST_SIZE]; uint8_t encSig[512]; uint32_t encSigSz; - wc_InitSha(&sha); - wc_ShaUpdate(&sha, ssh->h, ssh->hSz); - wc_ShaFinal(&sha, digest); + ret = wc_InitSha(&sha); + if (ret == 0) + ret = wc_ShaUpdate(&sha, ssh->h, ssh->hSz); + if (ret == 0) + ret = wc_ShaFinal(&sha, digest); + if (ret != 0) + ret = WS_CRYPTO_FAILED; - encSigSz = wc_EncodeSignature(encSig, digest, sizeof(digest), SHAh); - if (encSigSz <= 0) { - WLOG(WS_LOG_DEBUG, "SendKexDhReply: Bad Encode Sig"); - } - else { - /* At this point, sigSz should already be sizeof(sig) */ - sigSz = wc_RsaSSL_Sign(encSig, encSigSz, - sig, sigSz, &rsaKey, ssh->rng); - if (sigSz <= 0) { - WLOG(WS_LOG_DEBUG, "SendKexDhReply: Bad RSA Sign"); + if (ret == WS_SUCCESS) { + encSigSz = wc_EncodeSignature(encSig, digest, sizeof(digest), SHAh); + if (encSigSz <= 0) { + WLOG(WS_LOG_DEBUG, "SendKexDhReply: Bad Encode Sig"); + ret = WS_CRYPTO_FAILED; } else { - /* Success */ + /* At this point, sigSz should already be sizeof(sig) */ + sigSz = wc_RsaSSL_Sign(encSig, encSigSz, + sig, sigSz, &rsaKey, ssh->rng); + if (sigSz <= 0) { + WLOG(WS_LOG_DEBUG, "SendKexDhReply: Bad RSA Sign"); + ret = WS_RSA_E; + } + else { + /* Success */ + } } } } wc_FreeRsaKey(&rsaKey); sigBlockSz = (LENGTH_SZ * 2) + 7 + sigSz; - GenerateKeys(ssh); + if (ret == WS_SUCCESS) + ret = GenerateKeys(ssh); /* Get the buffer, copy the packet data, once f is laid into the buffer, * add it to the hash and then add K. */ - payloadSz = MSG_ID_SZ + (LENGTH_SZ * 3) + - rsaKeyBlockSz + fSz + fPad + sigBlockSz; - ret = PreparePacket(ssh, payloadSz); - if (ret != WS_SUCCESS) - return ret; - output = ssh->outputBuffer.buffer; - idx = ssh->outputBuffer.length; + if (ret == WS_SUCCESS) { + payloadSz = MSG_ID_SZ + (LENGTH_SZ * 3) + + rsaKeyBlockSz + fSz + fPad + sigBlockSz; + ret = PreparePacket(ssh, payloadSz); + } - output[idx++] = MSGID_KEXDH_REPLY; + if (ret == WS_SUCCESS) { + output = ssh->outputBuffer.buffer; + idx = ssh->outputBuffer.length; - /* Copy the rsaKeyBlock into the buffer. */ - c32toa(rsaKeyBlockSz, output + idx); - idx += LENGTH_SZ; - c32toa(7, output + idx); - idx += LENGTH_SZ; - WMEMCPY(output + idx, "ssh-rsa", 7); - idx += 7; - c32toa(rsaESz + rsaEPad, output + idx); - idx += LENGTH_SZ; - if (rsaEPad) output[idx++] = 0; - WMEMCPY(output + idx, rsaE, rsaESz); - idx += rsaESz; - c32toa(rsaNSz + rsaNPad, output + idx); - idx += LENGTH_SZ; - if (rsaNPad) output[idx++] = 0; - WMEMCPY(output + idx, rsaN, rsaNSz); - idx += rsaNSz; + output[idx++] = MSGID_KEXDH_REPLY; - c32toa(fSz + fPad, output + idx); - idx += LENGTH_SZ; - if (fPad) output[idx++] = 0; - WMEMCPY(output + idx, f, fSz); - idx += fSz; + /* Copy the rsaKeyBlock into the buffer. */ + c32toa(rsaKeyBlockSz, output + idx); + idx += LENGTH_SZ; + c32toa(7, output + idx); + idx += LENGTH_SZ; + WMEMCPY(output + idx, "ssh-rsa", 7); + idx += 7; + c32toa(rsaESz + rsaEPad, output + idx); + idx += LENGTH_SZ; + if (rsaEPad) output[idx++] = 0; + WMEMCPY(output + idx, rsaE, rsaESz); + idx += rsaESz; + c32toa(rsaNSz + rsaNPad, output + idx); + idx += LENGTH_SZ; + if (rsaNPad) output[idx++] = 0; + WMEMCPY(output + idx, rsaN, rsaNSz); + idx += rsaNSz; - c32toa(sigBlockSz, output + idx); - idx += LENGTH_SZ; - c32toa(7, output + idx); - idx += LENGTH_SZ; - WMEMCPY(output + idx, "ssh-rsa", 7); - idx += 7; - c32toa(sigSz, output + idx); - idx += LENGTH_SZ; - WMEMCPY(output + idx, sig, sigSz); - idx += sigSz; + c32toa(fSz + fPad, output + idx); + idx += LENGTH_SZ; + if (fPad) output[idx++] = 0; + WMEMCPY(output + idx, f, fSz); + idx += fSz; + + c32toa(sigBlockSz, output + idx); + idx += LENGTH_SZ; + c32toa(7, output + idx); + idx += LENGTH_SZ; + WMEMCPY(output + idx, "ssh-rsa", 7); + idx += 7; + c32toa(sigSz, output + idx); + idx += LENGTH_SZ; + WMEMCPY(output + idx, sig, sigSz); + idx += sigSz; - ssh->outputBuffer.length = idx; + ssh->outputBuffer.length = idx; - ret = BundlePacket(ssh); - if (ret == WS_SUCCESS) { - ret = SendNewKeys(ssh); + ret = BundlePacket(ssh); } + if (ret == WS_SUCCESS) + ret = SendNewKeys(ssh); + WLOG(WS_LOG_DEBUG, "Leaving SendKexDhReply(), ret = %d", ret); return ret; } From b76f3763815adb9304160301e3623e9e023d6f30 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 24 Oct 2016 15:08:58 -0700 Subject: [PATCH 16/17] scan-build fix. clear a whole buffer before filling it before checking with ConstantCompare(). --- src/internal.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/internal.c b/src/internal.c index 40a6b10db..35f93d038 100644 --- a/src/internal.c +++ b/src/internal.c @@ -2848,6 +2848,8 @@ static INLINE int VerifyMac(WOLFSSH* ssh, const uint8_t* in, uint32_t inSz, WLOG(WS_LOG_DEBUG, "VM: seq = %u", ssh->peerSeq); WLOG(WS_LOG_DEBUG, "VM: keyLen = %u", ssh->clientKeys.macKeySz); + WMEMSET(checkMac, 0, sizeof(checkMac)); + switch (ssh->peerMacId) { case ID_NONE: ret = WS_SUCCESS; From 4ff6a525b6fe5fe3db8d5af0d56ea520c565d477 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 24 Oct 2016 15:10:22 -0700 Subject: [PATCH 17/17] Update README with the change of name of the keys directory. --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 2b165d8a6..133cbf46d 100644 --- a/README.md +++ b/README.md @@ -64,7 +64,7 @@ testing notes After cloning the repository, be sure to make the testing private keys read- only for the user, otherwise ssh_client will tell you to do it. - $ chmod 0600 ./certs/key-gretel.pem ./certs/key-hansel.pem + $ chmod 0600 ./keys/key-gretel.pem ./keys/key-hansel.pem Authentication against the example echoserver can be done with a password or public key. To use a password the command line: @@ -78,7 +78,7 @@ Where the `USER` and password pairs are: To use public key authentication use the command line: - $ ssh_client -i ./certs/key-USER.pem -p 22222 USER@localhost + $ ssh_client -i ./keys/key-USER.pem -p 22222 USER@localhost Where the user can be `gretel` or `hansel`.