GeezerVison is showing github.com — http2: allow to configure maximum tolerated invalid frames · nodejs/node@3bed1fa
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file 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 number | Diff line number | Diff line change |
|---|
@@ -1939,6 +1939,9 @@ error will be thrown. | 1939 | 1939 | <!-- YAML | 1940 | 1940 | added: v8.4.0 | 1941 | 1941 | changes: | | 1942 | + - version: REPLACEME | | 1943 | + pr-url: https://github.com/nodejs/node/pull/30534 | | 1944 | + description: Added `maxSessionInvalidFrames` option with a default of 1000. | 1942 | 1945 | - version: v12.4.0 | 1943 | 1946 | pr-url: https://github.com/nodejs/node/pull/27782 | 1944 | 1947 | description: The `options` parameter now supports `net.createServer()` | @@ -1999,6 +2002,9 @@ changes: | 1999 | 2002 | streams for the remote peer as if a `SETTINGS` frame had been received. Will | 2000 | 2003 | be overridden if the remote peer sets its own value for | 2001 | 2004 | `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`. | 2002 | 2008 | * `selectPadding` {Function} When `options.paddingStrategy` is equal to | 2003 | 2009 | `http2.constants.PADDING_STRATEGY_CALLBACK`, provides the callback function | 2004 | 2010 | used to determine the padding. See [Using `options.selectPadding()`][]. | @@ -2054,6 +2060,9 @@ server.listen(80); | 2054 | 2060 | <!-- YAML | 2055 | 2061 | added: v8.4.0 | 2056 | 2062 | changes: | | 2063 | + - version: REPLACEME | | 2064 | + pr-url: https://github.com/nodejs/node/pull/30534 | | 2065 | + description: Added `maxSessionInvalidFrames` option with a default of 1000. | 2057 | 2066 | - version: v10.12.0 | 2058 | 2067 | pr-url: https://github.com/nodejs/node/pull/22956 | 2059 | 2068 | description: Added the `origins` option to automatically send an `ORIGIN` | @@ -2114,6 +2123,9 @@ changes: | 2114 | 2123 | streams for the remote peer as if a `SETTINGS` frame had been received. Will | 2115 | 2124 | be overridden if the remote peer sets its own value for | 2116 | 2125 | `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`. | 2117 | 2129 | * `selectPadding` {Function} When `options.paddingStrategy` is equal to | 2118 | 2130 | `http2.constants.PADDING_STRATEGY_CALLBACK`, provides the callback function | 2119 | 2131 | used to determine the padding. See [Using `options.selectPadding()`][]. | |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/http2/core.js +15-1Lines changed: 15 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -81,7 +81,11 @@ const { | 81 | 81 | }, | 82 | 82 | hideStackFrames | 83 | 83 | } = require('internal/errors'); | 84 | | -const { validateNumber, validateString } = require('internal/validators'); | | 84 | +const { validateNumber, | | 85 | + validateString, | | 86 | + validateUint32, | | 87 | + isUint32, | | 88 | +} = require('internal/validators'); | 85 | 89 | const fsPromisesInternal = require('internal/fs/promises'); | 86 | 90 | const { utcDate } = require('internal/http'); | 87 | 91 | const { onServerStream, | @@ -207,6 +211,7 @@ const { | 207 | 211 | kBitfield, | 208 | 212 | kSessionPriorityListenerCount, | 209 | 213 | kSessionFrameErrorListenerCount, | | 214 | + kSessionMaxInvalidFrames, | 210 | 215 | kSessionUint8FieldCount, | 211 | 216 | kSessionHasRemoteSettingsListeners, | 212 | 217 | kSessionRemoteSettingsIsUpToDate, | @@ -959,6 +964,12 @@ function setupHandle(socket, type, options) { | 959 | 964 | this[kEncrypted] = false; | 960 | 965 | } | 961 | 966 |
| | 967 | + if (isUint32(options.maxSessionInvalidFrames)) { | | 968 | + const uint32 = new Uint32Array( | | 969 | + this[kNativeFields].buffer, kSessionMaxInvalidFrames, 1); | | 970 | + uint32[0] = options.maxSessionInvalidFrames; | | 971 | + } | | 972 | + | 962 | 973 | const settings = typeof options.settings === 'object' ? | 963 | 974 | options.settings : {}; | 964 | 975 |
| @@ -2790,6 +2801,9 @@ function initializeOptions(options) { | 2790 | 2801 | assertIsObject(options.settings, 'options.settings'); | 2791 | 2802 | options.settings = { ...options.settings }; | 2792 | 2803 |
| | 2804 | + if (options.maxSessionInvalidFrames !== undefined) | | 2805 | + validateUint32(options.maxSessionInvalidFrames, 'maxSessionInvalidFrames'); | | 2806 | + | 2793 | 2807 | // Used only with allowHTTP1 | 2794 | 2808 | options.Http1IncomingMessage = options.Http1IncomingMessage || | 2795 | 2809 | http.IncomingMessage; | |
Collapse file Copy file name to clipboardExpand all lines: src/node_http2.cc +8-2Lines changed: 8 additions & 2 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -1045,8 +1045,13 @@ int Http2Session::OnInvalidFrame(nghttp2_session* handle, | 1045 | 1045 | void* user_data) { | 1046 | 1046 | Http2Session* session = static_cast<Http2Session*>(user_data); | 1047 | 1047 |
| 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 && | 1050 | 1055 | !IsReverted(SECURITY_REVERT_CVE_2019_9514)) { | 1051 | 1056 | return 1; | 1052 | 1057 | } | @@ -3105,6 +3110,7 @@ void Initialize(Local<Object> target, | 3105 | 3110 | NODE_DEFINE_CONSTANT(target, kBitfield); | 3106 | 3111 | NODE_DEFINE_CONSTANT(target, kSessionPriorityListenerCount); | 3107 | 3112 | NODE_DEFINE_CONSTANT(target, kSessionFrameErrorListenerCount); | | 3113 | + NODE_DEFINE_CONSTANT(target, kSessionMaxInvalidFrames); | 3108 | 3114 | NODE_DEFINE_CONSTANT(target, kSessionUint8FieldCount); | 3109 | 3115 |
| 3110 | 3116 | NODE_DEFINE_CONSTANT(target, kSessionHasRemoteSettingsListeners); | |
Collapse file Copy file name to clipboardExpand all lines: src/node_http2.h +3-1Lines changed: 3 additions & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -679,6 +679,7 @@ typedef struct { | 679 | 679 | uint8_t bitfield; | 680 | 680 | uint8_t priority_listener_count; | 681 | 681 | uint8_t frame_error_listener_count; | | 682 | + uint32_t max_invalid_frames = 1000; | 682 | 683 | } SessionJSFields; | 683 | 684 |
| 684 | 685 | // Indices for js_fields_, which serves as a way to communicate data with JS | @@ -691,6 +692,7 @@ enum SessionUint8Fields { | 691 | 692 | offsetof(SessionJSFields, priority_listener_count), | 692 | 693 | kSessionFrameErrorListenerCount = | 693 | 694 | offsetof(SessionJSFields, frame_error_listener_count), | | 695 | + kSessionMaxInvalidFrames = offsetof(SessionJSFields, max_invalid_frames), | 694 | 696 | kSessionUint8FieldCount = sizeof(SessionJSFields) | 695 | 697 | }; | 696 | 698 |
| @@ -1028,7 +1030,7 @@ class Http2Session : public AsyncWrap, public StreamListener { | 1028 | 1030 | // accepted again. | 1029 | 1031 | int32_t rejected_stream_count_ = 0; | 1030 | 1032 | // 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; | 1032 | 1034 |
| 1033 | 1035 | void PushOutgoingBuffer(nghttp2_stream_write&& write); | 1034 | 1036 | void CopyDataIntoOutgoing(const uint8_t* src, size_t src_length); | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-http2-createsecureserver-options.js +29-2Lines changed: 29 additions & 2 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -7,7 +7,7 @@ if (!common.hasCrypto) | 7 | 7 | const assert = require('assert'); | 8 | 8 | const http2 = require('http2'); | 9 | 9 |
| 10 | | -// Error if invalid options are passed to createSecureServer | | 10 | +// Error if invalid options are passed to createSecureServer. | 11 | 11 | const invalidOptions = [() => {}, 1, 'test', null, Symbol('test')]; | 12 | 12 | invalidOptions.forEach((invalidOption) => { | 13 | 13 | assert.throws( | @@ -21,7 +21,7 @@ invalidOptions.forEach((invalidOption) => { | 21 | 21 | ); | 22 | 22 | }); | 23 | 23 |
| 24 | | -// Error if invalid options.settings are passed to createSecureServer | | 24 | +// Error if invalid options.settings are passed to createSecureServer. | 25 | 25 | invalidOptions.forEach((invalidSettingsOption) => { | 26 | 26 | assert.throws( | 27 | 27 | () => http2.createSecureServer({ settings: invalidSettingsOption }), | @@ -33,3 +33,30 @@ invalidOptions.forEach((invalidSettingsOption) => { | 33 | 33 | } | 34 | 34 | ); | 35 | 35 | }); | | 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 Copy file name to clipboardExpand all lines: test/parallel/test-http2-createserver-options.js +29-2Lines changed: 29 additions & 2 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -7,7 +7,7 @@ if (!common.hasCrypto) | 7 | 7 | const assert = require('assert'); | 8 | 8 | const http2 = require('http2'); | 9 | 9 |
| 10 | | -// Error if invalid options are passed to createServer | | 10 | +// Error if invalid options are passed to createServer. | 11 | 11 | const invalidOptions = [1, true, 'test', null, Symbol('test')]; | 12 | 12 | invalidOptions.forEach((invalidOption) => { | 13 | 13 | assert.throws( | @@ -21,7 +21,7 @@ invalidOptions.forEach((invalidOption) => { | 21 | 21 | ); | 22 | 22 | }); | 23 | 23 |
| 24 | | -// Error if invalid options.settings are passed to createServer | | 24 | +// Error if invalid options.settings are passed to createServer. | 25 | 25 | invalidOptions.forEach((invalidSettingsOption) => { | 26 | 26 | assert.throws( | 27 | 27 | () => http2.createServer({ settings: invalidSettingsOption }), | @@ -33,3 +33,30 @@ invalidOptions.forEach((invalidSettingsOption) => { | 33 | 33 | } | 34 | 34 | ); | 35 | 35 | }); | | 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 Copy file name to clipboard +86Lines changed: 86 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 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 | +}); |
|