Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(aws): CloudTrail global service events rule #1401

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion avd_docs/aws/cloudtrail/AVD-AWS-0014/CloudFormation.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Enable Cloudtrail in all regions

```yaml---
Resources:
BadExample:
GoodExample:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why this has changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should say GoodExample, after updating it on the policy it still needs to be updated on the auto generated docs. Updated from this policy as part of this PR: https://github.com/aquasecurity/defsec/blob/master/rules/cloud/policies/aws/cloudtrail/enable_all_regions.cf.go

Type: AWS::CloudTrail::Trail
Properties:
IsLogging: true
Expand Down
2 changes: 1 addition & 1 deletion avd_docs/aws/cloudtrail/AVD-AWS-0015/CloudFormation.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Enable encryption at rest

```yaml---
Resources:
BadExample:
GoodExample:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to my previous comment, the policy script has been updated as part of this PR and this should say GoodExample:
https://github.com/aquasecurity/defsec/blob/master/rules/cloud/policies/aws/cloudtrail/enable_at_rest_encryption.cf.go

Type: AWS::CloudTrail::Trail
Properties:
IsLogging: true
Expand Down
2 changes: 1 addition & 1 deletion avd_docs/aws/cloudtrail/AVD-AWS-0016/CloudFormation.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Turn on log validation for Cloudtrail

```yaml---
Resources:
BadExample:
GoodExample:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to my previous comment, the policy script has been updated as part of this PR and this should say GoodExample:
https://github.com/aquasecurity/defsec/blob/master/rules/cloud/policies/aws/cloudtrail/enable_log_validation.cf.go

Type: AWS::CloudTrail::Trail
Properties:
IsLogging: true
Expand Down
15 changes: 15 additions & 0 deletions avd_docs/aws/cloudtrail/AVD-AWS-0345/CloudFormation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@

Enable include global service events for Cloudtrail

```yaml---
Resources:
GoodExampleTrail:
Type: AWS::CloudTrail::Trail
Properties:
IncludeGlobalServiceEvents: true
S3BucketName: "my-bucket"
TrailName: "Cloudtrail"

```


14 changes: 14 additions & 0 deletions avd_docs/aws/cloudtrail/AVD-AWS-0345/Terraform.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@

Enable include global service events for Cloudtrail

```hcl
resource "aws_cloudtrail" "good_example" {
include_global_service_events = true
s3_bucket_name = "abcdefgh"
}

```

#### Remediation Links
- https://registry.terraform.io/providers/rgeraskin/aws2/latest/docs/resources/cloudtrail#include_global_service_events

13 changes: 13 additions & 0 deletions avd_docs/aws/cloudtrail/AVD-AWS-0345/docs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

Include Global Service Events is a default value for Cloudtrail and it publishes events from global services that are not region specific such as IAM, STS and CloudFront. It is feasible that a rogue actor compromising an AWS account might want to disable this field to remove trace of their actions.

### Impact
Events from global services such as IAM are not being published to the log files

<!-- DO NOT CHANGE -->
{{ remediationActions }}

### Links
- https://docs.aws.amazon.com/awscloudtrail/latest/userguide/cloudtrail-concepts.html#cloudtrail-concepts-global-service-events


19 changes: 10 additions & 9 deletions internal/adapters/cloud/aws/cloudtrail/adapt.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,15 @@ func (a *adapter) adaptTrail(info types.TrailInfo) (*cloudtrail.Trail, error) {
}

return &cloudtrail.Trail{
Metadata: metadata,
Name: name,
EnableLogFileValidation: defsecTypes.Bool(response.Trail.LogFileValidationEnabled != nil && *response.Trail.LogFileValidationEnabled, metadata),
IsMultiRegion: defsecTypes.Bool(response.Trail.IsMultiRegionTrail != nil && *response.Trail.IsMultiRegionTrail, metadata),
CloudWatchLogsLogGroupArn: cloudWatchLogsArn,
KMSKeyID: defsecTypes.String(kmsKeyId, metadata),
IsLogging: isLogging,
BucketName: defsecTypes.String(bucketName, metadata),
EventSelectors: eventSelectors,
Metadata: metadata,
Name: name,
EnableLogFileValidation: defsecTypes.Bool(response.Trail.LogFileValidationEnabled != nil && *response.Trail.LogFileValidationEnabled, metadata),
IsMultiRegion: defsecTypes.Bool(response.Trail.IsMultiRegionTrail != nil && *response.Trail.IsMultiRegionTrail, metadata),
CloudWatchLogsLogGroupArn: cloudWatchLogsArn,
KMSKeyID: defsecTypes.String(kmsKeyId, metadata),
IsLogging: isLogging,
IncludeGlobalServiceEvents: defsecTypes.Bool(response.Trail.IncludeGlobalServiceEvents != nil && *response.Trail.IncludeGlobalServiceEvents, metadata),
BucketName: defsecTypes.String(bucketName, metadata),
EventSelectors: eventSelectors,
}, nil
}
17 changes: 9 additions & 8 deletions internal/adapters/cloudformation/aws/cloudtrail/trails.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@ func getCloudTrails(ctx parser.FileContext) (trails []cloudtrail.Trail) {

for _, r := range cloudtrailResources {
ct := cloudtrail.Trail{
Metadata: r.Metadata(),
Name: r.GetStringProperty("TrailName"),
EnableLogFileValidation: r.GetBoolProperty("EnableLogFileValidation"),
IsMultiRegion: r.GetBoolProperty("IsMultiRegionTrail"),
KMSKeyID: r.GetStringProperty("KmsKeyId"),
CloudWatchLogsLogGroupArn: r.GetStringProperty("CloudWatchLogsLogGroupArn"),
IsLogging: r.GetBoolProperty("IsLogging"),
BucketName: r.GetStringProperty("S3BucketName"),
Metadata: r.Metadata(),
Name: r.GetStringProperty("TrailName"),
EnableLogFileValidation: r.GetBoolProperty("EnableLogFileValidation"),
IsMultiRegion: r.GetBoolProperty("IsMultiRegionTrail"),
KMSKeyID: r.GetStringProperty("KmsKeyId"),
CloudWatchLogsLogGroupArn: r.GetStringProperty("CloudWatchLogsLogGroupArn"),
IsLogging: r.GetBoolProperty("IsLogging"),
IncludeGlobalServiceEvents: r.GetBoolProperty("IncludeGlobalServiceEvents"),
BucketName: r.GetStringProperty("S3BucketName"),
}

trails = append(trails, ct)
Expand Down
19 changes: 10 additions & 9 deletions internal/adapters/terraform/aws/cloudtrail/adapt.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,15 @@ func adaptTrail(resource *terraform.Block) cloudtrail.Trail {
}

return cloudtrail.Trail{
Metadata: resource.GetMetadata(),
Name: nameVal,
EnableLogFileValidation: enableLogFileValidationVal,
IsMultiRegion: isMultiRegionVal,
KMSKeyID: KMSKeyIDVal,
CloudWatchLogsLogGroupArn: resource.GetAttribute("cloud_watch_logs_group_arn").AsStringValueOrDefault("", resource),
IsLogging: resource.GetAttribute("enable_logging").AsBoolValueOrDefault(true, resource),
BucketName: resource.GetAttribute("s3_bucket_name").AsStringValueOrDefault("", resource),
EventSelectors: selectors,
Metadata: resource.GetMetadata(),
Name: nameVal,
EnableLogFileValidation: enableLogFileValidationVal,
IsMultiRegion: isMultiRegionVal,
KMSKeyID: KMSKeyIDVal,
CloudWatchLogsLogGroupArn: resource.GetAttribute("cloud_watch_logs_group_arn").AsStringValueOrDefault("", resource),
IsLogging: resource.GetAttribute("enable_logging").AsBoolValueOrDefault(true, resource),
IncludeGlobalServiceEvents: resource.GetAttribute("include_global_service_events").AsBoolValueOrDefault(true, resource),
BucketName: resource.GetAttribute("s3_bucket_name").AsStringValueOrDefault("", resource),
EventSelectors: selectors,
}
}
35 changes: 19 additions & 16 deletions internal/adapters/terraform/aws/cloudtrail/adapt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,19 @@ func Test_adaptTrail(t *testing.T) {
s3_bucket_name = "abcdefgh"
cloud_watch_logs_group_arn = "abc"
enable_logging = false
include_global_service_events = false
}
`,
expected: cloudtrail.Trail{
Metadata: defsecTypes.NewTestMetadata(),
Name: defsecTypes.String("example", defsecTypes.NewTestMetadata()),
EnableLogFileValidation: defsecTypes.Bool(true, defsecTypes.NewTestMetadata()),
IsMultiRegion: defsecTypes.Bool(true, defsecTypes.NewTestMetadata()),
KMSKeyID: defsecTypes.String("kms-key", defsecTypes.NewTestMetadata()),
CloudWatchLogsLogGroupArn: defsecTypes.String("abc", defsecTypes.NewTestMetadata()),
IsLogging: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
BucketName: defsecTypes.String("abcdefgh", defsecTypes.NewTestMetadata()),
Metadata: defsecTypes.NewTestMetadata(),
Name: defsecTypes.String("example", defsecTypes.NewTestMetadata()),
EnableLogFileValidation: defsecTypes.Bool(true, defsecTypes.NewTestMetadata()),
IsMultiRegion: defsecTypes.Bool(true, defsecTypes.NewTestMetadata()),
KMSKeyID: defsecTypes.String("kms-key", defsecTypes.NewTestMetadata()),
CloudWatchLogsLogGroupArn: defsecTypes.String("abc", defsecTypes.NewTestMetadata()),
IsLogging: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
IncludeGlobalServiceEvents: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
BucketName: defsecTypes.String("abcdefgh", defsecTypes.NewTestMetadata()),
},
},
{
Expand All @@ -52,14 +54,15 @@ func Test_adaptTrail(t *testing.T) {
}
`,
expected: cloudtrail.Trail{
Metadata: defsecTypes.NewTestMetadata(),
Name: defsecTypes.String("", defsecTypes.NewTestMetadata()),
EnableLogFileValidation: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
IsMultiRegion: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
KMSKeyID: defsecTypes.String("", defsecTypes.NewTestMetadata()),
BucketName: defsecTypes.String("", defsecTypes.NewTestMetadata()),
CloudWatchLogsLogGroupArn: defsecTypes.String("", defsecTypes.NewTestMetadata()),
IsLogging: defsecTypes.Bool(true, defsecTypes.NewTestMetadata()),
Metadata: defsecTypes.NewTestMetadata(),
Name: defsecTypes.String("", defsecTypes.NewTestMetadata()),
EnableLogFileValidation: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
IsMultiRegion: defsecTypes.Bool(false, defsecTypes.NewTestMetadata()),
KMSKeyID: defsecTypes.String("", defsecTypes.NewTestMetadata()),
BucketName: defsecTypes.String("", defsecTypes.NewTestMetadata()),
CloudWatchLogsLogGroupArn: defsecTypes.String("", defsecTypes.NewTestMetadata()),
IsLogging: defsecTypes.Bool(true, defsecTypes.NewTestMetadata()),
IncludeGlobalServiceEvents: defsecTypes.Bool(true, defsecTypes.NewTestMetadata()),
},
},
}
Expand Down
19 changes: 10 additions & 9 deletions pkg/providers/aws/cloudtrail/cloudtrail.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,16 @@ func (c CloudTrail) MultiRegionTrails() (multiRegionTrails []Trail) {
}

type Trail struct {
Metadata defsecTypes.Metadata
Name defsecTypes.StringValue
EnableLogFileValidation defsecTypes.BoolValue
IsMultiRegion defsecTypes.BoolValue
KMSKeyID defsecTypes.StringValue
CloudWatchLogsLogGroupArn defsecTypes.StringValue
IsLogging defsecTypes.BoolValue
BucketName defsecTypes.StringValue
EventSelectors []EventSelector
Metadata defsecTypes.Metadata
Name defsecTypes.StringValue
EnableLogFileValidation defsecTypes.BoolValue
IsMultiRegion defsecTypes.BoolValue
KMSKeyID defsecTypes.StringValue
CloudWatchLogsLogGroupArn defsecTypes.StringValue
IsLogging defsecTypes.BoolValue
IncludeGlobalServiceEvents defsecTypes.BoolValue
BucketName defsecTypes.StringValue
EventSelectors []EventSelector
}

type EventSelector struct {
Expand Down
4 changes: 4 additions & 0 deletions pkg/rego/schemas/cloud.json
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,10 @@
"$ref": "#/definitions/git.luolix.top.aquasecurity.defsec.pkg.providers.aws.cloudtrail.EventSelector"
}
},
"includeglobalserviceevents": {
"type": "object",
"$ref": "#/definitions/git.luolix.top.aquasecurity.defsec.pkg.types.BoolValue"
},
"islogging": {
"type": "object",
"$ref": "#/definitions/git.luolix.top.aquasecurity.defsec.pkg.types.BoolValue"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package cloudtrail
var cloudFormationEnableAllRegionsGoodExamples = []string{
`---
Resources:
BadExample:
GoodExample:
Type: AWS::CloudTrail::Trail
Properties:
IsLogging: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package cloudtrail
var cloudFormationEnableAtRestEncryptionGoodExamples = []string{
`---
Resources:
BadExample:
GoodExample:
Type: AWS::CloudTrail::Trail
Properties:
IsLogging: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package cloudtrail
var cloudFormationEnableLogValidationGoodExamples = []string{
`---
Resources:
BadExample:
GoodExample:
Type: AWS::CloudTrail::Trail
Properties:
IsLogging: true
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package cloudtrail

var cloudFormationIncludeGlobalServiceEventsGoodExamples = []string{
`---
Resources:
GoodExample:
Type: AWS::CloudTrail::Trail
Properties:
IncludeGlobalServiceEvents: true
S3BucketName: "my-bucket"
TrailName: "Cloudtrail"
`,
}

var cloudFormationIncludeGlobalServiceEventsBadExamples = []string{
`---
Resources:
BadExample:
Type: AWS::CloudTrail::Trail
Properties:
IncludeGlobalServiceEvents: false
S3BucketName: "my-bucket"
TrailName: "Cloudtrail"
`,
}

var cloudFormationIncludeGlobalServiceEventsLinks = []string{}

var cloudFormationIncludeGlobalServiceEventsRemediationMarkdown = ``
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package cloudtrail

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 checkIncludeGlobalServiceEvents = rules.Register(
scan.Rule{
AVDID: "AVD-AWS-0345",
Provider: providers.AWSProvider,
Service: "cloudtrail",
ShortCode: "include-global-service-events",
Summary: "Specifies whether Cloudtrail is publishing events from global services such as IAM to the log files. ",
Impact: "Events from global services such as IAM are not being published to the log files",
Resolution: "Enable include global service events for Cloudtrail",
Explanation: `Include Global Service Events is a default value for Cloudtrail and it publishes events from global services that are not region specific such as IAM, STS and CloudFront. It is feasible that a rogue actor compromising an AWS account might want to disable this field to remove trace of their actions.`,
Links: []string{
"https://docs.aws.amazon.com/awscloudtrail/latest/userguide/cloudtrail-concepts.html#cloudtrail-concepts-global-service-events",
},
Terraform: &scan.EngineMetadata{
GoodExamples: terraformIncludeGlobalServiceEventsGoodExamples,
BadExamples: terraformIncludeGlobalServiceEventsBadExamples,
Links: terraformIncludeGlobalServiceEventsLinks,
RemediationMarkdown: terraformIncludeGlobalServiceEventsRemediationMarkdown,
},
CloudFormation: &scan.EngineMetadata{
GoodExamples: cloudFormationIncludeGlobalServiceEventsGoodExamples,
BadExamples: cloudFormationIncludeGlobalServiceEventsBadExamples,
Links: cloudFormationIncludeGlobalServiceEventsLinks,
RemediationMarkdown: cloudFormationIncludeGlobalServiceEventsRemediationMarkdown,
},
Severity: severity.Medium,
},
func(s *state.State) (results scan.Results) {
for _, trail := range s.AWS.CloudTrail.Trails {
if trail.IncludeGlobalServiceEvents.IsFalse() {
results.Add(
"Trail is not publishing events from global services such as IAM to the log files.",
trail.IncludeGlobalServiceEvents,
)
} else {
results.AddPassed(&trail)
}
}
return
},
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package cloudtrail

var terraformIncludeGlobalServiceEventsGoodExamples = []string{
`
resource "aws_cloudtrail" "good_example" {
include_global_service_events = true
s3_bucket_name = "abcdefgh"
}
`,
}

var terraformIncludeGlobalServiceEventsBadExamples = []string{
`
resource "aws_cloudtrail" "bad_example" {
include_global_service_events = false
s3_bucket_name = "abcdefgh"
}
`,
}

var terraformIncludeGlobalServiceEventsLinks = []string{
`https://registry.terraform.io/providers/rgeraskin/aws2/latest/docs/resources/cloudtrail#include_global_service_events`,
}

var terraformIncludeGlobalServiceEventsRemediationMarkdown = ``
Loading