Skip to content

Guard remote forwards on a client, disconnect on a disallowed message #1214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
philljj merged 1 commit into from
Sep 1, 2026
Merged

Guard remote forwards on a client, disconnect on a disallowed message #1214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 84 additions & 1 deletion src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11617,6 +11617,22 @@ 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)
&& 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) {
#ifdef WOLFSSH_FWD
case ID_GLOBREQ_TCPIP_FWD:
Expand Down Expand Up @@ -12928,6 +12944,57 @@ static int DoChannelExtendedData(WOLFSSH* ssh,
}


/* Has DoPacket()'s dispatch a case for this id? Keep in step with it, guards
* included: a message compiled out is one this build does not recognize. */
INLINE static int MsgIdKnown(byte msg)
{
switch (msg) {
case MSGID_DISCONNECT:
case MSGID_IGNORE:
case MSGID_UNIMPLEMENTED:
case MSGID_REQUEST_SUCCESS:
case MSGID_REQUEST_FAILURE:
case MSGID_DEBUG:
case MSGID_EXT_INFO:
case MSGID_KEXINIT:
case MSGID_NEWKEYS:
case MSGID_KEXDH_INIT:
case MSGID_KEXDH_REPLY:
#ifndef WOLFSSH_NO_DH_GEX_SHA256
case MSGID_KEXDH_GEX_REQUEST:
#endif
case MSGID_KEXDH_GEX_INIT:
case MSGID_KEXDH_GEX_REPLY:
case MSGID_SERVICE_REQUEST:
case MSGID_SERVICE_ACCEPT:
case MSGID_USERAUTH_REQUEST:
#ifdef WOLFSSH_KEYBOARD_INTERACTIVE
case MSGID_USERAUTH_INFO_RESPONSE:
case MSGID_USERAUTH_INFO_REQUEST:
#endif
case MSGID_USERAUTH_FAILURE:
case MSGID_USERAUTH_SUCCESS:
case MSGID_USERAUTH_BANNER:
case MSGID_GLOBAL_REQUEST:
case MSGID_CHANNEL_OPEN:
case MSGID_CHANNEL_OPEN_CONF:
case MSGID_CHANNEL_OPEN_FAIL:
case MSGID_CHANNEL_WINDOW_ADJUST:
case MSGID_CHANNEL_DATA:
case MSGID_CHANNEL_EXTENDED_DATA:
case MSGID_CHANNEL_EOF:
case MSGID_CHANNEL_CLOSE:
case MSGID_CHANNEL_REQUEST:
case MSGID_CHANNEL_SUCCESS:
case MSGID_CHANNEL_FAILURE:
return 1;

default:
return 0;
}
}


static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed)
{
byte* buf = (byte*)ssh->inputBuffer.buffer;
Expand All @@ -12939,6 +13006,7 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed)
byte padSz;
byte msg;
word32 payloadIdx = 0;
int msgAllowed;
int ret = WS_SUCCESS;

WLOG(WS_LOG_DEBUG, "DoPacket sequence number: %d", ssh->peerSeq);
Expand Down Expand Up @@ -12973,7 +13041,15 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed)
return WS_OVERFLOW_E;
}

if (!IsMessageAllowed(ssh, msg, WS_MSG_RECV)) {
msgAllowed = IsMessageAllowed(ssh, msg, WS_MSG_RECV);

if (!msgAllowed && (MsgIdKnown(msg) || MSGIDLIMIT_POST_USERAUTH(msg))) {
/* RFC 4252 section 6: disconnect on a known id at the wrong time,
* and on any id of 80 or higher, which IsMessageAllowed() refuses
* only before auth. Silent once over, RFC 4253 section 11.1. */
if (!ssh->disconnected) {
(void)SendDisconnect(ssh, WOLFSSH_DISCONNECT_PROTOCOL_ERROR);
}
return WS_MSGID_NOT_ALLOWED_E;
Comment thread
ejohnstown marked this conversation as resolved.
}

Expand All @@ -12987,6 +13063,12 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed)
WLOG(WS_LOG_DEBUG, "Ignoring message ID %u after a disconnect",
(word32)msg);
}
else if (!msgAllowed) {
/* Refused, unimplemented, below 80. Answered off the dispatch so a
* refused id cannot reach a handler if MsgIdKnown() drifts. */
WLOG(WS_LOG_DEBUG, "Unimplemented message ID (%d)", msg);
ret = SendUnimplemented(ssh);
}
else switch (msg) {

case MSGID_DISCONNECT:
Expand Down Expand Up @@ -13190,6 +13272,7 @@ static int DoPacket(WOLFSSH* ssh, byte* bufferConsumed)
break;

default:
/* RFC 4253 section 11.4, reached whatever the state allows. */
WLOG(WS_LOG_DEBUG, "Unimplemented message ID (%d)", msg);
#ifdef SHOW_UNIMPLEMENTED
DumpOctetString(buf + idx, payloadSz);
Expand Down
223 changes: 218 additions & 5 deletions tests/regress.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2297,8 +2297,9 @@ static void TestClientServiceAcceptBlockedDuringKeying(WOLFSSH* ssh)


/* Drive the whole receive path with a CHANNEL_OPEN sent from a pre-auth
* connectState: no channel created, no reply emitted. The connectState
* gate does the rejecting; isKeying below is scene-setting only. */
* connectState: no channel created, and the only thing sent back is the
* disconnect RFC 4252 section 6 asks for. The connectState gate does the
* rejecting; isKeying below is scene-setting only. */
static void TestChannelOpenRejectedBeforeKex(byte connectState)
{
WOLFSSH_CTX* ctx;
Expand Down Expand Up @@ -2330,7 +2331,11 @@ static void TestChannelOpenRejectedBeforeKex(byte connectState)
AssertIntEQ(ssh->error, WS_MSGID_NOT_ALLOWED_E);
AssertNull(ssh->channelList);
AssertIntEQ(ssh->channelListSz, 0);
AssertIntEQ(io.outSz, 0);
/* Not silence: the peer is told why the session ended, and the message
* is a disconnect rather than anything answering the channel open. */
AssertTrue(io.outSz > 0);
AssertIntEQ(ParseMsgId(io.out, io.outSz), MSGID_DISCONNECT);
AssertTrue(ssh->disconnected);

wolfSSH_free(ssh);
wolfSSH_CTX_free(ctx);
Expand Down Expand Up @@ -2403,6 +2408,103 @@ static void TestServerUserauthBlockedBeforeKeyed(WOLFSSH* ssh)
}


/* One packet fed to a keyed but unauthenticated server. The cases below
* differ only in the packet and the answer it draws. */
static void RunServerMsgIdAtKeyed(const byte* pkt, word32 pktSz,
int expectRet, int expectErr, byte expectReplyMsgId,
int expectDisconnected)
{
WOLFSSH_CTX* ctx;
WOLFSSH* ssh;
MemIo io;
byte out[256];

ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL);
AssertNotNull(ctx);
wolfSSH_SetIORecv(ctx, MemRecv);
wolfSSH_SetIOSend(ctx, MemSend);

ssh = wolfSSH_new(ctx);
AssertNotNull(ssh);

MemIoInit(&io, (byte*)pkt, pktSz, out, sizeof(out));
wolfSSH_SetIOReadCtx(ssh, &io);
wolfSSH_SetIOWriteCtx(ssh, &io);

/* Past the key exchange, short of user auth being answered. */
ssh->acceptState = ACCEPT_KEYED;

AssertIntEQ(wolfSSH_TestDoReceive(ssh), expectRet);
AssertIntEQ(ssh->error, expectErr);
/* Nothing the filter refuses may leave a channel behind. */
AssertNull(ssh->channelList);
AssertIntEQ(ssh->channelListSz, 0);
AssertTrue(io.outSz > 0);
AssertIntEQ(ParseMsgId(io.out, io.outSz), expectReplyMsgId);
AssertIntEQ(ssh->disconnected != 0, expectDisconnected);

wolfSSH_free(ssh);
wolfSSH_CTX_free(ctx);
}


/* The case RFC 4252 section 6 names: an id of 80 or higher before user auth.
* Keyed, so the post-userauth limit rejects it, not the pre-keyed range. */
static void TestServerHighMsgIdBeforeAuthDisconnects(void)
{
byte pkt[256];
word32 pktSz;

pktSz = BuildChannelOpenPacket("session", 0, 131072, 16384, NULL, 0,
pkt, sizeof(pkt));

RunServerMsgIdAtKeyed(pkt, pktSz, WS_FATAL_ERROR, WS_MSGID_NOT_ALLOWED_E,
MSGID_DISCONNECT, 1);
}


/* Id 79 is refused too, but it is unimplemented and below 80, so RFC 4253
* section 11.4 rules: UNIMPLEMENTED, and the session lives. */
static void TestServerUnknownMsgIdBeforeAuthUnimplemented(void)
{
byte pkt[256];
word32 pktSz;

pktSz = WrapPacket(79, NULL, 0, pkt, sizeof(pkt));

RunServerMsgIdAtKeyed(pkt, pktSz, WS_SUCCESS, WS_SUCCESS,
MSGID_UNIMPLEMENTED, 0);
}


/* Below 80 the dispatch decides: 53 has a case, so it disconnects where
* 79 does not. */
static void TestServerKnownAuthMsgIdBeforeAuthDisconnects(void)
{
byte pkt[256];
word32 pktSz;

pktSz = WrapPacket(MSGID_USERAUTH_BANNER, NULL, 0, pkt, sizeof(pkt));

RunServerMsgIdAtKeyed(pkt, pktSz, WS_FATAL_ERROR, WS_MSGID_NOT_ALLOWED_E,
MSGID_DISCONNECT, 1);
}


/* At 80 and above the range decides instead: id 200 disconnects though
* nothing dispatches it. */
static void TestServerUnknownHighMsgIdBeforeAuthDisconnects(void)
{
byte pkt[256];
word32 pktSz;

pktSz = WrapPacket(200, NULL, 0, pkt, sizeof(pkt));

RunServerMsgIdAtKeyed(pkt, pktSz, WS_FATAL_ERROR, WS_MSGID_NOT_ALLOWED_E,
MSGID_DISCONNECT, 1);
}


/* Reject the user auth messages that only the server sends, while still
* accepting the keyboard-interactive info response that it receives. */
static void TestServerOnlyUserauthMsgsBlocked(WOLFSSH* ssh)
Expand Down Expand Up @@ -2492,8 +2594,11 @@ static void TestServerServiceRequestRejectedDuringKeying(void)
/* Not dispatched, so acceptState did not advance. */
AssertIntEQ(ssh->clientState, CLIENT_BEGIN);
AssertIntEQ(ssh->acceptState, ACCEPT_KEYED);
/* Nothing emitted in reply. */
AssertIntEQ(io.outSz, 0);
/* Not silence: nothing answers the service request, but the peer is told
* the session ended on a protocol error. */
AssertTrue(io.outSz > 0);
AssertIntEQ(ParseMsgId(io.out, io.outSz), MSGID_DISCONNECT);
AssertTrue(ssh->disconnected);

/* Allowed when only this side has started a rekey. */
ssh->error = 0;
Expand Down Expand Up @@ -3077,6 +3182,104 @@ static void TestGlobalRequestFwdNoCbSendsFailure(void)
FreeChannelOpenHarness(&harness);
}

#ifndef NO_WOLFSSH_CLIENT
/* A client is the side that asks for a remote forward, so a tcpip-forward it
* receives is answered with a failure and never registered. RFC 4254 section
* 7.1. A fwdCb is registered throughout: without the role check that callback
* is the only gate, and it would answer success. */
static void TestGlobalRequestFwdOnClientSendsFailure(void)
{
ChannelOpenHarness harness;
byte in[256];
word32 inSz;
int ret;

inSz = BuildGlobalRequestFwdPacket("0.0.0.0", 2222, 0, 1, in, sizeof(in));
InitChannelOpenHarnessClient(&harness, in, inSz);
AssertIntEQ(wolfSSH_CTX_SetFwdCb(harness.ctx, AcceptFwdCb, NULL),
WS_SUCCESS);

ret = DoReceive(harness.ssh);

AssertIntEQ(ret, WS_SUCCESS);
AssertGlobalRequestReply(&harness, MSGID_REQUEST_FAILURE);

FreeChannelOpenHarness(&harness);
}

/* cancel-tcpip-forward travels the same direction, so a client refuses it on
* the same grounds. */
static void TestGlobalRequestFwdCancelOnClientSendsFailure(void)
{
ChannelOpenHarness harness;
byte in[256];
word32 inSz;
int ret;

inSz = BuildGlobalRequestFwdPacket("0.0.0.0", 2222, 1, 1, in, sizeof(in));
InitChannelOpenHarnessClient(&harness, in, inSz);
AssertIntEQ(wolfSSH_CTX_SetFwdCb(harness.ctx, AcceptFwdCb, NULL),
WS_SUCCESS);

ret = DoReceive(harness.ssh);

AssertIntEQ(ret, WS_SUCCESS);
AssertGlobalRequestReply(&harness, MSGID_REQUEST_FAILURE);

FreeChannelOpenHarness(&harness);
}

/* The refusal is silent when the peer did not ask for a reply: nothing goes
* back, and the session carries on. Silence alone would also be the answer
* without the role check, since a request the callback accepts and that asks
* for no reply sends nothing either, so the callback is the discriminator
* here: the request must be turned away before it reaches one. */
static void TestGlobalRequestFwdOnClientNoReplyStaysQuiet(void)
{
ChannelOpenHarness harness;
byte in[256];
word32 inSz;
int ret;

inSz = BuildGlobalRequestFwdPacket("0.0.0.0", 2222, 0, 0, in, sizeof(in));
InitChannelOpenHarnessClient(&harness, in, inSz);
AssertIntEQ(wolfSSH_CTX_SetFwdCb(harness.ctx, CountingFwdCb, NULL),
WS_SUCCESS);
fwdCbCallCount = 0;

ret = DoReceive(harness.ssh);

AssertIntEQ(ret, WS_SUCCESS);
AssertIntEQ(harness.io.outSz, 0);
Comment thread
ejohnstown marked this conversation as resolved.
AssertIntEQ(fwdCbCallCount, 0);

FreeChannelOpenHarness(&harness);
}

#endif /* !NO_WOLFSSH_CLIENT */

/* The role check must not cost the server anything: the same request that a
* client refuses is still honoured here. */
static void TestGlobalRequestFwdOnServerStillSucceeds(void)
{
ChannelOpenHarness harness;
byte in[256];
word32 inSz;
int ret;

inSz = BuildGlobalRequestFwdPacket("0.0.0.0", 2222, 0, 1, in, sizeof(in));
InitChannelOpenHarness(&harness, in, inSz);
AssertIntEQ(wolfSSH_CTX_SetFwdCb(harness.ctx, AcceptFwdCb, NULL),
WS_SUCCESS);

ret = DoReceive(harness.ssh);

AssertIntEQ(ret, WS_SUCCESS);
AssertGlobalRequestReply(&harness, MSGID_REQUEST_SUCCESS);

FreeChannelOpenHarness(&harness);
}

static void TestGlobalRequestFwdNoCbNoReplyKeepsConnection(void)
{
ChannelOpenHarness harness;
Expand Down Expand Up @@ -10816,6 +11019,10 @@ int main(int argc, char** argv)
TestServerChannelBlockedBeforeAuth(serverSsh);
TestServerChannelAllowedAfterAuth(serverSsh);
TestServerUserauthBlockedBeforeKeyed(serverSsh);
TestServerHighMsgIdBeforeAuthDisconnects();
TestServerUnknownMsgIdBeforeAuthUnimplemented();
TestServerKnownAuthMsgIdBeforeAuthDisconnects();
TestServerUnknownHighMsgIdBeforeAuthDisconnects();
TestServerOnlyUserauthMsgsBlocked(serverSsh);
TestServerServiceRequestStateGated(serverSsh);
TestServerServiceRequestRejectedDuringKeying();
Expand All @@ -10836,6 +11043,12 @@ int main(int argc, char** argv)
TestDirectTcpipFwdCbRejectsChannelId();
TestForwardedTcpipOnServerSendsOpenFail();
TestGlobalRequestFwdNoCbSendsFailure();
#ifndef NO_WOLFSSH_CLIENT
TestGlobalRequestFwdOnClientSendsFailure();
TestGlobalRequestFwdCancelOnClientSendsFailure();
TestGlobalRequestFwdOnClientNoReplyStaysQuiet();
#endif
TestGlobalRequestFwdOnServerStillSucceeds();
TestGlobalRequestFwdNoCbNoReplyKeepsConnection();
TestGlobalRequestFwdWithCbSendsSuccess();
TestGlobalRequestFwdPort0ReturnsAllocatedPort();
Expand Down
Loading