From bcddf26aee58078d19b2ca38eea05c9e3e86b4a3 Mon Sep 17 00:00:00 2001 From: chrislu Date: Wed, 27 Aug 2025 19:44:19 -0700 Subject: [PATCH] address comments --- test/s3/iam/Makefile | 14 +- weed/iam/integration/iam_manager.go | 61 ++++- weed/iam/policy/policy_engine.go | 354 ++++++++++++++++++++++++++++ 3 files changed, 418 insertions(+), 11 deletions(-) diff --git a/test/s3/iam/Makefile b/test/s3/iam/Makefile index 0ed4b535e..dcd96e017 100644 --- a/test/s3/iam/Makefile +++ b/test/s3/iam/Makefile @@ -64,7 +64,8 @@ start-services: ## Start SeaweedFS services for testing -mdir=test-volume-data/m9333 > weed-master.log 2>&1 & \ echo $$! > $(MASTER_PID_FILE) - @sleep 2 + @echo "Waiting for master server to be ready..." + @timeout 30 bash -c 'until curl -s http://localhost:$(MASTER_PORT)/cluster/status > /dev/null; do sleep 1; done' || (echo "❌ Master failed to start" && exit 1) @echo "Starting volume server..." @$(WEED_BINARY) volume -port=$(VOLUME_PORT) \ @@ -74,7 +75,8 @@ start-services: ## Start SeaweedFS services for testing -mserver=localhost:$(MASTER_PORT) > weed-volume.log 2>&1 & \ echo $$! > $(VOLUME_PID_FILE) - @sleep 2 + @echo "Waiting for volume server to be ready..." + @timeout 30 bash -c 'until curl -s http://localhost:$(VOLUME_PORT)/status > /dev/null; do sleep 1; done' || (echo "❌ Volume server failed to start" && exit 1) @echo "Starting filer server..." @$(WEED_BINARY) filer -port=$(FILER_PORT) \ @@ -82,7 +84,8 @@ start-services: ## Start SeaweedFS services for testing -master=localhost:$(MASTER_PORT) > weed-filer.log 2>&1 & \ echo $$! > $(FILER_PID_FILE) - @sleep 2 + @echo "Waiting for filer server to be ready..." + @timeout 30 bash -c 'until curl -s http://localhost:$(FILER_PORT)/status > /dev/null; do sleep 1; done' || (echo "❌ Filer failed to start" && exit 1) @echo "Starting S3 API server with IAM..." @$(WEED_BINARY) -v=3 s3 -port=$(S3_PORT) \ @@ -91,7 +94,10 @@ start-services: ## Start SeaweedFS services for testing -iam.config=$(CURDIR)/iam_config.json > weed-s3.log 2>&1 & \ echo $$! > $(S3_PID_FILE) - @echo "✅ All services started" + @echo "Waiting for S3 API server to be ready..." + @timeout 30 bash -c 'until curl -s http://localhost:$(S3_PORT) > /dev/null 2>&1; do sleep 1; done' || (echo "❌ S3 API failed to start" && exit 1) + + @echo "✅ All services started and ready" wait-for-services: ## Wait for all services to be ready @echo "⏳ Waiting for services to be ready..." diff --git a/weed/iam/integration/iam_manager.go b/weed/iam/integration/iam_manager.go index 5fc3ffb88..7d1748514 100644 --- a/weed/iam/integration/iam_manager.go +++ b/weed/iam/integration/iam_manager.go @@ -468,15 +468,21 @@ func (m *IAMManager) evaluateTrustPolicy(trustPolicy *policy.PolicyDocument, eva principalMatches := false if principal, ok := statement.Principal.(map[string]interface{}); ok { // Check for Federated principal (OIDC/SAML) - if federated, ok := principal["Federated"].(string); ok { - // For web identity, check if the token issuer matches the federated provider - if tokenIssuer, exists := evalCtx.RequestContext["seaweed:FederatedProvider"]; exists { - if issuerStr, ok := tokenIssuer.(string); ok && issuerStr == federated { - principalMatches = true - } + if federatedValue, ok := principal["Federated"]; ok { + principalMatches = m.evaluatePrincipalValue(federatedValue, evalCtx, "seaweed:FederatedProvider") + } + // Check for AWS principal (IAM users/roles) + if !principalMatches { + if awsValue, ok := principal["AWS"]; ok { + principalMatches = m.evaluatePrincipalValue(awsValue, evalCtx, "seaweed:AWSPrincipal") + } + } + // Check for Service principal (AWS services) + if !principalMatches { + if serviceValue, ok := principal["Service"]; ok { + principalMatches = m.evaluatePrincipalValue(serviceValue, evalCtx, "seaweed:ServicePrincipal") } } - // Could add other principal types here (AWS, Service, etc.) } else if principalStr, ok := statement.Principal.(string); ok { // Handle string principal if principalStr == "*" { @@ -529,6 +535,47 @@ func (m *IAMManager) evaluateTrustPolicyConditions(conditions map[string]map[str return true } +// evaluatePrincipalValue evaluates a principal value (string or array) against the context +func (m *IAMManager) evaluatePrincipalValue(principalValue interface{}, evalCtx *policy.EvaluationContext, contextKey string) bool { + // Get the value from evaluation context + contextValue, exists := evalCtx.RequestContext[contextKey] + if !exists { + return false + } + + contextStr, ok := contextValue.(string) + if !ok { + return false + } + + // Handle single string value + if principalStr, ok := principalValue.(string); ok { + return principalStr == contextStr || principalStr == "*" + } + + // Handle array of strings + if principalArray, ok := principalValue.([]interface{}); ok { + for _, item := range principalArray { + if itemStr, ok := item.(string); ok { + if itemStr == contextStr || itemStr == "*" { + return true + } + } + } + } + + // Handle array of strings (alternative JSON unmarshaling format) + if principalStrArray, ok := principalValue.([]string); ok { + for _, itemStr := range principalStrArray { + if itemStr == contextStr || itemStr == "*" { + return true + } + } + } + + return false +} + // isOIDCToken checks if a token is an OIDC JWT token (vs STS session token) func isOIDCToken(token string) bool { // JWT tokens have three parts separated by dots and start with base64-encoded JSON diff --git a/weed/iam/policy/policy_engine.go b/weed/iam/policy/policy_engine.go index 5f1ca0cb3..126331cc9 100644 --- a/weed/iam/policy/policy_engine.go +++ b/weed/iam/policy/policy_engine.go @@ -6,8 +6,10 @@ import ( "net" "path/filepath" "regexp" + "strconv" "strings" "sync" + "time" ) // Effect represents the policy evaluation result @@ -363,16 +365,62 @@ func (e *PolicyEngine) matchesConditions(conditions map[string]map[string]interf // evaluateConditionBlock evaluates a single condition block func (e *PolicyEngine) evaluateConditionBlock(conditionType string, block map[string]interface{}, evalCtx *EvaluationContext) bool { switch conditionType { + // IP Address conditions case "IpAddress": return e.evaluateIPCondition(block, evalCtx, true) case "NotIpAddress": return e.evaluateIPCondition(block, evalCtx, false) + + // String conditions case "StringEquals": return e.EvaluateStringCondition(block, evalCtx, true, false) case "StringNotEquals": return e.EvaluateStringCondition(block, evalCtx, false, false) case "StringLike": return e.EvaluateStringCondition(block, evalCtx, true, true) + case "StringEqualsIgnoreCase": + return e.evaluateStringConditionIgnoreCase(block, evalCtx, true, false) + case "StringNotEqualsIgnoreCase": + return e.evaluateStringConditionIgnoreCase(block, evalCtx, false, false) + case "StringLikeIgnoreCase": + return e.evaluateStringConditionIgnoreCase(block, evalCtx, true, true) + + // Numeric conditions + case "NumericEquals": + return e.evaluateNumericCondition(block, evalCtx, "==") + case "NumericNotEquals": + return e.evaluateNumericCondition(block, evalCtx, "!=") + case "NumericLessThan": + return e.evaluateNumericCondition(block, evalCtx, "<") + case "NumericLessThanEquals": + return e.evaluateNumericCondition(block, evalCtx, "<=") + case "NumericGreaterThan": + return e.evaluateNumericCondition(block, evalCtx, ">") + case "NumericGreaterThanEquals": + return e.evaluateNumericCondition(block, evalCtx, ">=") + + // Date conditions + case "DateEquals": + return e.evaluateDateCondition(block, evalCtx, "==") + case "DateNotEquals": + return e.evaluateDateCondition(block, evalCtx, "!=") + case "DateLessThan": + return e.evaluateDateCondition(block, evalCtx, "<") + case "DateLessThanEquals": + return e.evaluateDateCondition(block, evalCtx, "<=") + case "DateGreaterThan": + return e.evaluateDateCondition(block, evalCtx, ">") + case "DateGreaterThanEquals": + return e.evaluateDateCondition(block, evalCtx, ">=") + + // Boolean conditions + case "Bool": + return e.evaluateBoolCondition(block, evalCtx) + + // Null conditions + case "Null": + return e.evaluateNullCondition(block, evalCtx) + default: // Unknown condition types default to false (more secure) return false @@ -731,3 +779,309 @@ func matchAction(pattern, action string) bool { return matched } + +// evaluateStringConditionIgnoreCase evaluates string conditions with case insensitivity +func (e *PolicyEngine) evaluateStringConditionIgnoreCase(block map[string]interface{}, evalCtx *EvaluationContext, shouldMatch bool, useWildcard bool) bool { + for key, expectedValues := range block { + contextValue, exists := evalCtx.RequestContext[key] + if !exists { + if !shouldMatch { + continue // For NotEquals, missing key is OK + } + return false + } + + contextStr, ok := contextValue.(string) + if !ok { + return false + } + + contextStr = strings.ToLower(contextStr) + matched := false + + // Handle different value types + switch v := expectedValues.(type) { + case string: + expectedStr := strings.ToLower(v) + if useWildcard { + matched, _ = filepath.Match(expectedStr, contextStr) + } else { + matched = expectedStr == contextStr + } + case []interface{}: + for _, val := range v { + if valStr, ok := val.(string); ok { + expectedStr := strings.ToLower(valStr) + if useWildcard { + if m, _ := filepath.Match(expectedStr, contextStr); m { + matched = true + break + } + } else { + if expectedStr == contextStr { + matched = true + break + } + } + } + } + } + + if shouldMatch && !matched { + return false + } + if !shouldMatch && matched { + return false + } + } + return true +} + +// evaluateNumericCondition evaluates numeric conditions +func (e *PolicyEngine) evaluateNumericCondition(block map[string]interface{}, evalCtx *EvaluationContext, operator string) bool { + for key, expectedValues := range block { + contextValue, exists := evalCtx.RequestContext[key] + if !exists { + return false + } + + contextNum, err := parseNumeric(contextValue) + if err != nil { + return false + } + + matched := false + + // Handle different value types + switch v := expectedValues.(type) { + case string: + expectedNum, err := parseNumeric(v) + if err != nil { + return false + } + matched = compareNumbers(contextNum, expectedNum, operator) + case []interface{}: + for _, val := range v { + expectedNum, err := parseNumeric(val) + if err != nil { + continue + } + if compareNumbers(contextNum, expectedNum, operator) { + matched = true + break + } + } + } + + if !matched { + return false + } + } + return true +} + +// evaluateDateCondition evaluates date conditions +func (e *PolicyEngine) evaluateDateCondition(block map[string]interface{}, evalCtx *EvaluationContext, operator string) bool { + for key, expectedValues := range block { + contextValue, exists := evalCtx.RequestContext[key] + if !exists { + return false + } + + contextTime, err := parseDateTime(contextValue) + if err != nil { + return false + } + + matched := false + + // Handle different value types + switch v := expectedValues.(type) { + case string: + expectedTime, err := parseDateTime(v) + if err != nil { + return false + } + matched = compareDates(contextTime, expectedTime, operator) + case []interface{}: + for _, val := range v { + expectedTime, err := parseDateTime(val) + if err != nil { + continue + } + if compareDates(contextTime, expectedTime, operator) { + matched = true + break + } + } + } + + if !matched { + return false + } + } + return true +} + +// evaluateBoolCondition evaluates boolean conditions +func (e *PolicyEngine) evaluateBoolCondition(block map[string]interface{}, evalCtx *EvaluationContext) bool { + for key, expectedValues := range block { + contextValue, exists := evalCtx.RequestContext[key] + if !exists { + return false + } + + contextBool, err := parseBool(contextValue) + if err != nil { + return false + } + + matched := false + + // Handle different value types + switch v := expectedValues.(type) { + case string: + expectedBool, err := parseBool(v) + if err != nil { + return false + } + matched = contextBool == expectedBool + case bool: + matched = contextBool == v + case []interface{}: + for _, val := range v { + expectedBool, err := parseBool(val) + if err != nil { + continue + } + if contextBool == expectedBool { + matched = true + break + } + } + } + + if !matched { + return false + } + } + return true +} + +// evaluateNullCondition evaluates null conditions +func (e *PolicyEngine) evaluateNullCondition(block map[string]interface{}, evalCtx *EvaluationContext) bool { + for key, expectedValues := range block { + _, exists := evalCtx.RequestContext[key] + + expectedNull := false + switch v := expectedValues.(type) { + case string: + expectedNull = v == "true" + case bool: + expectedNull = v + } + + // If we expect null (true) and key exists, or expect non-null (false) and key doesn't exist + if expectedNull == exists { + return false + } + } + return true +} + +// Helper functions for parsing and comparing values + +// parseNumeric parses a value as a float64 +func parseNumeric(value interface{}) (float64, error) { + switch v := value.(type) { + case float64: + return v, nil + case float32: + return float64(v), nil + case int: + return float64(v), nil + case int64: + return float64(v), nil + case string: + return strconv.ParseFloat(v, 64) + default: + return 0, fmt.Errorf("cannot parse %T as numeric", value) + } +} + +// compareNumbers compares two numbers using the given operator +func compareNumbers(a, b float64, operator string) bool { + switch operator { + case "==": + return a == b + case "!=": + return a != b + case "<": + return a < b + case "<=": + return a <= b + case ">": + return a > b + case ">=": + return a >= b + default: + return false + } +} + +// parseDateTime parses a value as a time.Time +func parseDateTime(value interface{}) (time.Time, error) { + switch v := value.(type) { + case string: + // Try common date formats + formats := []string{ + time.RFC3339, + "2006-01-02T15:04:05Z", + "2006-01-02T15:04:05", + "2006-01-02 15:04:05", + "2006-01-02", + } + for _, format := range formats { + if t, err := time.Parse(format, v); err == nil { + return t, nil + } + } + return time.Time{}, fmt.Errorf("cannot parse date: %s", v) + case time.Time: + return v, nil + default: + return time.Time{}, fmt.Errorf("cannot parse %T as date", value) + } +} + +// compareDates compares two dates using the given operator +func compareDates(a, b time.Time, operator string) bool { + switch operator { + case "==": + return a.Equal(b) + case "!=": + return !a.Equal(b) + case "<": + return a.Before(b) + case "<=": + return a.Before(b) || a.Equal(b) + case ">": + return a.After(b) + case ">=": + return a.After(b) || a.Equal(b) + default: + return false + } +} + +// parseBool parses a value as a boolean +func parseBool(value interface{}) (bool, error) { + switch v := value.(type) { + case bool: + return v, nil + case string: + return strconv.ParseBool(v) + default: + return false, fmt.Errorf("cannot parse %T as boolean", value) + } +}