mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 09:01:31 +00:00
fix: reject and enforce IAM Condition blocks on identity policies (#864)
This commit is contained in:
Julian Sommer
@@ -21,7 +21,7 @@ require (
|
||||
github.com/jackc/pgx/v5 v5.10.0
|
||||
github.com/klauspost/cpuid/v2 v2.4.0
|
||||
github.com/miekg/dns v1.1.73
|
||||
github.com/mulgadc/bluebottle v1.17.0
|
||||
github.com/mulgadc/bluebottle v1.17.1-0.20260824040243-7b109e8b0ca1
|
||||
github.com/mulgadc/northstar v1.17.0
|
||||
github.com/mulgadc/predastore v1.17.1-0.20260820015438-7883d5073cf0
|
||||
github.com/mulgadc/viperblock v1.17.1-0.20260819232234-77b4fbe47481
|
||||
@@ -62,10 +62,10 @@ require (
|
||||
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.37 // indirect
|
||||
github.com/aws/aws-sdk-go-v2/internal/v4a v1.4.38 // indirect
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.17 // indirect
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.9.29 // indirect
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.9.30 // indirect
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.37 // indirect
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.19.37 // indirect
|
||||
github.com/aws/aws-sdk-go-v2/service/s3 v1.107.1 // indirect
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.19.38 // indirect
|
||||
github.com/aws/aws-sdk-go-v2/service/s3 v1.107.2 // indirect
|
||||
github.com/aws/aws-sdk-go-v2/service/signin v1.5.6 // indirect
|
||||
github.com/aws/aws-sdk-go-v2/service/sso v1.33.6 // indirect
|
||||
github.com/aws/aws-sdk-go-v2/service/ssooidc v1.38.6 // indirect
|
||||
@@ -74,7 +74,7 @@ require (
|
||||
github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect
|
||||
github.com/beorn7/perks v1.0.1 // indirect
|
||||
github.com/boltdb/bolt v1.3.1 // indirect
|
||||
github.com/buraksezer/consistent v0.10.0 // indirect
|
||||
github.com/buraksezer/consistent v1.0.0 // indirect
|
||||
github.com/cenkalti/backoff/v5 v5.0.3 // indirect
|
||||
github.com/cenkalti/hub v1.0.2 // indirect
|
||||
github.com/cenkalti/rpc2 v1.0.5 // indirect
|
||||
@@ -134,7 +134,7 @@ require (
|
||||
github.com/jmespath/go-jmespath v0.4.0 // indirect
|
||||
github.com/josharian/native v1.1.0 // indirect
|
||||
github.com/klauspost/compress v1.19.2 // indirect
|
||||
github.com/klauspost/reedsolomon v1.14.1 // indirect
|
||||
github.com/klauspost/reedsolomon v1.14.2 // indirect
|
||||
github.com/leodido/go-urn v1.4.0 // indirect
|
||||
github.com/lithammer/fuzzysearch v1.1.8 // indirect
|
||||
github.com/lucasb-eyer/go-colorful v1.3.0 // indirect
|
||||
|
||||
@@ -562,17 +562,17 @@ github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.1/go.mod h1:
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.3/go.mod h1:0yKJC/kb8sAnmlYa6Zs3QVYqaC8ug2AbnNChv5Ox3uA=
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.17 h1:OvYZOB3qA6zvfdRFiRFRzVSiElMYrz3GdntkXZxlp1o=
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.13.17/go.mod h1:JgR/2Ew50ACfIWau1oeMRX59tMtC0kM+PYQGEaT04cY=
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.9.29 h1:E65Hj648dOV6FuUfI0mYXXhQRHbsi7n+B9h6fZPJO/E=
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.9.29/go.mod h1:xLrF9yNTCs92VZSpdEd68EJbgcdw3SMR74RO6QDzWHE=
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.9.30 h1:5437eMoOwqqQpZn2XJy74mlDCuPYL81texMT3mXqgtU=
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.9.30/go.mod h1:xfu2m3dOpvW8lj98wQYa8V9ku/Rta59hsbireGzhh3A=
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.9.23/go.mod h1:9uPh+Hrz2Vn6oMnQYiUi/zbh3ovbnQk19YKINkQny44=
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.2/go.mod h1:Ru7vg1iQ7cR4i7SZ/JTLYN9kaXtbL69UdgG0OQWQxW0=
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.15/go.mod h1:SwFBy2vjtA0vZbjjaFtfN045boopadnoVPhu4Fv66vY=
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.37 h1:a3D4AjrOrTrP8+d9ILBthqrElf0z1JNol09Xvnwcys8=
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.13.37/go.mod h1:ky0gTu+ukvUTuUKFIpp6Wid4oninrkCyvbFkVs0kpHM=
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.19.37 h1:KGHa9iZCrgtkOsFfXb0S4ywsjostA/hau7WE9aSb43E=
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.19.37/go.mod h1:FV79f0DSnZIEGsQjWenENGtUycrasyAaJZO+zRanLHA=
|
||||
github.com/aws/aws-sdk-go-v2/service/s3 v1.107.1 h1:VUTtUJMuRNMkb/7NIKmd8NQaeQLPGCMoTJxkYKre4qM=
|
||||
github.com/aws/aws-sdk-go-v2/service/s3 v1.107.1/go.mod h1:WvUaO0lP5GNMs1R6cs6qvB3mqo16GLta8yfOuf55Rpc=
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.19.38 h1:gX8B8y3Ho30B1LPxefDKMi/HZqWEb47U9ogs3DtSG0M=
|
||||
github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.19.38/go.mod h1:l5WblZlcmGPe4/O7JY2HO25Z+xqTBvyfTyFbRMf8gYw=
|
||||
github.com/aws/aws-sdk-go-v2/service/s3 v1.107.2 h1:GNU0/xtPEXMKilJZ/a8BedeuQnvu+Usi6qVm9EFfncc=
|
||||
github.com/aws/aws-sdk-go-v2/service/s3 v1.107.2/go.mod h1:4jYWUecEsQtE73jPl7p3jrbYXH5ffcR4gegyCygagfg=
|
||||
github.com/aws/aws-sdk-go-v2/service/signin v1.5.6 h1:i68sFvXidKlkiSvI7d7Ilc1/UvW4CtBOaivH7jhG4fs=
|
||||
github.com/aws/aws-sdk-go-v2/service/signin v1.5.6/go.mod h1:/h7Obr9WTtzbjTHGASRQwLN7Bupw+TC3x8x7fyx39hE=
|
||||
github.com/aws/aws-sdk-go-v2/service/sso v1.12.3/go.mod h1:jtLIhd+V+lft6ktxpItycqHqiVXrPIRjWIsFIlzMriw=
|
||||
@@ -610,8 +610,8 @@ github.com/boltdb/bolt v1.3.1 h1:JQmyP4ZBrce+ZQu0dY660FMfatumYDLun9hBCUVIkF4=
|
||||
github.com/boltdb/bolt v1.3.1/go.mod h1:clJnj/oiGkjum5o1McbSZDSLxVThjynRyGBgiAx27Ps=
|
||||
github.com/boombuler/barcode v1.0.0/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8=
|
||||
github.com/boombuler/barcode v1.0.1/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8=
|
||||
github.com/buraksezer/consistent v0.10.0 h1:hqBgz1PvNLC5rkWcEBVAL9dFMBWz6I0VgUCW25rrZlU=
|
||||
github.com/buraksezer/consistent v0.10.0/go.mod h1:6BrVajWq7wbKZlTOUPs/XVfR8c0maujuPowduSpZqmw=
|
||||
github.com/buraksezer/consistent v1.0.0 h1:ggz8fN8QW95rM4D+NlNMSUUCtoubMra3x/4fmFSv+fo=
|
||||
github.com/buraksezer/consistent v1.0.0/go.mod h1:L/Bczo4XsqEvJLOadAlIhamyNMhIj0iGt0+exc/KeV8=
|
||||
github.com/campoy/embedmd v1.0.0/go.mod h1:oxyr9RCiSXg0M3VJ3ks0UGfp98BpSSGr0kpiX3MzVl8=
|
||||
github.com/casbin/casbin/v2 v2.1.2/go.mod h1:YcPU1XXisHhLzuxH9coDNf2FbKpjGlbCg3n9yuLkIJQ=
|
||||
github.com/cenkalti/backoff v2.2.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM=
|
||||
@@ -1223,8 +1223,8 @@ github.com/klauspost/cpuid/v2 v2.2.3/go.mod h1:RVVoqg1df56z8g3pUjL/3lE5UfnlrJX8t
|
||||
github.com/klauspost/cpuid/v2 v2.2.5/go.mod h1:Lcz8mBdAVJIBVzewtcLocK12l3Y+JytZYpaMropDUws=
|
||||
github.com/klauspost/cpuid/v2 v2.4.0 h1:S6Hrbc7+ywsr0r+RLapfGBHfyefhCTwEh3A0tV913Dw=
|
||||
github.com/klauspost/cpuid/v2 v2.4.0/go.mod h1:19jmZ9mjzoF//ddRSUsv0zfBTJWh3QJh9FNxZTMrGxU=
|
||||
github.com/klauspost/reedsolomon v1.14.1 h1:swE9kzyWXD/wVG+l5Pe8bWnQ0giIY7D1GjCBKk3kG2U=
|
||||
github.com/klauspost/reedsolomon v1.14.1/go.mod h1:yjqqjgMTQkBUHSG97/rm4zipffCNbCiZcB3kTqr++sQ=
|
||||
github.com/klauspost/reedsolomon v1.14.2 h1:SafJYwpBBQBI6amHUygcjxZjXeN2HpiENHQDwuPWCCQ=
|
||||
github.com/klauspost/reedsolomon v1.14.2/go.mod h1:yjqqjgMTQkBUHSG97/rm4zipffCNbCiZcB3kTqr++sQ=
|
||||
github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
|
||||
github.com/konsorten/go-windows-terminal-sequences v1.0.3/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
|
||||
github.com/kr/fs v0.1.0/go.mod h1:FFnZGqtBN9Gxj7eW1uZ42v5BccTP0vu6NEaFoC2HwRg=
|
||||
@@ -1327,8 +1327,10 @@ 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.17.0 h1:K+tGfqwI8nnjV2naG5EGpWMO3Y6XpCTl7/5d5HmROiA=
|
||||
github.com/mulgadc/bluebottle v1.17.0/go.mod h1:kBSgbql3n7FY/emDWP2clK0jyEpOaPSCap+wggILwFQ=
|
||||
github.com/mulgadc/bluebottle v1.17.1-0.20260824024913-88edf0ac2a24 h1:FTeGBkbQjbMy28+Ul6GcHPz6g8oJ53mfp2m09HhC6d0=
|
||||
github.com/mulgadc/bluebottle v1.17.1-0.20260824024913-88edf0ac2a24/go.mod h1:1RZGHtu9w1PgxEr6WsnELwQOURjt5il8MNZ2hym+o9o=
|
||||
github.com/mulgadc/bluebottle v1.17.1-0.20260824040243-7b109e8b0ca1 h1:/PSbT+uGrofKToyIgp8RnKr41k/EfpyUv7jm/jiGRYI=
|
||||
github.com/mulgadc/bluebottle v1.17.1-0.20260824040243-7b109e8b0ca1/go.mod h1:KkUyzqfKxyI8PvKR27EVqJnVm4BSbajl1Q/u+PQxAYw=
|
||||
github.com/mulgadc/northstar v1.17.0 h1:bUkcGpKw06FcWyibkqP/O5J2gnXkM8EAtGX1AZjrmzc=
|
||||
github.com/mulgadc/northstar v1.17.0/go.mod h1:Qg559eZgF8GIsxhSyfDaZCnvhg1Ml66/nnsAQxE9ybg=
|
||||
github.com/mulgadc/predastore v1.17.1-0.20260820015438-7883d5073cf0 h1:AOe9Q6lwosNOnyE/KbagIcuAnmPjwDUUPmkZ1w+WvFE=
|
||||
|
||||
@@ -0,0 +1,191 @@
|
||||
package gateway
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/tls"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/mulgadc/bluebottle/pkg/iampolicy"
|
||||
"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"
|
||||
)
|
||||
|
||||
func TestRequestConditionKeys_PopulatesAvailableKeys(t *testing.T) {
|
||||
r := httptest.NewRequest(http.MethodPost, "/", nil)
|
||||
r.TLS = &tls.ConnectionState{}
|
||||
r.RemoteAddr = "10.4.1.9:52344"
|
||||
principal := principalContext{
|
||||
identity: "alice",
|
||||
accountID: "000000000001",
|
||||
principalType: principalTypeUser,
|
||||
}
|
||||
|
||||
keys := requestConditionKeys(r, principal)
|
||||
|
||||
assert.Equal(t, iampolicy.ConditionKeys{
|
||||
iampolicy.KeySourceIP: "10.4.1.9",
|
||||
iampolicy.KeySecureTransport: "true",
|
||||
iampolicy.KeyUsername: "alice",
|
||||
iampolicy.KeyPrincipalAccount: "000000000001",
|
||||
}, keys)
|
||||
}
|
||||
|
||||
// RoleSessionName is chosen by the caller of AssumeRole, so aws:username must
|
||||
// stay absent for a role session — otherwise anyone permitted to assume the
|
||||
// role satisfies an aws:username condition just by naming their session.
|
||||
func TestRequestConditionKeys_OmitsUsernameForAssumedRole(t *testing.T) {
|
||||
r := httptest.NewRequest(http.MethodPost, "/", nil)
|
||||
principal := principalContext{
|
||||
identity: "alice",
|
||||
accountID: "000000000001",
|
||||
principalType: principalTypeAssumedRole,
|
||||
assumedRoleARN: "arn:aws:sts::000000000001:assumed-role/SharedOps/alice",
|
||||
}
|
||||
|
||||
keys := requestConditionKeys(r, principal)
|
||||
|
||||
assert.NotContains(t, keys, iampolicy.KeyUsername)
|
||||
assert.Equal(t, "000000000001", keys[iampolicy.KeyPrincipalAccount])
|
||||
}
|
||||
|
||||
// s3:prefix has no meaning on the AWS API path, so a policy conditioned on it
|
||||
// must not fire here even though the same document works at predastore's door.
|
||||
func TestRequestConditionKeys_OmitsS3Prefix(t *testing.T) {
|
||||
r := httptest.NewRequest(http.MethodPost, "/?prefix=home/", nil)
|
||||
keys := requestConditionKeys(r, principalContext{
|
||||
identity: "alice",
|
||||
principalType: principalTypeUser,
|
||||
})
|
||||
|
||||
assert.NotContains(t, keys, iampolicy.KeyS3Prefix)
|
||||
assert.Equal(t, "false", keys[iampolicy.KeySecureTransport])
|
||||
}
|
||||
|
||||
// An empty value would compare as a real value that matches nothing, which on a
|
||||
// Deny widens access instead of narrowing it. Absent is the only safe reading.
|
||||
func TestRequestConditionKeys_OmitsEmptyValues(t *testing.T) {
|
||||
r := httptest.NewRequest(http.MethodPost, "/", nil)
|
||||
r.RemoteAddr = ""
|
||||
keys := requestConditionKeys(r, principalContext{principalType: principalTypeUser})
|
||||
|
||||
assert.NotContains(t, keys, iampolicy.KeySourceIP)
|
||||
assert.NotContains(t, keys, iampolicy.KeyUsername)
|
||||
assert.NotContains(t, keys, iampolicy.KeyPrincipalAccount)
|
||||
}
|
||||
|
||||
// The OCI registry chain never runs SigV4AuthMiddleware, so the source address
|
||||
// must come from the request itself or every aws:SourceIp condition is inert
|
||||
// on that door while working on the AWS API door.
|
||||
func TestRequestConditionKeys_SourceIPWithoutAuthMiddleware(t *testing.T) {
|
||||
r := httptest.NewRequest(http.MethodPost, "/v2/app/blobs/uploads/", nil)
|
||||
r.RemoteAddr = "10.4.1.9:52344"
|
||||
|
||||
keys := requestConditionKeys(r, principalContext{
|
||||
identity: "alice",
|
||||
principalType: principalTypeUser,
|
||||
})
|
||||
|
||||
assert.Equal(t, "10.4.1.9", keys[iampolicy.KeySourceIP])
|
||||
}
|
||||
|
||||
// conditionedPolicyIAMService serves one statement carrying cond, so a test can
|
||||
// drive a real condition through checkPolicyResources.
|
||||
func conditionedPolicyIAMService(effect string, cond map[string]map[string]handlers_iam.ConditionValue) *policyMockIAMService {
|
||||
docs := []handlers_iam.PolicyDocument{{
|
||||
Version: "2012-10-17",
|
||||
Statement: []handlers_iam.Statement{
|
||||
{Effect: "Allow", Action: handlers_iam.StringOrArr{"*"}, Resource: handlers_iam.StringOrArr{"*"}},
|
||||
{
|
||||
Effect: effect,
|
||||
Action: handlers_iam.StringOrArr{"ec2:*"},
|
||||
Resource: handlers_iam.StringOrArr{"*"},
|
||||
Condition: cond,
|
||||
},
|
||||
},
|
||||
}}
|
||||
if effect == "Allow" {
|
||||
docs[0].Statement = docs[0].Statement[1:]
|
||||
}
|
||||
return &policyMockIAMService{
|
||||
getUserPoliciesFn: func(_, _ string) ([]handlers_iam.PolicyDocument, error) { return docs, nil },
|
||||
getRolePoliciesFn: func(_, _ string) ([]handlers_iam.PolicyDocument, error) { return docs, nil },
|
||||
}
|
||||
}
|
||||
|
||||
// Proves the keys built from the request actually reach the evaluator. Passing
|
||||
// nil keys from checkPolicyResources would leave every other test green while
|
||||
// silently killing every conditional Allow on this path.
|
||||
func TestCheckPolicyResources_ConditionalAllowHonoursSourceIP(t *testing.T) {
|
||||
gw := &GatewayConfig{
|
||||
DisableLogging: true,
|
||||
IAMService: conditionedPolicyIAMService("Allow", map[string]map[string]handlers_iam.ConditionValue{
|
||||
iampolicy.OpIPAddress: {iampolicy.KeySourceIP: {"10.0.0.0/8"}},
|
||||
}),
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
remote string
|
||||
wantErr string
|
||||
}{
|
||||
{"in range", "10.4.1.9:52344", ""},
|
||||
{"out of range", "192.168.1.1:52344", awserrors.ErrorAccessDenied},
|
||||
{"no source address", "", awserrors.ErrorAccessDenied},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
req := httptest.NewRequest(http.MethodPost, "/", nil)
|
||||
req.RemoteAddr = tt.remote
|
||||
req = withTestIdentity(req)
|
||||
req = req.WithContext(context.WithValue(req.Context(), ctxAccountID, "000000000001"))
|
||||
|
||||
err := gw.checkPolicyResources(req, "ec2", "DescribeInstances", []string{"*"})
|
||||
if tt.wantErr == "" {
|
||||
require.NoError(t, err)
|
||||
return
|
||||
}
|
||||
require.Error(t, err)
|
||||
assert.Equal(t, tt.wantErr, err.Error())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// A Deny conditioned on a key this door cannot supply does not fire, which is
|
||||
// the one direction where an absent key widens access. Pin it so the omission
|
||||
// is a decision rather than an accident.
|
||||
func TestCheckPolicyResources_ConditionalDenyHonoursSourceIP(t *testing.T) {
|
||||
gw := &GatewayConfig{
|
||||
DisableLogging: true,
|
||||
IAMService: conditionedPolicyIAMService("Deny", map[string]map[string]handlers_iam.ConditionValue{
|
||||
iampolicy.OpIPAddress: {iampolicy.KeySourceIP: {"203.0.113.0/24"}},
|
||||
}),
|
||||
}
|
||||
|
||||
for _, tt := range []struct {
|
||||
name string
|
||||
remote string
|
||||
wantErr string
|
||||
}{
|
||||
{"denied range", "203.0.113.5:52344", awserrors.ErrorAccessDenied},
|
||||
{"other range", "10.4.1.9:52344", ""},
|
||||
} {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
req := httptest.NewRequest(http.MethodPost, "/", nil)
|
||||
req.RemoteAddr = tt.remote
|
||||
req = withTestIdentity(req)
|
||||
req = req.WithContext(context.WithValue(req.Context(), ctxAccountID, "000000000001"))
|
||||
|
||||
err := gw.checkPolicyResources(req, "ec2", "DescribeInstances", []string{"*"})
|
||||
if tt.wantErr == "" {
|
||||
require.NoError(t, err)
|
||||
return
|
||||
}
|
||||
require.Error(t, err)
|
||||
assert.Equal(t, tt.wantErr, err.Error())
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -55,6 +55,7 @@ func (gw *GatewayConfig) ecrOperationAuthorization(next http.Handler) http.Handl
|
||||
}
|
||||
if err := gw.evaluatePrincipalPolicyResources(
|
||||
principal, policy.IAMAction("ecr", req.Action), []string{resource},
|
||||
requestConditionKeys(r, principal),
|
||||
); err != nil {
|
||||
if err.Error() == awserrors.ErrorInternalError {
|
||||
// IAM/STS/NATS dependency failure: fail closed, never dispatch.
|
||||
|
||||
@@ -11,6 +11,7 @@ import (
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
@@ -560,7 +561,37 @@ func (gw *GatewayConfig) checkPolicyResources(r *http.Request, service, action s
|
||||
assumedRoleID: mustCtxString(r, ctxAssumedRoleID),
|
||||
underlyingRoleARN: mustCtxString(r, ctxUnderlyingRoleARN),
|
||||
}
|
||||
return gw.evaluatePrincipalPolicyResources(principal, policy.IAMAction(service, action), resources)
|
||||
return gw.evaluatePrincipalPolicyResources(principal, policy.IAMAction(service, action), resources,
|
||||
requestConditionKeys(r, principal))
|
||||
}
|
||||
|
||||
// requestConditionKeys resolves the IAM condition context keys available on the
|
||||
// AWS API path. s3:prefix has no meaning here and is deliberately absent, so a
|
||||
// policy conditioned on it does not fire.
|
||||
//
|
||||
// Every key is omitted rather than set empty when unknown: an empty value reads
|
||||
// as a real value that matches nothing, and on a Deny that silently widens
|
||||
// access instead of narrowing it.
|
||||
func requestConditionKeys(r *http.Request, principal principalContext) iampolicy.ConditionKeys {
|
||||
keys := iampolicy.ConditionKeys{
|
||||
iampolicy.KeySecureTransport: strconv.FormatBool(r.TLS != nil),
|
||||
}
|
||||
// aws:username is user-only in AWS. A role session's identity is the
|
||||
// caller-chosen RoleSessionName, so gating authorization on it here would
|
||||
// let any principal that may assume the role satisfy the condition at will.
|
||||
if principal.principalType == principalTypeUser && principal.identity != "" {
|
||||
keys[iampolicy.KeyUsername] = principal.identity
|
||||
}
|
||||
if principal.accountID != "" {
|
||||
keys[iampolicy.KeyPrincipalAccount] = principal.accountID
|
||||
}
|
||||
// Derived from the request rather than read from the context: the OCI
|
||||
// registry chain never runs SigV4AuthMiddleware, so a context-carried
|
||||
// address would be absent there and every aws:SourceIp condition inert.
|
||||
if ip := utils.ClientIP(r.RemoteAddr); ip != "" {
|
||||
keys[iampolicy.KeySourceIP] = ip
|
||||
}
|
||||
return keys
|
||||
}
|
||||
|
||||
// mustCtxString reads a string context value, defaulting to "" for an absent
|
||||
@@ -573,7 +604,7 @@ func mustCtxString(r *http.Request, key contextKey) string {
|
||||
// evaluatePrincipalPolicyResources resolves policies once and evaluates every
|
||||
// resource in the request against that same snapshot.
|
||||
func (gw *GatewayConfig) evaluatePrincipalPolicyResources(
|
||||
principal principalContext, iamAction string, resources []string,
|
||||
principal principalContext, iamAction string, resources []string, keys iampolicy.ConditionKeys,
|
||||
) error {
|
||||
if gw.IAMService == nil {
|
||||
slog.Error("evaluatePrincipalPolicy: IAM service not available", "action", iamAction)
|
||||
@@ -638,7 +669,7 @@ func (gw *GatewayConfig) evaluatePrincipalPolicyResources(
|
||||
}
|
||||
|
||||
for _, resource := range resources {
|
||||
if iampolicy.Evaluate(iamAction, resource, policies) == iampolicy.Deny {
|
||||
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)
|
||||
|
||||
@@ -37,7 +37,7 @@ func TestEvaluate_IAMInstanceProfileActionStrings(t *testing.T) {
|
||||
doc("Allow", "*", "*"),
|
||||
}
|
||||
for _, a := range actions {
|
||||
if got := iampolicy.Evaluate(a, "*", policies); got != iampolicy.Allow {
|
||||
if got := iampolicy.EvaluateWithKeys(a, "*", policies, nil); got != iampolicy.Allow {
|
||||
t.Errorf("expected Allow for action %q under wildcard policy, got %v", a, got)
|
||||
}
|
||||
}
|
||||
@@ -60,7 +60,7 @@ func TestEvaluate_IAMInstanceProfileActionStrings(t *testing.T) {
|
||||
{"ec2:RunInstances", "*", iampolicy.Deny},
|
||||
}
|
||||
for _, tt := range scopedTests {
|
||||
got := iampolicy.Evaluate(tt.action, tt.resource, scoped)
|
||||
got := iampolicy.EvaluateWithKeys(tt.action, tt.resource, scoped, nil)
|
||||
if got != tt.want {
|
||||
t.Errorf("scoped policy, action=%s resource=%s: expected %v, got %v",
|
||||
tt.action, tt.resource, tt.want, got)
|
||||
@@ -83,17 +83,17 @@ func TestEvaluate_STSActionStrings(t *testing.T) {
|
||||
scoped := []handlers_iam.PolicyDocument{doc("Allow", "sts:*", "*")}
|
||||
|
||||
for _, a := range actions {
|
||||
if got := iampolicy.Evaluate(a, "*", wildcard); got != iampolicy.Allow {
|
||||
if got := iampolicy.EvaluateWithKeys(a, "*", wildcard, nil); got != iampolicy.Allow {
|
||||
t.Errorf("wildcard policy: expected Allow for %q, got %v", a, got)
|
||||
}
|
||||
if got := iampolicy.Evaluate(a, "*", scoped); got != iampolicy.Allow {
|
||||
if got := iampolicy.EvaluateWithKeys(a, "*", scoped, nil); got != iampolicy.Allow {
|
||||
t.Errorf("sts:* policy: expected Allow for %q, got %v", a, got)
|
||||
}
|
||||
}
|
||||
|
||||
// Non-STS action must NOT match an sts:*-scoped policy — guards against a
|
||||
// pattern regression that would over-allow.
|
||||
if got := iampolicy.Evaluate("ec2:RunInstances", "*", scoped); got != iampolicy.Deny {
|
||||
if got := iampolicy.EvaluateWithKeys("ec2:RunInstances", "*", scoped, nil); got != iampolicy.Deny {
|
||||
t.Errorf("sts:* policy: expected Deny for ec2:RunInstances, got %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -219,7 +219,7 @@ func TestResourceARN_ResourceScopedDenyReachesTheSnapshotActions(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
got := iampolicy.Allow
|
||||
for _, resource := range resources {
|
||||
if iampolicy.Evaluate(policy.IAMAction("rds", tt.action), resource, policies) == iampolicy.Deny {
|
||||
if iampolicy.EvaluateWithKeys(policy.IAMAction("rds", tt.action), resource, policies, nil) == iampolicy.Deny {
|
||||
got = iampolicy.Deny
|
||||
}
|
||||
}
|
||||
|
||||
@@ -90,7 +90,7 @@ func TestRDSManagedShims_GrantNoInternalAction(t *testing.T) {
|
||||
doc, ok := builtinManagedPolicyDoc(arn)
|
||||
require.True(t, ok, "arn %q", arn)
|
||||
for _, action := range rdsInternalActions {
|
||||
assert.Equal(t, iampolicy.Deny, iampolicy.Evaluate(action, "*", []PolicyDocument{doc}),
|
||||
assert.Equal(t, iampolicy.Deny, iampolicy.EvaluateWithKeys(action, "*", []PolicyDocument{doc}, nil),
|
||||
"%s must not grant %s", arn, action)
|
||||
}
|
||||
}
|
||||
@@ -112,7 +112,7 @@ func TestRDSFullAccessShim_GrantsTheCustomerSurface(t *testing.T) {
|
||||
"rds:DescribeEvents",
|
||||
}
|
||||
for _, action := range granted {
|
||||
assert.Equal(t, iampolicy.Allow, iampolicy.Evaluate(action, "*", []PolicyDocument{doc}),
|
||||
assert.Equal(t, iampolicy.Allow, iampolicy.EvaluateWithKeys(action, "*", []PolicyDocument{doc}, nil),
|
||||
"AmazonRDSFullAccess must grant %s", action)
|
||||
}
|
||||
}
|
||||
@@ -125,13 +125,13 @@ func TestRDSReadOnlyAccessShim_GrantsReadsOnly(t *testing.T) {
|
||||
|
||||
for _, action := range []string{"rds:DescribeDBInstances", "rds:DescribeDBSnapshots",
|
||||
"rds:DescribeDBParameters", "rds:ListTagsForResource"} {
|
||||
assert.Equal(t, iampolicy.Allow, iampolicy.Evaluate(action, "*", []PolicyDocument{doc}),
|
||||
assert.Equal(t, iampolicy.Allow, iampolicy.EvaluateWithKeys(action, "*", []PolicyDocument{doc}, nil),
|
||||
"AmazonRDSReadOnlyAccess must grant %s", action)
|
||||
}
|
||||
for _, action := range []string{"rds:CreateDBInstance", "rds:DeleteDBInstance",
|
||||
"rds:ModifyDBInstance", "rds:StopDBInstance", "rds:AddTagsToResource",
|
||||
"rds:RestoreDBInstanceFromDBSnapshot"} {
|
||||
assert.Equal(t, iampolicy.Deny, iampolicy.Evaluate(action, "*", []PolicyDocument{doc}),
|
||||
assert.Equal(t, iampolicy.Deny, iampolicy.EvaluateWithKeys(action, "*", []PolicyDocument{doc}, nil),
|
||||
"AmazonRDSReadOnlyAccess must not grant %s", action)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ import (
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"maps"
|
||||
"net/netip"
|
||||
"slices"
|
||||
"strconv"
|
||||
"strings"
|
||||
@@ -19,6 +20,7 @@ import (
|
||||
"github.com/nats-io/nats.go"
|
||||
"github.com/nats-io/nats.go/jetstream"
|
||||
|
||||
"github.com/mulgadc/bluebottle/pkg/iampolicy"
|
||||
"github.com/mulgadc/bluebottle/pkg/masterkey"
|
||||
"github.com/mulgadc/spinifex/spinifex/admin"
|
||||
"github.com/mulgadc/spinifex/spinifex/awserrors"
|
||||
@@ -2016,11 +2018,65 @@ func ValidatePolicyDocument(docJSON string) (*PolicyDocument, error) {
|
||||
if len(stmt.Resource) == 0 {
|
||||
return nil, fmt.Errorf("statement %d: Resource is required", i)
|
||||
}
|
||||
if err := validateStatementRestrictions(i, stmt); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
return &doc, nil
|
||||
}
|
||||
|
||||
// validateStatementRestrictions rejects the clauses the evaluator cannot enforce,
|
||||
// so an identity policy is never accepted with an inert restriction on it.
|
||||
// Conditions inside the supported allowlist are accepted and enforced.
|
||||
func validateStatementRestrictions(i int, stmt Statement) error {
|
||||
if isRawJSONNonEmpty(stmt.Principal) {
|
||||
return fmt.Errorf("statement %d: Principal is not valid on an identity policy; use a resource or trust policy instead", i)
|
||||
}
|
||||
if len(stmt.NotAction) > 0 {
|
||||
return fmt.Errorf("statement %d: NotAction blocks are not supported in this release; use Action with an explicit list instead", i)
|
||||
}
|
||||
if len(stmt.NotResource) > 0 {
|
||||
return fmt.Errorf("statement %d: NotResource blocks are not supported in this release; use Resource with an explicit list instead", i)
|
||||
}
|
||||
for op, keys := range stmt.Condition {
|
||||
for key, values := range keys {
|
||||
if !iampolicy.SupportedCondition(op, key) {
|
||||
return fmt.Errorf("statement %d: Condition operator %q on key %q is not supported in this release", i, op, key)
|
||||
}
|
||||
if err := validateConditionValues(i, op, key, values); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// validateConditionValues rejects leaf values the matcher can only ever compare
|
||||
// false. An allowlisted operator over an unparseable value is still an inert
|
||||
// restriction, which is the failure this validation exists to prevent.
|
||||
func validateConditionValues(i int, op, key string, values ConditionValue) error {
|
||||
if len(values) == 0 {
|
||||
return fmt.Errorf("statement %d: Condition operator %q on key %q has no value", i, op, key)
|
||||
}
|
||||
for _, v := range values {
|
||||
switch op {
|
||||
case iampolicy.OpIPAddress:
|
||||
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",
|
||||
i, op, key, v)
|
||||
}
|
||||
}
|
||||
case iampolicy.OpBool:
|
||||
if !strings.EqualFold(v, "true") && !strings.EqualFold(v, "false") {
|
||||
return fmt.Errorf("statement %d: Condition %s on key %q: %q is not true or false", i, op, key, v)
|
||||
}
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// summaryQuotaDefaults holds the static SummaryMap entries returned by
|
||||
// GetAccountSummary. Spinifex's quota system (handlers/quota) only enforces
|
||||
// infrastructure dimensions (vCPUs, VPCs, subnets, EIPs, EBS) and models no IAM
|
||||
|
||||
@@ -1551,6 +1551,108 @@ func TestValidatePolicyDocument_MissingResource(t *testing.T) {
|
||||
assert.Contains(t, err.Error(), "Resource is required")
|
||||
}
|
||||
|
||||
func TestValidatePolicyDocument_SupportedCondition(t *testing.T) {
|
||||
doc, err := ValidatePolicyDocument(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*",
|
||||
"Condition":{"IpAddress":{"aws:SourceIp":"10.0.0.0/8"}}}]}`)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, ConditionValue{"10.0.0.0/8"},
|
||||
doc.Statement[0].Condition["IpAddress"]["aws:SourceIp"])
|
||||
}
|
||||
|
||||
func TestValidatePolicyDocument_UnsupportedConditionOperator(t *testing.T) {
|
||||
_, err := ValidatePolicyDocument(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*",
|
||||
"Condition":{"StringEquals":{"aws:SourceIp":"10.0.0.1"}}}]}`)
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "is not supported in this release")
|
||||
}
|
||||
|
||||
func TestValidatePolicyDocument_UnsupportedConditionKey(t *testing.T) {
|
||||
_, err := ValidatePolicyDocument(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*",
|
||||
"Condition":{"StringEquals":{"aws:PrincipalOrgID":"o-1234"}}}]}`)
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "aws:PrincipalOrgID")
|
||||
}
|
||||
|
||||
// MFA does not exist in spinifex, so this key could never be true and accepting
|
||||
// it would mint a grant that silently never fires.
|
||||
func TestValidatePolicyDocument_RejectsMFACondition(t *testing.T) {
|
||||
_, err := ValidatePolicyDocument(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"*","Resource":"*",
|
||||
"Condition":{"Bool":{"aws:MultiFactorAuthPresent":"true"}}}]}`)
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "aws:MultiFactorAuthPresent")
|
||||
}
|
||||
|
||||
func TestValidatePolicyDocument_RejectsPrincipal(t *testing.T) {
|
||||
_, err := ValidatePolicyDocument(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"*","Resource":"*",
|
||||
"Principal":{"AWS":"arn:aws:iam::123456789012:root"}}]}`)
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "Principal is not valid on an identity policy")
|
||||
}
|
||||
|
||||
func TestValidatePolicyDocument_RejectsNotAction(t *testing.T) {
|
||||
_, err := ValidatePolicyDocument(`{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Action":"*","Resource":"*",
|
||||
"NotAction":"sts:AssumeRole"}]}`)
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "NotAction blocks are not supported")
|
||||
}
|
||||
|
||||
// An allowlisted operator over a malformed value is still an inert grant: the
|
||||
// matcher cannot parse it, so the Allow never fires and the operator gets a bare
|
||||
// AccessDenied with nothing naming the cause.
|
||||
func TestValidatePolicyDocument_RejectsMalformedConditionValues(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
cond string
|
||||
wantErr string
|
||||
}{
|
||||
{"truncated CIDR", `{"IpAddress":{"aws:SourceIp":"10.0.0/8"}}`, "not a valid IP address or CIDR block"},
|
||||
{"comma string", `{"IpAddress":{"aws:SourceIp":"10.0.0.0/8, 192.168.0.0/16"}}`, "not a valid IP address or CIDR block"},
|
||||
{"hostname", `{"IpAddress":{"aws:SourceIp":"office.example.com"}}`, "not a valid IP address or CIDR block"},
|
||||
{"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"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
_, err := ValidatePolicyDocument(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow",
|
||||
"Action":"s3:*","Resource":"*","Condition":` + tt.cond + `}]}`)
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), tt.wantErr)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// D4's lenient leaf parsing must survive at the API surface: a native JSON bool
|
||||
// is what Terraform emits, and rejecting it would fail ordinary valid policies.
|
||||
func TestValidatePolicyDocument_AcceptsNativeBoolAndCIDRForms(t *testing.T) {
|
||||
for _, cond := range []string{
|
||||
`{"Bool":{"aws:SecureTransport":true}}`,
|
||||
`{"Bool":{"aws:SecureTransport":"TRUE"}}`,
|
||||
`{"IpAddress":{"aws:SourceIp":["10.0.0.0/8","192.168.1.1"]}}`,
|
||||
`{"IpAddress":{"aws:SourceIp":"2001:db8::/32"}}`,
|
||||
} {
|
||||
_, err := ValidatePolicyDocument(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow",
|
||||
"Action":"s3:*","Resource":"*","Condition":` + cond + `}]}`)
|
||||
assert.NoError(t, err, "condition %s must be accepted", cond)
|
||||
}
|
||||
}
|
||||
|
||||
// Map iteration order makes a break-instead-of-continue bug non-deterministic,
|
||||
// so a single-operator document cannot pin the loop.
|
||||
func TestValidatePolicyDocument_RejectsUnsupportedAlongsideSupported(t *testing.T) {
|
||||
_, err := ValidatePolicyDocument(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*",
|
||||
"Condition":{"IpAddress":{"aws:SourceIp":"10.0.0.0/8"},"StringEquals":{"aws:PrincipalOrgID":"o-1234"}}}]}`)
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "aws:PrincipalOrgID")
|
||||
}
|
||||
|
||||
func TestValidatePolicyDocument_RejectsNotResource(t *testing.T) {
|
||||
_, err := ValidatePolicyDocument(`{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Action":"*","Resource":"*",
|
||||
"NotResource":"arn:aws:s3:::public/*"}]}`)
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "NotResource blocks are not supported")
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
// Sensitive Data Not Logged Tests
|
||||
// ============================================================================
|
||||
|
||||
@@ -29,6 +29,7 @@ type (
|
||||
PolicyDocument = iampolicy.PolicyDocument
|
||||
Statement = iampolicy.Statement
|
||||
StringOrArr = iampolicy.StringOrArr
|
||||
ConditionValue = iampolicy.ConditionValue
|
||||
)
|
||||
|
||||
// User represents an IAM user stored in JetStream KV.
|
||||
|
||||
Reference in New Issue
Block a user