Skip to content

bitreq: add SOCKS5 proxy support#533

Draft
FreeOnlineUser wants to merge 1 commit intorust-bitcoin:masterfrom
FreeOnlineUser:socks5-proxy
Draft

bitreq: add SOCKS5 proxy support#533
FreeOnlineUser wants to merge 1 commit intorust-bitcoin:masterfrom
FreeOnlineUser:socks5-proxy

Conversation

@FreeOnlineUser
Copy link

Add SOCKS5 proxy support (RFC 1928) alongside existing HTTP CONNECT proxies.

Motivation

ldk-node needs to route HTTP calls (RGS, scoring, LNURL-auth) through Tor's SOCKS proxy when TorConfig is enabled. The existing proxy feature only supports HTTP CONNECT, which doesn't handle .onion addresses. See lightningdevkit/ldk-node#834.

SOCKS5 and HTTP CONNECT serve different use cases:

  • SOCKS5: Tor/general proxying with domain-based addressing (DNS resolved at the proxy)
  • HTTP CONNECT: forward proxies for HTTPS tunneling

Both coexist behind the existing proxy feature flag.

Changes

  • Proxy::new_socks5(addr) constructor (public API)
  • ProxyKind::Socks5 variant
  • Sync and async SOCKS5 handshake (RFC 1928, ATYP 0x03 domain addressing)
  • Connection paths branch on proxy.is_socks5() before the existing HTTP CONNECT path
  • Updated README and doc comments

Tests

10 new tests:

  • 5 parsing: host+port, socks5:// prefix, default port (1080), wrong protocol rejection, is_socks5 check
  • 5 mock handshake: successful connection, .onion domain, server rejection, port encoding (big-endian), domain >255 bytes

Notes

Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took a first very high-level look.

}
Some(proxy) => {
// do proxy things
// HTTP CONNECT proxy
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should be good to just drop the HTTP proxy behavior? Not sure anybody is going to use that, and it might make things simpler?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HTTP CONNECT is what payjoin originally requested in #472, so I'd keep it. They serve different use cases: SOCKS5 for Tor/general proxying, HTTP CONNECT for corporate forward proxies and HTTPS tunneling. Happy to refactor if the maintainers agree it should go, but wanted to flag the other stakeholders first.

/// let request = bitreq::post("http://example.com").with_proxy(proxy);
/// ```
///
pub fn new_socks5<S: AsRef<str>>(proxy: S) -> Result<Self, Error> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that Tor (mis-)uses user credentials to trigger circuit rebuilding/stream isolation. So we probably want to allow for an interface that at least lets the user refresh/change the credentials freely on the Proxy object, so that subsequent connections are using a new circuit.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I'll add username/password support (RFC 1929 sub-negotiation) to the SOCKS5 handshake. When credentials are set, the greeting advertises method 0x02 instead of 0x00, and Tor uses the unique credentials to isolate circuits. The Proxy API can accept optional credentials at construction or via a setter for rotation.

Add SOCKS5 proxy support (RFC 1928) alongside existing HTTP CONNECT.
Uses domain-based addressing (ATYP 0x03) so DNS resolution happens at
the proxy, enabling .onion routing through Tor.

New public API:
- Proxy::new_socks5(addr) for unauthenticated SOCKS5
- Proxy::new_socks5_with_credentials(addr, user, pass) for RFC 1929
  username/password auth (enables Tor circuit isolation)

Both sync and async handshake paths share protocol logic via helpers,
with only I/O differing between them.

Motivation: ldk-node needs to route HTTP calls (RGS, scoring) through
Tor's SOCKS proxy. See lightningdevkit/ldk-node#834.

Tests: 7 parsing tests + 8 mock handshake tests covering success, .onion
domains, server rejection, port encoding, domain length validation,
credential auth, credential rejection, and no-auth fallback.
@FreeOnlineUser
Copy link
Author

Updated: added Proxy::new_socks5_with_credentials for RFC 1929 auth (Tor circuit isolation), credential length validation, extracted shared protocol helpers to deduplicate sync/async, 15 tests (was 10).

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.

3 participants