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

Verify that addresses reported by identify are correct before inserting them in the DHT #564

Open
tomaka opened this issue Feb 13, 2020 · 2 comments
Labels
I1-security The node fails to follow expected, security-sensitive, behaviour.

Comments

@tomaka
Copy link
Contributor

tomaka commented Feb 13, 2020

Right now nodes tell us the addresses they are reachable from, and we immediately insert them in the DHT without any verification. This leads to a lot of unreachable addresses in the DHT, such as 127.0.0.1 or IPs behind NATs.
Instead, we should first attempt to connect to these addresses to see if they're reachable.

Note that connectivity is not transitive. For example if a node A is listening on a given address, this address might be reachable by B but not by C. If we implement this change, then C will not tell B about that address that A has, even though it would have been desirable. This is considered an acceptable trade-off.

On the implementation side, the good news is that Kademlia should automatically detect us reaching the node and add the address we connected to to the DHT, without having to call any method (although I'm not sure that this is the case, but I think that it should be the right logic).

In other words, all we have to do is call Swarm::dial for each address that a node reports through identity.

However, this is complicated by the "one connection per node" policy. If we connect to a node we're already connected to, we're going to drop the existing connection.
In other words, we first have to land libp2p/rust-libp2p#1440

cc @romanb @arkpar

@twittner
Copy link

We have to be careful though not to introduce a DDOS vector by just dialling every reported address. Rate-limiting the dialling attempts may help a bit.

@tomaka
Copy link
Contributor Author

tomaka commented Apr 20, 2020

Even after #563 there is still a problem to solve:

At the moment we insert nodes in the DHT as soon as we connect to them. We should instead only insert in the DHT nodes that belong to the correct chain.

In other words, we should wait until we know that node is on chain A before inserting it in the DHT of chain A.

See also paritytech/substrate#3303

@altonen altonen transferred this issue from paritytech/substrate Aug 24, 2023
@the-right-joyce the-right-joyce added I1-security The node fails to follow expected, security-sensitive, behaviour. and removed I2-security labels Aug 25, 2023
claravanstaden pushed a commit to Snowfork/polkadot-sdk that referenced this issue Dec 8, 2023
helin6 pushed a commit to boolnetwork/polkadot-sdk that referenced this issue Feb 5, 2024
* Make EvmConfig pub
Remove pallet-balances trait bound from pallet-evm

* cargo fmt
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Mar 26, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Mar 27, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I1-security The node fails to follow expected, security-sensitive, behaviour.
Projects
Status: Backlog 🗒
Development

No branches or pull requests

4 participants