Merge pull request #847 from harshavardhana/enhance-signature-handler

Enhance signature handler - throw back valid error messages

pkg/server/api/bucket-handlers.go

@@ -29,8 +29,21 @@ func (api Minio) isValidOp(w http.ResponseWriter, req *http.Request, acceptsCont
 	bucket := vars["bucket"]
 
 	bucketMetadata, err := api.Donut.GetBucketMetadata(bucket, nil)
-	if err == nil {
-		if _, err := StripAccessKeyID(req.Header.Get("Authorization")); err != nil {
+	if err != nil {
+		switch err.ToGoError().(type) {
+		case donut.BucketNotFound:
+			writeErrorResponse(w, req, NoSuchBucket, acceptsContentType, req.URL.Path)
+			return false
+		case donut.BucketNameInvalid:
+			writeErrorResponse(w, req, InvalidBucketName, acceptsContentType, req.URL.Path)
+			return false
+		default:
+			// log.Error.Println(err.Trace())
+			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
+			return false
+		}
+	}
+	if _, err = stripAccessKeyID(req.Header.Get("Authorization")); err != nil {
 		if bucketMetadata.ACL.IsPrivate() {
 			return true
 			//uncomment this when we have webcli
@@ -45,19 +58,6 @@ func (api Minio) isValidOp(w http.ResponseWriter, req *http.Request, acceptsCont
 		}
 	}
 	return true
-	}
-	switch err.ToGoError().(type) {
-	case donut.BucketNotFound:
-		writeErrorResponse(w, req, NoSuchBucket, acceptsContentType, req.URL.Path)
-		return false
-	case donut.BucketNameInvalid:
-		writeErrorResponse(w, req, InvalidBucketName, acceptsContentType, req.URL.Path)
-		return false
-	default:
-		// log.Error.Println(err.Trace())
-		writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
-		return false
-	}
 }
 
 // ListMultipartUploadsHandler - GET Bucket (List Multipart uploads)
@@ -98,7 +98,7 @@ func (api Minio) ListMultipartUploadsHandler(w http.ResponseWriter, req *http.Re
 	if _, ok := req.Header["Authorization"]; ok {
 		// Init signature V4 verification
 		var err *probe.Error
-		signature, err = InitSignatureV4(req)
+		signature, err = initSignatureV4(req)
 		if err != nil {
 			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
 			return
@@ -169,7 +169,7 @@ func (api Minio) ListObjectsHandler(w http.ResponseWriter, req *http.Request) {
 	if _, ok := req.Header["Authorization"]; ok {
 		// Init signature V4 verification
 		var err *probe.Error
-		signature, err = InitSignatureV4(req)
+		signature, err = initSignatureV4(req)
 		if err != nil {
 			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
 			return
@@ -230,7 +230,7 @@ func (api Minio) ListBucketsHandler(w http.ResponseWriter, req *http.Request) {
 	if _, ok := req.Header["Authorization"]; ok {
 		// Init signature V4 verification
 		var err *probe.Error
-		signature, err = InitSignatureV4(req)
+		signature, err = initSignatureV4(req)
 		if err != nil {
 			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
 			return
@@ -296,7 +296,7 @@ func (api Minio) PutBucketHandler(w http.ResponseWriter, req *http.Request) {
 	if _, ok := req.Header["Authorization"]; ok {
 		// Init signature V4 verification
 		var err *probe.Error
-		signature, err = InitSignatureV4(req)
+		signature, err = initSignatureV4(req)
 		if err != nil {
 			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
 			return
@@ -364,7 +364,7 @@ func (api Minio) PutBucketACLHandler(w http.ResponseWriter, req *http.Request) {
 	if _, ok := req.Header["Authorization"]; ok {
 		// Init signature V4 verification
 		var err *probe.Error
-		signature, err = InitSignatureV4(req)
+		signature, err = initSignatureV4(req)
 		if err != nil {
 			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
 			return
@@ -414,7 +414,7 @@ func (api Minio) HeadBucketHandler(w http.ResponseWriter, req *http.Request) {
 	if _, ok := req.Header["Authorization"]; ok {
 		// Init signature V4 verification
 		var err *probe.Error
-		signature, err = InitSignatureV4(req)
+		signature, err = initSignatureV4(req)
 		if err != nil {
 			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
 			return

pkg/server/api/errors.go

@@ -69,38 +69,39 @@ const (
 	MethodNotAllowed
 	InvalidPart
 	InvalidPartOrder
+	AuthorizationHeaderMalformed
 )
 
 // Error codes, non exhaustive list - standard HTTP errors
 const (
-	NotAcceptable = iota + 29
+	NotAcceptable = iota + 30
 )
 
 // Error code to Error structure map
 var errorCodeResponse = map[int]Error{
 	InvalidMaxUploads: {
 		Code:           "InvalidArgument",
-		Description:    "Argument maxUploads must be an integer between 0 and 2147483647",
+		Description:    "Argument maxUploads must be an integer between 0 and 2147483647.",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
 	InvalidMaxKeys: {
 		Code:           "InvalidArgument",
-		Description:    "Argument maxKeys must be an integer between 0 and 2147483647",
+		Description:    "Argument maxKeys must be an integer between 0 and 2147483647.",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
 	InvalidMaxParts: {
 		Code:           "InvalidArgument",
-		Description:    "Argument maxParts must be an integer between 1 and 10000",
+		Description:    "Argument maxParts must be an integer between 1 and 10000.",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
 	InvalidPartNumberMarker: {
 		Code:           "InvalidArgument",
-		Description:    "Argument partNumberMarker must be an integer",
+		Description:    "Argument partNumberMarker must be an integer.",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
 	AccessDenied: {
 		Code:           "AccessDenied",
-		Description:    "Access Denied",
+		Description:    "Access Denied.",
 		HTTPStatusCode: http.StatusForbidden,
 	},
 	BadDigest: {
@@ -125,7 +126,7 @@ var errorCodeResponse = map[int]Error{
 	},
 	IncompleteBody: {
 		Code:           "IncompleteBody",
-		Description:    "You did not provide the number of bytes specified by the Content-Length HTTP header",
+		Description:    "You did not provide the number of bytes specified by the Content-Length HTTP header.",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
 	InternalError: {
@@ -215,7 +216,7 @@ var errorCodeResponse = map[int]Error{
 	},
 	InvalidPart: {
 		Code:           "InvalidPart",
-		Description:    "One or more of the specified parts could not be found",
+		Description:    "One or more of the specified parts could not be found.",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
 	InvalidPartOrder: {
@@ -223,6 +224,11 @@ var errorCodeResponse = map[int]Error{
 		Description:    "The list of parts was not in ascending order. The parts list must be specified in order by part number.",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
+	AuthorizationHeaderMalformed: {
+		Code:           "AuthorizationHeaderMalformed",
+		Description:    "The authorization header is malformed; the region is wrong; expecting 'milkyway'.",
+		HTTPStatusCode: http.StatusBadRequest,
+	},
 }
 
 // errorCodeError provides errorCode to Error. It returns empty if the code provided is unknown

pkg/server/api/generic-handlers.go

@@ -122,9 +122,9 @@ func (h timeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	h.handler.ServeHTTP(w, r)
 }
 
-// ValidateAuthHeaderHandler -
-// validate auth header handler is wrapper handler used for request validation with authorization header.
-// Current authorization layer supports S3's standard HMAC based signature request.
+// ValidateAuthHeaderHandler - validate auth header handler is wrapper handler used
+// for request validation with authorization header. Current authorization layer
+// supports S3's standard HMAC based signature request.
 func ValidateAuthHeaderHandler(h http.Handler) http.Handler {
 	return validateAuthHandler{h}
 }
@@ -132,8 +132,14 @@ func ValidateAuthHeaderHandler(h http.Handler) http.Handler {
 // validate auth header handler ServeHTTP() wrapper
 func (h validateAuthHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	acceptsContentType := getContentType(r)
-	accessKeyID, err := StripAccessKeyID(r.Header.Get("Authorization"))
-	switch err.(type) {
+	accessKeyID, err := stripAccessKeyID(r.Header.Get("Authorization"))
+	switch err.ToGoError() {
+	case errInvalidRegion:
+		writeErrorResponse(w, r, AuthorizationHeaderMalformed, acceptsContentType, r.URL.Path)
+		return
+	case errAccessKeyIDInvalid:
+		writeErrorResponse(w, r, InvalidAccessKeyID, acceptsContentType, r.URL.Path)
+		return
 	case nil:
 		// load auth config
 		authConfig, err := auth.LoadConfig()
@@ -150,6 +156,7 @@ func (h validateAuthHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		}
 		writeErrorResponse(w, r, InvalidAccessKeyID, acceptsContentType, r.URL.Path)
 		return
+	// All other errors for now, serve them
 	default:
 		// control reaches here, we should just send the request up the stack - internally
 		// individual calls will validate themselves against un-authenticated requests

pkg/server/api/object-handlers.go

@@ -57,7 +57,7 @@ func (api Minio) GetObjectHandler(w http.ResponseWriter, req *http.Request) {
 	if _, ok := req.Header["Authorization"]; ok {
 		// Init signature V4 verification
 		var err *probe.Error
-		signature, err = InitSignatureV4(req)
+		signature, err = initSignatureV4(req)
 		if err != nil {
 			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
 			return
@@ -124,7 +124,7 @@ func (api Minio) HeadObjectHandler(w http.ResponseWriter, req *http.Request) {
 	if _, ok := req.Header["Authorization"]; ok {
 		// Init signature V4 verification
 		var err *probe.Error
-		signature, err = InitSignatureV4(req)
+		signature, err = initSignatureV4(req)
 		if err != nil {
 			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
 			return
@@ -218,7 +218,7 @@ func (api Minio) PutObjectHandler(w http.ResponseWriter, req *http.Request) {
 	if _, ok := req.Header["Authorization"]; ok {
 		// Init signature V4 verification
 		var err *probe.Error
-		signature, err = InitSignatureV4(req)
+		signature, err = initSignatureV4(req)
 		if err != nil {
 			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
 			return
@@ -288,7 +288,7 @@ func (api Minio) NewMultipartUploadHandler(w http.ResponseWriter, req *http.Requ
 	if _, ok := req.Header["Authorization"]; ok {
 		// Init signature V4 verification
 		var err *probe.Error
-		signature, err = InitSignatureV4(req)
+		signature, err = initSignatureV4(req)
 		if err != nil {
 			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
 			return
@@ -382,7 +382,7 @@ func (api Minio) PutObjectPartHandler(w http.ResponseWriter, req *http.Request)
 	if _, ok := req.Header["Authorization"]; ok {
 		// Init signature V4 verification
 		var err *probe.Error
-		signature, err = InitSignatureV4(req)
+		signature, err = initSignatureV4(req)
 		if err != nil {
 			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
 			return
@@ -442,7 +442,7 @@ func (api Minio) AbortMultipartUploadHandler(w http.ResponseWriter, req *http.Re
 	if _, ok := req.Header["Authorization"]; ok {
 		// Init signature V4 verification
 		var err *probe.Error
-		signature, err = InitSignatureV4(req)
+		signature, err = initSignatureV4(req)
 		if err != nil {
 			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
 			return
@@ -503,7 +503,7 @@ func (api Minio) ListObjectPartsHandler(w http.ResponseWriter, req *http.Request
 	if _, ok := req.Header["Authorization"]; ok {
 		// Init signature V4 verification
 		var err *probe.Error
-		signature, err = InitSignatureV4(req)
+		signature, err = initSignatureV4(req)
 		if err != nil {
 			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
 			return
@@ -557,7 +557,7 @@ func (api Minio) CompleteMultipartUploadHandler(w http.ResponseWriter, req *http
 	if _, ok := req.Header["Authorization"]; ok {
 		// Init signature V4 verification
 		var err *probe.Error
-		signature, err = InitSignatureV4(req)
+		signature, err = initSignatureV4(req)
 		if err != nil {
 			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
 			return

pkg/server/api/signature.go

@@ -30,50 +30,75 @@ const (
 	authHeaderPrefix = "AWS4-HMAC-SHA256"
 )
 
-// StripAccessKeyID - strip only access key id from auth header
-func StripAccessKeyID(ah string) (string, error) {
-	if ah == "" {
-		return "", errors.New("Missing auth header")
+// getCredentialsFromAuth parse credentials tag from authorization value
+func getCredentialsFromAuth(authValue string) ([]string, *probe.Error) {
+	if authValue == "" {
+		return nil, probe.NewError(errMissingAuthHeaderValue)
 	}
-	authFields := strings.Split(strings.TrimSpace(ah), ",")
+	authFields := strings.Split(strings.TrimSpace(authValue), ",")
 	if len(authFields) != 3 {
-		return "", errors.New("Missing fields in Auth header")
+		return nil, probe.NewError(errInvalidAuthHeaderValue)
 	}
 	authPrefixFields := strings.Fields(authFields[0])
 	if len(authPrefixFields) != 2 {
-		return "", errors.New("Missing fields in Auth header")
+		return nil, probe.NewError(errMissingFieldsAuthHeader)
 	}
 	if authPrefixFields[0] != authHeaderPrefix {
-		return "", errors.New("Missing fields is Auth header")
+		return nil, probe.NewError(errInvalidAuthHeaderPrefix)
 	}
 	credentials := strings.Split(strings.TrimSpace(authPrefixFields[1]), "=")
 	if len(credentials) != 2 {
-		return "", errors.New("Missing fields in Auth header")
+		return nil, probe.NewError(errMissingFieldsCredentialTag)
 	}
 	if len(strings.Split(strings.TrimSpace(authFields[1]), "=")) != 2 {
-		return "", errors.New("Missing fields in Auth header")
+		return nil, probe.NewError(errMissingFieldsSignedHeadersTag)
 	}
 	if len(strings.Split(strings.TrimSpace(authFields[2]), "=")) != 2 {
-		return "", errors.New("Missing fields in Auth header")
+		return nil, probe.NewError(errMissingFieldsSignatureTag)
 	}
-	accessKeyID := strings.Split(strings.TrimSpace(credentials[1]), "/")[0]
+	credentialElements := strings.Split(strings.TrimSpace(credentials[1]), "/")
+	if len(credentialElements) != 5 {
+		return nil, probe.NewError(errCredentialTagMalformed)
+	}
+	return credentialElements, nil
+}
+
+// verify if authHeader value has valid region
+func isValidRegion(authHeaderValue string) *probe.Error {
+	credentialElements, err := getCredentialsFromAuth(authHeaderValue)
+	if err != nil {
+		return err.Trace()
+	}
+	region := credentialElements[2]
+	if region != "milkyway" {
+		return probe.NewError(errInvalidRegion)
+	}
+	return nil
+}
+
+// stripAccessKeyID - strip only access key id from auth header
+func stripAccessKeyID(authHeaderValue string) (string, *probe.Error) {
+	if err := isValidRegion(authHeaderValue); err != nil {
+		return "", err.Trace()
+	}
+	credentialElements, err := getCredentialsFromAuth(authHeaderValue)
+	if err != nil {
+		return "", err.Trace()
+	}
+	accessKeyID := credentialElements[0]
 	if !auth.IsValidAccessKey(accessKeyID) {
-		return "", errors.New("Invalid access key")
+		return "", probe.NewError(errAccessKeyIDInvalid)
 	}
 	return accessKeyID, nil
 }
 
-// InitSignatureV4 initializing signature verification
-func InitSignatureV4(req *http.Request) (*donut.Signature, *probe.Error) {
+// initSignatureV4 initializing signature verification
+func initSignatureV4(req *http.Request) (*donut.Signature, *probe.Error) {
 	// strip auth from authorization header
-	ah := req.Header.Get("Authorization")
-	var accessKeyID string
-	{
-		var err error
-		accessKeyID, err = StripAccessKeyID(ah)
+	authHeaderValue := req.Header.Get("Authorization")
+	accessKeyID, err := stripAccessKeyID(authHeaderValue)
 	if err != nil {
-			return nil, probe.NewError(err)
-		}
+		return nil, err.Trace()
 	}
 	authConfig, err := auth.LoadConfig()
 	if err != nil {
@@ -84,11 +109,11 @@ func InitSignatureV4(req *http.Request) (*donut.Signature, *probe.Error) {
 			signature := &donut.Signature{
 				AccessKeyID:     user.AccessKeyID,
 				SecretAccessKey: user.SecretAccessKey,
-				AuthHeader:      ah,
+				AuthHeader:      authHeaderValue,
 				Request:         req,
 			}
 			return signature, nil
 		}
 	}
-	return nil, probe.NewError(errors.New("AccessID not found"))
+	return nil, probe.NewError(errors.New("AccessKeyID not found"))
 }

pkg/server/api/typed-errors.go

@@ -0,0 +1,60 @@
+/*
+ * Minio Cloud Storage, (C) 2015 Minio, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package api
+
+import "errors"
+
+// errMissingAuthHeader means that Authorization header
+// has missing value or it is empty
+var errMissingAuthHeaderValue = errors.New("Missing auth header value")
+
+// errInvalidAuthHeaderValue means that Authorization
+// header is available but is malformed and not in
+// accordance with signature v4
+var errInvalidAuthHeaderValue = errors.New("Invalid auth header value")
+
+// errInvalidAuthHeaderPrefix means that Authorization header
+// has a wrong prefix only supported value should be "AWS4-HMAC-SHA256"
+var errInvalidAuthHeaderPrefix = errors.New("Invalid auth header prefix")
+
+// errMissingFieldsAuthHeader means that Authorization
+// header is available but has some missing fields
+var errMissingFieldsAuthHeader = errors.New("Missing fields in auth header")
+
+// errMissingFieldsCredentialTag means that Authorization
+// header credentials tag has some missing fields
+var errMissingFieldsCredentialTag = errors.New("Missing fields in crendential tag")
+
+// errMissingFieldsSignedHeadersTag means that Authorization
+// header signed headers tag has some missing fields
+var errMissingFieldsSignedHeadersTag = errors.New("Missing fields in signed headers tag")
+
+// errMissingFieldsSignatureTag means that Authorization
+// header signature tag has missing fields
+var errMissingFieldsSignatureTag = errors.New("Missing fields in signature tag")
+
+// errCredentialTagMalformed means that Authorization header
+// credential tag is malformed
+var errCredentialTagMalformed = errors.New("Invalid credential tag malformed")
+
+// errInvalidRegion means that the region element from credential tag
+// in Authorization header is invalid
+var errInvalidRegion = errors.New("Invalid region")
+
+// errAccessKeyIDInvalid means that the accessKeyID element from
+// credential tag in Authorization header is invalid
+var errAccessKeyIDInvalid = errors.New("AccessKeyID invalid")

pkg/server/api_donut_cache_test.go

@@ -560,7 +560,7 @@ func (s *MyAPIDonutCacheSuite) TestListObjectsHandlerErrors(c *C) {
 	client = http.Client{}
 	response, err = client.Do(request)
 	c.Assert(err, IsNil)
-	verifyError(c, response, "InvalidArgument", "Argument maxKeys must be an integer between 0 and 2147483647", http.StatusBadRequest)
+	verifyError(c, response, "InvalidArgument", "Argument maxKeys must be an integer between 0 and 2147483647.", http.StatusBadRequest)
 }
 
 func (s *MyAPIDonutCacheSuite) TestPutBucketErrors(c *C) {
@@ -805,7 +805,7 @@ func (s *MyAPIDonutCacheSuite) TestObjectMultipartList(c *C) {
 
 	response4, err := client.Do(request)
 	c.Assert(err, IsNil)
-	verifyError(c, response4, "InvalidArgument", "Argument maxParts must be an integer between 1 and 10000", http.StatusBadRequest)
+	verifyError(c, response4, "InvalidArgument", "Argument maxParts must be an integer between 1 and 10000.", http.StatusBadRequest)
 }
 
 func (s *MyAPIDonutCacheSuite) TestObjectMultipart(c *C) {

pkg/server/api_donut_test.go

@@ -580,7 +580,7 @@ func (s *MyAPIDonutSuite) TestListObjectsHandlerErrors(c *C) {
 	client = http.Client{}
 	response, err = client.Do(request)
 	c.Assert(err, IsNil)
-	verifyError(c, response, "InvalidArgument", "Argument maxKeys must be an integer between 0 and 2147483647", http.StatusBadRequest)
+	verifyError(c, response, "InvalidArgument", "Argument maxKeys must be an integer between 0 and 2147483647.", http.StatusBadRequest)
 }
 
 func (s *MyAPIDonutSuite) TestPutBucketErrors(c *C) {
@@ -825,7 +825,7 @@ func (s *MyAPIDonutSuite) TestObjectMultipartList(c *C) {
 
 	response4, err := client.Do(request)
 	c.Assert(err, IsNil)
-	verifyError(c, response4, "InvalidArgument", "Argument maxParts must be an integer between 1 and 10000", http.StatusBadRequest)
+	verifyError(c, response4, "InvalidArgument", "Argument maxParts must be an integer between 1 and 10000.", http.StatusBadRequest)
 }
 
 func (s *MyAPIDonutSuite) TestObjectMultipart(c *C) {

pkg/server/api_signature_v4_test.go

@@ -572,7 +572,7 @@ func (s *MyAPISignatureV4Suite) TestListObjectsHandlerErrors(c *C) {
 	client = http.Client{}
 	response, err = client.Do(request)
 	c.Assert(err, IsNil)
-	verifyError(c, response, "InvalidArgument", "Argument maxKeys must be an integer between 0 and 2147483647", http.StatusBadRequest)
+	verifyError(c, response, "InvalidArgument", "Argument maxKeys must be an integer between 0 and 2147483647.", http.StatusBadRequest)
 }
 
 func (s *MyAPISignatureV4Suite) TestPutBucketErrors(c *C) {
@@ -824,7 +824,7 @@ func (s *MyAPISignatureV4Suite) TestObjectMultipartList(c *C) {
 
 	response4, err := client.Do(request)
 	c.Assert(err, IsNil)
-	verifyError(c, response4, "InvalidArgument", "Argument maxParts must be an integer between 1 and 10000", http.StatusBadRequest)
+	verifyError(c, response4, "InvalidArgument", "Argument maxParts must be an integer between 1 and 10000.", http.StatusBadRequest)
 }
 
 func (s *MyAPISignatureV4Suite) TestObjectMultipart(c *C) {