packages/nodejs
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix CVE

Browse Source
This commit is contained in:
liudabo 2021-03-15 14:27:27 +08:00
parent 64a6d79a66
commit 8de6e0b7f5
3 changed files with 274 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

229
CVE-2021-22883.patch Normal file
View File

@ -0,0 +1,229 @@
From 3f2e9dc40c9964965b075c00719829f9bb17e65f Mon Sep 17 00:00:00 2001
From: Daniel Bevenius <daniel.bevenius@gmail.com>
Date: Fri, 22 Jan 2021 12:34:21 +0100
Subject: [PATCH] http2: add unknownProtocol timeout
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This commit add a configuration options named unknownProtocolTimeout
which can be specified to set a value for the timeout in milliseconds
that a server should wait when an unknowProtocol is sent to it. When
this happens a timer will be started and the if the socket has not been
destroyed during that time the timer callback will destoy it.
CVE-ID: CVE-2021-22883
Refs: https://hackerone.com/reports/1043360
PR-URL: https://github.com/nodejs/node/pull/246
Backport PR-URL: https://github.com/nodejs/node/pull/248
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reference: https://github.com/nodejs/node/commit/3f2e9dc40c9964965b075c00719829f9bb17e65f
---
doc/api/http2.md | 25 +++++++++++++-
lib/internal/http2/core.js | 32 +++++++++++++++---
.../test-http2-server-unknown-protocol.js | 33 +++++++++++++++++++
3 files changed, 85 insertions(+), 5 deletions(-)
create mode 100644 test/parallel/test-http2-server-unknown-protocol.js
diff --git a/doc/api/http2.md b/doc/api/http2.md
index bb0ac52..9d0c258 100644
--- a/doc/api/http2.md
+++ b/doc/api/http2.md
@@ -1864,7 +1864,9 @@ added: v8.4.0
The `'unknownProtocol'` event is emitted when a connecting client fails to
negotiate an allowed protocol (i.e. HTTP/2 or HTTP/1.1). The event handler
receives the socket for handling. If no listener is registered for this event,
-the connection is terminated. See the [Compatibility API][].
+the connection is terminated. A timeout may be specified using the
+`'unknownProtocolTimeout'` option passed to [`http2.createSecureServer()`][].
+See the [Compatibility API][].
#### server.close([callback])
<!-- YAML
@@ -1900,6 +1902,9 @@ error will be thrown.
<!-- YAML
added: v8.4.0
changes:
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs-private/node-private/pull/248
+ description: Added `unknownProtocolTimeout` option with a default of 10000.
- version: v10.21.0
pr-url: https://github.com/nodejs-private/node-private/pull/204
description: Added `maxSettings` option with a default of 32.
@@ -1980,6 +1985,10 @@ changes:
`Http2ServerResponse` class to use.
Useful for extending the original `Http2ServerResponse`.
**Default:** `Http2ServerResponse`.
+ * `unknownProtocolTimeout` {number} Specifies a timeout in milliseconds that
+ a server should wait when an [`'unknownProtocol'`][] is emitted. If the
+ socket has not been destroyed by that time the server will destroy it.
+ **Default:** `10000`.
* `onRequestHandler` {Function} See [Compatibility API][]
* Returns: {Http2Server}
@@ -2015,6 +2024,9 @@ server.listen(80);
<!-- YAML
added: v8.4.0
changes:
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs-private/node-private/pull/248
+ description: Added `unknownProtocolTimeout` option with a default of 10000.
- version: v10.21.0
pr-url: https://github.com/nodejs-private/node-private/pull/204
description: Added `maxSettings` option with a default of 32.
@@ -2089,6 +2101,10 @@ changes:
servers, the identity options (`pfx` or `key`/`cert`) are usually required.
* `origins` {string[]} An array of origin strings to send within an `ORIGIN`
frame immediately following creation of a new server `Http2Session`.
+ * `unknownProtocolTimeout` {number} Specifies a timeout in milliseconds that
+ a server should wait when an [`'unknownProtocol'`][] event is emitted. If
+ the socket has not been destroyed by that time the server will destroy it.
+ **Default:** `10000`.
* `onRequestHandler` {Function} See [Compatibility API][]
* Returns: {Http2SecureServer}
@@ -2122,6 +2138,9 @@ server.listen(80);
<!-- YAML
added: v8.4.0
changes:
+ - version: REPLACEME
+ pr-url: https://github.com/nodejs-private/node-private/pull/248
+ description: Added `unknownProtocolTimeout` option with a default of 10000.
- version: v10.21.0
pr-url: https://github.com/nodejs-private/node-private/pull/204
description: Added `maxSettings` option with a default of 32.
@@ -2193,6 +2212,10 @@ changes:
instance passed to `connect` and the `options` object, and returns any
[`Duplex`][] stream that is to be used as the connection for this session.
* ...: Any [`net.connect()`][] or [`tls.connect()`][] options can be provided.
+ * `unknownProtocolTimeout` {number} Specifies a timeout in milliseconds that
+ a server should wait when an [`'unknownProtocol'`][] event is emitted. If
+ the socket has not been destroyed by that time the server will destroy it.
+ **Default:** `10000`.
* `listener` {Function}
* Returns: {ClientHttp2Session}
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index bb9e43c..9c5d737 100644
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -19,6 +19,7 @@ const { Duplex } = require('stream');
const tls = require('tls');
const { URL } = require('url');
const util = require('util');
+const { setImmediate, setTimeout, clearTimeout } = require('timers');
const { kIncomingMessage } = require('_http_common');
const { kServerResponse } = require('_http_server');
@@ -78,7 +79,7 @@ const {
ERR_SOCKET_CLOSED
}
} = require('internal/errors');
-const { validateNumber } = require('internal/validators');
+const { validateNumber, validateUint32 } = require('internal/validators');
const { utcDate } = require('internal/http');
const { onServerStream,
Http2ServerRequest,
@@ -2676,7 +2677,7 @@ function handleHeaderContinue(headers) {
this.emit('continue');
}
-const setTimeout = {
+const setTimeoutValue = {
configurable: true,
enumerable: true,
writable: true,
@@ -2710,8 +2711,8 @@ const setTimeout = {
return this;
}
};
-Object.defineProperty(Http2Stream.prototype, 'setTimeout', setTimeout);
-Object.defineProperty(Http2Session.prototype, 'setTimeout', setTimeout);
+Object.defineProperty(Http2Stream.prototype, 'setTimeout', setTimeoutValue);
+Object.defineProperty(Http2Session.prototype, 'setTimeout', setTimeoutValue);
// When the socket emits an error, destroy the associated Http2Session and
@@ -2771,6 +2772,22 @@ function connectionListener(socket) {
debug('Unknown protocol from %s:%s',
socket.remoteAddress, socket.remotePort);
if (!this.emit('unknownProtocol', socket)) {
+ debug('Unknown protocol timeout: %s', options.unknownProtocolTimeout);
+ // Install a timeout if the socket was not successfully closed, then
+ // destroy the socket to ensure that the underlying resources are
+ // released.
+ const timer = setTimeout(() => {
+ if (!socket.destroyed) {
+ debug('UnknownProtocol socket timeout, destroy socket');
+ socket.destroy();
+ }
+ }, options.unknownProtocolTimeout);
+ // Un-reference the timer to avoid blocking of application shutdown and
+ // clear the timeout if the socket was successfully closed.
+ timer.unref();
+
+ socket.once('close', () => clearTimeout(timer));
+
// We don't know what to do, so let's just tell the other side what's
// going on in a format that they *might* understand.
socket.end('HTTP/1.0 403 Forbidden\r\n' +
@@ -2810,6 +2827,13 @@ function initializeOptions(options) {
assertIsObject(options.settings, 'options.settings');
options.settings = Object.assign({}, options.settings);
+ if (options.unknownProtocolTimeout !== undefined)
+ validateUint32(options.unknownProtocolTimeout, 'unknownProtocolTimeout');
+ else
+ // TODO(danbev): is this a good default value?
+ options.unknownProtocolTimeout = 10000;
+
+
// Used only with allowHTTP1
options.Http1IncomingMessage = options.Http1IncomingMessage ||
http.IncomingMessage;
diff --git a/test/parallel/test-http2-server-unknown-protocol.js b/test/parallel/test-http2-server-unknown-protocol.js
new file mode 100644
index 0000000..2c7aea5
--- /dev/null
+++ b/test/parallel/test-http2-server-unknown-protocol.js
@@ -0,0 +1,33 @@
+'use strict';
+const common = require('../common');
+const fixtures = require('../common/fixtures');
+
+// This test verifies that when a server receives an unknownProtocol it will
+// not leave the socket open if the client does not close it.
+
+if (!common.hasCrypto)
+ common.skip('missing crypto');
+
+const h2 = require('http2');
+const tls = require('tls');
+
+const server = h2.createSecureServer({
+ key: fixtures.readKey('agent2-key.pem'),
+ cert: fixtures.readKey('agent2-cert.pem'),
+ unknownProtocolTimeout: 500,
+ allowHalfOpen: true
+});
+
+server.on('connection', (socket) => {
+ socket.on('close', common.mustCall(() => {
+ server.close();
+ }));
+});
+
+server.listen(0, function() {
+ tls.connect({
+ port: server.address().port,
+ rejectUnauthorized: false,
+ ALPNProtocols: ['bogus']
+ });
+});
--
2.23.0

39
CVE-2021-22884.patch Normal file
View File

@ -0,0 +1,39 @@
From d1cf6a9b0f74d587dea1d0f194d922ff94eddd06 Mon Sep 17 00:00:00 2001
From: Matteo Collina <hello@matteocollina.com>
Date: Thu, 14 Jan 2021 16:04:44 +0100
Subject: [PATCH] src: drop localhost6 as allowed host for inspector
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
CVE-ID: CVE-2021-22884
Refs: https://hackerone.com/bugs?report_id=1069487
PR-URL: https://github.com/nodejs/node/pull/244
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Mary Marchini <oss@mmarchini.me>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reference: https://github.com/nodejs/node/commit/d1cf6a9b0f74d587dea1d0f194d922ff94eddd06
---
src/inspector_socket.cc | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/inspector_socket.cc b/src/inspector_socket.cc
index dc36359..38b9ed4 100644
--- a/src/inspector_socket.cc
+++ b/src/inspector_socket.cc
@@ -576,8 +576,7 @@ class HttpHandler : public ProtocolHandler {
bool IsAllowedHost(const std::string& host_with_port) const {
std::string host = TrimPort(host_with_port);
return host.empty() || IsIPAddress(host)
- || node::StringEqualNoCase(host.data(), "localhost")
- || node::StringEqualNoCase(host.data(), "localhost6");
+ || node::StringEqualNoCase(host.data(), "localhost");
}
bool parsing_value_;
--
2.23.0

7
nodejs.spec
View File

@ -1,5 +1,5 @@
%bcond_with bootstrap
%global baserelease 5
%global baserelease 6
%{?!_pkgdocdir:%global _pkgdocdir %{_docdir}/%{name}-%{version}}
%global nodejs_epoch 1
%global nodejs_major 10
@ -76,6 +76,8 @@ Patch4: 0004-src-avoid-OOB-read-in-URL-parser.patch
Patch5: CVE-2020-8252.patch
Patch6: CVE-2020-8265.patch
Patch7: CVE-2020-8287.patch
Patch8: CVE-2021-22883.patch
Patch9: CVE-2021-22884.patch
BuildRequires: python2-devel python3-devel zlib-devel gcc >= 6.3.0
BuildRequires: gcc-c++ >= 6.3.0 nodejs-packaging chrpath libatomic
@ -463,6 +465,9 @@ end
%changelog
* Mon Mar 15 2021 xinghe <xinghe1@huawei.com> 1:10.21.0-6
- fix CVE-2021-22883 CVE-2021-22884
* Fri Feb 5 2021 xinghe <xinghe1@huawei.com> 1:10.21.0-5
- fix CVE-2020-8265 CVE-2020-8287