cerbo / aws-waf-security-automation

Amazon WAF Security Automation deployment (modular with Terraform)
Apache License 2.0
73 stars 27 forks source link

Update to apply latest changes from AWS #1

Closed jasonmcintosh closed 5 years ago

jasonmcintosh commented 6 years ago

Looks like a number of the lambda functions have changed (mostly seems some refactoring, but good to be current). Core pull requests of interest was the URI Whitelisting support

ventz commented 6 years ago

@jasonmcintosh Thanks for letting us know. Did you have a pull request (didn't see one), or are you asking for an update?

If asking for an update - we can take a look and merge them. It will take some time since it looks like while they have kept the script hooks the same, some of the stuff has changed quite a bit. I am sure it should work, but need to update and test it.

jasonmcintosh commented 6 years ago

Not had a chance to work on it YET will likely at some point if I get a spare few minutes late at night after the babies sleep. Thought I'd point it out first, see who works on it first ;) The URI whitelist support is kinda nifty though not an immediate need.

ventz commented 6 years ago

@jasonmcintosh Ok, v2 branch first release here: https://github.com/cerbo/aws-waf-security-automation/tree/v2

All of the new lambda functions are now incorporated and hooked in. It turned out that Amazon added some new variables/params without updating the documentation which is always awesome ;)

Test it out and let us know if it works. If there are no issues, we can merge it into the master branch.

jasonmcintosh commented 6 years ago

Nice fast work! Was going to schedule some work upcoming week on this. Guess instead I'll look at a pull request to get regional WAF support integrated so we can get this for both ALB's & CloudFront ;) Will dig in soon as I get a chance, do some testing & comparisons. I'll say the current CloudFormation stack has been driving me NUTS so BIG thanks for the work here! I've also got some changes to make this a full module, examples of how to use it in that case, including the option to create the bucket if need be. Should have a pull request later.

jasonmcintosh commented 6 years ago

Couple issues need to fix. Will have a pull request here later for it.

1) S3 bucket name uniqueness - really should use a random string (that's the way I did it) or similar to try and get a unique bucket name. Very easy otherwise to get duplicate buckets 2) Reference the bucket for lambdas by ARN/Name so if you DO change the pattern, you don't have a lot of places to update. 3) Use ${path.module}/ for file references so when used as a module it works. 4) Tied to #3, way I'm doing this is creating a main.tf from the template but changing default = cerbo to type = string for the variables. Let's this be used as a straight out module. This may not be something you want in your code.

Working on 1, 2 and 3 right now :)

jasonmcintosh commented 6 years ago

Bleh also - missing log-parser.zip file - will get that in my pull request :)

ventz commented 6 years ago

@jasonmcintosh

1 This was by design sadly -- and specifically because a lot of large companies still manage things "semi-manually". We could create an index table, but I think this just makes it more complicated. The idea was to do a 1-1 of ELB (and now ALB) per "ruleset name". Ex: project-blah with "blah-resource#". Where are you running into issues with this?

2 For the bucket name of the files - this was how Amazon used to do it -- by creating a named bucket. We can change it.

3 This we should change.

4 The design behind this was as a binary "deployer".

The "waf" bash script should be used, and that should take care of things for you -- prompt, get a copy of the template, and then create the "main".

ventz commented 6 years ago

@jasonmcintosh The log-parser.zip was created but somehow left out of the commit -- added and pushed out already :)

ventz commented 6 years ago

@jasonmcintosh By the way - thanks a lot for testing it. We just haven't had any time to test the v2 changes yet. Hopefully will find some time this week.

jasonmcintosh commented 6 years ago

Second thing I'm seeing is a RateBased rule not getting created when activating flood protection. LOOKS like in the lambda "custom-resource" function it's designed to create a flood rule and update the webacl... but I can't find anything that actually executes that handler. More I play with this the more I realize AWS's solution is kinda a mess. Don't suppose you'd seen that? Right now things look very similar, but don't have good tests on functionality yet.

jasonmcintosh commented 6 years ago

OH I hit this as well... https://github.com/awslabs/aws-waf-security-automations/issues/21

Need to see if I can fix it in the terraform project :)

jasonmcintosh commented 6 years ago

Regarding issue 1, we hit issues with duplicate bucket names when you use multiple accounts and try and create the web acl names the same :) Since "customer" is used in bucket creation, if you tried to create "default-waf-cloudfront" in multiple accounts or if I did that and you did that, it'd fail due to S3 global unique bucket name conflict issues. I'd guess the real solution would be to parameterize the bucket name, that way I could make it random for my use case, but others could do a named pattern of some sort.

ventz commented 6 years ago

The AWS solution is a mess. It's a great idea, but very "beta" -- this was one of the main reasons we wanted to implement it via Terraform, and have each part be modular/upgradable. That and the "all or nothing" Cloud Formation did not appeal.

The Issue you hit with the "ApiGatewayBadBot dependency error" is a Terraform issue I believe. It could be the Amazon API too, but the dependency block is definitely in terraform, and we have it implemented -- yet it seems to be ignored (order/dependency is not followed). The "solution" is to re-run it and it tends to fix it.

For the S3 bucket - we can add an override flag where it lets you rename it - it's easy enough.

jasonmcintosh commented 6 years ago

the ApiGatewayBadBot dependency isn't terraform specific :) It's AWS specific. Not had a chance to dig in and find out why - something on their handling. And yep, re-running seems to fix all the issues! Going to test your updates this morning if I can get time, see if there's any issues. We run a slightly tweaked version to allow this to be run as a module. Additionally, we set it up so that a parameter can change the Web ACL to "BLOCK" by default. This let's us setup some test distributions with limited access.