diff --git a/weed/iam/integration/iam_manager.go b/weed/iam/integration/iam_manager.go index e19eec2c3..ed73df5b9 100644 --- a/weed/iam/integration/iam_manager.go +++ b/weed/iam/integration/iam_manager.go @@ -27,17 +27,17 @@ const maxPoliciesForEvaluation = 1024 // IAMManager orchestrates all IAM components type IAMManager struct { - stsService *sts.STSService - policyEngine *policy.PolicyEngine - roleStore RoleStore - userStore UserStore - oidcProviderStore OIDCProviderStore - oidcAuditSink OIDCProviderAuditSink - revocationStore SessionRevocationStore - filerAddressProvider func() string // Function to get current filer address - initialized bool - runtimePolicyMu sync.Mutex - runtimePolicyNames map[string]struct{} + stsService *sts.STSService + policyEngine *policy.PolicyEngine + roleStore RoleStore + userStore UserStore + oidcProviderStore OIDCProviderStore + oidcAuditSink OIDCProviderAuditSink + revocationStore SessionRevocationStore + filerAddressProvider func() string // Function to get current filer address + initialized bool + runtimePolicyMu sync.Mutex + runtimePolicyNames map[string]struct{} } // SetOIDCProviderAuditSink configures the lifecycle event sink. When nil @@ -1206,6 +1206,44 @@ func (m *IAMManager) IsActionAllowed(ctx context.Context, request *ActionRequest return true, nil } +// IsPrincipalActionExplicitlyDenied reports whether any of the named policies +// contains a statement that explicitly denies the action on the resource. Unlike +// IsActionAllowed it does not require an allow — the absence of a matching +// statement is not a denial. Used to enforce AWS deny-always-wins when the allow +// is granted elsewhere (e.g. a role trust policy for sts:AssumeRole). +func (m *IAMManager) IsPrincipalActionExplicitlyDenied(ctx context.Context, principal, action, resource string, policyNames []string, requestContext map[string]interface{}) (bool, error) { + if !m.initialized { + return false, fmt.Errorf("IAM manager not initialized") + } + if len(policyNames) == 0 { + return false, nil + } + + if requestContext == nil { + requestContext = make(map[string]interface{}) + } + requestContext["principal"] = principal + requestContext["aws:PrincipalArn"] = principal + + evalCtx := &policy.EvaluationContext{ + Principal: principal, + Action: action, + Resource: resource, + RequestContext: requestContext, + } + + result, err := m.policyEngine.Evaluate(ctx, "", evalCtx, policyNames) + if err != nil { + return false, fmt.Errorf("policy evaluation failed: %w", err) + } + for _, stmt := range result.MatchingStatements { + if stmt.Effect == policy.EffectDeny { + return true, nil + } + } + return false, nil +} + // ValidateTrustPolicy validates if a principal can assume a role (for testing) func (m *IAMManager) ValidateTrustPolicy(ctx context.Context, roleArn, provider, userID string) bool { roleName := utils.ExtractRoleNameFromArn(roleArn) diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 044c4a6c4..252f11f97 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -2305,6 +2305,46 @@ func (iam *IdentityAccessManagement) evaluateIAMPolicies(r *http.Request, identi return explicitAllow } +// isActionExplicitlyDeniedByIAM reports whether the identity's attached IAM +// policies (or its groups') explicitly deny action on resource, evaluated by the +// advanced IAM manager — the same engine authorizeWithIAM uses, and where both +// static-config and runtime policies are kept in sync. Unlike VerifyActionPermission +// it does not require an allow, so it enforces AWS deny-always-wins where the allow +// comes from elsewhere (e.g. a role trust policy). Fails closed on evaluation error. +func (iam *IdentityAccessManagement) isActionExplicitlyDeniedByIAM(r *http.Request, identity *Identity, principal, action, resource string) bool { + if identity == nil || iam.iamIntegration == nil { + return false + } + provider, ok := iam.iamIntegration.(IAMManagerProvider) + if !ok { + return false + } + + policyNames := make([]string, len(identity.PolicyNames)) + copy(policyNames, identity.PolicyNames) + iam.m.RLock() + for _, gn := range iam.userGroups[identity.Name] { + if g, exists := iam.groups[gn]; exists && !g.Disabled { + policyNames = append(policyNames, g.PolicyNames...) + } + } + iam.m.RUnlock() + if len(policyNames) == 0 { + return false + } + + manager := provider.GetIAMManager() + if manager == nil { + return false + } + denied, err := manager.IsPrincipalActionExplicitlyDenied(r.Context(), principal, action, resource, policyNames, extractRequestContext(r)) + if err != nil { + glog.Warningf("AssumeRole explicit-deny check failed for %s, denying: %v", identity.Name, err) + return true + } + return denied +} + // 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 { diff --git a/weed/s3api/s3api_sts.go b/weed/s3api/s3api_sts.go index baee534c7..e94b02bcc 100644 --- a/weed/s3api/s3api_sts.go +++ b/weed/s3api/s3api_sts.go @@ -161,6 +161,16 @@ func (h *STSHandlers) getAccountID() string { return defaultAccountID } +// callerPrincipalArn resolves the identity's principal ARN, synthesizing the +// canonical user ARN when one was not set (e.g. legacy static identities) so +// trust policies that name a concrete principal still match. +func (h *STSHandlers) callerPrincipalArn(identity *Identity) string { + if identity.PrincipalArn != "" { + return identity.PrincipalArn + } + return fmt.Sprintf("arn:aws:iam::%s:user/%s", h.getAccountID(), identity.Name) +} + // assumeRoleWithWebIdentity dispatches the request through the IAMManager // wrapper when one is wired so its cross-account provider scope check and // per-role MaxSessionDuration clamp run for the public AWS-SDK path. Without @@ -378,35 +388,27 @@ func (h *STSHandlers) handleAssumeRole(w http.ResponseWriter, r *http.Request) { glog.V(2).Infof("AssumeRole: caller identity=%s, roleArn=%s, sessionName=%s", identity.Name, roleArn, roleSessionName) - // Check if the caller is authorized to assume the role (sts:AssumeRole permission) - // This validates that the caller has a policy allowing sts:AssumeRole on the target role - // Check authorizations + // A named role is authorized by its trust policy, which declares which + // principals may assume it, so no separate identity-side sts:AssumeRole allow + // is required. An explicit identity-side deny still wins (deny-always-wins). + // Without a RoleArn the caller assumes a session for itself. if roleArn != "" { - // Check if the caller is authorized to assume the role (sts:AssumeRole permission) - if authErr := h.iam.VerifyActionPermission(r, identity, Action(sts.ActionAssumeRole), "", roleArn); authErr != s3err.ErrNone { - glog.V(2).Infof("AssumeRole: caller %s is not authorized to assume role %s", identity.Name, roleArn) + callerArn := h.callerPrincipalArn(identity) + if err := h.iam.ValidateTrustPolicyForPrincipal(r.Context(), roleArn, callerArn); err != nil { + glog.V(2).Infof("AssumeRole: %s not authorized to assume %s: %v", identity.Name, roleArn, err) h.writeSTSErrorResponse(w, r, STSErrAccessDenied, fmt.Errorf("user %s is not authorized to assume role %s", identity.Name, roleArn)) return } - - // Validate that the target role trusts the caller (Trust Policy) - if err := h.iam.ValidateTrustPolicyForPrincipal(r.Context(), roleArn, identity.PrincipalArn); err != nil { - glog.V(2).Infof("AssumeRole: trust policy validation failed for %s to assume %s: %v", identity.Name, roleArn, err) - h.writeSTSErrorResponse(w, r, STSErrAccessDenied, fmt.Errorf("trust policy denies access")) + if h.iam.isActionExplicitlyDeniedByIAM(r, identity, callerArn, sts.ActionAssumeRole, roleArn) { + glog.V(2).Infof("AssumeRole: identity policy explicitly denies %s assuming %s", identity.Name, roleArn) + h.writeSTSErrorResponse(w, r, STSErrAccessDenied, + fmt.Errorf("user %s is not authorized to assume role %s", identity.Name, roleArn)) return } } else { - // If RoleArn is missing, default to the caller's identity (User Context) - // This allows the user to "assume" a session for themselves, inheriting their own permissions. roleArn = identity.PrincipalArn glog.V(2).Infof("AssumeRole: no RoleArn provided, defaulting to caller identity: %s", roleArn) - - // We still enforce a global "sts:AssumeRole" check, similar to how we'd check if they can assume *any* role. - // However, for self-assumption, this might be implicit. - // For safety/consistency with previous logic, we keep the check but strictly it might not be required by AWS for GetSessionToken. - // But since this IS AssumeRole, let's keep it. - // Admin/Global check when no specific role is requested if authErr := h.iam.VerifyActionPermission(r, identity, Action(sts.ActionAssumeRole), "", ""); authErr != s3err.ErrNone { glog.Warningf("AssumeRole: caller %s attempted to assume role without RoleArn and lacks global sts:AssumeRole permission", identity.Name) h.writeSTSErrorResponse(w, r, STSErrAccessDenied, fmt.Errorf("access denied")) @@ -938,12 +940,7 @@ func (h *STSHandlers) handleGetCallerIdentity(w http.ResponseWriter, r *http.Req } accountID := h.getAccountID() - - arn := identity.PrincipalArn - if arn == "" { - arn = fmt.Sprintf("arn:aws:iam::%s:user/%s", accountID, identity.Name) - } - + arn := h.callerPrincipalArn(identity) userId := identity.Name glog.V(2).Infof("GetCallerIdentity: identity=%s, arn=%s, account=%s", identity.Name, arn, accountID) diff --git a/weed/s3api/s3api_sts_assume_role_authz_test.go b/weed/s3api/s3api_sts_assume_role_authz_test.go new file mode 100644 index 000000000..d83315653 --- /dev/null +++ b/weed/s3api/s3api_sts_assume_role_authz_test.go @@ -0,0 +1,164 @@ +package s3api + +import ( + "context" + "encoding/xml" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/iam/integration" + "github.com/seaweedfs/seaweedfs/weed/iam/policy" + "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// A role's trust policy is the authority on who may assume it, so a non-admin +// caller can assume a role its trust policy admits without holding the Admin +// action or an identity-side sts:AssumeRole grant (which legacy static +// identities cannot express). +func TestAssumeRole_NonAdminCallerAuthorizedByTrustPolicy(t *testing.T) { + ctx := context.Background() + manager := newTestSTSIntegrationManager(t) + + require.NoError(t, manager.CreatePolicy(ctx, "", "WarehouseAccess", &policy.PolicyDocument{ + Version: "2012-10-17", + Statement: []policy.Statement{{ + Effect: "Allow", + Action: []string{"s3:*"}, + Resource: []string{"arn:aws:s3:::*", "arn:aws:s3:::*/*"}, + }}, + })) + require.NoError(t, manager.CreatePolicy(ctx, "", "DenyAssumeRole", &policy.PolicyDocument{ + Version: "2012-10-17", + Statement: []policy.Statement{{ + Effect: "Deny", + Action: []string{"sts:AssumeRole"}, + Resource: []string{"*"}, + }}, + })) + + const accessKey, secretKey = "lakekeeperkey", "lakekeepersecret" + const denyAccessKey, denySecretKey = "deniedkey", "deniedsecret" + iam := &IdentityAccessManagement{iamIntegration: NewS3IAMIntegration(manager, "")} + require.NoError(t, iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{ + Identities: []*iam_pb.Identity{ + { + Name: "lakekeeper", + Credentials: []*iam_pb.Credential{{AccessKey: accessKey, SecretKey: secretKey}}, + Actions: []string{"Read", "Write", "List", "Tagging"}, + }, + { + Name: "lakekeeper-denied", + Credentials: []*iam_pb.Credential{{AccessKey: denyAccessKey, SecretKey: denySecretKey}}, + Actions: []string{"Read", "Write", "List", "Tagging"}, + PolicyNames: []string{"DenyAssumeRole"}, + }, + }, + })) + stsHandlers := NewSTSHandlers(manager.GetSTSService(), iam) + + assume := func(t *testing.T, ak, sk, roleName string) *httptest.ResponseRecorder { + t.Helper() + body := url.Values{ + "Action": {"AssumeRole"}, + "Version": {"2011-06-15"}, + "RoleArn": {"arn:aws:iam::" + defaultAccountID + ":role/" + roleName}, + "RoleSessionName": {"lakekeeper-session"}, + }.Encode() + req, err := newTestRequest(http.MethodPost, "http://sts.seaweedfs.test/", int64(len(body)), strings.NewReader(body)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + require.NoError(t, signRequestV4(req, ak, sk)) + rec := httptest.NewRecorder() + stsHandlers.handleAssumeRole(rec, req) + return rec + } + + t.Run("trust policy admits the caller", func(t *testing.T) { + require.NoError(t, manager.CreateRole(ctx, "", "OpenWarehouse", &integration.RoleDefinition{ + RoleName: "OpenWarehouse", + TrustPolicy: &policy.PolicyDocument{ + Version: "2012-10-17", + Statement: []policy.Statement{{Effect: "Allow", Principal: "*", Action: []string{"sts:AssumeRole"}}}, + }, + AttachedPolicies: []string{"WarehouseAccess"}, + })) + + rec := assume(t, accessKey, secretKey, "OpenWarehouse") + require.Equal(t, http.StatusOK, rec.Code, "non-admin caller should assume a role its trust policy admits: %s", rec.Body.String()) + + var resp AssumeRoleResponse + require.NoError(t, xml.Unmarshal(rec.Body.Bytes(), &resp)) + require.NotEmpty(t, resp.Result.Credentials.SessionToken) + + session, err := manager.GetSTSService().ValidateSessionToken(ctx, resp.Result.Credentials.SessionToken) + require.NoError(t, err) + assert.Equal(t, []string{"WarehouseAccess"}, session.Policies, "session is scoped to the role, not the caller") + }) + + t.Run("trust policy admits a specific principal", func(t *testing.T) { + require.NoError(t, manager.CreateRole(ctx, "", "NamedWarehouse", &integration.RoleDefinition{ + RoleName: "NamedWarehouse", + TrustPolicy: &policy.PolicyDocument{ + Version: "2012-10-17", + Statement: []policy.Statement{{ + Effect: "Allow", + Principal: map[string]interface{}{"AWS": "arn:aws:iam::" + defaultAccountID + ":user/lakekeeper"}, + Action: []string{"sts:AssumeRole"}, + }}, + }, + AttachedPolicies: []string{"WarehouseAccess"}, + })) + + rec := assume(t, accessKey, secretKey, "NamedWarehouse") + require.Equal(t, http.StatusOK, rec.Code, "caller named by the trust policy should be admitted: %s", rec.Body.String()) + }) + + t.Run("trust policy rejects the caller", func(t *testing.T) { + require.NoError(t, manager.CreateRole(ctx, "", "PrivateWarehouse", &integration.RoleDefinition{ + RoleName: "PrivateWarehouse", + TrustPolicy: &policy.PolicyDocument{ + Version: "2012-10-17", + Statement: []policy.Statement{{ + Effect: "Allow", + Principal: map[string]interface{}{"AWS": "arn:aws:iam::" + defaultAccountID + ":user/someone-else"}, + Action: []string{"sts:AssumeRole"}, + }}, + }, + AttachedPolicies: []string{"WarehouseAccess"}, + })) + + rec := assume(t, accessKey, secretKey, "PrivateWarehouse") + assert.Equal(t, http.StatusForbidden, rec.Code, "caller not named by the trust policy must be denied") + }) + + t.Run("identity policy explicit deny wins over trust policy", func(t *testing.T) { + require.NoError(t, manager.CreateRole(ctx, "", "DenyTestWarehouse", &integration.RoleDefinition{ + RoleName: "DenyTestWarehouse", + TrustPolicy: &policy.PolicyDocument{ + Version: "2012-10-17", + Statement: []policy.Statement{{Effect: "Allow", Principal: "*", Action: []string{"sts:AssumeRole"}}}, + }, + AttachedPolicies: []string{"WarehouseAccess"}, + })) + + // Caller is admitted by the trust policy but has an attached identity + // policy that explicitly denies sts:AssumeRole; the deny must win. + rec := assume(t, denyAccessKey, denySecretKey, "DenyTestWarehouse") + assert.Equal(t, http.StatusForbidden, rec.Code, "explicit identity-side deny must block AssumeRole even when the trust policy admits the caller") + }) +} + +func TestCallerPrincipalArn(t *testing.T) { + h := &STSHandlers{} + assert.Equal(t, "arn:aws:iam::"+defaultAccountID+":user/lakekeeper", + h.callerPrincipalArn(&Identity{Name: "lakekeeper"}), + "synthesizes the canonical user ARN when one is not set") + assert.Equal(t, "arn:aws:sts::111122223333:assumed-role/Warehouse/sess", + h.callerPrincipalArn(&Identity{Name: "lakekeeper", PrincipalArn: "arn:aws:sts::111122223333:assumed-role/Warehouse/sess"}), + "keeps an explicit principal ARN") +}