Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: peer_control getinfo to show correct port on discovered IPs #5585

Merged
merged 2 commits into from
Sep 15, 2022
Merged

fix: peer_control getinfo to show correct port on discovered IPs #5585

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bcb6d32
peer_control: getinfo show correct port on discovered IPs
m-schmoock Sep 12, 2022
8fb6ed8
gossipd: rename remote_addr to discovered_ip within gossipd
m-schmoock Sep 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions gossipd/gossipd.c
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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