-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
HPE_INVALID_HEADER_TOKEN on https requests #30515
Comments
HPE_INVALID_HEADER_TOKEN
on http requests
Node.js 10.16.3 still uses Unless Node.js in docker is linking to an external |
To clarify: This is the build installed on alpine linux 3.10. Notice nodejs 10.16.3 and http_parser 2.9.2 below.
Details here. Is it worth raising an issue with the maintainer? I guess they are using an external http_parser. |
Also repros with node w/http-parser@2.9.1
Does not repro with 2.8.1 (build from v10.x-staging) @nodejs/http-parser PTAL |
This appears to be the same problem with Incapsula server as before:
|
See #29589 |
@indutny @nodejs/http I'm still not clear, is this a bug or not? Do you have a link to the "before" issue? It's clearly a change in the behaviour of the http-parser, before #30471 http parser 2.8.0 doesn't error, after with 2.9.1 it does error. Is that change a regression? If its not a regression, is it enough of a change to block release in an LTS version? cc: @nodejs/lts If the failure here is a result of the security fixes nodejs/http-parser#469 or nodejs/http-parser#458, we might need to add a |
My opinion here is that fixing this bug generally and without a runtime flag means likely security issue for most users. Incapsula is sending blatantly incorrect header value and we are right to reject it. HTTP/1.1 is very brittle as the protocol data is mixed with the protocol itself. It would be unwise to loosen our requirements since it could potentially lead to request smuggling and other nasty security vulnerabilities. However, I do believe that we have to re-introduce the lenient parsing to llhttp and in fact I just opened a PR for this: nodejs/llhttp#33 . Not sure if it should be a blocker for a release in LTS version, though. |
Allow insecure HTTP header parsing. Make clear it is insecure. See: - nodejs#30553 - nodejs#27711 (comment) - nodejs#30515
Allow insecure HTTP header parsing. Make clear it is insecure. See: - #30553 - #27711 (comment) - #30515 PR-URL: #30567 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Allow insecure HTTP header parsing. Make clear it is insecure. See: - #30553 - #27711 (comment) - #30515 PR-URL: #30567 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
I would like to be able to set this on a per request basis, via a |
@ianp Yes, feel free to go ahead! You can use |
Allow insecure HTTP header parsing. Make clear it is insecure. See: - nodejs#30553 - nodejs#27711 (comment) - nodejs#30515 PR-URL: nodejs#30567 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Allow insecure HTTP header parsing. Make clear it is insecure. See: - nodejs#30553 - nodejs#27711 (comment) - nodejs#30515 PR-URL: nodejs#30567 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Allow insecure HTTP header parsing. Make clear it is insecure. See: - #30553 - #27711 (comment) - #30515 PR-URL: #30567 Backport-PR-URL: #30473 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Closing, see #27711 (comment). |
Backport 496736f Original commit message: Allow insecure HTTP header parsing. Make clear it is insecure. See: - nodejs/node#30553 - nodejs/node#27711 (comment) - nodejs/node#30515 PR-URL: nodejs/node#30567 Backport-PR-URL: nodejs/node#30473 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Backport 496736f Original commit message: Allow insecure HTTP header parsing. Make clear it is insecure. See: - nodejs/node#30553 - nodejs/node#27711 (comment) - nodejs/node#30515 PR-URL: nodejs/node#30567 Backport-PR-URL: nodejs/node#30473 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Backport 496736f Original commit message: Allow insecure HTTP header parsing. Make clear it is insecure. See: - nodejs/node#30553 - nodejs/node#27711 (comment) - nodejs/node#30515 PR-URL: nodejs/node#30567 Backport-PR-URL: nodejs/node#30473 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Ported from OpenSUSE:nodejs8-8.17.0-lp152.147.1:CVE-2019-15605.patch Original commit message: commit e2c8f89 Author: Sam Roberts <vieuxtech@gmail.com> Date: Thu Jan 16 11:55:52 2020 -0800 test: using TE to smuggle reqs is not possible See: https://hackerone.com/reports/735748 PR-URL: https://github.com/nodejs-private/node-private/pull/192 Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com> commit 49f4220 Author: Sam Roberts <vieuxtech@gmail.com> Date: Tue Feb 4 10:36:57 2020 -0800 deps: upgrade http-parser to v2.9.3 PR-URL: https://github.com/nodejs-private/http-parser-private/pull/4 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> commit d616722 Author: Sam Roberts <vieuxtech@gmail.com> Date: Tue Jan 7 14:24:54 2020 -0800 test: check that --insecure-http-parser works Test that using --insecure-http-parser will disable validation of invalid characters in HTTP headers. See: - nodejs#30567 Backport-PR-URL: nodejs#30471 PR-URL: nodejs#31253 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> commit a9849c0 Author: Sam Roberts <vieuxtech@gmail.com> Date: Wed Nov 20 11:48:58 2019 -0800 http: opt-in insecure HTTP header parsing Allow insecure HTTP header parsing. Make clear it is insecure. See: - nodejs#30553 - nodejs#27711 (comment) - nodejs#30515 Backport-PR-URL: nodejs#30471 PR-URL: nodejs#30567 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> commit a28e5cc Author: Sam Roberts <vieuxtech@gmail.com> Date: Wed Nov 13 10:05:38 2019 -0800 deps: upgrade http-parser to v2.9.1 PR-URL: nodejs#30471 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com> Signed-off-by: Su Baocheng <baocheng.su@siemens.com>
Ported from OpenSUSE:nodejs8-8.17.0-lp152.147.1:CVE-2019-15605.patch Original commit message: commit e2c8f89 Author: Sam Roberts <vieuxtech@gmail.com> Date: Thu Jan 16 11:55:52 2020 -0800 test: using TE to smuggle reqs is not possible See: https://hackerone.com/reports/735748 PR-URL: https://github.com/nodejs-private/node-private/pull/192 Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com> commit 49f4220 Author: Sam Roberts <vieuxtech@gmail.com> Date: Tue Feb 4 10:36:57 2020 -0800 deps: upgrade http-parser to v2.9.3 PR-URL: https://github.com/nodejs-private/http-parser-private/pull/4 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> commit d616722 Author: Sam Roberts <vieuxtech@gmail.com> Date: Tue Jan 7 14:24:54 2020 -0800 test: check that --insecure-http-parser works Test that using --insecure-http-parser will disable validation of invalid characters in HTTP headers. See: - nodejs#30567 Backport-PR-URL: nodejs#30471 PR-URL: nodejs#31253 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> commit a9849c0 Author: Sam Roberts <vieuxtech@gmail.com> Date: Wed Nov 20 11:48:58 2019 -0800 http: opt-in insecure HTTP header parsing Allow insecure HTTP header parsing. Make clear it is insecure. See: - nodejs#30553 - nodejs#27711 (comment) - nodejs#30515 Backport-PR-URL: nodejs#30471 PR-URL: nodejs#30567 Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> commit a28e5cc Author: Sam Roberts <vieuxtech@gmail.com> Date: Wed Nov 13 10:05:38 2019 -0800 deps: upgrade http-parser to v2.9.1 PR-URL: nodejs#30471 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com> Signed-off-by: Su Baocheng <baocheng.su@siemens.com>
Version: v10.16.3
Platform: alpine:3.10 in docker
Subsystem: http_parser: '2.9.2'
HTTP request crashes with
HPE_INVALID_HEADER_TOKEN
.Script to reproduce:
Output:
I think it's related to http_parser: '2.9.2'. It does not happen with a slightly older build which uses http_parser: '2.8.0'.
Likely related to #27711 (comment).
The mentioned workaround does not work here, as the
--http-parser
option is not available on node v10.The text was updated successfully, but these errors were encountered: