From ade803a9237705de742a3f91ede9aadf6c49a481 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 27 Apr 2015 03:54:49 -0700 Subject: [PATCH] Verify if request date is 5minutes late, reject such a request as it could be a possible replay attack. This commit also fixes #505, by returning MethodNotAllowed instead of NotImplemented --- pkg/api/api_generic_handlers.go | 69 ++++++++++++++++++++++++++++----- pkg/api/api_object_handlers.go | 6 +-- pkg/api/api_router.go | 4 +- pkg/api/api_test.go | 1 + pkg/api/errors.go | 9 ++++- 5 files changed, 73 insertions(+), 16 deletions(-) diff --git a/pkg/api/api_generic_handlers.go b/pkg/api/api_generic_handlers.go index f7707cd61..3d2908f9e 100644 --- a/pkg/api/api_generic_handlers.go +++ b/pkg/api/api_generic_handlers.go @@ -17,18 +17,24 @@ package api import ( + "errors" "net/http" "strings" + "time" "github.com/minio-io/minio/pkg/api/config" ) -type vHandler struct { +type timeHandler struct { + handler http.Handler +} + +type validateHandler struct { conf config.Config handler http.Handler } -type rHandler struct { +type resourceHandler struct { handler http.Handler } @@ -45,17 +51,58 @@ func stripAccessKey(r *http.Request) string { return splits[0] } +func getDate(req *http.Request) (time.Time, error) { + if req.Header.Get("x-amz-date") != "" { + return time.Parse(http.TimeFormat, req.Header.Get("x-amz-date")) + } + if req.Header.Get("Date") != "" { + return time.Parse(http.TimeFormat, req.Header.Get("Date")) + } + return time.Time{}, errors.New("invalid request") +} + +func timeValidityHandler(h http.Handler) http.Handler { + return timeHandler{h} +} + +func (h timeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + acceptsContentType := getContentType(r) + if acceptsContentType == unknownContentType { + writeErrorResponse(w, r, NotAcceptable, acceptsContentType, r.URL.Path) + return + } + // Verify if date headers are set, if not reject the request + if r.Header.Get("x-amz-date") == "" && r.Header.Get("Date") == "" { + // there is no way to knowing if this is a valid request, could be a attack reject such clients + writeErrorResponse(w, r, RequestTimeTooSkewed, acceptsContentType, r.URL.Path) + return + } + date, err := getDate(r) + if err != nil { + // there is no way to knowing if this is a valid request, could be a attack reject such clients + writeErrorResponse(w, r, RequestTimeTooSkewed, acceptsContentType, r.URL.Path) + return + } + duration := time.Since(date) + minutes := time.Duration(5) * time.Minute + if duration.Minutes() > minutes.Minutes() { + writeErrorResponse(w, r, RequestTimeTooSkewed, acceptsContentType, r.URL.Path) + return + } + h.handler.ServeHTTP(w, r) +} + // Validate handler is wrapper handler used for API request validation with authorization header. // Current authorization layer supports S3's standard HMAC based signature request. -func validateHandler(conf config.Config, h http.Handler) http.Handler { - return vHandler{ +func validateRequestHandler(conf config.Config, h http.Handler) http.Handler { + return validateHandler{ conf: conf, handler: h, } } // Validate handler ServeHTTP() wrapper -func (h vHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { +func (h validateHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { acceptsContentType := getContentType(r) if acceptsContentType == unknownContentType { writeErrorResponse(w, r, NotAcceptable, acceptsContentType, r.URL.Path) @@ -105,21 +152,25 @@ func (h vHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // Since we do not support all the S3 queries, it is necessary for us to throw back a // valid error message indicating such a feature to have been not implemented. func ignoreResourcesHandler(h http.Handler) http.Handler { - return rHandler{h} + return resourceHandler{h} } // Resource handler ServeHTTP() wrapper -func (h rHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { +func (h resourceHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { acceptsContentType := getContentType(r) + if acceptsContentType == unknownContentType { + writeErrorResponse(w, r, NotAcceptable, acceptsContentType, r.URL.Path) + return + } if ignoreUnImplementedObjectResources(r) || ignoreUnImplementedBucketResources(r) { error := getErrorCode(NotImplemented) errorResponse := getErrorResponse(error, "") setCommonHeaders(w, getContentTypeString(acceptsContentType)) w.WriteHeader(error.HTTPStatusCode) w.Write(encodeErrorResponse(errorResponse, acceptsContentType)) - } else { - h.handler.ServeHTTP(w, r) + return } + h.handler.ServeHTTP(w, r) } //// helpers diff --git a/pkg/api/api_object_handlers.go b/pkg/api/api_object_handlers.go index d4dbb4c8b..aeb35e338 100644 --- a/pkg/api/api_object_handlers.go +++ b/pkg/api/api_object_handlers.go @@ -163,11 +163,7 @@ func (server *minioAPI) putObjectHandler(w http.ResponseWriter, req *http.Reques w.WriteHeader(http.StatusOK) case drivers.ObjectExists: { - // we need to debate about this, if this is the right message to send back - // https://github.com/minio-io/minio/issues/505 - - // Ideally we can use 405 Method No Allowed - writeErrorResponse(w, req, NotImplemented, acceptsContentType, req.URL.Path) + writeErrorResponse(w, req, MethodNotAllowed, acceptsContentType, req.URL.Path) } case drivers.BadDigest: { diff --git a/pkg/api/api_router.go b/pkg/api/api_router.go index c218e6a8a..bf9c370eb 100644 --- a/pkg/api/api_router.go +++ b/pkg/api/api_router.go @@ -92,7 +92,9 @@ func HTTPHandler(domain string, driver drivers.Driver) http.Handler { log.Fatal(iodine.New(err, map[string]string{"domain": domain})) } - h := validateHandler(conf, ignoreResourcesHandler(mux)) + h := timeValidityHandler(mux) + h = ignoreResourcesHandler(h) + h = validateRequestHandler(conf, h) h = quota.BandwidthCap(h, 25*1024*1024, time.Duration(30*time.Minute)) h = quota.BandwidthCap(h, 100*1024*1024, time.Duration(24*time.Hour)) h = quota.RequestLimit(h, 100, time.Duration(30*time.Minute)) diff --git a/pkg/api/api_test.go b/pkg/api/api_test.go index 371bcbf08..5ccafbcce 100644 --- a/pkg/api/api_test.go +++ b/pkg/api/api_test.go @@ -123,6 +123,7 @@ func setAuthHeader(req *http.Request) { encoder.Write(hm.Sum(nil)) encoder.Close() req.Header.Set("Authorization", authHeader.String()) + req.Header.Set("Date", time.Now().UTC().Format(http.TimeFormat)) } func (s *MySuite) TestNonExistantBucket(c *C) { diff --git a/pkg/api/errors.go b/pkg/api/errors.go index e7908b5e7..df72cba8c 100644 --- a/pkg/api/errors.go +++ b/pkg/api/errors.go @@ -51,6 +51,7 @@ const ( InvalidBucketName InvalidDigest InvalidRange + InvalidRequest MalformedXML MissingContentLength MissingRequestBodyError @@ -61,11 +62,12 @@ const ( RequestTimeTooSkewed SignatureDoesNotMatch TooManyBuckets + MethodNotAllowed ) // Error codes, non exhaustive list - standard HTTP errors const ( - NotAcceptable = iota + 21 + NotAcceptable = iota + 23 ) // Error code to Error structure map @@ -175,6 +177,11 @@ var errorCodeResponse = map[int]Error{ Description: "You have attempted to create more buckets than allowed.", HTTPStatusCode: http.StatusBadRequest, }, + MethodNotAllowed: { + Code: "MethodNotAllowed", + Description: "The specified method is not allowed against this resource.", + HTTPStatusCode: http.StatusMethodNotAllowed, + }, NotAcceptable: { Code: "NotAcceptable", Description: "The requested resource is only capable of generating content not acceptable according to the Accept headers sent in the request.",