From b0c62970418cb4ce8aca1f2901e074d09b3b41ab Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 3 Nov 2026 14:52:20 -0800 Subject: [PATCH] s3api: per-bucket Block Public Access (BlockPublicAcls, IgnorePublicAcls) (#12381) * s3api: store and serve a bucket's PublicAccessBlock configuration PutPublicAccessBlock, GetPublicAccessBlock and DeletePublicAccessBlock answered NotImplemented. Store the configuration in the bucket entry's extended attributes next to the ownership controls, carry it in the cached BucketConfig, and answer Get on an unset bucket with NoSuchPublicAccessBlockConfiguration like S3 does. Co-Authored-By: Claude Opus 5.5 * s3api: refuse public ACLs when a bucket blocks them With BlockPublicAcls set, a PutObject, CopyObject, CreateMultipartUpload, POST Object, PutObjectAcl or PutBucketAcl whose canned ACL or grants name the AllUsers or AuthenticatedUsers group is refused with AccessDenied. Private and account grants keep working, so clients that send x-amz-acl: private on every upload are unaffected. Co-Authored-By: Claude Opus 5.5 * s3api: ignore public ACL grants for anonymous reads under IgnorePublicAcls A bucket with IgnorePublicAcls no longer lets an AllUsers grant on the bucket or on an object answer an anonymous read or listing. Bucket policies and identity permissions are unaffected. Co-Authored-By: Claude Opus 5.5 * s3api: reject PublicAccessBlock bodies with unknown elements A misspelled element such as BlockPublicAcl was ignored, so the request succeeded and stored the flag as false. Answer MalformedXML instead, and store the configuration without the request's namespace. Co-Authored-By: Claude Opus 5.5 * s3api: fail closed when a bucket's public access block cannot be read A stored PublicAccessBlock that does not parse now blocks and ignores public ACLs instead of acting as if none were set. PutBucketAcl and POST Object return the bucket lookup error rather than skipping the BlockPublicAcls check when the bucket config cannot be loaded. Co-Authored-By: Claude Opus 5.5 * s3api: test BlockPublicAcls on copy, multipart initiate and a PUT-then-GET Co-Authored-By: Claude Opus 5.5 * s3api: keep stored public access block bytes on unrelated bucket updates An unreadable stored configuration was replaced with the in-memory all-true fallback whenever another bucket setting was updated, losing the original value. Write the key only when the update changes the public access block. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- .../filer_multipart_initiate_acl_test.go | 14 + weed/s3api/s3_constants/extend_key.go | 1 + weed/s3api/s3api_bucket_config.go | 27 +- weed/s3api/s3api_bucket_handlers.go | 12 +- weed/s3api/s3api_bucket_policy_handlers.go | 54 +++- weed/s3api/s3api_object_anonymous_read.go | 6 +- .../s3api/s3api_object_anonymous_read_test.go | 32 +++ weed/s3api/s3api_object_handlers_acl.go | 3 + .../s3api_object_handlers_copy_acl_test.go | 10 +- .../s3api/s3api_object_handlers_postpolicy.go | 15 ++ ...i_object_handlers_postpolicy_authz_test.go | 45 ++++ weed/s3api/s3api_object_upload_acl.go | 3 + weed/s3api/s3api_object_upload_acl_test.go | 18 +- weed/s3api/s3api_public_access_block.go | 67 +++++ weed/s3api/s3api_public_access_block_test.go | 251 ++++++++++++++++++ weed/s3api/s3err/s3api_errors.go | 7 + 16 files changed, 555 insertions(+), 10 deletions(-) create mode 100644 weed/s3api/s3api_public_access_block.go create mode 100644 weed/s3api/s3api_public_access_block_test.go diff --git a/weed/s3api/filer_multipart_initiate_acl_test.go b/weed/s3api/filer_multipart_initiate_acl_test.go index 8f5dc43e3b..e05a0e29c4 100644 --- a/weed/s3api/filer_multipart_initiate_acl_test.go +++ b/weed/s3api/filer_multipart_initiate_acl_test.go @@ -123,4 +123,18 @@ func TestNewMultipartUploadInitiateACL(t *testing.T) { } require.True(t, publicRead, "public-read grants should persist on the upload entry: %s", string(uploadEntry(t, f).Extended[s3_constants.ExtAmzAclKey])) }) + + t.Run("blocked public acl is refused", func(t *testing.T) { + s3a, f := setup(t, true) + config, _ := s3a.bucketConfigCache.Get(bucket) + config.PublicAccessBlock = &PublicAccessBlockConfiguration{BlockPublicAcls: true} + rr := initiate(t, s3a, map[string]string{s3_constants.AmzCannedAcl: "public-read"}) + require.Equal(t, http.StatusForbidden, rr.Code, rr.Body.String()) + require.Contains(t, rr.Body.String(), "AccessDenied") + f.mu.Lock() + defer f.mu.Unlock() + for key := range f.entries { + require.False(t, strings.HasPrefix(key, "/buckets/"+bucket+"/"+s3_constants.MultipartUploadsFolder+"/"), key) + } + }) } diff --git a/weed/s3api/s3_constants/extend_key.go b/weed/s3api/s3_constants/extend_key.go index 959a962473..00c256e1ef 100644 --- a/weed/s3api/s3_constants/extend_key.go +++ b/weed/s3api/s3_constants/extend_key.go @@ -5,6 +5,7 @@ const ( ExtAmzOwnerKey = "Seaweed-X-Amz-Owner" ExtAmzAclKey = "Seaweed-X-Amz-Acl" ExtOwnershipKey = "Seaweed-X-Amz-Ownership" + ExtPublicAccessBlockKey = "Seaweed-X-Amz-Public-Access-Block" ExtVersioningKey = "Seaweed-X-Amz-Versioning" ExtVersionIdKey = "Seaweed-X-Amz-Version-Id" ExtDeleteMarkerKey = "Seaweed-X-Amz-Delete-Marker" diff --git a/weed/s3api/s3api_bucket_config.go b/weed/s3api/s3api_bucket_config.go index 427e58dcf5..5b5a884376 100644 --- a/weed/s3api/s3api_bucket_config.go +++ b/weed/s3api/s3api_bucket_config.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "encoding/xml" "errors" "fmt" "path/filepath" @@ -60,6 +61,7 @@ type BucketConfig struct { DeclaredEncryption *s3_pb.EncryptionConfiguration // declared by a table bucket's record KMSKeyCache *BucketKMSCache // Per-bucket KMS key cache for SSE-KMS operations LastModified time.Time + PublicAccessBlock *PublicAccessBlockConfiguration } // BucketKMSCache represents per-bucket KMS key caching for SSE-KMS operations @@ -430,10 +432,18 @@ func (s3a *S3ApiServer) newBucketConfigFromEntry(bucket string, entry *filer_pb. if ownership, exists := entry.Extended[s3_constants.ExtOwnershipKey]; exists { config.Ownership = string(ownership) } + if data, exists := entry.Extended[s3_constants.ExtPublicAccessBlockKey]; exists && len(data) > 0 { + if pab, err := parsePublicAccessBlock(data); err == nil { + config.PublicAccessBlock = pab + } else { + glog.Errorf("newBucketConfigFromEntry: bucket %s public access block, blocking all public access: %v", bucket, err) + config.PublicAccessBlock = &PublicAccessBlockConfiguration{BlockPublicAcls: true, IgnorePublicAcls: true, BlockPublicPolicy: true, RestrictPublicBuckets: true} + } + } if acl, exists := entry.Extended[s3_constants.ExtAmzAclKey]; exists { config.ACL = acl // Parse ACL once and cache public-read status. - config.IsPublicRead = parseAndCachePublicReadStatus(acl) + config.IsPublicRead = !config.PublicAccessBlock.ignoresPublicAcls() && parseAndCachePublicReadStatus(acl) } config.Owner = bucketOwnerAccountId(s3a.iam, entry) if identityId, exists := entry.Extended[s3_constants.AmzIdentityId]; exists { @@ -509,6 +519,8 @@ func (s3a *S3ApiServer) updateBucketConfig(bucket string, updateFn func(*BucketC } config := s3a.newBucketConfigFromEntry(bucket, entry) + loadedPAB := config.PublicAccessBlock + loadedPABXML, _ := xml.Marshal(loadedPAB) // Apply update function if err := updateFn(config); err != nil { @@ -525,6 +537,10 @@ func (s3a *S3ApiServer) updateBucketConfig(bucket string, updateFn func(*BucketC glog.Errorf("updateBucketConfig: failed to serialize config for bucket %s: %v", bucket, err) return s3err.ErrInternalError } + // Keep the stored bytes, readable or not, unless updateFn changed the setting. + if loadedPAB != nil && config.PublicAccessBlock == loadedPAB && bytes.Equal(newExt[s3_constants.ExtPublicAccessBlockKey], loadedPABXML) { + newExt[s3_constants.ExtPublicAccessBlockKey] = oldExt[s3_constants.ExtPublicAccessBlockKey] + } // Patch only the changed/removed extended keys, leaving Entry.content // untouched so a concurrent content write (e.g. encryption) is preserved. @@ -571,6 +587,15 @@ func applyBucketConfigToExtended(config *BucketConfig, ext map[string][]byte) er } setOrDelete(s3_constants.ExtVersioningKey, []byte(config.Versioning)) setOrDelete(s3_constants.ExtOwnershipKey, []byte(config.Ownership)) + if config.PublicAccessBlock != nil { + data, err := xml.Marshal(config.PublicAccessBlock) + if err != nil { + return err + } + ext[s3_constants.ExtPublicAccessBlockKey] = data + } else { + delete(ext, s3_constants.ExtPublicAccessBlockKey) + } setOrDelete(s3_constants.ExtAmzAclKey, config.ACL) setOrDelete(s3_constants.ExtAmzOwnerKey, []byte(config.Owner)) setOrDelete(bucketLifecycleConfigurationXMLKey, config.LifecycleXML) diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go index b6886c0954..7a86457ff2 100644 --- a/weed/s3api/s3api_bucket_handlers.go +++ b/weed/s3api/s3api_bucket_handlers.go @@ -1122,8 +1122,13 @@ func (s3a *S3ApiServer) PutBucketAclHandler(w http.ResponseWriter, r *http.Reque // the bucket has one, else the request's account — an anonymous request // resolves to the admin account, the registry's default bucket owner. ownerId := aclAccountId - if config, errCode := s3a.getBucketConfig(bucket); errCode == s3err.ErrNone && config.Owner != "" { - ownerId = config.Owner + bucketConfig, errCode := s3a.getBucketConfig(bucket) + if errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + if bucketConfig.Owner != "" { + ownerId = bucketConfig.Owner } // Get bucket ownership settings (these would be used for ownership validation in a full implementation) @@ -1132,6 +1137,9 @@ func (s3a *S3ApiServer) PutBucketAclHandler(w http.ResponseWriter, r *http.Reque // Use the existing ACL parsing logic to handle both canned ACLs and XML body grants, errCode := ExtractAcl(r, s3a.iam, bucketOwnership, bucketOwnerId, ownerId, aclAccountId) + if errCode == s3err.ErrNone { + errCode = rejectPublicGrants(bucketConfig, grants) + } if errCode != s3err.ErrNone { s3err.WriteErrorResponse(w, r, errCode) return diff --git a/weed/s3api/s3api_bucket_policy_handlers.go b/weed/s3api/s3api_bucket_policy_handlers.go index 6477a18f45..95620ee772 100644 --- a/weed/s3api/s3api_bucket_policy_handlers.go +++ b/weed/s3api/s3api_bucket_policy_handlers.go @@ -479,13 +479,61 @@ func bucketPolicyMirrorOps(oldEntry, newEntry *filer_pb.Entry) (removeName, upda // GetPublicAccessBlockHandler Retrieves the PublicAccessBlock configuration for an S3 bucket // https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetPublicAccessBlock.html func (s3a *S3ApiServer) GetPublicAccessBlockHandler(w http.ResponseWriter, r *http.Request) { - s3err.WriteErrorResponse(w, r, s3err.ErrNotImplemented) + bucket, _ := s3_constants.GetBucketAndObject(r) + config, errCode := s3a.getBucketConfig(bucket) + if errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + if config.PublicAccessBlock == nil { + s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchPublicAccessBlockConfiguration) + return + } + response := *config.PublicAccessBlock + response.XMLNS = s3_constants.S3Namespace + writeSuccessResponseXML(w, r, response) } +// PutPublicAccessBlockHandler https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutPublicAccessBlock.html func (s3a *S3ApiServer) PutPublicAccessBlockHandler(w http.ResponseWriter, r *http.Request) { - s3err.WriteErrorResponse(w, r, s3err.ErrNotImplemented) + bucket, _ := s3_constants.GetBucketAndObject(r) + if err := s3a.checkBucket(r, bucket); err != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, err) + return + } + body, err := readRequestBody(r, maxPublicAccessBlockConfigurationSize) + if err != nil { + if errors.Is(err, errRequestBodyTooLarge) { + s3err.WriteErrorResponse(w, r, s3err.ErrEntityTooLarge) + return + } + s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest) + return + } + config, err := parsePublicAccessBlock(body) + if err != nil { + s3err.WriteErrorResponse(w, r, s3err.ErrMalformedXML) + return + } + if errCode := s3a.updateBucketConfig(bucket, func(bucketConfig *BucketConfig) error { + bucketConfig.PublicAccessBlock = config + return nil + }); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + writeSuccessResponseEmpty(w, r) } +// DeletePublicAccessBlockHandler https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeletePublicAccessBlock.html func (s3a *S3ApiServer) DeletePublicAccessBlockHandler(w http.ResponseWriter, r *http.Request) { - s3err.WriteErrorResponse(w, r, s3err.ErrNotImplemented) + bucket, _ := s3_constants.GetBucketAndObject(r) + if errCode := s3a.updateBucketConfig(bucket, func(bucketConfig *BucketConfig) error { + bucketConfig.PublicAccessBlock = nil + return nil + }); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + s3err.WriteEmptyResponse(w, r, http.StatusNoContent) } diff --git a/weed/s3api/s3api_object_anonymous_read.go b/weed/s3api/s3api_object_anonymous_read.go index 6fd86001aa..1e16c73d71 100644 --- a/weed/s3api/s3api_object_anonymous_read.go +++ b/weed/s3api/s3api_object_anonymous_read.go @@ -87,8 +87,10 @@ func (s3a *S3ApiServer) authorizeAnonymousObjectRead(r *http.Request, bucket, ob return s3err.ErrNone } // Match the upload/ACL path: legacy buckets without recorded ownership - // controls still accept ACLs. An enforced control explicitly disables them. - if config.Ownership == "" || s3_constants.EffectiveOwnership(config.Ownership) != s3_constants.OwnershipBucketOwnerEnforced { + // controls still accept ACLs. An enforced control explicitly disables them, + // and IgnorePublicAcls leaves an object ACL nothing it could grant here. + if !config.PublicAccessBlock.ignoresPublicAcls() && + (config.Ownership == "" || s3_constants.EffectiveOwnership(config.Ownership) != s3_constants.OwnershipBucketOwnerEnforced) { if data, exists := extended[s3_constants.ExtAmzAclKey]; exists { // An explicitly stored ACL replaces the legacy bucket-ACL fallback. // Empty, malformed and non-public ACLs must not grant anonymous read. diff --git a/weed/s3api/s3api_object_anonymous_read_test.go b/weed/s3api/s3api_object_anonymous_read_test.go index 2f27c78acf..0efac46b93 100644 --- a/weed/s3api/s3api_object_anonymous_read_test.go +++ b/weed/s3api/s3api_object_anonymous_read_test.go @@ -288,6 +288,38 @@ func TestAnonymousObjectACLLegacyOwnership(t *testing.T) { require.Equal(t, 200, rr.Code, rr.Body.String()) } +func TestAnonymousObjectACLIgnorePublicAcls(t *testing.T) { + publicBucket := &filer_pb.Entry{Name: "b", IsDirectory: true, Extended: map[string][]byte{ + s3_constants.ExtAmzAclKey: anonymousReadGrant("READ"), + }} + ignoring := proto.Clone(publicBucket).(*filer_pb.Entry) + ignoring.Extended[s3_constants.ExtPublicAccessBlockKey] = []byte(`true`) + for _, tc := range []struct { + name string + bucket *filer_pb.Entry + acl []byte + want int + }{ + {"public object", publicBucket, anonymousReadGrant("READ"), 200}, + {"public object ignored", ignoring, anonymousReadGrant("READ"), 403}, + {"public bucket", publicBucket, nil, 200}, + {"public bucket ignored", ignoring, nil, 403}, + } { + for _, method := range []string{http.MethodGet, http.MethodHead} { + t.Run(tc.name+"/"+method, func(t *testing.T) { + s3a, f := newAnonymousReadTestServer(t) + s3a.bucketConfigCache.Set("b", s3a.newBucketConfigFromEntry("b", tc.bucket)) + f.entries["/buckets/b/o"] = anonymousReadEntry(tc.acl) + rr := serveAnonymousRead(s3a, method, "o", "", nil) + require.Equal(t, tc.want, rr.Code, rr.Body.String()) + if tc.want == 403 { + require.NotContains(t, rr.Body.String(), "hello world") + } + }) + } + } +} + func TestAnonymousObjectACLConfiguredIdentity(t *testing.T) { for _, tc := range []struct { name string diff --git a/weed/s3api/s3api_object_handlers_acl.go b/weed/s3api/s3api_object_handlers_acl.go index 1cee201d84..f910b3a394 100644 --- a/weed/s3api/s3api_object_handlers_acl.go +++ b/weed/s3api/s3api_object_handlers_acl.go @@ -289,6 +289,9 @@ func (s3a *S3ApiServer) PutObjectAclHandler(w http.ResponseWriter, r *http.Reque // Extract ACL from request (either canned ACL or XML body) // This function also validates that the owner in the request matches the object owner grants, errCode := ExtractAcl(r, s3a.iam, bucketOwnership, bucketOwnerId, objectOwner, amzAccountId) + if errCode == s3err.ErrNone { + errCode = rejectPublicGrants(bucketConfig, grants) + } if errCode != s3err.ErrNone { s3err.WriteErrorResponse(w, r, errCode) return diff --git a/weed/s3api/s3api_object_handlers_copy_acl_test.go b/weed/s3api/s3api_object_handlers_copy_acl_test.go index ef2997e582..583b27aa08 100644 --- a/weed/s3api/s3api_object_handlers_copy_acl_test.go +++ b/weed/s3api/s3api_object_handlers_copy_acl_test.go @@ -153,6 +153,7 @@ func TestCopyObjectDestinationACL(t *testing.T) { mime string race, condition string prefix bool + blockPublicAcls bool }{ {name: "default private"}, {name: "replace metadata also defaults private", replace: true}, @@ -204,6 +205,9 @@ func TestCopyObjectDestinationACL(t *testing.T) { {name: "prefix private control", self: true, replace: true, routed: true, prefix: true}, {name: "prefix mode changes before commit", self: true, replace: true, routed: true, prefix: true, race: "commit"}, {name: "self copy reads owner ahead of replica", self: true, replace: true, routed: true, race: "replica"}, + {name: "block public acls private", acl: "private", blockPublicAcls: true}, + {name: "block public acls rejects public", acl: "public-read", blockPublicAcls: true, errorCode: "AccessDenied"}, + {name: "block public acls rejects public self copy", self: true, replace: true, routed: true, acl: "public-read", blockPublicAcls: true, errorCode: "AccessDenied"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -234,7 +238,11 @@ func TestCopyObjectDestinationACL(t *testing.T) { }} filer.entries["/buckets/"+bucket] = bucketEntry s3a.bucketConfigCache = NewBucketConfigCache(time.Minute) - s3a.bucketConfigCache.Set(bucket, &BucketConfig{Name: bucket, Owner: bucketOwner, Ownership: ownership, Versioning: tt.versioning}) + bucketConfig := &BucketConfig{Name: bucket, Owner: bucketOwner, Ownership: ownership, Versioning: tt.versioning} + if tt.blockPublicAcls { + bucketConfig.PublicAccessBlock = &PublicAccessBlockConfiguration{BlockPublicAcls: true} + } + s3a.bucketConfigCache.Set(bucket, bucketConfig) s3a.bucketRegistry = NewBucketRegistry(s3a) s3a.bucketRegistry.LoadBucketMetadata(bucketEntry) mode := tt.sourceMode diff --git a/weed/s3api/s3api_object_handlers_postpolicy.go b/weed/s3api/s3api_object_handlers_postpolicy.go index 51d21c4bf8..69139ce72c 100644 --- a/weed/s3api/s3api_object_handlers_postpolicy.go +++ b/weed/s3api/s3api_object_handlers_postpolicy.go @@ -158,6 +158,21 @@ func (s3a *S3ApiServer) PostPolicyBucketHandler(w http.ResponseWriter, r *http.R s3err.WriteErrorResponse(w, r, refineDenialForBucketName(r, identity, errCode)) return } + config, errCode := s3a.getBucketConfig(bucket) + if errCode != s3err.ErrNone && errCode != s3err.ErrNoSuchBucket { + s3err.WriteErrorResponse(w, r, errCode) + return + } + if errCode == s3err.ErrNone && config.PublicAccessBlock.blocksPublicAcls() { + _, grants, aclCode := ParseAclHeaders(r, config.Ownership, config.Owner, "", false) + if aclCode == s3err.ErrNone { + aclCode = rejectPublicGrants(config, grants) + } + if aclCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, aclCode) + return + } + } } versioningState, err := s3a.getVersioningState(bucket) diff --git a/weed/s3api/s3api_object_handlers_postpolicy_authz_test.go b/weed/s3api/s3api_object_handlers_postpolicy_authz_test.go index c341979181..0206710067 100644 --- a/weed/s3api/s3api_object_handlers_postpolicy_authz_test.go +++ b/weed/s3api/s3api_object_handlers_postpolicy_authz_test.go @@ -495,3 +495,48 @@ func TestPostPolicyBucketHandlerAutoCreateDeniedForNonAdmin(t *testing.T) { defer filer.mu.Unlock() assert.Empty(t, filer.entries, "a non-admin POST must not create the bucket") } + +// A bucket that blocks public ACLs refuses a POST upload whose acl or grant +// fields would make the object public, and still accepts a private one. +func TestPostPolicyBucketHandlerBlocksPublicAcls(t *testing.T) { + newServer := func(t *testing.T) (*S3ApiServer, *ambiguousPutFiler) { + filer := &ambiguousPutFiler{ + volume: startFakeVolumeServer(t), + entries: map[string]*filer_pb.Entry{"/buckets/" + postAuthzBucket: { + Name: postAuthzBucket, + IsDirectory: true, + Attributes: &filer_pb.FuseAttributes{}, + Extended: map[string][]byte{ + s3_constants.ExtAmzOwnerKey: []byte("000000000000"), + s3_constants.ExtPublicAccessBlockKey: []byte(`true`), + }, + }}, + apply: true, + } + return newPostAutoCreateServer(t, filer), filer + } + const key = "uploads/pic.png" + for _, fields := range []map[string]string{ + {"acl": "public-read"}, + {"acl": "authenticated-read"}, + {"x-amz-grant-read": `uri="` + s3_constants.GranteeGroupAllUsers + `"`}, + } { + s3a, filer := newServer(t) + rec := httptest.NewRecorder() + s3a.PostPolicyBucketHandler(rec, newSignedPostRequestFor(t, key, postAdminAccessKey, postAdminSecretKey, fields)) + assert.Equal(t, http.StatusForbidden, rec.Code, "%v must be refused, body: %s", fields, rec.Body.String()) + assert.Contains(t, rec.Body.String(), "AccessDenied") + filer.mu.Lock() + assert.Nil(t, filer.entries["/buckets/"+postAuthzBucket+"/"+key], "%v must not write the object", fields) + filer.mu.Unlock() + } + + s3a, filer := newServer(t) + rec := httptest.NewRecorder() + s3a.PostPolicyBucketHandler(rec, newSignedPostRequestFor(t, key, postAdminAccessKey, postAdminSecretKey, + map[string]string{"acl": "private", "success_action_status": "201"})) + assert.Equal(t, http.StatusCreated, rec.Code, "a private POST must succeed, body: %s", rec.Body.String()) + filer.mu.Lock() + defer filer.mu.Unlock() + assert.NotNil(t, filer.entries["/buckets/"+postAuthzBucket+"/"+key]) +} diff --git a/weed/s3api/s3api_object_upload_acl.go b/weed/s3api/s3api_object_upload_acl.go index 0fa9f44025..4c02ed3c24 100644 --- a/weed/s3api/s3api_object_upload_acl.go +++ b/weed/s3api/s3api_object_upload_acl.go @@ -212,6 +212,9 @@ func (s3a *S3ApiServer) preparePutObjectACL(r *http.Request, bucket string) (*ht if code == s3err.ErrNone && custom { grants, code = ValidateAndTransferGrants(s3a.iam, grants) } + if code == s3err.ErrNone { + code = rejectPublicGrants(metadata, grants) + } if code != s3err.ErrNone { return r, code } diff --git a/weed/s3api/s3api_object_upload_acl_test.go b/weed/s3api/s3api_object_upload_acl_test.go index 067145376f..b24ed15c68 100644 --- a/weed/s3api/s3api_object_upload_acl_test.go +++ b/weed/s3api/s3api_object_upload_acl_test.go @@ -45,6 +45,7 @@ func TestPutObjectUploadACL(t *testing.T) { copySource string grantAccount, grantEmail string unregisteredWriter bool + blockPublicAcls bool } tests := []uploadACLTest{ {name: "default private", status: 200}, @@ -151,6 +152,17 @@ func TestPutObjectUploadACL(t *testing.T) { {name: "single line grants preserve condition deny", grantHeader: s3_constants.AmzAclRead, grant: `id="bucket-owner",id="upload-writer"`, policy: "bucket-condition-deny", conditionValue: `id="bucket-owner",id="upload-writer"`, status: 403, errorCode: "AccessDenied"}, {name: "presigned grants preserve condition deny", grantHeader: s3_constants.AmzAclRead, grant: `id="bucket-owner",id="upload-writer"`, presigned: true, policy: "bucket-condition-deny", conditionValue: `id="bucket-owner",id="upload-writer"`, status: 403, errorCode: "AccessDenied"}, {name: "extra grantee defeats allow condition", grantHeader: s3_constants.AmzAclRead, grant: `id="bucket-owner"`, repeatedGrant: `id="upload-writer"`, writeOnly: true, policy: "bucket-condition-allow", conditionValue: `id="bucket-owner"`, status: 403, errorCode: "AccessDenied"}, + {name: "block public acls default", blockPublicAcls: true, status: 200}, + {name: "block public acls private", acl: "private", blockPublicAcls: true, status: 200}, + {name: "block public acls bucket owner full control", acl: "bucket-owner-full-control", blockPublicAcls: true, status: 200}, + {name: "block public acls account grant", grantHeader: s3_constants.AmzAclRead, grant: `id="bucket-owner"`, blockPublicAcls: true, status: 200}, + {name: "block public acls rejects public read", acl: "public-read", blockPublicAcls: true, status: 403, errorCode: "AccessDenied"}, + {name: "block public acls rejects public read write", acl: "public-read-write", blockPublicAcls: true, status: 403, errorCode: "AccessDenied"}, + {name: "block public acls rejects authenticated read", acl: "authenticated-read", blockPublicAcls: true, status: 403, errorCode: "AccessDenied"}, + {name: "block public acls rejects presigned public read", acl: "public-read", presigned: true, blockPublicAcls: true, status: 403, errorCode: "AccessDenied"}, + {name: "block public acls rejects all users grant", grantHeader: s3_constants.AmzAclRead, grant: `uri="` + s3_constants.GranteeGroupAllUsers + `"`, blockPublicAcls: true, status: 403, errorCode: "AccessDenied"}, + {name: "block public acls rejects authenticated users grant", grantHeader: s3_constants.AmzAclFullControl, grant: `id="bucket-owner",uri="` + s3_constants.GranteeGroupAuthenticatedUsers + `"`, blockPublicAcls: true, status: 403, errorCode: "AccessDenied"}, + {name: "block public acls rejected overwrite preserves acl", overwrite: true, acl: "public-read", blockPublicAcls: true, status: 403, errorCode: "AccessDenied"}, } // Invalid multipart parameters or copy headers must not bypass policy normalization on the actual regular-upload route. for _, policy := range []string{"bucket", "iam"} { @@ -373,7 +385,11 @@ func TestPutObjectUploadACL(t *testing.T) { } filer.entries["/buckets/"+bucket] = bucketEntry s3a.bucketConfigCache = NewBucketConfigCache(time.Minute) - s3a.bucketConfigCache.Set(bucket, &BucketConfig{Name: bucket, Owner: bucketOwner, Ownership: storedOwnership, Versioning: tt.versioning}) + bucketConfig := &BucketConfig{Name: bucket, Owner: bucketOwner, Ownership: storedOwnership, Versioning: tt.versioning} + if tt.blockPublicAcls { + bucketConfig.PublicAccessBlock = &PublicAccessBlockConfiguration{BlockPublicAcls: true} + } + s3a.bucketConfigCache.Set(bucket, bucketConfig) s3a.bucketRegistry = NewBucketRegistry(s3a) s3a.bucketRegistry.LoadBucketMetadata(bucketEntry) if tt.versioning == s3_constants.VersioningEnabled { diff --git a/weed/s3api/s3api_public_access_block.go b/weed/s3api/s3api_public_access_block.go new file mode 100644 index 0000000000..7e9ee204d3 --- /dev/null +++ b/weed/s3api/s3api_public_access_block.go @@ -0,0 +1,67 @@ +package s3api + +import ( + "encoding/xml" + "fmt" + + "github.com/aws/aws-sdk-go/service/s3" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" +) + +const maxPublicAccessBlockConfigurationSize = 4 << 10 + +// PublicAccessBlockConfiguration is a bucket's S3 Block Public Access setting. +type PublicAccessBlockConfiguration struct { + XMLNS string `xml:"xmlns,attr,omitempty"` + XMLName xml.Name `xml:"PublicAccessBlockConfiguration"` + BlockPublicAcls bool `xml:"BlockPublicAcls"` + IgnorePublicAcls bool `xml:"IgnorePublicAcls"` + BlockPublicPolicy bool `xml:"BlockPublicPolicy"` + RestrictPublicBuckets bool `xml:"RestrictPublicBuckets"` + Unknown []xml.Name `xml:",any"` +} + +func parsePublicAccessBlock(data []byte) (*PublicAccessBlockConfiguration, error) { + var config PublicAccessBlockConfiguration + if err := xml.Unmarshal(data, &config); err != nil { + return nil, err + } + if len(config.Unknown) > 0 { + return nil, fmt.Errorf("unknown element %s", config.Unknown[0].Local) + } + config.XMLNS, config.XMLName = "", xml.Name{} + return &config, nil +} + +func (c *PublicAccessBlockConfiguration) blocksPublicAcls() bool { + return c != nil && c.BlockPublicAcls +} + +func (c *PublicAccessBlockConfiguration) ignoresPublicAcls() bool { + return c != nil && c.IgnorePublicAcls +} + +// hasPublicGrant reports whether any grant names the AllUsers or +// AuthenticatedUsers group, which is what S3 treats as a public ACL. +func hasPublicGrant(grants []*s3.Grant) bool { + for _, grant := range grants { + if grant == nil || grant.Grantee == nil || grant.Grantee.URI == nil { + continue + } + switch *grant.Grantee.URI { + case s3_constants.GranteeGroupAllUsers, s3_constants.GranteeGroupAuthenticatedUsers: + return true + } + } + return false +} + +// rejectPublicGrants denies grants that would make an object or bucket +// public when the bucket blocks public ACLs. +func rejectPublicGrants(config *BucketConfig, grants []*s3.Grant) s3err.ErrorCode { + if config != nil && config.PublicAccessBlock.blocksPublicAcls() && hasPublicGrant(grants) { + return s3err.ErrAccessDenied + } + return s3err.ErrNone +} diff --git a/weed/s3api/s3api_public_access_block_test.go b/weed/s3api/s3api_public_access_block_test.go new file mode 100644 index 0000000000..b5bd4ff16d --- /dev/null +++ b/weed/s3api/s3api_public_access_block_test.go @@ -0,0 +1,251 @@ +package s3api + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/gorilla/mux" + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" + "github.com/stretchr/testify/require" +) + +const testPublicAccessBlockXML = `` + + `truetrue` + + `falsetrue` + + `` + +func newPublicAccessBlockTestServer(t *testing.T, entry *filer_pb.Entry) (*S3ApiServer, *fakeLookupFiler) { + t.Helper() + t.Setenv("AWS_ACCESS_KEY_ID", "") + t.Setenv("AWS_SECRET_ACCESS_KEY", "") + filer := &fakeLookupFiler{entry: entry} + s3a := newHeadBucketTestServer(t, filer) + s3a.bucketConfigCache = NewBucketConfigCache(time.Minute) + s3a.iam = NewIdentityAccessManagementWithStore(s3a.option, nil, "memory") + t.Cleanup(s3a.iam.Shutdown) + return s3a, filer +} + +func publicAccessBlockRequest(method, bucket, body string) *http.Request { + req := httptest.NewRequest(method, "http://s3/"+bucket+"?publicAccessBlock", strings.NewReader(body)) + return mux.SetURLVars(req, map[string]string{"bucket": bucket}) +} + +func TestPutPublicAccessBlockStoresConfiguration(t *testing.T) { + const bucket = "pab-put" + s3a, filer := newPublicAccessBlockTestServer(t, &filer_pb.Entry{Name: bucket, IsDirectory: true}) + + rr := httptest.NewRecorder() + s3a.PutPublicAccessBlockHandler(rr, publicAccessBlockRequest(http.MethodPut, bucket, testPublicAccessBlockXML)) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + + require.Len(t, filer.txnSeen, 1) + stored := filer.txnSeen[0].Mutations[0].SetExtended[s3_constants.ExtPublicAccessBlockKey] + config, err := parsePublicAccessBlock(stored) + require.NoError(t, err) + require.Equal(t, PublicAccessBlockConfiguration{ + BlockPublicAcls: true, + IgnorePublicAcls: true, + RestrictPublicBuckets: true, + }, *config) + + // The PUT loaded the bucket into the cache; a GET must not serve that copy. + filer.entry.Extended = filer.txnSeen[0].Mutations[0].SetExtended + rr = httptest.NewRecorder() + s3a.GetPublicAccessBlockHandler(rr, publicAccessBlockRequest(http.MethodGet, bucket, "")) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + require.Contains(t, rr.Body.String(), "true") +} + +func TestUnreadablePublicAccessBlockBlocksPublicAccess(t *testing.T) { + s3a, _ := newPublicAccessBlockTestServer(t, nil) + config := s3a.newBucketConfigFromEntry("b", &filer_pb.Entry{Name: "b", IsDirectory: true, Extended: map[string][]byte{ + s3_constants.ExtPublicAccessBlockKey: []byte(""), + s3_constants.ExtAmzAclKey: []byte(`[{"Grantee":{"Type":"Group","URI":"` + s3_constants.GranteeGroupAllUsers + `"},"Permission":"READ"}]`), + }}) + require.True(t, config.PublicAccessBlock.blocksPublicAcls()) + require.True(t, config.PublicAccessBlock.ignoresPublicAcls()) + require.False(t, config.IsPublicRead) +} + +func TestBucketUpdateKeepsStoredPublicAccessBlock(t *testing.T) { + const bucket = "pab-keep" + unreadable := []byte("") + for _, stored := range [][]byte{unreadable, []byte(testPublicAccessBlockXML)} { + s3a, filer := newPublicAccessBlockTestServer(t, &filer_pb.Entry{ + Name: bucket, + IsDirectory: true, + Extended: map[string][]byte{s3_constants.ExtPublicAccessBlockKey: stored}, + }) + require.Equal(t, s3err.ErrNone, s3a.updateBucketConfig(bucket, func(config *BucketConfig) error { + config.Versioning = s3_constants.VersioningEnabled + return nil + })) + require.Len(t, filer.txnSeen, 1) + require.NotContains(t, filer.txnSeen[0].Mutations[0].SetExtended, s3_constants.ExtPublicAccessBlockKey) + require.NotContains(t, filer.txnSeen[0].Mutations[0].DeleteExtended, s3_constants.ExtPublicAccessBlockKey) + } + + s3a, filer := newPublicAccessBlockTestServer(t, &filer_pb.Entry{ + Name: bucket, + IsDirectory: true, + Extended: map[string][]byte{s3_constants.ExtPublicAccessBlockKey: unreadable}, + }) + all := `truetrue` + + `truetrue` + rr := httptest.NewRecorder() + s3a.PutPublicAccessBlockHandler(rr, publicAccessBlockRequest(http.MethodPut, bucket, all)) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + require.Len(t, filer.txnSeen, 1) + written, err := parsePublicAccessBlock(filer.txnSeen[0].Mutations[0].SetExtended[s3_constants.ExtPublicAccessBlockKey]) + require.NoError(t, err) + require.True(t, written.BlockPublicAcls && written.IgnorePublicAcls && written.BlockPublicPolicy && written.RestrictPublicBuckets) +} + +func TestPutPublicAccessBlockRejectsMalformedXML(t *testing.T) { + const bucket = "pab-malformed" + s3a, filer := newPublicAccessBlockTestServer(t, &filer_pb.Entry{Name: bucket, IsDirectory: true}) + + for _, body := range []string{ + "", + "yes", + "true", + "true", + } { + rr := httptest.NewRecorder() + s3a.PutPublicAccessBlockHandler(rr, publicAccessBlockRequest(http.MethodPut, bucket, body)) + require.Equal(t, http.StatusBadRequest, rr.Code, rr.Body.String()) + require.Contains(t, rr.Body.String(), "MalformedXML") + } + require.Empty(t, filer.txnSeen) +} + +func TestGetPublicAccessBlock(t *testing.T) { + const bucket = "pab-get" + s3a, _ := newPublicAccessBlockTestServer(t, &filer_pb.Entry{Name: bucket, IsDirectory: true}) + + rr := httptest.NewRecorder() + s3a.GetPublicAccessBlockHandler(rr, publicAccessBlockRequest(http.MethodGet, bucket, "")) + require.Equal(t, http.StatusNotFound, rr.Code, rr.Body.String()) + require.Contains(t, rr.Body.String(), "NoSuchPublicAccessBlockConfiguration") + + s3a, _ = newPublicAccessBlockTestServer(t, &filer_pb.Entry{ + Name: bucket, + IsDirectory: true, + Extended: map[string][]byte{s3_constants.ExtPublicAccessBlockKey: []byte(testPublicAccessBlockXML)}, + }) + rr = httptest.NewRecorder() + s3a.GetPublicAccessBlockHandler(rr, publicAccessBlockRequest(http.MethodGet, bucket, "")) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + body := rr.Body.String() + require.Equal(t, 1, strings.Count(body, "xmlns="), body) + require.Contains(t, body, ``) + require.Contains(t, body, "true") + require.Contains(t, body, "true") + require.Contains(t, body, "false") + require.Contains(t, body, "true") +} + +func TestDeletePublicAccessBlockRemovesConfiguration(t *testing.T) { + const bucket = "pab-delete" + s3a, filer := newPublicAccessBlockTestServer(t, &filer_pb.Entry{ + Name: bucket, + IsDirectory: true, + Extended: map[string][]byte{s3_constants.ExtPublicAccessBlockKey: []byte(testPublicAccessBlockXML)}, + }) + + rr := httptest.NewRecorder() + s3a.DeletePublicAccessBlockHandler(rr, publicAccessBlockRequest(http.MethodDelete, bucket, "")) + require.Equal(t, http.StatusNoContent, rr.Code, rr.Body.String()) + require.Len(t, filer.txnSeen, 1) + require.Contains(t, filer.txnSeen[0].Mutations[0].DeleteExtended, s3_constants.ExtPublicAccessBlockKey) +} + +func TestPublicAccessBlockMissingBucket(t *testing.T) { + s3a, _ := newPublicAccessBlockTestServer(t, nil) + for _, tc := range []struct { + method string + body string + handler http.HandlerFunc + }{ + {http.MethodGet, "", s3a.GetPublicAccessBlockHandler}, + {http.MethodPut, testPublicAccessBlockXML, s3a.PutPublicAccessBlockHandler}, + {http.MethodDelete, "", s3a.DeletePublicAccessBlockHandler}, + } { + rr := httptest.NewRecorder() + tc.handler(rr, publicAccessBlockRequest(tc.method, "pab-missing", tc.body)) + require.Equal(t, http.StatusNotFound, rr.Code, tc.method) + require.Contains(t, rr.Body.String(), "NoSuchBucket", tc.method) + } +} + +func TestBlockPublicAclsRejectsPublicBucketAndObjectAcls(t *testing.T) { + const bucket, object = "pab-acl", "doc.txt" + newServer := func(t *testing.T) (*S3ApiServer, *fakeLookupFiler) { + // The fake filer answers every lookup with this entry, so it serves + // as both the bucket and the object. + return newPublicAccessBlockTestServer(t, &filer_pb.Entry{ + Name: object, + Attributes: &filer_pb.FuseAttributes{}, + Extended: map[string][]byte{ + s3_constants.ExtPublicAccessBlockKey: []byte(`true`), + s3_constants.ExtAmzOwnerKey: []byte(s3_constants.AccountAdminId), + }, + }) + } + for _, tc := range []struct { + acl string + status int + }{ + {s3_constants.CannedAclPrivate, http.StatusOK}, + {s3_constants.CannedAclPublicRead, http.StatusForbidden}, + {s3_constants.CannedAclPublicReadWrite, http.StatusForbidden}, + {s3_constants.CannedAclAuthenticatedRead, http.StatusForbidden}, + } { + t.Run("bucket "+tc.acl, func(t *testing.T) { + s3a, filer := newServer(t) + req := httptest.NewRequest(http.MethodPut, "http://s3/"+bucket+"?acl", http.NoBody) + req = mux.SetURLVars(req, map[string]string{"bucket": bucket}) + req.Header.Set(s3_constants.AmzCannedAcl, tc.acl) + rr := httptest.NewRecorder() + s3a.PutBucketAclHandler(rr, req) + require.Equal(t, tc.status, rr.Code, rr.Body.String()) + if tc.status == http.StatusForbidden { + require.Contains(t, rr.Body.String(), "AccessDenied") + require.Empty(t, filer.txnSeen) + } + }) + t.Run("object "+tc.acl, func(t *testing.T) { + s3a, filer := newServer(t) + req := httptest.NewRequest(http.MethodPut, "http://s3/"+bucket+"/"+object+"?acl", http.NoBody) + req = mux.SetURLVars(req, map[string]string{"bucket": bucket, "object": object}) + req.Header.Set(s3_constants.AmzCannedAcl, tc.acl) + rr := httptest.NewRecorder() + s3a.PutObjectAclHandler(rr, req) + require.Equal(t, tc.status, rr.Code, rr.Body.String()) + if tc.status == http.StatusForbidden { + require.Contains(t, rr.Body.String(), "AccessDenied") + require.Equal(t, s3_constants.AccountAdminId, string(filer.entry.Extended[s3_constants.ExtAmzOwnerKey])) + require.NotContains(t, filer.entry.Extended, s3_constants.ExtAmzAclKey) + } + }) + } + + t.Run("bucket xml all users grant", func(t *testing.T) { + s3a, filer := newServer(t) + body := `admin` + + `` + s3_constants.GranteeGroupAllUsers + `READ` + + `` + req := httptest.NewRequest(http.MethodPut, "http://s3/"+bucket+"?acl", strings.NewReader(body)) + req = mux.SetURLVars(req, map[string]string{"bucket": bucket}) + rr := httptest.NewRecorder() + s3a.PutBucketAclHandler(rr, req) + require.Equal(t, http.StatusForbidden, rr.Code, rr.Body.String()) + require.Empty(t, filer.txnSeen) + }) +} diff --git a/weed/s3api/s3err/s3api_errors.go b/weed/s3api/s3err/s3api_errors.go index f0c8bc9479..67358067d0 100644 --- a/weed/s3api/s3err/s3api_errors.go +++ b/weed/s3api/s3err/s3api_errors.go @@ -178,6 +178,7 @@ const ( ErrAccessControlListNotSupported // The request's snapshot changed during a conditional write; clients can retry. ErrConditionalRequestConflict + ErrNoSuchPublicAccessBlockConfiguration ) // Error message constants for checksum validation @@ -765,6 +766,12 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "The specified configuration does not exist.", HTTPStatusCode: http.StatusNotFound, }, + + ErrNoSuchPublicAccessBlockConfiguration: { + Code: "NoSuchPublicAccessBlockConfiguration", + Description: "The public access block configuration was not found", + HTTPStatusCode: http.StatusNotFound, + }, } // GetAPIError provides API Error for input API error code.