Merge pull request #86 from aquasecurity/liamg-aws-mfa

AWS IAM MFA enforcement and improved policy parsing/handling
aquasecurity · Jan 17, 2022 · b8e0d72 · b8e0d72
2 parents 7a84b62 + ba4759c
commit b8e0d72
Show file tree

Hide file tree

Showing 26 changed files with 513 additions and 243 deletions.
diff --git a/go.mod b/go.mod
@@ -5,6 +5,7 @@ go 1.17
 require (
 	github.com/liamg/clinch v1.5.6
 	github.com/liamg/gifwrap v0.0.6
+	github.com/liamg/iamgo v0.0.2
 	github.com/liamg/tml v0.4.0
 	github.com/owenrumney/go-sarif/v2 v2.0.13
 	github.com/owenrumney/squealer v0.3.1

diff --git a/go.sum b/go.sum
@@ -245,6 +245,8 @@ github.com/liamg/clinch v1.5.6 h1:cgv8uUroVWW+d23Gfee3v0/bSOKy9d4vUWFlMeNoNS8=
 github.com/liamg/clinch v1.5.6/go.mod h1:IXM+nLBuZ5sOQAYYf9+G51nkaA0WY9cszxE5nPXexhE=
 github.com/liamg/gifwrap v0.0.6 h1:U6wiiwViOEPoIvivVzgeM3CsOi1vpvmcR1+l5vzyR1s=
 github.com/liamg/gifwrap v0.0.6/go.mod h1:oW1r2vIWLYyxW+U0io7YbpPSDIJ79FTlZ+hPnXFLW6E=
+github.com/liamg/iamgo v0.0.2 h1:XWAPL98zyXxdCfKlEv/IaZIPGnLhc6obJ9p6p09LwnI=
+github.com/liamg/iamgo v0.0.2/go.mod h1:qMGOfySmVFw77/NNCY1Lk5Ez2ER5Pwz0xizTLpIbC08=
 github.com/liamg/tml v0.3.0/go.mod h1:0h4EAV/zBOsqI91EWONedjRpO8O0itjGJVd+wG5eC+E=
 github.com/liamg/tml v0.4.0 h1:iZwysIBOaGz6MuaPT/+PCZgvkpgoqBu3ZI6cgApK0zc=
 github.com/liamg/tml v0.4.0/go.mod h1:0h4EAV/zBOsqI91EWONedjRpO8O0itjGJVd+wG5eC+E=

diff --git a/provider/aws/ecr/ecr.go b/provider/aws/ecr/ecr.go
@@ -1,7 +1,6 @@
 package ecr
 
 import (
-	"github.com/aquasecurity/defsec/provider/aws/iam"
 	"github.com/aquasecurity/defsec/types"
 )
 
@@ -13,7 +12,7 @@ type Repository struct {
 	types.Metadata
 	ImageScanning      ImageScanning
 	ImageTagsImmutable types.BoolValue
-	Policy             iam.PolicyDocument
+	Policy             types.StringValue
 	Encryption         Encryption
 }
 

diff --git a/provider/aws/iam/iam.go b/provider/aws/iam/iam.go
@@ -1,25 +1,37 @@
 package iam
 
+import "github.com/aquasecurity/defsec/types"
+
 type IAM struct {
 	PasswordPolicy PasswordPolicy
 	Policies       []Policy
-	GroupPolicies  []GroupPolicy
-	UserPolicies   []UserPolicy
-	RolePolicies   []RolePolicy
+	Groups         []Group
+	Users          []User
+	Roles          []Role
 }
 
 type Policy struct {
-	Document PolicyDocument
+	types.Metadata
+	Name     types.StringValue
+	Document types.StringValue
 }
 
-type GroupPolicy struct {
-	Document PolicyDocument
+type Group struct {
+	types.Metadata
+	Name     types.StringValue
+	Users    []User
+	Policies []Policy
 }
 
-type UserPolicy struct {
-	Document PolicyDocument
+type User struct {
+	types.Metadata
+	Name     types.StringValue
+	Groups   []Group
+	Policies []Policy
 }
 
-type RolePolicy struct {
-	Document PolicyDocument
+type Role struct {
+	types.Metadata
+	Name     types.StringValue
+	Policies []Policy
 }
diff --git a/provider/aws/iam/policy.go b/provider/aws/iam/policy.go
diff --git a/provider/aws/sam/function.go b/provider/aws/sam/function.go
@@ -1,7 +1,6 @@
 package sam
 
 import (
-	"github.com/aquasecurity/defsec/provider/aws/iam"
 	"github.com/aquasecurity/defsec/types"
 )
 
@@ -10,7 +9,7 @@ type Function struct {
 	FunctionName    types.StringValue
 	Tracing         types.StringValue
 	ManagedPolicies []types.StringValue
-	Policies        []iam.PolicyDocument
+	Policies        []types.StringValue
 }
 
 const (

diff --git a/provider/aws/sam/state_machine.go b/provider/aws/sam/state_machine.go
@@ -1,7 +1,6 @@
 package sam
 
 import (
-	"github.com/aquasecurity/defsec/provider/aws/iam"
 	"github.com/aquasecurity/defsec/types"
 )
 
@@ -10,7 +9,7 @@ type StateMachine struct {
 	Name                 types.StringValue
 	LoggingConfiguration LoggingConfiguration
 	ManagedPolicies      []types.StringValue
-	Policies             []iam.PolicyDocument
+	Policies             []types.StringValue
 	Tracing              TracingConfiguration
 }
 

diff --git a/provider/aws/sqs/sqs.go b/provider/aws/sqs/sqs.go
@@ -1,7 +1,6 @@
 package sqs
 
 import (
-	"github.com/aquasecurity/defsec/provider/aws/iam"
 	"github.com/aquasecurity/defsec/types"
 )
 
@@ -12,7 +11,7 @@ type SQS struct {
 type Queue struct {
 	types.Metadata
 	Encryption Encryption
-	Policy     iam.PolicyDocument
+	Policy     types.StringValue
 }
 
 type Encryption struct {

diff --git a/rules/aws/ecr/no_public_access.go b/rules/aws/ecr/no_public_access.go
@@ -7,6 +7,7 @@ import (
 	"github.com/aquasecurity/defsec/rules"
 	"github.com/aquasecurity/defsec/severity"
 	"github.com/aquasecurity/defsec/state"
+	"github.com/liamg/iamgo"
 )
 
 var CheckNoPublicAccess = rules.Register(
@@ -22,26 +23,30 @@ var CheckNoPublicAccess = rules.Register(
 		Links: []string{
 			"https://docs.aws.amazon.com/AmazonECR/latest/public/public-repository-policies.html",
 		},
-		Terraform:   &rules.EngineMetadata{
-            GoodExamples:        terraformNoPublicAccessGoodExamples,
-            BadExamples:         terraformNoPublicAccessBadExamples,
-            Links:               terraformNoPublicAccessLinks,
-            RemediationMarkdown: terraformNoPublicAccessRemediationMarkdown,
-        },
-        CloudFormation:   &rules.EngineMetadata{
-            GoodExamples:        cloudFormationNoPublicAccessGoodExamples,
-            BadExamples:         cloudFormationNoPublicAccessBadExamples,
-            Links:               cloudFormationNoPublicAccessLinks,
-            RemediationMarkdown: cloudFormationNoPublicAccessRemediationMarkdown,
-        },
-        Severity: severity.High,
+		Terraform: &rules.EngineMetadata{
+			GoodExamples:        terraformNoPublicAccessGoodExamples,
+			BadExamples:         terraformNoPublicAccessBadExamples,
+			Links:               terraformNoPublicAccessLinks,
+			RemediationMarkdown: terraformNoPublicAccessRemediationMarkdown,
+		},
+		CloudFormation: &rules.EngineMetadata{
+			GoodExamples:        cloudFormationNoPublicAccessGoodExamples,
+			BadExamples:         cloudFormationNoPublicAccessBadExamples,
+			Links:               cloudFormationNoPublicAccessLinks,
+			RemediationMarkdown: cloudFormationNoPublicAccessRemediationMarkdown,
+		},
+		Severity: severity.High,
 	},
 	func(s *state.State) (results rules.Results) {
 		for _, repo := range s.AWS.ECR.Repositories {
 			if !repo.IsManaged() {
 				continue
 			}
-			for _, statement := range repo.Policy.Statements {
+			policy, err := iamgo.ParseString(repo.Policy.Value())
+			if err != nil {
+				continue
+			}
+			for _, statement := range policy.Statement {
 				var hasECRAction bool
 				for _, action := range statement.Action {
 					if strings.HasPrefix(action, "ecr:") {
@@ -53,16 +58,25 @@ var CheckNoPublicAccess = rules.Register(
 					continue
 				}
 				var foundIssue bool
-				for _, account := range statement.Principal.AWS {
-					if account == "*" {
-						foundIssue = true
-						results.Add(
-							"Policy provides public access to the ECR repository.",
-							&repo,
-							repo.Policy,
-						)
+				if statement.Principal.All {
+					foundIssue = true
+					results.Add(
+						"Policy provides public access to the ECR repository.",
+						&repo,
+						repo.Policy,
+					)
+				} else {
+					for _, account := range statement.Principal.AWS {
+						if account == "*" {
+							foundIssue = true
+							results.Add(
+								"Policy provides public access to the ECR repository.",
+								&repo,
+								repo.Policy,
+							)
+						}
+						continue
 					}
-					continue
 				}
 				if foundIssue {
 					results.AddPassed(&repo)

diff --git a/rules/aws/iam/enforce_mfa.go b/rules/aws/iam/enforce_mfa.go
@@ -1,10 +1,13 @@
 package iam
 
 import (
+	"strings"
+
 	"github.com/aquasecurity/defsec/provider"
 	"github.com/aquasecurity/defsec/rules"
 	"github.com/aquasecurity/defsec/severity"
 	"github.com/aquasecurity/defsec/state"
+	"github.com/liamg/iamgo"
 )
 
 var CheckEnforceMFA = rules.Register(
@@ -22,15 +25,37 @@ IAM user accounts should be protected with multi factor authentication to add sa
 		Links: []string{
 			"https://docs.aws.amazon.com/IAM/latest/UserGuide/id_credentials_passwords_account-policy.html#password-policy-details",
 		},
-		Terraform:   &rules.EngineMetadata{
-            GoodExamples:        terraformEnforceMfaGoodExamples,
-            BadExamples:         terraformEnforceMfaBadExamples,
-            Links:               terraformEnforceMfaLinks,
-            RemediationMarkdown: terraformEnforceMfaRemediationMarkdown,
-        },
-        Severity: severity.Medium,
+		Terraform: &rules.EngineMetadata{
+			GoodExamples:        terraformEnforceMfaGoodExamples,
+			BadExamples:         terraformEnforceMfaBadExamples,
+			Links:               terraformEnforceMfaLinks,
+			RemediationMarkdown: terraformEnforceMfaRemediationMarkdown,
+		},
+		Severity: severity.Medium,
 	},
 	func(s *state.State) (results rules.Results) {
+
+		for _, group := range s.AWS.IAM.Groups {
+			var mfaEnforced bool
+			for _, policy := range group.Policies {
+				document, err := iamgo.ParseString(policy.Document.Value())
+				if err != nil {
+					continue
+				}
+				for _, statement := range document.Statement {
+					for _, condition := range statement.Condition {
+						if strings.EqualFold(condition.Key, "aws:MultiFactorAuthPresent") {
+							mfaEnforced = true
+							break
+						}
+					}
+				}
+			}
+			if !mfaEnforced {
+				results.Add("Multi-Factor Authentication is not enforced for group", group)
+			}
+		}
+
 		return
 	},
 )