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 3bed1fa

Browse filesBrowse files
lundibunditargos
lundibundi
authored and
targos
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 46cb0da commit 3bed1fa
Copy full SHA for 3bed1fa

File tree

Expand file treeCollapse file tree

7 files changed

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

7 files changed

+182
-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
@@ -1939,6 +1939,9 @@ error will be thrown.
19391939
<!-- YAML
19401940
added: v8.4.0
19411941
changes:
1942+
- version: REPLACEME
1943+
pr-url: https://github.com/nodejs/node/pull/30534
1944+
description: Added `maxSessionInvalidFrames` option with a default of 1000.
19421945
- version: v12.4.0
19431946
pr-url: https://github.com/nodejs/node/pull/27782
19441947
description: The `options` parameter now supports `net.createServer()`
@@ -1999,6 +2002,9 @@ changes:
19992002
streams for the remote peer as if a `SETTINGS` frame had been received. Will
20002003
be overridden if the remote peer sets its own value for
20012004
`maxConcurrentStreams`. **Default:** `100`.
2005+
* `maxSessionInvalidFrames` {integer} Sets the maximum number of invalid
2006+
frames that will be tolerated before the session is closed.
2007+
**Default:** `1000`.
20022008
* `selectPadding` {Function} When `options.paddingStrategy` is equal to
20032009
`http2.constants.PADDING_STRATEGY_CALLBACK`, provides the callback function
20042010
used to determine the padding. See [Using `options.selectPadding()`][].
@@ -2054,6 +2060,9 @@ server.listen(80);
20542060
<!-- YAML
20552061
added: v8.4.0
20562062
changes:
2063+
- version: REPLACEME
2064+
pr-url: https://github.com/nodejs/node/pull/30534
2065+
description: Added `maxSessionInvalidFrames` option with a default of 1000.
20572066
- version: v10.12.0
20582067
pr-url: https://github.com/nodejs/node/pull/22956
20592068
description: Added the `origins` option to automatically send an `ORIGIN`
@@ -2114,6 +2123,9 @@ changes:
21142123
streams for the remote peer as if a `SETTINGS` frame had been received. Will
21152124
be overridden if the remote peer sets its own value for
21162125
`maxConcurrentStreams`. **Default:** `100`.
2126+
* `maxSessionInvalidFrames` {integer} Sets the maximum number of invalid
2127+
frames that will be tolerated before the session is closed.
2128+
**Default:** `1000`.
21172129
* `selectPadding` {Function} When `options.paddingStrategy` is equal to
21182130
`http2.constants.PADDING_STRATEGY_CALLBACK`, provides the callback function
21192131
used to determine the padding. See [Using `options.selectPadding()`][].
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,
@@ -207,6 +211,7 @@ const {
207211
kBitfield,
208212
kSessionPriorityListenerCount,
209213
kSessionFrameErrorListenerCount,
214+
kSessionMaxInvalidFrames,
210215
kSessionUint8FieldCount,
211216
kSessionHasRemoteSettingsListeners,
212217
kSessionRemoteSettingsIsUpToDate,
@@ -959,6 +964,12 @@ function setupHandle(socket, type, options) {
959964
this[kEncrypted] = false;
960965
}
961966

967+
if (isUint32(options.maxSessionInvalidFrames)) {
968+
const uint32 = new Uint32Array(
969+
this[kNativeFields].buffer, kSessionMaxInvalidFrames, 1);
970+
uint32[0] = options.maxSessionInvalidFrames;
971+
}
972+
962973
const settings = typeof options.settings === 'object' ?
963974
options.settings : {};
964975

@@ -2790,6 +2801,9 @@ function initializeOptions(options) {
27902801
assertIsObject(options.settings, 'options.settings');
27912802
options.settings = { ...options.settings };
27922803

2804+
if (options.maxSessionInvalidFrames !== undefined)
2805+
validateUint32(options.maxSessionInvalidFrames, 'maxSessionInvalidFrames');
2806+
27932807
// Used only with allowHTTP1
27942808
options.Http1IncomingMessage = options.Http1IncomingMessage ||
27952809
http.IncomingMessage;
Collapse file

‎src/node_http2.cc‎

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

1048-
Debug(session, "invalid frame received, code: %d", lib_error_code);
1049-
if (session->invalid_frame_count_++ > 1000 &&
1048+
Debug(session,
1049+
"invalid frame received (%u/%u), code: %d",
1050+
session->invalid_frame_count_,
1051+
session->js_fields_.max_invalid_frames,
1052+
lib_error_code);
1053+
if (session->invalid_frame_count_++ >
1054+
session->js_fields_.max_invalid_frames &&
10501055
!IsReverted(SECURITY_REVERT_CVE_2019_9514)) {
10511056
return 1;
10521057
}
@@ -3105,6 +3110,7 @@ void Initialize(Local<Object> target,
31053110
NODE_DEFINE_CONSTANT(target, kBitfield);
31063111
NODE_DEFINE_CONSTANT(target, kSessionPriorityListenerCount);
31073112
NODE_DEFINE_CONSTANT(target, kSessionFrameErrorListenerCount);
3113+
NODE_DEFINE_CONSTANT(target, kSessionMaxInvalidFrames);
31083114
NODE_DEFINE_CONSTANT(target, kSessionUint8FieldCount);
31093115

31103116
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
@@ -679,6 +679,7 @@ typedef struct {
679679
uint8_t bitfield;
680680
uint8_t priority_listener_count;
681681
uint8_t frame_error_listener_count;
682+
uint32_t max_invalid_frames = 1000;
682683
} SessionJSFields;
683684

684685
// Indices for js_fields_, which serves as a way to communicate data with JS
@@ -691,6 +692,7 @@ enum SessionUint8Fields {
691692
offsetof(SessionJSFields, priority_listener_count),
692693
kSessionFrameErrorListenerCount =
693694
offsetof(SessionJSFields, frame_error_listener_count),
695+
kSessionMaxInvalidFrames = offsetof(SessionJSFields, max_invalid_frames),
694696
kSessionUint8FieldCount = sizeof(SessionJSFields)
695697
};
696698

@@ -1028,7 +1030,7 @@ class Http2Session : public AsyncWrap, public StreamListener {
10281030
// accepted again.
10291031
int32_t rejected_stream_count_ = 0;
10301032
// Also use the invalid frame count as a measure for rejecting input frames.
1031-
int32_t invalid_frame_count_ = 0;
1033+
uint32_t invalid_frame_count_ = 0;
10321034

10331035
void PushOutgoingBuffer(nghttp2_stream_write&& write);
10341036
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.