aws-solutions / aws-waf-security-automations

This solution automatically deploys a single web access control list (web ACL) with a set of AWS WAF rules designed to filter common web-based attacks.
https://aws.amazon.com/solutions/aws-waf-security-automations
Apache License 2.0
845 stars 361 forks source link

Wrong s3 path location for CloudFront AWS::Glue::Table #81

Closed jecnua closed 1 year ago

jecnua commented 5 years ago

In the firehose stack, CloudFrontGlueAppAccessLogsTable uses as location:

"s3://${AppAccessLogBucket}/AWSLogs/"

However CloudFront does not use the same convention as WAF and LB. Link to code in master:

https://github.com/awslabs/aws-waf-security-automations/blob/master/deployment/aws-waf-security-automations-firehose-athena.template#L330

I needed to change the template to:

"s3://${AppAccessLogBucket}/"

And after this change the ScannersProbesProtection Athena query started to give result.

I would advice to remove 'AWSLogs/' and optionally take a parametric prefix.

hvital commented 5 years ago

Hi @jecnua

We added this prefix to avoid mixing CloudFront access log files and Athena result files in the same context.

Is this not working even when you configure AWSLogs/ as prefix for your CloudFront distribution logs configuration?

jecnua commented 5 years ago

Hi @hvital,

It may work if I set the prefix, however I am working on adding WAF on "a lot" of pre-existing CloudFront distribution so I don't have the luxury of changing the log prefix setting. I do believe that allowing a configurable/parametric "prefix" will work better for pre-existing distributions that have different patterns in place. For example, the prefix can be by default 'AWSLogs/' and so being retro-compatible with v2.3 while at the same time allowing this template to be used in more (legacy or just different) environments.

To solve the Athena result issue, I would advice on using another bucket ;) I am thinking on changing that too to avoid polluting my CloudFront distribution logs bucket with unrelated files (they are not CF logs after all). Imagine if you send that data to other buckets/team for analytics they don't need/want such info.

aijunpeng commented 1 year ago

We are adding user-defined s3 prefix in the upcoming release. closed this old ticket.