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 /meta | |
| 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>
Diffstat (limited to 'meta')
| -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 | ||
