From 8bda169dd0ccf6dad4a8481339910fa38095bf34 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 14 Aug 2026 15:11:47 -0700 Subject: [PATCH 01/12] Mark the forwarding packet builders maybe-unused BuildDirectTcpipExtra() and BuildGlobalRequestFwdPacket() have callers in several conditional blocks, and a build with none of them left the two functions unused, which -Werror turns into a build failure. ReadUint32() next to them already carries the attribute for the same reason. - --disable-server builds regress.c again. --- tests/regress.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/regress.c b/tests/regress.c index e7dff779d..333d9c75f 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -291,8 +291,10 @@ static word32 BuildChannelDataPacket(word32 peerChannelId, const char* data, #ifdef WOLFSSH_FWD -static word32 BuildDirectTcpipExtra(const char* host, word32 hostPort, - const char* origin, word32 originPort, byte* out, word32 outSz) +/* Callers sit in separate conditional blocks; some builds have none. */ +static WS_MAYBE_UNUSED word32 BuildDirectTcpipExtra(const char* host, + word32 hostPort, const char* origin, word32 originPort, byte* out, + word32 outSz) { word32 idx = 0; @@ -304,8 +306,8 @@ static word32 BuildDirectTcpipExtra(const char* host, word32 hostPort, return idx; } -static word32 BuildGlobalRequestFwdPacket(const char* bindAddr, word32 bindPort, - int isCancel, byte wantReply, byte* out, word32 outSz) +static WS_MAYBE_UNUSED word32 BuildGlobalRequestFwdPacket(const char* bindAddr, + word32 bindPort, int isCancel, byte wantReply, byte* out, word32 outSz) { byte payload[256]; word32 idx = 0; From 6362852cf458f61db5612a353825f35304a1113f Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 24 Jul 2026 14:10:24 -0700 Subject: [PATCH 02/12] Report whether a global request reached the peer A send's return code cannot tell a caller its request is on its way. The highwater callback runs after the last byte goes out, so a rekey's errors surface as the send's, and WS_WANT_WRITE leaves the packet framed for the next flush. - Count the flushes wolfSSH_SendPacket() completes. - Compare that count across a send to tell those outcomes apart. - SendGlobalRequest() and SendGlobalRequestFwd() carry the answer in an optional out-param. - Both callers pass NULL, so nothing acts on it yet. Issue: ZD-22195 --- src/internal.c | 50 ++++++++++++++++++++++++++++++++++++++++++---- src/ssh.c | 8 +++++--- wolfssh/internal.h | 10 ++++++++-- 3 files changed, 59 insertions(+), 9 deletions(-) diff --git a/src/internal.c b/src/internal.c index 5b385b1c8..8df903a9d 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4350,6 +4350,11 @@ int wolfSSH_SendPacket(WOLFSSH* ssh) * call a licence to push whatever gets queued next. */ ssh->disconnectTxd = 0; + /* Everything framed is on the wire. What runs below can fail, and the + * return code alone cannot tell a caller its packet was delivered, so + * record the flush first. */ + ssh->txFlushCount++; + WLOG(WS_LOG_DEBUG, "SB: Shrinking output buffer"); ShrinkBuffer(&ssh->outputBuffer, 0); return HighwaterCheck(ssh, WOLFSSH_HWSIDE_TRANSMIT); @@ -16995,13 +17000,34 @@ int SendIgnore(WOLFSSH* ssh, const unsigned char* data, word32 dataSz) return ret; } +/* Will the packet just framed reach the peer? A completed flush says so; the + * return does not, since the highwater callback runs after the last byte goes + * out and the rekey it starts fails with the same codes a lost send does. + * Comparing the flush count across the send tells those apart. + * + * Short of a flush, WS_WANT_WRITE is the one outcome that keeps the packet + * framed for the next one, and reading the buffer instead would call a packet + * delivered that a later purge or a discarding error path throws away. + * Anything else counts as not sent, which at worst leaves the peer holding a + * request this side did not register; guessing the other way would desync the + * reply queue for the life of the session. Call before anything else runs, + * since a later send flushes this packet and would read as this one's. */ +static INLINE int SendPacketDelivered(WOLFSSH* ssh, word32 flushes, int ret) +{ + return ssh->txFlushCount != flushes || ret == WS_WANT_WRITE; +} + + int SendGlobalRequest(WOLFSSH* ssh, - const unsigned char* data, word32 dataSz, int reply) + const unsigned char* data, word32 dataSz, int reply, int* sent) { byte* output; word32 idx = 0; int ret = WS_SUCCESS; + if (sent != NULL) + *sent = 0; + if (ssh == NULL || (data == NULL && dataSz > 0)) ret = WS_BAD_ARGUMENT; @@ -17029,9 +17055,15 @@ int SendGlobalRequest(WOLFSSH* ssh, ret = BundlePacket(ssh); } - if (ret == WS_SUCCESS) + if (ret == WS_SUCCESS) { + word32 flushes = ssh->txFlushCount; + ret = wolfSSH_SendPacket(ssh); + if (sent != NULL) + *sent = SendPacketDelivered(ssh, flushes, ret); + } + WLOG(WS_LOG_DEBUG, "Leaving SendGlobalRequest(), ret = %d", ret); return ret; @@ -17043,7 +17075,8 @@ int SendGlobalRequest(WOLFSSH* ssh, * address and port follow the want-reply boolean, an ordering the generic * SendGlobalRequest() framing cannot express. RFC 4254 7.1. */ int SendGlobalRequestFwd(WOLFSSH* ssh, - const char* bindAddr, word32 bindPort, int isCancel, int wantReply) + const char* bindAddr, word32 bindPort, int isCancel, int wantReply, + int* sent) { byte* output; word32 idx = 0; @@ -17054,6 +17087,9 @@ int SendGlobalRequestFwd(WOLFSSH* ssh, WLOG(WS_LOG_DEBUG, "Entering SendGlobalRequestFwd()"); + if (sent != NULL) + *sent = 0; + if (ssh == NULL || bindAddr == NULL) ret = WS_BAD_ARGUMENT; @@ -17088,9 +17124,15 @@ int SendGlobalRequestFwd(WOLFSSH* ssh, ret = BundlePacket(ssh); } - if (ret == WS_SUCCESS) + if (ret == WS_SUCCESS) { + word32 flushes = ssh->txFlushCount; + ret = wolfSSH_SendPacket(ssh); + if (sent != NULL) + *sent = SendPacketDelivered(ssh, flushes, ret); + } + WLOG(WS_LOG_DEBUG, "Leaving SendGlobalRequestFwd(), ret = %d", ret); return ret; diff --git a/src/ssh.c b/src/ssh.c index 9388c8d86..273e68f37 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1648,7 +1648,7 @@ int wolfSSH_global_request(WOLFSSH *ssh, const unsigned char* data, word32 dataS return WS_BAD_ARGUMENT; if (SendAfterDisconnect(ssh)) return WS_FATAL_ERROR; - return SendGlobalRequest(ssh, data, dataSz, reply); + return SendGlobalRequest(ssh, data, dataSz, reply, NULL); } @@ -3953,7 +3953,8 @@ int wolfSSH_FwdRemoteSetup(WOLFSSH* ssh, const char* bindAddr, ret = WS_REKEYING; if (ret == WS_SUCCESS) - ret = SendGlobalRequestFwd(ssh, bindAddr, bindPort, 0, wantReply); + ret = SendGlobalRequestFwd(ssh, bindAddr, bindPort, 0, wantReply, + NULL); WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_FwdRemoteSetup(), ret = %d", ret); return ret; @@ -3991,7 +3992,8 @@ int wolfSSH_FwdRemoteCancel(WOLFSSH* ssh, const char* bindAddr, ret = WS_REKEYING; if (ret == WS_SUCCESS) - ret = SendGlobalRequestFwd(ssh, bindAddr, bindPort, 1, wantReply); + ret = SendGlobalRequestFwd(ssh, bindAddr, bindPort, 1, wantReply, + NULL); WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_FwdRemoteCancel(), ret = %d", ret); return ret; diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 2627602e0..6254cb698 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1024,6 +1024,7 @@ struct WOLFSSH { word32 rxCount; word32 txMsgCount; /* Packets sent under current keys */ word32 rxMsgCount; /* Packets received under current keys */ + word32 txFlushCount; /* Output buffer drained, whatever came after */ word32 highwaterMark; word32 msgHighwaterMark; /* Per-key packet limit (RFC 4344 Sec 3.1) */ byte highwaterFlag; /* Set when highwater CB called */ @@ -1559,11 +1560,16 @@ WOLFSSH_LOCAL int SendIgnore(WOLFSSH* ssh, const unsigned char* data, word32 dataSz); WOLFSSH_LOCAL int SendGlobalRequestFwdSuccess(WOLFSSH * ssh, int success, word32 port); +/* The optional sent out-param reports whether the request is on its way to the + * peer -- flushed, or still framed for the next flush -- which the return does + * not answer: the highwater callback runs after the last byte goes out, so its + * failure surfaces as this call's. */ WOLFSSH_LOCAL int SendGlobalRequest(WOLFSSH * ssh, - const unsigned char * data, word32 dataSz, int reply); + const unsigned char * data, word32 dataSz, int reply, int* sent); #ifdef WOLFSSH_FWD WOLFSSH_LOCAL int SendGlobalRequestFwd(WOLFSSH* ssh, - const char* bindAddr, word32 bindPort, int isCancel, int wantReply); + const char* bindAddr, word32 bindPort, int isCancel, int wantReply, + int* sent); #endif WOLFSSH_LOCAL int SendDebug(WOLFSSH* ssh, byte alwaysDisplay, const char* msg); WOLFSSH_LOCAL int SendServiceRequest(WOLFSSH* ssh, byte serviceId); From 5b36c829475e43893d6de2c07c3700ed60c1ceeb Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 14 Aug 2026 15:14:34 -0700 Subject: [PATCH 03/12] Match forwarded-tcpip to registered forwards RFC 4254 7.2 says a forwarded-tcpip open answers a forward the client asked for, so refuse an open naming anything else. Enforcement starts at the first wolfSSH_FwdRemoteSetup(), leaving a client that frames tcpip-forward itself unaffected. - Register each setup per session. A wildcard bind matches on port alone. - Port 0 now requires want-reply, since only the reply names the port. - Repeat setups of one bind share a registration, so one cancel undoes it. - A cancel stops matching as it goes out, but a want-reply cancel stays registered until the peer answers: a refusal leaves the listener up. - Replies carry no request id, so a per-session queue pairs them in send order. A want-reply wolfSSH_global_request() takes a slot as well. - Registration is split around the send: allocate first, link once the request reached the wire. - A request resolves its registration on commit, since sending runs application callbacks that can reenter the library. - Tests cover matching, cancel, overlapping requests, send-order pairing, port 0, registration around the send, and reentrancy from a callback. - They drive a client session, so they sit outside the server-only block in regress.c and run in a --disable-server build. The harness struct, its teardown, the channel-open-failure helpers and the forwarding callback moved out with them, shared with the server-side tests. Contracts for wolfSSH_FwdRemoteSetup(), wolfSSH_FwdRemoteCancel() and wolfSSH_global_request() are in wolfssh/ssh.h. Issue: ZD-22195 --- src/internal.c | 597 +++++++++++++++++ src/ssh.c | 86 ++- tests/api.c | 4 + tests/regress.c | 1530 ++++++++++++++++++++++++++++++++++++++++++-- wolfssh/internal.h | 65 +- wolfssh/ssh.h | 42 ++ 6 files changed, 2270 insertions(+), 54 deletions(-) diff --git a/src/internal.c b/src/internal.c index 8df903a9d..5436eb098 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1834,6 +1834,9 @@ void SshResourceFree(WOLFSSH* ssh, void* heap) ssh->modesSz = 0; } #endif +#ifdef WOLFSSH_FWD + FwdRemoteFreeList(ssh, heap); +#endif #ifdef WOLFSSH_STATIC_MEMORY if (heap) { WOLFSSL_HEAP_HINT* hint = (WOLFSSL_HEAP_HINT*)heap; @@ -3946,6 +3949,578 @@ int ChannelUpdateForward(WOLFSSH_CHANNEL* channel, return ret; } + + +/* A bind address naming every local address. There is no telling which of + * these the peer echoes back, so a wildcard registration matches whatever + * address it reports. */ +static int FwdRemoteAddrIsWild(const char* addr) +{ + static const char* wild[] = { + "*", "0.0.0.0", "::", "::0", "0:0:0:0:0:0:0:0", "::ffff:0.0.0.0" + }; + word32 i; + + if (addr[0] == '\0') + return 1; + + for (i = 0; i < (word32)(sizeof(wild) / sizeof(wild[0])); i++) { + if (WSTRCMP(addr, wild[i]) == 0) + return 1; + } + + return 0; +} + + +/* Detach this forward from every queued reply slot. The slots stay queued to + * keep the send order; they just no longer name it. */ +static void FwdReplyVoid(WOLFSSH* ssh, const WOLFSSH_FWD_REMOTE* entry) +{ + WOLFSSH_FWD_REPLY* reply; + + for (reply = ssh->fwdReplyHead; reply != NULL; reply = reply->next) { + if (reply->entry == entry) + reply->entry = NULL; + } +} + + +static void FwdRemoteUnlink(WOLFSSH* ssh, void* heap, + WOLFSSH_FWD_REMOTE* entry) +{ + WOLFSSH_FWD_REMOTE* cur; + + if (ssh->fwdRemoteList == entry) { + ssh->fwdRemoteList = entry->next; + } + else { + for (cur = ssh->fwdRemoteList; cur != NULL; cur = cur->next) { + if (cur->next == entry) { + cur->next = entry->next; + break; + } + } + } + + FwdReplyVoid(ssh, entry); + + WFREE(entry->bindAddr, heap, DYNTYPE_STRING); + WFREE(entry, heap, DYNTYPE_FWD); +} + + +/* The last queued request naming this forward, or NULL. The queue is in send + * order, so this is what the application asked for most recently. */ +static WOLFSSH_FWD_REPLY* FwdReplyNewest(WOLFSSH* ssh, + const WOLFSSH_FWD_REMOTE* entry) +{ + WOLFSSH_FWD_REPLY* cur; + WOLFSSH_FWD_REPLY* newest = NULL; + + for (cur = ssh->fwdReplyHead; cur != NULL; cur = cur->next) { + if (cur->entry == entry) + newest = cur; + } + + return newest; +} + + +/* Is a tcpip-forward naming this forward still waiting on the peer? */ +static int FwdReplyHasSetup(WOLFSSH* ssh, const WOLFSSH_FWD_REMOTE* entry) +{ + WOLFSSH_FWD_REPLY* cur; + + for (cur = ssh->fwdReplyHead; cur != NULL; cur = cur->next) { + if (cur->entry == entry && !cur->isCancel) + return 1; + } + + return 0; +} + + +/* The registration for bindAddr:bindPort, or NULL. A port-0 request has no + * port to be found by until the peer's reply names the one it bound. An entry + * with a cancel outstanding is still found, since a later request names the + * same listener. */ +static WOLFSSH_FWD_REMOTE* FwdRemoteFind(WOLFSSH* ssh, const char* bindAddr, + word32 bindPort) +{ + WOLFSSH_FWD_REMOTE* cur; + + for (cur = ssh->fwdRemoteList; cur != NULL; cur = cur->next) { + if (cur->portPending || cur->bindPort != bindPort) + continue; + if (WSTRCMP(cur->bindAddr, bindAddr) == 0) + return cur; + } + + return NULL; +} + + +/* Apply an answer to the forward it names. Only a port-0 request has any use + * for the port the answer carried. */ +static void FwdRemoteSettle(WOLFSSH* ssh, WOLFSSH_FWD_REMOTE* entry, + int isCancel, int success, word32 port) +{ + /* The application framed this request itself, or the forward it named is + * already gone. */ + if (entry == NULL) + return; + + if (isCancel) { + if (!success) { + /* The peer kept the listener, so the forward stands and matching + * resumes unless a later cancel is outstanding. */ + return; + } + + /* The listener is down. A setup sent after this cancel asks the peer + * to bind anew, so the forward waits on that answer instead of + * going. */ + if (FwdReplyHasSetup(ssh, entry)) + entry->confirmed = 0; + else + FwdRemoteUnlink(ssh, ssh->ctx->heap, entry); + return; + } + + if (!success) { + /* The peer bound nothing for this request. Repeat setups share one + * registration and a peer refuses the duplicates it already has a + * listener for, so only unwind a forward nothing else has established + * or is still owed an answer on. */ + if (!entry->confirmed && FwdReplyNewest(ssh, entry) == NULL) + FwdRemoteUnlink(ssh, ssh->ctx->heap, entry); + return; + } + + if (entry->portPending) { + WOLFSSH_FWD_REMOTE* dup; + WOLFSSH_FWD_REMOTE* next; + + if (port == 0 || port > 65535) { + WLOG(WS_LOG_WARN, "Remote forward reply named no usable port"); + FwdRemoteUnlink(ssh, ssh->ctx->heap, entry); + return; + } + entry->bindPort = port; + entry->portPending = 0; + + /* The peer named a port another registration already stands for. It + * has one listener there, so the older entry is stale. A cancel names + * a forward by its bind alone, so a bind gets one registration. */ + for (dup = ssh->fwdRemoteList; dup != NULL; dup = next) { + next = dup->next; + if (dup == entry || dup->portPending || + dup->bindPort != entry->bindPort || + WSTRCMP(dup->bindAddr, entry->bindAddr) != 0) + continue; + + WLOG(WS_LOG_INFO, "Remote forward reply named a port already " + "registered"); + FwdRemoteUnlink(ssh, ssh->ctx->heap, dup); + } + } + + entry->confirmed = 1; +} + + +/* Take this request's place in the reply queue before it is sent, so a + * callback that reenters the library mid-send cannot queue ahead of it. Which + * forward the slot answers for is filled in on commit. */ +static WOLFSSH_FWD_REPLY* FwdReplyNew(WOLFSSH* ssh, int isCancel) +{ + WOLFSSH_FWD_REPLY* reply; + + reply = (WOLFSSH_FWD_REPLY*)WMALLOC(sizeof(WOLFSSH_FWD_REPLY), + ssh->ctx->heap, DYNTYPE_FWD); + if (reply != NULL) { + WMEMSET(reply, 0, sizeof(WOLFSSH_FWD_REPLY)); + reply->isCancel = (byte)(isCancel != 0); + /* The sender owns this slot until it commits; an answer arriving + * meanwhile parks its verdict here. */ + reply->uncommitted = 1; + + if (ssh->fwdReplyTail == NULL) + ssh->fwdReplyHead = reply; + else + ssh->fwdReplyTail->next = reply; + ssh->fwdReplyTail = reply; + } + + return reply; +} + + +/* Give back a slot the request never went out to claim. An answer that arrived + * mid-send may have dequeued it already. */ +static void FwdReplyUnqueue(WOLFSSH* ssh, WOLFSSH_FWD_REPLY* reply) +{ + WOLFSSH_FWD_REPLY* cur; + WOLFSSH_FWD_REPLY* prev = NULL; + + for (cur = ssh->fwdReplyHead; cur != NULL; cur = cur->next) { + if (cur == reply) + break; + prev = cur; + } + + if (cur == NULL) + return; + + if (prev == NULL) + ssh->fwdReplyHead = reply->next; + else + prev->next = reply->next; + + if (ssh->fwdReplyTail == reply) + ssh->fwdReplyTail = prev; + + WFREE(reply, ssh->ctx->heap, DYNTYPE_FWD); +} + + +/* Build the bookkeeping for a tcpip-forward or cancel-tcpip-forward the client + * is about to send. Allocating up front keeps every failure ahead of the send. + * Nothing is registered until the caller commits, but a want-reply request + * claims its reply-queue slot here, which a discard gives back. */ +int FwdRemotePrepare(WOLFSSH* ssh, const char* bindAddr, word32 bindPort, + int wantReply, int isCancel, WOLFSSH_FWD_PENDING* pend) +{ + WOLFSSH_FWD_REMOTE* found; + void* heap; + word32 addrSz; + int ret = WS_SUCCESS; + + WLOG(WS_LOG_DEBUG, "Entering FwdRemotePrepare()"); + + if (pend != NULL) + WMEMSET(pend, 0, sizeof(*pend)); + + if (ssh == NULL || ssh->ctx == NULL || bindAddr == NULL || pend == NULL) + return WS_BAD_ARGUMENT; + + heap = ssh->ctx->heap; + pend->isCancel = (byte)(isCancel != 0); + pend->bindAddr = bindAddr; + pend->bindPort = bindPort; + found = FwdRemoteFind(ssh, bindAddr, bindPort); + + if (isCancel) { + if (found == NULL) { + WOLFSSH_FWD_REMOTE* cur; + int pending = 0; + + /* A port-0 forward cannot be cancelled until the peer's reply + * names the port it bound. Worth telling apart from a bind that + * was never registered at all. */ + for (cur = ssh->fwdRemoteList; cur != NULL; cur = cur->next) { + if (cur->portPending && + WSTRCMP(cur->bindAddr, bindAddr) == 0) { + pending = 1; + break; + } + } + + if (pending) + WLOG(WS_LOG_WARN, "Cancelling a remote forward before the " + "peer has named the port it bound"); + else + WLOG(WS_LOG_WARN, + "Cancelling a remote forward that wasn't registered"); + } + } + else if (found == NULL) { + /* A repeat setup of a registered addr:port reuses that entry, so one + * cancel undoes it. Port 0 always makes a new entry, since the peer + * picks a different port each time. */ + addrSz = (word32)WSTRLEN(bindAddr); + pend->entry = (WOLFSSH_FWD_REMOTE*)WMALLOC(sizeof(WOLFSSH_FWD_REMOTE), + heap, DYNTYPE_FWD); + if (pend->entry == NULL) { + ret = WS_MEMORY_E; + } + else { + WMEMSET(pend->entry, 0, sizeof(WOLFSSH_FWD_REMOTE)); + pend->entry->bindAddr = (char*)WMALLOC(addrSz + 1, heap, + DYNTYPE_STRING); + if (pend->entry->bindAddr == NULL) { + WFREE(pend->entry, heap, DYNTYPE_FWD); + pend->entry = NULL; + ret = WS_MEMORY_E; + } + else { + WMEMCPY(pend->entry->bindAddr, bindAddr, addrSz); + pend->entry->bindAddr[addrSz] = '\0'; + pend->entry->bindPort = bindPort; + /* Nothing to match on until the peer's reply names the port + * it bound. */ + pend->entry->portPending = (byte)(bindPort == 0); + } + } + } + + if (ret == WS_SUCCESS && wantReply) { + pend->reply = FwdReplyNew(ssh, isCancel); + if (pend->reply == NULL) { + if (pend->entry != NULL) { + WFREE(pend->entry->bindAddr, heap, DYNTYPE_STRING); + WFREE(pend->entry, heap, DYNTYPE_FWD); + } + ret = WS_MEMORY_E; + } + } + + /* An error leaves nothing to commit and nothing to give back. */ + if (ret != WS_SUCCESS) + WMEMSET(pend, 0, sizeof(*pend)); + + WLOG(WS_LOG_DEBUG, "Leaving FwdRemotePrepare(), ret = %d", ret); + return ret; +} + + +/* Reserve a reply slot for a want-reply global request the application framed + * itself. It names no forward, but it consumes a reply, so it holds a place in + * the queue. */ +int FwdReplyPrepare(WOLFSSH* ssh, WOLFSSH_FWD_PENDING* pend) +{ + int ret; + + WLOG(WS_LOG_DEBUG, "Entering FwdReplyPrepare()"); + + if (pend != NULL) + WMEMSET(pend, 0, sizeof(*pend)); + + if (ssh == NULL || ssh->ctx == NULL || pend == NULL) + return WS_BAD_ARGUMENT; + + pend->reply = FwdReplyNew(ssh, 0); + ret = pend->reply == NULL ? WS_MEMORY_E : WS_SUCCESS; + + WLOG(WS_LOG_DEBUG, "Leaving FwdReplyPrepare(), ret = %d", ret); + return ret; +} + + +/* The request reached the wire, so link what was prepared. The registration it + * names is looked up again here: the send runs the application's send and + * highwater callbacks, which can reenter the library and free the entry a + * pointer held across the send would name. */ +void FwdPendingCommit(WOLFSSH* ssh, WOLFSSH_FWD_PENDING* pend) +{ + WOLFSSH_FWD_REMOTE* target = NULL; + WOLFSSH_FWD_REMOTE* cur; + void* heap; + + WLOG(WS_LOG_DEBUG, "Entering FwdPendingCommit()"); + + if (ssh == NULL || ssh->ctx == NULL || pend == NULL) + return; + + heap = ssh->ctx->heap; + + if (pend->bindAddr != NULL) + target = FwdRemoteFind(ssh, pend->bindAddr, pend->bindPort); + + if (pend->entry != NULL && target != NULL) { + /* A callback the send ran registered this bind first, so the entry + * built for it is one too many. */ + WFREE(pend->entry->bindAddr, heap, DYNTYPE_STRING); + WFREE(pend->entry, heap, DYNTYPE_FWD); + pend->entry = NULL; + } + + if (pend->entry != NULL) { + for (cur = ssh->fwdRemoteList; cur != NULL && cur->next != NULL; + cur = cur->next) { + /* walk to the tail */ + } + if (cur == NULL) + ssh->fwdRemoteList = pend->entry; + else + cur->next = pend->entry; + + /* From here on, forwarded-tcpip opens are matched against this list. A + * client that never calls wolfSSH_FwdRemoteSetup() never sets this and + * has its opens go unchecked. */ + ssh->fwdRemoteTracked = 1; + target = pend->entry; + } + + if (pend->reply != NULL && pend->reply->answered) { + /* The peer answered mid-send, parking its verdict on the slot. The + * forward it answers for is known now, so settle it. */ + FwdRemoteSettle(ssh, target, pend->reply->isCancel, + pend->reply->success, pend->reply->port); + WFREE(pend->reply, heap, DYNTYPE_FWD); + } + else if (pend->reply != NULL) { + /* The slot is queued already; naming the forward makes it the newest + * request outstanding on it. */ + pend->reply->entry = target; + pend->reply->uncommitted = 0; + } + else if (target != NULL) { + /* No reply was asked for, so this request is the last word on the + * forward. It went out after everything still queued, so those answers + * no longer speak for it. */ + if (pend->isCancel) { + FwdRemoteUnlink(ssh, heap, target); + } + else { + FwdReplyVoid(ssh, target); + target->confirmed = 1; + } + } + + WMEMSET(pend, 0, sizeof(*pend)); + + WLOG(WS_LOG_DEBUG, "Leaving FwdPendingCommit()"); +} + + +/* The request never went out. Give back the memory and the queue slot, leaving + * the session as it was. */ +void FwdPendingDiscard(WOLFSSH* ssh, WOLFSSH_FWD_PENDING* pend) +{ + void* heap; + + WLOG(WS_LOG_DEBUG, "Entering FwdPendingDiscard()"); + + if (ssh == NULL || ssh->ctx == NULL || pend == NULL) + return; + + heap = ssh->ctx->heap; + + if (pend->entry != NULL) { + WFREE(pend->entry->bindAddr, heap, DYNTYPE_STRING); + WFREE(pend->entry, heap, DYNTYPE_FWD); + } + if (pend->reply != NULL) { + /* An answer that arrived mid-send dequeued the slot already, and + * settles nothing now that the request isn't going out. */ + if (pend->reply->answered) + WFREE(pend->reply, heap, DYNTYPE_FWD); + else + FwdReplyUnqueue(ssh, pend->reply); + } + + WMEMSET(pend, 0, sizeof(*pend)); + + WLOG(WS_LOG_DEBUG, "Leaving FwdPendingDiscard()"); +} + + +/* Does an inbound forwarded-tcpip name a forward this client registered? */ +static int FwdRemoteMatch(WOLFSSH* ssh, const char* addr, word32 port) +{ + WOLFSSH_FWD_REMOTE* cur; + + if (ssh == NULL || addr == NULL) + return 0; + + for (cur = ssh->fwdRemoteList; cur != NULL; cur = cur->next) { + WOLFSSH_FWD_REPLY* newest; + + /* No port to match on until the peer's reply names the one it + * bound. */ + if (cur->portPending || cur->bindPort != port) + continue; + + /* The newest request governs: a cancel stops matching as it goes out, + * so revoking never waits on the peer, and the peer refusing it puts + * the forward back. */ + newest = FwdReplyNewest(ssh, cur); + if (newest != NULL && newest->isCancel) + continue; + + /* A forward stands on the peer having bound it, or on a request still + * owed an answer. With neither, nothing speaks for it. */ + if (!cur->confirmed && newest == NULL) + continue; + + if (FwdRemoteAddrIsWild(cur->bindAddr) || + WSTRCMP(cur->bindAddr, addr) == 0) + return 1; + } + + return 0; +} + + +/* Pair a REQUEST_SUCCESS or REQUEST_FAILURE with the request it answers. + * Replies carry no request id, so the queue answers them in send order. */ +static void FwdRemoteReply(WOLFSSH* ssh, int success, const byte* buf, + word32 len) +{ + WOLFSSH_FWD_REPLY* reply; + WOLFSSH_FWD_REMOTE* entry; + word32 port = 0; + byte isCancel; + + if (ssh == NULL || ssh->ctx == NULL) + return; + + reply = ssh->fwdReplyHead; + if (reply == NULL) + return; + + ssh->fwdReplyHead = reply->next; + if (ssh->fwdReplyHead == NULL) + ssh->fwdReplyTail = NULL; + + if (reply->uncommitted) { + /* The request this answers is still being sent -- a callback the send + * ran pumped it in. Which forward it settles isn't known until that + * send commits, so park the verdict for the commit to apply and leave + * the slot to the sender that owns it. */ + reply->answered = 1; + reply->success = (byte)(success != 0); + if (success && buf != NULL && len >= UINT32_SZ) + ato32(buf, &reply->port); + return; + } + + entry = reply->entry; + isCancel = reply->isCancel; + if (success && buf != NULL && len >= UINT32_SZ) + ato32(buf, &port); + WFREE(reply, ssh->ctx->heap, DYNTYPE_FWD); + + FwdRemoteSettle(ssh, entry, isCancel, success, port); +} + + +void FwdRemoteFreeList(WOLFSSH* ssh, void* heap) +{ + WOLFSSH_FWD_REMOTE* cur; + WOLFSSH_FWD_REMOTE* next; + WOLFSSH_FWD_REPLY* reply; + WOLFSSH_FWD_REPLY* replyNext; + + if (ssh == NULL) + return; + + for (cur = ssh->fwdRemoteList; cur != NULL; cur = next) { + next = cur->next; + WFREE(cur->bindAddr, heap, DYNTYPE_STRING); + WFREE(cur, heap, DYNTYPE_FWD); + } + ssh->fwdRemoteList = NULL; + + for (reply = ssh->fwdReplyHead; reply != NULL; reply = replyNext) { + replyNext = reply->next; + WFREE(reply, heap, DYNTYPE_FWD); + } + ssh->fwdReplyHead = NULL; + ssh->fwdReplyTail = NULL; +} #endif /* WOLFSSH_FWD */ @@ -8106,6 +8681,10 @@ static int DoRequestSuccess(WOLFSSH *ssh, byte *buf, word32 len, word32 *idx) WLOG(WS_LOG_DEBUG, "DoRequestSuccess, *idx=%d, len=%d", *idx, len); begin += len; +#ifdef WOLFSSH_FWD + FwdRemoteReply(ssh, 1, &(buf[*idx]), len); +#endif + if (ssh->ctx->reqSuccessCb != NULL) ret = ssh->ctx->reqSuccessCb(ssh, &(buf[*idx]), len, ssh->reqSuccessCtx); @@ -8122,6 +8701,10 @@ static int DoRequestFailure(WOLFSSH *ssh, byte *buf, word32 len, word32 *idx) WLOG(WS_LOG_DEBUG, "DoRequestFailure, *idx=%d, len=%d", *idx, len); begin += len; +#ifdef WOLFSSH_FWD + FwdRemoteReply(ssh, 0, NULL, 0); +#endif + if (ssh->ctx->reqFailureCb != NULL) ret = ssh->ctx->reqFailureCb(ssh, &(buf[*idx]), len, ssh->reqFailureCtx); @@ -11112,6 +11695,20 @@ static int DoChannelOpen(WOLFSSH* ssh, fail_reason = OPEN_ADMINISTRATIVELY_PROHIBITED; ret = WS_ERROR; } + + /* Per RFC 4254 7.2, a forwarded-tcpip open answers a forward the + * client registered with tcpip-forward, so refuse one naming + * anything else before the policy callback sees it. Only a client + * that used wolfSSH_FwdRemoteSetup() has a list to check. */ + if (ret == WS_SUCCESS && typeId == ID_CHANTYPE_TCPIP_FORWARD && + ssh->fwdRemoteTracked && + !FwdRemoteMatch(ssh, host, hostPort)) { + WLOG(WS_LOG_WARN, "Rejecting forwarded-tcpip channel open " + "for the unregistered forward %s:%u", + host != NULL ? host : "", hostPort); + fail_reason = OPEN_ADMINISTRATIVELY_PROHIBITED; + ret = WS_ERROR; + } #endif /* WOLFSSH_FWD */ if (ret == WS_SUCCESS) { if (ssh->ctx->channelOpenCb) { diff --git a/src/ssh.c b/src/ssh.c index 273e68f37..2fc9d8ea0 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1641,6 +1641,11 @@ int wolfSSH_stream_exit(WOLFSSH* ssh, int status) int wolfSSH_global_request(WOLFSSH *ssh, const unsigned char* data, word32 dataSz, int reply) { + int ret; +#ifdef WOLFSSH_FWD + WOLFSSH_FWD_PENDING pend; +#endif + WLOG(WS_LOG_DEBUG, "Entering wolfSSH_global_request"); if (ssh == NULL || data == NULL) return WS_BAD_ARGUMENT; @@ -1648,7 +1653,34 @@ int wolfSSH_global_request(WOLFSSH *ssh, const unsigned char* data, word32 dataS return WS_BAD_ARGUMENT; if (SendAfterDisconnect(ssh)) return WS_FATAL_ERROR; - return SendGlobalRequest(ssh, data, dataSz, reply, NULL); + +#ifdef WOLFSSH_FWD + /* A want-reply request consumes one of the peer's replies, so it takes a + * place in the same queue the forwarding requests use; otherwise its reply + * reads as the answer to an outstanding tcpip-forward. */ + if (reply) { + int sent = 0; + + ret = FwdReplyPrepare(ssh, &pend); + if (ret != WS_SUCCESS) + return ret; + + /* A request the peer received is owed a reply whatever this call + * returns, so the slot goes by what reached the wire, not by the + * error. */ + ret = SendGlobalRequest(ssh, data, dataSz, reply, &sent); + if (sent) + FwdPendingCommit(ssh, &pend); + else + FwdPendingDiscard(ssh, &pend); + + return ret; + } +#endif /* WOLFSSH_FWD */ + + ret = SendGlobalRequest(ssh, data, dataSz, reply, NULL); + + return ret; } @@ -3918,16 +3950,12 @@ WOLFSSH_CHANNEL* wolfSSH_ChannelFwdNew(WOLFSSH* ssh, } -/* Send "tcpip-forward", asking the peer to listen on bindAddr:bindPort. Port 0 - * lets the peer choose; with wantReply it names the port it bound through the - * request-success callback (wolfSSH_SetReqSuccess). Inbound connections arrive - * as "forwarded-tcpip" channels via the forwarding callback. - * - * RFC 4254 7.1 defines tcpip-forward as client-to-server, and a server rejects - * the resulting forwarded-tcpip opens, so this is client-only. */ +/* Send "tcpip-forward" and register the forward. See wolfssh/ssh.h for the + * matching rules and what a port-0 request needs. */ int wolfSSH_FwdRemoteSetup(WOLFSSH* ssh, const char* bindAddr, word32 bindPort, int wantReply) { + WOLFSSH_FWD_PENDING pend; int ret = WS_SUCCESS; WLOG(WS_LOG_DEBUG, "Entering wolfSSH_FwdRemoteSetup()"); @@ -3942,6 +3970,12 @@ int wolfSSH_FwdRemoteSetup(WOLFSSH* ssh, const char* bindAddr, if (ret == WS_SUCCESS && wantReply != 0 && wantReply != 1) ret = WS_BAD_ARGUMENT; + /* The peer's reply is the only place a port-0 request learns the port it + * got, and neither the caller nor the forwarded-tcpip check works without + * one. */ + if (ret == WS_SUCCESS && bindPort == 0 && !wantReply) + ret = WS_BAD_ARGUMENT; + if (ret == WS_SUCCESS && ssh->ctx->side != WOLFSSH_ENDPOINT_CLIENT) ret = WS_BAD_ARGUMENT; @@ -3952,9 +3986,26 @@ int wolfSSH_FwdRemoteSetup(WOLFSSH* ssh, const char* bindAddr, if (ret == WS_SUCCESS && ssh->isKeying) ret = WS_REKEYING; + /* Everything that can fail is allocated before the request goes out, so an + * error from here means the peer heard nothing. */ if (ret == WS_SUCCESS) + ret = FwdRemotePrepare(ssh, bindAddr, bindPort, wantReply, 0, &pend); + + if (ret == WS_SUCCESS) { + int sent = 0; + ret = SendGlobalRequestFwd(ssh, bindAddr, bindPort, 0, wantReply, - NULL); + &sent); + + /* Whether the peer will bind the listener, not whether this call + * succeeded: a request still framed and waiting to flush reaches it, + * and so does one the post-send highwater callback reports an error + * for. Only what never left unwinds. */ + if (sent) + FwdPendingCommit(ssh, &pend); + else + FwdPendingDiscard(ssh, &pend); + } WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_FwdRemoteSetup(), ret = %d", ret); return ret; @@ -3962,11 +4013,11 @@ int wolfSSH_FwdRemoteSetup(WOLFSSH* ssh, const char* bindAddr, /* Send "cancel-tcpip-forward", tearing down a wolfSSH_FwdRemoteSetup() - * listener. bindPort must be the port the peer bound, which after a port-0 - * request is the one it reported, not 0. Client-only, as with the setup. */ + * listener. See wolfssh/ssh.h for when the registration actually drops. */ int wolfSSH_FwdRemoteCancel(WOLFSSH* ssh, const char* bindAddr, word32 bindPort, int wantReply) { + WOLFSSH_FWD_PENDING pend; int ret = WS_SUCCESS; WLOG(WS_LOG_DEBUG, "Entering wolfSSH_FwdRemoteCancel()"); @@ -3992,8 +4043,19 @@ int wolfSSH_FwdRemoteCancel(WOLFSSH* ssh, const char* bindAddr, ret = WS_REKEYING; if (ret == WS_SUCCESS) + ret = FwdRemotePrepare(ssh, bindAddr, bindPort, wantReply, 1, &pend); + + if (ret == WS_SUCCESS) { + int sent = 0; + ret = SendGlobalRequestFwd(ssh, bindAddr, bindPort, 1, wantReply, - NULL); + &sent); + + if (sent) + FwdPendingCommit(ssh, &pend); + else + FwdPendingDiscard(ssh, &pend); + } WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_FwdRemoteCancel(), ret = %d", ret); return ret; diff --git a/tests/api.c b/tests/api.c index 7f9cc47ca..1f4b8fa35 100644 --- a/tests/api.c +++ b/tests/api.c @@ -7445,6 +7445,10 @@ static void test_wolfSSH_FwdRemote_badArgs(void) AssertIntEQ(wolfSSH_FwdRemoteCancel(ssh, "0.0.0.0", 0, 1), WS_BAD_ARGUMENT); + /* The reply is the only place a port-0 request learns its port. */ + AssertIntEQ(wolfSSH_FwdRemoteSetup(ssh, "0.0.0.0", 0, 0), + WS_BAD_ARGUMENT); + /* wantReply is a boolean. */ AssertIntEQ(wolfSSH_FwdRemoteSetup(ssh, "0.0.0.0", 22, 2), WS_BAD_ARGUMENT); diff --git a/tests/regress.c b/tests/regress.c index 333d9c75f..499fcc72a 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -330,6 +330,7 @@ typedef struct { byte* out; /* data written by client */ word32 outSz; word32 outCap; + byte blockNext; /* make the next send report a would-block */ } MemIo; static int MemRecv(WOLFSSH* ssh, void* buf, word32 sz, void* ctx) @@ -350,6 +351,10 @@ static int MemSend(WOLFSSH* ssh, void* buf, word32 sz, void* ctx) { (void)ssh; MemIo* io = (MemIo*)ctx; + if (io->blockNext) { + io->blockNext = 0; + return WS_CBIO_ERR_WANT_WRITE; + } if (io->outSz + sz > io->outCap) { return WS_CBIO_ERR_GENERAL; } @@ -366,13 +371,12 @@ static void MemIoInit(MemIo* io, byte* in, word32 inSz, byte* out, word32 outCap io->out = out; io->outSz = 0; io->outCap = outCap; + io->blockNext = 0; } -/* The harness below and everything built on it drive a server-side session. - * With NO_WOLFSSH_SERVER the message filter has no server branch, so every - * message on such a session is refused and those tests cannot run. */ -#ifndef NO_WOLFSSH_SERVER - +/* The in-memory session harness. The struct and its teardown are shared; the + * client-side setup drives a client session, so the forwarding tests built on + * it run without a server. */ typedef struct { WOLFSSH_CTX* ctx; WOLFSSH* ssh; @@ -380,6 +384,43 @@ typedef struct { byte out[256]; } ChannelOpenHarness; +static WS_MAYBE_UNUSED void FreeChannelOpenHarness(ChannelOpenHarness* harness) +{ + if (harness->ssh != NULL) + wolfSSH_free(harness->ssh); + if (harness->ctx != NULL) + wolfSSH_CTX_free(harness->ctx); +} + +#if defined(WOLFSSH_FWD) && !defined(NO_WOLFSSH_CLIENT) +/* The same harness on the client side, sitting past userauth so an inbound + * channel open is allowed through. */ +static void InitChannelOpenHarnessClient(ChannelOpenHarness* harness, + byte* in, word32 inSz) +{ + WMEMSET(harness, 0, sizeof(*harness)); + + harness->ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL); + AssertNotNull(harness->ctx); + + wolfSSH_SetIORecv(harness->ctx, MemRecv); + wolfSSH_SetIOSend(harness->ctx, MemSend); + + harness->ssh = wolfSSH_new(harness->ctx); + AssertNotNull(harness->ssh); + + MemIoInit(&harness->io, in, inSz, harness->out, sizeof(harness->out)); + wolfSSH_SetIOReadCtx(harness->ssh, &harness->io); + wolfSSH_SetIOWriteCtx(harness->ssh, &harness->io); + harness->ssh->connectState = CONNECT_SERVER_USERAUTH_ACCEPT_DONE; +} +#endif /* WOLFSSH_FWD && !NO_WOLFSSH_CLIENT */ + +/* The tests below drive a server-side session. With NO_WOLFSSH_SERVER the + * message filter has no server branch, so every message on such a session is + * refused and those tests cannot run. */ +#ifndef NO_WOLFSSH_SERVER + static void InitChannelOpenHarness(ChannelOpenHarness* harness, byte* in, word32 inSz) { @@ -400,13 +441,6 @@ static void InitChannelOpenHarness(ChannelOpenHarness* harness, harness->ssh->acceptState = ACCEPT_SERVER_USERAUTH_SENT; } -static void FreeChannelOpenHarness(ChannelOpenHarness* harness) -{ - if (harness->ssh != NULL) - wolfSSH_free(harness->ssh); - if (harness->ctx != NULL) - wolfSSH_CTX_free(harness->ctx); -} #ifdef WOLFSSH_KEYBOARD_INTERACTIVE /* Build a plaintext SSH_MSG_USERAUTH_INFO_RESPONSE. The wire response count is @@ -1734,20 +1768,9 @@ static void TestKexDhGexGroupBadGeneratorSendsDisconnect(void) #endif /* KEXDH_REPLY_REGRESS_KEX_ALGO */ -#ifndef NO_WOLFSSH_SERVER - -static word32 ParseChannelOpenFailRecipient(const byte* pkt, word32 sz) -{ - word32 chan; - /* SSH binary-packet layout: 4 (len) + 1 (pad_len) + 1 (msg_id) = 6; - * + 4 for the recipient_channel field itself gives the 10-byte minimum. */ - AssertTrue(sz >= 10); - AssertIntEQ(pkt[5], MSGID_CHANNEL_OPEN_FAIL); - WMEMCPY(&chan, pkt + 6, sizeof(chan)); - return ntohl(chan); -} - -static word32 ParseChannelOpenFailReason(const byte* pkt, word32 sz) +/* Shared with the client-side forwarding tests below. */ +static WS_MAYBE_UNUSED word32 ParseChannelOpenFailReason(const byte* pkt, + word32 sz) { word32 reason; /* SSH binary-packet layout: 4 (len) + 1 (pad_len) + 1 (msg_id) + 4 (chan) = 10; @@ -1758,8 +1781,8 @@ static word32 ParseChannelOpenFailReason(const byte* pkt, word32 sz) return ntohl(reason); } -static void AssertChannelOpenFailResponse(const ChannelOpenHarness* harness, - int ret) +static WS_MAYBE_UNUSED void AssertChannelOpenFailResponse( + const ChannelOpenHarness* harness, int ret) { byte msgId; @@ -1775,6 +1798,35 @@ static void AssertChannelOpenFailResponse(const ChannelOpenHarness* harness, AssertTrue(harness->ssh->channelList == NULL); } +#ifdef WOLFSSH_FWD +/* The port a peer picks for a port-0 forward in these tests. */ +#define REGRESS_FWD_ALLOC_PORT 49152 + +static WS_MAYBE_UNUSED int AcceptFwdCb(WS_FwdCbAction action, void* ctx, + const char* host, word32 port) +{ + (void)action; + (void)ctx; + (void)host; + (void)port; + + return WS_SUCCESS; +} +#endif /* WOLFSSH_FWD */ + +#ifndef NO_WOLFSSH_SERVER + +static word32 ParseChannelOpenFailRecipient(const byte* pkt, word32 sz) +{ + word32 chan; + /* SSH binary-packet layout: 4 (len) + 1 (pad_len) + 1 (msg_id) = 6; + * + 4 for the recipient_channel field itself gives the 10-byte minimum. */ + AssertTrue(sz >= 10); + AssertIntEQ(pkt[5], MSGID_CHANNEL_OPEN_FAIL); + WMEMCPY(&chan, pkt + 6, sizeof(chan)); + return ntohl(chan); +} + #ifdef WOLFSSH_FWD static const byte* ParseGlobalRequestName(const byte* packet, word32 packetSz, word32* nameSz) @@ -1887,17 +1939,6 @@ static int RejectDirectTcpipSetup(WS_FwdCbAction action, void* ctx, return WS_SUCCESS; } -static int AcceptFwdCb(WS_FwdCbAction action, void* ctx, - const char* host, word32 port) -{ - (void)action; - (void)ctx; - (void)host; - (void)port; - - return WS_SUCCESS; -} - /* Counts every action the library asks for. File scope rather than reached * through the callback ctx, so a zero reading means the hook did not run and * cannot instead mean the ctx stopped being delivered. */ @@ -1933,8 +1974,6 @@ static int CountingRejectChannelIdFwdCb(WS_FwdCbAction action, void* ctx, return WS_SUCCESS; } -#define REGRESS_FWD_ALLOC_PORT 49152 - static int AllocatePortFwdCb(WS_FwdCbAction action, void* ctx, const char* host, word32 port) { @@ -3481,6 +3520,1372 @@ static void TestAgentEd25519UserAuthRejectsOversizeSignature(void) +/* The client-side forwarding tests. They drive a client session, so they + * run whether or not this build has a server. */ +#if defined(WOLFSSH_FWD) && !defined(NO_WOLFSSH_CLIENT) + +static word32 BuildRequestSuccessPortPacket(word32 port, + byte* out, word32 outSz) +{ + byte payload[UINT32_SZ]; + word32 idx = 0; + + idx = AppendUint32(payload, sizeof(payload), idx, port); + + return WrapPacket(MSGID_REQUEST_SUCCESS, payload, idx, out, outSz); +} + +/* Swap in one packet and run a receive pass, dropping whatever the client + * wrote earlier so the response starts at offset 0. */ +static int FeedOnePacket(ChannelOpenHarness* harness, byte* pkt, word32 pktSz) +{ + harness->io.in = pkt; + harness->io.inSz = pktSz; + harness->io.inOff = 0; + harness->io.outSz = 0; + + return DoReceive(harness->ssh); +} + +static void FeedRequestSuccess(ChannelOpenHarness* harness) +{ + byte reply[64]; + word32 replySz; + + replySz = WrapPacket(MSGID_REQUEST_SUCCESS, NULL, 0, reply, sizeof(reply)); + AssertIntEQ(FeedOnePacket(harness, reply, replySz), WS_SUCCESS); +} + +static void FeedRequestFailure(ChannelOpenHarness* harness) +{ + byte reply[64]; + word32 replySz; + + replySz = WrapPacket(MSGID_REQUEST_FAILURE, NULL, 0, reply, sizeof(reply)); + AssertIntEQ(FeedOnePacket(harness, reply, replySz), WS_SUCCESS); +} + +static word32 BuildForwardedTcpipOpen(const char* openAddr, word32 openPort, + byte* out, word32 outSz) +{ + byte extra[128]; + word32 extraSz; + + extraSz = BuildDirectTcpipExtra(openAddr, openPort, "10.0.0.5", 4321, + extra, sizeof(extra)); + + return BuildChannelOpenPacket("forwarded-tcpip", 9, 0x4000, 0x8000, + extra, extraSz, out, outSz); +} + +/* A refused open asserts the channel list is empty, so within one harness + * every refusal has to be checked before the first accepted open. */ +static void AssertForwardedOpenRefused(ChannelOpenHarness* harness, + const char* openAddr, word32 openPort) +{ + byte in[192]; + word32 inSz; + int ret; + + inSz = BuildForwardedTcpipOpen(openAddr, openPort, in, sizeof(in)); + + ret = FeedOnePacket(harness, in, inSz); + AssertChannelOpenFailResponse(harness, ret); + AssertIntEQ(ParseChannelOpenFailReason(harness->io.out, harness->io.outSz), + OPEN_ADMINISTRATIVELY_PROHIBITED); +} + +static void AssertForwardedOpenAccepted(ChannelOpenHarness* harness, + const char* openAddr, word32 openPort, word32 expectChannels) +{ + byte in[192]; + word32 inSz; + int ret; + + inSz = BuildForwardedTcpipOpen(openAddr, openPort, in, sizeof(in)); + + ret = FeedOnePacket(harness, in, inSz); + AssertIntEQ(ret, WS_SUCCESS); + AssertTrue(harness->io.outSz > 0); + AssertIntEQ(ParseMsgId(harness->io.out, harness->io.outSz), + MSGID_CHANNEL_OPEN_CONF); + AssertIntEQ(harness->ssh->channelListSz, expectChannels); +} + +/* A client past userauth with the accepting fwdCb and no pending input. */ +static void InitFwdRemoteHarness(ChannelOpenHarness* harness) +{ + InitChannelOpenHarnessClient(harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetFwdCb(harness->ctx, AcceptFwdCb, NULL), + WS_SUCCESS); +} + +/* Set up a client that asked for one remote forward, then hand it a + * forwarded-tcpip open naming openAddr:openPort. The request the setup sends + * is dropped from the output so the open's response starts at offset 0. */ +static void RunForwardedTcpipMatchTest(const char* bindAddr, word32 bindPort, + const char* openAddr, word32 openPort, int expectAccept) +{ + ChannelOpenHarness harness; + byte extra[128]; + byte in[192]; + word32 extraSz; + word32 inSz; + int ret; + + extraSz = BuildDirectTcpipExtra(openAddr, openPort, "10.0.0.5", 4321, + extra, sizeof(extra)); + inSz = BuildChannelOpenPacket("forwarded-tcpip", 9, 0x4000, 0x8000, + extra, extraSz, in, sizeof(in)); + + InitChannelOpenHarnessClient(&harness, in, inSz); + AssertIntEQ(wolfSSH_CTX_SetFwdCb(harness.ctx, AcceptFwdCb, NULL), + WS_SUCCESS); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, bindAddr, bindPort, 1), + WS_SUCCESS); + harness.io.outSz = 0; + + ret = DoReceive(harness.ssh); + + if (expectAccept) { + AssertIntEQ(ret, WS_SUCCESS); + AssertTrue(harness.io.outSz > 0); + AssertIntEQ(ParseMsgId(harness.io.out, harness.io.outSz), + MSGID_CHANNEL_OPEN_CONF); + AssertIntEQ(harness.ssh->channelListSz, 1); + } + else { + AssertChannelOpenFailResponse(&harness, ret); + AssertIntEQ(ParseChannelOpenFailReason(harness.io.out, + harness.io.outSz), OPEN_ADMINISTRATIVELY_PROHIBITED); + } + + FreeChannelOpenHarness(&harness); +} + +static void TestForwardedTcpipRegisteredIsAccepted(void) +{ + /* The open names the forward the client registered, so it goes through. */ + RunForwardedTcpipMatchTest("127.0.0.1", 8080, "127.0.0.1", 8080, 1); +} + +static void TestForwardedTcpipUnregisteredSendsOpenFail(void) +{ + /* RFC 4254 7.2: a forwarded-tcpip answers a forward the client asked for. + * Neither of these was requested, so the peer invented them. */ + RunForwardedTcpipMatchTest("127.0.0.1", 8080, "10.0.0.1", 9999, 0); + RunForwardedTcpipMatchTest("127.0.0.1", 8080, "127.0.0.1", 9999, 0); + RunForwardedTcpipMatchTest("127.0.0.1", 8080, "10.0.0.1", 8080, 0); +} + +static void TestForwardedTcpipWildcardBindMatchesAnyAddr(void) +{ + /* A wildcard bind doesn't pin down what the peer will echo back, so the + * port alone decides the match. RFC 4254 7.1 names the empty string and + * "::"; "*" and "0.0.0.0" are the common spellings. */ + RunForwardedTcpipMatchTest("0.0.0.0", 8080, "192.168.1.7", 8080, 1); + RunForwardedTcpipMatchTest("0.0.0.0", 8080, "192.168.1.7", 9999, 0); + + RunForwardedTcpipMatchTest("", 8080, "192.168.1.7", 8080, 1); + RunForwardedTcpipMatchTest("", 8080, "192.168.1.7", 9999, 0); + + RunForwardedTcpipMatchTest("*", 8080, "192.168.1.7", 8080, 1); + RunForwardedTcpipMatchTest("*", 8080, "192.168.1.7", 9999, 0); + + RunForwardedTcpipMatchTest("::", 8080, "fe80::1", 8080, 1); + RunForwardedTcpipMatchTest("::", 8080, "fe80::1", 9999, 0); + + /* The other spellings of the IPv6 any-address mean the same thing. */ + RunForwardedTcpipMatchTest("::0", 8080, "fe80::1", 8080, 1); + RunForwardedTcpipMatchTest("0:0:0:0:0:0:0:0", 8080, "fe80::1", 8080, 1); + RunForwardedTcpipMatchTest("::ffff:0.0.0.0", 8080, "192.168.1.7", 8080, 1); +} + +static void TestForwardedTcpipCancelledSendsOpenFail(void) +{ + ChannelOpenHarness harness; + byte extra[128]; + byte in[192]; + word32 extraSz; + word32 inSz; + int ret; + + extraSz = BuildDirectTcpipExtra("127.0.0.1", 8080, "10.0.0.5", 4321, + extra, sizeof(extra)); + inSz = BuildChannelOpenPacket("forwarded-tcpip", 9, 0x4000, 0x8000, + extra, extraSz, in, sizeof(in)); + + InitChannelOpenHarnessClient(&harness, in, inSz); + AssertIntEQ(wolfSSH_CTX_SetFwdCb(harness.ctx, AcceptFwdCb, NULL), + WS_SUCCESS); + + /* Without want-reply there is nothing to wait for, so the cancel drops + * the registration as it goes out. */ + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 0), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 0), + WS_SUCCESS); + harness.io.outSz = 0; + + ret = DoReceive(harness.ssh); + AssertChannelOpenFailResponse(&harness, ret); + + FreeChannelOpenHarness(&harness); +} + +static void TestForwardedTcpipPortZeroMatchesBoundPort(void) +{ + ChannelOpenHarness harness; + byte extra[128]; + byte reply[64]; + byte in[192]; + word32 extraSz; + word32 replySz; + word32 inSz; + int ret; + + replySz = BuildRequestSuccessPortPacket(REGRESS_FWD_ALLOC_PORT, + reply, sizeof(reply)); + + InitChannelOpenHarnessClient(&harness, reply, replySz); + AssertIntEQ(wolfSSH_CTX_SetFwdCb(harness.ctx, AcceptFwdCb, NULL), + WS_SUCCESS); + + /* Port 0 asks the peer to allocate. Until its reply names the port, the + * registration has nothing to match on. */ + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 0, 1), + WS_SUCCESS); + harness.io.outSz = 0; + + ret = DoReceive(harness.ssh); + AssertIntEQ(ret, WS_SUCCESS); + + /* An open on the port the peer reported now matches. */ + extraSz = BuildDirectTcpipExtra("127.0.0.1", REGRESS_FWD_ALLOC_PORT, + "10.0.0.5", 4321, extra, sizeof(extra)); + inSz = BuildChannelOpenPacket("forwarded-tcpip", 9, 0x4000, 0x8000, + extra, extraSz, in, sizeof(in)); + + harness.io.in = in; + harness.io.inSz = inSz; + harness.io.inOff = 0; + harness.io.outSz = 0; + + ret = DoReceive(harness.ssh); + AssertIntEQ(ret, WS_SUCCESS); + AssertTrue(harness.io.outSz > 0); + AssertIntEQ(ParseMsgId(harness.io.out, harness.io.outSz), + MSGID_CHANNEL_OPEN_CONF); + AssertIntEQ(harness.ssh->channelListSz, 1); + + FreeChannelOpenHarness(&harness); +} + +static void TestForwardedTcpipRefusedForwardSendsOpenFail(void) +{ + ChannelOpenHarness harness; + byte extra[128]; + byte reply[64]; + byte in[192]; + word32 extraSz; + word32 replySz; + word32 inSz; + int ret; + + replySz = WrapPacket(MSGID_REQUEST_FAILURE, NULL, 0, reply, + sizeof(reply)); + + InitChannelOpenHarnessClient(&harness, reply, replySz); + AssertIntEQ(wolfSSH_CTX_SetFwdCb(harness.ctx, AcceptFwdCb, NULL), + WS_SUCCESS); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + harness.io.outSz = 0; + + /* The peer refused the forward, so it bound no listener and nothing may + * arrive for it. */ + ret = DoReceive(harness.ssh); + AssertIntEQ(ret, WS_SUCCESS); + + extraSz = BuildDirectTcpipExtra("127.0.0.1", 8080, "10.0.0.5", 4321, + extra, sizeof(extra)); + inSz = BuildChannelOpenPacket("forwarded-tcpip", 9, 0x4000, 0x8000, + extra, extraSz, in, sizeof(in)); + + harness.io.in = in; + harness.io.inSz = inSz; + harness.io.inOff = 0; + harness.io.outSz = 0; + + ret = DoReceive(harness.ssh); + AssertChannelOpenFailResponse(&harness, ret); + + FreeChannelOpenHarness(&harness); +} + +static void TestForwardedTcpipUntrackedClientUnchanged(void) +{ + ChannelOpenHarness harness; + byte extra[128]; + byte in[192]; + word32 extraSz; + word32 inSz; + int ret; + + extraSz = BuildDirectTcpipExtra("10.0.0.1", 9999, "10.0.0.5", 4321, + extra, sizeof(extra)); + inSz = BuildChannelOpenPacket("forwarded-tcpip", 9, 0x4000, 0x8000, + extra, extraSz, in, sizeof(in)); + + InitChannelOpenHarnessClient(&harness, in, inSz); + AssertIntEQ(wolfSSH_CTX_SetFwdCb(harness.ctx, AcceptFwdCb, NULL), + WS_SUCCESS); + + /* A client that frames tcpip-forward itself registers nothing, so there + * is no list to match against and its fwdCb stays the only gate. */ + ret = DoReceive(harness.ssh); + AssertIntEQ(ret, WS_SUCCESS); + AssertTrue(harness.io.outSz > 0); + AssertIntEQ(ParseMsgId(harness.io.out, harness.io.outSz), + MSGID_CHANNEL_OPEN_CONF); + AssertIntEQ(harness.ssh->channelListSz, 1); + + FreeChannelOpenHarness(&harness); +} + +static void TestForwardedTcpipCancelConfirmedSendsOpenFail(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 0), + WS_SUCCESS); + /* With want-reply the registration stays until the peer confirms, since + * until then its listener may still be feeding channels. */ + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + FeedRequestSuccess(&harness); + + /* Confirmed, so the listener is down and nothing may arrive for it. */ + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + +/* Revoking is not the peer's decision to delay: the forward stops matching as + * the cancel goes out, so a peer that never answers cannot hold a cancelled + * forward open. */ +static void TestForwardedTcpipCancelPendingStopsMatching(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + FeedRequestSuccess(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + /* No reply yet, and none needed. */ + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + AssertNotNull(harness.ssh->fwdRemoteList); + + FreeChannelOpenHarness(&harness); +} + +/* The registration is held while the cancel is unanswered, so a refusal can + * put the forward back. */ +static void TestForwardedTcpipCancelRefusedRestoresMatching(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + FeedRequestSuccess(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + /* The peer kept its listener, so what it opens is asked for again. */ + FeedRequestFailure(&harness); + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + +static void TestForwardedTcpipCancelRefusedKeepsForward(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 0), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + FeedRequestFailure(&harness); + + /* The peer refused the cancel, so its listener is still up and the + * channels it opens are still ones the client asked for. */ + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + +static void TestForwardedTcpipUnmatchedCancelKeepsForward(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 0), + WS_SUCCESS); + + /* Neither names the registered forward, so both drop nothing. */ + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "10.0.0.1", 9999, 0), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 9999, 0), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "10.0.0.1", 8080, 0), + WS_SUCCESS); + + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + +/* A cancel sent while the setup's reply is still outstanding leaves two + * replies owed on one forward. They have to be answered in send order, or the + * setup's reply gets read as the cancel's. */ +static void TestForwardedTcpipCancelBeforeSetupReply(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + /* The first reply answers the setup: the peer refused it and bound + * nothing, so an open naming it is the peer's invention. */ + FeedRequestFailure(&harness); + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + /* The second answers the cancel. The forward it named is already gone, + * so a confirmed cancel changes nothing. */ + FeedRequestSuccess(&harness); + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + +/* The same overlap, but with a second forward outstanding behind it. The + * cancelled forward's reply must not be spent on the one still waiting. */ +static void TestForwardedTcpipCancelBeforeSetupReplyKeepsOther(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 9090, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + /* Answers the first setup, which was refused. */ + FeedRequestFailure(&harness); + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + /* Answers the second setup, which the peer bound. */ + FeedRequestSuccess(&harness); + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 9090, 1); + + FreeChannelOpenHarness(&harness); +} + +/* Asking for the same bind twice is one listener on the peer, so it is one + * registration here and one cancel undoes it. */ +static void TestForwardedTcpipDuplicateSetupIsOneForward(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 0), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 0), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 0), + WS_SUCCESS); + + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + +/* A peer that already has the listener refuses the repeat bind. That refusal + * answers only the second request, so the forward the first one established + * has to survive it. */ +static void TestForwardedTcpipDuplicateSetupRefusalKeepsForward(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + /* The peer bound the listener, then refused the duplicate. */ + FeedRequestSuccess(&harness); + FeedRequestFailure(&harness); + + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + +/* The same pair answered the other way round: the first request is refused + * while a second is still outstanding, so the forward waits on that one + * instead of going on the first refusal. */ +static void TestForwardedTcpipDuplicateSetupLaterSuccessBinds(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + FeedRequestFailure(&harness); + FeedRequestSuccess(&harness); + + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + +/* Both requests refused leaves nothing bound, so the registration goes. */ +static void TestForwardedTcpipDuplicateSetupBothRefusedDrops(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + FeedRequestFailure(&harness); + FeedRequestFailure(&harness); + + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + +/* Asking for the bind again while its cancel is unanswered overrides that + * cancel: however the peer answers it, only the new request's own reply says + * whether the listener is up. */ +static void TestForwardedTcpipSetupAfterPendingCancelKeepsForward(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + FeedRequestSuccess(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + /* Answers the overridden cancel, which now decides nothing. */ + FeedRequestSuccess(&harness); + /* Answers the second setup, which is what binds the listener. */ + FeedRequestSuccess(&harness); + + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + +/* The same overlap where the peer refuses the re-setup. The earlier + * confirmation cannot stand in for it, so nothing is left registered. */ +static void TestForwardedTcpipSetupAfterPendingCancelRefusedDrops(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + FeedRequestSuccess(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + FeedRequestSuccess(&harness); + FeedRequestFailure(&harness); + + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + +/* A refused cancel leaves its listener up, and a setup sent behind it names + * that same listener rather than a second entry, so one later cancel revokes + * the bind. */ +static void TestForwardedTcpipRefusedCancelThenCancelDrops(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + FeedRequestSuccess(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + /* The refusal answers a cancel the later setup already overrode, and the + * success answers that setup. */ + FeedRequestFailure(&harness); + FeedRequestSuccess(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + FeedRequestSuccess(&harness); + + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + +/* Two cancels in flight on one bind. The first one's answer must not settle + * the second's: a refusal there cannot bring the forward back while a cancel + * the peer went on to confirm is still unanswered. */ +static void TestForwardedTcpipOverlappingCancelsLastOneSettles(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + FeedRequestSuccess(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + /* The peer refused the first cancel, bound the setup, then honoured the + * second cancel. Every answer is one a conforming peer can give. */ + FeedRequestFailure(&harness); + FeedRequestSuccess(&harness); + FeedRequestSuccess(&harness); + + /* The peer has no listener there, so nothing may arrive for it. */ + AssertTrue(harness.ssh->fwdRemoteList == NULL); + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + +/* A cancel without want-reply is unconditional even with an earlier cancel + * still unanswered: the registration goes as the request leaves. */ +static void TestForwardedTcpipNoReplyCancelOverridesPending(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + FeedRequestSuccess(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 0), + WS_SUCCESS); + + AssertTrue(harness.ssh->fwdRemoteList == NULL); + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + /* The first cancel is still owed an answer, and it now names nothing. */ + FeedRequestFailure(&harness); + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + +static int MemoryErrorHighwaterCb(byte side, void* ctx) +{ + WOLFSSH_UNUSED(side); + WOLFSSH_UNUSED(ctx); + + /* What a rekey that cannot allocate its handshake state returns. */ + return WS_MEMORY_E; +} + +static int SocketErrorHighwaterCb(byte side, void* ctx) +{ + WOLFSSH_UNUSED(side); + WOLFSSH_UNUSED(ctx); + + /* What a rekey whose own KEXINIT send fails returns, which is also what a + * send that lost the packet returns. */ + return WS_SOCKET_ERROR_E; +} + +static void RunForwardedTcpipPostSendErrorTest(WS_CallbackHighwater cb, + int expectRet) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + wolfSSH_SetHighwaterCb(harness.ctx, 1, cb); + /* Cross the mark on the request's own send. */ + harness.ssh->highwaterMark = 1; + harness.ssh->txCount = 1; + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 0), + expectRet); + AssertNotNull(harness.ssh->fwdRemoteList); + AssertIntEQ(harness.ssh->fwdRemoteTracked, 1); + + harness.io.outSz = 0; + + /* Matching has to be live, or an accepted open would only mean the check + * never ran. Refusals first: an open failure asserts an empty list. */ + AssertForwardedOpenRefused(&harness, "10.0.0.1", 9999); + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + +/* The highwater callback runs after the last byte of the request is on the + * wire, so an error it returns arrives with the peer already holding the + * request. The return code cannot tell that from a send that lost the packet, + * so what reached the wire has to. */ +static void TestForwardedTcpipPostSendErrorStillRegisters(void) +{ + RunForwardedTcpipPostSendErrorTest(MemoryErrorHighwaterCb, WS_MEMORY_E); + RunForwardedTcpipPostSendErrorTest(SocketErrorHighwaterCb, + WS_SOCKET_ERROR_E); +} + +/* A send that never reached the peer leaves the session exactly as it was: + * nothing registered, nothing owed, and matching still off. */ +static void TestForwardedTcpipFailedSendRegistersNothing(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + /* No room left in the transport, so MemSend reports a general error. */ + harness.io.outSz = harness.io.outCap; + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SOCKET_ERROR_E); + + AssertTrue(harness.ssh->fwdRemoteList == NULL); + AssertTrue(harness.ssh->fwdReplyHead == NULL); + AssertTrue(harness.ssh->fwdReplyTail == NULL); + AssertIntEQ(harness.ssh->fwdRemoteTracked, 0); + + FreeChannelOpenHarness(&harness); +} + +static word32 FwdRemoteCount(WOLFSSH* ssh) +{ + WOLFSSH_FWD_REMOTE* cur; + word32 count = 0; + + for (cur = ssh->fwdRemoteList; cur != NULL; cur = cur->next) + count++; + + return count; +} + +static int SetupDuringSendHighwaterCb(byte side, void* ctx) +{ + WOLFSSH* ssh = (WOLFSSH*)ctx; + + WOLFSSH_UNUSED(side); + + /* Reentering the library from here is the established pattern: the default + * callback starts a rekey. */ + if (ssh != NULL) + wolfSSH_FwdRemoteSetup(ssh, "127.0.0.1", 8080, 0); + + return WS_SUCCESS; +} + +/* A callback the send runs can register the very bind the request in flight is + * registering. One listener on the peer is one registration here, whichever + * call links it first. */ +static void TestForwardedTcpipReentrantSetupDuringSend(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + wolfSSH_SetHighwaterCb(harness.ctx, 1, SetupDuringSendHighwaterCb); + wolfSSH_SetHighwaterCtx(harness.ssh, harness.ssh); + /* Cross the mark on the request's own send. */ + harness.ssh->highwaterMark = 1; + harness.ssh->txCount = 1; + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + + harness.io.outSz = 0; + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + +static int GlobalRequestDuringSendHighwaterCb(byte side, void* ctx) +{ + WOLFSSH* ssh = (WOLFSSH*)ctx; + const byte req[] = "keepalive@openssh.com"; + + WOLFSSH_UNUSED(side); + + /* This goes out behind the request being sent, so it has to be answered + * behind it too. */ + if (ssh != NULL) + wolfSSH_global_request(ssh, req, (word32)sizeof(req) - 1, 1); + + return WS_SUCCESS; +} + +/* A want-reply request a callback sends from inside another request's send + * leaves the wire in one order and the queue in another, unless the place in + * the queue is claimed before the send rather than after it. */ +static void TestForwardedTcpipReentrantRequestKeepsSendOrder(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + wolfSSH_SetHighwaterCb(harness.ctx, 1, + GlobalRequestDuringSendHighwaterCb); + wolfSSH_SetHighwaterCtx(harness.ssh, harness.ssh); + /* Cross the mark on the forward request's own send. */ + harness.ssh->highwaterMark = 1; + harness.ssh->txCount = 1; + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + harness.io.outSz = 0; + + /* The forward went out first, so the first reply is its own: refused. */ + FeedRequestFailure(&harness); + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + /* The second answers the callback's request and touches no forward. */ + FeedRequestSuccess(&harness); + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + +/* What the callback feeds back during the send, and whether it then sends a + * request of its own. */ +static byte replyDuringSendMsgId = MSGID_REQUEST_SUCCESS; +static const byte* replyDuringSendData; +static word32 replyDuringSendDataSz; +static int replyDuringSendRequests; + +static int ReplyDuringSendHighwaterCb(byte side, void* ctx) +{ + ChannelOpenHarness* harness = (ChannelOpenHarness*)ctx; + const byte req[] = "keepalive@openssh.com"; + byte reply[64]; + word32 replySz; + + WOLFSSH_UNUSED(side); + + if (harness != NULL) { + /* The request is on the wire before this runs, so a peer answering it + * at once answers the slot the send is still holding. */ + replySz = WrapPacket(replyDuringSendMsgId, replyDuringSendData, + replyDuringSendDataSz, reply, sizeof(reply)); + FeedOnePacket(harness, reply, replySz); + + /* A request sent after that answer takes a slot of its own, which the + * allocator is free to place where the answered one was. */ + if (replyDuringSendRequests) { + wolfSSH_global_request(harness->ssh, req, + (word32)sizeof(req) - 1, 1); + } + } + + return WS_SUCCESS; +} + +static void InitReplyDuringSendHarness(ChannelOpenHarness* harness, byte msgId, + const byte* data, word32 dataSz, int alsoRequests) +{ + replyDuringSendMsgId = msgId; + replyDuringSendData = data; + replyDuringSendDataSz = dataSz; + replyDuringSendRequests = alsoRequests; + + InitFwdRemoteHarness(harness); + + wolfSSH_SetHighwaterCb(harness->ctx, 1, ReplyDuringSendHighwaterCb); + wolfSSH_SetHighwaterCtx(harness->ssh, harness); + /* Cross the mark on the request's own send. */ + harness->ssh->highwaterMark = 1; + harness->ssh->txCount = 1; +} + +/* A refusal in that window says the peer bound nothing. The slot it answers + * does not name the forward yet, so the verdict waits for the commit rather + * than being dropped, which would leave a refused forward matching. */ +static void TestForwardedTcpipRefusalDuringSendDropsForward(void) +{ + ChannelOpenHarness harness; + + InitReplyDuringSendHarness(&harness, MSGID_REQUEST_FAILURE, NULL, 0, 0); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + AssertIntEQ(FwdRemoteCount(harness.ssh), 0); + AssertTrue(harness.ssh->fwdReplyHead == NULL); + AssertTrue(harness.ssh->fwdReplyTail == NULL); + + harness.io.outSz = 0; + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + +/* A port-0 request answered in that window: the port the answer named has to + * reach the registration, or it stays unmatchable for the life of the session + * with nothing left to resolve it. */ +static void TestForwardedTcpipPortZeroReplyDuringSendBinds(void) +{ + ChannelOpenHarness harness; + byte port[UINT32_SZ]; + word32 portSz; + + portSz = AppendUint32(port, sizeof(port), 0, REGRESS_FWD_ALLOC_PORT); + InitReplyDuringSendHarness(&harness, MSGID_REQUEST_SUCCESS, port, portSz, + 0); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 0, 1), + WS_SUCCESS); + + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + AssertIntEQ(harness.ssh->fwdRemoteList->portPending, 0); + AssertIntEQ(harness.ssh->fwdRemoteList->bindPort, REGRESS_FWD_ALLOC_PORT); + + harness.io.outSz = 0; + AssertForwardedOpenAccepted(&harness, "127.0.0.1", REGRESS_FWD_ALLOC_PORT, + 1); + + FreeChannelOpenHarness(&harness); +} + +/* The answer frees a slot, and the request the callback sends next may be + * handed the same address. A send that identified its own slot by address + * alone would apply that request's answer to this forward. */ +static void TestForwardedTcpipRequestAfterReplyDuringSend(void) +{ + ChannelOpenHarness harness; + + InitReplyDuringSendHarness(&harness, MSGID_REQUEST_SUCCESS, NULL, 0, 1); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + /* The peer bound the forward, and the keepalive is the one thing still + * owed an answer. */ + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + AssertNotNull(harness.ssh->fwdReplyHead); + AssertTrue(harness.ssh->fwdReplyHead->entry == NULL); + AssertTrue(harness.ssh->fwdReplyHead->next == NULL); + + /* So the keepalive being refused says nothing about the forward. */ + harness.io.outSz = 0; + FeedRequestFailure(&harness); + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + + harness.io.outSz = 0; + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + +/* The slot takes its place in the queue before the send, so a reply the send + * takes in can pop and free it before the request commits. Committing has to + * find the slot gone rather than write through it. */ +static void TestForwardedTcpipReplyDuringSendTakesSlot(void) +{ + ChannelOpenHarness harness; + + InitReplyDuringSendHarness(&harness, MSGID_REQUEST_SUCCESS, NULL, 0, 0); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + /* The answer was parked on the slot and applied once the forward it names + * was known, so the queue owes nothing further. */ + AssertTrue(harness.ssh->fwdReplyHead == NULL); + AssertTrue(harness.ssh->fwdReplyTail == NULL); + + /* The peer bound the listener, so the forward stands confirmed. */ + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + AssertIntEQ(harness.ssh->fwdRemoteList->confirmed, 1); + + harness.io.outSz = 0; + AssertForwardedOpenRefused(&harness, "10.0.0.1", 9999); + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + +static int CancelDuringSendHighwaterCb(byte side, void* ctx) +{ + WOLFSSH* ssh = (WOLFSSH*)ctx; + + WOLFSSH_UNUSED(side); + + if (ssh != NULL) + wolfSSH_FwdRemoteCancel(ssh, "127.0.0.1", 8080, 0); + + return WS_SUCCESS; +} + +/* The same window, with the callback taking the forward instead of adding it. + * Committing against a registration resolved before the send would write to + * freed memory and leave the queued slot naming it. */ +static void TestForwardedTcpipReentrantCancelDuringSend(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 0), + WS_SUCCESS); + + wolfSSH_SetHighwaterCb(harness.ctx, 1, CancelDuringSendHighwaterCb); + wolfSSH_SetHighwaterCtx(harness.ssh, harness.ssh); + harness.ssh->highwaterMark = 1; + harness.ssh->txCount = 1; + + /* This one names the registration the callback cancels. */ + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + /* The cancel had no want-reply, so it took the registration as it went + * out, and this request's own reply answers for nothing. */ + AssertIntEQ(FwdRemoteCount(harness.ssh), 0); + + harness.io.outSz = 0; + FeedRequestFailure(&harness); + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + +/* A want-reply global request the application framed itself answers nothing + * of ours, so its reply must not be spent on a forward. */ +static void TestForwardedTcpipAppRequestKeepsItsOwnReply(void) +{ + ChannelOpenHarness harness; + const byte req[] = "keepalive@openssh.com"; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_global_request(harness.ssh, req, + (word32)sizeof(req) - 1, 1), WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + /* This answers the application's request, not the forward. */ + FeedRequestFailure(&harness); + + /* So the forward is still waiting, and its own reply binds it. */ + FeedRequestSuccess(&harness); + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + +/* A would-block send still leaves the request framed and waiting to flush, so + * the peer binds the listener and the forward has to be registered. */ +static void TestForwardedTcpipWantWriteStillRegisters(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + harness.io.blockNext = 1; + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 0), + WS_WANT_WRITE); + + /* The request is still framed and waiting; let it out, then drop it so + * the open's response starts at offset 0. */ + AssertIntEQ(wolfSSH_SendPacket(harness.ssh), WS_SUCCESS); + AssertTrue(harness.io.outSz > 0); + harness.io.outSz = 0; + + /* Matching has to be live, or "registered" would just mean the check + * never ran: a bind nobody asked for is still refused. */ + AssertForwardedOpenRefused(&harness, "10.0.0.1", 9999); + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + +/* A request the application sends without want-reply is answered by nothing, + * so it must not take a place in the reply queue and eat the answer owed to an + * outstanding tcpip-forward. */ +static void TestGlobalRequestNoReplyQueuesNothing(void) +{ + ChannelOpenHarness harness; + const byte req[] = "keepalive@openssh.com"; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_global_request(harness.ssh, req, + (word32)sizeof(req) - 1, 0), WS_SUCCESS); + AssertTrue(harness.ssh->fwdReplyHead == NULL); + AssertTrue(harness.ssh->fwdReplyTail == NULL); + + /* So the forward sent behind it still gets its own answer. */ + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + FeedRequestFailure(&harness); + + AssertTrue(harness.ssh->fwdRemoteList == NULL); + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + +/* A confirmed cancel tears down the listener a setup sent behind it will + * rebind. What the old listener settled cannot outlive it: refuse that setup + * and nothing may arrive for the bind. */ +static void TestForwardedTcpipConfirmedCancelDropsEarlierSuccess(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + /* The peer bound the first, honoured the cancel, then refused to bind + * again. Every answer is one a conforming peer can give. */ + FeedRequestSuccess(&harness); + FeedRequestSuccess(&harness); + FeedRequestFailure(&harness); + + AssertTrue(harness.ssh->fwdRemoteList == NULL); + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + +/* The same three requests with the peer binding the last one: the forward + * stands on that answer alone. */ +static void TestForwardedTcpipConfirmedCancelThenSetupBinds(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + FeedRequestSuccess(&harness); + FeedRequestSuccess(&harness); + FeedRequestSuccess(&harness); + + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + harness.io.outSz = 0; + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + +/* A port-0 setup the peer answers with a port another registration already + * stands for. There is one listener there, so the older registration is stale + * and the bind is left with one entry. */ +static void TestForwardedTcpipPortZeroReplyFoldsDuplicate(void) +{ + ChannelOpenHarness harness; + byte reply[64]; + word32 replySz; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 0, 1), + WS_SUCCESS); + + FeedRequestSuccess(&harness); + AssertIntEQ(FwdRemoteCount(harness.ssh), 2); + + /* The peer names the port the first registration already stands for. */ + replySz = BuildRequestSuccessPortPacket(8080, reply, sizeof(reply)); + AssertIntEQ(FeedOnePacket(&harness, reply, replySz), WS_SUCCESS); + + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + + /* One entry, so one cancel revokes the bind. */ + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + +static void TestForwardedTcpipRepliesPairInSendOrder(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + /* Replies carry no request id, so they pair with the outstanding + * requests in the order those went out. */ + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8081, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8082, 1), + WS_SUCCESS); + + FeedRequestSuccess(&harness); + FeedRequestFailure(&harness); + FeedRequestSuccess(&harness); + + /* Only the second forward was refused. Refusals first: an open failure + * asserts the channel list is empty. */ + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8081); + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8082, 2); + + FreeChannelOpenHarness(&harness); +} + +/* A port-0 request whose reply names no usable port leaves nothing to match + * on, so the registration goes. */ +static void RunForwardedTcpipBadPortReplyTest(const byte* reply, + word32 replySz) +{ + ChannelOpenHarness harness; + byte replyCopy[64]; + + AssertTrue(replySz <= sizeof(replyCopy)); + WMEMCPY(replyCopy, reply, replySz); + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 0, 1), + WS_SUCCESS); + + AssertIntEQ(FeedOnePacket(&harness, replyCopy, replySz), WS_SUCCESS); + + AssertForwardedOpenRefused(&harness, "127.0.0.1", REGRESS_FWD_ALLOC_PORT); + + FreeChannelOpenHarness(&harness); +} + +static void TestForwardedTcpipUnusablePortReplySendsOpenFail(void) +{ + byte reply[64]; + word32 replySz; + + /* Port 0 is not a port the peer could have bound. */ + replySz = BuildRequestSuccessPortPacket(0, reply, sizeof(reply)); + RunForwardedTcpipBadPortReplyTest(reply, replySz); + + /* A port is 16 bits on the wire. */ + replySz = BuildRequestSuccessPortPacket(70000, reply, sizeof(reply)); + RunForwardedTcpipBadPortReplyTest(reply, replySz); + + /* Want-reply on a port-0 request means the reply carries the port; a + * success without one names nothing. */ + replySz = WrapPacket(MSGID_REQUEST_SUCCESS, NULL, 0, reply, sizeof(reply)); + RunForwardedTcpipBadPortReplyTest(reply, replySz); +} + +static void TestForwardedTcpipPortZeroOtherPortSendsOpenFail(void) +{ + ChannelOpenHarness harness; + byte reply[64]; + word32 replySz; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 0, 1), + WS_SUCCESS); + + replySz = BuildRequestSuccessPortPacket(REGRESS_FWD_ALLOC_PORT, + reply, sizeof(reply)); + AssertIntEQ(FeedOnePacket(&harness, reply, replySz), WS_SUCCESS); + + /* Once the reply resolves the port, that port is the only match. */ + AssertForwardedOpenRefused(&harness, "127.0.0.1", + REGRESS_FWD_ALLOC_PORT + 1); + AssertForwardedOpenAccepted(&harness, "127.0.0.1", + REGRESS_FWD_ALLOC_PORT, 1); + + FreeChannelOpenHarness(&harness); +} + +#endif /* WOLFSSH_FWD && !NO_WOLFSSH_CLIENT */ + + /* Reject a peer KEXINIT once keying is in progress. */ static void TestKexInitRejectedWhenKeying(WOLFSSH* ssh) { @@ -8789,6 +10194,49 @@ int main(int argc, char** argv) TestAgentEd25519UserAuthPropagatesAgentError(); TestAgentEd25519UserAuthRejectsOversizeSignature(); #endif +#if defined(WOLFSSH_FWD) && !defined(NO_WOLFSSH_CLIENT) + TestForwardedTcpipRegisteredIsAccepted(); + TestForwardedTcpipUnregisteredSendsOpenFail(); + TestForwardedTcpipWildcardBindMatchesAnyAddr(); + TestForwardedTcpipCancelledSendsOpenFail(); + TestForwardedTcpipPortZeroMatchesBoundPort(); + TestForwardedTcpipRefusedForwardSendsOpenFail(); + TestForwardedTcpipUntrackedClientUnchanged(); + TestForwardedTcpipCancelConfirmedSendsOpenFail(); + TestForwardedTcpipCancelPendingStopsMatching(); + TestForwardedTcpipCancelRefusedRestoresMatching(); + TestForwardedTcpipCancelRefusedKeepsForward(); + TestForwardedTcpipUnmatchedCancelKeepsForward(); + TestForwardedTcpipCancelBeforeSetupReply(); + TestForwardedTcpipCancelBeforeSetupReplyKeepsOther(); + TestForwardedTcpipDuplicateSetupIsOneForward(); + TestForwardedTcpipDuplicateSetupRefusalKeepsForward(); + TestForwardedTcpipDuplicateSetupLaterSuccessBinds(); + TestForwardedTcpipDuplicateSetupBothRefusedDrops(); + TestForwardedTcpipSetupAfterPendingCancelKeepsForward(); + TestForwardedTcpipSetupAfterPendingCancelRefusedDrops(); + TestForwardedTcpipRefusedCancelThenCancelDrops(); + TestForwardedTcpipOverlappingCancelsLastOneSettles(); + TestForwardedTcpipNoReplyCancelOverridesPending(); + TestForwardedTcpipConfirmedCancelDropsEarlierSuccess(); + TestForwardedTcpipConfirmedCancelThenSetupBinds(); + TestForwardedTcpipPortZeroReplyFoldsDuplicate(); + TestForwardedTcpipPostSendErrorStillRegisters(); + TestForwardedTcpipFailedSendRegistersNothing(); + TestForwardedTcpipReentrantSetupDuringSend(); + TestForwardedTcpipReentrantRequestKeepsSendOrder(); + TestForwardedTcpipReplyDuringSendTakesSlot(); + TestForwardedTcpipRefusalDuringSendDropsForward(); + TestForwardedTcpipPortZeroReplyDuringSendBinds(); + TestForwardedTcpipRequestAfterReplyDuringSend(); + TestForwardedTcpipReentrantCancelDuringSend(); + TestForwardedTcpipAppRequestKeepsItsOwnReply(); + TestForwardedTcpipWantWriteStillRegisters(); + TestGlobalRequestNoReplyQueuesNothing(); + TestForwardedTcpipRepliesPairInSendOrder(); + TestForwardedTcpipUnusablePortReplySendsOpenFail(); + TestForwardedTcpipPortZeroOtherPortSendsOpenFail(); +#endif /* WOLFSSH_FWD && !NO_WOLFSSH_CLIENT */ TestKexInitRejectedWhenKeying(ssh); #if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) && !defined(WOLFSSH_NO_RSA) \ && !defined(WOLFSSH_NO_CURVE25519_SHA256) \ diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 6254cb698..7fba93379 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1010,6 +1010,55 @@ struct WS_SFTP_RENAME_STATE; struct WOLFSSH_AGENT_CTX; +#ifdef WOLFSSH_FWD + +/* A remote forward this client asked the peer to listen on with + * wolfSSH_FwdRemoteSetup(), kept so an inbound forwarded-tcpip open can be + * matched against it. */ +typedef struct WOLFSSH_FWD_REMOTE { + struct WOLFSSH_FWD_REMOTE* next; + char* bindAddr; + word32 bindPort; /* port the peer bound, 0 while still unknown */ + byte portPending; /* asked for port 0, so the reply names the port */ + byte confirmed; /* the peer bound it, or nothing will ever say */ +} WOLFSSH_FWD_REMOTE; + +/* One want-reply global request waiting on the peer. Replies carry no request + * id, so the queue answers them in send order. Requests an application sent + * through wolfSSH_global_request() queue here too with a NULL entry, so their + * replies consume their own slot instead of a forward's. + * + * Several requests can name one forward, and this queue is the only record of + * which are outstanding and of the order they will be answered in. What the + * application last asked for is the last slot naming the forward. */ +typedef struct WOLFSSH_FWD_REPLY { + struct WOLFSSH_FWD_REPLY* next; + WOLFSSH_FWD_REMOTE* entry; /* forward answered, NULL once it is gone or if + * the application sent the request */ + word32 port; /* port a parked answer named */ + byte isCancel; /* answers cancel-tcpip-forward */ + byte uncommitted; /* its request has not reached the wire, so the + * sender still owns this slot */ + byte answered; /* answered while still uncommitted, so the + * verdict is parked here for the commit */ + byte success; /* what that parked answer said */ +} WOLFSSH_FWD_REPLY; + +/* Bookkeeping for a global request that has not been sent yet. Everything that + * can fail is allocated into one of these first, so a caller that gets an error + * back knows nothing reached the peer. The registration is named by its bind + * rather than by a pointer, since sending runs application callbacks that may + * reenter the library and free it. */ +typedef struct WOLFSSH_FWD_PENDING { + WOLFSSH_FWD_REMOTE* entry; /* new registration to link on commit */ + WOLFSSH_FWD_REPLY* reply; /* reply slot to queue on commit */ + const char* bindAddr; /* bind the request names, NULL if none */ + word32 bindPort; + byte isCancel; +} WOLFSSH_FWD_PENDING; + +#endif /* WOLFSSH_FWD */ + /* our wolfSSH session */ struct WOLFSSH { WOLFSSH_CTX* ctx; /* owner context */ @@ -1242,6 +1291,10 @@ struct WOLFSSH { #endif /* WOLFSSH_AGENT */ #ifdef WOLFSSH_FWD void* fwdCbCtx; + WOLFSSH_FWD_REMOTE* fwdRemoteList; /* remote forwards this client asked for */ + WOLFSSH_FWD_REPLY* fwdReplyHead; /* oldest want-reply request owed */ + WOLFSSH_FWD_REPLY* fwdReplyTail; + byte fwdRemoteTracked; /* wolfSSH_FwdRemoteSetup() was used */ #endif /* WOLFSSH_FWD */ #ifdef WOLFSSH_TERM WS_CallbackTerminalSize termResizeCb; @@ -1570,6 +1623,15 @@ WOLFSSH_LOCAL int SendGlobalRequest(WOLFSSH * ssh, WOLFSSH_LOCAL int SendGlobalRequestFwd(WOLFSSH* ssh, const char* bindAddr, word32 bindPort, int isCancel, int wantReply, int* sent); +/* On success pend holds what to commit once the request reaches the wire; on + * error it is zeroed, so there is nothing to commit or give back. */ +WOLFSSH_LOCAL int FwdRemotePrepare(WOLFSSH* ssh, const char* bindAddr, + word32 bindPort, int wantReply, int isCancel, + WOLFSSH_FWD_PENDING* pend); +WOLFSSH_LOCAL int FwdReplyPrepare(WOLFSSH* ssh, WOLFSSH_FWD_PENDING* pend); +WOLFSSH_LOCAL void FwdPendingCommit(WOLFSSH* ssh, WOLFSSH_FWD_PENDING* pend); +WOLFSSH_LOCAL void FwdPendingDiscard(WOLFSSH* ssh, WOLFSSH_FWD_PENDING* pend); +WOLFSSH_LOCAL void FwdRemoteFreeList(WOLFSSH* ssh, void* heap); #endif WOLFSSH_LOCAL int SendDebug(WOLFSSH* ssh, byte alwaysDisplay, const char* msg); WOLFSSH_LOCAL int SendServiceRequest(WOLFSSH* ssh, byte serviceId); @@ -2010,7 +2072,8 @@ enum WS_DynamicTypes { DYNTYPE_FILE, DYNTYPE_TEMP, DYNTYPE_PATH, - DYNTYPE_SSHD + DYNTYPE_SSHD, + DYNTYPE_FWD }; diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index 110f96abd..6b83c169e 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -297,6 +297,43 @@ DEPRECATED WOLFSSH_API int wolfSSH_ChannelSetFwdFd(WOLFSSH_CHANNEL* channel, int fwdFd); DEPRECATED WOLFSSH_API int wolfSSH_ChannelGetFwdFd( const WOLFSSH_CHANNEL* channel); +/* Ask the peer to listen on bindAddr:bindPort and tunnel what arrives back as + * "forwarded-tcpip" channels. Client-only, per RFC 4254 7.1. bindPort 0 asks + * the peer to choose, and needs wantReply, since its reply is the only place + * the bound port is named; without it the call returns WS_BAD_ARGUMENT. + * + * From the first call on, this session refuses any "forwarded-tcpip" open + * naming a bind it did not register, per RFC 4254 7.2. A bind of "", "*", + * "0.0.0.0", or an IPv6 any-address matches on port alone; anything else must + * equal the address the peer reports. Register the spelling the peer will echo + * back, or a wildcard: a peer that canonicalises the bind, answering an open + * for "127.0.0.1" against a registered "localhost", has those opens refused. + * + * One bindAddr:bindPort is one registration however often it is registered, + * since it is one listener on the peer, so one cancel undoes it. That covers a + * port-0 request the peer answers with a port already registered, and a repeat + * the peer refuses because it already has that listener keeps what the first + * request registered. + * + * Several requests can name one bind at once, and the last one sent governs. + * Registering again while a cancel is outstanding brings the forward back as + * the request goes out, and no answer to that older cancel takes it away + * again, whatever order the peer answers in. + * + * WS_WANT_WRITE means the request is framed and goes out on the next flush, + * with the forward registered. So does an error reported after the request + * reached the peer: the rekey a send can trigger runs once the last byte is + * out and fails here. Retrying then is a repeat setup, which is harmless. Only + * an error that kept the request off the wire leaves nothing registered. + * + * Cancel takes the port the peer bound, which after a port-0 request is the + * one it reported, not 0; the forward cannot be cancelled before that reply + * arrives. The forward stops matching as the cancel goes out, so revoking one + * never waits on the peer and an open racing it is refused. Without wantReply + * that is the end of it. With it the registration is held until the peer + * answers: a refusal leaves the listener up and puts the forward back, and a + * confirmation drops it. With several cancels outstanding, every one of them + * has to be refused for the forward to come back. */ WOLFSSH_API int wolfSSH_FwdRemoteSetup(WOLFSSH* ssh, const char* bindAddr, word32 bindPort, int wantReply); WOLFSSH_API int wolfSSH_FwdRemoteCancel(WOLFSSH* ssh, const char* bindAddr, @@ -693,6 +730,11 @@ WOLFSSH_API int wolfSSH_SendIgnore(WOLFSSH* ssh, const byte* buf, word32 bufSz); * it too, with or without a channel. A disconnect from the peer is not that * case: it leaves only unrelated traffic queued, and that stays put. */ WOLFSSH_API int wolfSSH_SendDisconnect(WOLFSSH* ssh, word32 reason); +/* Send a global request under a name the caller supplies. The request-specific + * data RFC 4254 7.1 puts after the want-reply boolean cannot be carried here, + * so requests needing it have their own calls. Replies carry no request id, so + * with reply set this claims a place in the same send-order queue + * wolfSSH_FwdRemoteSetup() uses. */ WOLFSSH_API int wolfSSH_global_request(WOLFSSH* ssh, const unsigned char* data, word32 dataSz, int reply); /* Reads the channel named by channelId, with wolfSSH_ChannelRead()'s From 37eb0706dab75f1b34115369d205c36e7292db5c Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 6 Aug 2026 11:53:35 -0700 Subject: [PATCH 04/12] Fix interrupted and failed sends in SendPacket A signal was turned into a fatal error, and a refused send left its packet counted in the output buffer. Both fixes are in wolfSSH_SendPacket(), so they cover every sender. - WS_CBIO_ERR_ISR fell through to WS_SOCKET_ERROR_E. Nothing went out and the session is unharmed, so retry, as ReceiveData() already does. - Callers that discard a packet on error, like the KEX and userauth sends, were throwing away framed output the peer never refused. - On WS_CBIO_ERR_GENERAL the buffer was shrunk with the packet still counted in plainSz, so SendChannelData() flushed nothing and called it a success. Clear it with the packet it described. - Tests pin the retry from a forwarding sender and a plain global request, and drive a channel send through a would-block and a refused flush. --- src/internal.c | 25 ++++++++++---- tests/regress.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 6 deletions(-) diff --git a/src/internal.c b/src/internal.c index 5436eb098..a778e51e8 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4896,6 +4896,14 @@ int wolfSSH_SendPacket(WOLFSSH* ssh) ssh->error = WS_WANT_WRITE; return WS_WANT_WRITE; + case WS_CBIO_ERR_ISR: + /* A signal interrupted the send. Nothing went out and the + * session is unharmed, so retry, as ReceiveData() does for + * the same condition. Reporting it instead loses framed + * output the peer never refused, since callers discard + * their packet on an error. */ + continue; + case WS_CBIO_ERR_CONN_RST: /* connection reset */ ssh->connReset = 1; break; @@ -4905,6 +4913,11 @@ int wolfSSH_SendPacket(WOLFSSH* ssh) break; case WS_CBIO_ERR_GENERAL: + /* plainSz counts plaintext the caller was told was + * accepted, so it goes with the packet being discarded. + * Left standing, it has SendChannelData() flush an empty + * buffer and call that a success. */ + ssh->outputBuffer.plainSz = 0; ShrinkBuffer(&ssh->outputBuffer, 1); } return WS_SOCKET_ERROR_E; @@ -17603,12 +17616,12 @@ int SendIgnore(WOLFSSH* ssh, const unsigned char* data, word32 dataSz) * Comparing the flush count across the send tells those apart. * * Short of a flush, WS_WANT_WRITE is the one outcome that keeps the packet - * framed for the next one, and reading the buffer instead would call a packet - * delivered that a later purge or a discarding error path throws away. - * Anything else counts as not sent, which at worst leaves the peer holding a - * request this side did not register; guessing the other way would desync the - * reply queue for the life of the session. Call before anything else runs, - * since a later send flushes this packet and would read as this one's. */ + * framed for the next one; an interrupted send is retried inside + * wolfSSH_SendPacket() rather than reported. Anything else counts as not sent, + * which at worst leaves the peer holding a request this side did not register; + * guessing the other way would desync the reply queue for the life of the + * session. Call before anything else runs, since a later send flushes this + * packet and would read as this one's. */ static INLINE int SendPacketDelivered(WOLFSSH* ssh, word32 flushes, int ret) { return ssh->txFlushCount != flushes || ret == WS_WANT_WRITE; diff --git a/tests/regress.c b/tests/regress.c index 499fcc72a..5f56f0f56 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -331,6 +331,7 @@ typedef struct { word32 outSz; word32 outCap; byte blockNext; /* make the next send report a would-block */ + byte isrNext; /* make the next send report an interrupted call */ } MemIo; static int MemRecv(WOLFSSH* ssh, void* buf, word32 sz, void* ctx) @@ -355,6 +356,10 @@ static int MemSend(WOLFSSH* ssh, void* buf, word32 sz, void* ctx) io->blockNext = 0; return WS_CBIO_ERR_WANT_WRITE; } + if (io->isrNext) { + io->isrNext = 0; + return WS_CBIO_ERR_ISR; + } if (io->outSz + sz > io->outCap) { return WS_CBIO_ERR_GENERAL; } @@ -372,6 +377,7 @@ static void MemIoInit(MemIo* io, byte* in, word32 inSz, byte* out, word32 outCap io->outSz = 0; io->outCap = outCap; io->blockNext = 0; + io->isrNext = 0; } /* The in-memory session harness. The struct and its teardown are shared; the @@ -2500,6 +2506,45 @@ static void TestServerServiceRequestRejectedDuringKeying(void) } +/* A send the transport refuses discards the packet it had framed, and plainSz + * counted that packet's plaintext. Left standing over an emptied buffer, it + * has the next SendChannelData() flush nothing and call that a success. */ +static void TestFailedSendClearsPendingPlaintext(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte payload[16]; + + InitChannelOpenHarness(&harness, NULL, 0); + WMEMSET(payload, 'a', sizeof(payload)); + + channel = ChannelNew(harness.ssh, ID_CHANTYPE_SESSION, 1024, 1024); + AssertNotNull(channel); + AssertIntEQ(ChannelUpdatePeer(channel, 0, 1024, 1024), WS_SUCCESS); + AssertIntEQ(ChannelAppend(harness.ssh, channel), WS_SUCCESS); + + /* The transport blocks, so the packet stays framed and the caller is told + * its data was taken. */ + harness.io.blockNext = 1; + AssertIntEQ(wolfSSH_stream_send(harness.ssh, payload, sizeof(payload)), + (int)sizeof(payload)); + AssertIntEQ(harness.ssh->outputBuffer.plainSz, (int)sizeof(payload)); + AssertIntEQ(wolfSSH_OutputPending(harness.ssh), 1); + + /* The next call flushes that packet first, and this send fails outright, + * so what it was flushing is thrown away. */ + harness.io.outSz = harness.io.outCap; + AssertIntEQ(wolfSSH_stream_send(harness.ssh, payload, sizeof(payload)), + WS_SOCKET_ERROR_E); + + /* Nothing is framed any more, so nothing may still be counted as + * pending. */ + AssertIntEQ(wolfSSH_OutputPending(harness.ssh), 0); + AssertIntEQ(harness.ssh->outputBuffer.plainSz, 0); + + FreeChannelOpenHarness(&harness); +} + static void TestChannelOpenCallbackRejectSendsOpenFail(void) { ChannelOpenHarness harness; @@ -4676,6 +4721,50 @@ static void TestForwardedTcpipWantWriteStillRegisters(void) FreeChannelOpenHarness(&harness); } +/* A signal interrupts the send, which retries rather than reporting it, so the + * request goes out and the forward registers like any other. */ +static void TestForwardedTcpipInterruptedSendStillRegisters(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + harness.io.isrNext = 1; + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 0), + WS_SUCCESS); + + /* Retried and out, with nothing left framed and the session unharmed. */ + AssertTrue(harness.io.outSz > 0); + AssertIntEQ(wolfSSH_OutputPending(harness.ssh), 0); + AssertIntEQ(harness.ssh->connReset, 0); + AssertIntEQ(harness.ssh->isClosed, 0); + + harness.io.outSz = 0; + AssertForwardedOpenRefused(&harness, "10.0.0.1", 9999); + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + +/* The same retry on a sender unrelated to forwarding: it is in + * wolfSSH_SendPacket(), so it governs every send in the library. */ +static void TestInterruptedSendRetriesForAnySender(void) +{ + ChannelOpenHarness harness; + const byte req[] = "keepalive@openssh.com"; + + InitFwdRemoteHarness(&harness); + + harness.io.isrNext = 1; + AssertIntEQ(wolfSSH_global_request(harness.ssh, req, + (word32)sizeof(req) - 1, 0), WS_SUCCESS); + + AssertTrue(harness.io.outSz > 0); + AssertIntEQ(wolfSSH_OutputPending(harness.ssh), 0); + + FreeChannelOpenHarness(&harness); +} + /* A request the application sends without want-reply is answered by nothing, * so it must not take a place in the reply queue and eat the answer owed to an * outstanding tcpip-forward. */ @@ -10158,6 +10247,7 @@ int main(int argc, char** argv) TestServerOnlyUserauthMsgsBlocked(serverSsh); TestServerServiceRequestStateGated(serverSsh); TestServerServiceRequestRejectedDuringKeying(); + TestFailedSendClearsPendingPlaintext(); TestChannelOpenCallbackRejectSendsOpenFail(); TestSecondSessionChannelRejected(); TestUsernameChangeDisconnects(); @@ -10232,6 +10322,8 @@ int main(int argc, char** argv) TestForwardedTcpipReentrantCancelDuringSend(); TestForwardedTcpipAppRequestKeepsItsOwnReply(); TestForwardedTcpipWantWriteStillRegisters(); + TestForwardedTcpipInterruptedSendStillRegisters(); + TestInterruptedSendRetriesForAnySender(); TestGlobalRequestNoReplyQueuesNothing(); TestForwardedTcpipRepliesPairInSendOrder(); TestForwardedTcpipUnusablePortReplySendsOpenFail(); From 69ed0e025a4a329d34fca7e1a8938c6c1c389795 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 11 Aug 2026 14:12:14 -0700 Subject: [PATCH 05/12] Let the reply-queue scans see a slot still in its send window A slot named its forward only once its request committed, so the scans could not see a request mid-send. A cancel the peer confirms while a fresh setup for the same bind is still going out then found nothing standing for the forward and unlinked it, leaving the peer with a listener the client refuses every open for. - Name a slot by its bind until it commits. - FwdReplyNames() answers for both FwdReplyHasSetup() and FwdReplyNewest(). - The bind is borrowed from the caller and dropped at commit, the lifetime WOLFSSH_FWD_PENDING already assumes for it. - A pointer to the entry would not do: the send runs callbacks that can free and remake it, which is why the entry is re-resolved at commit. - Test drives a confirmed cancel against a re-setup still in its send window. --- src/internal.c | 47 ++++++++++++++++++++++++----- tests/regress.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++ wolfssh/internal.h | 6 ++++ 3 files changed, 119 insertions(+), 8 deletions(-) diff --git a/src/internal.c b/src/internal.c index a778e51e8..89764d2a4 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4010,6 +4010,29 @@ static void FwdRemoteUnlink(WOLFSSH* ssh, void* heap, } +/* Does this slot answer for that forward? A slot still in its send window + * doesn't name the forward yet, but the bind it will name is known, and an + * answer settling an earlier request turns on whether a later one is already + * on its way out. */ +static int FwdReplyNames(const WOLFSSH_FWD_REPLY* reply, + const WOLFSSH_FWD_REMOTE* entry) +{ + if (entry == NULL) + return 0; + + if (reply->entry == entry) + return 1; + + if (!reply->uncommitted || reply->bindAddr == NULL) + return 0; + + /* A port-0 forward has no port to be named by until the peer's reply + * says which one it bound. */ + return !entry->portPending && entry->bindPort == reply->bindPort && + WSTRCMP(entry->bindAddr, reply->bindAddr) == 0; +} + + /* The last queued request naming this forward, or NULL. The queue is in send * order, so this is what the application asked for most recently. */ static WOLFSSH_FWD_REPLY* FwdReplyNewest(WOLFSSH* ssh, @@ -4019,7 +4042,7 @@ static WOLFSSH_FWD_REPLY* FwdReplyNewest(WOLFSSH* ssh, WOLFSSH_FWD_REPLY* newest = NULL; for (cur = ssh->fwdReplyHead; cur != NULL; cur = cur->next) { - if (cur->entry == entry) + if (FwdReplyNames(cur, entry)) newest = cur; } @@ -4033,7 +4056,7 @@ static int FwdReplyHasSetup(WOLFSSH* ssh, const WOLFSSH_FWD_REMOTE* entry) WOLFSSH_FWD_REPLY* cur; for (cur = ssh->fwdReplyHead; cur != NULL; cur = cur->next) { - if (cur->entry == entry && !cur->isCancel) + if (!cur->isCancel && FwdReplyNames(cur, entry)) return 1; } @@ -4131,9 +4154,11 @@ static void FwdRemoteSettle(WOLFSSH* ssh, WOLFSSH_FWD_REMOTE* entry, /* Take this request's place in the reply queue before it is sent, so a - * callback that reenters the library mid-send cannot queue ahead of it. Which - * forward the slot answers for is filled in on commit. */ -static WOLFSSH_FWD_REPLY* FwdReplyNew(WOLFSSH* ssh, int isCancel) + * callback that reenters the library mid-send cannot queue ahead of it. The + * forward the slot answers for is named by its bind until it commits, since + * the entry it resolves to can be freed and remade across the send. */ +static WOLFSSH_FWD_REPLY* FwdReplyNew(WOLFSSH* ssh, int isCancel, + const char* bindAddr, word32 bindPort) { WOLFSSH_FWD_REPLY* reply; @@ -4142,6 +4167,8 @@ static WOLFSSH_FWD_REPLY* FwdReplyNew(WOLFSSH* ssh, int isCancel) if (reply != NULL) { WMEMSET(reply, 0, sizeof(WOLFSSH_FWD_REPLY)); reply->isCancel = (byte)(isCancel != 0); + reply->bindAddr = bindAddr; + reply->bindPort = bindPort; /* The sender owns this slot until it commits; an answer arriving * meanwhile parks its verdict here. */ reply->uncommitted = 1; @@ -4266,7 +4293,7 @@ int FwdRemotePrepare(WOLFSSH* ssh, const char* bindAddr, word32 bindPort, } if (ret == WS_SUCCESS && wantReply) { - pend->reply = FwdReplyNew(ssh, isCancel); + pend->reply = FwdReplyNew(ssh, isCancel, bindAddr, bindPort); if (pend->reply == NULL) { if (pend->entry != NULL) { WFREE(pend->entry->bindAddr, heap, DYNTYPE_STRING); @@ -4300,7 +4327,9 @@ int FwdReplyPrepare(WOLFSSH* ssh, WOLFSSH_FWD_PENDING* pend) if (ssh == NULL || ssh->ctx == NULL || pend == NULL) return WS_BAD_ARGUMENT; - pend->reply = FwdReplyNew(ssh, 0); + /* An application's own request names no forward, but it consumes a reply, + * so it holds a place in the queue. */ + pend->reply = FwdReplyNew(ssh, 0, NULL, 0); ret = pend->reply == NULL ? WS_MEMORY_E : WS_SUCCESS; WLOG(WS_LOG_DEBUG, "Leaving FwdReplyPrepare(), ret = %d", ret); @@ -4362,8 +4391,10 @@ void FwdPendingCommit(WOLFSSH* ssh, WOLFSSH_FWD_PENDING* pend) } else if (pend->reply != NULL) { /* The slot is queued already; naming the forward makes it the newest - * request outstanding on it. */ + * request outstanding on it. The bind gave the scans something to find + * it by meanwhile, and is the caller's to free from here. */ pend->reply->entry = target; + pend->reply->bindAddr = NULL; pend->reply->uncommitted = 0; } else if (target != NULL) { diff --git a/tests/regress.c b/tests/regress.c index 5f56f0f56..174fbd22d 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -4627,6 +4627,79 @@ static void TestForwardedTcpipReplyDuringSendTakesSlot(void) FreeChannelOpenHarness(&harness); } +static int CancelAnsweredDuringSendHighwaterCb(byte side, void* ctx) +{ + ChannelOpenHarness* harness = (ChannelOpenHarness*)ctx; + byte reply[64]; + word32 replySz; + + WOLFSSH_UNUSED(side); + + /* Answers the cancel queued ahead of the setup now going out. */ + if (harness != NULL) { + replySz = WrapPacket(MSGID_REQUEST_SUCCESS, NULL, 0, reply, + sizeof(reply)); + FeedOnePacket(harness, reply, replySz); + } + + return WS_SUCCESS; +} + +/* The peer confirming a cancel while a fresh setup for the same bind is still + * in its send window. That setup's slot does not name the forward until it + * commits, so a scan that only reads committed slots finds nothing standing + * for the forward and unlinks it -- leaving a request on the wire the peer + * will bind and no registration for its opens to match. */ +static void TestForwardedTcpipCancelAnsweredDuringResetupKeepsForward(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + harness.io.outSz = 0; + FeedRequestSuccess(&harness); + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + AssertIntEQ(harness.ssh->fwdRemoteList->confirmed, 1); + + /* The registration is held until the peer answers the cancel. */ + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + + /* Cross the mark on the re-setup's own send. */ + wolfSSH_SetHighwaterCb(harness.ctx, 1, + CancelAnsweredDuringSendHighwaterCb); + wolfSSH_SetHighwaterCtx(harness.ssh, &harness); + harness.ssh->highwaterMark = 1; + harness.ssh->highwaterFlag = 0; + harness.ssh->txCount = 1; + harness.io.outSz = 0; + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + /* The cancel took the listener down and the setup asked for it back, so + * the registration stands and waits on that answer instead of going. */ + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + AssertIntEQ(harness.ssh->fwdRemoteList->confirmed, 0); + AssertNotNull(harness.ssh->fwdReplyHead); + AssertTrue(harness.ssh->fwdReplyHead->entry == harness.ssh->fwdRemoteList); + AssertIntEQ(harness.ssh->fwdReplyHead->uncommitted, 0); + AssertTrue(harness.ssh->fwdReplyHead->next == NULL); + + harness.io.outSz = 0; + FeedRequestSuccess(&harness); + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + AssertIntEQ(harness.ssh->fwdRemoteList->confirmed, 1); + + harness.io.outSz = 0; + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + static int CancelDuringSendHighwaterCb(byte side, void* ctx) { WOLFSSH* ssh = (WOLFSSH*)ctx; @@ -10319,6 +10392,7 @@ int main(int argc, char** argv) TestForwardedTcpipRefusalDuringSendDropsForward(); TestForwardedTcpipPortZeroReplyDuringSendBinds(); TestForwardedTcpipRequestAfterReplyDuringSend(); + TestForwardedTcpipCancelAnsweredDuringResetupKeepsForward(); TestForwardedTcpipReentrantCancelDuringSend(); TestForwardedTcpipAppRequestKeepsItsOwnReply(); TestForwardedTcpipWantWriteStillRegisters(); diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 7fba93379..942ddf99c 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1035,6 +1035,12 @@ typedef struct WOLFSSH_FWD_REPLY { struct WOLFSSH_FWD_REPLY* next; WOLFSSH_FWD_REMOTE* entry; /* forward answered, NULL once it is gone or if * the application sent the request */ + const char* bindAddr; /* bind this slot will name once it commits, so + * the scans can see it meanwhile. Borrowed from + * the caller and dropped at commit, the same + * lifetime WOLFSSH_FWD_PENDING assumes. NULL + * once committed. */ + word32 bindPort; word32 port; /* port a parked answer named */ byte isCancel; /* answers cancel-tcpip-forward */ byte uncommitted; /* its request has not reached the wire, so the From 803d3ea66fc264b92a673f53f8f8db846d49fd59 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 11 Aug 2026 14:12:14 -0700 Subject: [PATCH 06/12] Cap the pending-reply queue Only the peer gives a slot back, by answering, so a peer that never answers a want-reply global request lets the queue grow for the life of the session, and the queue scans run once per forward on every inbound forwarded-tcpip open. - WOLFSSH_MAX_FWD_REPLIES bounds the queue. - Refuse before the request is framed: one whose slot was never queued would mispair every later reply. - Return WS_RESOURCE_E, not the WS_MEMORY_E that nothing failing to allocate would have yielded. - Test fills the queue, then covers the refusal, that a refused setup registers nothing, and a slot coming back on an answer. --- src/internal.c | 16 +++++++++++++++ tests/regress.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++ wolfssh/internal.h | 9 ++++++++ 3 files changed, 76 insertions(+) diff --git a/src/internal.c b/src/internal.c index 89764d2a4..57cee4b9f 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4178,6 +4178,7 @@ static WOLFSSH_FWD_REPLY* FwdReplyNew(WOLFSSH* ssh, int isCancel, else ssh->fwdReplyTail->next = reply; ssh->fwdReplyTail = reply; + ssh->fwdReplyCount++; } return reply; @@ -4207,6 +4208,7 @@ static void FwdReplyUnqueue(WOLFSSH* ssh, WOLFSSH_FWD_REPLY* reply) if (ssh->fwdReplyTail == reply) ssh->fwdReplyTail = prev; + ssh->fwdReplyCount--; WFREE(reply, ssh->ctx->heap, DYNTYPE_FWD); } @@ -4232,6 +4234,13 @@ int FwdRemotePrepare(WOLFSSH* ssh, const char* bindAddr, word32 bindPort, if (ssh == NULL || ssh->ctx == NULL || bindAddr == NULL || pend == NULL) return WS_BAD_ARGUMENT; + /* Refusing here keeps the request off the wire. Framing one whose slot was + * never queued would mispair every later reply for the session. */ + if (wantReply && ssh->fwdReplyCount >= WOLFSSH_MAX_FWD_REPLIES) { + WLOG(WS_LOG_ERROR, "Too many global requests await a reply"); + return WS_RESOURCE_E; + } + heap = ssh->ctx->heap; pend->isCancel = (byte)(isCancel != 0); pend->bindAddr = bindAddr; @@ -4327,6 +4336,11 @@ int FwdReplyPrepare(WOLFSSH* ssh, WOLFSSH_FWD_PENDING* pend) if (ssh == NULL || ssh->ctx == NULL || pend == NULL) return WS_BAD_ARGUMENT; + if (ssh->fwdReplyCount >= WOLFSSH_MAX_FWD_REPLIES) { + WLOG(WS_LOG_ERROR, "Too many global requests await a reply"); + return WS_RESOURCE_E; + } + /* An application's own request names no forward, but it consumes a reply, * so it holds a place in the queue. */ pend->reply = FwdReplyNew(ssh, 0, NULL, 0); @@ -4505,6 +4519,7 @@ static void FwdRemoteReply(WOLFSSH* ssh, int success, const byte* buf, ssh->fwdReplyHead = reply->next; if (ssh->fwdReplyHead == NULL) ssh->fwdReplyTail = NULL; + ssh->fwdReplyCount--; if (reply->uncommitted) { /* The request this answers is still being sent -- a callback the send @@ -4551,6 +4566,7 @@ void FwdRemoteFreeList(WOLFSSH* ssh, void* heap) } ssh->fwdReplyHead = NULL; ssh->fwdReplyTail = NULL; + ssh->fwdReplyCount = 0; } #endif /* WOLFSSH_FWD */ diff --git a/tests/regress.c b/tests/regress.c index 174fbd22d..3406903ab 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -4700,6 +4700,56 @@ static void TestForwardedTcpipCancelAnsweredDuringResetupKeepsForward(void) FreeChannelOpenHarness(&harness); } +/* Only the peer gives a reply slot back, so a peer that never answers would + * let the queue grow for the life of the session and lengthen every match. + * Refusing has to happen before the request is framed: one whose slot was + * never queued would mispair every later reply. */ +static void TestFwdReplyQueueIsCapped(void) +{ + ChannelOpenHarness harness; + const byte req[] = "keepalive@openssh.com"; + word32 i; + + InitFwdRemoteHarness(&harness); + + for (i = 0; i < WOLFSSH_MAX_FWD_REPLIES; i++) { + harness.io.outSz = 0; + AssertIntEQ(wolfSSH_global_request(harness.ssh, req, + (word32)sizeof(req) - 1, 1), WS_SUCCESS); + } + AssertIntEQ(harness.ssh->fwdReplyCount, WOLFSSH_MAX_FWD_REPLIES); + + harness.io.outSz = 0; + AssertIntEQ(wolfSSH_global_request(harness.ssh, req, + (word32)sizeof(req) - 1, 1), WS_RESOURCE_E); + AssertIntEQ(harness.io.outSz, 0); + AssertIntEQ(harness.ssh->fwdReplyCount, WOLFSSH_MAX_FWD_REPLIES); + + /* Forward requests share the queue, so they share the cap, and a refused + * one registers nothing. */ + harness.io.outSz = 0; + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_RESOURCE_E); + AssertIntEQ(harness.io.outSz, 0); + AssertIntEQ(FwdRemoteCount(harness.ssh), 0); + + /* Without wantReply nothing is queued, so nothing is capped. */ + harness.io.outSz = 0; + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 0), + WS_SUCCESS); + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + + /* An answer gives a slot back, and the next request fits in it. */ + harness.io.outSz = 0; + FeedRequestSuccess(&harness); + AssertIntEQ(harness.ssh->fwdReplyCount, WOLFSSH_MAX_FWD_REPLIES - 1); + harness.io.outSz = 0; + AssertIntEQ(wolfSSH_global_request(harness.ssh, req, + (word32)sizeof(req) - 1, 1), WS_SUCCESS); + + FreeChannelOpenHarness(&harness); +} + static int CancelDuringSendHighwaterCb(byte side, void* ctx) { WOLFSSH* ssh = (WOLFSSH*)ctx; @@ -10394,6 +10444,7 @@ int main(int argc, char** argv) TestForwardedTcpipRequestAfterReplyDuringSend(); TestForwardedTcpipCancelAnsweredDuringResetupKeepsForward(); TestForwardedTcpipReentrantCancelDuringSend(); + TestFwdReplyQueueIsCapped(); TestForwardedTcpipAppRequestKeepsItsOwnReply(); TestForwardedTcpipWantWriteStillRegisters(); TestForwardedTcpipInterruptedSendStillRegisters(); diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 942ddf99c..8292e5c13 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1012,6 +1012,14 @@ struct WOLFSSH_AGENT_CTX; #ifdef WOLFSSH_FWD +/* Most want-reply global requests a session may have outstanding at once. The + * peer gives a slot back by answering, so a peer that never does would + * otherwise let the queue grow for the life of the session. Set high enough + * that a non-blocking application pipelining setups stays well under it. */ +#ifndef WOLFSSH_MAX_FWD_REPLIES +#define WOLFSSH_MAX_FWD_REPLIES 1024 +#endif + /* A remote forward this client asked the peer to listen on with * wolfSSH_FwdRemoteSetup(), kept so an inbound forwarded-tcpip open can be * matched against it. */ @@ -1300,6 +1308,7 @@ struct WOLFSSH { WOLFSSH_FWD_REMOTE* fwdRemoteList; /* remote forwards this client asked for */ WOLFSSH_FWD_REPLY* fwdReplyHead; /* oldest want-reply request owed */ WOLFSSH_FWD_REPLY* fwdReplyTail; + word32 fwdReplyCount; /* slots queued, kept off the walk it bounds */ byte fwdRemoteTracked; /* wolfSSH_FwdRemoteSetup() was used */ #endif /* WOLFSSH_FWD */ #ifdef WOLFSSH_TERM From ab9359c573ccc5c38e04f0a8939c0b40fa5232aa Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 11 Aug 2026 14:12:14 -0700 Subject: [PATCH 07/12] Add a match setting for remote forwards The forwarded-tcpip check is a behaviour change on a shipped API, so an application that trips over it needs a way out that isn't abandoning wolfSSH_FwdRemoteSetup(). wolfSSH_SetFwdRemoteMatch() relaxes the check for the session. - STRICT is the default and keeps the bind-plus-port rule. - PORT compares the port alone, for a peer that rewrites the bind address it echoes back, which STRICT refuses every open from. - OFF accepts any open, as wolfSSH did before the check existed. - Tests cover each setting, and a refused one leaving the default in place. --- src/internal.c | 9 +++++++- src/ssh.c | 16 +++++++++++++ tests/regress.c | 56 ++++++++++++++++++++++++++++++++++++++++++++-- wolfssh/internal.h | 2 ++ wolfssh/ssh.h | 21 +++++++++++++++++ 5 files changed, 101 insertions(+), 3 deletions(-) diff --git a/src/internal.c b/src/internal.c index 57cee4b9f..6ac443ae3 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4470,6 +4470,10 @@ static int FwdRemoteMatch(WOLFSSH* ssh, const char* addr, word32 port) if (ssh == NULL || addr == NULL) return 0; + /* The application took responsibility for what it accepts. */ + if (ssh->fwdRemoteMatch == WOLFSSH_FWD_MATCH_OFF) + return 1; + for (cur = ssh->fwdRemoteList; cur != NULL; cur = cur->next) { WOLFSSH_FWD_REPLY* newest; @@ -4490,7 +4494,10 @@ static int FwdRemoteMatch(WOLFSSH* ssh, const char* addr, word32 port) if (!cur->confirmed && newest == NULL) continue; - if (FwdRemoteAddrIsWild(cur->bindAddr) || + /* A peer that rewrites the bind it echoes back can still be held to + * the port it was asked for. */ + if (ssh->fwdRemoteMatch == WOLFSSH_FWD_MATCH_PORT || + FwdRemoteAddrIsWild(cur->bindAddr) || WSTRCMP(cur->bindAddr, addr) == 0) return 1; } diff --git a/src/ssh.c b/src/ssh.c index 2fc9d8ea0..f117a2ef5 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -4061,6 +4061,22 @@ int wolfSSH_FwdRemoteCancel(WOLFSSH* ssh, const char* bindAddr, return ret; } + +int wolfSSH_SetFwdRemoteMatch(WOLFSSH* ssh, byte match) +{ + int ret = WS_SUCCESS; + + WLOG(WS_LOG_DEBUG, "Entering wolfSSH_SetFwdRemoteMatch()"); + + if (ssh == NULL || match > WOLFSSH_FWD_MATCH_OFF) + ret = WS_BAD_ARGUMENT; + else + ssh->fwdRemoteMatch = match; + + WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_SetFwdRemoteMatch(), ret = %d", ret); + return ret; +} + #endif /* WOLFSSH_FWD */ diff --git a/tests/regress.c b/tests/regress.c index 3406903ab..3b83ad151 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -3668,8 +3668,9 @@ static void InitFwdRemoteHarness(ChannelOpenHarness* harness) /* Set up a client that asked for one remote forward, then hand it a * forwarded-tcpip open naming openAddr:openPort. The request the setup sends * is dropped from the output so the open's response starts at offset 0. */ -static void RunForwardedTcpipMatchTest(const char* bindAddr, word32 bindPort, - const char* openAddr, word32 openPort, int expectAccept) +static void RunForwardedTcpipMatchModeTest(byte match, const char* bindAddr, + word32 bindPort, const char* openAddr, word32 openPort, + int expectAccept) { ChannelOpenHarness harness; byte extra[128]; @@ -3686,6 +3687,7 @@ static void RunForwardedTcpipMatchTest(const char* bindAddr, word32 bindPort, InitChannelOpenHarnessClient(&harness, in, inSz); AssertIntEQ(wolfSSH_CTX_SetFwdCb(harness.ctx, AcceptFwdCb, NULL), WS_SUCCESS); + AssertIntEQ(wolfSSH_SetFwdRemoteMatch(harness.ssh, match), WS_SUCCESS); AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, bindAddr, bindPort, 1), WS_SUCCESS); @@ -3709,6 +3711,13 @@ static void RunForwardedTcpipMatchTest(const char* bindAddr, word32 bindPort, FreeChannelOpenHarness(&harness); } +static void RunForwardedTcpipMatchTest(const char* bindAddr, word32 bindPort, + const char* openAddr, word32 openPort, int expectAccept) +{ + RunForwardedTcpipMatchModeTest(WOLFSSH_FWD_MATCH_STRICT, bindAddr, + bindPort, openAddr, openPort, expectAccept); +} + static void TestForwardedTcpipRegisteredIsAccepted(void) { /* The open names the forward the client registered, so it goes through. */ @@ -4700,6 +4709,46 @@ static void TestForwardedTcpipCancelAnsweredDuringResetupKeepsForward(void) FreeChannelOpenHarness(&harness); } +/* A peer that canonicalises the bind it echoes back has every open refused + * under the default, so the port it was asked for can be made the whole + * test. */ +static void TestFwdRemoteMatchPortIgnoresBindAddr(void) +{ + RunForwardedTcpipMatchModeTest(WOLFSSH_FWD_MATCH_STRICT, "localhost", 8080, + "127.0.0.1", 8080, 0); + RunForwardedTcpipMatchModeTest(WOLFSSH_FWD_MATCH_PORT, "localhost", 8080, + "127.0.0.1", 8080, 1); + + /* Relaxing the address does not relax the port. */ + RunForwardedTcpipMatchModeTest(WOLFSSH_FWD_MATCH_PORT, "localhost", 8080, + "127.0.0.1", 9999, 0); +} + +/* Off is what wolfSSH did before the check existed: the open reaches the + * channel-open policy callback whatever it names. */ +static void TestFwdRemoteMatchOffAcceptsUnregistered(void) +{ + RunForwardedTcpipMatchModeTest(WOLFSSH_FWD_MATCH_OFF, "127.0.0.1", 8080, + "10.0.0.1", 9999, 1); +} + +static void TestFwdRemoteMatchRejectsBadSetting(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_SetFwdRemoteMatch(NULL, WOLFSSH_FWD_MATCH_OFF), + WS_BAD_ARGUMENT); + AssertIntEQ(wolfSSH_SetFwdRemoteMatch(harness.ssh, + WOLFSSH_FWD_MATCH_OFF + 1), WS_BAD_ARGUMENT); + + /* A refused setting leaves the default in place. */ + AssertIntEQ(harness.ssh->fwdRemoteMatch, WOLFSSH_FWD_MATCH_STRICT); + + FreeChannelOpenHarness(&harness); +} + /* Only the peer gives a reply slot back, so a peer that never answers would * let the queue grow for the life of the session and lengthen every match. * Refusing has to happen before the request is framed: one whose slot was @@ -10444,6 +10493,9 @@ int main(int argc, char** argv) TestForwardedTcpipRequestAfterReplyDuringSend(); TestForwardedTcpipCancelAnsweredDuringResetupKeepsForward(); TestForwardedTcpipReentrantCancelDuringSend(); + TestFwdRemoteMatchPortIgnoresBindAddr(); + TestFwdRemoteMatchOffAcceptsUnregistered(); + TestFwdRemoteMatchRejectsBadSetting(); TestFwdReplyQueueIsCapped(); TestForwardedTcpipAppRequestKeepsItsOwnReply(); TestForwardedTcpipWantWriteStillRegisters(); diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 8292e5c13..065fcb5dd 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1310,6 +1310,8 @@ struct WOLFSSH { WOLFSSH_FWD_REPLY* fwdReplyTail; word32 fwdReplyCount; /* slots queued, kept off the walk it bounds */ byte fwdRemoteTracked; /* wolfSSH_FwdRemoteSetup() was used */ + byte fwdRemoteMatch; /* WOLFSSH_FWD_MATCH_*, how strictly an inbound + * forwarded-tcpip must name a registration */ #endif /* WOLFSSH_FWD */ #ifdef WOLFSSH_TERM WS_CallbackTerminalSize termResizeCb; diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index 6b83c169e..91e1b8d74 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -308,6 +308,7 @@ DEPRECATED WOLFSSH_API int wolfSSH_ChannelGetFwdFd( * equal the address the peer reports. Register the spelling the peer will echo * back, or a wildcard: a peer that canonicalises the bind, answering an open * for "127.0.0.1" against a registered "localhost", has those opens refused. + * wolfSSH_SetFwdRemoteMatch() relaxes this for peers that need it. * * One bindAddr:bindPort is one registration however often it is registered, * since it is one listener on the peer, so one cancel undoes it. That covers a @@ -339,6 +340,26 @@ WOLFSSH_API int wolfSSH_FwdRemoteSetup(WOLFSSH* ssh, const char* bindAddr, WOLFSSH_API int wolfSSH_FwdRemoteCancel(WOLFSSH* ssh, const char* bindAddr, word32 bindPort, int wantReply); +/* How strictly an inbound "forwarded-tcpip" open must name a registration + * made with wolfSSH_FwdRemoteSetup(). */ +enum WS_FwdRemoteMatch { + WOLFSSH_FWD_MATCH_STRICT = 0, /* bind and port, the default */ + WOLFSSH_FWD_MATCH_PORT = 1, /* port alone, the bind is not compared */ + WOLFSSH_FWD_MATCH_OFF = 2 /* accept any open, matching nothing */ +}; + +/* Relax the check wolfSSH_FwdRemoteSetup() turns on for this session. Set it + * before the first setup, since opens are matched from that point. + * + * STRICT is the default and is what RFC 4254 7.2 asks for. PORT is for a peer + * that rewrites the bind address it echoes back but keeps the port, which + * STRICT refuses every open from. OFF accepts any "forwarded-tcpip" open, as + * wolfSSH did before this check existed, leaving the channel-open policy + * callback as the only thing standing between the peer and a new channel. + * + * Returns WS_BAD_ARGUMENT for a NULL session or an unknown setting. */ +WOLFSSH_API int wolfSSH_SetFwdRemoteMatch(WOLFSSH* ssh, byte match); + WOLFSSH_API int wolfSSH_ChannelFree(WOLFSSH_CHANNEL* channel); WOLFSSH_API int wolfSSH_ChannelGetId(WOLFSSH_CHANNEL* channel, word32* id, byte peer); From 2bca99a116732cd638a3892cb9b6b497ca2ff35c Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 18 Aug 2026 16:31:00 -0700 Subject: [PATCH 08/12] Keep a rekey out of the mem-IO test harnesses Filling the reply queue sends enough to cross a lowered highwater mark. The rekey that fires sends a KEXINIT the 256-byte mem buffer cannot hold. - Disable the highwater; a bigger buffer only moves the cliff. - Both harnesses, since the buffer is the same size on each. --- tests/regress.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/regress.c b/tests/regress.c index 3b83ad151..4703f9e5a 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -418,6 +418,9 @@ static void InitChannelOpenHarnessClient(ChannelOpenHarness* harness, MemIoInit(&harness->io, in, inSz, harness->out, sizeof(harness->out)); wolfSSH_SetIOReadCtx(harness->ssh, &harness->io); wolfSSH_SetIOWriteCtx(harness->ssh, &harness->io); + /* A rekey cannot fit the 256-byte mem buffer, so keep a build-time + * DEFAULT_HIGHWATER_MARK from firing one mid-test. */ + AssertIntEQ(wolfSSH_SetHighwater(harness->ssh, 0), WS_SUCCESS); harness->ssh->connectState = CONNECT_SERVER_USERAUTH_ACCEPT_DONE; } #endif /* WOLFSSH_FWD && !NO_WOLFSSH_CLIENT */ @@ -444,6 +447,9 @@ static void InitChannelOpenHarness(ChannelOpenHarness* harness, MemIoInit(&harness->io, in, inSz, harness->out, sizeof(harness->out)); wolfSSH_SetIOReadCtx(harness->ssh, &harness->io); wolfSSH_SetIOWriteCtx(harness->ssh, &harness->io); + /* A rekey cannot fit the 256-byte mem buffer, so keep a build-time + * DEFAULT_HIGHWATER_MARK from firing one mid-test. */ + AssertIntEQ(wolfSSH_SetHighwater(harness->ssh, 0), WS_SUCCESS); harness->ssh->acceptState = ACCEPT_SERVER_USERAUTH_SENT; } From 715554c187ee468714686f7e0bb433420833a180 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 26 Aug 2026 09:50:30 -0700 Subject: [PATCH 09/12] Commit forward state before the post-send callback The forward a "tcpip-forward" or "cancel-tcpip-forward" establishes was committed after SendGlobalRequestFwd() returned, which is after the post-send highwater callback had run. A request that callback sends goes out behind this one but committed ahead of it, so the earlier request had the last word and the client ended up on the opposite side of the forward from the peer. A first setup whose callback cancels it left the forward registered with no listener on the peer, and a cancel whose callback re-establishes the forward unlinked it, refusing every open for a listener the peer holds. - Split the post-send highwater check off wolfSSH_SendPacket() as SendPacketFlush(), for a sender with state to commit first. - SendGlobalRequestFwd() takes the pending forward and settles it inside the send window, then runs the check. - Commit order is send order now, so the last request sent governs, whichever call made it. - FwdPendingCommit() still re-resolves the entry: the IO send callback can reenter mid-flush, which no ordering fixes. - Tests cover a reentrant cancel of a first setup, a reentrant setup during a cancel, and an inbound forwarded-tcpip open pumped from the callback. --- src/internal.c | 60 ++++++++++++++++++----- src/ssh.c | 30 +++--------- tests/regress.c | 120 +++++++++++++++++++++++++++++++++++++++++++++ wolfssh/internal.h | 6 ++- wolfssh/ssh.h | 4 +- 5 files changed, 181 insertions(+), 39 deletions(-) diff --git a/src/internal.c b/src/internal.c index 6ac443ae3..f2924865d 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4919,10 +4919,15 @@ static int GetInputLine(WOLFSSH* ssh, byte** pEol) } -/* returns WS_SUCCESS on success */ -int wolfSSH_SendPacket(WOLFSSH* ssh) +/* Push everything framed at the peer, stopping short of the post-send + * highwater check. A sender with state to commit runs that check itself, after + * committing: the callback it fires can reenter the library and send a request + * of its own, which goes out behind this one and has to commit behind it too. + * + * returns WS_SUCCESS on success */ +static int SendPacketFlush(WOLFSSH* ssh) { - WLOG(WS_LOG_DEBUG, "Entering wolfSSH_SendPacket()"); + WLOG(WS_LOG_DEBUG, "Entering SendPacketFlush()"); if (ssh->ctx->ioSendCb == NULL) { WLOG(WS_LOG_DEBUG, "Your IO Send callback is null, please set"); @@ -4999,7 +5004,23 @@ int wolfSSH_SendPacket(WOLFSSH* ssh) WLOG(WS_LOG_DEBUG, "SB: Shrinking output buffer"); ShrinkBuffer(&ssh->outputBuffer, 0); - return HighwaterCheck(ssh, WOLFSSH_HWSIDE_TRANSMIT); + return WS_SUCCESS; +} + + +/* returns WS_SUCCESS on success */ +int wolfSSH_SendPacket(WOLFSSH* ssh) +{ + int ret; + + ret = SendPacketFlush(ssh); + + /* Only a complete flush reaches the check, as the peer has the whole + * packet by then. */ + if (ret == WS_SUCCESS) + ret = HighwaterCheck(ssh, WOLFSSH_HWSIDE_TRANSMIT); + + return ret; } @@ -17737,23 +17758,24 @@ int SendGlobalRequest(WOLFSSH* ssh, #ifdef WOLFSSH_FWD /* Send a "tcpip-forward" or "cancel-tcpip-forward" global request. The bind * address and port follow the want-reply boolean, an ordering the generic - * SendGlobalRequest() framing cannot express. RFC 4254 7.1. */ + * SendGlobalRequest() framing cannot express. RFC 4254 7.1. + * + * What FwdRemotePrepare() built for the request is settled here rather than by + * the caller, since it has to happen inside the send window. */ int SendGlobalRequestFwd(WOLFSSH* ssh, const char* bindAddr, word32 bindPort, int isCancel, int wantReply, - int* sent) + WOLFSSH_FWD_PENDING* pend) { byte* output; word32 idx = 0; word32 reqNameSz; word32 bindAddrSz; const char* reqName; + int sent = 0; int ret = WS_SUCCESS; WLOG(WS_LOG_DEBUG, "Entering SendGlobalRequestFwd()"); - if (sent != NULL) - *sent = 0; - if (ssh == NULL || bindAddr == NULL) ret = WS_BAD_ARGUMENT; @@ -17791,12 +17813,24 @@ int SendGlobalRequestFwd(WOLFSSH* ssh, if (ret == WS_SUCCESS) { word32 flushes = ssh->txFlushCount; - ret = wolfSSH_SendPacket(ssh); - - if (sent != NULL) - *sent = SendPacketDelivered(ssh, flushes, ret); + ret = SendPacketFlush(ssh); + sent = SendPacketDelivered(ssh, flushes, ret); } + /* Whether the peer will bind the listener, not whether this call + * succeeded: a request still framed and waiting to flush reaches it. Only + * what never left unwinds. */ + if (sent) + FwdPendingCommit(ssh, pend); + else + FwdPendingDiscard(ssh, pend); + + /* Held back until the commit is done. The callback can reenter and send a + * request of its own, which goes out behind this one, and the last request + * sent is the one that governs. */ + if (ret == WS_SUCCESS) + ret = HighwaterCheck(ssh, WOLFSSH_HWSIDE_TRANSMIT); + WLOG(WS_LOG_DEBUG, "Leaving SendGlobalRequestFwd(), ret = %d", ret); return ret; diff --git a/src/ssh.c b/src/ssh.c index f117a2ef5..2e5019f23 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -3991,21 +3991,11 @@ int wolfSSH_FwdRemoteSetup(WOLFSSH* ssh, const char* bindAddr, if (ret == WS_SUCCESS) ret = FwdRemotePrepare(ssh, bindAddr, bindPort, wantReply, 0, &pend); - if (ret == WS_SUCCESS) { - int sent = 0; - + /* The send settles pend: what reached the peer registers, even when the + * post-send highwater callback reports an error afterwards. */ + if (ret == WS_SUCCESS) ret = SendGlobalRequestFwd(ssh, bindAddr, bindPort, 0, wantReply, - &sent); - - /* Whether the peer will bind the listener, not whether this call - * succeeded: a request still framed and waiting to flush reaches it, - * and so does one the post-send highwater callback reports an error - * for. Only what never left unwinds. */ - if (sent) - FwdPendingCommit(ssh, &pend); - else - FwdPendingDiscard(ssh, &pend); - } + &pend); WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_FwdRemoteSetup(), ret = %d", ret); return ret; @@ -4045,17 +4035,9 @@ int wolfSSH_FwdRemoteCancel(WOLFSSH* ssh, const char* bindAddr, if (ret == WS_SUCCESS) ret = FwdRemotePrepare(ssh, bindAddr, bindPort, wantReply, 1, &pend); - if (ret == WS_SUCCESS) { - int sent = 0; - + if (ret == WS_SUCCESS) ret = SendGlobalRequestFwd(ssh, bindAddr, bindPort, 1, wantReply, - &sent); - - if (sent) - FwdPendingCommit(ssh, &pend); - else - FwdPendingDiscard(ssh, &pend); - } + &pend); WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_FwdRemoteCancel(), ret = %d", ret); return ret; diff --git a/tests/regress.c b/tests/regress.c index 4703f9e5a..a0f9fa8b1 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -4718,6 +4718,123 @@ static void TestForwardedTcpipCancelAnsweredDuringResetupKeepsForward(void) /* A peer that canonicalises the bind it echoes back has every open refused * under the default, so the port it was asked for can be made the whole * test. */ +static int CancelFirstSetupHighwaterCb(byte side, void* ctx) +{ + WOLFSSH* ssh = (WOLFSSH*)ctx; + + WOLFSSH_UNUSED(side); + + if (ssh != NULL) + wolfSSH_FwdRemoteCancel(ssh, "127.0.0.1", 8080, 0); + + return WS_SUCCESS; +} + +/* The callback cancels the very forward the request in flight is establishing, + * and with no earlier registration to find it has nothing to work from but + * what this request left. The cancel went out behind the setup, so the peer + * holds no listener and neither may this side. */ +static void TestForwardedTcpipReentrantCancelOfFirstSetup(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + wolfSSH_SetHighwaterCb(harness.ctx, 1, CancelFirstSetupHighwaterCb); + wolfSSH_SetHighwaterCtx(harness.ssh, harness.ssh); + /* Cross the mark on the request's own send. */ + harness.ssh->highwaterMark = 1; + harness.ssh->txCount = 1; + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 0), + WS_SUCCESS); + + AssertIntEQ(FwdRemoteCount(harness.ssh), 0); + + harness.io.outSz = 0; + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + +static int SetupDuringCancelHighwaterCb(byte side, void* ctx) +{ + WOLFSSH* ssh = (WOLFSSH*)ctx; + + WOLFSSH_UNUSED(side); + + if (ssh != NULL) + wolfSSH_FwdRemoteSetup(ssh, "127.0.0.1", 8080, 0); + + return WS_SUCCESS; +} + +/* The same window the other way around: the callback re-establishes the + * forward the cancel in flight is taking down. The setup went out behind the + * cancel, so the peer binds a listener and this side keeps matching for it. */ +static void TestForwardedTcpipReentrantSetupDuringCancel(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 0), + WS_SUCCESS); + + wolfSSH_SetHighwaterCb(harness.ctx, 1, SetupDuringCancelHighwaterCb); + wolfSSH_SetHighwaterCtx(harness.ssh, harness.ssh); + harness.ssh->highwaterMark = 1; + harness.ssh->txCount = 1; + + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 0), + WS_SUCCESS); + + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + + harness.io.outSz = 0; + AssertForwardedOpenRefused(&harness, "10.0.0.1", 9999); + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + +static int InboundOpenDuringSendHighwaterCb(byte side, void* ctx) +{ + ChannelOpenHarness* harness = (ChannelOpenHarness*)ctx; + + WOLFSSH_UNUSED(side); + + /* The request is on the wire, so matching has to be live already: this is + * the first setup, and until it registers nothing is tracked and every + * open goes unchecked. */ + AssertIntEQ(harness->ssh->fwdRemoteTracked, 1); + AssertForwardedOpenRefused(harness, "10.0.0.1", 9999); + + return WS_SUCCESS; +} + +/* A callback that pumps the session sees the forwards the request in flight + * established, not the ones it found on the way in. */ +static void TestForwardedTcpipInboundOpenDuringSend(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + wolfSSH_SetHighwaterCb(harness.ctx, 1, InboundOpenDuringSendHighwaterCb); + wolfSSH_SetHighwaterCtx(harness.ssh, &harness); + harness.ssh->highwaterMark = 1; + harness.ssh->txCount = 1; + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 0), + WS_SUCCESS); + + harness.io.outSz = 0; + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + static void TestFwdRemoteMatchPortIgnoresBindAddr(void) { RunForwardedTcpipMatchModeTest(WOLFSSH_FWD_MATCH_STRICT, "localhost", 8080, @@ -10499,6 +10616,9 @@ int main(int argc, char** argv) TestForwardedTcpipRequestAfterReplyDuringSend(); TestForwardedTcpipCancelAnsweredDuringResetupKeepsForward(); TestForwardedTcpipReentrantCancelDuringSend(); + TestForwardedTcpipReentrantCancelOfFirstSetup(); + TestForwardedTcpipReentrantSetupDuringCancel(); + TestForwardedTcpipInboundOpenDuringSend(); TestFwdRemoteMatchPortIgnoresBindAddr(); TestFwdRemoteMatchOffAcceptsUnregistered(); TestFwdRemoteMatchRejectsBadSetting(); diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 065fcb5dd..2201d4caa 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1637,9 +1637,13 @@ WOLFSSH_LOCAL int SendGlobalRequestFwdSuccess(WOLFSSH * ssh, int success, WOLFSSH_LOCAL int SendGlobalRequest(WOLFSSH * ssh, const unsigned char * data, word32 dataSz, int reply, int* sent); #ifdef WOLFSSH_FWD +/* Sends the request and settles pend with it: committed once the request is on + * its way to the peer, discarded when it never left. Both happen before the + * post-send highwater callback runs, so a request that callback sends commits + * behind this one. */ WOLFSSH_LOCAL int SendGlobalRequestFwd(WOLFSSH* ssh, const char* bindAddr, word32 bindPort, int isCancel, int wantReply, - int* sent); + WOLFSSH_FWD_PENDING* pend); /* On success pend holds what to commit once the request reaches the wire; on * error it is zeroed, so there is nothing to commit or give back. */ WOLFSSH_LOCAL int FwdRemotePrepare(WOLFSSH* ssh, const char* bindAddr, diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index 91e1b8d74..ec1737471 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -319,7 +319,9 @@ DEPRECATED WOLFSSH_API int wolfSSH_ChannelGetFwdFd( * Several requests can name one bind at once, and the last one sent governs. * Registering again while a cancel is outstanding brings the forward back as * the request goes out, and no answer to that older cancel takes it away - * again, whatever order the peer answers in. + * again, whatever order the peer answers in. A request one of these calls + * makes from a callback it fires is no different: it goes out behind this + * one, so it is the one that governs. * * WS_WANT_WRITE means the request is framed and goes out on the next flush, * with the forward registered. So does an error reported after the request From a39595aa3fc27382041c193154de57a531ed914b Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 28 Aug 2026 18:49:51 -0700 Subject: [PATCH 10/12] Test a reply landing before the request commits The mid-send window closed to the highwater callback when the commit moved ahead of it, and the tests written for that window went with it: they now answer a settled slot and reach the ordinary path. The IO send callback is where a peer's answer can still land while the sender owns its slot, so drive the reply from there. - FwdReplyFromSendCb() answers the request from inside the flush, with a short-write and a fail-next mode for a send that loses the rest of the request after the answer is in. - Tests cover a parked success, a parked refusal, a parked port-0 reply, a failed send giving the answered slot back, and an answer settling an earlier request while a cancel naming the same bind is mid-send. - The highwater tests stay, for the committed path they now cover; their comments said they held an uncommitted slot. --- tests/regress.c | 237 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 224 insertions(+), 13 deletions(-) diff --git a/tests/regress.c b/tests/regress.c index a0f9fa8b1..548a0fe9e 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -4501,8 +4501,10 @@ static int ReplyDuringSendHighwaterCb(byte side, void* ctx) WOLFSSH_UNUSED(side); if (harness != NULL) { - /* The request is on the wire before this runs, so a peer answering it - * at once answers the slot the send is still holding. */ + /* The request is on the wire and its slot committed before this runs, + * so a peer answering it at once answers a settled slot. The window + * where the sender still owns it is the IO send callback's, covered + * by the FromSend tests below. */ replySz = WrapPacket(replyDuringSendMsgId, replyDuringSendData, replyDuringSendDataSz, reply, sizeof(reply)); FeedOnePacket(harness, reply, replySz); @@ -4535,9 +4537,9 @@ static void InitReplyDuringSendHarness(ChannelOpenHarness* harness, byte msgId, harness->ssh->txCount = 1; } -/* A refusal in that window says the peer bound nothing. The slot it answers - * does not name the forward yet, so the verdict waits for the commit rather - * than being dropped, which would leave a refused forward matching. */ +/* A refusal from the post-send callback says the peer bound nothing. The + * forward is registered by then, so the verdict has to unwind it rather than + * be dropped, which would leave a refused forward matching. */ static void TestForwardedTcpipRefusalDuringSendDropsForward(void) { ChannelOpenHarness harness; @@ -4557,9 +4559,9 @@ static void TestForwardedTcpipRefusalDuringSendDropsForward(void) FreeChannelOpenHarness(&harness); } -/* A port-0 request answered in that window: the port the answer named has to - * reach the registration, or it stays unmatchable for the life of the session - * with nothing left to resolve it. */ +/* A port-0 request answered from the post-send callback: the port the answer + * named has to reach the registration, or it stays unmatchable for the life of + * the session with nothing left to resolve it. */ static void TestForwardedTcpipPortZeroReplyDuringSendBinds(void) { ChannelOpenHarness harness; @@ -4614,9 +4616,8 @@ static void TestForwardedTcpipRequestAfterReplyDuringSend(void) FreeChannelOpenHarness(&harness); } -/* The slot takes its place in the queue before the send, so a reply the send - * takes in can pop and free it before the request commits. Committing has to - * find the slot gone rather than write through it. */ +/* The answer pops the slot and frees it. The commit that follows the send has + * to find the queue empty rather than write through a slot that is gone. */ static void TestForwardedTcpipReplyDuringSendTakesSlot(void) { ChannelOpenHarness harness; @@ -4626,8 +4627,8 @@ static void TestForwardedTcpipReplyDuringSendTakesSlot(void) AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), WS_SUCCESS); - /* The answer was parked on the slot and applied once the forward it names - * was known, so the queue owes nothing further. */ + /* The answer settled the forward and took its slot with it, so the queue + * owes nothing further. */ AssertTrue(harness.ssh->fwdReplyHead == NULL); AssertTrue(harness.ssh->fwdReplyTail == NULL); @@ -4642,6 +4643,211 @@ static void TestForwardedTcpipReplyDuringSendTakesSlot(void) FreeChannelOpenHarness(&harness); } +/* The highwater callback runs after the request has committed, so the answers + * it pumps in find a settled slot. The IO send callback is the one place a + * peer's answer can still land while the sender owns its slot, and no + * ordering closes that window: the callback reenters from inside the flush. + * These drive the reply from there. */ +typedef struct { + ChannelOpenHarness* harness; + byte msgId; + const byte* data; + word32 dataSz; + word32 chunkSz; /* short-write size, 0 to write the whole buffer */ + byte failNext; /* fail the send that follows the answer */ + int calls; +} FwdReplyFromSend; + +static FwdReplyFromSend fwdReplyFromSend; + +static int FwdReplyFromSendCb(WOLFSSH* ssh, void* buf, word32 sz, void* ctx) +{ + FwdReplyFromSend* state = &fwdReplyFromSend; + byte reply[64]; + word32 replySz; + int ret; + + state->calls++; + + /* The answer is in, so the rest of the request never makes it out. */ + if (state->calls > 1 && state->failNext) + return WS_CBIO_ERR_GENERAL; + + if (state->chunkSz != 0 && sz > state->chunkSz) + sz = state->chunkSz; + + ret = MemSend(ssh, buf, sz, ctx); + + /* Enough of the request is on the wire for the peer to answer it, and the + * send it answers has yet to commit. */ + if (state->calls == 1 && ret > 0 && state->harness != NULL) { + replySz = WrapPacket(state->msgId, state->data, state->dataSz, reply, + sizeof(reply)); + FeedOnePacket(state->harness, reply, replySz); + } + + return ret; +} + +static void InitReplyFromSendHarness(ChannelOpenHarness* harness, byte msgId, + const byte* data, word32 dataSz) +{ + InitFwdRemoteHarness(harness); + wolfSSH_SetIOSend(harness->ctx, FwdReplyFromSendCb); + + WMEMSET(&fwdReplyFromSend, 0, sizeof(fwdReplyFromSend)); + fwdReplyFromSend.harness = harness; + fwdReplyFromSend.msgId = msgId; + fwdReplyFromSend.data = data; + fwdReplyFromSend.dataSz = dataSz; +} + +/* The answer arrives with the slot still uncommitted, so its verdict parks + * there for the commit to apply. The forward stands confirmed all the same. */ +static void TestForwardedTcpipReplyFromSendCommits(void) +{ + ChannelOpenHarness harness; + + InitReplyFromSendHarness(&harness, MSGID_REQUEST_SUCCESS, NULL, 0); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + /* The parked verdict was applied and the slot freed with it. */ + AssertTrue(harness.ssh->fwdReplyHead == NULL); + AssertTrue(harness.ssh->fwdReplyTail == NULL); + AssertIntEQ(harness.ssh->fwdReplyCount, 0); + + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + AssertIntEQ(harness.ssh->fwdRemoteList->confirmed, 1); + + harness.io.outSz = 0; + AssertForwardedOpenRefused(&harness, "10.0.0.1", 9999); + AssertForwardedOpenAccepted(&harness, "127.0.0.1", 8080, 1); + + FreeChannelOpenHarness(&harness); +} + +/* A refusal parked the same way. The forward it names is registered by the + * commit and unwound by the verdict the commit then applies, so nothing is + * left matching a listener the peer never bound. */ +static void TestForwardedTcpipRefusalFromSendDropsForward(void) +{ + ChannelOpenHarness harness; + + InitReplyFromSendHarness(&harness, MSGID_REQUEST_FAILURE, NULL, 0); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + AssertIntEQ(FwdRemoteCount(harness.ssh), 0); + AssertTrue(harness.ssh->fwdReplyHead == NULL); + + /* Matching is on from the first registration, so the open is refused. */ + harness.io.outSz = 0; + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + +/* A port-0 request answered in that window. The port the answer named has to + * survive being parked, or the registration stays unmatchable for the life of + * the session with nothing left to resolve it. */ +static void TestForwardedTcpipPortZeroReplyFromSendBinds(void) +{ + ChannelOpenHarness harness; + byte port[UINT32_SZ]; + word32 portSz; + + portSz = AppendUint32(port, sizeof(port), 0, REGRESS_FWD_ALLOC_PORT); + InitReplyFromSendHarness(&harness, MSGID_REQUEST_SUCCESS, port, portSz); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 0, 1), + WS_SUCCESS); + + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + AssertIntEQ(harness.ssh->fwdRemoteList->portPending, 0); + AssertIntEQ(harness.ssh->fwdRemoteList->bindPort, REGRESS_FWD_ALLOC_PORT); + + harness.io.outSz = 0; + AssertForwardedOpenAccepted(&harness, "127.0.0.1", REGRESS_FWD_ALLOC_PORT, + 1); + + FreeChannelOpenHarness(&harness); +} + +/* The peer answered what it had seen of the request, then the rest of the + * send failed. The request never reached the peer whole, so the parked + * verdict settles nothing and goes back with the slot. */ +static void TestForwardedTcpipFailedSendDiscardsAnsweredSlot(void) +{ + ChannelOpenHarness harness; + + InitReplyFromSendHarness(&harness, MSGID_REQUEST_SUCCESS, NULL, 0); + /* Short-write the request so a second send is owed, and lose that one. */ + fwdReplyFromSend.chunkSz = 8; + fwdReplyFromSend.failNext = 1; + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SOCKET_ERROR_E); + + /* The session is as it was: nothing registered, nothing owed. */ + AssertTrue(harness.ssh->fwdRemoteList == NULL); + AssertTrue(harness.ssh->fwdReplyHead == NULL); + AssertTrue(harness.ssh->fwdReplyTail == NULL); + AssertIntEQ(harness.ssh->fwdReplyCount, 0); + AssertIntEQ(harness.ssh->fwdRemoteTracked, 0); + + FreeChannelOpenHarness(&harness); +} + +/* An answer settling an earlier request has to see the one still in its send + * window, which names its forward by the bind it will register rather than by + * the entry. Here a setup is refused while the cancel that follows it is on + * its way out: the cancel is still owed an answer, so the forward it names + * stays put for that answer to settle. */ +static void TestForwardedTcpipRefusalFromSendSeesSendingCancel(void) +{ + ChannelOpenHarness harness; + + InitReplyFromSendHarness(&harness, MSGID_REQUEST_FAILURE, NULL, 0); + /* The setup goes out on the plain transport; the cancel's send is the one + * that answers it. */ + fwdReplyFromSend.harness = NULL; + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + AssertIntEQ(harness.ssh->fwdRemoteList->confirmed, 0); + + fwdReplyFromSend.harness = &harness; + fwdReplyFromSend.calls = 0; + harness.io.outSz = 0; + + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + /* The setup's refusal found the cancel mid-send naming the same bind, so + * it left the forward for that cancel to answer for. */ + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + AssertNotNull(harness.ssh->fwdReplyHead); + AssertIntEQ(harness.ssh->fwdReplyHead->isCancel, 1); + AssertTrue(harness.ssh->fwdReplyHead->entry == harness.ssh->fwdRemoteList); + + /* A cancel stops matching as it goes out, before the peer answers. */ + harness.io.outSz = 0; + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + /* The peer took the listener down, and nothing is left to hold the + * registration. */ + harness.io.outSz = 0; + FeedRequestSuccess(&harness); + AssertIntEQ(FwdRemoteCount(harness.ssh), 0); + AssertTrue(harness.ssh->fwdReplyHead == NULL); + + FreeChannelOpenHarness(&harness); +} + static int CancelAnsweredDuringSendHighwaterCb(byte side, void* ctx) { ChannelOpenHarness* harness = (ChannelOpenHarness*)ctx; @@ -10611,6 +10817,11 @@ int main(int argc, char** argv) TestForwardedTcpipReentrantSetupDuringSend(); TestForwardedTcpipReentrantRequestKeepsSendOrder(); TestForwardedTcpipReplyDuringSendTakesSlot(); + TestForwardedTcpipReplyFromSendCommits(); + TestForwardedTcpipRefusalFromSendDropsForward(); + TestForwardedTcpipPortZeroReplyFromSendBinds(); + TestForwardedTcpipFailedSendDiscardsAnsweredSlot(); + TestForwardedTcpipRefusalFromSendSeesSendingCancel(); TestForwardedTcpipRefusalDuringSendDropsForward(); TestForwardedTcpipPortZeroReplyDuringSendBinds(); TestForwardedTcpipRequestAfterReplyDuringSend(); From f66d6b7a9a53202daa5e5322718eb4aca7014f4c Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 31 Aug 2026 11:14:17 -0700 Subject: [PATCH 11/12] Drop a forward the peer refused twice A cancel refusal leaves the forward standing, since the peer keeps a listener it would not drop. There is no listener to keep when the setup was refused too, so FwdRemoteSettle() unwinds a registration nothing establishes and nothing is still owed an answer on, rather than leaving it unconfirmed and unmatchable until the session ends. - A cancel refusal unlinks an unconfirmed forward with no setup queued - Test refuses a want-reply setup and the cancel behind it, and checks the registration is gone --- src/internal.c | 6 +++++- tests/regress.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index f2924865d..72aa63007 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4097,7 +4097,11 @@ static void FwdRemoteSettle(WOLFSSH* ssh, WOLFSSH_FWD_REMOTE* entry, if (isCancel) { if (!success) { /* The peer kept the listener, so the forward stands and matching - * resumes unless a later cancel is outstanding. */ + * resumes unless a later cancel is outstanding. An unconfirmed + * forward has no listener to keep, though: its setup was refused + * too, and with none still queued nothing will ever bind it. */ + if (!entry->confirmed && !FwdReplyHasSetup(ssh, entry)) + FwdRemoteUnlink(ssh, ssh->ctx->heap, entry); return; } diff --git a/tests/regress.c b/tests/regress.c index 548a0fe9e..c3fd4e0d5 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -4051,6 +4051,36 @@ static void TestForwardedTcpipCancelBeforeSetupReply(void) FreeChannelOpenHarness(&harness); } +/* The same overlap with both requests refused. The setup bound no listener, + * so the cancel's refusal is only the peer saying it has none to drop, and + * nothing is left to hold the registration open. */ +static void TestForwardedTcpipSetupAndCancelBothRefusedDrops(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + + /* The setup is refused, but the queued cancel still names the forward, so + * it is held for that answer. */ + FeedRequestFailure(&harness); + AssertNotNull(harness.ssh->fwdRemoteList); + + /* The cancel is refused too. Nothing establishes the forward and nothing + * is owed an answer on it, so it goes rather than sitting unmatchable + * until the session ends. */ + FeedRequestFailure(&harness); + AssertNull(harness.ssh->fwdRemoteList); + + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + /* The same overlap, but with a second forward outstanding behind it. The * cancelled forward's reply must not be spent on the one still waiting. */ static void TestForwardedTcpipCancelBeforeSetupReplyKeepsOther(void) @@ -10800,6 +10830,7 @@ int main(int argc, char** argv) TestForwardedTcpipUnmatchedCancelKeepsForward(); TestForwardedTcpipCancelBeforeSetupReply(); TestForwardedTcpipCancelBeforeSetupReplyKeepsOther(); + TestForwardedTcpipSetupAndCancelBothRefusedDrops(); TestForwardedTcpipDuplicateSetupIsOneForward(); TestForwardedTcpipDuplicateSetupRefusalKeepsForward(); TestForwardedTcpipDuplicateSetupLaterSuccessBinds(); From e304561f817d55f739adf7edd0519c7c795333c7 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 31 Aug 2026 11:14:38 -0700 Subject: [PATCH 12/12] Keep queued replies with the folded forward A port-0 reply naming a port another registration stands for merges the two, since one bind gets one registration. The requests still queued against the entry that goes named that same bind, so FwdReplyRebind() hands them to the survivor. Left naming nothing, a cancel among them settles no forward and the merged one keeps matching opens. - FwdReplyRebind() repoints queued slots before the stale entry is unlinked and FwdReplyVoid() clears them - Test folds a port-0 reply onto a forward with a cancel already queued, and checks the cancel takes the survivor down --- src/internal.c | 20 ++++++++++++++++++++ tests/regress.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/src/internal.c b/src/internal.c index 72aa63007..be4ae3c25 100644 --- a/src/internal.c +++ b/src/internal.c @@ -3986,6 +3986,21 @@ static void FwdReplyVoid(WOLFSSH* ssh, const WOLFSSH_FWD_REMOTE* entry) } +/* Hand every queued slot naming one forward over to another. A slot names the + * bind it asked about, so when two registrations turn out to be the same bind, + * the surviving one is what those requests were about. */ +static void FwdReplyRebind(WOLFSSH* ssh, const WOLFSSH_FWD_REMOTE* from, + WOLFSSH_FWD_REMOTE* to) +{ + WOLFSSH_FWD_REPLY* reply; + + for (reply = ssh->fwdReplyHead; reply != NULL; reply = reply->next) { + if (reply->entry == from) + reply->entry = to; + } +} + + static void FwdRemoteUnlink(WOLFSSH* ssh, void* heap, WOLFSSH_FWD_REMOTE* entry) { @@ -4149,6 +4164,11 @@ static void FwdRemoteSettle(WOLFSSH* ssh, WOLFSSH_FWD_REMOTE* entry, WLOG(WS_LOG_INFO, "Remote forward reply named a port already " "registered"); + /* Requests still queued on the stale entry asked about this bind, + * so they answer for the entry that stands at it now. Unlinking + * without this leaves them naming nothing, and a cancel among + * them would settle no forward. */ + FwdReplyRebind(ssh, dup, entry); FwdRemoteUnlink(ssh, ssh->ctx->heap, dup); } } diff --git a/tests/regress.c b/tests/regress.c index c3fd4e0d5..079d581d2 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -5409,6 +5409,47 @@ static void TestForwardedTcpipPortZeroReplyFoldsDuplicate(void) FreeChannelOpenHarness(&harness); } +/* A cancel queued against the entry the fold drops still names that bind, so + * the surviving registration is what it answers for. */ +static void TestForwardedTcpipPortZeroFoldSettlesQueuedCancel(void) +{ + ChannelOpenHarness harness; + byte reply[64]; + word32 replySz; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, "127.0.0.1", 0, 1), + WS_SUCCESS); + AssertIntEQ(wolfSSH_FwdRemoteCancel(harness.ssh, "127.0.0.1", 8080, 1), + WS_SUCCESS); + AssertIntEQ(FwdRemoteCount(harness.ssh), 2); + + /* The peer refused the explicit setup, but the cancel still names that + * registration, so it stands for now. */ + FeedRequestFailure(&harness); + AssertIntEQ(FwdRemoteCount(harness.ssh), 2); + + /* The port-0 request got the port the refused one asked for. One bind, + * one registration. */ + replySz = BuildRequestSuccessPortPacket(8080, reply, sizeof(reply)); + AssertIntEQ(FeedOnePacket(&harness, reply, replySz), WS_SUCCESS); + AssertIntEQ(FwdRemoteCount(harness.ssh), 1); + + /* The cancel went out after both setups, so it is the last word on the + * bind and matching stops on it alone. */ + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + /* Confirming it takes the merged registration down with it. */ + FeedRequestSuccess(&harness); + AssertIntEQ(FwdRemoteCount(harness.ssh), 0); + AssertForwardedOpenRefused(&harness, "127.0.0.1", 8080); + + FreeChannelOpenHarness(&harness); +} + static void TestForwardedTcpipRepliesPairInSendOrder(void) { ChannelOpenHarness harness; @@ -10843,6 +10884,7 @@ int main(int argc, char** argv) TestForwardedTcpipConfirmedCancelDropsEarlierSuccess(); TestForwardedTcpipConfirmedCancelThenSetupBinds(); TestForwardedTcpipPortZeroReplyFoldsDuplicate(); + TestForwardedTcpipPortZeroFoldSettlesQueuedCancel(); TestForwardedTcpipPostSendErrorStillRegisters(); TestForwardedTcpipFailedSendRegistersNothing(); TestForwardedTcpipReentrantSetupDuringSend();