diff --git a/examples/echoserver/echoserver.c b/examples/echoserver/echoserver.c index 0d0e44f2..15297ae5 100644 --- a/examples/echoserver/echoserver.c +++ b/examples/echoserver/echoserver.c @@ -529,6 +529,8 @@ 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); + word32 allocatedPort = 0; fwdCbCtx->hostName = WSTRDUP(name, NULL, 0); fwdCbCtx->hostPort = port; @@ -553,7 +555,7 @@ static int wolfSSH_FwdDefaultActions(WS_FwdCbAction action, void* vCtx, } else { printf("Not using IPv6 yet.\n"); - ret = WS_FWD_SETUP_E; + ret = -1; } } @@ -566,8 +568,35 @@ 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(). Recover it to report back to the caller. */ + WMEMSET(&addr, 0, sizeof addr); + if (getsockname(appCtx->listenFd, + (struct sockaddr*)&addr, &boundSz) == 0) { + allocatedPort = (word32)ntohs(addr.sin_port); + /* The library reads a return below WS_FWD_PORT_CHECK as a + * status, not a port, so an allocated port must be reportable. + * An unprivileged OS-chosen port always is; guard anyway. */ + if (allocatedPort < WS_FWD_PORT_CHECK) { + printf("Allocated port %u not reportable.\n", allocatedPort); + ret = -1; + } + else { + fwdCbCtx->hostPort = allocatedPort; + } + } + else { + printf("getsockname failed for forwarded port.\n"); + ret = -1; + } + } + if (ret == 0) { appCtx->state = APP_STATE_LISTEN; + /* Report any dynamically allocated port to the library through the + * return value; 0 keeps the port the peer requested. */ + ret = (int)allocatedPort; } else { if (fwdCbCtx->hostName != NULL) { 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 616e21c6..50321ac8 100644 --- a/ide/Espressif/ESP-IDF/examples/wolfssh_echoserver/main/echoserver.c +++ b/ide/Espressif/ESP-IDF/examples/wolfssh_echoserver/main/echoserver.c @@ -521,6 +521,8 @@ 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); + word32 allocatedPort = 0; ctx->hostName = WSTRDUP(name, NULL, 0); ctx->hostPort = port; @@ -545,7 +547,7 @@ static int wolfSSH_FwdDefaultActions(WS_FwdCbAction action, void* vCtx, } else { printf("Not using IPv6 yet.\n"); - ret = WS_FWD_SETUP_E; + ret = -1; } } @@ -558,8 +560,35 @@ 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(). Recover it to report back to the caller. */ + WMEMSET(&addr, 0, sizeof addr); + if (getsockname(ctx->listenFd, + (struct sockaddr*)&addr, &boundSz) == 0) { + allocatedPort = (word32)ntohs(addr.sin_port); + /* The library reads a return below WS_FWD_PORT_CHECK as a + * status, not a port, so an allocated port must be reportable. + * An unprivileged OS-chosen port always is; guard anyway. */ + if (allocatedPort < WS_FWD_PORT_CHECK) { + printf("Allocated port %u not reportable.\n", allocatedPort); + ret = -1; + } + else { + ctx->hostPort = allocatedPort; + } + } + else { + printf("getsockname failed for forwarded port.\n"); + ret = -1; + } + } + if (ret == 0) { ctx->state = FWD_STATE_LISTEN; + /* Report any dynamically allocated port to the library through the + * return value; 0 keeps the port the peer requested. */ + ret = (int)allocatedPort; } else { if (ctx->hostName != NULL) { diff --git a/src/internal.c b/src/internal.c index dfb92d72..1afcaba7 100644 --- a/src/internal.c +++ b/src/internal.c @@ -8957,6 +8957,7 @@ static int DoGlobalRequestFwd(WOLFSSH* ssh, int ret = WS_SUCCESS; char* bindAddr = NULL; word32 bindPort; + word32 requestedPort = 0; WLOG(WS_LOG_DEBUG, "Entering DoGlobalRequestFwd()"); @@ -8975,15 +8976,36 @@ 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); } if (ret == WS_SUCCESS) { if (ssh->ctx->fwdCb) { - ret = ssh->ctx->fwdCb(isCancel ? WOLFSSH_FWD_REMOTE_CLEANUP : + int cbRet = ssh->ctx->fwdCb(isCancel ? WOLFSSH_FWD_REMOTE_CLEANUP : WOLFSSH_FWD_REMOTE_SETUP, ssh->fwdCbCtx, bindAddr, bindPort); + /* A return at or above WS_FWD_PORT_CHECK is the unprivileged port + * the callback allocated for a remote port-0 request; anything + * below it is a WS_FwdCbError status, where WS_FWD_SUCCESS is + * success and any other value is a rejection. An allocated port is + * only meaningful for a port-0 (dynamic) request and must be a + * valid port number; for a non-zero request the callback should + * return WS_FWD_SUCCESS, so a port-like value is ignored and the + * requested port stands. An out-of-range value for a port-0 + * request leaves bindPort unchanged and is rejected by the + * port-0 compliance check below. */ + if (!isCancel && cbRet >= WS_FWD_PORT_CHECK) { + if (requestedPort == 0 && cbRet <= 65535) { + bindPort = (word32)cbRet; + } + } + else if (cbRet != WS_FWD_SUCCESS) { + WLOG(WS_LOG_WARN, "Forward callback rejected the request, " + "WS_FwdCbError = %d", cbRet); + ret = WS_RESOURCE_E; + } } else { WLOG(WS_LOG_WARN, "No forwarding callback set, rejecting request. " @@ -8992,6 +9014,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 actual unprivileged + * port allocated. A successful callback that did not report one leaves + * bindPort below WS_FWD_PORT_CHECK, so we cannot comply: undo the setup + * and reject instead of sending a non-compliant success. */ + if (requestedPort == 0 && bindPort < WS_FWD_PORT_CHECK) { + WLOG(WS_LOG_WARN, "Forward callback reported no unprivileged port " + "for a port-0 request; rejecting."); + if (ssh->ctx->fwdCb) { + int cleanupRet = ssh->ctx->fwdCb(WOLFSSH_FWD_REMOTE_CLEANUP, + ssh->fwdCbCtx, bindAddr, bindPort); + if (cleanupRet != WS_SUCCESS) { + WLOG(WS_LOG_WARN, "Forward cleanup after rejection failed, " + "ret = %d", cleanupRet); + } + ret = WS_RESOURCE_E; + } + } + } + if (wantReply) { if (ret == WS_SUCCESS) { if (isCancel) { @@ -9005,7 +9048,7 @@ static int DoGlobalRequestFwd(WOLFSSH* ssh, ret = SendRequestSuccess(ssh, 0); } } - else if (ret == WS_UNIMPLEMENTED_E) { + else if (ret == WS_UNIMPLEMENTED_E || ret == WS_RESOURCE_E) { /* No reply expected; silently reject without terminating connection. */ ret = WS_SUCCESS; } diff --git a/tests/regress.c b/tests/regress.c index de22281b..86e19500 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) @@ -1152,6 +1164,54 @@ 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; + + /* A return at or above WS_FWD_PORT_CHECK reports the allocated port for a + * port-0 request; WS_FWD_SUCCESS (0) otherwise. */ + if (action == WOLFSSH_FWD_REMOTE_SETUP && port == 0) + return 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; +} + +/* Rejects the remote setup with a WS_FwdCbError status. The server must send a + * failure and must NOT ask for cleanup, since the setup never succeeded. */ +static int RejectRemoteSetupFwdCb(WS_FwdCbAction action, void* ctx, + const char* host, word32 port) +{ + int* cleanupCalled = (int*)ctx; + (void)host; + (void)port; + + if (action == WOLFSSH_FWD_REMOTE_SETUP) + return WS_FWD_SETUP_E; + if (action == WOLFSSH_FWD_REMOTE_CLEANUP && cleanupCalled != NULL) + *cleanupCalled = 1; + + return WS_SUCCESS; +} #endif @@ -1523,6 +1583,108 @@ 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 TestGlobalRequestFwdRemoteSetupErrorSendsFailure(void) +{ + ChannelOpenHarness harness; + byte in[256]; + word32 inSz; + int ret; + int cleanupCalled = 0; + + /* The callback rejects the remote setup with a WS_FwdCbError status (below + * WS_FWD_PORT_CHECK). The server must reply with failure and must not run + * cleanup, since the setup never succeeded. */ + inSz = BuildGlobalRequestFwdPacket("0.0.0.0", 2222, 0, 1, in, sizeof(in)); + InitChannelOpenHarness(&harness, in, inSz); + AssertIntEQ(wolfSSH_CTX_SetFwdCb(harness.ctx, RejectRemoteSetupFwdCb, 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, 0); + + FreeChannelOpenHarness(&harness); +} + +static void TestGlobalRequestFwdPort0NoAllocNoReplyKeepsConnection(void) +{ + ChannelOpenHarness harness; + byte in[256]; + word32 inSz; + int ret; + int cleanupCalled = 0; + + /* Same port-0 rejection as above, but wantReply=0. The server must still + * tear the setup back down, send no reply, and keep the connection alive + * rather than treating the rejection as a fatal error. */ + inSz = BuildGlobalRequestFwdPacket("0.0.0.0", 0, 0, 0, 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); + AssertIntEQ(harness.io.outSz, 0); /* no reply sent */ + AssertIntEQ(cleanupCalled, 1); + + FreeChannelOpenHarness(&harness); +} + static void TestGlobalRequestFwdCancelNoCbSendsFailure(void) { ChannelOpenHarness harness; @@ -3974,6 +4136,10 @@ int main(int argc, char** argv) TestGlobalRequestFwdNoCbSendsFailure(); TestGlobalRequestFwdNoCbNoReplyKeepsConnection(); TestGlobalRequestFwdWithCbSendsSuccess(); + TestGlobalRequestFwdPort0ReturnsAllocatedPort(); + TestGlobalRequestFwdPort0NoAllocSendsFailure(); + TestGlobalRequestFwdRemoteSetupErrorSendsFailure(); + TestGlobalRequestFwdPort0NoAllocNoReplyKeepsConnection(); TestGlobalRequestFwdCancelNoCbSendsFailure(); TestGlobalRequestFwdCancelWithCbSendsSuccess(); TestRequestSuccessWithPortParsesCorrectly(); diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index 3b3f2279..6ce19aa9 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -206,6 +206,21 @@ typedef enum WS_FwdCbError { WS_FWD_PEER_E, } WS_FwdCbError; +#ifndef WS_FWD_PORT_CHECK + /* Boundary of the WS_CallbackFwd return convention below; not an error + * code. The lowest unprivileged port, and must stay above WS_FWD_PEER_E. */ + #define WS_FWD_PORT_CHECK 1024 +#else + #if (WS_FWD_PEER_E > WS_FWD_PORT_CHECK) + #error "WS_FWD_PORT_CHECK set to value in WS_FwdCbError range." + #endif +#endif + +/* Return value: below WS_FWD_PORT_CHECK is a WS_FwdCbError status + * (WS_FWD_SUCCESS is success); at or above it is the unprivileged port a + * WOLFSSH_FWD_REMOTE_SETUP allocated for a port-0 request, for the server to + * report to the peer. A rejected port-0 setup gets a WOLFSSH_FWD_REMOTE_CLEANUP + * even though the setup returned success. */ typedef int (*WS_CallbackFwd)(WS_FwdCbAction action, void* fwdCbCtx, const char* address, word32 port); typedef int (*WS_CallbackFwdIO)(WS_FwdIoCbAction action, void* buf,