test: avoid internet traffic in rpc

theStack commented at 10:14 pm on November 21, 2024: contributor

In order to avoid connecting to the internet in the functional test rpc_net.py, specify a non-working proxy (parameter -proxy=127.0.0.1:1, same approach as in #31142) for the nodes. There is at least one known instance where this is currently happening on master where a connection attempt to a public IP is made (see also the discussion in #31339):

https://github.com/bitcoin/bitcoin/blob/17834bd1976df7a2ff6c2f5f05a59ae3fd3f6875/test/functional/rpc_net.py#L253

Can be tested by running

0$ sudo tcpdump -i eth0 host 11.22.33.44

both on master and the PR branch and verifying that no packets appear in the tcpdump in the latter anymore.

DrahtBot commented at 10:14 pm on November 21, 2024: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31343.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	tdb3, vasild

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

DrahtBot added the label Tests on Nov 21, 2024

in test/functional/rpc_net.py:254 in 87d8b3efcb outdated

249@@ -250,7 +250,8 @@ def test_addnode_getaddednodeinfo(self):
250         assert_equal(len(added_nodes), 1)
251         assert_equal(added_nodes[0]['addednode'], ip_port)
252         self.log.info("Check that filtering by node works")
253-        self.nodes[0].addnode(node="11.22.33.44", command='add')
254+        # use link-local address (169.254.0.0/16), we don't want to open connections to the internet
255+        self.nodes[0].addnode(node="169.254.33.44", command='add')

vasild commented at 9:06 am on November 22, 2024:

I checked on my network and packets to 169.254.33.44 still leave the machine and are received by the default gateway.

What about using ~~-connect=0 or~~ (edit: this will not stop addnode) -proxy=127.0.0.1:1 for this particular test? One of the nodes it adds is available and it can connect to it (node2), but by reading the test I get the impression that this is not necessary. I.e. the test would still work and fulfill its purpose if it cannot connect to node2. So a setup like -proxy=127.0.0.1:1 where it cannot connect to anybody should be ok?

Or instead of 169.254.33.44 use a random Tor address here? E.g.

0        self.nodes[0].addnode(node="pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion", command='add')

theStack commented at 5:34 pm on November 22, 2024:

What about using -connect=0 or (edit: this will not stop addnode) -proxy=127.0.0.1:1 for this particular test?

I agree that the -proxy approach (as done also in #31142) is much better and it seems to work as expected. Note that I added the parameter not only for the problematic sub-test, but for the whole functional test, in order to prevent also other sub-tests from connecting outside.

vasild commented at 9:06 am on November 22, 2024: contributor

Concept ACK

test: avoid internet traffic in rpc_net.py

Can be tested by running

```
$ sudo tcpdump -i eth0 host 11.22.33.44
```

and verifying that no packets appear in the tcpdump output.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

988721d37a

theStack force-pushed on Nov 22, 2024

theStack commented at 5:41 pm on November 22, 2024: contributor

Thanks for reviewing @vasild! Force-pushed with your -proxy suggestion in #31343 (review) and updated the PR description accordingly with test instructions.

tdb3 approved

tdb3 commented at 5:18 am on November 25, 2024: contributor

ACK 988721d37a3c65e4ef86945133207fda243f2fba

Ran rpc_net.py in a virtual machine, running Wireshark on the VM’s sole external network interface. Saw no abnormal traffic (i.e. only typical background traffic such as stp). Re-ran the test on master, saw the TCP SYN attempts to 11.22.33.44.

Only briefly reviewed the existing code in the test. Will review more as time allows.

DrahtBot requested review from vasild on Nov 25, 2024

vasild approved

vasild commented at 8:31 am on November 25, 2024: contributor

ACK 988721d37a3c65e4ef86945133207fda243f2fba

The dummy proxy would also make it impossible to establish connections to the nodes running on localhost. E.g. nodes[0] now will not be able to connect to nodes[1]. I did not check the entire rpc_net.py, but I assume this must be ok, because the test passes with this setting.

test: avoid internet traffic in rpc_net.py #31343

Code Coverage & Benchmarks

Reviews