From 02ecaa015e42ff0020f133180b718d4eea5b05ca Mon Sep 17 00:00:00 2001
From: John Safranek <john@wolfssl.com>
Date: Thu, 10 Sep 2026 09:11:55 -0700
Subject: [PATCH] internal: bare success for a fixed-port forward

RFC 4254 7.1 gives a tcpip-forward success a trailing bound-port field
only for a port-0 (dynamic) request. DoGlobalRequestFwd() now sends that
field only when the peer asked the server to allocate a port, and
answers an explicit port with a bare SSH_MSG_REQUEST_SUCCESS, as OpenSSH
and libssh do.

- Key the reply builder off requestedPort, which the port-0 compliance
  check already tracks.
- Check the reply payload length against the requested port in the
  regress global-request helper, which had baked in the trailing field.
- Add ParseGlobalRequestFwdBindPort() to recover that port from the
  request the harness fed in.
- Cover the explicit-port reply.

Issue: #1246
---
 src/internal.c  |  9 ++++---
 tests/regress.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 70 insertions(+), 4 deletions(-)

diff --git a/src/internal.c b/src/internal.c
index 5d1d6112..db7d8620 100644
--- a/src/internal.c
+++ b/src/internal.c
@@ -12164,11 +12164,14 @@ static int DoGlobalRequestFwd(WOLFSSH* ssh,
 
     if (wantReply) {
         if (ret == WS_SUCCESS) {
-            if (isCancel) {
-                ret = SendRequestSuccess(ssh, 1);
+            /* RFC 4254 7.1 gives the success a trailing bound-port field only
+             * for a port-0 (dynamic) request. An explicit port, and a cancel,
+             * get a bare success with no response-specific data. */
+            if (!isCancel && requestedPort == 0) {
+                ret = SendGlobalRequestFwdSuccess(ssh, 1, bindPort);
             }
             else {
-                ret = SendGlobalRequestFwdSuccess(ssh, 1, bindPort);
+                ret = SendRequestSuccess(ssh, 1);
             }
         }
         else {
diff --git a/tests/regress.c b/tests/regress.c
index eaa84154..cce2a02d 100644
--- a/tests/regress.c
+++ b/tests/regress.c
@@ -2151,6 +2151,37 @@ static const byte* ParseGlobalRequestName(const byte* packet, word32 packetSz,
     return payload + 1 + sizeof(word32);
 }
 
+/* Bind port of a tcpip-forward global request. Past the request name and the
+ * want-reply byte come the bind address and then the port. */
+static word32 ParseGlobalRequestFwdBindPort(const byte* packet,
+        word32 packetSz)
+{
+    const byte* payload;
+    const byte* reqName;
+    word32 reqNameSz;
+    word32 payloadLen;
+    word32 idx;
+    word32 strSz;
+    word32 port;
+
+    reqName = ParseGlobalRequestName(packet, packetSz, &reqNameSz);
+    payload = packet + 5;
+    payloadLen = ParsePayloadLen(packet, packetSz);
+    idx = (word32)(reqName - payload) + reqNameSz;
+
+    AssertTrue(payloadLen >= idx + 1 + sizeof(word32));
+    idx += 1;
+
+    WMEMCPY(&strSz, payload + idx, sizeof(strSz));
+    strSz = ntohl(strSz);
+    idx += (word32)sizeof(word32) + strSz;
+
+    AssertTrue(payloadLen >= idx + sizeof(word32));
+    WMEMCPY(&port, payload + idx, sizeof(port));
+
+    return ntohl(port);
+}
+
 static void AssertGlobalRequestReply(const ChannelOpenHarness* harness,
         byte expectedMsgId)
 {
@@ -2175,7 +2206,14 @@ static void AssertGlobalRequestReply(const ChannelOpenHarness* harness,
         if (reqNameSz == sizeof("tcpip-forward") - 1 &&
                 WMEMCMP(reqName, "tcpip-forward",
                 sizeof("tcpip-forward") - 1) == 0) {
-            AssertIntEQ(payloadLen, 5);
+            /* The bound port trails the success only for a port-0 request. */
+            if (ParseGlobalRequestFwdBindPort(harness->io.in,
+                    harness->io.inSz) == 0) {
+                AssertIntEQ(payloadLen, 5);
+            }
+            else {
+                AssertIntEQ(payloadLen, 1);
+            }
         }
         else if (reqNameSz == sizeof("cancel-tcpip-forward") - 1 &&
                 WMEMCMP(reqName, "cancel-tcpip-forward",
@@ -5583,6 +5621,30 @@ static void TestGlobalRequestFwdPort0ReturnsAllocatedPort(void)
     FreeChannelOpenHarness(&harness);
 }
 
+/* RFC 4254 7.1 defines the trailing bound-port field only for a port-0
+ * request. An explicit port must be answered with a bare success. */
+static void TestGlobalRequestFwdExplicitPortReplyHasNoPort(void)
+{
+    ChannelOpenHarness harness;
+    byte in[256];
+    word32 inSz;
+    int ret;
+
+    inSz = BuildGlobalRequestFwdPacket("0.0.0.0", 8022, 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);
+    AssertIntEQ(ParseMsgId(harness.io.out, harness.io.outSz),
+            MSGID_REQUEST_SUCCESS);
+    AssertIntEQ(ParsePayloadLen(harness.io.out, harness.io.outSz), 1);
+
+    FreeChannelOpenHarness(&harness);
+}
+
 static void TestGlobalRequestFwdPort0NoAllocSendsFailure(void)
 {
     ChannelOpenHarness harness;
@@ -14955,6 +15017,7 @@ int main(int argc, char** argv)
     TestGlobalRequestFwdNoCbNoReplyKeepsConnection();
     TestGlobalRequestFwdWithCbSendsSuccess();
     TestGlobalRequestFwdPort0ReturnsAllocatedPort();
+    TestGlobalRequestFwdExplicitPortReplyHasNoPort();
     TestGlobalRequestFwdPort0NoAllocSendsFailure();
     TestGlobalRequestFwdRemoteSetupErrorSendsFailure();
     TestGlobalRequestFwdPort0NoAllocNoReplyKeepsConnection();