diff options
author | Archana Polampalli <archana.polampalli@windriver.com> | 2024-08-08 11:05:44 +0000 |
---|---|---|
committer | Steve Sakoman <steve@sakoman.com> | 2024-08-16 08:09:14 -0700 |
commit | 3be2b60b66e08006ad7efef472b7af92d28a174c (patch) | |
tree | c71efd6699629a590b2c13da3f85d437cceb6aa9 | |
parent | 5c036f07cc71c472be72f63d085b6a65afc5ce81 (diff) | |
download | poky-3be2b60b66e08006ad7efef472b7af92d28a174c.tar.gz |
go: fix CVE-2024-24791
(From OE-Core rev: f012f6a6e1d3111d6cae74c9c846d8bd0fca5dd5)
Signed-off-by: Archana Polampalli <archana.polampalli@windriver.com>
Signed-off-by: Steve Sakoman <steve@sakoman.com>
-rw-r--r-- | meta/recipes-devtools/go/go-1.17.13.inc | 1 | ||||
-rw-r--r-- | meta/recipes-devtools/go/go-1.21/CVE-2024-24791.patch | 359 |
2 files changed, 360 insertions, 0 deletions
diff --git a/meta/recipes-devtools/go/go-1.17.13.inc b/meta/recipes-devtools/go/go-1.17.13.inc index e83c4dfa80..349b0be6be 100644 --- a/meta/recipes-devtools/go/go-1.17.13.inc +++ b/meta/recipes-devtools/go/go-1.17.13.inc | |||
@@ -57,6 +57,7 @@ SRC_URI += "\ | |||
57 | file://CVE-2024-24785.patch \ | 57 | file://CVE-2024-24785.patch \ |
58 | file://CVE-2023-45288.patch \ | 58 | file://CVE-2023-45288.patch \ |
59 | file://CVE-2024-24789.patch \ | 59 | file://CVE-2024-24789.patch \ |
60 | file://CVE-2024-24791.patch \ | ||
60 | " | 61 | " |
61 | SRC_URI[main.sha256sum] = "a1a48b23afb206f95e7bbaa9b898d965f90826f6f1d1fc0c1d784ada0cd300fd" | 62 | SRC_URI[main.sha256sum] = "a1a48b23afb206f95e7bbaa9b898d965f90826f6f1d1fc0c1d784ada0cd300fd" |
62 | 63 | ||
diff --git a/meta/recipes-devtools/go/go-1.21/CVE-2024-24791.patch b/meta/recipes-devtools/go/go-1.21/CVE-2024-24791.patch new file mode 100644 index 0000000000..9f6a969cf6 --- /dev/null +++ b/meta/recipes-devtools/go/go-1.21/CVE-2024-24791.patch | |||
@@ -0,0 +1,359 @@ | |||
1 | From c9be6ae748b7679b644a38182d456cb5a6ac06ee Mon Sep 17 00:00:00 2001 | ||
2 | From: Damien Neil <dneil@google.com> | ||
3 | Date: Thu, 6 Jun 2024 12:50:46 -0700 | ||
4 | Subject: [PATCH] [release-branch.go1.21] net/http: send body or close | ||
5 | connection on expect-100-continue requests | ||
6 | |||
7 | When sending a request with an "Expect: 100-continue" header, | ||
8 | we must send the request body before sending any further requests | ||
9 | on the connection. | ||
10 | |||
11 | When receiving a non-1xx response to an "Expect: 100-continue" request, | ||
12 | send the request body if the connection isn't being closed after | ||
13 | processing the response. In other words, if either the request | ||
14 | or response contains a "Connection: close" header, then skip sending | ||
15 | the request body (because the connection will not be used for | ||
16 | further requests), but otherwise send it. | ||
17 | |||
18 | Correct a comment on the server-side Expect: 100-continue handling | ||
19 | that implied sending the request body is optional. It isn't. | ||
20 | |||
21 | For #67555 | ||
22 | Fixes #68199 | ||
23 | |||
24 | Change-Id: Ia2f12091bee697771087f32ac347509ec5922d54 | ||
25 | Reviewed-on: https://go-review.googlesource.com/c/go/+/591255 | ||
26 | LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> | ||
27 | Reviewed-by: Jonathan Amsterdam <jba@google.com> | ||
28 | (cherry picked from commit cf501e05e138e6911f759a5db786e90b295499b9) | ||
29 | Reviewed-on: https://go-review.googlesource.com/c/go/+/595096 | ||
30 | Reviewed-by: Joedian Reid <joedian@google.com> | ||
31 | Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> | ||
32 | |||
33 | CVE: CVE-2024-24791 | ||
34 | |||
35 | Upstream-Status: Backport [https://github.com/golang/go/commit/c9be6ae748b7679b644a38182d456cb5a6ac06ee ] | ||
36 | |||
37 | Signed-off-by: Archana Polampalli <archana.polampalli@windriver.com> | ||
38 | --- | ||
39 | src/net/http/server.go | 25 ++-- | ||
40 | src/net/http/transport.go | 34 ++++-- | ||
41 | src/net/http/transport_test.go | 203 ++++++++++++++++++++------------- | ||
42 | 3 files changed, 164 insertions(+), 98 deletions(-) | ||
43 | |||
44 | diff --git a/src/net/http/server.go b/src/net/http/server.go | ||
45 | index 4fc8fed..1648f1c 100644 | ||
46 | --- a/src/net/http/server.go | ||
47 | +++ b/src/net/http/server.go | ||
48 | @@ -1297,16 +1297,21 @@ func (cw *chunkWriter) writeHeader(p []byte) { | ||
49 | |||
50 | // If the client wanted a 100-continue but we never sent it to | ||
51 | // them (or, more strictly: we never finished reading their | ||
52 | - // request body), don't reuse this connection because it's now | ||
53 | - // in an unknown state: we might be sending this response at | ||
54 | - // the same time the client is now sending its request body | ||
55 | - // after a timeout. (Some HTTP clients send Expect: | ||
56 | - // 100-continue but knowing that some servers don't support | ||
57 | - // it, the clients set a timer and send the body later anyway) | ||
58 | - // If we haven't seen EOF, we can't skip over the unread body | ||
59 | - // because we don't know if the next bytes on the wire will be | ||
60 | - // the body-following-the-timer or the subsequent request. | ||
61 | - // See Issue 11549. | ||
62 | + // request body), don't reuse this connection. | ||
63 | + // | ||
64 | + // This behavior was first added on the theory that we don't know | ||
65 | + // if the next bytes on the wire are going to be the remainder of | ||
66 | + // the request body or the subsequent request (see issue 11549), | ||
67 | + // but that's not correct: If we keep using the connection, | ||
68 | + // the client is required to send the request body whether we | ||
69 | + // asked for it or not. | ||
70 | + // | ||
71 | + // We probably do want to skip reusing the connection in most cases, | ||
72 | + // however. If the client is offering a large request body that we | ||
73 | + // don't intend to use, then it's better to close the connection | ||
74 | + // than to read the body. For now, assume that if we're sending | ||
75 | + // headers, the handler is done reading the body and we should | ||
76 | + // drop the connection if we haven't seen EOF. | ||
77 | if ecr, ok := w.req.Body.(*expectContinueReader); ok && !ecr.sawEOF.isSet() { | ||
78 | w.closeAfterReply = true | ||
79 | } | ||
80 | diff --git a/src/net/http/transport.go b/src/net/http/transport.go | ||
81 | index 309194e..e46ddef 100644 | ||
82 | --- a/src/net/http/transport.go | ||
83 | +++ b/src/net/http/transport.go | ||
84 | @@ -2282,17 +2282,12 @@ func (pc *persistConn) readResponse(rc requestAndChan, trace *httptrace.ClientTr | ||
85 | return | ||
86 | } | ||
87 | resCode := resp.StatusCode | ||
88 | - if continueCh != nil { | ||
89 | - if resCode == 100 { | ||
90 | - if trace != nil && trace.Got100Continue != nil { | ||
91 | - trace.Got100Continue() | ||
92 | - } | ||
93 | - continueCh <- struct{}{} | ||
94 | - continueCh = nil | ||
95 | - } else if resCode >= 200 { | ||
96 | - close(continueCh) | ||
97 | - continueCh = nil | ||
98 | + if continueCh != nil && resCode == StatusContinue { | ||
99 | + if trace != nil && trace.Got100Continue != nil { | ||
100 | + trace.Got100Continue() | ||
101 | } | ||
102 | + continueCh <- struct{}{} | ||
103 | + continueCh = nil | ||
104 | } | ||
105 | is1xx := 100 <= resCode && resCode <= 199 | ||
106 | // treat 101 as a terminal status, see issue 26161 | ||
107 | @@ -2315,6 +2310,25 @@ func (pc *persistConn) readResponse(rc requestAndChan, trace *httptrace.ClientTr | ||
108 | if resp.isProtocolSwitch() { | ||
109 | resp.Body = newReadWriteCloserBody(pc.br, pc.conn) | ||
110 | } | ||
111 | + if continueCh != nil { | ||
112 | + // We send an "Expect: 100-continue" header, but the server | ||
113 | + // responded with a terminal status and no 100 Continue. | ||
114 | + // | ||
115 | + // If we're going to keep using the connection, we need to send the request body. | ||
116 | + // Tell writeLoop to skip sending the body if we're going to close the connection, | ||
117 | + // or to send it otherwise. | ||
118 | + // | ||
119 | + // The case where we receive a 101 Switching Protocols response is a bit | ||
120 | + // ambiguous, since we don't know what protocol we're switching to. | ||
121 | + // Conceivably, it's one that doesn't need us to send the body. | ||
122 | + // Given that we'll send the body if ExpectContinueTimeout expires, | ||
123 | + // be consistent and always send it if we aren't closing the connection. | ||
124 | + if resp.Close || rc.req.Close { | ||
125 | + close(continueCh) // don't send the body; the connection will close | ||
126 | + } else { | ||
127 | + continueCh <- struct{}{} // send the body | ||
128 | + } | ||
129 | + } | ||
130 | |||
131 | resp.TLS = pc.tlsState | ||
132 | return | ||
133 | diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go | ||
134 | index 58f12af..8000ecc 100644 | ||
135 | --- a/src/net/http/transport_test.go | ||
136 | +++ b/src/net/http/transport_test.go | ||
137 | @@ -1130,95 +1130,142 @@ func TestTransportGzip(t *testing.T) { | ||
138 | } | ||
139 | } | ||
140 | |||
141 | -// If a request has Expect:100-continue header, the request blocks sending body until the first response. | ||
142 | -// Premature consumption of the request body should not be occurred. | ||
143 | -func TestTransportExpect100Continue(t *testing.T) { | ||
144 | - setParallel(t) | ||
145 | - defer afterTest(t) | ||
146 | +// A transport100Continue test exercises Transport behaviors when sending a | ||
147 | +// request with an Expect: 100-continue header. | ||
148 | +type transport100ContinueTest struct { | ||
149 | + t *testing.T | ||
150 | |||
151 | - ts := httptest.NewServer(HandlerFunc(func(rw ResponseWriter, req *Request) { | ||
152 | - switch req.URL.Path { | ||
153 | - case "/100": | ||
154 | - // This endpoint implicitly responds 100 Continue and reads body. | ||
155 | - if _, err := io.Copy(io.Discard, req.Body); err != nil { | ||
156 | - t.Error("Failed to read Body", err) | ||
157 | - } | ||
158 | - rw.WriteHeader(StatusOK) | ||
159 | - case "/200": | ||
160 | - // Go 1.5 adds Connection: close header if the client expect | ||
161 | - // continue but not entire request body is consumed. | ||
162 | - rw.WriteHeader(StatusOK) | ||
163 | - case "/500": | ||
164 | - rw.WriteHeader(StatusInternalServerError) | ||
165 | - case "/keepalive": | ||
166 | - // This hijacked endpoint responds error without Connection:close. | ||
167 | - _, bufrw, err := rw.(Hijacker).Hijack() | ||
168 | - if err != nil { | ||
169 | - log.Fatal(err) | ||
170 | - } | ||
171 | - bufrw.WriteString("HTTP/1.1 500 Internal Server Error\r\n") | ||
172 | - bufrw.WriteString("Content-Length: 0\r\n\r\n") | ||
173 | - bufrw.Flush() | ||
174 | - case "/timeout": | ||
175 | - // This endpoint tries to read body without 100 (Continue) response. | ||
176 | - // After ExpectContinueTimeout, the reading will be started. | ||
177 | - conn, bufrw, err := rw.(Hijacker).Hijack() | ||
178 | - if err != nil { | ||
179 | - log.Fatal(err) | ||
180 | - } | ||
181 | - if _, err := io.CopyN(io.Discard, bufrw, req.ContentLength); err != nil { | ||
182 | - t.Error("Failed to read Body", err) | ||
183 | - } | ||
184 | - bufrw.WriteString("HTTP/1.1 200 OK\r\n\r\n") | ||
185 | - bufrw.Flush() | ||
186 | - conn.Close() | ||
187 | - } | ||
188 | + reqdone chan struct{} | ||
189 | + resp *Response | ||
190 | + respErr error | ||
191 | |||
192 | - })) | ||
193 | - defer ts.Close() | ||
194 | + conn net.Conn | ||
195 | + reader *bufio.Reader | ||
196 | +} | ||
197 | |||
198 | - tests := []struct { | ||
199 | - path string | ||
200 | - body []byte | ||
201 | - sent int | ||
202 | - status int | ||
203 | - }{ | ||
204 | - {path: "/100", body: []byte("hello"), sent: 5, status: 200}, // Got 100 followed by 200, entire body is sent. | ||
205 | - {path: "/200", body: []byte("hello"), sent: 0, status: 200}, // Got 200 without 100. body isn't sent. | ||
206 | - {path: "/500", body: []byte("hello"), sent: 0, status: 500}, // Got 500 without 100. body isn't sent. | ||
207 | - {path: "/keepalive", body: []byte("hello"), sent: 0, status: 500}, // Although without Connection:close, body isn't sent. | ||
208 | - {path: "/timeout", body: []byte("hello"), sent: 5, status: 200}, // Timeout exceeded and entire body is sent. | ||
209 | +const transport100ContinueTestBody = "request body" | ||
210 | + | ||
211 | +// newTransport100ContinueTest creates a Transport and sends an Expect: 100-continue | ||
212 | +// request on it. | ||
213 | +func newTransport100ContinueTest(t *testing.T, timeout time.Duration) *transport100ContinueTest { | ||
214 | + ln := newLocalListener(t) | ||
215 | + defer ln.Close() | ||
216 | + | ||
217 | + test := &transport100ContinueTest{ | ||
218 | + t: t, | ||
219 | + reqdone: make(chan struct{}), | ||
220 | } | ||
221 | |||
222 | - c := ts.Client() | ||
223 | - for i, v := range tests { | ||
224 | - tr := &Transport{ | ||
225 | - ExpectContinueTimeout: 2 * time.Second, | ||
226 | - } | ||
227 | - defer tr.CloseIdleConnections() | ||
228 | - c.Transport = tr | ||
229 | - body := bytes.NewReader(v.body) | ||
230 | - req, err := NewRequest("PUT", ts.URL+v.path, body) | ||
231 | - if err != nil { | ||
232 | - t.Fatal(err) | ||
233 | - } | ||
234 | + tr := &Transport{ | ||
235 | + ExpectContinueTimeout: timeout, | ||
236 | + } | ||
237 | + go func() { | ||
238 | + defer close(test.reqdone) | ||
239 | + body := strings.NewReader(transport100ContinueTestBody) | ||
240 | + req, _ := NewRequest("PUT", "http://"+ln.Addr().String(), body) | ||
241 | req.Header.Set("Expect", "100-continue") | ||
242 | - req.ContentLength = int64(len(v.body)) | ||
243 | + req.ContentLength = int64(len(transport100ContinueTestBody)) | ||
244 | + test.resp, test.respErr = tr.RoundTrip(req) | ||
245 | + test.resp.Body.Close() | ||
246 | + }() | ||
247 | |||
248 | - resp, err := c.Do(req) | ||
249 | - if err != nil { | ||
250 | - t.Fatal(err) | ||
251 | + c, err := ln.Accept() | ||
252 | + if err != nil { | ||
253 | + t.Fatalf("Accept: %v", err) | ||
254 | + } | ||
255 | + t.Cleanup(func() { | ||
256 | + c.Close() | ||
257 | + }) | ||
258 | + br := bufio.NewReader(c) | ||
259 | + _, err = ReadRequest(br) | ||
260 | + if err != nil { | ||
261 | + t.Fatalf("ReadRequest: %v", err) | ||
262 | + } | ||
263 | + test.conn = c | ||
264 | + test.reader = br | ||
265 | + t.Cleanup(func() { | ||
266 | + <-test.reqdone | ||
267 | + tr.CloseIdleConnections() | ||
268 | + got, _ := io.ReadAll(test.reader) | ||
269 | + if len(got) > 0 { | ||
270 | + t.Fatalf("Transport sent unexpected bytes: %q", got) | ||
271 | } | ||
272 | - resp.Body.Close() | ||
273 | + }) | ||
274 | |||
275 | - sent := len(v.body) - body.Len() | ||
276 | - if v.status != resp.StatusCode { | ||
277 | - t.Errorf("test %d: status code should be %d but got %d. (%s)", i, v.status, resp.StatusCode, v.path) | ||
278 | - } | ||
279 | - if v.sent != sent { | ||
280 | - t.Errorf("test %d: sent body should be %d but sent %d. (%s)", i, v.sent, sent, v.path) | ||
281 | + return test | ||
282 | +} | ||
283 | + | ||
284 | +// respond sends response lines from the server to the transport. | ||
285 | +func (test *transport100ContinueTest) respond(lines ...string) { | ||
286 | + for _, line := range lines { | ||
287 | + if _, err := test.conn.Write([]byte(line + "\r\n")); err != nil { | ||
288 | + test.t.Fatalf("Write: %v", err) | ||
289 | } | ||
290 | } | ||
291 | + if _, err := test.conn.Write([]byte("\r\n")); err != nil { | ||
292 | + test.t.Fatalf("Write: %v", err) | ||
293 | + } | ||
294 | +} | ||
295 | + | ||
296 | +// wantBodySent ensures the transport has sent the request body to the server. | ||
297 | +func (test *transport100ContinueTest) wantBodySent() { | ||
298 | + got, err := io.ReadAll(io.LimitReader(test.reader, int64(len(transport100ContinueTestBody)))) | ||
299 | + if err != nil { | ||
300 | + test.t.Fatalf("unexpected error reading body: %v", err) | ||
301 | + } | ||
302 | + if got, want := string(got), transport100ContinueTestBody; got != want { | ||
303 | + test.t.Fatalf("unexpected body: got %q, want %q", got, want) | ||
304 | + } | ||
305 | +} | ||
306 | + | ||
307 | +// wantRequestDone ensures the Transport.RoundTrip has completed with the expected status. | ||
308 | +func (test *transport100ContinueTest) wantRequestDone(want int) { | ||
309 | + <-test.reqdone | ||
310 | + if test.respErr != nil { | ||
311 | + test.t.Fatalf("unexpected RoundTrip error: %v", test.respErr) | ||
312 | + } | ||
313 | + if got := test.resp.StatusCode; got != want { | ||
314 | + test.t.Fatalf("unexpected response code: got %v, want %v", got, want) | ||
315 | + } | ||
316 | +} | ||
317 | + | ||
318 | +func TestTransportExpect100ContinueSent(t *testing.T) { | ||
319 | + test := newTransport100ContinueTest(t, 1*time.Hour) | ||
320 | + // Server sends a 100 Continue response, and the client sends the request body. | ||
321 | + test.respond("HTTP/1.1 100 Continue") | ||
322 | + test.wantBodySent() | ||
323 | + test.respond("HTTP/1.1 200", "Content-Length: 0") | ||
324 | + test.wantRequestDone(200) | ||
325 | +} | ||
326 | + | ||
327 | +func TestTransportExpect100Continue200ResponseNoConnClose(t *testing.T) { | ||
328 | + test := newTransport100ContinueTest(t, 1*time.Hour) | ||
329 | + // No 100 Continue response, no Connection: close header. | ||
330 | + test.respond("HTTP/1.1 200", "Content-Length: 0") | ||
331 | + test.wantBodySent() | ||
332 | + test.wantRequestDone(200) | ||
333 | +} | ||
334 | + | ||
335 | +func TestTransportExpect100Continue200ResponseWithConnClose(t *testing.T) { | ||
336 | + test := newTransport100ContinueTest(t, 1*time.Hour) | ||
337 | + // No 100 Continue response, Connection: close header set. | ||
338 | + test.respond("HTTP/1.1 200", "Connection: close", "Content-Length: 0") | ||
339 | + test.wantRequestDone(200) | ||
340 | +} | ||
341 | + | ||
342 | +func TestTransportExpect100Continue500ResponseNoConnClose(t *testing.T) { | ||
343 | + test := newTransport100ContinueTest(t, 1*time.Hour) | ||
344 | + // No 100 Continue response, no Connection: close header. | ||
345 | + test.respond("HTTP/1.1 500", "Content-Length: 0") | ||
346 | + test.wantBodySent() | ||
347 | + test.wantRequestDone(500) | ||
348 | +} | ||
349 | + | ||
350 | +func TestTransportExpect100Continue500ResponseTimeout(t *testing.T) { | ||
351 | + test := newTransport100ContinueTest(t, 5*time.Millisecond) // short timeout | ||
352 | + test.wantBodySent() // after timeout | ||
353 | + test.respond("HTTP/1.1 200", "Content-Length: 0") | ||
354 | + test.wantRequestDone(200) | ||
355 | } | ||
356 | |||
357 | func TestSOCKS5Proxy(t *testing.T) { | ||
358 | -- | ||
359 | 2.40.0 | ||