From 928a81885c64f9019787d2ed1e2c33aa5a925a46 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 23 Aug 2015 21:22:25 +0900 Subject: [PATCH] Limit the number of incoming reserved (remote) streams RFC 7540 does not enforce any limit on the number of incoming reserved streams (in RFC 7540 terms, streams in reserved (remote) state). This only affects client side, since only server can push streams. Malicious server can push arbitrary number of streams, and make client's memory exhausted. The new option, nghttp2_set_max_reserved_remote_streams, can set the maximum number of such incoming streams to avoid possible memory exhaustion. If this option is set, and pushed streams are automatically closed on reception, without calling user provided callback, if they exceed the given limit. The default value is 200. If session is configured as server side, this option has no effect. Server can control the number of streams to push. --- lib/includes/nghttp2/nghttp2.h | 19 +++++++++++ lib/nghttp2_option.c | 6 ++++ lib/nghttp2_option.h | 7 +++- lib/nghttp2_session.c | 27 +++++++++++++--- lib/nghttp2_session.h | 16 +++++++++ tests/nghttp2_session_test.c | 59 +++++++++++++++++++++++++++++++++- 6 files changed, 128 insertions(+), 6 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index d2d07fad..efb6f079 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -2031,6 +2031,25 @@ nghttp2_option_set_no_recv_client_magic(nghttp2_option *option, int val); NGHTTP2_EXTERN void nghttp2_option_set_no_http_messaging(nghttp2_option *option, int val); +/** + * @function + * + * RFC 7540 does not enforce any limit on the number of incoming + * reserved streams (in RFC 7540 terms, streams in reserved (remote) + * state). This only affects client side, since only server can push + * streams. Malicious server can push arbitrary number of streams, + * and make client's memory exhausted. This option can set the + * maximum number of such incoming streams to avoid possible memory + * exhaustion. If this option is set, and pushed streams are + * automatically closed on reception, without calling user provided + * callback, if they exceed the given limit. The default value is + * 200. If session is configured as server side, this option has no + * effect. Server can control the number of streams to push. + */ +NGHTTP2_EXTERN void +nghttp2_option_set_max_reserved_remote_streams(nghttp2_option *option, + uint32_t val); + /** * @function * diff --git a/lib/nghttp2_option.c b/lib/nghttp2_option.c index a45b5e0e..04dbbc6a 100644 --- a/lib/nghttp2_option.c +++ b/lib/nghttp2_option.c @@ -56,3 +56,9 @@ void nghttp2_option_set_no_http_messaging(nghttp2_option *option, int val) { option->opt_set_mask |= NGHTTP2_OPT_NO_HTTP_MESSAGING; option->no_http_messaging = val; } + +void nghttp2_option_set_max_reserved_remote_streams(nghttp2_option *option, + uint32_t val) { + option->opt_set_mask |= NGHTTP2_OPT_MAX_RESERVED_REMOTE_STREAMS; + option->max_reserved_remote_streams = val; +} diff --git a/lib/nghttp2_option.h b/lib/nghttp2_option.h index 739bd858..47acf619 100644 --- a/lib/nghttp2_option.h +++ b/lib/nghttp2_option.h @@ -58,7 +58,8 @@ typedef enum { */ NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS = 1 << 1, NGHTTP2_OPT_NO_RECV_CLIENT_MAGIC = 1 << 2, - NGHTTP2_OPT_NO_HTTP_MESSAGING = 1 << 3 + NGHTTP2_OPT_NO_HTTP_MESSAGING = 1 << 3, + NGHTTP2_OPT_MAX_RESERVED_REMOTE_STREAMS = 1 << 4 } nghttp2_option_flag; /** @@ -74,6 +75,10 @@ struct nghttp2_option { * NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS */ uint32_t peer_max_concurrent_streams; + /** + * NGHTTP2_OPT_MAX_RESERVED_REMOTE_STREAMS + */ + uint32_t max_reserved_remote_streams; /** * NGHTTP2_OPT_NO_AUTO_WINDOW_UPDATE */ diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 8254187a..4935a332 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -372,6 +372,9 @@ static int session_new(nghttp2_session **session_ptr, init_settings(&(*session_ptr)->remote_settings); init_settings(&(*session_ptr)->local_settings); + (*session_ptr)->max_incoming_reserved_streams = + NGHTTP2_MAX_INCOMING_RESERVED_STREAMS; + if (option) { if ((option->opt_set_mask & NGHTTP2_OPT_NO_AUTO_WINDOW_UPDATE) && option->no_auto_window_update) { @@ -385,6 +388,12 @@ static int session_new(nghttp2_session **session_ptr, option->peer_max_concurrent_streams; } + if (option->opt_set_mask & NGHTTP2_OPT_MAX_RESERVED_REMOTE_STREAMS) { + + (*session_ptr)->max_incoming_reserved_streams = + option->max_reserved_remote_streams; + } + if ((option->opt_set_mask & NGHTTP2_OPT_NO_RECV_CLIENT_MAGIC) && option->no_recv_client_magic) { @@ -910,11 +919,12 @@ nghttp2_stream *nghttp2_session_open_stream(nghttp2_session *session, switch (initial_state) { case NGHTTP2_STREAM_RESERVED: if (nghttp2_session_is_my_stream_id(session, stream_id)) { - /* half closed (remote) */ + /* reserved (local) */ nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_RD); } else { - /* half closed (local) */ + /* reserved (remote) */ nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_WR); + ++session->num_incoming_reserved_streams; } /* Reserved stream does not count in the concurrent streams limit. That is one of the DOS vector. */ @@ -1017,7 +1027,11 @@ int nghttp2_session_close_stream(nghttp2_session *session, int32_t stream_id, /* pushed streams which is not opened yet is not counted toward max concurrent limits */ - if ((stream->flags & NGHTTP2_STREAM_FLAG_PUSH) == 0) { + if ((stream->flags & NGHTTP2_STREAM_FLAG_PUSH)) { + if (!nghttp2_session_is_my_stream_id(session, stream_id)) { + --session->num_incoming_reserved_streams; + } + } else { if (nghttp2_session_is_my_stream_id(session, stream_id)) { --session->num_outgoing_streams; } else { @@ -3458,6 +3472,9 @@ int nghttp2_session_on_push_response_headers_received(nghttp2_session *session, } nghttp2_stream_promise_fulfilled(stream); + if (!nghttp2_session_is_my_stream_id(session, stream->stream_id)) { + --session->num_incoming_reserved_streams; + } ++session->num_incoming_streams; rv = session_call_on_begin_headers(session, frame); if (rv != 0) { @@ -4057,7 +4074,9 @@ int nghttp2_session_on_push_promise_received(nghttp2_session *session, session->last_recv_stream_id = frame->push_promise.promised_stream_id; stream = nghttp2_session_get_stream(session, frame->hd.stream_id); if (!stream || stream->state == NGHTTP2_STREAM_CLOSING || - !session->pending_enable_push) { + !session->pending_enable_push || + session->num_incoming_reserved_streams >= + session->max_incoming_reserved_streams) { if (!stream) { if (session_detect_idle_stream(session, frame->hd.stream_id)) { return session_inflate_handle_invalid_connection( diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h index 27e6aca7..8071faec 100644 --- a/lib/nghttp2_session.h +++ b/lib/nghttp2_session.h @@ -66,6 +66,9 @@ typedef struct { nghttp2_session_recv(). */ #define NGHTTP2_INBOUND_BUFFER_LENGTH 16384 +/* The default maximum number of incoming reserved streams */ +#define NGHTTP2_MAX_INCOMING_RESERVED_STREAMS 200 + /* Internal state when receiving incoming frame */ typedef enum { /* Receiving frame header */ @@ -194,6 +197,19 @@ struct nghttp2_session { /* The number of incoming streams. This will be capped by local_settings.max_concurrent_streams. */ size_t num_incoming_streams; + /* The number of incoming reserved streams. This is the number of + streams in reserved (remote) state. RFC 7540 does not limit this + number. nghttp2 offers + nghttp2_option_set_max_reserved_remote_streams() to achieve this. + If it is used, num_incoming_streams is capped by + max_incoming_reserved_streams. Client application should + consider to set this because without that server can send + arbitrary number of PUSH_PROMISE, and exhaust client's memory. */ + size_t num_incoming_reserved_streams; + /* The maximum number of incoming reserved streams (reserved + (remote) state). RST_STREAM will be sent for the pushed stream + which exceeds this limit. */ + size_t max_incoming_reserved_streams; /* The number of closed streams still kept in |streams| hash. The closed streams can be accessed through single linked list |closed_stream_head|. The current implementation only keeps diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 9cc0114a..f96c2bed 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -2065,9 +2065,11 @@ void test_nghttp2_session_on_push_response_headers_received(void) { user_data.begin_headers_cb_called = 0; user_data.invalid_frame_recv_cb_called = 0; + CU_ASSERT(1 == session->num_incoming_reserved_streams); CU_ASSERT(0 == nghttp2_session_on_push_response_headers_received( session, &frame, stream)); CU_ASSERT(1 == user_data.begin_headers_cb_called); + CU_ASSERT(0 == session->num_incoming_reserved_streams); CU_ASSERT(NGHTTP2_STREAM_OPENED == stream->state); CU_ASSERT(1 == session->num_incoming_streams); CU_ASSERT(0 == (stream->flags & NGHTTP2_STREAM_FLAG_PUSH)); @@ -2086,6 +2088,7 @@ void test_nghttp2_session_on_push_response_headers_received(void) { CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type); CU_ASSERT(NGHTTP2_REFUSED_STREAM == item->frame.rst_stream.error_code); CU_ASSERT(1 == session->num_incoming_streams); + CU_ASSERT(1 == session->num_incoming_reserved_streams); CU_ASSERT(0 == nghttp2_session_send(session)); CU_ASSERT(1 == session->num_incoming_streams); @@ -2106,6 +2109,7 @@ void test_nghttp2_session_on_push_response_headers_received(void) { CU_ASSERT(NGHTTP2_GOAWAY == item->frame.hd.type); CU_ASSERT(NGHTTP2_PROTOCOL_ERROR == item->frame.goaway.error_code); CU_ASSERT(1 == session->num_incoming_streams); + CU_ASSERT(1 == session->num_incoming_reserved_streams); nghttp2_frame_headers_free(&frame.headers, mem); nghttp2_session_del(session); @@ -2401,6 +2405,7 @@ void test_nghttp2_session_on_push_promise_received(void) { CU_ASSERT(0 == nghttp2_session_on_push_promise_received(session, &frame)); CU_ASSERT(1 == user_data.begin_headers_cb_called); + CU_ASSERT(1 == session->num_incoming_reserved_streams); promised_stream = nghttp2_session_get_stream(session, 2); CU_ASSERT(NGHTTP2_STREAM_RESERVED == promised_stream->state); CU_ASSERT(2 == session->last_recv_stream_id); @@ -2416,6 +2421,7 @@ void test_nghttp2_session_on_push_promise_received(void) { CU_ASSERT(0 == user_data.begin_headers_cb_called); CU_ASSERT(1 == user_data.invalid_frame_recv_cb_called); + CU_ASSERT(1 == session->num_incoming_reserved_streams); CU_ASSERT(NULL == nghttp2_session_get_stream(session, 4)); item = nghttp2_session_get_next_ob_item(session); CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type); @@ -2435,6 +2441,7 @@ void test_nghttp2_session_on_push_promise_received(void) { nghttp2_session_on_push_promise_received(session, &frame)); CU_ASSERT(0 == user_data.begin_headers_cb_called); + CU_ASSERT(1 == session->num_incoming_reserved_streams); CU_ASSERT(NULL == nghttp2_session_get_stream(session, 6)); item = nghttp2_session_get_next_ob_item(session); CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type); @@ -2452,6 +2459,7 @@ void test_nghttp2_session_on_push_promise_received(void) { nghttp2_session_on_push_promise_received(session, &frame)); CU_ASSERT(0 == user_data.begin_headers_cb_called); + CU_ASSERT(1 == session->num_incoming_reserved_streams); CU_ASSERT(NULL == nghttp2_session_get_stream(session, 8)); item = nghttp2_session_get_next_ob_item(session); CU_ASSERT(NGHTTP2_GOAWAY == item->frame.hd.type); @@ -2468,7 +2476,16 @@ void test_nghttp2_session_on_push_promise_received(void) { NGHTTP2_STREAM_OPENING, NULL); /* Same ID twice */ - stream->state = NGHTTP2_STREAM_OPENING; + frame.hd.stream_id = 1; + frame.push_promise.promised_stream_id = 2; + + user_data.begin_headers_cb_called = 0; + user_data.invalid_frame_recv_cb_called = 0; + CU_ASSERT(0 == nghttp2_session_on_push_promise_received(session, &frame)); + + CU_ASSERT(1 == user_data.begin_headers_cb_called); + CU_ASSERT(1 == session->num_incoming_reserved_streams); + CU_ASSERT(NULL != nghttp2_session_get_stream(session, 2)); user_data.begin_headers_cb_called = 0; user_data.invalid_frame_recv_cb_called = 0; @@ -2476,6 +2493,7 @@ void test_nghttp2_session_on_push_promise_received(void) { nghttp2_session_on_push_promise_received(session, &frame)); CU_ASSERT(0 == user_data.begin_headers_cb_called); + CU_ASSERT(1 == session->num_incoming_reserved_streams); CU_ASSERT(NULL == nghttp2_session_get_stream(session, 8)); item = nghttp2_session_get_next_ob_item(session); CU_ASSERT(NGHTTP2_GOAWAY == item->frame.hd.type); @@ -2491,6 +2509,7 @@ void test_nghttp2_session_on_push_promise_received(void) { nghttp2_session_on_push_promise_received(session, &frame)); CU_ASSERT(0 == user_data.begin_headers_cb_called); + CU_ASSERT(1 == session->num_incoming_reserved_streams); CU_ASSERT(NULL == nghttp2_session_get_stream(session, 10)); CU_ASSERT(NULL == nghttp2_session_get_next_ob_item(session)); @@ -2512,6 +2531,7 @@ void test_nghttp2_session_on_push_promise_received(void) { CU_ASSERT(0 == user_data.begin_headers_cb_called); CU_ASSERT(1 == user_data.invalid_frame_recv_cb_called); + CU_ASSERT(1 == session->num_incoming_reserved_streams); nghttp2_frame_push_promise_free(&frame.push_promise, mem); nghttp2_session_del(session); @@ -2534,6 +2554,7 @@ void test_nghttp2_session_on_push_promise_received(void) { CU_ASSERT(0 == user_data.begin_headers_cb_called); CU_ASSERT(1 == user_data.invalid_frame_recv_cb_called); + CU_ASSERT(0 == session->num_incoming_reserved_streams); nghttp2_frame_push_promise_free(&frame.push_promise, mem); nghttp2_session_del(session); @@ -2572,6 +2593,35 @@ void test_nghttp2_session_on_push_promise_received(void) { CU_ASSERT(NGHTTP2_ERR_IGN_HEADER_BLOCK == nghttp2_session_on_push_promise_received(session, &frame)); + CU_ASSERT(0 == session->num_incoming_reserved_streams); + + nghttp2_frame_push_promise_free(&frame.push_promise, mem); + nghttp2_session_del(session); + + /* Check max_incoming_reserved_streams */ + nghttp2_session_client_new(&session, &callbacks, &user_data); + session->max_incoming_reserved_streams = 1; + + nghttp2_session_open_stream(session, 1, NGHTTP2_STREAM_FLAG_NONE, + &pri_spec_default, NGHTTP2_STREAM_OPENING, NULL); + nghttp2_session_open_stream(session, 2, NGHTTP2_STREAM_FLAG_NONE, + &pri_spec_default, NGHTTP2_STREAM_RESERVED, NULL); + + CU_ASSERT(1 == session->num_incoming_reserved_streams); + + nghttp2_frame_push_promise_init(&frame.push_promise, NGHTTP2_FLAG_END_HEADERS, + 1, 4, NULL, 0); + + CU_ASSERT(NGHTTP2_ERR_IGN_HEADER_BLOCK == + nghttp2_session_on_push_promise_received(session, &frame)); + + CU_ASSERT(1 == session->num_incoming_reserved_streams); + + item = nghttp2_session_get_next_ob_item(session); + + CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type); + CU_ASSERT(NGHTTP2_REFUSED_STREAM == item->frame.rst_stream.error_code); + nghttp2_frame_push_promise_free(&frame.push_promise, mem); nghttp2_session_del(session); } @@ -5398,6 +5448,13 @@ void test_nghttp2_session_set_option(void) { CU_ASSERT(100 == session->remote_settings.max_concurrent_streams); nghttp2_session_del(session); + nghttp2_option_set_max_reserved_remote_streams(option, 99); + + nghttp2_session_client_new2(&session, &callbacks, NULL, option); + + CU_ASSERT(99 == session->max_incoming_reserved_streams); + nghttp2_session_del(session); + nghttp2_option_del(option); }