Repository: tfsec/tfsec
Pull request #840 (2021-07-09 08:35:01)

Add more checks for Digital Ocean
- Add a rule for digital ocean load balancer
- Add a rule for public read AC:
- add rule and tests for loadbalancer
- fix cyclo issues caused by refactoring hclattribute


Patch:
diff --git a/internal/app/tfsec/block/hclattribute.go b/internal/app/tfsec/block/hclattribute.go
index 4dc387454..fa1e4eef5 100644
--- a/internal/app/tfsec/block/hclattribute.go
+++ b/internal/app/tfsec/block/hclattribute.go
@@ -299,13 +299,8 @@ func (attr *HCLAttribute) IsEmpty() bool {
 	}
 	if attr.Value().IsNull() {
 		switch t := attr.hclAttribute.Expr.(type) {
-		case *hclsyntax.FunctionCallExpr:
-			return false
-		case *hclsyntax.ScopeTraversalExpr:
-			return false
-		case *hclsyntax.ConditionalExpr:
-			return false
-		case *hclsyntax.LiteralValueExpr:
+		case *hclsyntax.FunctionCallExpr, *hclsyntax.ScopeTraversalExpr,
+			*hclsyntax.ConditionalExpr, *hclsyntax.LiteralValueExpr:
 			return false
 		case *hclsyntax.TemplateExpr:
 			// walk the parts of the expression to ensure that it has a literal value
diff --git a/internal/app/tfsec/rules/dig004.go b/internal/app/tfsec/rules/dig004.go
new file mode 100644
index 000000000..c19eaffb3
--- /dev/null
+++ b/internal/app/tfsec/rules/dig004.go
@@ -0,0 +1,98 @@
+package rules
+
+import (
+	"fmt"
+
+	"github.com/tfsec/tfsec/internal/app/tfsec/block"
+	"github.com/tfsec/tfsec/internal/app/tfsec/hclcontext"
+	"github.com/tfsec/tfsec/internal/app/tfsec/scanner"
+	"github.com/tfsec/tfsec/pkg/provider"
+	"github.com/tfsec/tfsec/pkg/result"
+	"github.com/tfsec/tfsec/pkg/rule"
+	"github.com/tfsec/tfsec/pkg/severity"
+)
+
+const DIGLoadBalancerWithPlainHTTP = "DIG004"
+const DIGLoadBalancerWithPlainHTTPDescription = "The load balancer forwarding rule is using an insecure protocol as an entrypoint"
+const DIGLoadBalancerWithPlainHTTPImpact = "Your inbound traffic is not protected"
+const DIGLoadBalancerWithPlainHTTPResolution = "Switch to HTTPS to benefit from TLS security features"
+const DIGLoadBalancerWithPlainHTTPExplanation = `
+Plain HTTP is unencrypted and human-readable. This means that if a malicious actor was to eavesdrop on your connection, they would be able to see all of your data flowing back and forth.
+
+You should use HTTPS, which is HTTP over an encrypted (TLS) connection, meaning eavesdroppers cannot read your traffic.
+`
+const DIGLoadBalancerWithPlainHTTPBadExample = `
+resource "digitalocean_loadbalancer" "bad_example" {
+  name   = "bad_example-1"
+  region = "nyc3"
+
+  forwarding_rule {
+    entry_port     = 80
+    entry_protocol = "http"
+
+    target_port     = 80
+    target_protocol = "http"
+  }
+
+  droplet_ids = [digitalocean_droplet.web.id]
+}
+`
+const DIGLoadBalancerWithPlainHTTPGoodExample = `
+resource "digitalocean_loadbalancer" "bad_example" {
+  name   = "bad_example-1"
+  region = "nyc3"
+  
+  forwarding_rule {
+	entry_port     = 443
+	entry_protocol = "https"
+  
+	target_port     = 443
+	target_protocol = "https"
+  }
+  
+  droplet_ids = [digitalocean_droplet.web.id]
+}
+`
+
+func init() {
+	scanner.RegisterCheckRule(rule.Rule{
+		ID: DIGLoadBalancerWithPlainHTTP,
+		Documentation: rule.RuleDocumentation{
+			Summary:     DIGLoadBalancerWithPlainHTTPDescription,
+			Explanation: DIGLoadBalancerWithPlainHTTPExplanation,
+			Impact:      DIGLoadBalancerWithPlainHTTPImpact,
+			Resolution:  DIGLoadBalancerWithPlainHTTPResolution,
+			BadExample:  DIGLoadBalancerWithPlainHTTPBadExample,
+			GoodExample: DIGLoadBalancerWithPlainHTTPGoodExample,
+			Links: []string{
+				"https://registry.terraform.io/providers/digitalocean/digitalocean/latest/docs/resources/loadbalancer",
+				"https://docs.digitalocean.com/products/networking/load-balancers/",
+			},
+		},
+		Provider:        provider.DigitalOceanProvider,
+		RequiredTypes:   []string{"resource"},
+		RequiredLabels:  []string{"digitalocean_loadbalancer"},
+		DefaultSeverity: severity.Warning,
+		CheckFunc: func(set result.Set, resourceBlock block.Block, _ *hclcontext.Context) {
+
+			if resourceBlock.MissingChild("forwarding_rule") {
+				return
+			}
+
+			forwardingRules := resourceBlock.GetBlocks("forwarding_rule")
+			for _, rule := range forwardingRules {
+				if rule.MissingChild("entry_protocol") {
+					continue
+				}
+				entryPointAttr := rule.GetAttribute("entry_protocol")
+				if entryPointAttr.Equals("http", block.IgnoreCase) {
+					set.Add(result.New(resourceBlock).
+						WithDescription(fmt.Sprintf("Resource '%s' uses plain HTTP instead of HTTPS.", resourceBlock.FullName())).
+						WithSeverity(severity.Error).
+						WithAttributeAnnotation(entryPointAttr).
+						WithRange(entryPointAttr.Range()))
+				}
+			}
+		},
+	})
+}
diff --git a/internal/app/tfsec/rules/dig005.go b/internal/app/tfsec/rules/dig005.go
new file mode 100644
index 000000000..d41c3a27c
--- /dev/null
+++ b/internal/app/tfsec/rules/dig005.go
@@ -0,0 +1,88 @@
+package rules
+
+import (
+	"fmt"
+
+	"github.com/tfsec/tfsec/internal/app/tfsec/block"
+	"github.com/tfsec/tfsec/internal/app/tfsec/hclcontext"
+	"github.com/tfsec/tfsec/internal/app/tfsec/scanner"
+	"github.com/tfsec/tfsec/pkg/provider"
+	"github.com/tfsec/tfsec/pkg/result"
+	"github.com/tfsec/tfsec/pkg/rule"
+	"github.com/tfsec/tfsec/pkg/severity"
+)
+
+const DIGPublicReadAclOnSpacesBucket = "DIG005"
+const DIGPublicReadAclOnSpacesBucketDescription = "Spaces bucket or bucket object has public read acl set"
+const DIGPublicReadAclOnSpacesBucketImpact = "The contents of the space can be accessed publicly"
+const DIGPublicReadAclOnSpacesBucketResolution = "Applt more restrictive ACL"
+const DIGPublicReadAclOnSpacesBucketExplanation = `
+Space bucket and bucket object permissions should be set to deny public access unless explicitly required.
+`
+const DIGPublicReadAclOnSpacesBucketBadExample = `
+resource "digitalocean_spaces_bucket" "bad_example" {
+  name   = "public_space"
+  region = "nyc3"
+  acl    = "public-read"
+}
+
+resource "digitalocean_spaces_bucket_object" "index" {
+  region       = digitalocean_spaces_bucket.bad_example.region
+  bucket       = digitalocean_spaces_bucket.bad_example.name
+  key          = "index.html"
+  content      = "<html><body><p>This page is empty.</p></body></html>"
+  content_type = "text/html"
+  acl          = "public-read"
+}
+`
+const DIGPublicReadAclOnSpacesBucketGoodExample = `
+resource "digitalocean_spaces_bucket" "good_example" {
+  name   = "private_space"
+  region = "nyc3"
+  acl    = "private"
+}
+  
+resource "digitalocean_spaces_bucket_object" "index" {
+  region       = digitalocean_spaces_bucket.good_example.region
+  bucket       = digitalocean_spaces_bucket.good_example.name
+  key          = "index.html"
+  content      = "<html><body><p>This page is empty.</p></body></html>"
+  content_type = "text/html"
+}
+`
+
+func init() {
+	scanner.RegisterCheckRule(rule.Rule{
+		ID: DIGPublicReadAclOnSpacesBucket,
+		Documentation: rule.RuleDocumentation{
+			Summary:     DIGPublicReadAclOnSpacesBucketDescription,
+			Explanation: DIGPublicReadAclOnSpacesBucketExplanation,
+			Impact:      DIGPublicReadAclOnSpacesBucketImpact,
+			Resolution:  DIGPublicReadAclOnSpacesBucketResolution,
+			BadExample:  DIGPublicReadAclOnSpacesBucketBadExample,
+			GoodExample: DIGPublicReadAclOnSpacesBucketGoodExample,
+			Links: []string{
+				"https://registry.terraform.io/providers/digitalocean/digitalocean/latest/docs/resources/spaces_bucket#acl",
+				"https://registry.terraform.io/providers/digitalocean/digitalocean/latest/docs/resources/spaces_bucket_object#acl",
+				"https://docs.digitalocean.com/reference/api/spaces-api/#access-control-lists-acls",
+			},
+		},
+		Provider:        provider.DigitalOceanProvider,
+		RequiredTypes:   []string{"resource"},
+		RequiredLabels:  []string{"digitalocean_spaces_bucket", "digitalocean_spaces_bucket_object"},
+		DefaultSeverity: severity.Warning,
+		CheckFunc: func(set result.Set, resourceBlock block.Block, _ *hclcontext.Context) {
+
+			if resourceBlock.HasChild("acl") {
+				aclAttr := resourceBlock.GetAttribute("acl")
+				if aclAttr.Equals("public-read", block.IgnoreCase) {
+					set.Add(result.New(resourceBlock).
+						WithDescription(fmt.Sprintf("Resource '%s' has a publicly readable acl.", resourceBlock.FullName())).
+						WithSeverity(severity.Error).
+						WithAttributeAnnotation(aclAttr).
+						WithRange(aclAttr.Range()))
+				}
+			}
+		},
+	})
+}


Test patch:
diff --git a/internal/app/tfsec/test/dig004_test.go b/internal/app/tfsec/test/dig004_test.go
new file mode 100644
index 000000000..14bed05da
--- /dev/null
+++ b/internal/app/tfsec/test/dig004_test.go
@@ -0,0 +1,106 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/tfsec/tfsec/internal/app/tfsec/rules"
+)
+
+func Test_DIGLoadBalancerWithPlainHTTP(t *testing.T) {
+
+	var tests = []struct {
+		name                  string
+		source                string
+		mustIncludeResultCode string
+		mustExcludeResultCode string
+	}{
+		{
+			name: "Load balancer with http entry protocol fails check",
+			source: `
+resource "digitalocean_loadbalancer" "bad_example" {
+  name   = "bad_example-1"
+  region = "nyc3"
+
+  forwarding_rule {
+    entry_port     = 80
+    entry_protocol = "http"
+
+    target_port     = 80
+    target_protocol = "http"
+  }
+
+  droplet_ids = [digitalocean_droplet.web.id]
+}
+`,
+			mustIncludeResultCode: rules.DIGLoadBalancerWithPlainHTTP,
+		},
+		{
+			name: "Load blanacer with non plain http passes check (https)",
+			source: `
+resource "digitalocean_loadbalancer" "bad_example" {
+  name   = "bad_example-1"
+  region = "nyc3"
+  
+  forwarding_rule {
+	entry_port     = 443
+	entry_protocol = "https"
+  
+	target_port     = 443
+	target_protocol = "https"
+  }
+  
+  droplet_ids = [digitalocean_droplet.web.id]
+}
+`,
+			mustExcludeResultCode: rules.DIGLoadBalancerWithPlainHTTP,
+		},
+		{
+			name: "Load blanacer with non plain http passes check (http2)",
+			source: `
+resource "digitalocean_loadbalancer" "bad_example" {
+  name   = "bad_example-1"
+  region = "nyc3"
+  
+  forwarding_rule {
+	entry_port     = 443
+	entry_protocol = "http2"
+  
+	target_port     = 443
+	target_protocol = "https"
+  }
+  
+  droplet_ids = [digitalocean_droplet.web.id]
+}
+`,
+			mustExcludeResultCode: rules.DIGLoadBalancerWithPlainHTTP,
+		},
+		{
+			name: "Load blanacer with non plain http passes check (tcp)",
+			source: `
+resource "digitalocean_loadbalancer" "bad_example" {
+  name   = "bad_example-1"
+  region = "nyc3"
+  
+  forwarding_rule {
+	entry_port     = 3128
+	entry_protocol = "tcp"
+  
+	target_port     = 3128
+	target_protocol = "tcpv"
+  }
+  
+  droplet_ids = [digitalocean_droplet.web.id]
+}
+`,
+			mustExcludeResultCode: rules.DIGLoadBalancerWithPlainHTTP,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			results := scanHCL(test.source, t)
+			assertCheckCode(t, test.mustIncludeResultCode, test.mustExcludeResultCode, results)
+		})
+	}
+
+}
diff --git a/internal/app/tfsec/test/dig005_test.go b/internal/app/tfsec/test/dig005_test.go
new file mode 100644
index 000000000..3d27b7b9a
--- /dev/null
+++ b/internal/app/tfsec/test/dig005_test.go
@@ -0,0 +1,75 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/tfsec/tfsec/internal/app/tfsec/rules"
+)
+
+func Test_DIGPublicReadAclOnSpacesBucket(t *testing.T) {
+
+	var tests = []struct {
+		name                  string
+		source                string
+		mustIncludeResultCode string
+		mustExcludeResultCode string
+	}{
+		{
+			name: "Spaces bucket with public-read acl fails check",
+			source: `
+resource "digitalocean_spaces_bucket" "bad_example" {
+  name   = "public_space"
+  region = "nyc3"
+  acl    = "public-read"
+}
+`,
+			mustIncludeResultCode: rules.DIGPublicReadAclOnSpacesBucket,
+		},
+		{
+			name: "Spaces bucket object with public-read acl fails check",
+			source: `
+resource "digitalocean_spaces_bucket_object" "index" {
+  region       = digitalocean_spaces_bucket.bad_example.region
+  bucket       = digitalocean_spaces_bucket.bad_example.name
+  key          = "index.html"
+  content      = "<html><body><p>This page is empty.</p></body></html>"
+  content_type = "text/html"
+  acl          = "public-read"
+}
+`,
+			mustIncludeResultCode: rules.DIGPublicReadAclOnSpacesBucket,
+		},
+		{
+			name: "Spaces bucket object using default acl (private) passes check",
+			source: `
+resource "digitalocean_spaces_bucket_object" "index" {
+  region       = digitalocean_spaces_bucket.good_example.region
+  bucket       = digitalocean_spaces_bucket.good_example.name
+  key          = "index.html"
+  content      = "<html><body><p>This page is empty.</p></body></html>"
+  content_type = "text/html"
+}
+`,
+			mustExcludeResultCode: rules.DIGPublicReadAclOnSpacesBucket,
+		},
+		{
+			name: "Spaces bucket using explicit private acl passes check",
+			source: `
+resource "digitalocean_spaces_bucket" "good_example" {
+  name   = "private_space"
+  region = "nyc3"
+  acl    = "private"
+}
+`,
+			mustExcludeResultCode: rules.DIGPublicReadAclOnSpacesBucket,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			results := scanHCL(test.source, t)
+			assertCheckCode(t, test.mustIncludeResultCode, test.mustExcludeResultCode, results)
+		})
+	}
+
+}