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

Easy DoS/DDoS with WebSocket Transport #3518

Open
maxeth opened this issue Feb 9, 2025 · 0 comments
Open

Easy DoS/DDoS with WebSocket Transport #3518

maxeth opened this issue Feb 9, 2025 · 0 comments
Labels
help wanted Extra attention is needed websocket

Comments

@maxeth
Copy link

maxeth commented Feb 9, 2025

The current implementation of the WebSocket transport doesn't provide enough options to protect against DoS/DDoS by users sending large payloads over multiple WebSocket connections, which will then be processed in parallel.

The payload size limit (imposed by gorilla/websocket?) seems to be ~100 MB, and with less than 100 connections spamming such large single-frame payloads in parallel (with EnableCompression: true it's just a few kB sent over the network, and you can compose valid payloads that won't cause a disconnect, e.g. a ~100 MB long, easily compressible operationName), one may start noticing performance degradation. With 100-200+ connections I made one of my servers unresponsive.

Now, with InitFunc in combination with OnCloseFunc, we could already count concurrent connections per IP or some other public client identifier, and reject further upgrades in CheckOrigin if too many are opened to partially mitigate this.

However, unless I'm missing something, the InitFunc doesn't let us access the connection to set a safe, maximum read limit, like:

InitFunc: func(ctx context.Context, initPayload transport.InitPayload, conn *wsConnection) (context.Context, *transport.InitPayload, error) {
   conn.SetReadLimit(2500) // https://pkg.go.dev/github.com/gorilla/[email protected]#Conn.SetReadLimit
   // but conn isn't passed, also not in context
}

This would make such DoS/DDoS attacks unfeasible, since Gorilla would safely abort reading large payloads if a single frame or multiple frames would accumulatively exceed the limit, close the connection, which would then force the user to open a new one, where they'd be faced with our custom connection upgrade rate limiter.


If something speaks against passing the whole connection, I think it would be good to set a configurable read limit with some default that is much less than 100 MB - 1 MB perhaps - in the websocket init func, given that GraphQL payloads sent by clients via the WebSocket transport should always be < 1 MB.

type Websocket struct {
    PayloadReadLimit *int

    // [...]
}

func (c *wsConnection) init() bool {
    limit := someDefault
    if c.PayloadReadLimit != nil {
        limit = *c.PayloadReadLimit
    }
    c.conn.SetReadLimit(limit)

    // [...]
}

Obviously one could create a custom Transport, but this seems like a basic thing that could be added with 1 line of code without any downside.

Any thoughts?

@StevenACoffman StevenACoffman added websocket help wanted Extra attention is needed labels Feb 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed websocket
Projects
None yet
Development

No branches or pull requests

2 participants