Repository: tfsec/tfsec
Pull request #491 (2020-10-30 16:38:44)

Ingress acl rules
Have created 2 rules
049: Check ingress rules that have specific ports and allowed from 0.0.0.0/0
050: Check ingress rules that allow ALL ports and allowed from 0.0.0.0/0

I think for the most part it's likely 049 is more often "OK" whereas 050 should almost never be ignored, that's my logic behind the separation.


Patch:
diff --git a/internal/app/tfsec/checks/aws049.go b/internal/app/tfsec/checks/aws049.go
new file mode 100644
index 000000000..d4981f12d
--- /dev/null
+++ b/internal/app/tfsec/checks/aws049.go
@@ -0,0 +1,110 @@
+package checks
+
+import (
+	"fmt"
+	"github.com/tfsec/tfsec/internal/app/tfsec/parser"
+	"github.com/tfsec/tfsec/internal/app/tfsec/scanner"
+	"github.com/zclconf/go-cty/cty"
+)
+
+const AWSOpenIngressNetworkACLRule scanner.RuleCode = "AWS049"
+const AWSOpenIngressNetworkACLRuleDescription scanner.RuleSummary = "An ingress Network ACL rule allows specific ports from `/0`."
+const AWSOpenIngressNetworkACLRuleExplanation = `
+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.
+
+`
+const AWSOpenIngressNetworkACLRuleBadExample = `
+resource "aws_network_acl_rule" "my-rule" {
+  egress         = false
+  protocol       = "tcp"
+  from_port      = 22
+  to_port        = 22
+  rule_action    = "allow"
+  cidr_block     = "0.0.0.0/0"
+}
+`
+const AWSOpenIngressNetworkACLRuleGoodExample = `
+resource "aws_network_acl_rule" "my-rule" {
+  egress         = false
+  protocol       = "tcp"
+  from_port      = 22
+  to_port        = 22
+  rule_action    = "allow"
+  cidr_block     = "10.0.0.0/16"
+}
+`
+
+func init() {
+	scanner.RegisterCheck(scanner.Check{
+		Code: AWSOpenIngressNetworkACLRule,
+		Documentation: scanner.CheckDocumentation{
+			Summary:     AWSOpenIngressNetworkACLRuleDescription,
+			Explanation: AWSOpenIngressNetworkACLRuleExplanation,
+			BadExample:  AWSOpenIngressNetworkACLRuleBadExample,
+			GoodExample: AWSOpenIngressNetworkACLRuleGoodExample,
+			Links: []string{
+				"https://docs.aws.amazon.com/vpc/latest/userguide/vpc-network-acls.html",
+			},
+		},
+		Provider:       scanner.AWSProvider,
+		RequiredTypes:  []string{"resource"},
+		RequiredLabels: []string{"aws_network_acl_rule"},
+		CheckFunc: func(check *scanner.Check, block *parser.Block, _ *scanner.Context) []scanner.Result {
+
+			egressAttr := block.GetAttribute("egress")
+			actionAttr := block.GetAttribute("rule_action")
+			protoAttr := block.GetAttribute("protocol")
+
+			if egressAttr.Type() == cty.Bool && egressAttr.Value().True() {
+				return nil
+			}
+
+			if actionAttr == nil || actionAttr.Type() != cty.String {
+				return nil
+			}
+
+			if actionAttr.Value().AsString() != "allow" {
+				return nil
+			}
+
+			if cidrBlockAttr := block.GetAttribute("cidr_block"); cidrBlockAttr != nil {
+
+				if isOpenCidr(cidrBlockAttr, check.Provider) {
+					if protoAttr.Value().AsString() == "all" || protoAttr.Value().AsString() == "-1" {
+						return nil
+					} else {
+						return []scanner.Result{
+							check.NewResult(
+								fmt.Sprintf("Resource '%s' defines a Network ACL rule that allows specific ingress ports from anywhere.", block.FullName()),
+								cidrBlockAttr.Range(),
+								scanner.SeverityWarning,
+							),
+						}
+					}
+				}
+
+			}
+
+			if ipv6CidrBlockAttr := block.GetAttribute("ipv6_cidr_block"); ipv6CidrBlockAttr != nil {
+
+				if isOpenCidr(ipv6CidrBlockAttr, check.Provider) {
+					if protoAttr.Value().AsString() == "all" || protoAttr.Value().AsString() == "-1" {
+						return nil
+					} else {
+						return []scanner.Result{
+							check.NewResultWithValueAnnotation(
+								fmt.Sprintf("Resource '%s' defines a Network ACL rule that allows specific ingress ports from anywhere.", block.FullName()),
+								ipv6CidrBlockAttr.Range(),
+								ipv6CidrBlockAttr,
+								scanner.SeverityWarning,
+							),
+						}
+					}
+				}
+
+			}
+
+			return nil
+		},
+	})
+}
diff --git a/internal/app/tfsec/checks/aws050.go b/internal/app/tfsec/checks/aws050.go
new file mode 100644
index 000000000..f8fc7c455
--- /dev/null
+++ b/internal/app/tfsec/checks/aws050.go
@@ -0,0 +1,108 @@
+package checks
+
+import (
+	"fmt"
+	"github.com/tfsec/tfsec/internal/app/tfsec/parser"
+	"github.com/tfsec/tfsec/internal/app/tfsec/scanner"
+	"github.com/zclconf/go-cty/cty"
+)
+
+const AWSOpenAllIngressNetworkACLRule scanner.RuleCode = "AWS050"
+const AWSOpenAllIngressNetworkACLRuleDescription scanner.RuleSummary = "An ingress Network ACL rule allows ALL ports from `/0`."
+const AWSOpenAllIngressNetworkACLRuleExplanation = `
+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, and ensure that you specify required ports.
+
+`
+const AWSOpenAllIngressNetworkACLRuleBadExample = `
+resource "aws_network_acl_rule" "my-rule" {
+  egress         = false
+  protocol       = "all"
+  rule_action    = "allow"
+  cidr_block     = "0.0.0.0/0"
+}
+`
+const AWSOpenAllIngressNetworkACLRuleGoodExample = `
+resource "aws_network_acl_rule" "my-rule" {
+  egress         = false
+  protocol       = "tcp"
+  from_port      = 22
+  to_port        = 22
+  rule_action    = "allow"
+  cidr_block     = "0.0.0.0/0"
+}
+`
+
+func init() {
+	scanner.RegisterCheck(scanner.Check{
+		Code: AWSOpenAllIngressNetworkACLRule,
+		Documentation: scanner.CheckDocumentation{
+			Summary:     AWSOpenAllIngressNetworkACLRuleDescription,
+			Explanation: AWSOpenAllIngressNetworkACLRuleExplanation,
+			BadExample:  AWSOpenAllIngressNetworkACLRuleBadExample,
+			GoodExample: AWSOpenAllIngressNetworkACLRuleGoodExample,
+			Links: []string{
+				"https://docs.aws.amazon.com/vpc/latest/userguide/vpc-network-acls.html",
+			},
+		},
+		Provider:       scanner.AWSProvider,
+		RequiredTypes:  []string{"resource"},
+		RequiredLabels: []string{"aws_network_acl_rule"},
+		CheckFunc: func(check *scanner.Check, block *parser.Block, _ *scanner.Context) []scanner.Result {
+
+			egressAttr := block.GetAttribute("egress")
+			actionAttr := block.GetAttribute("rule_action")
+			protoAttr := block.GetAttribute("protocol")
+
+			if egressAttr.Type() == cty.Bool && egressAttr.Value().True() {
+				return nil
+			}
+
+			if actionAttr == nil || actionAttr.Type() != cty.String {
+				return nil
+			}
+
+			if actionAttr.Value().AsString() != "allow" {
+				return nil
+			}
+
+			if cidrBlockAttr := block.GetAttribute("cidr_block"); cidrBlockAttr != nil {
+
+				if isOpenCidr(cidrBlockAttr, check.Provider) {
+					if protoAttr.Value().AsString() == "all" || protoAttr.Value().AsString() == "-1" {
+						return []scanner.Result{
+							check.NewResult(
+								fmt.Sprintf("Resource '%s' defines a fully open ingress Network ACL rule with ALL ports open.", block.FullName()),
+								cidrBlockAttr.Range(),
+								scanner.SeverityError,
+							),
+						}
+					} else {
+						return nil
+					}
+				}
+
+			}
+
+			if ipv6CidrBlockAttr := block.GetAttribute("ipv6_cidr_block"); ipv6CidrBlockAttr != nil {
+
+				if isOpenCidr(ipv6CidrBlockAttr, check.Provider) {
+					if protoAttr.Value().AsString() == "all" || protoAttr.Value().AsString() == "-1" {
+						return []scanner.Result{
+							check.NewResultWithValueAnnotation(
+								fmt.Sprintf("Resource '%s' defines a fully open ingress Network ACL rule with ALL ports open.", block.FullName()),
+								ipv6CidrBlockAttr.Range(),
+								ipv6CidrBlockAttr,
+								scanner.SeverityError,
+							),
+						}
+					} else {
+						return nil
+					}
+				}
+
+			}
+
+			return nil
+		},
+	})
+}


Test patch:
diff --git a/internal/app/tfsec/test/aws049_test.go b/internal/app/tfsec/test/aws049_test.go
new file mode 100644
index 000000000..474031bb9
--- /dev/null
+++ b/internal/app/tfsec/test/aws049_test.go
@@ -0,0 +1,84 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/tfsec/tfsec/internal/app/tfsec/checks"
+	"github.com/tfsec/tfsec/internal/app/tfsec/scanner"
+)
+
+func Test_AWSOpenIngressNetworkACLRule(t *testing.T) {
+
+	var tests = []struct {
+		name                  string
+		source                string
+		mustIncludeResultCode scanner.RuleCode
+		mustExcludeResultCode scanner.RuleCode
+	}{
+		{
+			name: "check aws_network_acl_rule ingress on 0.0.0.0/0",
+			source: `
+resource "aws_network_acl_rule" "my-rule" {
+  egress         = false
+  protocol       = "tcp"
+  from_port      = 22
+  to_port        = 22
+  rule_action    = "allow"
+  cidr_block     = "0.0.0.0/0"
+}`,
+			mustIncludeResultCode: checks.AWSOpenIngressNetworkACLRule,
+		},{
+			name: "check aws_network_acl_rule ingress on 0.0.0.0/0 implied egress",
+			source: `
+resource "aws_network_acl_rule" "my-rule" {
+  protocol       = "tcp"
+  from_port      = 22
+  to_port        = 22
+  rule_action    = "allow"
+  cidr_block     = "0.0.0.0/0"
+}`,
+			mustIncludeResultCode: checks.AWSOpenIngressNetworkACLRule,
+		},
+		{
+			name: "check variable containing 0.0.0.0/0",
+			source: `
+resource "aws_network_acl_rule" "my-rule" {
+  egress         = false
+  protocol       = "tcp"
+  from_port      = 22
+  to_port        = 22
+  rule_action    = "allow"
+  cidr_block     = var.cidr
+}
+
+variable "cidr" {
+	default="0.0.0.0/0"
+}
+
+`,
+			mustIncludeResultCode: checks.AWSOpenIngressNetworkACLRule,
+		},
+		{
+			name: "check aws_network_acl_rule ingress on ::/0",
+			source: `
+resource "aws_network_acl_rule" "my-rule" {
+  rule_number    = 200
+  egress         = false
+  protocol       = "tcp"
+  from_port      = 22
+  to_port        = 22
+  rule_action    = "allow"
+  ipv6_cidr_block = "::/0"
+}`,
+			mustIncludeResultCode: checks.AWSOpenIngressNetworkACLRule,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			results := scanSource(test.source)
+			assertCheckCode(t, test.mustIncludeResultCode, test.mustExcludeResultCode, results)
+		})
+	}
+
+}
diff --git a/internal/app/tfsec/test/aws050_test.go b/internal/app/tfsec/test/aws050_test.go
new file mode 100644
index 000000000..dcb63549f
--- /dev/null
+++ b/internal/app/tfsec/test/aws050_test.go
@@ -0,0 +1,76 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/tfsec/tfsec/internal/app/tfsec/checks"
+	"github.com/tfsec/tfsec/internal/app/tfsec/scanner"
+)
+
+func Test_AWSOpenAllIngressNetworkACLRule(t *testing.T) {
+
+	var tests = []struct {
+		name                  string
+		source                string
+		mustIncludeResultCode scanner.RuleCode
+		mustExcludeResultCode scanner.RuleCode
+	}{
+		{
+			name: "check aws_network_acl_rule ingress on 0.0.0.0/0",
+			source: `
+resource "aws_network_acl_rule" "my-rule" {
+  egress         = false
+  protocol       = "all"
+  rule_action    = "allow"
+  cidr_block     = "0.0.0.0/0"
+}`,
+			mustIncludeResultCode: checks.AWSOpenAllIngressNetworkACLRule,
+		},{
+			name: "check aws_network_acl_rule ingress on 0.0.0.0/0 implied egress",
+			source: `
+resource "aws_network_acl_rule" "my-rule" {
+  protocol       = "all"
+  rule_action    = "allow"
+  cidr_block     = "0.0.0.0/0"
+}`,
+			mustIncludeResultCode: checks.AWSOpenAllIngressNetworkACLRule,
+		},
+		{
+			name: "check variable containing 0.0.0.0/0",
+			source: `
+resource "aws_network_acl_rule" "my-rule" {
+  egress         = false
+  protocol       = "-1"
+  rule_action    = "allow"
+  cidr_block     = var.cidr
+}
+
+variable "cidr" {
+	default="0.0.0.0/0"
+}
+
+`,
+			mustIncludeResultCode: checks.AWSOpenAllIngressNetworkACLRule,
+		},
+		{
+			name: "check aws_network_acl_rule ingress on ::/0",
+			source: `
+resource "aws_network_acl_rule" "my-rule" {
+  rule_number    = 200
+  egress         = false
+  protocol       = "all"
+  rule_action    = "allow"
+  ipv6_cidr_block = "::/0"
+}`,
+			mustIncludeResultCode: checks.AWSOpenAllIngressNetworkACLRule,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			results := scanSource(test.source)
+			assertCheckCode(t, test.mustIncludeResultCode, test.mustExcludeResultCode, results)
+		})
+	}
+
+}