Consider restrictions on bodies in Upgrade requests

[bemasc]

Formally, Upgrade can be attached to any HTTP request, in which case the request is processed in HTTP/1.1, and the server has the option to Upgrade before returning the response. This includes requests with and without nonempty bodies. Handling of Upgrade with a nonempty body is well-defined, but the Upgrade paths that are actually implemented today (WebSockets and MASQUE) only use GET requests with empty bodies.

@gstrauss raised concerns that this is something of a "sharp edge" for security. For example, a server implementation might assume that there is no body in an Upgrade request, leading to a mismatch between the client and server when a body is present.

We could make this problem go away by mandating that Upgrade only be used with "Content-Length: 0", and perhaps only with "GET". However, this requirement would appear to contradict RFC 2817, which says "A client MAY offer to switch to secured operation during any clear HTTP request", so that document would have to be Updated or marked Historic.

[LPardue]

FWIW HTTP/2 upgrade has some statements that might conflict with the proposal. From https://datatracker.ietf.org/doc/html/rfc7540#section-3.2

Requests that contain a payload body MUST be sent in their entirety before the client can send HTTP/2 frames. This means that a large request can block the use of the connection until it is completely sent.

If concurrency of an initial request with subsequent requests is important, an OPTIONS request can be used to perform the upgrade to HTTP/2, at the cost of an additional round trip.

On the other hand, RFC 9113 deprecates the h2 upgrade, so the topic of this issue is compatible with that notion with a bit of wordsmithing.

[gstrauss]

Handling of Upgrade with a nonempty body is well-defined, but the Upgrade paths that are actually implemented today (WebSockets and MASQUE) only use GET requests with empty bodies.

@gstrauss raised concerns that this is something of a "sharp edge" for security.

Well phrased.

For example, a server implementation might assume that there is no body in an Upgrade request, leading to a mismatch between the client and server when a body is present.

More concretely, for servers which read (and buffer) the full request -- including request header and request body -- before processing the request, this is a non-issue. However, many servers make routing and other decisions after the request headers are received. To handle upgrade after the request has been handled, all requests would need to check for Upgrade at the end of handling requests, not at the beginning, which is more common in code that I have seen and in code that I have written.

I think it would be ideal if Upgrade requests for new Upgrade tokens were sent to endpoints for which an alternate resource is not provided if the Upgrade does not succeed. If a request with Upgrade is rejected, the HTTP status could reflect that. At the moment, a client can request "GET /altresource.mp4 HTTP/1.1" with "Upgrade: websocket" and the RFCs specify that /altresource.mp4 should be delivered in its entirety before the server switches to WebSockets, and the HTTP response headers included Upgrade: websocket, along with Content-Length or Transfer-Encoding: chunked for /altresource.mp4. The combination of request/response bodies for resources tangential to the Upgrade request is what I would like to see avoided in future Upgrade tokens. As @bemasc worded so well, I think there are sharp security edges here, and complications in implementations. HTTP/2 extended CONNECT is a better approach as it is only the :protocol change request, and does not include a request body or a request for an alternate resource, at least as I understand RFC 8441 to describe HTTP/2 extended CONNECT for :protocol: websocket.

Another example of where request bodies can potentially be mishandled is if an origin server passes the Upgrade request to a backend via CGI, FastCGI, SCGI, reverse proxy, or something else. Even if the origin server handles the request body properly with the Upgrade request, I certainly do not trust an amateur PHP program to handle the request body properly before switching protocols. (Yes, I am sure that somewhere, someone handles this properly, but I am willing to wager there are also places where an empty request body is incorrectly assumed by naive scripts.)

To be proactive, lighttpd 1.4.74 and later will by default ignore Upgrade in all requests with a request body unless lighttpd 1.4.74 or later is explicitly configured to enable request body along with Upgrade via lighttpd.conf server.feature-flags += ("gw.upgrade-with-request-body" => "enable") (gw == gateway in lighttpd, for CGI, reverse proxy, and other backends)

tl;dr: I am not asking to restrict HTTP/1.1 Upgrade only to the GET method. I would like to request that optimistic-upgrade add guidance suggesting that future Upgrade tokens should suggest/recommend use in requests without a request body, and note that some proxies or origin servers may ignore HTTP/1.1 Upgrade when a non-empty request body is present.

Thank you for your consideration.

[gstrauss]

Since we are discussing this publicly, I hope these details are appropriate.

lighttpd 1.4.56 and later supports the HTTP/1.1 Upgrade: h2c (now deprecated by the RFCs), but lighttpd ignores Upgrade: h2c unconditionally if a request body is present. Among the reasons for doing so -- in addition to simpler implementation -- is that if an attacker controlled the request body content and wrote it in such a way to start with HTTP/2 Connection Preface and an HTTP/2 request, and if the server failed to handle the HTTP/1.1 request body before switching to HTTP/2 protocol, then an attacker could inject HTTP/2 requests via that mishandled (on the server) HTTP/1.1 request body.

[kazuho]

As I understand, this problem is specific to HTTP/1.1. I'm not sure if we could add new requirements, unless we change the core HTTP documents and can modify all existing HTTP clients that use upgrade to adopt the new behavior.

At the same time, IMO it would be perfectly reasonable to point out the "sharp edges" and provide guidelines to new extensions that would leverage the upgrade mechanism.

In that respect, I like the idea of recommending use of GET but not any other HTTP method for new extensions, as suggested in the original post from @bemasc.

[gstrauss]

@kazuho

As I understand, this problem is specific to HTTP/1.1.

Yes, the Upgrade request header is specific to HTTP/1.1.

In that respect, I like the idea of recommending use of GET but not any other HTTP method for new extensions

I can imagine a future Upgrade token which requires POST with some sort of (short) request body containing authentication and/or payment information. Even so, I still like the idea of recommending that any new HTTP/1.1 Upgrade tokens prefer to use GET without a request body, or to use other methods if a request body is required.

---

Aside: @kenballus you might be interested in adding some Upgrade tests to https://github.com/narfindustries/http-garden, specifically for request smuggling where servers support Upgrade: h2c and might not properly handle an unexpected request body. (lighttpd always ignores Upgrade: h2c if request body is non-zero length.)