fix(checks): align AVD-AWS-0107 and AVD-AWS-0105 checks with CIS Benc…

…hmarks (#257) Signed-off-by: Nikita Pivkin <[email protected]>
aquasecurity · Sep 26, 2024 · f7972d6 · f7972d6
1 parent 6dcabdd
commit f7972d6
Show file tree

Hide file tree

Showing 14 changed files with 189 additions and 30 deletions.
diff --git a/avd_docs/aws/ec2/AVD-AWS-0105/CloudFormation.md b/avd_docs/aws/ec2/AVD-AWS-0105/CloudFormation.md
@@ -1,5 +1,5 @@
 
-Set a more restrictive cidr range
+Set a more restrictive CIDR range
 
 ```yaml---
 AWSTemplateFormatVersion: 2010-09-09

diff --git a/avd_docs/aws/ec2/AVD-AWS-0105/Terraform.md b/avd_docs/aws/ec2/AVD-AWS-0105/Terraform.md
@@ -1,5 +1,5 @@
 
-Set a more restrictive cidr range
+Set a more restrictive CIDR range
 
 ```hcl
  resource "aws_network_acl_rule" "good_example" {

diff --git a/avd_docs/aws/ec2/AVD-AWS-0105/docs.md b/avd_docs/aws/ec2/AVD-AWS-0105/docs.md
@@ -1,5 +1,8 @@
 
-Opening up ACLs to the public internet is potentially dangerous. You should restrict access to IP addresses or ranges that explicitly require it where possible.
+The Network Access Control List (NACL) function provide stateless filtering of ingress and
+egress network traffic to AWS resources. It is recommended that no NACL allows
+unrestricted ingress access to remote server administration ports, such as SSH to port 22
+and RDP to port 3389.
 
 
 ### Impact
@@ -11,4 +14,6 @@ Opening up ACLs to the public internet is potentially dangerous. You should rest
 ### Links
 - https://docs.aws.amazon.com/vpc/latest/userguide/vpc-network-acls.html
 
+- https://docs.aws.amazon.com/securityhub/latest/userguide/ec2-controls.html#ec2-21
+
 
diff --git a/avd_docs/aws/ec2/AVD-AWS-0107/CloudFormation.md b/avd_docs/aws/ec2/AVD-AWS-0107/CloudFormation.md
@@ -1,5 +1,5 @@
 
-Set a more restrictive cidr range
+Set a more restrictive CIDR range
 
 ```yaml---
 Resources:

diff --git a/avd_docs/aws/ec2/AVD-AWS-0107/Terraform.md b/avd_docs/aws/ec2/AVD-AWS-0107/Terraform.md
@@ -1,5 +1,5 @@
 
-Set a more restrictive cidr range
+Set a more restrictive CIDR range
 
 ```hcl
  resource "aws_security_group_rule" "good_example" {

diff --git a/avd_docs/aws/ec2/AVD-AWS-0107/docs.md b/avd_docs/aws/ec2/AVD-AWS-0107/docs.md
@@ -1,5 +1,7 @@
 
-Opening up ports to the public internet is generally to be avoided. You should restrict access to IP addresses or ranges that explicitly require it where possible.
+Security groups provide stateful filtering of ingress and egress network traffic to AWS
+resources. It is recommended that no security group allows unrestricted ingress access to
+remote server administration ports, such as SSH to port 22 and RDP to port 3389.
 
 
 ### Impact
@@ -11,4 +13,8 @@ Opening up ports to the public internet is generally to be avoided. You should r
 ### Links
 - https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/security-group-rules-reference.html
 
+- https://docs.aws.amazon.com/securityhub/latest/userguide/ec2-controls.html#ec2-13
+
+- https://docs.aws.amazon.com/securityhub/latest/userguide/ec2-controls.html#ec2-14
+
 
diff --git a/checks/cloud/aws/ec2/no_public_ingress_acl.rego b/checks/cloud/aws/ec2/no_public_ingress_acl.rego
@@ -1,20 +1,29 @@
 # METADATA
-# title: An ingress Network ACL rule allows specific ports from /0.
+# title: Network ACLs should not allow ingress from 0.0.0.0/0 to port 22 or port 3389.
 # description: |
-#   Opening up ACLs to the public internet is potentially dangerous. You should restrict access to IP addresses or ranges that explicitly require it where possible.
+#   The Network Access Control List (NACL) function provide stateless filtering of ingress and
+#   egress network traffic to AWS resources. It is recommended that no NACL allows
+#   unrestricted ingress access to remote server administration ports, such as SSH to port 22
+#   and RDP to port 3389.
 # scope: package
 # schemas:
 #   - input: schema["cloud"]
 # related_resources:
 #   - https://docs.aws.amazon.com/vpc/latest/userguide/vpc-network-acls.html
+#   - https://docs.aws.amazon.com/securityhub/latest/userguide/ec2-controls.html#ec2-21
 # custom:
 #   id: AVD-AWS-0105
 #   avd_id: AVD-AWS-0105
 #   provider: aws
 #   service: ec2
-#   severity: CRITICAL
+#   severity: MEDIUM
 #   short_code: no-public-ingress-acl
-#   recommended_action: Set a more restrictive cidr range
+#   recommended_action: Set a more restrictive CIDR range
+#   frameworks:
+#     default:
+#       - null
+#     cis-aws-1.4:
+#       - "5.1"
 #   input:
 #     selector:
 #       - type: cloud
@@ -33,14 +42,17 @@ package builtin.aws.ec2.aws0105
 
 import rego.v1
 
+import data.lib.net
+
 deny contains res if {
 	some acl in input.aws.ec2.networkacls
 	some rule in acl.rules
 	is_ingress(rule)
 	is_allow(rule)
+	net.is_tcp_protocol(rule.protocol.value)
+	net.is_ssh_or_rdp_port(rule)
 	some block in rule.cidrs
-	cidr.is_public(block.value)
-	cidr.count_addresses(block.value) > 1
+	net.cidr_allows_all_ips(block.value)
 	res := result.new(
 		"Network ACL rule allows ingress from public internet.",
 		block,

diff --git a/checks/cloud/aws/ec2/no_public_ingress_acl_test.rego b/checks/cloud/aws/ec2/no_public_ingress_acl_test.rego
@@ -5,22 +5,45 @@ import rego.v1
 import data.builtin.aws.ec2.aws0105 as check
 import data.lib.test
 
-test_deny_acl_rule_with_wildcard_address if {
-	inp := {"aws": {"ec2": {"networkacls": [{"rules": [{
+import data.lib.net
+
+test_deny_acl_rule_all_ips_for_ssh_port_and_tcp if {
+	inp := build_input({
+		"protocol": {"value": "tcp"},
 		"type": {"value": "ingress"},
 		"action": {"value": "allow"},
 		"cidrs": [{"value": "0.0.0.0/0"}],
-	}]}]}}}
+		"fromport": {"value": net.ssh_port},
+		"toport": {"value": net.ssh_port},
+	})
 
 	test.assert_count(check.deny, 1) with input as inp
 }
 
-test_allow_acl_rule_with_specific_address if {
-	inp := {"aws": {"ec2": {"networkacls": [{"rules": [{
+test_deny_ingress_sq_all_ips_for_all_ports_and_all_ports if {
+	inp := build_input({
+		"protocol": {"value": "-1"},
+		"type": {"value": "ingress"},
+		"action": {"value": "allow"},
+		"cidrs": [{"value": "0.0.0.0/0"}],
+		"fromport": {"value": 0},
+		"toport": {"value": 0},
+	})
+
+	test.assert_count(check.deny, 1) with input as inp
+}
+
+test_allow_acl_rule_restrictive_cidr_range if {
+	inp := build_input({
+		"protocol": {"value": "tcp"},
 		"type": {"value": "ingress"},
 		"action": {"value": "allow"},
 		"cidrs": [{"value": "10.0.0.0/16"}],
-	}]}]}}}
+		"fromport": {"value": net.ssh_port},
+		"toport": {"value": net.ssh_port},
+	})
 
 	test.assert_empty(check.deny) with input as inp
 }
+
+build_input(rule) := {"aws": {"ec2": {"networkacls": [{"rules": [rule]}]}}}
diff --git a/checks/cloud/aws/ec2/no_public_ingress_sgr.rego b/checks/cloud/aws/ec2/no_public_ingress_sgr.rego
@@ -1,20 +1,24 @@
 # METADATA
-# title: An ingress security group rule allows traffic from /0.
+# title: Security groups should not allow ingress from 0.0.0.0/0 or ::/0 to port 22 or port 3389.
 # description: |
-#   Opening up ports to the public internet is generally to be avoided. You should restrict access to IP addresses or ranges that explicitly require it where possible.
+#   Security groups provide stateful filtering of ingress and egress network traffic to AWS
+#   resources. It is recommended that no security group allows unrestricted ingress access to
+#   remote server administration ports, such as SSH to port 22 and RDP to port 3389.
 # scope: package
 # schemas:
 #   - input: schema["cloud"]
 # related_resources:
 #   - https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/security-group-rules-reference.html
+#   - https://docs.aws.amazon.com/securityhub/latest/userguide/ec2-controls.html#ec2-13
+#   - https://docs.aws.amazon.com/securityhub/latest/userguide/ec2-controls.html#ec2-14
 # custom:
 #   id: AVD-AWS-0107
 #   avd_id: AVD-AWS-0107
 #   provider: aws
 #   service: ec2
-#   severity: CRITICAL
+#   severity: HIGH
 #   short_code: no-public-ingress-sgr
-#   recommended_action: Set a more restrictive cidr range
+#   recommended_action: Set a more restrictive CIDR range
 #   frameworks:
 #     default:
 #       - null
@@ -39,12 +43,15 @@ package builtin.aws.ec2.aws0107
 
 import rego.v1
 
+import data.lib.net
+
 deny contains res if {
 	some group in input.aws.ec2.securitygroups
 	some rule in group.ingressrules
+	net.is_tcp_or_udp_protocol(rule.protocol.value)
+	net.is_ssh_or_rdp_port(rule)
 	some block in rule.cidrs
-	cidr.is_public(block.value)
-	cidr.count_addresses(block.value) > 1
+	net.cidr_allows_all_ips(block.value)
 	res := result.new(
 		"Security group rule allows ingress from public internet.",
 		block,

diff --git a/checks/cloud/aws/ec2/no_public_ingress_sgr_test.rego b/checks/cloud/aws/ec2/no_public_ingress_sgr_test.rego
@@ -5,14 +5,50 @@ import rego.v1
 import data.builtin.aws.ec2.aws0107 as check
 import data.lib.test
 
-test_deny_ingress_sq_with_wildcard_address if {
-	inp := {"aws": {"ec2": {"securitygroups": [{"ingressrules": [{"cidrs": [{"value": "0.0.0.0/0"}]}]}]}}}
+import data.lib.net
+
+test_deny_ingress_sq_all_ips_for_ssh_port_and_tcp if {
+	inp := build_input({
+		"protocol": {"value": "tcp"},
+		"cidrs": [{"value": "0.0.0.0/0"}],
+		"fromport": {"value": net.ssh_port},
+		"toport": {"value": net.ssh_port},
+	})
+
+	test.assert_count(check.deny, 1) with input as inp
+}
+
+test_deny_ingress_sq_all_ips_for_rdp_port_and_udp if {
+	inp := build_input({
+		"protocol": {"value": "udp"},
+		"cidrs": [{"value": "0.0.0.0/0"}],
+		"fromport": {"value": net.rdp_port},
+		"toport": {"value": net.rdp_port},
+	})
 
 	test.assert_count(check.deny, 1) with input as inp
 }
 
-test_allow_ingress_sg_with_private_address if {
-	inp := {"aws": {"ec2": {"securitygroups": [{"ingressrules": [{"cidrs": [{"value": "10.0.0.0/16"}]}]}]}}}
+test_deny_ingress_sq_all_ips_for_all_ports_and_all_protocols if {
+	inp := build_input({
+		"protocol": {"value": "-1"},
+		"cidrs": [{"value": "0.0.0.0/0"}],
+		"fromport": {"value": 0},
+		"toport": {"value": 0},
+	})
+
+	test.assert_count(check.deny, 1) with input as inp
+}
+
+test_allow_ingress_sg_restrictive_cidr_range if {
+	inp := build_input({
+		"protocol": {"value": "tcp"},
+		"cidrs": [{"value": "10.0.0.0/16"}],
+		"fromport": {"value": net.ssh_port},
+		"toport": {"value": net.ssh_port},
+	})
 
 	test.assert_empty(check.deny) with input as inp
 }
+
+build_input(rule) := {"aws": {"ec2": {"securitygroups": [{"ingressrules": [rule]}]}}}
diff --git a/lib/cloud/net.rego b/lib/cloud/net.rego
@@ -0,0 +1,52 @@
+# METADATA
+# custom:
+#   library: true
+#   input:
+#     selector:
+#     - type: cloud
+package lib.net
+
+import rego.v1
+
+ssh_port := 22
+
+rdp_port := 3389
+
+all_ips := {"0.0.0.0/0", "0000:0000:0000:0000:0000:0000:0000:0000/0", "::/0"}
+
+# "-1" or "all" equivalent to all protocols
+# https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_AuthorizeSecurityGroupIngress.html
+# https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/compute_firewall#protocol
+all_protocols := {"-1", "all"}
+
+# "6" is ID of TCP
+# https://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml
+is_tcp_protocol(protocol) if protocol in {"tcp", "6"}
+
+is_tcp_protocol(protocol) if protocol in all_protocols
+
+# "17" is ID of UDP
+# https://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml
+is_udp_protocol(protocol) if protocol in {"udp", "17"}
+
+is_udp_protocol(protocol) if protocol in all_protocols
+
+is_tcp_or_udp_protocol(protocol) if is_tcp_protocol(protocol)
+
+is_tcp_or_udp_protocol(protocol) if is_udp_protocol(protocol)
+
+# protocol "-1" allows traffic on all ports, regardless of any port range you specify.
+# https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_AuthorizeSecurityGroupIngress.html
+is_ssh_or_rdp_port(rule) if rule.protocol.value in {"-1"}
+
+is_ssh_or_rdp_port(rule) if is_port_range_include(rule.fromport.value, rule.toport.value, ssh_port)
+
+is_ssh_or_rdp_port(rule) if is_port_range_include(rule.fromport.value, rule.toport.value, rdp_port)
+
+is_port_range_include(from, to, port) if {
+	from <= port
+	port <= to
+}
+
+# check if CIDR defines an IP block containing all possible IP addresses
+cidr_allows_all_ips(cidr) if cidr in all_ips
diff --git a/pkg/specs/compliance/aws-cis-1.2.yaml b/pkg/specs/compliance/aws-cis-1.2.yaml
@@ -206,7 +206,13 @@ spec:
     severity: LOW
   - id: "4.1"
     name: no-public-ingress-sgr
-    description: An ingress security group rule allows traffic from /0.
+    description: Security groups should not allow ingress from 0.0.0.0/0 or ::/0 to port 22 or port 3389.
     checks:
     - id: AVD-AWS-0107
-    severity: CRITICAL
+    severity: HIGH
+  - id: "4.2"
+    name: no-public-ingress-sgr
+    description: Security groups should not allow ingress from 0.0.0.0/0 or ::/0 to port 22 or port 3389.
+    checks:
+    - id: AVD-AWS-0107
+    severity: HIGH
diff --git a/pkg/specs/compliance/aws-cis-1.4.yaml b/pkg/specs/compliance/aws-cis-1.4.yaml
@@ -233,6 +233,12 @@ spec:
     checks:
     - id: AVD-AWS-0155
     severity: LOW
+  - id: "5.1"
+    name: aws-vpc-no-public-ingress-acl
+    description: Network ACLs should not allow ingress from 0.0.0.0/0 to port 22 or port 3389.
+    checks:
+    - id: AVD-AWS-0105
+    severity: MEDIUM
   - id: "5.3"
     name: restrict-all-in-default-sg
     description: Default security group should restrict all traffic

diff --git a/test/rego/aws_ec2_test.go b/test/rego/aws_ec2_test.go
@@ -498,6 +498,9 @@ var awsEc2TestCases = testCases{
 								Metadata: trivyTypes.NewTestMetadata(),
 								Type:     trivyTypes.String(ec2.TypeIngress, trivyTypes.NewTestMetadata()),
 								Action:   trivyTypes.String(ec2.ActionAllow, trivyTypes.NewTestMetadata()),
+								Protocol: trivyTypes.StringTest("tcp"),
+								FromPort: trivyTypes.IntTest(22),
+								ToPort:   trivyTypes.IntTest(22),
 								CIDRs: []trivyTypes.StringValue{
 									trivyTypes.String("0.0.0.0/0", trivyTypes.NewTestMetadata()),
 								},
@@ -543,6 +546,9 @@ var awsEc2TestCases = testCases{
 								CIDRs: []trivyTypes.StringValue{
 									trivyTypes.String("0.0.0.0/0", trivyTypes.NewTestMetadata()),
 								},
+								Protocol: trivyTypes.StringTest("tcp"),
+								FromPort: trivyTypes.IntTest(22),
+								ToPort:   trivyTypes.IntTest(22),
 							},
 						},
 					},