1berry/haproxy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

BUG/MINOR: h2: always trim leading and trailing LWS in header values

Browse Source 
Annika Wickert reported some occasional disconnections between haproxy
and varnish when communicating over HTTP/2, with varnish complaining
about protocol errors while captures looked apparently normal. Nils
Goroll managed to reproduce this on varnish by injecting the capture of
the outgoing haproxy traffic and noticed that haproxy was forwarding a
header value containing a trailing space, which is now explicitly
forbidden since RFC9113.

It turns out that the only way for such a header to pass through haproxy
is to arrive in h2 and not be edited, in which case it will arrive in
HTX with its undesired spaces. Since the code dealing with HTX headers
always trims spaces around them, these are not observable in dumps, but
only when started in debug mode (-d). Conversions to/from h1 also drop
the spaces.

With this patch we trim LWS both on input and on output. This way we
always present clean headers in the whole stack, and even if some are
manually crafted by the configuration or Lua, they will be trimmed on
the output.

This must be backported to all stable versions.

Thanks to Annika for the helpful capture and Nils for the help with
the analysis on the varnish side!
This commit is contained in:
Willy Tarreau 2025-02-24 09:17:22 +01:00
parent 9011b3621b
commit bbf824933f
2 changed files with 42 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
src/h2.c
View File

@ -318,6 +318,7 @@ int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *ms
struct htx_sl *sl = NULL;
unsigned int sl_flags = 0;
const char *ctl;
struct ist v;
lck = ck = -1; // no cookie for now
fields = 0;
@ -434,7 +435,15 @@ int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *ms
continue;
}
if (!htx_add_header(htx, list[idx].n, list[idx].v))
/* trim leading/trailing LWS as per RC9113#8.2.1 */
for (v = list[idx].v; v.len; v.len--) {
if (unlikely(HTTP_IS_LWS(*v.ptr)))
v.ptr++;
else if (!unlikely(HTTP_IS_LWS(v.ptr[v.len - 1])))
break;
}
if (!htx_add_header(htx, list[idx].n, v))
goto fail;
}
@ -623,6 +632,7 @@ int h2_make_htx_response(struct http_hdr *list, struct htx *htx, unsigned int *m
struct htx_sl *sl = NULL;
unsigned int sl_flags = 0;
const char *ctl;
struct ist v;
fields = 0;
for (idx = 0; list[idx].n.len != 0; idx++) {
@ -702,7 +712,15 @@ int h2_make_htx_response(struct http_hdr *list, struct htx *htx, unsigned int *m
isteq(list[idx].n, ist("transfer-encoding")))
goto fail;
if (!htx_add_header(htx, list[idx].n, list[idx].v))
/* trim leading/trailing LWS as per RC9113#8.2.1 */
for (v = list[idx].v; v.len; v.len--) {
if (unlikely(HTTP_IS_LWS(*v.ptr)))
v.ptr++;
else if (!unlikely(HTTP_IS_LWS(v.ptr[v.len - 1])))
break;
}
if (!htx_add_header(htx, list[idx].n, v))
goto fail;
}
@ -781,6 +799,7 @@ int h2_make_htx_response(struct http_hdr *list, struct htx *htx, unsigned int *m
int h2_make_htx_trailers(struct http_hdr *list, struct htx *htx)
{
const char *ctl;
struct ist v;
uint32_t idx;
int i;
@ -816,7 +835,15 @@ int h2_make_htx_trailers(struct http_hdr *list, struct htx *htx)
if (unlikely(ctl) && http_header_has_forbidden_char(list[idx].v, ctl))
goto fail;
if (!htx_add_trailer(htx, list[idx].n, list[idx].v))
/* trim leading/trailing LWS as per RC9113#8.2.1 */
for (v = list[idx].v; v.len; v.len--) {
if (unlikely(HTTP_IS_LWS(*v.ptr)))
v.ptr++;
else if (!unlikely(HTTP_IS_LWS(v.ptr[v.len - 1])))
break;
}
if (!htx_add_trailer(htx, list[idx].n, v))
goto fail;
}

14
src/mux_h2.c
View File

@ -19,6 +19,7 @@
#include <haproxy/hpack-dec.h>
#include <haproxy/hpack-enc.h>
#include <haproxy/hpack-tbl.h>
#include <haproxy/http.h>
#include <haproxy/http_htx.h>
#include <haproxy/htx.h>
#include <haproxy/istbuf.h>
@ -1209,11 +1210,20 @@ static inline int h2_encode_header(struct buffer *buf, const struct ist hn, cons
uint64_t mask, const struct ist trc_loc, const char *func,
const struct h2c *h2c, const struct h2s *h2s)
{
struct ist v;
int ret;
ret = hpack_encode_header(buf, hn, hv);
/* trim leading/trailing LWS as per RC9113#8.2.1 */
for (v = hv; v.len; v.len--) {
if (unlikely(HTTP_IS_LWS(*v.ptr)))
v.ptr++;
else if (!unlikely(HTTP_IS_LWS(v.ptr[v.len - 1])))
break;
}
ret = hpack_encode_header(buf, hn, v);
if (ret)
h2_trace_header(hn, hv, mask, trc_loc, func, h2c, h2s);
h2_trace_header(hn, v, mask, trc_loc, func, h2c, h2s);
return ret;
}