libks
libks copied to clipboard
signalwire
Fixes protocol handling. Smallest frame size is 6 for ping frames.
If server sends ping frames which are just 6 bytes current implementation aborts and closes connection since it expects the frame to be at least 9 bytes.
can you add a unit test that confirms your assertion that we will fail on a 6 byte packet and that your patch corrects that
Okay, this is a more convoluted issue than simply reducing the size. This might be acceptable given a certain situation, but it goes much deeper. First, we need to reference the RFC, which is RFC6455 here https://tools.ietf.org/html/rfc6455 Section 5.2 talks about base framing after the websocket upgrade.
The thing to note here is that technically a message could be as small as 2 bytes, with no additional data. The minimum is 1 byte for some flags, the opcode, and 1 byte for a mask bit plus 7 bits of length data for the smallest frames. The masking key is potentially optional, and it's possible that there would be zero bytes of data.
The specification on PING and PONG do not indicate that any data must be included. Now, according to the spec all frames sent from client to server must include the mask bit, which means they must also include the mask key. That's another 4 bytes. But this is only those sent client to server, anything going server->client can, according to spec, choose to not include the mask key.
Therefore, by spec, server->client minimum size is 2 bytes, and client->server minimum size is 6 bytes. Since this is reading a frame, as a client, this is server->client and thus the minimum is potentially 2 bytes.
To that end, I am not convinced this patch is correct according to RFC, and if we're going to change the behavior we best be to the actual specification and not just arbitrarily change things.
I think this is an appropriate place to report this, instead of creating an issue.
Starting in Chrome98 Beta, our FS 1.10.7 on Debian 10 with openssl 1.1.1d began regularly prompting verto to relogin multiple times. The FS log shows "BAD READ -1" for these. Chrome DevTools shows many non-control frames of length 1028 with content #SPB. But once verto sends a non-control length 4 message with content #SPE, this is what seems to result in the BAD READ. The length of 4 fits with the analysis above about allowing smaller frames.
We're using ws.c from your release branch for the iOS 15 problem, too.
