Fix DoGlobalRequestFwd() so that it rejects the port forwarding if no fwdCb is set.#918
Fix DoGlobalRequestFwd() so that it rejects the port forwarding if no fwdCb is set.#918yosuke-wolfssl wants to merge 1 commit intowolfSSL:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts server-side handling of tcpip-forward / cancel-tcpip-forward global requests (when WOLFSSH_FWD is enabled) to avoid approving port forwarding before the forwarding-policy callback is consulted, and adds regression tests around the new behavior.
Changes:
- Move the global-request success reply so it is only sent after the forwarding callback accepts the request.
- Reject remote port forwarding requests when no forwarding callback (
fwdCb) is registered. - Add regression tests covering success/failure replies for forward/cancel requests with and without
fwdCb.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/internal.c |
Reorders DoGlobalRequestFwd() reply/decision flow and adds explicit rejection when fwdCb is not set. |
tests/regress.c |
Adds packet builder + assertions and new regress tests for global forwarding request replies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@yosuke-wolfssl , please resolve the merge conflict |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hi @JacobBarthelmeh , |
| } | ||
| else if (ret != WS_SUCCESS) { | ||
| /* No reply expected; silently reject without terminating connection. */ | ||
| ret = WS_SUCCESS; |
There was a problem hiding this comment.
Overriding all non WS_SUCCESS ret values when wantReply is not set could mask returns like WS_BAD_ARGUMENT.
Background
When WOLFSSH_FWD is compiled in, the server-side handler for tcpip-forward global requests sends SSH_MSG_REQUEST_SUCCESS (via SendGlobalRequestFwdSuccess at line 8500) before invoking the fwdCb policy callback (line 8504). This has two consequences: (1) If no fwdCb is registered (the default), all remote port forwarding requests are silently accepted because the success reply has already been sent. (2) Even if fwdCb IS registered and rejects the request, the client has already received the approval response.
Changes
This PR fixes it so that DoGlobalRequestFwd() rejects the forwarding if no fwdCb is set.
Also, the new regression tests are added.