Re: POST+Upgrade, or unexpected limitation of RFC8441

[Ben Schwartz <[e-mail]>]

It seems like the underlying confusion here comes from two false (but understandable!) assumptions:

1. The ":protocol" pseudoheader's value is an Upgrade Token, so it can contain any Upgrade Token.
2. It is possible to convert HTTP/1.1 Upgrade to Extended CONNECT generically.

In fact, ":protocol" only supports a subset of the registered Upgrade Tokens (i.e. those that explicitly describe their use with Extended CONNECT), and even if an Upgrade Token is defined for use in both HTTP/1.1 Upgrade and Extended CONNECT, there is no generic process for converting between these forms without detailed knowledge of the specific Upgrade Token.

I wish it were otherwise.  And in fact, I think we could fix this for a subset of Upgrade Tokens (those that use the Capsule Protocol): https://datatracker.ietf.org/doc/html/draft-kb-capsule-conversion-00

That's not going to help you with whatever Docker is doing though.  Someone should tell them that "tcp" is not a registered Upgrade Token [1] and "h2c" is deprecated.

As for haproxy, it needs to restrict its HTTP Version Conversion logic to recognized Upgrade Tokens where the conversion logic is known to be correct.

--Ben Schwartz

[1] https://docs.docker.com/reference/api/engine/version/v1.47/#tag/Container/operation/ContainerAttach
________________________________
From: Willy Tarreau <[e-mail]>
Sent: Wednesday, December 18, 2024 8:35 AM
To: Glenn Strauss <[e-mail]>
Cc: HTTP Working Group <[e-mail]>; [e-mail] <[e-mail]>
Subject: Re: POST+Upgrade, or unexpected limitation of RFC8441

Hi Glenn,

On Wed, Dec 18, 2024 at 06:57:14AM -0500, Glenn Strauss wrote:
> HTTP/1.1 Upgrade overloads the request.
>
> There is an HTTP 1.1 request and there is an HTTP/1.1 Upgrade request.
>
> If the Upgrade is not handled, the HTTP request proceeds.
>
> If the Upgrade is to be handled, then the request be must read in full
> prior to the Upgrade taking effect on data *after the full request*
> received from the client.  There is a danger here for exposure to
> request smuggling if various intermediaries handle or do not handle the
> request body differently.

I pretty much agree with all of this, and that's clearly the problem
here of switching to CONNECT which doesn't offer option for marking a
boundary on a request payload.

> Some of these issues were discussed earlier this year regarding
> Security Considerations for Optimistic Use of HTTP Upgrade
> https://urldefense.com/v3/__https://datatracker.ietf.org/doc/draft-schwartz-httpbis-optimistic-upgrade/__;!!Bt8RZUm9aw!74ic5DESaXPLq1JkO-MTfn1BE3voxl1MrOzJT0bZxUt6hv0gSOoT-EbqHAsvFw3tXir3$
> both on list and at the IETF meetings, e.g.
> https://urldefense.com/v3/__https://httpwg.org/wg-materials/ietf120/minutes.html*security-considerations-for-optimistic-use-of-http-upgrade----ben-schwartz__;Iw!!Bt8RZUm9aw!74ic5DESaXPLq1JkO-MTfn1BE3voxl1MrOzJT0bZxUt6hv0gSOoT-EbqHAsvFwNdML1v$
>
> (I think on the list) I made a suggestion that the draft limit future
> Upgrade tokens for use in requests *without* a request body and with
> idempotent HTTP request methods.
>
> I still feel that way and so for these reasons would oppose adding
> complexity to extend RFC8441 CONNECT to allow a request body.

I'm not strongly against that, it's just that we may be ossifying some
applications what will require H1 exclusively and forever. But hey, we
have set-cookie as well...

> Why does the docker engine API use POST along with Upgrade?

I have no idea. But I can understand how some implementors could
consider that they need to open a bidirectional channel not working
on HTTP boundaries, hence an upgrade, and that the initial context to
start with is passed as a message body. We could for example imagine
that some applications repost the last dumped state of the previous
session, which the gateway searches among all known servers, to route
the final tunnel request to the proper region/server. That's certainly
not the case here but there can be valid use cases (which technically
do work).

> HTTP POST method is not intrinsically idempotent.
> Would it not be better if the docker API sent the POST request, and
> separately, depending on success/failure of the POST request, sent
> an HTTP/1.1 Upgrade: websocket request via GET?

Or even the reverse, request first, then use their protocol to send the
data. But anyway I don't know their protocol and am not qualified to
judge their choice. Maybe in their context it's the best one.

> From my quick look in Moby, the open source repo for community Docker:
>
> Search for "Upgrade:" to see the examples
> Upgrade: tcp and Upgrade: h2c
> https://github.com/moby/moby/blob/master/api/swagger.yaml
>
> Since swagger.yaml mentions Upgrade: h2c, I'll note that lighttpd will
> ignore HTTP/1.1 Upgrade: h2c if there is a non-zero HTTP request body
> included with HTTP/1.1 Upgrade: h2c.  That is done to avoid the
> potential for request smuggling with HTTP/1.1 Upgrade: h2c.

Same here. Actually it's not just about smuggling, it's also about
bypassing everything (routing rules, filtering etc).

> At the moment, I do not know why Docker API is using POST instead of GET
> and if the POST even has a body, or if it happens to use
> Transfer-Encoding: chunked even in cases where it could explicitly use
> Content-Length: 0.

I don't know either, I don't even have these tools nor do I know how
to test them. And if we had to try each and every strangely behaving
application, it would require more people than are likely subscribed
to this list!

> >   [...] but the problem remains that the mechanism proposed by
> > RFC8441 doesn't offer provisions for completely transporting H1
> > over H2, nor replacing H1 with H2. [...]
>
> True.  Perhaps HTTP/1.1 RFCs should be updated to deprecate
> non-zero request body and use an idempotent method when including
> an Upgrade token with the request?  Similar to your last suggestion:
>
> >   - or we could do nothing and consider that some parts of HTTP/1 will
> >     remain HTTP/1 forever and will not be transportable over newer
> >     versions. I'm not fundamentally against it but it would warrant
> >     some extra documentation (especially in the H1 related specs) to
> >     discourage such use so that we make sure no new protocol adopts
> >     them and stay stuck in a corner.
>
> BTW, there is already another outstanding issue mentioned on this list
> (but not followed with an errata) about HTTP/2 extended CONNECT breaking
> HTTP Digest authentication for an HTTP/1.x request translated to HTTP/2
> since the HTTP method is included in the digest, but the HTTP/1.1
> request sent over HTTP/2 uses HTTP method CONNECT. [1]  Ben noted that
> the issue may also apply to MASQUE protocols.
>
> [1] https://urldefense.com/v3/__https://lists.w3.org/Archives/Public/ietf-http-wg/2024JulSep/0169.html__;!!Bt8RZUm9aw!74ic5DESaXPLq1JkO-MTfn1BE3voxl1MrOzJT0bZxUt6hv0gSOoT-EbqHAsvF7ZDPB7v$

Hmmm interesting. Actually this class of problems can probably be extended
to any info passed in a the request as the method or a header field that's
normally not expected with CONNECT. After all, section 5 of 8441 says:


   The :protocol pseudo-header field MUST be included in the CONNECT
   request, and it MUST have a value of "websocket" to initiate a
   WebSocket connection on an HTTP/2 stream.  Other HTTP request and
   response-header fields, such as those for manipulating cookies, may
   be included in the HEADERS with the CONNECT method as usual.  This
   request replaces the GET-based request in [RFC6455] and is used to
   process the WebSockets opening handshake.

So there's a way to read it as "any field including content-length...".
And for the original method a specific header could be imagined. All of
this just doesn't have totally obvious implications and may require
implementors to be very careful about (which is already the case in
fact).

Cheers,
Willy