S3 Lockout check improvements based on PR feedback
MODIFY
checkov/terraform/checks/resource/aws/S3ProtectAgainstPolicyLockout.py
checkov/terraform/checks/resource/aws/S3ProtectAgainstPolicyLockout.py
from checkov.common.models.enums import CheckResult, CheckCategories
from checkov.terraform.checks.resource.base_resource_check import BaseResourceCheck
+from checkov.common.util.type_forcers import force_list
import json
@@ -7,7 +8,7 @@ class S3ProtectAgainstPolicyLockout(BaseResourceCheck):
def __init__(self):
name = "Ensure S3 bucket policy does not lockout all but root user. (Prevent lockouts needing root account fixes)"
- id = "CKV_AWS_89"
+ id = "CKV_AWS_93"
supported_resources = ['aws_s3_bucket', 'aws_s3_bucket_policy']
categories = [CheckCategories.IAM]
super().__init__(name=name, id=id, categories=categories, supported_resources=supported_resources)
@@ -18,7 +19,18 @@ class S3ProtectAgainstPolicyLockout(BaseResourceCheck):
try:
policy_block = json.loads(conf['policy'][0])
if 'Statement' in policy_block.keys():
- for statement in policy_block['Statement']:
+ for statement in force_list(policy_block['Statement']):
+ if 'Condition' in statement.keys():
+ # https://github.com/bridgecrewio/checkov/pull/627#issuecomment-714681751
+ continue
+
+ if 'NotAction' in statement.keys():
+ continue
+
+ if 'Effect' not in statement.keys():
+ # Defaults to allow if doesn't exist.
+ continue
+
if statement['Effect'] == 'Allow':
continue

MODIFY
tests/terraform/checks/resource/aws/test_S3ProtectAgainstPolicyLockout.py
tests/terraform/checks/resource/aws/test_S3ProtectAgainstPolicyLockout.py
@@ -90,6 +90,65 @@ class TestS3ProtectAgainstPolicyLockout(unittest.TestCase):
scan_result = check.scan_resource_conf(conf=resource_conf)
self.assertEqual(CheckResult.FAILED, scan_result)
+ def test_skip_noeffect(self):
+ hcl_res = hcl2.loads("""
+ resource "aws_s3_bucket_policy" "s3" {
+ bucket = "bucket"
+
+ policy = <<POLICY
+ {
+ "Id": "Policy1597273448050",
+ "Version": "2012-10-17",
+ "Statement": [
+ {
+ "Sid": "Stmt1597273446725",
+ "Action": [
+ "s3:GetObject"
+ ],
+ "Resource": "arn:aws:s3:::bucket/*",
+ "Principal": {
+ "AWS": "some_arn"
+ }
+ }
+ ]
+ }
+ POLICY
+ }
+ """)
+ resource_conf = hcl_res['resource'][0]['aws_s3_bucket_policy']['s3']
+ scan_result = check.scan_resource_conf(conf=resource_conf)
+ self.assertEqual(CheckResult.PASSED, scan_result)
+
+ def test_skip_notaction(self):
+ hcl_res = hcl2.loads("""
+ resource "aws_s3_bucket_policy" "s3" {
+ bucket = "bucket"
+
+ policy = <<POLICY
+ {
+ "Id": "Policy1597273448050",
+ "Version": "2012-10-17",
+ "Statement": [
+ {
+ "Sid": "Stmt1597273446725",
+ "NotAction": [
+ "s3:GetObject"
+ ],
+ "Effect": "Deny",
+ "Resource": "arn:aws:s3:::bucket/*",
+ "Principal": {
+ "AWS": "some_arn"
+ }
+ }
+ ]
+ }
+ POLICY
+ }
+ """)
+ resource_conf = hcl_res['resource'][0]['aws_s3_bucket_policy']['s3']
+ scan_result = check.scan_resource_conf(conf=resource_conf)
+ self.assertEqual(CheckResult.PASSED, scan_result)
+
def test_success_policyobj(self):
hcl_res = hcl2.loads("""
resource "aws_s3_bucket_policy" "s3" {
@@ -120,6 +179,33 @@ class TestS3ProtectAgainstPolicyLockout(unittest.TestCase):
scan_result = check.scan_resource_conf(conf=resource_conf)
self.assertEqual(CheckResult.PASSED, scan_result)
+ def test_success_statementnotlist(self):
+ hcl_res = hcl2.loads("""
+ resource "aws_s3_bucket_policy" "s3" {
+ bucket = "bucket"
+
+ policy = <<POLICY
+ {
+ "Id": "Policy1597273448050",
+ "Version": "2012-10-17",
+ "Statement": {
+ "Sid": "Stmt1597273446725",
+ "Action": [
+ "s3:GetObject"
+ ],
+ "Effect": "Deny",
+ "Resource": "arn:aws:s3:::bucket/*",
+ "Principal": {
+ "AWS": "some_arn"
+ }
+ }
+ }
+ POLICY
+ }
+ """)
+ resource_conf = hcl_res['resource'][0]['aws_s3_bucket_policy']['s3']
+ scan_result = check.scan_resource_conf(conf=resource_conf)
+ self.assertEqual(CheckResult.PASSED, scan_result)
if __name__ == '__main__':
unittest.main()

S3 Lockout check improvements based on PR feedback