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

Do not absolutely protect local peers; decide group ties based on time. #7438

Merged
merged 2 commits into from Feb 1, 2016

Conversation

@gmaxwell
Copy link
Contributor

gmaxwell commented Jan 28, 2016

[This is the first patch of #7082 broken out targeting 0.12]

With automatic tor HS support in place we should probably not be providing
absolute protection for local peers, since HS inbound could be used to
attack pretty easily. Instead, this counts on the latency metric inside
AttemptToEvictConnection to privilege actually local peers.

This also corrects a bug the case of tying group size where the code may
fail to select the group with the newest member. Since newest time
is the final selection criteria, failing to break ties on it
on the step before can undermine the final selection.

Tied netgroups are very common.

gmaxwell added 2 commits Jan 28, 2016
With automatic tor HS support in place we should probably not be providing
 absolute protection for local peers, since HS inbound could be used to
 attack pretty easily.  Instead, this counts on the latency metric inside
 AttemptToEvictConnection to privilege actually local peers.
This corrects a bug the case of tying group size where the code may
 fail to select the group with the newest member. Since newest time
 is the final selection criteria, failing to break ties on it
 on the step before can undermine the final selection.

Tied netgroups are very common.
@gmaxwell gmaxwell force-pushed the gmaxwell:dont_protect_local branch to 8e09f91 Jan 28, 2016
@dcousens
Copy link
Contributor

dcousens commented Jan 28, 2016

What is HS?

@gmaxwell
Copy link
Contributor Author

gmaxwell commented Jan 28, 2016

Hidden service.

@jonasschnelli jonasschnelli added the P2P label Jan 29, 2016
@laanwj laanwj added this to the 0.12.0 milestone Jan 29, 2016
@petertodd
Copy link
Contributor

petertodd commented Jan 31, 2016

@laanwj
Copy link
Member

laanwj commented Feb 1, 2016

utACK 8e09f91, thanks for doing this on such short notice

@laanwj laanwj merged commit 8e09f91 into bitcoin:0.12 Feb 1, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
laanwj added a commit that referenced this pull request Feb 1, 2016
… based on time.

8e09f91 Decide eviction group ties based on time. (Gregory Maxwell)
46dbcd4 Do not absolutely protect local peers from eviction. (Gregory Maxwell)
@laanwj
Copy link
Member

laanwj commented Feb 2, 2016

This was cherry-picked into master via 1e05727 and 1e9613a (#7453)

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

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.