pshaughn / okayworld

backend for play.okayideas.com
0 stars 1 forks source link

rate limiting #13

Open pshaughn opened 6 years ago

pshaughn commented 6 years ago

API calls close the connection when they fire. f events are rate limited by the minFrameNumber/horizon system. repeating a login event closes the connection. the only currently implemented thing a malicious or malfunctioning client might do an arbitrary number of, other than open new connections entirely, is o events.

I can't think of a one-size-fits-all way to rate limit o events; playsets would need to have some playset-specific information about what it makes sense for commands to do. Moderately excess events could just be left unacked and not sent to other clients, but it would make sense for really grossly excessive events to kick the client.

Moving forward, chat also needs rate limiting; this doesn't have to be playset-specific, and likewise it could have an unacked base case and a kicking more extreme case.

The client should be aware of limits and not send commands or chat that it expects to be unacked. Playset limits would be in playsets.js; chat limits, as a potentially configurable server thing, would need to be sent in the login response.

pshaughn commented 6 years ago

Chat rate limits are commented on in the chat issue. For commands:

Typically but not necessary always, commands refer to the triggering of game actions with preconditions or cooldowns, and issuing more than one command for the same action in the same frame would be pointless. This seems like so much the common case that the exceptional case could require playsets to actively work around it.

If the UI tries to issue two events for the same action on the same frame, the client logic doesn't bother putting the later one in the local game state or sending it to the server; if the server does see two events with the same action on the same frame, it knows the client misbehaved and can kick.

Playsets need to enable client and server code to answer the question "is it okay for these two events to coexist in a frame". The answer should be no for events that are syntactically irrelevant to the playset, to avoid letting a malicious client spam an unlimited number of different irrelevant events (it would be sort of okay if it just let one per frame in). The answer should be no for events that have the same "verb" and different "objects", e.g. firing the same weapon in different directions. The answer should be yes for syntactically relevant events that have different "verbs".

It seems like this verb/object distinction is worth baking into the data structures: commands get a command string .o and separately an argument string .a. A playset statically reports the set of command strings it accepts and how many copies per frame it's okay with, like it reports its own playset name. A playset is then guaranteed not to see a command string outside that set, and not to see more than the limit for the same controller on the same frame. Nothing stops the limit from being Infinity (it's not getting JSON-serialized), but of course that loses rate-limiting behavior against a malicious client. The playset is still responsible for handling arbitrary bad values of .a, like it is for .i.

A playset should be able to report other limits, too: the maximum length of an input string and the maximum length of a command parameter string. With those in place, the total length of what the server broadcasts from a client on a given frame is bounded in a way that doesn't depend on the overall maximum client->server message length.

pshaughn commented 6 years ago

rate limits are coded, not thoroughly tested

pshaughn commented 6 years ago

big changes in current rewrite; commands will just be gone, and input validation will be up to the playset (and not necessarily string-typed)