Document distinct read and idle timeouts

[propertyscott]

How do I protect against a client that stops sending data, either between messages or within a message?

With Gorilla, I can use a combination of read deadline and ping/pong to ensure that a client is sending messages. Once reading a message, I can set the read deadline to ensure that the client sends the message in a timely fashion.

With this package, I can specify one timeout using the context argument to Conn.Reader. How do I adjust that value after a successful call to Conn.Ping? How do I adjust that value down after receiving the first byte of message data?

[nhooyr]

To be clear, you want one timeout between messages and then another when actually reading a message right?

Right now, there's no straightforward way to do what you want. My thoughts were that most people would just want a single timeout anyway when reading the next message, e.g. 15 minutes might be reasonable. For most use cases, after 15 minutes of no messages, it's reasonable to disconnect.

It's nice to ensure that the client sends messages in a timely fashion but there is no significant resource difference between waiting for the next message and actually reading it unless the buffer you're reading into is massive which I don't think is common with WebSocket's. You're not really protecting yourself so much as just making each connection last longer which is good but it's better the protocol running on top of WebSockets be able to handle disconnects gracefully.

TCP keep alives should be able to catch connections that are actually disconnected easily. See #1 for discussion.

Thus I figured it'd be a simpler API to just have a single timeout for the entire read operation from waiting for the new message to actually reading it.

Note: This feature is very easy to add, just not exposed right now. If you can convince me it's a good idea to add, I'll be happy to add it. In fact, an early version of this library supported this feature but I removed it because I wasn't sure it was necessary. It'd be as simple as removing the context arg on Reader and Read and adding a SetReadContext method onto the reader. The implementation already supports it.

[nhooyr]

Something you could do now is pass in a cancellable context to Reader without a timeout. See https://golang.org/pkg/context/#WithCancel

In the goroutine sending pings, you could cancel the Reader context explicitly. Furthermore, after Reader succeeds, you could tell the goroutine sending pings that a message has come in and is being read. And then notify it when the message is fully read. Definitely convoluted though.

[propertyscott]

Yes, different timeouts for idle and reading a message.

For most use cases, after 15 minutes of no messages, it's reasonable to disconnect.

Consider Slack or other chat group chat scenarios. A user might be monitoring the activity in a channel or group without sending a message for long periods of time. It is not reasonable to disconnect after 15 minutes in this scenario.

It's nice to ensure that the client sends messages in a timely fashion but there is no significant resource difference between waiting for the next message and actually reading

If the application accepts large messages then an attacker can consume serve memory by opening connections and sending a large, but not complete, message.

See also https://en.wikipedia.org/wiki/Slowloris_(computer_security)

[nhooyr]

It is not reasonable to disconnect after 15 minutes in this scenario.

The client app would automatically reconnect so the user wouldn't notice.

If the application accepts large messages then an attacker can consume serve memory by opening connections and sending a large, but not complete, message.

I can't really think of a use case where you want to have a WebSocket server that accepts large messages. As soon as you want to send a large payload, it's best to use straight HTTP instead. I'd imagine most public WebSocket servers could reasonably limit the payload at 16 KB which isn't that much memory to be locked down for 15 minutes per abusive connection. Its likely any attacker will be IP rate limited anyway so they can't open too many, especially if you use something like cloud flare or google's load balancers in front.

See also https://en.wikipedia.org/wiki/Slowloris_(computer_security)

slowiris doesn't really affect Go servers because they're highly concurrent by default and goroutines are so lightweight. See https://groups.google.com/forum/#!topic/golang-nuts/MFZd6b8zQTQ

The other issue I have with this is I don't see a good way to include it in the API such that its usable with the wsjson and wspb packages. However, good thing is, I just realized I don't need to. I found a nice way for you to do what you want with the existing API.

func readLoop(ctx context.Context, c *websocket.Conn) error {
	// Limit the connection to 5 hours tops.
	ctx, cancel := context.WithTimeout(ctx, time.Hour*5)
	defer cancel()

	go func() {
		// Cancel context if ping loop fails.
		defer cancel()
		pingLoop(ctx, c)
	}()

	for {
		b, err := read(ctx, c)
		if err != nil {
			return err
		}

		// Do what you want with the bytes.
		_ = b
	}
}

func read(ctx context.Context, c *websocket.Conn) ([]byte, error) {
	ctx, cancel := context.WithTimeout(ctx, time.Minute*30)
	defer cancel()

	_, r, err := c.Reader(ctx)
	if err != nil {
		return nil, err
	}

	// One minute max to read the received message.
	time.AfterFunc(time.Minute, cancel)

	return ioutil.ReadAll(r)
}

func pingLoop(ctx context.Context, c *websocket.Conn) {
	t := time.NewTicker(time.Minute * 3)
	defer t.Stop()
	for {
		select {
		case <-ctx.Done():
			return
		case <-t.C:
		}
		err := ping(ctx, c)
		if err != nil {
			// The connection has disconnected if ping errors
			// and everything will automatically tear down.
			return
		}
	}
}

func ping(ctx context.Context, c *websocket.Conn) error {
	ctx, cancel := context.WithTimeout(ctx, time.Second*5)
	defer cancel()

	err := c.Ping(ctx)
	return err
}

Let me know if you need anything else.

[nhooyr]

Ah nvm, sec missed the idle timeout 🤦‍♂

[nhooyr]

Updated the example. It gives each connection max 5 hours, an idle timeout of 30 minutes between messages, 1 minute to read every message and sends WebSocket pings every 3 minutes.

[nhooyr]

That should be everything you need, feel free to reopen the issue if you have any other concerns.

[nhooyr]

Actually, I'll add this to the docs.

[propertyscott]

We at $company use large messages, but I am not allowed to share the details.

The point of referencing Sloworis was about the a general form of attack: starting something that consumes resources, keeping it alive and never finishing it.

The net/http server combats Sloworis through timeout settings, which I was was asking about here. The support for high concurrency does nothing to prevent the consumption of fds in Sloworis.

Thank you for sharing the code. That's enough for me to move onto my next step in evaluating the package.