bridgecrewio / checkov

Prevent cloud misconfigurations and find vulnerabilities during build-time in infrastructure as code, container images and open source packages with Checkov by Bridgecrew.
https://www.checkov.io/
Apache License 2.0
7.01k stars 1.1k forks source link

CKV_AWS_117 (Ensure AWS Lambda function is configured inside a VPC) shouldnt be enabled by default #5894

Closed WtfJoke closed 9 months ago

WtfJoke commented 9 months ago

Describe the issue Checkov currently flags any Lambda which doesn't have VPC networking configured with CKV_AWS_117 / BC_AWS_GENERAL_65: https://docs.bridgecrew.io/docs/ensure-that-aws-lambda-function-is-configured-inside-a-vpc-1.

I think in most cases its not needed to put a Lambda inside a VPC. Also the AWS Well Architecture document (Chapter"Performance efficiency pillar" > "Selection" > "AWS Lambda" on page 63) suggest exactly that:

Configure Amazon VPC access to your Lambda functions only when necessary (page 63).

There is also a decision tree inside that document grafik

Examples N/A

Version (please complete the following information):

Additional context Original issue from https://github.com/bridgecrewio/checkov/issues/1794. I think the original issue has been closed by accident. If that wasnt by accident I am sorry for reopening it, but would like to reevaluate this.

JamesWoolfenden commented 9 months ago

How do you know from static analysis that this lambda does not need to access resource inside your VPC?

gruebel commented 9 months ago

It should be clear checkov's main purpose is to offer security and compliance best practices, not performance or cost efficiency. To run all your compute workload through your own VPC is standard for bigger companies, because they typically use firewalls to control the traffic which goes in and out. It is also an official AWS Security Hub control https://docs.aws.amazon.com/securityhub/latest/userguide/lambda-controls.html#lambda-3

WtfJoke commented 9 months ago

How do you know from static analysis that this lambda does not need to access resource inside your VPC?

You know this probably better than me. My thoughts of this:

  1. You cant tell
  2. When you define any VPC in your Template/tf state, you most liekly use VPC and can emit this warning.

checkov's main purpose is to offer security and compliance best practices, not performance or cost efficiency

I understand that. However for most users it might not be clear, that there is a big tradeoff in that. Just by placing it in a VPC doesnt mean the traffic will be controlled by a firewall nor that the VPC is configured correctly/securely. Users could be tempted to just place it in any vpc to get rid of the warning.

As a good citizen I would at least expect that a hint to note that there is a massive performance impact / tradeoff on that.

maiksensi commented 9 months ago

@JamesWoolfenden I salute you for your attitude towards a perfectly fine and polite question. Especially answering with a retorical question and then closing the issue is absolutely superb social behavior. Obviously, you can do whatever suits you here. At least delete the Code Of Conduct then.

gruebel commented 9 months ago

@WtfJoke There is no massive performance impact by placing it inside a VPC, this statement is so old and is still spread through the internet. Just check out this blog post and you will be surprised https://tsh.io/blog/aws-lambda-performance/ this doesn't mean you can't suffer from performance degradation, but that's how it is sometimes.

When we find a VPC in a TF template it doesn't mean anything. It's like saying, because I found an IAM role it belongs to all resources there.

@maiksensi the issue was closed, because of my comment, which explains the reasoning of the check.

It is important to say, don't follow blindly any recommendations a tool or someone else gives you without validating the reasoning. checkov gives all kind of recommendations, but it doesn't mean you have to follow them, pick and choose what fits and sometimes it helps to rethink about the setup you have.

maiksensi commented 9 months ago

Thanks for clarifying this and taking the time to answer.

WtfJoke commented 9 months ago

Just check out this blog post and you will be surprised https://tsh.io/blog/aws-lambda-performance/ this doesn't mean you can't suffer from performance degradation, but that's how it is sometimes.

Thanks for the link. Surprising at theyre also quiet contradicting results reported here: https://lumigo.io/aws-lambda-deployment/lambda-vpc/#slow_cold_starts, where there was a massive performance impact.

Probaly take both numbers with a grain of salt, but they might have improved it, too. Probably would need own testing results.

However I still think the current docs can be improved. But I'll stop bothering you now.

It is important to say, don't follow blindly any recommendations a tool or someone else gives you without validating the reasoning. checkov gives all kind of recommendations, but it doesn't mean you have to follow them, pick and choose what fits and sometimes it helps to rethink about the setup you have.

Sure, I totally agree on that, that goes for any tool out there :D Still you need to put sometimes more work into explaining that to someone, because the reasoning is often that the tool thought about this more than you.