From 9be22198e258d0e7a5c41f4291792214a29405cf Mon Sep 17 00:00:00 2001 From: Martin Blix Grydeland Date: Tue, 22 Jun 2021 11:47:55 +0200 Subject: [PATCH] Take content length into account on H/2 request bodies When receiving H/2 data frames, make sure to take the advertised content length into account, and fail appropriately if the combined sum of the data frames does not match the content length. --- bin/varnishd/http2/cache_http2.h | 2 ++ bin/varnishd/http2/cache_http2_proto.c | 46 +++++++++++++++++++++----- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h index cfe8598..9f4fc0c 100644 --- a/bin/varnishd/http2/cache_http2.h +++ b/bin/varnishd/http2/cache_http2.h @@ -132,6 +132,8 @@ struct h2_req { /* Where to wake this stream up */ struct worker *wrk; + ssize_t reqbody_bytes; + VTAILQ_ENTRY(h2_req) tx_list; h2_error error; }; diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c index bb0f5f9..3526b49 100644 --- a/bin/varnishd/http2/cache_http2_proto.c +++ b/bin/varnishd/http2/cache_http2_proto.c @@ -528,7 +528,7 @@ h2_end_headers(struct worker *wrk, struct h2_sess *h2, struct req *req, struct h2_req *r2) { h2_error h2e; - const char *b; + ssize_t cl; ASSERT_RXTHR(h2); assert(r2->state == H2_S_OPEN); @@ -551,14 +551,24 @@ h2_end_headers(struct worker *wrk, struct h2_sess *h2, // XXX: Have I mentioned H/2 Is hodge-podge ? http_CollectHdrSep(req->http, H_Cookie, "; "); // rfc7540,l,3114,3120 + cl = http_GetContentLength(req->http); + assert(cl >= -2); + if (cl == -2) { + VSLb(h2->vsl, SLT_Debug, "Non-parseable Content-Length"); + return (H2SE_PROTOCOL_ERROR); + } + if (req->req_body_status == REQ_BODY_INIT) { - if (!http_GetHdr(req->http, H_Content_Length, &b)) + if (cl == -1) req->req_body_status = REQ_BODY_WITHOUT_LEN; else req->req_body_status = REQ_BODY_WITH_LEN; + req->htc->content_length = cl; } else { + /* A HEADER frame contained END_STREAM */ assert (req->req_body_status == REQ_BODY_NONE); - if (http_GetContentLength(req->http) > 0) + r2->state = H2_S_CLOS_REM; + if (cl > 0) return (H2CE_PROTOCOL_ERROR); //rfc7540,l,1838,1840 } @@ -695,6 +705,7 @@ h2_rx_data(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2) int w1 = 0, w2 = 0; char buf[4]; unsigned wi; + ssize_t cl; (void)wrk; ASSERT_RXTHR(h2); @@ -707,6 +718,23 @@ h2_rx_data(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2) Lck_Unlock(&h2->sess->mtx); return (h2->error ? h2->error : r2->error); } + + r2->reqbody_bytes += h2->rxf_len; + if (h2->rxf_flags & H2FF_DATA_END_STREAM) + r2->state = H2_S_CLOS_REM; + cl = r2->req->htc->content_length; + if (cl >= 0 && (r2->reqbody_bytes > cl || + (r2->state >= H2_S_CLOS_REM && r2->reqbody_bytes != cl))) { + VSLb(h2->vsl, SLT_Debug, + "H2: stream %u: Received data and Content-Length" + " mismatch", h2->rxf_stream); + r2->error = H2SE_PROTOCOL_ERROR; // rfc7540,l,3150,3163 + if (r2->cond) + AZ(pthread_cond_signal(r2->cond)); + Lck_Unlock(&h2->sess->mtx); + return (H2SE_PROTOCOL_ERROR); + } + AZ(h2->mailcall); h2->mailcall = r2; h2->req0->r_window -= h2->rxf_len; @@ -725,6 +753,8 @@ h2_rx_data(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2) r2->r_window += wi; w2 = 1; } + + Lck_Unlock(&h2->sess->mtx); if (w1 || w2) { @@ -747,7 +777,7 @@ h2_vfp_body(struct vfp_ctx *vc, struct vfp_entry *vfe, void *ptr, ssize_t *lp) struct h2_req *r2; struct h2_sess *h2; unsigned l; - enum vfp_status retval = VFP_OK; + enum vfp_status retval; CHECK_OBJ_NOTNULL(vc, VFP_CTX_MAGIC); CHECK_OBJ_NOTNULL(vfe, VFP_ENTRY_MAGIC); @@ -776,10 +806,10 @@ h2_vfp_body(struct vfp_ctx *vc, struct vfp_entry *vfe, void *ptr, ssize_t *lp) h2->rxf_len -= l; } *lp = l; - if (h2->rxf_len == 0) { - if (h2->rxf_flags & H2FF_DATA_END_STREAM) - retval = VFP_END; - } + if (h2->rxf_len == 0 && r2->state >= H2_S_CLOS_REM) + retval = VFP_END; + else + retval = VFP_OK; h2->mailcall = NULL; AZ(pthread_cond_signal(h2->cond)); } -- 2.27.0