Skip to content
This repository has been archived by the owner on Sep 3, 2024. It is now read-only.

[BUG] FG_R00068 fails if a data.aws_kms_key is set in the cloudwatch log group #299

Closed
rsareth opened this issue Feb 7, 2022 · 3 comments · Fixed by #315
Closed

[BUG] FG_R00068 fails if a data.aws_kms_key is set in the cloudwatch log group #299

rsareth opened this issue Feb 7, 2022 · 3 comments · Fixed by #315
Assignees
Labels
bug Something isn't working

Comments

@rsareth
Copy link

rsareth commented Feb 7, 2022

Describe the bug
The rule is incomplete. If the attribute kms_keys_id is set with a data.aws_kms_key.A.id instead of aws_kms_key.A.id, it fails.

We don't create the KMS Key in the same repository. It is created in another repository. So we need to fetch it with a data.aws_kms_key resource.

How you're running Regula
Please include versions of all relevant tools. Some examples:

  • I'm using Regula v2.4.0 as a Rego library with OPA v0.34.1

IaC Configuration

resource "aws_cloudwatch_log_group" "log_group" {
  name              = "/aws/lambda/${aws_lambda_function.A.function_name}"
  kms_key_id        = data.aws_kms_key.global_kms.id # <--- It fails because of this syntax
  retention_in_days = 365
}

Here it is the rego code in encrypted_logs.rego :

valid_log_groups[id] = lg {
  lg = log_groups[id]
  lg.kms_key_id != null
  valid_kms_arn_prefix[k]
  startswith(lg.kms_key_id, k)
} {
  fugue.input_type != "tf_runtime"
  lg = log_groups[id]
  lg.kms_key_id != null
  fugue.resources("aws_kms_key")[lg.kms_key_id]
} 

I'm a complete newbie in rego. So, in my test, I copied/pasted your rego rule in another file and added this:

{
  fugue.input_type != "tf_runtime"
  lg = log_groups[id]
  lg.kms_key_id != null
  fugue.resources("data.aws_kms_key")[lg.kms_key_id]
}

What do you think? And if I'm right, you might need to add the same kind of logic in every rule that exists.

Regards,
Rasmey

@jaspervdj-luminal jaspervdj-luminal added the bug Something isn't working label Feb 8, 2022
@jaspervdj-luminal
Copy link
Member

Thanks for reporting this! Yes, that workaround makes sense -- but I think there's a simpler solution:

fugue.resources("aws_kms_key")[lg.kms_key_id] checks that the responding key exists. This should not be part of the rule and this line should be removed; we should just verify that the format is valid. We don't need to check that the resource is there, since if it's not there, terraform apply will fail anyway.

I'm about to release a large number of updates to rules, I'll fix this one right after to prevent annoying merges.

@jaspervdj-luminal jaspervdj-luminal self-assigned this Feb 8, 2022
@rsareth
Copy link
Author

rsareth commented Feb 9, 2022

Thank you for your answer, @jaspervdj-luminal

But for another use case, if an aws_kms_alias is used, do you handle it in your refactoring ? Technically in terraform, the kms_key_id can also be fetched from an aws_kms_alias.

PS: Bad post. I just need to wait for your refactored rules. Sorry. It wouldn't trigger anymore some alerts.

Regards,
Rasmey

@jaspervdj-luminal
Copy link
Member

Thanks, aws_kms_alias is a great catch -- I'll make sure to add a test case for that as well in the updated rule.

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants