diff --git a/checkov/terraform/checks/resource/azure/KeyVaultDisablesPublicNetworkAccess.py b/checkov/terraform/checks/resource/azure/KeyVaultDisablesPublicNetworkAccess.py index 35798e5123..47cfc28ffe 100644 --- a/checkov/terraform/checks/resource/azure/KeyVaultDisablesPublicNetworkAccess.py +++ b/checkov/terraform/checks/resource/azure/KeyVaultDisablesPublicNetworkAccess.py @@ -38,6 +38,14 @@ def scan_resource_conf(self, conf) -> CheckResult: ip_rules = ip_rules[0] if ip_rules and isinstance(ip_rules, list) else ip_rules if ip_rules: return CheckResult.PASSED + # An explicitly empty or rendered-empty ip_rules list (e.g. [[]] or []) + # combined with an explicitly resolved default_action = "Deny" also blocks + # public access. + if isinstance(ip_rules, list) and not ip_rules: + default_action = network_acl.get("default_action") + default_action = default_action[0] if isinstance(default_action, list) else default_action + if default_action == "Deny": + return CheckResult.PASSED virtual_network_subnet_ids = network_acl.get("virtual_network_subnet_ids") # Get first element in virtual_network_subnet_ids (as parser wrap it with list). virtual_network_subnet_ids = virtual_network_subnet_ids[0] \ diff --git a/tests/terraform/checks/resource/azure/example_KeyVaultDisablesPublicNetworkAccess/main.tf b/tests/terraform/checks/resource/azure/example_KeyVaultDisablesPublicNetworkAccess/main.tf index 07733b1fb7..a453f08d2e 100644 --- a/tests/terraform/checks/resource/azure/example_KeyVaultDisablesPublicNetworkAccess/main.tf +++ b/tests/terraform/checks/resource/azure/example_KeyVaultDisablesPublicNetworkAccess/main.tf @@ -139,6 +139,24 @@ resource "azurerm_key_vault" "pass5" { } } +resource "azurerm_key_vault" "pass6" { + name = "examplepass6" + location = azurerm_resource_group.example.location + resource_group_name = azurerm_resource_group.example.name + enabled_for_disk_encryption = true + tenant_id = data.azurerm_client_config.current.tenant_id + soft_delete_retention_days = 90 + purge_protection_enabled = enabled + public_network_access_enabled = true + sku_name = "standard" + network_acls { + default_action = "Deny" + bypass = "None" + ip_rules = [] + } +} + + resource "azurerm_key_vault" "fail1" { name = "examplefail1" location = azurerm_resource_group.example.location diff --git a/tests/terraform/checks/resource/azure/test_KeyVaultDisablesPublicNetworkAccess.py b/tests/terraform/checks/resource/azure/test_KeyVaultDisablesPublicNetworkAccess.py index b35eb03a0d..441246c9a8 100644 --- a/tests/terraform/checks/resource/azure/test_KeyVaultDisablesPublicNetworkAccess.py +++ b/tests/terraform/checks/resource/azure/test_KeyVaultDisablesPublicNetworkAccess.py @@ -1,9 +1,12 @@ import os import unittest +import hcl2 + from checkov.runner_filter import RunnerFilter from checkov.terraform.runner import Runner from checkov.terraform.checks.resource.azure.KeyVaultDisablesPublicNetworkAccess import check +from checkov.common.models.enums import CheckResult class TestKeyVaultDisablesPublicNetworkAccess(unittest.TestCase): @@ -22,7 +25,8 @@ def test(self): 'azurerm_key_vault.pass2', 'azurerm_key_vault.pass3', 'azurerm_key_vault.pass4', - 'azurerm_key_vault.pass5' + 'azurerm_key_vault.pass5', + 'azurerm_key_vault.pass6' } failing_resources = { 'azurerm_key_vault.fail1', @@ -46,6 +50,61 @@ def test(self): self.assertEqual(passing_resources, passed_check_resources) self.assertEqual(failing_resources, failed_check_resources) + def _build_conf(self, network_acls_block): + hcl_res = hcl2.loads(f""" + resource "azurerm_key_vault" "example" {{ + name = "examplekeyvault" + location = azurerm_resource_group.example.location + resource_group_name = azurerm_resource_group.example.name + public_network_access_enabled = true + sku_name = "standard" + {network_acls_block} + }} + """) + return hcl_res['resource'][0]['azurerm_key_vault']['example'] + + def test_deny_empty_ip_rules_passes(self): + resource_conf = self._build_conf(""" + network_acls { + default_action = "Deny" + ip_rules = [] + } + """) + self.assertEqual(CheckResult.PASSED, check.scan_resource_conf(conf=resource_conf)) + + def test_allow_empty_ip_rules_fails(self): + resource_conf = self._build_conf(""" + network_acls { + default_action = "Allow" + ip_rules = [] + } + """) + self.assertEqual(CheckResult.FAILED, check.scan_resource_conf(conf=resource_conf)) + + def test_unknown_default_action_with_empty_ip_rules_fails(self): + resource_conf = self._build_conf(""" + network_acls { + default_action = var.action + ip_rules = [] + } + """) + self.assertEqual(CheckResult.FAILED, check.scan_resource_conf(conf=resource_conf)) + + def test_deny_normalized_empty_ip_rules_passes(self): + # A rendered/normalized pipeline can present ip_rules as a bare [] list + # (instead of the raw HCL parser's [[]]). + resource_conf = { + "public_network_access_enabled": [True], + "network_acls": [ + { + "default_action": ["Deny"], + "ip_rules": [], + } + ], + } + self.assertEqual(CheckResult.PASSED, check.scan_resource_conf(conf=resource_conf)) + + if __name__ == '__main__': unittest.main() \ No newline at end of file