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.

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

bug: 0.0.0.0 included in listenAddrs of identify message #1427

Closed
fryorcraken opened this issue Nov 28, 2022 · 8 comments · Fixed by #1982
Closed

bug: 0.0.0.0 included in listenAddrs of identify message #1427

fryorcraken opened this issue Nov 28, 2022 · 8 comments · Fixed by #1982
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@fryorcraken
Copy link
Collaborator

Problem

When the identify protocol is execute between js-waku and nwaku v0.13.0, the nwaku peer includes a multiaddrs with /ip4/0.0.0.0/... in the listenAddrs field of the identify message.

Note that js-libp2p uses listenAddrs when signedPeerRecord is not present in the identify message: https://github.com/libp2p/js-libp2p/blob/bae32bafce75a3801a7a96f77a9ccf43b3208f9c/src/identify/index.ts#L361

AFAIK, signedPeerRecord is not use by nwaku.

Note that according to libp2p dev, only valid addresses are expected:

Impact

This is the cause for waku-org/js-waku#751 which is the highest priority bug for js-waku

To reproduce

This happens for any connection, including connection to a local nwaku node or a wakuv2 prod fleet nwaku node.

  1. https://examples.waku.org/light-js
  2. set debug=waku*,libp2p* in local storage
  3. click dial

You will see:

 libp2p:identify no signed peer record received from 16Uiu2HAkvWiyFsgRhuJEb9JfjYxEkoHLgnUQmr1N5mKWnYjxYRVm +0ms index.js:29065:10
22:45:42.181 libp2p:identify falling back to legacy addresses from 16Uiu2HAkvWiyFsgRhuJEb9JfjYxEkoHLgnUQmr1N5mKWnYjxYRVm +0ms

To get the 0.0.0.0 value it's trickier as you would need to use a modify version of js-llbp2p that logs the content of the identify message (see libp2p link in first section).

Expected behavior

  1. listenAddrs only contains values that are expected to be reachable by remote node
  2. signedPeerRecord is used or equivalent functionality in ENR format https://rfc.vac.dev/spec/31/

nwaku version/commit hash

v0.13

@fryorcraken fryorcraken added bug Something isn't working track:maintenance labels Nov 28, 2022
@fryorcraken
Copy link
Collaborator Author

Cc @Menduist. Not sure if it's nwaku usage of nim-libp2p or an issue on nim-libp2p side.

@jm-clius
Copy link
Contributor

How it currently works:

  1. nwaku (seemingly correctly) updates the nim-libp2p switch with announced (external) addresses by providing an AddressMapper: https://github.com/waku-org/nwaku/blob/master/waku/v2/node/waku_node.nim#L962-L968
  2. nim-libp2p (seemingly correctly) uses this to populate the identify: https://github.com/status-im/nim-libp2p/blob/unstable/libp2p/protocols/identify.nim#L91-L92

@fryorcraken can you confirm if you only see 0.0.0.0 in the listenAddrs or if you sometimes see an external address included? What would the expected listenAddrs be if the node couldn't detemine its own external/public IP?

@fryorcraken
Copy link
Collaborator Author

I have tested with the wakuv2 prod and test fleet and it looks like 0.0.0.0 is not included.

Which means it's only included when I run the node locally. When running locally, I thought I was simulating the original issue waku-org/js-waku#751 but with faster feedback loop (I could restart nwaku node instead of waiting 10min for TCP/WS to timeout).

I'll have to resume investigations.

IMO, 0.0.0.0 is still not ideal even if own external/public ip can not be found. I'd expect to have the lan ip and/or the local loop (127.0.0.1) included instead.

As per RFC 1122, section 3.2.1.3 Addressing: RFC-791 Section 3.2:

This host on this network. MUST NOT be sent, except as a source address as part of an initialization procedure by which the host learns its own IP address.

As listenAddrs is expected to be used by the remote peer, it should only contains ip addresses that can can be used as a destination address.

@fryorcraken
Copy link
Collaborator Author

I can confirm that when it's ran locally:

./build/wakunode2 --relay=true --websocket-support=true --nodekey=83c5bcf4c39de830fc38e60d8e21f43a1f47e05a50a266890ee9acb613486630 --ports-shift=12

The address 0.0.0.0 is included in the identify message.

Logs:

▶ ./build/wakunode2 --relay=true --websocket-support=true --nodekey=83c5bcf4c39de830fc38e60d8e21f43a1f47e05a50a266890ee9acb613486630 --ports-shift=12
INF 2023-01-06 13:59:35.234+11:00 Initializing networking                    topics="wakunode" tid=901602 file=waku_node.nim:206 addrs="@[/ip4/0.0.0.0/tcp/60012, /ip4/0.0.0.0/tcp/8012/ws]"
INF 2023-01-06 13:59:35.235+11:00 mounting relay protocol                    topics="waku node" tid=901602 file=waku_node.nim:380
INF 2023-01-06 13:59:35.235+11:00 relay mounted successfully                 topics="waku node" tid=901602 file=waku_node.nim:403
INF 2023-01-06 13:59:35.236+11:00 mounting libp2p ping protocol              topics="waku node" tid=901602 file=waku_node.nim:772
INF 2023-01-06 13:59:35.236+11:00 Starting Waku node                         topics="waku node" tid=901602 file=waku_node.nim:897 version=v0.13.0-10-g9396f6
INF 2023-01-06 13:59:35.236+11:00 PeerInfo                                   topics="waku node" tid=901602 file=waku_node.nim:900 peerId=16U*cabij4 addrs=@[]
INF 2023-01-06 13:59:35.236+11:00 Listening on                               topics="waku node" tid=901602 file=waku_node.nim:907 full=[/ip4/0.0.0.0/tcp/60012/p2p/16Uiu2HAmRaMXmYh8zSMUEsQmu7Yb6zB2KfLEmKnAmJCrqScabij4][/ip4/0.0.0.0/tcp/8012/ws/p2p/16Uiu2HAmRaMXmYh8zSMUEsQmu7Yb6zB2KfLEmKnAmJCrqScabij4]
INF 2023-01-06 13:59:35.236+11:00 DNS: discoverable ENR                      topics="waku node" tid=901602 file=waku_node.nim:908 enr=enr:-KO4QJNizu5pkRa0Uan6g4HUp-ZFIu-VJbulF1VFUEv34ftnPvFyyMuotOFSnSzZDPo4divMaxBZa40Fkwod_wV5ho8BgmlkgnY0gmlwhAAAAACKbXVsdGlhZGRyc4wACgQAAAAABh9M3QOJc2VjcDI1NmsxoQO_9_mdf-esZZrLB3hswJT0HgQmpw5PxmyQ4MX0x1i4y4N0Y3CC6myFd2FrdTIB
INF 2023-01-06 13:59:35.236+11:00 starting relay protocol                    topics="waku node" tid=901602 file=waku_node.nim:347
INF 2023-01-06 13:59:35.236+11:00 subscribe                                  topics="waku node" tid=901602 file=waku_node.nim:265 topic=/waku/2/default-waku/proto
INF 2023-01-06 13:59:35.236+11:00 subscribe                                  topics="waku node" tid=901602 file=waku_node.nim:265 topic=/waku/2/default-waku/proto
INF 2023-01-06 13:59:35.236+11:00 relay started successfully                 topics="waku node" tid=901602 file=waku_node.nim:373
WRN 2023-01-06 13:59:35.237+11:00 Starting gossipsub twice                   topics="libp2p gossipsub" tid=901602 file=gossipsub.nim:604
INF 2023-01-06 13:59:35.238+11:00 Node started successfully                  topics="waku node" tid=901602 file=waku_node.nim:926
INF 2023-01-06 13:59:35.238+11:00 Starting JSON-RPC HTTP server              topics="JSONRPC-HTTP-SERVER" tid=901602 file=httpserver.nim:60 url=http://127.0.0.1:8557
INF 2023-01-06 13:59:35.238+11:00 subscribe                                  topics="waku node" tid=901602 file=waku_node.nim:265 topic=/waku/2/default-waku/proto
INF 2023-01-06 13:59:35.238+11:00 RPC Server started                         topics="wakunode jsonrpc" tid=901602 file=wakunode2_setup_rpc.nim:59 address=127.0.0.1:8557
INF 2023-01-06 13:59:35.238+11:00 Node setup complete 

However, if I specify the listen address, then the address specified is included in the identify message, for example:

▶ ./build/wakunode2 --relay=true --websocket-support=true --nodekey=83c5bcf4c39de830fc38e60d8e21f43a1f47e05a50a266890ee9acb613486630 --ports-shift=12 --listen-address=127.0.0.1
INF 2023-01-06 14:07:08.399+11:00 Initializing networking                    topics="wakunode" tid=903542 file=waku_node.nim:206 addrs="@[/ip4/127.0.0.1/tcp/60012, /ip4/127.0.0.1/tcp/8012/ws]"
INF 2023-01-06 14:07:08.401+11:00 mounting relay protocol                    topics="waku node" tid=903542 file=waku_node.nim:380
INF 2023-01-06 14:07:08.401+11:00 relay mounted successfully                 topics="waku node" tid=903542 file=waku_node.nim:403
INF 2023-01-06 14:07:08.401+11:00 mounting libp2p ping protocol              topics="waku node" tid=903542 file=waku_node.nim:772
INF 2023-01-06 14:07:08.401+11:00 Starting Waku node                         topics="waku node" tid=903542 file=waku_node.nim:897 version=v0.13.0-10-g9396f6
INF 2023-01-06 14:07:08.401+11:00 PeerInfo                                   topics="waku node" tid=903542 file=waku_node.nim:900 peerId=16U*cabij4 addrs=@[]
INF 2023-01-06 14:07:08.401+11:00 Listening on                               topics="waku node" tid=903542 file=waku_node.nim:907 full=[/ip4/127.0.0.1/tcp/60012/p2p/16Uiu2HAmRaMXmYh8zSMUEsQmu7Yb6zB2KfLEmKnAmJCrqScabij4][/ip4/127.0.0.1/tcp/8012/ws/p2p/16Uiu2HAmRaMXmYh8zSMUEsQmu7Yb6zB2KfLEmKnAmJCrqScabij4]
INF 2023-01-06 14:07:08.401+11:00 DNS: discoverable ENR                      topics="waku node" tid=903542 file=waku_node.nim:908 enr=enr:-KO4QPwiW8rEqJXkZsqjozIZxfE8TQ4FWQnuL3SS2WeLdNsyME_jqtfkPaAAiiKzmscRDSzpjYeghGo8TUFU74PV-ZYBgmlkgnY0gmlwhH8AAAGKbXVsdGlhZGRyc4wACgR_AAABBh9M3QOJc2VjcDI1NmsxoQO_9_mdf-esZZrLB3hswJT0HgQmpw5PxmyQ4MX0x1i4y4N0Y3CC6myFd2FrdTIB
INF 2023-01-06 14:07:08.402+11:00 starting relay protocol                    topics="waku node" tid=903542 file=waku_node.nim:347
INF 2023-01-06 14:07:08.402+11:00 subscribe                                  topics="waku node" tid=903542 file=waku_node.nim:265 topic=/waku/2/default-waku/proto
INF 2023-01-06 14:07:08.402+11:00 subscribe                                  topics="waku node" tid=903542 file=waku_node.nim:265 topic=/waku/2/default-waku/proto
INF 2023-01-06 14:07:08.402+11:00 relay started successfully                 topics="waku node" tid=903542 file=waku_node.nim:373
WRN 2023-01-06 14:07:08.403+11:00 Starting gossipsub twice                   topics="libp2p gossipsub" tid=903542 file=gossipsub.nim:604
INF 2023-01-06 14:07:08.403+11:00 Node started successfully                  topics="waku node" tid=903542 file=waku_node.nim:926
INF 2023-01-06 14:07:08.403+11:00 Starting JSON-RPC HTTP server              topics="JSONRPC-HTTP-SERVER" tid=903542 file=httpserver.nim:60 url=http://127.0.0.1:8557
INF 2023-01-06 14:07:08.403+11:00 subscribe                                  topics="waku node" tid=903542 file=waku_node.nim:265 topic=/waku/2/default-waku/proto
INF 2023-01-06 14:07:08.404+11:00 RPC Server started                         topics="wakunode jsonrpc" tid=903542 file=wakunode2_setup_rpc.nim:59 address=127.0.0.1:8557
INF 2023-01-06 14:07:08.404+11:00 Node setup complete  

@jm-clius
Copy link
Contributor

@fryorcraken revisiting expected behaviour here:

  • binding by default to localhost (127.0.0.1) rather than 0.0.0.0 will cause the node to only listen for other connections on localhost (thanks for clearing this up @Menduist) so should not be changed in config
  • not sure if simply "replacing" the "any" address (0.0.0.0) with the localhost/loopback IP address (127.0.0.) for announcing/identify would make sense as it may be inaccurate

So perhaps the correct behaviour would be to not include any listenAddrs in identify if no public IP addresses were configured/can be determined? How would that be interpreted on js side?

@fryorcraken
Copy link
Collaborator Author

@fryorcraken revisiting expected behaviour here:

* binding by default to localhost (`127.0.0.1`) rather than `0.0.0.0` will cause the node to only listen for other connections on localhost (thanks for clearing this up @Menduist) so should not be changed in config

* not sure if simply "replacing" the "any" address (`0.0.0.0`) with the localhost/loopback IP address (`127.0.0.`) for announcing/`identify` would make sense as it may be inaccurate

I agree with the reasoning.

So perhaps the correct behaviour would be to not include any listenAddrs in identify if no public IP addresses were configured/can be determined? How would that be interpreted on js side?

Yes, sounds reasonable. Nothing would happen: the address that the js node used to dial the nwaku node would not be overridden and afaik, there would not be any error or ill effect. See usage of listenAddrs in https://github.com/libp2p/js-libp2p/blob/bae32bafce75a3801a7a96f77a9ccf43b3208f9c/src/identify/index.ts

@jm-clius jm-clius removed this from the Release 0.16.0 milestone Mar 17, 2023
@vpavlin vpavlin moved this from To Do to Priority in Waku Jul 15, 2023
@vpavlin vpavlin added this to the Release 0.20.0 milestone Aug 1, 2023
@vpavlin vpavlin added the good first issue Good for newcomers label Aug 15, 2023
@gabrielmer gabrielmer self-assigned this Sep 5, 2023
@gabrielmer gabrielmer moved this from Priority to In Progress in Waku Sep 5, 2023
@gabrielmer
Copy link
Contributor

Just went over the Issue with @vpavlin.
It seems there's a bit of confusion related to addressing that might be causing out issues.
For example in the switch builder, we're using "bindIP" to ensure binding to 0.0.0.0, but in PeerInfo, it's being treated as "listenAddress."

There's the case when "--nat=extip:..." is used, as the advertised address differs from "bindIP."

It seems also that we are using "toRemotePeerInfo()" primarily for local and testing use and may not behave as expected when nodes communicate over the network, which could cause our current addressing issues.

We are as well considering the possibility of relying on ENR as a means to get the peerId and addresses.

It seems that there's lots of different addressing possibilities and we don't have a clear idea of for what purpose we should use each address source? @Menduist, could you please advise us on how to approach addressing?

@gabrielmer
Copy link
Contributor

gabrielmer commented Sep 11, 2023

Weekly Update

  • achieved: fixed bug, updated tests according to new fixes and raised PR

@github-project-automation github-project-automation bot moved this from In Progress to Done in Waku Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants