feat(aws): add check secretsmanager_has_restrictive_resource_policy#6985
feat(aws): add check secretsmanager_has_restrictive_resource_policy#6985kagahd wants to merge 16 commits intoprowler-cloud:masterfrom
secretsmanager_has_restrictive_resource_policy#6985Conversation
...nager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy_test.py
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6985 +/- ##
==========================================
- Coverage 93.37% 3.98% -89.40%
==========================================
Files 219 836 +617
Lines 30415 24095 -6320
==========================================
- Hits 28399 959 -27440
- Misses 2016 23136 +21120
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
secretsmanager_has_restrictive_resource_policy
1f9ea7d to
0b693ef
Compare
|
Hello @kagahd I apologise because this PR got lost in our inbox. The team is going to review it as soon as possible. Thanks! |
...etsmanager_has_restrictive_resource_policy/secretsmanager_has_restrictive_resource_policy.py
Show resolved
Hide resolved
|
Hi @kagahd, Are you planning on continuing this development? Just to know if we should proceed to close it/try to finish it ourselves. Thanks! |
Thanks @HugoPBrito for reminding me. |
Hi @kagahd! As you prefer. As long as they're linked in case you create a new one, that's up to what's best for you. |
|
✅ Conflict Markers Resolved All conflict markers have been successfully resolved in this pull request. |
|
Hi @HugoPBrito, I have now updated the PR to the latest version, including the revised description. |
|
Hi @kagahd, I get your point, but we will need to evaluate it. I believe this will end up merged, since it brings a lot of value, and if someone don't want to deal with it, they can always mute it. Since the pr is huge and complex, the review will be most likely slow. In the meantime, please adapt the metadata to the new standard, defined in our docs on the metadata guidelines page. Please feel free to add information you find valuable to the |
|
Hi @HugoPBrito
It's done. Let me know if anything needs further adjustment. Here's a summary of the changes:
|
…Deny statement of a resource-based policy
- Add cross-account access detection in Allow statements - Implement validation for ArnNotLike condition patterns with minimum 12-char prefix - Refactor Deny statement validation with stricter condition checks (case 1: both PrincipalArn and PrincipalServiceName with StringNotEqualsIfExists + Null; case 2: only PrincipalArn with StringNotEquals) - Add detailed error messages with up to 3 principals/services displayed - Integrate is_condition_block_restrictive from IAM policy utilities for consistent validation - Add improved handling for wildcard principals and service-based access - Include comprehensive validation for Allow statements with Service Principals
58c8701 to
683b4f8
Compare
Context
This feature request offers a new AWS check
secretsmanager_has_restrictive_resource_policy.The check evaluates resource-based policies for AWS Secrets Manager secrets.
Description
To pass the check, resource-based policies attached to secrets must meet all of the following criteria:
1. Explicit "Deny" Statement for Unauthorized Principals
The resource policy must include an explicit
Denystatement that applies to all principals, all actions, and all resources, unless exceptions are defined in theStringNotEqualsCondition block.Example
This structure ensures that unauthorized access is denied across the board.
Principal:
"*"Applies the denial to all entities, internal or external, preventing unauthorized principals from accessing the secret.
Action:
"*"or"secretsmanager:*"Covers all possible actions, ensuring no unauthorized operation—directly or indirectly—can be performed on the secret. Using
"secretsmanager:*"is acceptable to limit the denial to actions related specifically to Secrets Manager.Resource:
"*"Because resource-based policies are inherently tied to the secret they protect, this field can either use a wildcard (
"*") for broader applicability or the exact ARN of the secret for stricter targeting.The
StringNotEqualsCondition block refines the denial by allowing exceptions, such as specific roles defined inaws:PrincipalArn.One mandatory exception must be given to
<ASSUMED-ROLE-ARN-FOR-SECURY-AUDIT>because this role, which is deployed in every organization's AWS account, must be allowed to audit the secret by the IT security team.All Principal ARNs that will have their own policy entry must also be listed here. Otherwise, this rule would already block any access attempts.
Using wildcards in Principal ARNs
The check allows using an
ArnNotLikecondition with keyaws:PrincipalArnwithin the explicitDenystatement for unauthorized principals. The role name must contain at least 12 characters before the wildcard.Example
Excluding AWS services from the Deny
If AWS services must also be excluded from the Deny statement, the
StringNotEqualsIfExistscondition operator must be used instead ofStringNotEquals. This ensures that both principals and services can be excluded even though only one of them may exist in the request.Additionally, a
Nullcondition block must be included which contains bothaws:PrincipalArnandaws:PrincipalServiceNamewith value"true". This ensures that requests missing both keys are denied.Example
2. Restrict Access Outside the Organization
The second mandatory
Denystatement ensures that principals outside the AWS Organization are denied access to the secret.The
StringNotEqualscondition ensures that the denial applies to any request coming from outside of the organization, as identified by theaws:PrincipalOrgIDcondition key.The value(s) of
aws:PrincipalOrgIDcan be configured via the keyorganizations_trusted_idsin prowler's config.yaml.If the AWS account is not part of an organization, this value can be left empty in the configuration. In that case, the check
secretsmanager_has_restrictive_resource_policywill not require theDenyOutsideOrganizationstatement.Dealing with AWS Services
AWS services (e.g.
"appflow.amazonaws.com") do not send aPrincipalOrgIDin their requests. Therefore the service name must also be added to the condition block.Example
This statement alone does not guarantee that the service runs within the same AWS account or organization.
Therefore the service must additionally be restricted in its
Allowstatement usingaws:SourceAccount(see section 4. Allow Statement for AWS services).3. Deny Statement to Permit Only Specific Actions for Designated Principals
These
Denystatements ensure that specific principals are restricted to performing only specific actions.These statements are optional, but if omitted the principals listed in
DenyUnauthorizedPrincipalswill effectively be denied all actions.Any principal listed in the
StringNotEqualscondition ofDenyUnauthorizedPrincipalsmust have a correspondingDenystatement with a tailoredNotActionclause.Wildcards (
*) are not allowed inNotAction. Refer to the supported AWS Secrets Manager actions.The IT security team must be able to audit the resource-based policy. Therefore the principal
<ASSUMED-ROLE-ARN-FOR-SECURY-AUDIT>must be permitted the actions:secretsmanager:DescribeSecretsecretsmanager:GetResourcePolicyExample
Using wildcards for principals
The check allows using
ArnLikewithaws:PrincipalArnto define the allowed actions for principals that were specified viaArnNotLikein theDenyUnauthorizedPrincipalsstatement.The role name must again contain at least 12 characters before the wildcard.
Example
4. Allow Statement for AWS Services
AWS services require an explicit
Allowstatement to perform actions on the secret.Two requirements apply:
*) in any ActionExample
Cross-account access
Cross-account access to secrets is not allowed.
Resource-based policies that allow cross-account access will cause the check to fail on purpose. This behavior is intentional to ensure that any cross-account access is made explicitly visible during reviews and compliance checks, since from an IT Security perspective it expands the trust boundary and therefore requires deliberate assessment and clear justification.
Complete Resource-Based Policy Example
Below is a full example resource policy for scenarios where only IAM principals require access.
Unit Tests
As shown in
secretsmanager_has_restrictive_resource_policy_test.py, several unit tests were created.Three of them alone cover already 105 different scenarios by modifying parts of an otherwise valid resource policy to ensure the check correctly fails.
test_secretsmanager_policies_for_principals= 73 test casestest_secretsmanager_policies_for_principals_and_services= 29 test casestest_policy_with_arn_not_like= 3 test casesEach negative test case is followed by a counter-test that removes the invalid change to ensure the policy passes, verifying that the test logic itself is correct.
@pytest.mark.parametrizeis used to reduce redundancy while maintaining readability.Checklist
README.mdare required.API
CHANGELOG.mdif applicable.License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.