condure
condure copied to clipboard
Incompatibility with Envoy Proxy when downstream client uses HTTP/2
When the downstream client is connected over HTTP/2, Envoy sends upstream requests to Condure that are not fully compliant with RFC 6455. Specifically, Envoy sends Content-Length: 0
and does not send a Sec-WebSocket-Key
, which causes Condure to (correctly) deny the request.
Clearly this is a problem with Envoy and not Condure, but I'm posting an issue here to document the incompatibility and provide a workaround for anyone who needs it. Here's a small patch that has solved the problem for me:
diff --git a/src/connection.rs b/src/connection.rs
index b98f513..c601537 100644
--- a/src/connection.rs
+++ b/src/connection.rs
@@ -2636,7 +2636,7 @@ where
let req = handler.request(&mut scratch);
let mut websocket = false;
- let mut ws_key = None;
+ let mut ws_key: Option<&[u8]> = Some(&[]);
for h in req.headers.iter() {
if h.name.eq_ignore_ascii_case("Upgrade") {
@@ -2674,7 +2674,7 @@ where
);
let ws_accept: Option<ArrayString<[u8; WS_ACCEPT_MAX]>> = if websocket {
- if req.method != "GET" || req.body_size != http1::BodySize::NoBody || ws_key.is_none() {
+ if req.method != "GET" {
return Err(ServerError::InvalidWebSocketRequest);
}
As there's no plan to address this in Envoy, but it is popular and otherwise compatible with Condure, it would be nice to have a setting to enable this relaxed behavior in Condure. I don't think this would necessarily cause a security regression because Sec-WebSocket-Key
is only used to prevent unintended requests, and that problem can be addressed by enabling CORS in Envoy.
Further background:
- https://github.com/envoyproxy/envoy/issues/15464#issuecomment-799495181
- https://github.com/envoyproxy/envoy/issues/8547
For any brave souls who need to use the above patch, here's a Dockerfile that builds a custom Pushpin image from Condure source. On an arm64 system such as Apple M1, it will cross-compile Condure for amd64.
Interesting, I didn't realize anyone was doing WebSockets over HTTP/2 yet. Will digest this. Thank you.
BTW, the check has now been relaxed to allow Content-Length: 0
, for compatibility with Java, which partly helps with this issue too. The Sec-WebSocket-Key
header is still required though.