Skip to content

AppVeyor: bump golang 1.12.10 (CVE-2019-16276)#3686

Merged
estesp merged 1 commit intocontainerd:masterfrom
thaJeztah:bump_golang_1.12.10
Sep 26, 2019
Merged

AppVeyor: bump golang 1.12.10 (CVE-2019-16276)#3686
estesp merged 1 commit intocontainerd:masterfrom
thaJeztah:bump_golang_1.12.10

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

full diff: golang/go@go1.12.9...go1.12.10

Hi gophers,

We have just released Go 1.13.1 and Go 1.12.10 to address a recently reported security issue. We recommend that all affected users update to one of these releases (if you're not sure which, choose Go 1.13.1).

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 an uncommon 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 upstream connection by the proxy. Such invalid headers are now rejected by Go servers, and passed without normalization to Go client applications.

The issue is CVE-2019-16276 and Go issue golang.org/issue/34540.

Thanks to Andrew Stucki, Adam Scarr (99designs.com), and Jan Masarik (masarik.sh) for discovering and reporting this issue.

Downloads are available at https://golang.org/dl for all supported platforms.

Alla prossima,
Filippo on behalf of the Go team

From the patch: golang/go@6e6f4aa

net/textproto: don't normalize headers with spaces before the colon

RFC 7230 is clear about headers with a space before the colon, like

X-Answer : 42

being invalid, but we've been accepting and normalizing them for compatibility
purposes since CL 5690059 in 2012.

On the client side, this is harmless and indeed most browsers behave the same
to this day. On the server side, this becomes a security issue when the
behavior doesn't match that of a reverse proxy sitting in front of the server.

For example, if a WAF accepts them without normalizing them, it might be
possible to bypass its filters, because the Go server would interpret the
header differently. Worse, if the reverse proxy coalesces requests onto a
single HTTP/1.1 connection to a Go server, the understanding of the request
boundaries can get out of sync between them, allowing an attacker to tack an
arbitrary method and path onto a request by other clients, including
authentication headers unknown to the attacker.

This was recently presented at multiple security conferences:
https://portswigger.net/blog/http-desync-attacks-request-smuggling-reborn

net/http servers already reject header keys with invalid characters.
Simply stop normalizing extra spaces in net/textproto, let it return them
unchanged like it does for other invalid headers, and let net/http enforce
RFC 7230, which is HTTP specific. This loses us normalization on the client
side, but there's no right answer on the client side anyway, and hiding the
issue sounds worse than letting the application decide.

full diff: golang/go@go1.12.9...go1.12.10

```
Hi gophers,

We have just released Go 1.13.1 and Go 1.12.10 to address a recently reported security issue. We recommend that all affected users update to one of these releases (if you're not sure which, choose Go 1.13.1).

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 an uncommon 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 upstream connection by the proxy. Such invalid headers are now rejected by Go servers, and passed without normalization to Go client applications.

The issue is CVE-2019-16276 and Go issue golang.org/issue/34540.

Thanks to Andrew Stucki, Adam Scarr (99designs.com), and Jan Masarik (masarik.sh) for discovering and reporting this issue.

Downloads are available at https://golang.org/dl for all supported platforms.

Alla prossima,
Filippo on behalf of the Go team
```

From the patch: golang/go@6e6f4aa

```
net/textproto: don't normalize headers with spaces before the colon

RFC 7230 is clear about headers with a space before the colon, like

X-Answer : 42

being invalid, but we've been accepting and normalizing them for compatibility
purposes since CL 5690059 in 2012.

On the client side, this is harmless and indeed most browsers behave the same
to this day. On the server side, this becomes a security issue when the
behavior doesn't match that of a reverse proxy sitting in front of the server.

For example, if a WAF accepts them without normalizing them, it might be
possible to bypass its filters, because the Go server would interpret the
header differently. Worse, if the reverse proxy coalesces requests onto a
single HTTP/1.1 connection to a Go server, the understanding of the request
boundaries can get out of sync between them, allowing an attacker to tack an
arbitrary method and path onto a request by other clients, including
authentication headers unknown to the attacker.

This was recently presented at multiple security conferences:
https://portswigger.net/blog/http-desync-attacks-request-smuggling-reborn

net/http servers already reject header keys with invalid characters.
Simply stop normalizing extra spaces in net/textproto, let it return them
unchanged like it does for other invalid headers, and let net/http enforce
RFC 7230, which is HTTP specific. This loses us normalization on the client
side, but there's no right answer on the client side anyway, and hiding the
issue sounds worse than letting the application decide.
```

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Sep 26, 2019

Build succeeded.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Sep 26, 2019

Codecov Report

Merging #3686 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #3686   +/-   ##
======================================
  Coverage    42.1%   42.1%           
======================================
  Files         129     129           
  Lines       14307   14307           
======================================
  Hits         6024    6024           
  Misses       7383    7383           
  Partials      900     900
Flag Coverage Δ
#linux 45.61% <ø> (ø) ⬆️
#windows 37.05% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a46765...35d3bae. Read the comment docs.

@crosbymichael
Copy link
Copy Markdown
Member

LGTM

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit 277ab9e into containerd:master Sep 26, 2019
@estesp estesp added this to the 1.3 milestone Sep 26, 2019
@thaJeztah thaJeztah deleted the bump_golang_1.12.10 branch September 26, 2019 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants