From c8ab8192ca65a2b3ac3153dd881309a4dee76e2c Mon Sep 17 00:00:00 2001 From: Michael Schmoock Date: Mon, 12 Sep 2022 15:47:44 +0200 Subject: [PATCH] peer_control: getinfo show correct port on discovered IPs Changelog-Fixed: peer_control: getinfo shows the correct port on discovered IPs --- gossipd/gossipd.c | 3 --- lightningd/lightningd.c | 2 ++ lightningd/lightningd.h | 4 ++++ lightningd/peer_control.c | 39 +++++++++++++++++++++++++++------------ tests/test_connection.py | 12 ++++++++++++ 5 files changed, 45 insertions(+), 15 deletions(-) diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index 84a588775..86a268394 100644 --- a/gossipd/gossipd.c +++ b/gossipd/gossipd.c @@ -350,9 +350,6 @@ static void handle_remote_addr(struct daemon *daemon, const u8 *msg) if (!fromwire_gossipd_remote_addr(msg, &remote_addr)) master_badmsg(WIRE_GOSSIPD_REMOTE_ADDR, msg); - /* Best guess is that we use default port for the selected network */ - remote_addr.port = chainparams_get_ln_port(chainparams); - switch (remote_addr.type) { case ADDR_TYPE_IPV4: if (daemon->remote_addr_v4 != NULL && diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 50d71ee31..29d5da761 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -209,6 +209,8 @@ static struct lightningd *new_lightningd(const tal_t *ctx) ld->remote_addr_v4 = NULL; ld->remote_addr_v6 = NULL; + ld->discovered_ip_v4 = NULL; + ld->discovered_ip_v6 = NULL; ld->listen = true; ld->autolisten = true; ld->reconnect = true; diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index a519e538d..bfacc17e7 100644 --- a/lightningd/lightningd.h +++ b/lightningd/lightningd.h @@ -158,6 +158,10 @@ struct lightningd { struct node_id remote_addr_v4_peer; struct node_id remote_addr_v6_peer; + /* verified discovered IPs to be used for anouncement */ + struct wireaddr *discovered_ip_v4; + struct wireaddr *discovered_ip_v6; + /* Bearer of all my secrets. */ int hsm_fd; struct subd *hsm; diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 648fcf615..c80c17a7b 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -1274,10 +1274,17 @@ static void update_remote_addr(struct lightningd *ld, const struct wireaddr *remote_addr, const struct node_id peer_id) { + u16 public_port; + /* failsafe to prevent privacy leakage. */ if (ld->always_use_proxy || ld->config.disable_ip_discovery) return; + /* Peers will have likey reported our dynamic outbound TCP port. + * Best guess is that we use default port for the selected network, + * until we add a commandline switch to override this. */ + public_port = chainparams_get_ln_port(chainparams); + switch (remote_addr->type) { case ADDR_TYPE_IPV4: /* init pointers first time */ @@ -1292,10 +1299,14 @@ static void update_remote_addr(struct lightningd *ld, break; } /* tell gossip we have a valid update */ - if (wireaddr_eq_without_port(ld->remote_addr_v4, remote_addr)) + if (wireaddr_eq_without_port(ld->remote_addr_v4, remote_addr)) { + ld->discovered_ip_v4 = tal_dup(ld, struct wireaddr, + ld->remote_addr_v4); + ld->discovered_ip_v4->port = public_port; subd_send_msg(ld->gossip, towire_gossipd_remote_addr( tmpctx, - ld->remote_addr_v4)); + ld->discovered_ip_v4)); + } /* store latest values */ *ld->remote_addr_v4 = *remote_addr; ld->remote_addr_v4_peer = peer_id; @@ -1311,10 +1322,14 @@ static void update_remote_addr(struct lightningd *ld, *ld->remote_addr_v6 = *remote_addr; break; } - if (wireaddr_eq_without_port(ld->remote_addr_v6, remote_addr)) + if (wireaddr_eq_without_port(ld->remote_addr_v6, remote_addr)) { + ld->discovered_ip_v6 = tal_dup(ld, struct wireaddr, + ld->remote_addr_v6); + ld->discovered_ip_v6->port = public_port; subd_send_msg(ld->gossip, towire_gossipd_remote_addr( tmpctx, - ld->remote_addr_v6)); + ld->discovered_ip_v6)); + } *ld->remote_addr_v6 = *remote_addr; ld->remote_addr_v6_peer = peer_id; break; @@ -2247,22 +2262,22 @@ static struct command_result *json_getinfo(struct command *cmd, for (size_t i = 0; i < count_announceable; i++) json_add_address(response, NULL, cmd->ld->announceable+i); - /* Currently, IP discovery will only be announced by gossipd, if we - * don't already have usable addresses. + /* Currently, IP discovery will only be announced by gossipd, + * if we don't already have usable addresses. * See `create_node_announcement` in `gossip_generation.c`. */ if (count_announceable == 0) { - if (cmd->ld->remote_addr_v4 != NULL && + if (cmd->ld->discovered_ip_v4 != NULL && !wireaddr_arr_contains( cmd->ld->announceable, - cmd->ld->remote_addr_v4)) + cmd->ld->discovered_ip_v4)) json_add_address(response, NULL, - cmd->ld->remote_addr_v4); - if (cmd->ld->remote_addr_v6 != NULL && + cmd->ld->discovered_ip_v4); + if (cmd->ld->discovered_ip_v6 != NULL && !wireaddr_arr_contains( cmd->ld->announceable, - cmd->ld->remote_addr_v6)) + cmd->ld->discovered_ip_v6)) json_add_address(response, NULL, - cmd->ld->remote_addr_v6); + cmd->ld->discovered_ip_v6); } json_array_end(response); diff --git a/tests/test_connection.py b/tests/test_connection.py index 7040d4dcd..b880e2364 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -83,6 +83,7 @@ def test_remote_addr(node_factory, bitcoind): l2.daemon.opts['bind-addr'] = l2.daemon.opts['addr'] del l2.daemon.opts['addr'] l2.start() + assert len(l2.rpc.getinfo()['address']) == 0 l2.rpc.connect(l1.info['id'], 'localhost', l1.port) logmsg = l2.daemon.wait_for_log("Peer says it sees our address as: 127.0.0.1:[0-9]{5}") @@ -95,6 +96,7 @@ def test_remote_addr(node_factory, bitcoind): 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) + assert len(l2.rpc.getinfo()['address']) == 0 def_port = default_ln_port(l2.info["network"]) @@ -110,6 +112,7 @@ def test_remote_addr(node_factory, bitcoind): # Now l1 sees l2 but without announced addresses. assert(len(l1.rpc.listnodes(l2.info['id'])['nodes'][0]['addresses']) == 0) assert not l2.daemon.is_in_log("Update our node_announcement for discovered address: 127.0.0.1:{}".format(def_port)) + assert len(l2.rpc.getinfo()['address']) == 0 # connect second node. This will not yet trigger `node_annoucement` update, # as we again do not have a channel at the time we connected. @@ -120,6 +123,7 @@ def test_remote_addr(node_factory, bitcoind): l2.fundchannel(l3, wait_for_active=True) bitcoind.generate_block(5) assert not l2.daemon.is_in_log("Update our node_announcement for discovered address: 127.0.0.1:{}".format(def_port)) + assert len(l2.rpc.getinfo()['address']) == 0 # restart, reconnect and re-check for updated node_annoucement. This time # l2 sees that two different peers with channel reported the same `remote_addr`. @@ -129,11 +133,19 @@ def test_remote_addr(node_factory, bitcoind): l2.daemon.wait_for_log("Update our node_announcement for discovered address: 127.0.0.1:{}".format(def_port)) l1.daemon.wait_for_log(f"Received node_announcement for node {l2.info['id']}") + # check l1 sees the updated node announcement via CLI listnodes address = l1.rpc.listnodes(l2.info['id'])['nodes'][0]['addresses'][0] assert address['type'] == "ipv4" assert address['address'] == "127.0.0.1" assert address['port'] == def_port + # also check l2 returns the announced address (and port) via CLI getinfo + getinfo = l2.rpc.getinfo() + assert len(getinfo['address']) == 1 + assert getinfo['address'][0]['type'] == 'ipv4' + assert getinfo['address'][0]['address'] == '127.0.0.1' + assert getinfo['address'][0]['port'] == def_port + @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):