Skip to content

Conversation

@mcollina
Copy link
Member

@mcollina mcollina commented Dec 3, 2018

The backport of 618eebdd17 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
@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. v6.x labels Dec 3, 2018
@mcollina
Copy link
Member Author

mcollina commented Dec 3, 2018

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

@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

@addaleax addaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this target v6.x-staging?

@rvagg
Copy link
Member

rvagg commented Dec 3, 2018

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

@mcollina
Copy link
Member Author

mcollina 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
Copy link
Member

rvagg 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 nodejs@618eebdd17 was not complete, and the starting time to parse the headers was not reset. Fixes: nodejs#24760
@mcollina
Copy link
Member Author

mcollina commented Dec 3, 2018

@rvagg done, PTAL

@mcollina mcollina force-pushed the fix-reset-on-keepalive branch from b8d857a to 4341817 Compare December 3, 2018 12:21
rvagg pushed a commit that referenced this pull request Dec 3, 2018
The backport of 618eebdd17 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
Copy link
Member

rvagg 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 fix-reset-on-keepalive branch December 3, 2018 12:30
@mcollina
Copy link
Member Author

mcollina 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 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 pushed 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

Labels

http Issues or PRs related to the http subsystem.

5 participants