Skip to content

io: socks5 proxy resolves the node host name remotely (#1198) - #2166

Open
vsnation wants to merge 1 commit into
BeamMW:masterfrom
vsnation:fix/1198-proxy-remote-dns
Open

vsnation wants to merge 1 commit into
BeamMW:masterfrom
vsnation:fix/1198-proxy-remote-dns

Conversation

@vsnation

Copy link
Copy Markdown

Fix: with --proxy (Tor or another socks5 proxy), the wallet no longer looks up the node's host name with a local DNS query. It sends the name to the proxy (socks5 ATYP=3), so the proxy resolves it (#1198).

Why: CreateNetwork called nodeAddress.resolve(nodeURI) (a local getaddrinfo) before checking --proxy, and ProxyConnector could only encode an IPv4 address (ATYP=1). DOMAINNAME = 0x03 was defined but never used. So a Tor user's DNS query for the node went out over the normal network, and names only the proxy can resolve (e.g. .onion) didn't work at all.

How to see the bug (master, no internet needed): run a tiny socks5 stub on 127.0.0.1:39151 that prints the CONNECT request, then

beam-wallet listen --pass=p --wallet_path=w.db -n localhost:8100   --proxy=true --proxy_addr=127.0.0.1:39151
# master: the stub receives ATYP=01 ip=127.0.0.1, so the name was resolved locally
beam-wallet listen --pass=p --wallet_path=w.db -n node.invalid:8100 --proxy=true --proxy_addr=127.0.0.1:39151
# master: "unable to resolve node address: node.invalid:8100", and the proxy is never contacted

Improvement

-n with --proxy master this PR
node.invalid:8100 local lookup fails, proxy not contacted proxy gets ATYP=03 node.invalid:8100
localhost:8100 proxy gets ATYP=01 127.0.0.1 (resolved locally) proxy gets ATYP=03 localhost:8100
127.0.0.2:8100 (IP) ATYP=01 ATYP=01, same bytes
any, without --proxy local resolve unchanged

How:

  • With the proxy on, the node is passed as "host:port" (FlyClient::Config::m_vNodeHosts) to a new NodeConnection::Connect(host, port, proxy), then Reactor::tcp_connect_with_proxy(host, …), then Socks5_Protocol::makeRequest(host, port).
  • Host names longer than 255 bytes, an empty host, or port 0 are rejected with EC_EINVAL.
  • The duplicated proxy / no-proxy connect branches in FlyClient are merged into Connection::ConnectNode().
  • The --proxy_addr itself is still a local endpoint.
  • With the proxy on, the node address needs an explicit port.

Testing:

  • proxy_test has a new proxy_domain_name_request_test: exact request bytes for a domain and an IP, the 255/256-byte and empty hosts, host:port parsing, reactor rejections, and an end-to-end check against a local socks5 server that the CONNECT carries node.invalid. It passes. If the domain path is made to send the old IP request instead, it fails (CHECK failed … line 406).
  • address_test, tcpclient_test and node_test pass. The CLI rows above compare master and this branch against a stub proxy.
  • Fork CI: GitHub Actions had an incident when this was pushed. I'll add the run link here.

Fixes #1198

With --proxy (e.g. Tor), the CLI wallet still resolved the node host
name with a local DNS lookup before connecting, and sent the proxy only
the IP (socks5 ATYP=1). That leaks the lookup outside Tor, and names
that only the proxy can resolve (e.g. .onion) didn't work at all.

With the proxy on, a node given by name is now passed through as
"host:port" (FlyClient Config::m_vNodeHosts) down to ProxyConnector,
which sends a socks5 CONNECT with ATYP=3 (domain name), so the proxy
resolves it. IP literals still go as ATYP=1, byte for byte as before;
without the proxy nothing changes. The proxy/no-proxy connect branches
in FlyClient are folded into Connection::ConnectNode().

proxy_test: request encoding (domain, IP, 255/256-byte and empty
hosts), host:port parsing, reactor rejections, and an end-to-end check
with a local socks5 server that the CONNECT carries the host name.
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.

Proxy "RESOLVE" command

1 participant