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

feat(iam): accept negated, case-insensitive and NotIpAddress condition operators (#1143)

Browse Source
This commit is contained in:
Julian Sommer
2026-10-01 10:49:51 +10:00
committed by GitHub
parent c7513c0394
commit 2230e92ec2
5 changed files with 73 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
go.mod
+1 -1
View File
@@ -23,7 +23,7 @@ require (
github.com/klauspost/cpuid/v2 v2.4.0
github.com/miekg/dns v1.1.73
github.com/minio/highwayhash v1.0.4
github.com/mulgadc/bluebottle v1.21.1-0.20260930235459-f23b29cd08db
github.com/mulgadc/bluebottle v1.21.1-0.20261001004744-156519746c8a
github.com/mulgadc/northstar v1.21.0
github.com/mulgadc/predastore v1.21.0
github.com/mulgadc/viperblock v1.21.0
go.sum
+2 -2
View File
@@ -312,8 +312,8 @@ github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELU
github.com/muesli/cancelreader v0.2.2/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo=
github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc=
github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk=
github.com/mulgadc/bluebottle v1.21.1-0.20260930235459-f23b29cd08db h1:FyAx2PMpiCz/0xokJQOn8FWkZXyO4wKX965h000cMcs=
github.com/mulgadc/bluebottle v1.21.1-0.20260930235459-f23b29cd08db/go.mod h1:LH0MuQcFxet0dDEISlZsuULIYj592eCN2Fv39A6EH5o=
github.com/mulgadc/bluebottle v1.21.1-0.20261001004744-156519746c8a h1:Qoycvq0OWPk+KpN4Gb+dG3plkWx4AGGJOJWz0ZJsuSM=
github.com/mulgadc/bluebottle v1.21.1-0.20261001004744-156519746c8a/go.mod h1:LH0MuQcFxet0dDEISlZsuULIYj592eCN2Fv39A6EH5o=
github.com/mulgadc/northstar v1.21.0 h1:LE/5kiT3kBelCg02cXa3b7awhaYkGHsOXeJ4nHRLNaQ=
github.com/mulgadc/northstar v1.21.0/go.mod h1:O446cApNMB6qebqieBhDZfCIGVWX++v3QdOz+wIGhUk=
github.com/mulgadc/predastore v1.21.0 h1:pAm4fFWf0vnlwRgnoL2oJ+KSCw8mtjT2rQoBHFmnjzk=
spinifex/gateway/conditionkeys_completeness_test.go
+15 -6
View File
@@ -49,14 +49,23 @@ var allConditionKeys = []string{
// 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",
iampolicy.OpStringEquals: "alice",
iampolicy.OpStringNotEquals: "alice",
iampolicy.OpStringEqualsIgnoreCase: "alice",
iampolicy.OpStringNotEqualsIgnoreCase: "alice",
iampolicy.OpStringLike: "ali*",
iampolicy.OpStringNotLike: "ali*",
iampolicy.OpIPAddress: "10.0.0.0/8",
iampolicy.OpNotIPAddress: "10.0.0.0/8",
iampolicy.OpBool: "true",
// Implemented, but no supported key is ARN-valued, so every pair below is
// rejected until one is.
iampolicy.OpArnEquals: "arn:aws:iam::000000000001:user/alice",
iampolicy.OpArnLike: "arn:aws:iam::000000000001:user/*",
iampolicy.OpArnNotEquals: "arn:aws:iam::000000000001:user/alice",
iampolicy.OpArnNotLike: "arn:aws:iam::000000000001:user/*",
// 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",
}
spinifex/handlers/iam/service_impl.go
+11 -4
View File
@@ -2410,15 +2410,22 @@ func validateConditionValues(i int, op, key string, values ConditionValue) error
return fmt.Errorf("statement %d: Condition operator %q on key %q has no value", i, op, key)
}
for _, v := range values {
// Only the string operators expand variables; a reference in a Bool or
// IpAddress value is already rejected as unparseable below.
if op == iampolicy.OpStringEquals || op == iampolicy.OpStringLike {
// Only the string and ARN operators expand variables; a reference in a
// Bool or IpAddress value is already rejected as unparseable below.
switch op {
case iampolicy.OpIPAddress, iampolicy.OpNotIPAddress, iampolicy.OpBool:
default:
if err := validatePolicyVariables(i, fmt.Sprintf("Condition %s on key %q", op, key), v); err != nil {
return err
}
}
switch op {
case iampolicy.OpIPAddress:
case iampolicy.OpArnEquals, iampolicy.OpArnLike, iampolicy.OpArnNotEquals, iampolicy.OpArnNotLike:
if _, ok := iampolicy.SplitARN(v); !ok {
return fmt.Errorf("statement %d: Condition %s on key %q: %q is not an ARN of six colon-separated components",
i, op, key, v)
}
case iampolicy.OpIPAddress, iampolicy.OpNotIPAddress:
if _, prefixErr := netip.ParsePrefix(v); prefixErr != nil {
if _, addrErr := netip.ParseAddr(v); addrErr != nil {
return fmt.Errorf("statement %d: Condition %s on key %q: %q is not a valid IP address or CIDR block",
spinifex/handlers/iam/service_impl_test.go
+44
View File
@@ -9,6 +9,7 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/mulgadc/bluebottle/pkg/iampolicy"
"github.com/mulgadc/spinifex/spinifex/admin"
"github.com/mulgadc/spinifex/spinifex/awserrors"
"github.com/mulgadc/spinifex/spinifex/testutil"
@@ -2392,6 +2393,8 @@ func TestValidatePolicyDocument_RejectsMalformedConditionValues(t *testing.T) {
{"numeric bool", `{"Bool":{"aws:SecureTransport":1}}`, "not true or false"},
{"yes", `{"Bool":{"aws:SecureTransport":"yes"}}`, "not true or false"},
{"empty array", `{"IpAddress":{"aws:SourceIp":[]}}`, "has no value"},
{"NotIpAddress hostname", `{"NotIpAddress":{"aws:SourceIp":"office.example.com"}}`, "not a valid IP address or CIDR block"},
{"NotIpAddress empty array", `{"NotIpAddress":{"aws:SourceIp":[]}}`, "has no value"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@@ -2404,6 +2407,38 @@ func TestValidatePolicyDocument_RejectsMalformedConditionValues(t *testing.T) {
}
}
// No supported key carries an Arn operator yet, so the write path cannot reach
// this arm; it is exercised directly so the key that unlocks it lands validated.
func TestValidateConditionValues_ArnOperators(t *testing.T) {
t.Parallel()
const key = "aws:SourceArn"
tests := []struct {
name string
value string
wantErr string
}{
{"exact ARN", "arn:aws:ecs:us-east-1:111122223333:task/c/abc", ""},
{"wildcard components", "arn:aws:ecs:*:111122223333:*", ""},
{"resource with colons", "arn:aws:logs:us-east-1:111122223333:log-group:app:*", ""},
{"variable", "arn:aws:iam::${aws:PrincipalAccount}:role/*", ""},
{"too few components", "arn:aws:ecs:*", "not an ARN of six colon-separated components"},
{"bare wildcard", "*", "not an ARN of six colon-separated components"},
{"unknown variable", "arn:aws:iam::${aws:bogus}:role/*", `references policy variable "aws:bogus"`},
}
for _, op := range []string{
iampolicy.OpArnEquals, iampolicy.OpArnLike, iampolicy.OpArnNotEquals, iampolicy.OpArnNotLike,
} {
for _, tt := range tests {
err := validateConditionValues(0, op, key, ConditionValue{tt.value})
if tt.wantErr == "" {
assert.NoError(t, err, "%s %s", op, tt.name)
continue
}
assert.ErrorContains(t, err, tt.wantErr, "%s %s", op, tt.name)
}
}
}
// A reference the evaluator cannot resolve makes an Allow select nothing and a
// Deny select everything it named, so the policy does not do what it reads as.
// Rejecting it on write is the only place an operator can be told.
@@ -2422,6 +2457,15 @@ func TestValidatePolicyDocument_RejectsUnresolvableVariables(t *testing.T) {
{"MFA in a StringLike value",
`"Action":"s3:*","Resource":"*","Condition":{"StringLike":{"s3:prefix":"${aws:MultiFactorAuthPresent}/*"}}`,
`references policy variable "aws:MultiFactorAuthPresent"`},
{"MFA in a StringNotEquals value",
`"Action":"s3:*","Resource":"*","Condition":{"StringNotEquals":{"aws:username":"${aws:MultiFactorAuthPresent}"}}`,
`references policy variable "aws:MultiFactorAuthPresent"`},
{"MFA in a StringNotLike value",
`"Action":"s3:*","Resource":"*","Condition":{"StringNotLike":{"s3:prefix":"${aws:MultiFactorAuthPresent}/*"}}`,
`references policy variable "aws:MultiFactorAuthPresent"`},
{"unterminated in a StringEqualsIgnoreCase value",
`"Action":"s3:*","Resource":"*","Condition":{"StringEqualsIgnoreCase":{"aws:username":"${aws:username"}}`,
"unterminated policy variable reference"},
{"unknown key", `"Action":"s3:*","Resource":"arn:aws:s3:::home/${aws:bogus}/*"`,
`references policy variable "aws:bogus"`},
{"condition key that is not substitutable",