Skip to content

net: support unix domain sockets for -proxy and -onion#27375

Merged
achow101 merged 12 commits intobitcoin:masterfrom
pinheadmz:tor-unix-domain-socket
Mar 13, 2024
Merged

net: support unix domain sockets for -proxy and -onion#27375
achow101 merged 12 commits intobitcoin:masterfrom
pinheadmz:tor-unix-domain-socket

Conversation

@pinheadmz
Copy link
Member

@pinheadmz pinheadmz commented Mar 30, 2023

Closes #27252

UNIX domain sockets are a mechanism for inter-process communication that are faster than local TCP ports (because there is no need for TCP overhead) and potentially more secure because access is managed by the filesystem instead of serving an open port on the system.

There has been work on unix domain sockets before but for now I just wanted to start on this single use-case which is enabling unix sockets from the client side, specifically connecting to a local Tor proxy (Tor can listen on unix sockets and even enforces strict curent-user-only access permission before binding) configured by -onion= or -proxy=

I copied the prefix unix: usage from Tor. With this patch built locally you can test with your own filesystem path (example):

tor --SocksPort unix:/Users/matthewzipkin/torsocket/x

bitcoind -proxy=unix:/Users/matthewzipkin/torsocket/x

Prep work for this feature includes:

  • Moving where and how we create sockaddr and Sock to accommodate AF_UNIX without disturbing CService
  • Expanding Proxy class to represent either a CService or a UNIX socket (by its file path)

Future work:

  • Enable UNIX sockets for ZMQ (ZMQ: Support UNIX domain sockets #27679)
  • Enable UNIX sockets for I2P SAM proxy (some code is included in this PR but not tested or exposed to user options yet)
  • Enable UNIX sockets on windows where supported
  • Update Network Proxies dialog in GUI to support UNIX sockets

@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 30, 2023

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK Sjors, vasild, tdb3, achow101
Stale ACK willcl-ark

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #29641 (scripted-diff: Use LogInfo/LogDebug over LogPrintf/LogPrint by maflcko)
  • #29015 (kernel: Streamline util library by ryanofsky)
  • #28834 (net: attempts to connect to all resolved addresses when connecting to a node by sr-gi)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Copy link
Member

@willcl-ark willcl-ark left a comment

Choose a reason for hiding this comment

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

Concept ACK

With some light testing it seems to work pretty nicely:

will@ubuntu in ~/src/bitcoin on  tor-unix-domain-socket [$?] via 🐍 3.7.16 took 6s
₿ sudo exa -al /proc/(pidof bitcoind)/fd
lrwx------  64 will 31 Mar 09:21 0 -> /dev/null
lrwx------  64 will 31 Mar 09:21 1 -> /dev/null
lrwx------  64 will 31 Mar 09:21 2 -> /dev/null
lr-x------  64 will 31 Mar 09:21 3 -> pipe:[47084978]
l-wx------  64 will 31 Mar 09:21 4 -> pipe:[47084978]
lrwx------  64 will 31 Mar 09:21 5 -> /home/will/.bitcoin/signet/.lock
l-wx------  64 will 31 Mar 09:21 6 -> /home/will/.bitcoin/signet/debug.log
lrwx------  64 will 31 Mar 09:21 7 -> anon_inode:[eventpoll]
lr-x------  64 will 31 Mar 09:21 8 -> pipe:[47101090]
l-wx------  64 will 31 Mar 09:21 9 -> pipe:[47101090]
lrwx------  64 will 31 Mar 09:21 10 -> anon_inode:[eventfd]
lrwx------@ 64 will 31 Mar 09:21 11 -> socket:[47101093]
lrwx------@ 64 will 31 Mar 09:21 12 -> socket:[47101095]
lrwx------  64 will 31 Mar 09:21 13 -> /home/will/.bitcoin/signet/blocks/index/LOCK
l-wx------  64 will 31 Mar 09:21 14 -> /home/will/.bitcoin/signet/blocks/index/000387.log
l-wx------  64 will 31 Mar 09:21 15 -> /home/will/.bitcoin/signet/blocks/index/MANIFEST-000385
lrwx------  64 will 31 Mar 09:21 16 -> /home/will/.bitcoin/signet/chainstate/LOCK
l-wx------  64 will 31 Mar 09:21 17 -> /home/will/.bitcoin/signet/chainstate/002594.log
l-wx------  64 will 31 Mar 09:21 18 -> /home/will/.bitcoin/signet/chainstate/MANIFEST-002593
lrwx------  64 will 31 Mar 09:21 19 -> /home/will/.bitcoin/signet/wallets/legacy/.walletlock
l-wx------  64 will 31 Mar 09:21 20 -> /home/will/.bitcoin/signet/wallets/legacy/db.log
lrwx------  64 will 31 Mar 09:21 21 -> /home/will/.bitcoin/signet/wallets/legacy/wallet.dat
lrwx------  64 will 31 Mar 09:21 22 -> /home/will/.bitcoin/signet/wallets/legacy/database/log.0000000001
lrwx------  64 will 31 Mar 09:21 23 -> /home/will/.bitcoin/signet/wallets/test-descriptor/wallet.dat
lrwx------  64 will 31 Mar 09:21 24 -> /home/will/.bitcoin/signet/wallets/test-descriptor/wallet.dat-journal
lrwx------  64 will 31 Mar 09:21 25 -> anon_inode:[eventpoll]
lr-x------  64 will 31 Mar 09:21 26 -> pipe:[47084995]
l-wx------  64 will 31 Mar 09:21 27 -> pipe:[47084995]
lrwx------  64 will 31 Mar 09:21 28 -> anon_inode:[eventfd]
lrwx------@ 64 will 31 Mar 09:21 29 -> socket:[47099059]
lrwx------@ 64 will 31 Mar 09:21 30 -> socket:[47084996]
lrwx------@ 64 will 31 Mar 09:21 31 -> socket:[47084997]
lrwx------@ 64 will 31 Mar 09:21 32 -> socket:[47099065]
lrwx------@ 64 will 31 Mar 09:21 33 -> socket:[47084998]
lrwx------@ 64 will 31 Mar 09:21 34 -> socket:[47100197]
lrwx------@ 64 will 31 Mar 09:21 35 -> socket:[47099299]
lrwx------@ 64 will 31 Mar 09:21 36 -> socket:[47099533]
lrwx------@ 64 will 31 Mar 09:22 37 -> socket:[47099588]
lrwx------@ 64 will 31 Mar 09:22 38 -> socket:[47085043]
lrwx------@ 64 will 31 Mar 09:22 39 -> socket:[47085047]

With new socket connections being opened for each connection.

As I mentioned in one of my comments, I am curious why you didn't go for the -onion case too, to reduce the scope of the change?

Enabling -onion in the PortErrorMessage check loop did appear to also work nicely (although i did not check whether there were any other side-effects).

The info shown by -netinfo 4 looked correct in both cases.

Copy link
Member Author

@pinheadmz pinheadmz left a comment

Choose a reason for hiding this comment

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

Thanks for the feedback @willcl-ark. force-push to 2436b00f2231c52e6bee6d7f79690c5e709cd6f9 should address your notes. I'm still expecting some CI failures from platforms that don't support unix sockets, then I'll add configure options like wumpus did here

@pinheadmz pinheadmz force-pushed the tor-unix-domain-socket branch 2 times, most recently from 261ce7e to 2436b00 Compare March 31, 2023 18:49
@pinheadmz pinheadmz force-pushed the tor-unix-domain-socket branch 2 times, most recently from 0d19a39 to 3436ddb Compare April 2, 2023 14:29
@pinheadmz pinheadmz force-pushed the tor-unix-domain-socket branch from 3436ddb to 502db51 Compare April 3, 2023 00:21
@pinheadmz pinheadmz changed the title net: support unix domain sockets for -proxy net: support unix domain sockets for -proxy and -onion Apr 3, 2023
@pinheadmz pinheadmz force-pushed the tor-unix-domain-socket branch 3 times, most recently from 0267cfe to 44dd4cb Compare April 3, 2023 17:02
@pinheadmz pinheadmz marked this pull request as ready for review April 3, 2023 18:14
Copy link
Contributor

@vasild vasild left a comment

Choose a reason for hiding this comment

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

Concept ACK, this would be nice to have.

I do not like that the current approach expands enum Network and CNetAddr with the UNIX type, making it first-class-citizen network that has to be handled all over the networking code, whereas it is only needed to connect to the proxy. It is not like this PR adds support for connecting to other Bitcoin nodes over UNIX sockets (using the P2P protocol). This creates meaningless situations like having a CService on a UNIX socket (it is meaningless because CService has port, whereas UNIX sockets don't have port). Similarly CSubNet on a UNIX socket is meaningless.

I will play a bit with this to see if it would be possible to constrain the logic in the Proxy class and hide from the rest of the code whether the proxy is at addr:port or unix:/path.

@pinheadmz pinheadmz force-pushed the tor-unix-domain-socket branch from 44dd4cb to 3ea142e Compare April 13, 2023 20:29
@pinheadmz
Copy link
Member Author

Thanks so much for the great review @vasild I think I addressed all your feedback up to the adding UNIX to CNetAddr. I understand your concerns there and we can chat about it, I'll look at alternatives as well. What I did do in this last push was mostly separate unix sockets from regular sockets, and adding Proxy.GetSocket() to decide which socket factory to call.

@pinheadmz
Copy link
Member Author

git range-diff master 6dddc1c2eda514d35219cce03c229bd6f822be55 567cec9a05e1261e955535f734826b12341684b6

Rebased on master to fix conflicts and ( 🤞 ) pass CI.
@Sjors thank you so much for the review, I addressed all your nits except the one or two where indicated in discussion.

@Sjors
Copy link
Member

Sjors commented Mar 4, 2024

but is used to refer to the connection between bitcoind and the input to the proxy

I agree that shouldn't be called "manual".

re-ACK 567cec9

Copy link
Contributor

@vasild vasild left a comment

Choose a reason for hiding this comment

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

ACK 567cec9

I agree it is better to keep the hardcoded /*manual=*/true because in this case that is about connecting to the proxy itself, not to the bitcoin peer. In this case it is better to log with higher severity.

In the future maybe it would be better to rename the manual_connection argument of ConnectToSocket() to log_severity or error_severity or is_imporant_please_log_errorswithhigherseverity.

@tdb3
Copy link
Contributor

tdb3 commented Mar 8, 2024

re ACK for 567cec9.

Pulled, built, ran all unit and functional tests (all passed).

Re-ran the test actions (quote below) from #27375 (review)

Overall, similar results were observed.
Each test action was performed with a fresh signet directory (with the exception of blocks and chainstate, which were not cleared, to prevent IBD).
For the first action (i.e. running with proxy=unix:/run/tor/socks set, but onion and onlynet not set), peer connection was observed to grow over the span of a half our (with ipv4, ipv6, and onion peers connected).
debug log showed general socks connection failures (although these also occur in v26.0 when using localhost:9050 for socks):

<date> Socks5() connect to <IPv4 ADDR>:38333 failed: general failure

For the second action (i.e. running with proxy and onlynet=onion with manual seed added), onion peer connections were observed.

For both actions, UNIX socket traffic was sniffed and contained bitcoin protocol traffic.

ACK for 6dddc1c. Great work on a great feature. Seems to work well. Reviewed the code changes, but didn't see anything noteworthy over what vasild, luke-jr, and willcl-ark already saw.

Ran the following test actions:

* Cloned and checked out PR branch, compiled, and ran all functional tests (all passed)

* Configured Tor to listen on only the unix domain socket /run/tor/socks (SocksPort 9050 and ControlPort 9051 disabled)

* Started bitcoind (signet), `proxy=/run/tor/socks` set, `onion` and `onlynet` not set.  Connections were established with multiple peers and successful block download was observed.  Sniffed the unix socket (`/run/tor/socks`) with https://github.com/mechpen/sockdump, and observed bitcoin traffic in Wireshark with a pcap dump from sockdump.  `getpeerinfo` command to the node showed ipv4, ipv6, and onion addr peers.

* Started bitcoind (signet), `onion=/run/tor/socks` set, `proxy` not set, but `onlynet=onion` set.  Manually added seed `v7ajjeirttkbnt32wpy3c6w3emwnfr3fkla7hpxcfokr3ysd3kqtzmqd.onion:38333` with `addnode` command.  Connections were established with multiple peers and successful block download was observed.  Sniffed the unix socket and again observed bitcoin traffic over the socket.  `getpeerinfo` showed multiple onion addr peers.

@achow101
Copy link
Member

ACK 567cec9

@achow101
Copy link
Member

Seems like there's a silent merge conflict somewhere, getting a functional test failure when merged with master:

2024-03-12T18:12:19.695000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/home/ava/bitcoin/bitcoin/master/test/functional/test_framework/test_framework.py", line 565, in start_nodes
    node.wait_for_rpc_connection()
  File "/home/ava/bitcoin/bitcoin/master/test/functional/test_framework/test_node.py", line 255, in wait_for_rpc_connection
    raise FailedToStartError(self._node_msg(
test_framework.test_node.FailedToStartError: [node 5] bitcoind exited with status 1 during initialization. Error: Invalid -proxy address or hostname: 'unix:/tmp/tmp9eh_pwt0'

Comment on lines +6 to +8
#if defined(HAVE_CONFIG_H)
#include <config/bitcoin-config.h>
#endif
Copy link
Member Author

Choose a reason for hiding this comment

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

@achow101 Its very mysterious, when I rebase this branch on master (git rebase master) these lines disappear. I can't figure out why!

However when I do interactive git rebase master -i and stop on this commit with an edit command, DO NOTHING, and just git rebase --continue... the lines remain intact. I am force-pushing this interactive rebase branch now. 😵‍💫

Copy link
Member

Choose a reason for hiding this comment

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

when I rebase this branch on master (git rebase master) these lines disappear. I can't figure out why!

What are the exact steps to reproduce?

I did

git checkout  567cec9a05e1261e955535f734826b12341684b6
git rebase bitcoin-core/master
git range-diff bitcoin-core/master 6df896fc37f5b2023b891d5300345e2a4352e521

And got an empty diff (only the commit ids are different)

Copy link
Member

@maflcko maflcko Mar 13, 2024

Choose a reason for hiding this comment

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

Maybe you two are using a different git --version (with a bug)?

@maflcko
Copy link
Member

maflcko commented Mar 13, 2024

Seems like there's a silent merge conflict somewhere, getting a functional test failure when merged with master:

I re-ran the CI and it didn't fail. Are you sure this is a silent conflict, because if it was, the CI should be detecting it on a re-run?

@achow101
Copy link
Member

Oh! The test command I use doesn't rerun autogen.sh every time, I think that's what caused the test failure. @pinheadmz You can reset the branch back to 567cec9 and this can be merged. Sorry about the confusion.

@pinheadmz
Copy link
Member Author

Oh! The test command I use doesn't rerun autogen.sh every time, I think that's what caused the test failure. @pinheadmz You can reset the branch back to 567cec9 and this can be merged. Sorry about the confusion.

Ugh and when I was trying to reproduce I ALSO didn't rerun autogen or config 🤦 wow THANKS @maflcko for the sanity check

@vasild
Copy link
Contributor

vasild commented Mar 13, 2024

autogen

wen cmake?

@fanquake
Copy link
Member

fanquake commented Mar 13, 2024

Is the expectation here that we should see a lot more Cannot connect to socket for xxxxx in debug.logs? (when not using any of these new features).

@pinheadmz
Copy link
Member Author

Is the expectation here that we should see a lot more Cannot connect to socket for xxxxx in debug.logs? (when not using any of these new features).

Yes but that log message can be removed or set to debug in a follow-up. With -debug=net you can see what the original messages were for that code path, with the extra message following:

2024-03-13T14:08:49Z [net] connection attempt to 95.165.161.198:8333 timed out
2024-03-13T14:08:49Z Cannot connect to socket for 95.165.161.198:8333
...
2024-03-13T14:08:50Z [net] connect() to 31.150.240.73:8333 failed after wait: Connection refused (61)
2024-03-13T14:08:50Z Cannot connect to socket for 31.150.240.73:8333

I have a few unix sockets follow ups planned: RPC & bitcoin-cli (maybe speed up the tests!) and ZMQ. I'll clean up this log message in those

@fanquake
Copy link
Member

Ok, it needs to be cleaned up because it's just spammy/pointless. i.e:

2024-03-13T13:26:00Z Cannot connect to socket for 151.213.177.128:8333
2024-03-13T13:26:24Z Cannot connect to socket for 75.88.65.59:8333
2024-03-13T13:26:48Z Cannot connect to socket for 76.114.61.112:8333
2024-03-13T13:28:55Z Cannot connect to socket for [2600:8805:5c26:eb00:61b0:c6a7:451:1b39]:8333
2024-03-13T13:30:45Z New block-relay-only v1 peer connected: version: 70016, blocks=834513, peer=39
2024-03-13T13:32:01Z Cannot connect to socket for [2a04:6ec0:20d:d050:819a:ceb8:4416:2bc0]:8333
2024-03-13T13:32:14Z New block-relay-only v1 peer connected: version: 70016, blocks=834513, peer=40
2024-03-13T13:34:47Z New block-relay-only v1 peer connected: version: 70016, blocks=834513, peer=41
2024-03-13T13:34:58Z Cannot connect to socket for 122.206.190.60:8333
2024-03-13T13:36:23Z New block-relay-only v1 peer connected: version: 70016, blocks=834513, peer=42
2024-03-13T13:37:38Z Cannot connect to socket for 138.199.60.26:8333
2024-03-13T13:38:34Z Cannot connect to socket for 120.236.171.239:8333
2024-03-13T13:39:00Z Cannot connect to socket for 35.72.149.104:8333
2024-03-13T13:41:22Z Cannot connect to socket for 176.199.211.63:8333
2024-03-13T13:41:25Z Cannot connect to socket for [2804:7f0:7c80:196f:b0b2:1642:e4a9:b794]:8333
2024-03-13T13:41:37Z Cannot connect to socket for [2a00:b700::3:379]:8333
2024-03-13T13:42:55Z Cannot connect to socket for [2001:871:24b:ab3e:d1f2:1f7e:b950:692f]:8333
2024-03-13T13:43:01Z New block-relay-only v1 peer connected: version: 70016, blocks=834513, peer=43
2024-03-13T13:45:30Z Cannot connect to socket for 121.237.138.96:8333
2024-03-13T13:46:56Z Cannot connect to socket for 110.153.86.87:8333
2024-03-13T13:48:32Z Saw new header hash=0000000000000000000071923ef5a18dc17d47717fae386296dfe8902414ea71 height=834514
2024-03-13T13:48:32Z Saw new cmpctblock header hash=0000000000000000000071923ef5a18dc17d47717fae386296dfe8902414ea71 peer=16
2024-03-13T13:48:32Z UpdateTip: new best=0000000000000000000071923ef5a18dc17d47717fae386296dfe8902414ea71 height=834514 version=0x2fffe000 log2_work=94.792163 tx=976064192 date='2024-03-13T13:48:27Z' progress=1.000000 cache=232.4MiB(1767736txo)
2024-03-13T13:50:28Z Cannot connect to socket for 58.219.96.208:8333
2024-03-13T13:50:41Z Cannot connect to socket for 189.19.183.194:8333
2024-03-13T13:52:56Z New block-relay-only v1 peer connected: version: 70016, blocks=834514, peer=45
2024-03-13T13:53:12Z Cannot connect to socket for [2600:381:6a90:cfa5:b0a4:5c43:fdb8:9298]:8333
2024-03-13T13:56:25Z Cannot connect to socket for [2003:d7:df24:a900:53a3:8916:d525:9958]:8333
2024-03-13T14:00:04Z Cannot connect to socket for 97.96.100.48:8333
2024-03-13T14:03:56Z Cannot connect to socket for 195.181.167.197:8333
2024-03-13T14:04:02Z Saw new header hash=00000000000000000001ee20083bb8df96e256bb46dd856c48f686b692dbc29b height=834515
2024-03-13T14:04:02Z Saw new cmpctblock header hash=00000000000000000001ee20083bb8df96e256bb46dd856c48f686b692dbc29b peer=16
2024-03-13T14:04:02Z UpdateTip: new best=00000000000000000001ee20083bb8df96e256bb46dd856c48f686b692dbc29b height=834515 version=0x294c8000 log2_work=94.792178 tx=976067423 date='2024-03-13T14:03:37Z' progress=1.000000 cache=234.9MiB(1785815txo)
2024-03-13T14:04:31Z Cannot connect to socket for [2606:6080:1001:30:5f94:f242:c829:1017]:8333
2024-03-13T14:04:32Z Saw new header hash=00000000000000000001cee842398455cae402ab13037a450fe4829db7efd2e2 height=834516
2024-03-13T14:04:32Z Saw new cmpctblock header hash=00000000000000000001cee842398455cae402ab13037a450fe4829db7efd2e2 peer=16
2024-03-13T14:04:33Z UpdateTip: new best=00000000000000000001cee842398455cae402ab13037a450fe4829db7efd2e2 height=834516 version=0x27fd4000 log2_work=94.792192 tx=976069773 date='2024-03-13T14:04:13Z' progress=1.000000 cache=236.4MiB(1799558txo)
2024-03-13T14:04:59Z Cannot connect to socket for 178.70.102.186:8333
2024-03-13T14:05:43Z Cannot connect to socket for [2409:8a55:6ad:7101:49d4:4737:ccad:17c]:8333
2024-03-13T14:06:21Z Saw new header hash=000000000000000000030739bcd83bfdfc6a03a1a58e21e4c61d6b59ac047569 height=834517
2024-03-13T14:06:21Z Saw new cmpctblock header hash=000000000000000000030739bcd83bfdfc6a03a1a58e21e4c61d6b59ac047569 peer=16
2024-03-13T14:06:21Z UpdateTip: new best=000000000000000000030739bcd83bfdfc6a03a1a58e21e4c61d6b59ac047569 height=834517 version=0x2cbea000 log2_work=94.792206 tx=976071288 date='2024-03-13T14:05:33Z' progress=1.000000 cache=236.4MiB(1802022txo)
2024-03-13T14:06:45Z Cannot connect to socket for 86.168.50.224:8333
2024-03-13T14:08:29Z Cannot connect to socket for 194.163.131.91:39388
2024-03-13T14:11:12Z Cannot connect to socket for [2603:6080:7a0e:59ff:9da7:4932:3d76:58ec]:8333
2024-03-13T14:11:41Z Cannot connect to socket for 108.214.8.13:60501
2024-03-13T14:12:06Z Cannot connect to socket for 84.178.138.51:8333

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adding interprocess onion access instead local ip

10 participants