diff options
author | Chen Qi <Qi.Chen@windriver.com> | 2019-10-22 09:31:11 +0800 |
---|---|---|
committer | Richard Purdie <richard.purdie@linuxfoundation.org> | 2019-10-23 16:30:36 +0100 |
commit | 34d14c25ee896852fc31dcd860392f695b25aed4 (patch) | |
tree | 30e8315b343eb3b19cb42d9fbd430d3edd2fca75 | |
parent | 6b8c51ae0921e59423916e800cdfd7294882a53e (diff) | |
download | poky-34d14c25ee896852fc31dcd860392f695b25aed4.tar.gz |
go: fix CVE-2019-16276
(From OE-Core rev: e31f87e289dfd3bbca961e927447a9c7ba816d3f)
Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
-rw-r--r-- | meta/recipes-devtools/go/go-1.12.inc | 1 | ||||
-rw-r--r-- | meta/recipes-devtools/go/go-1.12/0001-release-branch.go1.12-security-net-textproto-don-t-n.patch | 163 |
2 files changed, 164 insertions, 0 deletions
diff --git a/meta/recipes-devtools/go/go-1.12.inc b/meta/recipes-devtools/go/go-1.12.inc index 39157ff882..ed14b175e6 100644 --- a/meta/recipes-devtools/go/go-1.12.inc +++ b/meta/recipes-devtools/go/go-1.12.inc | |||
@@ -16,6 +16,7 @@ SRC_URI += "\ | |||
16 | file://0006-cmd-dist-separate-host-and-target-builds.patch \ | 16 | file://0006-cmd-dist-separate-host-and-target-builds.patch \ |
17 | file://0007-cmd-go-make-GOROOT-precious-by-default.patch \ | 17 | file://0007-cmd-go-make-GOROOT-precious-by-default.patch \ |
18 | file://0008-use-GOBUILDMODE-to-set-buildmode.patch \ | 18 | file://0008-use-GOBUILDMODE-to-set-buildmode.patch \ |
19 | file://0001-release-branch.go1.12-security-net-textproto-don-t-n.patch \ | ||
19 | " | 20 | " |
20 | SRC_URI_append_libc-musl = " file://0009-ld-replace-glibc-dynamic-linker-with-musl.patch" | 21 | SRC_URI_append_libc-musl = " file://0009-ld-replace-glibc-dynamic-linker-with-musl.patch" |
21 | 22 | ||
diff --git a/meta/recipes-devtools/go/go-1.12/0001-release-branch.go1.12-security-net-textproto-don-t-n.patch b/meta/recipes-devtools/go/go-1.12/0001-release-branch.go1.12-security-net-textproto-don-t-n.patch new file mode 100644 index 0000000000..7b39dbd734 --- /dev/null +++ b/meta/recipes-devtools/go/go-1.12/0001-release-branch.go1.12-security-net-textproto-don-t-n.patch | |||
@@ -0,0 +1,163 @@ | |||
1 | From 265b691ac440bfb711d8de323346f7d72e620efe Mon Sep 17 00:00:00 2001 | ||
2 | From: Filippo Valsorda <filippo@golang.org> | ||
3 | Date: Thu, 12 Sep 2019 12:37:36 -0400 | ||
4 | Subject: [PATCH] [release-branch.go1.12-security] net/textproto: don't | ||
5 | normalize headers with spaces before the colon | ||
6 | |||
7 | RFC 7230 is clear about headers with a space before the colon, like | ||
8 | |||
9 | X-Answer : 42 | ||
10 | |||
11 | being invalid, but we've been accepting and normalizing them for compatibility | ||
12 | purposes since CL 5690059 in 2012. | ||
13 | |||
14 | On the client side, this is harmless and indeed most browsers behave the same | ||
15 | to this day. On the server side, this becomes a security issue when the | ||
16 | behavior doesn't match that of a reverse proxy sitting in front of the server. | ||
17 | |||
18 | For example, if a WAF accepts them without normalizing them, it might be | ||
19 | possible to bypass its filters, because the Go server would interpret the | ||
20 | header differently. Worse, if the reverse proxy coalesces requests onto a | ||
21 | single HTTP/1.1 connection to a Go server, the understanding of the request | ||
22 | boundaries can get out of sync between them, allowing an attacker to tack an | ||
23 | arbitrary method and path onto a request by other clients, including | ||
24 | authentication headers unknown to the attacker. | ||
25 | |||
26 | This was recently presented at multiple security conferences: | ||
27 | https://portswigger.net/blog/http-desync-attacks-request-smuggling-reborn | ||
28 | |||
29 | net/http servers already reject header keys with invalid characters. | ||
30 | Simply stop normalizing extra spaces in net/textproto, let it return them | ||
31 | unchanged like it does for other invalid headers, and let net/http enforce | ||
32 | RFC 7230, which is HTTP specific. This loses us normalization on the client | ||
33 | side, but there's no right answer on the client side anyway, and hiding the | ||
34 | issue sounds worse than letting the application decide. | ||
35 | |||
36 | Fixes CVE-2019-16276 | ||
37 | |||
38 | Change-Id: I6d272de827e0870da85d93df770d6a0e161bbcf1 | ||
39 | Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/549719 | ||
40 | Reviewed-by: Brad Fitzpatrick <bradfitz@google.com> | ||
41 | (cherry picked from commit 1280b868e82bf173ea3e988be3092d160ee66082) | ||
42 | Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/558776 | ||
43 | Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> | ||
44 | |||
45 | CVE: CVE-2019-16276 | ||
46 | |||
47 | Upstream-Status: Backport [https://github.com/golang/go/commit/6e6f4aaf70c8b1cc81e65a26332aa9409de03ad8] | ||
48 | |||
49 | Signed-off-by: Chen Qi <Qi.Chen@windriver.com> | ||
50 | --- | ||
51 | src/net/http/serve_test.go | 4 ++++ | ||
52 | src/net/http/transport_test.go | 27 +++++++++++++++++++++++++++ | ||
53 | src/net/textproto/reader.go | 10 ++-------- | ||
54 | src/net/textproto/reader_test.go | 13 ++++++------- | ||
55 | 4 files changed, 39 insertions(+), 15 deletions(-) | ||
56 | |||
57 | diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go | ||
58 | index 6eb0088a96..89bfdfbb82 100644 | ||
59 | --- a/src/net/http/serve_test.go | ||
60 | +++ b/src/net/http/serve_test.go | ||
61 | @@ -4748,6 +4748,10 @@ func TestServerValidatesHeaders(t *testing.T) { | ||
62 | {"foo\xffbar: foo\r\n", 400}, // binary in header | ||
63 | {"foo\x00bar: foo\r\n", 400}, // binary in header | ||
64 | {"Foo: " + strings.Repeat("x", 1<<21) + "\r\n", 431}, // header too large | ||
65 | + // Spaces between the header key and colon are not allowed. | ||
66 | + // See RFC 7230, Section 3.2.4. | ||
67 | + {"Foo : bar\r\n", 400}, | ||
68 | + {"Foo\t: bar\r\n", 400}, | ||
69 | |||
70 | {"foo: foo foo\r\n", 200}, // LWS space is okay | ||
71 | {"foo: foo\tfoo\r\n", 200}, // LWS tab is okay | ||
72 | diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go | ||
73 | index 5c329543e2..5e5438a708 100644 | ||
74 | --- a/src/net/http/transport_test.go | ||
75 | +++ b/src/net/http/transport_test.go | ||
76 | @@ -5133,3 +5133,30 @@ func TestTransportIgnores408(t *testing.T) { | ||
77 | } | ||
78 | t.Fatalf("timeout after %v waiting for Transport connections to die off", time.Since(t0)) | ||
79 | } | ||
80 | + | ||
81 | +func TestInvalidHeaderResponse(t *testing.T) { | ||
82 | + setParallel(t) | ||
83 | + defer afterTest(t) | ||
84 | + cst := newClientServerTest(t, h1Mode, HandlerFunc(func(w ResponseWriter, r *Request) { | ||
85 | + conn, buf, _ := w.(Hijacker).Hijack() | ||
86 | + buf.Write([]byte("HTTP/1.1 200 OK\r\n" + | ||
87 | + "Date: Wed, 30 Aug 2017 19:09:27 GMT\r\n" + | ||
88 | + "Content-Type: text/html; charset=utf-8\r\n" + | ||
89 | + "Content-Length: 0\r\n" + | ||
90 | + "Foo : bar\r\n\r\n")) | ||
91 | + buf.Flush() | ||
92 | + conn.Close() | ||
93 | + })) | ||
94 | + defer cst.close() | ||
95 | + res, err := cst.c.Get(cst.ts.URL) | ||
96 | + if err != nil { | ||
97 | + t.Fatal(err) | ||
98 | + } | ||
99 | + defer res.Body.Close() | ||
100 | + if v := res.Header.Get("Foo"); v != "" { | ||
101 | + t.Errorf(`unexpected "Foo" header: %q`, v) | ||
102 | + } | ||
103 | + if v := res.Header.Get("Foo "); v != "bar" { | ||
104 | + t.Errorf(`bad "Foo " header value: %q, want %q`, v, "bar") | ||
105 | + } | ||
106 | +} | ||
107 | diff --git a/src/net/textproto/reader.go b/src/net/textproto/reader.go | ||
108 | index 2c4f25d5ae..1a5e364cf7 100644 | ||
109 | --- a/src/net/textproto/reader.go | ||
110 | +++ b/src/net/textproto/reader.go | ||
111 | @@ -493,18 +493,12 @@ func (r *Reader) ReadMIMEHeader() (MIMEHeader, error) { | ||
112 | return m, err | ||
113 | } | ||
114 | |||
115 | - // Key ends at first colon; should not have trailing spaces | ||
116 | - // but they appear in the wild, violating specs, so we remove | ||
117 | - // them if present. | ||
118 | + // Key ends at first colon. | ||
119 | i := bytes.IndexByte(kv, ':') | ||
120 | if i < 0 { | ||
121 | return m, ProtocolError("malformed MIME header line: " + string(kv)) | ||
122 | } | ||
123 | - endKey := i | ||
124 | - for endKey > 0 && kv[endKey-1] == ' ' { | ||
125 | - endKey-- | ||
126 | - } | ||
127 | - key := canonicalMIMEHeaderKey(kv[:endKey]) | ||
128 | + key := canonicalMIMEHeaderKey(kv[:i]) | ||
129 | |||
130 | // As per RFC 7230 field-name is a token, tokens consist of one or more chars. | ||
131 | // We could return a ProtocolError here, but better to be liberal in what we | ||
132 | diff --git a/src/net/textproto/reader_test.go b/src/net/textproto/reader_test.go | ||
133 | index f85fbdc36d..b92fdcd3c7 100644 | ||
134 | --- a/src/net/textproto/reader_test.go | ||
135 | +++ b/src/net/textproto/reader_test.go | ||
136 | @@ -188,11 +188,10 @@ func TestLargeReadMIMEHeader(t *testing.T) { | ||
137 | } | ||
138 | } | ||
139 | |||
140 | -// Test that we read slightly-bogus MIME headers seen in the wild, | ||
141 | -// with spaces before colons, and spaces in keys. | ||
142 | +// TestReadMIMEHeaderNonCompliant checks that we don't normalize headers | ||
143 | +// with spaces before colons, and accept spaces in keys. | ||
144 | func TestReadMIMEHeaderNonCompliant(t *testing.T) { | ||
145 | - // Invalid HTTP response header as sent by an Axis security | ||
146 | - // camera: (this is handled by IE, Firefox, Chrome, curl, etc.) | ||
147 | + // These invalid headers will be rejected by net/http according to RFC 7230. | ||
148 | r := reader("Foo: bar\r\n" + | ||
149 | "Content-Language: en\r\n" + | ||
150 | "SID : 0\r\n" + | ||
151 | @@ -202,9 +201,9 @@ func TestReadMIMEHeaderNonCompliant(t *testing.T) { | ||
152 | want := MIMEHeader{ | ||
153 | "Foo": {"bar"}, | ||
154 | "Content-Language": {"en"}, | ||
155 | - "Sid": {"0"}, | ||
156 | - "Audio Mode": {"None"}, | ||
157 | - "Privilege": {"127"}, | ||
158 | + "SID ": {"0"}, | ||
159 | + "Audio Mode ": {"None"}, | ||
160 | + "Privilege ": {"127"}, | ||
161 | } | ||
162 | if !reflect.DeepEqual(m, want) || err != nil { | ||
163 | t.Fatalf("ReadMIMEHeader =\n%v, %v; want:\n%v", m, err, want) | ||