mirrors/seaweedfs
1
0
Fork 0
mirror of https://github.com/seaweedfs/seaweedfs.git synced 2026-07-20 06:52:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix PutObjectAcl writing back to the wrong object for nested keys (#10333)

Browse Source 
* Fix PutObjectAcl writing back to the wrong object for nested keys

PutObjectAcl set the update directory to the bucket root, so an ACL
change on allowed/protected.txt landed on protected.txt at the bucket
root instead. A principal scoped to one nested key could overwrite a
different object sharing the basename. Target the object's own parent
directory.

* Test object ACL update directory resolution for nested keys
This commit is contained in:
Chris Lu
2026-07-14 11:46:37 -07:00
committed by GitHub
parent 10cdaf3818
commit 311bc3a6df
2 changed files with 72 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
weed/s3api/s3api_object_handlers_acl.go
+25 -29
View File
@@ -10,6 +10,7 @@ import (
"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/seaweedfs/seaweedfs/weed/util"
)
// GetObjectAclHandler Get object ACL
@@ -268,35 +269,7 @@ func (s3a *S3ApiServer) PutObjectAclHandler(w http.ResponseWriter, r *http.Reque
return
}
// Calculate the correct directory for ACL update
var updateDirectory string
if versioningConfigured {
if versionId != "" && versionId != "null" {
// Versioned object - update the specific version file in .versions directory
updateDirectory = s3a.bucketDir(bucket) + "/" + object + s3_constants.VersionsFolder
} else {
// Latest version in versioned bucket - could be null version or versioned object
// Extract version ID from the entry to determine where it's stored
var actualVersionId string
if entry.Extended != nil {
if versionIdBytes, exists := entry.Extended[s3_constants.ExtVersionIdKey]; exists {
actualVersionId = string(versionIdBytes)
}
}
if actualVersionId == "null" || actualVersionId == "" {
// Null version (pre-versioning object) - stored as regular file
updateDirectory = s3a.bucketDir(bucket)
} else {
// Versioned object - stored in .versions directory
updateDirectory = s3a.bucketDir(bucket) + "/" + object + s3_constants.VersionsFolder
}
}
} else {
// Non-versioned object - stored as regular file
updateDirectory = s3a.bucketDir(bucket)
}
updateDirectory := s3a.objectAclUpdateDirectory(bucket, object, versioningConfigured, versionId, entry)
// Update the object with new ACL metadata
err = s3a.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
@@ -320,3 +293,26 @@ func (s3a *S3ApiServer) PutObjectAclHandler(w http.ResponseWriter, r *http.Reque
glog.V(3).Infof("PutObjectAclHandler: Successfully updated ACL for %s/%s by user %s", bucket, object, amzAccountId)
writeSuccessResponseEmpty(w, r)
}
// objectAclUpdateDirectory returns the filer directory holding the entry a PutObjectAcl
// update must target. A regular object lives under its full key's parent directory, so a
// nested key must not fall back to the bucket root, which would rewrite a different object
// sharing the same basename.
func (s3a *S3ApiServer) objectAclUpdateDirectory(bucket, object string, versioningConfigured bool, versionId string, entry *filer_pb.Entry) string {
if versioningConfigured {
if versionId != "" && versionId != "null" {
return s3a.bucketDir(bucket) + "/" + object + s3_constants.VersionsFolder
}
var actualVersionId string
if entry.Extended != nil {
if versionIdBytes, exists := entry.Extended[s3_constants.ExtVersionIdKey]; exists {
actualVersionId = string(versionIdBytes)
}
}
if actualVersionId != "null" && actualVersionId != "" {
return s3a.bucketDir(bucket) + "/" + object + s3_constants.VersionsFolder
}
}
dir, _ := util.NewFullPath(s3a.bucketDir(bucket), object).DirAndName()
return dir
}
weed/s3api/s3api_object_handlers_acl_test.go
+47
View File
@@ -0,0 +1,47 @@
package s3api
import (
"testing"
"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
)
// TestObjectAclUpdateDirectory guards against a PutObjectAcl scope bypass: a nested object
// key must resolve to its own parent directory, not the bucket root, otherwise updating the
// ACL of allowed/protected.txt would rewrite a different protected.txt at the bucket root.
func TestObjectAclUpdateDirectory(t *testing.T) {
s3a := &S3ApiServer{option: &S3ApiServerOption{BucketsPath: "/buckets"}}
const bucket = "target-bucket"
bucketDir := "/buckets/target-bucket"
versioned := func(id string) *filer_pb.Entry {
return &filer_pb.Entry{Extended: map[string][]byte{s3_constants.ExtVersionIdKey: []byte(id)}}
}
tests := []struct {
name string
object string
versioningConfigured bool
versionId string
entry *filer_pb.Entry
want string
}{
{"non-versioned nested key", "allowed/protected.txt", false, "", &filer_pb.Entry{}, bucketDir + "/allowed"},
{"non-versioned root key", "protected.txt", false, "", &filer_pb.Entry{}, bucketDir},
{"non-versioned deep key", "a/b/c/protected.txt", false, "", &filer_pb.Entry{}, bucketDir + "/a/b/c"},
{"null version nested key", "allowed/protected.txt", true, "", versioned("null"), bucketDir + "/allowed"},
{"empty version nested key", "allowed/protected.txt", true, "", &filer_pb.Entry{}, bucketDir + "/allowed"},
{"specific version nested key", "allowed/protected.txt", true, "v1", versioned("v1"), bucketDir + "/allowed/protected.txt" + s3_constants.VersionsFolder},
{"latest versioned nested key", "allowed/protected.txt", true, "", versioned("v1"), bucketDir + "/allowed/protected.txt" + s3_constants.VersionsFolder},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := s3a.objectAclUpdateDirectory(bucket, tt.object, tt.versioningConfigured, tt.versionId, tt.entry)
if got != tt.want {
t.Errorf("objectAclUpdateDirectory(%q) = %q, want %q", tt.object, got, tt.want)
}
})
}
}