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

Ratelimits and security concerns for public servers #81

Closed
ChillerDragon opened this issue Sep 16, 2024 · 11 comments
Closed

Ratelimits and security concerns for public servers #81

ChillerDragon opened this issue Sep 16, 2024 · 11 comments

Comments

@ChillerDragon
Copy link
Contributor

Is https://tw.thissma.fr/ a unedited version of the main branch? Is it save for me to also host a public instance? Have there been any trolling incidents? Are there ratelimits on anything? Can someone just fill up the hard drive with maps?

I quickly did a test deploy to https://editor.zillyhuhn.com/ which was super smooth BTW. And was wondering if i can just leave it there and forget about it :D

@k2d222
Copy link
Owner

k2d222 commented Sep 16, 2024

yeah there is no protection whatsoever, you can fuck up a server probably quite easily

these protective features are in the roadmap but got never implemented. tw.thissma.fr is deployed in a docker with a size-limited storage.

Glad to hear you could deploy easily!!

@ChillerDragon
Copy link
Contributor Author

Okay thanks for the fast response. Could also leave an issue open for ratelimits but I think it would be cleaner to recreate one if needed. I don't want to mess with your todo management. I guess I will take down at least my backend until I figure out a good way to sandbox it or until you implemented some ratelimits :P

@k2d222 k2d222 reopened this Sep 16, 2024
@k2d222
Copy link
Owner

k2d222 commented Sep 16, 2024

keeping open :)

@ChillerDragon
Copy link
Contributor Author

ChillerDragon commented Sep 16, 2024

Okay if it stays open here are some things I would like to see:

  • maximum number of total maps
  • maximum file size per map

And then at some point it would also be nice to have per user limits if there is ip tracking or accounts. But global limits should be there from the start with sane defaults.

@k2d222
Copy link
Owner

k2d222 commented Oct 19, 2024

Ok I implemented a few protections

  • maximum number of total maps, configurable
  • maximum file size per map, configurable
  • maximum number of simultaneous websocket connections, configurable
  • HTTP request rate limiting per ip, not configurable

I don't have any rate limiting for websocket messages. Idk how to do that.

@ChillerDragon
Copy link
Contributor Author

Amazing. Sounds good enough to me. Lets close the issue?

@ChillerDragon
Copy link
Contributor Author

Wait what was your motivation for http ratelimits? What problem does it solve? The limits in the readme sound a bit strict.

It allows bursts of 8 requests and then 500ms between requests.

So if a lan party of 10 people wants to collaborate together they run into rate limits? Or if my browser requests more than 8 resources in the background it does so as well (like style.css, favicon.ico, index.html, mapthumbnail.png)? What happens on http ratelimit? Does it retry? Delay? Will the errors be recoverable for a regular user?

Imo traffic rate limiting is dangerous. It could create annoyances for users. A legit power user should never run into traffic rate limits and if he does it should be handled cleanly with some error popup in the ui.

@k2d222
Copy link
Owner

k2d222 commented Oct 20, 2024

Ah, I agree that the ratelimit is too strict. Im using the default settings of tower_governor.

Wait what was your motivation for http ratelimits? What problem does it solve?

the http server does not only serve the static files, it can also query and modify maps with many GET, POST, PUT and DELETE routes. It complements the websocket server. The routes correspond roughly to the mapdir format. For example,

So if a lan party of 10 people wants to collaborate together they run into rate limits?

No, the rate limit is per ip and the default max connexions is 100.

if my browser requests more than 8 resources in the background it does so as well (like style.css, favicon.ico, index.html, mapthumbnail.png)? What happens on http ratelimit? Does it retry? Delay? Will the errors be recoverable for a regular user?

good points, I'll raise the default burst limit to 100 and make it configurable. I'll make sure that an error popup is shown in the UI. it may be already the case. IIRC when a request fails twwe rollback and shows the error msg.

@ChillerDragon
Copy link
Contributor Author

No, the rate limit is per ip and the default max connexions is 100.

Per ip does not always mean per person. That's why I said lan party. Or is it per ip and port combination?

Also why do you want to rate limit http and websockets? What problem does it solve.

@k2d222
Copy link
Owner

k2d222 commented Oct 20, 2024

That's why I said lan party

oops, misread

is it per ip and port combination?

just ip, you're perfectly right the LAN session would be an issue.

why do you want to rate limit http and websockets? What problem does it solve.

The problem it tries to solve is DOS attacks. Requests to maps content need to be synchronized on the server, so the server is full of mutexes everywhere. A user could freeze a map by simply spamming a request and keeping the mutex for themselves. For example if you spam https://server.com/maps then other users will not be able to list, create or delete maps. It's not overloading the server, it's just that it's a first come first serve situation.

A smarter fix is to give connections a 'niceness' score that decreases the more they spam the server and move them down the queue. But this would be significantly harder to implement.

I relaxed the limits and made them configurable, so feel free to disable rate-limits.

@ChillerDragon
Copy link
Contributor Author

Oh I see. Mutexs yes that sounds important to protect. A simple shell script with a while loop and curl should not take down a map.

@k2d222 k2d222 closed this as completed Oct 21, 2024
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

No branches or pull requests

2 participants