From 57aa36504d53de8cf8c57c0a9880c487e1121b59 Mon Sep 17 00:00:00 2001 From: marynaKK Date: Thu, 22 Sep 2022 09:31:23 +0300 Subject: [PATCH 1/2] fix of check CKV2_AZURE_8 --- ...StorageContainerActivityLogsNotPublic.yaml | 89 ++++++++----- .../expected.yaml | 13 +- .../main.tf | 122 ++++++++++++++---- 3 files changed, 165 insertions(+), 59 deletions(-) diff --git a/checkov/terraform/checks/graph_checks/azure/StorageContainerActivityLogsNotPublic.yaml b/checkov/terraform/checks/graph_checks/azure/StorageContainerActivityLogsNotPublic.yaml index c9ef7522b76..c57a6673473 100644 --- a/checkov/terraform/checks/graph_checks/azure/StorageContainerActivityLogsNotPublic.yaml +++ b/checkov/terraform/checks/graph_checks/azure/StorageContainerActivityLogsNotPublic.yaml @@ -3,38 +3,69 @@ metadata: id: "CKV2_AZURE_8" category: "LOGGING" definition: - and: - - cond_type: filter - attribute: resource_type - value: - - azurerm_storage_account - operator: within - - cond_type: connection - resource_types: - - azurerm_monitor_activity_log_alert - connected_resource_types: - - azurerm_storage_account - operator: exists - - cond_type: attribute - resource_types: - - azurerm_monitor_activity_log_alert - attribute: criteria.resource_id - operator: exists - - or: - - cond_type: attribute + or: + - and: + - cond_type: filter + attribute: resource_type + value: + - azurerm_storage_container + operator: within + + - cond_type: connection + resource_types: + - azurerm_storage_container + connected_resource_types: + - azurerm_storage_account + operator: exists + + - or: + - cond_type: connection resource_types: + - azurerm_storage_account + connected_resource_types: - azurerm_monitor_activity_log_alert - attribute: enabled operator: not_exists + + - and: + - cond_type: connection + resource_types: + - azurerm_storage_account + connected_resource_types: + - azurerm_monitor_activity_log_alert + operator: exists + + - cond_type: attribute + resource_types: + - azurerm_monitor_activity_log_alert + attribute: enabled + operator: equals + value: false + + - and: + - cond_type: filter + attribute: resource_type + value: + - azurerm_storage_container + operator: within + + - cond_type: connection + resource_types: + - azurerm_storage_account + connected_resource_types: + - azurerm_storage_container + - azurerm_monitor_activity_log_alert + operator: exists + + - or: - cond_type: attribute resource_types: - - azurerm_monitor_activity_log_alert - attribute: enabled + - azurerm_storage_container + attribute: container_access_type + operator: not_exists + + - cond_type: attribute + resource_types: + - azurerm_storage_container + attribute: container_access_type operator: equals - value: true - - cond_type: connection - resource_types: - - azurerm_storage_container - connected_resource_types: - - azurerm_storage_account - operator: exists \ No newline at end of file + value: private diff --git a/tests/terraform/graph/checks/resources/StorageContainerActivityLogsNotPublic/expected.yaml b/tests/terraform/graph/checks/resources/StorageContainerActivityLogsNotPublic/expected.yaml index c7b6c9cad7c..0b1074a5ba7 100644 --- a/tests/terraform/graph/checks/resources/StorageContainerActivityLogsNotPublic/expected.yaml +++ b/tests/terraform/graph/checks/resources/StorageContainerActivityLogsNotPublic/expected.yaml @@ -1,5 +1,12 @@ pass: - - "azurerm_storage_account.ok_account_1" - - "azurerm_storage_account.ok_account_2" + - "azurerm_storage_container.ok_container_log_enabled_by_default" + - "azurerm_storage_container.ok_container_log_enabled" + - "azurerm_storage_container.ok_container_log_disabled" + - "azurerm_storage_container.ok_container_log_enabled_by_default_2" + - "azurerm_storage_container.ok_container_log_enabled_2" + - "azurerm_storage_container.ok_container_log_disabled_2" + - "azurerm_storage_container.ok_container_log_disabled_3" + - "azurerm_storage_container.ok_container_4" fail: - - "azurerm_storage_account.not_ok_account" \ No newline at end of file + - "azurerm_storage_container.not_ok_container_log_enabled_by_default" + - "azurerm_storage_container.not_ok_container_log_enabled" diff --git a/tests/terraform/graph/checks/resources/StorageContainerActivityLogsNotPublic/main.tf b/tests/terraform/graph/checks/resources/StorageContainerActivityLogsNotPublic/main.tf index a76a3153626..81054ef6597 100644 --- a/tests/terraform/graph/checks/resources/StorageContainerActivityLogsNotPublic/main.tf +++ b/tests/terraform/graph/checks/resources/StorageContainerActivityLogsNotPublic/main.tf @@ -1,19 +1,20 @@ -resource "azurerm_storage_container" "ok_container_1" { +# -------------------------------------------------------------------- # +# default in azurerm_monitor_activity_log_alert is logging enabled +resource "azurerm_storage_container" "ok_container_log_enabled_by_default" { name = "vhds" storage_account_name = azurerm_storage_account.ok_account_1.name container_access_type = "private" } -resource "azurerm_storage_container" "ok_container_2" { +resource "azurerm_storage_container" "ok_container_log_enabled_by_default_2" { name = "vhds" - storage_account_name = azurerm_storage_account.ok_account_2.name - container_access_type = "private" + storage_account_name = azurerm_storage_account.ok_account_1.name } -resource "azurerm_storage_container" "not_ok_container" { +resource "azurerm_storage_container" "not_ok_container_log_enabled_by_default" { name = "vhds" - storage_account_name = azurerm_storage_account.not_ok_account.name - container_access_type = "private" + storage_account_name = azurerm_storage_account.ok_account_1.name + container_access_type = "blob" } resource "azurerm_storage_account" "ok_account_1" { @@ -24,22 +25,6 @@ resource "azurerm_storage_account" "ok_account_1" { account_replication_type = "GRS" } -resource "azurerm_storage_account" "ok_account_2" { - name = "examplesa" - resource_group_name = azurerm_resource_group.main.name - location = azurerm_resource_group.main.location - account_tier = "Standard" - account_replication_type = "GRS" -} - -resource "azurerm_storage_account" "not_ok_account" { - name = "examplesa" - resource_group_name = azurerm_resource_group.main.name - location = azurerm_resource_group.main.location - account_tier = "Standard" - account_replication_type = "GRS" -} - resource "azurerm_monitor_activity_log_alert" "ok_monitor_activity_log_alert_1" { name = "example-activitylogalert" resource_group_name = azurerm_resource_group.main.name @@ -52,7 +37,6 @@ resource "azurerm_monitor_activity_log_alert" "ok_monitor_activity_log_alert_1" category = "Recommendation" } - action { action_group_id = azurerm_monitor_action_group.main.id @@ -62,6 +46,33 @@ resource "azurerm_monitor_activity_log_alert" "ok_monitor_activity_log_alert_1" } } +# -------------------------------------------------------------------- # +# if log is enabled explicitly +resource "azurerm_storage_container" "ok_container_log_enabled" { + name = "vhds" + storage_account_name = azurerm_storage_account.ok_account_2.name + container_access_type = "private" +} + +resource "azurerm_storage_container" "ok_container_log_enabled_2" { + name = "vhds" + storage_account_name = azurerm_storage_account.ok_account_2.name +} + +resource "azurerm_storage_container" "not_ok_container_log_enabled" { + name = "vhds" + storage_account_name = azurerm_storage_account.ok_account_2.name + container_access_type = "blob" +} + +resource "azurerm_storage_account" "ok_account_2" { + name = "examplesa" + resource_group_name = azurerm_resource_group.main.name + location = azurerm_resource_group.main.location + account_tier = "Standard" + account_replication_type = "GRS" +} + resource "azurerm_monitor_activity_log_alert" "ok_monitor_activity_log_alert_2" { name = "example-activitylogalert" resource_group_name = azurerm_resource_group.main.name @@ -75,7 +86,6 @@ resource "azurerm_monitor_activity_log_alert" "ok_monitor_activity_log_alert_2" category = "Recommendation" } - action { action_group_id = azurerm_monitor_action_group.main.id @@ -85,7 +95,35 @@ resource "azurerm_monitor_activity_log_alert" "ok_monitor_activity_log_alert_2" } } -resource "azurerm_monitor_activity_log_alert" "not_ok_monitor_activity_log_alert" { +# -------------------------------------------------------------------- # +# logging disabled - doesn't care if container private or not + +resource "azurerm_storage_container" "ok_container_log_disabled_3" { + name = "vhds" + storage_account_name = azurerm_storage_account.ok_account_3.name + container_access_type = "blob" +} + +resource "azurerm_storage_container" "ok_container_log_disabled" { + name = "vhds" + storage_account_name = azurerm_storage_account.ok_account_3.name + container_access_type = "private" +} + +resource "azurerm_storage_container" "ok_container_log_disabled_2" { + name = "vhds" + storage_account_name = azurerm_storage_account.ok_account_3.name +} + +resource "azurerm_storage_account" "ok_account_3" { + name = "examplesa" + resource_group_name = azurerm_resource_group.main.name + location = azurerm_resource_group.main.location + account_tier = "Standard" + account_replication_type = "GRS" +} + +resource "azurerm_monitor_activity_log_alert" "not_enabled_monitor_activity_log_alert" { name = "example-activitylogalert" resource_group_name = azurerm_resource_group.main.name scopes = [azurerm_resource_group.main.id] @@ -93,7 +131,7 @@ resource "azurerm_monitor_activity_log_alert" "not_ok_monitor_activity_log_alert enabled = false criteria { - resource_id = azurerm_storage_account.not_ok_account.id + resource_id = azurerm_storage_account.ok_account_3.id operation_name = "Microsoft.Storage/storageAccounts/write" category = "Recommendation" } @@ -105,4 +143,34 @@ resource "azurerm_monitor_activity_log_alert" "not_ok_monitor_activity_log_alert from = "terraform" } } +} + +# -------------------------------------------------------------------- # +# container with no connection to logging at all - all good + +resource "azurerm_storage_container" "ok_container_4" { + name = "vhds" + storage_account_name = azurerm_storage_account.ok_account_4.name + container_access_type = "blob" +} + +resource "azurerm_storage_account" "ok_account_4" { + name = "examplesa" + resource_group_name = azurerm_resource_group.main.name + location = azurerm_resource_group.main.location + account_tier = "Standard" + account_replication_type = "GRS" +} + +# -------------------------------------------------------------------- # +# other resources +resource "azurerm_resource_group" "main" { + name = "okLegacyExample-resources" + location = "West Europe" +} + +resource "azurerm_monitor_action_group" "main" { + name = "CriticalAlertsAction" + resource_group_name = azurerm_resource_group.main.name + short_name = "p0action" } \ No newline at end of file From 8123af09ef8f9ff20efc133d223d36df441e43b1 Mon Sep 17 00:00:00 2001 From: marynaKK Date: Thu, 22 Sep 2022 12:19:00 +0300 Subject: [PATCH 2/2] improved as Anton suggested --- ...StorageContainerActivityLogsNotPublic.yaml | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/checkov/terraform/checks/graph_checks/azure/StorageContainerActivityLogsNotPublic.yaml b/checkov/terraform/checks/graph_checks/azure/StorageContainerActivityLogsNotPublic.yaml index c57a6673473..c9070441cd4 100644 --- a/checkov/terraform/checks/graph_checks/azure/StorageContainerActivityLogsNotPublic.yaml +++ b/checkov/terraform/checks/graph_checks/azure/StorageContainerActivityLogsNotPublic.yaml @@ -41,22 +41,7 @@ definition: operator: equals value: false - - and: - - cond_type: filter - attribute: resource_type - value: - - azurerm_storage_container - operator: within - - - cond_type: connection - resource_types: - - azurerm_storage_account - connected_resource_types: - - azurerm_storage_container - - azurerm_monitor_activity_log_alert - operator: exists - - - or: + - or: - cond_type: attribute resource_types: - azurerm_storage_container @@ -68,4 +53,4 @@ definition: - azurerm_storage_container attribute: container_access_type operator: equals - value: private + value: private \ No newline at end of file