diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 6cc0af40c..5581ffed2 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -55,7 +55,7 @@ type IdentityAccessManagement struct { grpcDialOption grpc.DialOption // IAM Integration for advanced features - iamIntegration *S3IAMIntegration + iamIntegration IAMIntegration // Bucket policy engine for evaluating bucket policies policyEngine *BucketPolicyEngine @@ -702,18 +702,9 @@ func (iam *IdentityAccessManagement) authRequestWithAuthType(r *http.Request, ac // Only check IAM if bucket policy didn't explicitly allow if !policyAllows { - // Traditional identities (with Actions from -s3.config) use legacy auth, - // JWT/STS identities (no Actions) use IAM authorization - if len(identity.Actions) > 0 { - if !identity.canDo(action, bucket, object) { - return identity, s3err.ErrAccessDenied, reqAuthType - } - } else if iam.iamIntegration != nil { - if errCode := iam.authorizeWithIAM(r, identity, action, bucket, object); errCode != s3err.ErrNone { - return identity, errCode, reqAuthType - } - } else { - return identity, s3err.ErrAccessDenied, reqAuthType + // Use centralized permission check + if errCode := iam.VerifyActionPermission(r, identity, action, bucket, object); errCode != s3err.ErrNone { + return identity, errCode, reqAuthType } } } @@ -990,6 +981,23 @@ func determineIAMAuthPath(sessionToken, principal, principalArn string) iamAuthP return iamAuthPathNone } +// VerifyActionPermission checks if the identity is allowed to perform the action on the resource. +// It handles both traditional identities (via Actions) and IAM/STS identities (via Policy). +func (iam *IdentityAccessManagement) VerifyActionPermission(r *http.Request, identity *Identity, action Action, bucket, object string) s3err.ErrorCode { + // Traditional identities (with Actions from -s3.config) use legacy auth, + // JWT/STS identities (no Actions) use IAM authorization + if len(identity.Actions) > 0 { + if !identity.canDo(action, bucket, object) { + return s3err.ErrAccessDenied + } + return s3err.ErrNone + } else if iam.iamIntegration != nil { + return iam.authorizeWithIAM(r, identity, action, bucket, object) + } + + return s3err.ErrAccessDenied +} + // authorizeWithIAM authorizes requests using the IAM integration policy engine func (iam *IdentityAccessManagement) authorizeWithIAM(r *http.Request, identity *Identity, action Action, bucket string, object string) s3err.ErrorCode { ctx := r.Context() diff --git a/weed/s3api/auth_signature_v4.go b/weed/s3api/auth_signature_v4.go index b4696bc17..c8127c638 100644 --- a/weed/s3api/auth_signature_v4.go +++ b/weed/s3api/auth_signature_v4.go @@ -248,8 +248,10 @@ func (iam *IdentityAccessManagement) verifyV4Signature(r *http.Request, shouldCh if r.Method != http.MethodGet && r.Method != http.MethodHead { action = s3_constants.ACTION_WRITE } - if !identity.canDo(Action(action), bucket, object) { - return nil, nil, "", nil, s3err.ErrAccessDenied + + // Use centralized permission check + if errCode = iam.VerifyActionPermission(r, identity, Action(action), bucket, object); errCode != s3err.ErrNone { + return nil, nil, "", nil, errCode } } @@ -301,15 +303,15 @@ func (iam *IdentityAccessManagement) verifyV4Signature(r *http.Request, shouldCh // validateSTSSessionToken validates an STS session token and extracts temporary credentials func (iam *IdentityAccessManagement) validateSTSSessionToken(r *http.Request, sessionToken string, accessKey string) (*Identity, *Credential, s3err.ErrorCode) { - // Check if IAM integration with STS is available - if iam.iamIntegration == nil || iam.iamIntegration.stsService == nil { - glog.V(2).Infof("STS service not available, cannot validate session token") + // Check if IAM integration is available + if iam.iamIntegration == nil { + glog.V(2).Infof("IAM integration not available, cannot validate session token") return nil, nil, s3err.ErrInvalidAccessKeyID } // Validate the session token with the STS service ctx := r.Context() - sessionInfo, err := iam.iamIntegration.stsService.ValidateSessionToken(ctx, sessionToken) + sessionInfo, err := iam.iamIntegration.ValidateSessionToken(ctx, sessionToken) if err != nil { glog.V(2).Infof("Failed to validate STS session token: %v", err) return nil, nil, s3err.ErrInvalidAccessKeyID diff --git a/weed/s3api/auth_signature_v4_sts_test.go b/weed/s3api/auth_signature_v4_sts_test.go new file mode 100644 index 000000000..1d9fd5d41 --- /dev/null +++ b/weed/s3api/auth_signature_v4_sts_test.go @@ -0,0 +1,265 @@ +package s3api + +import ( + "context" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/seaweedfs/seaweedfs/weed/iam/sts" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" +) + +// MockIAMIntegration is a mock implementation of IAM integration for testing +type MockIAMIntegration struct { + authorizeFunc func(ctx context.Context, identity *IAMIdentity, action Action, bucket, object string, r *http.Request) s3err.ErrorCode +} + +func (m *MockIAMIntegration) AuthorizeAction(ctx context.Context, identity *IAMIdentity, action Action, bucket, object string, r *http.Request) s3err.ErrorCode { + if m.authorizeFunc != nil { + return m.authorizeFunc(ctx, identity, action, bucket, object, r) + } + return s3err.ErrNone +} + +func (m *MockIAMIntegration) AuthenticateJWT(ctx context.Context, r *http.Request) (*IAMIdentity, s3err.ErrorCode) { + return nil, s3err.ErrNotImplemented +} + +func (m *MockIAMIntegration) ValidateSessionToken(ctx context.Context, token string) (*sts.SessionInfo, error) { + return nil, nil // Not needed for these tests +} + +// TestVerifyV4SignatureWithSTSIdentity tests that verifyV4Signature properly handles STS identities +// by falling back to IAM authorization when shouldCheckPermissions is true +func TestVerifyV4SignatureWithSTSIdentity(t *testing.T) { + tests := []struct { + name string + identity *Identity + shouldCheckPermissions bool + iamIntegration *MockIAMIntegration + expectedError s3err.ErrorCode + description string + }{ + { + name: "STS identity with IAM integration - should authorize via IAM", + identity: &Identity{ + Name: "arn:aws:sts::assumed-role/adminRole/s3-session", + Account: &AccountAdmin, + Actions: []Action{}, // Empty actions = STS identity + PrincipalArn: "arn:aws:sts::assumed-role/adminRole/s3-session", + }, + shouldCheckPermissions: true, + iamIntegration: &MockIAMIntegration{ + authorizeFunc: func(ctx context.Context, identity *IAMIdentity, action Action, bucket, object string, r *http.Request) s3err.ErrorCode { + // Simulate successful IAM authorization + return s3err.ErrNone + }, + }, + expectedError: s3err.ErrNone, + description: "STS identity should be authorized via IAM when Actions is empty", + }, + { + name: "STS identity with IAM integration - IAM denies", + identity: &Identity{ + Name: "arn:aws:sts::assumed-role/readOnlyRole/s3-session", + Account: &AccountAdmin, + Actions: []Action{}, // Empty actions = STS identity + PrincipalArn: "arn:aws:sts::assumed-role/readOnlyRole/s3-session", + }, + shouldCheckPermissions: true, + iamIntegration: &MockIAMIntegration{ + authorizeFunc: func(ctx context.Context, identity *IAMIdentity, action Action, bucket, object string, r *http.Request) s3err.ErrorCode { + // Simulate IAM denying access (e.g., read-only role trying to write) + if action == s3_constants.ACTION_WRITE { + return s3err.ErrAccessDenied + } + return s3err.ErrNone + }, + }, + expectedError: s3err.ErrAccessDenied, + description: "STS identity should be denied when IAM denies access", + }, + { + name: "STS identity without IAM integration - should deny", + identity: &Identity{ + Name: "arn:aws:sts::assumed-role/adminRole/s3-session", + Account: &AccountAdmin, + Actions: []Action{}, // Empty actions = STS identity + PrincipalArn: "arn:aws:sts::assumed-role/adminRole/s3-session", + }, + shouldCheckPermissions: true, + iamIntegration: nil, // No IAM integration + expectedError: s3err.ErrAccessDenied, + description: "STS identity should be denied when no IAM integration is available", + }, + { + name: "Traditional identity with Actions - should use canDo", + identity: &Identity{ + Name: "traditional-user", + Account: &AccountAdmin, + Actions: []Action{s3_constants.ACTION_WRITE}, // Has actions = traditional identity + }, + shouldCheckPermissions: true, + iamIntegration: nil, // IAM integration not needed for traditional identities + expectedError: s3err.ErrNone, + description: "Traditional identity with Actions should use canDo check", + }, + { + name: "Traditional identity with Actions - canDo denies", + identity: &Identity{ + Name: "read-only-user", + Account: &AccountAdmin, + Actions: []Action{s3_constants.ACTION_READ}, // Only has READ action + }, + shouldCheckPermissions: true, + iamIntegration: nil, + expectedError: s3err.ErrAccessDenied, + description: "Traditional identity should be denied when canDo fails (PUT requires WRITE)", + }, + { + name: "shouldCheckPermissions false - skip authorization", + identity: &Identity{ + Name: "arn:aws:sts::assumed-role/adminRole/s3-session", + Account: &AccountAdmin, + Actions: []Action{}, // Empty actions = STS identity + PrincipalArn: "arn:aws:sts::assumed-role/adminRole/s3-session", + }, + shouldCheckPermissions: false, // Skip permission check + iamIntegration: nil, + expectedError: s3err.ErrNone, + description: "When shouldCheckPermissions is false, authorization should be skipped", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a mock request for PUT operation (requires WRITE action) + req, err := http.NewRequest("PUT", "http://s3.amazonaws.com/test-bucket/test-object", nil) + require.NoError(t, err) + req.Header.Set("Host", "s3.amazonaws.com") + + // Mock the permission check logic from verifyV4Signature (now centralized in VerifyActionPermission) + var errCode s3err.ErrorCode + if tt.shouldCheckPermissions { + bucket, object := s3_constants.GetBucketAndObject(req) + action := s3_constants.ACTION_READ + if req.Method != http.MethodGet && req.Method != http.MethodHead { + action = s3_constants.ACTION_WRITE + } + + // Create minimal IAM instance with mock integration + var integration IAMIntegration + if tt.iamIntegration != nil { + integration = tt.iamIntegration + } + iam := &IdentityAccessManagement{ + iamIntegration: integration, + } + errCode = iam.VerifyActionPermission(req, tt.identity, Action(action), bucket, object) + } + + // Verify the result + assert.Equal(t, tt.expectedError, errCode, tt.description) + + // Additional verification for STS identities + if len(tt.identity.Actions) == 0 && tt.shouldCheckPermissions { + if tt.iamIntegration != nil { + // When IAM integration exists, it should have been called + // The result depends on what the mock returns + } else { + assert.Equal(t, s3err.ErrAccessDenied, errCode, "STS identity should be denied without IAM integration") + } + } + }) + } +} + +// TestVerifyV4SignatureSTSStreamingUpload tests the specific scenario from the bug report: +// STS identities in streaming/chunked uploads should be authorized via IAM +func TestVerifyV4SignatureSTSStreamingUpload(t *testing.T) { + // Create an STS identity (empty Actions) + stsIdentity := &Identity{ + Name: "arn:aws:sts::assumed-role/adminRole/s3-session", + Account: &AccountAdmin, + Actions: []Action{}, // Empty - this is an STS identity + PrincipalArn: "arn:aws:sts::assumed-role/adminRole/s3-session", + } + + // Track whether IAM authorization was called + iamAuthCalled := false + + // Create IAM integration mock that allows the action + iamMock := &MockIAMIntegration{ + authorizeFunc: func(ctx context.Context, identity *IAMIdentity, action Action, bucket, object string, r *http.Request) s3err.ErrorCode { + iamAuthCalled = true + // Verify we're checking the right identity + assert.Equal(t, "arn:aws:sts::assumed-role/adminRole/s3-session", identity.Name) + assert.Equal(t, s3_constants.ACTION_WRITE, string(action)) + assert.Equal(t, "test-bucket", bucket) + assert.Equal(t, "test-object", object) + return s3err.ErrNone + }, + } + + // Create a streaming upload request (PUT with streaming content) + req, err := http.NewRequest("PUT", "/test-bucket/test-object", nil) + require.NoError(t, err) + req.Host = "s3.amazonaws.com" + req.Header.Set("Host", "s3.amazonaws.com") + req.Header.Set("x-amz-content-sha256", "STREAMING-AWS4-HMAC-SHA256-PAYLOAD") + req.Header.Set("Content-Encoding", "aws-chunked") + req.Header.Set("X-Amz-Security-Token", "test-session-token") + + // Simulate the permission check with shouldCheckPermissions=true + // This is what happens in calculateSeedSignature -> verifyV4Signature(r, true) + bucket := "test-bucket" + object := "test-object" + action := s3_constants.ACTION_WRITE + + var errCode s3err.ErrorCode + + // Create minimal IAM instance logic + iam := &IdentityAccessManagement{ + iamIntegration: iamMock, + } + + errCode = iam.VerifyActionPermission(req, stsIdentity, Action(action), bucket, object) + + // Verify that the STS identity is authorized via IAM + assert.Equal(t, s3err.ErrNone, errCode, "STS identity should be authorized via IAM for streaming upload") + assert.True(t, iamAuthCalled, "IAM authorization should have been called for STS identity") +} + +// TestVerifyV4SignatureActionDetermination tests that the correct action is determined +// based on the HTTP method +func TestVerifyV4SignatureActionDetermination(t *testing.T) { + tests := []struct { + method string + expectedAction string + }{ + {http.MethodGet, s3_constants.ACTION_READ}, + {http.MethodHead, s3_constants.ACTION_READ}, + {http.MethodPut, s3_constants.ACTION_WRITE}, + {http.MethodPost, s3_constants.ACTION_WRITE}, + {http.MethodDelete, s3_constants.ACTION_WRITE}, + } + + for _, tt := range tests { + t.Run(tt.method, func(t *testing.T) { + req, err := http.NewRequest(tt.method, "http://s3.amazonaws.com/bucket/object", nil) + require.NoError(t, err) + + // Determine action the same way verifyV4Signature does + action := s3_constants.ACTION_READ + if req.Method != http.MethodGet && req.Method != http.MethodHead { + action = s3_constants.ACTION_WRITE + } + + assert.Equal(t, tt.expectedAction, action, "Action should match expected for method %s", tt.method) + }) + } +} diff --git a/weed/s3api/s3_iam_middleware.go b/weed/s3api/s3_iam_middleware.go index 70f74508b..3548b58a7 100644 --- a/weed/s3api/s3_iam_middleware.go +++ b/weed/s3api/s3_iam_middleware.go @@ -18,6 +18,13 @@ import ( "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" ) +// IAMIntegration defines the interface for IAM integration +type IAMIntegration interface { + AuthenticateJWT(ctx context.Context, r *http.Request) (*IAMIdentity, s3err.ErrorCode) + AuthorizeAction(ctx context.Context, identity *IAMIdentity, action Action, bucket string, objectKey string, r *http.Request) s3err.ErrorCode + ValidateSessionToken(ctx context.Context, token string) (*sts.SessionInfo, error) +} + // S3IAMIntegration provides IAM integration for S3 API type S3IAMIntegration struct { iamManager *integration.IAMManager @@ -167,6 +174,14 @@ func (s3iam *S3IAMIntegration) AuthenticateJWT(ctx context.Context, r *http.Requ return identity, s3err.ErrNone } +// ValidateSessionToken checks the validity of an STS session token +func (s3iam *S3IAMIntegration) ValidateSessionToken(ctx context.Context, token string) (*sts.SessionInfo, error) { + if s3iam.stsService == nil { + return nil, fmt.Errorf("STS service not available") + } + return s3iam.stsService.ValidateSessionToken(ctx, token) +} + // AuthorizeAction authorizes actions using our policy engine func (s3iam *S3IAMIntegration) AuthorizeAction(ctx context.Context, identity *IAMIdentity, action Action, bucket string, objectKey string, r *http.Request) s3err.ErrorCode { if !s3iam.enabled { @@ -463,7 +478,7 @@ func (s3a *S3ApiServer) SetIAMIntegration(iamManager *integration.IAMManager) { // EnhancedS3ApiServer extends S3ApiServer with IAM integration type EnhancedS3ApiServer struct { *S3ApiServer - iamIntegration *S3IAMIntegration + iamIntegration IAMIntegration } // NewEnhancedS3ApiServer creates an S3 API server with IAM integration