From eca6e84847cc299333daa6d1e4538177f2d3d89a Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 14 Aug 2026 15:11:47 -0700 Subject: [PATCH 1/9] 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 fc739fce6..7e02dfe4d 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -266,8 +266,10 @@ static WS_MAYBE_UNUSED word32 BuildExtInfoSigAlgs(byte* buf, word32 bufSz, } #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; @@ -279,8 +281,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 1de7a170dd45917ba8de3562bf4c1177bfd21b30 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 24 Jul 2026 14:10:24 -0700 Subject: [PATCH 2/9] 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 0656b0130..ea9d900c5 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4332,6 +4332,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); @@ -16873,13 +16878,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; @@ -16907,9 +16933,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; @@ -16921,7 +16953,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; @@ -16932,6 +16965,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; @@ -16966,9 +17002,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 80684df06..e96d48034 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1554,7 +1554,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); } @@ -3832,7 +3832,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; @@ -3870,7 +3871,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 c4a3d8951..3d710cc09 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 */ @@ -1557,11 +1558,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 1c218af9f914c4fe6d89ed5c8f113cc54c0724ac Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 14 Aug 2026 15:14:34 -0700 Subject: [PATCH 3/9] 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 | 1529 ++++++++++++++++++++++++++++++++++++++++++-- wolfssh/internal.h | 65 +- wolfssh/ssh.h | 42 ++ 6 files changed, 2270 insertions(+), 53 deletions(-) diff --git a/src/internal.c b/src/internal.c index ea9d900c5..bbff9afed 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1816,6 +1816,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; @@ -3928,6 +3931,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 */ @@ -8067,6 +8642,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); @@ -8083,6 +8662,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); @@ -11066,6 +11649,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 e96d48034..81e19f217 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1547,6 +1547,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; @@ -1554,7 +1559,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; } @@ -3797,16 +3829,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()"); @@ -3821,6 +3849,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; @@ -3831,9 +3865,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; @@ -3841,11 +3892,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()"); @@ -3871,8 +3922,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 58f7f5719..f8244e8fa 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 7e02dfe4d..7169e95d5 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -305,6 +305,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) @@ -325,6 +326,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; } @@ -341,13 +346,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; @@ -355,6 +359,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) { @@ -375,13 +416,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 @@ -1326,20 +1360,9 @@ static void TestKexDhReplyRejectsSigNameOverrun(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; @@ -1350,8 +1373,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; @@ -1367,6 +1390,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) @@ -1471,18 +1523,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; -} - -#define REGRESS_FWD_ALLOC_PORT 49152 static int AllocatePortFwdCb(WS_FwdCbAction action, void* ctx, const char* host, word32 port) @@ -2833,6 +2873,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) { @@ -7666,6 +9072,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 3d710cc09..c3bf59a7f 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 */ @@ -1240,6 +1289,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; @@ -1568,6 +1621,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); @@ -2004,7 +2066,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 083931a57..680c072bf 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -268,6 +268,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, @@ -610,6 +647,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); WOLFSSH_API int wolfSSH_ChannelIdRead(WOLFSSH* ssh, word32 channelId, From f4e34d6c39a31ef4d8f2af973dd7c474625e1874 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 6 Aug 2026 11:53:35 -0700 Subject: [PATCH 4/9] 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 bbff9afed..7622054df 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4878,6 +4878,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; @@ -4887,6 +4895,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; @@ -17481,12 +17494,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 7169e95d5..dade1b9db 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -306,6 +306,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) @@ -330,6 +331,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; } @@ -347,6 +352,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 @@ -1959,6 +1965,45 @@ static void TestServerServiceRequestStateGated(WOLFSSH* ssh) } +/* 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; @@ -4029,6 +4074,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. */ @@ -9039,6 +9128,7 @@ int main(int argc, char** argv) TestServerUserauthBlockedBeforeKeyed(serverSsh); TestServerOnlyUserauthMsgsBlocked(serverSsh); TestServerServiceRequestStateGated(serverSsh); + TestFailedSendClearsPendingPlaintext(); TestChannelOpenCallbackRejectSendsOpenFail(); TestSecondSessionChannelRejected(); TestUsernameChangeDisconnects(); @@ -9110,6 +9200,8 @@ int main(int argc, char** argv) TestForwardedTcpipReentrantCancelDuringSend(); TestForwardedTcpipAppRequestKeepsItsOwnReply(); TestForwardedTcpipWantWriteStillRegisters(); + TestForwardedTcpipInterruptedSendStillRegisters(); + TestInterruptedSendRetriesForAnySender(); TestGlobalRequestNoReplyQueuesNothing(); TestForwardedTcpipRepliesPairInSendOrder(); TestForwardedTcpipUnusablePortReplySendsOpenFail(); From f204a8a1de4956af198533c41d09653a26428a6b Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 11 Aug 2026 14:12:14 -0700 Subject: [PATCH 5/9] 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 7622054df..41432bd65 100644 --- a/src/internal.c +++ b/src/internal.c @@ -3992,6 +3992,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, @@ -4001,7 +4024,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; } @@ -4015,7 +4038,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; } @@ -4113,9 +4136,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; @@ -4124,6 +4149,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; @@ -4248,7 +4275,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); @@ -4282,7 +4309,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); @@ -4344,8 +4373,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 dade1b9db..ce5f2f534 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -3980,6 +3980,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; @@ -9197,6 +9270,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 c3bf59a7f..6ab22ad70 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 389011a4b894ad7d050ffe57ee86990f1123e9f5 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 11 Aug 2026 14:12:14 -0700 Subject: [PATCH 6/9] 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 41432bd65..ebce3fc03 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4160,6 +4160,7 @@ static WOLFSSH_FWD_REPLY* FwdReplyNew(WOLFSSH* ssh, int isCancel, else ssh->fwdReplyTail->next = reply; ssh->fwdReplyTail = reply; + ssh->fwdReplyCount++; } return reply; @@ -4189,6 +4190,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); } @@ -4214,6 +4216,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; @@ -4309,6 +4318,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); @@ -4487,6 +4501,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 @@ -4533,6 +4548,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 ce5f2f534..0b65fd077 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -4053,6 +4053,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; @@ -9272,6 +9322,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 6ab22ad70..0dc45c70d 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. */ @@ -1298,6 +1306,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 09a56bfa9bb6b0f7b268ab47af50481c59cf8798 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 11 Aug 2026 14:12:14 -0700 Subject: [PATCH 7/9] 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 ebce3fc03..b07c0278b 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4452,6 +4452,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; @@ -4472,7 +4476,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 81e19f217..c938f2467 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -3940,6 +3940,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 0b65fd077..10942a00f 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -3021,8 +3021,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]; @@ -3039,6 +3040,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); @@ -3062,6 +3064,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. */ @@ -4053,6 +4062,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 @@ -9322,6 +9371,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 0dc45c70d..55027b977 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1308,6 +1308,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 680c072bf..86ae78248 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -279,6 +279,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 @@ -310,6 +311,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 aab884d5a5028e0625bb1695aec2eceb1a4a3b4f Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 18 Aug 2026 16:31:00 -0700 Subject: [PATCH 8/9] 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 10942a00f..3e2b34932 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -393,6 +393,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 */ @@ -419,6 +422,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 4bc5af0ee59c104ace36b6896cac4ce7a05677ab Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 26 Aug 2026 09:50:30 -0700 Subject: [PATCH 9/9] 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 b07c0278b..95fb3bcfe 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4901,10 +4901,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"); @@ -4981,7 +4986,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; } @@ -17615,23 +17636,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; @@ -17669,12 +17691,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 c938f2467..686af7eb7 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -3870,21 +3870,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; @@ -3924,17 +3914,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 3e2b34932..e5b3b25a4 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -4071,6 +4071,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, @@ -9377,6 +9494,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 55027b977..4de87cd74 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1635,9 +1635,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 86ae78248..671c6ef48 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -290,7 +290,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