Skip to content

SOCKS5 Fix#486

Open
0typos wants to merge 5 commits into
erebe:mainfrom
0typos:fix/socks5
Open

SOCKS5 Fix#486
0typos wants to merge 5 commits into
erebe:mainfrom
0typos:fix/socks5

Conversation

@0typos

@0typos 0typos commented Feb 7, 2026

Copy link
Copy Markdown

Summary

This PR fixes SOCKS5 CONNECT reply timing for both forward and reverse tunnel paths. Reported as an issue in #485.

Before this change, SOCKS5 success could be returned before the upstream target connect result was known. In practice, proxy tooling (proxychains + nc / nmap -sT) could observe false-positive connect success for unreachable ports.

After this change, SOCKS5 success/failure is sent only when the real upstream connect outcome is known.

Problem

In affected paths, the SOCKS5 listener acknowledged CONNECT too early.
This breaks expected SOCKS5 semantics and can produce incorrect scan/probe results.

Root Cause

  • The TCP SOCKS5 path replied REP=Succeeded immediately after SOCKS handshake acceptance, instead of waiting for upstream tunnel connect outcome.
  • Reverse SOCKS5 flow did not have an explicit connect-result handshake across WS/HTTP2 transport.
  • On server side, replying to SOCKS clients required downcasting tunnel writers; we needed a safe and object-safe mechanism for that in the mixed writer pipeline.

What Changed

  1. Deferred SOCKS5 reply handling in the SOCKS server write half.
  2. Forward tunnel path now sends SOCKS5 success/failure based on actual server connect outcome.
  3. Reverse SOCKS5 adds explicit one-byte connect-result handshake:
    • client sends connected/failed outcome
    • server waits for that before replying to SOCKS client
  4. WS/HTTP2 read paths now support reading one handshake byte without losing first payload bytes (prefetch buffer).
  5. Added configurable connect timeout via CLI/env on both client and server:
    • --timeout-connect (alias --timeout-connect-sec)
    • WSTUNNEL_TIMEOUT_CONNECT
    • default set to 3s
  6. Added unit + integration tests for SOCKS reply timing and connect success/failure behavior.

Behavioral Outcomes

  • Forward SOCKS5:
    • success reply only after upstream connect succeeds
    • failure reply when upstream connect fails
  • Reverse SOCKS5:
    • server SOCKS reply synchronized with local connector result over WS/HTTP2
  • Timeout behavior:
    • connect/handshake timeout is now operator-tunable on both client and server
    • for reverse SOCKS5, client/server timeout values can differ; the shorter side effectively limits wait time
  • proxychains-based TCP connect probing now reflects target state more accurately.

Validation

Commands run:

  • cargo fmt --all -- --check
  • cargo check
  • cargo check -p wstunnel
  • cargo test -p wstunnel test_socks5_tunnel_connect_success --lib
  • cargo test -p wstunnel test_socks5_tunnel_connect_failure --lib
  • cargo test -p wstunnel reverse_socks5_handshake --lib

All passed.

Notes

  • Existing deprecation/lint warnings remain unchanged from baseline (fast_socks5::server::Socks5Server, jwt validation deprecation, unfulfilled lint expectation).
  • Added configurable connect timeout for both client and server via --timeout-connect (alias --timeout-connect-sec) and WSTUNNEL_TIMEOUT_CONNECT.
  • This timeout now governs upstream connect attempts and reverse SOCKS5 connect-handshake wait, which allows tuning scan speed vs accuracy tradeoff for tools like nmap/proxychains.

Reviewer-Oriented Change Summary (File-by-File)

  1. wstunnel/src/protocols/socks5/tcp_server.rs

    • Changed Socks5WriteHalf::Tcp to hold pending_reply state.
    • Stopped immediate TCP CONNECT success reply in accept loop.
    • Added send_reply_if_needed() to emit a one-shot SOCKS reply only when connect outcome is known.
    • Updated AsyncWrite impl for new enum shape.
    • Added unit tests for deferred-reply behavior (sent-once, skipped-when-not-pending).
    • Reason: this is the core fix for premature SOCKS success in forward/reverse paths.
  2. wstunnel/src/tunnel/client/client.rs

    • Added forward-path logic to send SOCKS success/failure based on actual connect() result to server transport.
    • Added reverse-path handshake writer (HANDSHAKE_CONNECT_OK/FAIL) to report local connector outcome back to server.
    • Reason: makes client side the authoritative source of real upstream connect status.
  3. wstunnel/src/tunnel/server/server.rs

    • handle_tunnel_request() now carries whether request is reverse-socks5 and uses a writer type that can be downcast safely.
    • Replaced hardcoded connect timeout usage with self.config.timeout_connect in connector paths.
    • Reason: server handlers need this context to wait for reverse handshake and then reply to SOCKS client, and connector timeout behavior should be configurable.
  4. wstunnel/src/tunnel/server/handler_websocket.rs

    • Added reverse-socks5 handshake read/wait on WS path.
    • Sends SOCKS success/failure reply only after handshake result.
    • Closes WS on failure paths to avoid dangling connection state.
    • Reason: synchronize server SOCKS reply with real connect outcome over websocket transport.
  5. wstunnel/src/tunnel/server/handler_http2.rs

    • Same behavior as websocket handler, for HTTP2 transport.
    • Reason: keep reverse-socks5 correctness consistent across both transports.
  6. wstunnel/src/tunnel/transport/websocket.rs

    • Added read_handshake_byte() and prefetched buffer in WebsocketTunnelRead.
    • Preserves payload bytes after consuming handshake byte.
    • Reason: handshake framing without data loss/corruption.
  7. wstunnel/src/tunnel/transport/http2.rs

    • Added read_handshake_byte() and prefetched buffer in Http2TunnelRead.
    • Reason: same as websocket, for HTTP2 stream body frames.
  8. wstunnel/src/tunnel/reverse_socks5.rs (new)

    • Defines reverse handshake constants and timeout-aware decoder utility.
    • Includes focused unit tests (connected, failed, timeout, IO error).
    • Reason: isolate reverse handshake protocol logic and keep handlers simple.
  9. wstunnel/src/tunnel/server/socks5_reply.rs (new)

    • Introduces AnyAsyncWrite trait and helper to downcast writer to Socks5WriteHalf safely and send reply if needed.
    • Reason: object-safe writer abstraction needed in server pipeline to trigger SOCKS reply correctly.
  10. wstunnel/src/tunnel/server/mod.rs

    • Wires in new socks5_reply module exports.
    • Reason: shared access for server handlers.
  11. wstunnel/src/tunnel/mod.rs

    • Exposes reverse_socks5 module internally.
    • Reason: shared handshake logic between client and server handlers.
  12. wstunnel/src/test_integrations.rs

    • Added real SOCKS5 integration tests for CONNECT success and failure over wstunnel.
    • Includes minimal SOCKS5 handshake helper used by tests.
    • Reason: verifies end-to-end behavior that specifically regressed with proxychains-like usage.
  13. wstunnel/src/config.rs

    • Added --timeout-connect (alias --timeout-connect-sec) and WSTUNNEL_TIMEOUT_CONNECT to both client and server options.
    • Set default connect timeout to 3s.
    • Added help text clarifying reverse SOCKS5 client/server timeout interaction.
    • Reason: expose operator control over connect/handshake latency and scanning behavior.
  14. wstunnel/src/lib.rs

    • Replaced hardcoded timeout_connect defaults with CLI-configured values for both client and server runtime config.
    • Reason: ensure new timeout setting is actually applied to runtime behavior.

Disclosure

Code changes, repro script, issue draft, and this PR draft were prepared with assistance from ChatGPT 5.3 Codex.
I manually reviewed the code and manually tested the behavior before submitting this PR.

@0typos 0typos changed the title DRAFT: SOCKS5 Fix SOCKS5 Fix Feb 7, 2026
@erebe
erebe force-pushed the main branch 3 times, most recently from 9b03594 to 84e6786 Compare May 3, 2026 18:24
@0typos

0typos commented Jul 27, 2026

Copy link
Copy Markdown
Author

Hi @erebe — following up on #485 and this PR.

Rebased onto current main, tests pass. Are you interested in this fix/PR? If not no-worries I'll go ahead and close #486 and #485. Let me know!

The bug still reproduces on v10.6.2. The CONNECT path in socks5/tcp_server.rs calls reply_success with a hardcoded 127.0.0.1:0 before any upstream connection is attempted, so the client is told it succeeded whether or not the target is reachable. This is distinct from #515, which bounds the handshake but doesn't change when the reply is sent.

One design note: #515 moved this path to Socks5ServerProtocol, which owns the socket and only hands it back from reply_success — so the reply is now withheld through a small adapter instead of taking the socket early. Happy to do it another way if you'd prefer.

If the objection is scope rather than the bug, the first two commits are the actual fix; the timeout work can be dropped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant