security: fix write-to-RAM DoS vulnerability (#5957)

This commit fixes a DoS vulnerability for certain APIs using
signature V4 by verifying the content-md5 and/or content-sha56 of
the request body in a streaming mode.

The issue was caused by reading the entire body of the request into
memory to verify the content-md5 or content-sha56 checksum if present.

The vulnerability could be exploited by either replaying a V4 request
(in the 15 min time frame) or sending a V4 presigned request with a
large body.

cmd/auth-handler.go

@@ -28,6 +28,7 @@ import (
 	"strings"
 
 	"github.com/minio/minio/cmd/logger"
+	"github.com/minio/minio/pkg/hash"
 	"github.com/minio/minio/pkg/policy"
 )
 
@@ -209,59 +210,45 @@ func reqSignatureV4Verify(r *http.Request, region string) (s3Error APIErrorCode)
 
 // Verify if request has valid AWS Signature Version '4'.
 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)
+	var (
+		err                       error
+		contentMD5, contentSHA256 []byte
+	)
+	// Extract 'Content-Md5' if present.
+	if _, ok := r.Header["Content-Md5"]; ok {
+		contentMD5, err = base64.StdEncoding.Strict().DecodeString(r.Header.Get("Content-Md5"))
+		if err != nil || len(contentMD5) == 0 {
+			return ErrInvalidDigest
+		}
+	}
+
+	// Extract either 'X-Amz-Content-Sha256' header or 'X-Amz-Content-Sha256' query parameter (if V4 presigned)
+	// Do not verify 'X-Amz-Content-Sha256' if skipSHA256.
+	if skipSHA256 := skipContentSha256Cksum(r); !skipSHA256 && isRequestPresignedSignatureV4(r) {
+		if sha256Sum, ok := r.URL.Query()["X-Amz-Content-Sha256"]; ok && len(sha256Sum) > 0 {
+			contentSHA256, err = hex.DecodeString(sha256Sum[0])
+			if err != nil {
+				return ErrContentSHA256Mismatch
+			}
+		}
+	} else if _, ok := r.Header["X-Amz-Content-Sha256"]; !skipSHA256 && ok {
+		contentSHA256, err = hex.DecodeString(r.Header.Get("X-Amz-Content-Sha256"))
+		if err != nil || len(contentSHA256) == 0 {
+			return ErrContentSHA256Mismatch
+		}
+	}
+
+	// Verify 'Content-Md5' and/or 'X-Amz-Content-Sha256' if present.
+	// The verification happens implicit during reading.
+	reader, err := hash.NewReader(r.Body, -1, hex.EncodeToString(contentMD5), hex.EncodeToString(contentSHA256))
 	if err != nil {
-		logger.LogIf(context.Background(), err)
-		return ErrInternalError
-	}
-
-	// Populate back the payload.
-	r.Body = ioutil.NopCloser(bytes.NewReader(payload))
-
-	// Verify Content-Md5, if payload is set.
-	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
-		}
-	}
-
-	if skipContentSha256Cksum(r) {
-		return ErrNone
-	}
-
-	// 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)
-	sumHex, ok := r.Header["X-Amz-Content-Sha256"]
-	if isRequestPresignedSignatureV4(r) {
-		sumHex, ok = r.URL.Query()["X-Amz-Content-Sha256"]
-	}
-	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 toAPIErrorCode(err)
 	}
+	r.Body = ioutil.NopCloser(reader)
 	return ErrNone
 }
 

cmd/auth-handler_test.go

@@ -19,6 +19,7 @@ package cmd
 import (
 	"bytes"
 	"io"
+	"io/ioutil"
 	"net/http"
 	"net/url"
 	"os"
@@ -361,8 +362,6 @@ func TestIsReqAuthenticated(t *testing.T) {
 		req     *http.Request
 		s3Error APIErrorCode
 	}{
-		// When request is nil, internal error is returned.
-		{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.
@@ -376,9 +375,11 @@ func TestIsReqAuthenticated(t *testing.T) {
 	}
 
 	// Validates all testcases.
-	for _, testCase := range testCases {
+	for i, testCase := range testCases {
 		if s3Error := isReqAuthenticated(testCase.req, globalServerConfig.GetRegion()); s3Error != testCase.s3Error {
-			t.Fatalf("Unexpected s3error returned wanted %d, got %d", testCase.s3Error, s3Error)
+			if _, err := ioutil.ReadAll(testCase.req.Body); toAPIErrorCode(err) != testCase.s3Error {
+				t.Fatalf("Test %d: Unexpected S3 error: want %d - got %d (got after reading request %d)", i, testCase.s3Error, s3Error, toAPIErrorCode(err))
+			}
 		}
 	}
 }

pkg/hash/reader.go

@@ -61,11 +61,13 @@ func NewReader(src io.Reader, size int64, md5Hex, sha256Hex string) (*Reader, er
 	if len(sha256sum) != 0 {
 		sha256Hash = sha256.New()
 	}
-
+	if size >= 0 {
+		src = io.LimitReader(src, size)
+	}
 	return &Reader{
 		md5sum:     md5sum,
 		sha256sum:  sha256sum,
-		src:        io.LimitReader(src, size),
+		src:        src,
 		size:       size,
 		md5Hash:    md5.New(),
 		sha256Hash: sha256Hash,