packages/golang
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

!402 [Backport]fix CVE-2024-24791

Browse Source 
From: @hcnbxx 
Reviewed-by: @jing-rui 
Signed-off-by: @jing-rui
This commit is contained in:
openeuler-ci-bot 2024-08-13 02:24:18 +00:00 committed by Gitee
commit 5fac4acb73
No known key found for this signature in database
GPG Key ID: 173E9B9CA92EEF8F
2 changed files with 372 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

364
0067-Backport-net-http-send-body-or-close-connection-on-e.patch Normal file
View File

@ -0,0 +1,364 @@
From 31d1f45b25f05a20429cd31bcf0cc2927e589390 Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Fri, 7 Jun 2024 03:50:46 +0800
Subject: [PATCH] [Backport] net/http: send body or close connection on
expect-100-continue requests
CVE: CVE-2024-24791
Reference: https://go-review.googlesource.com/c/go/+/595096
When sending a request with an "Expect: 100-continue" header,
we must send the request body before sending any further requests
on the connection.
When receiving a non-1xx response to an "Expect: 100-continue" request,
send the request body if the connection isn't being closed after
processing the response. In other words, if either the request
or response contains a "Connection: close" header, then skip sending
the request body (because the connection will not be used for
further requests), but otherwise send it.
Correct a comment on the server-side Expect: 100-continue handling
that implied sending the request body is optional. It isn't.
Note: The upstream does not submit this change to go1.17 according to the rules of MinorReleases.
Corego3.x are based on go1.17.8. Therefore, it need to submit the change to corego3.x.
Edited-by: machangwang m00509938
For #67555
Fixes #68199
Fixes CVE-2024-24791
Change-Id: Ia2f12091bee697771087f32ac347509ec5922d54
Reviewed-on: https://go-review.googlesource.com/c/go/+/591255
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
(cherry picked from commit cf501e05e138e6911f759a5db786e90b295499b9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/595096
Reviewed-by: Joedian Reid <joedian@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Signed-off-by: Ma Chang Wang machangwang@huawei.com
---
src/net/http/server.go | 25 ++--
src/net/http/transport.go | 34 ++++--
src/net/http/transport_test.go | 203 ++++++++++++++++++++-------------
3 files changed, 164 insertions(+), 98 deletions(-)
diff --git a/src/net/http/server.go b/src/net/http/server.go
index 5b113cff97..933eb97d92 100644
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -1297,16 +1297,21 @@ func (cw *chunkWriter) writeHeader(p []byte) {
// If the client wanted a 100-continue but we never sent it to
// them (or, more strictly: we never finished reading their
- // request body), don't reuse this connection because it's now
- // in an unknown state: we might be sending this response at
- // the same time the client is now sending its request body
- // after a timeout. (Some HTTP clients send Expect:
- // 100-continue but knowing that some servers don't support
- // it, the clients set a timer and send the body later anyway)
- // If we haven't seen EOF, we can't skip over the unread body
- // because we don't know if the next bytes on the wire will be
- // the body-following-the-timer or the subsequent request.
- // See Issue 11549.
+ // request body), don't reuse this connection.
+ //
+ // This behavior was first added on the theory that we don't know
+ // if the next bytes on the wire are going to be the remainder of
+ // the request body or the subsequent request (see issue 11549),
+ // but that's not correct: If we keep using the connection,
+ // the client is required to send the request body whether we
+ // asked for it or not.
+ //
+ // We probably do want to skip reusing the connection in most cases,
+ // however. If the client is offering a large request body that we
+ // don't intend to use, then it's better to close the connection
+ // than to read the body. For now, assume that if we're sending
+ // headers, the handler is done reading the body and we should
+ // drop the connection if we haven't seen EOF.
if ecr, ok := w.req.Body.(*expectContinueReader); ok && !ecr.sawEOF.isSet() {
w.closeAfterReply = true
}
diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 6cf5e65276..fa5cb30d2a 100644
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -2282,17 +2282,12 @@ func (pc *persistConn) readResponse(rc requestAndChan, trace *httptrace.ClientTr
return
}
resCode := resp.StatusCode
- if continueCh != nil {
- if resCode == 100 {
- if trace != nil && trace.Got100Continue != nil {
- trace.Got100Continue()
- }
- continueCh <- struct{}{}
- continueCh = nil
- } else if resCode >= 200 {
- close(continueCh)
- continueCh = nil
+ if continueCh != nil && resCode == StatusContinue {
+ if trace != nil && trace.Got100Continue != nil {
+ trace.Got100Continue()
}
+ continueCh <- struct{}{}
+ continueCh = nil
}
is1xx := 100 <= resCode && resCode <= 199
// treat 101 as a terminal status, see issue 26161
@@ -2315,6 +2310,25 @@ func (pc *persistConn) readResponse(rc requestAndChan, trace *httptrace.ClientTr
if resp.isProtocolSwitch() {
resp.Body = newReadWriteCloserBody(pc.br, pc.conn)
}
+ if continueCh != nil {
+ // We send an "Expect: 100-continue" header, but the server
+ // responded with a terminal status and no 100 Continue.
+ //
+ // If we're going to keep using the connection, we need to send the request body.
+ // Tell writeLoop to skip sending the body if we're going to close the connection,
+ // or to send it otherwise.
+ //
+ // The case where we receive a 101 Switching Protocols response is a bit
+ // ambiguous, since we don't know what protocol we're switching to.
+ // Conceivably, it's one that doesn't need us to send the body.
+ // Given that we'll send the body if ExpectContinueTimeout expires,
+ // be consistent and always send it if we aren't closing the connection.
+ if resp.Close || rc.req.Close {
+ close(continueCh) // don't send the body; the connection will close
+ } else {
+ continueCh <- struct{}{} // send the body
+ }
+ }
resp.TLS = pc.tlsState
return
diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index 58f12afd53..8000ecccfa 100644
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -1130,95 +1130,142 @@ func TestTransportGzip(t *testing.T) {
}
}
-// If a request has Expect:100-continue header, the request blocks sending body until the first response.
-// Premature consumption of the request body should not be occurred.
-func TestTransportExpect100Continue(t *testing.T) {
- setParallel(t)
- defer afterTest(t)
+// A transport100Continue test exercises Transport behaviors when sending a
+// request with an Expect: 100-continue header.
+type transport100ContinueTest struct {
+ t *testing.T
- ts := httptest.NewServer(HandlerFunc(func(rw ResponseWriter, req *Request) {
- switch req.URL.Path {
- case "/100":
- // This endpoint implicitly responds 100 Continue and reads body.
- if _, err := io.Copy(io.Discard, req.Body); err != nil {
- t.Error("Failed to read Body", err)
- }
- rw.WriteHeader(StatusOK)
- case "/200":
- // Go 1.5 adds Connection: close header if the client expect
- // continue but not entire request body is consumed.
- rw.WriteHeader(StatusOK)
- case "/500":
- rw.WriteHeader(StatusInternalServerError)
- case "/keepalive":
- // This hijacked endpoint responds error without Connection:close.
- _, bufrw, err := rw.(Hijacker).Hijack()
- if err != nil {
- log.Fatal(err)
- }
- bufrw.WriteString("HTTP/1.1 500 Internal Server Error\r\n")
- bufrw.WriteString("Content-Length: 0\r\n\r\n")
- bufrw.Flush()
- case "/timeout":
- // This endpoint tries to read body without 100 (Continue) response.
- // After ExpectContinueTimeout, the reading will be started.
- conn, bufrw, err := rw.(Hijacker).Hijack()
- if err != nil {
- log.Fatal(err)
- }
- if _, err := io.CopyN(io.Discard, bufrw, req.ContentLength); err != nil {
- t.Error("Failed to read Body", err)
- }
- bufrw.WriteString("HTTP/1.1 200 OK\r\n\r\n")
- bufrw.Flush()
- conn.Close()
- }
+ reqdone chan struct{}
+ resp *Response
+ respErr error
- }))
- defer ts.Close()
+ conn net.Conn
+ reader *bufio.Reader
+}
- tests := []struct {
- path string
- body []byte
- sent int
- status int
- }{
- {path: "/100", body: []byte("hello"), sent: 5, status: 200}, // Got 100 followed by 200, entire body is sent.
- {path: "/200", body: []byte("hello"), sent: 0, status: 200}, // Got 200 without 100. body isn't sent.
- {path: "/500", body: []byte("hello"), sent: 0, status: 500}, // Got 500 without 100. body isn't sent.
- {path: "/keepalive", body: []byte("hello"), sent: 0, status: 500}, // Although without Connection:close, body isn't sent.
- {path: "/timeout", body: []byte("hello"), sent: 5, status: 200}, // Timeout exceeded and entire body is sent.
+const transport100ContinueTestBody = "request body"
+
+// newTransport100ContinueTest creates a Transport and sends an Expect: 100-continue
+// request on it.
+func newTransport100ContinueTest(t *testing.T, timeout time.Duration) *transport100ContinueTest {
+ ln := newLocalListener(t)
+ defer ln.Close()
+
+ test := &transport100ContinueTest{
+ t: t,
+ reqdone: make(chan struct{}),
}
- c := ts.Client()
- for i, v := range tests {
- tr := &Transport{
- ExpectContinueTimeout: 2 * time.Second,
- }
- defer tr.CloseIdleConnections()
- c.Transport = tr
- body := bytes.NewReader(v.body)
- req, err := NewRequest("PUT", ts.URL+v.path, body)
- if err != nil {
- t.Fatal(err)
- }
+ tr := &Transport{
+ ExpectContinueTimeout: timeout,
+ }
+ go func() {
+ defer close(test.reqdone)
+ body := strings.NewReader(transport100ContinueTestBody)
+ req, _ := NewRequest("PUT", "http://"+ln.Addr().String(), body)
req.Header.Set("Expect", "100-continue")
- req.ContentLength = int64(len(v.body))
+ req.ContentLength = int64(len(transport100ContinueTestBody))
+ test.resp, test.respErr = tr.RoundTrip(req)
+ test.resp.Body.Close()
+ }()
- resp, err := c.Do(req)
- if err != nil {
- t.Fatal(err)
+ c, err := ln.Accept()
+ if err != nil {
+ t.Fatalf("Accept: %v", err)
+ }
+ t.Cleanup(func() {
+ c.Close()
+ })
+ br := bufio.NewReader(c)
+ _, err = ReadRequest(br)
+ if err != nil {
+ t.Fatalf("ReadRequest: %v", err)
+ }
+ test.conn = c
+ test.reader = br
+ t.Cleanup(func() {
+ <-test.reqdone
+ tr.CloseIdleConnections()
+ got, _ := io.ReadAll(test.reader)
+ if len(got) > 0 {
+ t.Fatalf("Transport sent unexpected bytes: %q", got)
}
- resp.Body.Close()
+ })
- sent := len(v.body) - body.Len()
- if v.status != resp.StatusCode {
- t.Errorf("test %d: status code should be %d but got %d. (%s)", i, v.status, resp.StatusCode, v.path)
- }
- if v.sent != sent {
- t.Errorf("test %d: sent body should be %d but sent %d. (%s)", i, v.sent, sent, v.path)
+ return test
+}
+
+// respond sends response lines from the server to the transport.
+func (test *transport100ContinueTest) respond(lines ...string) {
+ for _, line := range lines {
+ if _, err := test.conn.Write([]byte(line + "\r\n")); err != nil {
+ test.t.Fatalf("Write: %v", err)
}
}
+ if _, err := test.conn.Write([]byte("\r\n")); err != nil {
+ test.t.Fatalf("Write: %v", err)
+ }
+}
+
+// wantBodySent ensures the transport has sent the request body to the server.
+func (test *transport100ContinueTest) wantBodySent() {
+ got, err := io.ReadAll(io.LimitReader(test.reader, int64(len(transport100ContinueTestBody))))
+ if err != nil {
+ test.t.Fatalf("unexpected error reading body: %v", err)
+ }
+ if got, want := string(got), transport100ContinueTestBody; got != want {
+ test.t.Fatalf("unexpected body: got %q, want %q", got, want)
+ }
+}
+
+// wantRequestDone ensures the Transport.RoundTrip has completed with the expected status.
+func (test *transport100ContinueTest) wantRequestDone(want int) {
+ <-test.reqdone
+ if test.respErr != nil {
+ test.t.Fatalf("unexpected RoundTrip error: %v", test.respErr)
+ }
+ if got := test.resp.StatusCode; got != want {
+ test.t.Fatalf("unexpected response code: got %v, want %v", got, want)
+ }
+}
+
+func TestTransportExpect100ContinueSent(t *testing.T) {
+ test := newTransport100ContinueTest(t, 1*time.Hour)
+ // Server sends a 100 Continue response, and the client sends the request body.
+ test.respond("HTTP/1.1 100 Continue")
+ test.wantBodySent()
+ test.respond("HTTP/1.1 200", "Content-Length: 0")
+ test.wantRequestDone(200)
+}
+
+func TestTransportExpect100Continue200ResponseNoConnClose(t *testing.T) {
+ test := newTransport100ContinueTest(t, 1*time.Hour)
+ // No 100 Continue response, no Connection: close header.
+ test.respond("HTTP/1.1 200", "Content-Length: 0")
+ test.wantBodySent()
+ test.wantRequestDone(200)
+}
+
+func TestTransportExpect100Continue200ResponseWithConnClose(t *testing.T) {
+ test := newTransport100ContinueTest(t, 1*time.Hour)
+ // No 100 Continue response, Connection: close header set.
+ test.respond("HTTP/1.1 200", "Connection: close", "Content-Length: 0")
+ test.wantRequestDone(200)
+}
+
+func TestTransportExpect100Continue500ResponseNoConnClose(t *testing.T) {
+ test := newTransport100ContinueTest(t, 1*time.Hour)
+ // No 100 Continue response, no Connection: close header.
+ test.respond("HTTP/1.1 500", "Content-Length: 0")
+ test.wantBodySent()
+ test.wantRequestDone(500)
+}
+
+func TestTransportExpect100Continue500ResponseTimeout(t *testing.T) {
+ test := newTransport100ContinueTest(t, 5*time.Millisecond) // short timeout
+ test.wantBodySent() // after timeout
+ test.respond("HTTP/1.1 200", "Content-Length: 0")
+ test.wantRequestDone(200)
}
func TestSOCKS5Proxy(t *testing.T) {
--
2.33.0

9
golang.spec
View File

@ -63,7 +63,7 @@
Name: golang
Version: 1.17.3
Release: 34
Release: 35
Summary: The Go Programming Language
License: BSD and Public Domain
URL: https://golang.org/
@ -216,6 +216,7 @@ Patch6063: 0063-1.17-backport-runtime-adjust-netpollWaiters-after-go.patch
Patch6064: 0064-Backport-net-http-update-bundled-golang.org-x-net-ht.patch
Patch6065: 0065-Backport-cmd-go-disallow-lto_library-in-LDFLAGS.patch
Patch6066: 0066-Backport-archive-zip-treat-truncated-EOCDR-comment-a.patch
Patch6067: 0067-Backport-net-http-send-body-or-close-connection-on-e.patch
ExclusiveArch: %{golang_arches}
@ -454,6 +455,12 @@ fi
%files devel -f go-tests.list -f go-misc.list -f go-src.list
%changelog
* Tue Aug 06 2024 hanchao <hanchao63@huawei.com> - 1.17.3-35
- Type:CVE
- CVE:CVE-2024-24791
- SUG:NA
- DESC:fix CVE-2024-24791
* Mon Jun 24 2024 yangjiaqi <yangjiaqi16@huawei.com> - 1.17.3-34
- Type:CVE
- CVE:CVE-2024-24789