Skip to content

Commit

Permalink
feat(rule): improve wildcard checking in policies
Browse files Browse the repository at this point in the history
  • Loading branch information
nikpivkin committed Oct 30, 2023
1 parent 4c2cea5 commit 5cc23d9
Show file tree
Hide file tree
Showing 7 changed files with 131 additions and 210 deletions.
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,5 @@ require (
gopkg.in/warnings.v0 v0.1.2 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
)

replace github.com/aquasecurity/defsec => github.com/nikpivkin/defsec v0.0.0-20231030110125-d441bcbece99
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ github.com/apparentlymart/go-textseg/v13 v13.0.0 h1:Y+KvPE1NYz0xl601PVImeQfFyEy6
github.com/apparentlymart/go-textseg/v13 v13.0.0/go.mod h1:ZK2fH7c4NqDTLtiYLvIkEghdlcqw7yxLeM89kiTRPUo=
github.com/apparentlymart/go-textseg/v15 v15.0.0 h1:uYvfpb3DyLSCGWnctWKGj857c6ew1u1fNQOlOtuGxQY=
github.com/apparentlymart/go-textseg/v15 v15.0.0/go.mod h1:K8XmNZdhEBkdlyDdvbmmsvpAG721bKi0joRfFdHIWJ4=
github.com/aquasecurity/defsec v0.93.2-0.20231020041402-7ccc46780c09 h1:dYBDwBnNzDsJr6l+FkrkrvWysAKc6VAO/leOcjvJfaA=
github.com/aquasecurity/defsec v0.93.2-0.20231020041402-7ccc46780c09/go.mod h1:J30VViSgmoW2Ic/6aqVJO2qvuADsmZ3MYuNxPcU6Vt0=
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio=
github.com/bwesterb/go-ristretto v1.2.3/go.mod h1:fUIoIZaG73pV5biE2Blr2xEzDoMj7NFEuV9ekS419A0=
github.com/cenkalti/backoff/v4 v4.2.1 h1:y4OZtCnogmCPw98Zjyt5a6+QwPLGkiQsYW5oUqylYbM=
Expand Down Expand Up @@ -130,6 +128,8 @@ github.com/morikuni/aec v1.0.0 h1:nP9CBfwrvYnBRgY6qfDQkygYDmYwOilePFkwzv4dU8A=
github.com/morikuni/aec v1.0.0/go.mod h1:BbKIizmSmc5MMPqRYbxO4ZU0S0+P200+tUnFx7PXmsc=
github.com/mrunalp/fileutils v0.5.0/go.mod h1:M1WthSahJixYnrXQl/DFQuteStB1weuxD2QJNHXfbSQ=
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
github.com/nikpivkin/defsec v0.0.0-20231030110125-d441bcbece99 h1:eRvQrLFXhbDehGT4LGJKbiXurR6nau+sC3oIQSXz1X0=
github.com/nikpivkin/defsec v0.0.0-20231030110125-d441bcbece99/go.mod h1:LlOXGf1igy/PsiyHMwkPmIkmwMnZJXUkWkpuyaXFTLM=
github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U=
github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
github.com/opencontainers/image-spec v1.1.0-rc4 h1:oOxKUJWnFC4YGHCCMNql1x4YaDfYBTS5Y4x/Cgeo1E0=
Expand Down
127 changes: 10 additions & 117 deletions rules/cloud/policies/aws/iam/no_policy_wildcards.go
Original file line number Diff line number Diff line change
@@ -1,29 +1,14 @@
package iam

import (
"fmt"
"regexp"
"strings"

"github.com/aquasecurity/defsec/pkg/framework"

"github.com/aquasecurity/defsec/pkg/providers/aws/iam"
"github.com/aquasecurity/defsec/pkg/providers"
"github.com/aquasecurity/defsec/pkg/scan"
"github.com/aquasecurity/defsec/pkg/severity"

"github.com/aquasecurity/defsec/pkg/state"

"github.com/aquasecurity/defsec/pkg/scan"

"github.com/aquasecurity/trivy-policies/pkg/rules"

"github.com/aquasecurity/defsec/pkg/providers"

"github.com/liamg/iamgo"
)

var (
//arn:aws:logs:us-west-2:123456789012:log-group:SampleLogGroupName:*
cloudwatchLogStreamResourceRegex = regexp.MustCompile(`^arn:aws:logs:.*:.+:log-group:.+:\*`)
"github.com/aquasecurity/trivy-policies/rules/cloud/policies/aws"
)

var CheckNoPolicyWildcards = rules.Register(
Expand Down Expand Up @@ -57,111 +42,19 @@ var CheckNoPolicyWildcards = rules.Register(
},
Severity: severity.High,
},
func(s *state.State) (results scan.Results) {
for _, policy := range s.AWS.IAM.Policies {
if policy.Builtin.IsTrue() {
continue
}
results = checkPolicy(policy.Document, results)
}
func(s *state.State) scan.Results {
checker := aws.PolicyChecker{}

results := checker.CheckWildcards(s.AWS.IAM.Policies)
for _, group := range s.AWS.IAM.Groups {
for _, policy := range group.Policies {
if policy.Builtin.IsTrue() {
continue
}
results = checkPolicy(policy.Document, results)
}
results = append(results, checker.CheckWildcards(group.Policies)...)
}
for _, user := range s.AWS.IAM.Users {
for _, policy := range user.Policies {
if policy.Builtin.IsTrue() {
continue
}
results = checkPolicy(policy.Document, results)
}
results = append(results, checker.CheckWildcards(user.Policies)...)
}
for _, role := range s.AWS.IAM.Roles {
for _, policy := range role.Policies {
if policy.Builtin.IsTrue() {
continue
}
results = checkPolicy(policy.Document, results)
}
results = append(results, checker.CheckWildcards(role.Policies)...)
}
return results
},
)

func checkPolicy(src iam.Document, results scan.Results) scan.Results {
statements, _ := src.Parsed.Statements()
for _, statement := range statements {
results = checkStatement(src, statement, results)
}
return results
}

// nolint
func checkStatement(src iam.Document, statement iamgo.Statement, results scan.Results) scan.Results {
effect, _ := statement.Effect()
if effect != iamgo.EffectAllow {
return results
}

actions, r := statement.Actions()
for _, action := range actions {
if strings.Contains(action, "*") {
results.Add(
fmt.Sprintf(
"IAM policy document uses wildcarded action '%s'",
actions[0],
),
src.MetadataFromIamGo(statement.Range(), r),
)
} else {
results.AddPassed(src)
}
}

resources, r := statement.Resources()
for _, resource := range resources {
if strings.Contains(resource, "*") {
if allowed, action := iam.IsWildcardAllowed(actions...); !allowed {
if strings.HasSuffix(resource, "/*") && strings.HasPrefix(resource, "arn:aws:s3") {
continue
}
if cloudwatchLogStreamResourceRegex.MatchString(resource) {
continue
}

results.Add(
fmt.Sprintf("IAM policy document uses sensitive action '%s' on wildcarded resource '%s'", action, resources[0]),
src.MetadataFromIamGo(statement.Range(), r),
)
} else {
results.AddPassed(src)
}
} else {
results.AddPassed(src)
}
}
principals, _ := statement.Principals()
if all, r := principals.All(); all {
results.Add(
"IAM policy document uses wildcarded principal.",
src.MetadataFromIamGo(statement.Range(), r),
)
}
aws, r := principals.AWS()
for _, principal := range aws {
if strings.Contains(principal, "*") {
results.Add(
"IAM policy document uses wildcarded principal.",
src.MetadataFromIamGo(statement.Range(), r),
)
} else {
results.AddPassed(src)
}
}

return results
}
8 changes: 2 additions & 6 deletions rules/cloud/policies/aws/iam/no_policy_wildcards_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,11 @@ package iam
import (
"testing"

defsecTypes "github.com/aquasecurity/defsec/pkg/types"

"github.com/aquasecurity/defsec/pkg/state"

"github.com/aquasecurity/defsec/pkg/providers/aws/iam"
"github.com/aquasecurity/defsec/pkg/scan"

"github.com/aquasecurity/defsec/pkg/state"
defsecTypes "github.com/aquasecurity/defsec/pkg/types"
"github.com/liamg/iamgo"

"github.com/stretchr/testify/assert"
)

Expand Down
99 changes: 99 additions & 0 deletions rules/cloud/policies/aws/policy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
package aws

import (
"fmt"
"regexp"
"strings"

"github.com/aquasecurity/defsec/pkg/providers/aws/iam"
"github.com/aquasecurity/defsec/pkg/scan"
"github.com/liamg/iamgo"
)

var (
// The wildcard character (*) at the end of the Resource value means that the statement allows permission
// for the logs:CreateLogGroup, logs:CreateLogStream, logs:PutLogEvents, and logs:DescribeLogStreams
// actions on any log group
//arn:aws:logs:us-west-2:123456789012:log-group:SampleLogGroupName:*
cloudwatchLogStreamResourceRegex = regexp.MustCompile(`^arn:aws:logs:.*:.+:log-group:.+:\*`)
)

type PolicyChecker struct {
results scan.Results
}

func (c *PolicyChecker) CheckWildcards(policies []iam.Policy) scan.Results {
for _, policy := range policies {
if policy.Builtin.IsTrue() {
continue
}
statements, _ := policy.Document.Parsed.Statements()
for _, statement := range statements {
c.checkStatement(policy.Document, statement)
}
}

return c.results
}

func (c *PolicyChecker) checkStatement(src iam.Document, statement iamgo.Statement) {
effect, _ := statement.Effect()
if effect != iamgo.EffectAllow {
return
}

actions, r := statement.Actions()
for _, action := range actions {
if strings.Contains(action, "*") {
c.results.Add(
fmt.Sprintf("Policy document uses wildcarded action %v", actions),
src.MetadataFromIamGo(statement.Range(), r),
)
} else {
c.results.AddPassed(src)
}
}

resources, r := statement.Resources()
for _, resource := range resources {
if resource == "*" {
if actions, allowed := iam.IsWildcardAllowed(actions...); !allowed {
c.results.Add(
fmt.Sprintf("Policy document uses sensitive actions %v on wildcarded resource %q", actions, resource),
src.MetadataFromIamGo(statement.Range(), r),
)
} else {
c.results.AddPassed(src)
}
} else if strings.Contains(resource, "*") &&
// allow all objects in the bucket to be specified
!(strings.HasSuffix(resource, "/*") && strings.HasPrefix(resource, "arn:aws:s3")) &&
!(cloudwatchLogStreamResourceRegex.MatchString(resource)) {
c.results.Add(
fmt.Sprintf("Policy document uses sensitive actions %v on wildcarded resource %q", actions, resource),
src.MetadataFromIamGo(statement.Range(), r),
)

} else {
c.results.AddPassed(src)
}
}
principals, _ := statement.Principals()
if all, r := principals.All(); all {
c.results.Add(
"Policy document uses wildcarded principal.",
src.MetadataFromIamGo(statement.Range(), r),
)
}
aws, r := principals.AWS()
for _, principal := range aws {
if strings.Contains(principal, "*") {
c.results.Add(
"Policy document uses wildcarded principal.",
src.MetadataFromIamGo(statement.Range(), r),
)
} else {
c.results.AddPassed(src)
}
}
}
Loading

0 comments on commit 5cc23d9

Please sign in to comment.