Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v6.x] http: fix backport of Slowris headers #24796

Closed
wants to merge 1 commit into from

Conversation

@mcollina
Copy link
Member

commented Dec 3, 2018

The backport of 618eebd was
not complete, and the starting time to parse the headers was not reset.

Fixes: #24760

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
@mcollina

This comment has been minimized.

Copy link
Member Author

commented Dec 3, 2018

cc @nodejs/release @nodejs/tsc this would need to be released ASAP, Node v6.x is currently broken.

@rvagg
rvagg approved these changes Dec 3, 2018
@mcollina

This comment has been minimized.

Copy link
Member Author

commented Dec 3, 2018

@addaleax addaleax changed the title http: fix backport of Slowris headers [v6.x] http: fix backport of Slowris headers Dec 3, 2018
Copy link
Member

left a comment

Should this target v6.x-staging?

@rvagg

This comment has been minimized.

Copy link
Member

commented Dec 3, 2018

@mcollina commit summary needs to be changed: "Slowris" -> "Slowloris"

@mcollina

This comment has been minimized.

Copy link
Member Author

commented Dec 3, 2018

v6.x-staging is not up-to-date, and it's prerogative to the @nodejs/release time to get it up to date. As such, I've targeted v6.x.

@rvagg

This comment has been minimized.

Copy link
Member

commented Dec 3, 2018

@addaleax 6.x is appropriate, we can ship a release with only this commit. There's some things waiting on staging that probably should pollute a release to minimise safety concerns for users.

The backport of 618eebd was
not complete, and the starting time to parse the headers was not reset.

Fixes: #24760
@mcollina

This comment has been minimized.

Copy link
Member Author

commented Dec 3, 2018

@rvagg done, PTAL

@mcollina mcollina force-pushed the mcollina:fix-reset-on-keepalive branch from b8d857a to 4341817 Dec 3, 2018
rvagg added a commit that referenced this pull request Dec 3, 2018
The backport of 618eebd was
not complete, and the starting time to parse the headers was not reset.

PR-URL: #24796
Fixes: #24760
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@rvagg

This comment has been minimized.

Copy link
Member

commented Dec 3, 2018

Landed, expedited, 5d9005c. There's no reason to delay on this is there?

@rvagg rvagg closed this Dec 3, 2018
@mcollina mcollina deleted the mcollina:fix-reset-on-keepalive branch Dec 3, 2018
@mcollina

This comment has been minimized.

Copy link
Member Author

commented Dec 3, 2018

There is not.

rvagg added a commit that referenced this pull request Dec 3, 2018
Notable Changes:

This is a patch release to address a bad backport of the fix for "Slowloris
HTTP Denial of Service" (CVE-2018-12122). Node.js 6.15.0 misapplies the headers
timeout to the entire keep-alive HTTP session, resulting in prematurely
disconnected sockets.

PR-URL: #24803
Refs: #24796
Refs: #24760
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
rvagg added a commit that referenced this pull request Dec 3, 2018
Notable Changes:

This is a patch release to address a bad backport of the fix for "Slowloris
HTTP Denial of Service" (CVE-2018-12122). Node.js 6.15.0 misapplies the headers
timeout to an entire keep-alive HTTP session, resulting in prematurely
disconnected sockets.

PR-URL: #24803
Refs: #24796
Refs: #24760
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
rvagg added a commit that referenced this pull request Dec 3, 2018
Notable Changes:

This is a patch release to address a bad backport of the fix for "Slowloris
HTTP Denial of Service" (CVE-2018-12122). Node.js 6.15.0 misapplies the headers
timeout to an entire keep-alive HTTP session, resulting in prematurely
disconnected sockets.

PR-URL: #24803
Refs: #24796
Refs: #24760
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
refack added a commit to refack/node that referenced this pull request Jan 14, 2019
Notable Changes:

This is a patch release to address a bad backport of the fix for "Slowloris
HTTP Denial of Service" (CVE-2018-12122). Node.js 6.15.0 misapplies the headers
timeout to an entire keep-alive HTTP session, resulting in prematurely
disconnected sockets.

PR-URL: nodejs#24803
Refs: nodejs#24796
Refs: nodejs#24760
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.