Skip to content

Commit

Permalink
http2: implement support for max settings entries
Browse files Browse the repository at this point in the history
Adds the maxSettings option to limit the number of settings
entries allowed per SETTINGS frame. Default 32

Fixes: https://hackerone.com/reports/446662
CVE-ID: CVE-2020-11080
PR-URL: nodejs-private/node-private#204
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
jasnell authored and targos committed Jun 2, 2020
1 parent 0a7bf50 commit 55e4c72
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 3 deletions.
15 changes: 15 additions & 0 deletions doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -1991,6 +1991,9 @@ value only affects new connections to the server, not any existing connections.
<!-- YAML
added: v8.4.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs-private/node-private/pull/204
description: Added `maxSettings` option with a default of 32.
- version:
- v13.3.0
- v12.16.0
Expand Down Expand Up @@ -2027,6 +2030,8 @@ changes:
* `options` {Object}
* `maxDeflateDynamicTableSize` {number} Sets the maximum dynamic table size
for deflating header fields. **Default:** `4Kib`.
* `maxSettings` {number} Sets the maximum number of settings entries per
`SETTINGS` frame. The minimum value allowed is `1`. **Default:** `32`.
* `maxSessionMemory`{number} Sets the maximum memory that the `Http2Session`
is permitted to use. The value is expressed in terms of number of megabytes,
e.g. `1` equal 1 megabyte. The minimum value allowed is `1`.
Expand Down Expand Up @@ -2122,6 +2127,9 @@ server.listen(80);
<!-- YAML
added: v8.4.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs-private/node-private/pull/204
description: Added `maxSettings` option with a default of 32.
- version:
- v13.3.0
- v12.16.0
Expand Down Expand Up @@ -2158,6 +2166,8 @@ changes:
**Default:** `false`.
* `maxDeflateDynamicTableSize` {number} Sets the maximum dynamic table size
for deflating header fields. **Default:** `4Kib`.
* `maxSettings` {number} Sets the maximum number of settings entries per
`SETTINGS` frame. The minimum value allowed is `1`. **Default:** `32`.
* `maxSessionMemory`{number} Sets the maximum memory that the `Http2Session`
is permitted to use. The value is expressed in terms of number of megabytes,
e.g. `1` equal 1 megabyte. The minimum value allowed is `1`. This is a
Expand Down Expand Up @@ -2240,6 +2250,9 @@ server.listen(80);
<!-- YAML
added: v8.4.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs-private/node-private/pull/204
description: Added `maxSettings` option with a default of 32.
- version: v13.0.0
pr-url: https://github.com/nodejs/node/pull/29144
description: The `PADDING_STRATEGY_CALLBACK` has been made equivalent to
Expand All @@ -2263,6 +2276,8 @@ changes:
* `options` {Object}
* `maxDeflateDynamicTableSize` {number} Sets the maximum dynamic table size
for deflating header fields. **Default:** `4Kib`.
* `maxSettings` {number} Sets the maximum number of settings entries per
`SETTINGS` frame. The minimum value allowed is `1`. **Default:** `32`.
* `maxSessionMemory`{number} Sets the maximum memory that the `Http2Session`
is permitted to use. The value is expressed in terms of number of megabytes,
e.g. `1` equal 1 megabyte. The minimum value allowed is `1`.
Expand Down
8 changes: 7 additions & 1 deletion lib/internal/http2/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,8 @@ const IDX_OPTIONS_MAX_HEADER_LIST_PAIRS = 5;
const IDX_OPTIONS_MAX_OUTSTANDING_PINGS = 6;
const IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS = 7;
const IDX_OPTIONS_MAX_SESSION_MEMORY = 8;
const IDX_OPTIONS_FLAGS = 9;
const IDX_OPTIONS_MAX_SETTINGS = 9;
const IDX_OPTIONS_FLAGS = 10;

function updateOptionsBuffer(options) {
let flags = 0;
Expand Down Expand Up @@ -252,6 +253,11 @@ function updateOptionsBuffer(options) {
optionsBuffer[IDX_OPTIONS_MAX_SESSION_MEMORY] =
MathMax(1, options.maxSessionMemory);
}
if (typeof options.maxSettings === 'number') {
flags |= (1 << IDX_OPTIONS_MAX_SETTINGS);
optionsBuffer[IDX_OPTIONS_MAX_SETTINGS] =
MathMax(1, options.maxSettings);
}
optionsBuffer[IDX_OPTIONS_FLAGS] = flags;
}

Expand Down
6 changes: 6 additions & 0 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,12 @@ Http2Options::Http2Options(Http2State* http2_state, SessionType type) {
// terms of MB increments (i.e. the value 1 == 1 MB)
if (flags & (1 << IDX_OPTIONS_MAX_SESSION_MEMORY))
set_max_session_memory(buffer[IDX_OPTIONS_MAX_SESSION_MEMORY] * 1000000);

if (flags & (1 << IDX_OPTIONS_MAX_SETTINGS)) {
nghttp2_option_set_max_settings(
option,
static_cast<size_t>(buffer[IDX_OPTIONS_MAX_SETTINGS]));
}
}

#define GRABSETTING(entries, count, name) \
Expand Down
1 change: 1 addition & 0 deletions src/node_http2_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ namespace http2 {
IDX_OPTIONS_MAX_OUTSTANDING_PINGS,
IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS,
IDX_OPTIONS_MAX_SESSION_MEMORY,
IDX_OPTIONS_MAX_SETTINGS,
IDX_OPTIONS_FLAGS
};

Expand Down
35 changes: 35 additions & 0 deletions test/parallel/test-http2-max-settings.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

const http2 = require('http2');

const server = http2.createServer({ maxSettings: 1 });

// TODO(@jasnell): There is still a session event
// emitted on the server side but it will be destroyed
// immediately after creation and there will be no
// stream created.
server.on('session', common.mustCall((session) => {
session.on('stream', common.mustNotCall());
session.on('remoteSettings', common.mustNotCall());
}));
server.on('stream', common.mustNotCall());

server.listen(0, common.mustCall(() => {
// Specify two settings entries when a max of 1 is allowed.
// Connection should error immediately.
const client = http2.connect(
`http://localhost:${server.address().port}`, {
settings: {
// The actual settings values do not matter.
headerTableSize: 1000,
enablePush: false,
} });

client.on('error', common.mustCall(() => {
server.close();
}));
}));
8 changes: 6 additions & 2 deletions test/parallel/test-http2-util-update-options-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ const IDX_OPTIONS_MAX_HEADER_LIST_PAIRS = 5;
const IDX_OPTIONS_MAX_OUTSTANDING_PINGS = 6;
const IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS = 7;
const IDX_OPTIONS_MAX_SESSION_MEMORY = 8;
const IDX_OPTIONS_FLAGS = 9;
const IDX_OPTIONS_MAX_SETTINGS = 9;
const IDX_OPTIONS_FLAGS = 10;

{
updateOptionsBuffer({
Expand All @@ -34,7 +35,8 @@ const IDX_OPTIONS_FLAGS = 9;
maxHeaderListPairs: 6,
maxOutstandingPings: 7,
maxOutstandingSettings: 8,
maxSessionMemory: 9
maxSessionMemory: 9,
maxSettings: 10,
});

strictEqual(optionsBuffer[IDX_OPTIONS_MAX_DEFLATE_DYNAMIC_TABLE_SIZE], 1);
Expand All @@ -46,6 +48,7 @@ const IDX_OPTIONS_FLAGS = 9;
strictEqual(optionsBuffer[IDX_OPTIONS_MAX_OUTSTANDING_PINGS], 7);
strictEqual(optionsBuffer[IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS], 8);
strictEqual(optionsBuffer[IDX_OPTIONS_MAX_SESSION_MEMORY], 9);
strictEqual(optionsBuffer[IDX_OPTIONS_MAX_SETTINGS], 10);

const flags = optionsBuffer[IDX_OPTIONS_FLAGS];

Expand All @@ -57,6 +60,7 @@ const IDX_OPTIONS_FLAGS = 9;
ok(flags & (1 << IDX_OPTIONS_MAX_HEADER_LIST_PAIRS));
ok(flags & (1 << IDX_OPTIONS_MAX_OUTSTANDING_PINGS));
ok(flags & (1 << IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS));
ok(flags & (1 << IDX_OPTIONS_MAX_SETTINGS));
}

{
Expand Down

0 comments on commit 55e4c72

Please sign in to comment.