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

Add Ability to Restrict Outbound Dials #5794

Merged
merged 3 commits into from
May 9, 2020
Merged

Conversation

nisdas
Copy link
Member

@nisdas nisdas commented May 9, 2020

What type of PR is this?

Bug Fix

What does this PR do? Why is it needed?

This adds the ability for the beacon node to restrict outbound dials to undesirable ip ranges. Partially helps with #5663. This is a patch for users, so that they do not have to mess around and configure with ufw or iptables to prevent dials to internal ips.

Which issues(s) does this PR fix?

#5663

Other notes for review

N.A

@nisdas nisdas added the Ready For Review A pull request ready for code review label May 9, 2020
@nisdas nisdas requested a review from a team as a code owner May 9, 2020 11:21
@wgknowles
Copy link
Contributor

A suggestion, shouldn't private ranges be excluded by default? Anyone running multiple nodes on a private network would likely be advanced enough to know they need to enable this, but I don't think your average user just running defaults would know why this is a good idea.

10.0.0.0/8
172.16.0.0/12
192.168.0.0/16
100.64.0.0/10
169.254.0.0/16

@codecov
Copy link

codecov bot commented May 9, 2020

Codecov Report

Merging #5794 into master will increase coverage by 0.35%.
The diff coverage is 86.66%.

@@            Coverage Diff             @@
##           master    #5794      +/-   ##
==========================================
+ Coverage   59.59%   59.94%   +0.35%     
==========================================
  Files         311      311              
  Lines       26208    26284      +76     
==========================================
+ Hits        15618    15757     +139     
+ Misses       8462     8406      -56     
+ Partials     2128     2121       -7     

@nisdas
Copy link
Member Author

nisdas commented May 9, 2020

@wgknowles initially I wanted to go in that direction, but I realised it would break existing behaviour. if you are running a prysm node with other local peers you would have to white list all the relevant private ips, which becomes cumbersome to do.

Also importantly the source of all these excessive dials to our private ips is our kademlia discovery which will be going away in the next restart. DiscoveryV5 should be able to filter out private, un-reachable IPs better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants