From 3145462ad25f668a13d58e4999af57e03883df0a Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Fri, 16 Mar 2018 11:22:34 -0700 Subject: [PATCH] Return InvalidDigest when md5 sent by client is invalid (#5654) This is to ensure proper compatibility with AWS S3, handle special cases where - Content-Md5 is set to empty - Content-Md5 is set to invalid --- cmd/auth-handler.go | 32 ++++++++++++++++++++++++++------ cmd/auth-handler_test.go | 24 ++++++++++++++++++++++++ cmd/hasher.go | 11 ++++++++--- cmd/object-handlers.go | 34 ++++++++++++++++++++++------------ cmd/utils.go | 11 +++++++++-- 5 files changed, 89 insertions(+), 23 deletions(-) diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go index d50367ecc..9e54f9b6f 100644 --- a/cmd/auth-handler.go +++ b/cmd/auth-handler.go @@ -18,6 +18,8 @@ package cmd import ( "bytes" + "encoding/base64" + "encoding/hex" "errors" "io/ioutil" "net/http" @@ -167,9 +169,11 @@ func isReqAuthenticated(r *http.Request, region string) (s3Error APIErrorCode) { if r == nil { return ErrInternalError } + if errCode := reqSignatureV4Verify(r, region); errCode != ErrNone { return errCode } + payload, err := ioutil.ReadAll(r.Body) if err != nil { errorIf(err, "Unable to read request body for signature verification") @@ -180,8 +184,15 @@ func isReqAuthenticated(r *http.Request, region string) (s3Error APIErrorCode) { r.Body = ioutil.NopCloser(bytes.NewReader(payload)) // Verify Content-Md5, if payload is set. - if r.Header.Get("Content-Md5") != "" { - if r.Header.Get("Content-Md5") != getMD5HashBase64(payload) { + if clntMD5B64, ok := r.Header["Content-Md5"]; ok { + if clntMD5B64[0] == "" { + return ErrInvalidDigest + } + md5Sum, err := base64.StdEncoding.Strict().DecodeString(clntMD5B64[0]) + if err != nil { + return ErrInvalidDigest + } + if !bytes.Equal(md5Sum, getMD5Sum(payload)) { return ErrBadDigest } } @@ -192,12 +203,21 @@ func isReqAuthenticated(r *http.Request, region string) (s3Error APIErrorCode) { // Verify that X-Amz-Content-Sha256 Header == sha256(payload) // If X-Amz-Content-Sha256 header is not sent then we don't calculate/verify sha256(payload) - sum := r.Header.Get("X-Amz-Content-Sha256") + sumHex, ok := r.Header["X-Amz-Content-Sha256"] if isRequestPresignedSignatureV4(r) { - sum = r.URL.Query().Get("X-Amz-Content-Sha256") + sumHex, ok = r.URL.Query()["X-Amz-Content-Sha256"] } - if sum != "" && sum != getSHA256Hash(payload) { - return ErrContentSHA256Mismatch + if ok { + if sumHex[0] == "" { + return ErrContentSHA256Mismatch + } + sum, err := hex.DecodeString(sumHex[0]) + if err != nil { + return ErrContentSHA256Mismatch + } + if !bytes.Equal(sum, getSHA256Sum(payload)) { + return ErrContentSHA256Mismatch + } } return ErrNone } diff --git a/cmd/auth-handler_test.go b/cmd/auth-handler_test.go index ff5678626..d88b913eb 100644 --- a/cmd/auth-handler_test.go +++ b/cmd/auth-handler_test.go @@ -311,6 +311,26 @@ func mustNewPresignedRequest(method string, urlStr string, contentLength int64, return req } +func mustNewSignedShortMD5Request(method string, urlStr string, contentLength int64, body io.ReadSeeker, t *testing.T) *http.Request { + req := mustNewRequest(method, urlStr, contentLength, body, t) + req.Header.Set("Content-Md5", "invalid-digest") + cred := globalServerConfig.GetCredential() + if err := signRequestV4(req, cred.AccessKey, cred.SecretKey); err != nil { + t.Fatalf("Unable to initialized new signed http request %s", err) + } + return req +} + +func mustNewSignedEmptyMD5Request(method string, urlStr string, contentLength int64, body io.ReadSeeker, t *testing.T) *http.Request { + req := mustNewRequest(method, urlStr, contentLength, body, t) + req.Header.Set("Content-Md5", "") + cred := globalServerConfig.GetCredential() + if err := signRequestV4(req, cred.AccessKey, cred.SecretKey); err != nil { + t.Fatalf("Unable to initialized new signed http request %s", err) + } + return req +} + func mustNewSignedBadMD5Request(method string, urlStr string, contentLength int64, body io.ReadSeeker, t *testing.T) *http.Request { req := mustNewRequest(method, urlStr, contentLength, body, t) req.Header.Set("Content-Md5", "YWFhYWFhYWFhYWFhYWFhCg==") @@ -345,6 +365,10 @@ func TestIsReqAuthenticated(t *testing.T) { {nil, ErrInternalError}, // When request is unsigned, access denied is returned. {mustNewRequest("GET", "http://127.0.0.1:9000", 0, nil, t), ErrAccessDenied}, + // Empty Content-Md5 header. + {mustNewSignedEmptyMD5Request("PUT", "http://127.0.0.1:9000/", 5, bytes.NewReader([]byte("hello")), t), ErrInvalidDigest}, + // Short Content-Md5 header. + {mustNewSignedShortMD5Request("PUT", "http://127.0.0.1:9000/", 5, bytes.NewReader([]byte("hello")), t), ErrInvalidDigest}, // When request is properly signed, but has bad Content-MD5 header. {mustNewSignedBadMD5Request("PUT", "http://127.0.0.1:9000/", 5, bytes.NewReader([]byte("hello")), t), ErrBadDigest}, // When request is properly signed, error is none. diff --git a/cmd/hasher.go b/cmd/hasher.go index 2988ede1b..d38f319db 100644 --- a/cmd/hasher.go +++ b/cmd/hasher.go @@ -24,11 +24,16 @@ import ( "github.com/minio/sha256-simd" ) -// getSHA256Hash returns SHA-256 hash of given data. +// getSHA256Hash returns SHA-256 hash in hex encoding of given data. func getSHA256Hash(data []byte) string { + return hex.EncodeToString(getSHA256Sum(data)) +} + +// getSHA256Hash returns SHA-256 sum of given data. +func getSHA256Sum(data []byte) []byte { hash := sha256.New() hash.Write(data) - return hex.EncodeToString(hash.Sum(nil)) + return hash.Sum(nil) } // getMD5Sum returns MD5 sum of given data. @@ -38,7 +43,7 @@ func getMD5Sum(data []byte) []byte { return hash.Sum(nil) } -// getMD5Hash returns MD5 hash of given data. +// getMD5Hash returns MD5 hash in hex encoding of given data. func getMD5Hash(data []byte) string { return hex.EncodeToString(getMD5Sum(data)) } diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 5bdf6da01..486def878 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -572,7 +572,7 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req } // Get Content-Md5 sent by client and verify if valid - md5Bytes, err := checkValidMD5(r.Header.Get("Content-Md5")) + md5Bytes, err := checkValidMD5(r.Header) if err != nil { writeErrorResponse(w, ErrInvalidDigest, r.URL) return @@ -582,11 +582,16 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req size := r.ContentLength rAuthType := getRequestAuthType(r) if rAuthType == authTypeStreamingSigned { - sizeStr := r.Header.Get("x-amz-decoded-content-length") - size, err = strconv.ParseInt(sizeStr, 10, 64) - if err != nil { - writeErrorResponse(w, toAPIErrorCode(err), r.URL) - return + if sizeStr, ok := r.Header["X-Amz-Decoded-Content-Length"]; ok { + if sizeStr[0] == "" { + writeErrorResponse(w, ErrMissingContentLength, r.URL) + return + } + size, err = strconv.ParseInt(sizeStr[0], 10, 64) + if err != nil { + writeErrorResponse(w, toAPIErrorCode(err), r.URL) + return + } } } if size == -1 { @@ -1013,7 +1018,7 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http } // get Content-Md5 sent by client and verify if valid - md5Bytes, err := checkValidMD5(r.Header.Get("Content-Md5")) + md5Bytes, err := checkValidMD5(r.Header) if err != nil { writeErrorResponse(w, ErrInvalidDigest, r.URL) return @@ -1025,11 +1030,16 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http rAuthType := getRequestAuthType(r) // For auth type streaming signature, we need to gather a different content length. if rAuthType == authTypeStreamingSigned { - sizeStr := r.Header.Get("x-amz-decoded-content-length") - size, err = strconv.ParseInt(sizeStr, 10, 64) - if err != nil { - writeErrorResponse(w, toAPIErrorCode(err), r.URL) - return + if sizeStr, ok := r.Header["X-Amz-Decoded-Content-Length"]; ok { + if sizeStr[0] == "" { + writeErrorResponse(w, ErrMissingContentLength, r.URL) + return + } + size, err = strconv.ParseInt(sizeStr[0], 10, 64) + if err != nil { + writeErrorResponse(w, toAPIErrorCode(err), r.URL) + return + } } } if size == -1 { diff --git a/cmd/utils.go b/cmd/utils.go index 6bf50d64a..4c9c609ad 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -102,8 +102,15 @@ func xmlDecoder(body io.Reader, v interface{}, size int64) error { } // checkValidMD5 - verify if valid md5, returns md5 in bytes. -func checkValidMD5(md5 string) ([]byte, error) { - return base64.StdEncoding.DecodeString(strings.TrimSpace(md5)) +func checkValidMD5(h http.Header) ([]byte, error) { + md5B64, ok := h["Content-Md5"] + if ok { + if md5B64[0] == "" { + return nil, fmt.Errorf("Content-Md5 header set to empty value") + } + return base64.StdEncoding.DecodeString(md5B64[0]) + } + return []byte{}, nil } /// http://docs.aws.amazon.com/AmazonS3/latest/dev/UploadingObjects.html