Skip to content
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

feat(aws): new dynamodb_table_cross_account_access check #3932

Merged
merged 3 commits into from
May 7, 2024

Conversation

sergargar
Copy link
Member

@sergargar sergargar commented May 6, 2024

Description

@jchrisfarris in #3867: On march 20th, AWS announced Amazon DynamoDB now supports resource-based policies. It would be good to have a Prowler check to look for DDB tables that have been shared to other accounts (note: it's not possible to share a table publicly, so this is good).

Therefore, adding the new check: dynamodb_table_cross_account_access

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the provider/aws Issues/PRs related with the AWS provider label May 6, 2024
@sergargar sergargar linked an issue May 6, 2024 that may be closed by this pull request
Copy link

codecov bot commented May 6, 2024

Codecov Report

Attention: Patch coverage is 85.00000% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 86.40%. Comparing base (a54a0dd) to head (2d16813).
Report is 13 commits behind head on master.

Files Patch % Lines
...roviders/aws/services/dynamodb/dynamodb_service.py 65.00% 7 Missing ⚠️
...ount_access/dynamodb_table_cross_account_access.py 95.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3932      +/-   ##
==========================================
+ Coverage   86.37%   86.40%   +0.02%     
==========================================
  Files         748      749       +1     
  Lines       23333    23393      +60     
==========================================
+ Hits        20155    20213      +58     
- Misses       3178     3180       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sergargar sergargar changed the title feat(aws): new dynamodb_table_cross_account_access check feat(aws): new dynamodb_table_cross_account_access check May 6, 2024
@sergargar sergargar marked this pull request as ready for review May 6, 2024 15:29
@sergargar sergargar requested review from a team as code owners May 6, 2024 15:29
Copy link
Member

@jfagoagas jfagoagas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super great work @sergargar 👏

I left a suggestion and I think some lines are duplicated in the tests, other than that the rest is 🔝

Comment on lines +29 to +49
for statement in policy_statements:
if not cross_account_access:
if statement["Effect"] == "Allow":
if "AWS" in statement["Principal"]:
principals = statement["Principal"]["AWS"]
if not isinstance(principals, list):
principals = [principals]
else:
principals = [statement["Principal"]]
for aws_account in principals:
if (
dynamodb_client.audited_account not in aws_account
or "*" == aws_account
):
cross_account_access = True
# Check if the condition block is restrictive
conditions = statement.get("Condition", {})
if is_condition_block_restrictive(
conditions, dynamodb_client.audited_account
):
cross_account_access = False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please move this to a function in a lib, I think the same always, we are creating this over and over again.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right, I have created a task to create a function and using it in these checks!

@sergargar sergargar requested a review from jfagoagas May 7, 2024 11:09
@jfagoagas jfagoagas merged commit 6f0dc44 into master May 7, 2024
10 of 11 checks passed
@jfagoagas jfagoagas deleted the dynamodb-table-check branch May 7, 2024 11:36
pedrooot pushed a commit that referenced this pull request May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
provider/aws Issues/PRs related with the AWS provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Look for externally shared DynamoDB Tables
2 participants