From 2d556ac2a5a667774eccdc6b552f1a92c501b043 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Wed, 28 Jan 2026 16:15:34 -0800 Subject: [PATCH] S3 Tables API now properly enforces resource policies addressing the critical security gap where policies were created but never evaluated. --- go.mod | 8 +- go.sum | 16 +- weed/s3api/s3tables/handler_bucket_create.go | 2 +- .../handler_bucket_get_list_delete.go | 26 +- weed/s3api/s3tables/handler_namespace.go | 62 +++- weed/s3api/s3tables/handler_policy.go | 103 +++++- weed/s3api/s3tables/permissions.go | 308 ++++++++++-------- 7 files changed, 358 insertions(+), 167 deletions(-) diff --git a/go.mod b/go.mod index a63cdd81e..53a333fd1 100644 --- a/go.mod +++ b/go.mod @@ -254,7 +254,7 @@ require ( github.com/Azure/azure-sdk-for-go/sdk/internal v1.11.2 // indirect github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.6.3 github.com/Azure/azure-sdk-for-go/sdk/storage/azfile v1.5.3 // indirect - github.com/Azure/go-ntlmssp v0.0.2-0.20251110135918-10b7b7e7cd26 // indirect + github.com/Azure/go-ntlmssp v0.1.0 // indirect github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0 // indirect github.com/Files-com/files-sdk-go/v3 v3.2.264 // indirect github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.30.0 // indirect @@ -349,7 +349,7 @@ require ( github.com/gorilla/securecookie v1.1.2 // indirect github.com/gorilla/sessions v1.4.0 // indirect github.com/grpc-ecosystem/go-grpc-middleware v1.4.0 // indirect - github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.1 // indirect + github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.3 // indirect github.com/hashicorp/go-cleanhttp v0.5.2 // indirect github.com/hashicorp/go-hclog v1.6.3 // indirect github.com/hashicorp/go-immutable-radix v1.3.1 // indirect @@ -458,11 +458,11 @@ require ( go.opentelemetry.io/otel/sdk/metric v1.38.0 // indirect go.opentelemetry.io/otel/trace v1.38.0 // indirect go.uber.org/multierr v1.11.0 // indirect - go.uber.org/zap v1.27.0 // indirect + go.uber.org/zap v1.27.1 // indirect golang.org/x/arch v0.20.0 // indirect golang.org/x/term v0.39.0 // indirect golang.org/x/time v0.14.0 // indirect - google.golang.org/genproto/googleapis/api v0.0.0-20251111163417-95abcf5c77ba // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20251124214823-79d6a2a48846 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20251213004720-97cd9d5aeac2 // indirect gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect gopkg.in/validator.v2 v2.0.1 // indirect diff --git a/go.sum b/go.sum index 0020edc16..dffe8c673 100644 --- a/go.sum +++ b/go.sum @@ -560,8 +560,8 @@ github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.6.3/go.mod h1:URuDvhmATV github.com/Azure/azure-sdk-for-go/sdk/storage/azfile v1.5.3 h1:sxgSqOB9CDToiaVFpxuvb5wGgGqWa3lCShcm5o0n3bE= github.com/Azure/azure-sdk-for-go/sdk/storage/azfile v1.5.3/go.mod h1:XdED8i399lEVblYHTZM8eXaP07gv4Z58IL6ueMlVlrg= github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8= -github.com/Azure/go-ntlmssp v0.0.2-0.20251110135918-10b7b7e7cd26 h1:gy/jrlpp8EfSyA73a51fofoSfhp5rPNQAUvDr4Dm91c= -github.com/Azure/go-ntlmssp v0.0.2-0.20251110135918-10b7b7e7cd26/go.mod h1:NYqdhxd/8aAct/s4qSYZEerdPuH1liG2/X9DiVTbhpk= +github.com/Azure/go-ntlmssp v0.1.0 h1:DjFo6YtWzNqNvQdrwEyr/e4nhU3vRiwenz5QX7sFz+A= +github.com/Azure/go-ntlmssp v0.1.0/go.mod h1:NYqdhxd/8aAct/s4qSYZEerdPuH1liG2/X9DiVTbhpk= github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1 h1:WJTmL004Abzc5wDB5VtZG2PJk5ndYDgVacGqfirKxjM= github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1/go.mod h1:tCcJZ0uHAmvjsVYzEFivsRTN00oz5BEsRgQHu5JZ9WE= github.com/AzureAD/microsoft-authentication-library-for-go v1.6.0 h1:XRzhVemXdgvJqCH0sFfrBUTnUJSBrBf7++ypk+twtRs= @@ -1206,8 +1206,8 @@ github.com/grpc-ecosystem/grpc-gateway v1.16.0 h1:gmcG1KaJ57LophUzW0Hy8NmPhnMZb4 github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw= github.com/grpc-ecosystem/grpc-gateway/v2 v2.7.0/go.mod h1:hgWBS7lorOAVIJEQMi4ZsPv9hVvWI6+ch50m39Pf2Ks= github.com/grpc-ecosystem/grpc-gateway/v2 v2.11.3/go.mod h1:o//XUCC/F+yRGJoPO/VU0GSB0f8Nhgmxx0VIRUvaC0w= -github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.1 h1:X5VWvz21y3gzm9Nw/kaUeku/1+uBhcekkmy4IkffJww= -github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.1/go.mod h1:Zanoh4+gvIgluNqcfMVTJueD4wSS5hT7zTt4Mrutd90= +github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.3 h1:NmZ1PKzSTQbuGHw9DGPFomqkkLWMC+vZCkfs+FHv1Vg= +github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.3/go.mod h1:zQrxl1YP88HQlA6i9c63DSVPFklWpGX4OWAc9bFuaH4= github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I= github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= @@ -1919,8 +1919,8 @@ go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= go.uber.org/zap v1.18.1/go.mod h1:xg/QME4nWcxGxrpdeYfq7UvYrLh66cuVKdrbD1XF/NI= go.uber.org/zap v1.19.0/go.mod h1:xg/QME4nWcxGxrpdeYfq7UvYrLh66cuVKdrbD1XF/NI= -go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8= -go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E= +go.uber.org/zap v1.27.1 h1:08RqriUEv8+ArZRYSTXy1LeBScaMpVSTBhCeaZYfMYc= +go.uber.org/zap v1.27.1/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E= go.yaml.in/yaml/v2 v2.4.3 h1:6gvOSjQoTB3vt1l+CU+tSyi/HOjfOjRLJ4YwYZGwRO0= go.yaml.in/yaml/v2 v2.4.3/go.mod h1:zSxWcmIDjOzPXpjlTTbAsKokqkDNAVtZO0WOMiT90s8= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= @@ -2582,8 +2582,8 @@ google.golang.org/genproto v0.0.0-20230222225845-10f96fb3dbec/go.mod h1:3Dl5ZL0q google.golang.org/genproto v0.0.0-20230306155012-7f2fa6fef1f4/go.mod h1:NWraEVixdDnqcqQ30jipen1STv2r/n24Wb7twVTGR4s= google.golang.org/genproto v0.0.0-20250922171735-9219d122eba9 h1:LvZVVaPE0JSqL+ZWb6ErZfnEOKIqqFWUJE2D0fObSmc= google.golang.org/genproto v0.0.0-20250922171735-9219d122eba9/go.mod h1:QFOrLhdAe2PsTp3vQY4quuLKTi9j3XG3r6JPPaw7MSc= -google.golang.org/genproto/googleapis/api v0.0.0-20251111163417-95abcf5c77ba h1:B14OtaXuMaCQsl2deSvNkyPKIzq3BjfxQp8d00QyWx4= -google.golang.org/genproto/googleapis/api v0.0.0-20251111163417-95abcf5c77ba/go.mod h1:G5IanEx8/PgI9w6CFcYQf7jMtHQhZruvfM1i3qOqk5U= +google.golang.org/genproto/googleapis/api v0.0.0-20251124214823-79d6a2a48846 h1:ZdyUkS9po3H7G0tuh955QVyyotWvOD4W0aEapeGeUYk= +google.golang.org/genproto/googleapis/api v0.0.0-20251124214823-79d6a2a48846/go.mod h1:Fk4kyraUvqD7i5H6S43sj2W98fbZa75lpZz/eUyhfO0= google.golang.org/genproto/googleapis/rpc v0.0.0-20251213004720-97cd9d5aeac2 h1:2I6GHUeJ/4shcDpoUlLs/2WPnhg7yJwvXtqcMJt9liA= google.golang.org/genproto/googleapis/rpc v0.0.0-20251213004720-97cd9d5aeac2/go.mod h1:7i2o+ce6H/6BluujYR+kqX3GKH+dChPTQU19wjRPiGk= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= diff --git a/weed/s3api/s3tables/handler_bucket_create.go b/weed/s3api/s3tables/handler_bucket_create.go index eee627570..645d42944 100644 --- a/weed/s3api/s3tables/handler_bucket_create.go +++ b/weed/s3api/s3tables/handler_bucket_create.go @@ -16,7 +16,7 @@ func (h *S3TablesHandler) handleCreateTableBucket(w http.ResponseWriter, r *http // Check permission accountID := h.getAccountID(r) principal := h.getPrincipalFromRequest(r) - if !CanCreateTableBucket(principal, accountID) { + if !CanCreateTableBucket(principal, accountID, "") { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to create table buckets") return NewAuthError("CreateTableBucket", principal, "not authorized to create table buckets") } diff --git a/weed/s3api/s3tables/handler_bucket_get_list_delete.go b/weed/s3api/s3tables/handler_bucket_get_list_delete.go index 7471d0330..1a793cf36 100644 --- a/weed/s3api/s3tables/handler_bucket_get_list_delete.go +++ b/weed/s3api/s3tables/handler_bucket_get_list_delete.go @@ -35,6 +35,7 @@ func (h *S3TablesHandler) handleGetTableBucket(w http.ResponseWriter, r *http.Re bucketPath := getTableBucketPath(bucketName) var metadata tableBucketMetadata + var bucketPolicy string err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { data, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyMetadata) if err != nil { @@ -43,6 +44,13 @@ func (h *S3TablesHandler) handleGetTableBucket(w http.ResponseWriter, r *http.Re if err := json.Unmarshal(data, &metadata); err != nil { return fmt.Errorf("failed to unmarshal metadata: %w", err) } + + // Fetch bucket policy if it exists + policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy) + if err == nil { + bucketPolicy = string(policyData) + } + return nil }) @@ -55,8 +63,9 @@ func (h *S3TablesHandler) handleGetTableBucket(w http.ResponseWriter, r *http.Re return err } - // Check ownership - if accountID := h.getAccountID(r); accountID != metadata.OwnerAccountID { + // Check permission + principal := h.getPrincipalFromRequest(r) + if !CanGetTableBucket(principal, metadata.OwnerAccountID, bucketPolicy) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to get table bucket details") return ErrAccessDenied } @@ -83,7 +92,7 @@ func (h *S3TablesHandler) handleListTableBuckets(w http.ResponseWriter, r *http. // Check permission principal := h.getPrincipalFromRequest(r) accountID := h.getAccountID(r) - if !CanListTableBuckets(principal, accountID) { + if !CanListTableBuckets(principal, accountID, "") { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to list table buckets") return NewAuthError("ListTableBuckets", principal, "not authorized to list table buckets") } @@ -226,6 +235,7 @@ func (h *S3TablesHandler) handleDeleteTableBucket(w http.ResponseWriter, r *http // Check if bucket exists and perform ownership + emptiness check in one block var metadata tableBucketMetadata + var bucketPolicy string hasChildren := false err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { // 1. Get metadata for ownership check @@ -237,9 +247,15 @@ func (h *S3TablesHandler) handleDeleteTableBucket(w http.ResponseWriter, r *http return fmt.Errorf("failed to unmarshal metadata: %w", err) } - // 2. Check ownership + // Fetch bucket policy if it exists + policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy) + if err == nil { + bucketPolicy = string(policyData) + } + + // 2. Check permission principal := h.getPrincipalFromRequest(r) - if !CanDeleteTableBucket(principal, metadata.OwnerAccountID) { + if !CanDeleteTableBucket(principal, metadata.OwnerAccountID, bucketPolicy) { return NewAuthError("DeleteTableBucket", principal, fmt.Sprintf("not authorized to delete bucket %s", bucketName)) } diff --git a/weed/s3api/s3tables/handler_namespace.go b/weed/s3api/s3tables/handler_namespace.go index d99df4603..66efaeb3e 100644 --- a/weed/s3api/s3tables/handler_namespace.go +++ b/weed/s3api/s3tables/handler_namespace.go @@ -45,6 +45,7 @@ func (h *S3TablesHandler) handleCreateNamespace(w http.ResponseWriter, r *http.R // Check if table bucket exists bucketPath := getTableBucketPath(bucketName) var bucketMetadata tableBucketMetadata + var bucketPolicy string err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { data, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyMetadata) if err != nil { @@ -53,6 +54,13 @@ func (h *S3TablesHandler) handleCreateNamespace(w http.ResponseWriter, r *http.R if err := json.Unmarshal(data, &bucketMetadata); err != nil { return fmt.Errorf("failed to unmarshal bucket metadata: %w", err) } + + // Fetch bucket policy if it exists + policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy) + if err == nil { + bucketPolicy = string(policyData) + } + return nil }) @@ -65,8 +73,9 @@ func (h *S3TablesHandler) handleCreateNamespace(w http.ResponseWriter, r *http.R return err } - // Check ownership - if accountID := h.getAccountID(r); accountID != bucketMetadata.OwnerAccountID { + // Check permission + principal := h.getPrincipalFromRequest(r) + if !CanCreateNamespace(principal, bucketMetadata.OwnerAccountID, bucketPolicy) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to create namespace in this bucket") return ErrAccessDenied } @@ -155,15 +164,27 @@ func (h *S3TablesHandler) handleGetNamespace(w http.ResponseWriter, r *http.Requ } namespacePath := getNamespacePath(bucketName, namespaceName) + bucketPath := getTableBucketPath(bucketName) - // Get namespace + // Get namespace and bucket policy var metadata namespaceMetadata + var bucketPolicy string err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { data, err := h.getExtendedAttribute(r.Context(), client, namespacePath, ExtendedKeyMetadata) if err != nil { return err } - return json.Unmarshal(data, &metadata) + if err := json.Unmarshal(data, &metadata); err != nil { + return err + } + + // Fetch bucket policy if it exists + policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy) + if err == nil { + bucketPolicy = string(policyData) + } + + return nil }) if err != nil { @@ -175,8 +196,9 @@ func (h *S3TablesHandler) handleGetNamespace(w http.ResponseWriter, r *http.Requ return err } - // Check ownership - if accountID := h.getAccountID(r); accountID != metadata.OwnerAccountID { + // Check permission + principal := h.getPrincipalFromRequest(r) + if !CanGetNamespace(principal, metadata.OwnerAccountID, bucketPolicy) { h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, "namespace not found") return ErrAccessDenied } @@ -219,6 +241,7 @@ func (h *S3TablesHandler) handleListNamespaces(w http.ResponseWriter, r *http.Re // Check permission (check bucket ownership) var bucketMetadata tableBucketMetadata + var bucketPolicy string err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { data, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyMetadata) if err != nil { @@ -227,6 +250,13 @@ func (h *S3TablesHandler) handleListNamespaces(w http.ResponseWriter, r *http.Re if err := json.Unmarshal(data, &bucketMetadata); err != nil { return fmt.Errorf("failed to unmarshal bucket metadata: %w", err) } + + // Fetch bucket policy if it exists + policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy) + if err == nil { + bucketPolicy = string(policyData) + } + return nil }) @@ -239,8 +269,8 @@ func (h *S3TablesHandler) handleListNamespaces(w http.ResponseWriter, r *http.Re return err } - accountID := h.getAccountID(r) - if accountID != bucketMetadata.OwnerAccountID { + principal := h.getPrincipalFromRequest(r) + if !CanListNamespaces(principal, bucketMetadata.OwnerAccountID, bucketPolicy) { h.writeError(w, http.StatusNotFound, ErrCodeNoSuchBucket, fmt.Sprintf("table bucket %s not found", bucketName)) return ErrAccessDenied } @@ -306,7 +336,7 @@ func (h *S3TablesHandler) handleListNamespaces(w http.ResponseWriter, r *http.Re continue } - if metadata.OwnerAccountID != accountID { + if metadata.OwnerAccountID != bucketMetadata.OwnerAccountID { continue } @@ -377,9 +407,11 @@ func (h *S3TablesHandler) handleDeleteNamespace(w http.ResponseWriter, r *http.R } namespacePath := getNamespacePath(bucketName, namespaceName) + bucketPath := getTableBucketPath(bucketName) // Check if namespace exists and get metadata for permission check var metadata namespaceMetadata + var bucketPolicy string err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { data, err := h.getExtendedAttribute(r.Context(), client, namespacePath, ExtendedKeyMetadata) if err != nil { @@ -388,6 +420,13 @@ func (h *S3TablesHandler) handleDeleteNamespace(w http.ResponseWriter, r *http.R if err := json.Unmarshal(data, &metadata); err != nil { return fmt.Errorf("failed to unmarshal metadata: %w", err) } + + // Fetch bucket policy if it exists + policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy) + if err == nil { + bucketPolicy = string(policyData) + } + return nil }) @@ -400,8 +439,9 @@ func (h *S3TablesHandler) handleDeleteNamespace(w http.ResponseWriter, r *http.R return err } - // Check ownership - if accountID := h.getAccountID(r); accountID != metadata.OwnerAccountID { + // Check permission + principal := h.getPrincipalFromRequest(r) + if !CanDeleteNamespace(principal, metadata.OwnerAccountID, bucketPolicy) { h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, "namespace not found") return ErrAccessDenied } diff --git a/weed/s3api/s3tables/handler_policy.go b/weed/s3api/s3tables/handler_policy.go index 1c489dec4..19e86d1c8 100644 --- a/weed/s3api/s3tables/handler_policy.go +++ b/weed/s3api/s3tables/handler_policy.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net/http" + "strings" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" ) @@ -59,7 +60,7 @@ func (h *S3TablesHandler) handlePutTableBucketPolicy(w http.ResponseWriter, r *h // Check permission principal := h.getPrincipalFromRequest(r) - if !CanPutTableBucketPolicy(principal, bucketMetadata.OwnerAccountID) { + if !CanPutTableBucketPolicy(principal, bucketMetadata.OwnerAccountID, "") { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to put table bucket policy") return NewAuthError("PutTableBucketPolicy", principal, "not authorized to put table bucket policy") } @@ -132,7 +133,7 @@ func (h *S3TablesHandler) handleGetTableBucketPolicy(w http.ResponseWriter, r *h // Check permission principal := h.getPrincipalFromRequest(r) - if !CanGetTableBucketPolicy(principal, bucketMetadata.OwnerAccountID) { + if !CanGetTableBucketPolicy(principal, bucketMetadata.OwnerAccountID, string(policy)) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to get table bucket policy") return NewAuthError("GetTableBucketPolicy", principal, "not authorized to get table bucket policy") } @@ -169,6 +170,7 @@ func (h *S3TablesHandler) handleDeleteTableBucketPolicy(w http.ResponseWriter, r // Check if bucket exists and get metadata for ownership check var bucketMetadata tableBucketMetadata + var bucketPolicy string err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { data, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyMetadata) if err != nil { @@ -177,6 +179,13 @@ func (h *S3TablesHandler) handleDeleteTableBucketPolicy(w http.ResponseWriter, r if err := json.Unmarshal(data, &bucketMetadata); err != nil { return fmt.Errorf("failed to unmarshal bucket metadata: %w", err) } + + // Fetch bucket policy if it exists + policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy) + if err == nil { + bucketPolicy = string(policyData) + } + return nil }) if err != nil { @@ -190,7 +199,7 @@ func (h *S3TablesHandler) handleDeleteTableBucketPolicy(w http.ResponseWriter, r // Check permission principal := h.getPrincipalFromRequest(r) - if !CanDeleteTableBucketPolicy(principal, bucketMetadata.OwnerAccountID) { + if !CanDeleteTableBucketPolicy(principal, bucketMetadata.OwnerAccountID, bucketPolicy) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to delete table bucket policy") return NewAuthError("DeleteTableBucketPolicy", principal, "not authorized to delete table bucket policy") } @@ -246,8 +255,10 @@ func (h *S3TablesHandler) handlePutTablePolicy(w http.ResponseWriter, r *http.Re return err } tablePath := getTablePath(bucketName, namespaceName, tableName) + bucketPath := getTableBucketPath(bucketName) var metadata tableMetadataInternal + var bucketPolicy string err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { data, err := h.getExtendedAttribute(r.Context(), client, tablePath, ExtendedKeyMetadata) if err != nil { @@ -256,6 +267,13 @@ func (h *S3TablesHandler) handlePutTablePolicy(w http.ResponseWriter, r *http.Re if err := json.Unmarshal(data, &metadata); err != nil { return fmt.Errorf("failed to unmarshal table metadata: %w", err) } + + // Fetch bucket policy if it exists + policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy) + if err == nil { + bucketPolicy = string(policyData) + } + return nil }) @@ -270,7 +288,7 @@ func (h *S3TablesHandler) handlePutTablePolicy(w http.ResponseWriter, r *http.Re // Check permission principal := h.getPrincipalFromRequest(r) - if !CanPutTablePolicy(principal, metadata.OwnerAccountID) { + if !CanPutTablePolicy(principal, metadata.OwnerAccountID, bucketPolicy) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to put table policy") return NewAuthError("PutTablePolicy", principal, "not authorized to put table policy") } @@ -321,8 +339,10 @@ func (h *S3TablesHandler) handleGetTablePolicy(w http.ResponseWriter, r *http.Re return err } tablePath := getTablePath(bucketName, namespaceName, tableName) + bucketPath := getTableBucketPath(bucketName) var policy []byte var metadata tableMetadataInternal + var bucketPolicy string err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { // Get metadata for ownership check @@ -336,7 +356,17 @@ func (h *S3TablesHandler) handleGetTablePolicy(w http.ResponseWriter, r *http.Re // Get policy policy, err = h.getExtendedAttribute(r.Context(), client, tablePath, ExtendedKeyPolicy) - return err + if err != nil { + return err + } + + // Fetch bucket policy if it exists + policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy) + if err == nil { + bucketPolicy = string(policyData) + } + + return nil }) if err != nil { @@ -354,7 +384,7 @@ func (h *S3TablesHandler) handleGetTablePolicy(w http.ResponseWriter, r *http.Re // Check permission principal := h.getPrincipalFromRequest(r) - if !CanGetTablePolicy(principal, metadata.OwnerAccountID) { + if !CanGetTablePolicy(principal, metadata.OwnerAccountID, bucketPolicy) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to get table policy") return NewAuthError("GetTablePolicy", principal, "not authorized to get table policy") } @@ -399,9 +429,11 @@ func (h *S3TablesHandler) handleDeleteTablePolicy(w http.ResponseWriter, r *http return err } tablePath := getTablePath(bucketName, namespaceName, tableName) + bucketPath := getTableBucketPath(bucketName) // Check if table exists var metadata tableMetadataInternal + var bucketPolicy string err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { data, err := h.getExtendedAttribute(r.Context(), client, tablePath, ExtendedKeyMetadata) if err != nil { @@ -410,6 +442,13 @@ func (h *S3TablesHandler) handleDeleteTablePolicy(w http.ResponseWriter, r *http if err := json.Unmarshal(data, &metadata); err != nil { return fmt.Errorf("failed to unmarshal table metadata: %w", err) } + + // Fetch bucket policy if it exists + policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy) + if err == nil { + bucketPolicy = string(policyData) + } + return nil }) if err != nil { @@ -423,7 +462,7 @@ func (h *S3TablesHandler) handleDeleteTablePolicy(w http.ResponseWriter, r *http // Check permission principal := h.getPrincipalFromRequest(r) - if !CanDeleteTablePolicy(principal, metadata.OwnerAccountID) { + if !CanDeleteTablePolicy(principal, metadata.OwnerAccountID, bucketPolicy) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to delete table policy") return NewAuthError("DeleteTablePolicy", principal, "not authorized to delete table policy") } @@ -468,6 +507,7 @@ func (h *S3TablesHandler) handleTagResource(w http.ResponseWriter, r *http.Reque // Read existing tags and merge, AND check permissions based on metadata ownership existingTags := make(map[string]string) + var bucketPolicy string err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { // Read metadata for ownership check data, err := h.getExtendedAttribute(r.Context(), client, resourcePath, ExtendedKeyMetadata) @@ -476,23 +516,45 @@ func (h *S3TablesHandler) handleTagResource(w http.ResponseWriter, r *http.Reque } var ownerAccountID string + var bucketName string if rType == ResourceTypeTable { var meta tableMetadataInternal if err := json.Unmarshal(data, &meta); err != nil { return err } ownerAccountID = meta.OwnerAccountID + // Extract bucket name from resource path for tables + // resourcePath format: /tables/{bucket}/{namespace}/{table} + parts := strings.Split(strings.Trim(resourcePath, "/"), "/") + if len(parts) >= 2 { + bucketName = parts[1] + } } else { var meta tableBucketMetadata if err := json.Unmarshal(data, &meta); err != nil { return err } ownerAccountID = meta.OwnerAccountID + // Extract bucket name from resource path for buckets + // resourcePath format: /tables/{bucket} + parts := strings.Split(strings.Trim(resourcePath, "/"), "/") + if len(parts) >= 2 { + bucketName = parts[1] + } + } + + // Fetch bucket policy if we have a bucket name + if bucketName != "" { + bucketPath := getTableBucketPath(bucketName) + policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy) + if err == nil { + bucketPolicy = string(policyData) + } } // Check Permission inside the closure because we just got the ID principal := h.getPrincipalFromRequest(r) - if !CanManageTags(principal, ownerAccountID) { + if !CanManageTags(principal, ownerAccountID, bucketPolicy) { return NewAuthError("TagResource", principal, "not authorized to tag resource") } @@ -591,7 +653,7 @@ func (h *S3TablesHandler) handleListTagsForResource(w http.ResponseWriter, r *ht // Check Permission principal := h.getPrincipalFromRequest(r) - if !CheckPermission("ListTagsForResource", principal, ownerAccountID) { + if !CheckPermission("ListTagsForResource", principal, ownerAccountID, "") { return NewAuthError("ListTagsForResource", principal, "not authorized to list tags for resource") } @@ -654,6 +716,7 @@ func (h *S3TablesHandler) handleUntagResource(w http.ResponseWriter, r *http.Req // Read existing tags, check permission tags := make(map[string]string) + var bucketPolicy string err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { // Read metadata for ownership check data, err := h.getExtendedAttribute(r.Context(), client, resourcePath, ExtendedKeyMetadata) @@ -662,23 +725,43 @@ func (h *S3TablesHandler) handleUntagResource(w http.ResponseWriter, r *http.Req } var ownerAccountID string + var bucketName string if rType == ResourceTypeTable { var meta tableMetadataInternal if err := json.Unmarshal(data, &meta); err != nil { return err } ownerAccountID = meta.OwnerAccountID + // Extract bucket name from resource path for tables + parts := strings.Split(strings.Trim(resourcePath, "/"), "/") + if len(parts) >= 2 { + bucketName = parts[1] + } } else { var meta tableBucketMetadata if err := json.Unmarshal(data, &meta); err != nil { return err } ownerAccountID = meta.OwnerAccountID + // Extract bucket name from resource path for buckets + parts := strings.Split(strings.Trim(resourcePath, "/"), "/") + if len(parts) >= 2 { + bucketName = parts[1] + } + } + + // Fetch bucket policy if we have a bucket name + if bucketName != "" { + bucketPath := getTableBucketPath(bucketName) + policyData, err := h.getExtendedAttribute(r.Context(), client, bucketPath, ExtendedKeyPolicy) + if err == nil { + bucketPolicy = string(policyData) + } } // Check Permission principal := h.getPrincipalFromRequest(r) - if !CanManageTags(principal, ownerAccountID) { + if !CanManageTags(principal, ownerAccountID, bucketPolicy) { return NewAuthError("UntagResource", principal, "not authorized to untag resource") } diff --git a/weed/s3api/s3tables/permissions.go b/weed/s3api/s3tables/permissions.go index 52ade3978..b89837850 100644 --- a/weed/s3api/s3tables/permissions.go +++ b/weed/s3api/s3tables/permissions.go @@ -1,84 +1,28 @@ package s3tables import ( - "fmt" + "encoding/json" + "strings" ) // Permission represents a specific action permission type Permission string -const ( - // Table bucket permissions - PermCreateTableBucket Permission = "s3tables:CreateTableBucket" - PermDeleteTableBucket Permission = "s3tables:DeleteTableBucket" - PermGetTableBucket Permission = "s3tables:GetTableBucket" - PermListTableBuckets Permission = "s3tables:ListTableBuckets" - - // Namespace permissions - PermCreateNamespace Permission = "s3tables:CreateNamespace" - PermDeleteNamespace Permission = "s3tables:DeleteNamespace" - PermGetNamespace Permission = "s3tables:GetNamespace" - PermListNamespaces Permission = "s3tables:ListNamespaces" - - // Table permissions - PermCreateTable Permission = "s3tables:CreateTable" - PermDeleteTable Permission = "s3tables:DeleteTable" - PermGetTable Permission = "s3tables:GetTable" - PermListTables Permission = "s3tables:ListTables" - - // Policy permissions - PermPutTableBucketPolicy Permission = "s3tables:PutTableBucketPolicy" - PermGetTableBucketPolicy Permission = "s3tables:GetTableBucketPolicy" - PermDeleteTableBucketPolicy Permission = "s3tables:DeleteTableBucketPolicy" - PermPutTablePolicy Permission = "s3tables:PutTablePolicy" - PermGetTablePolicy Permission = "s3tables:GetTablePolicy" - PermDeleteTablePolicy Permission = "s3tables:DeleteTablePolicy" - - // Tagging permissions - PermTagResource Permission = "s3tables:TagResource" - PermListTagsForResource Permission = "s3tables:ListTagsForResource" - PermUntagResource Permission = "s3tables:UntagResource" -) +// IAM Policy structures for evaluation +type PolicyDocument struct { + Version string `json:"Version"` + Statement []Statement `json:"Statement"` +} -// PermissionSet represents a set of allowed permissions for a principal -type PermissionSet map[Permission]bool - -// PermissionPolicy defines access control rules -type PermissionPolicy struct { - // Owner has full access to all operations - Owner string - - // Permissions map principal (account ID) to allowed permissions - Permissions map[string]PermissionSet -} - -// OperationPermissions maps S3 Tables operations to required permissions -var OperationPermissions = map[string]Permission{ - "CreateTableBucket": PermCreateTableBucket, - "DeleteTableBucket": PermDeleteTableBucket, - "GetTableBucket": PermGetTableBucket, - "ListTableBuckets": PermListTableBuckets, - "CreateNamespace": PermCreateNamespace, - "DeleteNamespace": PermDeleteNamespace, - "GetNamespace": PermGetNamespace, - "ListNamespaces": PermListNamespaces, - "CreateTable": PermCreateTable, - "DeleteTable": PermDeleteTable, - "GetTable": PermGetTable, - "ListTables": PermListTables, - "PutTableBucketPolicy": PermPutTableBucketPolicy, - "GetTableBucketPolicy": PermGetTableBucketPolicy, - "DeleteTableBucketPolicy": PermDeleteTableBucketPolicy, - "PutTablePolicy": PermPutTablePolicy, - "GetTablePolicy": PermGetTablePolicy, - "DeleteTablePolicy": PermDeleteTablePolicy, - "TagResource": PermTagResource, - "ListTagsForResource": PermListTagsForResource, - "UntagResource": PermUntagResource, +type Statement struct { + Effect string `json:"Effect"` // "Allow" or "Deny" + Principal interface{} `json:"Principal"` // Can be string, []string, or map + Action interface{} `json:"Action"` // Can be string or []string + Resource interface{} `json:"Resource"` // Can be string or []string } // CheckPermission checks if a principal has permission to perform an operation -func CheckPermission(operation, principal, owner string) bool { +func CheckPermission(operation, principal, owner, resourcePolicy string) bool { // Deny access if identities are empty if principal == "" || owner == "" { return false @@ -89,105 +33,213 @@ func CheckPermission(operation, principal, owner string) bool { return true } - // For now, only the owner can perform operations - // This can be extended to support more granular permissions via policies - // TODO: Integrate with full IAM policy evaluation + // If no policy is provided, deny access (default deny) + if resourcePolicy == "" { + return false + } + + // Parse and evaluate policy + var policy PolicyDocument + if err := json.Unmarshal([]byte(resourcePolicy), &policy); err != nil { + return false + } + + // Evaluate policy statements + // Default is deny, so we need an explicit allow + hasAllow := false + + for _, stmt := range policy.Statement { + // Check if principal matches + if !matchesPrincipal(stmt.Principal, principal) { + continue + } + + // Check if action matches + if !matchesAction(stmt.Action, operation) { + continue + } + + // Statement matches - check effect + if stmt.Effect == "Allow" { + hasAllow = true + } else if stmt.Effect == "Deny" { + // Explicit deny always wins + return false + } + } + + return hasAllow +} + +// matchesPrincipal checks if the principal matches the statement's principal +func matchesPrincipal(principalSpec interface{}, principal string) bool { + if principalSpec == nil { + return false + } + + switch p := principalSpec.(type) { + case string: + // Direct string match or wildcard + return p == "*" || p == principal + case []interface{}: + // Array of principals + for _, item := range p { + if str, ok := item.(string); ok { + if str == "*" || str == principal { + return true + } + } + } + case map[string]interface{}: + // AWS-style principal with service prefix, e.g., {"AWS": "arn:aws:iam::..."} + // For S3 Tables, we primarily care about the AWS key + if aws, ok := p["AWS"]; ok { + return matchesPrincipal(aws, principal) + } + } + return false } -// CanCreateTableBucket checks if principal can create table buckets -func CanCreateTableBucket(principal, owner string) bool { - return CheckPermission("CreateTableBucket", principal, owner) +// matchesAction checks if the action matches the statement's action +func matchesAction(actionSpec interface{}, action string) bool { + if actionSpec == nil { + return false + } + + switch a := actionSpec.(type) { + case string: + // Direct match or wildcard + return matchesActionPattern(a, action) + case []interface{}: + // Array of actions + for _, item := range a { + if str, ok := item.(string); ok { + if matchesActionPattern(str, action) { + return true + } + } + } + } + + return false } -// CanDeleteTableBucket checks if principal can delete table buckets -func CanDeleteTableBucket(principal, owner string) bool { - return CheckPermission("DeleteTableBucket", principal, owner) +// matchesActionPattern checks if an action matches a pattern (supports wildcards) +func matchesActionPattern(pattern, action string) bool { + if pattern == "*" { + return true + } + + // Exact match + if pattern == action { + return true + } + + // Wildcard match (e.g., "s3tables:*" matches "s3tables:GetTable") + if strings.HasSuffix(pattern, "*") { + prefix := strings.TrimSuffix(pattern, "*") + return strings.HasPrefix(action, prefix) + } + + return false } -// CanGetTableBucket checks if principal can read table bucket details -func CanGetTableBucket(principal, owner string) bool { - return CheckPermission("GetTableBucket", principal, owner) +// Helper functions for specific permissions + +// CanCreateTableBucket checks if principal can create table buckets +func CanCreateTableBucket(principal, owner, resourcePolicy string) bool { + return CheckPermission("CreateTableBucket", principal, owner, resourcePolicy) +} + +// CanGetTableBucket checks if principal can get table bucket details +func CanGetTableBucket(principal, owner, resourcePolicy string) bool { + return CheckPermission("GetTableBucket", principal, owner, resourcePolicy) } // CanListTableBuckets checks if principal can list table buckets -func CanListTableBuckets(principal, owner string) bool { - return CheckPermission("ListTableBuckets", principal, owner) +func CanListTableBuckets(principal, owner, resourcePolicy string) bool { + return CheckPermission("ListTableBuckets", principal, owner, resourcePolicy) } -// CanCreateNamespace checks if principal can create namespaces -func CanCreateNamespace(principal, owner string) bool { - return CheckPermission("CreateNamespace", principal, owner) +// CanDeleteTableBucket checks if principal can delete table buckets +func CanDeleteTableBucket(principal, owner, resourcePolicy string) bool { + return CheckPermission("DeleteTableBucket", principal, owner, resourcePolicy) } -// CanDeleteNamespace checks if principal can delete namespaces -func CanDeleteNamespace(principal, owner string) bool { - return CheckPermission("DeleteNamespace", principal, owner) +// CanPutTableBucketPolicy checks if principal can put table bucket policies +func CanPutTableBucketPolicy(principal, owner, resourcePolicy string) bool { + return CheckPermission("PutTableBucketPolicy", principal, owner, resourcePolicy) } -// CanGetNamespace checks if principal can read namespace details -func CanGetNamespace(principal, owner string) bool { - return CheckPermission("GetNamespace", principal, owner) +// CanGetTableBucketPolicy checks if principal can get table bucket policies +func CanGetTableBucketPolicy(principal, owner, resourcePolicy string) bool { + return CheckPermission("GetTableBucketPolicy", principal, owner, resourcePolicy) } -// CanListNamespaces checks if principal can list namespaces -func CanListNamespaces(principal, owner string) bool { - return CheckPermission("ListNamespaces", principal, owner) +// CanDeleteTableBucketPolicy checks if principal can delete table bucket policies +func CanDeleteTableBucketPolicy(principal, owner, resourcePolicy string) bool { + return CheckPermission("DeleteTableBucketPolicy", principal, owner, resourcePolicy) } -// CanCreateTable checks if principal can create tables -func CanCreateTable(principal, owner string) bool { - return CheckPermission("CreateTable", principal, owner) +// CanCreateNamespace checks if principal can create namespaces +func CanCreateNamespace(principal, owner, resourcePolicy string) bool { + return CheckPermission("CreateNamespace", principal, owner, resourcePolicy) } -// CanDeleteTable checks if principal can delete tables -func CanDeleteTable(principal, owner string) bool { - return CheckPermission("DeleteTable", principal, owner) +// CanGetNamespace checks if principal can get namespace details +func CanGetNamespace(principal, owner, resourcePolicy string) bool { + return CheckPermission("GetNamespace", principal, owner, resourcePolicy) } -// CanGetTable checks if principal can read table details -func CanGetTable(principal, owner string) bool { - return CheckPermission("GetTable", principal, owner) +// CanListNamespaces checks if principal can list namespaces +func CanListNamespaces(principal, owner, resourcePolicy string) bool { + return CheckPermission("ListNamespaces", principal, owner, resourcePolicy) } -// CanListTables checks if principal can list tables -func CanListTables(principal, owner string) bool { - return CheckPermission("ListTables", principal, owner) +// CanDeleteNamespace checks if principal can delete namespaces +func CanDeleteNamespace(principal, owner, resourcePolicy string) bool { + return CheckPermission("DeleteNamespace", principal, owner, resourcePolicy) +} + +// CanCreateTable checks if principal can create tables +func CanCreateTable(principal, owner, resourcePolicy string) bool { + return CheckPermission("CreateTable", principal, owner, resourcePolicy) } -// CanPutTableBucketPolicy checks if principal can put table bucket policy -func CanPutTableBucketPolicy(principal, owner string) bool { - return CheckPermission("PutTableBucketPolicy", principal, owner) +// CanGetTable checks if principal can get table details +func CanGetTable(principal, owner, resourcePolicy string) bool { + return CheckPermission("GetTable", principal, owner, resourcePolicy) } -// CanGetTableBucketPolicy checks if principal can get table bucket policy -func CanGetTableBucketPolicy(principal, owner string) bool { - return CheckPermission("GetTableBucketPolicy", principal, owner) +// CanListTables checks if principal can list tables +func CanListTables(principal, owner, resourcePolicy string) bool { + return CheckPermission("ListTables", principal, owner, resourcePolicy) } -// CanDeleteTableBucketPolicy checks if principal can delete table bucket policy -func CanDeleteTableBucketPolicy(principal, owner string) bool { - return CheckPermission("DeleteTableBucketPolicy", principal, owner) +// CanDeleteTable checks if principal can delete tables +func CanDeleteTable(principal, owner, resourcePolicy string) bool { + return CheckPermission("DeleteTable", principal, owner, resourcePolicy) } -// CanPutTablePolicy checks if principal can put table policy -func CanPutTablePolicy(principal, owner string) bool { - return CheckPermission("PutTablePolicy", principal, owner) +// CanPutTablePolicy checks if principal can put table policies +func CanPutTablePolicy(principal, owner, resourcePolicy string) bool { + return CheckPermission("PutTablePolicy", principal, owner, resourcePolicy) } -// CanGetTablePolicy checks if principal can get table policy -func CanGetTablePolicy(principal, owner string) bool { - return CheckPermission("GetTablePolicy", principal, owner) +// CanGetTablePolicy checks if principal can get table policies +func CanGetTablePolicy(principal, owner, resourcePolicy string) bool { + return CheckPermission("GetTablePolicy", principal, owner, resourcePolicy) } -// CanDeleteTablePolicy checks if principal can delete table policy -func CanDeleteTablePolicy(principal, owner string) bool { - return CheckPermission("DeleteTablePolicy", principal, owner) +// CanDeleteTablePolicy checks if principal can delete table policies +func CanDeleteTablePolicy(principal, owner, resourcePolicy string) bool { + return CheckPermission("DeleteTablePolicy", principal, owner, resourcePolicy) } // CanManageTags checks if principal can manage tags -func CanManageTags(principal, owner string) bool { - return CheckPermission("TagResource", principal, owner) +func CanManageTags(principal, owner, resourcePolicy string) bool { + return CheckPermission("ManageTags", principal, owner, resourcePolicy) } // AuthError represents an authorization error @@ -198,7 +250,7 @@ type AuthError struct { } func (e *AuthError) Error() string { - return fmt.Sprintf("unauthorized: %s is not permitted to perform %s: %s", e.Principal, e.Operation, e.Message) + return "unauthorized: " + e.Principal + " is not permitted to perform " + e.Operation + ": " + e.Message } // NewAuthError creates a new authorization error