From 559e8c6a93b77f7b89578affc4653a164b6903de Mon Sep 17 00:00:00 2001 From: Mathew Payne <2772944+GeekMasher@users.noreply.github.com> Date: Tue, 24 Sep 2024 14:29:40 +0000 Subject: [PATCH 01/12] feat(tf): Add initial StorageAccount support --- ql/lib/codeql/hcl/providers/Azure.qll | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/ql/lib/codeql/hcl/providers/Azure.qll b/ql/lib/codeql/hcl/providers/Azure.qll index be1a6a5..a540e5f 100644 --- a/ql/lib/codeql/hcl/providers/Azure.qll +++ b/ql/lib/codeql/hcl/providers/Azure.qll @@ -93,6 +93,22 @@ module Azure { Expr getProperty(string name) { result = this.getProperties().getElementByName(name) } } + class StorageAccount extends AzureResource { + StorageAccount() { this.getResourceType() = "azurerm_storage_account" } + + boolean getEnableHttpsTrafficOnly() { + result = this.getAttribute("enable_https_traffic_only").(BooleanLiteral).getBool() + } + + boolean getPublicNetworkAccess() { + result = this.getAttribute("public_network_access_enabled").(BooleanLiteral).getBool() + } + + boolean getAllowNestedItemsToBePublic() { + result = this.getAttribute("allow_nested_items_to_be_public").(BooleanLiteral).getBool() + } + } + /** * Azure Databases */ From 28703f35c3542b4afa90e42237736508d6c6ad0d Mon Sep 17 00:00:00 2001 From: Mathew Payne <2772944+GeekMasher@users.noreply.github.com> Date: Tue, 24 Sep 2024 14:36:30 +0000 Subject: [PATCH 02/12] feat(tf): Add Public Storage abstraction --- ql/lib/codeql/hcl/security/PublicStorage.qll | 22 ++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 ql/lib/codeql/hcl/security/PublicStorage.qll diff --git a/ql/lib/codeql/hcl/security/PublicStorage.qll b/ql/lib/codeql/hcl/security/PublicStorage.qll new file mode 100644 index 0000000..82e1cd6 --- /dev/null +++ b/ql/lib/codeql/hcl/security/PublicStorage.qll @@ -0,0 +1,22 @@ +import iac + +abstract class PublicStorage extends Expr { } + +/** + * Azure Public Storage. + */ +class AzurePublicStorage extends PublicStorage { + AzurePublicStorage() { + // Azure Storage Container + exists(Azure::StorageContainer storage_container | + storage_container.getContainerAccessType() = "blob" and + storage_container.getProperty("publicAccess").(StringLiteral).getValue() = "blob" + ) + or + // Azure Storage Accounts (v3) + exists(Azure::StorageAccount storage_acount | + storage_acount.getPublicNetworkAccess() = true or + storage_acount.getAllowNestedItemsToBePublic() = true + ) + } +} From a72077f6ddf903b08a280f22716785c0d658cec2 Mon Sep 17 00:00:00 2001 From: Mathew Payne <2772944+GeekMasher@users.noreply.github.com> Date: Wed, 25 Sep 2024 09:38:24 +0000 Subject: [PATCH 03/12] feat(tf): Improve resource, update Azure provider, and Public storage query --- ql/lib/codeql/hcl/Resources.qll | 8 ++++++++ ql/lib/codeql/hcl/providers/Azure.qll | 12 ++++++++++++ ql/lib/codeql/hcl/security/PublicStorage.qll | 8 ++++++-- .../Terraform/Azure/ManagedDisk/PublicAccess.ql | 8 +++----- 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/ql/lib/codeql/hcl/Resources.qll b/ql/lib/codeql/hcl/Resources.qll index b418b0a..4306264 100644 --- a/ql/lib/codeql/hcl/Resources.qll +++ b/ql/lib/codeql/hcl/Resources.qll @@ -10,8 +10,16 @@ private import codeql.hcl.AST class Resource extends Block { Resource() { this.hasType("resource") } + /** + * Get the name of the resource. + */ string getName() { result = this.getLabel(1) } + /** + * Get the provider of the resource. + */ + string getProvider() { result = "Unknown Provider" } + /** * Returns the resource id. */ diff --git a/ql/lib/codeql/hcl/providers/Azure.qll b/ql/lib/codeql/hcl/providers/Azure.qll index a540e5f..b2ef0ea 100644 --- a/ql/lib/codeql/hcl/providers/Azure.qll +++ b/ql/lib/codeql/hcl/providers/Azure.qll @@ -10,6 +10,8 @@ module Azure { */ class AzureResource extends Resource, Block { AzureResource() { this.getResourceType().regexpMatch("^azurerm.*") } + + override string getProvider() { result = "Azurerm" } } /** @@ -78,6 +80,11 @@ module Azure { class StorageContainer extends AzureResource { StorageContainer() { this.getResourceType() = "azurerm_storage_container" } + /** + * Get the name of the storage container. + */ + override string getName() { result = this.getAttribute("name").(StringLiteral).getValue() } + string getContainerAccessType() { result = this.getAttribute("container_access_type").(StringLiteral).getValue() } @@ -96,6 +103,11 @@ module Azure { class StorageAccount extends AzureResource { StorageAccount() { this.getResourceType() = "azurerm_storage_account" } + /** + * Get the name of the storage account. + */ + override string getName() { result = this.getAttribute("name").(StringLiteral).getValue() } + boolean getEnableHttpsTrafficOnly() { result = this.getAttribute("enable_https_traffic_only").(BooleanLiteral).getBool() } diff --git a/ql/lib/codeql/hcl/security/PublicStorage.qll b/ql/lib/codeql/hcl/security/PublicStorage.qll index 82e1cd6..6b01bef 100644 --- a/ql/lib/codeql/hcl/security/PublicStorage.qll +++ b/ql/lib/codeql/hcl/security/PublicStorage.qll @@ -1,11 +1,13 @@ import iac -abstract class PublicStorage extends Expr { } +abstract class PublicStorage extends Expr { + abstract string getName(); +} /** * Azure Public Storage. */ -class AzurePublicStorage extends PublicStorage { +class AzurePublicStorage extends Azure::AzureResource, PublicStorage { AzurePublicStorage() { // Azure Storage Container exists(Azure::StorageContainer storage_container | @@ -19,4 +21,6 @@ class AzurePublicStorage extends PublicStorage { storage_acount.getAllowNestedItemsToBePublic() = true ) } + + override string getName() { result = this.getName() } } diff --git a/ql/src/security/Terraform/Azure/ManagedDisk/PublicAccess.ql b/ql/src/security/Terraform/Azure/ManagedDisk/PublicAccess.ql index 8867e18..fe6438b 100644 --- a/ql/src/security/Terraform/Azure/ManagedDisk/PublicAccess.ql +++ b/ql/src/security/Terraform/Azure/ManagedDisk/PublicAccess.ql @@ -13,10 +13,8 @@ */ import hcl +import codeql.hcl.security.PublicStorage // https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/managed_disk -from Azure::StorageContainer managed_disk -where - managed_disk.getContainerAccessType() = "blob" and - managed_disk.getProperty("publicAccess").(StringLiteral).getValue() = "blob" -select managed_disk, "Azure Storage is Unencrypted for '" + managed_disk.getName() + "'" +from AzurePublicStorage public_storage +select public_storage, "Azure Storage is Public for '" + public_storage.getName() + "'" From 4104c729662d1a659c05e4d5493e554b240f45d3 Mon Sep 17 00:00:00 2001 From: Mathew Payne <2772944+GeekMasher@users.noreply.github.com> Date: Wed, 25 Sep 2024 15:29:46 +0000 Subject: [PATCH 04/12] feat(tf): Update Azure provider and PublicStorage --- ql/lib/codeql/hcl/Resources.qll | 3 ++- ql/lib/codeql/hcl/Terraform.qll | 14 +++++++++++ ql/lib/codeql/hcl/providers/Azure.qll | 25 ++++++++++++++++++- ql/lib/codeql/hcl/security/PublicStorage.qll | 5 +++- .../Azure/Storage/PublicAccess/storage.tf | 13 ++++++++++ 5 files changed, 57 insertions(+), 3 deletions(-) diff --git a/ql/lib/codeql/hcl/Resources.qll b/ql/lib/codeql/hcl/Resources.qll index 4306264..1e05fda 100644 --- a/ql/lib/codeql/hcl/Resources.qll +++ b/ql/lib/codeql/hcl/Resources.qll @@ -1,5 +1,6 @@ private import codeql.Locations private import codeql.hcl.AST +private import codeql.hcl.Terraform::Terraform // Resources are the most important element in the Terraform language. // Each resource block describes one or more infrastructure objects, such as @@ -18,7 +19,7 @@ class Resource extends Block { /** * Get the provider of the resource. */ - string getProvider() { result = "Unknown Provider" } + RequiredProvider getProvider() { none() } /** * Returns the resource id. diff --git a/ql/lib/codeql/hcl/Terraform.qll b/ql/lib/codeql/hcl/Terraform.qll index a0c6cb7..9a72dc2 100644 --- a/ql/lib/codeql/hcl/Terraform.qll +++ b/ql/lib/codeql/hcl/Terraform.qll @@ -1,5 +1,6 @@ private import codeql.files.FileSystem private import codeql.hcl.AST +private import codeql.iac.Dependencies private import Resources module Terraform { @@ -42,12 +43,21 @@ module Terraform { */ abstract string getVersion(); + /** + * Gets the semantic version of the provider. + */ + abstract SemanticVersion getSemanticVersion(); + /** * Gets the source of the provider. */ abstract string getSource(); } + RequiredProvider getProviderByName(string name) { + exists(RequiredProvider provider | provider.getName() = name) + } + /** * Basic Terraform required provider String. */ @@ -62,6 +72,8 @@ module Terraform { override string getVersion() { result = this.getValue() } + override SemanticVersion getSemanticVersion() { result = this.getValue() } + /** * Basic providers are assumed to be from the Hashicorp namespace. */ @@ -93,5 +105,7 @@ module Terraform { override string getVersion() { result = this.getElementByName("version").(StringLiteral).getValue() } + + override SemanticVersion getSemanticVersion() { result = this.getVersion() } } } diff --git a/ql/lib/codeql/hcl/providers/Azure.qll b/ql/lib/codeql/hcl/providers/Azure.qll index b2ef0ea..0c1af52 100644 --- a/ql/lib/codeql/hcl/providers/Azure.qll +++ b/ql/lib/codeql/hcl/providers/Azure.qll @@ -1,6 +1,7 @@ private import codeql.hcl.AST private import codeql.hcl.Resources private import codeql.hcl.Constants +private import codeql.hcl.Terraform::Terraform module Azure { /** @@ -11,7 +12,7 @@ module Azure { class AzureResource extends Resource, Block { AzureResource() { this.getResourceType().regexpMatch("^azurerm.*") } - override string getProvider() { result = "Azurerm" } + override RequiredProvider getProvider() { result = getProviderByName("azurerm") } } /** @@ -108,14 +109,36 @@ module Azure { */ override string getName() { result = this.getAttribute("name").(StringLiteral).getValue() } + /** + * Get the `allow_blob_public_access` property of the storage account. Only available + * for `azurerm` v2 and not v3 onwards. + * + * https://github.com/hashicorp/terraform-provider-azurerm/blob/main/CHANGELOG-v3.md + */ + boolean getAllowBlobPublicAccess() { + this.getProvider().getSemanticVersion().maybeBefore("3.0.0") and + result = this.getAttribute("allow_blob_public_access").(BooleanLiteral).getBool() + or + result = false + } + + /** + * Get the `public_network_access_enabled` property of the storage account. + */ boolean getEnableHttpsTrafficOnly() { result = this.getAttribute("enable_https_traffic_only").(BooleanLiteral).getBool() } + /** + * Get the `public_network_access_enabled` property of the storage account. + */ boolean getPublicNetworkAccess() { result = this.getAttribute("public_network_access_enabled").(BooleanLiteral).getBool() } + /** + * Get the `allow_nested_items_to_be_public` property of the storage account. + */ boolean getAllowNestedItemsToBePublic() { result = this.getAttribute("allow_nested_items_to_be_public").(BooleanLiteral).getBool() } diff --git a/ql/lib/codeql/hcl/security/PublicStorage.qll b/ql/lib/codeql/hcl/security/PublicStorage.qll index 6b01bef..2b4d3c4 100644 --- a/ql/lib/codeql/hcl/security/PublicStorage.qll +++ b/ql/lib/codeql/hcl/security/PublicStorage.qll @@ -15,8 +15,11 @@ class AzurePublicStorage extends Azure::AzureResource, PublicStorage { storage_container.getProperty("publicAccess").(StringLiteral).getValue() = "blob" ) or - // Azure Storage Accounts (v3) + // Azure Storage Accounts exists(Azure::StorageAccount storage_acount | + // v2 + storage_acount.getAllowBlobPublicAccess() = true or + // v3 storage_acount.getPublicNetworkAccess() = true or storage_acount.getAllowNestedItemsToBePublic() = true ) diff --git a/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/storage.tf b/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/storage.tf index 615414a..2e7f811 100644 --- a/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/storage.tf +++ b/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/storage.tf @@ -13,3 +13,16 @@ resource "azurerm_storage_container" "insecure" { "publicAccess" = "blob" } } + +# insecure (v3) +resource "azurerm_storage_account" "insecure_storage_account" { + name = "insecure-storage-account" + location = var.location + account_kind = var.kind + account_tier = var.tier + account_replication_type = var.replication_type + resource_group_name = var.resource_group_name + public_network_access_enabled = true + allow_nested_items_to_be_public = true + min_tls_version = var.min_tls_version +} From e36714642f4fde911546f6cb25ee32b2d22ff58c Mon Sep 17 00:00:00 2001 From: Mathew Payne Date: Wed, 25 Sep 2024 16:41:41 +0100 Subject: [PATCH 05/12] fix: Update AST tests --- ql/test/library-tests/hcl/ast/AST.expected | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ql/test/library-tests/hcl/ast/AST.expected b/ql/test/library-tests/hcl/ast/AST.expected index c1cb30d..f68ba17 100644 --- a/ql/test/library-tests/hcl/ast/AST.expected +++ b/ql/test/library-tests/hcl/ast/AST.expected @@ -8,7 +8,7 @@ | sample.hcl:3:5:3:11 | Identifier | | sample.hcl:3:5:6:5 | ??? | | sample.hcl:3:5:6:5 | Attribute | -| sample.hcl:3:15:6:5 | Object | +| sample.hcl:3:15:6:5 | RequiredProvider azurerm | | sample.hcl:4:7:4:12 | Identifier | | sample.hcl:4:7:4:12 | source | | sample.hcl:4:7:4:35 | ObjectElement | From 5729f59170fbcc5b6bae75ed95860dc37f005ca7 Mon Sep 17 00:00:00 2001 From: Mathew Payne Date: Wed, 25 Sep 2024 17:21:52 +0100 Subject: [PATCH 06/12] feat(tf): Update PublicStorage and tests --- ql/lib/codeql/hcl/providers/Azure.qll | 34 ++++++++++++++----- ql/lib/codeql/hcl/security/PublicStorage.qll | 26 +++++++++----- .../Azure/ManagedDisk/PublicAccess.ql | 2 +- .../PublicAccess/PublicAccess.expected | 3 +- 4 files changed, 46 insertions(+), 19 deletions(-) diff --git a/ql/lib/codeql/hcl/providers/Azure.qll b/ql/lib/codeql/hcl/providers/Azure.qll index 0c1af52..be5cb3e 100644 --- a/ql/lib/codeql/hcl/providers/Azure.qll +++ b/ql/lib/codeql/hcl/providers/Azure.qll @@ -109,38 +109,54 @@ module Azure { */ override string getName() { result = this.getAttribute("name").(StringLiteral).getValue() } + Expr getAllowBlobPublicAccess() { + this.getProvider().getSemanticVersion().maybeBefore("3.0.0") and + result = this.getAttribute("allow_blob_public_access") + } + /** * Get the `allow_blob_public_access` property of the storage account. Only available * for `azurerm` v2 and not v3 onwards. * * https://github.com/hashicorp/terraform-provider-azurerm/blob/main/CHANGELOG-v3.md */ - boolean getAllowBlobPublicAccess() { - this.getProvider().getSemanticVersion().maybeBefore("3.0.0") and - result = this.getAttribute("allow_blob_public_access").(BooleanLiteral).getBool() + boolean getAllowBlobPublicAccessValue() { + result = this.getAllowBlobPublicAccess().(BooleanLiteral).getBool() or result = false } + Expr getEnableHttpsTrafficOnly() { + result = this.getAttribute("enable_https_traffic_only") + } + /** * Get the `public_network_access_enabled` property of the storage account. */ - boolean getEnableHttpsTrafficOnly() { - result = this.getAttribute("enable_https_traffic_only").(BooleanLiteral).getBool() + boolean getEnableHttpsTrafficOnlyValue() { + result = this.getEnableHttpsTrafficOnly().(BooleanLiteral).getBool() + } + + Expr getPublicNetworkAccess() { + result = this.getAttribute("public_network_access_enabled") } /** * Get the `public_network_access_enabled` property of the storage account. */ - boolean getPublicNetworkAccess() { - result = this.getAttribute("public_network_access_enabled").(BooleanLiteral).getBool() + boolean getPublicNetworkAccessValue() { + result = this.getPublicNetworkAccess().(BooleanLiteral).getBool() + } + + Expr getAllowNestedItemsToBePublic() { + result = this.getAttribute("allow_nested_items_to_be_public") } /** * Get the `allow_nested_items_to_be_public` property of the storage account. */ - boolean getAllowNestedItemsToBePublic() { - result = this.getAttribute("allow_nested_items_to_be_public").(BooleanLiteral).getBool() + boolean getAllowNestedItemsToBePublicValue() { + result = this.getPublicNetworkAccess().(BooleanLiteral).getBool() } } diff --git a/ql/lib/codeql/hcl/security/PublicStorage.qll b/ql/lib/codeql/hcl/security/PublicStorage.qll index 2b4d3c4..9deb1c1 100644 --- a/ql/lib/codeql/hcl/security/PublicStorage.qll +++ b/ql/lib/codeql/hcl/security/PublicStorage.qll @@ -1,29 +1,39 @@ import iac abstract class PublicStorage extends Expr { - abstract string getName(); + abstract string getProvider(); } /** * Azure Public Storage. */ -class AzurePublicStorage extends Azure::AzureResource, PublicStorage { +class AzurePublicStorage extends PublicStorage { AzurePublicStorage() { // Azure Storage Container exists(Azure::StorageContainer storage_container | storage_container.getContainerAccessType() = "blob" and storage_container.getProperty("publicAccess").(StringLiteral).getValue() = "blob" + and + this = storage_container.getProperty("publicAccess") ) or // Azure Storage Accounts exists(Azure::StorageAccount storage_acount | - // v2 - storage_acount.getAllowBlobPublicAccess() = true or - // v3 - storage_acount.getPublicNetworkAccess() = true or - storage_acount.getAllowNestedItemsToBePublic() = true + ( + // v2 + storage_acount.getAllowBlobPublicAccessValue() = true and + this = storage_acount.getAllowBlobPublicAccess() + ) or + ( + // v3 + storage_acount.getAllowNestedItemsToBePublicValue() = true + and + this = storage_acount.getAllowNestedItemsToBePublic() + ) ) } - override string getName() { result = this.getName() } + override string getProvider() { + result = "Azure" + } } diff --git a/ql/src/security/Terraform/Azure/ManagedDisk/PublicAccess.ql b/ql/src/security/Terraform/Azure/ManagedDisk/PublicAccess.ql index fe6438b..ea00008 100644 --- a/ql/src/security/Terraform/Azure/ManagedDisk/PublicAccess.ql +++ b/ql/src/security/Terraform/Azure/ManagedDisk/PublicAccess.ql @@ -17,4 +17,4 @@ import codeql.hcl.security.PublicStorage // https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/managed_disk from AzurePublicStorage public_storage -select public_storage, "Azure Storage is Public for '" + public_storage.getName() + "'" +select public_storage, "Azure Storage is Public" diff --git a/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/PublicAccess.expected b/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/PublicAccess.expected index 51ad646..16a5240 100644 --- a/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/PublicAccess.expected +++ b/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/PublicAccess.expected @@ -1 +1,2 @@ -| storage.tf:9:1:15:1 | resource azurerm_storage_container insecure | Azure Storage is Unencrypted for 'insecure' | +| storage.tf:13:22:13:27 | blob | Azure Storage is Public | +| storage.tf:26:37:26:40 | true | Azure Storage is Public | From 34ce9fb9ea4069d95eae2a7027ff90ffcb02995e Mon Sep 17 00:00:00 2001 From: Mathew Payne Date: Wed, 25 Sep 2024 17:27:10 +0100 Subject: [PATCH 07/12] fix: Update to point to the resource --- ql/lib/codeql/hcl/security/PublicStorage.qll | 15 ++++++++++----- .../Storage/PublicAccess/PublicAccess.expected | 4 ++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/ql/lib/codeql/hcl/security/PublicStorage.qll b/ql/lib/codeql/hcl/security/PublicStorage.qll index 9deb1c1..a6f3d05 100644 --- a/ql/lib/codeql/hcl/security/PublicStorage.qll +++ b/ql/lib/codeql/hcl/security/PublicStorage.qll @@ -14,7 +14,7 @@ class AzurePublicStorage extends PublicStorage { storage_container.getContainerAccessType() = "blob" and storage_container.getProperty("publicAccess").(StringLiteral).getValue() = "blob" and - this = storage_container.getProperty("publicAccess") + this = storage_container ) or // Azure Storage Accounts @@ -23,13 +23,18 @@ class AzurePublicStorage extends PublicStorage { // v2 storage_acount.getAllowBlobPublicAccessValue() = true and this = storage_acount.getAllowBlobPublicAccess() - ) or + ) + or + ( + // v3 ( - // v3 + storage_acount.getPublicNetworkAccessValue() = true + or storage_acount.getAllowNestedItemsToBePublicValue() = true - and - this = storage_acount.getAllowNestedItemsToBePublic() ) + and + this = storage_acount + ) ) } diff --git a/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/PublicAccess.expected b/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/PublicAccess.expected index 16a5240..311bf33 100644 --- a/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/PublicAccess.expected +++ b/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/PublicAccess.expected @@ -1,2 +1,2 @@ -| storage.tf:13:22:13:27 | blob | Azure Storage is Public | -| storage.tf:26:37:26:40 | true | Azure Storage is Public | +| storage.tf:9:1:15:1 | resource azurerm_storage_container insecure-storage-container | Azure Storage is Public | +| storage.tf:18:1:28:1 | resource azurerm_storage_account insecure-storage-account | Azure Storage is Public | From ddb5c397f833c856b674e49e18fbe875fc2f0359 Mon Sep 17 00:00:00 2001 From: Mathew Payne Date: Thu, 26 Sep 2024 13:12:26 +0100 Subject: [PATCH 08/12] fix(tf): Update getProviderByName --- ql/lib/codeql/hcl/Terraform.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ql/lib/codeql/hcl/Terraform.qll b/ql/lib/codeql/hcl/Terraform.qll index 9a72dc2..eaf3b66 100644 --- a/ql/lib/codeql/hcl/Terraform.qll +++ b/ql/lib/codeql/hcl/Terraform.qll @@ -55,7 +55,7 @@ module Terraform { } RequiredProvider getProviderByName(string name) { - exists(RequiredProvider provider | provider.getName() = name) + exists(RequiredProvider provider | provider.getName() = name | result = provider) } /** From d1c1ce163240924851da3de9736998bda5adbcab Mon Sep 17 00:00:00 2001 From: Mathew Payne Date: Thu, 26 Sep 2024 13:39:34 +0100 Subject: [PATCH 09/12] feat(tf): Improve Azure provide --- ql/lib/codeql/hcl/providers/Azure.qll | 101 ++++++++++++++++++++++++-- 1 file changed, 96 insertions(+), 5 deletions(-) diff --git a/ql/lib/codeql/hcl/providers/Azure.qll b/ql/lib/codeql/hcl/providers/Azure.qll index be5cb3e..7513e01 100644 --- a/ql/lib/codeql/hcl/providers/Azure.qll +++ b/ql/lib/codeql/hcl/providers/Azure.qll @@ -109,6 +109,12 @@ module Azure { */ override string getName() { result = this.getAttribute("name").(StringLiteral).getValue() } + /** + * Get the `allow_blob_public_access` property of the storage account. Only available + * for `azurerm` v2 and not v3 onwards. + * + * https://github.com/hashicorp/terraform-provider-azurerm/blob/main/CHANGELOG-v3.md + */ Expr getAllowBlobPublicAccess() { this.getProvider().getSemanticVersion().maybeBefore("3.0.0") and result = this.getAttribute("allow_blob_public_access") @@ -121,43 +127,128 @@ module Azure { * https://github.com/hashicorp/terraform-provider-azurerm/blob/main/CHANGELOG-v3.md */ boolean getAllowBlobPublicAccessValue() { - result = this.getAllowBlobPublicAccess().(BooleanLiteral).getBool() + exists(Expr e | e = this.getAllowBlobPublicAccess() | result = e.(BooleanLiteral).getBool()) or - result = false + not exists(this.getAllowBlobPublicAccess()) and + result = true } + /** + * Get the `public_network_access_enabled` property of the storage account. + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#public_network_access_enabled + */ Expr getEnableHttpsTrafficOnly() { result = this.getAttribute("enable_https_traffic_only") } /** * Get the `public_network_access_enabled` property of the storage account. + * + * Defaults to `true`. + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#public_network_access_enabled */ boolean getEnableHttpsTrafficOnlyValue() { - result = this.getEnableHttpsTrafficOnly().(BooleanLiteral).getBool() + exists(Expr e | e = this.getEnableHttpsTrafficOnly() | result = e.(BooleanLiteral).getBool()) + or + not exists(this.getEnableHttpsTrafficOnly()) and + result = true } + /** + * Get the `public_network_access_enabled` property of the storage account. + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#public_network_access_enabled + */ Expr getPublicNetworkAccess() { result = this.getAttribute("public_network_access_enabled") } /** * Get the `public_network_access_enabled` property of the storage account. + * + * Defaults to `true`. + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#public_network_access_enabled */ boolean getPublicNetworkAccessValue() { - result = this.getPublicNetworkAccess().(BooleanLiteral).getBool() + exists(Expr e | e = this.getPublicNetworkAccess() | result = e.(BooleanLiteral).getBool()) + or + not exists(this.getPublicNetworkAccess()) and + result = true } + /** + * Get the `allow_nested_items_to_be_public` property of the storage account. + * + * Defaults to `true` + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#allow_nested_items_to_be_public + */ Expr getAllowNestedItemsToBePublic() { result = this.getAttribute("allow_nested_items_to_be_public") } /** * Get the `allow_nested_items_to_be_public` property of the storage account. + * + * Defaults to `true` + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#allow_nested_items_to_be_public */ boolean getAllowNestedItemsToBePublicValue() { - result = this.getPublicNetworkAccess().(BooleanLiteral).getBool() + exists(Expr e | e = this.getAllowNestedItemsToBePublic() | result = e.(BooleanLiteral).getBool()) + or + not exists(this.getAllowNestedItemsToBePublic()) and + result = true } + + /** + * Get the `https_traffic_only_enabled` property of the storage account. + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#https_traffic_only_enabled + */ + Expr getHttpsTrafficOnlyEnabled() { + result = this.getAttribute("https_traffic_only_enabled") + } + + /** + * Get the `https_traffic_only_enabled` property of the storage account. + * + * Defaults to `true` + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#https_traffic_only_enabled + */ + boolean getHttpsTrafficOnlyEnabledValue() { + exists(Expr e | e = this.getHttpsTrafficOnlyEnabled() | result = e.(BooleanLiteral).getBool()) + or + not exists(this.getHttpsTrafficOnlyEnabled()) and + result = true + } + + /** + * Get the `min_tls_version` property of the storage account. + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#min_tls_version + */ + Expr getMinTlsVersion() { + result = this.getAttribute("min_tls_version") + } + + /** + * Get the `min_tls_version` property of the storage account. + * + * Defaults to `TLS1_2` + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#min_tls_version + */ + string getMinTlsVersionValue() { + exists(Expr e | e = this.getMinTlsVersion() | result = e.(StringLiteral).getValue()) + or + not exists(this.getMinTlsVersion()) and + result = "TLS1_2" + } } /** From becfdf5183cca7dbb217ded64b60498fa1d15c07 Mon Sep 17 00:00:00 2001 From: Mathew Payne Date: Thu, 26 Sep 2024 14:01:36 +0100 Subject: [PATCH 10/12] feat(tf): Update and split out Azure provider - Doesn't break compatibility --- ql/lib/codeql/hcl/providers/Azure.qll | 289 +----------------- .../codeql/hcl/providers/azure/Databases.qll | 70 +++++ .../codeql/hcl/providers/azure/KeyVault.qll | 39 +++ .../hcl/providers/azure/SecurityCenter.qll | 26 ++ ql/lib/codeql/hcl/providers/azure/Storage.qll | 217 +++++++++++++ .../library-tests/hcl/azure/azure.expected | 1 + ql/test/library-tests/hcl/azure/storage.hcl | 12 + 7 files changed, 370 insertions(+), 284 deletions(-) create mode 100644 ql/lib/codeql/hcl/providers/azure/Databases.qll create mode 100644 ql/lib/codeql/hcl/providers/azure/KeyVault.qll create mode 100644 ql/lib/codeql/hcl/providers/azure/SecurityCenter.qll create mode 100644 ql/lib/codeql/hcl/providers/azure/Storage.qll diff --git a/ql/lib/codeql/hcl/providers/Azure.qll b/ql/lib/codeql/hcl/providers/Azure.qll index 7513e01..b4f7268 100644 --- a/ql/lib/codeql/hcl/providers/Azure.qll +++ b/ql/lib/codeql/hcl/providers/Azure.qll @@ -43,288 +43,9 @@ module Azure { Expr getResourceLocation() { result = this.getAttribute("location") } } - /** - * Azure Managed Disk. - */ - class ManagedDisk extends AzureResource { - ManagedDisk() { this.getResourceType() = "azurerm_managed_disk" } - - override string toString() { result = "ManagedDisk " + this.getName() } - - override string getName() { result = this.getAttribute("name").(StringLiteral).getValue() } - - string getStorageAccountType() { - result = this.getAttribute("storage_account_type").(StringLiteral).getValue() - } - - /** - * Get the encryption settings of the managed disk. - */ - ManagedDiskEncryptionSettings getEncryptionSettings() { - result = this.getAttribute("encryption_settings") - } - } - - /** - * Azure Managed Disk Encryption Settings. - */ - class ManagedDiskEncryptionSettings extends Block { - private ManagedDisk disk; - - ManagedDiskEncryptionSettings() { disk.getAttribute("encryption_settings").(Block) = this } - - override string toString() { result = "ManagedDiskEncryptionSettings" } - - boolean getEnabled() { result = this.getAttribute("enabled").(BooleanLiteral).getBool() } - } - - class StorageContainer extends AzureResource { - StorageContainer() { this.getResourceType() = "azurerm_storage_container" } - - /** - * Get the name of the storage container. - */ - override string getName() { result = this.getAttribute("name").(StringLiteral).getValue() } - - string getContainerAccessType() { - result = this.getAttribute("container_access_type").(StringLiteral).getValue() - } - - /** - * Get the properties of the managed disk. - */ - Object getProperties() { result = this.getAttribute("properties") } - - /** - * Get a property of the managed disk. - */ - Expr getProperty(string name) { result = this.getProperties().getElementByName(name) } - } - - class StorageAccount extends AzureResource { - StorageAccount() { this.getResourceType() = "azurerm_storage_account" } - - /** - * Get the name of the storage account. - */ - override string getName() { result = this.getAttribute("name").(StringLiteral).getValue() } - - /** - * Get the `allow_blob_public_access` property of the storage account. Only available - * for `azurerm` v2 and not v3 onwards. - * - * https://github.com/hashicorp/terraform-provider-azurerm/blob/main/CHANGELOG-v3.md - */ - Expr getAllowBlobPublicAccess() { - this.getProvider().getSemanticVersion().maybeBefore("3.0.0") and - result = this.getAttribute("allow_blob_public_access") - } - - /** - * Get the `allow_blob_public_access` property of the storage account. Only available - * for `azurerm` v2 and not v3 onwards. - * - * https://github.com/hashicorp/terraform-provider-azurerm/blob/main/CHANGELOG-v3.md - */ - boolean getAllowBlobPublicAccessValue() { - exists(Expr e | e = this.getAllowBlobPublicAccess() | result = e.(BooleanLiteral).getBool()) - or - not exists(this.getAllowBlobPublicAccess()) and - result = true - } - - /** - * Get the `public_network_access_enabled` property of the storage account. - * - * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#public_network_access_enabled - */ - Expr getEnableHttpsTrafficOnly() { - result = this.getAttribute("enable_https_traffic_only") - } - - /** - * Get the `public_network_access_enabled` property of the storage account. - * - * Defaults to `true`. - * - * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#public_network_access_enabled - */ - boolean getEnableHttpsTrafficOnlyValue() { - exists(Expr e | e = this.getEnableHttpsTrafficOnly() | result = e.(BooleanLiteral).getBool()) - or - not exists(this.getEnableHttpsTrafficOnly()) and - result = true - } - - /** - * Get the `public_network_access_enabled` property of the storage account. - * - * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#public_network_access_enabled - */ - Expr getPublicNetworkAccess() { - result = this.getAttribute("public_network_access_enabled") - } - - /** - * Get the `public_network_access_enabled` property of the storage account. - * - * Defaults to `true`. - * - * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#public_network_access_enabled - */ - boolean getPublicNetworkAccessValue() { - exists(Expr e | e = this.getPublicNetworkAccess() | result = e.(BooleanLiteral).getBool()) - or - not exists(this.getPublicNetworkAccess()) and - result = true - } - - /** - * Get the `allow_nested_items_to_be_public` property of the storage account. - * - * Defaults to `true` - * - * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#allow_nested_items_to_be_public - */ - Expr getAllowNestedItemsToBePublic() { - result = this.getAttribute("allow_nested_items_to_be_public") - } - - /** - * Get the `allow_nested_items_to_be_public` property of the storage account. - * - * Defaults to `true` - * - * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#allow_nested_items_to_be_public - */ - boolean getAllowNestedItemsToBePublicValue() { - exists(Expr e | e = this.getAllowNestedItemsToBePublic() | result = e.(BooleanLiteral).getBool()) - or - not exists(this.getAllowNestedItemsToBePublic()) and - result = true - } - - /** - * Get the `https_traffic_only_enabled` property of the storage account. - * - * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#https_traffic_only_enabled - */ - Expr getHttpsTrafficOnlyEnabled() { - result = this.getAttribute("https_traffic_only_enabled") - } - - /** - * Get the `https_traffic_only_enabled` property of the storage account. - * - * Defaults to `true` - * - * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#https_traffic_only_enabled - */ - boolean getHttpsTrafficOnlyEnabledValue() { - exists(Expr e | e = this.getHttpsTrafficOnlyEnabled() | result = e.(BooleanLiteral).getBool()) - or - not exists(this.getHttpsTrafficOnlyEnabled()) and - result = true - } - - /** - * Get the `min_tls_version` property of the storage account. - * - * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#min_tls_version - */ - Expr getMinTlsVersion() { - result = this.getAttribute("min_tls_version") - } - - /** - * Get the `min_tls_version` property of the storage account. - * - * Defaults to `TLS1_2` - * - * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#min_tls_version - */ - string getMinTlsVersionValue() { - exists(Expr e | e = this.getMinTlsVersion() | result = e.(StringLiteral).getValue()) - or - not exists(this.getMinTlsVersion()) and - result = "TLS1_2" - } - } - - /** - * Azure Databases - */ - class Database extends AzureResource { - Database() { - this.getResourceType() - .regexpMatch("^azurerm_(sql|mariadb|mssql|postgresql)_(server|database)") - } - - override string toString() { result = "Database " + this.getName() } - - override string getName() { result = this.getAttribute("name").(StringLiteral).getValue() } - - string getVersion() { result = this.getAttribute("version").(StringLiteral).getValue() } - - boolean getSslEnforcementEnabled() { - result = this.getAttribute("ssl_enforcement_enabled").(BooleanLiteral).getBool() - } - - boolean getInfrastructureEncryptionEnabled() { - result = this.getAttribute("infrastructure_encryption_enabled").(BooleanLiteral).getBool() - } - - boolean getGeoRedundantBackupEnabled() { - result = this.getAttribute("geo_redundant_backup_enabled").(BooleanLiteral).getBool() - } - - Expr getAdministratorPassword() { result = this.getAttribute("administrator_login_password") } - } - - /** - * Azure Key Vault. - */ - class KeyVault extends AzureResource { - KeyVault() { this.getResourceType() = "azurerm_key_vault" } - - override string toString() { result = "KeyVault " + this.getName() } - } - - /** - * Azure Key Vault Key. - */ - class KeyVaultKey extends AzureResource { - KeyVaultKey() { this.getResourceType() = "azurerm_key_vault_key" } - - override string toString() { result = "KeyVaultKey " + this.getName() } - - string getKeyType() { result = this.getAttribute("key_type").(StringLiteral).getValue() } - - int getKeySize() { result = this.getAttribute("key_size").(NumericLiteral).getInt() } - // string getKeyOpts() { result = this.getAttribute("key_opts") } - } - - /** - * Azure Key Vault Secret. - */ - class KeyVaultSecret extends AzureResource { - KeyVaultSecret() { this.getResourceType() = "azurerm_key_vault_secret" } - } - - /** - * Azure Security Center Contact. - */ - class SecurityCenterContact extends AzureResource { - SecurityCenterContact() { this.getResourceType() = "azurerm_security_center_contact" } - - string getEmail() { result = this.getAttribute("email").(StringLiteral).getValue() } - - boolean getAlertNotifications() { - result = this.getAttribute("alert_notifications").(BooleanLiteral).getBool() - } - - boolean getAlertsToAdmins() { - result = this.getAttribute("alerts_to_admins").(BooleanLiteral).getBool() - } - } + // Re-export the Azure resources + import codeql.hcl.providers.azure.Storage::AzureStorage + import codeql.hcl.providers.azure.Databases::AzureDatabases + import codeql.hcl.providers.azure.KeyVault::AzureKeyVault + import codeql.hcl.providers.azure.SecurityCenter::AzureSecurityCenter } diff --git a/ql/lib/codeql/hcl/providers/azure/Databases.qll b/ql/lib/codeql/hcl/providers/azure/Databases.qll new file mode 100644 index 0000000..ef959d0 --- /dev/null +++ b/ql/lib/codeql/hcl/providers/azure/Databases.qll @@ -0,0 +1,70 @@ +private import codeql.hcl.AST +private import codeql.hcl.Resources +private import codeql.hcl.Constants +private import codeql.hcl.Terraform::Terraform + + +module AzureDatabases { + private import codeql.hcl.providers.Azure + + /** + * Azure Databases + */ + class Database extends Azure::AzureResource { + Database() { + this.getResourceType() + .regexpMatch("^azurerm_(sql|mariadb|mssql|postgresql)_(server|database)") + } + + override string toString() { result = "Database " + this.getName() } + + override string getName() { result = this.getAttribute("name").(StringLiteral).getValue() } + + string getVersion() { result = this.getAttribute("version").(StringLiteral).getValue() } + + boolean getSslEnforcementEnabled() { + result = this.getAttribute("ssl_enforcement_enabled").(BooleanLiteral).getBool() + } + + boolean getInfrastructureEncryptionEnabled() { + result = this.getAttribute("infrastructure_encryption_enabled").(BooleanLiteral).getBool() + } + + boolean getGeoRedundantBackupEnabled() { + result = this.getAttribute("geo_redundant_backup_enabled").(BooleanLiteral).getBool() + } + + Expr getAdministratorPassword() { result = this.getAttribute("administrator_login_password") } + } + + /** + * Azure Cosmos DB + */ + class CosmosDbAccount extends Azure::AzureResource { + CosmosDbAccount() { this.getResourceType() = "azurerm_cosmosdb_account" } + + /** + * Get the `minimal_tls_version` attribute of the Cosmos DB account. + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/cosmosdb_account#minimal_tls_version + */ + Expr getMinimalTlsVersion() { + result = this.getAttribute("minimal_tls_version") + } + + /** + * Get the value of the `minimal_tls_version` attribute of the Cosmos DB account. + * + * Defaults to `TLS1_2`. + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/cosmosdb_account#minimal_tls_version + */ + string getMinimalTlsVersionValue() { + exists(Expr e | e = this.getMinimalTlsVersion() | result = e.(StringLiteral).getValue()) + or + not exists(this.getMinimalTlsVersion()) + and + result = "TLS1_2" + } + } +} \ No newline at end of file diff --git a/ql/lib/codeql/hcl/providers/azure/KeyVault.qll b/ql/lib/codeql/hcl/providers/azure/KeyVault.qll new file mode 100644 index 0000000..fb60dca --- /dev/null +++ b/ql/lib/codeql/hcl/providers/azure/KeyVault.qll @@ -0,0 +1,39 @@ +private import codeql.hcl.AST +private import codeql.hcl.Resources +private import codeql.hcl.Constants +private import codeql.hcl.Terraform::Terraform + + +module AzureKeyVault { + private import codeql.hcl.providers.Azure + + /** + * Azure Key Vault. + */ + class KeyVault extends Azure::AzureResource { + KeyVault() { this.getResourceType() = "azurerm_key_vault" } + + override string toString() { result = "KeyVault " + this.getName() } + } + + /** + * Azure Key Vault Key. + */ + class KeyVaultKey extends Azure::AzureResource { + KeyVaultKey() { this.getResourceType() = "azurerm_key_vault_key" } + + override string toString() { result = "KeyVaultKey " + this.getName() } + + string getKeyType() { result = this.getAttribute("key_type").(StringLiteral).getValue() } + + int getKeySize() { result = this.getAttribute("key_size").(NumericLiteral).getInt() } + // string getKeyOpts() { result = this.getAttribute("key_opts") } + } + + /** + * Azure Key Vault Secret. + */ + class KeyVaultSecret extends Azure::AzureResource { + KeyVaultSecret() { this.getResourceType() = "azurerm_key_vault_secret" } + } +} \ No newline at end of file diff --git a/ql/lib/codeql/hcl/providers/azure/SecurityCenter.qll b/ql/lib/codeql/hcl/providers/azure/SecurityCenter.qll new file mode 100644 index 0000000..e2ade83 --- /dev/null +++ b/ql/lib/codeql/hcl/providers/azure/SecurityCenter.qll @@ -0,0 +1,26 @@ +private import codeql.hcl.AST +private import codeql.hcl.Resources +private import codeql.hcl.Constants +private import codeql.hcl.Terraform::Terraform + + +module AzureSecurityCenter { + private import codeql.hcl.providers.Azure + + /** + * Azure Security Center Contact. + */ + class SecurityCenterContact extends Azure::AzureResource { + SecurityCenterContact() { this.getResourceType() = "azurerm_security_center_contact" } + + string getEmail() { result = this.getAttribute("email").(StringLiteral).getValue() } + + boolean getAlertNotifications() { + result = this.getAttribute("alert_notifications").(BooleanLiteral).getBool() + } + + boolean getAlertsToAdmins() { + result = this.getAttribute("alerts_to_admins").(BooleanLiteral).getBool() + } + } +} \ No newline at end of file diff --git a/ql/lib/codeql/hcl/providers/azure/Storage.qll b/ql/lib/codeql/hcl/providers/azure/Storage.qll new file mode 100644 index 0000000..63136d2 --- /dev/null +++ b/ql/lib/codeql/hcl/providers/azure/Storage.qll @@ -0,0 +1,217 @@ +private import codeql.hcl.AST +private import codeql.hcl.Resources +private import codeql.hcl.Constants +private import codeql.hcl.Terraform::Terraform + + +module AzureStorage { + private import codeql.hcl.providers.Azure + + /** + * Azure Managed Disk. + */ + class ManagedDisk extends Azure::AzureResource { + ManagedDisk() { this.getResourceType() = "azurerm_managed_disk" } + + override string toString() { result = "ManagedDisk " + this.getName() } + + override string getName() { result = this.getAttribute("name").(StringLiteral).getValue() } + + string getStorageAccountType() { + result = this.getAttribute("storage_account_type").(StringLiteral).getValue() + } + + /** + * Get the encryption settings of the managed disk. + */ + ManagedDiskEncryptionSettings getEncryptionSettings() { + result = this.getAttribute("encryption_settings") + } + } + + /** + * Azure Managed Disk Encryption Settings. + */ + class ManagedDiskEncryptionSettings extends Block { + private ManagedDisk disk; + + ManagedDiskEncryptionSettings() { disk.getAttribute("encryption_settings").(Block) = this } + + override string toString() { result = "ManagedDiskEncryptionSettings" } + + boolean getEnabled() { result = this.getAttribute("enabled").(BooleanLiteral).getBool() } + } + + class StorageContainer extends Azure::AzureResource { + StorageContainer() { this.getResourceType() = "azurerm_storage_container" } + + /** + * Get the name of the storage container. + */ + override string getName() { result = this.getAttribute("name").(StringLiteral).getValue() } + + string getContainerAccessType() { + result = this.getAttribute("container_access_type").(StringLiteral).getValue() + } + + /** + * Get the properties of the managed disk. + */ + Object getProperties() { result = this.getAttribute("properties") } + + /** + * Get a property of the managed disk. + */ + Expr getProperty(string name) { result = this.getProperties().getElementByName(name) } + } + + class StorageAccount extends Azure::AzureResource { + StorageAccount() { this.getResourceType() = "azurerm_storage_account" } + + /** + * Get the name of the storage account. + */ + override string getName() { result = this.getAttribute("name").(StringLiteral).getValue() } + + /** + * Get the `allow_blob_public_access` property of the storage account. Only available + * for `azurerm` v2 and not v3 onwards. + * + * https://github.com/hashicorp/terraform-provider-azurerm/blob/main/CHANGELOG-v3.md + */ + Expr getAllowBlobPublicAccess() { + this.getProvider().getSemanticVersion().maybeBefore("3.0.0") and + result = this.getAttribute("allow_blob_public_access") + } + + /** + * Get the `allow_blob_public_access` property of the storage account. Only available + * for `azurerm` v2 and not v3 onwards. + * + * https://github.com/hashicorp/terraform-provider-azurerm/blob/main/CHANGELOG-v3.md + */ + boolean getAllowBlobPublicAccessValue() { + exists(Expr e | e = this.getAllowBlobPublicAccess() | result = e.(BooleanLiteral).getBool()) + or + not exists(this.getAllowBlobPublicAccess()) and + result = true + } + + /** + * Get the `public_network_access_enabled` property of the storage account. + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#public_network_access_enabled + */ + Expr getEnableHttpsTrafficOnly() { + result = this.getAttribute("enable_https_traffic_only") + } + + /** + * Get the `public_network_access_enabled` property of the storage account. + * + * Defaults to `true`. + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#public_network_access_enabled + */ + boolean getEnableHttpsTrafficOnlyValue() { + exists(Expr e | e = this.getEnableHttpsTrafficOnly() | result = e.(BooleanLiteral).getBool()) + or + not exists(this.getEnableHttpsTrafficOnly()) and + result = true + } + + /** + * Get the `public_network_access_enabled` property of the storage account. + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#public_network_access_enabled + */ + Expr getPublicNetworkAccess() { + result = this.getAttribute("public_network_access_enabled") + } + + /** + * Get the `public_network_access_enabled` property of the storage account. + * + * Defaults to `true`. + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#public_network_access_enabled + */ + boolean getPublicNetworkAccessValue() { + exists(Expr e | e = this.getPublicNetworkAccess() | result = e.(BooleanLiteral).getBool()) + or + not exists(this.getPublicNetworkAccess()) and + result = true + } + + /** + * Get the `allow_nested_items_to_be_public` property of the storage account. + * + * Defaults to `true` + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#allow_nested_items_to_be_public + */ + Expr getAllowNestedItemsToBePublic() { + result = this.getAttribute("allow_nested_items_to_be_public") + } + + /** + * Get the `allow_nested_items_to_be_public` property of the storage account. + * + * Defaults to `true` + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#allow_nested_items_to_be_public + */ + boolean getAllowNestedItemsToBePublicValue() { + exists(Expr e | e = this.getAllowNestedItemsToBePublic() | result = e.(BooleanLiteral).getBool()) + or + not exists(this.getAllowNestedItemsToBePublic()) and + result = true + } + + /** + * Get the `https_traffic_only_enabled` property of the storage account. + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#https_traffic_only_enabled + */ + Expr getHttpsTrafficOnlyEnabled() { + result = this.getAttribute("https_traffic_only_enabled") + } + + /** + * Get the `https_traffic_only_enabled` property of the storage account. + * + * Defaults to `true` + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#https_traffic_only_enabled + */ + boolean getHttpsTrafficOnlyEnabledValue() { + exists(Expr e | e = this.getHttpsTrafficOnlyEnabled() | result = e.(BooleanLiteral).getBool()) + or + not exists(this.getHttpsTrafficOnlyEnabled()) and + result = true + } + + /** + * Get the `min_tls_version` property of the storage account. + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#min_tls_version + */ + Expr getMinTlsVersion() { + result = this.getAttribute("min_tls_version") + } + + /** + * Get the `min_tls_version` property of the storage account. + * + * Defaults to `TLS1_2` + * + * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#min_tls_version + */ + string getMinTlsVersionValue() { + exists(Expr e | e = this.getMinTlsVersion() | result = e.(StringLiteral).getValue()) + or + not exists(this.getMinTlsVersion()) and + result = "TLS1_2" + } + } +} \ No newline at end of file diff --git a/ql/test/library-tests/hcl/azure/azure.expected b/ql/test/library-tests/hcl/azure/azure.expected index 4485f0a..7bfaaa8 100644 --- a/ql/test/library-tests/hcl/azure/azure.expected +++ b/ql/test/library-tests/hcl/azure/azure.expected @@ -5,6 +5,7 @@ azure | databases.hcl:20:1:25:1 | Database example-sqlserver | | storage.hcl:1:1:4:1 | ResourceGroup example-resources | | storage.hcl:6:1:13:1 | ManagedDisk acctestmd | +| storage.hcl:15:1:25:1 | resource azurerm_storage_account example-account | azureManagedDisk | storage.hcl:6:1:13:1 | ManagedDisk acctestmd | azureDatabases diff --git a/ql/test/library-tests/hcl/azure/storage.hcl b/ql/test/library-tests/hcl/azure/storage.hcl index 05db553..00a2c58 100644 --- a/ql/test/library-tests/hcl/azure/storage.hcl +++ b/ql/test/library-tests/hcl/azure/storage.hcl @@ -11,3 +11,15 @@ resource "azurerm_managed_disk" "example1" { create_option = "Empty" disk_size_gb = "1" } + +resource "azurerm_storage_account" "example2" { + name = "example-account" + location = azurerm_resource_group.example.location + account_kind = var.kind + account_tier = var.tier + account_replication_type = var.replication_type + resource_group_name = var.resource_group_name + public_network_access_enabled = false + allow_nested_items_to_be_public = false + min_tls_version = var.min_tls_version +} \ No newline at end of file From be15b6047b9893442befc37b57fb37eef72f959a Mon Sep 17 00:00:00 2001 From: Mathew Payne Date: Thu, 26 Sep 2024 14:07:45 +0100 Subject: [PATCH 11/12] style: Update formatting --- ql/lib/codeql/hcl/providers/Azure.qll | 2 +- ql/lib/codeql/hcl/providers/azure/Storage.qll | 74 +++++++++---------- 2 files changed, 36 insertions(+), 40 deletions(-) diff --git a/ql/lib/codeql/hcl/providers/Azure.qll b/ql/lib/codeql/hcl/providers/Azure.qll index b4f7268..8bbc35d 100644 --- a/ql/lib/codeql/hcl/providers/Azure.qll +++ b/ql/lib/codeql/hcl/providers/Azure.qll @@ -48,4 +48,4 @@ module Azure { import codeql.hcl.providers.azure.Databases::AzureDatabases import codeql.hcl.providers.azure.KeyVault::AzureKeyVault import codeql.hcl.providers.azure.SecurityCenter::AzureSecurityCenter -} +} \ No newline at end of file diff --git a/ql/lib/codeql/hcl/providers/azure/Storage.qll b/ql/lib/codeql/hcl/providers/azure/Storage.qll index 63136d2..40d94b6 100644 --- a/ql/lib/codeql/hcl/providers/azure/Storage.qll +++ b/ql/lib/codeql/hcl/providers/azure/Storage.qll @@ -3,7 +3,6 @@ private import codeql.hcl.Resources private import codeql.hcl.Constants private import codeql.hcl.Terraform::Terraform - module AzureStorage { private import codeql.hcl.providers.Azure @@ -65,6 +64,9 @@ module AzureStorage { Expr getProperty(string name) { result = this.getProperties().getElementByName(name) } } + /** + * Azure Storage Account. + */ class StorageAccount extends Azure::AzureResource { StorageAccount() { this.getResourceType() = "azurerm_storage_account" } @@ -76,7 +78,7 @@ module AzureStorage { /** * Get the `allow_blob_public_access` property of the storage account. Only available * for `azurerm` v2 and not v3 onwards. - * + * * https://github.com/hashicorp/terraform-provider-azurerm/blob/main/CHANGELOG-v3.md */ Expr getAllowBlobPublicAccess() { @@ -99,41 +101,37 @@ module AzureStorage { /** * Get the `public_network_access_enabled` property of the storage account. - * + * * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#public_network_access_enabled */ - Expr getEnableHttpsTrafficOnly() { - result = this.getAttribute("enable_https_traffic_only") - } + Expr getEnableHttpsTrafficOnly() { result = this.getAttribute("enable_https_traffic_only") } /** * Get the `public_network_access_enabled` property of the storage account. - * + * * Defaults to `true`. - * + * * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#public_network_access_enabled */ boolean getEnableHttpsTrafficOnlyValue() { - exists(Expr e | e = this.getEnableHttpsTrafficOnly() | result = e.(BooleanLiteral).getBool()) - or + exists(Expr e | e = this.getEnableHttpsTrafficOnly() | result = e.(BooleanLiteral).getBool()) + or not exists(this.getEnableHttpsTrafficOnly()) and result = true } /** * Get the `public_network_access_enabled` property of the storage account. - * + * * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#public_network_access_enabled */ - Expr getPublicNetworkAccess() { - result = this.getAttribute("public_network_access_enabled") - } + Expr getPublicNetworkAccess() { result = this.getAttribute("public_network_access_enabled") } /** * Get the `public_network_access_enabled` property of the storage account. - * + * * Defaults to `true`. - * + * * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#public_network_access_enabled */ boolean getPublicNetworkAccessValue() { @@ -145,9 +143,9 @@ module AzureStorage { /** * Get the `allow_nested_items_to_be_public` property of the storage account. - * + * * Defaults to `true` - * + * * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#allow_nested_items_to_be_public */ Expr getAllowNestedItemsToBePublic() { @@ -156,13 +154,15 @@ module AzureStorage { /** * Get the `allow_nested_items_to_be_public` property of the storage account. - * + * * Defaults to `true` - * + * * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#allow_nested_items_to_be_public */ boolean getAllowNestedItemsToBePublicValue() { - exists(Expr e | e = this.getAllowNestedItemsToBePublic() | result = e.(BooleanLiteral).getBool()) + exists(Expr e | e = this.getAllowNestedItemsToBePublic() | + result = e.(BooleanLiteral).getBool() + ) or not exists(this.getAllowNestedItemsToBePublic()) and result = true @@ -170,18 +170,16 @@ module AzureStorage { /** * Get the `https_traffic_only_enabled` property of the storage account. - * + * * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#https_traffic_only_enabled */ - Expr getHttpsTrafficOnlyEnabled() { - result = this.getAttribute("https_traffic_only_enabled") - } + Expr getHttpsTrafficOnlyEnabled() { result = this.getAttribute("https_traffic_only_enabled") } /** * Get the `https_traffic_only_enabled` property of the storage account. - * + * * Defaults to `true` - * + * * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#https_traffic_only_enabled */ boolean getHttpsTrafficOnlyEnabledValue() { @@ -193,25 +191,23 @@ module AzureStorage { /** * Get the `min_tls_version` property of the storage account. - * + * * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#min_tls_version */ - Expr getMinTlsVersion() { - result = this.getAttribute("min_tls_version") - } + Expr getMinTlsVersion() { result = this.getAttribute("min_tls_version") } /** * Get the `min_tls_version` property of the storage account. - * + * * Defaults to `TLS1_2` - * + * * https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/storage_account.html#min_tls_version */ - string getMinTlsVersionValue() { - exists(Expr e | e = this.getMinTlsVersion() | result = e.(StringLiteral).getValue()) - or - not exists(this.getMinTlsVersion()) and - result = "TLS1_2" - } + string getMinTlsVersionValue() { + exists(Expr e | e = this.getMinTlsVersion() | result = e.(StringLiteral).getValue()) + or + not exists(this.getMinTlsVersion()) and + result = "TLS1_2" + } } } \ No newline at end of file From 660cfa83c44515ac9b8c0d317d0ca300fd3bcc6a Mon Sep 17 00:00:00 2001 From: Mathew Payne Date: Thu, 26 Sep 2024 14:10:50 +0100 Subject: [PATCH 12/12] feat(tf): Small updates --- ql/lib/codeql/hcl/security/PublicStorage.qll | 12 +++--- .../PublicAccess/PublicAccess.expected | 5 ++- .../Azure/Storage/PublicAccess/storage.tf | 43 ++++++++++++++++--- 3 files changed, 46 insertions(+), 14 deletions(-) diff --git a/ql/lib/codeql/hcl/security/PublicStorage.qll b/ql/lib/codeql/hcl/security/PublicStorage.qll index a6f3d05..0e74c85 100644 --- a/ql/lib/codeql/hcl/security/PublicStorage.qll +++ b/ql/lib/codeql/hcl/security/PublicStorage.qll @@ -18,22 +18,22 @@ class AzurePublicStorage extends PublicStorage { ) or // Azure Storage Accounts - exists(Azure::StorageAccount storage_acount | + exists(Azure::StorageAccount storage_account | ( // v2 - storage_acount.getAllowBlobPublicAccessValue() = true and - this = storage_acount.getAllowBlobPublicAccess() + storage_account.getAllowBlobPublicAccessValue() = true and + this = storage_account.getAllowBlobPublicAccess() ) or ( // v3 ( - storage_acount.getPublicNetworkAccessValue() = true + storage_account.getPublicNetworkAccessValue() = true or - storage_acount.getAllowNestedItemsToBePublicValue() = true + storage_account.getAllowNestedItemsToBePublicValue() = true ) and - this = storage_acount + this = storage_account ) ) } diff --git a/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/PublicAccess.expected b/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/PublicAccess.expected index 311bf33..32b87f0 100644 --- a/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/PublicAccess.expected +++ b/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/PublicAccess.expected @@ -1,2 +1,3 @@ -| storage.tf:9:1:15:1 | resource azurerm_storage_container insecure-storage-container | Azure Storage is Public | -| storage.tf:18:1:28:1 | resource azurerm_storage_account insecure-storage-account | Azure Storage is Public | +| storage.tf:15:1:22:1 | resource azurerm_storage_container insecure-storage-container | Azure Storage is Public | +| storage.tf:25:1:33:1 | resource azurerm_storage_account insecure-storage-account | Azure Storage is Public | +| storage.tf:49:1:59:1 | resource azurerm_storage_account insecure-storage-account | Azure Storage is Public | diff --git a/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/storage.tf b/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/storage.tf index 2e7f811..0a3c390 100644 --- a/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/storage.tf +++ b/ql/test/queries-tests/Terraform/Azure/Storage/PublicAccess/storage.tf @@ -1,28 +1,59 @@ +# Resource Group +resource "azurerm_resource_group" "example" { + name = "example-resources" + location = "West Europe" +} # secure resource "azurerm_storage_container" "secure" { name = "secure-storage-container" + location = azurerm_resource_group.example.location container_access_type = "private" } # insecure resource "azurerm_storage_container" "insecure" { name = "insecure-storage-container" + location = azurerm_resource_group.example.location container_access_type = "blob" properties = { "publicAccess" = "blob" } } +# insecure defaults (v3) +resource "azurerm_storage_account" "insecure_storage_account" { + name = "insecure-storage-account" + location = azurerm_resource_group.example.location + account_kind = "BlobStorage" + account_tier = "Standard" + account_replication_type = "GRS" + resource_group_name = azurerm_resource_group.example + min_tls_version = "TLS1_2" +} + +# Secure (v3) +resource "azurerm_storage_account" "secure_storage_account" { + name = "secure-storage-account" + location = azurerm_resource_group.example.location + account_kind = "BlobStorage" + account_tier = "Standard" + account_replication_type = "GRS" + resource_group_name = azurerm_resource_group.example + public_network_access_enabled = false + allow_nested_items_to_be_public = false + min_tls_version = "TLS1_2" +} + # insecure (v3) resource "azurerm_storage_account" "insecure_storage_account" { name = "insecure-storage-account" - location = var.location - account_kind = var.kind - account_tier = var.tier - account_replication_type = var.replication_type - resource_group_name = var.resource_group_name + location = azurerm_resource_group.example.location + account_kind = "BlobStorage" + account_tier = "Standard" + account_replication_type = "GRS" + resource_group_name = azurerm_resource_group.example public_network_access_enabled = true allow_nested_items_to_be_public = true - min_tls_version = var.min_tls_version + min_tls_version = "TLS1_2" }