Merge pull request #511 from harshavardhana/pr_out_verify_if_request_date_is_5minutes_late_reject_such_a_request_as_it_could_be_a_possible_replay_attack

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

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:
 		{

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))

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) {

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.",