Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checkov crashes when evaluating a Terraform dynamic block in NSGRulePortAccessRestricted.py #488

Closed
davenicoll opened this issue Aug 8, 2020 · 0 comments · Fixed by #489
Assignees

Comments

@davenicoll
Copy link
Contributor

davenicoll commented Aug 8, 2020

Describe the bug
When checking azure_security_group_rule, azurerm_network_security_rule or azurerm_network_security_group Terraform resource types, NSGRulePortAccessRestricted.py throws a "TypeError: string indices must be integers" error whenever there's a dynamic block.

To Reproduce
Steps to reproduce the behavior:

  1. Create a resource in terraform, containing a dynamic security rule -
resource "azurerm_network_security_group" "snet_nsgs" {
  count               = "${length(local.subnets)}"
  name                = "${local.root}-snet-${lookup(local.subnets[count.index], "name")}-nsg"
  location            = "${azurerm_resource_group.net_rg.location}"
  resource_group_name = "${azurerm_resource_group.net_rg.name}"
  tags                = "${local.tags}"


  dynamic "security_rule" {
    for_each = [for s in local.subnets[count.index].nsg_rules : {
      name                       = s.name
      priority                   = s.priority
      direction                  = s.direction
      access                     = s.access
      protocol                   = s.protocol
      source_port_range          = s.source_port_range
      destination_port_range     = s.destination_port_range
      source_address_prefix      = s.source_address_prefix
      destination_address_prefix = s.destination_address_prefix
      description                = s.description
    }]
    content {
      name                       = security_rule.value.name
      priority                   = security_rule.value.priority
      direction                  = security_rule.value.direction
      access                     = security_rule.value.access
      protocol                   = security_rule.value.protocol
      source_port_range          = security_rule.value.source_port_range
      destination_port_range     = security_rule.value.destination_port_range
      source_address_prefix      = security_rule.value.source_address_prefix
      destination_address_prefix = security_rule.value.destination_address_prefix
      description                = security_rule.value.description
    }
  }
}
  1. Run checkov
  2. Error!

Expected behavior
As checkov cannot evaluate the dynamic block, I expect the check to be skipped without throwing an error.

Desktop (please complete the following information):

  • OS: Ubuntu
  • Checkov Version 1.0.479
schosterbarak pushed a commit that referenced this issue Aug 19, 2020
…#489)

* Fix NSGRulePortAccessRestricted crashing on a terraform dynamic block

See issue: #488

* More appropriate return code

* One more try...

Looking at the rest of the code, seems CheckResult.UNKNOWN is probably more suitable.

* Reduced scope

Co-authored-by: David Nicoll <[email protected]>
github-actions bot pushed a commit that referenced this issue Aug 19, 2020
…#489)

* Fix NSGRulePortAccessRestricted crashing on a terraform dynamic block

See issue: #488

* More appropriate return code

* One more try...

Looking at the rest of the code, seems CheckResult.UNKNOWN is probably more suitable.

* Reduced scope

Co-authored-by: David Nicoll <[email protected]>
github-actions bot pushed a commit that referenced this issue Aug 19, 2020
…#489)

* Fix NSGRulePortAccessRestricted crashing on a terraform dynamic block

See issue: #488

* More appropriate return code

* One more try...

Looking at the rest of the code, seems CheckResult.UNKNOWN is probably more suitable.

* Reduced scope

Co-authored-by: David Nicoll <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants