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