-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Generate DynamoDB resource policies #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
54173ae
231756c
72e5242
6b976a2
4f96035
8f4e6b9
6325dd4
a80ad6e
c631caf
f1b8a25
e54ce59
716d347
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 [[email protected]](mailto:[email protected]?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 | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| import { AccountRootPrincipal, Effect, PolicyDocument, PolicyStatement } from 'aws-cdk-lib/aws-iam'; | ||
| import * as iam from 'aws-cdk-lib/aws-iam'; | ||
| import { | ||
| AccessCapability, | ||
| canPrincipalsManageResources, | ||
| getAccessCapabilityFromValue, | ||
| IAccessSpec, | ||
| K9PolicyFactory, | ||
| } from './k9policy'; | ||
|
|
||
|
|
||
| export interface K9DynamoDBResourcePolicyProps { | ||
| readonly k9DesiredAccess: Array<IAccessSpec>; | ||
| } | ||
|
|
||
| let SUPPORTED_CAPABILITIES = new Array<AccessCapability>( | ||
| AccessCapability.ADMINISTER_RESOURCE, | ||
| AccessCapability.READ_CONFIG, | ||
| AccessCapability.READ_DATA, | ||
| AccessCapability.WRITE_DATA, | ||
| AccessCapability.DELETE_DATA, | ||
| ); | ||
|
|
||
| export const SID_DENY_EVERYONE_ELSE = 'DenyEveryoneElse'; | ||
|
|
||
| /** | ||
| * 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 { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This works for our needs, but I was wondering if you still wanted to wrap this in a |
||
| const policyFactory = new K9PolicyFactory(); | ||
| const policy = new iam.PolicyDocument(); | ||
|
|
||
| const resourceArns = ['*']; | ||
|
|
||
| let accessSpecsByCapabilityRecs = policyFactory.mergeDesiredAccessSpecsByCapability(SUPPORTED_CAPABILITIES, props.k9DesiredAccess); | ||
| let accessSpecsByCapability: Map<AccessCapability, IAccessSpec> = 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' + | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good check to have. |
||
| ' 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, | ||
| true); | ||
| policy.addStatements(...allowStatements); | ||
|
|
||
| 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, | ||
| ); | ||
|
|
||
| policy.validateForResourcePolicy(); | ||
|
|
||
| return policy; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| export * as k9policy from './k9policy'; | ||
| export * as dynamodb from './dynamodb'; | ||
| export * as kms from './kms'; | ||
| export * as s3 from './s3'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,6 +61,49 @@ 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<AccessCapability, IAccessSpec>) { | ||
| 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<string>(adminSpec.allowPrincipalArns); | ||
| const readConfigPrincipals = new Set<string>(readConfigSpec.allowPrincipalArns); | ||
| const intersection = new Set( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately, I don't see an Looks like an upgrade might help here. However, it's tough when CDK supports all the way back to Node |
||
| [...adminPrincipals].filter(x => readConfigPrincipals.has(x))); | ||
| return intersection.size > 0; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
|
|
||
| /** | ||
| * Converts a string to PascalCase, which is useful for e.g. policy types that don't | ||
| * do not support spaces or hyphens in statement ids. | ||
| * | ||
| * @param input | ||
| */ | ||
| export function toPascalCase(input: string): string { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you hit a different naming restriction here that forced the casing?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct. DDB doesn't allow spaces nor hyphens. |
||
| // 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 { | ||
|
|
||
| /** | ||
|
|
@@ -86,6 +129,7 @@ export class K9PolicyFactory { | |
| _SUPPORTED_SERVICES = new Set<string>([ | ||
| 'S3', | ||
| 'KMS', | ||
| 'DynamoDB', | ||
| ]); | ||
|
|
||
| /** @internal */ | ||
|
|
@@ -178,7 +222,8 @@ export class K9PolicyFactory { | |
| makeAllowStatements(serviceName: string, | ||
| supportedCapabilities: Array<AccessCapability>, | ||
| desiredAccess: Array<IAccessSpec>, | ||
| resourceArns: Array<string>): Array<PolicyStatement> { | ||
| resourceArns: Array<string>, | ||
| usePascalCase: boolean = false): Array<PolicyStatement> { | ||
| let policyStatements = new Array<PolicyStatement>(); | ||
| let accessSpecsByCapabilityRecs = this.mergeDesiredAccessSpecsByCapability(supportedCapabilities, desiredAccess); | ||
| let accessSpecsByCapability: Map<AccessCapability, IAccessSpec> = new Map(); | ||
|
|
@@ -201,7 +246,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, | ||
|
|
@@ -273,4 +323,3 @@ export class K9PolicyFactory { | |
| } | ||
|
|
||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These permissions for global tables very likely need to stay. The admin user should be allowed to make replicas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These permissions changes were the result of:
Here are the
administer-resourcepermissions from the full set of DDB perms that the DDB resource policy API rejected:(I have similar output for
read-config,read-data, andwrite-dataif you are interested)I can see why you logically you want an admin of a table to create a replica. However, I think that probably has to be granted from the Identity side. That aligns more with how
Create[Resource]is generally not supported in resource policy. And in this specific case, I would think the principal CFn uses to create the table resources would be the one that needs permissions.wdyt?