From 5ebeab6e9a6e0f46cca662ee7e4616228fbcfd13 Mon Sep 17 00:00:00 2001 From: Dag Haavi Finstad Date: Thu, 12 Oct 2023 10:50:55 +0200 Subject: [PATCH] h2: Add a rate limit facility for h/2 RST handling This adds parameters h2_rst_allowance and h2_rst_allowance_period, which govern the rate of which we allow clients to reset h/2 streams. If the limit is exceeded the connection is closed. Mitigates: #3996 --- bin/varnishd/http2/cache_http2.h | 2 ++ bin/varnishd/http2/cache_http2_proto.c | 22 +++++++++++++ bin/varnishd/http2/cache_http2_session.c | 3 ++ bin/varnishtest/tests/r03996.vtc | 41 ++++++++++++++++++++++++ include/tbl/params.h | 28 ++++++++++++++++ 5 files changed, 96 insertions(+) create mode 100644 bin/varnishtest/tests/r03996.vtc diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h index eb6e8f6a9f3..0bb485fde13 100644 --- a/bin/varnishd/http2/cache_http2.h +++ b/bin/varnishd/http2/cache_http2.h @@ -193,6 +193,8 @@ struct h2_sess { VTAILQ_HEAD(,h2_req) txqueue; h2_error error; + double rst_allowance; + vtim_real last_rst; }; #define ASSERT_RXTHR(h2) do {assert(h2->rxthr == pthread_self());} while(0) diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c index c62a8c429fa..9daac4cd562 100644 --- a/bin/varnishd/http2/cache_http2_proto.c +++ b/bin/varnishd/http2/cache_http2_proto.c @@ -323,6 +323,8 @@ h2_rx_push_promise(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2) static h2_error v_matchproto_(h2_rxframe_f) h2_rx_rst_stream(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2) { + vtim_real now; + vtim_dur d; CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); ASSERT_RXTHR(h2); @@ -333,6 +335,26 @@ h2_rx_rst_stream(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2) if (r2 == NULL) return (0); h2_kill_req(wrk, h2, r2, h2_streamerror(vbe32dec(h2->rxf_data))); + + if (cache_param->h2_rst_allowance == 0) + return (0); + + now = VTIM_real(); + d = now - h2->last_rst; + h2->rst_allowance += (1.0 * d / cache_param->h2_rst_allowance_period) * + cache_param->h2_rst_allowance; + h2->rst_allowance = vmin(h2->rst_allowance, + (double)cache_param->h2_rst_allowance); + h2->last_rst = now; + + if (h2->rst_allowance < 1.0) { + Lck_Lock(&h2->sess->mtx); + VSLb(h2->vsl, SLT_Error, "H2: Hit RST limit. Closing session."); + Lck_Unlock(&h2->sess->mtx); + return (H2CE_ENHANCE_YOUR_CALM); + } + h2->rst_allowance -= 1.0; + return (0); } diff --git a/bin/varnishd/http2/cache_http2_session.c b/bin/varnishd/http2/cache_http2_session.c index e3e3c116c8b..6fa9e69b5df 100644 --- a/bin/varnishd/http2/cache_http2_session.c +++ b/bin/varnishd/http2/cache_http2_session.c @@ -127,6 +127,9 @@ h2_init_sess(struct sess *sp, h2_local_settings(&h2->local_settings); h2->remote_settings = H2_proto_settings; h2->decode = decode; + h2->rst_allowance = cache_param->h2_rst_allowance; + h2->last_rst = sp->t_open; + AZ(isnan(h2->last_rst)); AZ(VHT_Init(h2->dectbl, h2->local_settings.header_table_size)); diff --git a/bin/varnishtest/tests/r03996.vtc b/bin/varnishtest/tests/r03996.vtc new file mode 100644 index 00000000000..7e09f02b85e --- /dev/null +++ b/bin/varnishtest/tests/r03996.vtc @@ -0,0 +1,41 @@ +varnishtest "h2: Test h2_rst_allowance" + +server s1 { + rxreq + txresp +} -start + +varnish v1 -cliok "param.set feature +http2" +varnish v1 -cliok "param.set h2_rst_allowance 3" + +varnish v1 -vcl+backend {} -start + +client c1 { + stream 0 { + rxgoaway + expect goaway.err == ENHANCE_YOUR_CALM + } -start + + stream 1 { + txreq + txrst + } -start + delay 0.1 + stream 3 { + txreq + txrst + } -start + delay 0.1 + stream 5 { + txreq + txrst + } -start + delay 0.1 + stream 7 { + txreq + txrst + } -start + + stream 0 -wait +} -run + diff --git a/include/tbl/params.h b/include/tbl/params.h index 83054eef3de..23224e47d14 100644 --- a/include/tbl/params.h +++ b/include/tbl/params.h @@ -1257,6 +1257,34 @@ PARAM_SIMPLE( "HTTP2 maximum size of an uncompressed header list." ) +PARAM_SIMPLE( + /* name */ h2_rst_allowance, + /* typ */ uint, + /* min */ "0", + /* max */ NULL, + /* def */ "0", + /* units */ NULL, + /* descr */ + "HTTP2 RST Allowance.\n" + "Specifies the maximum number of allowed stream resets issued by\n" + "a client over a time period before the connection is closed.\n" + "Setting this parameter to 0 disables the limit.", + /* flags */ EXPERIMENTAL, +) + +PARAM_SIMPLE( + /* name */ h2_rst_allowance_period, + /* typ */ timeout, + /* min */ "1.000", + /* max */ NULL, + /* def */ "60.000", + /* units */ "seconds", + /* descr */ + "HTTP2 RST allowance time period.", + /* flags */ EXPERIMENTAL|WIZARD, +) + + /*-------------------------------------------------------------------- * Memory pool parameters */