Skip to content

Commit

Permalink
connectd: use dev_allow_localhost for remote_addr testing
Browse files Browse the repository at this point in the history
Before this fix, there was the situation where a DEVELOPER=1 node would
announce non-public addresses on mainnet if detected. Since there
are some nodes on the internet that falsely report local addresses
we move this 'testing feature' to 'dev-allow-locahost' nodes.

Changelog-None
  • Loading branch information
m-schmoock authored and rustyrussell committed Jun 17, 2022
1 parent 32c4540 commit a2b75b6
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 18 deletions.
9 changes: 4 additions & 5 deletions connectd/peer_exchange_initmsg.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,10 @@ static struct io_plan *peer_init_received(struct io_conn *conn,
switch (remote_addr->type) {
case ADDR_TYPE_IPV4:
case ADDR_TYPE_IPV6:
#if DEVELOPER /* ignore private addresses (non-DEVELOPER builds) */
if (!address_routable(remote_addr, true))
#else
if (!address_routable(remote_addr, false))
#endif /* DEVELOPER */
/* Drop non-public addresses when not testing */
if (!address_routable(remote_addr,
IFDEV(peer->daemon->dev_allow_localhost,
false)))
remote_addr = tal_free(remote_addr);
break;
/* We are only interested in IP addresses */
Expand Down
42 changes: 30 additions & 12 deletions tests/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
expected_channel_features,
check_coin_moves, first_channel_id, account_balance, basic_fee,
scriptpubkey_addr,
DEVELOPER, EXPERIMENTAL_FEATURES, mine_funding_to_announce
EXPERIMENTAL_FEATURES, mine_funding_to_announce
)
from pyln.testing.utils import SLOW_MACHINE, VALGRIND, EXPERIMENTAL_DUAL_FUND, FUNDAMOUNT

Expand Down Expand Up @@ -46,9 +46,6 @@ def test_connect_basic(node_factory):
assert len(l1.rpc.listpeers()) == 1
assert len(l2.rpc.listpeers()) == 1

if DEVELOPER:
print(l1.daemon.wait_for_log("Peer says it sees our address as: 127.0.0.1:[0-9]{5}"))

# Should get reasonable error if unknown addr for peer.
with pytest.raises(RpcError, match=r'Unable to connect, no address known'):
l1.rpc.connect('032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e')
Expand All @@ -62,25 +59,41 @@ def test_connect_basic(node_factory):
l1.rpc.connect('032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e', 'localhost', l2.port)


@pytest.mark.developer("needs DEVELOPER=1 for having localhost remote_addr and fast gossip")
@pytest.mark.developer("needs DEVELOPER=1 for fast gossip and --dev-allow-localhost for local remote_addr")
def test_remote_addr(node_factory, bitcoind):
"""Check address discovery (BOLT1 #917) init remote_addr works as designed:
`node_announcement` update must only be send out when:
- at least two peers
- we have a channel with
- report the same `remote_addr`
We perform logic tests on L2, setup:
l1 --> [l2] <-- l3
"""
# don't announce anything per se
opts = {'announce-addr': [], 'may_reconnect': True}
l1, l2, l3 = node_factory.get_nodes(3, opts=opts)
opts = {'may_reconnect': True,
'dev-allow-localhost': None,
'dev-no-reconnect': None}
l1, l2, l3 = node_factory.get_nodes(3, opts)

# Disable announcing local autobind addresses with dev-allow-localhost.
# We need to have l2 opts 'bind-addr' to the (generated) value of 'addr'.
# So we stop, set 'bind-addr' option, delete 'addr' and restart first.
l2.stop()
l2.daemon.opts['bind-addr'] = l2.daemon.opts['addr']
del l2.daemon.opts['addr']
l2.start()

l2.rpc.connect(l1.info['id'], 'localhost', l1.port)
l2.daemon.wait_for_log("Peer says it sees our address as: 127.0.0.1:[0-9]{5}")

# Fund first channel so initial node_announcement is send
# and also check no addresses have been announced yet
l1.fundchannel(l2)
bitcoind.generate_block(5)
l1.daemon.wait_for_log(f"Received node_announcement for node {l2.info['id']}")
assert(len(l1.rpc.listnodes(l2.info['id'])['nodes'][0]['addresses']) == 0)

# when we restart l1 with a channel and reconnect, node_annoucement update
# must not yet be send as we need the same `remote_addr` confirmed from a
Expand All @@ -91,9 +104,8 @@ def test_remote_addr(node_factory, bitcoind):
l2.rpc.connect(l1.info['id'], 'localhost', l1.port)
l2.daemon.wait_for_log("Peer says it sees our address as: 127.0.0.1:[0-9]{5}")

# now only l1 sees l2 without announced addresses (disabled by opts)
# Now l1 sees l2 but without announced addresses.
assert(len(l1.rpc.listnodes(l2.info['id'])['nodes'][0]['addresses']) == 0)
assert(len(l3.rpc.listnodes(l2.info['id'])['nodes']) == 0)
assert not l2.daemon.is_in_log("Update our node_announcement for discovered address: 127.0.0.1:9735")

# connect second node. This will not yet trigger `node_annoucement` update,
Expand All @@ -120,12 +132,18 @@ def test_remote_addr(node_factory, bitcoind):
assert address['port'] == 9735


@pytest.mark.developer("needs DEVELOPER=1 for having localhost remote_addr and fast gossip")
@pytest.mark.developer("needs DEVELOPER=1 for fast gossip and --dev-allow-localhost for local remote_addr")
def test_remote_addr_disabled(node_factory, bitcoind):
"""Simply tests that IP address discovery annoucements can be turned off
We perform logic tests on L2, setup:
l1 --> [l2] <-- l3
"""
opts = {'announce-addr': [], 'disable-ip-discovery': None, 'may_reconnect': True}
l1, l2, l3 = node_factory.get_nodes(3, opts=opts)
opts = {'dev-allow-localhost': None,
'disable-ip-discovery': None,
'may_reconnect': True,
'dev-no-reconnect': None}
l1, l2, l3 = node_factory.get_nodes(3, opts=[opts, opts, opts])

# l1->l2
l2.rpc.connect(l1.info['id'], 'localhost', l1.port)
Expand Down
5 changes: 4 additions & 1 deletion tests/test_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,10 @@ def test_peer_connected_remote_addr(node_factory):
The optional tlv `remote_addr` should only be visible to the initiator l1.
"""
l1, l2 = node_factory.get_nodes(2, opts={'plugin': os.path.join(os.getcwd(), 'tests/plugins/peer_connected_logger_a.py')})
pluginpath = os.path.join(os.getcwd(), 'tests/plugins/peer_connected_logger_a.py')
l1, l2 = node_factory.get_nodes(2, opts={
'plugin': pluginpath,
'dev-allow-localhost': None})
l1id = l1.info['id']
l2id = l2.info['id']

Expand Down

0 comments on commit a2b75b6

Please sign in to comment.