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

feat(iam): accept the numeric condition operators on s3:max-keys and aws:EpochTime (#1148)

Browse Source
This commit is contained in:
Julian Sommer
2026-10-01 13:34:48 +10:00
committed by GitHub
parent 04c80f3897
commit 6f2035821f
5 changed files with 55 additions and 12 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.20261001020941-036da4967e1c
github.com/mulgadc/bluebottle v1.21.1-0.20261001033338-1f3267ed0373
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.20261001020941-036da4967e1c h1:oRb9PlvooMenQjKU9dD76/bxUOvJaYi6taxm3famoj4=
github.com/mulgadc/bluebottle v1.21.1-0.20261001020941-036da4967e1c/go.mod h1:LH0MuQcFxet0dDEISlZsuULIYj592eCN2Fv39A6EH5o=
github.com/mulgadc/bluebottle v1.21.1-0.20261001033338-1f3267ed0373 h1:QootF30+HJfaZ1ZAwBsgGZYb8R/XbA8366WdvHMgeZM=
github.com/mulgadc/bluebottle v1.21.1-0.20261001033338-1f3267ed0373/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
@@ -37,6 +37,7 @@ var gatewayDoorKeys = []string{
var allConditionKeys = []string{
iampolicy.KeySourceIP,
iampolicy.KeyS3Prefix,
iampolicy.KeyS3MaxKeys,
iampolicy.KeySecureTransport,
iampolicy.KeyUsername,
iampolicy.KeyPrincipalAccount,
@@ -45,9 +46,10 @@ var allConditionKeys = []string{
iampolicy.KeyPassedToService,
iampolicy.KeyCurrentTime,
iampolicy.KeyEpochTime,
// 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.
// Deliberately outside the allowlist: there is no MFA in the stack, so neither
// key could ever be supplied. They are here to prove the validator says no.
"aws:MultiFactorAuthPresent",
"aws:MultiFactorAuthAge",
}
// One value per operator that the leaf validator accepts, so a rejection can
@@ -69,6 +71,12 @@ var operatorValues = map[string]string{
iampolicy.OpDateLessThanEquals: "2026-01-01T00:00:00Z",
iampolicy.OpDateGreaterThan: "1767225600",
iampolicy.OpDateGreaterThanEquals: "1767225600",
iampolicy.OpNumericEquals: "100",
iampolicy.OpNumericNotEquals: "100",
iampolicy.OpNumericLessThan: "100",
iampolicy.OpNumericLessThanEquals: "1767225600",
iampolicy.OpNumericGreaterThan: "10.5",
iampolicy.OpNumericGreaterThanEquals: "1e3",
// 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",
@@ -77,7 +85,7 @@ var operatorValues = map[string]string{
iampolicy.OpArnNotLike: "arn:aws:iam::000000000001:user/*",
// Operators the evaluator does not implement. Accepting one would store a
// restriction that compares false forever.
"NumericLessThan": "3",
"BinaryEquals": "QmluYXJ5",
}
// withIfExists adds the IfExists form of every operator, so the gate below covers
@@ -113,7 +121,7 @@ func emittedKeys(t *testing.T) map[string]string {
gw := &GatewayConfig{DisableLogging: true, IAMService: &mockIAMService{}}
union := make(map[string]string)
for name, principal := range principals {
r := httptest.NewRequest(http.MethodPost, "/?prefix=home/", nil)
r := httptest.NewRequest(http.MethodPost, "/?prefix=home/&max-keys=100", nil)
r.TLS = &tls.ConnectionState{}
r.RemoteAddr = "10.4.1.9:52344"
// Resolved the way the middleware resolves it, so the gate cannot pass on
@@ -154,9 +162,10 @@ func TestRequestConditionKeys_MatchesTheDoorKeySet(t *testing.T) {
"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.
// s3:prefix and s3:max-keys are the keys the S3 gate adds over this set, and
// they are absent here even when the request carries the parameters.
assert.NotContains(t, emitted, iampolicy.KeyS3Prefix)
assert.NotContains(t, emitted, iampolicy.KeyS3MaxKeys)
}
// The write path and the evaluator are the same allowlist or they are a bug:
spinifex/handlers/iam/service_impl.go
+10 -3
View File
@@ -2414,12 +2414,14 @@ func validateConditionValues(i int, op, key string, values ConditionValue) error
// An IfExists form takes its base operator's values.
base, _ := iampolicy.BaseOperator(op)
for _, v := range values {
// Only the string and ARN operators expand variables; a reference in a
// Bool, Null, IpAddress or date value is already rejected as unparseable below.
// Only the string and ARN operators expand variables; a reference in a Bool,
// Null, IpAddress, date or numeric value is already rejected as unparseable below.
switch base {
case iampolicy.OpIPAddress, iampolicy.OpNotIPAddress, iampolicy.OpBool, iampolicy.OpNull,
iampolicy.OpDateEquals, iampolicy.OpDateNotEquals, iampolicy.OpDateLessThan,
iampolicy.OpDateLessThanEquals, iampolicy.OpDateGreaterThan, iampolicy.OpDateGreaterThanEquals:
iampolicy.OpDateLessThanEquals, iampolicy.OpDateGreaterThan, iampolicy.OpDateGreaterThanEquals,
iampolicy.OpNumericEquals, iampolicy.OpNumericNotEquals, iampolicy.OpNumericLessThan,
iampolicy.OpNumericLessThanEquals, iampolicy.OpNumericGreaterThan, iampolicy.OpNumericGreaterThanEquals:
default:
if err := validatePolicyVariables(i, fmt.Sprintf("Condition %s on key %q", op, key), v); err != nil {
return err
@@ -2448,6 +2450,11 @@ func validateConditionValues(i int, op, key string, values ConditionValue) error
return fmt.Errorf("statement %d: Condition %s on key %q: %q is not an ISO 8601 date or epoch seconds",
i, op, key, v)
}
case iampolicy.OpNumericEquals, iampolicy.OpNumericNotEquals, iampolicy.OpNumericLessThan,
iampolicy.OpNumericLessThanEquals, iampolicy.OpNumericGreaterThan, iampolicy.OpNumericGreaterThanEquals:
if _, err := iampolicy.ParseNumber(v); err != nil {
return fmt.Errorf("statement %d: Condition %s on key %q: %q is not a number", i, op, key, v)
}
}
}
return nil
spinifex/handlers/iam/service_impl_test.go
+27
View File
@@ -2321,6 +2321,25 @@ func TestValidatePolicyDocument_DateOperators(t *testing.T) {
}
}
// Numeric values take an integer or decimal, with an optional sign and exponent,
// as a string or a JSON number, on s3:max-keys and aws:EpochTime.
func TestValidatePolicyDocument_NumericOperators(t *testing.T) {
t.Parallel()
for _, cond := range []string{
`{"NumericLessThanEquals":{"s3:max-keys":"10"}}`,
`{"NumericGreaterThan":{"s3:max-keys":100}}`,
`{"NumericEquals":{"s3:max-keys":["+10","1e1","10.0"]}}`,
`{"NumericNotEquals":{"s3:max-keys":".5"}}`,
`{"NumericLessThan":{"aws:EpochTime":1790856000}}`,
`{"NumericGreaterThanEqualsIfExists":{"s3:max-keys":"-1.5e2"}}`,
`{"Null":{"s3:max-keys":"true"}}`,
} {
_, err := ValidatePolicyDocument(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow",
"Action":"s3:*","Resource":"*","Condition":` + cond + `}]}`)
assert.NoError(t, err, cond)
}
}
func TestValidatePolicyDocument_UnsupportedConditionOperator(t *testing.T) {
t.Parallel()
_, err := ValidatePolicyDocument(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*",
@@ -2443,6 +2462,14 @@ func TestValidatePolicyDocument_RejectsMalformedConditionValues(t *testing.T) {
{"DateEquals variable", `{"DateEquals":{"aws:CurrentTime":"${aws:CurrentTime}"}}`, "not an ISO 8601 date or epoch seconds"},
{"DateLessThanIfExists empty", `{"DateLessThanIfExists":{"aws:EpochTime":""}}`, "not an ISO 8601 date or epoch seconds"},
{"DateGreaterThan on a string key", `{"DateGreaterThan":{"aws:username":"2026"}}`, "is not supported in this release"},
{"NumericLessThan prose", `{"NumericLessThan":{"s3:max-keys":"ten"}}`, "is not a number"},
{"NumericEquals hex", `{"NumericEquals":{"s3:max-keys":"0x0A"}}`, "is not a number"},
{"NumericEquals padded", `{"NumericEquals":{"s3:max-keys":" 10"}}`, "is not a number"},
{"NumericEquals variable", `{"NumericEquals":{"aws:EpochTime":"${aws:EpochTime}"}}`, "is not a number"},
{"NumericGreaterThanIfExists empty", `{"NumericGreaterThanIfExists":{"s3:max-keys":""}}`, "is not a number"},
{"NumericLessThan scale overflow", `{"NumericLessThan":{"s3:max-keys":"1e2147483648"}}`, "is not a number"},
{"NumericLessThan on a date key", `{"NumericLessThan":{"aws:CurrentTime":"10"}}`, "is not supported in this release"},
{"NumericLessThan on MFA age", `{"NumericLessThan":{"aws:MultiFactorAuthAge":"3600"}}`, "is not supported in this release"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {