s3: enforce dedicated Object Lock actions (#11362)

s3: enforce dedicated object lock actions
This commit is contained in:
Chris Lu
2026-09-16 20:34:06 -07:00
committed by GitHub
parent 994e1f7d64
commit 66f1754896
5 changed files with 191 additions and 44 deletions
+18 -6
View File
@@ -136,7 +136,7 @@ var baseS3ActionMap = map[string]string{
"DeleteBucketCors": s3_constants.ACTION_WRITE,
"GetBucketNotification": s3_constants.ACTION_READ,
"PutBucketNotification": s3_constants.ACTION_WRITE,
"GetBucketObjectLockConfiguration": s3_constants.ACTION_READ,
"GetBucketObjectLockConfiguration": s3_constants.ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG,
"PutBucketObjectLockConfiguration": s3_constants.ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG,
// Multipart upload operations
"CreateMultipartUpload": s3_constants.ACTION_WRITE,
@@ -146,11 +146,11 @@ var baseS3ActionMap = map[string]string{
"ListMultipartUploads": s3_constants.ACTION_WRITE,
"ListParts": s3_constants.ACTION_WRITE,
// Retention and legal hold operations
"GetObjectRetention": s3_constants.ACTION_READ,
"PutObjectRetention": s3_constants.ACTION_WRITE,
"GetObjectLegalHold": s3_constants.ACTION_READ,
"PutObjectLegalHold": s3_constants.ACTION_WRITE,
"BypassGovernanceRetention": s3_constants.ACTION_WRITE,
"GetObjectRetention": s3_constants.ACTION_GET_OBJECT_RETENTION,
"PutObjectRetention": s3_constants.ACTION_PUT_OBJECT_RETENTION,
"GetObjectLegalHold": s3_constants.ACTION_GET_OBJECT_LEGAL_HOLD,
"PutObjectLegalHold": s3_constants.ACTION_PUT_OBJECT_LEGAL_HOLD,
"BypassGovernanceRetention": s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION,
}
func init() {
@@ -191,6 +191,18 @@ func MapToIdentitiesAction(action string) string {
return StatementActionTagging
case s3_constants.ACTION_DELETE_BUCKET:
return StatementActionDelete
case s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION:
return "BypassGovernanceRetention"
case s3_constants.ACTION_GET_OBJECT_RETENTION:
return "GetObjectRetention"
case s3_constants.ACTION_PUT_OBJECT_RETENTION:
return "PutObjectRetention"
case s3_constants.ACTION_GET_OBJECT_LEGAL_HOLD:
return "GetObjectLegalHold"
case s3_constants.ACTION_PUT_OBJECT_LEGAL_HOLD:
return "PutObjectLegalHold"
case s3_constants.ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG:
return "GetBucketObjectLockConfiguration"
case s3_constants.ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG:
return "PutBucketObjectLockConfiguration"
case s3_constants.ACTION_PUT_BUCKET_POLICY:
@@ -179,19 +179,15 @@ func TestCheckGovernanceBypassPermissionIntegrationBehavior(t *testing.T) {
// This test documents the expected behavior when checkGovernanceBypassPermission
// is called with a full IAM system:
//
// 1. Function calls s3a.iam.authRequest() with the bypass action
// 1. Function calls s3a.iam.authRequest() with the base bypass action;
// authRequest derives the bucket and object resource from the request.
// 2. If authRequest returns errCode != s3err.ErrNone, function returns false
// 3. If authRequest succeeds, function checks identity.CanDo() with the bypass action
// 4. If CanDo() returns true, function returns true
// 5. If bypass permission fails, function checks admin action with identity.CanDo()
// 6. If admin action succeeds, function returns true and logs admin access
// 7. If all checks fail, function returns false
// 3. If authRequest succeeds through a legacy action, IAM/STS policy, bucket
// policy, or Admin grant, function returns true without a second legacy check.
//
// The function correctly uses:
// - s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION for bypass permission
// - s3_constants.ACTION_ADMIN for admin permission
// - Proper resource path generation with bucket/object format
// - Trimming of leading slashes from object names
// - The normal authRequest resource and Admin semantics
}
// TestGovernanceBypassPermission was removed because it tested the old
@@ -0,0 +1,147 @@
package s3api
import (
"context"
"net/http"
"net/http/httptest"
"testing"
"github.com/gorilla/mux"
iamlib "github.com/seaweedfs/seaweedfs/weed/iam"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
"github.com/stretchr/testify/require"
)
func TestObjectLockRoutesUseDedicatedActions(t *testing.T) {
bindings := handlerActionBindings(t)
for _, tc := range []struct{ handler, want string }{
{"GetObjectRetentionHandler", "ACTION_GET_OBJECT_RETENTION"},
{"PutObjectRetentionHandler", "ACTION_PUT_OBJECT_RETENTION"},
{"GetObjectLegalHoldHandler", "ACTION_GET_OBJECT_LEGAL_HOLD"},
{"PutObjectLegalHoldHandler", "ACTION_PUT_OBJECT_LEGAL_HOLD"},
{"GetObjectLockConfigurationHandler", "ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG"},
{"PutObjectLockConfigurationHandler", "ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG"},
} {
if got := bindings[tc.handler]; got != tc.want {
t.Errorf("%s is gated on %s, want %s", tc.handler, got, tc.want)
}
}
}
func TestGovernanceBypassUsesDedicatedAuthorization(t *testing.T) {
for _, tc := range []struct {
name string
action Action
allowed bool
}{
{"dedicated permission", Action(s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION + ":test-bucket/*"), true},
{"coarse write", Action(s3_constants.ACTION_WRITE + ":test-bucket/*"), false},
{"admin", Action(s3_constants.ACTION_ADMIN), true},
} {
t.Run(tc.name, func(t *testing.T) {
iam := newTestIAM()
iam.identities[0].Actions = []Action{tc.action}
iam.identities[0].Account = &Account{Id: "test-account"}
s3a := &S3ApiServer{iam: iam}
req := httptest.NewRequest(http.MethodDelete, "http://localhost:8333/test-bucket/test-object", nil)
req = mux.SetURLVars(req, map[string]string{"bucket": "test-bucket", "object": "test-object"})
require.NoError(t, signRawHTTPRequest(context.Background(), req,
"AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", "us-east-1"))
if got := s3a.checkGovernanceBypassPermission(req, "test-bucket", "/test-object"); got != tc.allowed {
t.Errorf("checkGovernanceBypassPermission() = %v, want %v", got, tc.allowed)
}
})
}
}
func TestGovernanceBypassUsesBodyObjectKey(t *testing.T) {
iam := newTestIAM()
iam.identities[0].Actions = []Action{
Action(s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION + ":test-bucket/allowed/*"),
}
iam.identities[0].Account = &Account{Id: "test-account"}
s3a := &S3ApiServer{iam: iam}
req := httptest.NewRequest(http.MethodPost, "http://localhost:8333/test-bucket?delete", nil)
req = mux.SetURLVars(req, map[string]string{"bucket": "test-bucket"})
req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), iam.identities[0]))
if !s3a.checkGovernanceBypassPermission(req, "test-bucket", "allowed/object") {
t.Fatal("object-scoped bypass grant did not authorize a DeleteObjects body key")
}
if s3a.checkGovernanceBypassPermission(req, "test-bucket", "denied/object") {
t.Fatal("object-scoped bypass grant authorized a key outside its prefix")
}
}
func TestGovernanceBypassNormalizesBodyObjectKey(t *testing.T) {
iam := newTestIAM()
iam.identities[0].Account = &Account{Id: "test-account"}
s3a := &S3ApiServer{iam: iam}
req := httptest.NewRequest(http.MethodPost, "http://localhost:8333/test-bucket?delete", nil)
req = mux.SetURLVars(req, map[string]string{"bucket": "test-bucket"})
req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), iam.identities[0]))
iam.identities[0].Actions = []Action{
Action(s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION + ":test-bucket/allowed/o?ject"),
}
if !s3a.checkGovernanceBypassPermission(req, "test-bucket", "//allowed//object") {
t.Fatal("canonical object grant did not authorize the equivalent noncanonical body key")
}
iam.identities[0].Actions = []Action{
Action(s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION + ":test-bucket/allowed//o?ject"),
}
if s3a.checkGovernanceBypassPermission(req, "test-bucket", "//allowed//object") {
t.Fatal("noncanonical alias grant authorized the canonical mutation target")
}
}
func TestCoarseReadWriteDoNotGrantObjectLockActions(t *testing.T) {
identity := &Identity{
Name: "object-reader-writer",
Actions: []Action{
Action(s3_constants.ACTION_READ + ":test-bucket"),
Action(s3_constants.ACTION_WRITE + ":test-bucket"),
},
}
for _, action := range []string{
s3_constants.ACTION_GET_OBJECT_RETENTION,
s3_constants.ACTION_PUT_OBJECT_RETENTION,
s3_constants.ACTION_GET_OBJECT_LEGAL_HOLD,
s3_constants.ACTION_PUT_OBJECT_LEGAL_HOLD,
s3_constants.ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG,
s3_constants.ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG,
s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION,
} {
if identity.CanDo(Action(action), "test-bucket", "some/key") {
t.Errorf("coarse Read/Write unexpectedly granted %s", action)
}
}
}
func TestObjectLockActionsRoundTrip(t *testing.T) {
for _, tc := range []struct{ policyAction, identityAction string }{
{"GetObjectRetention", s3_constants.ACTION_GET_OBJECT_RETENTION},
{"PutObjectRetention", s3_constants.ACTION_PUT_OBJECT_RETENTION},
{"GetObjectLegalHold", s3_constants.ACTION_GET_OBJECT_LEGAL_HOLD},
{"PutObjectLegalHold", s3_constants.ACTION_PUT_OBJECT_LEGAL_HOLD},
{"GetBucketObjectLockConfiguration", s3_constants.ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG},
{"PutBucketObjectLockConfiguration", s3_constants.ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG},
{"BypassGovernanceRetention", s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION},
} {
t.Run(tc.policyAction, func(t *testing.T) {
for _, action := range []string{tc.policyAction, "s3:" + tc.policyAction} {
if got := iamlib.MapToStatementAction(action); got != tc.identityAction {
t.Errorf("MapToStatementAction(%q) = %q, want %q", action, got, tc.identityAction)
}
}
if got := iamlib.MapToIdentitiesAction(tc.identityAction); got != tc.policyAction {
t.Errorf("MapToIdentitiesAction(%q) = %q, want %q", tc.identityAction, got, tc.policyAction)
}
})
}
}
+16 -24
View File
@@ -6,7 +6,6 @@ import (
"fmt"
"net/http"
"strconv"
"strings"
"time"
"github.com/seaweedfs/seaweedfs/weed/glog"
@@ -521,33 +520,26 @@ func (s3a *S3ApiServer) getLegalHoldFromEntry(entry *filer_pb.Entry) (*ObjectLeg
// checkGovernanceBypassPermission checks if user has permission to bypass governance retention
func (s3a *S3ApiServer) checkGovernanceBypassPermission(request *http.Request, bucket, object string) bool {
// Use the existing IAM auth system to check the specific permission
// Create the governance bypass action with proper bucket/object concatenation
// Note: path.Join would drop bucket if object has leading slash, so use explicit formatting
resource := fmt.Sprintf("%s/%s", bucket, strings.TrimPrefix(object, "/"))
action := Action(fmt.Sprintf("%s:%s", s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION, resource))
identity, _ := s3_constants.GetIdentityFromContext(request).(*Identity)
if identity == nil {
// Most handlers arrive through Auth and carry the authenticated identity
// in context. Keep direct callers correct without authorizing against the
// request URL, which is bucket-only for DeleteObjects.
var errCode s3err.ErrorCode
identity, errCode, _ = s3a.iam.authenticateRequestInternal(request)
if errCode != s3err.ErrNone {
glog.V(3).Infof("IAM authentication failed for governance bypass: %v", errCode)
return false
}
}
// Use the IAM system to authenticate and authorize this specific action
identity, errCode := s3a.iam.authRequest(request, action)
errCode := s3a.iam.authorizeObjectKeyAction(request, identity, request.Method,
s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION, bucket, s3_constants.NormalizeObjectKey(object), request.URL.Query().Get("versionId"))
if errCode != s3err.ErrNone {
glog.V(3).Infof("IAM auth failed for governance bypass: %v", errCode)
glog.V(3).Infof("IAM authorization failed for governance bypass on %s/%s: %v", bucket, object, errCode)
return false
}
// Verify that the authenticated identity can perform this action
if identity != nil && identity.CanDo(action, bucket, object) {
return true
}
// Additional check: allow users with Admin action to bypass governance retention
// Use the proper S3 Admin action constant instead of generic isAdmin() method
adminAction := Action(fmt.Sprintf("%s:%s", s3_constants.ACTION_ADMIN, resource))
if identity != nil && identity.CanDo(adminAction, bucket, object) {
glog.V(2).Infof("Admin user %s granted governance bypass permission for %s/%s", identity.Name, bucket, object)
return true
}
return false
return true
}
// evaluateGovernanceBypassRequest evaluates if governance bypass is requested and permitted
+5 -5
View File
@@ -870,16 +870,16 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) {
// PutObjectACL
bucket.Methods(http.MethodPut).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutObjectAclHandler, ACTION_WRITE_ACP)), "PUT")).Queries("acl", "")
// PutObjectRetention
bucket.Methods(http.MethodPut).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutObjectRetentionHandler, ACTION_WRITE)), "PUT")).Queries("retention", "")
bucket.Methods(http.MethodPut).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutObjectRetentionHandler, ACTION_PUT_OBJECT_RETENTION)), "PUT")).Queries("retention", "")
// PutObjectLegalHold
bucket.Methods(http.MethodPut).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutObjectLegalHoldHandler, ACTION_WRITE)), "PUT")).Queries("legal-hold", "")
bucket.Methods(http.MethodPut).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutObjectLegalHoldHandler, ACTION_PUT_OBJECT_LEGAL_HOLD)), "PUT")).Queries("legal-hold", "")
// GetObjectACL
bucket.Methods(http.MethodGet).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectAclHandler, ACTION_READ_ACP)), "GET")).Queries("acl", "")
// GetObjectRetention
bucket.Methods(http.MethodGet).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectRetentionHandler, ACTION_READ)), "GET")).Queries("retention", "")
bucket.Methods(http.MethodGet).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectRetentionHandler, ACTION_GET_OBJECT_RETENTION)), "GET")).Queries("retention", "")
// GetObjectLegalHold
bucket.Methods(http.MethodGet).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectLegalHoldHandler, ACTION_READ)), "GET")).Queries("legal-hold", "")
bucket.Methods(http.MethodGet).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectLegalHoldHandler, ACTION_GET_OBJECT_LEGAL_HOLD)), "GET")).Queries("legal-hold", "")
// objects with query
@@ -964,7 +964,7 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) {
bucket.Methods(http.MethodPut).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutBucketVersioningHandler, ACTION_WRITE)), "PUT")).Queries("versioning", "")
// GetObjectLockConfiguration / PutObjectLockConfiguration (bucket-level operations)
bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectLockConfigurationHandler, ACTION_READ)), "GET")).Queries("object-lock", "")
bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectLockConfigurationHandler, ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG)), "GET")).Queries("object-lock", "")
bucket.Methods(http.MethodPut).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutObjectLockConfigurationHandler, ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG)), "PUT")).Queries("object-lock", "")
// GetBucketTagging