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

net/http: invalid headers are normalized, allowing request smuggling #34540

Closed
FiloSottile opened this issue Sep 25, 2019 · 9 comments
Closed

net/http: invalid headers are normalized, allowing request smuggling #34540

FiloSottile opened this issue Sep 25, 2019 · 9 comments
Assignees
Milestone

Comments

@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Sep 25, 2019

net/http (through net/textproto) used to accept and normalize invalid HTTP/1.1 headers with a space before the colon, in violation of RFC 7230. If a Go server is used behind a reverse proxy that accepts and forwards but doesn't normalize such invalid headers, the reverse proxy and the server can interpret the headers differently. This can lead to filter bypasses or request smuggling, the latter if requests from separate clients are multiplexed onto the same connection by the proxy. Such invalid headers are now rejected by Go servers, and passed without normalization to Go client applications.

This issue is CVE-2019-16276 and is fixed in Go 1.13.1 and Go 1.12.10.

@FiloSottile FiloSottile added this to the Go1.14 milestone Sep 25, 2019
@FiloSottile FiloSottile self-assigned this Sep 25, 2019
@FiloSottile

This comment has been minimized.

Copy link
Member Author

@FiloSottile FiloSottile commented Sep 25, 2019

@gopherbot Please open backport issues for this. This was a security problem.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Sep 25, 2019

Backport issue(s) opened: #34541 (for 1.12), #34542 (for 1.13).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

dongsupark added a commit to flatcar-linux/coreos-overlay that referenced this issue Sep 26, 2019
To fix security issues in Go,
[CVE-2019-16276](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-16276),
let's update dev-lang/go from 1.12.9 to 1.12.10, and add go 1.13.1.

See also golang/go#34540
dongsupark added a commit to kinvolk/flatcar-linux-update-operator that referenced this issue Sep 26, 2019
To fix security issues in Go,
[CVE-2019-16276](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-16276),
we should upgrade Go to 1.13.1.

See also golang/go#34540
notnoop added a commit to hashicorp/nomad that referenced this issue Sep 26, 2019
To get fix for golang/go#34540 .
@thaJeztah

This comment has been minimized.

Copy link
Contributor

@thaJeztah thaJeztah commented Sep 26, 2019

Looks like this fix is not in master, only in the release branches, correct?

@desimone desimone mentioned this issue Sep 26, 2019
3 of 3 tasks complete
@FiloSottile

This comment has been minimized.

Copy link
Member Author

@FiloSottile FiloSottile commented Sep 26, 2019

Looks like this fix is not in master, only in the release branches, correct?

At the moment, correct. I will land it in master today.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Sep 26, 2019

Change https://golang.org/cl/197503 mentions this issue: net/textproto: don't normalize headers with spaces before the colon

@thaJeztah

This comment has been minimized.

Copy link
Contributor

@thaJeztah thaJeztah commented Sep 26, 2019

👍 I just noticed it, looking for the commit it was cherry-picked/backported from (which was missing)

@thausler786

This comment has been minimized.

Copy link

@thausler786 thausler786 commented Sep 30, 2019

Thanks for remediating this vulnerability so fast! We've got a couple of questions, mostly bookkeeping:

  1. When do you plan to publish the details of this vulnerability in MITRE?
  2. Do you have a CVSS score for this vulnerability?
@FiloSottile

This comment has been minimized.

Copy link
Member Author

@FiloSottile FiloSottile commented Sep 30, 2019

Just notified MITRE of the publication. We never assign the CVSS on our side.

woodsts pushed a commit to woodsts/buildroot that referenced this issue Oct 2, 2019
Fixes the following security vulnerabilities:

- CVE-2019-16276: Go before 1.12.10 and 1.13.x before 1.13.1 allow HTTP
  Request Smuggling.
  golang/go#34540

>From the release notes:

go1.12.10 (released 2019/09/25) includes security fixes to the net/http and
net/textproto packages

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
buildroot-auto-update pushed a commit to buildroot/buildroot that referenced this issue Oct 2, 2019
Fixes the following security vulnerabilities:

- CVE-2019-16276: Go before 1.12.10 and 1.13.x before 1.13.1 allow HTTP
  Request Smuggling.
  golang/go#34540

>From the release notes:

go1.12.10 (released 2019/09/25) includes security fixes to the net/http and
net/textproto packages

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
(cherry picked from commit bd574c4)
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
buildroot-auto-update pushed a commit to buildroot/buildroot that referenced this issue Oct 2, 2019
Fixes the following security vulnerability:

- CVE-2019-16276: Go before 1.12.10 and 1.13.x before 1.13.1 allow HTTP
  Request Smuggling.
  golang/go#34540

Upstream has not provided a go 1.11.x release with a fix for this, so
instead include the Debian backport of the upstream security fix from:

https://sources.debian.org/src/golang-1.11/1.11.6-1+deb10u2/debian/patches/0007-Fix-CVE-2019-16276.patch/

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
buildroot-auto-update pushed a commit to buildroot/buildroot that referenced this issue Oct 2, 2019
Fixes the following security vulnerabilities:

- CVE-2019-16276: Go before 1.12.10 and 1.13.x before 1.13.1 allow HTTP
  Request Smuggling.
  golang/go#34540

>From the release notes:

go1.12.10 (released 2019/09/25) includes security fixes to the net/http and
net/textproto packages

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
(cherry picked from commit bd574c4)
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
@sporkmonger

This comment has been minimized.

Copy link

@sporkmonger sporkmonger commented Oct 12, 2019

One thing that's potentially worth noting, even after the fix, it's still difficult for handler code to identify the characteristics of a desync payload. There's enough information to reliably identify it's happening inside the textproto package, but not when you've got a *http.Request. This makes it challenging to write detection logic for this type of attack in Go without duplicating a substantial portion of Go's HTTP stack or using lower-level functions that complicate the use of middleware, etc. If you were trying to write, e.g. a web application firewall in Go, visibility on malicious traffic is pretty useful.

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.