From f6e8a9bf9ce3072cfd1d27157cc2a83f91d669df Mon Sep 17 00:00:00 2001 From: Riccardo Bertossa <33728857+rikigigi@users.noreply.github.com> Date: Fri, 17 May 2024 13:54:09 +0200 Subject: [PATCH] added s3 iam DeleteBucket permission management (#5599) --- weed/iamapi/iamapi_management_handlers.go | 5 +++++ weed/s3api/auth_credentials.go | 4 ++++ weed/s3api/auth_credentials_test.go | 15 +++++++++++++-- weed/s3api/s3_constants/s3_actions.go | 15 ++++++++------- weed/s3api/s3_constants/s3_config.go | 2 +- weed/s3api/s3api_bucket_handlers.go | 12 +++++++++--- weed/s3api/s3api_server.go | 2 +- 7 files changed, 41 insertions(+), 14 deletions(-) diff --git a/weed/iamapi/iamapi_management_handlers.go b/weed/iamapi/iamapi_management_handlers.go index d63bc8849..6b0f9bbfc 100644 --- a/weed/iamapi/iamapi_management_handlers.go +++ b/weed/iamapi/iamapi_management_handlers.go @@ -33,6 +33,7 @@ const ( StatementActionReadAcp = "GetBucketAcl" StatementActionList = "List*" StatementActionTagging = "Tagging*" + StatementActionDelete = "DeleteBucket*" ) var ( @@ -58,6 +59,8 @@ func MapToStatementAction(action string) string { return s3_constants.ACTION_LIST case StatementActionTagging: return s3_constants.ACTION_TAGGING + case StatementActionDelete: + return s3_constants.ACTION_DELETE_BUCKET default: return "" } @@ -79,6 +82,8 @@ func MapToIdentitiesAction(action string) string { return StatementActionList case s3_constants.ACTION_TAGGING: return StatementActionTagging + case s3_constants.ACTION_DELETE_BUCKET: + return StatementActionDelete default: return "" } diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index a2b1fd90f..6121aecba 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -317,6 +317,7 @@ func (iam *IdentityAccessManagement) Auth(f http.HandlerFunc, action Action) htt } identity, errCode := iam.authRequest(r, action) + glog.V(3).Infof("auth error: %v", errCode) if errCode == s3err.ErrNone { if identity != nil && identity.Name != "" { r.Header.Set(s3_constants.AmzIdentityId, identity.Name) @@ -453,6 +454,7 @@ func (identity *Identity) canDo(action Action, bucket string, objectKey string) } } if bucket == "" { + glog.V(3).Infof("identity %s is not allowed to perform action %s on %s -- bucket is empty", identity.Name, action, bucket+objectKey) return false } target := string(action) + ":" + bucket + objectKey @@ -477,6 +479,8 @@ func (identity *Identity) canDo(action Action, bucket string, objectKey string) } } } + //log error + glog.V(3).Infof("identity %s is not allowed to perform action %s on %s", identity.Name, action, bucket+objectKey) return false } diff --git a/weed/s3api/auth_credentials_test.go b/weed/s3api/auth_credentials_test.go index f9f87fc54..1d9f1a95f 100644 --- a/weed/s3api/auth_credentials_test.go +++ b/weed/s3api/auth_credentials_test.go @@ -1,11 +1,12 @@ package s3api import ( - . "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" - "github.com/stretchr/testify/assert" "reflect" "testing" + . "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/stretchr/testify/assert" + "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" jsonpb "google.golang.org/protobuf/encoding/protojson" ) @@ -79,6 +80,7 @@ func TestCanDo(t *testing.T) { } // object specific assert.Equal(t, true, ident1.canDo(ACTION_WRITE, "bucket1", "/a/b/c/d.txt")) + assert.Equal(t, false, ident1.canDo(ACTION_DELETE_BUCKET, "bucket1", "")) assert.Equal(t, false, ident1.canDo(ACTION_WRITE, "bucket1", "/a/b/other/some"), "action without *") // bucket specific @@ -141,6 +143,15 @@ func TestCanDo(t *testing.T) { }, } assert.Equal(t, true, ident6.canDo(ACTION_READ, "anything_bucket", "/a/b/c/d.txt")) + + //test deleteBucket operation + ident7 := &Identity{ + Name: "anything", + Actions: []Action{ + "DeleteBucket:bucket1", + }, + } + assert.Equal(t, true, ident7.canDo(ACTION_DELETE_BUCKET, "bucket1", "")) } type LoadS3ApiConfigurationTestCase struct { diff --git a/weed/s3api/s3_constants/s3_actions.go b/weed/s3api/s3_constants/s3_actions.go index 8d770e408..864979784 100644 --- a/weed/s3api/s3_constants/s3_actions.go +++ b/weed/s3api/s3_constants/s3_actions.go @@ -1,13 +1,14 @@ package s3_constants const ( - ACTION_READ = "Read" - ACTION_READ_ACP = "ReadAcp" - ACTION_WRITE = "Write" - ACTION_WRITE_ACP = "WriteAcp" - ACTION_ADMIN = "Admin" - ACTION_TAGGING = "Tagging" - ACTION_LIST = "List" + ACTION_READ = "Read" + ACTION_READ_ACP = "ReadAcp" + ACTION_WRITE = "Write" + ACTION_WRITE_ACP = "WriteAcp" + ACTION_ADMIN = "Admin" + ACTION_TAGGING = "Tagging" + ACTION_LIST = "List" + ACTION_DELETE_BUCKET = "DeleteBucket" SeaweedStorageDestinationHeader = "x-seaweedfs-destination" MultipartUploadsFolder = ".uploads" diff --git a/weed/s3api/s3_constants/s3_config.go b/weed/s3api/s3_constants/s3_config.go index cb44b9484..d2d2c257a 100644 --- a/weed/s3api/s3_constants/s3_config.go +++ b/weed/s3api/s3_constants/s3_config.go @@ -7,7 +7,7 @@ import ( var ( CircuitBreakerConfigDir = "/etc/s3" CircuitBreakerConfigFile = "circuit_breaker.json" - AllowedActions = []string{ACTION_READ, ACTION_READ_ACP, ACTION_WRITE, ACTION_WRITE_ACP, ACTION_LIST, ACTION_TAGGING, ACTION_ADMIN} + AllowedActions = []string{ACTION_READ, ACTION_READ_ACP, ACTION_WRITE, ACTION_WRITE_ACP, ACTION_LIST, ACTION_TAGGING, ACTION_ADMIN, ACTION_DELETE_BUCKET} LimitTypeCount = "Count" LimitTypeBytes = "MB" Separator = ":" diff --git a/weed/s3api/s3api_bucket_handlers.go b/weed/s3api/s3api_bucket_handlers.go index 151bdaca5..12d2c0432 100644 --- a/weed/s3api/s3api_bucket_handlers.go +++ b/weed/s3api/s3api_bucket_handlers.go @@ -6,14 +6,15 @@ import ( "encoding/xml" "errors" "fmt" - "github.com/aws/aws-sdk-go/private/protocol/xml/xmlutil" - "github.com/seaweedfs/seaweedfs/weed/s3api/s3bucket" - "github.com/seaweedfs/seaweedfs/weed/util" "math" "net/http" "strings" "time" + "github.com/aws/aws-sdk-go/private/protocol/xml/xmlutil" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3bucket" + "github.com/seaweedfs/seaweedfs/weed/util" + "github.com/seaweedfs/seaweedfs/weed/filer" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/storage/needle" @@ -218,6 +219,10 @@ func (s3a *S3ApiServer) checkBucket(r *http.Request, bucket string) s3err.ErrorC return s3err.ErrNoSuchBucket } + //if iam is enabled, the access was already checked before + if s3a.iam.isEnabled() { + return s3err.ErrNone + } if !s3a.hasAccess(r, entry) { return s3err.ErrAccessDenied } @@ -236,6 +241,7 @@ func (s3a *S3ApiServer) hasAccess(r *http.Request, entry *filer_pb.Entry) bool { identityId := r.Header.Get(s3_constants.AmzIdentityId) if id, ok := entry.Extended[s3_constants.AmzIdentityId]; ok { if identityId != string(id) { + glog.V(3).Infof("hasAccess: %s != %s (entry.Extended = %v)", identityId, id, entry.Extended) return false } } diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index 1477d650f..9422318ce 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -279,7 +279,7 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { bucket.Methods("PUT").HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutBucketHandler, ACTION_ADMIN)), "PUT")) // DeleteBucket - bucket.Methods("DELETE").HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.DeleteBucketHandler, ACTION_ADMIN)), "DELETE")) + bucket.Methods("DELETE").HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.DeleteBucketHandler, ACTION_DELETE_BUCKET)), "DELETE")) // ListObjectsV1 (Legacy) bucket.Methods("GET").HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.ListObjectsV1Handler, ACTION_LIST)), "LIST"))