http2: make maximum tolerated rejected streams configurable

lundibundi · addaleax · commit 7d37bcebea61 · 2019-11-30T03:57:20.000+01:00
PR-URL: #30534 Fixes: #30505 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/doc/api/http2.md b/doc/api/http2.md
@@ -1941,6 +1941,9 @@ error will be thrown.
 <!-- YAML
 added: v8.4.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/30534
+    description: Added `maxSessionRejectedStreams` option with a default of 100.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/30534
     description: Added `maxSessionInvalidFrames` option with a default of 1000.
@@ -2007,6 +2010,12 @@ changes:
   * `maxSessionInvalidFrames` {integer} Sets the maximum number of invalid
     frames that will be tolerated before the session is closed.
     **Default:** `1000`.
+  * `maxSessionRejectedStreams` {integer} Sets the maximum number of rejected
+    upon creation streams that will be tolerated before the session is closed.
+    Each rejection is associated with an `NGHTTP2_ENHANCE_YOUR_CALM`
+    error that should tell the peer to not open any more streams, continuing
+    to open streams is therefore regarded as a sign of a misbehaving peer.
+    **Default:** `100`.
   * `settings` {HTTP/2 Settings Object} The initial settings to send to the
     remote peer upon connection.
   * `Http1IncomingMessage` {http.IncomingMessage} Specifies the
@@ -2059,6 +2068,9 @@ server.listen(80);
 <!-- YAML
 added: v8.4.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/30534
+    description: Added `maxSessionRejectedStreams` option with a default of 100.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/30534
     description: Added `maxSessionInvalidFrames` option with a default of 1000.
@@ -2125,6 +2137,12 @@ changes:
   * `maxSessionInvalidFrames` {integer} Sets the maximum number of invalid
     frames that will be tolerated before the session is closed.
     **Default:** `1000`.
+  * `maxSessionRejectedStreams` {integer} Sets the maximum number of rejected
+    upon creation streams that will be tolerated before the session is closed.
+    Each rejection is associated with an `NGHTTP2_ENHANCE_YOUR_CALM`
+    error that should tell the peer to not open any more streams, continuing
+    to open streams is therefore regarded as a sign of a misbehaving peer.
+    **Default:** `100`.
   * `settings` {HTTP/2 Settings Object} The initial settings to send to the
     remote peer upon connection.
   * ...: Any [`tls.createServer()`][] options can be provided. For
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -204,6 +204,7 @@ const {
   kSessionPriorityListenerCount,
   kSessionFrameErrorListenerCount,
   kSessionMaxInvalidFrames,
+  kSessionMaxRejectedStreams,
   kSessionUint8FieldCount,
   kSessionHasRemoteSettingsListeners,
   kSessionRemoteSettingsIsUpToDate,
@@ -948,6 +949,12 @@ function setupHandle(socket, type, options) {
     uint32[0] = options.maxSessionInvalidFrames;
   }
 
+  if (isUint32(options.maxSessionRejectedStreams)) {
+    const uint32 = new Uint32Array(
+      this[kNativeFields].buffer, kSessionMaxRejectedStreams, 1);
+    uint32[0] = options.maxSessionRejectedStreams;
+  }
+
   const settings = typeof options.settings === 'object' ?
     options.settings : {};
 
@@ -2782,6 +2789,13 @@ function initializeOptions(options) {
   if (options.maxSessionInvalidFrames !== undefined)
     validateUint32(options.maxSessionInvalidFrames, 'maxSessionInvalidFrames');
 
+  if (options.maxSessionRejectedStreams !== undefined) {
+    validateUint32(
+      options.maxSessionRejectedStreams,
+      'maxSessionRejectedStreams'
+    );
+  }
+
   // Used only with allowHTTP1
   options.Http1IncomingMessage = options.Http1IncomingMessage ||
     http.IncomingMessage;
diff --git a/src/node_http2.cc b/src/node_http2.cc
@@ -920,7 +920,8 @@ int Http2Session::OnBeginHeadersCallback(nghttp2_session* handle,
     if (UNLIKELY(!session->CanAddStream() ||
                  Http2Stream::New(session, id, frame->headers.cat) ==
                      nullptr)) {
-      if (session->rejected_stream_count_++ > 100)
+      if (session->rejected_stream_count_++ >
+          session->js_fields_.max_rejected_streams)
         return NGHTTP2_ERR_CALLBACK_FAILURE;
       // Too many concurrent streams being opened
       nghttp2_submit_rst_stream(**session, NGHTTP2_FLAG_NONE, id,
@@ -3062,6 +3063,7 @@ void Initialize(Local<Object> target,
   NODE_DEFINE_CONSTANT(target, kSessionPriorityListenerCount);
   NODE_DEFINE_CONSTANT(target, kSessionFrameErrorListenerCount);
   NODE_DEFINE_CONSTANT(target, kSessionMaxInvalidFrames);
+  NODE_DEFINE_CONSTANT(target, kSessionMaxRejectedStreams);
   NODE_DEFINE_CONSTANT(target, kSessionUint8FieldCount);
 
   NODE_DEFINE_CONSTANT(target, kSessionHasRemoteSettingsListeners);
diff --git a/src/node_http2.h b/src/node_http2.h
@@ -678,6 +678,7 @@ typedef struct {
   uint8_t priority_listener_count;
   uint8_t frame_error_listener_count;
   uint32_t max_invalid_frames = 1000;
+  uint32_t max_rejected_streams = 100;
 } SessionJSFields;
 
 // Indices for js_fields_, which serves as a way to communicate data with JS
@@ -691,6 +692,7 @@ enum SessionUint8Fields {
   kSessionFrameErrorListenerCount =
       offsetof(SessionJSFields, frame_error_listener_count),
   kSessionMaxInvalidFrames = offsetof(SessionJSFields, max_invalid_frames),
+  kSessionMaxRejectedStreams = offsetof(SessionJSFields, max_rejected_streams),
   kSessionUint8FieldCount = sizeof(SessionJSFields)
 };
 
@@ -1024,7 +1026,7 @@ class Http2Session : public AsyncWrap, public StreamListener {
   // limit will result in the session being destroyed, as an indication of a
   // misbehaving peer. This counter is reset once new streams are being
   // accepted again.
-  int32_t rejected_stream_count_ = 0;
+  uint32_t rejected_stream_count_ = 0;
   // Also use the invalid frame count as a measure for rejecting input frames.
   uint32_t invalid_frame_count_ = 0;
 
diff --git a/test/parallel/test-http2-createsecureserver-options.js b/test/parallel/test-http2-createsecureserver-options.js
@@ -52,6 +52,22 @@ Object.entries({
       },
     },
   ],
+  maxSessionRejectedStreams: [
+    {
+      val: -1,
+      err: {
+        name: 'RangeError',
+        code: 'ERR_OUT_OF_RANGE',
+      },
+    },
+    {
+      val: Number.NEGATIVE_INFINITY,
+      err: {
+        name: 'RangeError',
+        code: 'ERR_OUT_OF_RANGE',
+      },
+    },
+  ],
 }).forEach(([opt, tests]) => {
   tests.forEach(({ val, err }) => {
     assert.throws(
diff --git a/test/parallel/test-http2-createserver-options.js b/test/parallel/test-http2-createserver-options.js
@@ -52,6 +52,22 @@ Object.entries({
       },
     },
   ],
+  maxSessionRejectedStreams: [
+    {
+      val: -1,
+      err: {
+        name: 'RangeError',
+        code: 'ERR_OUT_OF_RANGE',
+      },
+    },
+    {
+      val: Number.NEGATIVE_INFINITY,
+      err: {
+        name: 'RangeError',
+        code: 'ERR_OUT_OF_RANGE',
+      },
+    },
+  ]
 }).forEach(([opt, tests]) => {
   tests.forEach(({ val, err }) => {
     assert.throws(
