Change Key Vault to use RBAC instead of Access Policies (#4115)

microsoft · Nov 8, 2024 · 07173fe · 07173fe
1 parent 15fa48b
commit 07173fe
Show file tree

Hide file tree

Showing 28 changed files with 104 additions and 150 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@
 FEATURES:
 
 ENHANCEMENTS:
+* Key Vaults should use RBAC instead of access policies for access control ([#4000](https://github.com/microsoft/AzureTRE/issues/4000))
 * Split log entries with [Log chunk X of Y] for better readability. ([[#3992](https://github.com/microsoft/AzureTRE/issues/3992)
 * Expose APP_SERVICE_SKU build variable to allow enablement of App Gateway WAF ([#4111](https://github.com/microsoft/AzureTRE/pull/4111))
 * Update Terraform to use Azure AD authentication rather than storage account keys ([#4103](https://github.com/microsoft/AzureTRE/issues/4103))

diff --git a/core/terraform/appgateway/certificate.tf b/core/terraform/appgateway/certificate.tf
@@ -1,15 +1,7 @@
-resource "azurerm_key_vault_access_policy" "app_gw_managed_identity" {
-  key_vault_id = var.keyvault_id
-  tenant_id    = azurerm_user_assigned_identity.agw_id.tenant_id
-  object_id    = azurerm_user_assigned_identity.agw_id.principal_id
-
-  key_permissions = [
-    "Get",
-  ]
-
-  secret_permissions = [
-    "Get",
-  ]
+resource "azurerm_role_assignment" "keyvault_appgw_role" {
+  scope                = var.keyvault_id
+  role_definition_name = "Key Vault Secrets User"
+  principal_id         = azurerm_user_assigned_identity.agw_id.principal_id // id-agw-<TRE_ID>
 }
 
 resource "azurerm_key_vault_certificate" "tlscert" {

diff --git a/core/terraform/cosmos_mongo.tf b/core/terraform/cosmos_mongo.tf
@@ -97,7 +97,7 @@ resource "azurerm_key_vault_secret" "cosmos_mongo_connstr" {
   key_vault_id = azurerm_key_vault.kv.id
   tags         = local.tre_core_tags
   depends_on = [
-    azurerm_key_vault_access_policy.deployer
+    azurerm_role_assignment.keyvault_deployer_role
   ]
 
   lifecycle { ignore_changes = [tags] }

diff --git a/core/terraform/keyvault.tf b/core/terraform/keyvault.tf
@@ -1,34 +1,26 @@
 resource "azurerm_key_vault" "kv" {
-  name                     = "kv-${var.tre_id}"
-  tenant_id                = data.azurerm_client_config.current.tenant_id
-  location                 = azurerm_resource_group.core.location
-  resource_group_name      = azurerm_resource_group.core.name
-  sku_name                 = "standard"
-  purge_protection_enabled = var.kv_purge_protection_enabled
-  tags                     = local.tre_core_tags
+  name                      = "kv-${var.tre_id}"
+  tenant_id                 = data.azurerm_client_config.current.tenant_id
+  location                  = azurerm_resource_group.core.location
+  resource_group_name       = azurerm_resource_group.core.name
+  sku_name                  = "standard"
+  enable_rbac_authorization = true
+  purge_protection_enabled  = var.kv_purge_protection_enabled
+  tags                      = local.tre_core_tags
 
   lifecycle { ignore_changes = [access_policy, tags] }
 }
 
-resource "azurerm_key_vault_access_policy" "deployer" {
-  key_vault_id = azurerm_key_vault.kv.id
-  tenant_id    = data.azurerm_client_config.current.tenant_id
-  object_id    = data.azurerm_client_config.current.object_id
-
-  key_permissions         = ["Get", "List", "Update", "Create", "Import", "Delete", "Recover"]
-  secret_permissions      = ["Get", "List", "Set", "Delete", "Purge", "Recover"]
-  certificate_permissions = ["Get", "List", "Update", "Create", "Import", "Delete", "Purge", "Recover"]
-  storage_permissions     = ["Get", "List", "Update", "Delete"]
+resource "azurerm_role_assignment" "keyvault_deployer_role" {
+  scope                = azurerm_key_vault.kv.id
+  role_definition_name = "Key Vault Administrator"
+  principal_id         = data.azurerm_client_config.current.object_id // deployer - either CICD service principal or local user
 }
 
-resource "azurerm_key_vault_access_policy" "managed_identity" {
-  key_vault_id = azurerm_key_vault.kv.id
-  tenant_id    = azurerm_user_assigned_identity.id.tenant_id
-  object_id    = azurerm_user_assigned_identity.id.principal_id
-
-  key_permissions         = ["Get", "List", ]
-  secret_permissions      = ["Get", "List", ]
-  certificate_permissions = ["Get", "List", ]
+resource "azurerm_role_assignment" "keyvault_apiidentity_role" {
+  scope                = azurerm_key_vault.kv.id
+  role_definition_name = "Key Vault Secrets User"
+  principal_id         = azurerm_user_assigned_identity.id.principal_id // id-api-<TRE_ID>
 }
 
 data "azurerm_private_dns_zone" "vaultcore" {
@@ -68,7 +60,7 @@ resource "azurerm_key_vault_secret" "api_client_id" {
   key_vault_id = azurerm_key_vault.kv.id
   tags         = local.tre_core_tags
   depends_on = [
-    azurerm_key_vault_access_policy.deployer
+    azurerm_role_assignment.keyvault_deployer_role
   ]
 
   lifecycle { ignore_changes = [tags] }
@@ -80,7 +72,7 @@ resource "azurerm_key_vault_secret" "api_client_secret" {
   key_vault_id = azurerm_key_vault.kv.id
   tags         = local.tre_core_tags
   depends_on = [
-    azurerm_key_vault_access_policy.deployer
+    azurerm_role_assignment.keyvault_deployer_role
   ]
 
   lifecycle { ignore_changes = [tags] }
@@ -92,7 +84,7 @@ resource "azurerm_key_vault_secret" "auth_tenant_id" {
   key_vault_id = azurerm_key_vault.kv.id
   tags         = local.tre_core_tags
   depends_on = [
-    azurerm_key_vault_access_policy.deployer
+    azurerm_role_assignment.keyvault_deployer_role
   ]
 
   lifecycle { ignore_changes = [tags] }
@@ -104,7 +96,7 @@ resource "azurerm_key_vault_secret" "application_admin_client_id" {
   key_vault_id = azurerm_key_vault.kv.id
   tags         = local.tre_core_tags
   depends_on = [
-    azurerm_key_vault_access_policy.deployer
+    azurerm_role_assignment.keyvault_deployer_role
   ]
 
   lifecycle { ignore_changes = [tags] }
@@ -116,7 +108,7 @@ resource "azurerm_key_vault_secret" "application_admin_client_secret" {
   key_vault_id = azurerm_key_vault.kv.id
   tags         = local.tre_core_tags
   depends_on = [
-    azurerm_key_vault_access_policy.deployer
+    azurerm_role_assignment.keyvault_deployer_role
   ]
 
   lifecycle { ignore_changes = [tags] }

diff --git a/core/terraform/main.tf b/core/terraform/main.tf
@@ -104,7 +104,7 @@ module "appgateway" {
   depends_on = [
     module.network,
     azurerm_key_vault.kv,
-    azurerm_key_vault_access_policy.deployer,
+    azurerm_role_assignment.keyvault_deployer_role,
     azurerm_private_endpoint.api_private_endpoint
   ]
 }
@@ -175,7 +175,7 @@ module "resource_processor_vmss_porter" {
     module.network,
     module.azure_monitor,
     azurerm_key_vault.kv,
-    azurerm_key_vault_access_policy.deployer
+    azurerm_role_assignment.keyvault_deployer_role
   ]
 }
 

diff --git a/core/terraform/modules_move_definitions.tf b/core/terraform/modules_move_definitions.tf
@@ -148,16 +148,6 @@ moved {
   to   = azurerm_key_vault.kv
 }
 
-moved {
-  from = module.keyvault.azurerm_key_vault_access_policy.deployer
-  to   = azurerm_key_vault_access_policy.deployer
-}
-
-moved {
-  from = module.keyvault.azurerm_key_vault_access_policy.managed_identity
-  to   = azurerm_key_vault_access_policy.managed_identity
-}
-
 moved {
   from = module.keyvault.azurerm_private_endpoint.kvpe
   to   = azurerm_private_endpoint.kvpe

diff --git a/core/terraform/resource_processor/vmss_porter/main.tf b/core/terraform/resource_processor/vmss_porter/main.tf
@@ -189,13 +189,10 @@ resource "azurerm_role_assignment" "subscription_contributor" {
   principal_id         = azurerm_user_assigned_identity.vmss_msi.principal_id
 }
 
-resource "azurerm_key_vault_access_policy" "resource_processor" {
-  key_vault_id = var.key_vault_id
-  tenant_id    = azurerm_user_assigned_identity.vmss_msi.tenant_id
-  object_id    = azurerm_user_assigned_identity.vmss_msi.principal_id
-
-  secret_permissions      = ["Get", "List", "Set", "Delete", "Purge", "Recover"]
-  certificate_permissions = ["Get", "Recover", "Import", "Delete", "Purge"]
+resource "azurerm_role_assignment" "keyvault_vmss_role" {
+  scope                = var.key_vault_id
+  role_definition_name = "Key Vault Administrator"
+  principal_id         = azurerm_user_assigned_identity.vmss_msi.principal_id // id-vmss-<TRE_ID>
 }
 
 module "terraform_azurerm_environment_configuration" {

diff --git a/core/version.txt b/core/version.txt
@@ -1 +1 @@
-__version__ = "0.10.12"
+__version__ = "0.11.0"
diff --git a/templates/shared_services/certs/porter.yaml b/templates/shared_services/certs/porter.yaml
@@ -1,7 +1,7 @@
 ---
 schemaVersion: 1.0.0
 name: tre-shared-service-certs
-version: 0.5.6
+version: 0.6.0
 description: "An Azure TRE shared service to generate certificates for a specified internal domain using Letsencrypt"
 registry: azuretre
 dockerfile: Dockerfile.tmpl

diff --git a/templates/shared_services/certs/terraform/appgateway.tf b/templates/shared_services/certs/terraform/appgateway.tf
@@ -162,6 +162,6 @@ resource "azurerm_application_gateway" "agw" {
   }
 
   depends_on = [
-    azurerm_key_vault_access_policy.app_gw_managed_identity,
+    azurerm_role_assignment.keyvault_appgwcerts_role,
   ]
 }
diff --git a/templates/shared_services/certs/terraform/certificate.tf b/templates/shared_services/certs/terraform/certificate.tf
@@ -1,10 +1,7 @@
-resource "azurerm_key_vault_access_policy" "app_gw_managed_identity" {
-  key_vault_id = data.azurerm_key_vault.key_vault.id
-  tenant_id    = azurerm_user_assigned_identity.agw_id.tenant_id
-  object_id    = azurerm_user_assigned_identity.agw_id.principal_id
-
-  key_permissions    = ["Get"]
-  secret_permissions = ["Get"]
+resource "azurerm_role_assignment" "keyvault_appgwcerts_role" {
+  scope                = data.azurerm_key_vault.key_vault.id
+  role_definition_name = "Key Vault Secrets User"
+  principal_id         = azurerm_user_assigned_identity.agw_id.principal_id
 }
 
 resource "azurerm_key_vault_certificate" "tlscert" {

diff --git a/templates/shared_services/gitea/porter.yaml b/templates/shared_services/gitea/porter.yaml
@@ -1,7 +1,7 @@
 ---
 schemaVersion: 1.0.0
 name: tre-shared-service-gitea
-version: 1.0.6
+version: 1.1.0
 description: "A Gitea shared service"
 dockerfile: Dockerfile.tmpl
 registry: azuretre

diff --git a/templates/shared_services/gitea/terraform/gitea-webapp.tf b/templates/shared_services/gitea/terraform/gitea-webapp.tf
@@ -141,12 +141,10 @@ resource "azurerm_monitor_diagnostic_setting" "webapp_gitea" {
   }
 }
 
-resource "azurerm_key_vault_access_policy" "gitea_policy" {
-  key_vault_id = data.azurerm_key_vault.keyvault.id
-  tenant_id    = azurerm_user_assigned_identity.gitea_id.tenant_id
-  object_id    = azurerm_user_assigned_identity.gitea_id.principal_id
-
-  secret_permissions = ["Get", "List", ]
+resource "azurerm_role_assignment" "keyvault_gitea_role" {
+  scope                = data.azurerm_key_vault.keyvault.id
+  role_definition_name = "Key Vault Secrets User"
+  principal_id         = azurerm_user_assigned_identity.gitea_id.principal_id
 }
 
 resource "azurerm_key_vault_secret" "gitea_password" {
@@ -156,7 +154,7 @@ resource "azurerm_key_vault_secret" "gitea_password" {
   tags         = local.tre_shared_service_tags
 
   depends_on = [
-    azurerm_key_vault_access_policy.gitea_policy
+    azurerm_role_assignment.keyvault_gitea_role
   ]
 
   lifecycle { ignore_changes = [tags] }

diff --git a/templates/shared_services/gitea/terraform/mysql.tf b/templates/shared_services/gitea/terraform/mysql.tf
@@ -65,7 +65,7 @@ resource "azurerm_key_vault_secret" "db_password" {
   tags         = local.tre_shared_service_tags
 
   depends_on = [
-    azurerm_key_vault_access_policy.gitea_policy
+    azurerm_role_assignment.keyvault_gitea_role
   ]
 
   lifecycle { ignore_changes = [tags] }

diff --git a/templates/shared_services/sonatype-nexus-vm/porter.yaml b/templates/shared_services/sonatype-nexus-vm/porter.yaml
@@ -1,7 +1,7 @@
 ---
 schemaVersion: 1.0.0
 name: tre-shared-service-sonatype-nexus
-version: 3.0.4
+version: 3.1.0
 description: "A Sonatype Nexus shared service"
 dockerfile: Dockerfile.tmpl
 registry: azuretre

diff --git a/templates/shared_services/sonatype-nexus-vm/terraform/vm.tf b/templates/shared_services/sonatype-nexus-vm/terraform/vm.tf
@@ -87,12 +87,10 @@ resource "azurerm_user_assigned_identity" "nexus_msi" {
   lifecycle { ignore_changes = [tags] }
 }
 
-resource "azurerm_key_vault_access_policy" "nexus_msi" {
-  key_vault_id = data.azurerm_key_vault.kv.id
-  tenant_id    = azurerm_user_assigned_identity.nexus_msi.tenant_id
-  object_id    = azurerm_user_assigned_identity.nexus_msi.principal_id
-
-  secret_permissions = ["Get", "List"]
+resource "azurerm_role_assignment" "keyvault_nexus_role" {
+  scope                = data.azurerm_key_vault.kv.id
+  role_definition_name = "Key Vault Secrets User"
+  principal_id         = azurerm_user_assigned_identity.nexus_msi.principal_id
 }
 
 resource "azurerm_linux_virtual_machine" "nexus" {
@@ -134,7 +132,7 @@ resource "azurerm_linux_virtual_machine" "nexus" {
   }
 
   depends_on = [
-    azurerm_key_vault_access_policy.nexus_msi
+    azurerm_role_assignment.keyvault_nexus_role
   ]
 
   connection {

diff --git a/templates/workspace_services/gitea/porter.yaml b/templates/workspace_services/gitea/porter.yaml
@@ -1,7 +1,7 @@
 ---
 schemaVersion: 1.0.0
 name: tre-workspace-service-gitea
-version: 1.0.8
+version: 1.1.0
 description: "A Gitea workspace service"
 dockerfile: Dockerfile.tmpl
 registry: azuretre

diff --git a/templates/workspace_services/gitea/terraform/gitea-webapp.tf b/templates/workspace_services/gitea/terraform/gitea-webapp.tf
@@ -150,12 +150,10 @@ resource "azurerm_monitor_diagnostic_setting" "gitea" {
   }
 }
 
-resource "azurerm_key_vault_access_policy" "gitea_policy" {
-  key_vault_id = data.azurerm_key_vault.ws.id
-  tenant_id    = azurerm_user_assigned_identity.gitea_id.tenant_id
-  object_id    = azurerm_user_assigned_identity.gitea_id.principal_id
-
-  secret_permissions = ["Get", "List", ]
+resource "azurerm_role_assignment" "keyvault_gitea_ws_role" {
+  scope                = data.azurerm_key_vault.ws.id
+  role_definition_name = "Key Vault Secrets User"
+  principal_id         = azurerm_user_assigned_identity.gitea_id.principal_id
 }
 
 resource "azurerm_key_vault_secret" "gitea_password" {
@@ -165,7 +163,7 @@ resource "azurerm_key_vault_secret" "gitea_password" {
   tags         = local.workspace_service_tags
 
   depends_on = [
-    azurerm_key_vault_access_policy.gitea_policy
+    azurerm_role_assignment.keyvault_gitea_ws_role
   ]
 
   lifecycle { ignore_changes = [tags] }

diff --git a/templates/workspace_services/gitea/terraform/mysql.tf b/templates/workspace_services/gitea/terraform/mysql.tf
@@ -65,7 +65,7 @@ resource "azurerm_key_vault_secret" "db_password" {
   tags         = local.workspace_service_tags
 
   depends_on = [
-    azurerm_key_vault_access_policy.gitea_policy
+    azurerm_role_assignment.keyvault_gitea_ws_role
   ]
 
   lifecycle { ignore_changes = [tags] }

diff --git a/templates/workspace_services/guacamole/porter.yaml b/templates/workspace_services/guacamole/porter.yaml
@@ -1,7 +1,7 @@
 ---
 schemaVersion: 1.0.0
 name: tre-service-guacamole
-version: 0.10.12
+version: 0.11.0
 description: "An Azure TRE service for Guacamole"
 dockerfile: Dockerfile.tmpl
 registry: azuretre

diff --git a/templates/workspace_services/guacamole/terraform/web_app.tf b/templates/workspace_services/guacamole/terraform/web_app.tf
@@ -91,7 +91,7 @@ resource "azurerm_linux_web_app" "guacamole" {
 
   depends_on = [
     azurerm_role_assignment.guac_acr_pull,
-    azurerm_key_vault_access_policy.guacamole_policy
+    azurerm_role_assignment.keyvault_guacamole_ws_role
   ]
 }
 
@@ -143,10 +143,8 @@ resource "azurerm_private_endpoint" "guacamole" {
   lifecycle { ignore_changes = [tags] }
 }
 
-resource "azurerm_key_vault_access_policy" "guacamole_policy" {
-  key_vault_id = data.azurerm_key_vault.ws.id
-  tenant_id    = azurerm_user_assigned_identity.guacamole_id.tenant_id
-  object_id    = azurerm_user_assigned_identity.guacamole_id.principal_id
-
-  secret_permissions = ["Get", "List", ]
+resource "azurerm_role_assignment" "keyvault_guacamole_ws_role" {
+  scope                = data.azurerm_key_vault.ws.id
+  role_definition_name = "Key Vault Secrets User"
+  principal_id         = azurerm_user_assigned_identity.guacamole_id.principal_id
 }
diff --git a/templates/workspace_services/mlflow/porter.yaml b/templates/workspace_services/mlflow/porter.yaml
@@ -1,7 +1,7 @@
 ---
 schemaVersion: 1.0.0
 name: tre-service-mlflow
-version: 0.7.12
+version: 0.8.0
 description: "An Azure TRE service for MLflow machine learning lifecycle"
 dockerfile: Dockerfile.tmpl
 registry: azuretre