diff --git a/test/s3/iam/Makefile b/test/s3/iam/Makefile index 7b4358ca0..9d02473be 100644 --- a/test/s3/iam/Makefile +++ b/test/s3/iam/Makefile @@ -87,7 +87,7 @@ start-services: ## Start SeaweedFS services for testing @$(WEED_BINARY) -v=3 s3 -port=$(S3_PORT) \ -filer=localhost:$(FILER_PORT) \ -config=test_config.json \ - -iam.config=$(shell pwd)/iam_config.json > weed-s3.log 2>&1 & \ + -iam.config=$(CURDIR)/iam_config.json > weed-s3.log 2>&1 & \ echo $$! > $(S3_PID_FILE) @echo "โœ… All services started" @@ -234,9 +234,11 @@ docker-logs: ## Show logs from all services docker-test: docker-up ## Run tests with Docker Compose including Keycloak @echo "๐Ÿงช Running Keycloak integration tests..." + @echo "โณ Waiting for services to be ready..." + @timeout 120 bash -c 'until curl -s http://localhost:8080/health/ready > /dev/null 2>&1; do sleep 2; done' || (echo "โŒ Keycloak failed to become ready" && exit 1) + @timeout 60 bash -c 'until curl -s http://localhost:8333 > /dev/null 2>&1; do sleep 2; done' || (echo "โŒ S3 API failed to become ready" && exit 1) @export KEYCLOAK_URL="http://localhost:8080" && \ export S3_ENDPOINT="http://localhost:8333" && \ - sleep 10 && \ go test -v -timeout $(TEST_TIMEOUT) -run "TestKeycloak" ./... @echo "๐Ÿณ Stopping services after tests..." @make docker-down @@ -274,9 +276,11 @@ test-versioning-stress: ## Run S3 versioning stress tests test-keycloak-full: docker-up ## Run complete Keycloak integration tests @echo "๐Ÿ” Running complete Keycloak integration tests..." + @echo "โณ Waiting for services to be ready..." + @timeout 120 bash -c 'until curl -s http://localhost:8080/health/ready > /dev/null 2>&1; do sleep 2; done' || (echo "โŒ Keycloak failed to become ready" && exit 1) + @timeout 60 bash -c 'until curl -s http://localhost:8333 > /dev/null 2>&1; do sleep 2; done' || (echo "โŒ S3 API failed to become ready" && exit 1) @export KEYCLOAK_URL="http://localhost:8080" && \ export S3_ENDPOINT="http://localhost:8333" && \ - sleep 15 && \ go test -v -timeout $(TEST_TIMEOUT) -run "TestKeycloak" ./... @make docker-down diff --git a/test/s3/iam/s3_iam_distributed_test.go b/test/s3/iam/s3_iam_distributed_test.go index 61716d86b..9634640f3 100644 --- a/test/s3/iam/s3_iam_distributed_test.go +++ b/test/s3/iam/s3_iam_distributed_test.go @@ -218,13 +218,11 @@ func TestS3IAMDistributedTests(t *testing.T) { // STRICT CONCURRENCY TESTING: Use appropriate thresholds for the operation count // For totalOperations=6, error thresholds need to be adjusted to avoid unreachable conditions - // Serious errors (race conditions, deadlocks) should be zero or very rare - maxSeriousErrors := 1 // Allow 1 serious error max (16.7%) for small test size + // Serious errors (race conditions, deadlocks) should be zero for reliable testing + maxSeriousErrors := 0 // Allow 0 serious errors max - any serious error indicates system issues if len(seriousErrors) > maxSeriousErrors { - t.Errorf("โŒ Too many serious errors: %d (%.1f%%) - max allowed: %d. This indicates potential race conditions, deadlocks, or other concurrency bugs.", - len(seriousErrors), seriousErrorRate*100, maxSeriousErrors) - } else if len(seriousErrors) > 0 { - t.Logf("โš ๏ธ %d serious error detected (%.1f%%) - within threshold but should be investigated", len(seriousErrors), seriousErrorRate*100) + t.Errorf("โŒ %d serious error(s) detected, indicating potential concurrency bugs. First error: %v", + len(seriousErrors), seriousErrors[0]) } // For total errors, allow more flexibility with small operation counts diff --git a/weed/iam/integration/iam_manager.go b/weed/iam/integration/iam_manager.go index 14622f198..4d69b89ba 100644 --- a/weed/iam/integration/iam_manager.go +++ b/weed/iam/integration/iam_manager.go @@ -95,6 +95,9 @@ func (m *IAMManager) Initialize(config *IAMConfig) error { return fmt.Errorf("failed to initialize STS service: %w", err) } + // CRITICAL SECURITY: Set trust policy validator to ensure proper role assumption validation + m.stsService.SetTrustPolicyValidator(m) + // Initialize policy engine m.policyEngine = policy.NewPolicyEngine() if err := m.policyEngine.Initialize(config.Policy); err != nil { @@ -514,8 +517,6 @@ func (m *IAMManager) evaluateTrustPolicyConditions(conditions map[string]map[str return true } - - // 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 @@ -527,3 +528,50 @@ func isOIDCToken(token string) bool { // JWT tokens typically start with "eyJ" (base64 encoded JSON starting with "{") return strings.HasPrefix(token, "eyJ") } + +// TrustPolicyValidator interface implementation +// These methods allow the IAMManager to serve as the trust policy validator for the STS service + +// ValidateTrustPolicyForWebIdentity implements the TrustPolicyValidator interface +func (m *IAMManager) ValidateTrustPolicyForWebIdentity(ctx context.Context, roleArn string, webIdentityToken string) error { + if !m.initialized { + return fmt.Errorf("IAM manager not initialized") + } + + // Extract role name from ARN + roleName := extractRoleNameFromArn(roleArn) + + // Get role definition + roleDef, err := m.roleStore.GetRole(ctx, "", roleName) + if err != nil { + return fmt.Errorf("role not found: %s", roleName) + } + + // Use existing trust policy validation logic + return m.validateTrustPolicyForWebIdentity(ctx, roleDef, webIdentityToken) +} + +// ValidateTrustPolicyForCredentials implements the TrustPolicyValidator interface +func (m *IAMManager) ValidateTrustPolicyForCredentials(ctx context.Context, roleArn string, identity *providers.ExternalIdentity) error { + if !m.initialized { + return fmt.Errorf("IAM manager not initialized") + } + + // Extract role name from ARN + roleName := extractRoleNameFromArn(roleArn) + + // Get role definition + roleDef, err := m.roleStore.GetRole(ctx, "", roleName) + if err != nil { + return fmt.Errorf("role not found: %s", roleName) + } + + // For credentials, we need to create a mock request to reuse existing validation + // This is a bit of a hack, but it allows us to reuse the existing logic + mockRequest := &sts.AssumeRoleWithCredentialsRequest{ + ProviderName: identity.Provider, // Use the provider name from the identity + } + + // Use existing trust policy validation logic + return m.validateTrustPolicyForCredentials(ctx, roleDef, mockRequest) +} diff --git a/weed/iam/sts/sts_service.go b/weed/iam/sts/sts_service.go index 94e1e4478..4808f0be0 100644 --- a/weed/iam/sts/sts_service.go +++ b/weed/iam/sts/sts_service.go @@ -10,16 +10,26 @@ import ( "github.com/seaweedfs/seaweedfs/weed/iam/providers" ) +// TrustPolicyValidator interface for validating trust policies during role assumption +type TrustPolicyValidator interface { + // ValidateTrustPolicyForWebIdentity validates if a web identity token can assume a role + ValidateTrustPolicyForWebIdentity(ctx context.Context, roleArn string, webIdentityToken string) error + + // ValidateTrustPolicyForCredentials validates if credentials can assume a role + ValidateTrustPolicyForCredentials(ctx context.Context, roleArn string, identity *providers.ExternalIdentity) error +} + // STSService provides Security Token Service functionality // This service is now completely stateless - all session information is embedded // in JWT tokens, eliminating the need for session storage and enabling true // distributed operation without shared state type STSService struct { - Config *STSConfig // Public for access by other components - initialized bool - providers map[string]providers.IdentityProvider - issuerToProvider map[string]providers.IdentityProvider // Efficient issuer-based provider lookup - tokenGenerator *TokenGenerator + Config *STSConfig // Public for access by other components + initialized bool + providers map[string]providers.IdentityProvider + issuerToProvider map[string]providers.IdentityProvider // Efficient issuer-based provider lookup + tokenGenerator *TokenGenerator + trustPolicyValidator TrustPolicyValidator // Interface for trust policy validation } // STSConfig holds STS service configuration @@ -315,6 +325,11 @@ func (s *STSService) GetProviders() map[string]providers.IdentityProvider { return s.providers } +// SetTrustPolicyValidator sets the trust policy validator for role assumption validation +func (s *STSService) SetTrustPolicyValidator(validator TrustPolicyValidator) { + s.trustPolicyValidator = validator +} + // AssumeRoleWithWebIdentity assumes a role using a web identity token (OIDC) // This method is now completely stateless - all session information is embedded in the JWT token func (s *STSService) AssumeRoleWithWebIdentity(ctx context.Context, request *AssumeRoleWithWebIdentityRequest) (*AssumeRoleResponse, error) { @@ -337,8 +352,8 @@ func (s *STSService) AssumeRoleWithWebIdentity(ctx context.Context, request *Ass return nil, fmt.Errorf("failed to validate web identity token: %w", err) } - // 2. Check if the role exists and can be assumed - if err := s.validateRoleAssumption(request.RoleArn, externalIdentity); err != nil { + // 2. Check if the role exists and can be assumed (includes trust policy validation) + if err := s.validateRoleAssumptionForWebIdentity(ctx, request.RoleArn, request.WebIdentityToken); err != nil { return nil, fmt.Errorf("role assumption denied: %w", err) } @@ -416,8 +431,8 @@ func (s *STSService) AssumeRoleWithCredentials(ctx context.Context, request *Ass return nil, fmt.Errorf("failed to authenticate credentials: %w", err) } - // 3. Check if the role exists and can be assumed - if err := s.validateRoleAssumption(request.RoleArn, externalIdentity); err != nil { + // 3. Check if the role exists and can be assumed (includes trust policy validation) + if err := s.validateRoleAssumptionForCredentials(ctx, request.RoleArn, externalIdentity); err != nil { return nil, fmt.Errorf("role assumption denied: %w", err) } @@ -583,14 +598,46 @@ func (s *STSService) extractIssuerFromJWT(token string) (string, error) { return issuer, nil } -// validateRoleAssumption checks if the role can be assumed by the external identity -func (s *STSService) validateRoleAssumption(roleArn string, identity *providers.ExternalIdentity) error { - // For now, we'll do basic validation - // In a full implementation, this would check: - // 1. Role exists - // 2. Role trust policy allows assumption by this identity - // 3. Identity has permission to assume the role +// validateRoleAssumptionForWebIdentity validates role assumption for web identity tokens +// This method performs complete trust policy validation to prevent unauthorized role assumptions +func (s *STSService) validateRoleAssumptionForWebIdentity(ctx context.Context, roleArn string, webIdentityToken string) error { + if roleArn == "" { + return fmt.Errorf("role ARN cannot be empty") + } + if webIdentityToken == "" { + return fmt.Errorf("web identity token cannot be empty") + } + + // Basic role ARN format validation + expectedPrefix := "arn:seaweed:iam::role/" + if len(roleArn) < len(expectedPrefix) || roleArn[:len(expectedPrefix)] != expectedPrefix { + return fmt.Errorf("invalid role ARN format: got %s, expected format: %s*", roleArn, expectedPrefix) + } + + // For testing, reject non-existent roles + roleName := extractRoleNameFromArn(roleArn) + if roleName == "NonExistentRole" { + return fmt.Errorf("role does not exist: %s", roleName) + } + + // CRITICAL SECURITY: Perform trust policy validation + if s.trustPolicyValidator != nil { + if err := s.trustPolicyValidator.ValidateTrustPolicyForWebIdentity(ctx, roleArn, webIdentityToken); err != nil { + return fmt.Errorf("trust policy validation failed: %w", err) + } + } else { + // If no trust policy validator is configured, fail closed for security + glog.Errorf("SECURITY WARNING: No trust policy validator configured - denying role assumption for security") + return fmt.Errorf("trust policy validation not available - role assumption denied for security") + } + + return nil +} + +// validateRoleAssumptionForCredentials validates role assumption for credential-based authentication +// This method performs complete trust policy validation to prevent unauthorized role assumptions +func (s *STSService) validateRoleAssumptionForCredentials(ctx context.Context, roleArn string, identity *providers.ExternalIdentity) error { if roleArn == "" { return fmt.Errorf("role ARN cannot be empty") } @@ -611,6 +658,17 @@ func (s *STSService) validateRoleAssumption(roleArn string, identity *providers. return fmt.Errorf("role does not exist: %s", roleName) } + // CRITICAL SECURITY: Perform trust policy validation + if s.trustPolicyValidator != nil { + if err := s.trustPolicyValidator.ValidateTrustPolicyForCredentials(ctx, roleArn, identity); err != nil { + return fmt.Errorf("trust policy validation failed: %w", err) + } + } else { + // If no trust policy validator is configured, fail closed for security + glog.Errorf("SECURITY WARNING: No trust policy validator configured - denying role assumption for security") + return fmt.Errorf("trust policy validation not available - role assumption denied for security") + } + return nil }