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: request body errors don't close a connection #11930

Closed
jeddenlea opened this issue Jul 30, 2015 · 5 comments

Comments

@jeddenlea
Copy link
Contributor

commented Jul 30, 2015

After an HTTP server parses a request's headers, it essentially passes control of the protocol handling to a request body Reader. Generally, this Reader either consumes up to the number bytes specified by Content-Length, or follows a chunked encoded entity.

But, errors encountered at this stage are completely ignored by the server. Broken connections are left in tact, and the server will attempt to read further requests from them.

This is a vector for request smuggling.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jul 30, 2015

I don't quite follow.

What type of errors?

Can you give a concrete example?

How can you trick a server into reading a further request? We shouldn't ever be reusing the connection unless the body has been consumed.

@jeddenlea

This comment has been minimized.

Copy link
Contributor Author

commented Jul 30, 2015

@jeddenlea

This comment has been minimized.

Copy link
Contributor Author

commented Jul 30, 2015

A simple example is TestRequestBodyReadErrorClosesConnection in https://go-review.googlesource.com/#/c/12865/1/src/net/http/serve_test.go.

Timeout errors are ignored, too. though. You can smuggle a request basically by doing:

POST / HTTP/1.1
Host: foo
Content-Lenght: 1000

[ pause for the server's request timeout ]
GET /quitquitquit HTTP/1.1
Host: foo
@gopherbot

This comment has been minimized.

Copy link

commented Jul 30, 2015

CL https://golang.org/cl/12865 mentions this issue.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jul 31, 2015

I do think we need to fix this for Go 1.5, now that I've looked at it. Timing sucks because I'm on vacation but I'll review the CL more when I get laptop net access again in a few hours.

@bradfitz bradfitz closed this in c2db5f4 Aug 2, 2015

@mikioh mikioh modified the milestones: Go1.5, Go1.5Maybe Aug 2, 2015

@golang golang locked and limited conversation to collaborators Aug 5, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants
You can’t perform that action at this time.