s3api: evaluate object-tag policy conditions on delete, ACL and lock operations (#11807)

* s3api: resolve the target version in tag-condition policy lookups

* s3api: apply tag-conditioned policy to object deletes

* s3api: apply tag-conditioned policy to object ACL reads and writes

* s3api: apply tag-conditioned policy to retention and legal hold

* s3api: cover tag-conditioned delete, ACL and object-lock rechecks

* s3api: let tag-conditioned grants redecide denied batch deletes

* s3api: resolve the tagged version in delete policy tests

weed/s3api/policy_engine/README_POLICY_ENGINE.md

@@ -193,9 +193,13 @@ This allows anonymous access only to objects that have a tag `status=public`.
 **Supported Operations for Tag-Based Conditions:**
 
 Tag-based conditions (`s3:ExistingObjectTag/<key>`) are evaluated for the following operations:
-- `s3:GetObject` (GET object)
-- `s3:GetObjectVersion` (GET object with versionId)
-- `s3:HeadObject` (HEAD object)
+- `s3:GetObject` / `s3:GetObjectVersion` / `s3:HeadObject` (object reads, including the source of CopyObject, UploadPartCopy and RenameObject)
+- `s3:DeleteObject` / `s3:DeleteObjectVersion` (single and batch deletes)
+- `s3:GetObjectAcl` / `s3:PutObjectAcl`
+- `s3:GetObjectTagging` / `s3:PutObjectTagging` / `s3:DeleteObjectTagging`
+- `s3:GetObjectAttributes`
+- `s3:GetObjectRetention` / `s3:PutObjectRetention`
+- `s3:GetObjectLegalHold` / `s3:PutObjectLegalHold`
 
 Note: For these conditions to be evaluated, the object must exist and the policy engine re-checks access after fetching the object metadata.
 

weed/s3api/s3_existing_object_tag_test.go

@@ -369,3 +369,210 @@ func TestRecheckPolicyForKeyWithLookup(t *testing.T) {
 	assert.Equal(t, s3err.ErrAccessDenied,
 		s3a.recheckPolicyForKeyWithLookup(req, http.MethodGet, "src-bucket", "src-key", "", string(s3_constants.ACTION_READ), "test"))
 }
+
+// A Deny on s3:DeleteObject / s3:DeleteObjectVersion conditioned on
+// s3:ExistingObjectTag only matches once the entry's tags are known; the
+// delete path's deferred lookup recheck must enforce it on the resolved
+// object. The batch-delete variant evaluates a synthesized DELETE request so
+// a key carrying a versionId resolves to s3:DeleteObjectVersion.
+func TestRecheckTagConditionedPolicyWithLookupDelete(t *testing.T) {
+	filerImpl := &ambiguousPutFiler{
+		entries: map[string]*filer_pb.Entry{
+			"/buckets/b/secret.txt": {
+				Name: "secret.txt",
+				Extended: map[string][]byte{
+					s3_constants.AmzObjectTaggingPrefix + "classification": []byte("secret"),
+				},
+			},
+		},
+	}
+	s3a := newPutTestServer(t, startFakeFiler(t, filerImpl))
+	s3a.policyEngine = NewBucketPolicyEngine()
+	s3a.bucketConfigCache = NewBucketConfigCache(time.Minute)
+	s3a.bucketConfigCache.Set("b", &BucketConfig{Name: "b"})
+
+	policyJSON := `{
+		"Version": "2012-10-17",
+		"Statement": [
+			{
+				"Effect": "Allow",
+				"Principal": "*",
+				"Action": "s3:*",
+				"Resource": ["arn:aws:s3:::b", "arn:aws:s3:::b/*"]
+			},
+			{
+				"Effect": "Deny",
+				"Principal": "*",
+				"Action": ["s3:DeleteObject", "s3:DeleteObjectVersion"],
+				"Resource": "arn:aws:s3:::b/*",
+				"Condition": {
+					"StringEquals": {
+						"s3:ExistingObjectTag/classification": ["secret"]
+					}
+				}
+			}
+		]
+	}`
+	require.NoError(t, s3a.policyEngine.engine.SetBucketPolicy("b", policyJSON))
+
+	identity := &Identity{Name: "writer", PrincipalArn: "arn:aws:iam::123456789012:user/writer"}
+
+	req := httptest.NewRequest(http.MethodDelete, "/b/secret.txt", nil)
+	req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), identity))
+
+	assert.Equal(t, s3err.ErrAccessDenied,
+		s3a.recheckTagConditionedPolicyWithLookup(req, "b", "secret.txt", "", string(s3_constants.ACTION_WRITE), "test"),
+		"a tag-conditioned Deny on s3:DeleteObject must block deleting the tagged object")
+
+	postReq := httptest.NewRequest(http.MethodPost, "/bv?delete", nil)
+	postReq = postReq.WithContext(s3_constants.SetIdentityInContext(postReq.Context(), identity))
+
+	filerImpl.entries["/buckets/bv/secret.txt"] = &filer_pb.Entry{
+		Name: "secret.txt",
+		Extended: map[string][]byte{
+			s3_constants.AmzObjectTaggingPrefix + "classification": []byte("public"),
+		},
+	}
+	filerImpl.entries["/buckets/bv/secret.txt.versions/v_v1"] = &filer_pb.Entry{
+		Name: "v_v1",
+		Extended: map[string][]byte{
+			s3_constants.AmzObjectTaggingPrefix + "classification": []byte("secret"),
+		},
+	}
+	filerImpl.entries["/buckets/bv/secret.txt.versions/v_v2"] = &filer_pb.Entry{
+		Name: "v_v2",
+		Extended: map[string][]byte{
+			s3_constants.AmzObjectTaggingPrefix + "classification": []byte("public"),
+		},
+	}
+	s3a.bucketConfigCache.Set("bv", &BucketConfig{Name: "bv", Versioning: s3_constants.VersioningEnabled})
+	require.NoError(t, s3a.policyEngine.engine.SetBucketPolicy("bv", `{
+		"Version": "2012-10-17",
+		"Statement": [
+			{
+				"Effect": "Allow",
+				"Principal": "*",
+				"Action": "s3:*",
+				"Resource": ["arn:aws:s3:::bv", "arn:aws:s3:::bv/*"]
+			},
+			{
+				"Effect": "Deny",
+				"Principal": "*",
+				"Action": "s3:DeleteObjectVersion",
+				"Resource": "arn:aws:s3:::bv/*",
+				"Condition": {
+					"StringEquals": {
+						"s3:ExistingObjectTag/classification": ["secret"]
+					}
+				}
+			}
+		]
+	}`))
+
+	keyReq := objectKeyPolicyRequest(postReq, http.MethodDelete, "bv", "secret.txt", "v1")
+	assert.Equal(t, s3err.ErrAccessDenied,
+		s3a.recheckTagConditionedPolicyWithLookup(keyReq, "bv", "secret.txt", "v1", string(s3_constants.ACTION_WRITE), "test"),
+		"a versioned batch-delete key must be denied by the s3:DeleteObjectVersion statement")
+
+	keyReq = objectKeyPolicyRequest(postReq, http.MethodDelete, "bv", "secret.txt", "v2")
+	assert.Equal(t, s3err.ErrNone,
+		s3a.recheckTagConditionedPolicyWithLookup(keyReq, "bv", "secret.txt", "v2", string(s3_constants.ACTION_WRITE), "test"),
+		"a publicly tagged version of the same object stays deletable")
+
+	filerImpl.entries["/buckets/ba/key"] = &filer_pb.Entry{
+		Name: "key",
+		Extended: map[string][]byte{
+			s3_constants.AmzObjectTaggingPrefix + "classification": []byte("secret"),
+		},
+	}
+	s3a.bucketConfigCache.Set("ba", &BucketConfig{Name: "ba"})
+	require.NoError(t, s3a.policyEngine.engine.SetBucketPolicy("ba", `{
+		"Version": "2012-10-17",
+		"Statement": [
+			{
+				"Effect": "Allow",
+				"Principal": "*",
+				"Action": "s3:DeleteObject",
+				"Resource": "arn:aws:s3:::ba/*",
+				"Condition": {
+					"StringEquals": {
+						"s3:ExistingObjectTag/classification": ["secret"]
+					}
+				}
+			}
+		]
+	}`))
+	assert.Equal(t, s3err.ErrNone,
+		s3a.recheckPolicyForKeyWithLookup(postReq, http.MethodDelete, "ba", "key", "", string(s3_constants.ACTION_WRITE), "test"),
+		"a tag-conditioned Allow on s3:DeleteObject must grant the resolved tagged key")
+
+	filerImpl.entries["/buckets/ba/key"].Extended[s3_constants.AmzObjectTaggingPrefix+"classification"] = []byte("public")
+	assert.Equal(t, s3err.ErrAccessDenied,
+		s3a.recheckPolicyForKeyWithLookup(postReq, http.MethodDelete, "ba", "key", "", string(s3_constants.ACTION_WRITE), "test"),
+		"a non-matching tag keeps the first-phase deny")
+
+	filerImpl.entries["/buckets/b/secret.txt"].Extended[s3_constants.AmzObjectTaggingPrefix+"classification"] = []byte("public")
+	assert.Equal(t, s3err.ErrNone,
+		s3a.recheckTagConditionedPolicyWithLookup(req, "b", "secret.txt", "", string(s3_constants.ACTION_WRITE), "test"),
+		"a non-matching tag leaves the delete allowed")
+
+	// A missing object reports no entry; the handler answers its own NoSuchKey.
+	delete(filerImpl.entries, "/buckets/b/secret.txt")
+	assert.Equal(t, s3err.ErrNone,
+		s3a.recheckTagConditionedPolicyWithLookup(req, "b", "secret.txt", "", string(s3_constants.ACTION_WRITE), "test"))
+}
+
+// Tag-conditioned Deny on the ACL subresources is enforced once the object
+// entry is in hand, the same way it is on a plain read.
+func TestRecheckObjectAclTagConditionedPolicy(t *testing.T) {
+	s3a := &S3ApiServer{
+		policyEngine: NewBucketPolicyEngine(),
+	}
+
+	policyJSON := `{
+		"Version": "2012-10-17",
+		"Statement": [
+			{
+				"Effect": "Allow",
+				"Principal": "*",
+				"Action": "s3:*",
+				"Resource": ["arn:aws:s3:::b", "arn:aws:s3:::b/*"]
+			},
+			{
+				"Effect": "Deny",
+				"Principal": "*",
+				"Action": ["s3:GetObjectAcl", "s3:PutObjectAcl"],
+				"Resource": "arn:aws:s3:::b/*",
+				"Condition": {
+					"StringEquals": {
+						"s3:ExistingObjectTag/classification": ["secret"]
+					}
+				}
+			}
+		]
+	}`
+	require.NoError(t, s3a.policyEngine.engine.SetBucketPolicy("b", policyJSON))
+
+	identity := &Identity{Name: "writer", PrincipalArn: "arn:aws:iam::123456789012:user/writer"}
+
+	getReq := httptest.NewRequest(http.MethodGet, "/b/secret.txt?acl", nil)
+	getReq = getReq.WithContext(s3_constants.SetIdentityInContext(getReq.Context(), identity))
+	putReq := httptest.NewRequest(http.MethodPut, "/b/secret.txt?acl", nil)
+	putReq = putReq.WithContext(s3_constants.SetIdentityInContext(putReq.Context(), identity))
+
+	secretEntry := map[string][]byte{
+		s3_constants.AmzObjectTaggingPrefix + "classification": []byte("secret"),
+	}
+	publicEntry := map[string][]byte{
+		s3_constants.AmzObjectTaggingPrefix + "classification": []byte("public"),
+	}
+
+	assert.Equal(t, s3err.ErrAccessDenied,
+		s3a.recheckPolicyWithObjectEntry(getReq, "b", "secret.txt", s3_constants.S3_ACTION_GET_OBJECT_ACL, secretEntry, "test"))
+	assert.Equal(t, s3err.ErrAccessDenied,
+		s3a.recheckPolicyWithObjectEntry(putReq, "b", "secret.txt", s3_constants.S3_ACTION_PUT_OBJECT_ACL, secretEntry, "test"))
+	assert.Equal(t, s3err.ErrNone,
+		s3a.recheckPolicyWithObjectEntry(getReq, "b", "secret.txt", s3_constants.S3_ACTION_GET_OBJECT_ACL, publicEntry, "test"))
+	assert.Equal(t, s3err.ErrNone,
+		s3a.recheckPolicyWithObjectEntry(putReq, "b", "secret.txt", s3_constants.S3_ACTION_PUT_OBJECT_ACL, publicEntry, "test"))
+}

weed/s3api/s3api_object_handlers_acl.go

@@ -89,6 +89,11 @@ func (s3a *S3ApiServer) GetObjectAclHandler(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
+	if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, s3_constants.S3_ACTION_GET_OBJECT_ACL, entry.Extended, "GetObjectAclHandler"); errCode != s3err.ErrNone {
+		s3err.WriteErrorResponse(w, r, errCode)
+		return
+	}
+
 	// Get object owner from metadata, fallback to request account
 	var objectOwner string
 	var objectOwnerDisplayName string
@@ -196,6 +201,11 @@ func (s3a *S3ApiServer) PutObjectAclHandler(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
+	if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, s3_constants.S3_ACTION_PUT_OBJECT_ACL, entry.Extended, "PutObjectAclHandler"); errCode != s3err.ErrNone {
+		s3err.WriteErrorResponse(w, r, errCode)
+		return
+	}
+
 	// Get current object owner from metadata
 	var objectOwner string
 	amzAccountId := r.Header.Get(s3_constants.AmzAccountId)

weed/s3api/s3api_object_handlers_delete.go

@@ -237,6 +237,11 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
+	if errCode := s3a.recheckTagConditionedPolicyWithLookup(r, bucket, object, versionId, string(s3_constants.ACTION_WRITE), "DeleteObjectHandler"); errCode != s3err.ErrNone {
+		s3err.WriteErrorResponse(w, r, errCode)
+		return
+	}
+
 	var auditLog *s3err.AccessLog
 	if s3err.Logger != nil {
 		auditLog = s3err.GetAccessLog(r, http.StatusNoContent, s3err.ErrNone)
@@ -479,7 +484,15 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h
 				continue
 			}
 			if authErr := s3a.iam.AuthorizeObjectDelete(r, identity, bucket, object.Key, object.VersionId); authErr != s3err.ErrNone {
-				deleteErrors = append(deleteErrors, deleteErrorFromCode(authErr, object.Key, object.VersionId))
+				// Defer to the recheck, mirroring the copy-source fallback: a
+				// tag-conditioned Allow may still grant what phase one denied.
+				if authErr != s3err.ErrAccessDenied || s3a.recheckPolicyForKeyWithLookup(r, http.MethodDelete, bucket, object.Key, object.VersionId, string(s3_constants.ACTION_WRITE), "DeleteMultipleObjectsHandler") != s3err.ErrNone {
+					deleteErrors = append(deleteErrors, deleteErrorFromCode(authErr, object.Key, object.VersionId))
+					continue
+				}
+			}
+			if errCode := s3a.recheckTagConditionedPolicyWithLookup(objectKeyPolicyRequest(r, http.MethodDelete, bucket, object.Key, object.VersionId), bucket, object.Key, object.VersionId, string(s3_constants.ACTION_WRITE), "DeleteMultipleObjectsHandler"); errCode != s3err.ErrNone {
+				deleteErrors = append(deleteErrors, deleteErrorFromCode(errCode, object.Key, object.VersionId))
 				continue
 			}
 

weed/s3api/s3api_object_handlers_legal_hold.go

@@ -25,6 +25,11 @@ func (s3a *S3ApiServer) PutObjectLegalHoldHandler(w http.ResponseWriter, r *http
 	// Get version ID from query parameters
 	versionId := r.URL.Query().Get("versionId")
 
+	if errCode := s3a.recheckTagConditionedPolicyWithLookup(r, bucket, object, versionId, s3_constants.ACTION_PUT_OBJECT_LEGAL_HOLD, "PutObjectLegalHoldHandler"); errCode != s3err.ErrNone {
+		s3err.WriteErrorResponse(w, r, errCode)
+		return
+	}
+
 	// Parse legal hold configuration from request body
 	legalHold, err := parseObjectLegalHold(r)
 	if err != nil {
@@ -81,6 +86,11 @@ func (s3a *S3ApiServer) GetObjectLegalHoldHandler(w http.ResponseWriter, r *http
 	// Get version ID from query parameters
 	versionId := r.URL.Query().Get("versionId")
 
+	if errCode := s3a.recheckTagConditionedPolicyWithLookup(r, bucket, object, versionId, s3_constants.ACTION_GET_OBJECT_LEGAL_HOLD, "GetObjectLegalHoldHandler"); errCode != s3err.ErrNone {
+		s3err.WriteErrorResponse(w, r, errCode)
+		return
+	}
+
 	// Get legal hold configuration for the object
 	legalHold, err := s3a.getObjectLegalHold(bucket, object, versionId)
 	if err != nil {

weed/s3api/s3api_object_handlers_retention.go

@@ -25,6 +25,11 @@ func (s3a *S3ApiServer) PutObjectRetentionHandler(w http.ResponseWriter, r *http
 	// Get version ID from query parameters
 	versionId := r.URL.Query().Get("versionId")
 
+	if errCode := s3a.recheckTagConditionedPolicyWithLookup(r, bucket, object, versionId, s3_constants.ACTION_PUT_OBJECT_RETENTION, "PutObjectRetentionHandler"); errCode != s3err.ErrNone {
+		s3err.WriteErrorResponse(w, r, errCode)
+		return
+	}
+
 	// Evaluate governance bypass request (header + permission validation)
 	governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object)
 
@@ -90,6 +95,11 @@ func (s3a *S3ApiServer) GetObjectRetentionHandler(w http.ResponseWriter, r *http
 	// Get version ID from query parameters
 	versionId := r.URL.Query().Get("versionId")
 
+	if errCode := s3a.recheckTagConditionedPolicyWithLookup(r, bucket, object, versionId, s3_constants.ACTION_GET_OBJECT_RETENTION, "GetObjectRetentionHandler"); errCode != s3err.ErrNone {
+		s3err.WriteErrorResponse(w, r, errCode)
+		return
+	}
+
 	// Get retention configuration for the object
 	retention, err := s3a.getObjectRetention(bucket, object, versionId)
 	if err != nil {

weed/s3api/s3api_object_handlers_tagging.go

@@ -185,7 +185,7 @@ func (s3a *S3ApiServer) PutObjectTaggingHandler(w http.ResponseWriter, r *http.R
 		target := util.FullPath(fmt.Sprintf("%s/%s", s3a.bucketDir(bucket), object))
 		dir, name := target.DirAndName()
 
-		if errCode := s3a.recheckTagConditionedPolicyWithLookup(r, bucket, object, s3_constants.S3_ACTION_PUT_OBJECT_TAGGING, "PutObjectTaggingHandler"); errCode != s3err.ErrNone {
+		if errCode := s3a.recheckTagConditionedPolicyWithLookup(r, bucket, object, versionId, s3_constants.S3_ACTION_PUT_OBJECT_TAGGING, "PutObjectTaggingHandler"); errCode != s3err.ErrNone {
 			s3err.WriteErrorResponse(w, r, errCode)
 			return
 		}
@@ -313,7 +313,7 @@ func (s3a *S3ApiServer) DeleteObjectTaggingHandler(w http.ResponseWriter, r *htt
 		target := util.FullPath(fmt.Sprintf("%s/%s", s3a.bucketDir(bucket), object))
 		dir, name := target.DirAndName()
 
-		if errCode := s3a.recheckTagConditionedPolicyWithLookup(r, bucket, object, s3_constants.S3_ACTION_DELETE_OBJECT_TAGGING, "DeleteObjectTaggingHandler"); errCode != s3err.ErrNone {
+		if errCode := s3a.recheckTagConditionedPolicyWithLookup(r, bucket, object, versionId, s3_constants.S3_ACTION_DELETE_OBJECT_TAGGING, "DeleteObjectTaggingHandler"); errCode != s3err.ErrNone {
 			s3err.WriteErrorResponse(w, r, errCode)
 			return
 		}

weed/s3api/s3api_server.go

@@ -19,6 +19,8 @@ import (
 
 	"github.com/gorilla/mux"
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 
 	"github.com/seaweedfs/seaweedfs/weed/cluster"
 	"github.com/seaweedfs/seaweedfs/weed/credential"
@@ -687,21 +689,19 @@ func (s3a *S3ApiServer) recheckCopySourcePolicy(r *http.Request, bucket, object,
 }
 
 // recheckTagConditionedPolicyWithLookup repeats policy evaluation with the
-// object's entry in hand so s3:ExistingObjectTag conditions apply to tagging
-// writes that did not otherwise fetch the entry. It only does the extra
+// object's entry in hand so s3:ExistingObjectTag conditions apply to
+// operations that did not otherwise fetch the entry. It only does the extra
 // lookup when the bucket has a policy; entry misses return ErrNone because
 // the handler reports its own NoSuchKey.
-func (s3a *S3ApiServer) recheckTagConditionedPolicyWithLookup(r *http.Request, bucket, object, action, handlerName string) s3err.ErrorCode {
+func (s3a *S3ApiServer) recheckTagConditionedPolicyWithLookup(r *http.Request, bucket, object, versionId, action, handlerName string) s3err.ErrorCode {
 	if s3a.policyEngine == nil || !s3a.policyEngine.HasPolicyForBucket(bucket) {
 		return s3err.ErrNone
 	}
-	target := util.FullPath(fmt.Sprintf("%s/%s", s3a.bucketDir(bucket), object))
-	dir, name := target.DirAndName()
-	entry, err := s3a.getEntry(dir, name)
+	entry, err := s3a.resolveObjectEntry(bucket, object, versionId)
 	if err != nil {
 		// The tag conditions cannot be evaluated without the entry; failing
 		// open would let a tag-gated Deny slip through a transient miss.
-		if errors.Is(err, filer_pb.ErrNotFound) {
+		if errors.Is(err, filer_pb.ErrNotFound) || status.Code(err) == codes.NotFound {
 			return s3err.ErrNone
 		}
 		glog.Errorf("%s: recheck lookup for %s/%s failed: %v", handlerName, bucket, object, err)