Skip to content
New issue

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

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

Already on GitHub? # to your account

p2p: do not advertise private and non-routable addresses #6092

Merged

Conversation

algorandskiy
Copy link
Contributor

@algorandskiy algorandskiy commented Aug 2, 2024

Summary

When p2p net listens on all interfaces ("0.0.0.0:0", ":0", ":N"), it advertises everything including loopback and private interface addresses. The solution is to handle this specification and apply libp2p's AddressFactory filter.

Note, when some specific address is set like loopback or a private interface, it recognized as intentional action and no filters applied. This allows to all unit tests starting network on "127.0.0.1:0" to continue to operate, as well allows private nets to run if NetAddress is set appropriate (say to "10.1.0.101:0").

During algonet testing I found EC2 instances only have loopback and a private 172.31.x.x addresses so that filtered.
libp2p has address observation facility (part of netidentity) that reports addresses seen at least 4 times (activated() piece) in last 30 minutes (TTL piece).
This means we need at least 4 p2p bootstrap nodes to allow other nodes to advertise their observed addresses over DHT.

Test Plan

  1. Added unit tests for addressFilter' function and for MakeHostwith differentNetAddress` specifications.
  2. Successfully tested with algonet the following subset of s1-p2p (8 relays, 5 part, 5 non-part nodes):
PARAMS=-w 5 -R 8 -N 5 -n 5 --npn-algod-nodes 5 --node-template node.json --relay-template relay.json --non-participating-node-template nonPartNode.json

with 4 of 8 relays being p2p bootstrap nodes (via DNS TXT) and with all nodes "EnableDHTProviders": true. I was able to add an extra node with "GossipFanout": 8 to this network and have it connected to all 8 relays despite the fact only 8 are registered in DNS.

@algorandskiy algorandskiy added the p2p Work related to the p2p project label Aug 2, 2024
@algorandskiy algorandskiy self-assigned this Aug 2, 2024
Copy link

codecov bot commented Aug 2, 2024

Codecov Report

Attention: Patch coverage is 61.11111% with 21 lines in your changes missing coverage. Please review.

Project coverage is 56.23%. Comparing base (d2c4ca7) to head (0453241).

Files Patch % Lines
network/p2p/p2p.go 61.11% 17 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6092      +/-   ##
==========================================
- Coverage   56.26%   56.23%   -0.03%     
==========================================
  Files         489      489              
  Lines       69637    69691      +54     
==========================================
+ Hits        39182    39192      +10     
- Misses      27796    27831      +35     
- Partials     2659     2668       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@algorandskiy algorandskiy marked this pull request as ready for review August 2, 2024 16:47
@algorandskiy algorandskiy changed the title WIP: p2p: do not advertise private and non-routable addresses p2p: do not advertise private and non-routable addresses Aug 2, 2024
@algorandskiy algorandskiy requested a review from gmalouf August 5, 2024 17:54
gmalouf
gmalouf previously approved these changes Aug 5, 2024
jasonpaulos
jasonpaulos previously approved these changes Aug 6, 2024
Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good just some nits

@algorandskiy algorandskiy dismissed stale reviews from jasonpaulos and gmalouf via 0453241 August 6, 2024 14:58
@algorandskiy algorandskiy merged commit c6a433b into algorand:master Aug 7, 2024
18 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Enhancement p2p Work related to the p2p project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants