Core key vault firewall should not be set to "Allow public access from all networks"

CHANGELOG.md

@@ -32,6 +32,7 @@ ENHANCEMENTS:
 * Upgrade Python version from 3.8 to 3.12 ([#3949](https://github.com/microsoft/AzureTRE/issues/3949))Upgrade Python version from 3.8 to 3.12 (#3949)
 * Disable storage account key usage ([[#4227](https://github.com/microsoft/AzureTRE/issues/4227)])
 * Update Guacamole dependencies ([[#4232](https://github.com/microsoft/AzureTRE/issues/4232)])
+* Core key vault firewall should not be set to "Allow public access from all networks" ([#4250](https://github.com/microsoft/AzureTRE/issues/4250))
 * Add option to force tunnel TRE's Firewall ([#4237](https://github.com/microsoft/AzureTRE/issues/4237))
 * Add EventGrid diagnostics to identify airlock issues ([#4258](https://github.com/microsoft/AzureTRE/issues/4258))
 

core/terraform/deploy.sh

@@ -5,6 +5,14 @@ set -o pipefail
 set -o nounset
 # set -o xtrace
 
+# add trap to remove kv network exception
+# shellcheck disable=SC1091
+trap 'source "../../devops/scripts/kv_remove_network_exception.sh"' EXIT
+
+# now add kv network exception
+# shellcheck disable=SC1091
+source "../../devops/scripts/kv_add_network_exception.sh"
+
 # This is where we can migrate any Terraform before we plan and apply
 # For instance deprecated Terraform resources
 # shellcheck disable=SC1091

core/terraform/destroy.sh

@@ -5,6 +5,14 @@ set -o pipefail
 set -o nounset
 # set -o xtrace
 
+# add trap to remove kv network exception
+# shellcheck disable=SC1091
+trap 'source "../../devops/scripts/kv_remove_network_exception.sh"' EXIT
+
+# now add kv network exception
+# shellcheck disable=SC1091
+source "../../devops/scripts/kv_add_network_exception.sh"
+
 # These variables are loaded in for us
 # shellcheck disable=SC2154
 ../../devops/scripts/terraform_wrapper.sh -g "${TF_VAR_mgmt_resource_group_name}" \

core/terraform/keyvault.tf

@@ -1,5 +1,5 @@
 resource "azurerm_key_vault" "kv" {
-  name                      = "kv-${var.tre_id}"
+  name                      = local.kv_name
   tenant_id                 = data.azurerm_client_config.current.tenant_id
   location                  = azurerm_resource_group.core.location
   resource_group_name       = azurerm_resource_group.core.name
@@ -8,7 +8,27 @@ resource "azurerm_key_vault" "kv" {
   purge_protection_enabled  = var.kv_purge_protection_enabled
   tags                      = local.tre_core_tags
 
-  lifecycle { ignore_changes = [access_policy, tags] }
+  public_network_access_enabled = local.kv_public_network_access_enabled
+
+  network_acls {
+    default_action = local.kv_network_default_action
+    bypass         = local.kv_network_bypass
+    ip_rules       = [local.myip] # exception for deployment IP, this is removed in kv_remove_network_exception.sh
+  }
+
+  lifecycle {
+    ignore_changes = [access_policy, tags]
+  }
+
+  # create provisioner required due to https://github.com/hashicorp/terraform-provider-azurerm/issues/18970
+  #
+  provisioner "local-exec" {
+    when    = create
+    command = <<EOT
+az keyvault update --name ${local.kv_name} --public-network-access ${local.kv_public_network_access_enabled ? "Enabled" : "Disabled"} --default-action ${local.kv_network_default_action} --bypass "${local.kv_network_bypass}" --output none
+az keyvault network-rule add --name ${local.kv_name} --ip-address ${local.myip} --output none
+EOT
+  }
 }
 
 resource "azurerm_role_assignment" "keyvault_deployer_role" {

core/terraform/locals.tf

@@ -46,4 +46,10 @@ locals {
 
   cmk_name                 = "tre-encryption-${var.tre_id}"
   encryption_identity_name = "id-encryption-${var.tre_id}"
+
+  # key vault variables
+  kv_name                          = "kv-${var.tre_id}"
+  kv_public_network_access_enabled = true
+  kv_network_default_action        = var.enable_local_debugging ? "Allow" : "Deny"
+  kv_network_bypass                = "AzureServices"
 }

core/terraform/scripts/letsencrypt.sh

@@ -3,6 +3,15 @@ set -e
 
 script_dir=$(realpath "$(dirname "${BASH_SOURCE[0]}")")
 
+# add trap to remove kv network exception
+# shellcheck disable=SC1091
+trap 'source "$script_dir/../../../devops/scripts/kv_remove_network_exception.sh"' EXIT
+
+# now add kv network exception
+# shellcheck disable=SC1091
+source "$script_dir/../../../devops/scripts/kv_add_network_exception.sh"
+
+
 if [[ -z ${STORAGE_ACCOUNT} ]]; then
   echo "STORAGE_ACCOUNT not set"
   exit 1

core/version.txt

@@ -1 +1 @@
-__version__ = "0.11.17"
+__version__ = "0.11.18"

devops/scripts/destroy_env_no_terraform.sh

@@ -66,6 +66,16 @@ then
   no_wait_option="--no-wait"
 fi
 
+script_dir=$(realpath "$(dirname "${BASH_SOURCE[0]}")")
+
+# add trap to remove kv network exception
+# shellcheck disable=SC1091
+trap 'source "$script_dir/kv_remove_network_exception.sh"' EXIT
+
+# now add kv network exception
+# shellcheck disable=SC1091
+source "$script_dir/kv_add_network_exception.sh"
+
 group_show_result=$(az group show --name "${core_tre_rg}" > /dev/null 2>&1; echo $?)
 if [[ "$group_show_result" !=  "0" ]]; then
   echo "Resource group ${core_tre_rg} not found - skipping destroy"

devops/scripts/key_vault_list.sh

@@ -7,11 +7,21 @@ fi
 
 echo "DEBUG: Check keyvault and secrets exist"
 
+script_dir=$(realpath "$(dirname "${BASH_SOURCE[0]}")")
+
+# add trap to remove kv network exception
+# shellcheck disable=SC1091
+trap 'source "$script_dir/kv_remove_network_exception.sh"' EXIT
+
+# now add kv network exception
+# shellcheck disable=SC1091
+source "$script_dir/kv_add_network_exception.sh"
+
 echo "az keyvault show"
-az keyvault show --name kv-${TRE_ID}
+az keyvault show --name "kv-${TRE_ID}"
 
 echo "az keyvault secret list"
-az keyvault secret list --vault-name kv-${TRE_ID}
+az keyvault secret list --vault-name "kv-${TRE_ID}"
 
 echo "az keyvault secret list-deleted"
-az keyvault secret list-deleted --vault-name kv-${TRE_ID}
+az keyvault secret list-deleted --vault-name "kv-${TRE_ID}"

devops/scripts/kv_add_network_exception.sh

@@ -0,0 +1,76 @@
+#!/bin/bash
+
+function main() {
+
+    set -o errexit
+    set -o pipefail
+
+    # attempt to determine our tre id
+    #
+    local TRE_ID_LOCAL="${TRE_ID:-}"
+
+    if [[ -z "$TRE_ID_LOCAL" ]]; then
+      if [[ "${core_tre_rg:-}" == rg-* ]]; then  # TRE_ID may not be available when called from destroy_env_no_terraform.sh
+        TRE_ID_LOCAL="${core_tre_rg#rg-}"
+      fi
+    fi
+
+    if [[ -z "$TRE_ID_LOCAL" ]]; then
+      echo -e "Could not add keyvault deployment network exception: TRE_ID is not set\nExiting...\n"
+      exit 1
+    fi
+
+    # set up variables
+    #
+    local RG_NAME="rg-${TRE_ID_LOCAL}"
+    local KV_NAME="kv-${TRE_ID_LOCAL}"
+    local MY_IP="${PUBLIC_DEPLOYMENT_IP_ADDRESS:-}"
+
+    if [[ -z "$MY_IP" ]]; then
+      MY_IP=$(curl -s "ipecho.net/plain"; echo)
+    fi
+
+
+    # add keyvault network exception
+    #
+    echo -e "\nAdding deployment network exception to key vault $KV_NAME..."
+
+    if [[ -z "$(az group list --query "[?name=='$RG_NAME']" --output tsv)" ]]; then
+        echo -e " Core resource group $RG_NAME not found\n"
+        return 0
+    fi
+
+    if [[ -z "$(az keyvault list --resource-group "$RG_NAME" --query "[?name=='$KV_NAME'].id" --output tsv)" ]]; then
+        echo -e " Core key vault $KV_NAME not found\n"
+        return 0
+    fi
+
+    az keyvault network-rule add --resource-group "$RG_NAME" --name "$KV_NAME" --ip-address "$MY_IP" --output none
+
+    local ATTEMPT=1
+    local MAX_ATTEMPTS=10
+
+    while true; do
+
+      if KV_OUTPUT=$(az keyvault secret list --vault-name "$KV_NAME" --query '[].name' --output tsv 2>&1); then
+        echo -e " Keyvault $KV_NAME is now accessible\n"
+        break
+      fi
+
+      if [ $ATTEMPT -eq $MAX_ATTEMPTS ]; then
+        echo -e "Could not add deployment network exception for $KV_NAME"
+        echo -e "Unable to access keyvault $KV_NAME after $ATTEMPT/$MAX_ATTEMPTS.\n"
+        echo -e "$KV_OUTPUT\n"
+
+        exit 1
+      fi
+
+      echo " Unable to access keyvault $KV_NAME after $ATTEMPT/$MAX_ATTEMPTS. Waiting for network rules to take effect."
+      sleep 5
+      ((ATTEMPT++))
+
+    done
+
+}
+
+main "$@"

devops/scripts/kv_remove_network_exception.sh

@@ -0,0 +1,53 @@
+#!/bin/bash
+
+function main() {
+
+    set -o errexit
+    set -o pipefail
+
+    # attempt to determine our tre id
+    #
+    local TRE_ID_LOCAL="${TRE_ID:-}"
+
+    if [[ -z "$TRE_ID_LOCAL" ]]; then
+      if [[ "${core_tre_rg:-}" == rg-* ]]; then  # TRE_ID may not be available when called from destroy_env_no_terraform.sh
+        TRE_ID_LOCAL="${core_tre_rg#rg-}"
+      fi
+    fi
+
+    if [[ -z "$TRE_ID_LOCAL" ]]; then
+      echo -e "Could not remove keyvault deployment network exception: TRE_ID is not set\nExiting...\n"
+      exit 1
+    fi
+
+    # set up variables
+    #
+    local RG_NAME="rg-${TRE_ID_LOCAL}"
+    local KV_NAME="kv-${TRE_ID_LOCAL}"
+    local MY_IP="${PUBLIC_DEPLOYMENT_IP_ADDRESS:-}"
+
+    if [[ -z "$MY_IP" ]]; then
+      MY_IP=$(curl -s "ipecho.net/plain"; echo)
+    fi
+
+
+    # remove keyvault network exception
+    #
+    echo -e "\nRemoving deployment network exception to key vault $KV_NAME..."
+
+    if [[ -z "$(az group list --query "[?name=='$RG_NAME']" --output tsv)" ]]; then
+        echo -e " Core resource group $RG_NAME not found\n"
+        return 0
+    fi
+
+    if [[ -z "$(az keyvault list --resource-group "$RG_NAME" --query "[?name=='$KV_NAME'].id" --output tsv)" ]]; then
+        echo -e " Core key vault $KV_NAME not found\n"
+        return 0
+    fi
+
+    az keyvault network-rule remove --name "$KV_NAME" --ip-address "$MY_IP" --output none
+    echo -e " Deployment network exception removed\n"
+
+}
+
+main "$@"

devops/scripts/set_contributor_sp_secrets.sh

@@ -16,7 +16,19 @@ set -e
 #
 
 echo -e "\n\e[34m»»» 🤖 \e[96mCreating (or updating) service principal ID and secret to Key Vault\e[0m..."
+
+script_dir=$(realpath "$(dirname "${BASH_SOURCE[0]}")")
+
+# add trap to remove kv network exception
+# shellcheck disable=SC1091
+trap 'source "$script_dir/kv_remove_network_exception.sh"' EXIT
+
+# now add kv network exception
+# shellcheck disable=SC1091
+source "$script_dir/kv_add_network_exception.sh"
+
+
 key_vault_name="kv-$TRE_ID"
-az account set --subscription $ARM_SUBSCRIPTION_ID
-az keyvault secret set --name deployment-processor-azure-client-id --vault-name $key_vault_name --value $RESOURCE_PROCESSOR_CLIENT_ID
-az keyvault secret set --name deployment-processor-azure-client-secret --vault-name $key_vault_name --value $RESOURCE_PROCESSOR_CLIENT_SECRET > /dev/null
+az account set --subscription "$ARM_SUBSCRIPTION_ID"
+az keyvault secret set --name deployment-processor-azure-client-id --vault-name "$key_vault_name" --value "$RESOURCE_PROCESSOR_CLIENT_ID"
+az keyvault secret set --name deployment-processor-azure-client-secret --vault-name "$key_vault_name" --value "$RESOURCE_PROCESSOR_CLIENT_SECRET" > /dev/null