TES-363: Cleaned up configuration and security issues

Cleaned up issues reported by tflint, tfsec, Trivy and Checkov. - Added content type for the key vault secrets - Disabled VMSS password login - Enabled VMSS encryption at rest - Added networking rules for the storage account to limit the access only from the VMSS subnet and management CIDRs - Changed the default storage account redundancy to ZRS (multi zonal) - Added the latest recommended SSL policy profile - Disabled the public access to the managed disks - Enabled encryption at host for the VMs - Added configurations for key vault purge protection - Disabled the ability for storage blobs to become public - Variables formatting - Replaced tfsec with trivy in the GitHub workflow
Ontotext-AD · Nov 24, 2023 · c7b21e4 · c7b21e4
1 parent c7a7d23
commit c7b21e4
Show file tree

Hide file tree

Showing 15 changed files with 135 additions and 41 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -30,13 +30,16 @@ jobs:
       - name: Run Terraform validate check
         run: terraform validate
 
-      - name: Run tfsec
-        uses: aquasecurity/tfsec-sarif-action@v0.1.4
+      - name: Run Trivy vulnerability scanner
+        uses: aquasecurity/trivy-action@master
         with:
-          config_file: tfsec.yml
-          sarif_file: tfsec.sarif
+          scan-type: config
+          trivy-config: trivy.yaml
+          hide-progress: false
+          format: sarif
+          output: trivy.sarif
 
-      - name: Upload SARIF file
+      - name: Upload Trivy SARIF results
         uses: github/codeql-action/upload-sarif@v2
         with:
-          sarif_file: tfsec.sarif
+          sarif_file: trivy.sarif
diff --git a/.trivyignore b/.trivyignore
@@ -0,0 +1,3 @@
+# Rule: Key vault should have purge protection enabled
+# Note: There is a variable for controlling the purge protection
+AVD-AZU-0016
diff --git a/main.tf b/main.tf
@@ -59,7 +59,7 @@ resource "azurerm_subnet" "graphdb-vmss" {
   resource_group_name  = azurerm_resource_group.graphdb.name
   virtual_network_name = azurerm_virtual_network.graphdb.name
   address_prefixes     = var.graphdb_subnet_address_prefix
-  service_endpoints    = ["Microsoft.KeyVault"]
+  service_endpoints    = ["Microsoft.KeyVault", "Microsoft.Storage"]
 }
 
 resource "azurerm_network_security_group" "graphdb-gateway" {
@@ -149,6 +149,9 @@ module "vault" {
   nacl_subnet_ids = [azurerm_subnet.graphdb-gateway.id, azurerm_subnet.graphdb-vmss.id]
   nacl_ip_rules   = var.management_cidr_blocks
 
+  key_vault_enable_purge_protection = var.key_vault_enable_purge_protection
+  key_vault_retention_days          = var.key_vault_retention_days
+
   tags = local.tags
 }
 
@@ -294,7 +297,9 @@ module "backup" {
   location             = var.location
   resource_group_name  = azurerm_resource_group.graphdb.name
 
-  identity_name                    = module.identity.identity_name
+  nacl_subnet_ids = [azurerm_subnet.graphdb-vmss.id]
+  nacl_ip_rules   = var.management_cidr_blocks
+
   identity_principal_id            = module.identity.identity_principal_id
   storage_account_tier             = var.storage_account_tier
   storage_account_replication_type = var.storage_account_replication_type

diff --git a/modules/backup/main.tf b/modules/backup/main.tf
@@ -21,13 +21,22 @@ locals {
 
 # Create an Azure Storage Account for backups
 resource "azurerm_storage_account" "graphdb-backup" {
-  name                      = local.storage_account_name
-  resource_group_name       = var.resource_group_name
-  location                  = var.location
-  account_tier              = var.storage_account_tier
-  account_replication_type  = var.storage_account_replication_type
-  enable_https_traffic_only = true
-  min_tls_version           = "TLS1_2"
+  name                = local.storage_account_name
+  resource_group_name = var.resource_group_name
+  location            = var.location
+
+  account_tier                    = var.storage_account_tier
+  account_replication_type        = var.storage_account_replication_type
+  enable_https_traffic_only       = true
+  allow_nested_items_to_be_public = false
+  min_tls_version                 = "TLS1_2"
+
+  network_rules {
+    bypass                     = ["AzureServices"]
+    default_action             = "Deny"
+    virtual_network_subnet_ids = var.nacl_subnet_ids
+    ip_rules                   = var.nacl_ip_rules
+  }
 
   tags = var.tags
 }

diff --git a/modules/backup/variables.tf b/modules/backup/variables.tf
@@ -21,13 +21,22 @@ variable "resource_group_name" {
   type        = string
 }
 
-# Identity
+# Networking
 
-variable "identity_name" {
-  description = "Name of a user assigned identity for assigning permissions"
-  type        = string
+variable "nacl_subnet_ids" {
+  description = "List of subnet identifiers allowed to access the storage account internally over a service link"
+  type        = list(string)
+  default     = []
 }
 
+variable "nacl_ip_rules" {
+  description = "List of CIDR blocks allowed to access the storage account"
+  type        = list(string)
+  default     = []
+}
+
+# Identity
+
 variable "identity_principal_id" {
   description = "Principal identifier of a user assigned identity for assigning permissions"
   type        = string
@@ -36,12 +45,13 @@ variable "identity_principal_id" {
 # Storage specifics
 
 variable "storage_account_tier" {
-  default     = "Standard"
   description = "Specify the performance and redundancy characteristics of the Azure Storage Account that you are creating"
   type        = string
+  default     = "Standard"
 }
 
 variable "storage_account_replication_type" {
-  default     = "LRS"
   description = "Specify the data redundancy strategy for your Azure Storage Account"
+  type        = string
+  default     = "ZRS"
 }
diff --git a/modules/configuration/main.tf b/modules/configuration/main.tf
@@ -17,26 +17,29 @@ locals {
 resource "azurerm_key_vault_secret" "graphdb-license" {
   key_vault_id = var.key_vault_id
 
-  name  = var.graphdb_license_secret_name
-  value = filebase64(var.graphdb_license_path)
+  name         = var.graphdb_license_secret_name
+  value        = filebase64(var.graphdb_license_path)
+  content_type = "text/plain"
 
   tags = var.tags
 }
 
 resource "azurerm_key_vault_secret" "graphdb-cluster-token" {
   key_vault_id = var.key_vault_id
 
-  name  = var.graphdb_cluster_token_name
-  value = base64encode(local.graphdb_cluster_token)
+  name         = var.graphdb_cluster_token_name
+  value        = base64encode(local.graphdb_cluster_token)
+  content_type = "text/plain"
 
   tags = var.tags
 }
 
 resource "azurerm_key_vault_secret" "graphdb-password" {
   key_vault_id = var.key_vault_id
 
-  name  = var.graphdb_password_secret_name
-  value = base64encode(local.graphdb_password)
+  name         = var.graphdb_password_secret_name
+  value        = base64encode(local.graphdb_password)
+  content_type = "text/plain"
 
   tags = var.tags
 }
@@ -46,8 +49,9 @@ resource "azurerm_key_vault_secret" "graphdb-properties" {
 
   key_vault_id = var.key_vault_id
 
-  name  = var.graphdb_properties_secret_name
-  value = filebase64(var.graphdb_properties_path)
+  name         = var.graphdb_properties_secret_name
+  value        = filebase64(var.graphdb_properties_path)
+  content_type = "text/plain"
 
   tags = var.tags
 }
@@ -57,8 +61,9 @@ resource "azurerm_key_vault_secret" "graphdb-java-options" {
 
   key_vault_id = var.key_vault_id
 
-  name  = var.graphdb_java_options_secret_name
-  value = base64encode(var.graphdb_java_options)
+  name         = var.graphdb_java_options_secret_name
+  value        = base64encode(var.graphdb_java_options)
+  content_type = "text/plain"
 
   tags = var.tags
 }

diff --git a/modules/gateway/main.tf b/modules/gateway/main.tf
@@ -26,8 +26,6 @@ resource "azurerm_application_gateway" "graphdb" {
 
   enable_http2 = true
 
-  # TODO: Connection draining?
-
   sku {
     name = "Standard_v2"
     tier = "Standard_v2"
@@ -43,6 +41,11 @@ resource "azurerm_application_gateway" "graphdb" {
     key_vault_secret_id = var.gateway_tls_certificate_secret_id
   }
 
+  ssl_policy {
+    policy_type = "Predefined"
+    policy_name = var.gateway_ssl_policy_profile
+  }
+
   gateway_ip_configuration {
     name      = local.gateway_ip_configuration_name
     subnet_id = var.gateway_subnet_id

diff --git a/modules/gateway/variables.tf b/modules/gateway/variables.tf
@@ -47,6 +47,12 @@ variable "gateway_max_capacity" {
   default     = 2
 }
 
+variable "gateway_ssl_policy_profile" {
+  description = "The predefined SSL policy to use in the Application Gateway"
+  type        = string
+  default     = "AppGwSslPolicy20220101S"
+}
+
 variable "gateway_backend_port" {
   description = "Backend port for the Application Gateway rules"
   type        = number

diff --git a/modules/vault/main.tf b/modules/vault/main.tf
@@ -19,8 +19,10 @@ resource "azurerm_key_vault" "graphdb" {
   location            = var.location
   tenant_id           = data.azurerm_client_config.current.tenant_id
 
-  sku_name                  = "standard"
-  enable_rbac_authorization = true
+  sku_name                   = "standard"
+  enable_rbac_authorization  = true
+  purge_protection_enabled   = var.key_vault_enable_purge_protection
+  soft_delete_retention_days = var.key_vault_retention_days
 
   network_acls {
     bypass                     = "AzureServices"

diff --git a/modules/vault/variables.tf b/modules/vault/variables.tf
@@ -34,3 +34,18 @@ variable "nacl_ip_rules" {
   type        = list(string)
   default     = []
 }
+
+# Key Vault
+
+variable "key_vault_enable_purge_protection" {
+  description = "Prevents purging the key vault and its contents by soft deleting it. It will be deleted once the soft delete retention has passed."
+  type        = bool
+  # Enable only for production
+  default     = false
+}
+
+variable "key_vault_retention_days" {
+  description = "Retention period in days during which soft deleted secrets are kept"
+  type        = number
+  default     = 30
+}
diff --git a/modules/vm/main.tf b/modules/vm/main.tf
@@ -30,8 +30,10 @@ resource "azurerm_linux_virtual_machine_scale_set" "graphdb" {
   upgrade_mode  = "Manual"
   overprovision = false
 
-  computer_name_prefix = "${var.resource_name_prefix}-"
-  admin_username       = "graphdb"
+  computer_name_prefix            = "${var.resource_name_prefix}-"
+  admin_username                  = "graphdb"
+  disable_password_authentication = true
+  encryption_at_host_enabled      = var.encryption_at_host
 
   scale_in {
     # In case of re-balancing, remove the newest VM which might have not been IN-SYNC yet with the cluster
@@ -62,7 +64,7 @@ resource "azurerm_linux_virtual_machine_scale_set" "graphdb" {
 
   tags = var.tags
 
-  depends_on = [azurerm_role_assignment.rg-contributor-role]
+  depends_on = [azurerm_role_assignment.rg-contributor-role, azurerm_role_assignment.rg-reader-role]
 }
 
 resource "azurerm_monitor_autoscale_setting" "graphdb-autoscale-settings" {

diff --git a/modules/vm/templates/entrypoint.sh.tpl b/modules/vm/templates/entrypoint.sh.tpl
@@ -54,7 +54,9 @@ if [ -z "$existingUnattachedDisk" ]; then
    --zone $ZONE_ID \
    --os-type Linux \
    --disk-iops-read-write $DISK_IOPS \
-   --disk-mbps-read-write $DISK_THROUGHPUT
+   --disk-mbps-read-write $DISK_THROUGHPUT \
+   --public-network-access Disabled \
+   --network-access-policy DenyAll
 fi
 
 # Checks if a managed disk is attached to the instance

diff --git a/modules/vm/variables.tf b/modules/vm/variables.tf
@@ -101,6 +101,12 @@ variable "custom_user_data" {
   default     = null
 }
 
+variable "encryption_at_host" {
+  description = "Enables encryption at rest on the VM host"
+  type        = bool
+  default     = true
+}
+
 # Managed Data Disks
 
 variable "disk_size_gb" {

diff --git a/trivy.yaml b/trivy.yaml
@@ -0,0 +1,7 @@
+timeout: 5m
+exit-code: 1
+severity:
+  - HIGH
+  - CRITICAL
+  - MEDIUM
+format: table
diff --git a/variables.tf b/variables.tf
@@ -66,6 +66,21 @@ variable "tls_certificate_password" {
   default     = null
 }
 
+# Key Vault
+
+variable "key_vault_enable_purge_protection" {
+  description = "Prevents purging the key vault and its contents by soft deleting it. It will be deleted once the soft delete retention has passed."
+  type        = bool
+  # Enable only for production
+  default     = false
+}
+
+variable "key_vault_retention_days" {
+  description = "Retention period in days during which soft deleted secrets are kept"
+  type        = number
+  default     = 30
+}
+
 # GraphDB
 
 variable "graphdb_version" {
@@ -139,14 +154,15 @@ variable "custom_graphdb_vm_user_data" {
 # Storage account
 
 variable "storage_account_tier" {
-  default     = "Standard"
   description = "Specify the performance and redundancy characteristics of the Azure Storage Account that you are creating"
   type        = string
+  default     = "Standard"
 }
 
 variable "storage_account_replication_type" {
-  default     = "LRS"
   description = "Specify the data redundancy strategy for your Azure Storage Account"
+  type        = string
+  default     = "ZRS"
 }
 
 # Backup configurations