-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Conversation
Codecov ReportAttention: Patch coverage is
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. |
dynamodb_table_cross_account_access
check
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.
Super great work @sergargar 👏
I left a suggestion and I think some lines are duplicated in the tests, other than that the rest is 🔝
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 |
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.
Could you please move this to a function in a lib
, I think the same always, we are creating this over and over again.
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.
That's right, I have created a task to create a function and using it in these checks!
...ces/dynamodb/dynamodb_table_cross_account_access/dynamodb_table_cross_account_access_test.py
Outdated
Show resolved
Hide resolved
...ces/dynamodb/dynamodb_table_cross_account_access/dynamodb_table_cross_account_access_test.py
Outdated
Show resolved
Hide resolved
...ces/dynamodb/dynamodb_table_cross_account_access/dynamodb_table_cross_account_access_test.py
Outdated
Show resolved
Hide resolved
...ces/dynamodb/dynamodb_table_cross_account_access/dynamodb_table_cross_account_access_test.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Pepe Fagoaga <[email protected]>
Co-authored-by: Pepe Fagoaga <[email protected]>
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.