From 00b4fea7740b627a1c0b02c707ced4ce2244cb61 Mon Sep 17 00:00:00 2001 From: paddymorgan84 Date: Tue, 8 Oct 2024 13:16:48 +0000 Subject: [PATCH] fix: Update CKV_AZURE_164 to correct check on trust policy --- .../resource/azure/ACRUseSignedImages.py | 28 +++++++--- .../azure/example_ACRGeoreplicated/main.tf | 52 ++++++++----------- .../azure/example_ACRUseSignedImages/main.tf | 21 +++++++- .../resource/azure/test_ACRUseSignedImages.py | 6 ++- 4 files changed, 66 insertions(+), 41 deletions(-) diff --git a/checkov/terraform/checks/resource/azure/ACRUseSignedImages.py b/checkov/terraform/checks/resource/azure/ACRUseSignedImages.py index 200ac6e9c79..c7b412c78ba 100644 --- a/checkov/terraform/checks/resource/azure/ACRUseSignedImages.py +++ b/checkov/terraform/checks/resource/azure/ACRUseSignedImages.py @@ -1,14 +1,15 @@ from __future__ import annotations -from checkov.common.models.enums import CheckCategories -from checkov.terraform.checks.resource.base_resource_value_check import BaseResourceValueCheck +from typing import Dict, List, Any +from checkov.common.models.enums import CheckResult, CheckCategories +from checkov.terraform.checks.resource.base_resource_value_check import BaseResourceCheck -class ACRUseSignedImages(BaseResourceValueCheck): +class ACRUseSignedImages(BaseResourceCheck): def __init__(self): - # IN ARM - Set properties.trustPolicy.status to enabled, set - # properties.trustPolicy.type to Notary + # IN ARM - Set properties.policies.trustPolicy.status to enabled, set + # properties.policies.trustPolicy.type to Notary # This is the default behaviour by the tf provider when the trust policy is enabled name = "Ensures that ACR uses signed/trusted images" id = "CKV_AZURE_164" @@ -16,8 +17,21 @@ def __init__(self): categories = (CheckCategories.GENERAL_SECURITY,) super().__init__(name=name, id=id, categories=categories, supported_resources=supported_resources) - def get_inspected_key(self): - return "trust_policy/[0]/enabled" + def scan_resource_conf(self, conf: Dict[str, List[Any]]) -> CheckResult: + if 'trust_policy_enabled' in conf: + trust_policy_enabled = conf.get('trust_policy_enabled') + if isinstance(trust_policy_enabled, list) and trust_policy_enabled == [True]: + return CheckResult.PASSED + + if 'trust_policy' in conf: + trust_policy = conf['trust_policy'][0] + if isinstance(trust_policy, dict) and trust_policy.get('enabled') == [True]: + return CheckResult.PASSED + + return CheckResult.FAILED + + def get_evaluated_keys(self) -> List[str]: + return ['trust_policy_enabled', 'trust_policy/enabled'] check = ACRUseSignedImages() diff --git a/tests/terraform/checks/resource/azure/example_ACRGeoreplicated/main.tf b/tests/terraform/checks/resource/azure/example_ACRGeoreplicated/main.tf index c650211a430..68a01e9ce70 100644 --- a/tests/terraform/checks/resource/azure/example_ACRGeoreplicated/main.tf +++ b/tests/terraform/checks/resource/azure/example_ACRGeoreplicated/main.tf @@ -1,12 +1,10 @@ resource "azurerm_container_registry" "fail" { - name = var.acr.name - resource_group_name = var.acr.resource_group_name - location = var.acr.location - sku = "Basic" - anonymous_pull_enabled = var.anonymous_pull_enabled - trust_policy { - enabled = var.trust_policy_enabled - } + name = var.acr.name + resource_group_name = var.acr.resource_group_name + location = var.acr.location + sku = "Basic" + anonymous_pull_enabled = var.anonymous_pull_enabled + trust_policy_enabled = var.trust_policy_enabled public_network_access_enabled = var.public_network_access @@ -22,13 +20,11 @@ resource "azurerm_container_registry" "fail" { } resource "azurerm_container_registry" "fail2" { - name = var.acr.name - resource_group_name = var.acr.resource_group_name - location = var.acr.location - anonymous_pull_enabled = var.anonymous_pull_enabled - trust_policy { - enabled = var.trust_policy_enabled - } + name = var.acr.name + resource_group_name = var.acr.resource_group_name + location = var.acr.location + anonymous_pull_enabled = var.anonymous_pull_enabled + trust_policy_enabled = var.trust_policy_enabled public_network_access_enabled = var.public_network_access @@ -44,25 +40,21 @@ resource "azurerm_container_registry" "fail2" { } resource "azurerm_container_registry" "fail3" { - name = var.acr.name - resource_group_name = var.acr.resource_group_name - location = var.acr.location - anonymous_pull_enabled = var.anonymous_pull_enabled - trust_policy { - enabled = var.trust_policy_enabled - } + name = var.acr.name + resource_group_name = var.acr.resource_group_name + location = var.acr.location + anonymous_pull_enabled = var.anonymous_pull_enabled + trust_policy_enabled = var.trust_policy_enabled sku = "Premium" public_network_access_enabled = var.public_network_access } resource "azurerm_container_registry" "pass" { - name = var.acr.name - resource_group_name = var.acr.resource_group_name - location = var.acr.location - anonymous_pull_enabled = var.anonymous_pull_enabled - trust_policy { - enabled = var.trust_policy_enabled - } + name = var.acr.name + resource_group_name = var.acr.resource_group_name + location = var.acr.location + anonymous_pull_enabled = var.anonymous_pull_enabled + trust_policy_enabled = var.trust_policy_enabled sku = "Premium" public_network_access_enabled = var.public_network_access georeplications { @@ -71,4 +63,4 @@ resource "azurerm_container_registry" "pass" { zone_redundancy_enabled = var.georeplications.value["zone_redundancy_enabled"] tags = var.georeplications.value["tags"] } -} \ No newline at end of file +} diff --git a/tests/terraform/checks/resource/azure/example_ACRUseSignedImages/main.tf b/tests/terraform/checks/resource/azure/example_ACRUseSignedImages/main.tf index fa6662c5213..abadf97a181 100644 --- a/tests/terraform/checks/resource/azure/example_ACRUseSignedImages/main.tf +++ b/tests/terraform/checks/resource/azure/example_ACRUseSignedImages/main.tf @@ -1,5 +1,14 @@ -resource "azurerm_container_registry" "pass" { +resource "azurerm_container_registry" "pass_new" { + name = "containerRegistry1" + resource_group_name = azurerm_resource_group.rg.name + location = azurerm_resource_group.rg.location + sku = "Premium" + anonymous_pull_enabled = false + trust_policy_enabled = true +} + +resource "azurerm_container_registry" "pass_old" { name = "containerRegistry1" resource_group_name = azurerm_resource_group.rg.name location = azurerm_resource_group.rg.location @@ -18,7 +27,15 @@ resource "azurerm_container_registry" "fail" { } -resource "azurerm_container_registry" "fail2" { +resource "azurerm_container_registry" "fail2_new" { + name = "containerRegistry1" + resource_group_name = azurerm_resource_group.rg.name + location = azurerm_resource_group.rg.location + sku = "Standard" + trust_policy_enabled = false +} + +resource "azurerm_container_registry" "fail2_old" { name = "containerRegistry1" resource_group_name = azurerm_resource_group.rg.name location = azurerm_resource_group.rg.location diff --git a/tests/terraform/checks/resource/azure/test_ACRUseSignedImages.py b/tests/terraform/checks/resource/azure/test_ACRUseSignedImages.py index 6b783fe96b4..b354efeefad 100644 --- a/tests/terraform/checks/resource/azure/test_ACRUseSignedImages.py +++ b/tests/terraform/checks/resource/azure/test_ACRUseSignedImages.py @@ -18,11 +18,13 @@ def test(self): summary = report.get_summary() passing_resources = { - 'azurerm_container_registry.pass', + 'azurerm_container_registry.pass_new', + 'azurerm_container_registry.pass_old', } failing_resources = { 'azurerm_container_registry.fail', - 'azurerm_container_registry.fail2' + 'azurerm_container_registry.fail2_new', + 'azurerm_container_registry.fail2_old' } skipped_resources = {}