Mirror
/
seaweedfs
mirror of https://github.com/seaweedfs/seaweedfs.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Revert "Fix IAM defaults and s3tables identities" 

This reverts commit bf71fe0039.
pull/8374/head
Chris Lu 2 days ago
parent
bf71fe0039
commit
eda4a000cc
5 changed files with 12 additions and 260 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 123
      weed/s3api/iam_defaults_test.go
  2. 8
      weed/s3api/s3api_server.go
  3. 56
      weed/s3api/s3tables/handler.go
  4. 81
      weed/s3api/s3tables/handler_identity_test.go
  5. 4
      weed/s3api/s3tables/permissions.go

123
weed/s3api/iam_defaults_test.go
View File

@ -1,12 +1,10 @@
package s3api
import (
"context"
"os"
"path/filepath"
"testing"
"github.com/seaweedfs/seaweedfs/weed/iam/integration"
"github.com/stretchr/testify/assert"
)
@ -156,124 +154,3 @@ func TestLoadIAMManagerFromConfig_MissingKeyError(t *testing.T) {
assert.Error(t, err)
assert.Contains(t, err.Error(), "no signing key found for STS service")
}
func TestLoadIAMManagerFromConfig_ExplicitFileDefaultsToDeny(t *testing.T) {
tmpDir := t.TempDir()
configPath := filepath.Join(tmpDir, "iam_config_implicit_policy_defaults.json")
// Explicit config file with no policy.defaultEffect should default to Deny.
configContent := `{
"sts": {
"issuer": "explicit-config"
}
}`
err := os.WriteFile(configPath, []byte(configContent), 0644)
assert.NoError(t, err)
filerProvider := func() string { return "localhost:8888" }
defaultSigningKeyProvider := func() string { return "fallback-key-for-explicit-config" }
manager, err := loadIAMManagerFromConfig(configPath, filerProvider, defaultSigningKeyProvider)
assert.NoError(t, err)
assert.NotNil(t, manager)
assert.False(t, manager.DefaultAllow())
}
func TestLoadIAMManagerFromConfig_NoFileDefaultsToAllow(t *testing.T) {
// No explicit IAM file should preserve zero-config startup behavior.
filerProvider := func() string { return "localhost:8888" }
defaultSigningKeyProvider := func() string { return "fallback-key-for-zero-config" }
manager, err := loadIAMManagerFromConfig("", filerProvider, defaultSigningKeyProvider)
assert.NoError(t, err)
assert.NotNil(t, manager)
assert.True(t, manager.DefaultAllow())
}
func TestLoadIAMManagerFromConfig_ExplicitFileEnforcesUserScopedPolicy(t *testing.T) {
tmpDir := t.TempDir()
configPath := filepath.Join(tmpDir, "iam_regression_8366.json")
// Regression coverage for #8366:
// with explicit IAM config and omitted policy.defaultEffect, unrestricted bucket creation
// must NOT be allowed.
configContent := `{
"sts": {
"issuer": "seaweedfs-sts"
},
"policies": [
{
"name": "S3UserPolicy",
"document": {
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": ["s3:ListAllMyBuckets"],
"Resource": ["arn:aws:s3:::*"]
},
{
"Effect": "Allow",
"Action": ["s3:*"],
"Resource": [
"arn:aws:s3:::user-${jwt:preferred_username}",
"arn:aws:s3:::user-${jwt:preferred_username}/*"
]
}
]
}
}
],
"roles": [
{
"roleName": "S3UserRole",
"roleArn": "arn:aws:iam::role/S3UserRole",
"attachedPolicies": ["S3UserPolicy"],
"trustPolicy": {
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Principal": {"Federated": "*"},
"Action": ["sts:AssumeRoleWithWebIdentity"]
}
]
}
}
]
}`
err := os.WriteFile(configPath, []byte(configContent), 0644)
assert.NoError(t, err)
filerProvider := func() string { return "localhost:8888" }
defaultSigningKeyProvider := func() string { return "fallback-key-for-regression-8366" }
manager, err := loadIAMManagerFromConfig(configPath, filerProvider, defaultSigningKeyProvider)
assert.NoError(t, err)
assert.NotNil(t, manager)
assert.False(t, manager.DefaultAllow())
ctx := context.Background()
principal := "arn:aws:sts::000000000000:assumed-role/S3UserRole/alice-session"
reqCtx := map[string]interface{}{"jwt:preferred_username": "alice"}
allowed, err := manager.IsActionAllowed(ctx, &integration.ActionRequest{
Principal: principal,
Action: "s3:CreateBucket",
Resource: "arn:aws:s3:::arbitrary-bucket",
RequestContext: reqCtx,
})
assert.NoError(t, err)
assert.False(t, allowed, "arbitrary bucket creation should be denied")
allowed, err = manager.IsActionAllowed(ctx, &integration.ActionRequest{
Principal: principal,
Action: "s3:CreateBucket",
Resource: "arn:aws:s3:::user-alice",
RequestContext: reqCtx,
})
assert.NoError(t, err)
assert.True(t, allowed, "user-scoped bucket creation should be allowed")
}

8
weed/s3api/s3api_server.go
View File

@ -884,10 +884,10 @@ func loadIAMManagerFromConfig(configPath string, filerAddressProvider func() str
configRoot.Policy.StoreType = sts.StoreTypeMemory
}
if configRoot.Policy.DefaultEffect == "" {
// Secure default when an explicit IAM config file is provided:
// omitted defaultEffect should be Deny to avoid unintentional privilege escalation.
// Keep zero-config startup behavior (no config file path) open for memory store.
if configPath == "" && configRoot.Policy.StoreType == sts.StoreTypeMemory {
// Default to Allow (open) with in-memory store so that
// users can start using STS without locking themselves out immediately.
// For other stores (e.g. filer), default to Deny (closed) for security.
if configRoot.Policy.StoreType == sts.StoreTypeMemory {
configRoot.Policy.DefaultEffect = sts.EffectAllow
} else {
configRoot.Policy.DefaultEffect = sts.EffectDeny

56
weed/s3api/s3tables/handler.go
View File

@ -168,46 +168,26 @@ func (h *S3TablesHandler) HandleRequest(w http.ResponseWriter, r *http.Request,
// Principal/authorization helpers
// getAccountID returns a stable caller identifier for ownership and permission checks.
// Prefer JWT/STS user claims to avoid collapsing all assumed-role callers to the same
// account ID (e.g. 000000000000), which would effectively grant cross-user access.
// getAccountID returns the authenticated account ID from the request or the handler's default.
// This is also used as the principal for permission checks, ensuring alignment between
// the caller identity and ownership verification when IAM is enabled.
func (h *S3TablesHandler) getAccountID(r *http.Request) string {
identityRaw := s3_constants.GetIdentityFromContext(r)
if identityRaw != nil {
// Use reflection to access identity fields and avoid import cycles.
// Use reflection to access the Account.Id field to avoid import cycle
val := reflect.ValueOf(identityRaw)
if val.Kind() == reflect.Ptr {
val = val.Elem()
}
if val.Kind() == reflect.Struct {
// Prefer stable user claims from JWT/STS identities.
claimsField := val.FieldByName("Claims")
if claimsField.IsValid() && claimsField.Kind() == reflect.Map && !claimsField.IsNil() && claimsField.Type().Key().Kind() == reflect.String {
for _, claimKey := range []string{"preferred_username", "sub", "email"} {
claimVal := claimsField.MapIndex(reflect.ValueOf(claimKey))
if !claimVal.IsValid() {
continue
}
if claimVal.Kind() == reflect.Interface && !claimVal.IsNil() {
claimVal = claimVal.Elem()
}
if claimVal.Kind() == reflect.String {
if principal := normalizePrincipalID(claimVal.String()); principal != "" {
return principal
}
}
}
}
accountField := val.FieldByName("Account")
if accountField.IsValid() && !accountField.IsNil() {
accountVal := accountField.Elem()
if accountVal.Kind() == reflect.Struct {
idField := accountVal.FieldByName("Id")
if idField.IsValid() && idField.Kind() == reflect.String {
if principal := normalizePrincipalID(idField.String()); principal != "" {
return principal
}
id := idField.String()
return id
}
}
}
@ -215,35 +195,15 @@ func (h *S3TablesHandler) getAccountID(r *http.Request) string {
}
if identityName := s3_constants.GetIdentityNameFromContext(r); identityName != "" {
if principal := normalizePrincipalID(identityName); principal != "" {
return principal
}
return identityName
}
if accountID := r.Header.Get(s3_constants.AmzAccountId); accountID != "" {
if principal := normalizePrincipalID(accountID); principal != "" {
return principal
}
return accountID
}
return h.accountID
}
func normalizePrincipalID(id string) string {
id = strings.TrimSpace(id)
if id == "" {
return ""
}
// If this is an ARN (common for assumed roles), use the trailing segment as a
// stable-ish principal key instead of embedding the full ARN in ownership fields.
if strings.HasPrefix(id, "arn:") {
if idx := strings.LastIndex(id, "/"); idx >= 0 && idx+1 < len(id) {
return strings.TrimSpace(id[idx+1:])
}
return ""
}
return id
}
// getIdentityActions extracts the action list from the identity object in the request context.
// Uses reflection to avoid import cycles with s3api package.
func getIdentityActions(r *http.Request) []string {

81
weed/s3api/s3tables/handler_identity_test.go
View File

@ -1,81 +0,0 @@
package s3tables
import (
"net/http"
"net/http/httptest"
"testing"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
)
type testIdentityAccount struct {
Id string
}
type testIdentity struct {
Account *testIdentityAccount
Claims map[string]interface{}
}
func TestGetAccountIDPrefersClaimsOverAccountID(t *testing.T) {
h := NewS3TablesHandler()
id := &testIdentity{
Account: &testIdentityAccount{Id: s3_constants.AccountAdminId},
Claims: map[string]interface{}{
"preferred_username": "alice",
"sub": "alice-sub",
},
}
req := httptest.NewRequest(http.MethodGet, "/", nil)
req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), id))
got := h.getAccountID(req)
if got != "alice" {
t.Fatalf("expected preferred_username claim to be used, got %q", got)
}
}
func TestGetAccountIDUsesSubWhenPreferredUsernameMissing(t *testing.T) {
h := NewS3TablesHandler()
id := &testIdentity{
Account: &testIdentityAccount{Id: s3_constants.AccountAdminId},
Claims: map[string]interface{}{
"sub": "user-123",
},
}
req := httptest.NewRequest(http.MethodGet, "/", nil)
req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), id))
got := h.getAccountID(req)
if got != "user-123" {
t.Fatalf("expected sub claim to be used, got %q", got)
}
}
func TestGetAccountIDFallsBackToAccountID(t *testing.T) {
h := NewS3TablesHandler()
id := &testIdentity{
Account: &testIdentityAccount{Id: s3_constants.AccountAdminId},
}
req := httptest.NewRequest(http.MethodGet, "/", nil)
req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), id))
got := h.getAccountID(req)
if got != s3_constants.AccountAdminId {
t.Fatalf("expected fallback account id %q, got %q", s3_constants.AccountAdminId, got)
}
}
func TestGetAccountIDNormalizesArnIdentityName(t *testing.T) {
h := NewS3TablesHandler()
req := httptest.NewRequest(http.MethodGet, "/", nil)
req = req.WithContext(s3_constants.SetIdentityNameInContext(req.Context(), "arn:aws:sts::123456789012:assumed-role/S3UserRole/alice-session"))
got := h.getAccountID(req)
if got != "alice-session" {
t.Fatalf("expected ARN session suffix, got %q", got)
}
}

4
weed/s3api/s3tables/permissions.go
View File

@ -208,10 +208,6 @@ func hasIdentityPermission(operation string, ctx *PolicyContext) bool {
candidates = append(candidates, operation+":"+ctx.TableBucketName, fullAction+":"+ctx.TableBucketName)
}
for _, action := range ctx.IdentityActions {
// Legacy static identities may still use broad admin markers.
if action == "*" || action == "Admin" || action == string(s3_constants.ACTION_ADMIN) || action == "s3:*" || action == "s3tables:*" {
return true
}
for _, candidate := range candidates {
if action == candidate {
return true

Write Preview
Loading…
Cancel
Save