[master] e082a30c8 validate_headers feature
Nils Goroll
nils.goroll at uplex.de
Fri Oct 9 13:30:07 UTC 2020
commit e082a30c83657004727b94df2444b366d550a888
Author: Nils Goroll <nils.goroll at uplex.de>
Date: Fri Oct 9 13:45:41 2020 +0200
validate_headers feature
We now validate all header set operations to conform with the allowed
characters by RFC7230:
* HTAB 0x09
* VCHAR 0x20 to 0x7e
* obs-text 0x80 to 0xff
Ref https://httpwg.org/specs/rfc7230.html#header.fields
See #3407
diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index 68597ad84..25fb07fd8 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -633,19 +633,23 @@ VRT_SetHdr(VRT_CTX , VCL_HEADER hs, const char *p, ...)
AN(hs->what);
hp = VRT_selecthttp(ctx, hs->where);
CHECK_OBJ_NOTNULL(hp, HTTP_MAGIC);
- va_start(ap, p);
if (p == vrt_magic_string_unset) {
http_Unset(hp, hs->what);
} else {
+ va_start(ap, p);
b = VRT_String(hp->ws, hs->what + 1, p, ap);
+ va_end(ap);
if (b == NULL) {
VSLb(ctx->vsl, SLT_LostHeader, "%s", hs->what + 1);
- } else {
- http_Unset(hp, hs->what);
- http_SetHeader(hp, b);
+ return;
+ }
+ if (FEATURE(FEATURE_VALIDATE_HEADERS) && ! validhdr(b)) {
+ VRT_fail(ctx, "Bad header %s", b);
+ return;
}
+ http_Unset(hp, hs->what);
+ http_SetHeader(hp, b);
}
- va_end(ap);
}
/*--------------------------------------------------------------------*/
diff --git a/bin/varnishd/mgt/mgt_param_bits.c b/bin/varnishd/mgt/mgt_param_bits.c
index cc0812624..6a113e72e 100644
--- a/bin/varnishd/mgt/mgt_param_bits.c
+++ b/bin/varnishd/mgt/mgt_param_bits.c
@@ -222,7 +222,12 @@ tweak_feature(struct vsb *vsb, const struct parspec *par, const char *arg)
(void)par;
if (arg != NULL && arg != JSON_FMT) {
- if (!strcmp(arg, "none")) {
+ if (!strcmp(arg, "default")) {
+ memset(mgt_param.feature_bits,
+ 0, sizeof mgt_param.feature_bits);
+ (void)bit(mgt_param.feature_bits,
+ FEATURE_VALIDATE_HEADERS, BSET);
+ } else if (!strcmp(arg, "none")) {
memset(mgt_param.feature_bits,
0, sizeof mgt_param.feature_bits);
} else {
@@ -271,9 +276,10 @@ struct parspec VSL_parspec[] = {
#undef DEBUG_BIT
},
{ "feature", tweak_feature, NULL,
- NULL, NULL, "none",
+ NULL, NULL, "default",
NULL,
"Enable/Disable various minor features.\n"
+ "\tdefault\tSet default value\n"
"\tnone\tDisable all features.\n\n"
"Use +/- prefix to enable/disable individual feature:"
#define FEATURE_BIT(U, l, d) "\n\t" #l "\t" d
diff --git a/bin/varnishtest/tests/b00040.vtc b/bin/varnishtest/tests/b00040.vtc
index cc7479d05..5e396b386 100644
--- a/bin/varnishtest/tests/b00040.vtc
+++ b/bin/varnishtest/tests/b00040.vtc
@@ -1,12 +1,22 @@
-varnishtest "test certain mailformed requests"
+varnishtest "test certain malformed requests and validate_headers"
server s1 {
rxreq
expect req.url == /4
txresp
+ rxreq
+ expect req.url == /9
+ txresp
} -start
-varnish v1 -vcl+backend { } -start
+varnish v1 -vcl+backend {
+ sub vcl_recv {
+ if (req.url == "/9") {
+ set req.http.foo = {"
+ "};
+ }
+ }
+} -start
logexpect l1 -v v1 -g raw {
expect * 1001 BogoHeader {1st header has white space:.*}
@@ -16,6 +26,7 @@ logexpect l1 -v v1 -g raw {
expect * 1012 BogoHeader {Header has ctrl char 0x0d}
expect * 1014 BogoHeader {Header has ctrl char 0x0d}
expect * 1016 BogoHeader {Missing header name:.*}
+ expect * 1018 VCL_Error {Bad header foo:}
} -start
client c1 {
@@ -80,5 +91,18 @@ client c1 {
expect resp.status == 400
} -run
+client c1 {
+ txreq -url /9
+ rxresp
+ expect resp.status == 503
+} -run
+
logexpect l1 -wait
+varnish v1 -cliok "param.set feature -validate_headers"
+
+client c1 {
+ txreq -url /9
+ rxresp
+ expect resp.status == 200
+} -run
diff --git a/doc/changes.rst b/doc/changes.rst
index 5a81f289e..b2c192e43 100644
--- a/doc/changes.rst
+++ b/doc/changes.rst
@@ -33,6 +33,10 @@ Varnish Cache Next (2021-03-15)
* counters MAIN.s_req_bodybytes and VBE.*.tools.beresp_bodybytes
are now always the number of bodybytes moved on the wire.
+* Unless the new ``validate_headers`` feature is disabled, all newly
+ set headers are now validated to contain only characters allowed by
+ RFC7230. A (runtime) VCL failure is triggered if not.
+
================================
Varnish Cache 6.5.1 (2020-09-25)
================================
diff --git a/include/tbl/feature_bits.h b/include/tbl/feature_bits.h
index 2f314cfb2..207e18e11 100644
--- a/include/tbl/feature_bits.h
+++ b/include/tbl/feature_bits.h
@@ -74,6 +74,10 @@ FEATURE_BIT(WAIT_SILO, wait_silo,
"Wait for persistent silos to completely load before serving requests."
)
+FEATURE_BIT(VALIDATE_HEADERS, validate_headers,
+ "Validate all header set operations to conform to RFC7230."
+)
+
#undef FEATURE_BIT
/*lint -restore */
More information about the varnish-commit
mailing list