gsadmin/spinifex
1
0
Fork 0
mirror of https://github.com/mulgadc/spinifex.git synced 2026-10-15 09:01:31 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: gate sts:AssumeRole on the caller's identity policy (#889)

Browse Source
This commit is contained in:
Julian Sommer
2026-08-27 09:04:25 +10:00
committed by GitHub
parent 6fd9b17f62
commit 3bf0379daa
36 changed files with 997 additions and 342 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/identity/iam-roles-and-instance-profiles/README.md
+1 -1
View File
@@ -281,7 +281,7 @@ aws ec2 disassociate-iam-instance-profile \
## Assuming a Role
Users and services can assume a role directly with STS, provided the trust policy allows their principal:
Users and services can assume a role directly with STS, provided **both** gates pass: the role's trust policy admits their principal, and their own identity policy grants `sts:AssumeRole` on the role's ARN. See [STS](/docs/sts) for the identity-side grant.
```bash
aws sts assume-role \
docs/identity/sts/README.md
+26 -4
View File
@@ -42,7 +42,7 @@ STS (Security Token Service) issues **temporary credentials**: an `ASIA`-prefixe
Spinifex supports the three main ways to obtain them:
- **`assume-role`** — exchange your IAM user credentials for a [role's](/docs/iam-roles-and-instance-profiles) permissions, gated by the role's trust policy.
- **`assume-role`** — exchange your IAM user credentials for a [role's](/docs/iam-roles-and-instance-profiles) permissions, gated by both the role's trust policy and your own `sts:AssumeRole` grant.
- **`get-session-token`** — get a time-boxed copy of your own [IAM user's](/docs/iam-users-and-policies) permissions.
- **`assume-role-with-web-identity`** — exchange an OIDC ID token (a Kubernetes ServiceAccount token) for role credentials, with no IAM credentials at all.
@@ -79,7 +79,14 @@ Run with temporary role credentials, the ARN switches to the assumed-role form
## Assuming a Role
`assume-role` works when the target role's trust policy allows your principal. Create a role that trusts an IAM user (see [IAM Roles and Instance Profiles](/docs/iam-roles-and-instance-profiles) for the full role lifecycle):
`assume-role` needs two independent grants, and a missing one denies the call:
1. The target role's **trust policy** must admit your principal.
2. Your own **identity policy** must grant `sts:AssumeRole` on the role's ARN.
The second is why `"Principal": {"AWS": "arn:aws:iam::000000000001:root"}` in a trust policy is safe: it delegates the decision to identity policies in that account rather than opening the role to everyone in it.
Create a role that trusts an IAM user (see [IAM Roles and Instance Profiles](/docs/iam-roles-and-instance-profiles) for the full role lifecycle):
```bash
cat > /tmp/deploy-trust.json << 'EOF'
@@ -102,6 +109,15 @@ aws iam put-role-policy --role-name deploy --policy-name ec2-read \
--policy-document '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":["ec2:Describe*"],"Resource":"*"}]}'
```
That policy is what the *session* may do. The caller still needs its own grant to make the call at all:
```bash
aws iam put-user-policy --user-name admin --policy-name assume-deploy \
--policy-document '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"sts:AssumeRole","Resource":"arn:aws:iam::000000000001:role/deploy"}]}'
```
Scope `Resource` to the role ARN exactly as `create-role` returned it, path included — a wildcard such as `arn:aws:iam::000000000001:role/*` grants assumption of every role the account's trust policies admit you to.
Then assume it:
```bash
@@ -128,7 +144,7 @@ aws sts assume-role \
The session name tags every session so audit logs can tell *who* used the role; it appears in the assumed-role ARN and the `UserId`.
If the trust policy does not allow your principal, `assume-role` fails with `AccessDenied` — no matter what IAM permissions you hold.
If the trust policy does not allow your principal, `assume-role` fails with `AccessDenied` — no matter what IAM permissions you hold. It fails the same way when your own identity policy carries no `sts:AssumeRole` grant on the role, no matter what the trust policy says.
### Session Duration
@@ -303,7 +319,13 @@ Everything the validator accepts is evaluated at assume time; explicit `Deny` st
### AccessDenied When Assuming a Role
The role's trust policy does not allow your principal, or the role does not exist — a missing role is deliberately indistinguishable from a denied one, matching AWS. Check who you are, that the role exists, and what it trusts:
Either gate can produce this. A denial from your own identity policy names the principal, action and resource:
```
User: arn:aws:iam::000000000001:user/admin is not authorized to perform: sts:AssumeRole on resource: arn:aws:iam::000000000001:role/deploy
```
A bare `AccessDenied` means the role's trust policy does not allow your principal, or the role does not exist — a missing role is deliberately indistinguishable from a denied one, matching AWS. Check who you are, that the role exists, what it trusts, and what you are granted:
```bash
aws sts get-caller-identity
spinifex/gateway/admin.go
+18 -4
View File
@@ -101,7 +101,7 @@ func (gw *GatewayConfig) Admin_Request(w http.ResponseWriter, r *http.Request) {
}
if err := gw.authorizeAdmin(r, method); err != nil {
gw.writeAdminError(w, requestID, err.Error(), "")
gw.writeAdminErrorFor(w, requestID, err)
return
}
@@ -141,7 +141,7 @@ func (gw *GatewayConfig) Admin_Request(w http.ResponseWriter, r *http.Request) {
err = errors.New(awserrors.ErrorInvalidAction)
}
if err != nil {
gw.writeAdminError(w, requestID, err.Error(), "")
gw.writeAdminErrorFor(w, requestID, err)
return
}
@@ -199,9 +199,23 @@ func (gw *GatewayConfig) authorizeAdmin(r *http.Request, method string) error {
return gw.checkPolicy(r, "spinifex", method)
}
// writeAdminErrorFor resolves an error to its code and any message its call
// site attached, so a wrapped error renders as its own code instead of falling
// through to InternalError.
func (gw *GatewayConfig) writeAdminErrorFor(w http.ResponseWriter, requestID string, err error) {
code, message, ok := awserrors.ResolveErrorDetail(err)
if !ok {
// The cause reaches neither the wire nor the envelope's own unmapped-code
// log, so this is the only place it is recorded.
slog.Error("Admin: unresolvable error", "requestId", requestID, "err", err)
code = awserrors.ErrorInternalError
}
gw.writeAdminError(w, requestID, code, message)
}
// writeAdminError writes the JSON error envelope. message overrides the
// registry text when a specific cause is safe to disclose; it must never carry
// a parameter value, only a parameter name.
// registry text when a specific cause is safe to disclose; a policy denial
// names principal, action and resource, all of them caller-derivable.
func (gw *GatewayConfig) writeAdminError(w http.ResponseWriter, requestID, code, message string) {
detail, ok := awserrors.ErrorLookup[code]
if !ok {
spinifex/gateway/admin_deleteaccount_job_test.go
+1 -1
View File
@@ -361,7 +361,7 @@ func TestAdminDeleteAccountRefusesProtectedAccounts(t *testing.T) {
deleteAccountBody(t, accountID, "super-admin@example.com", deleteClientToken))
require.Error(t, err)
assert.Equal(t, awserrors.ErrorAccessDenied, err.Error())
assertDenied(t, err)
})
}
}
spinifex/gateway/admin_test.go
+3 -2
View File
@@ -145,7 +145,8 @@ func TestAuthorizeAdminGates(t *testing.T) {
return
}
require.Error(t, err)
assert.Equal(t, tc.wantErr, err.Error())
code, _ := awserrors.ResolveErrorCode(err)
assert.Equal(t, tc.wantErr, code)
})
}
}
@@ -158,7 +159,7 @@ func TestAuthorizeAdminDeniesUnauthenticated(t *testing.T) {
err := gw.authorizeAdmin(req, "CreateAccount")
require.Error(t, err)
assert.Equal(t, awserrors.ErrorAccessDenied, err.Error())
assertDenied(t, err)
}
// adminRequest drives Admin_Request with the chi route param the router sets.
spinifex/gateway/conditionkeys_test.go
+4 -2
View File
@@ -153,7 +153,8 @@ func TestCheckPolicyResources_ConditionalAllowHonoursSourceIP(t *testing.T) {
return
}
require.Error(t, err)
assert.Equal(t, tt.wantErr, err.Error())
code, _ := awserrors.ResolveErrorCode(err)
assert.Equal(t, tt.wantErr, code)
})
}
}
@@ -189,7 +190,8 @@ func TestCheckPolicyResources_ConditionalDenyHonoursSourceIP(t *testing.T) {
return
}
require.Error(t, err)
assert.Equal(t, tt.wantErr, err.Error())
code, _ := awserrors.ResolveErrorCode(err)
assert.Equal(t, tt.wantErr, code)
})
}
}
spinifex/gateway/ec2_authz_test.go
+15 -1
View File
@@ -50,10 +50,24 @@ func dispatchEC2(t *testing.T, gw *GatewayConfig, body string) error {
return gw.EC2_Request(httptest.NewRecorder(), setupEC2Request(body, authzAccountID))
}
// assertDenied matches on the resolved code, not the message: a policy denial
// names the principal, action and resource ahead of the code.
func assertDenied(t *testing.T, err error) {
t.Helper()
require.Error(t, err)
assert.Equal(t, awserrors.ErrorAccessDenied, err.Error())
code, ok := awserrors.ResolveErrorCode(err)
require.True(t, ok, "unresolvable error code: %v", err)
assert.Equal(t, awserrors.ErrorAccessDenied, code)
}
// assertNotDenied asserts the policy gate passed where the handler failure that
// follows is not fixed. It resolves the code for the same reason assertDenied
// does: the denial message is no longer the bare code.
func assertNotDenied(t *testing.T, err error) {
t.Helper()
require.Error(t, err)
code, _ := awserrors.ResolveErrorCode(err)
assert.NotEqual(t, awserrors.ErrorAccessDenied, code)
}
// assertPermitted asserts the policy gate passed. The request then fails on the
spinifex/gateway/ecr_authz_test.go
+1 -9
View File
@@ -82,15 +82,7 @@ func TestECRRequest_OmittedRepositoryListEnumeratesTheRegistry(t *testing.T) {
wide := scopedPolicyGateway(
statement("Allow", "ecr:DescribeRepositories", "arn:aws:ecr:*:*:repository/*"),
)
assert.NotEqual(t, awserrors.ErrorAccessDenied,
errString(dispatchECR(t, wide, "DescribeRepositories", `{}`)))
}
func errString(err error) string {
if err == nil {
return ""
}
return err.Error()
assertNotDenied(t, dispatchECR(t, wide, "DescribeRepositories", `{}`))
}
// A caller-supplied registryId would let a request slide out from under a Deny
spinifex/gateway/ecs_authz_test.go
+1 -1
View File
@@ -32,7 +32,7 @@ func dispatchECS(t *testing.T, gw *GatewayConfig, action, body string) error {
func assertReachedHandler(t *testing.T, err error) {
t.Helper()
require.Error(t, err)
assert.NotEqual(t, awserrors.ErrorAccessDenied, err.Error())
assertNotDenied(t, err)
}
func ecsResourceARN(resource string) string {
spinifex/gateway/ecs_passrole_test.go
+5 -7
View File
@@ -9,7 +9,6 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ecs"
"github.com/mulgadc/spinifex/spinifex/awserrors"
handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -69,7 +68,7 @@ func TestECSRegisterTaskDefinition_DeniedWithoutPassRole(t *testing.T) {
err := gw.ECS_Request(httptest.NewRecorder(), req)
require.Error(t, err)
assert.Equal(t, awserrors.ErrorAccessDenied, err.Error())
assertDenied(t, err)
}
func TestECSRegisterTaskDefinition_AllowedWithPassRole(t *testing.T) {
@@ -87,7 +86,7 @@ func TestECSRegisterTaskDefinition_AllowedWithPassRole(t *testing.T) {
// disconnected) NATS dispatch; whatever fails downstream, it must not be
// the PassRole denial.
require.Error(t, err)
assert.NotEqual(t, awserrors.ErrorAccessDenied, err.Error())
assertNotDenied(t, err)
}
func TestECSRegisterTaskDefinition_NoRoleSkipsPassRole(t *testing.T) {
@@ -104,12 +103,11 @@ func TestECSRegisterTaskDefinition_NoRoleSkipsPassRole(t *testing.T) {
err := gw.ECS_Request(httptest.NewRecorder(), req)
require.Error(t, err) // still fails downstream (nil NATS conn), just not on PassRole
assert.NotEqual(t, awserrors.ErrorAccessDenied, err.Error())
assertNotDenied(t, err)
}
// TestECSRegisterTaskDefinition_DenialRendersAccessDenied403 proves the denial
// reaches the wire as AWS's real PassRole-denial shape: HTTP 403, AWS JSON 1.1
// envelope, the same AccessDenied code/message the EC2 iam:PassRole path uses.
// TestECSRegisterTaskDefinition_DenialRendersAccessDenied403 proves shared
// policy denials stay opaque while retaining the service's 403 envelope.
func TestECSRegisterTaskDefinition_DenialRendersAccessDenied403(t *testing.T) {
mock := &policyMockIAMService{
getUserPoliciesFn: func(_, _ string) ([]handlers_iam.PolicyDocument, error) {
spinifex/gateway/gateway.go
+28 -1
View File
@@ -692,12 +692,39 @@ func (gw *GatewayConfig) evaluatePrincipalPolicyResources(
if iampolicy.EvaluateWithKeys(iamAction, resource, policies, keys) == iampolicy.Deny {
slog.Info("evaluatePrincipalPolicy: access denied",
"identity", logIdentity, "action", iamAction, "resource", resource)
return errors.New(awserrors.ErrorAccessDenied)
return &identityPolicyDenialError{
principal: principal, logIdentity: logIdentity,
action: iamAction, resource: resource,
}
}
}
return nil
}
// identityPolicyDenialError keeps shared authorization failures opaque. A
// caller that proves its resource is non-sensitive may opt into details.
type identityPolicyDenialError struct {
principal principalContext
logIdentity string
action string
resource string
}
func (d *identityPolicyDenialError) Error() string {
return awserrors.ErrorAccessDenied
}
func (d *identityPolicyDenialError) detailedError() error {
callerARN, err := buildCallerARN(d.principal.accountID, d.principal.identity,
d.principal.principalType, d.principal.assumedRoleARN)
if err != nil {
callerARN = d.logIdentity
}
return awserrors.Errorf(awserrors.ErrorAccessDenied,
"User: %s is not authorized to perform: %s on resource: %s",
callerARN, d.action, d.resource)
}
func (gw *GatewayConfig) ErrorHandler(w http.ResponseWriter, r *http.Request, err error) {
svc, _ := gw.GetService(r)
slog.Debug("ErrorHandler", "service", svc, "error", err.Error())
spinifex/gateway/gateway_test.go
+33 -6
View File
@@ -1115,9 +1115,36 @@ func TestCheckPolicy_FailsClosed(t *testing.T) {
}
}
// Shared authorization errors must not expose a canonical resource that was
// resolved from storage. STS opts into detailed wording at its dispatcher.
func TestCheckPolicyResources_DenialIsOpaque(t *testing.T) {
mock := &policyMockIAMService{
getUserPoliciesFn: func(_, _ string) ([]handlers_iam.PolicyDocument, error) {
return nil, nil
},
}
gw := &GatewayConfig{DisableLogging: true, IAMService: mock}
req := httptest.NewRequest(http.MethodPost, "/", nil)
ctx := context.WithValue(req.Context(), ctxIdentity, "alice")
ctx = context.WithValue(ctx, ctxAccountID, "123456789012")
ctx = context.WithValue(ctx, ctxPrincipalType, principalTypeUser)
req = req.WithContext(ctx)
resource := "arn:aws:iam::123456789012:role/private/target"
err := gw.checkPolicyResources(req, "iam", "GetRole", []string{resource})
require.Error(t, err)
assert.Equal(t, awserrors.ErrorAccessDenied, err.Error())
code, message, ok := awserrors.ResolveErrorDetail(err)
require.True(t, ok)
assert.Equal(t, awserrors.ErrorAccessDenied, code)
assert.Empty(t, message)
assert.NotContains(t, err.Error(), resource)
}
// TestCheckPolicy_PassRoleFailsClosed proves the change reaches a real gated
// call site, not just the helper: an unauthenticated RunInstances carrying an
// instance profile is denied rather than passing the iam:PassRole check.
// call site: an unauthenticated RunInstances carrying an instance profile is
// denied rather than passing the iam:PassRole check.
func TestCheckPolicy_PassRoleFailsClosed(t *testing.T) {
gw := &GatewayConfig{DisableLogging: true, IAMService: allowAllIAMService()}
req := httptest.NewRequest(http.MethodPost, "/", nil)
@@ -1125,7 +1152,7 @@ func TestCheckPolicy_PassRoleFailsClosed(t *testing.T) {
err := gw.checkPolicyResources(req, "iam", "PassRole",
[]string{"arn:aws:iam::123456789012:role/app"})
require.Error(t, err)
assert.Equal(t, awserrors.ErrorAccessDenied, err.Error())
assertDenied(t, err)
}
func TestCheckPolicy_RootUserGlobalAccount(t *testing.T) {
@@ -1154,7 +1181,7 @@ func TestCheckPolicy_AssumedRoleSessionNamedRoot(t *testing.T) {
err := gw.checkPolicy(req, "ec2", "DescribeInstances")
require.Error(t, err)
assert.Equal(t, awserrors.ErrorAccessDenied, err.Error())
assertDenied(t, err)
}
func TestCheckPolicy_NonRootAllowPolicy(t *testing.T) {
@@ -1203,7 +1230,7 @@ func TestCheckPolicy_NonRootDenyPolicy(t *testing.T) {
err := gw.checkPolicy(req, "ec2", "DescribeInstances")
require.Error(t, err)
assert.Equal(t, awserrors.ErrorAccessDenied, err.Error())
assertDenied(t, err)
}
func TestCheckPolicy_NonRootNoPolicies(t *testing.T) {
@@ -1221,7 +1248,7 @@ func TestCheckPolicy_NonRootNoPolicies(t *testing.T) {
err := gw.checkPolicy(req, "ec2", "DescribeInstances")
require.Error(t, err)
assert.Equal(t, awserrors.ErrorAccessDenied, err.Error())
assertDenied(t, err)
}
func TestCheckPolicy_GetUserPoliciesError(t *testing.T) {
spinifex/gateway/iam_authz_test.go
+3 -3
View File
@@ -78,7 +78,7 @@ func TestIAMRequest_ScopedDenyFires(t *testing.T) {
)
err := dispatchIAM(t, gw, "Action=DeleteRole&RoleName=prod")
require.EqualError(t, err, awserrors.ErrorAccessDenied)
assertDenied(t, err)
require.NoError(t, dispatchIAM(t, gw, "Action=DeleteRole&RoleName=dev"))
assert.Equal(t, []string{"dev"}, svc.deleted)
}
@@ -91,7 +91,7 @@ func TestIAMRequest_ScopedAllowGrants(t *testing.T) {
require.NoError(t, dispatchIAM(t, gw, "Action=DeleteRole&RoleName=dev"))
err := dispatchIAM(t, gw, "Action=DeleteRole&RoleName=prod")
require.EqualError(t, err, awserrors.ErrorAccessDenied)
assertDenied(t, err)
assert.Equal(t, []string{"dev"}, svc.deleted)
}
@@ -107,7 +107,7 @@ func TestIAMRequest_MissingTargetAndLookupFailure(t *testing.T) {
gw, _ := iamScopedGateway(nil,
statement("Allow", "iam:DeleteRole", "arn:aws:iam::*:role/missing"))
err := dispatchIAM(t, gw, "Action=DeleteRole&RoleName=missing")
require.EqualError(t, err, awserrors.ErrorAccessDenied)
assertDenied(t, err)
})
t.Run("storage failure fails closed", func(t *testing.T) {
spinifex/gateway/iam_request_test.go
+11
View File
@@ -36,6 +36,17 @@ type flexMockIAMService struct {
lookupAccessKeyFn func(string) (*handlers_iam.AccessKey, error)
getAccountSummaryFn func(string, *iam.GetAccountSummaryInput) (*iam.GetAccountSummaryOutput, error)
getRoleFn func(string, *iam.GetRoleInput) (*iam.GetRoleOutput, error)
}
// GetRole answers NoSuchEntity by default: the STS identity gate resolves the
// target role, and a mock that invented one would let a test pass against a
// role the account does not hold.
func (m *flexMockIAMService) GetRole(accountID string, input *iam.GetRoleInput) (*iam.GetRoleOutput, error) {
if m.getRoleFn != nil {
return m.getRoleFn(accountID, input)
}
return nil, errors.New(awserrors.ErrorIAMNoSuchEntity)
}
func (m *flexMockIAMService) CreateUser(accountID string, input *iam.CreateUserInput) (*iam.CreateUserOutput, error) {
spinifex/gateway/policy/evaluator_test.go
+1 -1
View File
@@ -69,7 +69,7 @@ func TestEvaluate_IAMInstanceProfileActionStrings(t *testing.T) {
}
// TestEvaluate_STSActionStrings pins the action strings emitted by the STS
// gateway dispatcher (gateway/sts.go stsActions + checkPolicy(r, "sts", action)
// gateway dispatcher (gateway/sts.go stsActions + the stsPolicyGatedActions
// call site). Locks in that every STS verb the dispatcher accepts is matchable
// by the evaluator under both wildcard and service-scoped policies, so a future
// rename of any stsActions key surfaces here.
spinifex/gateway/rds_test.go
+7 -7
View File
@@ -144,7 +144,7 @@ func TestRDSRequest_InternalActionsDeniedToCustomerAdmin(t *testing.T) {
err := gw.RDS_Request(httptest.NewRecorder(),
setupRDSUserRequest("Action="+action, rdsTestAccountID))
require.Error(t, err)
assert.Equal(t, awserrors.ErrorAccessDenied, err.Error())
assertDenied(t, err)
})
}
}
@@ -169,7 +169,7 @@ func TestRDSRequest_AgentRoleCannotCallCustomerActions(t *testing.T) {
err := gw.RDS_Request(httptest.NewRecorder(),
setupRDSAgentRequest("Action=DescribeDBInstances", "i-0abc123"))
require.Error(t, err)
assert.Equal(t, awserrors.ErrorAccessDenied, err.Error())
assertDenied(t, err)
}
// A policy scoped to db:prod-* has to actually restrict: checked against "*" it
@@ -189,7 +189,7 @@ func TestRDSRequest_ResourceScopedPolicyRestrictsByIdentifier(t *testing.T) {
err = gw.RDS_Request(httptest.NewRecorder(),
setupRDSUserRequest("Action=DeleteDBInstance&DBInstanceIdentifier=dev-orders", rdsTestAccountID))
require.Error(t, err)
assert.Equal(t, awserrors.ErrorAccessDenied, err.Error())
assertDenied(t, err)
}
func TestRDSRequest_SnapshotActionsAuthorizeSourceAndTarget(t *testing.T) {
@@ -254,7 +254,7 @@ func TestRDSRequest_SnapshotActionsUseOnePolicySnapshot(t *testing.T) {
rdsTestAccountID,
))
require.Error(t, err)
assert.Equal(t, awserrors.ErrorAccessDenied, err.Error())
assertDenied(t, err)
assert.Equal(t, 1, probe.consulted)
}
@@ -291,7 +291,7 @@ func TestRDSRequest_SnapshotActionsHonorTargetDeny(t *testing.T) {
err := gw.RDS_Request(httptest.NewRecorder(), setupRDSUserRequest(tt.query, rdsTestAccountID))
require.Error(t, err)
assert.Equal(t, 1, probe.consulted)
assert.Equal(t, awserrors.ErrorAccessDenied, err.Error())
assertDenied(t, err)
})
}
}
@@ -303,7 +303,7 @@ func TestRDSRequest_UnscopedActionsEvaluateAgainstAnyResource(t *testing.T) {
err := gw.RDS_Request(httptest.NewRecorder(),
setupRDSUserRequest("Action=CreateDBInstance&DBInstanceIdentifier=prod-orders", rdsTestAccountID))
require.Error(t, err)
assert.Equal(t, awserrors.ErrorAccessDenied, err.Error())
assertDenied(t, err)
}
// Cross-account references are refused at the ARN, not merely unauthorized:
@@ -331,7 +331,7 @@ func TestRDSRequest_NoIdentityDenied(t *testing.T) {
err := gw.RDS_Request(httptest.NewRecorder(), req.WithContext(ctx))
require.Error(t, err)
assert.Equal(t, awserrors.ErrorAccessDenied, err.Error())
assertDenied(t, err)
assert.Zero(t, probe.consulted, "a request with no identity must not be evaluated at all")
}
spinifex/gateway/sts.go
+59 -4
View File
@@ -5,11 +5,13 @@ import (
"log/slog"
"net/http"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/sts"
spxarn "github.com/mulgadc/spinifex/spinifex/arn"
"github.com/mulgadc/spinifex/spinifex/awsec2query"
"github.com/mulgadc/spinifex/spinifex/awserrors"
gateway_sts "github.com/mulgadc/spinifex/spinifex/gateway/sts"
handlers_sts "github.com/mulgadc/spinifex/spinifex/handlers/sts"
"github.com/mulgadc/spinifex/spinifex/utils"
)
@@ -76,6 +78,47 @@ var anonymousSTSActions = map[string]bool{
"AssumeRoleWithWebIdentity": true,
}
// stsPolicyGatedActions lists the actions requiring a pass on the caller's
// identity policy. GetCallerIdentity and GetSessionToken are authentication
// operations AWS requires no permission for, and AssumeRoleWithWebIdentity
// arrives with no identity to evaluate.
var stsPolicyGatedActions = map[string]bool{
"AssumeRole": true,
}
// stsPolicyResource returns the ARN a gated action is evaluated against.
// AssumeRole resolves the target role so the resource is the ARN IAM stored,
// which a policy naming a pathed role matches and an invented path does not.
func (gw *GatewayConfig) stsPolicyResource(queryArgs map[string]string) (string, error) {
// An absent RoleArn is a malformed request, not an unauthorized one: saying
// so beats sending the caller to widen a policy that was never the cause.
roleARN := queryArgs["RoleArn"]
if roleARN == "" {
return "", errors.New(awserrors.ErrorMissingParameter)
}
if gw.IAMService == nil {
slog.Error("STS: IAM service not available", "action", "AssumeRole")
return "", errors.New(awserrors.ErrorInternalError)
}
_, role, err := handlers_sts.ResolveRoleByARN(gw.IAMService, roleARN)
switch {
case err == nil:
return aws.StringValue(role.Arn), nil
case errors.Is(err, handlers_sts.ErrRoleUnresolved):
// Echoing the supplied ARN keeps the denial identical whether or not the
// role exists. The handler refuses an ARN that is not the stored one, so
// a grant matched here on an invented path still ends in a denial there.
return roleARN, nil
}
if code, ok := awserrors.ResolveErrorCode(err); ok && code == awserrors.ErrorValidationError {
slog.Debug("STS: unparseable RoleArn", "roleArn", roleARN)
return "", err
}
slog.Error("STS: role lookup failed ahead of the identity gate", "roleArn", roleARN, "err", err)
return "", errors.New(awserrors.ErrorInternalError)
}
func (gw *GatewayConfig) STS_Request(w http.ResponseWriter, r *http.Request) error {
queryArgs, err := readQueryArgs(r)
if err != nil {
@@ -98,10 +141,6 @@ func (gw *GatewayConfig) STS_Request(w http.ResponseWriter, r *http.Request) err
return errors.New(awserrors.ErrorInternalError)
}
// STS actions are not gated by caller IAM policy: each action enforces its
// own rule in the handler (role trust policy, web-identity JWT, or the
// always-allowed GetCallerIdentity), so no checkPolicy pass runs here.
// Anonymous actions carry no SigV4 envelope; handler ignores the zero caller.
var caller stsCaller
if !anonymousSTSActions[action] {
@@ -111,6 +150,22 @@ func (gw *GatewayConfig) STS_Request(w http.ResponseWriter, r *http.Request) err
}
}
// The identity policy is the first of two gates; the handler still applies
// the role's trust policy. Scoping to the target role lets a policy grant
// assumption of one role without granting all of them.
if stsPolicyGatedActions[action] {
resource, rerr := gw.stsPolicyResource(queryArgs)
if rerr != nil {
return rerr
}
if err := gw.checkPolicyResources(r, "sts", action, []string{resource}); err != nil {
if denial, ok := errors.AsType[*identityPolicyDenialError](err); ok {
return denial.detailedError()
}
return err
}
}
xmlOutput, err := handler(action, queryArgs, gw, caller)
if err != nil {
return err
spinifex/gateway/sts/AssumeRole.go
+3 -2
View File
@@ -10,8 +10,9 @@ import (
)
// AssumeRole validates the request and delegates to STSService. Authorization
// is gated solely by the role's trust policy (no checkPolicy call), matching
// AWS behaviour. callerARN is resolved by the dispatcher from the SigV4 principal.
// takes two gates: the dispatcher checks the caller's identity policy against
// the target role ARN, and STSService checks the role's trust policy.
// callerARN is resolved by the dispatcher from the SigV4 principal.
func AssumeRole(callerAccountID, callerARN, callerIdentity string, input *sts.AssumeRoleInput, svc handlers_sts.STSService) (*sts.AssumeRoleOutput, error) {
if input == nil {
return nil, errors.New(awserrors.ErrorMissingParameter)
spinifex/gateway/sts/GetSessionToken.go
+3 -3
View File
@@ -5,9 +5,9 @@ import (
handlers_sts "github.com/mulgadc/spinifex/spinifex/handlers/sts"
)
// GetSessionToken delegates to the STSService; not gated by caller IAM policy
// (STS_Request runs no checkPolicy pass). callerUserName and callerAccessKeyID
// come from the SigV4 context.
// GetSessionToken delegates to the STSService; not gated by caller IAM policy,
// as AWS requires no permission for authentication operations. callerUserName
// and callerAccessKeyID come from the SigV4 context.
func GetSessionToken(callerAccountID, callerUserName, callerPrincipalType, callerAccessKeyID string, input *sts.GetSessionTokenInput, svc handlers_sts.STSService) (*sts.GetSessionTokenOutput, error) {
return svc.GetSessionToken(callerAccountID, callerUserName, callerPrincipalType, callerAccessKeyID, input)
}
spinifex/gateway/sts_policy_test.go
+323
View File
@@ -0,0 +1,323 @@
// (stsRequestParams, principalTypeUser, the context keys) and shares the
// policy mocks with the other in-package authz suites.
//
//test:in-package — the gate is exercised through unexported request wiring
package gateway
import (
"errors"
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/aws/aws-sdk-go/service/sts"
"github.com/mulgadc/spinifex/spinifex/awserrors"
handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam"
"github.com/mulgadc/spinifex/spinifex/utils"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
const (
stsTestRoleName = "app"
stsTestRoleARN = "arn:aws:iam::000000000000:role/app"
)
// stsPolicyIAMService serves the identity policies the STS gate evaluates, the
// GetUser lookup GetCallerIdentity performs, and the GetRole the gate resolves
// the target role with.
type stsPolicyIAMService struct {
policyMockIAMService
roles map[string]string
}
func (m *stsPolicyIAMService) GetUser(_ string, _ *iam.GetUserInput) (*iam.GetUserOutput, error) {
return &iam.GetUserOutput{User: &iam.User{UserId: aws.String("AIDAALICE000")}}, nil
}
func (m *stsPolicyIAMService) GetRole(_ string, input *iam.GetRoleInput) (*iam.GetRoleOutput, error) {
arn, ok := m.roles[aws.StringValue(input.RoleName)]
if !ok {
return nil, errors.New(awserrors.ErrorIAMNoSuchEntity)
}
return &iam.GetRoleOutput{Role: &iam.Role{
RoleName: input.RoleName,
Arn: aws.String(arn),
}}, nil
}
// stsIdentityPolicy builds an IAM service returning the given statements as the
// caller's only identity policy, for a user and for an assumed role alike. The
// one stored role is the pathless `app`; withRoles replaces the set.
func stsIdentityPolicy(statements ...handlers_iam.Statement) *stsPolicyIAMService {
docs := []handlers_iam.PolicyDocument{{Version: "2012-10-17", Statement: statements}}
svc := &stsPolicyIAMService{roles: map[string]string{stsTestRoleName: stsTestRoleARN}}
svc.getUserPoliciesFn = func(_, _ string) ([]handlers_iam.PolicyDocument, error) {
return docs, nil
}
svc.getRolePoliciesFn = func(_, _ string) ([]handlers_iam.PolicyDocument, error) {
return docs, nil
}
return svc
}
// withRoles replaces the roles GetRole answers for, keyed by name and mapped to
// the ARN IAM stored — which is what a resource-scoped grant is matched against.
func (m *stsPolicyIAMService) withRoles(roles map[string]string) *stsPolicyIAMService {
m.roles = roles
return m
}
// stsStatement is shorthand for a single-action, single-resource statement.
func stsStatement(effect, action, resource string) handlers_iam.Statement {
return handlers_iam.Statement{
Effect: effect,
Action: handlers_iam.StringOrArr{action},
Resource: handlers_iam.StringOrArr{resource},
}
}
// deniedSTSService fails the test if any gated handler is reached, so a passing
// denial can only have come from the gate.
func deniedSTSService(t *testing.T) *flexMockSTSService {
t.Helper()
return &flexMockSTSService{
assumeRoleFn: func(string, string, string, *sts.AssumeRoleInput) (*sts.AssumeRoleOutput, error) {
t.Fatal("handler reached: the identity-policy gate should have denied this request")
return nil, nil
},
}
}
// stsPolicyRequest dispatches an STS action as alice with the given identity
// policies.
func stsPolicyRequest(t *testing.T, iamSvc handlers_iam.IAMService, stsSvc *flexMockSTSService, body string) *http.Response {
t.Helper()
if stsSvc == nil {
stsSvc = deniedSTSService(t)
}
return stsPolicyRequestAs(t, stsRequestParams{
accountID: utils.GlobalAccountID,
identity: "alice",
principalType: principalTypeUser,
accessKey: "AKIAEXAMPLE",
stsSvc: stsSvc,
iamSvc: iamSvc,
}, body)
}
func stsPolicyRequestAs(t *testing.T, p stsRequestParams, body string) *http.Response {
t.Helper()
req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
return doRequest(setupSTSRequestHandler(p), req)
}
func assertSTSAccessDenied(t *testing.T, resp *http.Response) string {
t.Helper()
require.Equal(t, http.StatusForbidden, resp.StatusCode)
b, _ := io.ReadAll(resp.Body)
assert.Contains(t, string(b), "AccessDenied")
return string(b)
}
// TestSTSRequest_AssumeRole_IdentityPolicyGate covers the identity-side gate.
// The STS mock would happily mint credentials, so a denial can only come from
// the policy check that now runs ahead of the role's trust policy.
func TestSTSRequest_AssumeRole_IdentityPolicyGate(t *testing.T) {
body := "Action=AssumeRole&RoleArn=" + stsTestRoleARN + "&RoleSessionName=s1"
t.Run("empty policy denied", func(t *testing.T) {
assertSTSAccessDenied(t, stsPolicyRequest(t, stsIdentityPolicy(), nil, body))
})
t.Run("explicit deny beats allow", func(t *testing.T) {
svc := stsIdentityPolicy(
stsStatement("Allow", "sts:*", "*"),
stsStatement("Deny", "sts:AssumeRole", "*"),
)
assertSTSAccessDenied(t, stsPolicyRequest(t, svc, nil, body))
})
t.Run("allowed principal reaches handler", func(t *testing.T) {
called := false
stsSvc := &flexMockSTSService{
assumeRoleFn: func(string, string, string, *sts.AssumeRoleInput) (*sts.AssumeRoleOutput, error) {
called = true
return &sts.AssumeRoleOutput{
Credentials: &sts.Credentials{AccessKeyId: aws.String("ASIAEXAMPLE123")},
}, nil
},
}
svc := stsIdentityPolicy(stsStatement("Allow", "sts:AssumeRole", "*"))
resp := stsPolicyRequest(t, svc, stsSvc, body)
require.Equal(t, http.StatusOK, resp.StatusCode)
assert.True(t, called)
})
}
// TestSTSRequest_AssumeRole_AssumedRoleCallerIsGated is the case that would
// have caught the ECS agent regression: a role session whose underlying role
// grants a non-sts action and nothing else must not be able to assume anything.
func TestSTSRequest_AssumeRole_AssumedRoleCallerIsGated(t *testing.T) {
body := "Action=AssumeRole&RoleArn=" + stsTestRoleARN + "&RoleSessionName=s1"
params := func(iamSvc handlers_iam.IAMService, stsSvc *flexMockSTSService) stsRequestParams {
return stsRequestParams{
accountID: utils.GlobalAccountID,
identity: "s1",
principalType: principalTypeAssumedRole,
accessKey: "ASIAEXAMPLE",
assumedRoleARN: "arn:aws:sts::000000000000:assumed-role/ecsInstanceRole/s1",
assumedRoleID: "AROAECS:s1",
underlyingRoleARN: "arn:aws:iam::000000000000:role/ecsInstanceRole",
stsSvc: stsSvc,
iamSvc: iamSvc,
}
}
t.Run("ecs-only role denied", func(t *testing.T) {
svc := stsIdentityPolicy(stsStatement("Allow", "ecs:*", "*"))
resp := stsPolicyRequestAs(t, params(svc, deniedSTSService(t)), body)
assertSTSAccessDenied(t, resp)
})
t.Run("role granted sts:AssumeRole reaches handler", func(t *testing.T) {
svc := stsIdentityPolicy(
stsStatement("Allow", "ecs:*", "*"),
stsStatement("Allow", "sts:AssumeRole", "arn:aws:iam::000000000000:role/*"),
)
resp := stsPolicyRequestAs(t, params(svc, &flexMockSTSService{}), body)
assert.Equal(t, http.StatusOK, resp.StatusCode)
})
}
// TestSTSRequest_AssumeRole_PolicyScopedToRole proves the gate evaluates the
// target role ARN, not "*", so a grant on one role does not open another.
func TestSTSRequest_AssumeRole_PolicyScopedToRole(t *testing.T) {
roles := map[string]string{
stsTestRoleName: stsTestRoleARN,
"other": "arn:aws:iam::000000000000:role/other",
}
t.Run("granted role allowed", func(t *testing.T) {
svc := stsIdentityPolicy(stsStatement("Allow", "sts:AssumeRole", stsTestRoleARN)).withRoles(roles)
resp := stsPolicyRequest(t, svc, &flexMockSTSService{},
"Action=AssumeRole&RoleArn="+stsTestRoleARN+"&RoleSessionName=s1")
assert.Equal(t, http.StatusOK, resp.StatusCode)
})
t.Run("other role denied", func(t *testing.T) {
svc := stsIdentityPolicy(stsStatement("Allow", "sts:AssumeRole", stsTestRoleARN)).withRoles(roles)
resp := stsPolicyRequest(t, svc, nil,
"Action=AssumeRole&RoleArn=arn:aws:iam::000000000000:role/other&RoleSessionName=s1")
assertSTSAccessDenied(t, resp)
})
}
// TestSTSRequest_AssumeRole_PathIsResolvedNotTrusted pins the gate to the ARN
// IAM stored, so a role genuinely stored under a path is matched by its full
// ARN. An ARN that is not the stored one is refused by role resolution itself
// — see TestResolveRoleByARN in handlers/sts.
func TestSTSRequest_AssumeRole_PathIsResolvedNotTrusted(t *testing.T) {
t.Run("stored ARN matching exactly is admitted", func(t *testing.T) {
svc := stsIdentityPolicy(
stsStatement("Allow", "sts:AssumeRole", "arn:aws:iam::000000000000:role/app-*"),
).withRoles(map[string]string{"app-worker": "arn:aws:iam::000000000000:role/app-worker"})
resp := stsPolicyRequest(t, svc, &flexMockSTSService{},
"Action=AssumeRole&RoleArn=arn:aws:iam::000000000000:role/app-worker&RoleSessionName=s1")
assert.Equal(t, http.StatusOK, resp.StatusCode)
})
t.Run("a pathed role matches a policy naming its full ARN", func(t *testing.T) {
const pathed = "arn:aws:iam::000000000000:role/team/app-worker"
svc := stsIdentityPolicy(stsStatement("Allow", "sts:AssumeRole", pathed)).
withRoles(map[string]string{"app-worker": pathed})
resp := stsPolicyRequest(t, svc, &flexMockSTSService{},
"Action=AssumeRole&RoleArn="+pathed+"&RoleSessionName=s1")
assert.Equal(t, http.StatusOK, resp.StatusCode)
})
t.Run("malformed ARN rejected before evaluation", func(t *testing.T) {
resp := stsPolicyRequest(t, stsIdentityPolicy(), nil,
"Action=AssumeRole&RoleArn=not-an-arn&RoleSessionName=s1")
require.Equal(t, http.StatusBadRequest, resp.StatusCode)
b, _ := io.ReadAll(resp.Body)
assert.Contains(t, string(b), "ValidationError")
})
}
// TestSTSRequest_AssumeRole_DenialDoesNotDiscloseRoleExistence keeps the
// detailed denial message safe to return: it must read identically whether or
// not the role exists, which a lookup added ahead of the gate would break.
func TestSTSRequest_AssumeRole_DenialDoesNotDiscloseRoleExistence(t *testing.T) {
body := "Action=AssumeRole&RoleArn=" + stsTestRoleARN + "&RoleSessionName=s1"
existing := assertSTSAccessDenied(t, stsPolicyRequest(t, stsIdentityPolicy(), nil, body))
missing := assertSTSAccessDenied(t,
stsPolicyRequest(t, stsIdentityPolicy().withRoles(nil), nil, body))
assert.Equal(t, stripRequestID(existing), stripRequestID(missing))
assert.Contains(t, existing, "is not authorized to perform: sts:AssumeRole on resource: "+stsTestRoleARN)
// A pathed role is where the two can diverge: a resolved role names its
// stored ARN, so an unresolved one must name what the caller supplied
// rather than any form derived from it.
const pathed = "arn:aws:iam::000000000000:role/team/svc"
pathedBody := "Action=AssumeRole&RoleArn=" + pathed + "&RoleSessionName=s1"
pathedExisting := assertSTSAccessDenied(t, stsPolicyRequest(t,
stsIdentityPolicy().withRoles(map[string]string{"svc": pathed}), nil, pathedBody))
pathedMissing := assertSTSAccessDenied(t,
stsPolicyRequest(t, stsIdentityPolicy().withRoles(nil), nil, pathedBody))
assert.Equal(t, stripRequestID(pathedExisting), stripRequestID(pathedMissing))
assert.Contains(t, pathedExisting, "on resource: "+pathed)
}
// stripRequestID removes the per-request UUID so two error bodies can be
// compared for everything else.
func stripRequestID(body string) string {
start := strings.Index(body, "<RequestId>")
end := strings.Index(body, "</RequestId>")
if start < 0 || end < start {
return body
}
return body[:start] + body[end:]
}
// TestSTSRequest_GetSessionToken_NotGated pins the AWS contract: it is an
// authentication operation requiring no permission, so an explicit Deny on the
// identity policy does not stop it reaching the handler.
func TestSTSRequest_GetSessionToken_NotGated(t *testing.T) {
svc := stsIdentityPolicy(stsStatement("Deny", "sts:*", "*"))
called := false
stsSvc := &flexMockSTSService{
getSessionTokenFn: func(string, string, string, string, *sts.GetSessionTokenInput) (*sts.GetSessionTokenOutput, error) {
called = true
return &sts.GetSessionTokenOutput{
Credentials: &sts.Credentials{AccessKeyId: aws.String("ASIAEXAMPLE123")},
}, nil
},
}
resp := stsPolicyRequest(t, svc, stsSvc, "Action=GetSessionToken&DurationSeconds=3600")
require.Equal(t, http.StatusOK, resp.StatusCode)
assert.True(t, called)
}
// TestSTSRequest_GetCallerIdentity_NotGated pins the AWS contract: the action
// requires no permission and cannot be denied by policy.
func TestSTSRequest_GetCallerIdentity_NotGated(t *testing.T) {
svc := stsIdentityPolicy(stsStatement("Deny", "sts:*", "*"))
resp := stsPolicyRequest(t, svc, &flexMockSTSService{}, "Action=GetCallerIdentity")
require.Equal(t, http.StatusOK, resp.StatusCode)
b, _ := io.ReadAll(resp.Body)
assert.Contains(t, string(b), "GetCallerIdentityResult")
}
spinifex/gateway/sts_request_test.go
+46 -2
View File
@@ -83,8 +83,11 @@ type stsRequestParams struct {
accessKey string
assumedRoleARN string
assumedRoleID string
stsSvc handlers_sts.STSService
iamSvc handlers_iam.IAMService
// underlyingRoleARN is what the policy gate resolves an assumed-role
// caller's policies by; SessionName never participates.
underlyingRoleARN string
stsSvc handlers_sts.STSService
iamSvc handlers_iam.IAMService
}
func setupSTSRequestHandler(p stsRequestParams) http.Handler {
@@ -109,6 +112,9 @@ func setupSTSRequestHandler(p stsRequestParams) http.Handler {
if p.assumedRoleID != "" {
ctx = context.WithValue(ctx, ctxAssumedRoleID, p.assumedRoleID)
}
if p.underlyingRoleARN != "" {
ctx = context.WithValue(ctx, ctxUnderlyingRoleARN, p.underlyingRoleARN)
}
r = r.WithContext(ctx)
if err := gw.STS_Request(w, r); err != nil {
gw.ErrorHandler(w, r, err)
@@ -140,6 +146,7 @@ func TestSTSRequest_AssumeRole_Success(t *testing.T) {
principalType: principalTypeUser,
accessKey: "AKIAEXAMPLE",
stsSvc: svc,
iamSvc: stsIdentityPolicy(stsStatement("Allow", "sts:AssumeRole", "*")),
})
body := "Action=AssumeRole&RoleArn=arn:aws:iam::000000000000:role/app&RoleSessionName=s1"
@@ -310,6 +317,9 @@ func TestSTSRequest_AssumeRole_MissingRoleArn(t *testing.T) {
principalType: principalTypeUser,
accessKey: "AKIAEXAMPLE",
stsSvc: svc,
// An empty policy: a request missing RoleArn is malformed whatever the
// caller holds, so the gate must not answer it with a denial.
iamSvc: stsIdentityPolicy(),
})
req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader("Action=AssumeRole&RoleSessionName=s1"))
@@ -345,6 +355,39 @@ func TestSTSRequest_NilService_InternalError(t *testing.T) {
assert.Contains(t, string(b), "InternalError")
}
// TestSTSRequest_AssumeRole_NilIAMService_InternalError pins the gate to the
// same fail-closed answer checkPolicyResources gives: the target-role lookup
// runs ahead of it, so an absent IAM service must not reach a nil interface.
func TestSTSRequest_AssumeRole_NilIAMService_InternalError(t *testing.T) {
svc := &flexMockSTSService{
assumeRoleFn: func(string, string, string, *sts.AssumeRoleInput) (*sts.AssumeRoleOutput, error) {
t.Fatal("handler reached: an unavailable IAM service must fail closed")
return nil, nil
},
}
gw := &GatewayConfig{DisableLogging: true, STSService: svc, IAMService: nil}
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
ctx := context.WithValue(r.Context(), ctxService, "sts")
ctx = context.WithValue(ctx, ctxAccountID, utils.GlobalAccountID)
ctx = context.WithValue(ctx, ctxIdentity, "alice")
ctx = context.WithValue(ctx, ctxPrincipalType, principalTypeUser)
ctx = context.WithValue(ctx, ctxAccessKey, "AKIAEXAMPLE")
r = r.WithContext(ctx)
if err := gw.STS_Request(w, r); err != nil {
gw.ErrorHandler(w, r, err)
}
})
req := httptest.NewRequest(http.MethodPost, "/",
strings.NewReader("Action=AssumeRole&RoleArn="+stsTestRoleARN+"&RoleSessionName=s1"))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
resp := doRequest(handler, req)
assert.Equal(t, 500, resp.StatusCode)
b, _ := io.ReadAll(resp.Body)
assert.Contains(t, string(b), "InternalError")
}
func TestSTSRequest_AssumeRole_ServiceError_PropagatesAccessDenied(t *testing.T) {
// AccessDenied from the handler (trust-policy denial) must reach the wire as IAM ErrorResponse + 403.
svc := &flexMockSTSService{
@@ -358,6 +401,7 @@ func TestSTSRequest_AssumeRole_ServiceError_PropagatesAccessDenied(t *testing.T)
principalType: principalTypeUser,
accessKey: "AKIAEXAMPLE",
stsSvc: svc,
iamSvc: stsIdentityPolicy(stsStatement("Allow", "sts:AssumeRole", "*")),
})
body := "Action=AssumeRole&RoleArn=arn:aws:iam::000000000000:role/app&RoleSessionName=s1"
spinifex/handlers/ecs/capacity_test.go
+2 -13
View File
@@ -7,7 +7,6 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/mulgadc/spinifex/spinifex/awserrors"
iammock "github.com/mulgadc/spinifex/spinifex/handlers/iam/mock"
"github.com/mulgadc/spinifex/spinifex/tags"
@@ -66,9 +65,8 @@ func imageMatchesFilters(img *ec2.Image, filters []*ec2.Filter) bool {
return true
}
// stubIAM embeds the shared SystemInstanceRoleEnsurer mock for the Get*/Create*
// find-or-create behaviour, and adds CreatePolicy/AttachRolePolicy locally
// since ecsIAM needs both beyond what the ensurer interface covers.
// stubIAM is the shared SystemInstanceRoleEnsurer mock, which covers the whole
// ecsIAM surface.
type stubIAM struct {
*iammock.SystemInstanceRoleEnsurer
}
@@ -83,15 +81,6 @@ var (
_ ecsImageResolver = (*filteringStubImages)(nil)
)
func (stubIAM) CreatePolicy(accountID string, _ *iam.CreatePolicyInput) (*iam.CreatePolicyOutput, error) {
arn := "arn:aws:iam::" + accountID + ":policy/" + ecsInstanceRolePolicyName
return &iam.CreatePolicyOutput{Policy: &iam.Policy{Arn: aws.String(arn)}}, nil
}
func (stubIAM) AttachRolePolicy(_ string, _ *iam.AttachRolePolicyInput) (*iam.AttachRolePolicyOutput, error) {
return &iam.AttachRolePolicyOutput{}, nil
}
func ecsNodeImage() []*ec2.Image {
return []*ec2.Image{{
ImageId: aws.String("ami-ecs"),
spinifex/handlers/ecs/instancerole.go
+28 -147
View File
@@ -2,12 +2,8 @@ package handlers_ecs
import (
"fmt"
"time"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/mulgadc/spinifex/spinifex/arn"
"github.com/mulgadc/spinifex/spinifex/awserrors"
handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam"
)
const (
@@ -15,153 +11,38 @@ const (
// container instances. The agent draws its creds from this role over IMDS.
ecsInstanceRoleName = "ecsInstanceRole"
// ecsInstanceRolePolicyName is the customer-managed policy granting ecs:* so
// the gateway's assumed-role ECS authz (checkPolicy) admits the agent.
// ecsInstanceRolePolicyName is the inline policy granting the agent the
// actions the gateway enforces for its assumed-role principal.
ecsInstanceRolePolicyName = "ecsInstanceRolePolicy"
// ecsAssumeRolePolicy is the EC2 trust the STS AssumeRoleForInstance path
// whitelists; it must match exactly for IMDS role-cred vending to admit it.
ecsAssumeRolePolicy = `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Service":"ec2.amazonaws.com"},"Action":"sts:AssumeRole"}]}`
// ecsInstanceRolePolicyDoc grants the agent the ECS control-plane actions the
// gateway enforces for assumed-role principals, plus ECR read so it can pull
// task images via the instance role when a task carries no execution role.
ecsInstanceRolePolicyDoc = `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"ecs:*","Resource":"*"},{"Effect":"Allow","Action":["ecr:GetAuthorizationToken","ecr:BatchCheckLayerAvailability","ecr:GetDownloadUrlForLayer","ecr:BatchGetImage"],"Resource":"*"}]}`
// ecsAttachRetries bounds the wait for a just-created policy to become
// visible to the attach, and ecsAttachRetryBase scales the linear backoff.
ecsAttachRetries = 5
ecsAttachRetryBase = 100 * time.Millisecond
)
// ecsAttachRetrySleep is the test seam for the attach backoff.
var ecsAttachRetrySleep = time.Sleep
// ecsInstanceRolePolicyDoc grants the agent the ECS control-plane actions the
// gateway enforces for assumed-role principals, ECR read so it can pull task
// images when a task carries no execution role, and sts:AssumeRole so it can
// mint task and execution role credentials. Assumption stays bounded by each
// target role's trust policy.
func ecsInstanceRolePolicyDoc(accountID string) string {
return fmt.Sprintf(`{"Version":"2012-10-17","Statement":[`+
`{"Effect":"Allow","Action":"ecs:*","Resource":"*"},`+
`{"Effect":"Allow","Action":["ecr:GetAuthorizationToken","ecr:BatchCheckLayerAvailability",`+
`"ecr:GetDownloadUrlForLayer","ecr:BatchGetImage"],"Resource":"*"},`+
`{"Effect":"Allow","Action":"sts:AssumeRole","Resource":"arn:aws:iam::%s:role/*"}]}`, accountID)
}
// ensureECSInstanceProfile find-or-creates the ecsInstanceRole, its ecs:* policy
// attachment, and the matching instance profile, returning the profile ARN.
// Every step is idempotent so concurrent ProvisionCapacity calls converge.
// ensureECSInstanceProfile find-or-creates the ecsInstanceRole, re-asserts its
// inline policy, and binds the matching instance profile, returning the profile
// ARN. Every step is idempotent so concurrent callers converge, and the policy
// is written rather than created so an existing role picks up a changed
// document instead of keeping the one it was provisioned with.
func (s *Service) ensureECSInstanceProfile(accountID string) (string, error) {
if err := s.ensureECSRole(accountID); err != nil {
return "", err
}
if err := s.ensureECSRolePolicy(accountID); err != nil {
return "", err
}
return s.ensureECSInstanceProfileBinding(accountID)
return handlers_iam.EnsureSystemInstanceProfile(s.deps.IAM, accountID,
ecsInstanceRoleName, ecsInstanceRolePolicyName, ecsInstanceRolePolicyDoc(accountID))
}
// ensureECSRole find-or-creates ecsInstanceRole with the EC2 trust policy,
// converging on a racing creator via re-GetRole.
func (s *Service) ensureECSRole(accountID string) error {
_, err := s.deps.IAM.GetRole(accountID, &iam.GetRoleInput{RoleName: aws.String(ecsInstanceRoleName)})
if err == nil {
return nil
}
if err.Error() != awserrors.ErrorIAMNoSuchEntity {
return fmt.Errorf("get role %q: %w", ecsInstanceRoleName, err)
}
_, err = s.deps.IAM.CreateRole(accountID, &iam.CreateRoleInput{
RoleName: aws.String(ecsInstanceRoleName),
AssumeRolePolicyDocument: aws.String(ecsAssumeRolePolicy),
})
if err == nil {
return nil
}
if err.Error() != awserrors.ErrorIAMEntityAlreadyExists {
return fmt.Errorf("create role %q: %w", ecsInstanceRoleName, err)
}
// A racing call created it first; re-read to converge.
if _, gerr := s.deps.IAM.GetRole(accountID, &iam.GetRoleInput{RoleName: aws.String(ecsInstanceRoleName)}); gerr != nil {
return fmt.Errorf("re-get role %q: %w", ecsInstanceRoleName, gerr)
}
return nil
}
// ensureECSRolePolicy creates the ecs:* policy (resolving an existing one on a
// race) and attaches it to the role; an already-attached policy is success.
func (s *Service) ensureECSRolePolicy(accountID string) error {
policyARN := arn.FormatIAMPath(arn.IAMPolicy, accountID, "/", ecsInstanceRolePolicyName)
out, err := s.deps.IAM.CreatePolicy(accountID, &iam.CreatePolicyInput{
PolicyName: aws.String(ecsInstanceRolePolicyName),
PolicyDocument: aws.String(ecsInstanceRolePolicyDoc),
})
switch {
case err == nil:
if out != nil && out.Policy != nil {
policyARN = aws.StringValue(out.Policy.Arn)
}
case err.Error() == awserrors.ErrorIAMEntityAlreadyExists:
// Policy already exists; the account ARN form is deterministic.
default:
return fmt.Errorf("create policy %q: %w", ecsInstanceRolePolicyName, err)
}
// The attach checks the policy exists, and the policy buckets are
// replicated, so a read that lands on a replica which has not caught up
// answers NoSuchEntity for the write a microsecond above. Retry that one
// code rather than fail a provision over a write known to have landed.
for attempt := 0; ; attempt++ {
_, err = s.deps.IAM.AttachRolePolicy(accountID, &iam.AttachRolePolicyInput{
RoleName: aws.String(ecsInstanceRoleName),
PolicyArn: aws.String(policyARN),
})
if err == nil || err.Error() == awserrors.ErrorIAMEntityAlreadyExists {
return nil
}
if err.Error() != awserrors.ErrorIAMNoSuchEntity || attempt == ecsAttachRetries {
return fmt.Errorf("attach policy %q to role %q: %w", policyARN, ecsInstanceRoleName, err)
}
ecsAttachRetrySleep(time.Duration(attempt+1) * ecsAttachRetryBase)
}
}
// ensureECSInstanceProfileBinding guarantees the ecsInstanceRole instance
// profile exists and carries the role, returning its ARN. Mirrors the EKS
// node-profile ensure path; idempotent under concurrent launches.
func (s *Service) ensureECSInstanceProfileBinding(accountID string) (string, error) {
out, err := s.deps.IAM.GetInstanceProfile(accountID, &iam.GetInstanceProfileInput{
InstanceProfileName: aws.String(ecsInstanceRoleName),
})
if err == nil {
return s.attachECSRoleToProfile(accountID, aws.StringValue(out.InstanceProfile.Arn), len(out.InstanceProfile.Roles) > 0)
}
if err.Error() != awserrors.ErrorIAMNoSuchEntity {
return "", fmt.Errorf("get instance profile %q: %w", ecsInstanceRoleName, err)
}
created, err := s.deps.IAM.CreateInstanceProfile(accountID, &iam.CreateInstanceProfileInput{
InstanceProfileName: aws.String(ecsInstanceRoleName),
})
if err != nil {
if err.Error() != awserrors.ErrorIAMEntityAlreadyExists {
return "", fmt.Errorf("create instance profile %q: %w", ecsInstanceRoleName, err)
}
got, gerr := s.deps.IAM.GetInstanceProfile(accountID, &iam.GetInstanceProfileInput{
InstanceProfileName: aws.String(ecsInstanceRoleName),
})
if gerr != nil {
return "", fmt.Errorf("re-get instance profile %q: %w", ecsInstanceRoleName, gerr)
}
return s.attachECSRoleToProfile(accountID, aws.StringValue(got.InstanceProfile.Arn), len(got.InstanceProfile.Roles) > 0)
}
return s.attachECSRoleToProfile(accountID, aws.StringValue(created.InstanceProfile.Arn), false)
}
// attachECSRoleToProfile adds the role to the profile unless already attached,
// returning the profile ARN. A LimitExceeded error means a racing launch
// attached it first and is treated as success.
func (s *Service) attachECSRoleToProfile(accountID, profileARN string, alreadyAttached bool) (string, error) {
if alreadyAttached {
return profileARN, nil
}
_, err := s.deps.IAM.AddRoleToInstanceProfile(accountID, &iam.AddRoleToInstanceProfileInput{
InstanceProfileName: aws.String(ecsInstanceRoleName),
RoleName: aws.String(ecsInstanceRoleName),
})
if err != nil && err.Error() != awserrors.ErrorIAMLimitExceeded {
return "", fmt.Errorf("add role to instance profile %q: %w", ecsInstanceRoleName, err)
}
return profileARN, nil
// convergeECSInstanceRole re-asserts the inline policy on an account that
// already has the role, so a running cluster picks up a changed document
// without creating IAM entities in accounts that never launched capacity.
func (s *Service) convergeECSInstanceRole(accountID string) error {
return handlers_iam.ConvergeSystemRolePolicy(s.deps.IAM, accountID,
ecsInstanceRoleName, ecsInstanceRolePolicyName, ecsInstanceRolePolicyDoc(accountID))
}
spinifex/handlers/ecs/instancerole_retry_test.go
-90
View File
@@ -1,90 +0,0 @@
// Drives ensureECSRolePolicy directly and swaps the unexported retry sleep, so
// it needs the package's own scope.
//
//test:in-package
package handlers_ecs
import (
"errors"
"testing"
"time"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/mulgadc/spinifex/spinifex/awserrors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// flakyAttachIAM answers NoSuchEntity from AttachRolePolicy a set number of
// times before succeeding, standing in for a replica that has not yet caught up
// with the policy written immediately before.
type flakyAttachIAM struct {
stubIAM
notFoundTimes int
attaches int
}
func (f *flakyAttachIAM) AttachRolePolicy(_ string, _ *iam.AttachRolePolicyInput) (*iam.AttachRolePolicyOutput, error) {
f.attaches++
if f.attaches <= f.notFoundTimes {
return nil, errors.New(awserrors.ErrorIAMNoSuchEntity)
}
return &iam.AttachRolePolicyOutput{}, nil
}
// alwaysDeniedIAM answers a code the retry must not swallow.
type alwaysDeniedIAM struct {
stubIAM
attaches int
}
func (a *alwaysDeniedIAM) AttachRolePolicy(_ string, _ *iam.AttachRolePolicyInput) (*iam.AttachRolePolicyOutput, error) {
a.attaches++
return nil, errors.New(awserrors.ErrorAccessDenied)
}
func noAttachSleep(t *testing.T) {
t.Helper()
original := ecsAttachRetrySleep
ecsAttachRetrySleep = func(time.Duration) {}
t.Cleanup(func() { ecsAttachRetrySleep = original })
}
// The policy is created and then attached. A NoSuchEntity from that attach is a
// stale read of a write that landed, so provisioning must converge rather than
// fail the whole call.
func TestEnsureECSRolePolicyRetriesAStaleNoSuchEntity(t *testing.T) {
noAttachSleep(t)
fake := &flakyAttachIAM{stubIAM: newStubIAM(), notFoundTimes: 3}
svc := &Service{deps: Deps{IAM: fake}}
require.NoError(t, svc.ensureECSRolePolicy("000000000106"))
assert.Equal(t, 4, fake.attaches, "three stale reads then the one that sees the policy")
}
// The retry is bounded: a policy that genuinely never appears still fails.
func TestEnsureECSRolePolicyGivesUpOnAPersistentNoSuchEntity(t *testing.T) {
noAttachSleep(t)
fake := &flakyAttachIAM{stubIAM: newStubIAM(), notFoundTimes: 1000}
svc := &Service{deps: Deps{IAM: fake}}
err := svc.ensureECSRolePolicy("000000000106")
require.Error(t, err)
assert.ErrorContains(t, err, awserrors.ErrorIAMNoSuchEntity)
assert.Equal(t, ecsAttachRetries+1, fake.attaches, "bounded by ecsAttachRetries")
}
// Only the stale-read code is retried. Anything else is a real answer and must
// come back on the first attempt.
func TestEnsureECSRolePolicyDoesNotRetryOtherErrors(t *testing.T) {
noAttachSleep(t)
fake := &alwaysDeniedIAM{stubIAM: newStubIAM()}
svc := &Service{deps: Deps{IAM: fake}}
err := svc.ensureECSRolePolicy("000000000106")
require.Error(t, err)
assert.ErrorContains(t, err, awserrors.ErrorAccessDenied)
assert.Equal(t, 1, fake.attaches, "a denial is not a visibility race")
}
spinifex/handlers/ecs/scheduler.go
+53 -3
View File
@@ -3,6 +3,8 @@ package handlers_ecs
import (
"context"
"encoding/json"
"errors"
"fmt"
"log/slog"
"strings"
"sync"
@@ -30,6 +32,11 @@ const (
// UI exit reason) before it is dropped, matching AWS's ~1h STOPPED window.
sweepInterval = 60 * time.Second
stoppedTaskRetention = 1 * time.Hour
// convergeInterval is how often the leader re-asserts the instance-role
// policy. The document changes only across releases, so this trades a slower
// pickup after an upgrade for a fleet-wide write every reconcile tick.
convergeInterval = 5 * time.Minute
)
// Scheduler is the per-daemon ECS control loop. A single leader (elected via the
@@ -59,9 +66,10 @@ func NewScheduler(nc *nats.Conn, svc *Service, holder string) *Scheduler {
TTL: KVBucketECSLeaderTTL,
Renew: leaseRefresh,
Retry: leaseRefresh,
// The leader owns the Layer-2 bus subscriptions. A node that cannot wire them stands
// down rather than hold the lease and process nothing.
OnGained: sc.subscribeBus,
// The leader owns the Layer-2 bus subscriptions and the instance-role
// converge pass. A node that can do neither stands down rather than hold
// the lease and process nothing.
OnGained: sc.onGained,
OnLost: sc.unsubscribeBus,
})
return sc
@@ -85,9 +93,11 @@ func (sc *Scheduler) Run(ctx context.Context) {
reaperTicker := time.NewTicker(reaperInterval)
reconcileTicker := time.NewTicker(reconcileInterval)
sweepTicker := time.NewTicker(sweepInterval)
convergeTicker := time.NewTicker(convergeInterval)
defer reaperTicker.Stop()
defer reconcileTicker.Stop()
defer sweepTicker.Stop()
defer convergeTicker.Stop()
leaseDone := make(chan struct{})
go func() {
@@ -107,6 +117,8 @@ func (sc *Scheduler) Run(ctx context.Context) {
sc.runIfLeader("service reconcile", func() error { return sc.svc.reconcileAllServices(ctx) })
case <-sweepTicker.C:
sc.runIfLeader("stopped-task sweep", func() error { return sc.sweepStoppedTasks(ctx) })
case <-convergeTicker.C:
sc.runIfLeader("instance role converge", func() error { return sc.convergeInstanceRoles(ctx) })
}
}
}
@@ -123,6 +135,16 @@ func (sc *Scheduler) runIfLeader(pass string, run func() error) {
}
}
// onGained refuses the lease when this node cannot converge instance roles.
// A leader without IAM holds the lease and never writes the grant, so every
// agent's credentials lapse an hour later with nothing but a per-tick log.
func (sc *Scheduler) onGained(ctx context.Context) error {
if sc.svc.deps.IAM == nil {
return errors.New("IAM dependency not wired: cannot converge instance roles")
}
return sc.subscribeBus(ctx)
}
// subscribeBus wires the wildcard Layer-2 subscriptions onto the service KV
// writers. All clusters/accounts fan into the leader.
func (sc *Scheduler) subscribeBus(_ context.Context) error {
@@ -268,6 +290,34 @@ func (sc *Scheduler) stopInstanceTasks(ctx context.Context, kv jetstream.KeyValu
}
}
// convergeInstanceRoles re-asserts the ecsInstanceRole policy on every account
// that already holds the role. Provisioning runs only when capacity changes, so
// a cluster that is merely running would otherwise never pick up a changed
// document and its agent would fail its next credential refresh.
func (sc *Scheduler) convergeInstanceRoles(ctx context.Context) error {
// A pass that cannot converge anything is a misconfiguration that silently
// breaks agent credentials an hour later, so it is not a benign no-op.
if sc.svc.deps.IAM == nil {
return errors.New("instance-role converge disabled: IAM dependency not wired")
}
buckets, err := accountBuckets(ctx, sc.nc)
if err != nil {
return err
}
failed := 0
for _, bucket := range buckets {
if err := sc.svc.convergeECSInstanceRole(bucket.accountID); err != nil {
failed++
slog.Error("ECS scheduler: converge instance role failed",
"accountId", bucket.accountID, "err", err)
}
}
if failed > 0 {
return fmt.Errorf("converge instance roles: %d of %d accounts failed", failed, len(buckets))
}
return nil
}
// accountIDFromBucket extracts the account ID from an ECS per-account bucket name.
func accountIDFromBucket(bucket string) (string, bool) {
if !strings.HasPrefix(bucket, KVBucketECSAccountPrefix) {
spinifex/handlers/ecs/scheduler_test.go
+96
View File
@@ -0,0 +1,96 @@
// Leader election, the converge pass and the instance-role document are all
// unexported, and the pass is only reachable through the scheduler's own state.
//
//test:in-package — the converge pass has no exported entry point
package handlers_ecs
import (
"context"
"testing"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ecs"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/mulgadc/spinifex/spinifex/testutil"
)
// TestScheduler_StandsDownWithoutIAM is the fleet outage this guards: a leader
// that cannot converge instance roles holds the lease, writes no sts:AssumeRole
// grant, and every agent's credentials lapse an hour later.
func TestScheduler_StandsDownWithoutIAM(t *testing.T) {
_, nc, js := testutil.StartTestJetStream(t)
svc := NewService(nc, testRegion, "")
sc := NewScheduler(nc, svc, "holder-a")
require.NoError(t, sc.leaseErr)
assert.False(t, sc.lease.TryAcquire(t.Context()), "a node without IAM must refuse the lease")
assert.False(t, sc.lease.Held())
assert.Zero(t, sc.subCount())
// The key must be free, or a healthy node waits out the full TTL.
kv, err := InitLeaderBucket(t.Context(), js)
require.NoError(t, err)
_, err = kv.Get(t.Context(), schedulerLeaderKey)
require.Error(t, err)
}
// TestConvergeInstanceRoles_RewritesProvisionedAccounts pins the pass that
// carries the grant to already-running clusters, which provisioning alone
// never reaches once capacity has stopped changing.
func TestConvergeInstanceRoles_RewritesProvisionedAccounts(t *testing.T) {
_, nc, _ := testutil.StartTestJetStream(t)
iam := newStubIAM()
svc := NewService(nc, testRegion, "").WithDeps(Deps{IAM: iam})
_, err := svc.CreateCluster(context.Background(), &ecs.CreateClusterInput{ClusterName: aws.String("web")}, testAccountID)
require.NoError(t, err)
_, err = svc.ensureECSInstanceProfile(testAccountID)
require.NoError(t, err)
iam.RolePolicies[ecsInstanceRoleName] = `{"Version":"2012-10-17","Statement":[]}`
sc := NewScheduler(nc, svc, "holder-a")
require.NoError(t, sc.convergeInstanceRoles(t.Context()))
assert.Contains(t, iam.RolePolicies[ecsInstanceRoleName], "sts:AssumeRole")
}
// TestConvergeInstanceRoles_SkipsAccountsWithoutTheRole keeps the pass from
// creating customer-visible IAM entities in an account that only ever created
// a cluster: the bucket exists there, but nothing draws credentials.
func TestConvergeInstanceRoles_SkipsAccountsWithoutTheRole(t *testing.T) {
_, nc, _ := testutil.StartTestJetStream(t)
iam := newStubIAM()
svc := NewService(nc, testRegion, "").WithDeps(Deps{IAM: iam})
_, err := svc.CreateCluster(context.Background(), &ecs.CreateClusterInput{ClusterName: aws.String("web")}, testAccountID)
require.NoError(t, err)
sc := NewScheduler(nc, svc, "holder-a")
require.NoError(t, sc.convergeInstanceRoles(t.Context()))
assert.Zero(t, iam.CreateRoleCalls)
assert.Zero(t, iam.CreateInstanceProfileCalls)
assert.Empty(t, iam.PolicyCalls)
}
// TestConvergeInstanceRoles_NilIAMIsAnError keeps the guard fail-closed for a
// caller that reaches the pass without going through leader election.
func TestConvergeInstanceRoles_NilIAMIsAnError(t *testing.T) {
_, nc, _ := testutil.StartTestJetStream(t)
sc := NewScheduler(nc, NewService(nc, testRegion, ""), "holder-a")
err := sc.convergeInstanceRoles(t.Context())
require.Error(t, err)
assert.Contains(t, err.Error(), "IAM", "the log must name the missing dependency")
}
// TestECSInstanceRolePolicyDoc_GrantsAssumeRole is the identity-side half of the
// two-gate model for the agent: without it every task-role and execution-role
// credential mint is denied about an hour after an upgrade.
func TestECSInstanceRolePolicyDoc_GrantsAssumeRole(t *testing.T) {
doc := ecsInstanceRolePolicyDoc(testAccountID)
assert.Contains(t, doc, `{"Effect":"Allow","Action":"sts:AssumeRole","Resource":"arn:aws:iam::`+testAccountID+`:role/*"}`)
assert.Contains(t, doc, `"ecs:*"`)
assert.Contains(t, doc, `"ecr:GetAuthorizationToken"`)
}
spinifex/handlers/ecs/service.go
+4 -12
View File
@@ -15,8 +15,8 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/aws/aws-sdk-go/service/ecs"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/mulgadc/spinifex/spinifex/awserrors"
handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam"
"github.com/nats-io/nats.go"
"github.com/nats-io/nats.go/jetstream"
)
@@ -87,17 +87,9 @@ type ecsImageResolver interface {
}
// ecsIAM is the narrow IAM surface ProvisionCapacity needs to find-or-create the
// ecsInstanceRole, its ecs:* policy, and the instance profile workers expose
// over IMDS. Method signatures match the daemon IAM service implementation.
type ecsIAM interface {
GetRole(accountID string, input *iam.GetRoleInput) (*iam.GetRoleOutput, error)
CreateRole(accountID string, input *iam.CreateRoleInput) (*iam.CreateRoleOutput, error)
CreatePolicy(accountID string, input *iam.CreatePolicyInput) (*iam.CreatePolicyOutput, error)
AttachRolePolicy(accountID string, input *iam.AttachRolePolicyInput) (*iam.AttachRolePolicyOutput, error)
GetInstanceProfile(accountID string, input *iam.GetInstanceProfileInput) (*iam.GetInstanceProfileOutput, error)
CreateInstanceProfile(accountID string, input *iam.CreateInstanceProfileInput) (*iam.CreateInstanceProfileOutput, error)
AddRoleToInstanceProfile(accountID string, input *iam.AddRoleToInstanceProfileInput) (*iam.AddRoleToInstanceProfileOutput, error)
}
// ecsInstanceRole, its inline policy, and the instance profile workers expose
// over IMDS. It is the same shape the ELBv2 and EKS system roles use.
type ecsIAM = handlers_iam.SystemInstanceRoleEnsurer
// Deps are the collaborators ProvisionCapacity needs: the gateway endpoint/CA to
// seed the agent's config, the IAM service to back the instance profile, the
spinifex/handlers/ecs/service_test.go
+4 -4
View File
@@ -449,7 +449,7 @@ func TestScheduler_ReapBucket_StopsStaleInstanceTasks(t *testing.T) {
// Leader election: first holder wins Create; a second holder is rejected.
func TestScheduler_AcquireLease_SingleLeader(t *testing.T) {
_, nc, _ := testutil.StartTestJetStream(t)
svc := NewService(nc, testRegion, "")
svc := NewService(nc, testRegion, "").WithDeps(Deps{IAM: newStubIAM()})
a := NewScheduler(nc, svc, "holder-a")
b := NewScheduler(nc, svc, "holder-b")
require.True(t, a.lease.TryAcquire(t.Context()))
@@ -462,7 +462,7 @@ func TestScheduler_AcquireLease_SingleLeader(t *testing.T) {
// conversion could introduce and the one thing kvlease cannot check itself.
func TestScheduler_LeadershipWiresBusSubscriptions(t *testing.T) {
_, nc, _ := testutil.StartTestJetStream(t)
svc := NewService(nc, testRegion, "")
svc := NewService(nc, testRegion, "").WithDeps(Deps{IAM: newStubIAM()})
sc := NewScheduler(nc, svc, "holder-a")
require.NoError(t, sc.leaseErr)
@@ -479,7 +479,7 @@ func TestScheduler_LeadershipWiresBusSubscriptions(t *testing.T) {
// and loser writing the same KV records is the split-brain this lease prevents.
func TestScheduler_LoserHoldsNoSubscriptions(t *testing.T) {
_, nc, _ := testutil.StartTestJetStream(t)
svc := NewService(nc, testRegion, "")
svc := NewService(nc, testRegion, "").WithDeps(Deps{IAM: newStubIAM()})
a := NewScheduler(nc, svc, "holder-a")
b := NewScheduler(nc, svc, "holder-b")
@@ -491,7 +491,7 @@ func TestScheduler_LoserHoldsNoSubscriptions(t *testing.T) {
func TestScheduler_RunReleasesLeaseOnShutdown(t *testing.T) {
_, nc, js := testutil.StartTestJetStream(t)
svc := NewService(nc, testRegion, "")
svc := NewService(nc, testRegion, "").WithDeps(Deps{IAM: newStubIAM()})
sc := NewScheduler(nc, svc, "holder-a")
require.NoError(t, sc.leaseErr)
spinifex/handlers/iam/roles.go
+10 -1
View File
@@ -362,10 +362,17 @@ func (s *IAMServiceImpl) PutRolePolicy(accountID string, input *iam.PutRolePolic
return nil, errors.New(awserrors.ErrorIAMMalformedPolicyDocument)
}
// An unchanged document writes nothing: converge loops re-assert the same
// policy on every pass, and each write would burn a KV revision.
changed := false
err := s.updateRoleCAS(ctx, accountID, roleName, func(role *Role) (bool, error) {
if role.InlinePolicies == nil {
role.InlinePolicies = map[string]string{}
}
changed = role.InlinePolicies[policyName] != policyDoc
if !changed {
return false, nil
}
role.InlinePolicies[policyName] = policyDoc
return true, nil
})
@@ -373,7 +380,9 @@ func (s *IAMServiceImpl) PutRolePolicy(accountID string, input *iam.PutRolePolic
return nil, err
}
slog.Info("IAM inline policy put on role", "accountID", accountID, "roleName", roleName, "policyName", policyName)
if changed {
slog.Info("IAM inline policy put on role", "accountID", accountID, "roleName", roleName, "policyName", policyName)
}
return &iam.PutRolePolicyOutput{}, nil
}
spinifex/handlers/iam/system_role.go
+20
View File
@@ -43,6 +43,26 @@ func EnsureSystemInstanceProfile(e SystemInstanceRoleEnsurer, accountID, roleNam
return ensureSystemInstanceProfile(e, accountID, roleName)
}
// ConvergeSystemRolePolicy re-asserts a system role's inline policy on an
// account that already carries the role. An absent role is a no-op: nothing
// there draws credentials from it, so provisioning one would be unasked-for.
func ConvergeSystemRolePolicy(e SystemInstanceRoleEnsurer, accountID, roleName, policyName, policyDoc string) error {
if _, err := e.GetRole(accountID, &iam.GetRoleInput{RoleName: aws.String(roleName)}); err != nil {
if err.Error() == awserrors.ErrorIAMNoSuchEntity {
return nil
}
return fmt.Errorf("get role %q: %w", roleName, err)
}
if _, err := e.PutRolePolicy(accountID, &iam.PutRolePolicyInput{
RoleName: aws.String(roleName),
PolicyName: aws.String(policyName),
PolicyDocument: aws.String(policyDoc),
}); err != nil {
return fmt.Errorf("put role policy %q on %q: %w", policyName, roleName, err)
}
return nil
}
// ensureSystemRole creates the role with the EC2 trust policy when absent. A
// racing create that lost (EntityAlreadyExists) is tolerated as success.
func ensureSystemRole(e SystemInstanceRoleEnsurer, accountID, roleName string) error {
spinifex/handlers/iam/system_role_test.go
+38
View File
@@ -49,3 +49,41 @@ func TestEnsureSystemInstanceProfile_Idempotent(t *testing.T) {
assert.Equal(t, 1, f.CreateInstanceProfileCalls, "profile created once")
assert.Equal(t, 1, f.AddRoleToInstanceProfileCalls, "role attached once")
}
// TestConvergeSystemRolePolicy_RewritesExistingRole asserts an account that
// already carries the role picks up a changed document.
func TestConvergeSystemRolePolicy_RewritesExistingRole(t *testing.T) {
f := iammock.New()
_, err := handlers_iam.EnsureSystemInstanceProfile(f, testRoleAcct, testRoleName, testPolicyName, testPolicyDoc)
require.NoError(t, err)
const updated = `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":["svc:DoThing","sts:AssumeRole"],"Resource":"*"}]}`
require.NoError(t, handlers_iam.ConvergeSystemRolePolicy(f, testRoleAcct, testRoleName, testPolicyName, updated))
assert.Equal(t, updated, f.RolePolicies[testRoleName])
}
// TestConvergeSystemRolePolicy_AbsentRoleIsNotProvisioned is the property that
// keeps a fleet-wide converge pass from writing IAM entities into accounts that
// never launched capacity: no role means nothing to converge.
func TestConvergeSystemRolePolicy_AbsentRoleIsNotProvisioned(t *testing.T) {
f := iammock.New()
require.NoError(t, handlers_iam.ConvergeSystemRolePolicy(f, testRoleAcct, testRoleName, testPolicyName, testPolicyDoc))
assert.Zero(t, f.CreateRoleCalls)
assert.Zero(t, f.CreateInstanceProfileCalls)
assert.Empty(t, f.PolicyCalls)
assert.NotContains(t, f.RolePolicies, testRoleName)
}
// TestConvergeSystemRolePolicy_PropagatesPutFailure keeps a failed write a
// failure: swallowing it reports a converged fleet that is not converged.
func TestConvergeSystemRolePolicy_PropagatesPutFailure(t *testing.T) {
f := iammock.New()
_, err := handlers_iam.EnsureSystemInstanceProfile(f, testRoleAcct, testRoleName, testPolicyName, testPolicyDoc)
require.NoError(t, err)
f.PutRolePolicyErr = assert.AnError
err = handlers_iam.ConvergeSystemRolePolicy(f, testRoleAcct, testRoleName, testPolicyName, testPolicyDoc)
require.Error(t, err)
assert.ErrorIs(t, err, assert.AnError)
}
spinifex/handlers/sts/assume_role.go
+4 -10
View File
@@ -21,7 +21,6 @@ import (
"github.com/aws/aws-sdk-go/service/sts"
"github.com/nats-io/nats.go/jetstream"
"github.com/mulgadc/bluebottle/pkg/auth"
"github.com/mulgadc/spinifex/spinifex/awserrors"
handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam"
"github.com/mulgadc/spinifex/spinifex/utils"
@@ -152,20 +151,15 @@ func (s *STSServiceImpl) AssumeRoleForInstance(accountID, roleARN, instanceID st
// resolves the role, clamps the duration, evaluates the trust policy, and mints credentials.
// principalSource is "" for HTTPS (derived from callerARN), ec2.amazonaws.com for IMDS.
func (s *STSServiceImpl) assumeRoleForCaller(ctx context.Context, callerARN, principalSource, roleARN, sessionName, sourceIdentity string, requestedDuration int64) (*sts.AssumeRoleOutput, error) {
roleAccountID, roleName, err := auth.ParseRoleARN(roleARN)
roleAccountID, role, err := ResolveRoleByARN(s.iamSvc, roleARN)
if err != nil {
return nil, errors.New(awserrors.ErrorValidationError)
}
roleOut, err := s.iamSvc.GetRole(roleAccountID, &iam.GetRoleInput{RoleName: aws.String(roleName)})
if err != nil {
// All misses are masked to AccessDenied, matching AWS and preventing role enumeration.
if err.Error() == awserrors.ErrorIAMNoSuchEntity {
// A miss and a non-canonical ARN are both masked to AccessDenied,
// matching AWS and preventing role enumeration.
if errors.Is(err, ErrRoleUnresolved) {
return nil, errors.New(awserrors.ErrorAccessDenied)
}
return nil, err
}
role := roleOut.Role
duration := requestedDuration
if duration == 0 {
spinifex/handlers/sts/role_arn.go
+43
View File
@@ -0,0 +1,43 @@
package handlers_sts
import (
"errors"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/mulgadc/bluebottle/pkg/auth"
"github.com/mulgadc/spinifex/spinifex/awserrors"
)
// ErrRoleUnresolved reports a caller-supplied role ARN that names no role the
// caller may learn about: either no such role, or an ARN that is not the form
// IAM stored. Both are masked to AccessDenied on the wire.
var ErrRoleUnresolved = errors.New("role ARN does not resolve to a stored role")
// RoleGetter is the IAM surface role resolution needs.
type RoleGetter interface {
GetRole(accountID string, input *iam.GetRoleInput) (*iam.GetRoleOutput, error)
}
// ResolveRoleByARN resolves the role a caller-supplied ARN names and verifies
// the ARN is the one IAM stored, returning the role's account. The lookup
// discards any path in the supplied ARN, so comparing the stored ARN back is
// what stops an invented path reaching a role the ARN does not name.
func ResolveRoleByARN(svc RoleGetter, roleARN string) (string, *iam.Role, error) {
accountID, roleName, err := auth.ParseRoleARN(roleARN)
if err != nil {
return "", nil, errors.New(awserrors.ErrorValidationError)
}
out, err := svc.GetRole(accountID, &iam.GetRoleInput{RoleName: aws.String(roleName)})
if err != nil {
if err.Error() == awserrors.ErrorIAMNoSuchEntity {
return "", nil, ErrRoleUnresolved
}
return "", nil, err
}
if out == nil || out.Role == nil || aws.StringValue(out.Role.Arn) != roleARN {
return "", nil, ErrRoleUnresolved
}
return accountID, out.Role, nil
}
spinifex/handlers/sts/role_arn_test.go
+75
View File
@@ -0,0 +1,75 @@
package handlers_sts_test
import (
"errors"
"testing"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/mulgadc/spinifex/spinifex/awserrors"
handlers_sts "github.com/mulgadc/spinifex/spinifex/handlers/sts"
)
// roleGetterStub answers GetRole from a name-to-stored-ARN map, the way IAM
// keys roles, and reports NoSuchEntity for anything else.
type roleGetterStub map[string]string
func (s roleGetterStub) GetRole(_ string, input *iam.GetRoleInput) (*iam.GetRoleOutput, error) {
arn, ok := s[aws.StringValue(input.RoleName)]
if !ok {
return nil, errors.New(awserrors.ErrorIAMNoSuchEntity)
}
return &iam.GetRoleOutput{Role: &iam.Role{RoleName: input.RoleName, Arn: aws.String(arn)}}, nil
}
// TestResolveRoleByARN pins the comparison every caller-supplied role ARN rests
// on. The lookup discards any path the caller wrote, so an ARN that is not the
// one IAM stored must not reach the role its trailing name happens to match.
func TestResolveRoleByARN(t *testing.T) {
const account = "000000000000"
roles := roleGetterStub{
"admin": "arn:aws:iam::000000000000:role/admin",
"app-worker": "arn:aws:iam::000000000000:role/team/app-worker",
}
t.Run("stored ARN resolves", func(t *testing.T) {
gotAccount, role, err := handlers_sts.ResolveRoleByARN(roles, "arn:aws:iam::000000000000:role/admin")
require.NoError(t, err)
assert.Equal(t, account, gotAccount)
assert.Equal(t, "arn:aws:iam::000000000000:role/admin", aws.StringValue(role.Arn))
})
t.Run("a pathed role resolves by its full stored ARN", func(t *testing.T) {
_, role, err := handlers_sts.ResolveRoleByARN(roles, "arn:aws:iam::000000000000:role/team/app-worker")
require.NoError(t, err)
assert.Equal(t, "arn:aws:iam::000000000000:role/team/app-worker", aws.StringValue(role.Arn))
})
t.Run("an invented path does not reach the role it names", func(t *testing.T) {
// `app-x/admin` looks up `admin`, whose stored ARN carries no such path.
_, _, err := handlers_sts.ResolveRoleByARN(roles, "arn:aws:iam::000000000000:role/app-x/admin")
require.ErrorIs(t, err, handlers_sts.ErrRoleUnresolved)
})
t.Run("a pathed role is not reachable by its pathless ARN", func(t *testing.T) {
_, _, err := handlers_sts.ResolveRoleByARN(roles, "arn:aws:iam::000000000000:role/app-worker")
require.ErrorIs(t, err, handlers_sts.ErrRoleUnresolved)
})
t.Run("a role that does not exist is unresolved", func(t *testing.T) {
_, _, err := handlers_sts.ResolveRoleByARN(roles, "arn:aws:iam::000000000000:role/nobody")
require.ErrorIs(t, err, handlers_sts.ErrRoleUnresolved)
})
t.Run("a malformed ARN is a validation error", func(t *testing.T) {
_, _, err := handlers_sts.ResolveRoleByARN(roles, "not-an-arn")
require.Error(t, err)
assert.NotErrorIs(t, err, handlers_sts.ErrRoleUnresolved)
code, ok := awserrors.ResolveErrorCode(err)
require.True(t, ok)
assert.Equal(t, awserrors.ErrorValidationError, code)
})
}
tests/e2e/ecs/ecs_test.go
+10 -1
View File
@@ -380,7 +380,16 @@ func ensureInstanceProfile(t *testing.T, c *harness.AWSClient) {
})
tolerateExists(t, err, "create-role ecsInstanceRole")
const policy = `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"ecs:*","Resource":"*"}]}`
// Mirrors what the control plane provisions: ecs:*, ECR read, and the
// sts:AssumeRole the agent needs to mint task-role credentials. Seeding less
// than production would leave the harness failing alongside it rather than
// ahead of it.
policy := `{"Version":"2012-10-17","Statement":[` +
`{"Effect":"Allow","Action":"ecs:*","Resource":"*"},` +
`{"Effect":"Allow","Action":["ecr:GetAuthorizationToken","ecr:BatchCheckLayerAvailability",` +
`"ecr:GetDownloadUrlForLayer","ecr:BatchGetImage"],"Resource":"*"},` +
`{"Effect":"Allow","Action":"sts:AssumeRole","Resource":"arn:aws:iam::` +
harness.IAMAccountID(t, c) + `:role/*"}]}`
_, err = c.IAM.PutRolePolicy(&iam.PutRolePolicyInput{
RoleName: aws.String(name),
PolicyName: aws.String("ecsInstanceRolePolicy"),
tests/integration/sts_test.go
+18
View File
@@ -282,6 +282,18 @@ func TestSTSAssumeRoleAndGetCallerIdentity(t *testing.T) {
require.NoError(t, err, "create-role (chain)")
chainedRoleARN := aws.StringValue(chainCreateOut.Role.Arn)
// The chained assume needs both gates: the trust policy above, and an
// identity-side grant on the session's own role. An inline policy keeps
// the teardown below free of managed-policy detaches.
_, err = iamCli.PutRolePolicy(&iam.PutRolePolicyInput{
RoleName: aws.String(stsRoleName),
PolicyName: aws.String("sts-role-chain-assume"),
PolicyDocument: aws.String(fmt.Sprintf(
`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"sts:AssumeRole","Resource":%q}]}`,
chainedRoleARN)),
})
require.NoError(t, err, "put-role-policy (chain)")
chainOut, err := sessionCli.STS.AssumeRole(&sts.AssumeRoleInput{
RoleArn: aws.String(chainedRoleARN),
RoleSessionName: aws.String(stsSessionNameChain),
@@ -365,6 +377,12 @@ func TestSTSAssumeRoleAndGetCallerIdentity(t *testing.T) {
// is unaffected by trust-policy references in either direction.
_, err = iamCli.DeleteRole(&iam.DeleteRoleInput{RoleName: aws.String(stsRoleNameChain)})
require.NoError(t, err, "delete-role (chain)")
// An inline policy is a subordinate entity: DeleteRole rejects until it goes.
_, err = iamCli.DeleteRolePolicy(&iam.DeleteRolePolicyInput{
RoleName: aws.String(stsRoleName),
PolicyName: aws.String("sts-role-chain-assume"),
})
require.NoError(t, err, "delete-role-policy (chain)")
_, err = iamCli.DeleteRole(&iam.DeleteRoleInput{RoleName: aws.String(stsRoleName)})
require.NoError(t, err, "delete-role")
}