-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Allow max request size to be user-specified #4824
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,9 @@ | ||
package http | ||
|
||
import ( | ||
"context" | ||
"encoding/json" | ||
"errors" | ||
"fmt" | ||
"io" | ||
"net" | ||
|
@@ -52,10 +54,11 @@ const ( | |
// soft-mandatory Sentinel policies. | ||
PolicyOverrideHeaderName = "X-Vault-Policy-Override" | ||
|
||
// MaxRequestSize is the maximum accepted request size. This is to prevent | ||
// a denial of service attack where no Content-Length is provided and the server | ||
// is fed ever more data until it exhausts memory. | ||
MaxRequestSize = 32 * 1024 * 1024 | ||
// DefaultMaxRequestSize is the default maximum accepted request size. This | ||
// is to prevent a denial of service attack where no Content-Length is | ||
// provided and the server is fed ever more data until it exhausts memory. | ||
// Can be overridden per listener. | ||
DefaultMaxRequestSize = 32 * 1024 * 1024 | ||
) | ||
|
||
var ( | ||
|
@@ -67,7 +70,9 @@ var ( | |
|
||
// Handler returns an http.Handler for the API. This can be used on | ||
// its own to mount the Vault API within another web server. | ||
func Handler(core *vault.Core) http.Handler { | ||
func Handler(props *vault.HandlerProperties) http.Handler { | ||
core := props.Core | ||
|
||
// Create the muxer to handle the actual endpoints | ||
mux := http.NewServeMux() | ||
mux.Handle("/v1/sys/init", handleSysInit(core)) | ||
|
@@ -108,7 +113,7 @@ func Handler(core *vault.Core) http.Handler { | |
|
||
// Wrap the help wrapped handler with another layer with a generic | ||
// handler | ||
genericWrappedHandler := wrapGenericHandler(corsWrappedHandler) | ||
genericWrappedHandler := wrapGenericHandler(corsWrappedHandler, props.MaxRequestSize) | ||
|
||
// Wrap the handler with PrintablePathCheckHandler to check for non-printable | ||
// characters in the request path. | ||
|
@@ -120,12 +125,20 @@ func Handler(core *vault.Core) http.Handler { | |
// wrapGenericHandler wraps the handler with an extra layer of handler where | ||
// tasks that should be commonly handled for all the requests and/or responses | ||
// are performed. | ||
func wrapGenericHandler(h http.Handler) http.Handler { | ||
func wrapGenericHandler(h http.Handler, maxRequestSize int64) http.Handler { | ||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
// Set the Cache-Control header for all the responses returned | ||
// by Vault | ||
w.Header().Set("Cache-Control", "no-store") | ||
h.ServeHTTP(w, r) | ||
|
||
// Add a context and put the request limit for this handler in it | ||
if maxRequestSize > 0 { | ||
ctx := context.WithValue(r.Context(), "max_request_size", maxRequestSize) | ||
h.ServeHTTP(w, r.WithContext(ctx)) | ||
} else { | ||
h.ServeHTTP(w, r) | ||
} | ||
|
||
return | ||
}) | ||
} | ||
|
@@ -326,8 +339,19 @@ func (fs *UIAssetWrapper) Open(name string) (http.File, error) { | |
func parseRequest(r *http.Request, w http.ResponseWriter, out interface{}) error { | ||
// Limit the maximum number of bytes to MaxRequestSize to protect | ||
// against an indefinite amount of data being read. | ||
limit := http.MaxBytesReader(w, r.Body, MaxRequestSize) | ||
err := jsonutil.DecodeJSONFromReader(limit, out) | ||
reader := r.Body | ||
ctx := r.Context() | ||
maxRequestSize := ctx.Value("max_request_size") | ||
if maxRequestSize != nil { | ||
max, ok := maxRequestSize.(int64) | ||
if !ok { | ||
return errors.New("could not parse max_request_size from request context") | ||
} | ||
if max > 0 { | ||
reader = http.MaxBytesReader(w, r.Body, max) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of curiosity, why do we use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Different function signatures, based on what's available. |
||
} | ||
} | ||
err := jsonutil.DecodeJSONFromReader(reader, out) | ||
if err != nil && err != io.EOF { | ||
return errwrap.Wrapf("failed to parse JSON input: {{err}}", err) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,13 @@ const ( | |
replTimeout = 10 * time.Second | ||
) | ||
|
||
// HanlderProperties is used to seed configuration into a vaulthttp.Handler. | ||
// It's in this package to avoid a circular dependency | ||
type HandlerProperties struct { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Nitpick] Can we call this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I called it Properties since it's not just user-set options (e.g. Core). |
||
Core *Core | ||
MaxRequestSize int64 | ||
} | ||
|
||
// fetchEntityAndDerivedPolicies returns the entity object for the given entity | ||
// ID. If the entity is merged into a different entity object, the entity into | ||
// which the given entity ID is merged into will be returned. This function | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't we ever need a size limit on the cluster connection?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the time we get to the HTTP handler we've already handshaked the cluster connection so we already know if the connection is authorized -- if so we shouldn't limit it.