Repository: bridgecrewio/checkov
Pull request: feat(azure): Add check - azure keyvalut public network access
**By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.**

[//]: # "
    # PR Title
    Be aware that we use the title to create changelog automatically and therefore only allow specific prefixes
    - break:    to indicate a breaking change, this supersedes any of the types
    - feat:     to indicate new features or checks
    - fix:      to indicate a bugfix or handling of edge cases of existing checks
    - docs:     to indicate an update to our documentation
    - chore:    to indicate adjustments to workflow files or dependency updates
    - platform: to indicate a change needed for the platform
    Additionally a scope is needs to be added to the prefix, which indicates the targeted framework, in doubt choose 'general'.
    #    
    Allowed prefixs:
    argo|arm|azure|bicep|bitbucket|circleci|cloudformation|dockerfile|github|gha|gitlab|helm|kubernetes|kustomize|openapi|sca|secrets|serverless|terraform|general|graph|terraform_plan
    #
    ex.
    feat(terraform): add CKV_AWS_123 to ensure that VPC Endpoint Service is configured for Manual Acceptance
"

## Description

Adding Azure KeyVaultDisablesPublicNetworkAccess check

Fixes #3832 

## New/Edited policies (Delete if not relevant)
Adding Azure KeyVaultDisablesPublicNetworkAccess check

## Checklist:

- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have made corresponding changes to the documentation
- [x] I have added tests that prove my feature, policy, or fix is effective and works
- [x] New and existing tests pass locally with my changes
- [x] Any dependent changes have been merged and published in downstream modules


Problem statement:
Ensure that Public Network Access is disabled for Azure Key Vault
**Describe the issue**
It seems that there are no Checkov checks related to the best practice of not allowing Public Network Access to Azure Key Vault
It should be either:

- A list of pre-approved ip_rules in a network_acls block
- A private endpoint

**Examples**

````hcl
resource "azurerm_key_vault" "example" {
  name                                   = "examplekeyvault"
  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          = false
  sku_name                               = "standard"

  access_policy {
    tenant_id = data.azurerm_client_config.current.tenant_id
    object_id = data.azurerm_client_config.current.object_id

    key_permissions = [
      "Get",
    ]

    secret_permissions = [
      "Get",
    ]

    storage_permissions = [
      "Get",
    ]
  }
}
````

**Version (please complete the following information):**
 - N/A

Hints from issue discussion:
hey @tdefise thanks for reaching out. Nice find 💪  this sounds like a good addition. Are you interested in contributing this check to `checkov`?
Hi @gruebel,

Yes, I will read how to correctly contribute to Checkov and do it in the coming days.
(I will need a bit of time as it will be the first time)

Hopes it's fine.
yeah, sure. if you need some help, feel free to reach out here.
@gruebel I will start to work on it next week and let you know if I need any help.
Hey
I am new to Opensource
@gruebel can I collaborate with you on this task
hey @tdefise are you still interested in contributing this new check? if not, then @saipavanbm your welcome to give it a try.
Hey @gruebel I was going through the repository
I didn't get how to add ID
Ex: id = "CKV_AZURE_124"
can you help on this

Hello @gruebel, I didn't had time to do it.
Please @saipavanbm go ahead
@saipavanbm let me know if i can help/complete

Commit message: update test

Patch to review:
diff --git a/checkov/terraform/checks/resource/azure/KeyVaultDisablesPublicNetworkAccess.py b/checkov/terraform/checks/resource/azure/KeyVaultDisablesPublicNetworkAccess.py
new file mode 100644
index 0000000000..32231f48c2
--- /dev/null
+++ b/checkov/terraform/checks/resource/azure/KeyVaultDisablesPublicNetworkAccess.py
@@ -0,0 +1,38 @@
+from checkov.common.models.enums import CheckCategories, CheckResult
+from checkov.terraform.checks.resource.base_resource_value_check import BaseResourceValueCheck
+
+
+class KeyVaultDisablesPublicNetworkAccess(BaseResourceValueCheck):
+    def __init__(self):
+        name = "Ensure that key vault disable public network access"
+        id = "CKV_AZURE_189"
+        supported_resources = ['azurerm_key_vault']
+        categories = [CheckCategories.NETWORKING]
+        super().__init__(name=name, id=id, categories=categories, supported_resources=supported_resources)
+
+    def get_inspected_key(self):
+        return "public_network_access_enabled"
+
+    def get_expected_value(self):
+        return False
+
+    def scan_resource_conf(self, conf) -> CheckResult:
+        """
+        KeyVaultDisablesPublicNetworkAccess unique logic.
+        public_network_access_enabled default value is True (when creating it)
+        If it False - check pass
+        Otherwise, we check that network_acls configure exists.
+
+        """
+        network_acls_key = "network_acls"
+        if self.get_inspected_key() in conf:
+            conf_value = conf.get(self.get_inspected_key())
+            conf_value = conf_value[0] if isinstance(conf_value, list) else conf_value
+            if self.get_expected_value() == conf_value:
+                return CheckResult.PASSED
+        if conf.get(network_acls_key):
+            return CheckResult.PASSED
+        return CheckResult.FAILED
+
+
+check = KeyVaultDisablesPublicNetworkAccess()
diff --git a/tests/terraform/checks/resource/azure/example_KeyVaultDisablesPublicNetworkAccess/main.tf b/tests/terraform/checks/resource/azure/example_KeyVaultDisablesPublicNetworkAccess/main.tf
new file mode 100644
index 0000000000..3bd45b36d1
--- /dev/null
+++ b/tests/terraform/checks/resource/azure/example_KeyVaultDisablesPublicNetworkAccess/main.tf
@@ -0,0 +1,177 @@
+resource "azurerm_key_vault" "pass1" {
+  name                                   = "examplepass1"
+  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          = false
+  sku_name                               = "standard"
+  access_policy {
+    tenant_id = data.azurerm_client_config.current.tenant_id
+    object_id = data.azurerm_client_config.current.object_id
+
+    key_permissions = [
+      "Get",
+    ]
+
+    secret_permissions = [
+      "Get",
+    ]
+
+    storage_permissions = [
+      "Get",
+    ]
+  }
+}
+
+resource "azurerm_key_vault" "pass2" {
+  name                                   = "examplepass2"
+  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          = false
+  sku_name                               = "standard"
+  network_acls {
+    default_action = "Allow"
+    bypass         = "AzureServices"
+  }
+  access_policy {
+    tenant_id = data.azurerm_client_config.current.tenant_id
+    object_id = data.azurerm_client_config.current.object_id
+
+    key_permissions = [
+      "Get",
+    ]
+
+    secret_permissions = [
+      "Get",
+    ]
+
+    storage_permissions = [
+      "Get",
+    ]
+  }
+}
+
+resource "azurerm_key_vault" "pass3" {
+  name                                   = "examplepass3"
+  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 = "Allow"
+    bypass         = "AzureServices"
+  }
+  access_policy {
+    tenant_id = data.azurerm_client_config.current.tenant_id
+    object_id = data.azurerm_client_config.current.object_id
+
+    key_permissions = [
+      "Get",
+    ]
+
+    secret_permissions = [
+      "Get",
+    ]
+
+    storage_permissions = [
+      "Get",
+    ]
+  }
+}
+
+resource "azurerm_key_vault" "pass4" {
+  name                                   = "examplepass4"
+  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
+  sku_name                               = "standard"
+  network_acls {
+    default_action = "Allow"
+    bypass         = "AzureServices"
+  }
+  access_policy {
+    tenant_id = data.azurerm_client_config.current.tenant_id
+    object_id = data.azurerm_client_config.current.object_id
+
+    key_permissions = [
+      "Get",
+    ]
+
+    secret_permissions = [
+      "Get",
+    ]
+
+    storage_permissions = [
+      "Get",
+    ]
+  }
+}
+
+resource "azurerm_key_vault" "fail1" {
+  name                                   = "examplefail1"
+  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
+  sku_name                               = "standard"
+  public_network_access_enabled          = true
+  access_policy {
+    tenant_id = data.azurerm_client_config.current.tenant_id
+    object_id = data.azurerm_client_config.current.object_id
+
+    key_permissions = [
+      "Get",
+    ]
+
+    secret_permissions = [
+      "Get",
+    ]
+
+    storage_permissions = [
+      "Get",
+    ]
+  }
+}
+
+resource "azurerm_key_vault" "fail2" {
+  name                                   = "examplefail2"
+  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
+  sku_name                               = "standard"
+  access_policy {
+    tenant_id = data.azurerm_client_config.current.tenant_id
+    object_id = data.azurerm_client_config.current.object_id
+
+    key_permissions = [
+      "Get",
+    ]
+
+    secret_permissions = [
+      "Get",
+    ]
+
+    storage_permissions = [
+      "Get",
+    ]
+  }
+}
diff --git a/tests/terraform/checks/resource/azure/test_KeyVaultDisablesPublicNetworkAccess.py b/tests/terraform/checks/resource/azure/test_KeyVaultDisablesPublicNetworkAccess.py
new file mode 100644
index 0000000000..011f7d8c35
--- /dev/null
+++ b/tests/terraform/checks/resource/azure/test_KeyVaultDisablesPublicNetworkAccess.py
@@ -0,0 +1,46 @@
+import os
+import unittest
+
+from checkov.runner_filter import RunnerFilter
+from checkov.terraform.runner import Runner
+from checkov.terraform.checks.resource.azure.KeyVaultDisablesPublicNetworkAccess import check
+
+
+class TestKeyVaultDisablesPublicNetworkAccess(unittest.TestCase):
+
+    def test(self):
+        runner = Runner()
+        current_dir = os.path.dirname(os.path.realpath(__file__))
+
+        test_files_dir = os.path.join(current_dir, "example_KeyVaultDisablesPublicNetworkAccess")
+        report = runner.run(root_folder=test_files_dir,
+                            runner_filter=RunnerFilter(checks=[check.id]))
+        summary = report.get_summary()
+
+        passing_resources = {
+            'azurerm_key_vault.pass1',
+            'azurerm_key_vault.pass2',
+            'azurerm_key_vault.pass3',
+            'azurerm_key_vault.pass4'
+        }
+        failing_resources = {
+            'azurerm_key_vault.fail1',
+            'azurerm_key_vault.fail2'
+        }
+
+        skipped_resources = {}
+
+        passed_check_resources = set([c.resource for c in report.passed_checks])
+        failed_check_resources = set([c.resource for c in report.failed_checks])
+
+        self.assertEqual(summary['passed'], len(passing_resources))
+        self.assertEqual(summary['failed'], len(failing_resources))
+        self.assertEqual(summary['skipped'], len(skipped_resources))
+        self.assertEqual(summary['parsing_errors'], 0)
+
+        self.assertEqual(passing_resources, passed_check_resources)
+        self.assertEqual(failing_resources, failed_check_resources)
+
+
+if __name__ == '__main__':
+    unittest.main()
\ No newline at end of file


Code review comments:
diff_hunk: @@ -0,0 +1,38 @@
+from checkov.common.models.enums import CheckCategories, CheckResult
+from checkov.terraform.checks.resource.base_resource_value_check import BaseResourceValueCheck
+
+
+class KeyVaultDisablesPublicNetworkAccess(BaseResourceValueCheck):
+    def __init__(self):
+        name = "Ensure that key vault disable public network access"
+        id = "CKV_AZURE_189"
+        supported_resources = ['azurerm_key_vault']
+        categories = [CheckCategories.NETWORKING]
+        super().__init__(name=name, id=id, categories=categories, supported_resources=supported_resources)
+
+    def get_inspected_key(self):
+        return "public_network_access_enabled"
+
+    def get_expected_value(self):
+        return False
+
+    def scan_resource_conf(self, conf) -> CheckResult:
+        """
+        KeyVaultDisablesPublicNetworkAccess unique logic.
+        public_network_access_enabled default value is True (when creating it)
+        If it False - check pass
+        Otherwise, we check that network_acls configure exists.
+
+        """
+        network_acls_key = "network_acls"
+        if self.get_inspected_key() in conf:
path: checkov/terraform/checks/resource/azure/KeyVaultDisablesPublicNetworkAccess.py
text: @user1:
You can drop this condition, and the logic will remain the same