From f7972d6e5978b2d17b9aef3e0d2ccb714922fdb5 Mon Sep 17 00:00:00 2001 From: Nikita Pivkin Date: Thu, 26 Sep 2024 11:17:23 +0600 Subject: [PATCH] fix(checks): align AVD-AWS-0107 and AVD-AWS-0105 checks with CIS Benchmarks (#257) Signed-off-by: Nikita Pivkin --- .../aws/ec2/AVD-AWS-0105/CloudFormation.md | 2 +- avd_docs/aws/ec2/AVD-AWS-0105/Terraform.md | 2 +- avd_docs/aws/ec2/AVD-AWS-0105/docs.md | 7 ++- .../aws/ec2/AVD-AWS-0107/CloudFormation.md | 2 +- avd_docs/aws/ec2/AVD-AWS-0107/Terraform.md | 2 +- avd_docs/aws/ec2/AVD-AWS-0107/docs.md | 8 ++- .../cloud/aws/ec2/no_public_ingress_acl.rego | 24 ++++++--- .../aws/ec2/no_public_ingress_acl_test.rego | 35 ++++++++++--- .../cloud/aws/ec2/no_public_ingress_sgr.rego | 19 ++++--- .../aws/ec2/no_public_ingress_sgr_test.rego | 44 ++++++++++++++-- lib/cloud/net.rego | 52 +++++++++++++++++++ pkg/specs/compliance/aws-cis-1.2.yaml | 10 +++- pkg/specs/compliance/aws-cis-1.4.yaml | 6 +++ test/rego/aws_ec2_test.go | 6 +++ 14 files changed, 189 insertions(+), 30 deletions(-) create mode 100644 lib/cloud/net.rego diff --git a/avd_docs/aws/ec2/AVD-AWS-0105/CloudFormation.md b/avd_docs/aws/ec2/AVD-AWS-0105/CloudFormation.md index 456412e0..d7cd8366 100644 --- 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 index 62d3cb4f..bfc09845 100644 --- 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 index b47833ec..587650bd 100644 --- 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 index 6aa99a86..e33601e2 100644 --- 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 index 4f72892d..2ccab774 100644 --- 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 index c96eb465..d66717e5 100644 --- 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 index 154093b0..f5547b64 100644 --- 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 index a6ead488..aeb583f3 100644 --- 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 index 77262245..4919fd9f 100644 --- 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 index 0183f187..944565d8 100644 --- 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 new file mode 100644 index 00000000..38c01aa7 --- /dev/null +++ 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 index 7e488052..3d8d4eeb 100644 --- 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 \ No newline at end of file + 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 \ No newline at end of file diff --git a/pkg/specs/compliance/aws-cis-1.4.yaml b/pkg/specs/compliance/aws-cis-1.4.yaml index a2289fa7..9193a446 100644 --- 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 index 3867ccf8..f8526666 100644 --- 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), }, }, },