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

Immediately inserting a Connection into a HashMap opens the door for DoS attacks. #187

Closed
bmisiak opened this issue Jun 4, 2019 · 2 comments

Comments

@bmisiak
Copy link

bmisiak commented Jun 4, 2019

Hello.
Please correct me if I am missing something, but I want to bring your attention to this issue in case it hasn't been accounted for yet.

As UDP packets can easily be spoofed to be seemingly coming from tons of random sources, every such malicious packet would contribute to filling up the ActiveConnections HashMap and could eventually lead to a DoS. This has been exploited in other UDP game servers.

The server-side solution, typically, is a UDP equivalent of SYN cookies: A packet is not acknowledged unless it contains a cookie, which the client requests from the server upon connection. The cookie is generated statelessly based on the SocketAddr and a secret seed, so that no allocations are necessary.

Unfortunately, the laminar API currently does not allow for this mechanism. Maintaining a list of connections this early into the packet parsing process also seems like a source of contention for servers which might want to use SO_REUSEPORT to accept packets on multiple threads.

@jstnlef
Copy link
Contributor

jstnlef commented Jun 5, 2019

You make a good point. We definitely need to consider options for fixing this attack vector. There is another ticket that has already floated the idea of a handshake which could be a possible way to solve that problem.
However, I'm less worried about the SO_REUSEPORT case because laminar is not designed to work in a multi-threaded capacity at this moment. Also, realistically speaking, even if we rework it to be multi-thread friendly, SO_REUSEPORT was implemented as a way to handle load at google scale. https://lwn.net/Articles/542629/. I suspect it's a bit of an unnecessary optimization.

@kstrafe
Copy link
Contributor

kstrafe commented Jun 17, 2019

This can now be closed as of merging #194

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

3 participants