Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 092a3c2

Browse filesBrowse files
lundibundiaddaleax
lundibundi
authored and
addaleax
committed
http2: allow to configure maximum tolerated invalid frames
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>
1 parent e92afd9 commit 092a3c2
Copy full SHA for 092a3c2

File tree

Expand file treeCollapse file tree

7 files changed

+181
-8
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

7 files changed

+181
-8
lines changed
Open diff view settings
Collapse file

‎doc/api/http2.md‎

Copy file name to clipboardExpand all lines: doc/api/http2.md
+12Lines changed: 12 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -1941,6 +1941,9 @@ error will be thrown.
19411941
<!-- YAML
19421942
added: v8.4.0
19431943
changes:
1944+
- version: REPLACEME
1945+
pr-url: https://github.com/nodejs/node/pull/30534
1946+
description: Added `maxSessionInvalidFrames` option with a default of 1000.
19441947
- version: v13.0.0
19451948
pr-url: https://github.com/nodejs/node/pull/29144
19461949
description: The `PADDING_STRATEGY_CALLBACK` has been made equivalent to
@@ -2001,6 +2004,9 @@ changes:
20012004
streams for the remote peer as if a `SETTINGS` frame had been received. Will
20022005
be overridden if the remote peer sets its own value for
20032006
`maxConcurrentStreams`. **Default:** `100`.
2007+
* `maxSessionInvalidFrames` {integer} Sets the maximum number of invalid
2008+
frames that will be tolerated before the session is closed.
2009+
**Default:** `1000`.
20042010
* `settings` {HTTP/2 Settings Object} The initial settings to send to the
20052011
remote peer upon connection.
20062012
* `Http1IncomingMessage` {http.IncomingMessage} Specifies the
@@ -2053,6 +2059,9 @@ server.listen(80);
20532059
<!-- YAML
20542060
added: v8.4.0
20552061
changes:
2062+
- version: REPLACEME
2063+
pr-url: https://github.com/nodejs/node/pull/30534
2064+
description: Added `maxSessionInvalidFrames` option with a default of 1000.
20562065
- version: v13.0.0
20572066
pr-url: https://github.com/nodejs/node/pull/29144
20582067
description: The `PADDING_STRATEGY_CALLBACK` has been made equivalent to
@@ -2113,6 +2122,9 @@ changes:
21132122
streams for the remote peer as if a `SETTINGS` frame had been received. Will
21142123
be overridden if the remote peer sets its own value for
21152124
`maxConcurrentStreams`. **Default:** `100`.
2125+
* `maxSessionInvalidFrames` {integer} Sets the maximum number of invalid
2126+
frames that will be tolerated before the session is closed.
2127+
**Default:** `1000`.
21162128
* `settings` {HTTP/2 Settings Object} The initial settings to send to the
21172129
remote peer upon connection.
21182130
* ...: Any [`tls.createServer()`][] options can be provided. For
Collapse file

‎lib/internal/http2/core.js‎

Copy file name to clipboardExpand all lines: lib/internal/http2/core.js
+15-1Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,11 @@ const {
8181
},
8282
hideStackFrames
8383
} = require('internal/errors');
84-
const { validateNumber, validateString } = require('internal/validators');
84+
const { validateNumber,
85+
validateString,
86+
validateUint32,
87+
isUint32,
88+
} = require('internal/validators');
8589
const fsPromisesInternal = require('internal/fs/promises');
8690
const { utcDate } = require('internal/http');
8791
const { onServerStream,
@@ -199,6 +203,7 @@ const {
199203
kBitfield,
200204
kSessionPriorityListenerCount,
201205
kSessionFrameErrorListenerCount,
206+
kSessionMaxInvalidFrames,
202207
kSessionUint8FieldCount,
203208
kSessionHasRemoteSettingsListeners,
204209
kSessionRemoteSettingsIsUpToDate,
@@ -937,6 +942,12 @@ function setupHandle(socket, type, options) {
937942
this[kEncrypted] = false;
938943
}
939944

945+
if (isUint32(options.maxSessionInvalidFrames)) {
946+
const uint32 = new Uint32Array(
947+
this[kNativeFields].buffer, kSessionMaxInvalidFrames, 1);
948+
uint32[0] = options.maxSessionInvalidFrames;
949+
}
950+
940951
const settings = typeof options.settings === 'object' ?
941952
options.settings : {};
942953

@@ -2768,6 +2779,9 @@ function initializeOptions(options) {
27682779
assertIsObject(options.settings, 'options.settings');
27692780
options.settings = { ...options.settings };
27702781

2782+
if (options.maxSessionInvalidFrames !== undefined)
2783+
validateUint32(options.maxSessionInvalidFrames, 'maxSessionInvalidFrames');
2784+
27712785
// Used only with allowHTTP1
27722786
options.Http1IncomingMessage = options.Http1IncomingMessage ||
27732787
http.IncomingMessage;
Collapse file

‎src/node_http2.cc‎

Copy file name to clipboardExpand all lines: src/node_http2.cc
+7-2Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,8 +1011,12 @@ int Http2Session::OnInvalidFrame(nghttp2_session* handle,
10111011
void* user_data) {
10121012
Http2Session* session = static_cast<Http2Session*>(user_data);
10131013

1014-
Debug(session, "invalid frame received, code: %d", lib_error_code);
1015-
if (session->invalid_frame_count_++ > 1000)
1014+
Debug(session,
1015+
"invalid frame received (%u/%u), code: %d",
1016+
session->invalid_frame_count_,
1017+
session->js_fields_.max_invalid_frames,
1018+
lib_error_code);
1019+
if (session->invalid_frame_count_++ > session->js_fields_.max_invalid_frames)
10161020
return 1;
10171021

10181022
// If the error is fatal or if error code is ERR_STREAM_CLOSED... emit error
@@ -3057,6 +3061,7 @@ void Initialize(Local<Object> target,
30573061
NODE_DEFINE_CONSTANT(target, kBitfield);
30583062
NODE_DEFINE_CONSTANT(target, kSessionPriorityListenerCount);
30593063
NODE_DEFINE_CONSTANT(target, kSessionFrameErrorListenerCount);
3064+
NODE_DEFINE_CONSTANT(target, kSessionMaxInvalidFrames);
30603065
NODE_DEFINE_CONSTANT(target, kSessionUint8FieldCount);
30613066

30623067
NODE_DEFINE_CONSTANT(target, kSessionHasRemoteSettingsListeners);
Collapse file

‎src/node_http2.h‎

Copy file name to clipboardExpand all lines: src/node_http2.h
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,7 @@ typedef struct {
677677
uint8_t bitfield;
678678
uint8_t priority_listener_count;
679679
uint8_t frame_error_listener_count;
680+
uint32_t max_invalid_frames = 1000;
680681
} SessionJSFields;
681682

682683
// Indices for js_fields_, which serves as a way to communicate data with JS
@@ -689,6 +690,7 @@ enum SessionUint8Fields {
689690
offsetof(SessionJSFields, priority_listener_count),
690691
kSessionFrameErrorListenerCount =
691692
offsetof(SessionJSFields, frame_error_listener_count),
693+
kSessionMaxInvalidFrames = offsetof(SessionJSFields, max_invalid_frames),
692694
kSessionUint8FieldCount = sizeof(SessionJSFields)
693695
};
694696

@@ -1024,7 +1026,7 @@ class Http2Session : public AsyncWrap, public StreamListener {
10241026
// accepted again.
10251027
int32_t rejected_stream_count_ = 0;
10261028
// Also use the invalid frame count as a measure for rejecting input frames.
1027-
int32_t invalid_frame_count_ = 0;
1029+
uint32_t invalid_frame_count_ = 0;
10281030

10291031
void PushOutgoingBuffer(nghttp2_stream_write&& write);
10301032
void CopyDataIntoOutgoing(const uint8_t* src, size_t src_length);
Collapse file

‎test/parallel/test-http2-createsecureserver-options.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-http2-createsecureserver-options.js
+29-2Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ if (!common.hasCrypto)
77
const assert = require('assert');
88
const http2 = require('http2');
99

10-
// Error if invalid options are passed to createSecureServer
10+
// Error if invalid options are passed to createSecureServer.
1111
const invalidOptions = [() => {}, 1, 'test', null, Symbol('test')];
1212
invalidOptions.forEach((invalidOption) => {
1313
assert.throws(
@@ -21,7 +21,7 @@ invalidOptions.forEach((invalidOption) => {
2121
);
2222
});
2323

24-
// Error if invalid options.settings are passed to createSecureServer
24+
// Error if invalid options.settings are passed to createSecureServer.
2525
invalidOptions.forEach((invalidSettingsOption) => {
2626
assert.throws(
2727
() => http2.createSecureServer({ settings: invalidSettingsOption }),
@@ -33,3 +33,30 @@ invalidOptions.forEach((invalidSettingsOption) => {
3333
}
3434
);
3535
});
36+
37+
// Test that http2.createSecureServer validates input options.
38+
Object.entries({
39+
maxSessionInvalidFrames: [
40+
{
41+
val: -1,
42+
err: {
43+
name: 'RangeError',
44+
code: 'ERR_OUT_OF_RANGE',
45+
},
46+
},
47+
{
48+
val: Number.NEGATIVE_INFINITY,
49+
err: {
50+
name: 'RangeError',
51+
code: 'ERR_OUT_OF_RANGE',
52+
},
53+
},
54+
],
55+
}).forEach(([opt, tests]) => {
56+
tests.forEach(({ val, err }) => {
57+
assert.throws(
58+
() => http2.createSecureServer({ [opt]: val }),
59+
err
60+
);
61+
});
62+
});
Collapse file

‎test/parallel/test-http2-createserver-options.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-http2-createserver-options.js
+29-2Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ if (!common.hasCrypto)
77
const assert = require('assert');
88
const http2 = require('http2');
99

10-
// Error if invalid options are passed to createServer
10+
// Error if invalid options are passed to createServer.
1111
const invalidOptions = [1, true, 'test', null, Symbol('test')];
1212
invalidOptions.forEach((invalidOption) => {
1313
assert.throws(
@@ -21,7 +21,7 @@ invalidOptions.forEach((invalidOption) => {
2121
);
2222
});
2323

24-
// Error if invalid options.settings are passed to createServer
24+
// Error if invalid options.settings are passed to createServer.
2525
invalidOptions.forEach((invalidSettingsOption) => {
2626
assert.throws(
2727
() => http2.createServer({ settings: invalidSettingsOption }),
@@ -33,3 +33,30 @@ invalidOptions.forEach((invalidSettingsOption) => {
3333
}
3434
);
3535
});
36+
37+
// Test that http2.createServer validates input options.
38+
Object.entries({
39+
maxSessionInvalidFrames: [
40+
{
41+
val: -1,
42+
err: {
43+
name: 'RangeError',
44+
code: 'ERR_OUT_OF_RANGE',
45+
},
46+
},
47+
{
48+
val: Number.NEGATIVE_INFINITY,
49+
err: {
50+
name: 'RangeError',
51+
code: 'ERR_OUT_OF_RANGE',
52+
},
53+
},
54+
],
55+
}).forEach(([opt, tests]) => {
56+
tests.forEach(({ val, err }) => {
57+
assert.throws(
58+
() => http2.createServer({ [opt]: val }),
59+
err
60+
);
61+
});
62+
});
Collapse file

‎test/parallel/test-http2-max-invalid-frames.js‎

Copy file name to clipboard
+86Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
'use strict';
2+
const common = require('../common');
3+
if (!common.hasCrypto)
4+
common.skip('missing crypto');
5+
6+
const assert = require('assert');
7+
const http2 = require('http2');
8+
const net = require('net');
9+
10+
// Verify that creating a number of invalid HTTP/2 streams will
11+
// result in the peer closing the session within maxSessionInvalidFrames
12+
// frames.
13+
14+
const maxSessionInvalidFrames = 100;
15+
const server = http2.createServer({ maxSessionInvalidFrames });
16+
server.on('stream', (stream) => {
17+
stream.respond({
18+
'content-type': 'text/plain',
19+
':status': 200
20+
});
21+
stream.end('Hello, world!\n');
22+
});
23+
24+
server.listen(0, () => {
25+
const h2header = Buffer.alloc(9);
26+
const conn = net.connect(server.address().port);
27+
28+
conn.write('PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n');
29+
30+
h2header[3] = 4; // Send a settings frame.
31+
conn.write(Buffer.from(h2header));
32+
33+
let inbuf = Buffer.alloc(0);
34+
let state = 'settingsHeader';
35+
let settingsFrameLength;
36+
conn.on('data', (chunk) => {
37+
inbuf = Buffer.concat([inbuf, chunk]);
38+
switch (state) {
39+
case 'settingsHeader':
40+
if (inbuf.length < 9) return;
41+
settingsFrameLength = inbuf.readIntBE(0, 3);
42+
inbuf = inbuf.slice(9);
43+
state = 'readingSettings';
44+
// Fallthrough
45+
case 'readingSettings':
46+
if (inbuf.length < settingsFrameLength) return;
47+
inbuf = inbuf.slice(settingsFrameLength);
48+
h2header[3] = 4; // Send a settings ACK.
49+
h2header[4] = 1;
50+
conn.write(Buffer.from(h2header));
51+
state = 'ignoreInput';
52+
writeRequests();
53+
}
54+
});
55+
56+
let gotError = false;
57+
let streamId = 1;
58+
let reqCount = 0;
59+
60+
function writeRequests() {
61+
for (let i = 1; i < 10 && !gotError; i++) {
62+
h2header[3] = 1; // HEADERS
63+
h2header[4] = 0x5; // END_HEADERS|END_STREAM
64+
h2header.writeIntBE(1, 0, 3); // Length: 1
65+
h2header.writeIntBE(streamId, 5, 4); // Stream ID
66+
streamId += 2;
67+
// 0x88 = :status: 200
68+
if (!conn.write(Buffer.concat([h2header, Buffer.from([0x88])]))) {
69+
break;
70+
}
71+
reqCount++;
72+
}
73+
// Timeout requests to slow down the rate so we get more accurate reqCount.
74+
if (!gotError)
75+
setTimeout(writeRequests, 10);
76+
}
77+
78+
conn.once('error', common.mustCall(() => {
79+
gotError = true;
80+
assert.ok(Math.abs(reqCount - maxSessionInvalidFrames) < 100,
81+
`Request count (${reqCount}) must be around (±100)` +
82+
` maxSessionInvalidFrames option (${maxSessionInvalidFrames})`);
83+
conn.destroy();
84+
server.close();
85+
}));
86+
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.