checkov icon indicating copy to clipboard operation
checkov copied to clipboard

Terraform - Support optional() in variable type constraints

Open ppawlowski opened this issue 1 year ago • 8 comments

Describe the issue

While adding keys to the Azure KeyVault via azurerm_key_vault_key looped with for_each approach, checkov is not recognizing key_type properly and check CKV_AZURE_112 fails.

Examples

main.tf:

  provider "azurerm" {
      features {}
  }
  
  variable "keys" {
    type = list(object({
      name = string
      type = optional(string, "RSA-HSM")
      size = optional(number, 2048)
    }))
    default     = [{
      name = "examplekey"
    }]
  }
  
  data "azurerm_client_config" "current" {}
  
  resource "azurerm_resource_group" "this" {
      name     = "example-rg"
      location = "West Europe"
  }
  
  resource "azurerm_key_vault" "this" {
      name                = "examplekv"
      tenant_id           = data.azurerm_client_config.current.tenant_id
      resource_group_name = azurerm_resource_group.this.name
      location            = azurerm_resource_group.this.location
      sku_name            = "standard"
  }
  
  resource "azurerm_key_vault_key" "this" {
    for_each     = { for key in var.keys : key.name => key }
    name         = each.value.name
    key_vault_id = azurerm_key_vault.this.id
    key_type     = each.value.type
    key_size     = each.value.size
    key_opts = [
      "decrypt",
      "encrypt"
    ]
  }

checkov execution (irrelevant output omitted):

$ checkov -d keyvault_checkov -c CKV_AZURE_112

terraform scan results:

Passed checks: 0, Failed checks: 1, Skipped checks: 0

Check: CKV_AZURE_112: "Ensure that key vault key is backed by HSM"
	FAILED for resource: azurerm_key_vault_key.this
	File: /main.tf:32-42
	Guide: https://docs.bridgecrew.io/docs/ensure-that-key-vault-key-is-backed-by-hsm

		32 | resource "azurerm_key_vault_key" "this" {
		33 |   for_each = { for key in var.keys : key.name => key }
		34 |   name         = each.value.name
		35 |   key_vault_id = azurerm_key_vault.this.id
		36 |   key_type     = each.value.type
		37 |   key_size     = each.value.size
		38 |   key_opts = [
		39 |     "decrypt",
		40 |     "encrypt"
		41 |   ]
		42 | }

Expected checkov output (irrelevant output omitted):

$ checkov -d keyvault_checkov -c CKV_AZURE_112

terraform scan results:

Passed checks: 1, Failed checks: 0, Skipped checks: 0

Check: CKV_AZURE_112: "Ensure that key vault key is backed by HSM"
	PASSED for resource: azurerm_key_vault_key.this
	File: /main.tf:32-42
	Guide: https://docs.bridgecrew.io/docs/ensure-that-key-vault-key-is-backed-by-hsm

Version (please complete the following information):

  • 2.3.158

ppawlowski avatar Apr 11 '23 09:04 ppawlowski

hey @ppawlowski thanks for reaching out.

We progressed quite far with for_each support in checkov both on resource and module level. We are still keeping it behind a feature flag, which you can enable by setting a specific env var. We plan to enable it by default in the near future, but have to investigate some performance implications.

CHECKOV_ENABLE_FOREACH_HANDLING=True checkov -d keyvault_checkov -c CKV_AZURE_112

I keep the ticket open till we enable it by default 🙂

gruebel avatar Apr 11 '23 10:04 gruebel

In the presented example, checkov still fails despite adding CHECKOV_ENABLE_FOREACH_HANDLING=True . Seems like it is not able to handle default value set by optional() function. After adding type value to variable's default, check is performed correctly.

ppawlowski avatar Apr 11 '23 13:04 ppawlowski

we don't support any type constraints at the moment, we just grab the default value of the variable.

gruebel avatar Apr 11 '23 17:04 gruebel

As you mention - this issue is not related to the for_each effort. Removing from the for_each milestone. @gruebel fyi

ChanochShayner avatar Apr 13 '23 13:04 ChanochShayner

Thanks for contributing to Checkov! We've automatically marked this issue as stale to keep our issues list tidy, because it has not had any activity for 6 months. It will be closed in 14 days if no further activity occurs. Commenting on this issue will remove the stale tag. If you want to talk through the issue or help us understand the priority and context, feel free to add a comment or join us in the Checkov slack channel at https://slack.bridgecrew.io Thanks!

stale[bot] avatar Oct 15 '23 05:10 stale[bot]

Not stale.

ppawlowski avatar Oct 17 '23 08:10 ppawlowski

Thanks for contributing to Checkov! We've automatically marked this issue as stale to keep our issues list tidy, because it has not had any activity for 6 months. It will be closed in 14 days if no further activity occurs. Commenting on this issue will remove the stale tag. If you want to talk through the issue or help us understand the priority and context, feel free to add a comment or join us in the Checkov slack channel at codifiedsecurity.slack.com Thanks!

stale[bot] avatar Apr 14 '24 14:04 stale[bot]

Not stale.

ppawlowski avatar Apr 22 '24 07:04 ppawlowski