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

@@ -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
 }

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.

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

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 {

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