341 lines
12 KiB
Diff
341 lines
12 KiB
Diff
From 34670cfbc56f1c63ec046c38b9ad518010b5c84d Mon Sep 17 00:00:00 2001
|
|
From: James M Snell <jasnell@gmail.com>
|
|
Date: Fri, 17 Apr 2020 16:53:51 -0700
|
|
Subject: [PATCH 1/2] Implement max settings option
|
|
|
|
Upstream-commit: 336a98feb0d56b9ac54e12736b18785c27f75090
|
|
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
|
|
---
|
|
doc/CMakeLists.txt | 1 +
|
|
doc/Makefile.am | 1 +
|
|
lib/includes/nghttp2/nghttp2.h | 23 +++++++++++++
|
|
lib/nghttp2_helper.c | 2 ++
|
|
lib/nghttp2_option.c | 5 +++
|
|
lib/nghttp2_option.h | 5 +++
|
|
lib/nghttp2_session.c | 21 ++++++++++++
|
|
lib/nghttp2_session.h | 2 ++
|
|
tests/main.c | 2 ++
|
|
tests/nghttp2_session_test.c | 61 ++++++++++++++++++++++++++++++++++
|
|
tests/nghttp2_session_test.h | 1 +
|
|
11 files changed, 124 insertions(+)
|
|
|
|
diff --git a/doc/CMakeLists.txt b/doc/CMakeLists.txt
|
|
index 34c0279..f3aec84 100644
|
|
--- a/doc/CMakeLists.txt
|
|
+++ b/doc/CMakeLists.txt
|
|
@@ -42,6 +42,7 @@ set(APIDOCS
|
|
nghttp2_option_set_no_recv_client_magic.rst
|
|
nghttp2_option_set_peer_max_concurrent_streams.rst
|
|
nghttp2_option_set_user_recv_extension_type.rst
|
|
+ nghttp2_option_set_max_settings.rst
|
|
nghttp2_pack_settings_payload.rst
|
|
nghttp2_priority_spec_check_default.rst
|
|
nghttp2_priority_spec_default_init.rst
|
|
diff --git a/doc/Makefile.am b/doc/Makefile.am
|
|
index c17d933..5a58f8e 100644
|
|
--- a/doc/Makefile.am
|
|
+++ b/doc/Makefile.am
|
|
@@ -68,6 +68,7 @@ APIDOCS= \
|
|
nghttp2_option_set_peer_max_concurrent_streams.rst \
|
|
nghttp2_option_set_user_recv_extension_type.rst \
|
|
nghttp2_option_set_max_outbound_ack.rst \
|
|
+ nghttp2_option_set_max_settings.rst \
|
|
nghttp2_pack_settings_payload.rst \
|
|
nghttp2_priority_spec_check_default.rst \
|
|
nghttp2_priority_spec_default_init.rst \
|
|
diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h
|
|
index d79bf48..b0ff6c5 100644
|
|
--- a/lib/includes/nghttp2/nghttp2.h
|
|
+++ b/lib/includes/nghttp2/nghttp2.h
|
|
@@ -222,6 +222,13 @@ typedef struct {
|
|
*/
|
|
#define NGHTTP2_CLIENT_MAGIC_LEN 24
|
|
|
|
+/**
|
|
+ * @macro
|
|
+ *
|
|
+ * The default max number of settings per SETTINGS frame
|
|
+ */
|
|
+#define NGHTTP2_DEFAULT_MAX_SETTINGS 32
|
|
+
|
|
/**
|
|
* @enum
|
|
*
|
|
@@ -392,6 +399,11 @@ typedef enum {
|
|
* receives an other type of frame.
|
|
*/
|
|
NGHTTP2_ERR_SETTINGS_EXPECTED = -536,
|
|
+ /**
|
|
+ * When a local endpoint receives too many settings entries
|
|
+ * in a single SETTINGS frame.
|
|
+ */
|
|
+ NGHTTP2_ERR_TOO_MANY_SETTINGS = -537,
|
|
/**
|
|
* The errors < :enum:`NGHTTP2_ERR_FATAL` mean that the library is
|
|
* under unexpected condition and processing was terminated (e.g.,
|
|
@@ -2648,6 +2660,17 @@ NGHTTP2_EXTERN void nghttp2_option_set_no_closed_streams(nghttp2_option *option,
|
|
NGHTTP2_EXTERN void nghttp2_option_set_max_outbound_ack(nghttp2_option *option,
|
|
size_t val);
|
|
|
|
+/**
|
|
+ * @function
|
|
+ *
|
|
+ * This function sets the maximum number of SETTINGS entries per
|
|
+ * SETTINGS frame that will be accepted. If more than those entries
|
|
+ * are received, the peer is considered to be misbehaving and session
|
|
+ * will be closed. The default value is 32.
|
|
+ */
|
|
+NGHTTP2_EXTERN void nghttp2_option_set_max_settings(nghttp2_option *option,
|
|
+ size_t val);
|
|
+
|
|
/**
|
|
* @function
|
|
*
|
|
diff --git a/lib/nghttp2_helper.c b/lib/nghttp2_helper.c
|
|
index 3b282c7..49bbf07 100644
|
|
--- a/lib/nghttp2_helper.c
|
|
+++ b/lib/nghttp2_helper.c
|
|
@@ -334,6 +334,8 @@ const char *nghttp2_strerror(int error_code) {
|
|
case NGHTTP2_ERR_FLOODED:
|
|
return "Flooding was detected in this HTTP/2 session, and it must be "
|
|
"closed";
|
|
+ case NGHTTP2_ERR_TOO_MANY_SETTINGS:
|
|
+ return "SETTINGS frame contained more than the maximum allowed entries";
|
|
default:
|
|
return "Unknown error code";
|
|
}
|
|
diff --git a/lib/nghttp2_option.c b/lib/nghttp2_option.c
|
|
index e53f22d..34348e6 100644
|
|
--- a/lib/nghttp2_option.c
|
|
+++ b/lib/nghttp2_option.c
|
|
@@ -121,3 +121,8 @@ void nghttp2_option_set_max_outbound_ack(nghttp2_option *option, size_t val) {
|
|
option->opt_set_mask |= NGHTTP2_OPT_MAX_OUTBOUND_ACK;
|
|
option->max_outbound_ack = val;
|
|
}
|
|
+
|
|
+void nghttp2_option_set_max_settings(nghttp2_option *option, size_t val) {
|
|
+ option->opt_set_mask |= NGHTTP2_OPT_MAX_SETTINGS;
|
|
+ option->max_settings = val;
|
|
+}
|
|
diff --git a/lib/nghttp2_option.h b/lib/nghttp2_option.h
|
|
index 1f740aa..939729f 100644
|
|
--- a/lib/nghttp2_option.h
|
|
+++ b/lib/nghttp2_option.h
|
|
@@ -67,6 +67,7 @@ typedef enum {
|
|
NGHTTP2_OPT_MAX_DEFLATE_DYNAMIC_TABLE_SIZE = 1 << 9,
|
|
NGHTTP2_OPT_NO_CLOSED_STREAMS = 1 << 10,
|
|
NGHTTP2_OPT_MAX_OUTBOUND_ACK = 1 << 11,
|
|
+ NGHTTP2_OPT_MAX_SETTINGS = 1 << 12,
|
|
} nghttp2_option_flag;
|
|
|
|
/**
|
|
@@ -85,6 +86,10 @@ struct nghttp2_option {
|
|
* NGHTTP2_OPT_MAX_OUTBOUND_ACK
|
|
*/
|
|
size_t max_outbound_ack;
|
|
+ /**
|
|
+ * NGHTTP2_OPT_MAX_SETTINGS
|
|
+ */
|
|
+ size_t max_settings;
|
|
/**
|
|
* Bitwise OR of nghttp2_option_flag to determine that which fields
|
|
* are specified.
|
|
diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c
|
|
index 670f83f..7638823 100644
|
|
--- a/lib/nghttp2_session.c
|
|
+++ b/lib/nghttp2_session.c
|
|
@@ -458,6 +458,7 @@ static int session_new(nghttp2_session **session_ptr,
|
|
|
|
(*session_ptr)->max_send_header_block_length = NGHTTP2_MAX_HEADERSLEN;
|
|
(*session_ptr)->max_outbound_ack = NGHTTP2_DEFAULT_MAX_OBQ_FLOOD_ITEM;
|
|
+ (*session_ptr)->max_settings = NGHTTP2_DEFAULT_MAX_SETTINGS;
|
|
|
|
if (option) {
|
|
if ((option->opt_set_mask & NGHTTP2_OPT_NO_AUTO_WINDOW_UPDATE) &&
|
|
@@ -521,6 +522,11 @@ static int session_new(nghttp2_session **session_ptr,
|
|
if (option->opt_set_mask & NGHTTP2_OPT_MAX_OUTBOUND_ACK) {
|
|
(*session_ptr)->max_outbound_ack = option->max_outbound_ack;
|
|
}
|
|
+
|
|
+ if ((option->opt_set_mask & NGHTTP2_OPT_MAX_SETTINGS) &&
|
|
+ option->max_settings) {
|
|
+ (*session_ptr)->max_settings = option->max_settings;
|
|
+ }
|
|
}
|
|
|
|
rv = nghttp2_hd_deflate_init2(&(*session_ptr)->hd_deflater,
|
|
@@ -5658,6 +5664,16 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in,
|
|
iframe->max_niv =
|
|
iframe->frame.hd.length / NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH + 1;
|
|
|
|
+ if (iframe->max_niv - 1 > session->max_settings) {
|
|
+ rv = nghttp2_session_terminate_session_with_reason(
|
|
+ session, NGHTTP2_ENHANCE_YOUR_CALM,
|
|
+ "SETTINGS: too many setting entries");
|
|
+ if (nghttp2_is_fatal(rv)) {
|
|
+ return rv;
|
|
+ }
|
|
+ return (ssize_t)inlen;
|
|
+ }
|
|
+
|
|
iframe->iv = nghttp2_mem_malloc(mem, sizeof(nghttp2_settings_entry) *
|
|
iframe->max_niv);
|
|
|
|
@@ -7413,6 +7429,11 @@ static int nghttp2_session_upgrade_internal(nghttp2_session *session,
|
|
if (settings_payloadlen % NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH) {
|
|
return NGHTTP2_ERR_INVALID_ARGUMENT;
|
|
}
|
|
+ /* SETTINGS frame contains too many settings */
|
|
+ if (settings_payloadlen / NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH
|
|
+ > session->max_settings) {
|
|
+ return NGHTTP2_ERR_TOO_MANY_SETTINGS;
|
|
+ }
|
|
rv = nghttp2_frame_unpack_settings_payload2(&iv, &niv, settings_payload,
|
|
settings_payloadlen, mem);
|
|
if (rv != 0) {
|
|
diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h
|
|
index 2969364..e62a3bb 100644
|
|
--- a/lib/nghttp2_session.h
|
|
+++ b/lib/nghttp2_session.h
|
|
@@ -269,6 +269,8 @@ struct nghttp2_session {
|
|
/* The maximum length of header block to send. Calculated by the
|
|
same way as nghttp2_hd_deflate_bound() does. */
|
|
size_t max_send_header_block_length;
|
|
+ /* The maximum number of settings accepted per SETTINGS frame. */
|
|
+ size_t max_settings;
|
|
/* Next Stream ID. Made unsigned int to detect >= (1 << 31). */
|
|
uint32_t next_stream_id;
|
|
/* The last stream ID this session initiated. For client session,
|
|
diff --git a/tests/main.c b/tests/main.c
|
|
index 13865de..1f795cd 100644
|
|
--- a/tests/main.c
|
|
+++ b/tests/main.c
|
|
@@ -313,6 +313,8 @@ int main() {
|
|
test_nghttp2_session_set_local_window_size) ||
|
|
!CU_add_test(pSuite, "session_cancel_from_before_frame_send",
|
|
test_nghttp2_session_cancel_from_before_frame_send) ||
|
|
+ !CU_add_test(pSuite, "session_too_many_settings",
|
|
+ test_nghttp2_session_too_many_settings) ||
|
|
!CU_add_test(pSuite, "session_removed_closed_stream",
|
|
test_nghttp2_session_removed_closed_stream) ||
|
|
!CU_add_test(pSuite, "session_pause_data",
|
|
diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c
|
|
index 0013e92..ab76ab4 100644
|
|
--- a/tests/nghttp2_session_test.c
|
|
+++ b/tests/nghttp2_session_test.c
|
|
@@ -10450,6 +10450,67 @@ void test_nghttp2_session_cancel_from_before_frame_send(void) {
|
|
nghttp2_session_del(session);
|
|
}
|
|
|
|
+void test_nghttp2_session_too_many_settings(void) {
|
|
+ nghttp2_session *session;
|
|
+ nghttp2_option *option;
|
|
+ nghttp2_session_callbacks callbacks;
|
|
+ nghttp2_frame frame;
|
|
+ nghttp2_bufs bufs;
|
|
+ nghttp2_buf *buf;
|
|
+ ssize_t rv;
|
|
+ my_user_data ud;
|
|
+ nghttp2_settings_entry iv[3];
|
|
+ nghttp2_mem *mem;
|
|
+ nghttp2_outbound_item *item;
|
|
+
|
|
+ mem = nghttp2_mem_default();
|
|
+ frame_pack_bufs_init(&bufs);
|
|
+
|
|
+ memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
|
|
+ callbacks.on_frame_recv_callback = on_frame_recv_callback;
|
|
+ callbacks.send_callback = null_send_callback;
|
|
+
|
|
+ nghttp2_option_new(&option);
|
|
+ nghttp2_option_set_max_settings(option, 1);
|
|
+
|
|
+ nghttp2_session_client_new2(&session, &callbacks, &ud, option);
|
|
+
|
|
+ CU_ASSERT(1 == session->max_settings);
|
|
+
|
|
+ nghttp2_option_del(option);
|
|
+
|
|
+ iv[0].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE;
|
|
+ iv[0].value = 3000;
|
|
+
|
|
+ iv[1].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE;
|
|
+ iv[1].value = 16384;
|
|
+
|
|
+ nghttp2_frame_settings_init(&frame.settings, NGHTTP2_FLAG_NONE, dup_iv(iv, 2),
|
|
+ 2);
|
|
+
|
|
+ rv = nghttp2_frame_pack_settings(&bufs, &frame.settings);
|
|
+
|
|
+ CU_ASSERT(0 == rv);
|
|
+ CU_ASSERT(nghttp2_bufs_len(&bufs) > 0);
|
|
+
|
|
+ nghttp2_frame_settings_free(&frame.settings, mem);
|
|
+
|
|
+ buf = &bufs.head->buf;
|
|
+ assert(nghttp2_bufs_len(&bufs) == nghttp2_buf_len(buf));
|
|
+
|
|
+ ud.frame_recv_cb_called = 0;
|
|
+
|
|
+ rv = nghttp2_session_mem_recv(session, buf->pos, nghttp2_buf_len(buf));
|
|
+ CU_ASSERT((ssize_t)nghttp2_buf_len(buf) == rv);
|
|
+
|
|
+ item = nghttp2_session_get_next_ob_item(session);
|
|
+ CU_ASSERT(NGHTTP2_GOAWAY == item->frame.hd.type);
|
|
+
|
|
+ nghttp2_bufs_reset(&bufs);
|
|
+ nghttp2_bufs_free(&bufs);
|
|
+ nghttp2_session_del(session);
|
|
+}
|
|
+
|
|
static void
|
|
prepare_session_removed_closed_stream(nghttp2_session *session,
|
|
nghttp2_hd_deflater *deflater) {
|
|
diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h
|
|
index 35a48b8..c5095c2 100644
|
|
--- a/tests/nghttp2_session_test.h
|
|
+++ b/tests/nghttp2_session_test.h
|
|
@@ -155,6 +155,7 @@ void test_nghttp2_session_repeated_priority_change(void);
|
|
void test_nghttp2_session_repeated_priority_submission(void);
|
|
void test_nghttp2_session_set_local_window_size(void);
|
|
void test_nghttp2_session_cancel_from_before_frame_send(void);
|
|
+void test_nghttp2_session_too_many_settings(void);
|
|
void test_nghttp2_session_removed_closed_stream(void);
|
|
void test_nghttp2_session_pause_data(void);
|
|
void test_nghttp2_session_no_closed_streams(void);
|
|
--
|
|
2.21.3
|
|
|
|
|
|
From da3f4a5730ffa015a9e2d62e6e876a02f1dced20 Mon Sep 17 00:00:00 2001
|
|
From: James M Snell <jasnell@gmail.com>
|
|
Date: Sun, 19 Apr 2020 09:12:24 -0700
|
|
Subject: [PATCH 2/2] Earlier check for settings flood
|
|
|
|
Upstream-commit: f8da73bd042f810f34d19f9eae02b46d870af394
|
|
Signed-off-by: Kamil Dudka <kdudka@redhat.com>
|
|
---
|
|
lib/nghttp2_session.c | 6 ++++++
|
|
1 file changed, 6 insertions(+)
|
|
|
|
diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c
|
|
index 7638823..8271198 100644
|
|
--- a/lib/nghttp2_session.c
|
|
+++ b/lib/nghttp2_session.c
|
|
@@ -5654,6 +5654,12 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in,
|
|
break;
|
|
}
|
|
|
|
+ /* Check the settings flood counter early to be safe */
|
|
+ if (session->obq_flood_counter_ >= session->max_outbound_ack &&
|
|
+ !(iframe->frame.hd.flags & NGHTTP2_FLAG_ACK)) {
|
|
+ return NGHTTP2_ERR_FLOODED;
|
|
+ }
|
|
+
|
|
iframe->state = NGHTTP2_IB_READ_SETTINGS;
|
|
|
|
if (iframe->payloadleft) {
|
|
--
|
|
2.21.3
|
|
|