Gorouter: Websockets over HTTP/2 - invalid pseudo-header ":protocol"
thomas-kaltenbach opened this issue · 7 comments
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-wrappernode <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 golang/go#32763
I don't know if gorouter can workaround the problem or at least return a proper http response.
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 haproxy/haproxy@befeae8, though I'm not sure how it impacts HAProxy's backend connections.
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.
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
@thomas-kaltenbach Given the changes introduced in cloudfoundry/haproxy-boshrelease#263, is there any remaining work for this issue?
Hi @Gerg, note that Thomas moved within our Org, taking over.
In cloudfoundry/haproxy-boshrelease#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 golang/go#32763.
Hi @plowin ,
Checking in on this issue. Is there any further work at this point or is it safe to close this out?
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 golang/go#32763