Repository: aquasecurity/tfsec
Pull request #930 (2021-07-22 12:49:17)

Add check for security contact phone
update generator to support hyphenated services



Patch:
diff --git a/cmd/tfsec-skeleton/generate.go b/cmd/tfsec-skeleton/generate.go
index 3fd0edd4c..f292f0651 100644
--- a/cmd/tfsec-skeleton/generate.go
+++ b/cmd/tfsec-skeleton/generate.go
@@ -27,6 +27,7 @@ type checkSkeleton struct {
 	RequiredLabels   string
 	CheckFilename    string
 	TestFileName     string
+	Package          string
 }
 
 var funcMap = template.FuncMap{
@@ -43,9 +44,7 @@ func generateCheckBody() error {
 	checkPath := fmt.Sprintf("internal/app/tfsec/rules/%s", details.CheckFilename)
 	testPath := fmt.Sprintf("internal/app/tfsec/rules/%s", details.TestFileName)
 
-	if err := os.Mkdir(filepath.Dir(checkPath), 0700); err != nil {
-		panic(err)
-	}
+	_ = os.Mkdir(filepath.Dir(checkPath), 0700)
 
 	if err = verifyCheckPath(checkPath); err != nil {
 		return err
@@ -113,11 +112,12 @@ func populateSkeleton(summary, selected, service, shortCode, impact, resolution,
 	checkBody.Summary = summary
 	checkBody.ShortCode = shortCode
 	checkBody.FullCode = strings.ToLower(fmt.Sprintf("%s-%s-%s", selected, service, shortCode))
-	checkBody.Service = strings.ReplaceAll(service, "-", "_")
+	checkBody.Service = service
 	checkBody.Impact = impact
 	checkBody.Resolution = resolution
 	checkBody.Provider = providers[selected]
 	checkBody.ProviderLongName = selected
+	checkBody.Package = strings.ReplaceAll(service, "-", "")
 
 	if err != nil {
 		return nil, err
@@ -126,7 +126,7 @@ func populateSkeleton(summary, selected, service, shortCode, impact, resolution,
 	checkBody.CheckName = fmt.Sprintf("%s%s", strings.Title(selected), strings.ReplaceAll(strings.Title(shortCode), "-", ""))
 	checkBody.RequiredTypes = fmt.Sprintf("{\"%s\"}", strings.Join(strings.Split(blockTypes, " "), "\", \""))
 	checkBody.RequiredLabels = fmt.Sprintf("{\"%s\"}", strings.Join(strings.Split(blockLabels, " "), "\", \""))
-	filename := fmt.Sprintf("%s/%s/%s", selected, strings.ReplaceAll(service, "-", "_"), strings.ReplaceAll(shortCode, "-", "_"))
+	filename := fmt.Sprintf("%s/%s/%s", selected, checkBody.Package, strings.ReplaceAll(shortCode, "-", "_"))
 	checkBody.CheckFilename = fmt.Sprintf("%s_rule.go", strings.ToLower(filename))
 	checkBody.TestFileName = fmt.Sprintf("%s_rule_test.go", strings.ToLower(filename))
 
diff --git a/cmd/tfsec-skeleton/templates.go b/cmd/tfsec-skeleton/templates.go
index 13ea70687..1ca7782fb 100644
--- a/cmd/tfsec-skeleton/templates.go
+++ b/cmd/tfsec-skeleton/templates.go
@@ -1,6 +1,6 @@
 package main
 
-const checkTemplate = `package {{ .Service}}
+const checkTemplate = `package {{ .Package}}
 
 import (
 	"github.com/aquasecurity/tfsec/pkg/result"
@@ -50,7 +50,7 @@ func init() {
 }
 `
 
-const checkTestTemplate = `package {{ .Service}}
+const checkTestTemplate = `package {{ .Package}}
 
 import (
 	"testing"
diff --git a/internal/app/tfsec/rules/azure/securitycenter/alert_on_severe_notifications_rule.go b/internal/app/tfsec/rules/azure/securitycenter/alert_on_severe_notifications_rule.go
new file mode 100644
index 000000000..a6971b059
--- /dev/null
+++ b/internal/app/tfsec/rules/azure/securitycenter/alert_on_severe_notifications_rule.go
@@ -0,0 +1,78 @@
+package securitycenter
+
+import (
+	"fmt"
+
+	"github.com/aquasecurity/tfsec/pkg/result"
+	"github.com/aquasecurity/tfsec/pkg/severity"
+
+	"github.com/aquasecurity/tfsec/pkg/provider"
+
+	"github.com/aquasecurity/tfsec/internal/app/tfsec/hclcontext"
+
+	"github.com/aquasecurity/tfsec/internal/app/tfsec/block"
+
+	"github.com/aquasecurity/tfsec/pkg/rule"
+
+	"github.com/aquasecurity/tfsec/internal/app/tfsec/scanner"
+)
+
+func init() {
+	scanner.RegisterCheckRule(rule.Rule{
+		Service:   "security-center",
+		ShortCode: "alert-on-severe-notifications",
+		Documentation: rule.RuleDocumentation{
+			Summary: "Send notification emails for high severity alerts",
+			Explanation: `It is recommended that at least one valid contact is configured for the security center. 
+Microsoft will notify the security contact directly in the event of a security incident using email and require alerting to be turned on.`,
+			Impact:     "The ability to react to high severity notifications could be delayed",
+			Resolution: " Set alert notifications to be on",
+			BadExample: `
+resource "azurerm_security_center_contact" "bad_example" {
+  email = "bad_example@example.com"
+  phone = "+1-555-555-5555"
+
+  alert_notifications = false
+  alerts_to_admins    = false
+}
+			`,
+			GoodExample: `
+resource "azurerm_security_center_contact" "good_example" {
+  email = "good_example@example.com"
+  phone = "+1-555-555-5555"
+
+  alert_notifications = true
+  alerts_to_admins    = true
+}
+			`,
+			Links: []string{
+				"https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/security_center_contact#alert_notifications",
+				"https://azure.microsoft.com/en-us/services/security-center/",
+			},
+		},
+		Provider:        provider.AzureProvider,
+		RequiredTypes:   []string{"resource"},
+		RequiredLabels:  []string{"azurerm_security_center_contact"},
+		DefaultSeverity: severity.Medium,
+		CheckFunc: func(set result.Set, resourceBlock block.Block, _ *hclcontext.Context) {
+
+			if resourceBlock.MissingChild("alert_notifications") {
+				set.Add(
+					result.New(resourceBlock).WithDescription(fmt.Sprintf("Resource '%s' is missing the required setting for alert_notifications", resourceBlock.FullName())),
+				)
+
+				return
+			}
+
+			alertNotificationsAttr := resourceBlock.GetAttribute("alert_notifications")
+			if alertNotificationsAttr.IsFalse() {
+				set.Add(
+					result.New(resourceBlock).WithDescription(fmt.Sprintf("Resource '%s' has alert_notifications turned off", resourceBlock.FullName())).
+						WithAttributeAnnotation(alertNotificationsAttr).
+						WithRange(alertNotificationsAttr.Range()),
+				)
+			}
+
+		},
+	})
+}
diff --git a/internal/app/tfsec/rules/azure/securitycenter/enable_standard_subscription_rule.go b/internal/app/tfsec/rules/azure/securitycenter/enable_standard_subscription_rule.go
index 57a5bd520..d8602a3dd 100644
--- a/internal/app/tfsec/rules/azure/securitycenter/enable_standard_subscription_rule.go
+++ b/internal/app/tfsec/rules/azure/securitycenter/enable_standard_subscription_rule.go
@@ -49,7 +49,7 @@ resource "azurerm_security_center_subscription_pricing" "good_example" {
 		Provider:        provider.AzureProvider,
 		RequiredTypes:   []string{"resource"},
 		RequiredLabels:  []string{"azurerm_security_center_subscription_pricing"},
-		DefaultSeverity: severity.Medium,
+		DefaultSeverity: severity.Low,
 		CheckFunc: func(set result.Set, resourceBlock block.Block, _ *hclcontext.Context) {
 			tierAttr := resourceBlock.GetAttribute("tier")
 			if tierAttr != nil && tierAttr.Equals("Free", block.IgnoreCase) {
diff --git a/internal/app/tfsec/rules/azure/securitycenter/set_required_contact_details_rule.go b/internal/app/tfsec/rules/azure/securitycenter/set_required_contact_details_rule.go
new file mode 100644
index 000000000..4b1e89ad8
--- /dev/null
+++ b/internal/app/tfsec/rules/azure/securitycenter/set_required_contact_details_rule.go
@@ -0,0 +1,77 @@
+package securitycenter
+
+import (
+	"fmt"
+
+	"github.com/aquasecurity/tfsec/pkg/result"
+	"github.com/aquasecurity/tfsec/pkg/severity"
+
+	"github.com/aquasecurity/tfsec/pkg/provider"
+
+	"github.com/aquasecurity/tfsec/internal/app/tfsec/hclcontext"
+
+	"github.com/aquasecurity/tfsec/internal/app/tfsec/block"
+
+	"github.com/aquasecurity/tfsec/pkg/rule"
+
+	"github.com/aquasecurity/tfsec/internal/app/tfsec/scanner"
+)
+
+func init() {
+	scanner.RegisterCheckRule(rule.Rule{
+		Service:   "security-center",
+		ShortCode: "set-required-contact-details",
+		Documentation: rule.RuleDocumentation{
+			Summary: "The required contact details should be set for security center",
+			Explanation: `It is recommended that at least one valid contact is configured for the security center. 
+Microsoft will notify the security contact directly in the event of a security incident and will look to use a telephone number in cases where a prompt response is required.`,
+			Impact:     "Without a telephone number set, Azure support can't contact",
+			Resolution: "Set a telephone number for security center contact",
+			BadExample: `
+resource "azurerm_security_center_contact" "bad_example" {
+  email = "bad_contact@example.com"
+  phone = ""
+
+  alert_notifications = true
+  alerts_to_admins    = true
+}
+`,
+			GoodExample: `
+resource "azurerm_security_center_contact" "good_example" {
+  email = "good_contact@example.com"
+  phone = "+1-555-555-5555"
+
+  alert_notifications = true
+  alerts_to_admins    = true
+}
+`,
+			Links: []string{
+				"https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/security_center_contact#phone",
+				"https://azure.microsoft.com/en-us/services/security-center/",
+			},
+		},
+		Provider:        provider.AzureProvider,
+		RequiredTypes:   []string{"resource"},
+		RequiredLabels:  []string{"azurerm_security_center_contact"},
+		DefaultSeverity: severity.Low,
+		CheckFunc: func(set result.Set, resourceBlock block.Block, _ *hclcontext.Context) {
+
+			if resourceBlock.MissingChild("phone") {
+				set.Add(
+					result.New(resourceBlock).WithDescription(fmt.Sprintf("Resource '%s' does not have a phone number set for the security contact", resourceBlock.FullName())),
+				)
+				return
+			}
+
+			phoneAttr := resourceBlock.GetAttribute("phone")
+			if phoneAttr.IsEmpty() {
+				set.Add(
+					result.New(resourceBlock).
+						WithDescription(fmt.Sprintf("Resource '%s' does not have a phone number set for the security contact", resourceBlock.FullName())).
+						WithAttributeAnnotation(phoneAttr).
+						WithRange(phoneAttr.Range()),
+				)
+			}
+		},
+	})
+}


Test patch:
diff --git a/internal/app/tfsec/rules/azure/securitycenter/alert_on_severe_notifications_rule_test.go b/internal/app/tfsec/rules/azure/securitycenter/alert_on_severe_notifications_rule_test.go
new file mode 100644
index 000000000..4a0074cab
--- /dev/null
+++ b/internal/app/tfsec/rules/azure/securitycenter/alert_on_severe_notifications_rule_test.go
@@ -0,0 +1,53 @@
+package securitycenter
+
+import (
+	"testing"
+
+	"github.com/aquasecurity/tfsec/internal/app/tfsec/testutil"
+)
+
+func Test_AzureAlertOnSevereNotifications(t *testing.T) {
+	expectedCode := "azure-security-center-alert-on-severe-notifications"
+
+	var tests = []struct {
+		name                  string
+		source                string
+		mustIncludeResultCode string
+		mustExcludeResultCode string
+	}{
+		{
+			name: "contact with alerts turned off fails check",
+			source: `
+resource "azurerm_security_center_contact" "bad_example" {
+  email = "bad_example@example.com"
+  phone = "+1-555-555-5555"
+
+  alert_notifications = false
+  alerts_to_admins    = false
+}
+`,
+			mustIncludeResultCode: expectedCode,
+		},
+		{
+			name: "contact with alerts turned on passes check",
+			source: `
+resource "azurerm_security_center_contact" "good_example" {
+  email = "good_example@example.com"
+  phone = "+1-555-555-5555"
+
+  alert_notifications = true
+  alerts_to_admins    = true
+}
+`,
+			mustExcludeResultCode: expectedCode,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+
+			results := testutil.ScanHCL(test.source, t)
+			testutil.AssertCheckCode(t, test.mustIncludeResultCode, test.mustExcludeResultCode, results)
+		})
+	}
+}
diff --git a/internal/app/tfsec/rules/azure/securitycenter/set_required_contact_details_rule_test.go b/internal/app/tfsec/rules/azure/securitycenter/set_required_contact_details_rule_test.go
new file mode 100644
index 000000000..1b6d39d63
--- /dev/null
+++ b/internal/app/tfsec/rules/azure/securitycenter/set_required_contact_details_rule_test.go
@@ -0,0 +1,65 @@
+package securitycenter
+
+import (
+	"testing"
+
+	"github.com/aquasecurity/tfsec/internal/app/tfsec/testutil"
+)
+
+func Test_AzureSetRequiredContactDetails(t *testing.T) {
+	expectedCode := "azure-security-center-set-required-contact-details"
+
+	var tests = []struct {
+		name                  string
+		source                string
+		mustIncludeResultCode string
+		mustExcludeResultCode string
+	}{
+		{
+			name: "contact with no phone specified fails check",
+			source: `
+resource "azurerm_security_center_contact" "bad_example" {
+  email = "bad_contact@example.com"
+
+  alert_notifications = true
+  alerts_to_admins    = true
+}
+`,
+			mustIncludeResultCode: expectedCode,
+		},
+		{
+			name: "contact with phone empty fails check",
+			source: `
+resource "azurerm_security_center_contact" "bad_example" {
+  email = "bad_contact@example.com"
+  phone = ""
+
+  alert_notifications = true
+  alerts_to_admins    = true
+}
+`,
+			mustIncludeResultCode: expectedCode,
+		},
+		{
+			name: "contact with phone set passes check",
+			source: `
+resource "azurerm_security_center_contact" "good_example" {
+  email = "good_contact@example.com"
+  phone = "+1-555-555-5555"
+
+  alert_notifications = true
+  alerts_to_admins    = true
+}
+`,
+			mustExcludeResultCode: expectedCode,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+
+			results := testutil.ScanHCL(test.source, t)
+			testutil.AssertCheckCode(t, test.mustIncludeResultCode, test.mustExcludeResultCode, results)
+		})
+	}
+}