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: Denial of Service Protection in Go HTTP Servers #2093
Labels
Comments
Owner changed to @bradfitz. Status changed to Accepted. |
Thanks for filing this. This sort of stuff's been on my mind but I hadn't gotten around to it or filed specific bugs, so glad there is one now. It is better than it used to be, though... we used to not even have read/write timeouts, or a Server struct to hang new options off of. :) I run a public webserver with Go's http package so I'm quite motivated to fix this all. Feel free to add related stuff to this bug. |
Cap on header size is now in. It defaults to 1MB but can be set per-Server: http://code.google.com/p/go/source/detail?r=d499fb951a9e9ea0c370d15a7b1ccdfabfe71ba8 I don't see the need for LimitRequestBody, though. That's per-Handler to decide. Keeping this bug open, though, as there's still more to be done. |
Comment 4 by tav@espians.com: Thanks for enabling the cap on the header size Brad. Much appreciated! As for the handler enforcing a limit on the body, are you imagining that request.Body.Reader would be wrapped within the handler by an io.LimitReader? If so, could you make the http.body struct be public, i.e. http.Body. Then a handler could turn the request.Body ReadCloser into an http.Body and wrap/modify the Body.Reader... Or were you imagining something else? -- Thanks again, tav |
Comment 5 by tav@espians.com: By the way, the bug in http.Request.ParseMultipartForm is still present... |
Re comment 5, fixed: http://code.google.com/p/go/source/detail?r=48ec728c6 |
Change out for review to limit request body size: http://golang.org/cl/4921049 |
And in: http://code.google.com/p/go/source/detail?r=d67e691bae3f4fd115cdc6fee6f454738ebd94fb http://tip.goneat.org/pkg/http/#MaxBytesReader It's not on by default yet, though. For now, install a wrapper handler or at the beginning of your handler: req.Body = http.MaxBytesReader(rw, req.Body, 5 << 20) Keeping this bug open still, though. |
The main thing I still want to do before closing this is change the Server's behavior of trying really hard to maintain a keep-alive connection by fully consuming the client's body, regardless of its size. Currently an attacker can do a multi-gigabyte POST to some path, have the server reply with 401 Unauthorized, and then happily slurp up gigabytes just to throw them away. I'd like to change the behavior so that if the request body has not been fully consumed when WriteHeader is sent, it'll just be a Connection: close response instead. Alternatively, a threshold size under which we will slurp. I keep meaning to go look up what RFC 2616 says (if anything) about whether responses before requests are fully consumed are legal or not. I can't remember. |
They are, in certain circumstances. eg, post with a 1Gb body can result in a server sending a 413 if it isn't prepared to accept that much data. However, that presupposes that the client send a Content-Length: header, or better sent an Expect: header. With Transfer-Encoding: chunked, things get less clear. RFC 2616 says the server MAY close the connection. However in the presence of things like pipelining, imho the server MUST close the connection, not just shutdown it's send side to make it clear the request isn't acceptable. |
I think this will the final fix for this bug, unless anybody can think of other DoS vectors: http://golang.org/cl/5268043 |
This issue was closed by revision 5079129. Status changed to Fixed. |
Comment 14 by tav@espians.com: Thanks for the great work Brad. It looks great! And, given the significance of it, perhaps it warrants a blog post on blog.golang.org? -- Thanks again, tav |
mikioh
changed the title
Denial of Service Protection in Go HTTP Servers
net/http: Denial of Service Protection in Go HTTP Servers
Jan 22, 2015
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
by tav@espians.com:
The text was updated successfully, but these errors were encountered: