diff --git a/go.mod b/go.mod index 51c2dbea5..fc8f0c8b4 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/go.sum b/go.sum index b5a688c67..362930951 100644 --- a/go.sum +++ b/go.sum @@ -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= diff --git a/spinifex/gateway/conditionkeys_completeness_test.go b/spinifex/gateway/conditionkeys_completeness_test.go index 213669ad8..a3970ce55 100644 --- a/spinifex/gateway/conditionkeys_completeness_test.go +++ b/spinifex/gateway/conditionkeys_completeness_test.go @@ -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: diff --git a/spinifex/handlers/iam/service_impl.go b/spinifex/handlers/iam/service_impl.go index 10a01251c..5ffb0d2cf 100644 --- a/spinifex/handlers/iam/service_impl.go +++ b/spinifex/handlers/iam/service_impl.go @@ -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 diff --git a/spinifex/handlers/iam/service_impl_test.go b/spinifex/handlers/iam/service_impl_test.go index 4ababa305..012a7907a 100644 --- a/spinifex/handlers/iam/service_impl_test.go +++ b/spinifex/handlers/iam/service_impl_test.go @@ -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) {