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

[Networking] Handling iHave broken promises (and part-2 of overpromising) spams #4566

Merged
merged 50 commits into from
Jul 19, 2023

Conversation

yhassanzadeh13
Copy link
Contributor

@yhassanzadeh13 yhassanzadeh13 commented Jul 18, 2023

This PR introduces changes to our GossipSub parameter configurations to effectively address and mitigate iHave Broken Promise attacks. An iHave Broken Promise attack is characterized by a peer advertising an iHave (indicating they have a certain message), but failing to respond to the iWant requests for that message.

We have fine-tuned the GossipSub parameters, notably defaultBehaviourPenaltyThreshold, defaultBehaviourPenaltyWeight, and defaultBehaviourPenaltyDecay to penalize such behavior more appropriately.

The defaultBehaviourPenaltyThreshold is set to 10, meaning we at most tolerate 10 RPCs containing iHave Broken Promises. The defaultBehaviourPenaltyWeight applies a penalty when a peer's misbehavior exceeds the threshold. The penalty is applied to the square of the difference between the misbehavior counter and the threshold, to provide a substantial disincentive for malicious peers. If a peer misbehaves more than sqrt(2) * 10 times the threshold, they will be graylisted or disconnected.

The defaultBehaviourPenaltyDecay is set to 0.9, causing the misbehavior counter to decay by 10% per decay interval. This means that peers have a chance to recover from their previous misbehavior and incentivizes improved behavior in the future.

@codecov-commenter
Copy link

codecov-commenter commented Jul 18, 2023

Codecov Report

Merging #4566 (80d470e) into master (1aff7f9) will increase coverage by 0.00%.
The diff coverage is 70.58%.

@@           Coverage Diff           @@
##           master    #4566   +/-   ##
=======================================
  Coverage   54.47%   54.47%           
=======================================
  Files         914      914           
  Lines       85278    85290   +12     
=======================================
+ Hits        46454    46464   +10     
- Misses      35239    35242    +3     
+ Partials     3585     3584    -1     
Flag Coverage Δ
unittests 54.47% <70.58%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
utils/unittest/protected_map.go 0.00% <0.00%> (ø)
network/p2p/scoring/score_option.go 79.87% <92.30%> (+1.10%) ⬆️

... and 11 files with indirect coverage changes

Base automatically changed from yahya/6459-ihave-broken-promises to master July 18, 2023 21:17
@yhassanzadeh13 yhassanzadeh13 marked this pull request as ready for review July 18, 2023 21:24
@yhassanzadeh13 yhassanzadeh13 requested a review from gomisha as a code owner July 18, 2023 21:24
@yhassanzadeh13 yhassanzadeh13 requested a review from kc1116 July 18, 2023 21:24
Copy link
Contributor

@kc1116 kc1116 left a comment

Choose a reason for hiding this comment

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

🚀

Copy link
Contributor

@peterargue peterargue 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. added one comment about parallelizing the tests since they seem to have long waits. long term we should considering adding some functionality to mock the time interface to make these tests more flexible and less likely to be flaky due to timing.

// this threshold to 10 meaning that the first 10 broken promises are ignored. This is to allow for some network churn.
// Also, per hearbeat (i.e., decay interval), the spammer is allowed to send at most 5000 ihave messages (gossip sub parameter) on aggregate, and
// excess messages are dropped (without being counted as broken promises).
func TestGossipSubIHaveBrokenPromises_Below_Threshold(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can this be run with t.Parallel()? same question for all the other tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yhassanzadeh13
Copy link
Contributor Author

bors merge

bors bot added a commit that referenced this pull request Jul 19, 2023
4566: [Networking] Handling iHave broken promises (and part-2 of overpromising) spams r=yhassanzadeh13 a=yhassanzadeh13

This PR introduces changes to our GossipSub parameter configurations to effectively address and mitigate iHave Broken Promise attacks. An iHave Broken Promise attack is characterized by a peer advertising an iHave (indicating they have a certain message), but failing to respond to the iWant requests for that message.

We have fine-tuned the GossipSub parameters, notably `defaultBehaviourPenaltyThreshold`, `defaultBehaviourPenaltyWeight`, and `defaultBehaviourPenaltyDecay` to penalize such behavior more appropriately.

The `defaultBehaviourPenaltyThreshold` is set to `10`, meaning we at most tolerate `10` RPCs containing iHave Broken Promises. The `defaultBehaviourPenaltyWeight` applies a penalty when a peer's misbehavior exceeds the threshold. The penalty is applied to the square of the difference between the misbehavior counter and the threshold, to provide a substantial disincentive for malicious peers. If a peer misbehaves more than `sqrt(2) * 10` times the threshold, they will be graylisted or disconnected.

The `defaultBehaviourPenaltyDecay` is set to `0.9`, causing the misbehavior counter to decay by `10%` per decay interval. This means that peers have a chance to recover from their previous misbehavior and incentivizes improved behavior in the future.

Co-authored-by: Yahya Hassanzadeh <yhassanzadeh13@ku.edu.tr>
Co-authored-by: Yahya Hassanzadeh, Ph.D <yhassanzadeh@ieee.org>
@bors
Copy link
Contributor

bors bot commented Jul 19, 2023

Canceled.

@yhassanzadeh13 yhassanzadeh13 merged commit a2e8fb1 into master Jul 19, 2023
@yhassanzadeh13 yhassanzadeh13 deleted the yahya/6459-ihave-broken-promises-part-2 branch July 19, 2023 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants