h2: honor http_req_hdr_len

Honor http_req_hdr_len by checking the size of each individual header field. In case a header exceeds the limit, we make sure to process all the headers before failing the stream to keep a consistent HPACK table state for the connection.
varnishcache · May 29, 2023 · 9a73b57 · 9a73b57
1 parent 271ec87
commit 9a73b57
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 5 deletions.
diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h
@@ -141,6 +141,7 @@ struct h2_req {
 	int64_t				t_window;
 	int64_t				r_window;
 	uint64_t			req_len;
+	char				hpack_err;
 
 	/* Where to wake this stream up */
 	struct worker			*wrk;
@@ -227,7 +228,7 @@ struct h2h_decode {
 void h2h_decode_init(const struct h2_sess *h2);
 h2_error h2h_decode_fini(const struct h2_sess *h2);
 h2_error h2h_decode_bytes(struct h2_sess *h2, const uint8_t *ptr,
-    size_t len);
+    size_t len, struct h2_req *r2);
 
 /* cache_http2_send.c */
 void H2_Send_Get(struct worker *, struct h2_sess *, struct h2_req *);

diff --git a/bin/varnishd/http2/cache_http2_hpack.c b/bin/varnishd/http2/cache_http2_hpack.c
@@ -263,7 +263,7 @@ h2h_decode_fini(const struct h2_sess *h2)
  * H2E_PROTOCOL_ERROR: Malformed header or duplicate pseudo-header.
  */
 h2_error
-h2h_decode_bytes(struct h2_sess *h2, const uint8_t *in, size_t in_l)
+h2h_decode_bytes(struct h2_sess *h2, const uint8_t *in, size_t in_l, struct h2_req *r2)
 {
 	struct http *hp;
 	struct h2h_decode *d;
@@ -335,6 +335,8 @@ h2h_decode_bytes(struct h2_sess *h2, const uint8_t *in, size_t in_l)
 			    d->out_u);
 			if (d->error)
 				break;
+			if (d->out_u > cache_param->http_req_hdr_len)
+				r2->hpack_err = 1;
 			d->error = h2h_addhdr(hp, d->out, d->namelen, d->out_u);
 			if (d->error)
 				break;

diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
@@ -155,6 +155,7 @@ h2_new_req(struct h2_sess *h2, unsigned stream, struct req *req)
 	r2->stream = stream;
 	r2->req = req;
 	r2->req_len = 0;
+	r2->hpack_err = 0;
 	if (stream)
 		r2->counted = 1;
 	r2->r_window = h2->local_settings.initial_window_size;
@@ -567,7 +568,7 @@ h2_end_headers(struct worker *wrk, struct h2_sess *h2,
 	ASSERT_RXTHR(h2);
 	assert(r2->state == H2_S_OPEN);
 	r2->req_len = (uint64_t)(h2->decode->out - (char*)WS_Reservation(req->http->ws));
-	if (r2->req_len > cache_param->http_req_size ||
+	if (r2->hpack_err || r2->req_len > cache_param->http_req_size ||
 		r2->req_len > cache_param->h2_max_header_list_size) {
 		VSLb(h2->new_req->http->vsl, SLT_BogoHeader,
 				"H2: stream %u: Received request header(s) that "
@@ -719,7 +720,7 @@ h2_rx_headers(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
 		l -= 5;
 		p += 5;
 	}
-	h2e = h2h_decode_bytes(h2, p, l);
+	h2e = h2h_decode_bytes(h2, p, l, r2);
 	if (h2e != NULL) {
 		Lck_Lock(&h2->sess->mtx);
 		VSLb(h2->vsl, SLT_Debug, "HPACK(hdr) %s", h2e->name);
@@ -753,7 +754,7 @@ h2_rx_continuation(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
 	if (r2 == NULL || r2->state != H2_S_OPEN || r2->req != h2->new_req)
 		return (H2CE_PROTOCOL_ERROR);	// XXX spec ?
 	req = r2->req;
-	h2e = h2h_decode_bytes(h2, h2->rxf_data, h2->rxf_len);
+	h2e = h2h_decode_bytes(h2, h2->rxf_data, h2->rxf_len, r2);
 	r2->req->acct.req_hdrbytes += h2->rxf_len;
 	if (h2e != NULL) {
 		Lck_Lock(&h2->sess->mtx);