Iam sid errors

src/cfnlint/rules/resources/iam/Sid.py

@@ -0,0 +1,95 @@
+"""
+Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+SPDX-License-Identifier: MIT-0
+"""
+import re
+from cfnlint.rules import CloudFormationLintRule
+from cfnlint.rules import RuleMatch
+
+
+class Sid(CloudFormationLintRule):
+    """Check if IAM Policy Sid is correct"""
+    id = 'W2512'
+    shortdesc = 'Check IAM Resource Policies Sid syntax'
+    description = 'See if the elements inside an IAM Resource policy are configured correctly.'
+    source_url = 'https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_grammar.html'
+    tags = ['properties', 'iam']
+
+    def __init__(self):
+        """Init"""
+        super(Sid, self).__init__()
+        self.resources_and_keys = {
+            'AWS::ECR::Repository': 'RepositoryPolicyText',
+            'AWS::Elasticsearch::Domain': 'AccessPolicies',
+            'AWS::KMS::Key': 'KeyPolicy',
+            'AWS::S3::BucketPolicy': 'PolicyDocument',
+            'AWS::SNS::TopicPolicy': 'PolicyDocument',
+            'AWS::SQS::QueuePolicy': 'PolicyDocument',
+        }
+        self.idp_and_keys = {
+            'AWS::IAM::Group': 'Policies',
+            'AWS::IAM::ManagedPolicy': 'PolicyDocument',
+            'AWS::IAM::Policy': 'PolicyDocument',
+            'AWS::IAM::Role': 'Policies',
+            'AWS::IAM::User': 'Policies',
+        }
+        for resource_type in self.resources_and_keys:
+            self.resource_property_types.append(resource_type)
+        for resource_type in self.idp_and_keys:
+            self.resource_property_types.append(resource_type)
+        self.sid_regex = re.compile('^[A-Za-z0-9]*$')
+
+    def check_policy_document(self, value, path):
+        """Check policy document"""
+        sids = [] # sids must be unique
+        matches = []
+
+        if not isinstance(value, dict):
+            return matches
+
+        for e_v, e_p in value.items_safe(path[:]):
+            for p_vs, p_p in e_v.items_safe(e_p[:]):
+                statements = p_vs.get('Statement', [])
+                if 'get' in dir(statements):
+                    statements = [statements]
+                for statement in statements:
+                    if 'Sid' in statement:
+                        sid = statement.get('Sid')
+                        if self.sid_regex.search(sid) is None:
+                            message = 'basic alphanumeric characters (A-Z,a-z,0-9) are the only allowed characters in the Sid value'
+                            matches.append(RuleMatch(p_p + ['Sid'], message))
+                        if sid in sids:
+                            message = 'IAM Sid values should be unique'
+                            matches.append(RuleMatch(p_p + ['Sid'], message))
+                        else:
+                            sids.append(sid)
+
+        return matches
+
+    def match_resource_properties(self, properties, resourcetype, path, cfn):
+        """Check CloudFormation Properties"""
+        matches = []
+
+        key = None
+        if resourcetype in self.resources_and_keys:
+            key = self.resources_and_keys.get(resourcetype)
+        else:
+            key = self.idp_and_keys.get(resourcetype)
+
+        if key == 'Policies':
+            for index, policy in enumerate(properties.get(key, [])):
+                matches.extend(
+                    cfn.check_value(
+                        obj=policy, key='PolicyDocument',
+                        path=path[:] + ['Policies', index],
+                        check_value=self.check_policy_document,
+                    ))
+        else:
+            matches.extend(
+                cfn.check_value(
+                    obj=properties, key=key,
+                    path=path[:],
+                    check_value=self.check_policy_document
+                ))
+
+        return matches

test/fixtures/templates/bad/resources/iam/policy_sid.yaml

@@ -0,0 +1,29 @@
+---
+AWSTemplateFormatVersion: "2010-09-09"
+Description: >
+  Test bad Policy Version
+Resources:
+  rIamRole:
+    Type: AWS::IAM::Role
+    Properties:
+      AssumeRolePolicyDocument: {}
+      Policies:
+      - PolicyName: String
+        PolicyDocument:
+          Version: "2012-10-17"
+          Statement:
+            - Sid: "invalid regex"
+              Effect: "Allow"
+              Action:
+                - "s3:ListBucket"
+              Resource: '*'
+            - Effect: "Allow"
+              Sid: "Sid1"
+              Action:
+                - "s3:GetBucketLocation"
+              Resource: '*'
+            - Effect: "Allow"
+              Sid: "Sid1"
+              Action:
+                - "s3:GetObject"
+              Resource: '*'

test/fixtures/templates/bad/resources/iam/policy_sid_2.yaml

@@ -0,0 +1,19 @@
+---
+AWSTemplateFormatVersion: "2010-09-09"
+Description: >
+  Test bad Policy Version
+Resources:
+  rIamRole:
+    Type: AWS::IAM::Role
+    Properties:
+      AssumeRolePolicyDocument: {}
+      Policies:
+      - PolicyName: String
+        PolicyDocument:
+          Version: "2012-10-17"
+          Statement:
+              Sid: "invalid regex"
+              Effect: "Allow"
+              Action:
+                - "s3:ListBucket"
+              Resource: '*'

test/fixtures/templates/good/resources/iam/policy.yaml

@@ -20,6 +20,11 @@ Resources:
               - "s3:ListBucket"
               - "s3:GetObject"
             Resource: '*'
+          - Effect: "Allow"
+            Sid: "FindMyBucket"
+            Action:
+              - "s3:GetBucketLocation"
+            Resource: '*'
           - Fn::If:
             - SomeCondition
             - Effect: "Allow"

test/fixtures/templates/quickstart/nist_logging.yaml

@@ -81,7 +81,7 @@ Resources:
               - ':s3:::'
               - Ref: rArchiveLogsBucket
               - /*
-          Sid: Enforce HTTPS Connections
+          Sid: EnforceHTTPSConnections
         - Action: s3:Delete*
           Effect: Deny
           Principal: '*'
@@ -96,7 +96,7 @@ Resources:
               - ':s3:::'
               - Ref: rArchiveLogsBucket
               - /*
-          Sid: Restrict Delete* Actions
+          Sid: RestrictDeleteActions
         - Action: s3:PutObject
           Condition:
             StringNotEquals:
@@ -286,7 +286,7 @@ Resources:
               - ':s3:::'
               - Ref: rCloudTrailBucket
               - /*
-          Sid: Enforce HTTPS Connections
+          Sid: EnforceHTTPSConnections
         - Action: s3:Delete*
           Effect: Deny
           Principal: '*'
@@ -301,7 +301,7 @@ Resources:
               - ':s3:::'
               - Ref: rCloudTrailBucket
               - /*
-          Sid: Restrict Delete* Actions
+          Sid: RestrictDeleteActions
         - Action: s3:PutObject
           Condition:
             StringNotEquals:

test/unit/module/config/test_config_mixin.py

@@ -158,4 +158,4 @@ def test_config_expand_ignore_templates(self, yaml_mock):
         # test defaults
         self.assertNotIn(
             'test/fixtures/templates/bad/resources/iam/resource_policy.yaml', config.templates)
-        self.assertEqual(len(config.templates), 4)
+        self.assertEqual(len(config.templates), 6)

test/unit/rules/resources/iam/test_iam_sid.py

@@ -0,0 +1,30 @@
+"""
+Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+SPDX-License-Identifier: MIT-0
+"""
+from test.unit.rules import BaseRuleTestCase
+from cfnlint.rules.resources.iam.Sid import Sid  # pylint: disable=E0401
+
+
+class TestSid(BaseRuleTestCase):
+    """Test IAM Resource Policies"""
+
+    def setUp(self):
+        """Setup"""
+        super(TestSid, self).setUp()
+        self.collection.register(Sid())
+        self.success_templates = [
+            #'test/fixtures/templates/good/resources/iam/resource_policy.yaml',
+            'test/fixtures/templates/good/resources/iam/policy.yaml',
+        ]
+
+    def test_file_positive(self):
+        """Test Positive"""
+        self.helper_file_positive()
+
+    def test_file_negative(self):
+        """Test failure"""
+        self.helper_file_negative(
+            'test/fixtures/templates/bad/resources/iam/policy_sid.yaml', 2)
+        self.helper_file_negative(
+            'test/fixtures/templates/bad/resources/iam/policy_sid_2.yaml', 1)