Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http: expect: 100-continue handling is broken in various ways [1.21 backport] #68199

Closed
gopherbot opened this issue Jun 26, 2024 · 4 comments
Labels
CherryPickApproved Used during the release process for point releases
Milestone

Comments

@gopherbot
Copy link
Contributor

@neild requested issue #67555 to be considered for backport to the next 1.21 minor release.

@gopherbot please open backport issues. This is a significant bug with no workaround.

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Jun 26, 2024
@gopherbot gopherbot added this to the Go1.21.12 milestone Jun 26, 2024
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/595096 mentions this issue: [release-branch.go1.21] net/http: send body or close connection on expect-100-continue requests

@joedian joedian added the CherryPickApproved Used during the release process for point releases label Jun 26, 2024
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Jun 26, 2024
gopherbot pushed a commit that referenced this issue Jun 26, 2024
…pect-100-continue requests

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.

For #67555
Fixes #68199

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 cf501e0)
Reviewed-on: https://go-review.googlesource.com/c/go/+/595096
Reviewed-by: Joedian Reid <joedian@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
@gopherbot
Copy link
Contributor Author

Closed by merging c9be6ae to release-branch.go1.21.

@thanm
Copy link
Contributor

thanm commented Jul 2, 2024

From the security team related to this issue:

The net/http HTTP/1.1 client mishandled the case where a server responds to a request with an "Expect: 100-continue" header with a non-informational (200 or higher) status. This mishandling could leave a client connection in an invalid state, where the next request sent on the connection will fail.

An attacker sending a request to a net/http/httputil.ReverseProxy proxy can exploit this mishandling to cause a denial of service by sending "Expect: 100-continue" requests which elicit a non-informational response from the backend. Each such request leaves the proxy with an invalid connection, and causes one subsequent request using that connection to fail.

Thanks to Geoff Franks for reporting this issue.

This is CVE-2024-24791.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CherryPickApproved Used during the release process for point releases
4 participants