Connections with bad Sec-WebSocket-Key header, read key from wrong position and send back bad data

[SoylentGraham]

Sec-WebSocket-Key is the correct spec key, but many clients send the wrong key Sec-Websocket-Key.

Ngrok is one example (thus, bayou is incompatible with ngrok agent for WSS/SSL tunneling)

The bayou code does this

static void GetKey(string msg, byte[] keyBuffer)
        {
            int start = msg.IndexOf(KeyHeaderString) + KeyHeaderString.Length;

            Log.Verbose($"Handshake Key: {msg.Substring(start, KeyLength)}");
            Encoding.ASCII.GetBytes(msg, start, KeyLength, keyBuffer, 0);
        }

Due to the lack of error checking; this becomes
int start = -1 + 19

then reads the wrong key, adds the wrong guid and sends back duff data, and gets disconnected by any competent client.

Solutions

• Do proper error checking anyway
• Find headers case insensitively (this is what 99% of clients & servers do, because, frankly, all humans make terrible programmers :)

Will submit a PR

[FirstGearGames]

Did you submit a PR to Bayou or SimpleWebTransport? Bayou depends on SimpleWebTransport, so that might be the best place to push a PR.

[SoylentGraham]

I've got the changes in a private fork.
Do you update from SimpleWebTransport often? (I don't know how out of date your version is...)

[FirstGearGames]

I've got the changes in a private fork. Do you update from SimpleWebTransport often? (I don't know how out of date your version is...)

Most issues within SimpleWebTransport are found first here, so not often we need to update from their git.

[SoylentGraham]

As mentioned in the PR, on our side, I re-wrote the whole handshake/http part to parse the http headers independently, and the code is far more robust, and far cleaner now (instead of just doing string searches) - and also allowed me to read other http headers (eg. "x-forwarded-for" which gives me the true external address of a client when going through a proxy -eg. nginx or ngrok)

I can't share that fork but the gist of it is

public void ParseRequestHeaders()
        {
            RequestHeaders = new Dictionary<string,string>();
            
            //  read headers
            while ( !this.hasDisposed )
            {
                var NextLine = stream.ReadLine();
                if ( RequestFirstLine == null )
                {
                    RequestFirstLine = NextLine;
                    if ( NextLine == null )
                        throw new Exception($"First line of HTTP request is null");
                }
                else
                {
                    //  parse header
                    //  final header is \r\n\r\n (and thus an empty line)
                    if ( NextLine.Length == 0 )
                        return;
                    var KeyValue = NextLine.Split(":",2);
                    //  gr: shouldn't get this, but a line with just a semi colon??
                    if ( KeyValue.Length == 0 )
                        KeyValue = new String[]{NextLine};
                    var Key = KeyValue[0].ToLower();
                    //  trim whitespace around value, the split of "x: y" will result in "Key" " value" 
                    var Value = KeyValue.Length > 1 ? KeyValue[1].TrimStart() : null;
                    RequestHeaders[Key] = Value;
                }
            }
            throw new Exception($"Disposed before header parsing finished");
        }

RequestFirstLine is saved for later for method/GET/POST/http version checks etc

Ran here

void HandshakeAndReceiveLoop(Connection conn)
        {
            try
            {
                if ( !sslHelper.TryCreateStream(conn) )
                    throw new Exception($"Failed to create SSL Stream {conn}");

                conn.ParseRequestHeaders();
                //Debug.Log($"ClientWebsocket headers; {conn.DebugRequestHeaders()}");