src/proto_http.c | 10 10 + 0 - 0 !
1 file changed, 10 insertions(+)

 [patch] bug/medium: http: also reject messages where "chunked" is
 missing from transfer-enoding

Nathan Davison (@ndavison) reported that in legacy mode we don't
correctly reject requests or responses featuring a transfer-encoding
header missing the "chunked" value. As mandated in the protocol spec,
the test verifies that "chunked" is the last one, but only does so when
it is present. As such, "transfer-encoding: foobar" is not rejected,
only "transfer-encoding: chunked, foobar" will be.

The impact is limited, but if combined with "http-reuse always", it
could be used as a help to construct a content smuggling attack against
a vulnerable component employing a lenient parser which would ignore
the content-length header as soon as it sees a transfer-encoding one,
without even parsing it. In this case haproxy would fail to protect it.

The fix consists in completing the existing checks to verify that
"chunked" was present if any "transfer-encoding" header was met,
otherwise either reject the request message or make the response
end on a close.

This fix is only for 2.0 and older versions as legacy mode was
removed from 2.1. It should be backported to all maintained versions.

(cherry picked from commit 196a7df44d8129d1adc795da020b722614d6a581)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 5513fcaa601dd344be548430fc1760dbedebf4f2)
Signed-off-by: Willy Tarreau <w@1wt.eu>

contrib/halog/Makefile | 16 5 + 11 - 0 !
1 file changed, 5 insertions(+), 11 deletions(-)

 use dpkg-buildflags to build halog

contrib/systemd/haproxy.service.in | 2 1 + 1 - 0 !
1 file changed, 1 insertion(+), 1 deletion(-)

 start after rsyslog.service

As HAProxy is running chrooted by default, we rely on an additional syslog
socket created by rsyslog inside the chroot for logging. As this socket cannot
trigger syslog activation, we explicitly order HAProxy after rsyslog.service.
Note that we are not using syslog.service here, since the additional socket is
rsyslog-specific.

contrib/systemd/haproxy.service.in | 2 2 + 0 - 0 !
1 file changed, 2 insertions(+)

 add documentation field to the systemd unit

contrib/systemd/haproxy.service.in | 7 4 + 3 - 0 !
1 file changed, 4 insertions(+), 3 deletions(-)

 use the variables from /etc/default/haproxy

This will allow seamless upgrades from the sysvinit system while respecting
any changes the users may have made. It will also make local configuration
easier than overriding the systemd unit file.

include/common/ist.h | 41 41 + 0 - 0 !
1 file changed, 41 insertions(+)

 minor: ist: add ist_find_ctl()

This new function looks for the first control character in a string (a
char whose value is between 0x00 and 0x1F included) and returns it, or
NULL if there is none. It is optimized for quickly evicting non-matching
strings and scans ~0.43 bytes per cycle. It can be used as an accelerator
when it's needed to look up several of these characters (e.g. CR/LF/NUL).

(cherry picked from commit 8f3ce06f14e13719c9353794d60001eab8d43717)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 658e0ddd64100874193a1c543cc191e540a277f0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 6a9ffa70ecd747993505236557140a7081b65ee2)
Signed-off-by: Willy Tarreau <w@1wt.eu>

src/h2.c | 25 25 + 0 - 0 !
1 file changed, 25 insertions(+)

 bug/major: h2: reject header values containing invalid chars
MIME-Version: 1.0
Content-Type: text/plain; charset=latin1
Content-Transfer-Encoding: 8bit

Tim Dsterhus reported an annoying problem in the H2 decoder related to
an ambiguity in the H2 spec. The spec says in section 10.3 that HTTP/2
allows header field values that are not valid (since they're binary) and
at the same time that an H2 to H1 gateway must be careful to reject headers
whose values contain \0, \r or \n.

Till now, and for the sake of the ability to maintain end-to-end binary
transparency in H2-to-H2, the H2 mux wouldn't reject this since it does
not know what version will be used on the other side.

In theory we should in fact perform such a check when converting an HTX
header to H1. But this causes a problem as it means that all our rule sets,
sample fetches, captures, logs or redirects may still find an LF in a header
coming from H2. Also in 2.0 and older in legacy mode, the frames are instantly
converted to H1 and HTX couldn't help there. So this means that in practice
we must refrain from delivering such a header upwards, regardless of any
outgoing protocol consideration.

Applying such a lookup on all headers leaving the mux comes with a
significant performance hit, especially for large ones. A first attempt
was made at placing this into the HPACK decoder to refrain from learning
invalid literals but error reporting becomes more complicated. Additional
tests show that doing this within the HTX transcoding loop benefits from
the hot L1 cache, and that by skipping up to 8 bytes per iteration the
CPU cost remains within noise margin, around ~0.5%.

This patch must be backported as far as 1.8 since this bug could be
exploited and serve as the base for an attack. In 2.0 and earlier the
fix must also be added to functions h2_make_h1_request() and
h2_make_h1_trailers() to handle legacy mode. It relies on previous patch
"MINOR: ist: add ist_find_ctl()" to speed up the control bytes lookup.

All credits go to Tim for his detailed bug report and his initial patch.

(cherry picked from commit 54f53ef7ce4102be596130b44c768d1818570344)
[wt: checks added to h2_make_h1_request() and h2_make_h1_trailers()]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit c36c6789d1fc5e80e1f606a2828b72d5dd0ce22d)
[wt: minor ctx adjustments in trailers code -- was an H1 block in 1.9]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 00c8abd54e0d32cadca4183989e8274b1660d935)
[wt: dropped htx & trailers part since not present in 1.8]
Signed-off-by: Willy Tarreau <w@1wt.eu>

src/h2.c | 17 11 + 6 - 0 !
1 file changed, 11 insertions(+), 6 deletions(-)

 bug/major: h2: make header field name filtering stronger
MIME-Version: 1.0
Content-Type: text/plain; charset=latin1
Content-Transfer-Encoding: 8bit

Tim Dsterhus found that the amount of sanitization we perform on HTTP
header field names received in H2 is insufficient. Currently we reject
upper case letters as mandated by RFC7540#8.1.2, but section 10.3 also
requires that intermediaries translating streams to HTTP/1 further
refine the filtering to also reject invalid names (which means any name
that doesn't match a token). There is a small trick here which is that
the colon character used to start pseudo-header names doesn't match a
token, so pseudo-header names fall into that category, thus we have to
swap the pseudo-header name lookup with this check so that we only check
from the second character (past the ':') in case of pseudo-header names.

Another possibility could have been to perform this check only in the
HTX-to-H1 trancoder but doing would still expose the configured rules
and logs to such header names.

This fix must be backported as far as 1.8 since this bug could be
exploited and serve as the base for an attack. In 2.0 and earlier,
functions h2_make_h1_request() and h2_make_h1_trailers() must also
be adapted to sanitize requests coming in legacy mode.

(cherry picked from commit 146f53ae7e97dbfe496d0445c2802dd0a30b0878)
[wt: check added to h2_make_h1_request() and h2_make_h1_trailers()]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 5eaeec588d1de4f262f4ecffe0d39a36212965c3)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 8cf848ae60040620bd9293d912c3e3d68505379e)
[wt: dropped HTX and trailers parts since absent in 1.8; include h1.h]
Signed-off-by: Willy Tarreau <w@1wt.eu>

src/hpack-tbl.c | 4 2 + 2 - 0 !
1 file changed, 2 insertions(+), 2 deletions(-)

 bug/critical: hpack: never index a header into the headroom after
 wrapping

The HPACK header table is implemented as a wrapping list inside a contigous
area. Headers names and values are stored from right to left while indexes
are stored from left to right. When there's no more room to store a new one,
we wrap to the right again, or possibly defragment it if needed. The condition
do use the right part (called tailroom) or the left part (called headroom)
depends on the location of the last inserted header. After wrapping happens,
the code forces to stick to tailroom by pretending there's no more headroom,
so that the size fit test always fails. The problem is that nothing prevents
from storing a header with an empty name and empty value, resulting in a
total size of zero bytes, which satisfies the condition to use the headroom.
Doing this in a wrapped buffer results in changing the "front" header index
and causing miscalculations on the available size and the addresses of the
next headers. This may even allow to overwrite some parts of the index,
opening the possibility to perform arbitrary writes into a 32-bit relative
address space.

This patch fixes the issue by making sure the headroom is considered only
when the buffer does not wrap, instead of relying on the zero size. This
must be backported to all versions supporting H2, which is as far as 1.8.

Many thanks to Felix Wilhelm of Google Project Zero for responsibly
reporting this problem with a reproducer and a detailed analysis.