fugue / regula

Regula checks infrastructure as code templates (Terraform, CloudFormation, k8s manifests) for AWS, Azure, Google Cloud, and Kubernetes security and compliance using Open Policy Agent/Rego
https://regula.dev/
Apache License 2.0
962 stars 108 forks source link

S3 Block public access rule doesn't work when aws_s3_bucket_public_access_block is used for conditional resource or in the module #295

Open marioerceg opened 2 years ago

marioerceg commented 2 years ago

Hi,

It seems that S3 Block public access rule (https://github.com/fugue/regula/blob/master/rego/rules/tf/aws/s3/block_public_access.rego) doesn't work when aws_s3_bucket_public_access_block is used for conditional resource OR within the module.

Example file with working and not working resources:

`terraform {

backend "local" { path = "../terraform.tfstate.27.01.2022" }

}

provider "aws" { version = "~> 2.0" region = var.aws_region allowed_account_ids = [var.aws_account_id] }

resource "aws_s3_bucket" "working_1" { bucket = "working-1-random-string" } resource "aws_s3_bucket_public_access_block" "working_1_block" { bucket = aws_s3_bucket.working_1.id

block_public_acls = true block_public_policy = true ignore_public_acls = true restrict_public_buckets = true }

resource "aws_s3_bucket" "not_working_1" { count = var.aws_region == "eu-west-1" ? 1 : 0 bucket = "not-working-1-random-string" acl = "private" } resource "aws_s3_bucket_public_access_block" "not_working_1_block" { count = var.aws_region == "eu-west-1" ? 1 : 0 bucket = aws_s3_bucket.not_working_1[count.index].id block_public_acls = true block_public_policy = true ignore_public_acls = true restrict_public_buckets = true }

module "working_2" { source = "git@github.com:rackspace-infrastructure-automation/aws-terraform-s3//?ref=v0.12.6" name = "working-2-random-string" } resource "aws_s3_bucket_public_access_block" "working_2_block" { bucket = module.working_2.bucket_id

block_public_acls = true block_public_policy = true ignore_public_acls = true restrict_public_buckets = true }

module "not_working_2" { source = "git@github.com:rackspace-infrastructure-automation/aws-terraform-s3//?ref=v0.12.6"

block_public_access = true block_public_access_acl = true block_public_access_ignore_acl = true block_public_access_policy = true block_public_access_restrict_bucket = true name = "not-working-2-random-string" }`

ameliafugue commented 2 years ago

Hi @marioerceg

Thank you for reporting this. The team is investigating this issue.

marioerceg commented 2 years ago

Hi @ameliafugue . Is there any update on this? Thanks!

jaspervdj-luminal commented 2 years ago

@marioerceg Are you using Regula against .tf files or a JSON plan?

We need to improve our count support in the HCL view a little bit to get aws_s3_bucket.not_working_1[count.index].id to work -- currently the HCL processor constructs the resource without the extra index. I'll see if I can get some time to work on that.

I took at the look at the source code of the module you are using, and they are using the same count pattern, so the problem is likely this.

marioerceg commented 2 years ago

Thanks @jaspervdj-luminal !

I am using json plan for this.

marioerceg commented 2 years ago

Hi @jaspervdj-luminal . Did you have time to check this? Thanks!

jaspervdj-luminal commented 2 years ago

Hey @marioerceg, sorry for the delay! It was a bit chaotic the past few weeks because we were acquired by Snyk. I should have some time to look into this today or tomorrow though. I'll let you know when I have a branch you can test!

marioerceg commented 2 years ago

Congrats @jaspervdj-luminal ! Deal. Waiting for your reply.

jaspervdj-luminal commented 2 years ago

I've started a count-improvements branch, however there's some work left. This turned out to be significantly more difficult than I thought, since it requires an update to our dependency analysis, as well as the way we pass resources as values into the HCL evaluator. I expect to finish this by the end of this week.

marioerceg commented 2 years ago

Great! Thanks @jaspervdj-luminal !

jaspervdj-luminal commented 2 years ago

@marioerceg I think the count-improvements branch should work now. I need to test it a bit more before we merge it, but feel free to give it a try for your use case.

marioerceg commented 2 years ago

Thanks @jaspervdj-luminal ! Will try it tomorrow

marioerceg commented 2 years ago

Hi @jaspervdj-luminal it seems it doesn't work, or I am not testing well. It still works for 2 working examples.

I am using "opa eval .... 'data.fugue.regula.report'"

Tested with TF versions 0.14.11 and 1.0.11.

Tested with the last opa as well as with 0.30.1

jaspervdj-luminal commented 2 years ago

@marioerceg Oh, I see. Unfortunately improving the count attribute turned it to be relatively complex, so I don't think it's feasible to do in pure Rego. We do the transformation in the hcl preprocessor in the Regula binary. Since we are able to be a bit more flexible there, it has a number of other improvements over the pure Rego library as well.

You can get similar output to opa eval using something like: regula run -f json. If for some reason, it's not possible to use a regula invocation (e.g. you are using conftest), you can still use the preprocessor like this:

regula show input . >regula_input.json  # read in TF files and preprocess
opa run -d regula/rego/lib -d regula/rego/rules -i regula_input.json 'data.fugue.regula.report'  # use preprocessed input

Does that help?

marioerceg commented 2 years ago

Hi @jaspervdj-luminal . After I added comment, I tried with the last regula version "regula run" and got the same results (but again from terraform generated json plan) Will it give different results if I download the new branch and try with commands you specified? Thanks!

jaspervdj-luminal commented 2 years ago

@marioerceg Yeah, it will give different results if you point it at the folder with your .tf files. That way regula will be able to parse and preprocess it all from source, which as a nice side effect gives you some other things like file names and line numbers.

marioerceg commented 2 years ago

@jaspervdj-luminal Let me try it tomorrow. I hope it will work and that the code and all the modules are processed well

marioerceg commented 2 years ago

Hi @jaspervdj-luminal . I tried with the last available prebuilt binary version of regula, and it worked for not_working_2, but it didn't recognize not_working_1 bucket at all - not showing it.

marioerceg commented 2 years ago

Just small update:

It is recognized if not using variable, like this:

count = "eu-west-1" == "eu-west-1" ? 1 : 0

but block access settings is not linked with it even if not using variable.

With terraform json it is recognized.

jaspervdj-luminal commented 2 years ago

@marioerceg The PR hasn't merged yet, it's here: https://github.com/fugue/regula/pull/321

If you have Go installed, it should be relatively easy to build a binary using make binary. I can also provide you with one if necessary (or I expect we will get this into a release sometime next week).

marioerceg commented 2 years ago

Hi @jaspervdj-luminal . Tried but can't get not_working_1 recognized at all.

One more question: is it possible to have full description in "table output" of regula run, and not only the short summary one? Thanks!

jaspervdj-luminal commented 2 years ago

@marioerceg Can you try this again with Regula v2.6.1?

I think having the full description in the table output would be hard to fit on a screen nicely for most of users? Some descriptions are quite long, and this output is meant to be human-readable first.

marioerceg commented 2 years ago

Hi @jaspervdj-luminal . Tested with v2.6.1, build 0d40e66 (built with OPA v0.37.2), downloaded the latest verion of the count-improvement branch as well, and still doesn't recognize not_working_1 at all (not showing that resource in the output at all)

Thanks, Mario

marioerceg commented 2 years ago

Hi @jaspervdj-luminal . Regarding adding descriptions to the table it would be good to be optional, or at least to have another field where link to description could be added. I have some custom rules, and want to give description what should be done to fix the issue. Thanks!

marioerceg commented 2 years ago

Hi @jaspervdj-luminal ! Any update on this? Thanks

cxystras-xm commented 2 years ago

Hi @jaspervdj-luminal , can we get some traction here. This issue affects us in our daily operations. thanks

marioerceg commented 2 years ago

Hi @jaspervdj-luminal . Could you share if there is any ETA to fix this? Thanks