http2: implement support for max settings entries

jasnell · targos · commit 3948830ce640 · 2020-06-02T20:35:51.000+02:00
Adds the maxSettings option to limit the number of settings entries allowed per SETTINGS frame. Default 32 Fixes: https://hackerone.com/reports/446662 CVE-ID: CVE-2020-11080 PR-URL: nodejs-private/node-private#204 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
diff --git a/doc/api/http2.md b/doc/api/http2.md
@@ -2001,6 +2001,9 @@ value only affects new connections to the server, not any existing connections.
 <!-- YAML
 added: v8.4.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs-private/node-private/pull/204
+    description: Added `maxSettings` option with a default of 32.
   - version:
      - v13.3.0
      - v12.16.0
@@ -2037,6 +2040,8 @@ changes:
 * `options` {Object}
   * `maxDeflateDynamicTableSize` {number} Sets the maximum dynamic table size
     for deflating header fields. **Default:** `4Kib`.
+  * `maxSettings` {number} Sets the maximum number of settings entries per
+    `SETTINGS` frame. The minimum value allowed is `1`. **Default:** `32`.
   * `maxSessionMemory`{number} Sets the maximum memory that the `Http2Session`
     is permitted to use. The value is expressed in terms of number of megabytes,
     e.g. `1` equal 1 megabyte. The minimum value allowed is `1`.
@@ -2132,6 +2137,9 @@ server.listen(80);
 <!-- YAML
 added: v8.4.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs-private/node-private/pull/204
+    description: Added `maxSettings` option with a default of 32.
   - version:
      - v13.3.0
      - v12.16.0
@@ -2168,6 +2176,8 @@ changes:
     **Default:** `false`.
   * `maxDeflateDynamicTableSize` {number} Sets the maximum dynamic table size
     for deflating header fields. **Default:** `4Kib`.
+  * `maxSettings` {number} Sets the maximum number of settings entries per
+    `SETTINGS` frame. The minimum value allowed is `1`. **Default:** `32`.
   * `maxSessionMemory`{number} Sets the maximum memory that the `Http2Session`
     is permitted to use. The value is expressed in terms of number of megabytes,
     e.g. `1` equal 1 megabyte. The minimum value allowed is `1`. This is a
@@ -2250,6 +2260,9 @@ server.listen(80);
 <!-- YAML
 added: v8.4.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs-private/node-private/pull/204
+    description: Added `maxSettings` option with a default of 32.
   - version: v13.0.0
     pr-url: https://github.com/nodejs/node/pull/29144
     description: The `PADDING_STRATEGY_CALLBACK` has been made equivalent to
@@ -2273,6 +2286,8 @@ changes:
 * `options` {Object}
   * `maxDeflateDynamicTableSize` {number} Sets the maximum dynamic table size
     for deflating header fields. **Default:** `4Kib`.
+  * `maxSettings` {number} Sets the maximum number of settings entries per
+    `SETTINGS` frame. The minimum value allowed is `1`. **Default:** `32`.
   * `maxSessionMemory`{number} Sets the maximum memory that the `Http2Session`
     is permitted to use. The value is expressed in terms of number of megabytes,
     e.g. `1` equal 1 megabyte. The minimum value allowed is `1`.
diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js
@@ -203,7 +203,8 @@ const IDX_OPTIONS_MAX_HEADER_LIST_PAIRS = 5;
 const IDX_OPTIONS_MAX_OUTSTANDING_PINGS = 6;
 const IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS = 7;
 const IDX_OPTIONS_MAX_SESSION_MEMORY = 8;
-const IDX_OPTIONS_FLAGS = 9;
+const IDX_OPTIONS_MAX_SETTINGS = 9;
+const IDX_OPTIONS_FLAGS = 10;
 
 function updateOptionsBuffer(options) {
   let flags = 0;
@@ -252,6 +253,11 @@ function updateOptionsBuffer(options) {
     optionsBuffer[IDX_OPTIONS_MAX_SESSION_MEMORY] =
       MathMax(1, options.maxSessionMemory);
   }
+  if (typeof options.maxSettings === 'number') {
+    flags |= (1 << IDX_OPTIONS_MAX_SETTINGS);
+    optionsBuffer[IDX_OPTIONS_MAX_SETTINGS] =
+      MathMax(1, options.maxSettings);
+  }
   optionsBuffer[IDX_OPTIONS_FLAGS] = flags;
 }
 
diff --git a/src/node_http2.cc b/src/node_http2.cc
@@ -193,6 +193,12 @@ Http2Options::Http2Options(Http2State* http2_state, SessionType type) {
   //            terms of MB increments (i.e. the value 1 == 1 MB)
   if (flags & (1 << IDX_OPTIONS_MAX_SESSION_MEMORY))
     set_max_session_memory(buffer[IDX_OPTIONS_MAX_SESSION_MEMORY] * 1000000);
+
+  if (flags & (1 << IDX_OPTIONS_MAX_SETTINGS)) {
+    nghttp2_option_set_max_settings(
+        option,
+        static_cast<size_t>(buffer[IDX_OPTIONS_MAX_SETTINGS]));
+  }
 }
 
 #define GRABSETTING(entries, count, name)                                      \
diff --git a/src/node_http2_state.h b/src/node_http2_state.h
@@ -54,6 +54,7 @@ namespace http2 {
     IDX_OPTIONS_MAX_OUTSTANDING_PINGS,
     IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS,
     IDX_OPTIONS_MAX_SESSION_MEMORY,
+    IDX_OPTIONS_MAX_SETTINGS,
     IDX_OPTIONS_FLAGS
   };
 
diff --git a/test/parallel/test-http2-max-settings.js b/test/parallel/test-http2-max-settings.js
@@ -0,0 +1,35 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const http2 = require('http2');
+
+const server = http2.createServer({ maxSettings: 1 });
+
+// TODO(@jasnell): There is still a session event
+// emitted on the server side but it will be destroyed
+// immediately after creation and there will be no
+// stream created.
+server.on('session', common.mustCall((session) => {
+  session.on('stream', common.mustNotCall());
+  session.on('remoteSettings', common.mustNotCall());
+}));
+server.on('stream', common.mustNotCall());
+
+server.listen(0, common.mustCall(() => {
+  // Specify two settings entries when a max of 1 is allowed.
+  // Connection should error immediately.
+  const client = http2.connect(
+    `http://localhost:${server.address().port}`, {
+      settings: {
+        // The actual settings values do not matter.
+        headerTableSize: 1000,
+        enablePush: false,
+      } });
+
+  client.on('error', common.mustCall(() => {
+    server.close();
+  }));
+}));
diff --git a/test/parallel/test-http2-util-update-options-buffer.js b/test/parallel/test-http2-util-update-options-buffer.js
@@ -22,7 +22,8 @@ const IDX_OPTIONS_MAX_HEADER_LIST_PAIRS = 5;
 const IDX_OPTIONS_MAX_OUTSTANDING_PINGS = 6;
 const IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS = 7;
 const IDX_OPTIONS_MAX_SESSION_MEMORY = 8;
-const IDX_OPTIONS_FLAGS = 9;
+const IDX_OPTIONS_MAX_SETTINGS = 9;
+const IDX_OPTIONS_FLAGS = 10;
 
 {
   updateOptionsBuffer({
@@ -34,7 +35,8 @@ const IDX_OPTIONS_FLAGS = 9;
     maxHeaderListPairs: 6,
     maxOutstandingPings: 7,
     maxOutstandingSettings: 8,
-    maxSessionMemory: 9
+    maxSessionMemory: 9,
+    maxSettings: 10,
   });
 
   strictEqual(optionsBuffer[IDX_OPTIONS_MAX_DEFLATE_DYNAMIC_TABLE_SIZE], 1);
@@ -46,6 +48,7 @@ const IDX_OPTIONS_FLAGS = 9;
   strictEqual(optionsBuffer[IDX_OPTIONS_MAX_OUTSTANDING_PINGS], 7);
   strictEqual(optionsBuffer[IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS], 8);
   strictEqual(optionsBuffer[IDX_OPTIONS_MAX_SESSION_MEMORY], 9);
+  strictEqual(optionsBuffer[IDX_OPTIONS_MAX_SETTINGS], 10);
 
   const flags = optionsBuffer[IDX_OPTIONS_FLAGS];
 
@@ -57,6 +60,7 @@ const IDX_OPTIONS_FLAGS = 9;
   ok(flags & (1 << IDX_OPTIONS_MAX_HEADER_LIST_PAIRS));
   ok(flags & (1 << IDX_OPTIONS_MAX_OUTSTANDING_PINGS));
   ok(flags & (1 << IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS));
+  ok(flags & (1 << IDX_OPTIONS_MAX_SETTINGS));
 }
 
 {
