kics icon indicating copy to clipboard operation
kics copied to clipboard

Add support for terraform variables with optional values

Open hilariocoelho opened this issue 2 years ago • 0 comments

Is your feature request related to a problem? Please describe.

Terraform will be introducing the optional support on the upcoming version 1.3.0, the release concludes the module_variable_optional_attrs experiment.

This is critical for KICS in scenarios like the following:

terraform {
  required_providers {
    azurerm = {
      source  = "hashicorp/azurerm"
      version = ">=3.1.0, <4.0.0"
    }
  }
  required_version = ">= 1.3.0"
}

variable "storage_account_firewall_with_default" {
  type = object({
    default_action             = string
    ip_rules                   = list(string)
    virtual_network_subnet_ids = list(string)
    bypass                     = list(string)
  })
  default = {
    default_action             = "Deny"
    ip_rules                   = null
    virtual_network_subnet_ids = null
    bypass                     = ["AzureServices"]
  }
  description = "Storage account with default values"
}

variable "storage_account_firewall_with_optional" {
  type = object({
    default_action             = optional(string, "Deny")
    ip_rules                   = list(string)
    virtual_network_subnet_ids = list(string)
    bypass                     = optional(list(string), ["AzureServices"])
  })
  description = "Storage account with optional values"
}

resource "azurerm_storage_account" "sa_with_defaults" {
  name                      = "sawithdefaults"
  resource_group_name       = var.resource_group.name
  location                  = var.resource_group.location
  account_tier              = "Standard"
  account_replication_type  = "LRS"
  access_tier               = "Cold"
  min_tls_version           = "TLS1_2"
  enable_https_traffic_only = true

  network_rules {
    default_action             = var.storage_account_firewall_with_optional.default_action
    ip_rules                   = var.storage_account_firewall_with_optional.ip_rules
    virtual_network_subnet_ids = var.storage_account_firewall_with_optional.virtual_network_subnet_ids
    bypass                     = var.storage_account_firewall_with_optional.bypass
  }
}

resource "azurerm_storage_account" "sa_with_optionals" {
  name                      = "sawithoptionals"
  resource_group_name       = var.resource_group.name
  location                  = var.resource_group.location
  account_tier              = "Standard"
  account_replication_type  = "LRS"
  access_tier               = "Cold"
  min_tls_version           = "TLS1_2"
  enable_https_traffic_only = true

  network_rules {
    default_action             = var.storage_account_firewall_with_default.default_action
    ip_rules                   = var.storage_account_firewall_with_default.ip_rules
    virtual_network_subnet_ids = var.storage_account_firewall_with_default.virtual_network_subnet_ids
    bypass                     = var.storage_account_firewall_with_default.bypass
  }
}

Produces the following output:

Trusted Microsoft Services Not Enabled, Severity: HIGH, Results: 1
Description: Trusted Microsoft Services should be enabled for Storage Account access
Platform: Terraform

        [1]: ../../path/kics-test.tf:51

                050:     virtual_network_subnet_ids = var.storage_account_firewall_with_optional.virtual_network_subnet_ids
                051:     bypass                     = var.storage_account_firewall_with_optional.bypass
                052:   }



Results Summary:
HIGH: 1
MEDIUM: 0
LOW: 0
INFO: 0
TOTAL: 1

Describe the solution you'd like The expected output would be the same for both storage accounts defined with default and with optional values.

I'm not even sure if Kics should report this as a security issue since the value of bypass is a variable that will be defined on the module initialization (considering this code belongs to a module). Kics is forcing to create variables with default values that comply with security checks (and I can't use optional since it is not yet supported) but since it is a variable, the instantiation of the module might define the variable with some value introducing security issues.

Describe alternatives you've considered Add kics scan exception using # kics-scan ignore-line as described here

Additional context This was tested using latest Kics version, v1.5.12.

hilariocoelho avatar Jul 18 '22 14:07 hilariocoelho