p2p: rate-limit incoming connections by IP#6286
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6286 +/- ##
==========================================
- Coverage 60.90% 60.84% -0.06%
==========================================
Files 281 282 +1
Lines 26660 26707 +47
==========================================
+ Hits 16237 16251 +14
- Misses 8738 8771 +33
Partials 1685 1685
|
alexanderbez
left a comment
There was a problem hiding this comment.
Great work @tychoish 👍
|
|
||
| switch { | ||
| case o.IncomingConnectionWindow == 0: | ||
| o.IncomingConnectionWindow = 100 * time.Millisecond |
There was a problem hiding this comment.
Is 100ms kind of arbitrary or do you have some gut judgement here? (totally cool with either answer :-p)
There was a problem hiding this comment.
some of both? Mostly arbitrary, but also I can't imagine connection attempts more frequent than 100ms being a viable use case?
| RemoveConn(net.IP) | ||
| } | ||
|
|
||
| type connTrackerImpl struct { |
There was a problem hiding this comment.
It seems like this implementation should do the trick. Any reason you opted for a in-house implementation instead of a ready-to-use rate limier, e.g. https://github.com/sethvargo/go-limiter?
The above already garbage collects and maps directly to our API & needs. We can certainly keep this implementation, but I was just curious. Generally in favor of having to maintain less code :)
There was a problem hiding this comment.
This makes sense. I think it's pretty simple, and I don't see places where we're using go-limiter otherwise (and it looks like we'd need some kind of wrapper code/etc. anyway, so I'm not sure if it's a huge burden.) I put the rate-limiting behind an interface so we could change it later if we needed.
implements simple incoming peer connection rate limiting both by time
window to prevent connection storms, and by count to prevent multiple
connections from the same peer.
This happens at the router level which means the transport layer could
still present a vector for some kind of disruption.