Subject: Publication has been requested for draft-ietf-httpbis-http2bis-05

From: Mark Nottingham via Datatracker <[email-masked]>
Date: Mon, 27 Sep 2021 16:00:15 -0700

Mark Nottingham has requested publication of draft-ietf-httpbis-http2bis-05 as Proposed Standard on behalf of the HTTPBIS working group.

Please verify the document's state at https://datatracker.ietf.org/doc/draft-ietf-httpbis-http2bis/

---

From: Willy Tarreau <[email-masked]>
Date: Tue, 28 Sep 2021 10:56:43 +0200

Hi Mark, Martin, Cory

> Please verify the document's state at https://datatracker.ietf.org/doc/draft-ietf-httpbis-http2bis/

Sadly, it seems we've missed that part that we were discussing with
Martin earlier this month:

   https://github.com/httpwg/http2-spec/pull/936#issuecomment-910260086

I'd have liked that we suggest to be extremely careful about checking
dangerous characters in some pseudo headers, which can be abused when
concatenated to reconstruct a URI, and for which there are no indications
in Semantics since they do not really exist outside of H2. Something like
this could have done the job:

  In addition, implementations that operate on a URI or request line
  reconstructed from the concatenation of :method, :scheme, :authority,
  and :path SHOULD validate each of these fields individually and
  according to the rules in RFC3986#3 and MUST at least perform this
  minimal validation:
    - none of the aforementioned pseudo-header field values contains any
      character among NUL/CR/LF/LWS
    - :scheme does not contain the COLON character
    - :path exclusively starts with "/" or "*"

  Failure to do so exposes the implementation to risks of request smuggling
  attacks or authority splitting.

I guess it's too late if the publication request was sent, but I prefer
to ask just in case...

Thanks,
Willy

---

From: Cory Benfield <[email-masked]>
Date: Tue, 28 Sep 2021 10:55:03 +0100

> this could have done the job:

I don't think this is true: -semantics defines these as "Control Data"
(Section 6.2) and points most of these constructs to other sections.

For example, method is referenced in Section 9 and given the ABNF
`token`, which is a stricter constraint than you ask for here. Section
7.2 covers :authority and gives it the ABNF uri-host [ ":" port ],
which again is a stricter constraint. Finally, the http2bis definition
of :path header calls out that its value is absolute-path, optionally
followed by ? and query, unless it is *.

I don't know that we gain much by further constraints unless trying to
offer intermediaries an off-ramp for less specific enforcement. On the
other hand, most intermediaries cannot be agnostic to control data, so
they likely have to police this themselves.

---

From: Willy Tarreau <[email-masked]>
Date: Tue, 28 Sep 2021 12:13:18 +0200

Hi Cory,

> followed by ? and query, unless it is *.

I know, but there is one particular case that significantly increases
this risk, which is when you're adding H2 to code already supporting H1
where the controls are performed later by code that you already trust for
doing the right thing with elements extracted from H1. Then you assemble
everything and parse the result via your well-trusted request-line parser.
But it's too late, the space in :method, the "://" in :scheme, the "/" or
space in :authority, or the space in :path have already defined different
delimiters.

> they likely have to police this themselves.

Sure but these are really not natural to process on the output of HPACK,
in part because some of them most of the time come from the static
dictionary itself (:method and :scheme) where nobody expects to find
a ":" in a scheme for example :-/

I mean, it's really easy to get trapped, and the long list of examples
below tends to confirm it:

   https://portswigger.net/research/http2

I'd have liked to at least add that to the security recommendations.

Thanks,
Willy

---

From: Julian Reschke <[email-masked]>
Date: Tue, 28 Sep 2021 12:36:33 +0200

Am 28.09.2021 um 10:56 schrieb Willy Tarreau:
> ...
> ...

IETF Last Call has not even started :-)

Best regards, Julian

---

From: Cory Benfield <[email-masked]>
Date: Tue, 28 Sep 2021 14:23:25 +0100

> space in :authority, or the space in :path have already defined different

I'm nervous about the idea of adding normative requirements that are
there to defend against an implementation choice. In general if you
transform a HTTP/2 message into a HTTP/1.1 message before you parse it
you need to be careful, because it is extremely hard to identically
reproduce a HTTP/2 message in a HTTP/1.1 wire format. In some cases
it's not possible: for example, HTTP/2 does not allow chunked encoding
but does allow bodies on requests without a content length header,
while HTTP/1.1 is the other way around.

> I'd have liked to at least add that to the security recommendations.

I think my security recommendation would be much stronger: do not
naively attempt to serialise a HTTP/1.1 message from a HTTP/2 message.
There exists no simple string-formatting transformation of a valid
HTTP/2 message into a valid HTTP/1.1 message. To perform this
transformation it is necessary to parse the HTTP/2 message into a
structured, semantic form, and then re-serialize that message into
HTTP/1.1.

I'm not strongly opposed to doing this because, well, we did provide
guidance for other fields. But this does seem like it's a good
indication of how difficult this kind of guidance is: we end up having
to provide endless detailed exceptions that ultimately boil down to
"you cannot printf HTTP/2 to HTTP/1".

---

From: Willy Tarreau <[email-masked]>
Date: Tue, 28 Sep 2021 15:47:08 +0200

On Tue, Sep 28, 2021 at 02:23:25PM +0100, Cory Benfield wrote:
> reproduce a HTTP/2 message in a HTTP/1.1 wire format.

I know but to be honest it's not always plain H2 to H1. Sometimes it's
just that you recompose the protocol elements that you already know how
to parse (i.e. the URI) and it seems perfectly legit at first glance to
say "I'm having a URI parser that I'm using for H1, it's rock-solid,
I'll naturally use it for the H2 URI". Given that the URI is split into
blocks you do not necessarily see the problem when you reassemble them.
We did the mistake exactly on this one :-)

> naively attempt to serialise a HTTP/1.1 message from a HTTP/2 message.

I agree with this, which is also why I objected to seeing HTTP/1.1
appearing in normative text. But quite frankly, a URI is not HTTP/1.1
it's HTTP. Many of us have to recompose an RFC3986-compliant URI from
a request, and it's easy to overlook this aspect that is specific to
H2 (and H3). In addition, some implementations will not even have the
required code to check for abuse in these sub-components, because they're
normally only extracted from the single string by stopping on the
delimitors, which guarantees they are correct. Thus the check on each
individual one can possible not even exist.

> structured, semantic form, and then re-serialize that message into

I totally agree and that's what we're doing. The semantics support
a "URI" :-/

> guidance for other fields.

That was my feeling as well, to remind implementers not to take the
task lightly.

> But this does seem like it's a good

I agree with that and that's not what I'm seeking. When you see a URI
cut into pieces in a diagram where the scheme goes at one place, the
authority at another and the path in a third one, with only valid
examples, it's really difficult to think differently than gluing these
valid elements together to perform the inverse operation.

We do have examples of how H2 requests could be decoded in an HTTP/1-like
representation for easier reading. Maybe we could simply have warnings
showing what could result from a lack of individual components checks.

Just my two cents,
Willy