Skip to content

Commit

Permalink
fix(iam): IAM Policies are too large to deploy
Browse files Browse the repository at this point in the history
The policies we generate sometimes have a lot of duplication between
statements. This duplication can lead to the policy going over the size
limit an IAM policy (either 2k, 6k or 10k bytes, depending on the resource
type).

This change combines multiple statements together, as long as it
doesn't change the meaning of the final policy.

Because doing so for all existing stacks will probably provoke minor
heart attacks in operators everywhere, the new behavior is gated
behind a feature flag. It can be retroactively switched on by
people currently being bit by the size issues:

```
@aws-cdk/aws-iam:minimizePolicies
```

Fixes #18774, fixes #16350, fixes #18457.
  • Loading branch information
rix0rrr committed Feb 23, 2022
1 parent e6f74be commit f8208e9
Show file tree
Hide file tree
Showing 5 changed files with 227 additions and 6 deletions.
40 changes: 37 additions & 3 deletions 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
Expand All @@ -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;
}

/**
Expand All @@ -43,16 +62,21 @@ export class PolicyDocument implements cdk.IResolvable {
public readonly creationStack: string[];
private readonly statements = new Array<PolicyStatement>();
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();
}

Expand Down Expand Up @@ -157,15 +181,25 @@ 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 {
if (!input || !input.Statement) {
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<string>();
const uniqueStatements: any[] = [];

Expand Down
176 changes: 176 additions & 0 deletions 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<string, IamValue>;
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<StatementMerge>();
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<A extends keyof StatementSchema>(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<A extends StatementSchema>(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;
}
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-iam/package.json
Expand Up @@ -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"
},
Expand Down
6 changes: 3 additions & 3 deletions 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
Expand All @@ -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
Expand Down
10 changes: 10 additions & 0 deletions packages/@aws-cdk/cx-api/lib/features.ts
Expand Up @@ -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
*
Expand All @@ -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,
};

/**
Expand Down

0 comments on commit f8208e9

Please sign in to comment.