rpms/varnish
7
0
Fork 0
Code Issues Pull Requests Releases Activity
3f72d7f16e
varnish/varnish-6.6.2-CVE-2023-44487-vcl_vrt.patch
Tomas Korbar 3f72d7f16e Add parameters h2_rst_allowance and h2_rst_allowance_period to mitigate CVE-2023-44487 
Resolves: RHEL-12818
2023-10-20 13:13:04 +02:00

329 lines
8.7 KiB
Diff
Raw Blame History

commit bb44b34d5e9078ede3769ef519badb65d340351a
Author: Tomas Korbar <tkorbar@redhat.com>
Date: Wed Oct 18 12:32:24 2023 +0200
vcl_vrt: Skip VCL execution if the client is gone
Upstream PR #3998
and 4991d9f6e40f381d058a83fc21ceed90e34a822e for r03996.vtc
diff --git a/bin/varnishd/VSC_main.vsc b/bin/varnishd/VSC_main.vsc
index d55b9df..0978c2f 100644
--- a/bin/varnishd/VSC_main.vsc
+++ b/bin/varnishd/VSC_main.vsc
@@ -342,6 +342,15 @@
Number of times an HTTP/2 stream was refused because the queue was
too long already. See also parameter thread_queue_limit.
+.. varnish_vsc:: req_reset
+ :group: wrk
+ :oneliner: Requests reset
+
+ Number of times a client left before the VCL processing of its
+ requests completed. For HTTP/2 sessions, either the stream was
+ reset by an RST_STREAM frame from the client, or a stream or
+ connection error occurred.
+
.. varnish_vsc:: n_object
:type: gauge
:group: wrk
diff --git a/bin/varnishd/cache/cache_transport.h b/bin/varnishd/cache/cache_transport.h
index 3650291..be396b9 100644
--- a/bin/varnishd/cache/cache_transport.h
+++ b/bin/varnishd/cache/cache_transport.h
@@ -44,6 +44,7 @@ typedef void vtr_sess_panic_f (struct vsb *, const struct sess *);
typedef void vtr_req_panic_f (struct vsb *, const struct req *);
typedef void vtr_req_fail_f (struct req *, enum sess_close);
typedef void vtr_reembark_f (struct worker *, struct req *);
+typedef int vtr_poll_f (struct req *);
typedef int vtr_minimal_response_f (struct req *, uint16_t status);
struct transport {
@@ -64,6 +65,7 @@ struct transport {
vtr_sess_panic_f *sess_panic;
vtr_req_panic_f *req_panic;
vtr_reembark_f *reembark;
+ vtr_poll_f *poll;
vtr_minimal_response_f *minimal_response;
VTAILQ_ENTRY(transport) list;
diff --git a/bin/varnishd/cache/cache_vrt_vcl.c b/bin/varnishd/cache/cache_vrt_vcl.c
index 023ba00..2fbaff6 100644
--- a/bin/varnishd/cache/cache_vrt_vcl.c
+++ b/bin/varnishd/cache/cache_vrt_vcl.c
@@ -42,6 +42,7 @@
#include "vbm.h"
#include "cache_director.h"
+#include "cache_transport.h"
#include "cache_vcl.h"
#include "vcc_interface.h"
@@ -437,6 +438,40 @@ VRT_VCL_Allow_Discard(struct vclref **refp)
FREE_OBJ(ref);
}
+/*--------------------------------------------------------------------
+ */
+
+static int
+req_poll(struct worker *wrk, struct req *req)
+{
+ struct req *top;
+
+ /* NB: Since a fail transition leads to vcl_synth, the request may be
+ * short-circuited twice.
+ */
+ if (req->req_reset) {
+ wrk->handling = VCL_RET_FAIL;
+ return (-1);
+ }
+
+ top = req->top->topreq;
+ CHECK_OBJ_NOTNULL(top, REQ_MAGIC);
+ CHECK_OBJ_NOTNULL(top->transport, TRANSPORT_MAGIC);
+
+ if (!FEATURE(FEATURE_VCL_REQ_RESET))
+ return (0);
+ if (top->transport->poll == NULL)
+ return (0);
+ if (top->transport->poll(top) >= 0)
+ return (0);
+
+ VSLb_ts_req(req, "Reset", W_TIM_real(wrk));
+ wrk->stats->req_reset++;
+ wrk->handling = VCL_RET_FAIL;
+ req->req_reset = 1;
+ return (-1);
+}
+
/*--------------------------------------------------------------------
* Method functions to call into VCL programs.
*
@@ -468,6 +503,8 @@ vcl_call_method(struct worker *wrk, struct req *req, struct busyobj *bo,
CHECK_OBJ_NOTNULL(req->sp, SESS_MAGIC);
CHECK_OBJ_NOTNULL(req->vcl, VCL_MAGIC);
CHECK_OBJ_NOTNULL(req->top, REQTOP_MAGIC);
+ if (req_poll(wrk, req))
+ return;
VCL_Req2Ctx(&ctx, req);
}
assert(ctx.now != 0);
diff --git a/bin/varnishd/http2/cache_http2_session.c b/bin/varnishd/http2/cache_http2_session.c
index f81c94a..f978763 100644
--- a/bin/varnishd/http2/cache_http2_session.c
+++ b/bin/varnishd/http2/cache_http2_session.c
@@ -439,6 +439,16 @@ h2_new_session(struct worker *wrk, void *arg)
h2_del_sess(wrk, h2, h2->error->reason);
}
+static int v_matchproto_(vtr_poll_f)
+h2_poll(struct req *req)
+{
+ struct h2_req *r2;
+
+ CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
+ CAST_OBJ_NOTNULL(r2, req->transport_priv, H2_REQ_MAGIC);
+ return (r2->error ? -1 : 1);
+}
+
struct transport H2_transport = {
.name = "H2",
.magic = TRANSPORT_MAGIC,
@@ -448,4 +458,5 @@ struct transport H2_transport = {
.req_body = h2_req_body,
.req_fail = h2_req_fail,
.sess_panic = h2_sess_panic,
+ .poll = h2_poll,
};
diff --git a/bin/varnishd/mgt/mgt_param_bits.c b/bin/varnishd/mgt/mgt_param_bits.c
index d6a9c3f..6d9b32a 100644
--- a/bin/varnishd/mgt/mgt_param_bits.c
+++ b/bin/varnishd/mgt/mgt_param_bits.c
@@ -276,7 +276,7 @@ struct parspec VSL_parspec[] = {
#undef DEBUG_BIT
},
{ "feature", tweak_feature, NULL,
- NULL, NULL, "default",
+ NULL, NULL, "+validate_headers +vcl_req_reset",
NULL,
"Enable/Disable various minor features.\n"
"\tdefault\tSet default value\n"
diff --git a/bin/varnishtest/tests/r03996.vtc b/bin/varnishtest/tests/r03996.vtc
index 3fee370..7faf783 100644
--- a/bin/varnishtest/tests/r03996.vtc
+++ b/bin/varnishtest/tests/r03996.vtc
@@ -1,6 +1,7 @@
varnishtest "h2 rapid reset"
-barrier b1 sock 5
+barrier b1 sock 2 -cyclic
+barrier b2 sock 5 -cyclic
server s1 {
rxreq
@@ -16,7 +17,10 @@ varnish v1 -vcl+backend {
import vtc;
sub vcl_recv {
- vtc.barrier_sync("${b1_sock}");
+ if (req.http.barrier) {
+ vtc.barrier_sync(req.http.barrier);
+ }
+ vtc.barrier_sync("${b2_sock}");
}
} -start
@@ -27,6 +31,41 @@ client c1 {
expect goaway.err == ENHANCE_YOUR_CALM
} -start
+ stream 1 {
+ txreq -hdr barrier ${b1_sock}
+ barrier b1 sync
+ txrst
+ } -run
+ stream 3 {
+ txreq -hdr barrier ${b1_sock}
+ barrier b1 sync
+ txrst
+ } -run
+ stream 5 {
+ txreq -hdr barrier ${b1_sock}
+ barrier b1 sync
+ txrst
+ } -run
+ stream 7 {
+ txreq -hdr barrier ${b1_sock}
+ barrier b1 sync
+ txrst
+ } -run
+
+ barrier b2 sync
+ stream 0 -wait
+} -run
+
+varnish v1 -expect sc_rapid_reset == 1
+
+varnish v1 -cliok "param.set feature -vcl_req_reset"
+
+client c2 {
+ stream 0 {
+ rxgoaway
+ expect goaway.err == ENHANCE_YOUR_CALM
+ } -start
+
stream 1 {
txreq
txrst
@@ -44,8 +83,8 @@ client c1 {
txrst
} -run
- barrier b1 sync
+ barrier b2 sync
stream 0 -wait
} -run
-varnish v1 -expect sc_rapid_reset == 1
+varnish v1 -expect sc_rapid_reset == 2
diff --git a/bin/varnishtest/tests/t02025.vtc b/bin/varnishtest/tests/t02025.vtc
new file mode 100644
index 0000000..3b7e90e
--- /dev/null
+++ b/bin/varnishtest/tests/t02025.vtc
@@ -0,0 +1,49 @@
+varnishtest "h2 reset interrupt"
+
+barrier b1 sock 2
+barrier b2 sock 2
+
+varnish v1 -cliok "param.set feature +http2"
+varnish v1 -cliok "param.set debug +syncvsl"
+varnish v1 -vcl {
+ import vtc;
+
+ backend be none;
+
+ sub vcl_recv {
+ vtc.barrier_sync("${b1_sock}");
+ vtc.barrier_sync("${b2_sock}");
+ }
+
+ sub vcl_miss {
+ vtc.panic("unreachable");
+ }
+} -start
+
+logexpect l1 -v v1 -g raw -i Debug {
+ expect * * Debug "^H2RXF RST_STREAM"
+} -start
+
+client c1 {
+ stream 1 {
+ txreq
+ barrier b1 sync
+ txrst
+ } -run
+} -start
+
+logexpect l1 -wait
+barrier b2 sync
+
+varnish v1 -vsl_catchup
+varnish v1 -expect req_reset == 1
+
+# NB: The varnishncsa command below shows a minimal pattern to collect
+# "rapid reset" suspects per session, with the IP address. Here rapid
+# is interpreted as before a second elapsed. Session VXIDs showing up
+# numerous times become increasingly more suspicious. The format can of
+# course be extended to add anything else useful for data mining.
+shell -expect "1000 ${localhost}" {
+ varnishncsa -n ${v1_name} -d \
+ -q 'Timestamp:Reset[2] < 1.0' -F '%{VSL:Begin[2]}x %h'
+}
diff --git a/doc/sphinx/reference/vsl.rst b/doc/sphinx/reference/vsl.rst
index cf63089..f1ed987 100644
--- a/doc/sphinx/reference/vsl.rst
+++ b/doc/sphinx/reference/vsl.rst
@@ -76,6 +76,11 @@ Resp
Restart
Client request is being restarted.
+Reset
+ The client closed its connection, reset its stream or caused
+ a stream error that forced Varnish to reset the stream. Request
+ processing is interrupted and considered failed.
+
Pipe handling timestamps
~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/include/tbl/feature_bits.h b/include/tbl/feature_bits.h
index d51b22c..3d6ac35 100644
--- a/include/tbl/feature_bits.h
+++ b/include/tbl/feature_bits.h
@@ -82,6 +82,11 @@ FEATURE_BIT(BUSY_STATS_RATE, busy_stats_rate,
"Make busy workers comply with thread_stats_rate."
)
+FEATURE_BIT(VCL_REQ_RESET, vcl_req_reset,
+ "Stop processing client VCL once the client is gone. "
+ "When this happens MAIN.req_reset is incremented."
+)
+
#undef FEATURE_BIT
/*lint -restore */
diff --git a/include/tbl/req_flags.h b/include/tbl/req_flags.h
index 2e82660..9e72312 100644
--- a/include/tbl/req_flags.h
+++ b/include/tbl/req_flags.h
@@ -41,6 +41,7 @@ REQ_FLAG(is_hitpass, 1, 0, "")
REQ_FLAG(waitinglist, 0, 0, "")
REQ_FLAG(want100cont, 0, 0, "")
REQ_FLAG(late100cont, 0, 0, "")
+REQ_FLAG(req_reset, 0, 0, "")
#undef REQ_FLAG
/*lint -restore */
Reference in New Issue View Git Blame Copy Permalink