Merge branch 'master' into reed/avd_docs_gen_tf_cf

avd_docs/github/branch_protections/AVD-GIT-0004/Terraform.md

@@ -0,0 +1,14 @@
+
+Require signed commits for a repository
+
+```hcl
+resource "github_branch_protection" "good_example" {
+  repository_id = "example"
+  pattern       = "main"
+
+  require_signed_commits = true
+}
+```
+
+#### Remediation Links
+ - https://registry.terraform.io/providers/integrations/github/latest/docs/resources/branch_protection#require_signed_commits

avd_docs/github/branch_protections/AVD-GIT-0004/docs.md

@@ -0,0 +1,19 @@
+
+GitHub branch protection should be set to require signed commits.
+
+You can do this by setting the <code>require_signed_commits</code> attribute to 'true'.
+
+### Impact
+Commits may not be verified and signed as coming from a trusted developer
+
+<!-- DO NOT CHANGE -->
+{{ remediationActions }}
+
+### Links
+- https://registry.terraform.io/providers/integrations/github/latest/docs/resources/branch_protection#require_signed_commits
+
+- https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification
+
+- https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/about-protected-branches#require-signed-commits
+
+

internal/adapters/terraform/aws/s3/public_access_block.go

@@ -18,17 +18,6 @@ func (a *adapter) adaptPublicAccessBlocks() {
 
 		var bucketName string
 		bucketAttr := b.GetAttribute("bucket")
-
-		if bucketAttr.IsNotNil() {
-			if referencedBlock, err := a.modules.GetReferencedBlock(bucketAttr, b); err == nil {
-				if bucket, ok := a.bucketMap[referencedBlock.ID()]; ok {
-					bucket.PublicAccessBlock = &pba
-					a.bucketMap[referencedBlock.ID()] = bucket
-					continue
-				}
-			}
-		}
-
 		if bucketAttr.IsString() {
 			bucketName = bucketAttr.Value().AsString()
 			for id, bucket := range a.bucketMap {

internal/adapters/terraform/azure/storage/adapt.go

@@ -85,6 +85,13 @@ func adaptAccounts(modules terraform.Modules) ([]storage.Account, []string, []st
 				accountedForNetworkRules = append(accountedForNetworkRules, networkRuleBlock.ID())
 				account.NetworkRules = append(account.NetworkRules, adaptNetworkRule(networkRuleBlock))
 			}
+			for _, queueBlock := range module.GetReferencingResources(resource, "azurerm_storage_queue", "storage_account_name") {
+				queue := storage.Queue{
+					Metadata: queueBlock.GetMetadata(),
+					Name:     queueBlock.GetAttribute("name").AsStringValueOrDefault("", queueBlock),
+				}
+				account.Queues = append(account.Queues, queue)
+			}
 			accounts = append(accounts, account)
 		}
 	}

internal/adapters/terraform/github/adapt.go

@@ -1,6 +1,7 @@
 package github
 
 import (
+	"github.com/aquasecurity/defsec/internal/adapters/terraform/github/branch_protections"
 	"github.com/aquasecurity/defsec/internal/adapters/terraform/github/repositories"
 	"github.com/aquasecurity/defsec/internal/adapters/terraform/github/secrets"
 	"github.com/aquasecurity/defsec/pkg/providers/github"
@@ -11,5 +12,6 @@ func Adapt(modules terraform.Modules) github.GitHub {
 	return github.GitHub{
 		Repositories:       repositories.Adapt(modules),
 		EnvironmentSecrets: secrets.Adapt(modules),
+		BranchProtections:  branch_protections.Adapt(modules),
 	}
 }

internal/adapters/terraform/github/branch_protections/adapt.go

@@ -0,0 +1,30 @@
+package branch_protections
+
+import (
+	"github.com/aquasecurity/defsec/pkg/providers/github"
+	"github.com/aquasecurity/defsec/pkg/terraform"
+)
+
+func Adapt(modules terraform.Modules) []github.BranchProtection {
+	return adaptBranchProtections(modules)
+}
+
+func adaptBranchProtections(modules terraform.Modules) []github.BranchProtection {
+	var branchProtections []github.BranchProtection
+	for _, module := range modules {
+		for _, resource := range module.GetResourcesByType("github_branch_protection") {
+			branchProtections = append(branchProtections, adaptBranchProtection(resource))
+		}
+	}
+	return branchProtections
+}
+
+func adaptBranchProtection(resource *terraform.Block) github.BranchProtection {
+
+	branchProtection := github.BranchProtection{
+		Metadata:             resource.GetMetadata(),
+		RequireSignedCommits: resource.GetAttribute("require_signed_commits").AsBoolValueOrDefault(false, resource),
+	}
+
+	return branchProtection
+}

internal/adapters/terraform/github/branch_protections/adapt_test.go

@@ -0,0 +1,59 @@
+package branch_protections
+
+import (
+	"testing"
+
+	"github.com/aquasecurity/defsec/internal/adapters/terraform/tftestutil"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func Test_AdaptDefaults(t *testing.T) {
+
+	src := `
+resource "github_branch_protection" "my-repo" {
+	
+}
+`
+	modules := tftestutil.CreateModulesFromSource(t, src, ".tf")
+	branchProtections := Adapt(modules)
+	require.Len(t, branchProtections, 1)
+	branchProtection := branchProtections[0]
+
+	assert.True(t, branchProtection.RequireSignedCommits.IsFalse())
+}
+
+func Test_Adapt_RequireSignedCommitsEnabled(t *testing.T) {
+
+	src := `
+resource "github_branch_protection" "my-repo" {
+	require_signed_commits = true
+}
+`
+	modules := tftestutil.CreateModulesFromSource(t, src, ".tf")
+	branchProtections := Adapt(modules)
+	require.Len(t, branchProtections, 1)
+	branchProtection := branchProtections[0]
+
+	assert.True(t, branchProtection.RequireSignedCommits.IsTrue())
+	assert.Equal(t, 3, branchProtection.RequireSignedCommits.GetMetadata().Range().GetStartLine())
+	assert.Equal(t, 3, branchProtection.RequireSignedCommits.GetMetadata().Range().GetEndLine())
+}
+
+func Test_Adapt_RequireSignedCommitsDisabled(t *testing.T) {
+
+	src := `
+resource "github_branch_protection" "my-repo" {
+	require_signed_commits = false
+}
+`
+	modules := tftestutil.CreateModulesFromSource(t, src, ".tf")
+	branchProtections := Adapt(modules)
+	require.Len(t, branchProtections, 1)
+	branchProtection := branchProtections[0]
+
+	assert.False(t, branchProtection.RequireSignedCommits.IsTrue())
+	assert.Equal(t, 3, branchProtection.RequireSignedCommits.GetMetadata().Range().GetStartLine())
+	assert.Equal(t, 3, branchProtection.RequireSignedCommits.GetMetadata().Range().GetEndLine())
+}

internal/rules/azure/storage/queue_services_logging_enabled.go

@@ -35,7 +35,7 @@ Requests are logged on a best-effort basis.`,
 	},
 	func(s *state.State) (results scan.Results) {
 		for _, account := range s.Azure.Storage.Accounts {
-			if account.IsUnmanaged() {
+			if account.IsUnmanaged() || len(account.Queues) == 0 {
 				continue
 			}
 			if account.QueueProperties.EnableLogging.IsFalse() {

internal/rules/azure/storage/queue_services_logging_enabled.tf.go

@@ -32,6 +32,11 @@ var terraformQueueServicesLoggingEnabledBadExamples = []string{
      queue_properties  {
    }
  }
+
+  resource "azurerm_storage_queue" "bad_example" {
+	 name                 = "my-queue"
+	 storage_account_name  = azurerm_storage_account.bad_example.name
+  }
  `,
 }
 

internal/rules/azure/storage/queue_services_logging_enabled_test.go

@@ -29,11 +29,32 @@ func TestCheckQueueServicesLoggingEnabled(t *testing.T) {
 							Metadata:      defsecTypes.NewTestMetadata(),
 							EnableLogging: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
 						},
+						Queues: []storage.Queue{
+							{
+								Metadata: defsecTypes.NewTestMetadata(),
+								Name:     defsecTypes.String("my-queue", defsecTypes.NewTestMetadata()),
+							},
+						},
 					},
 				},
 			},
 			expected: true,
 		},
+		{
+			name: "Storage account queue properties logging disabled with no queues",
+			input: storage.Storage{
+				Accounts: []storage.Account{
+					{
+						Metadata: defsecTypes.NewTestMetadata(),
+						QueueProperties: storage.QueueProperties{
+							Metadata:      defsecTypes.NewTestMetadata(),
+							EnableLogging: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
+						},
+					},
+				},
+			},
+			expected: false,
+		},
 		{
 			name: "Storage account queue properties logging enabled",
 			input: storage.Storage{

internal/rules/github/branch_protections/require_signed_commits.go

@@ -0,0 +1,49 @@
+package branch_protections
+
+import (
+	"github.com/aquasecurity/defsec/internal/rules"
+	"github.com/aquasecurity/defsec/pkg/providers"
+	"github.com/aquasecurity/defsec/pkg/scan"
+	"github.com/aquasecurity/defsec/pkg/severity"
+	"github.com/aquasecurity/defsec/pkg/state"
+)
+
+var CheckRequireSignedCommits = rules.Register(
+	scan.Rule{
+		AVDID:      "AVD-GIT-0004",
+		Provider:   providers.GitHubProvider,
+		Service:    "branch_protections",
+		ShortCode:  "require_signed_commits",
+		Summary:    "GitHub branch protection does not require signed commits.",
+		Impact:     "Commits may not be verified and signed as coming from a trusted developer",
+		Resolution: "Require signed commits",
+		Explanation: `GitHub branch protection should be set to require signed commits.
+
+You can do this by setting the <code>require_signed_commits</code> attribute to 'true'.`,
+		Links: []string{
+			"https://registry.terraform.io/providers/integrations/github/latest/docs/resources/branch_protection#require_signed_commits",
+			"https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification",
+			"https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/about-protected-branches#require-signed-commits",
+		},
+		Terraform: &scan.EngineMetadata{
+			GoodExamples:        terraformRequireSignedCommitsGoodExamples,
+			BadExamples:         terraformRequireSignedCommitsBadExamples,
+			Links:               terraformRequireSignedCommitsLinks,
+			RemediationMarkdown: terraformRequireSignedCommitsRemediationMarkdown,
+		},
+		Severity: severity.High,
+	},
+	func(s *state.State) (results scan.Results) {
+		for _, branchProtection := range s.GitHub.BranchProtections {
+			if branchProtection.RequireSignedCommits.IsFalse() {
+				results.Add(
+					"Branch protection does not require signed commits,",
+					branchProtection.RequireSignedCommits,
+				)
+			} else {
+				results.AddPassed(branchProtection)
+			}
+		}
+		return
+	},
+)

internal/rules/github/branch_protections/require_signed_commits.tf.go

@@ -0,0 +1,29 @@
+package branch_protections
+
+var terraformRequireSignedCommitsGoodExamples = []string{
+	`
+ resource "github_branch_protection" "good_example" {
+   repository_id = "example"
+   pattern       = "main"
+
+   require_signed_commits = true
+ }
+ `,
+}
+
+var terraformRequireSignedCommitsBadExamples = []string{
+	`
+ resource "github_branch_protection" "good_example" {
+   repository_id = "example"
+   pattern       = "main"
+
+   require_signed_commits = false
+ }
+ `,
+}
+
+var terraformRequireSignedCommitsLinks = []string{
+	`https://registry.terraform.io/providers/integrations/github/latest/docs/resources/branch_protection`,
+}
+
+var terraformRequireSignedCommitsRemediationMarkdown = ``

internal/rules/github/branch_protections/require_signed_commits_test.go

@@ -0,0 +1,61 @@
+package branch_protections
+
+import (
+	"testing"
+
+	defsecTypes "github.com/aquasecurity/defsec/pkg/types"
+
+	"github.com/aquasecurity/defsec/pkg/state"
+
+	"github.com/aquasecurity/defsec/pkg/providers/github"
+	"github.com/aquasecurity/defsec/pkg/scan"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCheckRequireSignedCommits(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    []github.BranchProtection
+		expected bool
+	}{
+		{
+			name: "Require signed commits enabled for branch",
+			input: []github.BranchProtection{
+				{
+					Metadata:             defsecTypes.NewTestMetadata(),
+					RequireSignedCommits: defsecTypes.Bool(true, defsecTypes.NewTestMetadata()),
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "Require signed commits disabled for repository",
+			input: []github.BranchProtection{
+				{
+					Metadata:             defsecTypes.NewTestMetadata(),
+					RequireSignedCommits: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
+				},
+			},
+			expected: true,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			var testState state.State
+			testState.GitHub.BranchProtections = test.input
+			results := CheckRequireSignedCommits.Evaluate(&testState)
+			var found bool
+			for _, result := range results {
+				if result.Status() != scan.StatusPassed && result.Rule().LongID() == CheckRequireSignedCommits.Rule().LongID() {
+					found = true
+				}
+			}
+			if test.expected {
+				assert.True(t, found, "Rule should have been found")
+			} else {
+				assert.False(t, found, "Rule should not have been found")
+			}
+		})
+	}
+}