FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http2: allow to configure maximum tolerated invalid frames · nodejs/node@3bed1fa · GitHub

/ node Public

Commit 3bed1fa

Browse files
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

7 files changed

Lines changed: 182 additions & 8 deletions

File tree

‎doc/api/http2.md‎

Lines changed: 12 additions & 0 deletions
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()`][].

‎lib/internal/http2/core.js‎

Lines 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;

‎src/node_http2.cc‎

Lines 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);

‎src/node_http2.h‎

Lines 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);

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

Lines 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+
});

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

Lines 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+
});

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

Lines 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)
Back | FazBrowse Home | New Git URL