BUG/MINOR: mux-h2: make sure to produce a log on invalid requests
As reported by Dominik Froehlich in github issue #1968, some H2 request
parsing errors do not result in a log being emitted. This is annoying
for debugging because while an RST_STREAM is correctly emitted to the
client, there's no way without enabling traces to find it on the
haproxy side.
After some testing with various abnormal requests, a few places were
found where logs were missing and could be added. In this case, we
simply use sess_log() so some sample fetch functions might not be
available since the stream is not created. But at least there will
be a BADREQ in the logs. A good eaxmple of this consists in sending
forbidden headers or header syntax (e.g. presence of LF in value).
Some quick tests can be done this way:
- protocol error (LF in value):
curl -iv --http2-prior-knowledge -H "$(printf 'a:b\na')" http://0:8001/
- too large header block after decoding:
curl -v --http2-prior-knowledge -H "a:$(perl -e "print('a'x10000)")" -H "a:$(perl -e "print('a'x10000)")" http://localhost:8001/
This should be backported where needed, most likely 2.7 and 2.6 at
least for a start, and progressively to other versions.
(cherry picked from commit f43f36da5be06da0f08efdb496e36a0edb35206a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 5341eee2ffcbc24ea6b903f73e9781d1b074e40e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit a627d15da26d5493114955b55f831a1033685ead)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit c864cf0e6cbd8358c7b69dc0e73b34050be9dd05)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/src/mux_h2.c b/src/mux_h2.c
index 4d56ea6..0ab8653 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -2760,6 +2760,7 @@
/* unrecoverable error ? */
if (h2c->st0 >= H2_CS_ERROR) {
TRACE_USER("Unrecoverable error decoding H2 trailers", H2_EV_RX_FRAME|H2_EV_RX_HDR|H2_EV_STRM_NEW|H2_EV_STRM_END, h2c->conn, 0, &rxbuf);
+ sess_log(h2c->conn->owner);
goto out;
}
@@ -2774,6 +2775,7 @@
/* Failed to decode this frame (e.g. too large request)
* but the HPACK decompressor is still synchronized.
*/
+ sess_log(h2c->conn->owner);
h2s_error(h2s, H2_ERR_INTERNAL_ERROR);
TRACE_USER("Stream error decoding H2 trailers", H2_EV_RX_FRAME|H2_EV_RX_HDR|H2_EV_STRM_NEW|H2_EV_STRM_END, h2c->conn, 0, &rxbuf);
h2c->st0 = H2_CS_FRAME_E;
@@ -2785,6 +2787,7 @@
* the data and send another RST.
*/
error = h2c_decode_headers(h2c, &rxbuf, &flags, &body_len, NULL);
+ sess_log(h2c->conn->owner);
h2s = (struct h2s*)h2_error_stream;
goto send_rst;
}
@@ -2804,6 +2807,7 @@
/* unrecoverable error ? */
if (h2c->st0 >= H2_CS_ERROR) {
TRACE_USER("Unrecoverable error decoding H2 request", H2_EV_RX_FRAME|H2_EV_RX_HDR|H2_EV_STRM_NEW|H2_EV_STRM_END, h2c->conn, 0, &rxbuf);
+ sess_log(h2c->conn->owner);
goto out;
}
@@ -2818,6 +2822,7 @@
/* Failed to decode this stream (e.g. too large request)
* but the HPACK decompressor is still synchronized.
*/
+ sess_log(h2c->conn->owner);
h2s = (struct h2s*)h2_error_stream;
goto send_rst;
}