diff --git a/go.mod b/go.mod index fc8f0c8b4..44b8fe953 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.20261001033338-1f3267ed0373 + github.com/mulgadc/bluebottle v1.21.1-0.20261001041718-8a7bd5d4f2e3 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 362930951..772fa382f 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.20261001033338-1f3267ed0373 h1:QootF30+HJfaZ1ZAwBsgGZYb8R/XbA8366WdvHMgeZM= -github.com/mulgadc/bluebottle v1.21.1-0.20261001033338-1f3267ed0373/go.mod h1:LH0MuQcFxet0dDEISlZsuULIYj592eCN2Fv39A6EH5o= +github.com/mulgadc/bluebottle v1.21.1-0.20261001041718-8a7bd5d4f2e3 h1:ICj61S//Rs6Unc+H03kQZ3/kHyEE8CPLljPjLSFjcgc= +github.com/mulgadc/bluebottle v1.21.1-0.20261001041718-8a7bd5d4f2e3/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/handlers/iam/groups.go b/spinifex/handlers/iam/groups.go index 630be0101..c076bc34f 100644 --- a/spinifex/handlers/iam/groups.go +++ b/spinifex/handlers/iam/groups.go @@ -404,6 +404,9 @@ func (s *IAMServiceImpl) PutGroupPolicy(accountID string, input *iam.PutGroupPol if err := validateIAMName("policyName", policyName, 128); err != nil { return nil, err } + if err := checkPolicyDocumentLength(policyDoc); err != nil { + return nil, err + } if _, err := ValidatePolicyDocument(policyDoc); err != nil { return nil, awserrors.Errorf(awserrors.ErrorIAMMalformedPolicyDocument, "policy %q on group %q: %w", policyName, groupName, err) @@ -413,6 +416,9 @@ func (s *IAMServiceImpl) PutGroupPolicy(accountID string, input *iam.PutGroupPol if err != nil { return nil, err } + if err := checkInlinePolicySize("group", groupName, groupPolicySizeQuota, group.InlinePolicies, policyName, policyDoc); err != nil { + return nil, err + } if group.InlinePolicies == nil { group.InlinePolicies = map[string]string{} diff --git a/spinifex/handlers/iam/groups_test.go b/spinifex/handlers/iam/groups_test.go index 3ff901a24..b125ed527 100644 --- a/spinifex/handlers/iam/groups_test.go +++ b/spinifex/handlers/iam/groups_test.go @@ -790,21 +790,6 @@ func TestPutGroupPolicy_MalformedDocument(t *testing.T) { assert.Contains(t, err.Error(), awserrors.ErrorIAMMalformedPolicyDocument) } -func TestPutGroupPolicy_OversizedDocument(t *testing.T) { - t.Parallel() - svc := setupTestIAMService(t) - createTestGroup(t, svc, "oversized-group") - - huge := strings.Repeat("a", maxPolicyDocumentSize+1) - _, err := svc.PutGroupPolicy(testAccountID, &iam.PutGroupPolicyInput{ - GroupName: aws.String("oversized-group"), - PolicyName: aws.String("Huge"), - PolicyDocument: aws.String(huge), - }) - assert.Error(t, err) - assert.Contains(t, err.Error(), awserrors.ErrorIAMMalformedPolicyDocument) -} - func TestPutGroupPolicy_GroupNotFound(t *testing.T) { t.Parallel() svc := setupTestIAMService(t) diff --git a/spinifex/handlers/iam/policy_size_test.go b/spinifex/handlers/iam/policy_size_test.go new file mode 100644 index 000000000..47e6730a6 --- /dev/null +++ b/spinifex/handlers/iam/policy_size_test.go @@ -0,0 +1,228 @@ +package handlers_iam + +import ( + "fmt" + "strings" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/iam" + "github.com/mulgadc/spinifex/spinifex/awserrors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// sizedPolicy returns a valid identity policy whose size, as AWS counts it, is +// exactly size characters, padded through its resource ARN. +func sizedPolicy(t *testing.T, size int) string { + t.Helper() + const format = `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:GetObject","Resource":"arn:aws:s3:::%s"}]}` + base := policySize(fmt.Sprintf(format, "")) + require.GreaterOrEqual(t, size, base) + doc := fmt.Sprintf(format, strings.Repeat("a", size-base)) + require.Equal(t, size, policySize(doc)) + return doc +} + +func TestPolicySize_IgnoresWhiteSpace(t *testing.T) { + t.Parallel() + compact := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]}` + pretty := "{\n \"Version\": \"2012-10-17\",\n\t\"Statement\": [ {\"Effect\": \"Allow\", \"Action\": \"s3:*\", \"Resource\": \"*\"} ]\r\n}" + assert.Equal(t, len(compact), policySize(compact)) + assert.Equal(t, policySize(compact), policySize(pretty)) + assert.Equal(t, 3, policySize("aéb"), "size counts characters, not bytes") +} + +// inlinePutter puts an inline policy on one entity type, for running the same +// quota cases against users, groups and roles. +type inlinePutter struct { + entityType string + quota int + create func(t *testing.T, svc *IAMServiceImpl, name string) + put func(svc *IAMServiceImpl, name, policyName, doc string) error +} + +var inlinePutters = []inlinePutter{ + { + entityType: "user", + quota: 2048, + create: func(t *testing.T, svc *IAMServiceImpl, name string) { createTestUser(t, svc, name) }, + put: func(svc *IAMServiceImpl, name, policyName, doc string) error { + _, err := svc.PutUserPolicy(testAccountID, &iam.PutUserPolicyInput{ + UserName: aws.String(name), PolicyName: aws.String(policyName), PolicyDocument: aws.String(doc), + }) + return err + }, + }, + { + entityType: "group", + quota: 5120, + create: func(t *testing.T, svc *IAMServiceImpl, name string) { createTestGroup(t, svc, name) }, + put: func(svc *IAMServiceImpl, name, policyName, doc string) error { + _, err := svc.PutGroupPolicy(testAccountID, &iam.PutGroupPolicyInput{ + GroupName: aws.String(name), PolicyName: aws.String(policyName), PolicyDocument: aws.String(doc), + }) + return err + }, + }, + { + entityType: "role", + quota: 10240, + create: func(t *testing.T, svc *IAMServiceImpl, name string) { createTestRole(t, svc, name) }, + put: func(svc *IAMServiceImpl, name, policyName, doc string) error { + _, err := svc.PutRolePolicy(testAccountID, &iam.PutRolePolicyInput{ + RoleName: aws.String(name), PolicyName: aws.String(policyName), PolicyDocument: aws.String(doc), + }) + return err + }, + }, +} + +func TestPutInlinePolicy_SingleDocumentQuota(t *testing.T) { + t.Parallel() + for _, p := range inlinePutters { + t.Run(p.entityType, func(t *testing.T) { + t.Parallel() + svc := setupTestIAMService(t) + p.create(t, svc, "sized") + + err := p.put(svc, "sized", "Over", sizedPolicy(t, p.quota+1)) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorIAMLimitExceeded) + assert.Contains(t, err.Error(), fmt.Sprintf("Maximum policy size of %d bytes exceeded for %s sized", p.quota, p.entityType)) + + require.NoError(t, p.put(svc, "sized", "AtQuota", sizedPolicy(t, p.quota))) + }) + } +} + +// The quota applies to the sum of an entity's inline policies. +func TestPutInlinePolicy_AggregateQuota(t *testing.T) { + t.Parallel() + for _, p := range inlinePutters { + t.Run(p.entityType, func(t *testing.T) { + t.Parallel() + svc := setupTestIAMService(t) + p.create(t, svc, "aggregate") + + first := p.quota / 2 + require.NoError(t, p.put(svc, "aggregate", "First", sizedPolicy(t, first))) + + err := p.put(svc, "aggregate", "Second", sizedPolicy(t, p.quota-first+1)) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorIAMLimitExceeded) + + require.NoError(t, p.put(svc, "aggregate", "Second", sizedPolicy(t, p.quota-first))) + }) + } +} + +// Replacing a policy counts only its new document, not the one it replaces. +func TestPutInlinePolicy_ReplacementCountsOnlyNewDocument(t *testing.T) { + t.Parallel() + for _, p := range inlinePutters { + t.Run(p.entityType, func(t *testing.T) { + t.Parallel() + svc := setupTestIAMService(t) + p.create(t, svc, "replace") + + require.NoError(t, p.put(svc, "replace", "Only", sizedPolicy(t, p.quota-10))) + require.NoError(t, p.put(svc, "replace", "Only", sizedPolicy(t, p.quota))) + }) + } +} + +func TestPutInlinePolicy_WhiteSpaceNotCounted(t *testing.T) { + t.Parallel() + for _, p := range inlinePutters { + t.Run(p.entityType, func(t *testing.T) { + t.Parallel() + svc := setupTestIAMService(t) + p.create(t, svc, "spaced") + + doc := strings.Replace(sizedPolicy(t, p.quota), `"Statement":`, `"Statement":`+strings.Repeat(" ", p.quota), 1) + require.NoError(t, p.put(svc, "spaced", "Spaced", doc)) + }) + } +} + +// A PolicyDocument over AWS's API length bound fails parameter validation, as in +// AWS, rather than reading as a malformed document. +func TestPolicyDocumentLength_ValidationError(t *testing.T) { + t.Parallel() + svc := setupTestIAMService(t) + tooLong := strings.Repeat("a", maxPolicyDocumentLength+1) + const want = "Member must have length less than or equal to 131072" + + _, err := svc.CreatePolicy(testAccountID, &iam.CreatePolicyInput{ + PolicyName: aws.String("TooLong"), PolicyDocument: aws.String(tooLong), + }) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorValidationError) + assert.Contains(t, err.Error(), want) + + out, err := svc.CreatePolicy(testAccountID, &iam.CreatePolicyInput{ + PolicyName: aws.String("Base"), PolicyDocument: aws.String(sizedPolicy(t, 200)), + }) + require.NoError(t, err) + _, err = svc.CreatePolicyVersion(testAccountID, &iam.CreatePolicyVersionInput{ + PolicyArn: out.Policy.Arn, PolicyDocument: aws.String(tooLong), + }) + require.Error(t, err) + assert.Contains(t, err.Error(), want) + + for _, p := range inlinePutters { + p.create(t, svc, "long-"+p.entityType) + err := p.put(svc, "long-"+p.entityType, "TooLong", tooLong) + require.Error(t, err, p.entityType) + assert.Contains(t, err.Error(), awserrors.ErrorValidationError, p.entityType) + assert.Contains(t, err.Error(), want, p.entityType) + } +} + +func TestCreatePolicy_ManagedQuota(t *testing.T) { + t.Parallel() + svc := setupTestIAMService(t) + + _, err := svc.CreatePolicy(testAccountID, &iam.CreatePolicyInput{ + PolicyName: aws.String("Over"), PolicyDocument: aws.String(sizedPolicy(t, 6145)), + }) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorIAMLimitExceeded) + assert.Contains(t, err.Error(), "Cannot exceed quota for PolicySize: 6144") + + out, err := svc.CreatePolicy(testAccountID, &iam.CreatePolicyInput{ + PolicyName: aws.String("AtQuota"), PolicyDocument: aws.String(sizedPolicy(t, 6144)), + }) + require.NoError(t, err) + + _, err = svc.CreatePolicyVersion(testAccountID, &iam.CreatePolicyVersionInput{ + PolicyArn: out.Policy.Arn, PolicyDocument: aws.String(sizedPolicy(t, 6145)), + }) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorIAMLimitExceeded) + + _, err = svc.CreatePolicyVersion(testAccountID, &iam.CreatePolicyVersionInput{ + PolicyArn: out.Policy.Arn, PolicyDocument: aws.String(sizedPolicy(t, 6144)), + }) + require.NoError(t, err) +} + +// Under 2008-10-17 a ${...} reference is literal text, so a key no request +// supplies is accepted rather than rejected as unresolvable. +func TestValidatePolicyDocument_Version2008(t *testing.T) { + t.Parallel() + const legacy = `{"Version":"2008-10-17","Statement":[{"Effect":"Allow","Action":"s3:GetObject",` + + `"Resource":"arn:aws:s3:::home/${unknown}/*","Condition":{"StringLike":{"s3:prefix":"${unknown}/*"}}},` + + `{"Effect":"Deny","Action":"s3:DeleteObject","NotResource":"arn:aws:s3:::${unknown}/*"}]}` + _, err := ValidatePolicyDocument(legacy) + require.NoError(t, err) + + _, err = ValidatePolicyDocument(strings.Replace(legacy, "2008-10-17", "2012-10-17", 1)) + require.Error(t, err) + assert.Contains(t, err.Error(), "no request supplies") + + _, err = ValidatePolicyDocument(strings.Replace(legacy, "2008-10-17", "2010-01-01", 1)) + require.Error(t, err) + assert.Contains(t, err.Error(), "unsupported policy version") +} diff --git a/spinifex/handlers/iam/policy_versions.go b/spinifex/handlers/iam/policy_versions.go index aaf9c0bbc..b7f569ec9 100644 --- a/spinifex/handlers/iam/policy_versions.go +++ b/spinifex/handlers/iam/policy_versions.go @@ -125,10 +125,16 @@ func policyCASTarget(accountID, policyARN string) (string, kvutil.CASConfig, err func (s *IAMServiceImpl) CreatePolicyVersion(accountID string, input *iam.CreatePolicyVersionInput) (*iam.CreatePolicyVersionOutput, error) { ctx := context.Background() + if err := checkPolicyDocumentLength(*input.PolicyDocument); err != nil { + return nil, err + } if _, err := ValidatePolicyDocument(*input.PolicyDocument); err != nil { return nil, awserrors.Errorf(awserrors.ErrorIAMMalformedPolicyDocument, "policy %q: %w", *input.PolicyArn, err) } + if err := checkManagedPolicySize(*input.PolicyDocument); err != nil { + return nil, err + } setAsDefault := aws.BoolValue(input.SetAsDefault) var created PolicyVersionRecord diff --git a/spinifex/handlers/iam/roles.go b/spinifex/handlers/iam/roles.go index 1e08612b2..89cda91e1 100644 --- a/spinifex/handlers/iam/roles.go +++ b/spinifex/handlers/iam/roles.go @@ -373,6 +373,9 @@ func (s *IAMServiceImpl) PutRolePolicy(accountID string, input *iam.PutRolePolic if err := validateIAMName("policyName", policyName, 128); err != nil { return nil, err } + if err := checkPolicyDocumentLength(policyDoc); err != nil { + return nil, err + } if _, err := ValidatePolicyDocument(policyDoc); err != nil { return nil, awserrors.Errorf(awserrors.ErrorIAMMalformedPolicyDocument, "policy %q on role %q: %w", policyName, roleName, err) @@ -389,6 +392,9 @@ func (s *IAMServiceImpl) PutRolePolicy(accountID string, input *iam.PutRolePolic if !changed { return false, nil } + if err := checkInlinePolicySize("role", roleName, rolePolicySizeQuota, role.InlinePolicies, policyName, policyDoc); err != nil { + return false, err + } role.InlinePolicies[policyName] = policyDoc return true, nil }) diff --git a/spinifex/handlers/iam/roles_test.go b/spinifex/handlers/iam/roles_test.go index 5c62fc536..d5ad42c7f 100644 --- a/spinifex/handlers/iam/roles_test.go +++ b/spinifex/handlers/iam/roles_test.go @@ -1053,21 +1053,6 @@ func TestPutRolePolicy_MalformedDocument(t *testing.T) { assert.Contains(t, err.Error(), awserrors.ErrorIAMMalformedPolicyDocument) } -func TestPutRolePolicy_OversizedDocument(t *testing.T) { - t.Parallel() - svc := setupTestIAMService(t) - createTestRole(t, svc, "oversized-role") - - huge := strings.Repeat("a", maxPolicyDocumentSize+1) - _, err := svc.PutRolePolicy(testAccountID, &iam.PutRolePolicyInput{ - RoleName: aws.String("oversized-role"), - PolicyName: aws.String("Huge"), - PolicyDocument: aws.String(huge), - }) - assert.Error(t, err) - assert.Contains(t, err.Error(), awserrors.ErrorIAMMalformedPolicyDocument) -} - func TestPutRolePolicy_RoleNotFound(t *testing.T) { t.Parallel() svc := setupTestIAMService(t) diff --git a/spinifex/handlers/iam/service_impl.go b/spinifex/handlers/iam/service_impl.go index 5ffb0d2cf..531edd817 100644 --- a/spinifex/handlers/iam/service_impl.go +++ b/spinifex/handlers/iam/service_impl.go @@ -15,6 +15,7 @@ import ( "strconv" "strings" "time" + "unicode" "unicode/utf8" "github.com/aws/aws-sdk-go/aws" @@ -1286,10 +1287,16 @@ func (s *IAMServiceImpl) CreatePolicy(accountID string, input *iam.CreatePolicyI } } + if err := checkPolicyDocumentLength(*input.PolicyDocument); err != nil { + return nil, err + } if _, err := ValidatePolicyDocument(*input.PolicyDocument); err != nil { return nil, awserrors.Errorf(awserrors.ErrorIAMMalformedPolicyDocument, "policy %q: %w", policyName, err) } + if err := checkManagedPolicySize(*input.PolicyDocument); err != nil { + return nil, err + } if err := validateTags(input.Tags, exactKeys); err != nil { return nil, err @@ -1705,6 +1712,9 @@ func (s *IAMServiceImpl) PutUserPolicy(accountID string, input *iam.PutUserPolic if err := validateIAMName("policyName", policyName, 128); err != nil { return nil, err } + if err := checkPolicyDocumentLength(policyDoc); err != nil { + return nil, err + } if _, err := ValidatePolicyDocument(policyDoc); err != nil { return nil, awserrors.Errorf(awserrors.ErrorIAMMalformedPolicyDocument, "policy %q on user %q: %w", policyName, userName, err) @@ -1714,6 +1724,9 @@ func (s *IAMServiceImpl) PutUserPolicy(accountID string, input *iam.PutUserPolic if err != nil { return nil, err } + if err := checkInlinePolicySize("user", userName, userPolicySizeQuota, user.InlinePolicies, policyName, policyDoc); err != nil { + return nil, err + } if user.InlinePolicies == nil { user.InlinePolicies = map[string]string{} @@ -2204,7 +2217,64 @@ func parseCreatedAt(raw string) time.Time { return t } -const maxPolicyDocumentSize = 6144 +// maxPolicyDocumentLength is AWS's API bound on a PolicyDocument parameter. The +// per-type limits below are the quotas a valid document is held to. +const maxPolicyDocumentLength = 131072 + +// AWS's policy size quotas, in characters excluding white space. Inline limits +// apply to the sum of an entity's inline policies. +const ( + managedPolicySizeQuota = 6144 + userPolicySizeQuota = 2048 + groupPolicySizeQuota = 5120 + rolePolicySizeQuota = 10240 +) + +// policySize is a document's size as AWS counts it against the quotas: its +// characters, leaving out white space. +func policySize(doc string) int { + n := 0 + for _, r := range doc { + if !unicode.IsSpace(r) { + n++ + } + } + return n +} + +// checkPolicyDocumentLength refuses a PolicyDocument parameter over AWS's API +// bound with the ValidationError AWS returns, before the document is parsed. +func checkPolicyDocumentLength(doc string) error { + if utf8.RuneCountInString(doc) > maxPolicyDocumentLength { + return lengthViolation("policyDocument", fmt.Sprintf("less than or equal to %d", maxPolicyDocumentLength)) + } + return nil +} + +// checkManagedPolicySize refuses a managed policy document over its quota. +func checkManagedPolicySize(doc string) error { + if policySize(doc) > managedPolicySizeQuota { + return awserrors.Errorf(awserrors.ErrorIAMLimitExceeded, "Cannot exceed quota for PolicySize: %d", managedPolicySizeQuota) + } + return nil +} + +// checkInlinePolicySize refuses putting doc as policyName when the entity's +// inline policies would then exceed quota in total. A policy of the same name +// is replaced, so only the new document counts. +func checkInlinePolicySize(entityType, entityName string, quota int, inline map[string]string, policyName, doc string) error { + total := policySize(doc) + for name, existing := range inline { + if name != policyName { + total += policySize(existing) + } + } + if total > quota { + return awserrors.Errorf(awserrors.ErrorIAMLimitExceeded, + "Maximum policy size of %d bytes exceeded for %s %s", quota, entityType, entityName) + } + return nil +} // isIAMNameChar returns true if c is allowed in IAM user/policy names. func isIAMNameChar(c byte) bool { @@ -2288,8 +2358,8 @@ func validatePath(path string) error { // ValidatePolicyDocument parses and validates an IAM policy document JSON string. func ValidatePolicyDocument(docJSON string) (*PolicyDocument, error) { - if len(docJSON) > maxPolicyDocumentSize { - return nil, fmt.Errorf("policy document exceeds maximum size of %d bytes", maxPolicyDocumentSize) + if len(docJSON) > maxPolicyDocumentLength { + return nil, fmt.Errorf("policy document exceeds maximum length of %d", maxPolicyDocumentLength) } var doc PolicyDocument @@ -2297,9 +2367,11 @@ func ValidatePolicyDocument(docJSON string) (*PolicyDocument, error) { return nil, fmt.Errorf("invalid JSON: %w", err) } - if doc.Version != "2012-10-17" { + if doc.Version != iampolicy.Version2012 && doc.Version != iampolicy.Version2008 { return nil, fmt.Errorf("unsupported policy version: %q", doc.Version) } + // Under 2008-10-17 a ${...} reference is literal text, so it cannot be unresolvable. + resolvesVariables := doc.Version == iampolicy.Version2012 if len(doc.Statement) == 0 { return nil, fmt.Errorf("policy must contain at least one statement") @@ -2318,7 +2390,7 @@ func ValidatePolicyDocument(docJSON string) (*PolicyDocument, error) { if err := validateSelectorEntries(i, stmt); err != nil { return nil, err } - if err := validateStatementRestrictions(i, stmt); err != nil { + if err := validateStatementRestrictions(i, stmt, resolvesVariables); err != nil { return nil, err } } @@ -2362,19 +2434,22 @@ func validateSelectorEntries(i int, stmt Statement) error { // 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 { +// Conditions inside the supported allowlist are accepted and enforced. Policy +// variables are checked only when the document's Version resolves them. +func validateStatementRestrictions(i int, stmt Statement, resolvesVariables bool) 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) } - for _, resource := range stmt.Resource { - if err := validatePolicyVariables(i, "Resource", resource); err != nil { - return err + if resolvesVariables { + for _, resource := range stmt.Resource { + if err := validatePolicyVariables(i, "Resource", resource); err != nil { + return err + } } - } - for _, resource := range stmt.NotResource { - if err := validatePolicyVariables(i, "NotResource", resource); err != nil { - return err + for _, resource := range stmt.NotResource { + if err := validatePolicyVariables(i, "NotResource", resource); err != nil { + return err + } } } for op, keys := range stmt.Condition { @@ -2382,7 +2457,7 @@ func validateStatementRestrictions(i int, stmt Statement) error { if !iampolicy.SupportedCondition(op, key) { return fmt.Errorf("statement %d: Condition operator %q on key %q is not supported in this release%s", i, op, key, unsupportedConditionKeyReason(key)) } - if err := validateConditionValues(i, op, key, values); err != nil { + if err := validateConditionValues(i, op, key, values, resolvesVariables); err != nil { return err } } @@ -2407,7 +2482,7 @@ func unsupportedConditionKeyReason(key string) string { // 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 { +func validateConditionValues(i int, op, key string, values ConditionValue, resolvesVariables bool) error { if len(values) == 0 { return fmt.Errorf("statement %d: Condition operator %q on key %q has no value", i, op, key) } @@ -2423,8 +2498,10 @@ func validateConditionValues(i int, op, key string, values ConditionValue) error 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 + if resolvesVariables { + if err := validatePolicyVariables(i, fmt.Sprintf("Condition %s on key %q", op, key), v); err != nil { + return err + } } } switch base { @@ -2491,13 +2568,8 @@ func validatePolicyVariables(i int, field, value string) error { } // 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 -// entity limits, so these IAM quota values are AWS-parity constants for -// informational compatibility only, not enforced limits. Resource types -// Spinifex does not model are reported as 0 rather than omitted so CIS/audit -// tooling that reads these keys keeps working. Real per-account resource counts -// are overlaid on top of this table at call time. +// GetAccountSummary, at AWS's defaults. Unmodelled resource types report 0 so +// audit tooling finds every key; real per-account counts are overlaid at call time. var summaryQuotaDefaults = map[string]int64{ // Account-wide quotas (AWS defaults). "UsersQuota": 5000, @@ -2515,12 +2587,12 @@ var summaryQuotaDefaults = map[string]int64{ "AttachedPoliciesPerGroupQuota": 10, "AttachedPoliciesPerRoleQuota": 10, "SigningCertificatesPerUserQuota": 2, - "UserPolicySizeQuota": 2048, - "GroupPolicySizeQuota": 5120, - "PolicySizeQuota": 6144, + "UserPolicySizeQuota": userPolicySizeQuota, + "GroupPolicySizeQuota": groupPolicySizeQuota, + "PolicySizeQuota": managedPolicySizeQuota, "VersionsPerPolicyQuota": 5, "AssumeRolePolicySizeQuota": maxTrustPolicyDocumentSize, - "RolePolicySizeQuota": 10240, + "RolePolicySizeQuota": rolePolicySizeQuota, // AWS's default: STS global endpoint tokens valid only in default Regions. "GlobalEndpointTokenVersion": 1, diff --git a/spinifex/handlers/iam/service_impl_test.go b/spinifex/handlers/iam/service_impl_test.go index 012a7907a..5e5fa9227 100644 --- a/spinifex/handlers/iam/service_impl_test.go +++ b/spinifex/handlers/iam/service_impl_test.go @@ -460,10 +460,10 @@ func TestCreatePolicy_EmptyPathRefusedBeforeDocument(t *testing.T) { func TestValidatePolicyDocument_TooLarge(t *testing.T) { t.Parallel() - largeDoc := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"*","Resource":"` + strings.Repeat("a", maxPolicyDocumentSize) + `"}]}` + largeDoc := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"*","Resource":"` + strings.Repeat("a", maxPolicyDocumentLength) + `"}]}` _, err := ValidatePolicyDocument(largeDoc) assert.Error(t, err) - assert.Contains(t, err.Error(), "exceeds maximum size") + assert.Contains(t, err.Error(), "exceeds maximum length") } // ============================================================================ @@ -1145,7 +1145,7 @@ func TestCreatePolicy_InvalidVersion(t *testing.T) { _, err := svc.CreatePolicy(testAccountID, &iam.CreatePolicyInput{ PolicyName: aws.String("BadVersion"), - PolicyDocument: aws.String(`{"Version":"2008-10-17","Statement":[{"Effect":"Allow","Action":"*","Resource":"*"}]}`), + PolicyDocument: aws.String(`{"Version":"2010-01-01","Statement":[{"Effect":"Allow","Action":"*","Resource":"*"}]}`), }) assert.Error(t, err) assert.Contains(t, err.Error(), awserrors.ErrorIAMMalformedPolicyDocument) @@ -2243,7 +2243,7 @@ func TestValidatePolicyDocument_BadJSON(t *testing.T) { func TestValidatePolicyDocument_WrongVersion(t *testing.T) { t.Parallel() - _, err := ValidatePolicyDocument(`{"Version":"2008-10-17","Statement":[{"Effect":"Allow","Action":"*","Resource":"*"}]}`) + _, err := ValidatePolicyDocument(`{"Version":"2010-01-01","Statement":[{"Effect":"Allow","Action":"*","Resource":"*"}]}`) assert.Error(t, err) assert.Contains(t, err.Error(), "unsupported policy version") } @@ -2504,7 +2504,7 @@ func TestValidateConditionValues_ArnOperators(t *testing.T) { iampolicy.OpArnEquals, iampolicy.OpArnLike, iampolicy.OpArnNotEquals, iampolicy.OpArnNotLike, } { for _, tt := range tests { - err := validateConditionValues(0, op, key, ConditionValue{tt.value}) + err := validateConditionValues(0, op, key, ConditionValue{tt.value}, true) if tt.wantErr == "" { assert.NoError(t, err, "%s %s", op, tt.name) continue @@ -2740,21 +2740,6 @@ func TestInputValidation_PathLength(t *testing.T) { assert.Error(t, err, "513-char path should be rejected") } -func TestInputValidation_PolicyDocumentSize(t *testing.T) { - t.Parallel() - // 6144 bytes — should pass - filler6144 := strings.Repeat("a", 6144-len(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"*","Resource":"arn:aws:s3:::`)-len(`"}]}`)) - doc6144 := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"*","Resource":"arn:aws:s3:::` + filler6144 + `"}]}` - assert.Len(t, doc6144, 6144) - _, err := ValidatePolicyDocument(doc6144) - assert.NoError(t, err, "6144-byte policy document should be valid") - - // 6145 bytes — should fail - doc6145 := doc6144 + " " - _, err = ValidatePolicyDocument(doc6145) - assert.Error(t, err, "6145-byte policy document should be rejected") -} - func TestValidatePolicyDocument_OverlappingDenyAllow(t *testing.T) { t.Parallel() doc, err := ValidatePolicyDocument(`{ diff --git a/spinifex/handlers/iam/user_inline_test.go b/spinifex/handlers/iam/user_inline_test.go index 140a90b82..70b28e0fa 100644 --- a/spinifex/handlers/iam/user_inline_test.go +++ b/spinifex/handlers/iam/user_inline_test.go @@ -2,7 +2,6 @@ package handlers_iam import ( "encoding/json" - "strings" "testing" "github.com/aws/aws-sdk-go/aws" @@ -130,21 +129,6 @@ func TestPutUserPolicy_UnresolvableVariableNamesTheCause(t *testing.T) { assert.Error(t, err) } -func TestPutUserPolicy_OversizedDocument(t *testing.T) { - t.Parallel() - svc := setupTestIAMService(t) - createTestUser(t, svc, "oversized-user") - - huge := strings.Repeat("a", maxPolicyDocumentSize+1) - _, err := svc.PutUserPolicy(testAccountID, &iam.PutUserPolicyInput{ - UserName: aws.String("oversized-user"), - PolicyName: aws.String("Huge"), - PolicyDocument: aws.String(huge), - }) - assert.Error(t, err) - assert.Contains(t, err.Error(), awserrors.ErrorIAMMalformedPolicyDocument) -} - func TestPutUserPolicy_UserNotFound(t *testing.T) { t.Parallel() svc := setupTestIAMService(t)