Merge pull request #117 from Skyscanner/do_docs_do

Update content of rules documentation

cfripper/rules/ec2_security_group.py

@@ -89,8 +89,8 @@ class EC2SecurityGroupOpenToWorldRule(SecurityGroupOpenToWorldRule):
     All other ports should be closed off from public access to prevent a serious security misconfiguration.
 
     Fix:
-        Most security groups only need to be accessed privately, and this can typically be done by specifying
-        the CIDR of a Security Group's ingress to `10.0.0.0/8` or similar (https://en.wikipedia.org/wiki/Private_network).
+        Most security groups only need to be [accessed privately](https://en.wikipedia.org/wiki/Private_network), and
+        this can typically be done by specifying the CIDR of a Security Group's ingress to `10.0.0.0/8` or similar.
 
         Unless required, do not use the following IP ranges in your Security Group Ingress:
 
@@ -100,7 +100,7 @@ class EC2SecurityGroupOpenToWorldRule(SecurityGroupOpenToWorldRule):
 
           - `172/8` or `192/8` (use `172.16/12` and `192.168/16` ranges, per RFC1918 specification).
 
-        As per RFC4193, fd00::/8 IPv6 addresses should be used to define a private network.
+        As per RFC4193, `fd00::/8` IPv6 addresses should be used to define a private network.
 
     Code for fix:
         ````json
@@ -168,8 +168,8 @@ class EC2SecurityGroupIngressOpenToWorldRule(SecurityGroupOpenToWorldRule):
     All other ports should be closed off from public access to prevent a serious security misconfiguration.
 
     Fix:
-        Most security groups only need to be accessed privately, and this can typically be done by specifying
-        the CIDR of a Security Group's ingress to `10.0.0.0/8` or similar (https://en.wikipedia.org/wiki/Private_network).
+        Most security groups only need to be [accessed privately](https://en.wikipedia.org/wiki/Private_network), and
+        this can typically be done by specifying the CIDR of a Security Group's ingress to `10.0.0.0/8` or similar.
 
         Unless required, do not use the following IP ranges in your Security Group Ingress:
 
@@ -179,7 +179,7 @@ class EC2SecurityGroupIngressOpenToWorldRule(SecurityGroupOpenToWorldRule):
 
           - `172/8` or `192/8` (use `172.16/12` and `192.168/16` ranges, per RFC1918 specification).
 
-        As per RFC4193, fd00::/8 IPv6 addresses should be used to define a private network.
+        As per RFC4193, `fd00::/8` IPv6 addresses should be used to define a private network.
 
     Code for fix:
         ````json
@@ -261,7 +261,7 @@ class EC2SecurityGroupMissingEgressRule(Rule):
         some services may need this, it is usually the case that the security group can be locked down
         more. A NAT instance for example may require a completely open egress policy.
 
-        Allowing unrestricted (0.0.0.0/0 or ::/0) outbound/egress access can increase opportunities for
+        Allowing unrestricted (`0.0.0.0/0` or `::/0`) outbound/egress access can increase opportunities for
         malicious activity such as such as Denial of Service (DoS) attacks or Distributed Denial of Service (DDoS)
         attacks.
 

cfripper/rules/iam_roles.py

@@ -61,9 +61,8 @@ def check_inline_policies(self, result: Result, logical_id: str, role: IAMRole):
 
 class IAMRoleWildcardActionOnPolicyRule(Rule):
     """
-    Checks for use of wildcard characters in all IAM Role policies (including AssumeRolePolicyDocument)
-    and AWS Managed Policies
-    (https://docs.aws.amazon.com/IAM/latest/UserGuide/access_policies_managed-vs-inline.html).
+    Checks for use of wildcard characters in all IAM Role policies (including `AssumeRolePolicyDocument`)
+    and [AWS Managed Policies](https://docs.aws.amazon.com/IAM/latest/UserGuide/access_policies_managed-vs-inline.html).
     """
 
     GRANULARITY = RuleGranularity.RESOURCE

cfripper/rules/managed_policy_on_user.py

@@ -13,6 +13,53 @@
 class ManagedPolicyOnUserRule(Rule):
     """
     Checks if any IAM managed policy is applied to a group and not a user.
+
+    Risk:
+        Instead of defining permissions for individual IAM users, it's usually more convenient and secure
+        to create [IAM groups](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_groups.html) that relate
+        to different functions. IAM users can be assigned to these groups.
+        All the users in an IAM group inherit the permissions assigned to the group. That way, you can make
+        changes for everyone in a group in just one place. As people move around in your company, you can
+        simply change what IAM group their IAM user belongs to, without risking a user having too much
+        privilege.
+
+    Fix:
+        Use IAM Groups as opposed to users in IAM Managed Policies.
+
+    Code for fix:
+        This is an example which will be flagged by CFRipper:
+
+        ```json
+        "BadPolicy": {
+          "Type": "AWS::IAM::ManagedPolicy",
+          "Properties": {
+            "Description": "Policy for something.",
+            "Path": "/",
+            "PolicyDocument": {
+              "Version": "2012-10-17",
+              "Statement": [...]
+            },
+            "Users": [{"Ref": "TestUser"}]
+          }
+        }
+        ```
+
+        This is an example of a more acceptable CloudFormation policy:
+
+        ```json
+        "GoodPolicy": {
+          "Type": "AWS::IAM::ManagedPolicy",
+          "Properties": {
+            "Description": "Policy for something.",
+            "Path": "/",
+            "PolicyDocument": {
+              "Version": "2012-10-17",
+              "Statement": [...]
+            },
+            "Groups": ["user_group"]
+          }
+        }
+        ```
     """
 
     REASON = "IAM managed policy {} should not apply directly to users. Should be on group"

cfripper/rules/policy_on_user.py

@@ -13,6 +13,53 @@
 class PolicyOnUserRule(Rule):
     """
     Checks if any IAM policy is applied to a group and not a user.
+
+    Risk:
+        Instead of defining permissions for individual IAM users, it's usually more convenient and secure
+        to create [IAM groups](https://docs.aws.amazon.com/IAM/latest/UserGuide/id_groups.html) that relate
+        to different functions. IAM users can be assigned to these groups.
+        All the users in an IAM group inherit the permissions assigned to the group. That way, you can make
+        changes for everyone in a group in just one place. As people move around in your company, you can
+        simply change what IAM group their IAM user belongs to, without risking a user having too much
+        privilege.
+
+    Fix:
+        Use IAM Groups as opposed to users in IAM Policies.
+
+    Code for fix:
+        This is an example which will be flagged by CFRipper:
+
+        ```json
+        "BadPolicy": {
+          "Type": "AWS::IAM::Policy",
+          "Properties": {
+            "Description": "Policy for something.",
+            "Path": "/",
+            "PolicyDocument": {
+              "Version": "2012-10-17",
+              "Statement": [...]
+            },
+            "Users": [{"Ref": "TestUser"}]
+          }
+        }
+        ```
+
+        This is an example of a more acceptable CloudFormation policy:
+
+        ```json
+        "GoodPolicy": {
+          "Type": "AWS::IAM::Policy",
+          "Properties": {
+            "Description": "Policy for something.",
+            "Path": "/",
+            "PolicyDocument": {
+              "Version": "2012-10-17",
+              "Statement": [...]
+            },
+            "Groups": ["user_group"]
+          }
+        }
+        ```
     """
 
     GRANULARITY = RuleGranularity.RESOURCE

cfripper/rules/s3_public_access.py

@@ -46,6 +46,13 @@ def invoke(self, cfmodel: CFModel, extras: Optional[Dict] = None) -> Result:
 class S3BucketPublicReadWriteAclRule(Rule):
     """
     Checks if any S3 bucket policy has access control set to `PublicReadWrite`.
+
+    Risk:
+        Unless required, S3 buckets should not have Public Write available on a bucket. This allows anyone
+        to write any objects to your S3 bucket.
+
+    Fix:
+        Remove any configuration that looks like `"AccessControl": "PublicReadWrite"` from your S3 bucket policy.
     """
 
     GRANULARITY = RuleGranularity.RESOURCE

cfripper/rules/sns_topic_policy_not_principal.py

@@ -12,7 +12,12 @@
 
 class SNSTopicPolicyNotPrincipalRule(Rule):
     """
-    Checks if an SNS topic policy has an Allow + a NotPrincipal (exclusive principal).
+    Checks if an SNS topic policy has an Allow + a NotPrincipal.
+
+    Risk:
+        AWS **strongly** recommends against using `NotPrincipal` in the same policy statement as `"Effect": "Allow"`.
+        Doing so grants the permissions specified in the policy statement to all principals except the one named
+        in the `NotPrincipal` element. By doing this, you might grant access to anonymous (unauthenticated) users.
     """
 
     GRANULARITY = RuleGranularity.RESOURCE

cfripper/rules/sqs_queue_policy.py

@@ -16,7 +16,12 @@
 
 class SQSQueuePolicyNotPrincipalRule(Rule):
     """
-    Checks if an SQS Queue policy has an Allow + a NotPrincipal (exclusive principal).
+    Checks if an SQS Queue policy has an Allow + a NotPrincipal.
+
+    Risk:
+        AWS **strongly** recommends against using `NotPrincipal` in the same policy statement as `"Effect": "Allow"`.
+        Doing so grants the permissions specified in the policy statement to all principals except the one named
+        in the `NotPrincipal` element. By doing this, you might grant access to anonymous (unauthenticated) users.
     """
 
     GRANULARITY = RuleGranularity.RESOURCE

docs/macros.py

@@ -15,7 +15,7 @@
 def define_env(env):
     @env.macro
     def cfripper_rules():
-        severity_map = {RuleRisk.HIGH: "High", RuleRisk.MEDIUM: "Medium", RuleRisk.LOW: "Low"}
+        severity_map = {RuleRisk.HIGH: "**High**", RuleRisk.MEDIUM: "Medium", RuleRisk.LOW: "Low"}
         rules_inspection = inspect.getmembers(rules, inspect.isclass)
         results = []
         for _, klass in rules_inspection:
@@ -28,12 +28,11 @@ def cfripper_rules():
                     # Remove ABCMeta default docstring
                     if not paragraph_text.startswith("Helper class that"):
                         content += paragraph_text
-                    content += f"\n\nSeverity: {severity_map[klass.RISK_VALUE]}\n"
+                    content += f"\n\n>Severity: {severity_map[klass.RISK_VALUE]}\n"
                     if klass.RULE_MODE == RuleMode.MONITOR:
-                        content += "\nDefaults to monitor mode (rule not enforced)\n"
+                        content += "\n>Defaults to monitor mode (rule not enforced)\n"
                     if klass.RULE_MODE == RuleMode.DEBUG:
-                        content += "\nDefaults to debug mode (rule not enforced)\n"
-
+                        content += "\n>Defaults to debug mode (rule not enforced)\n"
                 else:
                     content += f"\n#### {paragraph_title}\n"
                     content += f"{paragraph_text}\n"

docs/rules.md

@@ -10,7 +10,7 @@ When running CFRipper the CloudFormation stack will be checked against each rule
 {% if rule.1 -%}
 {{ rule.1 }}
 {% endif -%}
-
+---
 {% endfor %}
 
 ## Custom Rules