From d612c491b860231327e9394897836282f76286b1 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 23 Jun 2026 12:09:36 -0700 Subject: [PATCH 1/2] Accumulate extended data, ignore unknown types - PutBuffer appends instead of resetting, preserving unread stderr across multiple packets. - Ignore unknown EXTENDED_DATA type codes per RFC instead of returning WS_INVALID_EXTDATA. Issues: F-864, F-2078 --- src/internal.c | 58 +++++++++++++++------------ tests/unit.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++ wolfssh/error.h | 6 ++- 3 files changed, 141 insertions(+), 25 deletions(-) diff --git a/src/internal.c b/src/internal.c index dfb92d728..c0e59b133 100644 --- a/src/internal.c +++ b/src/internal.c @@ -10165,25 +10165,25 @@ static int DoChannelData(WOLFSSH* ssh, } -/* deletes current buffer and updates it - * return WS_SUCCESS on success */ +/* Appends data to the buffer, preserving any bytes not yet read. This is an + * accumulating buffer: unread data is kept, already-read data (everything + * below idx) is compacted away, and the new data is appended after the unread + * region. Returns WS_SUCCESS on success. */ static int PutBuffer(WOLFSSH_BUFFER* buf, byte* data, word32 dataSz) { int ret; - /* reset "used" section of buffer back to 0 */ - buf->length = 0; - buf->idx = 0; - - if (dataSz > buf->bufferSz) { - if ((ret = GrowBuffer(buf, dataSz)) != WS_SUCCESS) { - return ret; - } + /* GrowBuffer compacts the consumed prefix (bytes below idx) down to the + * front and guarantees room for dataSz more bytes after the unread data, + * reallocating only when needed. On return idx is 0 and length is the + * count of unread bytes. */ + ret = GrowBuffer(buf, dataSz); + if (ret == WS_SUCCESS) { + WMEMCPY(buf->buffer + buf->length, data, dataSz); + buf->length += dataSz; } - WMEMCPY(buf->buffer, data, dataSz); - buf->length = dataSz; - return WS_SUCCESS; + return ret; } @@ -10202,9 +10202,6 @@ static int DoChannelExtendedData(WOLFSSH* ssh, ret = GetUint32(&channelId, buf, len, &begin); if (ret == WS_SUCCESS) ret = GetUint32(&dataTypeCode, buf, len, &begin); - if (ret == WS_SUCCESS) - ret = (dataTypeCode == CHANNEL_EXTENDED_DATA_STDERR) ? - WS_SUCCESS : WS_INVALID_EXTDATA; if (ret == WS_SUCCESS) ret = GetSize(&dataSz, buf, len, &begin); @@ -10214,23 +10211,36 @@ static int DoChannelExtendedData(WOLFSSH* ssh, ret = WS_INVALID_CHANID; else if (dataSz > channel->maxPacketSz) ret = WS_RECV_OVERFLOW_E; - else { - ret = PutBuffer(&ssh->extDataBuffer, buf + begin, dataSz); + else if (dataTypeCode == CHANNEL_EXTENDED_DATA_STDERR) { + /* Buffer the stderr data for the application to read. The + * extended data buffer accumulates unread data instead of + * overwriting it, so a blob arriving before the application + * reads the previous one is no longer silently lost. */ + ret = PutBuffer(&ssh->extDataBuffer, buf + begin, dataSz); #ifdef DEBUG_WOLFSSH DumpOctetString(buf + begin, dataSz); #endif - if (ret == WS_SUCCESS) { + if (ret == WS_SUCCESS) ret = SendChannelWindowAdjust(ssh, channel->channel, dataSz); + if (ret == WS_SUCCESS) { + ssh->lastRxId = channelId; + ret = WS_EXTDATA; } } + else { + /* Unknown extended data type. RFC 4254/4251 call for unknown + * values to be ignored rather than rejected, so consume and + * discard the payload. Nothing is buffered for the application, + * so replenish the window immediately. */ + WLOG(WS_LOG_INFO, "Ignoring unknown extended data type %u", + dataTypeCode); + ret = SendChannelWindowAdjust(ssh, channel->channel, dataSz); + if (ret == WS_SUCCESS) + ssh->lastRxId = channelId; + } *idx = begin + dataSz; } - if (ret == WS_SUCCESS) { - ssh->lastRxId = channelId; - ret = WS_EXTDATA; - } - WLOG(WS_LOG_DEBUG, "Leaving DoChannelExtendedData(), ret = %d", ret); return ret; } diff --git a/tests/unit.c b/tests/unit.c index e18981ef7..f3d1074a8 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -1769,6 +1769,103 @@ static int test_DoChannelExtendedData_overflow(void) return result; } +/* Exercises the accumulating extended-data buffer: two stderr blobs that + * arrive before the application reads must both be preserved (no silent + * overwrite), and an unknown extended data type must be ignored rather than + * rejected. */ +static int test_DoChannelExtendedData_flow(void) +{ + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + WOLFSSH_CHANNEL* ch = NULL; + int result = 0; + int ret; + word32 idx; + byte out[32]; + + /* channelId=0, type=1 (stderr), dataSz=10, payload all 0x11 */ + static const byte blob1[] = { + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x0A, + 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11 + }; + /* channelId=0, type=1 (stderr), dataSz=10, payload all 0x22 */ + static const byte blob2[] = { + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x0A, + 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22 + }; + /* channelId=0, unknown type=2, dataSz=5, payload all 0x33 */ + static const byte unknownBlob[] = { + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x02, + 0x00, 0x00, 0x00, 0x05, + 0x33, 0x33, 0x33, 0x33, 0x33 + }; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + if (ctx == NULL) + return -600; + wolfSSH_SetIOSend(ctx, DiscardIoSend); + + ssh = wolfSSH_new(ctx); + if (ssh == NULL) { result = -601; goto done; } + /* Allow MSGID_CHANNEL_WINDOW_ADJUST on this bare session. */ + ssh->acceptState = ACCEPT_SERVER_USERAUTH_SENT; + + /* windowSz=128, maxPacketSz=64 */ + ch = ChannelNew(ssh, ID_CHANTYPE_SESSION, 128, 64); + if (ch == NULL) { result = -602; goto done; } + if (ChannelAppend(ssh, ch) != WS_SUCCESS) { + ChannelDelete(ch, ssh->ctx->heap); + result = -603; + goto done; + } + + /* First stderr blob: buffered. */ + idx = 0; + ret = wolfSSH_TestDoChannelExtendedData(ssh, (byte*)blob1, + (word32)sizeof(blob1), &idx); + if (ret != WS_EXTDATA) { result = -604; goto done; } + + /* Second stderr blob before any read: must accumulate, not overwrite. */ + idx = 0; + ret = wolfSSH_TestDoChannelExtendedData(ssh, (byte*)blob2, + (word32)sizeof(blob2), &idx); + if (ret != WS_EXTDATA) { result = -606; goto done; } + + /* Read everything: both blobs present and in order (no data loss). */ + ret = wolfSSH_extended_data_read(ssh, out, (word32)sizeof(out)); + if (ret != 20) { result = -608; goto done; } + { + int i; + for (i = 0; i < 10; i++) + if (out[i] != 0x11) { result = -609; goto done; } + for (i = 10; i < 20; i++) + if (out[i] != 0x22) { result = -610; goto done; } + } + + /* Buffer drained; a further read returns 0. */ + ret = wolfSSH_extended_data_read(ssh, out, (word32)sizeof(out)); + if (ret != 0) { result = -612; goto done; } + + /* Unknown extended data type: ignored (consumed), not rejected. Nothing + * is buffered for the application. */ + idx = 0; + ret = wolfSSH_TestDoChannelExtendedData(ssh, (byte*)unknownBlob, + (word32)sizeof(unknownBlob), &idx); + if (ret != WS_SUCCESS) { result = -613; goto done; } + ret = wolfSSH_extended_data_read(ssh, out, (word32)sizeof(out)); + if (ret != 0) { result = -615; goto done; } + +done: + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + return result; +} + static int test_SendChannelData_eofTxd(void) { WOLFSSH_CTX* ctx = NULL; @@ -5147,6 +5244,11 @@ int wolfSSH_UnitTest(int argc, char** argv) (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; + unitResult = test_DoChannelExtendedData_flow(); + printf("DoChannelExtendedData_flow: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; + unitResult = test_SendChannelData_eofTxd(); printf("SendChannelData_eofTxd: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; diff --git a/wolfssh/error.h b/wolfssh/error.h index 307173af6..c8dc84fab 100644 --- a/wolfssh/error.h +++ b/wolfssh/error.h @@ -97,7 +97,11 @@ enum WS_ErrorCodes { WS_SFTP_COMPLETE = -1055, /* SFTP connection established */ WS_NEXT_ERROR = -1056, /* Getting next value/state is error */ WS_CHAN_RXD = -1057, /* Status that channel data received. */ - WS_INVALID_EXTDATA = -1058, /* invalid Channel Extended Data Type */ + WS_INVALID_EXTDATA = -1058, /* invalid Channel Extended Data Type. + * Retained for compat; unknown extended + * data types are now ignored per RFC 4254 + * rather than rejected, so this is no + * longer produced. */ WS_SFTP_BAD_REQ_ID = -1060, /* SFTP Bad request ID */ WS_SFTP_BAD_REQ_TYPE = -1061, /* SFTP Bad request ID */ WS_SFTP_STATUS_NOT_OK = -1062, /* SFTP Status not OK */ From cbf4c37b1c6e5e48525a97e71efda46df7d1db81 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 23 Jun 2026 12:14:56 -0700 Subject: [PATCH 2/2] Window flow control for extended data - Charge the channel window on receipt; replenish only when the application consumes bytes via wolfSSH_extended_data_read. - Track extDataChannelId so the read path adjusts the right window. - Reject extended data larger than the advertised window. - stderr now shares the channel window with normal data (RFC 4254 section 5.2 back-pressure). - Gate window adjusts on isKeying both ways: IsMessageAllowed does not gate connection-layer messages, so they must not go out between KEXINIT and NEWKEYS (RFC 4253 section 7.1). The read path returns WS_REKEYING; the receive path parks owed credit in pendingWindowAdjust, flushed by SendPendingChannelWindowAdjust once keying completes. - Fail safe when a second channel sends stderr while another still has unread data: drop it and replenish its window instead of leaking the displaced channel's window and over-crediting the survivor. - Document the mandatory-drain contract in the public header. Issue: F-864 --- src/internal.c | 137 ++++++++++++++++++++--- src/ssh.c | 61 ++++++++++- tests/unit.c | 268 +++++++++++++++++++++++++++++++++++++++++++-- wolfssh/internal.h | 29 ++++- wolfssh/ssh.h | 7 ++ 5 files changed, 478 insertions(+), 24 deletions(-) diff --git a/src/internal.c b/src/internal.c index c0e59b133..77f44696a 100644 --- a/src/internal.c +++ b/src/internal.c @@ -6480,6 +6480,56 @@ static int DoKexDhReply(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) } +/* Flush any receive-window credit that was deferred while a rekey was in + * progress. DoChannelExtendedData() parks the credit for discarded extended + * data on the channel (pendingWindowAdjust) instead of sending a WINDOW_ADJUST, + * because RFC 4253 section 7.1 forbids connection-layer messages between KEXINIT + * and NEWKEYS and IsMessageAllowed() does not gate connection-layer messages on + * isKeying. This is called when keying completes. */ +static int SendPendingChannelWindowAdjust(WOLFSSH* ssh) +{ + WOLFSSH_CHANNEL* cur; + int savedError; + + if (ssh == NULL) + return WS_BAD_ARGUMENT; + + /* Still mid-KEX: leave the credit parked until both sides finish. */ + if (ssh->isKeying) + return WS_SUCCESS; + + /* Flushing parked credit is incidental to the keying flow that triggered + * it, so do not disturb ssh->error: SendChannelWindowAdjust -> + * wolfSSH_SendPacket sets it to WS_WANT_WRITE when the bundled adjust + * cannot flush in non-blocking mode, and the bundled bytes are sent on the + * next flush regardless. Mirror wolfSSH_extended_data_read(). */ + savedError = ssh->error; + + for (cur = ssh->channelList; cur != NULL; cur = cur->next) { + word32 amount = cur->pendingWindowAdjust; + int adjustResult; + + if (amount == 0) + continue; + + /* Clear before sending: the credit is owed regardless of the send + * result. On WS_WANT_WRITE the adjust is bundled into outputBuffer and + * reaches the peer on a later flush; on a hard failure the channel is + * already failing, so dropping the deferred credit is moot. */ + cur->pendingWindowAdjust = 0; + adjustResult = SendChannelWindowAdjust(ssh, cur->channel, amount); + if (adjustResult != WS_SUCCESS && adjustResult != WS_WANT_WRITE) + WLOG(WS_LOG_ERROR, + "SendPendingChannelWindowAdjust: window adjust send failed " + "(%d) for channel %u; deferred credit dropped", + adjustResult, cur->channel); + } + + ssh->error = savedError; + return WS_SUCCESS; +} + + static int DoNewKeys(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) { int ret = WS_SUCCESS; @@ -6565,6 +6615,9 @@ static int DoNewKeys(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) ssh->isKeying &= ~WOLFSSH_PEER_IS_KEYING; HandshakeInfoFree(ssh->handshake, ssh->ctx->heap); ssh->handshake = NULL; + /* If this was the last keying flag to clear, flush any window credit + * that was deferred during the rekey (RFC 4253 section 7.1). */ + SendPendingChannelWindowAdjust(ssh); WLOG(WS_LOG_DEBUG, "Keying completed"); if (ssh->ctx->keyingCompletionCb) ssh->ctx->keyingCompletionCb(ssh->keyingCompletionCtx); @@ -10211,32 +10264,81 @@ static int DoChannelExtendedData(WOLFSSH* ssh, ret = WS_INVALID_CHANID; else if (dataSz > channel->maxPacketSz) ret = WS_RECV_OVERFLOW_E; + else if (dataSz > channel->windowSz) + /* Peer sent more than the window we advertised. */ + ret = WS_RECV_OVERFLOW_E; else if (dataTypeCode == CHANNEL_EXTENDED_DATA_STDERR) { /* Buffer the stderr data for the application to read. The * extended data buffer accumulates unread data instead of * overwriting it, so a blob arriving before the application - * reads the previous one is no longer silently lost. */ - ret = PutBuffer(&ssh->extDataBuffer, buf + begin, dataSz); - #ifdef DEBUG_WOLFSSH - DumpOctetString(buf + begin, dataSz); - #endif - if (ret == WS_SUCCESS) - ret = SendChannelWindowAdjust(ssh, channel->channel, dataSz); - if (ret == WS_SUCCESS) { - ssh->lastRxId = channelId; - ret = WS_EXTDATA; + * reads the previous one is no longer silently lost. + * + * Extended data consumes channel window the same as ordinary + * data (RFC 4254 section 5.2). Charge the window now and only + * replenish it once the application drains the data in + * wolfSSH_extended_data_read(). That gives stderr real + * back-pressure instead of replenishing on receipt, which + * previously let the peer keep clobbering the buffer. */ + int hadPending = ssh->extDataBuffer.idx < ssh->extDataBuffer.length; + + if (hadPending && ssh->extDataChannelId != channel->channel) { + /* Buffer and extDataChannelId are shared (single-session + * limitation, see WOLFSSH.extDataBuffer in internal.h), so only + * one channel's stderr buffers at a time. Buffering a second + * would desync windows (displaced channel never credited, this + * one over-credited). Fail safe: drop it and replenish its + * window (now, or deferred past a rekey) so nothing leaks. */ + WLOG(WS_LOG_ERROR, + "Extended data from channel %u dropped; channel %u still " + "has unread stderr buffered (only one channel's stderr " + "can be buffered at a time)", + channel->channel, ssh->extDataChannelId); + if (ssh->isKeying) { + /* RFC 4253 section 7.1: no WINDOW_ADJUST between KEXINIT + * and NEWKEYS. Park the credit; flushed when keying ends. */ + channel->pendingWindowAdjust += dataSz; + ssh->lastRxId = channelId; + ret = WS_SUCCESS; + } + else { + ret = SendChannelWindowAdjust(ssh, channel->channel, + dataSz); + if (ret == WS_SUCCESS) + ssh->lastRxId = channelId; + } + } + else { + ret = PutBuffer(&ssh->extDataBuffer, buf + begin, dataSz); + #ifdef DEBUG_WOLFSSH + DumpOctetString(buf + begin, dataSz); + #endif + if (ret == WS_SUCCESS) { + channel->windowSz -= dataSz; + ssh->extDataChannelId = channel->channel; + ssh->lastRxId = channelId; + ret = WS_EXTDATA; + } } } else { /* Unknown extended data type. RFC 4254/4251 call for unknown * values to be ignored rather than rejected, so consume and * discard the payload. Nothing is buffered for the application, - * so replenish the window immediately. */ + * so replenish the window (now, or deferred past a rekey). */ WLOG(WS_LOG_INFO, "Ignoring unknown extended data type %u", dataTypeCode); - ret = SendChannelWindowAdjust(ssh, channel->channel, dataSz); - if (ret == WS_SUCCESS) + if (ssh->isKeying) { + /* RFC 4253 section 7.1: no WINDOW_ADJUST between KEXINIT and + * NEWKEYS. Park the credit; flushed when keying ends. */ + channel->pendingWindowAdjust += dataSz; ssh->lastRxId = channelId; + ret = WS_SUCCESS; + } + else { + ret = SendChannelWindowAdjust(ssh, channel->channel, dataSz); + if (ret == WS_SUCCESS) + ssh->lastRxId = channelId; + } } *idx = begin + dataSz; } @@ -13734,6 +13836,10 @@ int SendNewKeys(WOLFSSH* ssh) /* Clear self is keying flag */ ssh->isKeying &= ~WOLFSSH_SELF_IS_KEYING; + + /* If this was the last keying flag to clear, flush any window credit + * that was deferred during the rekey (RFC 4253 section 7.1). */ + SendPendingChannelWindowAdjust(ssh); } WLOG(WS_LOG_DEBUG, "Leaving SendNewKeys(), ret = %d", ret); @@ -18512,6 +18618,11 @@ int wolfSSH_TestDoChannelExtendedData(WOLFSSH* ssh, byte* buf, word32 len, return DoChannelExtendedData(ssh, buf, len, idx); } +int wolfSSH_TestSendPendingChannelWindowAdjust(WOLFSSH* ssh) +{ + return SendPendingChannelWindowAdjust(ssh); +} + int wolfSSH_TestDoUserAuthRequest(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) { diff --git a/src/ssh.c b/src/ssh.c index 45761d808..068b7ffcf 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1404,9 +1404,14 @@ int wolfSSH_extended_data_send(WOLFSSH* ssh, byte* buf, word32 bufSz) } -/* Reads pending data from extended data buffer. Currently can be used to get - * STDERR information sent across the channel. - * Returns the number of bytes read on success */ +/* Reads pending STDERR data from the extended data buffer. Returns bytes read + * (>= 0) or a negative WS_ error. See wolfssh/ssh.h for the public contract + * (callers MUST drain stderr; WS_REKEYING during key exchange). + * + * A successful read drains the buffer and sends a window adjust for the bytes + * consumed, releasing back-pressure. The read still succeeds if that send + * cannot complete: WS_WANT_WRITE is absorbed (adjust stays queued) and a hard + * failure is logged, not returned. ssh->error is saved/restored across it. */ int wolfSSH_extended_data_read(WOLFSSH* ssh, byte* out, word32 outSz) { byte* buf; @@ -1416,6 +1421,14 @@ int wolfSSH_extended_data_read(WOLFSSH* ssh, byte* out, word32 outSz) return WS_BAD_ARGUMENT; } + /* A read sends a window adjust, which must not go out between KEXINIT and + * NEWKEYS. IsMessageAllowed() gates only transport messages on isKeying, + * not connection-layer ones, so gate here like wolfSSH_stream_read(). */ + if (ssh->isKeying) { + ssh->error = WS_REKEYING; + return WS_REKEYING; + } + /* sanity check to make sure idx is not in a bad state */ if (ssh->extDataBuffer.idx > ssh->extDataBuffer.length) { WLOG(WS_LOG_ERROR, "Bad internal state for buffer index"); @@ -1425,6 +1438,48 @@ int wolfSSH_extended_data_read(WOLFSSH* ssh, byte* out, word32 outSz) buf = ssh->extDataBuffer.buffer + ssh->extDataBuffer.idx; WMEMCPY(out, buf, bufSz); ssh->extDataBuffer.idx += bufSz; + + /* Replenish the channel window for the bytes just consumed. The window + * was charged when the extended data arrived (DoChannelExtendedData), so + * draining it here is what releases back-pressure to the peer. If the + * channel is gone the data is still returned; there is just no window to + * adjust. */ + if (bufSz > 0) { + WOLFSSH_CHANNEL* channel; + + channel = ChannelFind(ssh, ssh->extDataChannelId, WS_CHANNEL_ID_SELF); + if (channel != NULL) { + int adjustResult; + int savedError = ssh->error; + + /* Credit the window locally regardless of send result, mirroring + * _UpdateChannelWindow(). On WS_WANT_WRITE the adjust is already + * bundled into outputBuffer and the peer is credited on a later + * flush; on a hard failure the channel is already failing, so a + * transient local over-advertisement is moot and crediting avoids + * permanently leaking the window for the bytes just consumed. + * + * The bytes are already in out, so this is a completed read: it + * reports the byte count and must not disturb ssh->error. Stomping + * ssh->error here would be invisible to callers that only check the + * return value and surprising to callers that inspect + * wolfSSH_get_error() after a successful read. SendChannelWindow- + * Adjust -> wolfSSH_SendPacket sets ssh->error = WS_WANT_WRITE when + * the bundled adjust cannot be flushed in non-blocking mode, so + * save ssh->error across the send and restore it; a hard send + * failure is logged, not returned. */ + adjustResult = SendChannelWindowAdjust(ssh, channel->channel, + bufSz); + channel->windowSz += bufSz; + ssh->error = savedError; + if (adjustResult != WS_SUCCESS && adjustResult != WS_WANT_WRITE) + WLOG(WS_LOG_ERROR, + "wolfSSH_extended_data_read: window adjust send failed " + "(%d); window credited locally, read still succeeded", + adjustResult); + } + } + return bufSz; } diff --git a/tests/unit.c b/tests/unit.c index f3d1074a8..2caec2a14 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -1769,10 +1769,16 @@ static int test_DoChannelExtendedData_overflow(void) return result; } -/* Exercises the accumulating extended-data buffer: two stderr blobs that - * arrive before the application reads must both be preserved (no silent - * overwrite), and an unknown extended data type must be ignored rather than - * rejected. */ +/* Exercises the accumulating extended-data buffer and its window + * back-pressure: two stderr blobs that arrive before the application reads + * must both be preserved (no silent overwrite), the channel window must be + * charged on receipt and replenished on read, a partial read followed by an + * append must preserve byte ordering and window accounting across the + * GrowBuffer compaction, and an unknown extended data type must be ignored + * rather than rejected. A final case pins the documented single-session + * limitation: stderr arriving on a second channel before the first is drained + * is dropped (its window replenished immediately) rather than corrupting the + * shared buffer's window accounting. */ static int test_DoChannelExtendedData_flow(void) { WOLFSSH_CTX* ctx = NULL; @@ -1824,17 +1830,20 @@ static int test_DoChannelExtendedData_flow(void) goto done; } - /* First stderr blob: buffered. */ + /* First stderr blob: buffered, window charged 128 -> 118. */ idx = 0; ret = wolfSSH_TestDoChannelExtendedData(ssh, (byte*)blob1, (word32)sizeof(blob1), &idx); if (ret != WS_EXTDATA) { result = -604; goto done; } + if (ch->windowSz != 118) { result = -605; goto done; } - /* Second stderr blob before any read: must accumulate, not overwrite. */ + /* Second stderr blob before any read: must accumulate, not overwrite. + * Window charged 118 -> 108. */ idx = 0; ret = wolfSSH_TestDoChannelExtendedData(ssh, (byte*)blob2, (word32)sizeof(blob2), &idx); if (ret != WS_EXTDATA) { result = -606; goto done; } + if (ch->windowSz != 108) { result = -607; goto done; } /* Read everything: both blobs present and in order (no data loss). */ ret = wolfSSH_extended_data_read(ssh, out, (word32)sizeof(out)); @@ -1846,20 +1855,260 @@ static int test_DoChannelExtendedData_flow(void) for (i = 10; i < 20; i++) if (out[i] != 0x22) { result = -610; goto done; } } + /* Draining replenishes the window: 108 + 20 -> 128. */ + if (ch->windowSz != 128) { result = -611; goto done; } /* Buffer drained; a further read returns 0. */ ret = wolfSSH_extended_data_read(ssh, out, (word32)sizeof(out)); if (ret != 0) { result = -612; goto done; } + /* Partial-read-then-append: read part of one blob (leaving idx > 0), then + * receive another blob. The append forces GrowBuffer to compact a non-zero + * idx in place (WMEMMOVE) before adding the new data, and the window must + * track a read that is split across the receipt of more data. */ + idx = 0; + ret = wolfSSH_TestDoChannelExtendedData(ssh, (byte*)blob1, + (word32)sizeof(blob1), &idx); + if (ret != WS_EXTDATA) { result = -630; goto done; } + if (ch->windowSz != 118) { result = -631; goto done; } + + /* Read 4 of the 10 buffered bytes: window 118 + 4 -> 122, idx left at 4. */ + ret = wolfSSH_extended_data_read(ssh, out, 4); + if (ret != 4) { result = -632; goto done; } + { + int i; + for (i = 0; i < 4; i++) + if (out[i] != 0x11) { result = -633; goto done; } + } + if (ch->windowSz != 122) { result = -634; goto done; } + + /* Append blob2 with 6 unread bytes still pending (idx=4, length=10): the + * 6 remaining 0x11 bytes must be preserved ahead of the new 0x22 bytes. + * Window charged 122 -> 112. */ + idx = 0; + ret = wolfSSH_TestDoChannelExtendedData(ssh, (byte*)blob2, + (word32)sizeof(blob2), &idx); + if (ret != WS_EXTDATA) { result = -635; goto done; } + if (ch->windowSz != 112) { result = -636; goto done; } + + /* Read the rest: 6 leftover 0x11 followed by 10 0x22, in order. Window + * 112 + 16 -> 128. */ + ret = wolfSSH_extended_data_read(ssh, out, (word32)sizeof(out)); + if (ret != 16) { result = -637; goto done; } + { + int i; + for (i = 0; i < 6; i++) + if (out[i] != 0x11) { result = -638; goto done; } + for (i = 6; i < 16; i++) + if (out[i] != 0x22) { result = -639; goto done; } + } + if (ch->windowSz != 128) { result = -640; goto done; } + + /* Drained again. */ + ret = wolfSSH_extended_data_read(ssh, out, (word32)sizeof(out)); + if (ret != 0) { result = -641; goto done; } + /* Unknown extended data type: ignored (consumed), not rejected. Nothing - * is buffered for the application. */ + * is buffered and the window is left intact (replenished on receipt). */ idx = 0; ret = wolfSSH_TestDoChannelExtendedData(ssh, (byte*)unknownBlob, (word32)sizeof(unknownBlob), &idx); if (ret != WS_SUCCESS) { result = -613; goto done; } + if (ch->windowSz != 128) { result = -614; goto done; } ret = wolfSSH_extended_data_read(ssh, out, (word32)sizeof(out)); if (ret != 0) { result = -615; goto done; } + /* Window-overflow branch: draw the window down below a blob that is + * still <= maxPacketSz, then confirm the next blob is rejected by the + * "dataSz > windowSz" branch and not the maxPacketSz branch. dataSz=60 + * stays under maxPacketSz=64 so the maxPacketSz check cannot fire first. */ + { + byte big[12 + 60]; + int i; + + WMEMSET(big, 0, sizeof(big)); + big[7] = 0x01; /* type = stderr */ + big[11] = 0x3C; /* dataSz = 60 */ + for (i = 12; i < (int)sizeof(big); i++) + big[i] = 0x44; + + /* window 128 -> 68 */ + idx = 0; + ret = wolfSSH_TestDoChannelExtendedData(ssh, big, + (word32)sizeof(big), &idx); + if (ret != WS_EXTDATA) { result = -616; goto done; } + if (ch->windowSz != 68) { result = -617; goto done; } + + /* window 68 -> 8 */ + idx = 0; + ret = wolfSSH_TestDoChannelExtendedData(ssh, big, + (word32)sizeof(big), &idx); + if (ret != WS_EXTDATA) { result = -618; goto done; } + if (ch->windowSz != 8) { result = -619; goto done; } + + /* dataSz=60 <= maxPacketSz but > windowSz=8: window-overflow reject, + * window left unchanged. */ + idx = 0; + ret = wolfSSH_TestDoChannelExtendedData(ssh, big, + (word32)sizeof(big), &idx); + if (ret != WS_RECV_OVERFLOW_E) { result = -620; goto done; } + if (ch->windowSz != 8) { result = -621; goto done; } + } + + /* Cross-channel fail safe: the extData buffer and extDataChannelId are + * shared across channels, so only one channel's stderr can be buffered at a + * time. Stderr arriving on a second channel before the first is drained is + * dropped (its window replenished immediately) instead of corrupting the + * window accounting. This pins the documented single-session limitation. + * First drain the 120 bytes still pending on channel 0 from the overflow + * block (8 + 120 -> 128 on read) to reach a clean state. */ + { + WOLFSSH_CHANNEL* chB = NULL; + /* channelId=1, type=1 (stderr), dataSz=10, payload all 0x22 */ + static const byte blobB[] = { + 0x00, 0x00, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x0A, + 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22 + }; + + ret = wolfSSH_extended_data_read(ssh, out, (word32)sizeof(out)); + if (ret != 32) { result = -650; goto done; } + ret = wolfSSH_extended_data_read(ssh, out, (word32)sizeof(out)); + if (ret != 32) { result = -651; goto done; } + ret = wolfSSH_extended_data_read(ssh, out, (word32)sizeof(out)); + if (ret != 32) { result = -652; goto done; } + ret = wolfSSH_extended_data_read(ssh, out, (word32)sizeof(out)); + if (ret != 24) { result = -653; goto done; } + ret = wolfSSH_extended_data_read(ssh, out, (word32)sizeof(out)); + if (ret != 0) { result = -654; goto done; } + if (ch->windowSz != 128) { result = -655; goto done; } + + /* Second channel, id 1, same window/packet sizes as channel 0. */ + chB = ChannelNew(ssh, ID_CHANTYPE_SESSION, 128, 64); + if (chB == NULL) { result = -656; goto done; } + if (ChannelAppend(ssh, chB) != WS_SUCCESS) { + ChannelDelete(chB, ssh->ctx->heap); + result = -657; + goto done; + } + + /* Channel 0 buffers stderr first: charged 128 -> 118, extDataChannelId + * becomes 0, no warning (nothing was pending). */ + idx = 0; + ret = wolfSSH_TestDoChannelExtendedData(ssh, (byte*)blob1, + (word32)sizeof(blob1), &idx); + if (ret != WS_EXTDATA) { result = -658; goto done; } + if (ch->windowSz != 118) { result = -659; goto done; } + + /* Channel 1 sends stderr while channel 0 still has unread data. The + * shared single-session buffer can only hold one channel's stderr, so + * this blob is dropped (fail safe) rather than corrupting the window + * accounting. Its window is replenished immediately (SendChannelWindow- + * Adjust returns WS_SUCCESS), so chB stays at 128, the data is not + * buffered, and extDataChannelId is left at channel 0. */ + idx = 0; + ret = wolfSSH_TestDoChannelExtendedData(ssh, (byte*)blobB, + (word32)sizeof(blobB), &idx); + if (ret != WS_SUCCESS) { result = -660; goto done; } + if (chB->windowSz != 128) { result = -661; goto done; } + + /* One read drains only channel 0's buffered 10 bytes (channel 1's were + * dropped, not buffered) and credits them back to channel 0: + * 118 + 10 -> 128. No window is leaked and no channel is over-credited. */ + ret = wolfSSH_extended_data_read(ssh, out, (word32)sizeof(out)); + if (ret != 10) { result = -662; goto done; } + { + int i; + for (i = 0; i < 10; i++) + if (out[i] != 0x11) { result = -663; goto done; } + } + if (ch->windowSz != 128) { result = -665; goto done; } + if (chB->windowSz != 128) { result = -666; goto done; } + } + +done: + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + return result; +} + +/* Counts IoSend calls so a test can assert whether anything went on the wire. */ +static word32 s_extSendCount = 0; +static int CountIoSend(WOLFSSH* ssh, void* buf, word32 sz, void* ctx) +{ + (void)ssh; (void)buf; (void)ctx; + s_extSendCount++; + return (int)sz; +} + +/* RFC 4253 section 7.1: no connection-layer message (such as + * SSH_MSG_CHANNEL_WINDOW_ADJUST) may go out between KEXINIT and NEWKEYS. The + * receive-path discard branches in DoChannelExtendedData() must therefore not + * send a window adjust mid-KEX; they park the owed credit on the channel and + * SendPendingChannelWindowAdjust() flushes it once keying completes. */ +static int test_DoChannelExtendedData_keying(void) +{ + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + WOLFSSH_CHANNEL* ch = NULL; + int result = 0; + int ret; + word32 idx; + + /* channelId=0, unknown type=2, dataSz=5, payload all 0x33 */ + static const byte unknownBlob[] = { + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x02, + 0x00, 0x00, 0x00, 0x05, + 0x33, 0x33, 0x33, 0x33, 0x33 + }; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + if (ctx == NULL) + return -700; + wolfSSH_SetIOSend(ctx, CountIoSend); + + ssh = wolfSSH_new(ctx); + if (ssh == NULL) { result = -701; goto done; } + /* Allow MSGID_CHANNEL_WINDOW_ADJUST on this bare session. */ + ssh->acceptState = ACCEPT_SERVER_USERAUTH_SENT; + + ch = ChannelNew(ssh, ID_CHANTYPE_SESSION, 128, 64); + if (ch == NULL) { result = -702; goto done; } + if (ChannelAppend(ssh, ch) != WS_SUCCESS) { + ChannelDelete(ch, ssh->ctx->heap); + result = -703; + goto done; + } + + /* Pretend a rekey is in progress. Unknown extended data is consumed, but + * the owed window credit must not go on the wire; it is parked instead. */ + ssh->isKeying = WOLFSSH_SELF_IS_KEYING; + s_extSendCount = 0; + + idx = 0; + ret = wolfSSH_TestDoChannelExtendedData(ssh, (byte*)unknownBlob, + (word32)sizeof(unknownBlob), &idx); + if (ret != WS_SUCCESS) { result = -704; goto done; } + if (s_extSendCount != 0) { result = -705; goto done; } + if (ch->pendingWindowAdjust != 5) { result = -706; goto done; } + /* Discarded data is never charged to the local window. */ + if (ch->windowSz != 128) { result = -707; goto done; } + + /* Flushing while still keying is a no-op: credit stays parked. */ + ret = wolfSSH_TestSendPendingChannelWindowAdjust(ssh); + if (ret != WS_SUCCESS) { result = -708; goto done; } + if (s_extSendCount != 0) { result = -709; goto done; } + if (ch->pendingWindowAdjust != 5) { result = -710; goto done; } + + /* Keying complete: the parked WINDOW_ADJUST is sent and the credit clears. */ + ssh->isKeying = 0; + ret = wolfSSH_TestSendPendingChannelWindowAdjust(ssh); + if (ret != WS_SUCCESS) { result = -711; goto done; } + if (s_extSendCount != 1) { result = -712; goto done; } + if (ch->pendingWindowAdjust != 0) { result = -713; goto done; } + done: wolfSSH_free(ssh); wolfSSH_CTX_free(ctx); @@ -5249,6 +5498,11 @@ int wolfSSH_UnitTest(int argc, char** argv) (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; + unitResult = test_DoChannelExtendedData_keying(); + printf("DoChannelExtendedData_keying: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; + unitResult = test_SendChannelData_eofTxd(); printf("SendChannelData_eofTxd: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 858c9a9bc..4e8b98a24 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -874,7 +874,24 @@ struct WOLFSSH { WOLFSSH_BUFFER inputBuffer; WOLFSSH_BUFFER outputBuffer; - WOLFSSH_BUFFER extDataBuffer; /* extended data ready to be read */ + WOLFSSH_BUFFER extDataBuffer; /* extended data ready to be read. Shared + * across channels, so stderr buffering and + * its window back-pressure are effectively + * single-channel: only extDataChannelId is + * replenished on read. */ + word32 extDataChannelId; /* self channel id whose window the buffered + * extended data was charged against; its + * window is replenished as the app reads. + * The buffer and this id are shared, so only + * one channel's stderr can be buffered at a + * time. While that channel's stderr is still + * unread, stderr arriving on a different + * channel is dropped (fail safe) and that + * channel's window is credited back, rather + * than overwriting this id and desyncing the + * windows; see DoChannelExtendedData(). + * Buffered stderr is intended for the + * single-session case. */ WC_RNG* rng; byte h[WC_MAX_DIGEST_SIZE]; @@ -1009,6 +1026,15 @@ struct WOLFSSH_CHANNEL { word32 peerChannel; word32 peerWindowSz; word32 peerMaxPacketSz; + word32 pendingWindowAdjust; /* receive-window credit owed to the peer for + * discarded extended data (unknown type, or + * stderr dropped while another channel's stderr + * is still buffered) that could not be sent + * because a rekey was in progress. RFC 4253 + * section 7.1 forbids connection-layer messages + * between KEXINIT and NEWKEYS. Flushed by + * SendPendingChannelWindowAdjust() once keying + * completes. */ #ifdef WOLFSSH_FWD char* host; word32 hostPort; @@ -1404,6 +1430,7 @@ enum WS_MessageIdLimits { word32 len, word32* idx); WOLFSSH_API int wolfSSH_TestDoChannelExtendedData(WOLFSSH* ssh, byte* buf, word32 len, word32* idx); + WOLFSSH_API int wolfSSH_TestSendPendingChannelWindowAdjust(WOLFSSH* ssh); WOLFSSH_API int wolfSSH_TestDoKexInit(WOLFSSH* ssh, byte* buf, word32 len, word32* idx); WOLFSSH_API int wolfSSH_TestDoNewKeys(WOLFSSH* ssh, byte* buf, diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index 3b3f2279b..ff79cadd7 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -442,6 +442,13 @@ WOLFSSH_API int wolfSSH_stream_read(WOLFSSH* ssh, byte* buf, word32 bufSz); WOLFSSH_API int wolfSSH_stream_send(WOLFSSH* ssh, byte* buf, word32 bufSz); WOLFSSH_API int wolfSSH_stream_exit(WOLFSSH* ssh, int status); WOLFSSH_API int wolfSSH_extended_data_send(WOLFSSH* ssh, byte* buf, word32 bufSz); +/* Reads buffered stderr into out; returns bytes read (>= 0) or a negative + * WS_ error (WS_REKEYING during key exchange). + * + * Apps MUST drain stderr: it shares the channel receive window with stdout + * (RFC 4254 5.2) and is only replenished here, so unread stderr stalls the + * channel. Only one channel's stderr buffers at a time; a second channel's is + * dropped (window replenished, nothing leaks). */ WOLFSSH_API int wolfSSH_extended_data_read(WOLFSSH* ssh, byte* out, word32 outSz); WOLFSSH_API int wolfSSH_TriggerKeyExchange(WOLFSSH* ssh);