fix(s3api): authorize DeleteObjects per key so object-scoped policies match (#9793)

Bulk DeleteObjects carries the keys in the request body, so the route Auth middleware ran a single bucket-level check with object="", building the resource ARN as arn:aws:s3:::<bucket>. That never matches an s3:DeleteObject policy scoped to <bucket>/*, so the entire batch was denied even though the single-key DELETE worked with the same credentials. Defer authorization to the handler and check each key via AuthorizeBatchDeleteKey, mirroring AuthorizeCopySource: a synthetic DELETE /<bucket>/<key> request resolves s3:DeleteObject (or s3:DeleteObjectVersion when a versionId is given) against the object ARN. Denied keys come back as per-key errors while authorized keys still delete, matching AWS semantics.
2026-08-16 12:16:36 +00:00 · 2026-06-02 14:45:05 -07:00
parent b5a952bcb1
commit 8e4022d5c7
3 changed files with 180 additions and 0 deletions
@@ -1459,6 +1459,15 @@ func (iam *IdentityAccessManagement) authRequestWithAuthType(r *http.Request, ac
 		object = prefix
 	}

+	// Batch DeleteObjects keys arrive in the body, not the URL: a bucket-level check
+	// here can't match object-scoped policies. DeleteMultipleObjectsHandler authorizes
+	// each key via AuthorizeBatchDeleteKey.
+	if action == s3_constants.ACTION_WRITE && r.Method == http.MethodPost &&
+		object == "" && r.URL.Query().Has("delete") {
+		r.Header.Set(s3_constants.AmzAccountId, identity.Account.Id)
+		return identity, s3err.ErrNone, reqAuthType
+	}
+
 	// For ListBuckets, authorization is performed in the handler by iterating
 	// through buckets and checking permissions for each. Skip the global check here.
 	policyAllows := false
@@ -2334,6 +2343,73 @@ func (iam *IdentityAccessManagement) AuthorizeCopySource(r *http.Request, identi
 	return iam.VerifyActionPermission(srcReq, identity, Action(action), srcBucket, srcObject)
 }

+// AuthorizeBatchDeleteKey authorizes one key from a DeleteObjects body. The route
+// Auth middleware only authenticated the caller (keys arrive in the body, not the
+// URL), so each key is checked here against a synthetic DELETE /<bucket>/<key> that
+// makes ResolveS3Action and buildResourceARN target the object. Mirrors AuthorizeCopySource.
+func (iam *IdentityAccessManagement) AuthorizeBatchDeleteKey(r *http.Request, identity *Identity, bucket, objectKey, versionId string) s3err.ErrorCode {
+	if !iam.isEnabled() {
+		return s3err.ErrNone
+	}
+	if bucket == "" || objectKey == "" {
+		return s3err.ErrNone
+	}
+	if identity == nil {
+		return s3err.ErrAccessDenied
+	}
+	if identity.isAdmin() {
+		return s3err.ErrNone
+	}
+
+	// Shallow copy: authorization only reads headers, and this runs once per key.
+	keyReq := new(http.Request)
+	*keyReq = *r
+	keyURL := &url.URL{
+		Scheme: r.URL.Scheme,
+		Host:   r.URL.Host,
+		Path:   "/" + bucket + "/" + objectKey,
+	}
+	// Build the query from scratch so the envelope's "delete" param can't steer
+	// ResolveS3Action; keep the STS token and per-key versionId for policy eval.
+	keyQuery := make(url.Values)
+	if versionId != "" {
+		keyQuery.Set("versionId", versionId)
+	}
+	if strings.Contains(r.URL.RawQuery, "X-Amz-Security-Token") {
+		if token := r.URL.Query().Get("X-Amz-Security-Token"); token != "" {
+			keyQuery.Set("X-Amz-Security-Token", token)
+		}
+	}
+	if len(keyQuery) > 0 {
+		keyURL.RawQuery = keyQuery.Encode()
+	}
+	keyReq.URL = keyURL
+	keyReq.Method = http.MethodDelete
+	keyReq.RequestURI = ""
+	keyReq.Body = nil
+	keyReq.GetBody = nil
+	keyReq.ContentLength = 0
+
+	action := s3_constants.ACTION_WRITE
+
+	if iam.policyEngine != nil {
+		principal := buildPrincipalARN(identity, keyReq)
+		allowed, evaluated, err := iam.policyEngine.EvaluatePolicy(bucket, objectKey, action, principal, keyReq, identity.Claims, nil)
+		if err != nil {
+			glog.Errorf("DeleteObjects key policy evaluation failed for %s/%s: %v - denying", bucket, objectKey, err)
+			return s3err.ErrAccessDenied
+		}
+		if evaluated {
+			if allowed {
+				return s3err.ErrNone
+			}
+			return s3err.ErrAccessDenied
+		}
+	}
+
+	return iam.VerifyActionPermission(keyReq, identity, Action(action), bucket, objectKey)
+}
+
 // authorizeWithIAM authorizes requests using the IAM integration policy engine
 func (iam *IdentityAccessManagement) authorizeWithIAM(r *http.Request, identity *Identity, action Action, bucket string, object string) s3err.ErrorCode {
 	ctx := r.Context()
@@ -0,0 +1,93 @@
+package s3api
+
+import (
+	"encoding/json"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
+	"github.com/stretchr/testify/require"
+)
+
+// TestAuthorizeBatchDeleteKey_AwsCanonicalPolicy: a policy granting s3:DeleteObject
+// on <bucket>/* must allow per-key batch deletes. Pre-fix the bucket-level check
+// built arn:aws:s3:::<bucket> and never matched the object-scoped policy.
+func TestAuthorizeBatchDeleteKey_AwsCanonicalPolicy(t *testing.T) {
+	const bucket = "test-bucket"
+	const policyName = "delete-test-bucket-objects"
+
+	policyDoc, err := json.Marshal(map[string]any{
+		"Version": "2012-10-17",
+		"Statement": []map[string]any{
+			{
+				"Effect":   "Allow",
+				"Action":   "s3:DeleteObject",
+				"Resource": "arn:aws:s3:::" + bucket + "/*",
+			},
+		},
+	})
+	require.NoError(t, err)
+
+	iam := &IdentityAccessManagement{
+		isAuthEnabled: true,
+	}
+	require.NoError(t, iam.PutPolicy(policyName, string(policyDoc)))
+
+	identity := &Identity{
+		Name:        "alice",
+		Account:     &AccountAdmin,
+		PolicyNames: []string{policyName},
+		Credentials: []*Credential{{AccessKey: "AKIAEXAMPLE", SecretKey: "secret"}},
+	}
+
+	r := httptest.NewRequest("POST", "/"+bucket+"?delete", nil)
+
+	require.Equal(t, s3err.ErrNone,
+		iam.AuthorizeBatchDeleteKey(r, identity, bucket, "objects/a.txt", ""),
+		"s3:DeleteObject on arn:aws:s3:::%s/* must allow deleting %s/objects/a.txt", bucket, bucket)
+
+	require.Equal(t, s3err.ErrAccessDenied,
+		iam.AuthorizeBatchDeleteKey(r, identity, "other-bucket", "objects/a.txt", ""),
+		"keys outside the granted bucket must be denied")
+}
+
+// TestAuthorizeBatchDeleteKey_PrefixScopedPolicy: a prefix-scoped policy must allow
+// batch deletes under the prefix and deny keys outside it, per-key.
+func TestAuthorizeBatchDeleteKey_PrefixScopedPolicy(t *testing.T) {
+	const bucket = "test-bucket"
+	const policyName = "delete-prefix-only"
+
+	policyDoc, err := json.Marshal(map[string]any{
+		"Version": "2012-10-17",
+		"Statement": []map[string]any{
+			{
+				"Effect":   "Allow",
+				"Action":   "s3:DeleteObject",
+				"Resource": "arn:aws:s3:::" + bucket + "/safe/*",
+			},
+		},
+	})
+	require.NoError(t, err)
+
+	iam := &IdentityAccessManagement{
+		isAuthEnabled: true,
+	}
+	require.NoError(t, iam.PutPolicy(policyName, string(policyDoc)))
+
+	identity := &Identity{
+		Name:        "alice",
+		Account:     &AccountAdmin,
+		PolicyNames: []string{policyName},
+		Credentials: []*Credential{{AccessKey: "AKIAEXAMPLE", SecretKey: "secret"}},
+	}
+
+	r := httptest.NewRequest("POST", "/"+bucket+"?delete", nil)
+
+	require.Equal(t, s3err.ErrNone,
+		iam.AuthorizeBatchDeleteKey(r, identity, bucket, "safe/inside.txt", ""),
+		"key under granted prefix must be allowed")
+
+	require.Equal(t, s3err.ErrAccessDenied,
+		iam.AuthorizeBatchDeleteKey(r, identity, bucket, "danger/outside.txt", ""),
+		"key outside the granted prefix must be denied per-key, not at the batch level")
+}
@@ -409,6 +409,13 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h
 	versioningConfigured := (versioningState != "")
 	deletedCount := 0

+	// Per-key authorization: keys arrive in the body, so the route Auth middleware
+	// only authenticated. Authorize each key via AuthorizeBatchDeleteKey below.
+	var identity *Identity
+	if id := s3_constants.GetIdentityFromContext(r); id != nil {
+		identity, _ = id.(*Identity)
+	}
+
 	err = s3a.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
 		// delete file entries
 		for _, object := range deleteObjects.Objects {
@@ -419,6 +426,10 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h
 				deleteErrors = append(deleteErrors, deleteErrorFromCode(s3err.ErrAccessDenied, object.Key, object.VersionId))
 				continue
 			}
+			if authErr := s3a.iam.AuthorizeBatchDeleteKey(r, identity, bucket, object.Key, object.VersionId); authErr != s3err.ErrNone {
+				deleteErrors = append(deleteErrors, deleteErrorFromCode(authErr, object.Key, object.VersionId))
+				continue
+			}

 			var deleteResult deleteMutationResult
 			deleteCode := s3a.withObjectWriteLock(bucket, object.Key, func() s3err.ErrorCode {