CKV_AZURE_218 failing

[horiagunica]

Describe the issue Check CKV_AZURE_218 - "Ensure Application Gateway defines secure protocols for in transit communication" is failing in the following scenario :

Check: CKV_AZURE_218: "Ensure Application Gateway defines secure protocols for in transit communication"
        FAILED for resource: module.appgw.azurerm_application_gateway.this
        File: /modules/appgw/main.tf:48-313
        Calling File: /examples/standalone_vmseries/main.tf:307-339

Repository : https://github.com/PaloAltoNetworks/terraform-azurerm-vmseries-modules/tree/checkov-version-bump-fixes

Examples

Inside the module - I am specifically setting the default values :

variable "ssl_policy_type" {
  description = <<-EOF
  Type of an SSL policy. Possible values are `Predefined` or `Custom`.
  If the value is `Custom` the following values are mandatory: `ssl_policy_cipher_suites` and `ssl_policy_min_protocol_version`.
  EOF
  default     = "Predefined"
  type        = string
  nullable    = false
}

variable "ssl_policy_name" {
  description = <<-EOF
  Name of an SSL policy. Supported only for `ssl_policy_type` set to `Predefined`. Normally you can set it also for `Custom` policies but the name is discarded on Azure side causing an update to Application Gateway each time terraform code is run. Therefore this property is omitted in the code for `Custom` policies. 
  
  For the `Predefined` polcies, check the [Microsoft documentation](https://docs.microsoft.com/en-us/azure/application-gateway/application-gateway-ssl-policy-overview) for possible values as they tend to change over time. The default value is currently (Q1 2022) a Microsoft's default.
  EOF
  default     = "AppGwSslPolicy20220101S"
  type        = string
  nullable    = false
}

variable "ssl_policy_min_protocol_version" {
  description = <<-EOF
  Minimum version of the TLS protocol for SSL Policy. Required only for `ssl_policy_type` set to `Custom`. 

  Possible values are: `TLSv1_0`, `TLSv1_1`, `TLSv1_2` or `null` (only to be used with a `Predefined` policy).
  EOF
  default     = "TLSv1_2"
  type        = string
}

variable "ssl_policy_cipher_suites" {
  description = <<-EOF
  A list of accepted cipher suites. Required only for `ssl_policy_type` set to `Custom`. 
  For possible values see [documentation](https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/application_gateway#cipher_suites).
  EOF
  default     = ["TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"]
  type        = list(string)
}

The calling file standalone_vmseries example calls the module but in .tfvars it doesn't have a value for the map - which means the resource is not actually used there (it's present for modularity and flexibility only).

Version (please complete the following information):

• Checkov Version 2.3.310

Additional context

We are running checkov from pre-commit inside a runner (local tests offered the same results):

- hooks:
  - args:
    - --compact
    - --quiet
    - --skip-check
    - CKV_AZURE_118,CKV_AZURE_119,CKV_AZURE_120,CKV2_AZURE_10,CKV2_AZURE_12,CKV_AZURE_35,CKV_AZURE_206,CKV_AZURE_93,CKV2_AZURE_1,CKV2_AZURE_18,CKV_AZURE_97,CKV_AZURE_59,CKV_AZURE_190,CKV2_AZURE_33,CKV_AZURE_179,CKV_AZURE_1,CKV_AZURE_49,CKV_AZURE_217
    id: checkov
    verbose: true
  repo: https://github.com/bridgecrewio/checkov.git
  rev: 2.3.310

[JamesWoolfenden]

This is the checkov parser. I don't think theres any support for the try function your using for a lot of your variables, so we end up with policy_type='try(each.value.ssl_policy_type,None)' inside of a nested for_each. and the use of policy_name = var.ssl_policy_type == "Predefined" ? var.ssl_policy_name : null and that within a dynamic block.

[horiagunica]

Hi @JamesWoolfenden ! Thank you for investigating this. Development of such deep parsing sounds quite complex...I'm guessing we should not expect such a feature soon ? We can just skip the check for now - I just wanted to ask before we actually do 🙇

[JamesWoolfenden]

If your using TLSv1_2 or better and none of the "bad cyphers" https://github.com/bridgecrewio/checkov/blob/main/checkov/terraform/checks/resource/azure/AppGWDefinesSecureProtocols.py then you should be golden.

[horiagunica]

@JamesWoolfenden Hello!

Unfortunately it doesn't work - I already set those as defaults inside the module (and not modifying them in the main calling example TF code) and the issue still triggers (copy-pasting from the initial issue description) :

variable "ssl_policy_min_protocol_version" {
  description = <<-EOF
  Minimum version of the TLS protocol for SSL Policy. Required only for `ssl_policy_type` set to `Custom`. 

  Possible values are: `TLSv1_0`, `TLSv1_1`, `TLSv1_2` or `null` (only to be used with a `Predefined` policy).
  EOF
  default     = "TLSv1_2"
  type        = string
}

variable "ssl_policy_cipher_suites" {
  description = <<-EOF
  A list of accepted cipher suites. Required only for `ssl_policy_type` set to `Custom`. 
  For possible values see [documentation](https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/application_gateway#cipher_suites).
  EOF
  default     = ["TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"]
  type        = list(string)
}

[horiagunica]

@JamesWoolfenden is there any chance to fix this here ? Or shall we just suppress it until further notice ?

[JamesWoolfenden]

@horiagunica id send you requests to @gruebel as im not in the engineering team.

[acelebanski]

Hi @gruebel, are there any updates on this?

[gruebel]

hey @acelebanski support for try was added via #6043, so there is a chance it works now, but of course there can be other factors making it still fail.

[acelebanski]

Hi @gruebel, I did some research on that and we still have a problem. In the ssl_policy block, we use property min_protocol_version (we don't use disabled_protocols property at all). Its value is passed through a variable. The variable has a validation block where we allow only values TLSv1_2 & TLSv1_3. The check fails but when I put the value in main.tf explicitly, the check passes. I suppose checkov doesn't look into validation blocks right? Specifying variable's default of one of those 2 values doesn't seem to help either. Is there a way we can pass this value through a variable and don't trigger the check failure?

P.S. We don't use the try function anymore.

[acelebanski]

Hi @JamesWoolfenden & @gruebel, did you have a chance to analyse my last comment? Thanks.

[ChanochShayner]

Hey @acelebanski :) If you are not using try could you please provide a full example of your use case to better investigate it? Thanks.

[acelebanski]

Hi @ChanochShayner, this is what I meant:

main.tf:

resource "azurerm_application_gateway" "this" {
  (...)
  ssl_policy {
    policy_name          = var.global_ssl_policy.type == "Predefined" ? var.global_ssl_policy.name : null
    policy_type          = var.global_ssl_policy.type
    min_protocol_version = var.global_ssl_policy.min_protocol_version
    cipher_suites        = var.global_ssl_policy.cipher_suites
  }
  (...)
}

variables.tf:

variable "global_ssl_policy" {
  description = ""
  default     = {}
  nullable    = false
  type = object({
    type = optional(string, "Predefined")
    name = optional(string, "AppGwSslPolicy20220101S")
    min_protocol_version = optional(string)
    cipher_suites = optional(list(string), [])
  })
  (...)
  validation { # min_protocol_version
    condition = var.global_ssl_policy.min_protocol_version == null ? true : contains(
      ["TLSv1_2", "TLSv1_3"], var.global_ssl_policy.min_protocol_version
    )
    error_message = <<-EOF
    The `global_ssl_policy.min_protocol_version` property can be one of: \"TLSv1_2\" and \"TLSv1_3\".
    EOF
  }
  (...)
}

Is this taken into account when processing the check or do we have to do something else? Or are we doomed to bypass this check? Thanks!