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

Skip private networks #49

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Skip private networks #49

wants to merge 3 commits into from

Conversation

konradkonrad
Copy link

@konradkonrad konradkonrad commented Oct 10, 2016

PR opened for discussion
ping @fjl @heikoheiko
please fill in other interested parties!

Description

devp2p did not discriminate "private" and "public" network addresses.
This can lead to at least the following issues:

  1. If peers (incorrectly) report a "private" network addresses as endpoints,
    this fills the discovery with a lot of incorrect entries.
    see also: https://www.reddit.com/r/ethereum/comments/55xjrb/yet_another_new_attack_using_faked_ip_src/

  2. On the other hand is it hard to keep nodes in a private network protected
    from discovery requests from the outside.

With this change, each Address from devp2p will be checked against a range of
"allowed networks", which consists of a whitelist (default empty) and a blacklist
(default populated with most entries from ipaddress._IPv4Constants._private_networks).
If a new Address is not in the "allowed" list, it will invalidate with a ValueError.

Entries to the whitelist take precedence over the blacklist. User libraries
can adjust the behaviour via environment variables or changing the instance
devp2p.discovery.ALLOWED_NETWORKS.

`devp2p` did not discriminate "private" and public network addresses.
This can lead to at least the following issues:

1) If peers (incorrectly) reports a private network address as endpoint,
this fills the discovery with a lot of incorrect entries.

2) On the other hand is it hard to keep nodes in a private network protected
from discovery requests from the outside.

With this change, each Address from devp2p will be checked against a range of
"allowed networks", which consists of a whitelist (default empty) and a blacklist
(default populated with most entries from `ipaddress._IPv4Constants._private_networks`).

Entries to the whitelist take precedence over the blacklist. User libraries
can adjust the behaviour via environment variables or changing the instance
`devp2p.discovery.ALLOWED_NETWORKS`.
In tests we need to enable local networks in the discovery module.
@coveralls
Copy link

Coverage Status

Coverage decreased (-6.9%) to 81.261% when pulling c2f4f1b on skip_private_networks into 08b4862 on develop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants