Skip to content

Commit

Permalink
peer_control: getinfo show correct port on discovered IPs
Browse files Browse the repository at this point in the history
Changelog-Fixed: peer_control: getinfo shows the correct port on discovered IPs
  • Loading branch information
m-schmoock authored and niftynei committed Sep 19, 2022
1 parent 9f4dba1 commit e1290b4
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 15 deletions.
3 changes: 0 additions & 3 deletions gossipd/gossipd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down
2 changes: 2 additions & 0 deletions lightningd/lightningd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions lightningd/lightningd.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
39 changes: 27 additions & 12 deletions lightningd/peer_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
12 changes: 12 additions & 0 deletions tests/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand All @@ -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"])

Expand All @@ -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.
Expand All @@ -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`.
Expand All @@ -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):
Expand Down

0 comments on commit e1290b4

Please sign in to comment.