BUG/MAJOR: h2: enforce checks on the method syntax before translating to HTX
authorWilly Tarreau <w@1wt.eu>
Wed, 11 Aug 2021 09:12:46 +0000 (11:12 +0200)
committerRaspbian forward porter <root@raspbian.org>
Thu, 9 Sep 2021 16:57:32 +0000 (17:57 +0100)
commit8f1bfce1f8a9fde7a02112331e33afbec6ffd8eb
tree0044e029e835b2cd4980a08c121de5c522818cc6
parent9d7cb4184e06a65160739a2fedd7a1ef373ae993
BUG/MAJOR: h2: enforce checks on the method syntax before translating to HTX

The situation with message components in H2 is always troubling. They're
produced by the HPACK layer which contains a dictionary of well-known
hardcoded values, yet wants to remain binary transparent and protocol-
agnostic with HTTP just being one user, yet at the H2 layer we're
supposed to enforce some checks on some selected pseudo-headers that
come from internal constants... The :method pseudo-header is no exception
and is not tested when coming from the HPACK layer. This makes it possible
to pass random chars into methods, that can be serialized on another H2
connection (where they would not harm), or worse, on an H1 connection
where they can be used to transform the forwareded request. This is
similar to the request line injection described here:

   https://portswigger.net/research/http2

A workaround here is to reject malformed methods by placing this rule
in the frontend or backend, at least before leaving haproxy in H1:

   http-request reject if { method -m reg [^A-Z0-9] }

Alternately H2 may be globally disabled by commenting out the "alpn"
directive on "bind" lines, and by rejecting H2 streams creation by
adding the following statement to the global section:

   tune.h2.max-concurrent-streams 0

This patch adds a check for each character of the method to be part of
the ones permitted in a token, as mentioned in RFC7231#4.1. This should
be backported to versions 2.0 and above, maybe even 1.8. For older
versions not having HTX_FL_PARSING_ERROR, a "goto fail" works as well
as it results in a protocol error at the stream level. Non-HTX versions
were initially thought to be safe but must be carefully rechecked since
they transcode the request into H1 before processing it.

Thanks to Tim Düsterhus for reporting that one.

(cherry picked from commit b4be735a0a7c4a00bf3d774334763536774d7eea)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 6b827f661374704e91322a82197bbfbfbf910f70)
[wt: adapted since no meth_sl in 2.3]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit fbeb053d1a83faedbf3edbe04bde39bc7304cddd)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Gbp-Pq: Name 2.2-0004-BUG-MAJOR-h2-enforce-checks-on-the-method-syntax-bef.patch
src/h2.c