From 54173ae06117c9db44cb681a262d3c159b9f5dd3 Mon Sep 17 00:00:00 2001 From: Stephen Kuenzli Date: Mon, 9 Dec 2024 17:02:35 -0700 Subject: [PATCH 01/12] (feat) Control access to DynamoDB resources using a resource policy. --- src/dynamodb.ts | 94 +++++++++++++++++++++++++++++++++++++++++++++++++ src/index.ts | 1 + src/k9policy.ts | 1 + test/k9.test.ts | 94 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 190 insertions(+) create mode 100644 src/dynamodb.ts diff --git a/src/dynamodb.ts b/src/dynamodb.ts new file mode 100644 index 0000000..753a200 --- /dev/null +++ b/src/dynamodb.ts @@ -0,0 +1,94 @@ +import { TableV2 } from 'aws-cdk-lib/aws-dynamodb'; +import { AccountRootPrincipal, Effect, PolicyDocument, PolicyStatement } from 'aws-cdk-lib/aws-iam'; +import * as iam from 'aws-cdk-lib/aws-iam'; +import { AccessCapability, getAccessCapabilityFromValue, IAccessSpec, K9PolicyFactory } from './k9policy'; + + +export interface K9DynamoDBResourcePolicyProps { + readonly k9DesiredAccess: Array; +} + +let SUPPORTED_CAPABILITIES = new Array( + AccessCapability.ADMINISTER_RESOURCE, + AccessCapability.READ_CONFIG, + AccessCapability.READ_DATA, + AccessCapability.WRITE_DATA, + AccessCapability.DELETE_DATA, +); + +export const SID_DENY_EVERYONE_ELSE = 'DenyEveryoneElse'; + +export function grantAccessViaResourcePolicy(table: TableV2, props: K9DynamoDBResourcePolicyProps): PolicyDocument { + const policyFactory = new K9PolicyFactory(); + const policy = new iam.PolicyDocument(); + + console.log('making resource policy with props: ' + props); + + const resourceArns = ['*']; + + let accessSpecsByCapabilityRecs = policyFactory.mergeDesiredAccessSpecsByCapability(SUPPORTED_CAPABILITIES, props.k9DesiredAccess); + let accessSpecsByCapability: Map = new Map(); + + for (let [capabilityStr, accessSpec] of Object.entries(accessSpecsByCapabilityRecs)) { + accessSpecsByCapability.set(getAccessCapabilityFromValue(capabilityStr), accessSpec); + } + + if (!canPrincipalsManageResources(accessSpecsByCapability)) { + throw Error('At least one principal must be able to administer and read-config for DynamoDB resources' + + ' so data data remains accessible; found:\n' + + `administer-resource: '${accessSpecsByCapability.get(AccessCapability.ADMINISTER_RESOURCE)?.allowPrincipalArns}'\n` + + `read-config: '${accessSpecsByCapability.get(AccessCapability.READ_CONFIG)?.allowPrincipalArns}'`, + ); + } + + const allowStatements = policyFactory.makeAllowStatements('DynamoDB', + SUPPORTED_CAPABILITIES, + Array.from(accessSpecsByCapability.values()), + resourceArns); + policy.addStatements(...allowStatements); + + for (const allowStatement of allowStatements) { + table.addToResourcePolicy(allowStatement); + } + + // console.log('Adding Allow root and DenyEveryoneElse statements'); + const denyEveryoneElseStatement = new PolicyStatement({ + sid: SID_DENY_EVERYONE_ELSE, + effect: Effect.DENY, + principals: policyFactory.makeDenyEveryoneElsePrincipals(), + actions: ['dynamodb:*'], + resources: resourceArns, + }); + denyEveryoneElseStatement.addCondition('Bool', { + 'aws:PrincipalIsAWSService': ['false'], + }); + const denyEveryoneElseTest = policyFactory.wasLikeUsed(props.k9DesiredAccess) ? + 'ArnNotLike' : + 'ArnNotEquals'; + const allAllowedPrincipalArns = policyFactory.getAllowedPrincipalArns(props.k9DesiredAccess); + const accountRootPrincipal = new AccountRootPrincipal(); + denyEveryoneElseStatement.addCondition(denyEveryoneElseTest, { + 'aws:PrincipalArn': [ + // Place Root Principal arn in stable, prominent position; + // will render as an object Fn::Join'ing Partition & AccountId + accountRootPrincipal.arn, + ...allAllowedPrincipalArns, + ], + }); + + policy.addStatements( + denyEveryoneElseStatement, + ); + + table.addToResourcePolicy(denyEveryoneElseStatement); + table.resourcePolicy?.validateForResourcePolicy(); + + policy.validateForResourcePolicy(); + + return policy; +} + +function canPrincipalsManageResources(accessSpecsByCapability: Map) { + console.log(accessSpecsByCapability); + return true; +} \ No newline at end of file diff --git a/src/index.ts b/src/index.ts index 263e9ee..dd1de73 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,3 +1,4 @@ export * as k9policy from './k9policy'; +export * as dynamodb from './dynamodb'; export * as kms from './kms'; export * as s3 from './s3'; diff --git a/src/k9policy.ts b/src/k9policy.ts index 07f31d1..f23b509 100644 --- a/src/k9policy.ts +++ b/src/k9policy.ts @@ -86,6 +86,7 @@ export class K9PolicyFactory { _SUPPORTED_SERVICES = new Set([ 'S3', 'KMS', + 'DynamoDB', ]); /** @internal */ diff --git a/test/k9.test.ts b/test/k9.test.ts index 76713bb..5443a8f 100644 --- a/test/k9.test.ts +++ b/test/k9.test.ts @@ -1,4 +1,5 @@ import { expect as expectCDK, haveResource, SynthUtils } from '@aws-cdk/assert'; +import * as dynamodb from 'aws-cdk-lib/aws-dynamodb'; import { AddToResourcePolicyResult } from 'aws-cdk-lib/aws-iam'; import * as kms from 'aws-cdk-lib/aws-kms'; import * as s3 from 'aws-cdk-lib/aws-s3'; @@ -16,6 +17,7 @@ import { SID_DENY_UNEXPECTED_ENCRYPTION_METHOD, CloudFrontOACReadAccessGenerator, } from '../lib/s3'; +import { K9DynamoDBResourcePolicyProps } from '../src/dynamodb'; // @ts-ignore // Test the primary public interface to k9 cdk @@ -534,6 +536,98 @@ describe('K9KeyPolicy', () => { }); +describe('DynamoDBResourcePolicy', () => { + const desiredAccess = new Array( + { + accessCapabilities: [ + AccessCapability.ADMINISTER_RESOURCE, + AccessCapability.READ_CONFIG, + ], + allowPrincipalArns: administerResourceArns, + }, + { + accessCapabilities: AccessCapability.WRITE_DATA, + allowPrincipalArns: writeDataArns, + }, + { + accessCapabilities: AccessCapability.READ_DATA, + allowPrincipalArns: readDataArns, + }, + { + accessCapabilities: AccessCapability.DELETE_DATA, + allowPrincipalArns: deleteDataArns, + }, + ); + test('Typical usage', () => { + const stack = new cdk.Stack(app, 'K9DDBResourcePolicyTestTypicalUsage', { env: { region: 'us-east-1' } }); + + const ddbResourcePolicyProps: K9DynamoDBResourcePolicyProps = { + k9DesiredAccess: desiredAccess, + }; + + const table = new dynamodb.TableV2(stack, 'test-table-typical-usage', { + partitionKey: { name: 'pk', type: dynamodb.AttributeType.STRING }, + removalPolicy: cdk.RemovalPolicy.DESTROY, + // resourcePolicy: resourcePolicy, + }); + + let resourcePolicy = k9.dynamodb.grantAccessViaResourcePolicy(table, ddbResourcePolicyProps); + + console.log('table: ' + table); + console.log('table.resourcePolicy: ' + stringifyPolicy(table.resourcePolicy)); + console.log('resourcePolicy: ' + stringifyPolicy(resourcePolicy)); + + + expect(table.resourcePolicy).toBeDefined(); + + // const k9BucketPolicyProps: K9BucketPolicyProps = { + // bucket: table, + // k9DesiredAccess: new Array( + // { + // accessCapabilities: AccessCapability.ADMINISTER_RESOURCE, + // allowPrincipalArns: administerResourceArns, + // }, + // { + // accessCapabilities: AccessCapability.WRITE_DATA, + // allowPrincipalArns: writeDataArns, + // }, + // { + // accessCapabilities: AccessCapability.READ_DATA, + // allowPrincipalArns: readDataArns, + // }, + // { + // accessCapabilities: AccessCapability.DELETE_DATA, + // allowPrincipalArns: deleteDataArns, + // }, + // ), + // }; + // let addToResourcePolicyResults = k9.s3.grantAccessViaResourcePolicy(stack, 'S3Bucket', k9BucketPolicyProps); + // expect(table.resourcePolicy).toBeDefined(); + // + // let policyStr = stringifyPolicy(table.resourcePolicy); + // console.log('table.resourcePolicy?: ' + policyStr); + // expect(table.resourcePolicy).toBeDefined(); + // + // assertK9StatementsAddedToS3ResourcePolicy(addToResourcePolicyResults); + // let policyObj = JSON.parse(policyStr); + // let actualPolicyStatements = policyObj.Statement; + // expect(actualPolicyStatements).toBeDefined(); + // + // for (let stmt of actualPolicyStatements) { + // if (SID_DENY_UNEXPECTED_ENCRYPTION_METHOD == stmt.Sid) { + // expect(stmt.Condition.StringNotEquals['s3:x-amz-server-side-encryption']).toEqual('aws:kms'); + // } + // } + // console.log('stack to json: ' + stack.toJsonString(stack)) + + // expectCDK(stack).to(haveResource('AWS::DynamoDB::Table')); + // expectCDK(stack).to(haveResource('AWS::S3::BucketPolicy')); + // expect(SynthUtils.toCloudFormation(stack)).toMatchSnapshot(); + }); + +}); + + function assertContainsStatementWithId(expectStmtId:string, statements:any) { let foundStmt = false; console.log(`looking for statement id: ${expectStmtId}`); From 231756c5130dd302b65d7ebaee09b1c2b4e3384e Mon Sep 17 00:00:00 2001 From: Stephen Kuenzli Date: Tue, 10 Dec 2024 11:26:10 -0700 Subject: [PATCH 02/12] Optionally generate allow statements with statement ids in PascalCase. --- src/k9policy.ts | 26 +++++++++++++++++++++++--- test/k9policy.test.ts | 18 +++++++++++++++++- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/k9policy.ts b/src/k9policy.ts index f23b509..51e2c9a 100644 --- a/src/k9policy.ts +++ b/src/k9policy.ts @@ -61,6 +61,21 @@ export interface IAWSServiceAccessGenerator { makeConditionsToExceptFromDenyEveryoneElse(): Conditions; } +export function toPascalCase(input: string): string { + // Remove placeholders like ${something} and trim whitespace + const cleanedInput = input.replace(/\$\{.*?\}/g, '').trim(); + + // Split the input into words based on spaces, hyphens, underscores, or other delimiters + const words = cleanedInput.split(/[\s_\-]+/); + + // Convert each word to PascalCase + return words + .map( + word => word.charAt(0).toUpperCase() + word.slice(1).toLowerCase(), // Capitalize the first letter, lower the rest + ) + .join(''); +} + export class K9PolicyFactory { /** @@ -179,7 +194,8 @@ export class K9PolicyFactory { makeAllowStatements(serviceName: string, supportedCapabilities: Array, desiredAccess: Array, - resourceArns: Array): Array { + resourceArns: Array, + usePascalCase: boolean = false): Array { let policyStatements = new Array(); let accessSpecsByCapabilityRecs = this.mergeDesiredAccessSpecsByCapability(supportedCapabilities, desiredAccess); let accessSpecsByCapability: Map = new Map(); @@ -202,7 +218,12 @@ export class K9PolicyFactory { let arnConditionTest = accessSpec.test || 'ArnEquals'; - let statement = this.makeAllowStatement(`Allow Restricted ${supportedCapability}`, + let sid = `Allow Restricted ${supportedCapability}`; + if (usePascalCase) { + sid = toPascalCase(sid); + } + + let statement = this.makeAllowStatement(sid, this.getActions(serviceName, supportedCapability), accessSpec.allowPrincipalArns, arnConditionTest, @@ -274,4 +295,3 @@ export class K9PolicyFactory { } } - diff --git a/test/k9policy.test.ts b/test/k9policy.test.ts index 86272c6..8e21741 100644 --- a/test/k9policy.test.ts +++ b/test/k9policy.test.ts @@ -1,6 +1,12 @@ import { AnyPrincipal, PolicyStatement } from 'aws-cdk-lib/aws-iam'; import { stringifyStatement } from './helpers'; -import { AccessCapability, getAccessCapabilityFromValue, IAccessSpec, K9PolicyFactory } from '../lib/k9policy'; +import { + AccessCapability, + getAccessCapabilityFromValue, + IAccessSpec, + K9PolicyFactory, + toPascalCase, +} from '../lib/k9policy'; // @ts-ignore const S3_SUPPORTED_CAPABILITIES = new Array( @@ -26,6 +32,16 @@ test('getAccessCapabilityFromValue throws error for undefined capabilities', () }).toThrow('Could not get AccessCapability from value: unknown-capability'); }); +test('toPascalCase converts classic Allow Restricted X SID', () => { + expect(toPascalCase('Allow Restricted administer-resource')) + .toEqual('AllowRestrictedAdministerResource'); +}); + +test('toPascalCase trims leading and trailing spaces', () => { + expect(toPascalCase(' Allow Restricted With Leading and Trailing Spaces')) + .toEqual('AllowRestrictedWithLeadingAndTrailingSpaces'); +}); + test('K9PolicyFactory#wasLikeUsed', () => { let k9PolicyFactory = new K9PolicyFactory(); expect(k9PolicyFactory.wasLikeUsed([])).toBeFalsy(); From 72e5242c8fea959b727588f5876ff75a9c61bc9b Mon Sep 17 00:00:00 2001 From: Stephen Kuenzli Date: Tue, 10 Dec 2024 13:17:36 -0700 Subject: [PATCH 03/12] Update DynamoDB capability summary with those supported by DDB resource policy https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/rbac-iam-actions.html --- resources/capability_summary.json | 39 +++++++------------------------ 1 file changed, 9 insertions(+), 30 deletions(-) diff --git a/resources/capability_summary.json b/resources/capability_summary.json index e2b03f4..511c2e9 100644 --- a/resources/capability_summary.json +++ b/resources/capability_summary.json @@ -70,70 +70,49 @@ "DynamoDB": { "administer-resource": [ "dynamodb:CreateBackup", - "dynamodb:CreateGlobalTable", - "dynamodb:CreateTable", - "dynamodb:CreateTableReplica", + "dynamodb:DeleteResourcePolicy", "dynamodb:DeleteTableReplica", + "dynamodb:DisableKinesisStreamingDestination", + "dynamodb:EnableKinesisStreamingDestination", "dynamodb:ExportTableToPointInTime", - "dynamodb:PurchaseReservedCapacityOfferings", - "dynamodb:RestoreTableFromBackup", + "dynamodb:PutResourcePolicy", "dynamodb:RestoreTableToPointInTime", "dynamodb:TagResource", "dynamodb:UntagResource", "dynamodb:UpdateContinuousBackups", "dynamodb:UpdateContributorInsights", - "dynamodb:UpdateGlobalTable", - "dynamodb:UpdateGlobalTableSettings", + "dynamodb:UpdateKinesisStreamingDestination", "dynamodb:UpdateTable", "dynamodb:UpdateTableReplicaAutoScaling", "dynamodb:UpdateTimeToLive" ], "delete-data": [ - "dynamodb:DeleteBackup", "dynamodb:DeleteItem", "dynamodb:DeleteTable", "dynamodb:DeleteTableReplica", "dynamodb:PartiQLDelete" ], "read-config": [ - "dynamodb:DescribeBackup", "dynamodb:DescribeContinuousBackups", "dynamodb:DescribeContributorInsights", "dynamodb:DescribeExport", - "dynamodb:DescribeGlobalTable", - "dynamodb:DescribeGlobalTableSettings", - "dynamodb:DescribeLimits", - "dynamodb:DescribeReservedCapacity", - "dynamodb:DescribeReservedCapacityOfferings", - "dynamodb:DescribeStream", + "dynamodb:DescribeKinesisStreamingDestination", "dynamodb:DescribeTable", "dynamodb:DescribeTableReplicaAutoScaling", "dynamodb:DescribeTimeToLive", - "dynamodb:ListBackups", - "dynamodb:ListContributorInsights", - "dynamodb:ListExports", - "dynamodb:ListGlobalTables", - "dynamodb:ListStreams", - "dynamodb:ListTables", - "dynamodb:ListTagsOfResource", - "dynamodbstreams:DescribeStream", - "dynamodbstreams:ListStreams" + "dynamodb:GetResourcePolicy", + "dynamodb:ListTagsOfResource" ], "read-data": [ "dynamodb:BatchGetItem", "dynamodb:ConditionCheckItem", "dynamodb:GetItem", - "dynamodb:GetRecords", - "dynamodb:GetShardIterator", "dynamodb:PartiQLSelect", "dynamodb:Query", - "dynamodb:Scan", - "dynamodbstreams:GetRecords", - "dynamodbstreams:GetShardIterator" + "dynamodb:Scan" ], "write-data": [ "dynamodb:BatchWriteItem", - "dynamodb:CreateTableReplica", "dynamodb:PartiQLInsert", "dynamodb:PartiQLUpdate", "dynamodb:PutItem", From 6b976a2944a6a723ec6376dc712d6733afda09ec Mon Sep 17 00:00:00 2001 From: Stephen Kuenzli Date: Tue, 10 Dec 2024 14:17:50 -0700 Subject: [PATCH 04/12] Extract canPrincipalsManageResources function to share across KMS and DDB --- src/dynamodb.ts | 18 +++++------ src/k9policy.ts | 23 ++++++++++++++ src/kms.ts | 18 ++--------- test/k9policy.test.ts | 71 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 25 deletions(-) diff --git a/src/dynamodb.ts b/src/dynamodb.ts index 753a200..f7abbf0 100644 --- a/src/dynamodb.ts +++ b/src/dynamodb.ts @@ -1,7 +1,13 @@ import { TableV2 } from 'aws-cdk-lib/aws-dynamodb'; import { AccountRootPrincipal, Effect, PolicyDocument, PolicyStatement } from 'aws-cdk-lib/aws-iam'; import * as iam from 'aws-cdk-lib/aws-iam'; -import { AccessCapability, getAccessCapabilityFromValue, IAccessSpec, K9PolicyFactory } from './k9policy'; +import { + AccessCapability, + canPrincipalsManageResources, + getAccessCapabilityFromValue, + IAccessSpec, + K9PolicyFactory, +} from './k9policy'; export interface K9DynamoDBResourcePolicyProps { @@ -22,8 +28,6 @@ export function grantAccessViaResourcePolicy(table: TableV2, props: K9DynamoDBRe const policyFactory = new K9PolicyFactory(); const policy = new iam.PolicyDocument(); - console.log('making resource policy with props: ' + props); - const resourceArns = ['*']; let accessSpecsByCapabilityRecs = policyFactory.mergeDesiredAccessSpecsByCapability(SUPPORTED_CAPABILITIES, props.k9DesiredAccess); @@ -44,7 +48,8 @@ export function grantAccessViaResourcePolicy(table: TableV2, props: K9DynamoDBRe const allowStatements = policyFactory.makeAllowStatements('DynamoDB', SUPPORTED_CAPABILITIES, Array.from(accessSpecsByCapability.values()), - resourceArns); + resourceArns, + true); policy.addStatements(...allowStatements); for (const allowStatement of allowStatements) { @@ -87,8 +92,3 @@ export function grantAccessViaResourcePolicy(table: TableV2, props: K9DynamoDBRe return policy; } - -function canPrincipalsManageResources(accessSpecsByCapability: Map) { - console.log(accessSpecsByCapability); - return true; -} \ No newline at end of file diff --git a/src/k9policy.ts b/src/k9policy.ts index 51e2c9a..ca99857 100644 --- a/src/k9policy.ts +++ b/src/k9policy.ts @@ -61,6 +61,29 @@ export interface IAWSServiceAccessGenerator { makeConditionsToExceptFromDenyEveryoneElse(): Conditions; } +/** + * Check whether the provided access specs ensure that at least one principal can both read and administer configuration. + * @param accessSpecsByCapability is a map of access specs keyed by access capability + * + * @return true when at least one principal that can administer and read configuration exists + */ +export function canPrincipalsManageResources(accessSpecsByCapability: Map) { + console.log(`canPrincipalsManageResources eval'ing ${accessSpecsByCapability}`); + let adminSpec = accessSpecsByCapability.get(AccessCapability.ADMINISTER_RESOURCE); + let readConfigSpec = accessSpecsByCapability.get(AccessCapability.READ_CONFIG); + + if ((adminSpec?.allowPrincipalArns && adminSpec.allowPrincipalArns.length > 0) + && (readConfigSpec?.allowPrincipalArns && readConfigSpec.allowPrincipalArns.length > 0)) { + const adminPrincipals = new Set(adminSpec.allowPrincipalArns); + const readConfigPrincipals = new Set(readConfigSpec.allowPrincipalArns); + const intersection = new Set( + [...adminPrincipals].filter(x => readConfigPrincipals.has(x))); + return intersection.size > 0; + } + return false; +} + + export function toPascalCase(input: string): string { // Remove placeholders like ${something} and trim whitespace const cleanedInput = input.replace(/\$\{.*?\}/g, '').trim(); diff --git a/src/kms.ts b/src/kms.ts index e31cd3a..af7b617 100644 --- a/src/kms.ts +++ b/src/kms.ts @@ -9,6 +9,7 @@ import { } from 'aws-cdk-lib/aws-iam'; import { AccessCapability, + canPrincipalsManageResources, getAccessCapabilityFromValue, IAccessSpec, IAWSServiceAccessGenerator, @@ -90,21 +91,6 @@ export class CloudFrontOACReadAccessGenerator implements IAWSServiceAccessGenera } -function canPrincipalsManageKey(accessSpecsByCapability: Map) { - let adminSpec = accessSpecsByCapability.get(AccessCapability.ADMINISTER_RESOURCE); - let readConfigSpec = accessSpecsByCapability.get(AccessCapability.READ_CONFIG); - - if ((adminSpec?.allowPrincipalArns && adminSpec.allowPrincipalArns.length > 0) - && (readConfigSpec?.allowPrincipalArns && readConfigSpec.allowPrincipalArns.length > 0)) { - const adminPrincipals = new Set(adminSpec.allowPrincipalArns); - const readConfigPrincipals = new Set(readConfigSpec.allowPrincipalArns); - const intersection = new Set( - [...adminPrincipals].filter(x => readConfigPrincipals.has(x))); - return intersection.size > 0; - } - return false; -} - export function makeKeyPolicy(props: K9KeyPolicyProps): PolicyDocument { const policyFactory = new K9PolicyFactory(); const policy = new iam.PolicyDocument(); @@ -118,7 +104,7 @@ export function makeKeyPolicy(props: K9KeyPolicyProps): PolicyDocument { accessSpecsByCapability.set(getAccessCapabilityFromValue(capabilityStr), accessSpec); } - if (!canPrincipalsManageKey(accessSpecsByCapability)) { + if (!canPrincipalsManageResources(accessSpecsByCapability)) { throw Error('At least one principal must be able to administer and read-config for keys' + ' so encrypted data remains accessible; found:\n' + `administer-resource: '${accessSpecsByCapability.get(AccessCapability.ADMINISTER_RESOURCE)?.allowPrincipalArns}'\n` + diff --git a/test/k9policy.test.ts b/test/k9policy.test.ts index 8e21741..a099c9d 100644 --- a/test/k9policy.test.ts +++ b/test/k9policy.test.ts @@ -2,6 +2,7 @@ import { AnyPrincipal, PolicyStatement } from 'aws-cdk-lib/aws-iam'; import { stringifyStatement } from './helpers'; import { AccessCapability, + canPrincipalsManageResources, getAccessCapabilityFromValue, IAccessSpec, K9PolicyFactory, @@ -32,6 +33,76 @@ test('getAccessCapabilityFromValue throws error for undefined capabilities', () }).toThrow('Could not get AccessCapability from value: unknown-capability'); }); +describe('canPrincipalsManageResources', () => { + test('returns false when no principals have both administer-resource and read-config', () => { + const unmanageableCapabilityCombos = [ + [], + [AccessCapability.ADMINISTER_RESOURCE], + [AccessCapability.READ_CONFIG], + [AccessCapability.ADMINISTER_RESOURCE, AccessCapability.WRITE_DATA], + ]; + + for (let unmanageableAccessCapabilities of unmanageableCapabilityCombos) { + let accessSpecsByCapability = new Map(); + + for (let capability of unmanageableAccessCapabilities) { + let unmanageableAccessSpec = { + accessCapabilities: capability, + allowPrincipalArns: [ + 'arn:aws:iam::123456789012:role/not-an-admin', + ], + }; + accessSpecsByCapability.set(capability, unmanageableAccessSpec); + } + + expect(canPrincipalsManageResources(accessSpecsByCapability)).toBeFalsy(); + } + }); + + test('returns false when no there are principals with either administer-resource and read-config but not both', () => { + let accessSpecsByCapability = new Map(); + + accessSpecsByCapability.set(AccessCapability.ADMINISTER_RESOURCE, { + accessCapabilities: AccessCapability.ADMINISTER_RESOURCE, + allowPrincipalArns: [ + 'arn:aws:iam::123456789012:role/not-an-admin-1', + ], + }); + accessSpecsByCapability.set(AccessCapability.READ_CONFIG, { + accessCapabilities: AccessCapability.READ_CONFIG, + allowPrincipalArns: [ + 'arn:aws:iam::123456789012:role/not-an-admin-2', + ], + }); + + expect(canPrincipalsManageResources(accessSpecsByCapability)).toBeFalsy(); + }); + + test('returns true when a principal has both administer-resource and read-config', () => { + const manageableCapabilityCombos = [ + [AccessCapability.ADMINISTER_RESOURCE, AccessCapability.READ_CONFIG], + [AccessCapability.ADMINISTER_RESOURCE, AccessCapability.READ_CONFIG, AccessCapability.WRITE_DATA], + ]; + + for (let manageableAccessCapabilities of manageableCapabilityCombos) { + let accessSpecsByCapability = new Map(); + + for (let capability of manageableAccessCapabilities) { + let unmanageableAccessSpec = { + accessCapabilities: capability, + allowPrincipalArns: [ + 'arn:aws:iam::123456789012:role/admin', + ], + }; + accessSpecsByCapability.set(capability, unmanageableAccessSpec); + } + + expect(canPrincipalsManageResources(accessSpecsByCapability)).toBeTruthy(); + } + }); +}); + + test('toPascalCase converts classic Allow Restricted X SID', () => { expect(toPascalCase('Allow Restricted administer-resource')) .toEqual('AllowRestrictedAdministerResource'); From 4f96035f23aa0400831a0e6681713fbedd426338 Mon Sep 17 00:00:00 2001 From: Stephen Kuenzli Date: Tue, 10 Dec 2024 15:21:34 -0700 Subject: [PATCH 05/12] Revise DynamoDB resource policy generator to return array of AddToResourcePolicyResult like S3 * Return AddToResourcePolicyResult[] instead of a (parallel, disconnected) PolicyDocument * Expand assertions for typical use of DynamoDB generator --- src/dynamodb.ts | 29 +++++++++---------- src/k9policy.ts | 1 - test/k9.test.ts | 76 +++++++++++++++++++------------------------------ 3 files changed, 44 insertions(+), 62 deletions(-) diff --git a/src/dynamodb.ts b/src/dynamodb.ts index f7abbf0..4f0ba16 100644 --- a/src/dynamodb.ts +++ b/src/dynamodb.ts @@ -1,6 +1,10 @@ import { TableV2 } from 'aws-cdk-lib/aws-dynamodb'; -import { AccountRootPrincipal, Effect, PolicyDocument, PolicyStatement } from 'aws-cdk-lib/aws-iam'; -import * as iam from 'aws-cdk-lib/aws-iam'; +import { + AccountRootPrincipal, + AddToResourcePolicyResult, + Effect, + PolicyStatement, +} from 'aws-cdk-lib/aws-iam'; import { AccessCapability, canPrincipalsManageResources, @@ -24,12 +28,13 @@ let SUPPORTED_CAPABILITIES = new Array( export const SID_DENY_EVERYONE_ELSE = 'DenyEveryoneElse'; -export function grantAccessViaResourcePolicy(table: TableV2, props: K9DynamoDBResourcePolicyProps): PolicyDocument { +export function grantAccessViaResourcePolicy(table: TableV2, props: K9DynamoDBResourcePolicyProps): AddToResourcePolicyResult[] { const policyFactory = new K9PolicyFactory(); - const policy = new iam.PolicyDocument(); const resourceArns = ['*']; + const addToResourcePolicyResults = new Array(); + let accessSpecsByCapabilityRecs = policyFactory.mergeDesiredAccessSpecsByCapability(SUPPORTED_CAPABILITIES, props.k9DesiredAccess); let accessSpecsByCapability: Map = new Map(); @@ -50,13 +55,12 @@ export function grantAccessViaResourcePolicy(table: TableV2, props: K9DynamoDBRe Array.from(accessSpecsByCapability.values()), resourceArns, true); - policy.addStatements(...allowStatements); for (const allowStatement of allowStatements) { - table.addToResourcePolicy(allowStatement); + let addToResourcePolicyResult = table.addToResourcePolicy(allowStatement); + addToResourcePolicyResults.push(addToResourcePolicyResult); } - // console.log('Adding Allow root and DenyEveryoneElse statements'); const denyEveryoneElseStatement = new PolicyStatement({ sid: SID_DENY_EVERYONE_ELSE, effect: Effect.DENY, @@ -81,14 +85,9 @@ export function grantAccessViaResourcePolicy(table: TableV2, props: K9DynamoDBRe ], }); - policy.addStatements( - denyEveryoneElseStatement, - ); - - table.addToResourcePolicy(denyEveryoneElseStatement); + let addDenyEveryoneElseResult = table.addToResourcePolicy(denyEveryoneElseStatement); + addToResourcePolicyResults.push(addDenyEveryoneElseResult); table.resourcePolicy?.validateForResourcePolicy(); - policy.validateForResourcePolicy(); - - return policy; + return addToResourcePolicyResults; } diff --git a/src/k9policy.ts b/src/k9policy.ts index ca99857..ddd25a4 100644 --- a/src/k9policy.ts +++ b/src/k9policy.ts @@ -68,7 +68,6 @@ export interface IAWSServiceAccessGenerator { * @return true when at least one principal that can administer and read configuration exists */ export function canPrincipalsManageResources(accessSpecsByCapability: Map) { - console.log(`canPrincipalsManageResources eval'ing ${accessSpecsByCapability}`); let adminSpec = accessSpecsByCapability.get(AccessCapability.ADMINISTER_RESOURCE); let readConfigSpec = accessSpecsByCapability.get(AccessCapability.READ_CONFIG); diff --git a/test/k9.test.ts b/test/k9.test.ts index 5443a8f..e2f97ba 100644 --- a/test/k9.test.ts +++ b/test/k9.test.ts @@ -558,6 +558,7 @@ describe('DynamoDBResourcePolicy', () => { allowPrincipalArns: deleteDataArns, }, ); + test('Typical usage', () => { const stack = new cdk.Stack(app, 'K9DDBResourcePolicyTestTypicalUsage', { env: { region: 'us-east-1' } }); @@ -571,58 +572,41 @@ describe('DynamoDBResourcePolicy', () => { // resourcePolicy: resourcePolicy, }); - let resourcePolicy = k9.dynamodb.grantAccessViaResourcePolicy(table, ddbResourcePolicyProps); + let addToResourcePolicyResults = k9.dynamodb.grantAccessViaResourcePolicy(table, ddbResourcePolicyProps); console.log('table: ' + table); console.log('table.resourcePolicy: ' + stringifyPolicy(table.resourcePolicy)); - console.log('resourcePolicy: ' + stringifyPolicy(resourcePolicy)); - + console.log('addToResourcePolicyResults: ' + addToResourcePolicyResults); expect(table.resourcePolicy).toBeDefined(); - // const k9BucketPolicyProps: K9BucketPolicyProps = { - // bucket: table, - // k9DesiredAccess: new Array( - // { - // accessCapabilities: AccessCapability.ADMINISTER_RESOURCE, - // allowPrincipalArns: administerResourceArns, - // }, - // { - // accessCapabilities: AccessCapability.WRITE_DATA, - // allowPrincipalArns: writeDataArns, - // }, - // { - // accessCapabilities: AccessCapability.READ_DATA, - // allowPrincipalArns: readDataArns, - // }, - // { - // accessCapabilities: AccessCapability.DELETE_DATA, - // allowPrincipalArns: deleteDataArns, - // }, - // ), - // }; - // let addToResourcePolicyResults = k9.s3.grantAccessViaResourcePolicy(stack, 'S3Bucket', k9BucketPolicyProps); - // expect(table.resourcePolicy).toBeDefined(); - // - // let policyStr = stringifyPolicy(table.resourcePolicy); - // console.log('table.resourcePolicy?: ' + policyStr); - // expect(table.resourcePolicy).toBeDefined(); - // - // assertK9StatementsAddedToS3ResourcePolicy(addToResourcePolicyResults); - // let policyObj = JSON.parse(policyStr); - // let actualPolicyStatements = policyObj.Statement; - // expect(actualPolicyStatements).toBeDefined(); - // - // for (let stmt of actualPolicyStatements) { - // if (SID_DENY_UNEXPECTED_ENCRYPTION_METHOD == stmt.Sid) { - // expect(stmt.Condition.StringNotEquals['s3:x-amz-server-side-encryption']).toEqual('aws:kms'); - // } - // } - // console.log('stack to json: ' + stack.toJsonString(stack)) - - // expectCDK(stack).to(haveResource('AWS::DynamoDB::Table')); - // expectCDK(stack).to(haveResource('AWS::S3::BucketPolicy')); - // expect(SynthUtils.toCloudFormation(stack)).toMatchSnapshot(); + let policyStr = stringifyPolicy(table.resourcePolicy); + + let policyObj = JSON.parse(policyStr); + let actualPolicyStatements = policyObj.Statement; + expect(actualPolicyStatements).toBeDefined(); + + const expectStmtIds = [ + SID_DENY_EVERYONE_ELSE, + 'AllowRestrictedAdministerResource', + 'AllowRestrictedReadConfig', + 'AllowRestrictedReadData', + 'AllowRestrictedWriteData', + 'AllowRestrictedDeleteData', + ]; + expect(actualPolicyStatements).toHaveLength(expectStmtIds.length); + expect(addToResourcePolicyResults).toHaveLength(expectStmtIds.length); + + const policyStatementMap: { [key: string]: any } = {}; + for (let stmt of actualPolicyStatements) { + if (stmt.Sid) { + policyStatementMap[stmt.Sid] = stmt; + } + } + + for (let expectStmtId of expectStmtIds) { + expect(policyStatementMap[expectStmtId]).toBeTruthy(); + } }); }); From 8f4e6b9f25f9fb4dc69a3636b8d2b0466a707799 Mon Sep 17 00:00:00 2001 From: Stephen Kuenzli Date: Tue, 10 Dec 2024 16:50:54 -0700 Subject: [PATCH 06/12] Add integration test. Note: creates the (global) table but does not attach the resource policy. --- bin/k9-cdk.ts | 33 ++++++++++++++++++++++++++++++ test/__snapshots__/k9.test.ts.snap | 32 +++++++++++++++++++++++++++++ test/k9.test.ts | 4 +++- 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/bin/k9-cdk.ts b/bin/k9-cdk.ts index 4315738..578508e 100644 --- a/bin/k9-cdk.ts +++ b/bin/k9-cdk.ts @@ -9,6 +9,7 @@ import * as s3 from "aws-cdk-lib/aws-s3"; import {BlockPublicAccess, BucketEncryption} from "aws-cdk-lib/aws-s3"; import * as k9 from "../lib"; +import * as dynamodb from "aws-cdk-lib/aws-dynamodb"; const administerResourceArns = [ // for development @@ -182,6 +183,37 @@ const cloudfrontOACBucketPolicyProps: k9.s3.K9BucketPolicyProps = { k9.s3.grantAccessViaResourcePolicy(stack, "CloudFrontOACBucket", cloudfrontOACBucketPolicyProps); +// Demonstrate generating and applying a DynamoDB resource policy +const table = new dynamodb.TableV2(stack, 'k9-cdk-v2-int-test', { + partitionKey: { name: 'pk', type: dynamodb.AttributeType.STRING }, + removalPolicy: cdk.RemovalPolicy.DESTROY, +}); +const ddbResourcePolicyProps: k9.dynamodb.K9DynamoDBResourcePolicyProps = { + k9DesiredAccess: new Array( + { + accessCapabilities: k9.k9policy.AccessCapability.ADMINISTER_RESOURCE, + allowPrincipalArns: administerResourceArns, + }, + { + accessCapabilities: k9.k9policy.AccessCapability.READ_CONFIG, + allowPrincipalArns: readConfigArns, + }, + { + accessCapabilities: k9.k9policy.AccessCapability.READ_DATA, + allowPrincipalArns: readWriteDataArns, + }, + { + accessCapabilities: k9.k9policy.AccessCapability.WRITE_DATA, + allowPrincipalArns: readWriteDataArns, + }, + { + accessCapabilities: k9.k9policy.AccessCapability.DELETE_DATA, + allowPrincipalArns: readWriteDataArns, + }, + ) +}; +k9.dynamodb.grantAccessViaResourcePolicy(table, ddbResourcePolicyProps); + for (let construct of [bucket, websiteBucket, autoDeleteBucket, @@ -189,6 +221,7 @@ for (let construct of [bucket, // cloudfrontDistribution, cloudfrontOACBucket, cloudfrontOACKey, + table, ]) { Tags.of(construct).add('k9security:analysis', 'include'); } diff --git a/test/__snapshots__/k9.test.ts.snap b/test/__snapshots__/k9.test.ts.snap index e122bd9..cf907fe 100644 --- a/test/__snapshots__/k9.test.ts.snap +++ b/test/__snapshots__/k9.test.ts.snap @@ -1,5 +1,37 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`DynamoDBResourcePolicy Typical usage 1`] = ` +Object { + "Resources": Object { + "testtabletypicalusageD052F216": Object { + "DeletionPolicy": "Delete", + "Properties": Object { + "AttributeDefinitions": Array [ + Object { + "AttributeName": "pk", + "AttributeType": "S", + }, + ], + "BillingMode": "PAY_PER_REQUEST", + "KeySchema": Array [ + Object { + "AttributeName": "pk", + "KeyType": "HASH", + }, + ], + "Replicas": Array [ + Object { + "Region": "us-east-1", + }, + ], + }, + "Type": "AWS::DynamoDB::GlobalTable", + "UpdateReplacePolicy": "Delete", + }, + }, +} +`; + exports[`K9BucketPolicy - IAccessSpec with set of capabilities 1`] = ` Object { "Resources": Object { diff --git a/test/k9.test.ts b/test/k9.test.ts index e2f97ba..486a3ad 100644 --- a/test/k9.test.ts +++ b/test/k9.test.ts @@ -569,7 +569,6 @@ describe('DynamoDBResourcePolicy', () => { const table = new dynamodb.TableV2(stack, 'test-table-typical-usage', { partitionKey: { name: 'pk', type: dynamodb.AttributeType.STRING }, removalPolicy: cdk.RemovalPolicy.DESTROY, - // resourcePolicy: resourcePolicy, }); let addToResourcePolicyResults = k9.dynamodb.grantAccessViaResourcePolicy(table, ddbResourcePolicyProps); @@ -607,6 +606,9 @@ describe('DynamoDBResourcePolicy', () => { for (let expectStmtId of expectStmtIds) { expect(policyStatementMap[expectStmtId]).toBeTruthy(); } + + expectCDK(stack).to(haveResource('AWS::DynamoDB::GlobalTable')); + expect(SynthUtils.toCloudFormation(stack)).toMatchSnapshot(); }); }); From 6325dd42e713563e7d5596dbd6d3c45e696c288f Mon Sep 17 00:00:00 2001 From: Stephen Kuenzli Date: Tue, 10 Dec 2024 17:04:27 -0700 Subject: [PATCH 07/12] Switch back to generating a PolicyDocument for DynamoDB resource policy and providing on construction This is what actually works. It's not clear to me: 1. why the TableV2 resourcePolicy property is not populated from constructor within a unit test 2. why adding resource policy statements works in unit tests but not at runtime --- bin/k9-cdk.ts | 15 ++- src/dynamodb.ts | 30 ++--- src/k9policy.ts | 1 + test/__snapshots__/k9.test.ts.snap | 176 +++++++++++++++++++++++++++++ test/k9.test.ts | 30 ++--- 5 files changed, 213 insertions(+), 39 deletions(-) diff --git a/bin/k9-cdk.ts b/bin/k9-cdk.ts index 578508e..7732b87 100644 --- a/bin/k9-cdk.ts +++ b/bin/k9-cdk.ts @@ -184,10 +184,6 @@ const cloudfrontOACBucketPolicyProps: k9.s3.K9BucketPolicyProps = { k9.s3.grantAccessViaResourcePolicy(stack, "CloudFrontOACBucket", cloudfrontOACBucketPolicyProps); // Demonstrate generating and applying a DynamoDB resource policy -const table = new dynamodb.TableV2(stack, 'k9-cdk-v2-int-test', { - partitionKey: { name: 'pk', type: dynamodb.AttributeType.STRING }, - removalPolicy: cdk.RemovalPolicy.DESTROY, -}); const ddbResourcePolicyProps: k9.dynamodb.K9DynamoDBResourcePolicyProps = { k9DesiredAccess: new Array( { @@ -212,7 +208,16 @@ const ddbResourcePolicyProps: k9.dynamodb.K9DynamoDBResourcePolicyProps = { }, ) }; -k9.dynamodb.grantAccessViaResourcePolicy(table, ddbResourcePolicyProps); + + +const ddbResourcePolicy = k9.dynamodb.grantAccessViaResourcePolicy(ddbResourcePolicyProps); + +const table = new dynamodb.TableV2(stack, 'k9-cdk-v2-int-test', { + partitionKey: { name: 'pk', type: dynamodb.AttributeType.STRING }, + removalPolicy: cdk.RemovalPolicy.DESTROY, + resourcePolicy: ddbResourcePolicy +}); + for (let construct of [bucket, websiteBucket, diff --git a/src/dynamodb.ts b/src/dynamodb.ts index 4f0ba16..a053452 100644 --- a/src/dynamodb.ts +++ b/src/dynamodb.ts @@ -1,10 +1,5 @@ -import { TableV2 } from 'aws-cdk-lib/aws-dynamodb'; -import { - AccountRootPrincipal, - AddToResourcePolicyResult, - Effect, - PolicyStatement, -} from 'aws-cdk-lib/aws-iam'; +import { AccountRootPrincipal, Effect, PolicyDocument, PolicyStatement } from 'aws-cdk-lib/aws-iam'; +import * as iam from 'aws-cdk-lib/aws-iam'; import { AccessCapability, canPrincipalsManageResources, @@ -28,13 +23,12 @@ let SUPPORTED_CAPABILITIES = new Array( export const SID_DENY_EVERYONE_ELSE = 'DenyEveryoneElse'; -export function grantAccessViaResourcePolicy(table: TableV2, props: K9DynamoDBResourcePolicyProps): AddToResourcePolicyResult[] { +export function grantAccessViaResourcePolicy(props: K9DynamoDBResourcePolicyProps): PolicyDocument { const policyFactory = new K9PolicyFactory(); + const policy = new iam.PolicyDocument(); const resourceArns = ['*']; - const addToResourcePolicyResults = new Array(); - let accessSpecsByCapabilityRecs = policyFactory.mergeDesiredAccessSpecsByCapability(SUPPORTED_CAPABILITIES, props.k9DesiredAccess); let accessSpecsByCapability: Map = new Map(); @@ -55,11 +49,7 @@ export function grantAccessViaResourcePolicy(table: TableV2, props: K9DynamoDBRe Array.from(accessSpecsByCapability.values()), resourceArns, true); - - for (const allowStatement of allowStatements) { - let addToResourcePolicyResult = table.addToResourcePolicy(allowStatement); - addToResourcePolicyResults.push(addToResourcePolicyResult); - } + policy.addStatements(...allowStatements); const denyEveryoneElseStatement = new PolicyStatement({ sid: SID_DENY_EVERYONE_ELSE, @@ -85,9 +75,11 @@ export function grantAccessViaResourcePolicy(table: TableV2, props: K9DynamoDBRe ], }); - let addDenyEveryoneElseResult = table.addToResourcePolicy(denyEveryoneElseStatement); - addToResourcePolicyResults.push(addDenyEveryoneElseResult); - table.resourcePolicy?.validateForResourcePolicy(); + policy.addStatements( + denyEveryoneElseStatement, + ); + + policy.validateForResourcePolicy(); - return addToResourcePolicyResults; + return policy; } diff --git a/src/k9policy.ts b/src/k9policy.ts index ddd25a4..ca99857 100644 --- a/src/k9policy.ts +++ b/src/k9policy.ts @@ -68,6 +68,7 @@ export interface IAWSServiceAccessGenerator { * @return true when at least one principal that can administer and read configuration exists */ export function canPrincipalsManageResources(accessSpecsByCapability: Map) { + console.log(`canPrincipalsManageResources eval'ing ${accessSpecsByCapability}`); let adminSpec = accessSpecsByCapability.get(AccessCapability.ADMINISTER_RESOURCE); let readConfigSpec = accessSpecsByCapability.get(AccessCapability.READ_CONFIG); diff --git a/test/__snapshots__/k9.test.ts.snap b/test/__snapshots__/k9.test.ts.snap index cf907fe..e241a17 100644 --- a/test/__snapshots__/k9.test.ts.snap +++ b/test/__snapshots__/k9.test.ts.snap @@ -22,6 +22,182 @@ Object { "Replicas": Array [ Object { "Region": "us-east-1", + "ResourcePolicy": Object { + "PolicyDocument": Object { + "Statement": Array [ + Object { + "Action": Array [ + "dynamodb:CreateBackup", + "dynamodb:DeleteResourcePolicy", + "dynamodb:DeleteTableReplica", + "dynamodb:DisableKinesisStreamingDestination", + "dynamodb:EnableKinesisStreamingDestination", + "dynamodb:ExportTableToPointInTime", + "dynamodb:PutResourcePolicy", + "dynamodb:RestoreTableToPointInTime", + "dynamodb:TagResource", + "dynamodb:UntagResource", + "dynamodb:UpdateContinuousBackups", + "dynamodb:UpdateContributorInsights", + "dynamodb:UpdateKinesisStreamingDestination", + "dynamodb:UpdateTable", + "dynamodb:UpdateTableReplicaAutoScaling", + "dynamodb:UpdateTimeToLive", + ], + "Condition": Object { + "ArnEquals": Object { + "aws:PrincipalArn": Array [ + "arn:aws:iam::139710491120:user/ci", + ], + }, + }, + "Effect": "Allow", + "Principal": Object { + "AWS": "*", + }, + "Resource": "*", + "Sid": "AllowRestrictedAdministerResource", + }, + Object { + "Action": Array [ + "dynamodb:DescribeContinuousBackups", + "dynamodb:DescribeContributorInsights", + "dynamodb:DescribeExport", + "dynamodb:DescribeKinesisStreamingDestination", + "dynamodb:DescribeTable", + "dynamodb:DescribeTableReplicaAutoScaling", + "dynamodb:DescribeTimeToLive", + "dynamodb:GetResourcePolicy", + "dynamodb:ListTagsOfResource", + ], + "Condition": Object { + "ArnEquals": Object { + "aws:PrincipalArn": Array [ + "arn:aws:iam::139710491120:user/ci", + ], + }, + }, + "Effect": "Allow", + "Principal": Object { + "AWS": "*", + }, + "Resource": "*", + "Sid": "AllowRestrictedReadConfig", + }, + Object { + "Action": Array [ + "dynamodb:BatchGetItem", + "dynamodb:ConditionCheckItem", + "dynamodb:GetItem", + "dynamodb:PartiQLSelect", + "dynamodb:Query", + "dynamodb:Scan", + ], + "Condition": Object { + "ArnEquals": Object { + "aws:PrincipalArn": Array [ + "arn:aws:iam::123456789012:role/app-backend", + "arn:aws:iam::123456789012:role/customer-service", + ], + }, + }, + "Effect": "Allow", + "Principal": Object { + "AWS": "*", + }, + "Resource": "*", + "Sid": "AllowRestrictedReadData", + }, + Object { + "Action": Array [ + "dynamodb:BatchWriteItem", + "dynamodb:PartiQLInsert", + "dynamodb:PartiQLUpdate", + "dynamodb:PutItem", + "dynamodb:UpdateItem", + ], + "Condition": Object { + "ArnEquals": Object { + "aws:PrincipalArn": Array [ + "arn:aws:iam::123456789012:role/app-backend", + ], + }, + }, + "Effect": "Allow", + "Principal": Object { + "AWS": "*", + }, + "Resource": "*", + "Sid": "AllowRestrictedWriteData", + }, + Object { + "Action": Array [ + "dynamodb:DeleteItem", + "dynamodb:DeleteTable", + "dynamodb:DeleteTableReplica", + "dynamodb:PartiQLDelete", + ], + "Condition": Object { + "ArnEquals": Object { + "aws:PrincipalArn": Array [ + "arn:aws:iam::139710491120:user/super-admin", + ], + }, + }, + "Effect": "Allow", + "Principal": Object { + "AWS": "*", + }, + "Resource": "*", + "Sid": "AllowRestrictedDeleteData", + }, + Object { + "Action": "dynamodb:*", + "Condition": Object { + "ArnNotEquals": Object { + "aws:PrincipalArn": Array [ + Object { + "Fn::Join": Array [ + "", + Array [ + "arn:", + Object { + "Ref": "AWS::Partition", + }, + ":iam::", + Object { + "Ref": "AWS::AccountId", + }, + ":root", + ], + ], + }, + "arn:aws:iam::139710491120:user/ci", + "arn:aws:iam::123456789012:role/app-backend", + "arn:aws:iam::123456789012:role/customer-service", + "arn:aws:iam::139710491120:user/super-admin", + ], + }, + "Bool": Object { + "aws:PrincipalIsAWSService": Array [ + "false", + ], + }, + }, + "Effect": "Deny", + "Principal": Object { + "AWS": Array [ + "*", + "*", + ], + }, + "Resource": "*", + "Sid": "DenyEveryoneElse", + }, + ], + "Version": "2012-10-17", + }, + }, }, ], }, diff --git a/test/k9.test.ts b/test/k9.test.ts index 486a3ad..f514be8 100644 --- a/test/k9.test.ts +++ b/test/k9.test.ts @@ -566,23 +566,15 @@ describe('DynamoDBResourcePolicy', () => { k9DesiredAccess: desiredAccess, }; - const table = new dynamodb.TableV2(stack, 'test-table-typical-usage', { - partitionKey: { name: 'pk', type: dynamodb.AttributeType.STRING }, - removalPolicy: cdk.RemovalPolicy.DESTROY, - }); - - let addToResourcePolicyResults = k9.dynamodb.grantAccessViaResourcePolicy(table, ddbResourcePolicyProps); + let resourcePolicy = k9.dynamodb.grantAccessViaResourcePolicy(ddbResourcePolicyProps); + console.log('resourcePolicy: ' + stringifyPolicy(resourcePolicy)); - console.log('table: ' + table); - console.log('table.resourcePolicy: ' + stringifyPolicy(table.resourcePolicy)); - console.log('addToResourcePolicyResults: ' + addToResourcePolicyResults); - - expect(table.resourcePolicy).toBeDefined(); - - let policyStr = stringifyPolicy(table.resourcePolicy); + expect(resourcePolicy).toBeDefined(); + let policyStr = stringifyPolicy(resourcePolicy); let policyObj = JSON.parse(policyStr); let actualPolicyStatements = policyObj.Statement; + expect(actualPolicyStatements).toBeDefined(); const expectStmtIds = [ @@ -594,7 +586,6 @@ describe('DynamoDBResourcePolicy', () => { 'AllowRestrictedDeleteData', ]; expect(actualPolicyStatements).toHaveLength(expectStmtIds.length); - expect(addToResourcePolicyResults).toHaveLength(expectStmtIds.length); const policyStatementMap: { [key: string]: any } = {}; for (let stmt of actualPolicyStatements) { @@ -606,7 +597,16 @@ describe('DynamoDBResourcePolicy', () => { for (let expectStmtId of expectStmtIds) { expect(policyStatementMap[expectStmtId]).toBeTruthy(); } - + + const table = new dynamodb.TableV2(stack, 'test-table-typical-usage', { + partitionKey: { name: 'pk', type: dynamodb.AttributeType.STRING }, + removalPolicy: cdk.RemovalPolicy.DESTROY, + resourcePolicy: resourcePolicy, + }); + + console.log('table: ' + table); + console.log('table.resourcePolicy: ' + stringifyPolicy(table.resourcePolicy)); + expectCDK(stack).to(haveResource('AWS::DynamoDB::GlobalTable')); expect(SynthUtils.toCloudFormation(stack)).toMatchSnapshot(); }); From a80ad6ee51470d9cb172c08ea1b414604574361c Mon Sep 17 00:00:00 2001 From: Stephen Kuenzli Date: Tue, 10 Dec 2024 17:23:35 -0700 Subject: [PATCH 08/12] Allow the AWS Access Analyzer service role to read the table config and produce findings. --- bin/k9-cdk.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bin/k9-cdk.ts b/bin/k9-cdk.ts index 7732b87..6004c91 100644 --- a/bin/k9-cdk.ts +++ b/bin/k9-cdk.ts @@ -192,7 +192,9 @@ const ddbResourcePolicyProps: k9.dynamodb.K9DynamoDBResourcePolicyProps = { }, { accessCapabilities: k9.k9policy.AccessCapability.READ_CONFIG, - allowPrincipalArns: readConfigArns, + allowPrincipalArns: readConfigArns.concat([ + "arn:aws:iam::139710491120:role/aws-service-role/access-analyzer.amazonaws.com/AWSServiceRoleForAccessAnalyzer" + ]), }, { accessCapabilities: k9.k9policy.AccessCapability.READ_DATA, From c631cafd389fd68f7d3c77d7d8e3c2be7a1fdae7 Mon Sep 17 00:00:00 2001 From: Stephen Kuenzli Date: Wed, 11 Dec 2024 10:38:52 -0700 Subject: [PATCH 09/12] Rename DynamoDB policy generation function to makeResourcePolicy Better communicates that the function does not have side effects. Aligns to name for side-effect free KMS policy generator. --- bin/k9-cdk.ts | 4 ++-- src/dynamodb.ts | 9 ++++++++- test/k9.test.ts | 2 +- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/bin/k9-cdk.ts b/bin/k9-cdk.ts index 6004c91..c1d0690 100644 --- a/bin/k9-cdk.ts +++ b/bin/k9-cdk.ts @@ -6,10 +6,10 @@ import {RemovalPolicy, Tags} from "aws-cdk-lib"; // import * as cforigins from "aws-cdk-lib/aws-cloudfront-origins"; import * as kms from "aws-cdk-lib/aws-kms"; import * as s3 from "aws-cdk-lib/aws-s3"; +import * as dynamodb from "aws-cdk-lib/aws-dynamodb"; import {BlockPublicAccess, BucketEncryption} from "aws-cdk-lib/aws-s3"; import * as k9 from "../lib"; -import * as dynamodb from "aws-cdk-lib/aws-dynamodb"; const administerResourceArns = [ // for development @@ -212,7 +212,7 @@ const ddbResourcePolicyProps: k9.dynamodb.K9DynamoDBResourcePolicyProps = { }; -const ddbResourcePolicy = k9.dynamodb.grantAccessViaResourcePolicy(ddbResourcePolicyProps); +const ddbResourcePolicy = k9.dynamodb.makeResourcePolicy(ddbResourcePolicyProps); const table = new dynamodb.TableV2(stack, 'k9-cdk-v2-int-test', { partitionKey: { name: 'pk', type: dynamodb.AttributeType.STRING }, diff --git a/src/dynamodb.ts b/src/dynamodb.ts index a053452..bab9e7d 100644 --- a/src/dynamodb.ts +++ b/src/dynamodb.ts @@ -23,7 +23,14 @@ let SUPPORTED_CAPABILITIES = new Array( export const SID_DENY_EVERYONE_ELSE = 'DenyEveryoneElse'; -export function grantAccessViaResourcePolicy(props: K9DynamoDBResourcePolicyProps): PolicyDocument { +/** + * Generate a DynamoDB resource policy from the provided props that can be attached to DynamoDB + * resources, particularly tables & indices. + * + * @param props specifying desired access + * @return a PolicyDocument that can be attached to DynamoDB resources + */ +export function makeResourcePolicy(props: K9DynamoDBResourcePolicyProps): PolicyDocument { const policyFactory = new K9PolicyFactory(); const policy = new iam.PolicyDocument(); diff --git a/test/k9.test.ts b/test/k9.test.ts index f514be8..72a6b91 100644 --- a/test/k9.test.ts +++ b/test/k9.test.ts @@ -566,7 +566,7 @@ describe('DynamoDBResourcePolicy', () => { k9DesiredAccess: desiredAccess, }; - let resourcePolicy = k9.dynamodb.grantAccessViaResourcePolicy(ddbResourcePolicyProps); + let resourcePolicy = k9.dynamodb.makeResourcePolicy(ddbResourcePolicyProps); console.log('resourcePolicy: ' + stringifyPolicy(resourcePolicy)); expect(resourcePolicy).toBeDefined(); From f1b8a257466619d2d65a783bcc991d1c03dcec02 Mon Sep 17 00:00:00 2001 From: Stephen Kuenzli Date: Wed, 11 Dec 2024 10:49:45 -0700 Subject: [PATCH 10/12] Demonstrate use of DynamoDB policy generator in README --- README.md | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 1b134ec..c6a6e83 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,7 @@ Supported services: * S3 * KMS +* DynamoDB This library [simplifies IAM as described in Effective IAM for AWS](https://www.effectiveiam.com/simplify-aws-iam) and is fully-supported by k9 Security. We're happy to answer questions or help you integrate it via a [GitHub issue](https://github.com/k9securityio/k9-cdk/issues) or email to [support@k9security.io](mailto:support@k9security.io?subject=k9-cdk). @@ -34,7 +35,8 @@ const administerResourceArns = [ ]; const readConfigArns = administerResourceArns.concat([ - "arn:aws:iam::123456789012:role/k9-auditor" + "arn:aws:iam::123456789012:role/k9-auditor", + "arn:aws:iam::123456789012:role/aws-service-role/access-analyzer.amazonaws.com/AWSServiceRoleForAccessAnalyzer" ]); const app = new cdk.App(); @@ -93,20 +95,40 @@ new kms.Key(stack, 'KMSKey', { }); ``` -The example stack demonstrates full use of the k9 S3 and KMS policy generators. Generated policies: +Protecting a DynamoDB table follows the same path as KMS, generating a policy then providing it to the DynamoDB table construct via props: + +```typescript +import * as dynamodb from "aws-cdk-lib/aws-dynamodb"; + +const ddbResourcePolicyProps: k9.dynamodb.K9DynamoDBResourcePolicyProps = { + k9DesiredAccess: k9BucketPolicyProps.k9DesiredAccess +}; + + +const ddbResourcePolicy = k9.dynamodb.makeResourcePolicy(ddbResourcePolicyProps); + +const table = new dynamodb.TableV2(stack, 'app-table-with-k9-policy', { + partitionKey: { name: 'pk', type: dynamodb.AttributeType.STRING }, + resourcePolicy: ddbResourcePolicy, +}); +``` + +The example stack demonstrates full use of the k9 S3, KMS, and DynamoDB policy generators. Generated policies: S3 Bucket Policy: + * [Templatized Bucket Policy](examples/generated.bucket-policy.json) * [BucketPolicy resource in CFn template](examples/K9Example.template.json) KMS Key Policy: + * [Templatized Key Policy](examples/generated.key-policy.json) * [KeyPolicy attribute of Key resource in CFn template](examples/K9Example.template.json) ## Specialized Use Cases k9-cdk can be configured to support specialized use cases, including: -* [Public Bucket](docs/use-case-public-bucket.md) - Publicaly readable objects, least privilege for all other actions +* [Public Bucket](docs/use-case-public-bucket.md) - Publicly readable objects, least privilege for all other actions ## Local Development and Testing From e54ce5949b76d0a3d5f7c86d2f018edf5fedc7d8 Mon Sep 17 00:00:00 2001 From: Stephen Kuenzli Date: Thu, 12 Dec 2024 08:27:03 -0700 Subject: [PATCH 11/12] Remove unintended console logging --- src/k9policy.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/k9policy.ts b/src/k9policy.ts index ca99857..ddd25a4 100644 --- a/src/k9policy.ts +++ b/src/k9policy.ts @@ -68,7 +68,6 @@ export interface IAWSServiceAccessGenerator { * @return true when at least one principal that can administer and read configuration exists */ export function canPrincipalsManageResources(accessSpecsByCapability: Map) { - console.log(`canPrincipalsManageResources eval'ing ${accessSpecsByCapability}`); let adminSpec = accessSpecsByCapability.get(AccessCapability.ADMINISTER_RESOURCE); let readConfigSpec = accessSpecsByCapability.get(AccessCapability.READ_CONFIG); From 716d34710bdad55814c6fedf9edd058b7cf33828 Mon Sep 17 00:00:00 2001 From: Stephen Kuenzli Date: Thu, 12 Dec 2024 08:28:59 -0700 Subject: [PATCH 12/12] Document toPascalCase function. --- src/k9policy.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/k9policy.ts b/src/k9policy.ts index ddd25a4..761a8fd 100644 --- a/src/k9policy.ts +++ b/src/k9policy.ts @@ -83,6 +83,12 @@ export function canPrincipalsManageResources(accessSpecsByCapability: Map