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

switch per per-IP to per-UUID rate limiting #146

Closed
wants to merge 1 commit into from

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Aug 8, 2020

I rolled out per-IP rate limiting in our network, but it turns out that even rates as low as 20 connections per 10 minutes are problematic. I am not sure what exactly these misbehaving clients are doing; probably the actual issue is not the load caused by these connection attempts but by other things they do.

Doing the limiting per-UUID lets me reduce the threshhold to something like "max 5 connections per 2 minutes". I would not want to do that per-IP, because people might run 5 nodes behind the same IP and then that limit is reached too easily.

Cc @RobWei does per-UUID rate limiting work for you, or do you require a per-IP limit?

Fixes #144

@mitar
Copy link
Member

mitar commented Aug 8, 2020

What about some way to collects some stats per UUID? To get to determine what are those clients doing? I worry that if they are malicious clients doing something on purpose, they would just rotate UUIDs.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 9, 2020

Sure they would. If they are malicious they could do tons of things, I don't think tunneldigger is very well prepared to deal with that. We currently have DoS problems through likely non-malicious misbehaving clients (#143) which this PR helps handle; that says to me that we are quit far away from being resistant against malicious clients.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 9, 2020

With #148 we do not have the load problems any more. Absent that, I agree that per-IP rate limiting makes most sense, so let's close this.

@RalfJung RalfJung closed this Aug 9, 2020
@RalfJung RalfJung deleted the ratelimit branch August 9, 2020 14:56
@mitar
Copy link
Member

mitar commented Aug 10, 2020

We currently have DoS problems through likely non-malicious misbehaving clients

I hope you are right that it is just non-malicious.

that says to me that we are quit far away from being resistant against malicious clients.

Yea, that was not even original intent to make it so. For that we always assumed that security is done on another level (IPSec).

we do not have the load problems any more

I am glad to hear that.

@RalfJung
Copy link
Member Author

For that we always assumed that security is done on another level (IPSec).

Well, that's not really possible for Freifunk because we run open infrastructure...
(Also I don't think the docs ever said that, so when switching to tunneldigger there was no way we could have been aware of this. Now it's a bit too late with several big Freifunk networks relying on tunneldigger already.^^)

@mitar
Copy link
Member

mitar commented Aug 10, 2020

It didn't say because it was not the goal of the project. Same as you wrote, it is for open infrastructure. Not to mention that IPSec would add some overhead. So yea, in theory you should make that using IPSec, but given our networks design, we probably do not want that, but that opens a window to malicious attacks. I do not think there is much one can do about that, it is a conscious design choice.

I mean, same holds for routing protocols we all use. :-)

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.

Change rate limiting to be per-UUID
2 participants