bridgecrewio / checkov

Prevent cloud misconfigurations and find vulnerabilities during build-time in infrastructure as code, container images and open source packages with Checkov by Bridgecrew.
https://www.checkov.io/
Apache License 2.0
7.13k stars 1.12k forks source link

CKV_AZURE_23 - Reports incorrect fail when using azurerm_mssql_server_extended_auditing_policy #1035

Closed Marcus-James-Adams closed 3 years ago

Marcus-James-Adams commented 3 years ago

Check: CKV_AZURE_23: "Ensure that 'Auditing' is set to 'On' for SQL servers" FAILED for resource: azurerm_mssql_server.server File: /sql_server.tf:19-37 Guide: https://docs.bridgecrew.io/docs/bc_azr_logging_2

            19 | resource "azurerm_mssql_server" "server" {
            20 |   name                          = "${var.org_short}-${var.env_short}-${var.loc_short}-${var.service}-mssql"
            21 |   location                      = var.location
            22 |   resource_group_name           = azurerm_resource_group.server.name
            23 |   version                       = var.sql_server_server_version
            24 |   administrator_login           = "${var.org_short}-${var.env_short}-${var.loc_short}-${var.service}-mssql-admin"
            25 |   administrator_login_password  = random_password.server.result
            26 |   minimum_tls_version           = var.min_tls_version
            27 |   public_network_access_enabled = false
            28 |   azuread_administrator {
            29 |     login_username = "AzureAD Admin"
            30 |     object_id      = data.azuread_group.sqldba.object_id
            31 |     tenant_id      = data.azurerm_client_config.current.tenant_id
            32 |   }
            33 |   identity {
            34 |     type = "SystemAssigned"
            35 |   }
            36 |   tags = local.default_tags
            37 | }

When using Checkov v1.0.861

However The auditing policy is set by the separate resource azurerm_mssql_server_extended_auditing_policy

resource "azurerm_mssql_server_extended_auditing_policy" "server" {
  server_id                                                     = azurerm_mssql_server.server.id
  log_monitoring_enabled                            = true
  storage_endpoint                                       = azurerm_storage_account.server.primary_blob_endpoint
  storage_account_access_key                       = azurerm_storage_account.server.primary_access_key
  storage_account_access_key_is_secondary = true
  retention_in_days                                        = var.log_rention_policy_days
}
daniel-shuy commented 3 years ago

azurerm_mssql_server allows setting the Extended Auditing Policy using either the extended_auditing_policy argument or the mssql_server_extended_auditing_policy resource. Currently checkov only supports the former use case.

Supporting mssql_server_extended_auditing_policy will be difficult to implement, as each check is run on each resource, and the check registry does not pass/expose all the resources/entities to the checks.

daniel-shuy commented 3 years ago

I see that in #155 that the feature to correlate multiple resources has been added in #1023, is there any example of how to do so? The #1023 PR is too big to go through

daniel-shuy commented 3 years ago

I see that in #155 that the feature to correlate multiple resources has been added in #1023, is there any example of how to do so? The #1023 PR is too big to go through

Nevermind, after going through the code base I realized that the YAML based checks supports correlating multiple resources. I've created a PR (#1818) to support this.