Gorouter: Websockets over HTTP/2 - invalid pseudo-header ":protocol"

[thomas-kaltenbach]

Is this a security vulnerability?

no

Issue

Gorouter is not able to handle HTTP/2 Websockets requests (RFC 8441).

Affected Versions

latest version when http2 is enabled

Context

We are testing currently the http/2 feature and we noticed that websockets over http/2 are not working. Gorouter is closing the connection without any http response. After activating golang verbose logging (GODEBUG=http2debug=2) you can see the followed log entries:

2021/09/30 10:23:47 http2: invalid pseudo headers: invalid pseudo-header ":protocol"
2021/09/30 10:23:47 http2: Framer 0xc0003ca0e0: wrote RST_STREAM stream=1 len=4 ErrCode=PROTOCOL_ERROR

Traffic Diagram

           +----+----+            +----------+     +-------+
  \o/      |         |            |          |     |       |
   +  +--->+ HAproxy +--http/2--->+ Gorouter +---->+  App  |
  / \      |         |            |          |     |       |
 client    +---------+            +----------+     +-------+

Steps to Reproduce

I could also reproduce the issue with the followed nodejs client

'use strict';
'use strict';
const WebSocket = require('ws');
const http2 = require('http2-wrapper');

const head = Buffer.from('');

var urlArg = process.argv[2].trim();
console.log("try to connect to url '" + urlArg + "'")

const connect = (url, options) => {
    const ws = new WebSocket(null);
    ws._isServer = false;

    const destroy = async error => {
        ws._readyState = WebSocket.CLOSING;

        await Promise.resolve();
        ws.emit('error', error);
    };

    (async () => {
        try {
            const stream = await http2.globalAgent.request(url, options, {
                ...options,
                ':method': 'CONNECT',
                ':protocol': 'websocket',
                origin: (new URL(url)).origin
            });

            stream.once('error', destroy);

            stream.once('response', _headers => {
                stream.off('error', destroy);

                stream.setNoDelay = () => {};
                ws.setSocket(stream, head, 100 * 1024 * 1024);
            });
        } catch (error) {
            destroy(error);
        }
    })();

    return ws;
};

const ws = connect(urlArg, {
    rejectUnauthorized: false
});

ws.once('open', () => {
    console.log('CONNECTED!');

    ws.send('WebSockets over HTTP/2');
});

ws.once('close', () => {
    console.log('DISCONNECTED!');
});

ws.once('message', message => {
    console.log(message);

    ws.close();
});

ws.once('error', error => {
    console.error(error);
});

Steps to run it:

• save coding as file
• npm install ws http2-wrapper
• node <filename> https://<url_gorouter>

Expected result

One of the following behaviors would be acceptable:

• Gorouter can handle the connection
• downgrade in case of a websocket connection to HTTP/1.1
• http response 400 Bad request

Current result

HAproxy reporting the issue with the termination state SH-- in the accesslog.

nodejs client reports followed error:

Error [ERR_HTTP2_STREAM_ERROR]: Stream closed with error code NGHTTP2_REFUSED_STREAM
    at ClientHttp2Stream._destroy [as __destroy] (internal/http2/core.js:2148:13)
    at ClientHttp2Stream.stream._destroy (/Users/d064325/git/mytools/socket_test/node_modules/http2-wrapper/source/utils/delay-async-destroy.js:12:23)
    at ClientHttp2Stream.destroy (internal/streams/destroy.js:38:8)
    at ClientHttp2Stream.[kMaybeDestroy] (internal/http2/core.js:2164:12)
    at Http2Stream.onStreamClose (internal/http2/core.js:511:26) {
  code: 'ERR_HTTP2_STREAM_ERROR'
}

Possible Fix

Root cause of the problem is the followed check: https://github.com/golang/go/blob/e180e2c27c3c3f06a4df6352386efedc15a1e38c/src/net/http/h2_bundle.go#L2770

func (mh *http2MetaHeadersFrame) checkPseudos() error {
    var isRequest, isResponse bool
    pf := mh.PseudoFields()
    for i, hf := range pf {
        switch hf.Name {
        case ":method", ":path", ":scheme", ":authority":
            isRequest = true
        case ":status":
            isResponse = true
        default:
            return http2pseudoHeaderError(hf.Name)
        }
        // Check for duplicates.
        // This would be a bad algorithm, but N is 4.
        // And this doesn't allocate.
        for _, hf2 := range pf[:i] {
            if hf.Name == hf2.Name {
                return http2duplicatePseudoHeaderError(hf.Name)
            }
        }
    }
    if isRequest && isResponse {
        return http2errMixPseudoHeaderTypes
    }
    return nil
}

I also found followed issue https://github.com/golang/go/issues/32763

I don't know if gorouter can workaround the problem or at least return a proper http response.

[Gerg]

My initial thoughts:

Given the discussion in golang/go#32763, it seems like it will be difficult to get RFC 8441 support for Gorouter any time soon (let me know if I'm missing something that would make it possible 🙂).

Based on that, it seems like the next best option would be to make HAProxy (and other clients) use HTTP/1.X for websockets requests. We already impose some Load Balancer configuration requirements for supporting websockets on CF: https://docs.cloudfoundry.org/adminguide/supporting-websockets.html#config.

Doing a little digging, I found https://github.com/haproxy/haproxy/commit/befeae88e8c7e2cbc19a63420bfa7d67ec176342, though I'm not sure how it impacts HAProxy's backend connections.

[Gerg]

I applied haproxy/haproxy@befeae8 as a patch to the existing HAProxy release and configured h2-workaround-bogus-websocket-clients. It did not seem to have any effect on HAProxy's communication with backends.

[Gerg]

I was able to hack together a HAProxy config that appears to work for websockets:

Relevant excerpts:

# HTTPS Frontend {{{
frontend https-in
    mode http
      bind :443  ssl crt /var/vcap/jobs/haproxy/config/ssl   alpn h2,http/1.1

    http-request del-header X-Forwarded-Client-Cert
    http-request del-header X-SSL-Client
    http-request del-header X-SSL-Client-Session-ID
    http-request del-header X-SSL-Client-Verify
    http-request del-header X-SSL-Client-Subject-DN
    http-request del-header X-SSL-Client-Subject-CN
    http-request del-header X-SSL-Client-Issuer-DN
    http-request del-header X-SSL-Client-NotBefore
    http-request del-header X-SSL-Client-NotAfter

    capture request header Host len 256
    default_backend http-routers
    acl xfp_exists hdr_cnt(X-Forwarded-Proto) gt 0
    acl is_websocket hdr(Upgrade) -i WebSocket          # Add is_websocket ACL looking for upgrade header
    acl is_websocket hdr_beg(Host) -i ws                # is_websocket ACL also looks for ws* scheme
    use_backend http-routers-ws if is_websocket         # If request matches ACL, route to http-routers-ws backend instead
    http-request add-header X-Forwarded-Proto "https" if ! xfp_exists
# }}}

# Default Backend {{{
backend http-routers
    mode http
    balance roundrobin
        errorfile 503 /var/vcap/jobs/haproxy/errorfiles/custom503.http

    server node0 10.244.0.34:443 check inter 1000  ssl verify required ca-file /var/vcap/jobs/haproxy/config/backend-ca-certs.pem alpn h2,http/1.1
# }}}
backend http-routers-ws         # New backend, identical to http-routers, except without alpn
    mode http
    balance roundrobin
        errorfile 503 /var/vcap/jobs/haproxy/errorfiles/custom503.http

    server node0 10.244.0.34:443 check inter 1000  ssl verify required ca-file /var/vcap/jobs/haproxy/config/backend-ca-certs.pem

[Gerg]

@thomas-kaltenbach Given the changes introduced in cloudfoundry/haproxy-boshrelease#263, is there any remaining work for this issue?

[plowin]

Hi @Gerg, note that Thomas moved within our Org, taking over.

In https://github.com/cloudfoundry/haproxy-boshrelease/pull/263 we downgrade all websockets to h/1 even though h/2 is configured. Accordingly, this is still an issue and this workaround should not remain for ever. Seems to depend on https://github.com/golang/go/issues/32763.

[MarcPaquette]

Hi @plowin ,

Checking in on this issue. Is there any further work at this point or is it safe to close this out?

[plowin]

Hi @MarcPaquette , thx for re-assigning. I prefer keeping it open in state blocked. At some point in the far future, all traffic might be H/2 and we need support for it in gorouter. Keeping an eye on https://github.com/golang/go/issues/32763