diff --git a/src/internal.c b/src/internal.c index 57cee4b9..6ac443ae 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4470,6 +4470,10 @@ static int FwdRemoteMatch(WOLFSSH* ssh, const char* addr, word32 port) if (ssh == NULL || addr == NULL) return 0; + /* The application took responsibility for what it accepts. */ + if (ssh->fwdRemoteMatch == WOLFSSH_FWD_MATCH_OFF) + return 1; + for (cur = ssh->fwdRemoteList; cur != NULL; cur = cur->next) { WOLFSSH_FWD_REPLY* newest; @@ -4490,7 +4494,10 @@ static int FwdRemoteMatch(WOLFSSH* ssh, const char* addr, word32 port) if (!cur->confirmed && newest == NULL) continue; - if (FwdRemoteAddrIsWild(cur->bindAddr) || + /* A peer that rewrites the bind it echoes back can still be held to + * the port it was asked for. */ + if (ssh->fwdRemoteMatch == WOLFSSH_FWD_MATCH_PORT || + FwdRemoteAddrIsWild(cur->bindAddr) || WSTRCMP(cur->bindAddr, addr) == 0) return 1; } diff --git a/src/ssh.c b/src/ssh.c index 2fc9d8ea..f117a2ef 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -4061,6 +4061,22 @@ int wolfSSH_FwdRemoteCancel(WOLFSSH* ssh, const char* bindAddr, return ret; } + +int wolfSSH_SetFwdRemoteMatch(WOLFSSH* ssh, byte match) +{ + int ret = WS_SUCCESS; + + WLOG(WS_LOG_DEBUG, "Entering wolfSSH_SetFwdRemoteMatch()"); + + if (ssh == NULL || match > WOLFSSH_FWD_MATCH_OFF) + ret = WS_BAD_ARGUMENT; + else + ssh->fwdRemoteMatch = match; + + WLOG(WS_LOG_DEBUG, "Leaving wolfSSH_SetFwdRemoteMatch(), ret = %d", ret); + return ret; +} + #endif /* WOLFSSH_FWD */ diff --git a/tests/regress.c b/tests/regress.c index 3406903a..3b83ad15 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -3668,8 +3668,9 @@ static void InitFwdRemoteHarness(ChannelOpenHarness* harness) /* Set up a client that asked for one remote forward, then hand it a * forwarded-tcpip open naming openAddr:openPort. The request the setup sends * is dropped from the output so the open's response starts at offset 0. */ -static void RunForwardedTcpipMatchTest(const char* bindAddr, word32 bindPort, - const char* openAddr, word32 openPort, int expectAccept) +static void RunForwardedTcpipMatchModeTest(byte match, const char* bindAddr, + word32 bindPort, const char* openAddr, word32 openPort, + int expectAccept) { ChannelOpenHarness harness; byte extra[128]; @@ -3686,6 +3687,7 @@ static void RunForwardedTcpipMatchTest(const char* bindAddr, word32 bindPort, InitChannelOpenHarnessClient(&harness, in, inSz); AssertIntEQ(wolfSSH_CTX_SetFwdCb(harness.ctx, AcceptFwdCb, NULL), WS_SUCCESS); + AssertIntEQ(wolfSSH_SetFwdRemoteMatch(harness.ssh, match), WS_SUCCESS); AssertIntEQ(wolfSSH_FwdRemoteSetup(harness.ssh, bindAddr, bindPort, 1), WS_SUCCESS); @@ -3709,6 +3711,13 @@ static void RunForwardedTcpipMatchTest(const char* bindAddr, word32 bindPort, FreeChannelOpenHarness(&harness); } +static void RunForwardedTcpipMatchTest(const char* bindAddr, word32 bindPort, + const char* openAddr, word32 openPort, int expectAccept) +{ + RunForwardedTcpipMatchModeTest(WOLFSSH_FWD_MATCH_STRICT, bindAddr, + bindPort, openAddr, openPort, expectAccept); +} + static void TestForwardedTcpipRegisteredIsAccepted(void) { /* The open names the forward the client registered, so it goes through. */ @@ -4700,6 +4709,46 @@ static void TestForwardedTcpipCancelAnsweredDuringResetupKeepsForward(void) FreeChannelOpenHarness(&harness); } +/* A peer that canonicalises the bind it echoes back has every open refused + * under the default, so the port it was asked for can be made the whole + * test. */ +static void TestFwdRemoteMatchPortIgnoresBindAddr(void) +{ + RunForwardedTcpipMatchModeTest(WOLFSSH_FWD_MATCH_STRICT, "localhost", 8080, + "127.0.0.1", 8080, 0); + RunForwardedTcpipMatchModeTest(WOLFSSH_FWD_MATCH_PORT, "localhost", 8080, + "127.0.0.1", 8080, 1); + + /* Relaxing the address does not relax the port. */ + RunForwardedTcpipMatchModeTest(WOLFSSH_FWD_MATCH_PORT, "localhost", 8080, + "127.0.0.1", 9999, 0); +} + +/* Off is what wolfSSH did before the check existed: the open reaches the + * channel-open policy callback whatever it names. */ +static void TestFwdRemoteMatchOffAcceptsUnregistered(void) +{ + RunForwardedTcpipMatchModeTest(WOLFSSH_FWD_MATCH_OFF, "127.0.0.1", 8080, + "10.0.0.1", 9999, 1); +} + +static void TestFwdRemoteMatchRejectsBadSetting(void) +{ + ChannelOpenHarness harness; + + InitFwdRemoteHarness(&harness); + + AssertIntEQ(wolfSSH_SetFwdRemoteMatch(NULL, WOLFSSH_FWD_MATCH_OFF), + WS_BAD_ARGUMENT); + AssertIntEQ(wolfSSH_SetFwdRemoteMatch(harness.ssh, + WOLFSSH_FWD_MATCH_OFF + 1), WS_BAD_ARGUMENT); + + /* A refused setting leaves the default in place. */ + AssertIntEQ(harness.ssh->fwdRemoteMatch, WOLFSSH_FWD_MATCH_STRICT); + + FreeChannelOpenHarness(&harness); +} + /* Only the peer gives a reply slot back, so a peer that never answers would * let the queue grow for the life of the session and lengthen every match. * Refusing has to happen before the request is framed: one whose slot was @@ -10444,6 +10493,9 @@ int main(int argc, char** argv) TestForwardedTcpipRequestAfterReplyDuringSend(); TestForwardedTcpipCancelAnsweredDuringResetupKeepsForward(); TestForwardedTcpipReentrantCancelDuringSend(); + TestFwdRemoteMatchPortIgnoresBindAddr(); + TestFwdRemoteMatchOffAcceptsUnregistered(); + TestFwdRemoteMatchRejectsBadSetting(); TestFwdReplyQueueIsCapped(); TestForwardedTcpipAppRequestKeepsItsOwnReply(); TestForwardedTcpipWantWriteStillRegisters(); diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 8292e5c1..065fcb5d 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1310,6 +1310,8 @@ struct WOLFSSH { WOLFSSH_FWD_REPLY* fwdReplyTail; word32 fwdReplyCount; /* slots queued, kept off the walk it bounds */ byte fwdRemoteTracked; /* wolfSSH_FwdRemoteSetup() was used */ + byte fwdRemoteMatch; /* WOLFSSH_FWD_MATCH_*, how strictly an inbound + * forwarded-tcpip must name a registration */ #endif /* WOLFSSH_FWD */ #ifdef WOLFSSH_TERM WS_CallbackTerminalSize termResizeCb; diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index 6b83c169..91e1b8d7 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -308,6 +308,7 @@ DEPRECATED WOLFSSH_API int wolfSSH_ChannelGetFwdFd( * equal the address the peer reports. Register the spelling the peer will echo * back, or a wildcard: a peer that canonicalises the bind, answering an open * for "127.0.0.1" against a registered "localhost", has those opens refused. + * wolfSSH_SetFwdRemoteMatch() relaxes this for peers that need it. * * One bindAddr:bindPort is one registration however often it is registered, * since it is one listener on the peer, so one cancel undoes it. That covers a @@ -339,6 +340,26 @@ WOLFSSH_API int wolfSSH_FwdRemoteSetup(WOLFSSH* ssh, const char* bindAddr, WOLFSSH_API int wolfSSH_FwdRemoteCancel(WOLFSSH* ssh, const char* bindAddr, word32 bindPort, int wantReply); +/* How strictly an inbound "forwarded-tcpip" open must name a registration + * made with wolfSSH_FwdRemoteSetup(). */ +enum WS_FwdRemoteMatch { + WOLFSSH_FWD_MATCH_STRICT = 0, /* bind and port, the default */ + WOLFSSH_FWD_MATCH_PORT = 1, /* port alone, the bind is not compared */ + WOLFSSH_FWD_MATCH_OFF = 2 /* accept any open, matching nothing */ +}; + +/* Relax the check wolfSSH_FwdRemoteSetup() turns on for this session. Set it + * before the first setup, since opens are matched from that point. + * + * STRICT is the default and is what RFC 4254 7.2 asks for. PORT is for a peer + * that rewrites the bind address it echoes back but keeps the port, which + * STRICT refuses every open from. OFF accepts any "forwarded-tcpip" open, as + * wolfSSH did before this check existed, leaving the channel-open policy + * callback as the only thing standing between the peer and a new channel. + * + * Returns WS_BAD_ARGUMENT for a NULL session or an unknown setting. */ +WOLFSSH_API int wolfSSH_SetFwdRemoteMatch(WOLFSSH* ssh, byte match); + WOLFSSH_API int wolfSSH_ChannelFree(WOLFSSH_CHANNEL* channel); WOLFSSH_API int wolfSSH_ChannelGetId(WOLFSSH_CHANNEL* channel, word32* id, byte peer);