net/http: drop support for any other TransferEncoding value than "chunked" #38867
Labels
NeedsFix
The path to resolution is known, but the work has not been done.
Milestone
Comments
FiloSottile
added
the
NeedsFix
|
Change https://golang.org/cl/231418 mentions this issue: |
gopherbot
pushed a commit
that referenced
this issue
May 6, 2020
This is a security hardening measure against HTTP request smuggling. Thank you to ZeddYu for reporting this issue. We weren't parsing things correctly anyway, allowing "identity" to be combined with "chunked", and ignoring any Transfer-Encoding header past the first. This is a delicate security surface that already broke before, just be strict and don't add complexity to support cases not observed in the wild (nginx removed "identity" support [1] and multiple TE header support [2]) and removed by RFC 7230 (see page 81). It'd probably be good to also drop support for anything other than "chunked" in outbound TE headers, as "identity" is not a thing anymore, and we are probably off-spec for anything other than "chunked", but it should not be a security concern, so leaving it for now. See #38867. [1]: https://hg.nginx.org/nginx/rev/fe5976aae0e3 [2]: https://hg.nginx.org/nginx/rev/aca005d232ff Change-Id: If17d0827f9c6167a0b19a158e2bc5844ec803288 Reviewed-on: https://go-review.googlesource.com/c/go/+/231418 Reviewed-by: Katie Hockman <katie@golang.org>
xujianhai666
pushed a commit
to xujianhai666/go-1
that referenced
this issue
May 21, 2020
This is a security hardening measure against HTTP request smuggling. Thank you to ZeddYu for reporting this issue. We weren't parsing things correctly anyway, allowing "identity" to be combined with "chunked", and ignoring any Transfer-Encoding header past the first. This is a delicate security surface that already broke before, just be strict and don't add complexity to support cases not observed in the wild (nginx removed "identity" support [1] and multiple TE header support [2]) and removed by RFC 7230 (see page 81). It'd probably be good to also drop support for anything other than "chunked" in outbound TE headers, as "identity" is not a thing anymore, and we are probably off-spec for anything other than "chunked", but it should not be a security concern, so leaving it for now. See golang#38867. [1]: https://hg.nginx.org/nginx/rev/fe5976aae0e3 [2]: https://hg.nginx.org/nginx/rev/aca005d232ff Change-Id: If17d0827f9c6167a0b19a158e2bc5844ec803288 Reviewed-on: https://go-review.googlesource.com/c/go/+/231418 Reviewed-by: Katie Hockman <katie@golang.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
As of RFC 7230, "identity" is not a thing anymore, and we are probably off-spec for any other value. nginx only supports "chunked" anyway. There is a lot of code that can be removed or simplified in the sending path. (The parsing path is already being made strict as security hardening.)
However, we document setting TransferEncoding to "identity" as the way to affect some automatic chunking behavior, so we should probably still catch it at a shallow level, and behave accordingly, even if we don't send it on the wire.
The text was updated successfully, but these errors were encountered: