make sure to validate signature unsigned trailer stream (#21103)

This is a security incident fix, it would seem like since
the implementation of unsigned payload trailer on PUTs,
we do not validate the signature of the incoming request.

The signature can be invalid and is totally being ignored,
this in-turn allows any arbitrary secret to upload objects
given the user has "WRITE" permissions on the bucket, since
acces-key is a public information in general exposes these
potential users with WRITE on the bucket to be used by any
arbitrary client to make a fake request to MinIO the signature
under Authorization: header is totally ignored.

A test has been added to cover this scenario and fail
appropriately.

cmd/auth-handler.go

@@ -96,7 +96,7 @@ func isRequestSignStreamingTrailerV4(r *http.Request) bool {
 // Verify if the request has AWS Streaming Signature Version '4', with unsigned content and trailer.
 func isRequestUnsignedTrailerV4(r *http.Request) bool {
 	return r.Header.Get(xhttp.AmzContentSha256) == unsignedPayloadTrailer &&
-		r.Method == http.MethodPut && strings.Contains(r.Header.Get(xhttp.ContentEncoding), streamingContentEncoding)
+		r.Method == http.MethodPut
 }
 
 // Authorization type.
@@ -363,7 +363,7 @@ func authenticateRequest(ctx context.Context, r *http.Request, action policy.Act
 	var cred auth.Credentials
 	var owner bool
 	switch getRequestAuthType(r) {
-	case authTypeUnknown, authTypeStreamingSigned:
+	case authTypeUnknown, authTypeStreamingSigned, authTypeStreamingSignedTrailer, authTypeStreamingUnsignedTrailer:
 		return ErrSignatureVersionNotSupported
 	case authTypePresignedV2, authTypeSignedV2:
 		if s3Err = isReqAuthenticatedV2(r); s3Err != ErrNone {
@@ -674,32 +674,6 @@ func setAuthMiddleware(h http.Handler) http.Handler {
 	})
 }
 
-func validateSignature(atype authType, r *http.Request) (auth.Credentials, bool, APIErrorCode) {
-	var cred auth.Credentials
-	var owner bool
-	var s3Err APIErrorCode
-	switch atype {
-	case authTypeUnknown, authTypeStreamingSigned:
-		return cred, owner, ErrSignatureVersionNotSupported
-	case authTypeSignedV2, authTypePresignedV2:
-		if s3Err = isReqAuthenticatedV2(r); s3Err != ErrNone {
-			return cred, owner, s3Err
-		}
-		cred, owner, s3Err = getReqAccessKeyV2(r)
-	case authTypePresigned, authTypeSigned:
-		region := globalSite.Region()
-		if s3Err = isReqAuthenticated(GlobalContext, r, region, serviceS3); s3Err != ErrNone {
-			return cred, owner, s3Err
-		}
-		cred, owner, s3Err = getReqAccessKeyV4(r, region, serviceS3)
-	}
-	if s3Err != ErrNone {
-		return cred, owner, s3Err
-	}
-
-	return cred, owner, ErrNone
-}
-
 func isPutRetentionAllowed(bucketName, objectName string, retDays int, retDate time.Time, retMode objectlock.RetMode, byPassSet bool, r *http.Request, cred auth.Credentials, owner bool) (s3Err APIErrorCode) {
 	var retSet bool
 	if cred.AccessKey == "" {

cmd/object-handlers.go

@@ -1850,7 +1850,7 @@ func (api objectAPIHandlers) PutObjectHandler(w http.ResponseWriter, r *http.Req
 		}
 	case authTypeStreamingUnsignedTrailer:
 		// Initialize stream chunked reader with optional trailers.
-		rd, s3Err = newUnsignedV4ChunkedReader(r, true)
+		rd, s3Err = newUnsignedV4ChunkedReader(r, true, r.Header.Get(xhttp.Authorization) != "")
 		if s3Err != ErrNone {
 			writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Err), r.URL)
 			return
@@ -2879,14 +2879,8 @@ func (api objectAPIHandlers) PutObjectRetentionHandler(w http.ResponseWriter, r
 	}
 
 	// Check permissions to perform this object retention operation
-	if s3Err := checkRequestAuthType(ctx, r, policy.PutObjectRetentionAction, bucket, object); s3Err != ErrNone {
+	if s3Error := authenticateRequest(ctx, r, policy.PutObjectRetentionAction); s3Error != ErrNone {
-		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Err), r.URL)
+		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
-		return
-	}
-
-	cred, owner, s3Err := validateSignature(getRequestAuthType(r), r)
-	if s3Err != ErrNone {
-		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Err), r.URL)
 		return
 	}
 
@@ -2912,6 +2906,7 @@ func (api objectAPIHandlers) PutObjectRetentionHandler(w http.ResponseWriter, r
 		writeErrorResponse(ctx, w, apiErr, r.URL)
 		return
 	}
+
 	reqInfo := logger.GetReqInfo(ctx)
 	reqInfo.SetTags("retention", objRetention.String())
 
@@ -2925,7 +2920,7 @@ func (api objectAPIHandlers) PutObjectRetentionHandler(w http.ResponseWriter, r
 		MTime:     opts.MTime,
 		VersionID: opts.VersionID,
 		EvalMetadataFn: func(oi *ObjectInfo, gerr error) (dsc ReplicateDecision, err error) {
-			if err := enforceRetentionBypassForPut(ctx, r, *oi, objRetention, cred, owner); err != nil {
+			if err := enforceRetentionBypassForPut(ctx, r, *oi, objRetention, reqInfo.Cred, reqInfo.Owner); err != nil {
 				return dsc, err
 			}
 			if objRetention.Mode.Valid() {

cmd/object-multipart-handlers.go

@@ -682,7 +682,7 @@ func (api objectAPIHandlers) PutObjectPartHandler(w http.ResponseWriter, r *http
 		}
 	case authTypeStreamingUnsignedTrailer:
 		// Initialize stream signature verifier.
-		reader, s3Error = newUnsignedV4ChunkedReader(r, true)
+		reader, s3Error = newUnsignedV4ChunkedReader(r, true, r.Header.Get(xhttp.Authorization) != "")
 		if s3Error != ErrNone {
 			writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
 			return

cmd/server_test.go

@@ -37,6 +37,7 @@ import (
 	"github.com/dustin/go-humanize"
 	jwtgo "github.com/golang-jwt/jwt/v4"
 	"github.com/minio/minio-go/v7/pkg/set"
+	"github.com/minio/minio-go/v7/pkg/signer"
 	xhttp "github.com/minio/minio/internal/http"
 	"github.com/minio/pkg/v3/policy"
 )
@@ -126,6 +127,7 @@ func runAllTests(suite *TestSuiteCommon, c *check) {
 	suite.TestMetricsV3Handler(c)
 	suite.TestBucketSQSNotificationWebHook(c)
 	suite.TestBucketSQSNotificationAMQP(c)
+	suite.TestUnsignedCVE(c)
 	suite.TearDownSuite(c)
 }
 
@@ -354,6 +356,59 @@ func (s *TestSuiteCommon) TestObjectDir(c *check) {
 	c.Assert(response.StatusCode, http.StatusNoContent)
 }
 
+func (s *TestSuiteCommon) TestUnsignedCVE(c *check) {
+	c.Helper()
+
+	// generate a random bucket Name.
+	bucketName := getRandomBucketName()
+
+	// HTTP request to create the bucket.
+	request, err := newTestSignedRequest(http.MethodPut, getMakeBucketURL(s.endPoint, bucketName),
+		0, nil, s.accessKey, s.secretKey, s.signer)
+	c.Assert(err, nil)
+
+	// execute the request.
+	response, err := s.client.Do(request)
+	c.Assert(err, nil)
+
+	// assert the http response status code.
+	c.Assert(response.StatusCode, http.StatusOK)
+
+	req, err := http.NewRequest(http.MethodPut, getPutObjectURL(s.endPoint, bucketName, "test-cve-object.txt"), nil)
+	c.Assert(err, nil)
+
+	req.Body = io.NopCloser(bytes.NewReader([]byte("foobar!\n")))
+	req.Trailer = http.Header{}
+	req.Trailer.Set("x-amz-checksum-crc32", "rK0DXg==")
+
+	now := UTCNow()
+
+	req = signer.StreamingUnsignedV4(req, "", 8, now)
+
+	maliciousHeaders := http.Header{
+		"Authorization":                []string{fmt.Sprintf("AWS4-HMAC-SHA256 Credential=%s/%s/us-east-1/s3/aws4_request, SignedHeaders=invalidheader, Signature=deadbeefdeadbeefdeadbeeddeadbeeddeadbeefdeadbeefdeadbeefdeadbeef", s.accessKey, now.Format(yyyymmdd))},
+		"User-Agent":                   []string{"A malicious request"},
+		"X-Amz-Decoded-Content-Length": []string{"8"},
+		"Content-Encoding":             []string{"aws-chunked"},
+		"X-Amz-Trailer":                []string{"x-amz-checksum-crc32"},
+		"x-amz-content-sha256":         []string{unsignedPayloadTrailer},
+	}
+
+	for k, v := range maliciousHeaders {
+		req.Header.Set(k, v[0])
+	}
+
+	// execute the request.
+	response, err = s.client.Do(req)
+	c.Assert(err, nil)
+
+	// out, err = httputil.DumpResponse(response, true)
+	// fmt.Println("RESPONSE ===\n", string(out), err)
+
+	// assert the http response status code.
+	c.Assert(response.StatusCode, http.StatusBadRequest)
+}
+
 func (s *TestSuiteCommon) TestBucketSQSNotificationAMQP(c *check) {
 	// Sample bucket notification.
 	bucketNotificationBuf := `<NotificationConfiguration><QueueConfiguration><Event>s3:ObjectCreated:Put</Event><Filter><S3Key><FilterRule><Name>prefix</Name><Value>images/</Value></FilterRule></S3Key></Filter><Id>1</Id><Queue>arn:minio:sqs:us-east-1:444455556666:amqp</Queue></QueueConfiguration></NotificationConfiguration>`

cmd/streaming-v4-unsigned.go

@@ -29,7 +29,12 @@ import (
 // newUnsignedV4ChunkedReader returns a new s3UnsignedChunkedReader that translates the data read from r
 // out of HTTP "chunked" format before returning it.
 // The s3ChunkedReader returns io.EOF when the final 0-length chunk is read.
-func newUnsignedV4ChunkedReader(req *http.Request, trailer bool) (io.ReadCloser, APIErrorCode) {
+func newUnsignedV4ChunkedReader(req *http.Request, trailer bool, signature bool) (io.ReadCloser, APIErrorCode) {
+	if signature {
+		if errCode := doesSignatureMatch(unsignedPayloadTrailer, req, globalSite.Region(), serviceS3); errCode != ErrNone {
+			return nil, errCode
+		}
+	}
 	if trailer {
 		// Discard anything unsigned.
 		req.Trailer = make(http.Header)