mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 09:01:31 +00:00
test: add completeness gates for the ECS, ECR and RDS scope tables and the condition keys
ECS and ECR take the existing both-directions shape against their Actions tables. RDS carries its scopes on the dispatch table itself, so the gate is the account-level half instead: every served action is either scoped or carries a written reason, and one in both sets or neither fails. HasScope and ScopedActions are added to drive it. The condition-key gate pins the door's key set and asserts the IAM write path accepts exactly the operator/key pairs the evaluator enforces.
This commit is contained in:
Julian Sommer
@@ -0,0 +1,160 @@
|
||||
//test:in-package — requestConditionKeys, principalContext and the principal type
|
||||
// constants are unexported, and the gate exists to compare what they emit with
|
||||
// the evaluator's registries.
|
||||
|
||||
package gateway
|
||||
|
||||
import (
|
||||
"crypto/tls"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/mulgadc/bluebottle/pkg/iampolicy"
|
||||
handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// The keys this door resolves. predastore's mirror of this list adds s3:prefix
|
||||
// and nothing else; a key added at one door and not the other fails there.
|
||||
var gatewayDoorKeys = []string{
|
||||
iampolicy.KeySecureTransport,
|
||||
iampolicy.KeyUsername,
|
||||
iampolicy.KeyPrincipalAccount,
|
||||
iampolicy.KeySourceIP,
|
||||
}
|
||||
|
||||
// Every condition key the evaluator names, so the write-path gate below covers
|
||||
// the whole namespace rather than the subset this door happens to supply.
|
||||
var allConditionKeys = []string{
|
||||
iampolicy.KeySourceIP,
|
||||
iampolicy.KeyS3Prefix,
|
||||
iampolicy.KeySecureTransport,
|
||||
iampolicy.KeyUsername,
|
||||
iampolicy.KeyPrincipalAccount,
|
||||
iampolicy.KeyUserID,
|
||||
// Deliberately outside the allowlist: there is no MFA in the stack, so the
|
||||
// key could never be true. It is here to prove the validator says no.
|
||||
"aws:MultiFactorAuthPresent",
|
||||
}
|
||||
|
||||
// One value per operator that the leaf validator accepts, so a rejection can
|
||||
// only come from the operator/key allowlist and not from the value.
|
||||
var operatorValues = map[string]string{
|
||||
iampolicy.OpStringEquals: "alice",
|
||||
iampolicy.OpStringLike: "ali*",
|
||||
iampolicy.OpIPAddress: "10.0.0.0/8",
|
||||
iampolicy.OpBool: "true",
|
||||
// Operators the evaluator does not implement. Accepting one would store a
|
||||
// restriction that compares false forever.
|
||||
"StringNotEquals": "alice",
|
||||
"ArnLike": "arn:aws:iam::000000000001:user/alice",
|
||||
"NumericLessThan": "3",
|
||||
"DateGreaterThan": "2026-01-01T00:00:00Z",
|
||||
}
|
||||
|
||||
// emittedKeys drives requestConditionKeys with everything a request can carry,
|
||||
// for both principal types, and returns the union.
|
||||
func emittedKeys(t *testing.T) map[string]string {
|
||||
t.Helper()
|
||||
|
||||
principals := map[string]principalContext{
|
||||
"user": {
|
||||
identity: "alice",
|
||||
accountID: "000000000001",
|
||||
principalType: principalTypeUser,
|
||||
},
|
||||
"assumed-role": {
|
||||
identity: "session",
|
||||
accountID: "000000000001",
|
||||
principalType: principalTypeAssumedRole,
|
||||
assumedRoleARN: "arn:aws:sts::000000000001:assumed-role/SharedOps/session",
|
||||
},
|
||||
}
|
||||
|
||||
union := make(map[string]string)
|
||||
for name, principal := range principals {
|
||||
r := httptest.NewRequest(http.MethodPost, "/?prefix=home/", nil)
|
||||
r.TLS = &tls.ConnectionState{}
|
||||
r.RemoteAddr = "10.4.1.9:52344"
|
||||
for key := range requestConditionKeys(r, principal) {
|
||||
union[key] = name
|
||||
}
|
||||
}
|
||||
return union
|
||||
}
|
||||
|
||||
// A key this door supplies that no policy can name is carried to the evaluator
|
||||
// and dropped there — the silent half of the write-path/evaluator disagreement.
|
||||
func TestRequestConditionKeys_EveryEmittedKeyIsUsableInAPolicy(t *testing.T) {
|
||||
for key, principal := range emittedKeys(t) {
|
||||
supported := false
|
||||
for op := range operatorValues {
|
||||
if iampolicy.SupportedCondition(op, key) {
|
||||
supported = true
|
||||
break
|
||||
}
|
||||
}
|
||||
_, unresolvable := iampolicy.UnsupportedVariable("${" + key + "}")
|
||||
assert.True(t, supported || !unresolvable,
|
||||
"requestConditionKeys emits %q for a %s principal, but the evaluator neither enforces a "+
|
||||
"condition on it nor substitutes it: a policy naming it can never fire", key, principal)
|
||||
}
|
||||
}
|
||||
|
||||
// The door's key set as a whole, so a key gained or lost here is a deliberate
|
||||
// change made at both doors rather than a drift between them.
|
||||
func TestRequestConditionKeys_MatchesTheDoorKeySet(t *testing.T) {
|
||||
emitted := make([]string, 0, len(gatewayDoorKeys))
|
||||
for key := range emittedKeys(t) {
|
||||
emitted = append(emitted, key)
|
||||
}
|
||||
assert.ElementsMatch(t, gatewayDoorKeys, emitted,
|
||||
"the AWS gateway door key set changed: update predastore's mirror in "+
|
||||
"internal/gate/conditionkeys_completeness_test.go and the door table in bluebottle's door_test.go")
|
||||
|
||||
// s3:prefix is the one key the S3 gate adds over this set, and it is absent
|
||||
// here even when the request carries a prefix parameter.
|
||||
assert.NotContains(t, emitted, iampolicy.KeyS3Prefix)
|
||||
}
|
||||
|
||||
// The write path and the evaluator are the same allowlist or they are a bug:
|
||||
// a pair the validator accepts but the evaluator cannot enforce is stored as an
|
||||
// inert restriction, and one it rejects that the evaluator does enforce is a
|
||||
// policy an operator cannot write.
|
||||
func TestValidatePolicyDocument_AcceptsExactlyTheSupportedConditions(t *testing.T) {
|
||||
for _, key := range allConditionKeys {
|
||||
for op, value := range operatorValues {
|
||||
doc := conditionDocument(t, op, key, value)
|
||||
_, err := handlers_iam.ValidatePolicyDocument(doc)
|
||||
|
||||
if iampolicy.SupportedCondition(op, key) {
|
||||
assert.NoError(t, err,
|
||||
"the evaluator enforces %s on %q but the write path rejects it, so the policy "+
|
||||
"cannot be written", op, key)
|
||||
continue
|
||||
}
|
||||
assert.Error(t, err,
|
||||
"the write path accepts %s on %q, which the evaluator does not enforce: the condition "+
|
||||
"would be stored and then compare false forever", op, key)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func conditionDocument(t *testing.T, op, key, value string) string {
|
||||
t.Helper()
|
||||
|
||||
doc, err := json.Marshal(map[string]any{
|
||||
"Version": "2012-10-17",
|
||||
"Statement": []map[string]any{{
|
||||
"Effect": "Allow",
|
||||
"Action": []string{"s3:GetObject"},
|
||||
"Resource": []string{"arn:aws:s3:::reports/*"},
|
||||
"Condition": map[string]any{op: map[string]any{key: []string{value}}},
|
||||
}},
|
||||
})
|
||||
require.NoError(t, err, "marshalling the %s/%s document", op, key)
|
||||
return string(doc)
|
||||
}
|
||||
@@ -0,0 +1,38 @@
|
||||
//test:in-package — ecrInlineActions is unexported here, and ECR dispatch is the
|
||||
// union of it and the ecrapi namespace.
|
||||
|
||||
package gateway
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
gateway_ecrapi "github.com/mulgadc/spinifex/spinifex/gateway/ecrapi"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
// TestECRScopeTableIsExhaustive is what stops the next ECR action being added
|
||||
// with a silent account-wide grant. It asserts both directions, so a scope left
|
||||
// behind by a deleted or renamed action fails too.
|
||||
func TestECRScopeTableIsExhaustive(t *testing.T) {
|
||||
for action := range gateway_ecrapi.Actions {
|
||||
assert.True(t, gateway_ecrapi.HasScope(action),
|
||||
"ecr action %q has no resource scope entry: add one to ecrScopes in gateway/ecrapi/authz.go", action)
|
||||
}
|
||||
|
||||
for _, action := range gateway_ecrapi.ScopedActions() {
|
||||
_, ok := gateway_ecrapi.Actions[action]
|
||||
assert.True(t, ok,
|
||||
"ecrScopes has an entry for %q, which the dispatch table does not serve: remove it from gateway/ecrapi/authz.go", action)
|
||||
}
|
||||
}
|
||||
|
||||
// ECR_Request resolves the action against the ecrapi namespace before it reaches
|
||||
// the inline table, so an inline handler outside that namespace is unreachable —
|
||||
// and, more to the point, would never have had its resources scoped.
|
||||
func TestECRInlineActionsAreInTheNamespace(t *testing.T) {
|
||||
for action := range ecrInlineActions {
|
||||
_, ok := gateway_ecrapi.Actions[action]
|
||||
assert.True(t, ok,
|
||||
"inline ECR handler %q is not in gateway_ecrapi.Actions, so ECR_Request rejects it as InvalidAction", action)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,24 @@
|
||||
package gateway_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
gateway_ecs "github.com/mulgadc/spinifex/spinifex/gateway/ecs"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
// TestECSScopeTableIsExhaustive is what stops the next ECS action being added
|
||||
// with a silent account-wide grant. It asserts both directions, so a scope left
|
||||
// behind by a deleted or renamed action fails too.
|
||||
func TestECSScopeTableIsExhaustive(t *testing.T) {
|
||||
for action := range gateway_ecs.Actions {
|
||||
assert.True(t, gateway_ecs.HasScope(action),
|
||||
"ecs action %q has no resource scope entry: add one to ecsScopes in gateway/ecs/authz.go", action)
|
||||
}
|
||||
|
||||
for _, action := range gateway_ecs.ScopedActions() {
|
||||
_, ok := gateway_ecs.Actions[action]
|
||||
assert.True(t, ok,
|
||||
"ecsScopes has an entry for %q, which the dispatch table does not serve: remove it from gateway/ecs/authz.go", action)
|
||||
}
|
||||
}
|
||||
@@ -61,6 +61,19 @@ func requireAgentPrincipal(ctx context.Context, caller Caller) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// HasScope reports whether the action's policy check evaluates a resource ARN
|
||||
// rather than the account-wide "*".
|
||||
func HasScope(action string) bool {
|
||||
def, ok := actions[action]
|
||||
return ok && len(def.scopes) > 0
|
||||
}
|
||||
|
||||
// ScopedActions returns every action whose policy check evaluates a resource
|
||||
// ARN, in stable order.
|
||||
func ScopedActions() []string {
|
||||
return actionNames(func(def actionDef) bool { return len(def.scopes) > 0 })
|
||||
}
|
||||
|
||||
// ResourceARN builds every resource the action's policy checks evaluate. A
|
||||
// policy written for arn:aws:rds:*:*:db:prod-* but checked against "*" would
|
||||
// permit every instance in the account, which is worse than no policy at all.
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
package gateway_test
|
||||
|
||||
import (
|
||||
"slices"
|
||||
"testing"
|
||||
|
||||
gateway_rds "github.com/mulgadc/spinifex/spinifex/gateway/rds"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
// RDS carries its scopes on the dispatch table itself, so the two cannot
|
||||
// disagree the way the other services' can. What can still go wrong is an action
|
||||
// landing with no scopes and nobody noticing it now evaluates against "*", so
|
||||
// the gate is the account-level half: every entry here is a written decision
|
||||
// that the action addresses no single resource.
|
||||
var rdsAccountLevelActions = map[string]string{
|
||||
// Creates: the resource does not exist yet, which is how AWS evaluates them.
|
||||
"CreateDBInstance": "the instance does not exist until this call makes it",
|
||||
"CreateDBSubnetGroup": "the subnet group does not exist until this call makes it",
|
||||
"CreateDBParameterGroup": "the parameter group does not exist until this call makes it",
|
||||
|
||||
// Describes that filter rather than address one resource.
|
||||
"DescribeDBInstances": "filters the account's instances",
|
||||
"DescribeDBSnapshots": "filters the account's snapshots",
|
||||
"DescribeDBInstanceAutomatedBackups": "filters the account's automated backups",
|
||||
"DescribeDBSubnetGroups": "filters the account's subnet groups",
|
||||
"DescribeDBParameterGroups": "filters the account's parameter groups",
|
||||
"DescribeEvents": "the event ring is per-account and a filter names no one resource",
|
||||
|
||||
// Catalogs read no per-account state at all.
|
||||
"DescribeDBEngineVersions": "a static engine catalog, not tenant state",
|
||||
"DescribeOrderableDBInstanceOptions": "cluster capability, not tenant state",
|
||||
|
||||
// Internal agent actions. AuthorizeCaller refuses these to every customer
|
||||
// principal by class before any policy is evaluated, so a resource scope
|
||||
// would narrow nothing.
|
||||
"RegisterDBInstance": "agent-only, gated by principal class",
|
||||
"SubmitDBStateChange": "agent-only, gated by principal class",
|
||||
"PollDBCommands": "agent-only, gated by principal class",
|
||||
"GetDBBootstrapConfig": "agent-only, gated by principal class",
|
||||
"AcknowledgeDBBootstrap": "agent-only, gated by principal class",
|
||||
}
|
||||
|
||||
// TestRDSScopeTableIsExhaustive is what stops the next RDS action being added
|
||||
// with a silent account-wide grant. Every served action is either scoped or
|
||||
// carries a written reason for being account-level, and an action in both sets
|
||||
// or in neither fails.
|
||||
func TestRDSScopeTableIsExhaustive(t *testing.T) {
|
||||
unsupported := gateway_rds.UnsupportedActionNames()
|
||||
|
||||
for _, action := range gateway_rds.ActionNames() {
|
||||
if slices.Contains(unsupported, action) {
|
||||
continue
|
||||
}
|
||||
_, accountLevel := rdsAccountLevelActions[action]
|
||||
assert.NotEqual(t, gateway_rds.HasScope(action), accountLevel,
|
||||
"rds action %q must either carry a scope in gateway/rds/handler.go or a written reason "+
|
||||
"in rdsAccountLevelActions, not both and not neither", action)
|
||||
}
|
||||
|
||||
for action := range rdsAccountLevelActions {
|
||||
assert.True(t, gateway_rds.HasAction(action),
|
||||
"rdsAccountLevelActions names %q, which RDS does not serve: remove it", action)
|
||||
assert.NotContains(t, unsupported, action,
|
||||
"rdsAccountLevelActions names %q, which is recognised but not offered: remove it", action)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user