diff --git a/packages/@aws-cdk/aws-iam/lib/policy-document.ts b/packages/@aws-cdk/aws-iam/lib/policy-document.ts index da43cce541158..561972b66e217 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy-document.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy-document.ts @@ -1,5 +1,7 @@ import * as cdk from '@aws-cdk/core'; +import * as cxapi from '@aws-cdk/cx-api'; import { PolicyStatement } from './policy-statement'; +import { mergeStatements } from './private/merge-statements'; /** * Properties for a new PolicyDocument @@ -18,6 +20,23 @@ export interface PolicyDocumentProps { * @default - No statements */ readonly statements?: PolicyStatement[]; + + /** + * Try to minimize the policy by merging statements + * + * To avoid overrunning the maximum policy size, combine statements if they produce + * the same result. Merging happens according to the following rules: + * + * - The Effect of both statements is 'Allow' ('Deny' statements cannot be merged) + * - Neither of the statements have a 'SId' + * - Combine Principals if the rest of the statement is exactly the same. + * - Combine Resources if the rest of the statement is exactly the same. + * - Combine Actions if the rest of the statement is exactly the same. + * - We will never combine NotPrincipals, NotResources or NotActions. + * + * @default false, unless the feature flag `@aws-cdk/aws-iam:minimizePolicies` is set + */ + readonly minimize?: boolean; } /** @@ -43,16 +62,21 @@ export class PolicyDocument implements cdk.IResolvable { public readonly creationStack: string[]; private readonly statements = new Array(); private readonly autoAssignSids: boolean; + private readonly minimize?: boolean; constructor(props: PolicyDocumentProps = {}) { this.creationStack = cdk.captureStackTrace(); this.autoAssignSids = !!props.assignSids; + this.minimize = props.minimize; this.addStatements(...props.statements || []); } public resolve(context: cdk.IResolveContext): any { - context.registerPostProcessor(new RemoveDuplicateStatements(this.autoAssignSids)); + context.registerPostProcessor(new ReducePolicyDocument( + this.autoAssignSids, + this.minimize ?? cdk.FeatureFlags.of(context.scope).isEnabled(cxapi.IAM_MINIMIZE_POLICIES) ?? false, + )); return this.render(); } @@ -157,8 +181,8 @@ export class PolicyDocument implements cdk.IResolvable { /** * Removes duplicate statements and assign Sids if necessary */ -class RemoveDuplicateStatements implements cdk.IPostProcessor { - constructor(private readonly autoAssignSids: boolean) { +class ReducePolicyDocument implements cdk.IPostProcessor { + constructor(private readonly autoAssignSids: boolean, private readonly minimize: boolean) { } public postProcess(input: any, _context: cdk.IResolveContext): any { @@ -166,6 +190,16 @@ class RemoveDuplicateStatements implements cdk.IPostProcessor { return input; } + if (this.minimize) { + // New behavior: merge statements, this will naturally + // get rid of duplicates. + return { + ...input, + Statement: mergeStatements(input.Statement), + }; + } + + // Old behavior: just remove full-on duplicates const jsonStatements = new Set(); const uniqueStatements: any[] = []; diff --git a/packages/@aws-cdk/aws-iam/lib/private/merge-statements.ts b/packages/@aws-cdk/aws-iam/lib/private/merge-statements.ts new file mode 100644 index 0000000000000..7453651355174 --- /dev/null +++ b/packages/@aws-cdk/aws-iam/lib/private/merge-statements.ts @@ -0,0 +1,176 @@ +const LENGTH_CACHE_SYM = Symbol(); + +type IamValue = unknown | unknown[]; + +/** + * Only the parts of the policy schema we're interested in + */ +interface StatementSchema { + readonly Sid?: string; + readonly Effect?: string; + readonly Principal?: IamValue | Record; + readonly Resource?: IamValue; + readonly Action?: IamValue; +} + +/** + * Merge as many statements as possible to shrink the total policy doc, modifying the input array in place + */ +export function mergeStatements(statements: StatementSchema[]): StatementSchema[] { + let merges = findMerges(statements); + while (merges.length > 0) { + merges.sort((a, b) => a.sizeDelta - b.sizeDelta); // Most negative number first + if (merges[0].sizeDelta >= 0) { break; } // Nothing more to be gained + + // Apply all merges, order biggest to smallest gain + while (merges.length > 0 && merges[0].sizeDelta < 0) { // Only while we're not adding size + const merge = merges.shift()!; + statements[merge.i0] = merge.combined; + statements.splice(merge.i1, 1); + + // This invalidates all merges involving i0 or i1, and adjusts all indices > i1 + const invalidIndices = [merge.i0, merge.i1]; + let j = 0; + while (j < merges.length) { + if (invalidIndices.includes(merges[j].i0) || invalidIndices.includes(merges[j].i1)) { + merges.splice(j, 1); + } else { + if (merges[j].i0 > merge.i1) { merges[j].i0 -= 1; } + if (merges[j].i1 > merge.i1) { merges[j].i1 -= 1; } + j++; + } + } + } + + merges = findMerges(statements); + } + return statements; +} + +function findMerges(statements: StatementSchema[]): StatementMerge[] { + const ret = new Array(); + for (let i0 = 0; i0 < statements.length; i0++) { + for (let i1 = i0 + 1; i1 < statements.length; i1++) { + tryMerge(statements, i0, i1, ret); + } + } + return ret; +} + +function tryMerge(statements: StatementSchema[], i0: number, i1: number, into: StatementMerge[]) { + const a = statements[i0]; + const b = statements[i1]; + if (a.Effect !== 'Allow' || b.Effect !== 'Allow') { return; } + if (a.Sid || b.Sid) { return; } + + const beforeLen = jsonLength(a) + jsonLength(b); + + tryMerging('Resource'); + tryMerging('Action'); + tryMerging('Principal'); + + function tryMerging(key: A) { + if (!deepEqual(a, b, [key])) { return; } + + const combined = { + ...a, + [key]: mergeValues(a[key], b[key]), + }; + + into.push({ + i0, + i1, + combined, + sizeDelta: jsonLength(combined) - beforeLen, + }); + } +} + +/** + * Return the length of a JSON representation of the given object, cached on the object + */ +function jsonLength(x: A): number { + if ((x as any)[LENGTH_CACHE_SYM]) { + return (x as any)[LENGTH_CACHE_SYM]; + } + + const length = JSON.stringify(x).length; + Object.defineProperty(x, LENGTH_CACHE_SYM, { + value: length, + enumerable: false, + }); + return length; +} + +function deepEqual(a: any, b: any, ignoreKeys: string[]): boolean { + // Short-circuit same object identity as well + if (a === b) { return true; } + + if (Array.isArray(a) || Array.isArray(b)) { + if (!Array.isArray(a) || !Array.isArray(b) || a.length !== b.length) { return false; } + return a.every((x, i) => deepEqual(x, b[i], [])); + } + + if (typeof a === 'object' || typeof b === 'object') { + if (typeof a !== 'object' || typeof b !== 'object') { return false; } + + const keysA = new Set(Object.keys(a)); + const keysB = new Set(Object.keys(b)); + for (const k of ignoreKeys) { + keysA.delete(k); + keysB.delete(k); + } + + for (const k of keysA) { + if (!deepEqual(a[k], b[k], [])) { return false; } + keysB.delete(k); + } + + return keysB.size === 0; + } + + return false; +} + +function mergeValues(a: IamValue, b: IamValue): any { + if (!a && !b) { return a; } + if (Array.isArray(a) && Array.isArray(b)) { return normalizedArray(...a, ...b); } + if (Array.isArray(a) && typeof b === 'string') { return normalizedArray(...a, b); } + if (Array.isArray(b) && typeof a === 'string') { return normalizedArray(a, ...b); } + if (typeof a === 'string' && typeof b === 'string') { return normalizedArray(a, b); } + if (typeof a === 'object' && typeof b === 'object' && b != null) { + const ret: any = { ...a }; + for (const [k, v] of Object.entries(b)) { + ret[k] = ret[k] ? mergeValues(ret[k], v) : v; + } + return ret; + } + + throw new Error(`Don't know how to merge ${JSON.stringify(a)} and ${JSON.stringify(b)}`); +} + +function normalizedArray(...xs: unknown[]) { + return Array.from(new Set(xs)).sort(); +} + +interface StatementMerge { + /** + * Index of statement 0 + */ + i0: number; + + /** + * Index of statement 1 + */ + i1: number; + + /** + * The result of combining them + */ + readonly combined: StatementSchema; + + /** + * How many bytes the new schema is bigger than the old one + */ + readonly sizeDelta: number; +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-iam/package.json b/packages/@aws-cdk/aws-iam/package.json index 7172a6e453245..96ed3c82b8fc4 100644 --- a/packages/@aws-cdk/aws-iam/package.json +++ b/packages/@aws-cdk/aws-iam/package.json @@ -92,6 +92,7 @@ }, "dependencies": { "@aws-cdk/core": "0.0.0", + "@aws-cdk/cx-api": "0.0.0", "@aws-cdk/region-info": "0.0.0", "constructs": "^3.3.69" }, diff --git a/packages/@aws-cdk/core/lib/feature-flags.ts b/packages/@aws-cdk/core/lib/feature-flags.ts index 926a60168732f..44fd5e138bdc7 100644 --- a/packages/@aws-cdk/core/lib/feature-flags.ts +++ b/packages/@aws-cdk/core/lib/feature-flags.ts @@ -1,5 +1,5 @@ import * as cxapi from '@aws-cdk/cx-api'; -import { Construct } from '../lib/construct-compat'; +import { IConstruct } from '../lib/construct-compat'; /** * Features that are implemented behind a flag in order to preserve backwards @@ -12,11 +12,11 @@ export class FeatureFlags { /** * Inspect feature flags on the construct node's context. */ - public static of(scope: Construct) { + public static of(scope: IConstruct) { return new FeatureFlags(scope); } - private constructor(private readonly construct: Construct) {} + private constructor(private readonly construct: IConstruct) {} /** * Check whether a feature flag is enabled. If configured, the flag is present in diff --git a/packages/@aws-cdk/cx-api/lib/features.ts b/packages/@aws-cdk/cx-api/lib/features.ts index 9278717ef1c15..9b5dc4e872892 100644 --- a/packages/@aws-cdk/cx-api/lib/features.ts +++ b/packages/@aws-cdk/cx-api/lib/features.ts @@ -215,6 +215,15 @@ export const ECS_SERVICE_EXTENSIONS_ENABLE_DEFAULT_LOG_DRIVER = '@aws-cdk-contai */ export const EC2_UNIQUE_IMDSV2_LAUNCH_TEMPLATE_NAME = '@aws-cdk/aws-ec2:uniqueImdsv2TemplateName'; +/** + * Minimize IAM policies by combining Principals, Actions and Resources of two + * Statements in the policies, as long as it doesn't change the meaning of the + * policy. + * + * [PERMANENT] + */ +export const IAM_MINIMIZE_POLICIES = '@aws-cdk/aws-iam:minimizePolicies'; + /** * Flag values that should apply for new projects * @@ -240,6 +249,7 @@ export const FUTURE_FLAGS: { [key: string]: boolean } = { [CLOUDFRONT_DEFAULT_SECURITY_POLICY_TLS_V1_2_2021]: true, [ECS_SERVICE_EXTENSIONS_ENABLE_DEFAULT_LOG_DRIVER]: true, [EC2_UNIQUE_IMDSV2_LAUNCH_TEMPLATE_NAME]: true, + [IAM_MINIMIZE_POLICIES]: true, }; /**