fix(auth): secure proxied tagging and streaming trailers

Signed-off-by: Feng Ruohang <rh@vonng.com>

cmd/auth-handler.go

@@ -689,7 +689,15 @@ func setAuthMiddleware(h http.Handler) http.Handler {
 
 		aType := getRequestAuthType(r)
 		switch aType {
-		case authTypeSigned, authTypeSignedV2, authTypeStreamingSigned, authTypeStreamingSignedTrailer:
+		case authTypeSigned, authTypeSignedV2, authTypeStreamingSigned, authTypeStreamingSignedTrailer, authTypeStreamingUnsignedTrailer:
+			// STREAMING-UNSIGNED-PAYLOAD-TRAILER is valid for a genuinely
+			// anonymous request. Once Authorization is present, it is a
+			// signed REST-HEADER request and must obey the same date/skew
+			// bound as the other SigV4 streaming forms.
+			if aType == authTypeStreamingUnsignedTrailer && r.Header.Get(xhttp.Authorization) == "" {
+				h.ServeHTTP(w, r)
+				return
+			}
 			// Verify if date headers are set, if not reject the request
 			amzDate, errCode := parseAmzDateHeader(r)
 			if errCode != ErrNone {

cmd/bucket-policy.go

@@ -213,16 +213,27 @@ func getConditionValuesWithTags(r *http.Request, lc string, cred auth.Credential
 	switch authType {
 	case authTypeSignedV2, authTypePresignedV2:
 		signatureVersion = signV2Algorithm
-	case authTypeSigned, authTypePresigned, authTypeStreamingSigned, authTypePostPolicy:
+	case authTypeSigned, authTypePresigned, authTypeStreamingSigned,
+		authTypeStreamingSignedTrailer, authTypePostPolicy:
 		signatureVersion = signV4Algorithm
+	case authTypeStreamingUnsignedTrailer:
+		// This form is also valid for a genuinely anonymous request. Only
+		// expose SigV4 condition values when header authentication is present.
+		if r.Header.Get(xhttp.Authorization) != "" {
+			signatureVersion = signV4Algorithm
+		}
 	}
 
 	var authtype string
 	switch authType {
 	case authTypePresignedV2, authTypePresigned:
 		authtype = "REST-QUERY-STRING"
-	case authTypeSignedV2, authTypeSigned, authTypeStreamingSigned:
+	case authTypeSignedV2, authTypeSigned, authTypeStreamingSigned, authTypeStreamingSignedTrailer:
 		authtype = "REST-HEADER"
+	case authTypeStreamingUnsignedTrailer:
+		if r.Header.Get(xhttp.Authorization) != "" {
+			authtype = "REST-HEADER"
+		}
 	case authTypePostPolicy:
 		authtype = "POST"
 	}

cmd/bucket-replication.go

@@ -2852,6 +2852,7 @@ func proxyGetTaggingToRepTarget(ctx context.Context, bucket, object string, opts
 	var wg sync.WaitGroup
 	errs := make([]error, len(proxyTargets.Targets))
 	tagSlc := make([]map[string]string, len(proxyTargets.Targets))
+	contacted := 0
 	for idx, t := range proxyTargets.Targets {
 		tgt := globalBucketTargetSys.GetRemoteTargetClient(bucket, t.Arn)
 		if tgt == nil || globalBucketTargetSys.isOffline(tgt.EndpointURL()) {
@@ -2861,37 +2862,50 @@ func proxyGetTaggingToRepTarget(ctx context.Context, bucket, object string, opts
 		if tgt.disableProxy {
 			continue
 		}
+		contacted++
 		idx := idx
 		wg.Add(1)
 		go func(idx int, tgt *TargetClient) {
 			defer wg.Done()
-			var err error
 			gopts := minio.GetObjectTaggingOptions{
 				VersionID: opts.VersionID,
 				Internal: minio.AdvancedObjectTaggingOptions{
 					ReplicationProxyRequest: "true",
 				},
 			}
-			tgs, err = tgt.GetObjectTagging(ctx, tgt.Bucket, object, gopts)
+			remoteTags, err := tgt.GetObjectTagging(ctx, tgt.Bucket, object, gopts)
 			if err != nil {
 				errs[idx] = err
-			} else {
+			} else if remoteTags != nil {
-				tagSlc[idx] = tgs.ToMap()
+				tagSlc[idx] = remoteTags.ToMap()
 			}
 		}(idx, tgt)
 	}
 	wg.Wait()
+	if contacted == 0 {
+		return nil, proxyResult{Err: RemoteTargetConnectionErr{
+			Bucket: bucket,
+			Err:    errors.New("no available remote replication target"),
+		}}
+	}
+	var firstNotFound error
 	for idx, err := range errs {
 		errCode := minio.ToErrorResponse(err).Code
-		if err != nil && errCode != "NoSuchKey" && errCode != "NoSuchVersion" {
+		if err != nil {
+			if errCode != "NoSuchKey" && errCode != "NoSuchVersion" {
 				return nil, proxyResult{Err: err}
 			}
-		if err == nil {
+			if firstNotFound == nil {
+				firstNotFound = err
+			}
+			continue
+		}
+		if tagSlc[idx] != nil && tgs == nil {
 			tgs, _ = tags.MapToObjectTags(tagSlc[idx])
 		}
 	}
-	if len(errs) == 1 {
+	if tgs == nil {
-		proxy.Err = errs[0]
+		proxy.Err = firstNotFound
 	}
 	return tgs, proxy
 }

cmd/object-handlers.go

@@ -3754,6 +3754,18 @@ func (api objectAPIHandlers) GetObjectTaggingHandler(w http.ResponseWriter, r *h
 	writeSuccessResponseXML(w, encodeResponse(otags))
 }
 
+func isRemoteTaggingNotFound(err error) bool {
+	if err == nil {
+		return false
+	}
+	switch miniogo.ToErrorResponse(err).Code {
+	case "NoSuchKey", "NoSuchVersion":
+		return true
+	default:
+		return false
+	}
+}
+
 // PutObjectTaggingHandler - PUT object tagging
 func (api objectAPIHandlers) PutObjectTaggingHandler(w http.ResponseWriter, r *http.Request) {
 	ctx := newContext(r, w, "PutObjectTagging")
@@ -3773,6 +3785,13 @@ func (api objectAPIHandlers) PutObjectTaggingHandler(w http.ResponseWriter, r *h
 		return
 	}
 
+	logger.GetReqInfo(ctx).BucketName = bucket
+	logger.GetReqInfo(ctx).ObjectName = object
+	if s3Error := authenticateRequest(ctx, r, policy.PutObjectTaggingAction); s3Error != ErrNone {
+		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
+		return
+	}
+
 	// Tags XML will not be bigger than 1MiB in size, fail if its bigger.
 	tags, err := tags.ParseObjectXML(io.LimitReader(r.Body, 1<<20))
 	if err != nil {
@@ -3781,13 +3800,6 @@ func (api objectAPIHandlers) PutObjectTaggingHandler(w http.ResponseWriter, r *h
 	}
 	tagsStr := tags.String()
 
-	logger.GetReqInfo(ctx).BucketName = bucket
-	logger.GetReqInfo(ctx).ObjectName = object
-	if s3Error := authenticateRequest(ctx, r, policy.PutObjectTaggingAction); s3Error != ErrNone {
-		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
-		return
-	}
-
 	// Set this such that authorization policies can be applied on the object
 	// tags. This is derived from the request body, so it must be injected only
 	// after signature verification: the SigV4 verifier now rejects unsigned
@@ -3801,16 +3813,6 @@ func (api objectAPIHandlers) PutObjectTaggingHandler(w http.ResponseWriter, r *h
 	}
 
 	objInfo, err := objAPI.GetObjectInfo(ctx, bucket, object, opts)
-	var s3Error APIErrorCode
-	if err != nil {
-		s3Error = authorizeRequestWithUnreadTags(ctx, r, policy.PutObjectTaggingAction, err)
-	} else {
-		s3Error = authorizeRequestWithExistingTags(ctx, r, policy.PutObjectTaggingAction, objInfo.UserTags)
-	}
-	if s3Error != ErrNone {
-		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
-		return
-	}
 	if err != nil {
 		// if object is not found locally, but exists on peer site - proxy
 		// the tagging request to peer site. The response to client will
@@ -3818,6 +3820,23 @@ func (api objectAPIHandlers) PutObjectTaggingHandler(w http.ResponseWriter, r *h
 		if isErrObjectNotFound(err) || isErrVersionNotFound(err) {
 			proxytgts := getProxyTargets(ctx, bucket, object, opts)
 			if !proxytgts.Empty() {
+				// The local miss does not authorize a remote mutation. Read
+				// the addressed remote version first, then evaluate the
+				// caller against its actual ExistingObjectTag values.
+				remoteTags, readProxy := proxyGetTaggingToRepTarget(ctx, bucket, object, opts, proxytgts)
+				existingTags := ""
+				if remoteTags != nil {
+					existingTags = remoteTags.String()
+				}
+				if s3Error := authorizeRequestWithExistingTags(ctx, r, policy.PutObjectTaggingAction, existingTags); s3Error != ErrNone {
+					writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
+					return
+				}
+				if readProxy.Err != nil && !isRemoteTaggingNotFound(readProxy.Err) {
+					globalReplicationStats.Load().incProxy(bucket, putObjectTaggingAPI, true)
+					writeErrorResponse(ctx, w, toAPIError(ctx, readProxy.Err), r.URL)
+					return
+				}
 				globalReplicationStats.Load().incProxy(bucket, putObjectTaggingAPI, false)
 				// proxy to replication target if site replication is in place.
 				perr := proxyTaggingToRepTarget(ctx, bucket, object, tags, opts, proxytgts)
@@ -3841,9 +3860,26 @@ func (api objectAPIHandlers) PutObjectTaggingHandler(w http.ResponseWriter, r *h
 				return
 			}
 		}
+		// A local read failure must never be turned into a write grant. A
+		// server error may still be disclosed to an otherwise authorized
+		// caller for retry, while a missing object is evaluated as untagged.
+		var s3Error APIErrorCode
+		if isErrObjectNotFound(err) || isErrVersionNotFound(err) {
+			s3Error = authorizeRequestWithExistingTags(ctx, r, policy.PutObjectTaggingAction, "")
+		} else {
+			s3Error = authorizeRequestWithUnreadTags(ctx, r, policy.PutObjectTaggingAction, err)
+		}
+		if s3Error != ErrNone {
+			writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
+			return
+		}
 		writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
 		return
 	}
+	if s3Error := authorizeRequestWithExistingTags(ctx, r, policy.PutObjectTaggingAction, objInfo.UserTags); s3Error != ErrNone {
+		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
+		return
+	}
 
 	dsc := mustReplicate(ctx, bucket, object, getMustReplicateOptions(objInfo.UserDefined, tagsStr, objInfo.ReplicationStatus, replication.MetadataReplicationType, opts))
 	stamp := UTCNow().Format(time.RFC3339Nano)
@@ -3900,6 +3936,13 @@ func (api objectAPIHandlers) DeleteObjectTaggingHandler(w http.ResponseWriter, r
 		return
 	}
 
+	logger.GetReqInfo(ctx).BucketName = bucket
+	logger.GetReqInfo(ctx).ObjectName = object
+	if s3Error := authenticateRequest(ctx, r, policy.DeleteObjectTaggingAction); s3Error != ErrNone {
+		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
+		return
+	}
+
 	opts, err := getOpts(ctx, r, bucket, object)
 	if err != nil {
 		writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
@@ -3914,6 +3957,22 @@ func (api objectAPIHandlers) DeleteObjectTaggingHandler(w http.ResponseWriter, r
 		if isErrObjectNotFound(err) || isErrVersionNotFound(err) {
 			proxytgts := getProxyTargets(ctx, bucket, object, opts)
 			if !proxytgts.Empty() {
+				// Authenticate above and authorize against the remote version's
+				// tags before sending a destructive proxy request.
+				remoteTags, readProxy := proxyGetTaggingToRepTarget(ctx, bucket, object, opts, proxytgts)
+				existingTags := ""
+				if remoteTags != nil {
+					existingTags = remoteTags.String()
+				}
+				if s3Error := authorizeRequestWithExistingTags(ctx, r, policy.DeleteObjectTaggingAction, existingTags); s3Error != ErrNone {
+					writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
+					return
+				}
+				if readProxy.Err != nil && !isRemoteTaggingNotFound(readProxy.Err) {
+					globalReplicationStats.Load().incProxy(bucket, removeObjectTaggingAPI, true)
+					writeErrorResponse(ctx, w, toAPIError(ctx, readProxy.Err), r.URL)
+					return
+				}
 				globalReplicationStats.Load().incProxy(bucket, removeObjectTaggingAPI, false)
 				// proxy to replication target if active-active replication is in place.
 				perr := proxyTaggingToRepTarget(ctx, bucket, object, nil, opts, proxytgts)
@@ -3938,12 +3997,16 @@ func (api objectAPIHandlers) DeleteObjectTaggingHandler(w http.ResponseWriter, r
 				return
 			}
 		}
+		if s3Error := authorizeRequestWithExistingTags(ctx, r, policy.DeleteObjectTaggingAction, ""); s3Error != ErrNone {
+			writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
+			return
+		}
 		writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
 		return
 	}
 
 	// Allow deleteObjectTagging if policy action is set
-	if s3Error := checkRequestAuthTypeWithExistingTags(ctx, r, policy.DeleteObjectTaggingAction, bucket, object, oi.UserTags); s3Error != ErrNone {
+	if s3Error := authorizeRequestWithExistingTags(ctx, r, policy.DeleteObjectTaggingAction, oi.UserTags); s3Error != ErrNone {
 		writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
 		return
 	}

cmd/streaming-auth-boundary_test.go

@@ -24,6 +24,7 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/minio/minio/internal/auth"
 	xhttp "github.com/minio/minio/internal/http"
@@ -35,7 +36,7 @@ func TestAPIStreamingAuthenticationBoundaries(t *testing.T) {
 		t: t, endpoints: []string{"PutObject"},
 		objAPITest: func(obj ObjectLayer, backend, bucket string, router http.Handler, cred auth.Credentials, t *testing.T) {
 			data := []byte("authenticated streaming payload")
-			for _, mode := range []string{"valid", "missing-algorithm", "wrong-algorithm", "wrong-signature"} {
+			for _, mode := range []string{"valid", "missing-algorithm", "wrong-algorithm", "wrong-signature", "invalid-credential"} {
 				req, err := newStreamingUnsignedTrailerRequest(http.MethodPut, getPutObjectURL("", bucket, mode), data, UTCNow())
 				if err != nil {
 					t.Fatal(err)
@@ -55,6 +56,8 @@ func TestAPIStreamingAuthenticationBoundaries(t *testing.T) {
 						t.Fatal("signed request has no signature")
 					}
 					authorization = prefix + "Signature=" + strings.Repeat("0", 64)
+				case "invalid-credential":
+					authorization = strings.Replace(authorization, cred.AccessKey, strings.Repeat("A", len(cred.AccessKey)), 1)
 				}
 				req.Header.Set(xhttp.Authorization, authorization)
 				rec := httptest.NewRecorder()
@@ -112,3 +115,86 @@ func TestAPIStreamingAuthenticationBoundaries(t *testing.T) {
 		},
 	})
 }
+
+func TestStreamingTrailerConditionValues(t *testing.T) {
+	tests := []struct {
+		name, payload, authorization string
+		wantVersion, wantAuthType    string
+	}{
+		{
+			name:          "signed trailer",
+			payload:       streamingContentSHA256Trailer,
+			authorization: "AWS4-HMAC-SHA256 Credential=test",
+			wantVersion:   signV4Algorithm,
+			wantAuthType:  "REST-HEADER",
+		},
+		{
+			name:          "authenticated unsigned trailer",
+			payload:       unsignedPayloadTrailer,
+			authorization: "AWS4-HMAC-SHA256 Credential=test",
+			wantVersion:   signV4Algorithm,
+			wantAuthType:  "REST-HEADER",
+		},
+		{
+			name:         "anonymous unsigned trailer",
+			payload:      unsignedPayloadTrailer,
+			wantVersion:  "",
+			wantAuthType: "",
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			r := httptest.NewRequest(http.MethodPut, "http://minio.local/bucket/object", nil)
+			r.Header.Set(xhttp.AmzContentSha256, tc.payload)
+			if tc.authorization != "" {
+				r.Header.Set(xhttp.Authorization, tc.authorization)
+			}
+			values := getConditionValues(r, "", auth.Credentials{})
+			if got := strings.Join(values["signatureversion"], ""); got != tc.wantVersion {
+				t.Fatalf("signatureversion = %q, want %q", got, tc.wantVersion)
+			}
+			if got := strings.Join(values["authType"], ""); got != tc.wantAuthType {
+				t.Fatalf("authType = %q, want %q", got, tc.wantAuthType)
+			}
+		})
+	}
+}
+
+func TestStreamingUnsignedTrailerDateGate(t *testing.T) {
+	tests := []struct {
+		name       string
+		date       string
+		authorized bool
+		wantCall   bool
+	}{
+		{name: "anonymous without date", authorized: false, wantCall: true},
+		{name: "authenticated current", date: UTCNow().Format(iso8601Format), authorized: true, wantCall: true},
+		{name: "authenticated missing", authorized: true, wantCall: false},
+		{name: "authenticated malformed", date: "not-a-date", authorized: true, wantCall: false},
+		{name: "authenticated old", date: UTCNow().Add(-globalMaxSkewTime - time.Second).Format(iso8601Format), authorized: true, wantCall: false},
+		{name: "authenticated future", date: UTCNow().Add(globalMaxSkewTime + time.Second).Format(iso8601Format), authorized: true, wantCall: false},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			r := httptest.NewRequest(http.MethodPut, "http://minio.local/bucket/object", nil)
+			r.Header.Set(xhttp.AmzContentSha256, unsignedPayloadTrailer)
+			if tc.authorized {
+				r.Header.Set(xhttp.Authorization, "AWS4-HMAC-SHA256 Credential=test")
+			}
+			if tc.date != "" {
+				r.Header.Set(xhttp.AmzDate, tc.date)
+			}
+
+			called := false
+			h := setAuthMiddleware(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+				called = true
+				w.WriteHeader(http.StatusNoContent)
+			}))
+			rec := httptest.NewRecorder()
+			h.ServeHTTP(rec, r)
+			if called != tc.wantCall {
+				t.Fatalf("handler called = %v, want %v; status=%d body=%q", called, tc.wantCall, rec.Code, rec.Body.String())
+			}
+		})
+	}
+}