feat(aws): add check secretsmanager_has_restrictive_resource_policy

[kagahd]

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 Deny statement that applies to all principals, all actions, and all resources, unless exceptions are defined in the StringNotEquals or StringNotEqualsIfExists Condition block.
Here is an example:

 {
  "Sid": "DenyUnauthorizedPrincipals",
  "Effect": "Deny",
  "Principal": "*",
  "Action": "*",
  "Resource": "*",
  "Condition": {
    "StringNotEquals": {
      "aws:PrincipalArn": [
        "<ASSUMED-ROLE-ARN-FOR-SECURY-AUDIT>",
        "arn:aws:iam::<YOUR_AWS_ACCOUNT_ID>:role/Role2"
      ]
    }
  }
}

The StringNotEquals or StringNotEqualsIfExists Condition block refines the denial by allowing exceptions, such as specific roles defined in aws:PrincipalArn.
One mandatory exception must be given to <ASSUMED-ROLE-ARN-FOR-SECURY-AUDIT> because this role, which is deployed in every organizations AWS account, must be allowed to audit it by the IT security team.
All Principal ARNs that will have an own Policy Entry need to be listed here too, as shown in this example by the entry arn:aws:iam::<YOUR_AWS_ACCOUNT_ID>:role/Role2. Otherwise this rule already blocks any access tries.

When to use which Condition Operator?
Use StringNotEquals if you want to specify Principals by their ARN, as shown in the example above.
Use StringNotEqualsIfExists if you furthermore need to specify AWS services by their name, as shown in the following example. Since the request will contain either an AWS principal or the AWS service, we need to use StringNotEqualsIfExists instead of StringNotEquals in such cases.

    {
      "Sid": "DenyUnauthorizedPrincipals",
      "Effect": "Deny",
      "Principal": "*",
      "Action": "*",
      "Resource": "*",
      "Condition": {
        "StringNotEqualsIfExists": {
          "aws:PrincipalArn": [
            "<ASSUMED-ROLE-ARN-FOR-SECURY-AUDIT>",
            "arn:aws:iam::<YOUR_AWS_ACCOUNT_ID>:role/Role2"
          ],
          "aws:PrincipalServiceName": "appflow.amazonaws.com"
        }
      }
    }

2. Restrict Access Outside the Organization

The second mandatory Deny statement ensures that principals outside of the organization are denied to access the secret:

{
  "Sid": "DenyOutsideOrganization",
  "Effect": "Deny",
  "Principal": "*",
  "Action": "*",
  "Resource": "*",
  "Condition": {
    "StringNotEquals": {
      "aws:PrincipalOrgID": "<YOUR_ORG_ID>"
    }
  }
}

The first four fields, Effect, Principal, Action and Resource have the same meaning as in the Deny statement DenyUnauthorizedPrincipals shown and described above.
The StringNotEquals Condition ensures that the denial applies to any request coming from outside of the organization, as identified by the aws:PrincipalOrgID condition key. Only requests originating from the organization are exempted.
The value(s) of aws:PrincipalOrgID is configurable by the key organizations_trusted_ids within prowler's config.yaml file.
Leave this value empty in prowler's config.yaml if the AWS account is not bound to an organization. The check secretsmanager_has_restrictive_resource_policy will then not require the DenyOutsideOrganization statement in the policy.

However, AWS services e.g. "appflow.amazonaws.com", do not send a PrincipalOrgID in their request. Therefore, the policy statement must exclude those service principals by using NotPrincipal as the following "DenyOutsideOrganization" statement demonstrates:

{
  "Sid": "DenyOutsideOrganization",
  "Effect": "Deny",
  "NotPrincipal": {
        "Service": "appflow.amazonaws.com"
   }
  "Action": "*",
  "Resource": "*",
  "Condition": {
    "StringNotEquals": {
      "aws:PrincipalOrgID": "<YOUR_ORG_ID>"
    }
  }
}

3. Deny Statement to Permit Only Specific Actions for Designated Principals

The third Deny statements ensures that specific principals are restricted to performing only specific actions. This Deny statements is not mandatory but if you omit it, the AWS principals specified in the DenyUnauthorizedPrincipals statement will be denied for all actions.
Any principal listed in the StringNotEquals condition of the example Deny statement DenyUnauthorizedPrincipals shown above (e.g.,<ASSUMED-ROLE-ARN-FOR-SECURY-AUDIT>, arn:aws:iam::<YOUR_AWS_ACCOUNT_ID>:role/Role2) needs a corresponding Deny statement with a tailored NotAction clause to ensure, that each principal can perform only the specific actions they require, while all other actions are denied by default.

Wildcards * in the NotAction field are not permitted, please refer to the supported AWS Secrets Manager actions.
It is mandatory to ensure, that the IT Security team can audit the resource-based policy attached to a Secrets Manager secret. That's why the principal representing the security team (e.g., <ASSUMED-ROLE-ARN-FOR-SECURY-AUDIT>) must be permitted to both actions secretsmanager:DescribeSecret and secretsmanager:GetResourcePolicy, as show in the following example.

{
  "Sid": "AllowAuditPolicyRead",
  "Effect": "Deny",
  "Principal": {
    "AWS": "<ASSUMED-ROLE-ARN-FOR-SECURY-AUDIT>"
  },
  "NotAction": [ "secretsmanager:DescribeSecret", "secretsmanager:GetResourcePolicy" ],
  "Resource": "*"
},
{
  "Sid": "AllowSecretAccessForRole2",
  "Effect": "Deny",
  "Principal": {
    "AWS": "arn:aws:iam::<YOUR_AWS_ACCOUNT_ID>:role/Role2"
  },
  "NotAction": [ "secretsmanager:GetSecretValue" ],
  "Resource": "*"
}

4. Allow Statement for AWS services

AWS services need an explizit Allow Statement in order to be granted to execute actions on the secret. There are two requirements:

1. No wildcard * in any Action.
2. The AWS source account must be the same AWS account to avoid that services running in external AWS accounts get access to your secrets. Please use the StringEquals Condition operator as shown in the following example:

{
    "Sid": "AllowAppFlowAccess",
    "Effect": "Allow",
    "Principal":
    {
        "Service": "appflow.amazonaws.com"
    },
    "Action":
    [
        "secretsmanager:GetSecretValue",
        "secretsmanager:PutSecretValue",
        "secretsmanager:DeleteSecret",
        "secretsmanager:DescribeSecret",
        "secretsmanager:UpdateSecret"
    ],
    "Resource": "*",
    "Condition":
    {
        "StringEquals":
        {
            "aws:SourceAccount": "<YOUR_AWS_ACCOUNT_ID>"
        }
    }
}

Cross-account access to secrets is not allowed. Resource based policies attached to secrets that allow cross-account access to the secret result in failing the check. But maybe this should/could be configurable.

Unit Tests

As you can see in the secretsmanager_has_restrictive_resource_policy_test.py, I have created five unit tests, but three of them cover around 50 test cases by modifying specific parts of a valid resource based policy to let the check FAIL. In order to proof that the check PASSes without this invalid part of the policy, a corresponding test case is run right after. This counter-test is also intended to proof that the logic in the unit test itself is correct.
These around 50 test cases in only three unit tests reduce redundancies in the code. We could reduce still more code redundancies but I think that would have affected the readability of the unit tests.
Update: As suggested by @jfagoagas , @pytest.mark.parametrize is now used for these unit tests.

Checklist

• Are there new checks included in this PR? Yes
  • If so, do we need to update permissions for the provider? Please review this carefully. No
• Review if the code is being covered by tests.
• Review if code is being documented following this specification https://github.com/google/styleguide/blob/gh-pages/pyguide.md#38-comments-and-docstrings
• Review if backport is needed.
• Review if is needed to change the Readme.md

API

• Verify if API specs need to be regenerated.
• Check if version updates are required (e.g., specs, Poetry, etc.).
• Ensure new entries are added to CHANGELOG.md, if applicable.

License

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

[codecov]

Codecov Report

Attention: Patch coverage is 91.66667% with 8 lines in your changes missing coverage. Please review.

Project coverage is 88.76%. Comparing base (cc4b19c) to head (d60c701).
Report is 58 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6985      +/-   ##
==========================================
+ Coverage   88.66%   88.76%   +0.09%     
==========================================
  Files        1200     1208       +8     
  Lines       34647    35120     +473     
==========================================
+ Hits        30721    31175     +454     
- Misses       3926     3945      +19     

Flag	Coverage Δ
prowler	88.76% <91.66%> (+0.09%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
prowler	88.76% <91.66%> (+0.09%)	⬆️
api	∅ <ø> (∅)