diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index 571882a91..3eb149fb4 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -497,7 +497,7 @@ static WS_SOCKET_T connect_addr(const char* name, word16 port) static int wolfSSH_FwdDefaultActions(WS_FwdCbAction action, void* vCtx, - const char* name, word32 port) + const char* name, word32* port) { WS_AppCtx *appCtx = (WS_AppCtx *)vCtx; WS_FwdCbActionCtx* fwdCbCtx = (WS_FwdCbActionCtx *)appCtx->privateData; @@ -505,7 +505,7 @@ static int wolfSSH_FwdDefaultActions(WS_FwdCbAction action, void* vCtx, if (action == WOLFSSH_FWD_LOCAL_SETUP) { fwdCbCtx->hostName = WSTRDUP(name, NULL, 0); - fwdCbCtx->hostPort = port; + fwdCbCtx->hostPort = *port; fwdCbCtx->isDirect = 1; appCtx->state = APP_STATE_CONNECT; } @@ -525,9 +525,10 @@ static int wolfSSH_FwdDefaultActions(WS_FwdCbAction action, void* vCtx, else if (action == WOLFSSH_FWD_REMOTE_SETUP) { struct sockaddr_in addr; socklen_t addrSz = 0; + socklen_t boundSz = sizeof(addr); fwdCbCtx->hostName = WSTRDUP(name, NULL, 0); - fwdCbCtx->hostPort = port; + fwdCbCtx->hostPort = *port; appCtx->listenFd = socket(AF_INET, SOCK_STREAM, 0); if (appCtx->listenFd == -1) { @@ -544,7 +545,7 @@ static int wolfSSH_FwdDefaultActions(WS_FwdCbAction action, void* vCtx, addr.sin_addr.s_addr = INADDR_ANY; addr.sin_family = AF_INET; - addr.sin_port = htons((word16)port); + addr.sin_port = htons((word16)*port); addrSz = sizeof addr; } else { @@ -562,6 +563,21 @@ static int wolfSSH_FwdDefaultActions(WS_FwdCbAction action, void* vCtx, ret = listen(appCtx->listenFd, 5); } + if (ret == 0 && *port == 0) { + /* The peer requested port 0, so the OS picked the port during + * bind(). Report the actual port back to the caller. */ + WMEMSET(&addr, 0, sizeof addr); + if (getsockname(appCtx->listenFd, + (struct sockaddr*)&addr, &boundSz) == 0) { + *port = (word32)ntohs(addr.sin_port); + fwdCbCtx->hostPort = *port; + } + else { + printf("getsockname failed for forwarded port.\n"); + ret = -1; + } + } + if (ret == 0) { appCtx->state = APP_STATE_LISTEN; } @@ -593,7 +609,7 @@ static int wolfSSH_FwdDefaultActions(WS_FwdCbAction action, void* vCtx, appCtx->state = APP_STATE_INIT; } else if (action == WOLFSSH_FWD_CHANNEL_ID) { - appCtx->channelId = port; + appCtx->channelId = *port; } else ret = WS_FWD_INVALID_ACTION; diff --git a/ide/Espressif/ESP-IDF/examples/wolfssh_echoserver/main/echoserver.c b/ide/Espressif/ESP-IDF/examples/wolfssh_echoserver/main/echoserver.c index 616e21c6d..0b8c96240 100644 --- a/ide/Espressif/ESP-IDF/examples/wolfssh_echoserver/main/echoserver.c +++ b/ide/Espressif/ESP-IDF/examples/wolfssh_echoserver/main/echoserver.c @@ -495,14 +495,14 @@ static WS_SOCKET_T connect_addr(const char* name, word16 port) static int wolfSSH_FwdDefaultActions(WS_FwdCbAction action, void* vCtx, - const char* name, word32 port) + const char* name, word32* port) { WS_FwdCbActionCtx* ctx = (WS_FwdCbActionCtx*)vCtx; int ret = 0; if (action == WOLFSSH_FWD_LOCAL_SETUP) { ctx->hostName = WSTRDUP(name, NULL, 0); - ctx->hostPort = port; + ctx->hostPort = *port; ctx->isDirect = 1; ctx->state = FWD_STATE_DIRECT; } @@ -521,9 +521,10 @@ static int wolfSSH_FwdDefaultActions(WS_FwdCbAction action, void* vCtx, else if (action == WOLFSSH_FWD_REMOTE_SETUP) { struct sockaddr_in addr; socklen_t addrSz = 0; + socklen_t boundSz = sizeof(addr); ctx->hostName = WSTRDUP(name, NULL, 0); - ctx->hostPort = port; + ctx->hostPort = *port; ctx->listenFd = socket(AF_INET, SOCK_STREAM, 0); if (ctx->listenFd == -1) { @@ -540,7 +541,7 @@ static int wolfSSH_FwdDefaultActions(WS_FwdCbAction action, void* vCtx, addr.sin_addr.s_addr = INADDR_ANY; addr.sin_family = AF_INET; - addr.sin_port = htons((word16)port); + addr.sin_port = htons((word16)*port); addrSz = sizeof addr; } else { @@ -558,6 +559,21 @@ static int wolfSSH_FwdDefaultActions(WS_FwdCbAction action, void* vCtx, ret = listen(ctx->listenFd, 5); } + if (ret == 0 && *port == 0) { + /* The peer requested port 0, so the OS picked the port during + * bind(). Report the actual port back to the caller. */ + WMEMSET(&addr, 0, sizeof addr); + if (getsockname(ctx->listenFd, + (struct sockaddr*)&addr, &boundSz) == 0) { + *port = (word32)ntohs(addr.sin_port); + ctx->hostPort = *port; + } + else { + printf("getsockname failed for forwarded port.\n"); + ret = -1; + } + } + if (ret == 0) { ctx->state = FWD_STATE_LISTEN; } @@ -589,7 +605,7 @@ static int wolfSSH_FwdDefaultActions(WS_FwdCbAction action, void* vCtx, ctx->state = FWD_STATE_INIT; } else if (action == WOLFSSH_FWD_CHANNEL_ID) { - ctx->channelId = port; + ctx->channelId = *port; } else ret = WS_FWD_INVALID_ACTION; diff --git a/src/internal.c b/src/internal.c index a66226917..798bfe6fe 100644 --- a/src/internal.c +++ b/src/internal.c @@ -8782,6 +8782,7 @@ static int DoGlobalRequestFwd(WOLFSSH* ssh, int ret = WS_SUCCESS; char* bindAddr = NULL; word32 bindPort; + word32 requestedPort = 0; WLOG(WS_LOG_DEBUG, "Entering DoGlobalRequestFwd()"); @@ -8800,6 +8801,7 @@ static int DoGlobalRequestFwd(WOLFSSH* ssh, } if (ret == WS_SUCCESS) { + requestedPort = bindPort; WLOG(WS_LOG_INFO, "Requesting forwarding%s for address %s on port %u.", isCancel ? " cancel" : "", bindAddr, bindPort); } @@ -8808,7 +8810,7 @@ static int DoGlobalRequestFwd(WOLFSSH* ssh, if (ssh->ctx->fwdCb) { ret = ssh->ctx->fwdCb(isCancel ? WOLFSSH_FWD_REMOTE_CLEANUP : WOLFSSH_FWD_REMOTE_SETUP, - ssh->fwdCbCtx, bindAddr, bindPort); + ssh->fwdCbCtx, bindAddr, &bindPort); } else { WLOG(WS_LOG_WARN, "No forwarding callback set, rejecting request. " @@ -8817,6 +8819,27 @@ static int DoGlobalRequestFwd(WOLFSSH* ssh, } } + if (ret == WS_SUCCESS && !isCancel) { + /* A remote forward was set up successfully. RFC 4254 7.1 requires a + * port-0 (dynamic) request to be answered with the allocated port; if + * the callback reported none we cannot comply, so undo the setup and + * reject instead of sending a success carrying port 0. */ + if (requestedPort == 0 && bindPort == 0) { + int cleanupRet; + word32 cleanupPort = bindPort; + + WLOG(WS_LOG_WARN, "Forward callback reported no allocated port " + "for a port-0 request; rejecting."); + cleanupRet = ssh->ctx->fwdCb(WOLFSSH_FWD_REMOTE_CLEANUP, + ssh->fwdCbCtx, bindAddr, &cleanupPort); + if (cleanupRet != WS_SUCCESS) { + WLOG(WS_LOG_WARN, "Forward cleanup after rejection failed, " + "ret = %d", cleanupRet); + } + ret = WS_FWD_SETUP_E; + } + } + if (wantReply) { if (ret == WS_SUCCESS) { if (isCancel) { @@ -8830,7 +8853,7 @@ static int DoGlobalRequestFwd(WOLFSSH* ssh, ret = SendRequestSuccess(ssh, 0); } } - else if (ret == WS_UNIMPLEMENTED_E) { + else if (ret == WS_UNIMPLEMENTED_E || ret == WS_FWD_SETUP_E) { /* No reply expected; silently reject without terminating connection. */ ret = WS_SUCCESS; } @@ -8970,6 +8993,7 @@ static int DoChannelOpen(WOLFSSH* ssh, char* host = NULL; char* origin = NULL; word32 hostPort = 0, originPort = 0; + word32 channelId = 0; int isDirect = 0; #endif /* WOLFSSH_FWD */ WOLFSSH_CHANNEL* newChannel = NULL; @@ -9066,10 +9090,13 @@ static int DoChannelOpen(WOLFSSH* ssh, if (ssh->ctx->fwdCb) { ret = ssh->ctx->fwdCb(WOLFSSH_FWD_LOCAL_SETUP, - ssh->fwdCbCtx, host, hostPort); + ssh->fwdCbCtx, host, &hostPort); if (ret == WS_SUCCESS) { + /* Pass a copy so the callback cannot mutate the + * channel id and corrupt channel bookkeeping. */ + channelId = newChannel->channel; ret = ssh->ctx->fwdCb(WOLFSSH_FWD_CHANNEL_ID, - ssh->fwdCbCtx, NULL, newChannel->channel); + ssh->fwdCbCtx, NULL, &channelId); } } else { diff --git a/src/ssh.c b/src/ssh.c index d3d3e0d1e..216b8cc9c 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -2759,6 +2759,7 @@ WOLFSSH_CHANNEL* wolfSSH_ChannelFwdNewRemote(WOLFSSH* ssh, const char* origin, word32 originPort) { WOLFSSH_CHANNEL* newChannel = NULL; + word32 channelId = 0; int ret = WS_SUCCESS; WLOG(WS_LOG_DEBUG, "Entering wolfSSH_ChannelFwdNewRemote()"); @@ -2779,8 +2780,11 @@ WOLFSSH_CHANNEL* wolfSSH_ChannelFwdNewRemote(WOLFSSH* ssh, ret = SendChannelOpenForward(ssh, newChannel); if (ret == WS_SUCCESS) { if (ssh->ctx->fwdCb) { + /* Pass a copy so the callback cannot mutate the channel id and + * corrupt channel bookkeeping. */ + channelId = newChannel->channel; ret = ssh->ctx->fwdCb(WOLFSSH_FWD_CHANNEL_ID, ssh->fwdCbCtx, - NULL, newChannel->channel); + NULL, &channelId); } } diff --git a/tests/regress.c b/tests/regress.c index 639b1075a..e4110eb6e 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -1118,6 +1118,18 @@ static void AssertGlobalRequestReply(const ChannelOpenHarness* harness, } } } + +static word32 ParseGlobalRequestSuccessPort(const byte* packet, word32 packetSz) +{ + word32 port; + + AssertNotNull(packet); + AssertTrue(packetSz >= 10); + AssertIntEQ(packet[5], MSGID_REQUEST_SUCCESS); + WMEMCPY(&port, packet + 6, sizeof(port)); + + return ntohl(port); +} #endif static int RejectChannelOpenCb(WOLFSSH_CHANNEL* channel, void* ctx) @@ -1130,7 +1142,7 @@ static int RejectChannelOpenCb(WOLFSSH_CHANNEL* channel, void* ctx) #ifdef WOLFSSH_FWD static int RejectDirectTcpipSetup(WS_FwdCbAction action, void* ctx, - const char* host, word32 port) + const char* host, word32* port) { (void)ctx; (void)host; @@ -1143,7 +1155,7 @@ static int RejectDirectTcpipSetup(WS_FwdCbAction action, void* ctx, } static int AcceptFwdCb(WS_FwdCbAction action, void* ctx, - const char* host, word32 port) + const char* host, word32* port) { (void)action; (void)ctx; @@ -1152,6 +1164,35 @@ static int AcceptFwdCb(WS_FwdCbAction action, void* ctx, return WS_SUCCESS; } + +#define REGRESS_FWD_ALLOC_PORT 49152 + +static int AllocatePortFwdCb(WS_FwdCbAction action, void* ctx, + const char* host, word32* port) +{ + (void)ctx; + (void)host; + + if (action == WOLFSSH_FWD_REMOTE_SETUP && port != NULL && *port == 0) + *port = REGRESS_FWD_ALLOC_PORT; + + return WS_SUCCESS; +} + +/* Accepts the remote setup but never reports an allocated port. Records + * whether the server asks it to clean the setup back up. */ +static int NoPortFwdCb(WS_FwdCbAction action, void* ctx, + const char* host, word32* port) +{ + int* cleanupCalled = (int*)ctx; + (void)host; + (void)port; + + if (action == WOLFSSH_FWD_REMOTE_CLEANUP && cleanupCalled != NULL) + *cleanupCalled = 1; + + return WS_SUCCESS; +} #endif @@ -1523,6 +1564,56 @@ static void TestGlobalRequestFwdWithCbSendsSuccess(void) FreeChannelOpenHarness(&harness); } +static void TestGlobalRequestFwdPort0ReturnsAllocatedPort(void) +{ + ChannelOpenHarness harness; + byte in[256]; + word32 inSz; + int ret; + + /* A bind port of 0 asks the server to allocate a port. The success reply + * must carry the port the callback allocated, not the requested 0. */ + inSz = BuildGlobalRequestFwdPacket("0.0.0.0", 0, 0, 1, in, sizeof(in)); + InitChannelOpenHarness(&harness, in, inSz); + AssertIntEQ(wolfSSH_CTX_SetFwdCb(harness.ctx, AllocatePortFwdCb, NULL), + WS_SUCCESS); + + ret = DoReceive(harness.ssh); + + AssertIntEQ(ret, WS_SUCCESS); + AssertGlobalRequestReply(&harness, MSGID_REQUEST_SUCCESS); + AssertIntEQ(ParseGlobalRequestSuccessPort(harness.io.out, harness.io.outSz), + REGRESS_FWD_ALLOC_PORT); + + FreeChannelOpenHarness(&harness); +} + +static void TestGlobalRequestFwdPort0NoAllocSendsFailure(void) +{ + ChannelOpenHarness harness; + byte in[256]; + word32 inSz; + int ret; + int cleanupCalled = 0; + + /* The peer asked the server to choose a port (0), but the callback + * accepts without reporting one. The server must reject and tear the + * setup back down rather than reply with a non-compliant port 0. */ + inSz = BuildGlobalRequestFwdPacket("0.0.0.0", 0, 0, 1, in, sizeof(in)); + InitChannelOpenHarness(&harness, in, inSz); + AssertIntEQ(wolfSSH_CTX_SetFwdCb(harness.ctx, NoPortFwdCb, NULL), + WS_SUCCESS); + AssertIntEQ(wolfSSH_SetFwdCbCtx(harness.ssh, &cleanupCalled), WS_SUCCESS); + + ret = DoReceive(harness.ssh); + + AssertIntEQ(ret, WS_SUCCESS); + AssertGlobalRequestReply(&harness, MSGID_REQUEST_FAILURE); + AssertIntEQ(cleanupCalled, 1); + + FreeChannelOpenHarness(&harness); +} + static void TestGlobalRequestFwdCancelNoCbSendsFailure(void) { ChannelOpenHarness harness; @@ -3416,6 +3507,8 @@ int main(int argc, char** argv) TestGlobalRequestFwdNoCbSendsFailure(); TestGlobalRequestFwdNoCbNoReplyKeepsConnection(); TestGlobalRequestFwdWithCbSendsSuccess(); + TestGlobalRequestFwdPort0ReturnsAllocatedPort(); + TestGlobalRequestFwdPort0NoAllocSendsFailure(); TestGlobalRequestFwdCancelNoCbSendsFailure(); TestGlobalRequestFwdCancelWithCbSendsSuccess(); TestRequestSuccessWithPortParsesCorrectly(); diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index 32143d58f..553d15c15 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -206,7 +206,10 @@ typedef enum WS_FwdCbError { WS_FWD_PEER_E, } WS_FwdCbError; -typedef int (*WS_CallbackFwd)(WS_FwdCbAction, void*, const char*, word32); +/* The port argument is in/out: for WOLFSSH_FWD_REMOTE_SETUP a requested port + * of 0 means the callback should bind an unprivileged port and write the + * allocated port back so the server can report it to the peer. */ +typedef int (*WS_CallbackFwd)(WS_FwdCbAction, void*, const char*, word32*); typedef int (*WS_CallbackFwdIO)(WS_FwdIoCbAction, void*, word32, void*);