From 19895714b2d81173ec05e1cd76ade982490b4e4d Mon Sep 17 00:00:00 2001 From: Martin Blix Grydeland Date: Thu, 17 Nov 2022 17:35:13 +0100 Subject: [PATCH 1/4] Implement HTTP request and response egress validation This implements HTTP character set validation run on the `struct http` after vcl_deliver, vcl_synth and vcl_backend_fetch. The idea is to pick up if the VCL program has ended up filling in illegal values that would violate the HTTP protocol. If a problem is found, we will error out with a 503. If the error is caught in vcl_synth, the connection will be closed. --- bin/varnishd/cache/cache.h | 2 + bin/varnishd/cache/cache_fetch.c | 5 + bin/varnishd/cache/cache_http.c | 160 +++++++++++++++++++++++++++++ bin/varnishd/cache/cache_req_fsm.c | 11 ++ bin/varnishtest/tests/c00125.vtc | 112 ++++++++++++++++++++ include/tbl/vsl_tags_http.h | 3 + 6 files changed, 293 insertions(+) create mode 100644 bin/varnishtest/tests/c00125.vtc diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h index 10fab7e8d53..df88755e452 100644 --- a/bin/varnishd/cache/cache.h +++ b/bin/varnishd/cache/cache.h @@ -666,6 +666,8 @@ int HTTP_IterHdrPack(struct worker *, struct objcore *, const char **); const char *HTTP_GetHdrPack(struct worker *, struct objcore *, hdr_t); stream_close_t http_DoConnection(struct http *hp, stream_close_t sc_close); int http_IsFiltered(const struct http *hp, unsigned u, unsigned how); +int HTTP_ValidateReq(const struct http *hp); +int HTTP_ValidateResp(const struct http *hp); #define HTTPH_R_PASS (1 << 0) /* Request (c->b) in pass mode */ #define HTTPH_R_FETCH (1 << 1) /* Request (c->b) for fetch */ diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c index 0f86d0305c7..5e46f6d7ebc 100644 --- a/bin/varnishd/cache/cache_fetch.c +++ b/bin/varnishd/cache/cache_fetch.c @@ -409,6 +409,11 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo) http_PrintfHeader(bo->bereq, "X-Varnish: %ju", VXID(bo->vsl->wid)); VCL_backend_fetch_method(bo->vcl, wrk, NULL, bo, NULL); + if (wrk->vpi->handling == VCL_RET_FETCH && HTTP_ValidateReq(bo->bereq)) { + VSLb(bo->vsl, SLT_VCL_Error, + "Backend request failed HTTP validation"); + wrk->vpi->handling = VCL_RET_FAIL; + } if (wrk->vpi->handling == VCL_RET_ABANDON || wrk->vpi->handling == VCL_RET_FAIL) diff --git a/bin/varnishd/cache/cache_http.c b/bin/varnishd/cache/cache_http.c index 63818eaad3c..b93de511044 100644 --- a/bin/varnishd/cache/cache_http.c +++ b/bin/varnishd/cache/cache_http.c @@ -1643,3 +1643,163 @@ http_Unset(struct http *hp, hdr_t hdr) } hp->nhd = v; } + +/*--------------------------------------------------------------------*/ + +static inline int +http_valid_Method(struct vsl_log *vsl, const txt t) +{ + const char *p; + + if (t.b == t.e) { + VSLb(vsl, SLT_HttpGarbage, "Empty method"); + return (-1); + } + for (p = t.b; p < t.e && *p != ':'; p++) { + if (!vct_istchar(*p)) { + VSLb(vsl, SLT_HttpGarbage, "Method: %.*s", + (int)(t.e - t.b), t.b); + return (-1); + } + } + return (0); +} + +static inline int +http_valid_URL(struct vsl_log *vsl, const txt t) +{ + const char *p; + + if (t.b == t.e) { + VSLb(vsl, SLT_HttpGarbage, "Empty URL"); + return (-1); + } + for (p = t.b; p < t.e; p++) { + if (vct_islws(*p) || vct_isctl(*p)) { + VSLb(vsl, SLT_HttpGarbage, "URL: %.*s", + (int)(t.e - t.b), t.b); + return (-1); + } + } + return (0); +} + +static inline int +http_valid_Protocol(struct vsl_log *vsl, const txt t) +{ + const char *p; + + for (p = t.b; p < t.e; p++) { + if (vct_islws(*p) || vct_isctl(*p)) { + VSLb(vsl, SLT_HttpGarbage, "Protocol: %.*s", + (int)(t.e - t.b), t.b); + return (-1); + } + } + return (0); +} + +static inline int +http_valid_Status(struct vsl_log *vsl, const txt t) +{ + const char *p; + int n; + + for (p = t.b, n = 0; p < t.e; p++, n++) { + if (!vct_isdigit(*p)) { + VSLb(vsl, SLT_HttpGarbage, "Status: %.*s", + (int)(t.e - t.b), t.b); + return (-1); + } + } + if (n != 3) { + VSLb(vsl, SLT_HttpGarbage, "Status: %.*s", + (int)(t.e - t.b), t.b); + return (-1); + } + return (0); +} + +static inline int +http_valid_Reason(struct vsl_log *vsl, const txt t) +{ + const char *p; + + for (p = t.b; p < t.e; p++) { + if (!vct_ishdrval(*p)) { + VSLb(vsl, SLT_HttpGarbage, "Reason: %.*s", + (int)(t.e - t.b), t.b); + return (-1); + } + } + return (0); +} + +static inline int +http_valid_header(struct vsl_log *vsl, const txt t) +{ + const char *p; + + (void)vsl; + for (p = t.b; p < t.e && *p != ':'; p++) { + if (!vct_istchar(*p)) { + VSLb(vsl, SLT_HttpGarbage, "Header: %.*s", + (int)(t.e - t.b), t.b); + return (-1); + } + } + if (*p != ':') { + VSLb(vsl, SLT_HttpGarbage, "Header: %.*s", + (int)(t.e - t.b), t.b); + return (-1); + } + for (; p < t.e; p++) { + if (!vct_ishdrval(*p)) { + VSLb(vsl, SLT_HttpGarbage, "Header: %.*s", + (int)(t.e - t.b), t.b); + return (-1); + } + } + return (0); +} + +static inline int +http_validate(const struct http *hp, int do_req, int do_resp) +{ + unsigned u; + int r = 0; + + CHECK_OBJ_NOTNULL(hp, HTTP_MAGIC); + for (u = 0; r == 0 && u < hp->nhd; u++) { +#define SLTH(tag, ind, req, resp, sdesc, ldesc) \ + if (req && do_req && u == ind) { \ + r = http_valid_##tag(hp->vsl, hp->hd[u]); \ + continue; \ + } \ + if (resp && do_resp && u == ind) { \ + r = http_valid_##tag(hp->vsl, hp->hd[u]); \ + continue; \ + } +#define SLTH_SKIP_EXTRA +#include "tbl/vsl_tags_http.h" + if (u >= HTTP_HDR_FIRST) + r = http_valid_header(hp->vsl, hp->hd[u]); + } + + return (r); +} + +int +HTTP_ValidateReq(const struct http *hp) +{ + return (http_validate(hp, 1, 0)); +} + +int +HTTP_ValidateResp(const struct http *hp) +{ + return (http_validate(hp, 0, 1)); +} + +/*--------------------------------------------------------------------*/ + diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c index ed97a01107a..eec85592293 100644 --- a/bin/varnishd/cache/cache_req_fsm.c +++ b/bin/varnishd/cache/cache_req_fsm.c @@ -233,6 +233,12 @@ cnt_deliver(struct worker *wrk, struct req *req) req->t_resp = W_TIM_real(wrk); VCL_deliver_method(req->vcl, wrk, req, NULL, NULL); + if (wrk->vpi->handling == VCL_RET_DELIVER && HTTP_ValidateResp(req->resp)) { + VSLb(req->vsl, SLT_VCL_Error, + "Response failed HTTP validation"); + wrk->vpi->handling = VCL_RET_FAIL; + } + assert(req->restarts <= cache_param->max_restarts); if (wrk->vpi->handling != VCL_RET_DELIVER) { @@ -336,6 +342,11 @@ cnt_synth(struct worker *wrk, struct req *req) AZ(VSB_finish(synth_body)); + if (wrk->vpi->handling == VCL_RET_DELIVER && HTTP_ValidateResp(req->resp)) { + VSLb(req->vsl, SLT_VCL_Error, + "Synthetic response failed HTTP validation"); + wrk->vpi->handling = VCL_RET_FAIL; + } VSLb_ts_req(req, "Process", W_TIM_real(wrk)); while (wrk->vpi->handling == VCL_RET_FAIL) { diff --git a/bin/varnishtest/tests/c00125.vtc b/bin/varnishtest/tests/c00125.vtc new file mode 100644 index 00000000000..888417a0a44 --- /dev/null +++ b/bin/varnishtest/tests/c00125.vtc @@ -0,0 +1,112 @@ +varnishtest "HTTP request and response egress validation" + +server s1 { + rxreq + txresp +} -start + +varnish v1 -syntax 4.0 -vcl+backend { + import blob; + + sub vcl_recv { + if (req.http.do_recv_url) { + set req.url = blob.transcode(encoded="new%0aline", decoding=URL); + } + } + + sub vcl_deliver { + if (req.http.do_deliver_proto) { + set resp.proto = blob.transcode(encoded="new%0aline", decoding=URL); + } + if (req.http.do_deliver_reason) { + set resp.reason = blob.transcode(encoded="new%0aline", decoding=URL); + } + if (req.http.do_deliver_header) { + set resp.http.foo = blob.transcode(encoded="new%0aline", decoding=URL); + } + if (req.http.do_synth_header) { + return (synth(200)); + } + } + + sub vcl_synth { + if (req.http.do_synth_header) { + set resp.http.foo = blob.transcode(encoded="new%0aline", decoding=URL); + } + } + + sub vcl_backend_fetch { + if (bereq.url == "/do_backend_fetch_method") { + set bereq.method = blob.transcode(encoded="new%0aline", decoding=URL); + } + if (bereq.url == "/do_backend_fetch_url") { + set bereq.url = blob.transcode(encoded="new%0aline", decoding=URL); + } + if (bereq.url == "/do_backend_fetch_header") { + set bereq.http.foo = blob.transcode(encoded="new%0aline", decoding=URL); + } + } +} -start + +# Prime the cache +client c1 { + txreq + rxresp + expect resp.status == 200 +} -run + +# Test bad vcl_deliver +client c1 { + txreq -hdr "do_deliver_proto: true" + rxresp + expect resp.status == 503 + expect_close +} -run +client c1 { + txreq -hdr "do_deliver_reason: true" + rxresp + expect resp.status == 503 + expect_close +} -run +client c1 { + txreq -hdr "do_deliver_header: true" + rxresp + expect resp.status == 503 + expect_close +} -run + +# Test bad vcl_synth +client c2 { + txreq -hdr "do_synth_header: true" + rxresp + expect resp.status == 500 + expect_close +} -run + +# Test bad vcl_backend_fetch +client c3 { + txreq -url /do_backend_fetch_method + rxresp + expect resp.status == 503 + # No expect_close as this will be a regular 503 object delivery +} -run +client c3 { + txreq -url /do_backend_fetch_url + rxresp + expect resp.status == 503 + # No expect_close as this will be a regular 503 object delivery +} -run +client c3 { + txreq -url /do_backend_fetch_header + rxresp + expect resp.status == 503 + # No expect_close as this will be a regular 503 object delivery +} -run + +# Test vcl_backend_fetch on bad URL inherited from request +client c4 { + txreq -hdr "do_recv_url: true" + rxresp + expect resp.status == 503 + # No expect_close as this will be a regular 503 object delivery +} -run diff --git a/include/tbl/vsl_tags_http.h b/include/tbl/vsl_tags_http.h index 29d73130023..eab2af885f5 100644 --- a/include/tbl/vsl_tags_http.h +++ b/include/tbl/vsl_tags_http.h @@ -72,6 +72,7 @@ SLTH(Reason, HTTP_HDR_REASON, 0, 1, "reason", "The HTTP response reason string.\n\n" ) +#ifndef SLTH_SKIP_EXTRA SLTH(Header, HTTP_HDR_FIRST, 1, 1, "header", "HTTP header contents.\n\n" "The format is::\n\n" @@ -97,8 +98,10 @@ SLTH(Unset, HTTP_HDR_UNSET, 1, 1, "unset header", SLTH(Lost, HTTP_HDR_LOST, 0, 0, "lost header", "" ) +#endif /* SLTH_SKIP_EXTRA */ #undef HEADER_NOTICE +#undef SLTH_SKIP_EXTRA #undef SLTH /*lint -restore */ From 67d1766cda9095948e9aa355c6d0b10444561e38 Mon Sep 17 00:00:00 2001 From: Martin Blix Grydeland Date: Thu, 17 Nov 2022 18:25:12 +0100 Subject: [PATCH 2/4] Add feature flags for controlling resp and bereq validation --- bin/varnishd/cache/cache_fetch.c | 3 +- bin/varnishd/cache/cache_req_fsm.c | 6 ++-- bin/varnishtest/tests/b00040.vtc | 2 +- bin/varnishtest/tests/c00127.vtc | 46 ++++++++++++++++++++++++++++++ include/tbl/feature_bits.h | 14 +++++++++ include/tbl/params.h | 4 ++- vmod/tests/blob_b00009.vtc | 2 ++ 7 files changed, 72 insertions(+), 5 deletions(-) create mode 100644 bin/varnishtest/tests/c00127.vtc diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c index 5e46f6d7ebc..11632360a71 100644 --- a/bin/varnishd/cache/cache_fetch.c +++ b/bin/varnishd/cache/cache_fetch.c @@ -409,7 +409,8 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo) http_PrintfHeader(bo->bereq, "X-Varnish: %ju", VXID(bo->vsl->wid)); VCL_backend_fetch_method(bo->vcl, wrk, NULL, bo, NULL); - if (wrk->vpi->handling == VCL_RET_FETCH && HTTP_ValidateReq(bo->bereq)) { + if (FEATURE(FEATURE_VALIDATE_BACKEND_REQUESTS) && + wrk->vpi->handling == VCL_RET_FETCH && HTTP_ValidateReq(bo->bereq)) { VSLb(bo->vsl, SLT_VCL_Error, "Backend request failed HTTP validation"); wrk->vpi->handling = VCL_RET_FAIL; diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c index eec85592293..7afcca37334 100644 --- a/bin/varnishd/cache/cache_req_fsm.c +++ b/bin/varnishd/cache/cache_req_fsm.c @@ -233,7 +233,8 @@ cnt_deliver(struct worker *wrk, struct req *req) req->t_resp = W_TIM_real(wrk); VCL_deliver_method(req->vcl, wrk, req, NULL, NULL); - if (wrk->vpi->handling == VCL_RET_DELIVER && HTTP_ValidateResp(req->resp)) { + if (FEATURE(FEATURE_VALIDATE_CLIENT_RESPONSES) && + wrk->vpi->handling == VCL_RET_DELIVER && HTTP_ValidateResp(req->resp)) { VSLb(req->vsl, SLT_VCL_Error, "Response failed HTTP validation"); wrk->vpi->handling = VCL_RET_FAIL; @@ -342,7 +343,8 @@ cnt_synth(struct worker *wrk, struct req *req) AZ(VSB_finish(synth_body)); - if (wrk->vpi->handling == VCL_RET_DELIVER && HTTP_ValidateResp(req->resp)) { + if (FEATURE(FEATURE_VALIDATE_CLIENT_RESPONSES) && + wrk->vpi->handling == VCL_RET_DELIVER && HTTP_ValidateResp(req->resp)) { VSLb(req->vsl, SLT_VCL_Error, "Synthetic response failed HTTP validation"); wrk->vpi->handling = VCL_RET_FAIL; diff --git a/bin/varnishtest/tests/b00040.vtc b/bin/varnishtest/tests/b00040.vtc index 7edce650947..5371896be9a 100644 --- a/bin/varnishtest/tests/b00040.vtc +++ b/bin/varnishtest/tests/b00040.vtc @@ -109,7 +109,7 @@ client c1 { logexpect l1 -wait -varnish v1 -cliok "param.set feature -validate_headers" +varnish v1 -cliok "param.set feature -validate_headers,-validate_backend_requests" client c1 { txreq -url /9 diff --git a/bin/varnishtest/tests/c00127.vtc b/bin/varnishtest/tests/c00127.vtc new file mode 100644 index 00000000000..78a60880e53 --- /dev/null +++ b/bin/varnishtest/tests/c00127.vtc @@ -0,0 +1,46 @@ +varnishtest "resp and bereq character validation feature flags" + +server s1 { + rxreq + txresp + + rxreq + expect req.url == /2 + expect req.http.foo == FOO + expect req.http.bar == BAR + txresp -hdr "from: /2" +} -start + +varnish v1 -vcl+backend { + import blob; + + sub vcl_recv { + if (req.url == "/2") { + set req.http.foo = blob.transcode(encoded="FOO%0d%0abar: BAR", decoding=URL); + } + } + + sub vcl_deliver { + if (req.url == "/1") { + set resp.http.foo = blob.transcode(encoded="FOO%0d%0abar: BAR", decoding=URL); + } + } +} -start + +varnish v1 -cliok "param.set feature -validate_client_responses,-validate_headers" +varnish v1 -cliok "param.set feature -validate_backend_requests,-validate_headers" + +client c1 { + txreq -url /1 + rxresp + expect resp.status == 200 + expect resp.http.foo == FOO + expect resp.http.bar == BAR +} -run + +client c2 { + txreq -url /2 + rxresp + expect resp.status == 200 + expect resp.http.from == /2 +} -run diff --git a/include/tbl/feature_bits.h b/include/tbl/feature_bits.h index 808efe9174c..8cc442afab6 100644 --- a/include/tbl/feature_bits.h +++ b/include/tbl/feature_bits.h @@ -96,6 +96,20 @@ FEATURE_BIT(VCL_REQ_RESET, vcl_req_reset, "When this happens MAIN.req_reset is incremented." ) +FEATURE_BIT(VALIDATE_CLIENT_RESPONSES, validate_client_responses, + "Check client HTTP responses for invalid characters." + " All HTTP responses will be checked for illegal characters set by the" + " VCL program before sending. Failures will cause a VCL_Error state" + " to be logged, and `vcl_synth` to be called." +) + +FEATURE_BIT(VALIDATE_BACKEND_REQUESTS, validate_backend_requests, + "Check backend HTTP requests for invalid characters." + " All backend HTTP requests will be checked for illegal characters set" + " by the VCL program before sending. Failures will cause a VCL_Error" + " state to be logged, and `vcl_backend_error` to be called." +) + #undef FEATURE_BIT /*lint -restore */ diff --git a/include/tbl/params.h b/include/tbl/params.h index 938b00f4b91..67c6975bdd4 100644 --- a/include/tbl/params.h +++ b/include/tbl/params.h @@ -1882,7 +1882,9 @@ PARAM_BITS( /* def */ "none," "+validate_headers," - "+vcl_req_reset", + "+vcl_req_reset," + "+validate_client_responses," + "+validate_backend_requests", /* descr */ "Enable/Disable various minor features.\n" "\tdefault\tSet default value (deprecated: use param.reset)\n" diff --git a/vmod/tests/blob_b00009.vtc b/vmod/tests/blob_b00009.vtc index edad4d479ca..9f9fe6bca53 100644 --- a/vmod/tests/blob_b00009.vtc +++ b/vmod/tests/blob_b00009.vtc @@ -140,6 +140,8 @@ varnish v1 -arg "-p http_max_hdr=128" -vcl+backend { } -start +varnish v1 -cliok "param.set feature -validate_backend_requests" + client c1 { txreq -url "/foo%20bar" rxresp From 103ad9a4540c7ce8218ad06556e76a822f839eec Mon Sep 17 00:00:00 2001 From: Dag Haavi Finstad Date: Fri, 14 Apr 2023 09:07:49 +0200 Subject: [PATCH 3/4] validate_client_resp: Limit this to esi_level == 0 There is no need to validate client response headers that we do not intend to transmit. --- bin/varnishd/cache/cache_req_fsm.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c index 7afcca37334..2422b15d5eb 100644 --- a/bin/varnishd/cache/cache_req_fsm.c +++ b/bin/varnishd/cache/cache_req_fsm.c @@ -234,7 +234,8 @@ cnt_deliver(struct worker *wrk, struct req *req) VCL_deliver_method(req->vcl, wrk, req, NULL, NULL); if (FEATURE(FEATURE_VALIDATE_CLIENT_RESPONSES) && - wrk->vpi->handling == VCL_RET_DELIVER && HTTP_ValidateResp(req->resp)) { + req->esi_level == 0 && wrk->vpi->handling == VCL_RET_DELIVER && + HTTP_ValidateResp(req->resp)) { VSLb(req->vsl, SLT_VCL_Error, "Response failed HTTP validation"); wrk->vpi->handling = VCL_RET_FAIL; @@ -344,7 +345,8 @@ cnt_synth(struct worker *wrk, struct req *req) AZ(VSB_finish(synth_body)); if (FEATURE(FEATURE_VALIDATE_CLIENT_RESPONSES) && - wrk->vpi->handling == VCL_RET_DELIVER && HTTP_ValidateResp(req->resp)) { + req->esi_level == 0 && wrk->vpi->handling == VCL_RET_DELIVER && + HTTP_ValidateResp(req->resp)) { VSLb(req->vsl, SLT_VCL_Error, "Synthetic response failed HTTP validation"); wrk->vpi->handling = VCL_RET_FAIL; From 504d274042876bec35b0bd08c831861e35eff6de Mon Sep 17 00:00:00 2001 From: Walid Boudebouda Date: Mon, 18 Sep 2023 18:03:46 +0200 Subject: [PATCH 4/4] transport: Move request/response validation to transport --- bin/varnishd/cache/cache_fetch.c | 6 ------ bin/varnishd/cache/cache_req_fsm.c | 15 --------------- bin/varnishd/http1/cache_http1_deliver.c | 9 +++++++++ bin/varnishd/http1/cache_http1_fetch.c | 9 +++++++++ bin/varnishd/http2/cache_http2_deliver.c | 7 +++++++ bin/varnishtest/tests/c00125.vtc | 11 +++++++++++ 6 files changed, 36 insertions(+), 21 deletions(-) diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c index 11632360a71..0f86d0305c7 100644 --- a/bin/varnishd/cache/cache_fetch.c +++ b/bin/varnishd/cache/cache_fetch.c @@ -409,12 +409,6 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo) http_PrintfHeader(bo->bereq, "X-Varnish: %ju", VXID(bo->vsl->wid)); VCL_backend_fetch_method(bo->vcl, wrk, NULL, bo, NULL); - if (FEATURE(FEATURE_VALIDATE_BACKEND_REQUESTS) && - wrk->vpi->handling == VCL_RET_FETCH && HTTP_ValidateReq(bo->bereq)) { - VSLb(bo->vsl, SLT_VCL_Error, - "Backend request failed HTTP validation"); - wrk->vpi->handling = VCL_RET_FAIL; - } if (wrk->vpi->handling == VCL_RET_ABANDON || wrk->vpi->handling == VCL_RET_FAIL) diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c index 2422b15d5eb..ed97a01107a 100644 --- a/bin/varnishd/cache/cache_req_fsm.c +++ b/bin/varnishd/cache/cache_req_fsm.c @@ -233,14 +233,6 @@ cnt_deliver(struct worker *wrk, struct req *req) req->t_resp = W_TIM_real(wrk); VCL_deliver_method(req->vcl, wrk, req, NULL, NULL); - if (FEATURE(FEATURE_VALIDATE_CLIENT_RESPONSES) && - req->esi_level == 0 && wrk->vpi->handling == VCL_RET_DELIVER && - HTTP_ValidateResp(req->resp)) { - VSLb(req->vsl, SLT_VCL_Error, - "Response failed HTTP validation"); - wrk->vpi->handling = VCL_RET_FAIL; - } - assert(req->restarts <= cache_param->max_restarts); if (wrk->vpi->handling != VCL_RET_DELIVER) { @@ -344,13 +336,6 @@ cnt_synth(struct worker *wrk, struct req *req) AZ(VSB_finish(synth_body)); - if (FEATURE(FEATURE_VALIDATE_CLIENT_RESPONSES) && - req->esi_level == 0 && wrk->vpi->handling == VCL_RET_DELIVER && - HTTP_ValidateResp(req->resp)) { - VSLb(req->vsl, SLT_VCL_Error, - "Synthetic response failed HTTP validation"); - wrk->vpi->handling = VCL_RET_FAIL; - } VSLb_ts_req(req, "Process", W_TIM_real(wrk)); while (wrk->vpi->handling == VCL_RET_FAIL) { diff --git a/bin/varnishd/http1/cache_http1_deliver.c b/bin/varnishd/http1/cache_http1_deliver.c index 15bc706b925..9886573619f 100644 --- a/bin/varnishd/http1/cache_http1_deliver.c +++ b/bin/varnishd/http1/cache_http1_deliver.c @@ -34,6 +34,7 @@ #include "cache/cache_varnishd.h" #include "cache/cache_filter.h" #include "cache_http1.h" +#include "cache/cache_transport.h" #include "vtcp.h" @@ -115,6 +116,14 @@ V1D_Deliver(struct req *req, struct boc *boc, int sendbody) return; } + if (FEATURE(FEATURE_VALIDATE_CLIENT_RESPONSES) && HTTP_ValidateResp(req->resp)) { + VSLb(req->vsl, SLT_VCL_Error, + "Response failed HTTP validation"); + req->doclose = SC_TX_ERROR; + req->transport->minimal_response(req, 503); + return; + } + V1L_Open(req->wrk, req->wrk->aws, &req->sp->fd, req->vsl, req->t_prev + SESS_TMO(req->sp, send_timeout), cache_param->http1_iovs); diff --git a/bin/varnishd/http1/cache_http1_fetch.c b/bin/varnishd/http1/cache_http1_fetch.c index 1a2ced1e89d..bbf3d7d0f7f 100644 --- a/bin/varnishd/http1/cache_http1_fetch.c +++ b/bin/varnishd/http1/cache_http1_fetch.c @@ -97,6 +97,15 @@ V1F_SendReq(struct worker *wrk, struct busyobj *bo, uint64_t *ctr_hdrbytes, } VTCP_blocking(*htc->rfd); /* XXX: we should timeout instead */ + + if (FEATURE(FEATURE_VALIDATE_BACKEND_REQUESTS) && + HTTP_ValidateReq(hp)) { + VSLb(bo->vsl, SLT_VCL_Error, + "Backend request failed HTTP validation"); + htc->doclose = SC_TX_ERROR; + return (-1); + } + /* XXX: need a send_timeout for the backend side */ V1L_Open(wrk, wrk->aws, htc->rfd, bo->vsl, nan(""), 0); hdrbytes = HTTP1_Write(wrk, hp, HTTP1_Req); diff --git a/bin/varnishd/http2/cache_http2_deliver.c b/bin/varnishd/http2/cache_http2_deliver.c index cdd356d7dc6..ff189e7ddca 100644 --- a/bin/varnishd/http2/cache_http2_deliver.c +++ b/bin/varnishd/http2/cache_http2_deliver.c @@ -332,6 +332,13 @@ h2_deliver(struct req *req, struct boc *boc, int sendbody) sendbody = 0; } + if (FEATURE(FEATURE_VALIDATE_CLIENT_RESPONSES) && HTTP_ValidateResp(req->resp)) { + VSLb(req->vsl, SLT_VCL_Error, + "Response failed HTTP validation"); + req->doclose = SC_TX_ERROR; + req->transport->minimal_response(req, 503); + return; + } AZ(req->wrk->v1l); r2->t_send = req->t_prev; diff --git a/bin/varnishtest/tests/c00125.vtc b/bin/varnishtest/tests/c00125.vtc index 888417a0a44..626a6d744b1 100644 --- a/bin/varnishtest/tests/c00125.vtc +++ b/bin/varnishtest/tests/c00125.vtc @@ -110,3 +110,14 @@ client c4 { expect resp.status == 503 # No expect_close as this will be a regular 503 object delivery } -run + +varnish v1 -cliok "param.set feature +http2" + +client c5 { + stream 1 { + txreq -hdr "do_deliver_proto" "true" + rxresp + expect resp.status == 503 + + } -run +} -run