-
Notifications
You must be signed in to change notification settings - Fork 551
feat(misconf): Adds CloudFront standard logging v2 support to AVD-AWS-0010 #10848
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
Changes from 7 commits
4431e31
5469868
501e63c
ddf6ca5
2b97e16
6a4b3d5
fd3aa58
c64562c
f3cba5a
3b2afa1
2e07db1
f960b34
b3982d5
2946f5b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,6 +58,82 @@ Resources: | |
| Distributions: []cloudfront.Distribution{{}}, | ||
| }, | ||
| }, | ||
|
|
||
| { | ||
| name: "v2 logging configured", | ||
| source: `AWSTemplateFormatVersion: 2010-09-09 | ||
| Resources: | ||
| CloudFrontDistribution: | ||
| Type: AWS::CloudFront::Distribution | ||
| Properties: | ||
| DistributionConfig: | ||
| DefaultCacheBehavior: | ||
| ViewerProtocolPolicy: "redirect-to-https" | ||
|
|
||
| CloudFrontAccessLogsDeliverySource: | ||
| Type: AWS::Logs::DeliverySource | ||
| Properties: | ||
| LogType: ACCESS_LOGS | ||
| Name: cloudfront-log-delivery-source | ||
| ResourceArn: !Sub | ||
| - arn:aws:cloudfront::${AWS::AccountId}:distribution/${D} | ||
| - D: !GetAtt CloudFrontDistribution.Id | ||
|
|
||
| CloudFrontAccessLogsDelivery: | ||
| Type: AWS::Logs::Delivery | ||
| Properties: | ||
| DeliverySourceName: cloudfront-log-delivery-source | ||
| `, | ||
| expected: cloudfront.Cloudfront{ | ||
| Distributions: []cloudfront.Distribution{ | ||
| { | ||
| Logging: cloudfront.Logging{ | ||
| V2: cloudfront.LoggingV2{ | ||
| Enabled: types.BoolTest(true), | ||
| }, | ||
| }, | ||
| DefaultCacheBehaviour: cloudfront.CacheBehaviour{ | ||
| ViewerProtocolPolicy: types.StringTest("redirect-to-https"), | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| name: "v2 logging source exists but no delivery", | ||
| source: `AWSTemplateFormatVersion: 2010-09-09 | ||
| Resources: | ||
| CloudFrontDistribution: | ||
| Type: AWS::CloudFront::Distribution | ||
| Properties: | ||
| DistributionConfig: | ||
| DefaultCacheBehavior: | ||
| ViewerProtocolPolicy: "redirect-to-https" | ||
|
|
||
| CloudFrontAccessLogsDeliverySource: | ||
| Type: AWS::Logs::DeliverySource | ||
| Properties: | ||
| LogType: ACCESS_LOGS | ||
| Name: cloudfront-log-delivery-source | ||
| ResourceArn: !Sub | ||
| - arn:aws:cloudfront::${AWS::AccountId}:distribution/${D} | ||
| - D: !GetAtt CloudFrontDistribution.Id | ||
| `, | ||
| expected: cloudfront.Cloudfront{ | ||
| Distributions: []cloudfront.Distribution{ | ||
| { | ||
| Logging: cloudfront.Logging{ | ||
| V2: cloudfront.LoggingV2{ | ||
| Enabled: types.BoolTest(false), | ||
| }, | ||
| }, | ||
| DefaultCacheBehaviour: cloudfront.CacheBehaviour{ | ||
| ViewerProtocolPolicy: types.StringTest("redirect-to-https"), | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two cases missing here:
|
||
| } | ||
|
|
||
| for _, tt := range tests { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,18 @@ | ||
| package cloudfront | ||
|
|
||
| import ( | ||
| "strings" | ||
|
|
||
| "github.com/aquasecurity/trivy/pkg/iac/providers/aws/cloudfront" | ||
| "github.com/aquasecurity/trivy/pkg/iac/scanners/cloudformation/parser" | ||
| iacTypes "github.com/aquasecurity/trivy/pkg/iac/types" | ||
| ) | ||
|
|
||
| func getDistributions(ctx parser.FileContext) (distributions []cloudfront.Distribution) { | ||
|
|
||
| distributionResources := ctx.GetResourcesByType("AWS::CloudFront::Distribution") | ||
| deliverySources := ctx.GetResourcesByType("AWS::Logs::DeliverySource") | ||
| deliveries := ctx.GetResourcesByType("AWS::Logs::Delivery") | ||
|
|
||
| for _, r := range distributionResources { | ||
| distribution := cloudfront.Distribution{ | ||
|
|
@@ -16,6 +21,10 @@ func getDistributions(ctx parser.FileContext) (distributions []cloudfront.Distri | |
| Logging: cloudfront.Logging{ | ||
| Metadata: r.Metadata(), | ||
| Bucket: r.GetStringProperty("DistributionConfig.Logging.Bucket"), | ||
| V2: cloudfront.LoggingV2{ | ||
| Metadata: r.Metadata(), | ||
| Enabled: hasV2Logging(r, deliverySources, deliveries), | ||
| }, | ||
| }, | ||
| DefaultCacheBehaviour: getDefaultCacheBehaviour(r), | ||
| OrdererCacheBehaviours: nil, | ||
|
|
@@ -43,3 +52,27 @@ func getDefaultCacheBehaviour(r *parser.Resource) cloudfront.CacheBehaviour { | |
| ViewerProtocolPolicy: defaultCache.GetStringProperty("ViewerProtocolPolicy"), | ||
| } | ||
| } | ||
|
|
||
| func hasV2Logging(distribution *parser.Resource, deliverySources, deliveries []*parser.Resource) iacTypes.BoolValue { | ||
|
|
||
| for _, source := range deliverySources { | ||
| logType := source.GetStringProperty("LogType") | ||
| if logType.Value() != "ACCESS_LOGS" { | ||
|
Aakarsh133 marked this conversation as resolved.
Outdated
|
||
| continue | ||
| } | ||
| resourceArn := source.GetStringProperty("ResourceArn") | ||
| if !strings.Contains(resourceArn.Value(), distribution.ID()) { | ||
| continue | ||
| } | ||
| sourceName := source.GetStringProperty("Name") | ||
|
|
||
| for _, delivery := range deliveries { | ||
| deliverySourceName := delivery.GetStringProperty("DeliverySourceName") | ||
| if deliverySourceName.Value() == sourceName.Value() || deliverySourceName.Value() == source.ID() { | ||
| return iacTypes.Bool(true, distribution.Metadata()) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The metadata for |
||
| } | ||
| } | ||
|
|
||
| } | ||
| return iacTypes.Bool(false, distribution.Metadata()) | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,7 +16,14 @@ func adaptDistributions(modules terraform.Modules) []cloudfront.Distribution { | |||||||
| var distributions []cloudfront.Distribution | ||||||||
| for _, module := range modules { | ||||||||
| for _, resource := range module.GetResourcesByType("aws_cloudfront_distribution") { | ||||||||
| distributions = append(distributions, adaptDistribution(resource)) | ||||||||
| distribution := adaptDistribution(resource) | ||||||||
|
|
||||||||
| distribution.Logging.V2 = cloudfront.LoggingV2{ | ||||||||
| Metadata: resource.GetMetadata(), | ||||||||
| Enabled: hasV2Logging(modules, resource), | ||||||||
| } | ||||||||
|
|
||||||||
| distributions = append(distributions, distribution) | ||||||||
| } | ||||||||
| } | ||||||||
| return distributions | ||||||||
|
|
@@ -77,3 +84,23 @@ func adaptDistribution(resource *terraform.Block) cloudfront.Distribution { | |||||||
|
|
||||||||
| return distribution | ||||||||
| } | ||||||||
|
|
||||||||
| func hasV2Logging(modules terraform.Modules, distributionBlock *terraform.Block) types.BoolValue { | ||||||||
| metadata := distributionBlock.GetMetadata() | ||||||||
|
|
||||||||
| sources := modules.GetReferencingResources(distributionBlock, "aws_cloudwatch_log_delivery_source", "resource_arn") | ||||||||
| for _, source := range sources { | ||||||||
| logType := source.GetAttribute("log_type").AsStringValueOrDefault("", source) | ||||||||
| if logType.Value() != "ACCESS_LOGS" { | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be simplified to:
Suggested change
|
||||||||
| continue | ||||||||
| } | ||||||||
|
|
||||||||
| deliveries := modules.GetReferencingResources(source, "aws_cloudwatch_log_delivery", "delivery_source_name") | ||||||||
| if len(deliveries) > 0 { | ||||||||
| return types.Bool(true, metadata) | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just as in CF, it's best to return the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Until now it was under assumption that merely an existence of a
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that's fine, since there's supposed to be at least one delivery resource here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also just a note for both CF and Terraform in case of false, distribution metadata is returned |
||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| return types.Bool(false, metadata) | ||||||||
|
|
||||||||
| } | ||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -89,6 +89,72 @@ func Test_adaptDistribution(t *testing.T) { | |
| } | ||
| } | ||
|
|
||
| func Test_adaptDistributionV2(t *testing.T) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a negative case where the delivery source references the distribution and a delivery exists, but |
||
| tests := []struct { | ||
| name string | ||
| terraform string | ||
| expected cloudfront.Distribution | ||
| }{ | ||
| { | ||
| name: "v2 logging configured", | ||
| terraform: ` | ||
| resource "aws_cloudfront_distribution" "example" {} | ||
|
|
||
| resource "aws_cloudwatch_log_delivery_source" "example" { | ||
| log_type = "ACCESS_LOGS" | ||
| resource_arn = aws_cloudfront_distribution.example.arn | ||
| } | ||
|
|
||
| resource "aws_cloudwatch_log_delivery" "example" { | ||
| delivery_source_name = aws_cloudwatch_log_delivery_source.example.name | ||
| } | ||
| `, | ||
| expected: cloudfront.Distribution{ | ||
| Logging: cloudfront.Logging{ | ||
| V2: cloudfront.LoggingV2{ | ||
| Enabled: iacTypes.BoolTest(true), | ||
| }, | ||
| }, | ||
| DefaultCacheBehaviour: cloudfront.CacheBehaviour{}, | ||
| ViewerCertificate: cloudfront.ViewerCertificate{ | ||
| MinimumProtocolVersion: iacTypes.StringTest("TLSv1"), | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| name: "v2 logging source exists but no delivery", | ||
| terraform: ` | ||
| resource "aws_cloudfront_distribution" "example" {} | ||
|
|
||
| resource "aws_cloudwatch_log_delivery_source" "example" { | ||
| log_type = "ACCESS_LOGS" | ||
| resource_arn = aws_cloudfront_distribution.example.arn | ||
| } | ||
| `, | ||
| expected: cloudfront.Distribution{ | ||
| Logging: cloudfront.Logging{ | ||
| V2: cloudfront.LoggingV2{ | ||
| Enabled: iacTypes.BoolTest(false), | ||
| }, | ||
| }, | ||
| DefaultCacheBehaviour: cloudfront.CacheBehaviour{}, | ||
| ViewerCertificate: cloudfront.ViewerCertificate{ | ||
| MinimumProtocolVersion: iacTypes.StringTest("TLSv1"), | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| for _, test := range tests { | ||
| t.Run(test.name, func(t *testing.T) { | ||
| modules := tftestutil.CreateModulesFromSource(t, test.terraform, ".tf") | ||
| adapted := Adapt(modules) | ||
| require.Len(t, adapted.Distributions, 1) | ||
| testutil.AssertDefsecEqual(t, test.expected, adapted.Distributions[0]) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestLines(t *testing.T) { | ||
| src := ` | ||
| resource "aws_cloudfront_distribution" "example" { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -944,6 +944,23 @@ | |
| "bucket": { | ||
| "type": "object", | ||
| "$ref": "#/definitions/github.qkg1.top.aquasecurity.trivy.pkg.iac.types.StringValue" | ||
| }, | ||
| "v2": { | ||
| "type": "object", | ||
| "$ref": "#/definitions/github.qkg1.top.aquasecurity.trivy.pkg.iac.providers.aws.cloudfront.LoggingV2" | ||
| } | ||
| } | ||
| }, | ||
| "github.qkg1.top.aquasecurity.trivy.pkg.iac.providers.aws.cloudfront.LoggingV2": { | ||
| "type": "object", | ||
| "properties": { | ||
| "__defsec_metadata": { | ||
| "type": "object", | ||
| "$ref": "#/definitions/github.qkg1.top.aquasecurity.trivy.pkg.iac.types.Metadata" | ||
| }, | ||
| "enabled": { | ||
| "type": "object", | ||
| "$ref": "#/definitions/github.qkg1.top.aquasecurity.trivy.pkg.iac.types.BoolValue" | ||
| } | ||
| } | ||
| }, | ||
|
|
@@ -4566,7 +4583,7 @@ | |
| "type": "object", | ||
| "$ref": "#/definitions/github.qkg1.top.aquasecurity.trivy.pkg.iac.providers.azure.appservice.Identity" | ||
| }, | ||
| "platform": { | ||
| "resource": { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just flagging that this schema diff in |
||
| "type": "object", | ||
| "$ref": "#/definitions/github.qkg1.top.aquasecurity.trivy.pkg.iac.types.StringValue" | ||
| }, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.