mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-09-20 15:04:37 +00:00
fix(sts): authorize AssumeRole by the role's trust policy (#10097)
* fix(sts): authorize AssumeRole by the role's trust policy The role's trust policy already declares who may assume it, but the caller also had to pass an identity-side sts:AssumeRole check that only the Admin action could satisfy — legacy static identities have no way to express sts:AssumeRole on a role. So assuming any role required a full admin identity. Drop the redundant check and let the trust policy be the authority; scope it to specific principals to restrict who can assume. * sts: resolve caller principal ARN for the trust-policy check A legacy static identity can reach AssumeRole without a PrincipalArn set; passing the empty value would miss a trust policy that names a concrete principal. Resolve it to the canonical user ARN, sharing the logic GetCallerIdentity already used inline. * sts: enforce explicit identity-side deny for AssumeRole Authorizing a named role by its trust policy alone dropped identity-side evaluation entirely, so a caller whose attached policy explicitly denies sts:AssumeRole could still assume any role the trust policy admits. Re-check the caller's policies through the IAM manager for an explicit deny (deny-always-wins) without requiring an allow; the trust policy stays the allow authority.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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 {
|
||||
|
||||
+22
-25
@@ -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)
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
Reference in New Issue
Block a user