diff --git a/weed/s3api/policy_engine/README_POLICY_ENGINE.md b/weed/s3api/policy_engine/README_POLICY_ENGINE.md index 9a5ab3b3c..b4846628d 100644 --- a/weed/s3api/policy_engine/README_POLICY_ENGINE.md +++ b/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/`) 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. diff --git a/weed/s3api/s3_existing_object_tag_test.go b/weed/s3api/s3_existing_object_tag_test.go index 9a986eb78..b3cb54f10 100644 --- a/weed/s3api/s3_existing_object_tag_test.go +++ b/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")) +} diff --git a/weed/s3api/s3api_object_handlers_acl.go b/weed/s3api/s3api_object_handlers_acl.go index 5f7340e6b..dc5f79eae 100644 --- a/weed/s3api/s3api_object_handlers_acl.go +++ b/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) diff --git a/weed/s3api/s3api_object_handlers_delete.go b/weed/s3api/s3api_object_handlers_delete.go index dec8e3ee4..4e28d9c5c 100644 --- a/weed/s3api/s3api_object_handlers_delete.go +++ b/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 } diff --git a/weed/s3api/s3api_object_handlers_legal_hold.go b/weed/s3api/s3api_object_handlers_legal_hold.go index da7af82b7..326ebb59a 100644 --- a/weed/s3api/s3api_object_handlers_legal_hold.go +++ b/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 { diff --git a/weed/s3api/s3api_object_handlers_retention.go b/weed/s3api/s3api_object_handlers_retention.go index 5695be25d..157bb99c4 100644 --- a/weed/s3api/s3api_object_handlers_retention.go +++ b/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 { diff --git a/weed/s3api/s3api_object_handlers_tagging.go b/weed/s3api/s3api_object_handlers_tagging.go index 9cb867603..37761322f 100644 --- a/weed/s3api/s3api_object_handlers_tagging.go +++ b/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 } diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index 9b01528bf..126cb124d 100644 --- a/weed/s3api/s3api_server.go +++ b/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)