From 2cd1f4db1ee8a0c2952c9e263f4d5cbc553a40db Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 2 Sep 2026 11:57:27 -0700 Subject: [PATCH 1/2] ssh: add generic request callbacks wolfSSH_CTX_SetChannelReqAnyCb() and wolfSSH_CTX_SetGlobalReqAnyCb() register a callback consulted first for a channel or global request, with the name, the type-specific part, and whether a reply is wanted. A tri-state answer grants, refuses, or leaves it to the handling already there, so a policy reaches the types with no hook of their own. - the name is the one that arrived, since a copy into a buffer truncates a long name and ends it at an embedded NUL, and a policy has to answer on what the peer sent - a grant still parses and records what the library needs, so a granted session request commits the session, the typed callbacks are not consulted, and a granted unknown type is answered CHANNEL_SUCCESS - a port-0 tcpip-forward skips the callback, since only the forward callback can report the port bound and a policy that had bound a listener would then have to be refused, per RFC 4254 7.1 - a client refuses tcpip-forward and cancel-tcpip-forward ahead of any policy, matched on the name that arrived so it holds in a build with no forwarding, where neither name is in the name table - window-change, exit-status and exit-signal are cleared of a reply before the handling runs, so a refusal leaves them unanswered too, per RFC 4254 6.7 and 6.10 - regress.c covers the answers, the data delivered, the names that do not fit a copy, and which callbacks each answer leaves out --- src/internal.c | 230 ++++++++--- src/ssh.c | 27 ++ tests/regress.c | 962 ++++++++++++++++++++++++++++++++++++++++++++- wolfssh/internal.h | 2 + wolfssh/ssh.h | 73 ++++ 5 files changed, 1235 insertions(+), 59 deletions(-) diff --git a/src/internal.c b/src/internal.c index 5d1d61124..6e3eb28d7 100644 --- a/src/internal.c +++ b/src/internal.c @@ -12188,6 +12188,79 @@ static int DoGlobalRequestFwd(WOLFSSH* ssh, } #endif +/* Exact match on a global request name, as ChannelRequestIs() does for a + * channel request type. The name is the one that arrived, so the match is + * on length and bytes. */ +static int GlobalRequestIs(const byte* name, word32 nameSz, const char* want) +{ + word32 wantSz = (word32)WSTRLEN(want); + + return (nameSz == wantSz) && (WMEMCMP(name, want, wantSz) == 0); +} + + +/* Whether a tcpip-forward is the built-in handling's to answer rather than + * the generic callback's. RFC 4254 7.1: a port-0 request is answered with + * the port bound, which only the forward callback can report, and one + * whose body does not parse has no port to read. Granting either in the + * callback would mean refusing it afterwards, once the callback may + * already have bound a listener. */ +static int GlobalRequestNeedsFwdCb(const byte* name, word32 nameSz, + const byte* buf, word32 len, word32 begin) +{ + const byte* bindAddr; + word32 bindAddrSz, bindPort = 0, peek = begin; + + if (!GlobalRequestIs(name, nameSz, "tcpip-forward")) { + return 0; + } + if (GetStringRef(&bindAddrSz, &bindAddr, buf, len, &peek) != WS_SUCCESS + || GetUint32(&bindPort, buf, len, &peek) != WS_SUCCESS) { + return 1; + } + + return bindPort == 0; +} + + +/* Puts a global request to the generic callback, which sees the name and + * the type-specific part to parse itself. Returns 1 when the callback + * settled the request, with any wanted reply sent and *ret carrying the + * result, or 0 to leave it to the built-in handling. The name is the one + * that arrived, nameSz bytes, not the truncated copy the older callback + * is handed. */ +static int DoGlobalRequestAny(WOLFSSH* ssh, const byte* name, word32 nameSz, + byte* buf, word32 len, word32 begin, byte wantReply, int* ret) +{ + int decision, success; + + if (ssh->ctx->globalReqAnyCb == NULL) { + return 0; + } + + if (GlobalRequestNeedsFwdCb(name, nameSz, buf, len, begin)) { + WLOG(WS_LOG_DEBUG, "DGR: a port-0 forward is left to the handling " + "that can bind it"); + return 0; + } + + decision = ssh->ctx->globalReqAnyCb(ssh, name, nameSz, buf + begin, + len - begin, wantReply, ssh->globalReqCtx); + if (decision != WOLFSSH_REQ_ACCEPT && decision != WOLFSSH_REQ_REJECT) { + return 0; + } + success = (decision == WOLFSSH_REQ_ACCEPT); + + WLOG(WS_LOG_DEBUG, "DGR: global request callback %s", + success ? "granted" : "refused"); + if (wantReply) { + *ret = SendRequestSuccess(ssh, success); + } + + return 1; +} + + static int DoGlobalRequest(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) { @@ -12195,6 +12268,8 @@ static int DoGlobalRequest(WOLFSSH* ssh, int ret = WS_SUCCESS; char name[80]; word32 nameSz = (word32)sizeof(name); + const byte* wireName; + word32 wireNameSz; int globReqId = ID_UNKNOWN; byte wantReply = 0; @@ -12207,8 +12282,19 @@ static int DoGlobalRequest(WOLFSSH* ssh, } if (ret == WS_SUCCESS) { + word32 copyBegin; + + /* Read twice: the generic callback answers on the name as it + * arrived, where the callback below it has always been handed a + * copy, NUL terminated and truncated to the buffer. */ begin = *idx; - ret = GetString(name, &nameSz, buf, len, &begin); + copyBegin = begin; + ret = GetStringRef(&wireNameSz, &wireName, buf, len, &begin); + if (ret == WS_SUCCESS) { + if (wireName == NULL) + wireName = (const byte*)""; /* an empty name parses as NULL */ + ret = GetString(name, &nameSz, buf, len, ©Begin); + } } if (ret == WS_SUCCESS) { @@ -12218,47 +12304,53 @@ static int DoGlobalRequest(WOLFSSH* ssh, } if (ret == WS_SUCCESS) { -#ifdef WOLFSSH_FWD /* RFC 4254 section 7.1: a remote forward is the client's to ask for, * so a client that receives the request answers a failure rather than * registering a forward on the peer's say-so. Answered here and not * in DoGlobalRequestFwd(), so the request body is never parsed and no - * forward state is touched. */ - if ((globReqId == ID_GLOBREQ_TCPIP_FWD - || globReqId == ID_GLOBREQ_TCPIP_FWD_CANCEL) + * forward state is touched. Matched on the name as it arrived rather + * than on an ID: the name table carries these two only in a + * WOLFSSH_FWD build, while a client answers them in any build. */ + if ((GlobalRequestIs(wireName, wireNameSz, "tcpip-forward") + || GlobalRequestIs(wireName, wireNameSz, + "cancel-tcpip-forward")) && ssh->ctx->side == WOLFSSH_ENDPOINT_CLIENT) { WLOG(WS_LOG_WARN, "DGR: rejecting %s received by a client", name); if (wantReply) { ret = SendRequestSuccess(ssh, 0); } } - else -#endif - switch (globReqId) { + else if (!DoGlobalRequestAny(ssh, wireName, wireNameSz, + buf, len, begin, wantReply, &ret)) { + switch (globReqId) { #ifdef WOLFSSH_FWD - case ID_GLOBREQ_TCPIP_FWD: - ret = DoGlobalRequestFwd(ssh, buf, len, &begin, wantReply, 0); - wantReply = 0; - break; - case ID_GLOBREQ_TCPIP_FWD_CANCEL: - ret = DoGlobalRequestFwd(ssh, buf, len, &begin, wantReply, 1); - wantReply = 0; - break; + case ID_GLOBREQ_TCPIP_FWD: + ret = DoGlobalRequestFwd(ssh, buf, len, &begin, + wantReply, 0); + wantReply = 0; + break; + case ID_GLOBREQ_TCPIP_FWD_CANCEL: + ret = DoGlobalRequestFwd(ssh, buf, len, &begin, + wantReply, 1); + wantReply = 0; + break; #endif - default: - if (ssh->ctx->globalReqCb != NULL) { - ret = ssh->ctx->globalReqCb(ssh, name, nameSz, wantReply, - (void *)ssh->globalReqCtx); + default: + if (ssh->ctx->globalReqCb != NULL) { + ret = ssh->ctx->globalReqCb(ssh, name, nameSz, + wantReply, (void *)ssh->globalReqCtx); - if (wantReply) { - ret = SendRequestSuccess(ssh, (ret == WS_SUCCESS)); + if (wantReply) { + ret = SendRequestSuccess(ssh, + (ret == WS_SUCCESS)); + } } - } - else if (wantReply) - ret = SendRequestSuccess(ssh, 0); - /* response SSH_MSG_REQUEST_FAILURE to Keep-Alive. - * IETF:draft-ssh-global-requests */ - break; + else if (wantReply) + ret = SendRequestSuccess(ssh, 0); + /* response SSH_MSG_REQUEST_FAILURE to Keep-Alive. + * IETF:draft-ssh-global-requests */ + break; + } } } @@ -13074,12 +13166,14 @@ int wolfSSH_DoModes(const byte* modes, word32 modesSz, int fd) #endif /* !NO_TERMIOS && WOLFSSH_TERM */ -/* Exact match on a channel request type, as NameToIdType() does for names. */ -static int ChannelRequestIs(const char* type, word32 typeSz, const char* name) +/* Exact match on a channel request type, as NameToIdType() does for names. + * The type is the name as it arrived, so the match is on length and bytes; + * a type with an embedded NUL is some other type. */ +static int ChannelRequestIs(const byte* type, word32 typeSz, const char* name) { word32 nameSz = (word32)WSTRLEN(name); - return (typeSz == nameSz) && (WSTRNCMP(type, name, nameSz) == 0); + return (typeSz == nameSz) && (WMEMCMP(type, name, nameSz) == 0); } @@ -13101,10 +13195,11 @@ static void SetTerminalSize(WOLFSSH* ssh, word32 widthChar, word32 heightRows, /* Answers a shell, exec, or subsystem request. Sets the session type and - * command for the callback to read, and keeps them only if it accepts. */ + * command for the callback to read, and keeps them only if it accepts. + * A request the generic callback already granted asks no callback. */ static int DoChannelRequestSession(WOLFSSH* ssh, word32 channelId, WOLFSSH_CHANNEL* channel, byte sessionType, WS_CallbackChannelReq cb, - byte* buf, word32 len, word32* idx, int* rej) + int granted, byte* buf, word32 len, word32* idx, int* rej) { void* heap = ssh->ctx->heap; byte prevType = channel->sessionType; @@ -13135,7 +13230,9 @@ static int DoChannelRequestSession(WOLFSSH* ssh, word32 channelId, } channel->sessionType = sessionType; - if (cb != NULL) + if (granted) + *rej = 0; + else if (cb != NULL) *rej = cb(channel, ssh->channelReqCtx); else *rej = ssh->appChannels; @@ -13175,17 +13272,22 @@ static int DoChannelRequest(WOLFSSH* ssh, word32 begin = *idx; word32 channelId; word32 typeSz; - char type[32]; + const byte* type; byte wantReply; - int ret, rej = 0; + int ret, rej = 0, granted = 0; WLOG(WS_LOG_DEBUG, "Entering DoChannelRequest()"); ret = GetUint32(&channelId, buf, len, &begin); - typeSz = (word32)sizeof(type); - if (ret == WS_SUCCESS) - ret = GetString(type, &typeSz, buf, len, &begin); + /* Taken as it arrived rather than copied into a buffer, which would + * truncate a long type and end a type at an embedded NUL. The generic + * callback below answers on what the peer actually sent. */ + if (ret == WS_SUCCESS) { + ret = GetStringRef(&typeSz, &type, buf, len, &begin); + if (ret == WS_SUCCESS && type == NULL) + type = (const byte*)""; /* an empty type parses as NULL */ + } if (ret == WS_SUCCESS) ret = GetBoolean(&wantReply, buf, len, &begin); @@ -13202,9 +13304,45 @@ static int DoChannelRequest(WOLFSSH* ssh, if (ret == WS_SUCCESS) { WLOG(WS_LOG_DEBUG, " channelId = %u", channelId); - WLOG(WS_LOG_DEBUG, " type = %s", type); + WLOG(WS_LOG_DEBUG, " type = %.*s", (int)typeSz, type); WLOG(WS_LOG_DEBUG, " wantReply = %u", wantReply); + /* The generic callback sees every request first, with the + * type-specific part to parse itself. A refusal skips the handling + * below; a grant runs it with the decision already made. */ + if (ssh->ctx->channelReqAnyCb != NULL) { + int decision = ssh->ctx->channelReqAnyCb(channel, type, typeSz, + buf + begin, len - begin, wantReply, + ssh->channelReqCtx); + if (decision == WOLFSSH_REQ_REJECT) { + WLOG(WS_LOG_DEBUG, " channel request callback refused."); + rej = 1; + } + else if (decision == WOLFSSH_REQ_ACCEPT) { + granted = 1; + } + } + } + +#if defined(WOLFSSH_TERM) || defined(WOLFSSH_SHELL) + /* RFC 4254 sec 6.10: these are never answered, whatever the handling + * below makes of them. Cleared here rather than in each branch, which + * a refusal or a freed channel skips. */ + if (ret == WS_SUCCESS + && (ChannelRequestIs(type, typeSz, "exit-status") + || ChannelRequestIs(type, typeSz, "exit-signal"))) { + wantReply = 0; + } +#endif +#if defined(WOLFSSH_SHELL) && defined(WOLFSSH_TERM) + /* RFC 4254 sec 6.7: the same for window-change. */ + if (ret == WS_SUCCESS + && ChannelRequestIs(type, typeSz, "window-change")) { + wantReply = 0; + } +#endif + + if (ret == WS_SUCCESS && !rej) { if (ChannelRequestIs(type, typeSz, "env")) { char name[WOLFSSH_MAX_NAMESZ]; word32 nameSz; @@ -13229,17 +13367,17 @@ static int DoChannelRequest(WOLFSSH* ssh, else if (ChannelRequestIs(type, typeSz, "shell")) { ret = DoChannelRequestSession(ssh, channelId, channel, WOLFSSH_SESSION_SHELL, ssh->ctx->channelReqShellCb, - buf, len, &begin, &rej); + granted, buf, len, &begin, &rej); } else if (ChannelRequestIs(type, typeSz, "exec")) { ret = DoChannelRequestSession(ssh, channelId, channel, WOLFSSH_SESSION_EXEC, ssh->ctx->channelReqExecCb, - buf, len, &begin, &rej); + granted, buf, len, &begin, &rej); } else if (ChannelRequestIs(type, typeSz, "subsystem")) { ret = DoChannelRequestSession(ssh, channelId, channel, WOLFSSH_SESSION_SUBSYSTEM, ssh->ctx->channelReqSubsysCb, - buf, len, &begin, &rej); + granted, buf, len, &begin, &rej); } #ifdef WOLFSSH_TERM else if (ChannelRequestIs(type, typeSz, "pty-req")) { @@ -13286,7 +13424,6 @@ static int DoChannelRequest(WOLFSSH* ssh, else if (ChannelRequestIs(type, typeSz, "window-change")) { word32 widthChar, heightRows, widthPixels, heightPixels; - wantReply = 0; /* RFC 4254 sec 6.7: no reply for window-change */ ret = GetUint32(&widthChar, buf, len, &begin); if (ret == WS_SUCCESS) ret = GetUint32(&heightRows, buf, len, &begin); @@ -13321,7 +13458,6 @@ static int DoChannelRequest(WOLFSSH* ssh, #endif /* WOLFSSH_SHELL && WOLFSSH_TERM */ #if defined(WOLFSSH_TERM) || defined(WOLFSSH_SHELL) else if (ChannelRequestIs(type, typeSz, "exit-status")) { - wantReply = 0; /* RFC 4254 sec 6.10: no reply for exit-status */ ret = GetUint32(&ssh->exitStatus, buf, len, &begin); WLOG(WS_LOG_AGENT, "Got exit status %u.", ssh->exitStatus); } @@ -13330,7 +13466,6 @@ static int DoChannelRequest(WOLFSSH* ssh, word32 sigSz; byte coreDumped; - wantReply = 0; /* RFC 4254 sec 6.10: no reply for exit-signal */ WLOG(WS_LOG_AGENT, "Got exit signal, remote command terminated"); sigSz = WOLFSSH_MAX_NAMESZ; @@ -13364,6 +13499,9 @@ static int DoChannelRequest(WOLFSSH* ssh, WLOG(WS_LOG_AGENT, "Agent callback not set, not using."); } #endif /* WOLFSSH_AGENT */ + else if (granted) { + WLOG(WS_LOG_DEBUG, " unknown channel request type, granted."); + } else { WLOG(WS_LOG_DEBUG, " unknown channel request type, rejecting."); rej = 1; diff --git a/src/ssh.c b/src/ssh.c index 17054f834..a38af96d1 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -397,6 +397,19 @@ void wolfSSH_SetReqFailure(WOLFSSH_CTX *ctx, WS_CallbackReqSuccess cb) ctx->reqFailureCb = cb; } +int wolfSSH_CTX_SetGlobalReqAnyCb(WOLFSSH_CTX* ctx, WS_CallbackGlobalReqAny cb) +{ + int ret = WS_SSH_CTX_NULL_E; + + if (ctx != NULL) { + ctx->globalReqAnyCb = cb; + ret = WS_SUCCESS; + } + + return ret; +} + + void wolfSSH_SetGlobalReqCtx(WOLFSSH* ssh, void *ctx) { WLOG(WS_LOG_DEBUG, "Entering wolfSSH_SetGlobalReqCtx()"); @@ -5830,6 +5843,20 @@ int wolfSSH_CTX_SetChannelReqSubsysCb(WOLFSSH_CTX* ctx, } +int wolfSSH_CTX_SetChannelReqAnyCb(WOLFSSH_CTX* ctx, + WS_CallbackChannelReqAny cb) +{ + int ret = WS_SSH_CTX_NULL_E; + + if (ctx != NULL) { + ctx->channelReqAnyCb = cb; + ret = WS_SUCCESS; + } + + return ret; +} + + int wolfSSH_CTX_SetAppChannels(WOLFSSH_CTX* ctx, byte enable) { int ret = WS_SSH_CTX_NULL_E; diff --git a/tests/regress.c b/tests/regress.c index eaa841542..b4513a799 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -205,15 +205,20 @@ static word32 AppendData(byte* buf, word32 bufSz, word32 idx, return idx; } -static word32 AppendString(byte* buf, word32 bufSz, word32 idx, - const char* value) +/* A string of any bytes, for the names a C string cannot hold. */ +static word32 AppendStringSz(byte* buf, word32 bufSz, word32 idx, + const char* value, word32 valueSz) { - word32 valueSz = (word32)WSTRLEN(value); - idx = AppendUint32(buf, bufSz, idx, valueSz); return AppendData(buf, bufSz, idx, (const byte*)value, valueSz); } +static word32 AppendString(byte* buf, word32 bufSz, word32 idx, + const char* value) +{ + return AppendStringSz(buf, bufSz, idx, value, (word32)WSTRLEN(value)); +} + static word32 WrapPacket(byte msgId, const byte* payload, word32 payloadSz, byte* out, word32 outSz) { @@ -443,6 +448,49 @@ static void InitChannelOpenHarnessClient(ChannelOpenHarness* harness, } #endif /* !NO_WOLFSSH_CLIENT */ +static void RepointHarnessInput(ChannelOpenHarness* harness, + byte* in, word32 inSz) +{ + harness->io.in = in; + harness->io.inSz = inSz; + harness->io.inOff = 0; + harness->io.outSz = 0; +} + +static word32 BuildGlobalRequestPacketSz(const char* name, word32 nameSz, + byte wantReply, const byte* tail, word32 tailSz, byte* out, + word32 outSz) +{ + byte payload[128]; + word32 idx = 0; + + idx = AppendStringSz(payload, sizeof(payload), idx, name, nameSz); + idx = AppendByte(payload, sizeof(payload), idx, wantReply); + idx = AppendData(payload, sizeof(payload), idx, tail, tailSz); + + return WrapPacket(MSGID_GLOBAL_REQUEST, payload, idx, out, outSz); +} + +static word32 BuildGlobalRequestPacket(const char* name, byte wantReply, + const byte* tail, word32 tailSz, byte* out, word32 outSz) +{ + return BuildGlobalRequestPacketSz(name, (word32)WSTRLEN(name), wantReply, + tail, tailSz, out, outSz); +} + +/* Feeds one packet to the harness and returns the id of the reply, or 0 + * when nothing was sent. */ +static byte ReplyToPacket(ChannelOpenHarness* harness, byte* in, + word32 inSz) +{ + RepointHarnessInput(harness, in, inSz); + AssertIntEQ(DoReceive(harness->ssh), WS_SUCCESS); + AssertIntEQ(harness->io.inOff, harness->io.inSz); + + return harness->io.outSz == 0 ? 0 : ParseMsgId(harness->io.out, + harness->io.outSz); +} + /* 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. */ @@ -3481,15 +3529,6 @@ static void InitUserAuthHarness(ChannelOpenHarness* harness, harness->ssh->acceptState = ACCEPT_SERVER_USERAUTH_ACCEPT_SENT; } -static void RepointHarnessInput(ChannelOpenHarness* harness, - byte* in, word32 inSz) -{ - harness->io.in = in; - harness->io.inSz = inSz; - harness->io.inOff = 0; - harness->io.outSz = 0; -} - /* Builds a plaintext SSH_MSG_CHANNEL_OPEN_CONFIRMATION. */ static word32 BuildChannelOpenConfPacket(word32 recipientChannelId, word32 senderChannelId, word32 initialWindowSz, word32 maxPacketSz, @@ -3958,6 +3997,822 @@ static void TestChannelReqSubsysCallbackRuns(void) WOLFSSH_SESSION_SUBSYSTEM), MSGID_CHANNEL_FAILURE); } +/* Builds a plaintext SSH_MSG_CHANNEL_REQUEST with a raw type-specific + * tail, so a test can send any request type. */ +static word32 BuildChannelRequestPacketSz(word32 recipientChannelId, + const char* type, word32 typeSz, byte wantReply, const byte* tail, + word32 tailSz, byte* out, word32 outSz) +{ + byte payload[128]; + word32 idx = 0; + + idx = AppendUint32(payload, sizeof(payload), idx, recipientChannelId); + idx = AppendStringSz(payload, sizeof(payload), idx, type, typeSz); + idx = AppendByte(payload, sizeof(payload), idx, wantReply); + idx = AppendData(payload, sizeof(payload), idx, tail, tailSz); + + return WrapPacket(MSGID_CHANNEL_REQUEST, payload, idx, out, outSz); +} + +static word32 BuildChannelRequestPacket(word32 recipientChannelId, + const char* type, byte wantReply, const byte* tail, word32 tailSz, + byte* out, word32 outSz) +{ + return BuildChannelRequestPacketSz(recipientChannelId, type, + (word32)WSTRLEN(type), wantReply, tail, tailSz, out, outSz); +} + +/* Builds a plaintext SSH_MSG_GLOBAL_REQUEST with a raw type-specific + * tail. */ +/* What the generic request callbacks saw, and what they answer. The name + * arrives as it was sent, so it is kept with its length and terminated + * here for the checks that compare it as a string. */ +static int anyReqCbCalls; +static char anyReqCbName[128]; +static word32 anyReqCbNameSz; +static byte anyReqCbData[64]; +static word32 anyReqCbDataSz; +static int anyReqCbWantReply; +static void* anyReqCbCtx; +static int anyReqCbReturn; + +static void ResetAnyReqCb(int cbReturn) +{ + anyReqCbCalls = 0; + anyReqCbName[0] = 0; + anyReqCbNameSz = 0; + anyReqCbDataSz = 0; + anyReqCbWantReply = -1; + anyReqCbCtx = NULL; + anyReqCbReturn = cbReturn; +} + +static void RecordAnyReq(const byte* name, word32 nameSz, const byte* data, + word32 dataSz, void* ctx) +{ + anyReqCbCalls++; + AssertNotNull(name); /* a name of no bytes is still a name */ + AssertTrue(nameSz < sizeof(anyReqCbName)); + anyReqCbNameSz = nameSz; + if (nameSz > 0) { + WMEMCPY(anyReqCbName, name, nameSz); + } + anyReqCbName[nameSz] = 0; + anyReqCbDataSz = dataSz; + if (dataSz > 0) { + AssertTrue(dataSz <= sizeof(anyReqCbData)); + WMEMCPY(anyReqCbData, data, dataSz); + } + anyReqCbCtx = ctx; +} + +static int RecordingChannelReqAnyCb(WOLFSSH_CHANNEL* channel, + const byte* type, word32 typeSz, const byte* data, word32 dataSz, + int wantReply, void* ctx) +{ + AssertNotNull(channel); + RecordAnyReq(type, typeSz, data, dataSz, ctx); + anyReqCbWantReply = wantReply; + return anyReqCbReturn; +} + +static int RecordingGlobalReqAnyCb(WOLFSSH* ssh, const byte* name, + word32 nameSz, const byte* data, word32 dataSz, int wantReply, + void* ctx) +{ + AssertNotNull(ssh); + RecordAnyReq(name, nameSz, data, dataSz, ctx); + anyReqCbWantReply = wantReply; + return anyReqCbReturn; +} + +/* What the older global request callback saw, and what it answers. Its + * name buffer is the size of the one the library copies into, so a + * truncated name fills it exactly. */ +static int legacyGlobalReqCbCalls; +static char legacyGlobalReqCbName[80]; +static word32 legacyGlobalReqCbNameSz; +static int legacyGlobalReqCbWantReply; +static void* legacyGlobalReqCbCtx; +static int legacyGlobalReqCbReturn; + +static int RecordingGlobalReqCb(WOLFSSH* ssh, void* buf, word32 sz, + int reply, void* ctx) +{ + (void)ssh; + legacyGlobalReqCbCalls++; + AssertNotNull(buf); + legacyGlobalReqCbNameSz = sz; + if (sz >= (word32)sizeof(legacyGlobalReqCbName)) + sz = (word32)sizeof(legacyGlobalReqCbName) - 1; + WMEMCPY(legacyGlobalReqCbName, buf, sz); + legacyGlobalReqCbName[sz] = 0; + legacyGlobalReqCbWantReply = reply; + legacyGlobalReqCbCtx = ctx; + return legacyGlobalReqCbReturn; +} + +/* A typed session callback that only counts, to show whether the generic + * callback left the request to it. */ +static int typedReqCbCalls; + +static int CountingSessionReqCb(WOLFSSH_CHANNEL* channel, void* ctx) +{ + (void)channel; + (void)ctx; + typedReqCbCalls++; + return 0; +} + +/* Seeds a confirmed session channel on the harness, the state a channel is + * in when requests arrive on it. */ +static WOLFSSH_CHANNEL* SeedConfirmedSessionChannel( + ChannelOpenHarness* harness) +{ + WOLFSSH_CHANNEL* channel; + + channel = ChannelNew(harness->ssh, ID_CHANTYPE_SESSION, 1024, 1024); + AssertNotNull(channel); + AssertIntEQ(ChannelAppend(harness->ssh, channel), WS_SUCCESS); + AssertIntEQ(ChannelUpdatePeer(channel, 5, 1024, 1024), WS_SUCCESS); + channel->openConfirmed = 1; + + return channel; +} + +/* The generic channel request callback sees every request first, with + * the type-specific part to parse itself, and can refuse a type the + * library would otherwise take in. */ +static void TestChannelReqCallbackSeesRequestAndRefuses(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte tail[32]; + word32 tailSz = 0; + byte in[128]; + word32 inSz; + int cbCtx = 0; + + ResetAnyReqCb(WOLFSSH_REQ_REJECT); + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetChannelReqAnyCb(harness.ctx, + RecordingChannelReqAnyCb), WS_SUCCESS); + AssertIntEQ(wolfSSH_SetChannelReqCtx(harness.ssh, &cbCtx), WS_SUCCESS); + channel = SeedConfirmedSessionChannel(&harness); + + tailSz = AppendString(tail, sizeof(tail), tailSz, "FOO"); + tailSz = AppendString(tail, sizeof(tail), tailSz, "bar"); + inSz = BuildChannelRequestPacket(channel->channel, "env", 1, + tail, tailSz, in, sizeof(in)); + + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_CHANNEL_FAILURE); + AssertIntEQ(anyReqCbCalls, 1); + AssertIntEQ(WSTRCMP(anyReqCbName, "env"), 0); + AssertIntEQ(anyReqCbDataSz, tailSz); + AssertIntEQ(WMEMCMP(anyReqCbData, tail, tailSz), 0); + AssertTrue(anyReqCbCtx == &cbCtx); + + /* Left to the built-in handling, the same request is taken in. */ + anyReqCbReturn = WOLFSSH_REQ_UNHANDLED; + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_CHANNEL_SUCCESS); + AssertIntEQ(anyReqCbCalls, 2); + + FreeChannelOpenHarness(&harness); +} + +/* A type the library does not know is refused unless the callback grants + * it, which is how an application answers its own request types. */ +static void TestChannelReqCallbackGrantsUnknownType(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + static const byte tail[] = { 1, 2, 3 }; + byte in[128]; + word32 inSz; + + ResetAnyReqCb(WOLFSSH_REQ_ACCEPT); + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetChannelReqAnyCb(harness.ctx, + RecordingChannelReqAnyCb), WS_SUCCESS); + channel = SeedConfirmedSessionChannel(&harness); + + inSz = BuildChannelRequestPacket(channel->channel, "x-custom@wolfssh", + 1, tail, sizeof(tail), in, sizeof(in)); + + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_CHANNEL_SUCCESS); + AssertIntEQ(WSTRCMP(anyReqCbName, "x-custom@wolfssh"), 0); + AssertIntEQ(anyReqCbDataSz, sizeof(tail)); + + anyReqCbReturn = WOLFSSH_REQ_UNHANDLED; + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_CHANNEL_FAILURE); + + FreeChannelOpenHarness(&harness); +} + +/* The type reaches the callback as it arrived: a type too long to fit a + * copy, and one carrying an embedded NUL, both arrive whole, so a policy + * answers on what the peer sent. */ +static void TestChannelReqCallbackSeesWholeType(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + static const char longType[] = + "x-very-long-request-type-name@wolfssh.example.com"; + static const char nulType[] = "env\0trailer"; + word32 longTypeSz = (word32)sizeof(longType) - 1; + word32 nulTypeSz = (word32)sizeof(nulType) - 1; + byte in[192]; + word32 inSz; + + ResetAnyReqCb(WOLFSSH_REQ_REJECT); + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetChannelReqAnyCb(harness.ctx, + RecordingChannelReqAnyCb), WS_SUCCESS); + channel = SeedConfirmedSessionChannel(&harness); + + inSz = BuildChannelRequestPacketSz(channel->channel, longType, + longTypeSz, 1, NULL, 0, in, sizeof(in)); + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_CHANNEL_FAILURE); + AssertIntEQ(anyReqCbNameSz, longTypeSz); + AssertIntEQ(WMEMCMP(anyReqCbName, longType, longTypeSz), 0); + + /* env with a trailer is not env. The callback sees all of it, and left + * unhandled it is refused as the unknown type it is. */ + anyReqCbReturn = WOLFSSH_REQ_UNHANDLED; + inSz = BuildChannelRequestPacketSz(channel->channel, nulType, + nulTypeSz, 1, NULL, 0, in, sizeof(in)); + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_CHANNEL_FAILURE); + AssertIntEQ(anyReqCbNameSz, nulTypeSz); + AssertIntEQ(WMEMCMP(anyReqCbName, nulType, nulTypeSz), 0); + + /* A type of no bytes is refused the same way, and the callback is + * handed a name it can read rather than a NULL. */ + inSz = BuildChannelRequestPacketSz(channel->channel, "", 0, 1, NULL, 0, + in, sizeof(in)); + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_CHANNEL_FAILURE); + AssertIntEQ(anyReqCbNameSz, 0); + + FreeChannelOpenHarness(&harness); +} + +/* Drives one exec request through a fresh harness that registers both the + * generic and the typed exec callback, the generic one set to answer + * anyReturn, and returns the reply id. The harness is left for the caller + * to inspect and free. */ +static byte RunExecThroughBothCallbacks(ChannelOpenHarness* harness, + WOLFSSH_CHANNEL** channel, int anyReturn) +{ + byte tail[32]; + word32 tailSz = 0; + byte in[128]; + word32 inSz; + + ResetAnyReqCb(anyReturn); + typedReqCbCalls = 0; + InitChannelOpenHarness(harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetChannelReqAnyCb(harness->ctx, + RecordingChannelReqAnyCb), WS_SUCCESS); + AssertIntEQ(wolfSSH_CTX_SetChannelReqExecCb(harness->ctx, + CountingSessionReqCb), WS_SUCCESS); + *channel = SeedConfirmedSessionChannel(harness); + + tailSz = AppendString(tail, sizeof(tail), tailSz, "ls"); + inSz = BuildChannelRequestPacket((*channel)->channel, "exec", 1, + tail, tailSz, in, sizeof(in)); + + return ReplyToPacket(harness, in, inSz); +} + +/* A session request the generic callback settles asks the typed callback + * nothing. A grant still commits the session, since the library needs the + * type and command whoever decided; a refusal commits nothing. */ +static void TestChannelReqCallbackSettlesSessionRequest(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + + AssertIntEQ(RunExecThroughBothCallbacks(&harness, &channel, + WOLFSSH_REQ_ACCEPT), MSGID_CHANNEL_SUCCESS); + AssertIntEQ(typedReqCbCalls, 0); + AssertIntEQ(channel->sessionType, WOLFSSH_SESSION_EXEC); + AssertNotNull(channel->command); + AssertIntEQ(WSTRCMP(channel->command, "ls"), 0); + AssertIntEQ(harness.ssh->clientState, CLIENT_DONE); + FreeChannelOpenHarness(&harness); + + AssertIntEQ(RunExecThroughBothCallbacks(&harness, &channel, + WOLFSSH_REQ_REJECT), MSGID_CHANNEL_FAILURE); + AssertIntEQ(typedReqCbCalls, 0); + AssertIntEQ(channel->sessionType, WOLFSSH_SESSION_UNKNOWN); + AssertNull(channel->command); + AssertTrue(harness.ssh->clientState < CLIENT_DONE); + FreeChannelOpenHarness(&harness); + + AssertIntEQ(RunExecThroughBothCallbacks(&harness, &channel, + WOLFSSH_REQ_UNHANDLED), MSGID_CHANNEL_SUCCESS); + AssertIntEQ(typedReqCbCalls, 1); + AssertIntEQ(channel->sessionType, WOLFSSH_SESSION_EXEC); + FreeChannelOpenHarness(&harness); +} + +/* With application-driven channels on and no shell callback, a shell + * request is refused unless the generic callback grants it. */ +static void TestChannelReqCallbackGrantOverridesAppChannels(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte in[128]; + word32 inSz; + + ResetAnyReqCb(WOLFSSH_REQ_UNHANDLED); + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetChannelReqAnyCb(harness.ctx, + RecordingChannelReqAnyCb), WS_SUCCESS); + AssertIntEQ(wolfSSH_SetAppChannels(harness.ssh, 1), WS_SUCCESS); + channel = SeedConfirmedSessionChannel(&harness); + + inSz = BuildChannelRequestPacket(channel->channel, "shell", 1, + NULL, 0, in, sizeof(in)); + + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_CHANNEL_FAILURE); + AssertIntEQ(channel->sessionType, WOLFSSH_SESSION_UNKNOWN); + + anyReqCbReturn = WOLFSSH_REQ_ACCEPT; + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_CHANNEL_SUCCESS); + AssertIntEQ(channel->sessionType, WOLFSSH_SESSION_SHELL); + AssertIntEQ(harness.ssh->clientState, CLIENT_DONE); + + FreeChannelOpenHarness(&harness); +} + +/* Both setters answer a NULL context, the only error either has. */ +static void TestReqAnyCallbackSettersRejectNullCtx(void) +{ + WOLFSSH_CTX* ctx; + + AssertIntEQ(wolfSSH_CTX_SetChannelReqAnyCb(NULL, + RecordingChannelReqAnyCb), WS_SSH_CTX_NULL_E); + AssertIntEQ(wolfSSH_CTX_SetGlobalReqAnyCb(NULL, + RecordingGlobalReqAnyCb), WS_SSH_CTX_NULL_E); + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + AssertNotNull(ctx); + AssertIntEQ(wolfSSH_CTX_SetChannelReqAnyCb(ctx, + RecordingChannelReqAnyCb), WS_SUCCESS); + AssertIntEQ(wolfSSH_CTX_SetGlobalReqAnyCb(ctx, + RecordingGlobalReqAnyCb), WS_SUCCESS); + wolfSSH_CTX_free(ctx); +} + +#ifdef WOLFSSH_TERM +/* A granted request the library knows is still parsed and recorded, and a + * request that does not fit its type is refused whatever the callback + * said. pty-req is the case that does both. */ +static void TestChannelReqCallbackKeepsTypeChecks(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte tail[64]; + word32 tailSz; + byte in[192]; + word32 inSz; + + ResetAnyReqCb(WOLFSSH_REQ_ACCEPT); + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetChannelReqAnyCb(harness.ctx, + RecordingChannelReqAnyCb), WS_SUCCESS); + channel = SeedConfirmedSessionChannel(&harness); + + tailSz = AppendString(tail, sizeof(tail), 0, "vt100"); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 80); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 24); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 640); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 480); + tailSz = AppendString(tail, sizeof(tail), tailSz, ""); + inSz = BuildChannelRequestPacket(channel->channel, "pty-req", 1, + tail, tailSz, in, sizeof(in)); + + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_CHANNEL_SUCCESS); + AssertIntEQ(anyReqCbCalls, 1); + AssertIntEQ(anyReqCbWantReply, 1); + AssertIntEQ(channel->ptyReq, 1); + AssertIntEQ(harness.ssh->widthChar, 80); + AssertIntEQ(harness.ssh->heightRows, 24); + AssertIntEQ(harness.ssh->widthPixels, 640); + AssertIntEQ(harness.ssh->heightPixels, 480); + + FreeChannelOpenHarness(&harness); + + /* The same grant on a pty-req that stops after the term name. The + * grant does not stand in for the parse: the request is refused, and + * the malformed packet fails the session as one always has. */ + ResetAnyReqCb(WOLFSSH_REQ_ACCEPT); + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetChannelReqAnyCb(harness.ctx, + RecordingChannelReqAnyCb), WS_SUCCESS); + channel = SeedConfirmedSessionChannel(&harness); + + tailSz = AppendString(tail, sizeof(tail), 0, "vt100"); + inSz = BuildChannelRequestPacket(channel->channel, "pty-req", 1, + tail, tailSz, in, sizeof(in)); + + RepointHarnessInput(&harness, in, inSz); + AssertIntEQ(DoReceive(harness.ssh), WS_FATAL_ERROR); + AssertIntEQ(anyReqCbCalls, 1); + AssertIntEQ(channel->ptyReq, 0); + AssertTrue(harness.io.outSz > 0); + AssertIntEQ(ParseMsgId(harness.io.out, harness.io.outSz), + MSGID_CHANNEL_FAILURE); + + FreeChannelOpenHarness(&harness); +} +#endif /* WOLFSSH_TERM */ + +#if defined(WOLFSSH_TERM) || defined(WOLFSSH_SHELL) +/* RFC 4254 sec 6.7 and 6.10 leave exit-status, exit-signal and + * window-change unanswered. A peer that asks for a reply anyway still gets + * none, whatever the generic callback decided. */ +static void TestChannelReqCallbackKeepsNoReplyTypes(void) +{ + ChannelOpenHarness harness; + WOLFSSH_CHANNEL* channel; + byte tail[32]; + word32 tailSz; + byte in[128]; + word32 inSz; + + ResetAnyReqCb(WOLFSSH_REQ_REJECT); + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetChannelReqAnyCb(harness.ctx, + RecordingChannelReqAnyCb), WS_SUCCESS); + channel = SeedConfirmedSessionChannel(&harness); + + tailSz = AppendUint32(tail, sizeof(tail), 0, 7); + inSz = BuildChannelRequestPacket(channel->channel, "exit-status", 1, + tail, tailSz, in, sizeof(in)); + + /* Refused, so the status is not taken, and still nothing is sent. */ + AssertIntEQ(ReplyToPacket(&harness, in, inSz), 0); + AssertIntEQ(anyReqCbCalls, 1); + AssertIntEQ(anyReqCbWantReply, 1); + AssertIntEQ(harness.ssh->exitStatus, 0); + + anyReqCbReturn = WOLFSSH_REQ_ACCEPT; + AssertIntEQ(ReplyToPacket(&harness, in, inSz), 0); + AssertIntEQ(harness.ssh->exitStatus, 7); + + FreeChannelOpenHarness(&harness); + +#if defined(WOLFSSH_SHELL) && defined(WOLFSSH_TERM) + /* window-change is refused without a pty on the channel, whatever the + * callback said, and is unanswered either way. */ + ResetAnyReqCb(WOLFSSH_REQ_ACCEPT); + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetChannelReqAnyCb(harness.ctx, + RecordingChannelReqAnyCb), WS_SUCCESS); + channel = SeedConfirmedSessionChannel(&harness); + + tailSz = AppendUint32(tail, sizeof(tail), 0, 100); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 40); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 0); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 0); + inSz = BuildChannelRequestPacket(channel->channel, "window-change", 1, + tail, tailSz, in, sizeof(in)); + + AssertIntEQ(ReplyToPacket(&harness, in, inSz), 0); + AssertIntEQ(anyReqCbCalls, 1); + AssertIntEQ(harness.ssh->widthChar, 0); + + /* With a pty on it the grant stands, and the resize is taken. */ + channel->ptyReq = 1; + AssertIntEQ(ReplyToPacket(&harness, in, inSz), 0); + AssertIntEQ(harness.ssh->widthChar, 100); + AssertIntEQ(harness.ssh->heightRows, 40); + + FreeChannelOpenHarness(&harness); +#endif /* WOLFSSH_SHELL && WOLFSSH_TERM */ +} +#endif /* WOLFSSH_TERM || WOLFSSH_SHELL */ + +/* The generic global request callback sees the name, the type-specific + * part and whether a reply is wanted, and its answer is the reply. Left + * unhandled, a name nothing else answers is refused as before. */ +static void TestGlobalReqCallbackSettlesRequest(void) +{ + ChannelOpenHarness harness; + static const byte tail[] = { 7, 8 }; + byte in[128]; + word32 inSz; + int cbCtx = 0; + + ResetAnyReqCb(WOLFSSH_REQ_ACCEPT); + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetGlobalReqAnyCb(harness.ctx, + RecordingGlobalReqAnyCb), WS_SUCCESS); + wolfSSH_SetGlobalReqCtx(harness.ssh, &cbCtx); + + inSz = BuildGlobalRequestPacket("keepalive@openssh.com", 1, + tail, sizeof(tail), in, sizeof(in)); + + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_SUCCESS); + AssertIntEQ(anyReqCbCalls, 1); + AssertIntEQ(WSTRCMP(anyReqCbName, "keepalive@openssh.com"), 0); + AssertIntEQ(anyReqCbDataSz, sizeof(tail)); + AssertIntEQ(WMEMCMP(anyReqCbData, tail, sizeof(tail)), 0); + AssertIntEQ(anyReqCbWantReply, 1); + AssertTrue(anyReqCbCtx == &cbCtx); + + anyReqCbReturn = WOLFSSH_REQ_REJECT; + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_FAILURE); + + anyReqCbReturn = WOLFSSH_REQ_UNHANDLED; + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_FAILURE); + + /* No reply wanted, none sent, whatever the answer. */ + anyReqCbReturn = WOLFSSH_REQ_REJECT; + inSz = BuildGlobalRequestPacket("keepalive@openssh.com", 0, + tail, sizeof(tail), in, sizeof(in)); + AssertIntEQ(ReplyToPacket(&harness, in, inSz), 0); + AssertIntEQ(anyReqCbWantReply, 0); + + FreeChannelOpenHarness(&harness); +} + +/* The same for a global request: the name arrives whole, past the length + * of the copy the callback below it is handed. */ +static void TestGlobalReqCallbackSeesWholeName(void) +{ + ChannelOpenHarness harness; + char longName[100]; + word32 longNameSz = (word32)sizeof(longName); + byte in[192]; + word32 inSz; + + WMEMSET(longName, 'a', sizeof(longName)); + longName[0] = 'x'; + + ResetAnyReqCb(WOLFSSH_REQ_REJECT); + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetGlobalReqAnyCb(harness.ctx, + RecordingGlobalReqAnyCb), WS_SUCCESS); + + inSz = BuildGlobalRequestPacketSz(longName, longNameSz, 1, NULL, 0, + in, sizeof(in)); + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_FAILURE); + AssertIntEQ(anyReqCbNameSz, longNameSz); + AssertIntEQ(WMEMCMP(anyReqCbName, longName, longNameSz), 0); + + inSz = BuildGlobalRequestPacketSz("", 0, 1, NULL, 0, in, sizeof(in)); + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_FAILURE); + AssertIntEQ(anyReqCbNameSz, 0); + + FreeChannelOpenHarness(&harness); +} + +/* The generic callback sits ahead of the older global request callback, + * which still answers what it leaves unhandled. The two are handed + * different names: the older one has always seen a NUL terminated copy, + * truncated to its buffer, where the generic one sees the name whole. */ +static void TestGlobalReqCallbackLeavesLegacyCallback(void) +{ + ChannelOpenHarness harness; + char longName[100]; + word32 longNameSz = (word32)sizeof(longName); + word32 copySz = (word32)sizeof(legacyGlobalReqCbName) - 1; + byte in[192]; + word32 inSz; + + WMEMSET(longName, 'a', sizeof(longName)); + longName[0] = 'x'; + + ResetAnyReqCb(WOLFSSH_REQ_UNHANDLED); + legacyGlobalReqCbCalls = 0; + legacyGlobalReqCbReturn = WS_SUCCESS; + InitChannelOpenHarness(&harness, NULL, 0); + wolfSSH_SetGlobalReq(harness.ctx, RecordingGlobalReqCb); + AssertIntEQ(wolfSSH_CTX_SetGlobalReqAnyCb(harness.ctx, + RecordingGlobalReqAnyCb), WS_SUCCESS); + wolfSSH_SetGlobalReqCtx(harness.ssh, &harness); + + /* Left unhandled, the request carries on to the older callback, which + * answers it. */ + inSz = BuildGlobalRequestPacketSz(longName, longNameSz, 1, NULL, 0, + in, sizeof(in)); + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_SUCCESS); + AssertIntEQ(anyReqCbCalls, 1); + AssertIntEQ(legacyGlobalReqCbCalls, 1); + AssertIntEQ(legacyGlobalReqCbWantReply, 1); + + /* Both read the request context. */ + AssertTrue(anyReqCbCtx == (void*)&harness); + AssertTrue(legacyGlobalReqCbCtx == (void*)&harness); + + /* The whole name to the generic callback, the truncated copy to the + * older one. */ + AssertIntEQ(anyReqCbNameSz, longNameSz); + AssertIntEQ(WMEMCMP(anyReqCbName, longName, longNameSz), 0); + AssertIntEQ(legacyGlobalReqCbNameSz, copySz); + AssertIntEQ((word32)WSTRLEN(legacyGlobalReqCbName), copySz); + AssertIntEQ(WMEMCMP(legacyGlobalReqCbName, longName, copySz), 0); + + /* The older callback's return decides the reply. */ + legacyGlobalReqCbReturn = WS_FATAL_ERROR; + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_FAILURE); + AssertIntEQ(legacyGlobalReqCbCalls, 2); + legacyGlobalReqCbReturn = WS_SUCCESS; + + /* Settled by the generic callback, the older one hears nothing. */ + anyReqCbReturn = WOLFSSH_REQ_ACCEPT; + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_SUCCESS); + AssertIntEQ(legacyGlobalReqCbCalls, 2); + + anyReqCbReturn = WOLFSSH_REQ_REJECT; + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_FAILURE); + AssertIntEQ(legacyGlobalReqCbCalls, 2); + + FreeChannelOpenHarness(&harness); +} + +/* A port-0 tcpip-forward is not the generic callback's to answer, since + * only the forward callback can report the port bound. It is not asked at + * all, rather than asked and overruled after it may have bound a + * listener. None of this is a WOLFSSH_FWD matter: the callback can be set + * in any build, and a build without forwarding has to refuse the request + * rather than answer it without a port. */ +static void TestGlobalReqCallbackSkipsPortZeroForward(void) +{ + ChannelOpenHarness harness; + byte tail[32]; + word32 tailSz; + byte in[128]; + word32 inSz; + + ResetAnyReqCb(WOLFSSH_REQ_ACCEPT); + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetGlobalReqAnyCb(harness.ctx, + RecordingGlobalReqAnyCb), WS_SUCCESS); + + /* The callback never sees it, and with no forward callback to bind it + * the request is refused. */ + tailSz = AppendString(tail, sizeof(tail), 0, "localhost"); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 0); + inSz = BuildGlobalRequestPacket("tcpip-forward", 1, tail, tailSz, + in, sizeof(in)); + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_FAILURE); + AssertIntEQ(anyReqCbCalls, 0); + + /* One that ends before the port has no port to read, so it is left + * alone the same way. */ + tailSz = AppendString(tail, sizeof(tail), 0, "localhost"); + inSz = BuildGlobalRequestPacket("tcpip-forward", 1, tail, tailSz, + in, sizeof(in)); + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_FAILURE); + AssertIntEQ(anyReqCbCalls, 0); + + /* A port the peer picked is the callback's to grant. */ + tailSz = AppendString(tail, sizeof(tail), 0, "localhost"); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 8080); + inSz = BuildGlobalRequestPacket("tcpip-forward", 1, tail, tailSz, + in, sizeof(in)); + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_SUCCESS); + AssertIntEQ(anyReqCbCalls, 1); + AssertIntEQ(WSTRCMP(anyReqCbName, "tcpip-forward"), 0); + + /* The skip is for tcpip-forward alone: a port-0 cancel still reaches + * the callback. */ + tailSz = AppendString(tail, sizeof(tail), 0, "localhost"); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 0); + inSz = BuildGlobalRequestPacket("cancel-tcpip-forward", 1, tail, tailSz, + in, sizeof(in)); + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_SUCCESS); + AssertIntEQ(anyReqCbCalls, 2); + + FreeChannelOpenHarness(&harness); +} + +#ifdef WOLFSSH_FWD +/* A tcpip-forward the generic callback grants is answered without the + * forward callback, so a forward can be set up from either. A port-0 + * request is the exception: only the forward callback can report the + * port bound, so a grant there is refused. */ +static void TestGlobalReqCallbackAnswersTcpipForward(void) +{ + ChannelOpenHarness harness; + byte tail[32]; + word32 tailSz; + byte in[128]; + word32 inSz; + + ResetAnyReqCb(WOLFSSH_REQ_ACCEPT); + fwdCbCallCount = 0; + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetGlobalReqAnyCb(harness.ctx, + RecordingGlobalReqAnyCb), WS_SUCCESS); + AssertIntEQ(wolfSSH_CTX_SetFwdCb(harness.ctx, CountingFwdCb, NULL), + WS_SUCCESS); + + tailSz = AppendString(tail, sizeof(tail), 0, "localhost"); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 8080); + inSz = BuildGlobalRequestPacket("tcpip-forward", 1, tail, tailSz, + in, sizeof(in)); + + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_SUCCESS); + AssertIntEQ(anyReqCbCalls, 1); + AssertIntEQ(WSTRCMP(anyReqCbName, "tcpip-forward"), 0); + AssertIntEQ(fwdCbCallCount, 0); + + anyReqCbReturn = WOLFSSH_REQ_UNHANDLED; + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_SUCCESS); + AssertIntEQ(fwdCbCallCount, 1); + + /* A port-0 request goes to the forward callback whatever the policy + * would say, since only that callback can report the port bound. The + * generic callback is not asked. This one reports no port, so the + * request is refused and the setup undone, per RFC 4254 7.1. */ + ResetAnyReqCb(WOLFSSH_REQ_ACCEPT); + tailSz = AppendString(tail, sizeof(tail), 0, "localhost"); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 0); + inSz = BuildGlobalRequestPacket("tcpip-forward", 1, tail, tailSz, + in, sizeof(in)); + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_FAILURE); + AssertIntEQ(anyReqCbCalls, 0); + AssertIntEQ(fwdCbCallCount, 3); /* the setup, then the cleanup */ + + /* A tcpip-forward that ends before the port has no port to read, so + * it is left alone the same way. */ + tailSz = AppendString(tail, sizeof(tail), 0, "localhost"); + inSz = BuildGlobalRequestPacket("tcpip-forward", 1, tail, tailSz, + in, sizeof(in)); + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_FAILURE); + AssertIntEQ(anyReqCbCalls, 0); + + /* The port-0 check is for tcpip-forward alone: a granted cancel is + * answered as asked, and the forward callback hears nothing. */ + tailSz = AppendString(tail, sizeof(tail), 0, "localhost"); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 0); + inSz = BuildGlobalRequestPacket("cancel-tcpip-forward", 1, tail, tailSz, + in, sizeof(in)); + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_SUCCESS); + AssertIntEQ(WSTRCMP(anyReqCbName, "cancel-tcpip-forward"), 0); + AssertIntEQ(fwdCbCallCount, 3); + + /* Left unhandled, the same cancel reaches the forward callback. */ + anyReqCbReturn = WOLFSSH_REQ_UNHANDLED; + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_SUCCESS); + AssertIntEQ(fwdCbCallCount, 4); + + FreeChannelOpenHarness(&harness); +} +/* A port-0 forward still works with the generic callback registered. The + * callback is passed over so the forward callback can report the port it + * bound, and the reply carries that port, per RFC 4254 7.1. Refusing a + * port-0 request outright rather than passing it over would break this. */ +static int portReportingFwdCbPort; + +static int PortReportingFwdCb(WS_FwdCbAction action, void* ctx, + const char* host, word32 port) +{ + (void)ctx; + (void)host; + (void)port; + + fwdCbCallCount++; + if (action == WOLFSSH_FWD_REMOTE_SETUP) + return portReportingFwdCbPort; + + return WS_SUCCESS; +} + +static void TestGlobalReqCallbackKeepsPortZeroForwardWorking(void) +{ + ChannelOpenHarness harness; + byte tail[32]; + word32 tailSz; + byte in[128]; + word32 inSz; + + portReportingFwdCbPort = 49152; + fwdCbCallCount = 0; + ResetAnyReqCb(WOLFSSH_REQ_ACCEPT); + InitChannelOpenHarness(&harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetGlobalReqAnyCb(harness.ctx, + RecordingGlobalReqAnyCb), WS_SUCCESS); + AssertIntEQ(wolfSSH_CTX_SetFwdCb(harness.ctx, PortReportingFwdCb, NULL), + WS_SUCCESS); + + tailSz = AppendString(tail, sizeof(tail), 0, "localhost"); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 0); + inSz = BuildGlobalRequestPacket("tcpip-forward", 1, tail, tailSz, + in, sizeof(in)); + + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_SUCCESS); + AssertIntEQ(anyReqCbCalls, 0); + AssertIntEQ(fwdCbCallCount, 1); + + FreeChannelOpenHarness(&harness); +} +#endif /* WOLFSSH_FWD */ + /* What a length-aware session request callback saw. */ static word32 sessionReqCbCommandSz; static word32 sessionReqCbCommandStrLen; @@ -5916,6 +6771,64 @@ static void TestAgentChannelOpenOnClientRefused(void) #endif /* NO_WOLFSSH_SERVER */ +#ifndef NO_WOLFSSH_CLIENT +/* RFC 4254 7.1: a remote forward is the client's to ask for, so a client + * refuses tcpip-forward and cancel-tcpip-forward rather than register one + * on the peer's say-so. The refusal comes before any policy callback, and + * matches the name as it arrived, so it holds in a build without + * forwarding, where neither name is in the name table. */ +static int clientFwdPolicyCalls; + +static int AcceptingGlobalReqAnyCb(WOLFSSH* ssh, const byte* name, + word32 nameSz, const byte* data, word32 dataSz, int wantReply, + void* ctx) +{ + WOLFSSH_UNUSED(ssh); + WOLFSSH_UNUSED(name); + WOLFSSH_UNUSED(nameSz); + WOLFSSH_UNUSED(data); + WOLFSSH_UNUSED(dataSz); + WOLFSSH_UNUSED(wantReply); + WOLFSSH_UNUSED(ctx); + + clientFwdPolicyCalls++; + return WOLFSSH_REQ_ACCEPT; +} + +static void TestClientRefusesForwardRequestsBeforePolicy(void) +{ + const char* names[2]; + int i; + + names[0] = "tcpip-forward"; + names[1] = "cancel-tcpip-forward"; + + for (i = 0; i < 2; i++) { + ChannelOpenHarness harness; + byte tail[32]; + word32 tailSz; + byte in[128]; + word32 inSz; + + clientFwdPolicyCalls = 0; + InitChannelOpenHarnessClient(&harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetGlobalReqAnyCb(harness.ctx, + AcceptingGlobalReqAnyCb), WS_SUCCESS); + + tailSz = AppendString(tail, sizeof(tail), 0, "localhost"); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 8080); + inSz = BuildGlobalRequestPacket(names[i], 1, tail, tailSz, + in, sizeof(in)); + + AssertIntEQ(ReplyToPacket(&harness, in, inSz), MSGID_REQUEST_FAILURE); + AssertIntEQ(clientFwdPolicyCalls, 0); + + FreeChannelOpenHarness(&harness); + } +} +#endif /* !NO_WOLFSSH_CLIENT */ + + #if defined(WOLFSSH_AGENT) && !defined(WOLFSSH_NO_ED25519) \ && !defined(NO_WOLFSSH_CLIENT) @@ -14891,6 +15804,26 @@ int main(int argc, char** argv) TestServerServiceRequestRejectedDuringKeying(); TestFailedSendClearsPendingPlaintext(); TestChannelOpenCallbackRejectSendsOpenFail(); + TestChannelReqCallbackSeesRequestAndRefuses(); + TestChannelReqCallbackGrantsUnknownType(); + TestChannelReqCallbackSeesWholeType(); + TestChannelReqCallbackSettlesSessionRequest(); + TestChannelReqCallbackGrantOverridesAppChannels(); + TestReqAnyCallbackSettersRejectNullCtx(); +#ifdef WOLFSSH_TERM + TestChannelReqCallbackKeepsTypeChecks(); +#endif +#if defined(WOLFSSH_TERM) || defined(WOLFSSH_SHELL) + TestChannelReqCallbackKeepsNoReplyTypes(); +#endif + TestGlobalReqCallbackSettlesRequest(); + TestGlobalReqCallbackSeesWholeName(); + TestGlobalReqCallbackLeavesLegacyCallback(); + TestGlobalReqCallbackSkipsPortZeroForward(); +#ifdef WOLFSSH_FWD + TestGlobalReqCallbackAnswersTcpipForward(); + TestGlobalReqCallbackKeepsPortZeroForwardWorking(); +#endif TestChannelOpenConfCallbackRuns(); TestChannelOpenFailCallbackRuns(); TestChannelOpenConfCallbackRejects(); @@ -14974,6 +15907,9 @@ int main(int argc, char** argv) #endif #endif #endif /* NO_WOLFSSH_SERVER */ +#ifndef NO_WOLFSSH_CLIENT + TestClientRefusesForwardRequestsBeforePolicy(); +#endif #if defined(WOLFSSH_AGENT) && !defined(WOLFSSH_NO_ED25519) \ && !defined(NO_WOLFSSH_CLIENT) TestAgentEd25519UserAuthEmitsSignature(); diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 706c6fc51..a18c8b429 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -856,6 +856,7 @@ struct WOLFSSH_CTX { WS_CallbackUserAuthResult userAuthResultCb; /* User Authentication Result */ WS_CallbackHighwater highwaterCb; /* Data Highwater Mark Callback */ WS_CallbackGlobalReq globalReqCb; /* Global Request Callback */ + WS_CallbackGlobalReqAny globalReqAnyCb; /* Global Request, any name */ WS_CallbackReqSuccess reqSuccessCb; /* Global Request Success Callback */ WS_CallbackReqSuccess reqFailureCb; /* Global Request Failure Callback */ WS_CallbackChannelOpen channelOpenCb; /* Channel Open Requested */ @@ -864,6 +865,7 @@ struct WOLFSSH_CTX { WS_CallbackChannelReq channelReqShellCb; /* Channel Request "Shell" */ WS_CallbackChannelReq channelReqExecCb; /* Channel Request "Exec" */ WS_CallbackChannelReq channelReqSubsysCb; /* Channel Request "Subsystem" */ + WS_CallbackChannelReqAny channelReqAnyCb; /* Channel Request, any */ WS_CallbackChannelEof channelEofCb; /* Channel Eof Callback */ WS_CallbackChannelClose channelCloseCb; /* Channel Close Callback */ #ifdef WOLFSSH_SCP diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index e2e4b4c77..b034ce30c 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -475,6 +475,45 @@ WOLFSSH_API int wolfSSH_CTX_SetChannelReqSubsysCb(WOLFSSH_CTX* ctx, WOLFSSH_API int wolfSSH_SetChannelReqCtx(WOLFSSH* ssh, void* ctx); WOLFSSH_API void* wolfSSH_GetChannelReqCtx(WOLFSSH* ssh); +/* What a request callback decides. UNHANDLED is what a missing callback + * answers, and leaves the request to the built-in handling. + * + * Note that 0 is UNHANDLED here, where the older request callbacks above + * read a 0 return as acceptance. A callback of this family returns one of + * these three and not WS_SUCCESS: returning 0 out of habit leaves the + * request to the handling below, which refuses a type the library does + * not know. */ +typedef enum WS_ReqCbResult { + WOLFSSH_REQ_UNHANDLED = 0, + WOLFSSH_REQ_ACCEPT, + WOLFSSH_REQ_REJECT +} WS_ReqCbResult; + +/* Consulted first for every channel request, ahead of the three callbacks + * above and of the built-in handling, so a request with no callback of its + * own -- env, pty-req, window-change, exit-status, auth-agent-req, or a + * type the library does not know -- can be granted or refused by policy. + * type is the request name as it arrived, typeSz bytes, and data is the + * request's type-specific part, dataSz bytes, for the callback to parse. + * Neither is NUL terminated, and a name may hold any byte, so a policy + * matches on typeSz bytes rather than with the string functions. + * wantReply is what the peer asked for, before the library clears it for + * the types RFC 4254 never answers. + * + * ACCEPT and REJECT settle the request, and the shell, exec and subsystem + * callbacks are not consulted. The library still parses and records what + * it needs from a request it knows, so a session request accepted here + * sets the channel's session type and the modes of an accepted pty-req are + * kept; a request that does not fit its type is refused whatever the + * callback said. A type the library does not know is answered + * CHANNEL_SUCCESS on ACCEPT, where it is otherwise refused. Shares the + * channel request context. */ +typedef int (*WS_CallbackChannelReqAny)(WOLFSSH_CHANNEL* channel, + const byte* type, word32 typeSz, const byte* data, word32 dataSz, + int wantReply, void* ctx); +WOLFSSH_API int wolfSSH_CTX_SetChannelReqAnyCb(WOLFSSH_CTX* ctx, + WS_CallbackChannelReqAny cb); + /* Application-driven channel handling, server side, off by default. * * Off, wolfSSH_accept() runs the session state machine through to an @@ -546,6 +585,40 @@ WOLFSSH_API void wolfSSH_SetGlobalReq(WOLFSSH_CTX* ctx, WS_CallbackGlobalReq cb); WOLFSSH_API void wolfSSH_SetGlobalReqCtx(WOLFSSH* ssh, void* ctx); WOLFSSH_API void *wolfSSH_GetGlobalReqCtx(WOLFSSH* ssh); +/* Consulted first for a global request, ahead of the forward callback + * that answers tcpip-forward and cancel-tcpip-forward and of the callback + * above that answers the rest, but for the two requests named below. + * name is the request name as it arrived, nameSz bytes, and data is the + * request's type-specific part, dataSz bytes, for the callback to parse, + * so a tcpip-forward naming a port can be set up from here without a + * forward callback. Neither is NUL terminated, and a name may hold any + * byte, so a policy matches on nameSz bytes rather than with the string + * functions. UNHANDLED leaves the request to those callbacks. ACCEPT and + * REJECT settle it, and no other callback is consulted; the reply, when + * one is wanted, is REQUEST_SUCCESS or REQUEST_FAILURE. + * + * Two requests never reach this callback, both per RFC 4254 7.1. A + * tcpip-forward asking for port 0 is answered with the port bound, which + * only the forward callback can report, so it is left to the handling + * that can bind it; so is one whose body does not parse, having no port + * to read. Were a policy asked about either, a grant would have to be + * refused once the policy had already bound a listener. And a client + * answers tcpip-forward and cancel-tcpip-forward with a failure, in any + * build, whatever a policy would make of them. + * + * Shares the global request context. + * + * name and data point into the session's input buffer and are good only + * for the length of the call, so a callback keeping either copies it. + * The packet is still being parsed, so the callback must not re-enter + * the receive side of the library on this session -- wolfSSH_worker(), + * wolfSSH_stream_read(), wolfSSH_accept(), the SFTP calls -- which may + * grow or compact that buffer and leave both pointers behind. */ +typedef int (*WS_CallbackGlobalReqAny)(WOLFSSH* ssh, const byte* name, + word32 nameSz, const byte* data, word32 dataSz, int wantReply, + void* ctx); +WOLFSSH_API int wolfSSH_CTX_SetGlobalReqAnyCb(WOLFSSH_CTX* ctx, + WS_CallbackGlobalReqAny cb); typedef int (*WS_CallbackReqSuccess)(WOLFSSH* ssh, void* buf, word32 sz, void* ctx); WOLFSSH_API void wolfSSH_SetReqSuccess(WOLFSSH_CTX* ctx, From e0b114e7cdba0f3c0a5252fe6d10f9bd2aec78b8 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 14 Sep 2026 08:37:01 -0700 Subject: [PATCH 2/2] internal: re-find the channel after the callback The generic channel request callback may free the channel it was handed, so DoChannelRequest() looks it up again before the type handling reads it. Gone, the request ends there, and a reply the peer wanted fails on the missing channel the way one after a typed session callback does. - ssh.h says the callback may free its channel and what the request does from there - regress.c frees the channel from the callback on each of the three answers, and on a pty-req, the type that wrote to the channel outside a session request --- src/internal.c | 12 +++++- tests/regress.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++ wolfssh/ssh.h | 14 ++++++- 3 files changed, 125 insertions(+), 2 deletions(-) diff --git a/src/internal.c b/src/internal.c index 6e3eb28d7..ea5c4dc12 100644 --- a/src/internal.c +++ b/src/internal.c @@ -13321,6 +13321,16 @@ static int DoChannelRequest(WOLFSSH* ssh, else if (decision == WOLFSSH_REQ_ACCEPT) { granted = 1; } + + /* A callback may free its own channel, so look it up again + * before the handling below reads it. Gone, the request ends + * here, and a wanted reply fails on the missing channel the + * way one after a typed callback does. */ + channel = ChannelFind(ssh, channelId, WS_CHANNEL_ID_SELF); + if (channel == NULL) { + WLOG(WS_LOG_DEBUG, + " channel request callback freed the channel."); + } } } @@ -13342,7 +13352,7 @@ static int DoChannelRequest(WOLFSSH* ssh, } #endif - if (ret == WS_SUCCESS && !rej) { + if (ret == WS_SUCCESS && !rej && channel != NULL) { if (ChannelRequestIs(type, typeSz, "env")) { char name[WOLFSSH_MAX_NAMESZ]; word32 nameSz; diff --git a/tests/regress.c b/tests/regress.c index b4513a799..1592f2b81 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -4345,6 +4345,106 @@ static void TestChannelReqCallbackGrantOverridesAppChannels(void) FreeChannelOpenHarness(&harness); } +/* A generic channel request callback that frees the channel it was handed, + * the way a policy that tears the channel down in place does, and answers + * anyReqCbReturn. */ +static int FreeingChannelReqAnyCb(WOLFSSH_CHANNEL* channel, const byte* type, + word32 typeSz, const byte* data, word32 dataSz, int wantReply, + void* ctx) +{ + RecordAnyReq(type, typeSz, data, dataSz, ctx); + anyReqCbWantReply = wantReply; + AssertIntEQ(wolfSSH_ChannelFree(channel), WS_SUCCESS); + + return anyReqCbReturn; +} + +/* Runs one request through a fresh harness whose generic callback frees the + * channel and answers anyReturn, and returns what DoReceive() made of it. + * The channel id is handed back, since the channel itself is gone. */ +static int RunRequestThroughFreeingCb(ChannelOpenHarness* harness, + word32* channelId, const char* type, byte wantReply, + const byte* tail, word32 tailSz, int anyReturn) +{ + WOLFSSH_CHANNEL* channel; + byte in[192]; + word32 inSz; + + ResetAnyReqCb(anyReturn); + typedReqCbCalls = 0; + InitChannelOpenHarness(harness, NULL, 0); + AssertIntEQ(wolfSSH_CTX_SetChannelReqAnyCb(harness->ctx, + FreeingChannelReqAnyCb), WS_SUCCESS); + AssertIntEQ(wolfSSH_CTX_SetChannelReqExecCb(harness->ctx, + CountingSessionReqCb), WS_SUCCESS); + channel = SeedConfirmedSessionChannel(harness); + *channelId = channel->channel; + + inSz = BuildChannelRequestPacket(*channelId, type, wantReply, + tail, tailSz, in, sizeof(in)); + RepointHarnessInput(harness, in, inSz); + + return DoReceive(harness->ssh); +} + +/* The generic callback may free the channel it was called on. Nothing below + * it reads the channel again after that, and the request ends there: the + * type is not handled, no typed callback runs, and a wanted reply fails on + * the channel that is gone. */ +static void TestChannelReqCallbackMayFreeChannel(void) +{ + ChannelOpenHarness harness; + word32 channelId; + byte tail[32]; + word32 tailSz; + + /* Left unhandled, the exec handling would read the channel next. */ + tailSz = AppendString(tail, sizeof(tail), 0, "ls"); + AssertIntEQ(RunRequestThroughFreeingCb(&harness, &channelId, "exec", 1, + tail, tailSz, WOLFSSH_REQ_UNHANDLED), WS_FATAL_ERROR); + AssertIntEQ(harness.ssh->error, WS_INVALID_CHANID); + AssertIntEQ(anyReqCbCalls, 1); + AssertIntEQ(typedReqCbCalls, 0); + AssertIntEQ(harness.io.outSz, 0); + AssertNull(ChannelFind(harness.ssh, channelId, WS_CHANNEL_ID_SELF)); + FreeChannelOpenHarness(&harness); + + /* Granted, the same request would have committed the session on it. */ + AssertIntEQ(RunRequestThroughFreeingCb(&harness, &channelId, "exec", 1, + tail, tailSz, WOLFSSH_REQ_ACCEPT), WS_FATAL_ERROR); + AssertIntEQ(harness.ssh->error, WS_INVALID_CHANID); + AssertIntEQ(typedReqCbCalls, 0); + AssertTrue(harness.ssh->clientState < CLIENT_DONE); + AssertNull(ChannelFind(harness.ssh, channelId, WS_CHANNEL_ID_SELF)); + FreeChannelOpenHarness(&harness); + + /* Refused, the handling was skipped whatever the channel did. */ + AssertIntEQ(RunRequestThroughFreeingCb(&harness, &channelId, "exec", 1, + tail, tailSz, WOLFSSH_REQ_REJECT), WS_FATAL_ERROR); + AssertIntEQ(harness.ssh->error, WS_INVALID_CHANID); + AssertIntEQ(typedReqCbCalls, 0); + FreeChannelOpenHarness(&harness); + +#ifdef WOLFSSH_TERM + /* With no reply wanted there is nothing left to fail, so the packet is + * taken and the session carries on. A pty-req is the case that wrote to + * the channel outside a session request. */ + tailSz = AppendString(tail, sizeof(tail), 0, "vt100"); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 80); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 24); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 0); + tailSz = AppendUint32(tail, sizeof(tail), tailSz, 0); + tailSz = AppendString(tail, sizeof(tail), tailSz, ""); + AssertIntEQ(RunRequestThroughFreeingCb(&harness, &channelId, "pty-req", 0, + tail, tailSz, WOLFSSH_REQ_UNHANDLED), WS_SUCCESS); + AssertIntEQ(harness.ssh->error, WS_SUCCESS); + AssertIntEQ(anyReqCbCalls, 1); + AssertIntEQ(harness.io.outSz, 0); + AssertNull(ChannelFind(harness.ssh, channelId, WS_CHANNEL_ID_SELF)); + FreeChannelOpenHarness(&harness); +#endif /* WOLFSSH_TERM */ +} + /* Both setters answer a NULL context, the only error either has. */ static void TestReqAnyCallbackSettersRejectNullCtx(void) { @@ -15809,6 +15909,7 @@ int main(int argc, char** argv) TestChannelReqCallbackSeesWholeType(); TestChannelReqCallbackSettlesSessionRequest(); TestChannelReqCallbackGrantOverridesAppChannels(); + TestChannelReqCallbackMayFreeChannel(); TestReqAnyCallbackSettersRejectNullCtx(); #ifdef WOLFSSH_TERM TestChannelReqCallbackKeepsTypeChecks(); diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index b034ce30c..4a5405b67 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -507,7 +507,19 @@ typedef enum WS_ReqCbResult { * kept; a request that does not fit its type is refused whatever the * callback said. A type the library does not know is answered * CHANNEL_SUCCESS on ACCEPT, where it is otherwise refused. Shares the - * channel request context. */ + * channel request context. + * + * The callback may free the channel it was handed, with + * wolfSSH_ChannelFree(). The request ends there whatever the answer: the + * type is not handled, and a request wanting a reply has nothing left to + * answer on, so it fails with WS_INVALID_CHANID. + * + * type and data point into the session's input buffer and are good only + * for the length of the call, so a callback keeping either copies it. + * The packet is still being parsed, so the callback must not re-enter + * the receive side of the library on this session -- wolfSSH_worker(), + * wolfSSH_stream_read(), wolfSSH_accept(), the SFTP calls -- which may + * grow or compact that buffer and leave both pointers behind. */ typedef int (*WS_CallbackChannelReqAny)(WOLFSSH_CHANNEL* channel, const byte* type, word32 typeSz, const byte* data, word32 dataSz, int wantReply, void* ctx);