Core key vault firewall should not be set to "Allow public access fro…

…m all networks" (#4260)

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

CHANGELOG.md

@@ -4,6 +4,7 @@
 **BREAKING CHANGES & MIGRATIONS**:
 
 ENHANCEMENTS:
+* Core key vault firewall should not be set to "Allow public access from all networks" ([#4250](https://github.com/microsoft/AzureTRE/issues/4250))
 * Allow workspace App Service Plan SKU to be updated ([#4331](https://github.com/microsoft/AzureTRE/issues/4331))
 * Add core requests endpoint and UI to enable requests to be managed TRE wide. ([[#2510](https://github.com/microsoft/AzureTRE/issues/2510)])
 * Remove public IP from TRE's firewall when forced tunneling is configured ([#4346](https://github.com/microsoft/AzureTRE/pull/4346))

core/terraform/deploy.sh

@@ -5,6 +5,9 @@ set -o pipefail
 set -o nounset
 # set -o xtrace
 
+# 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,9 @@ set -o pipefail
 set -o nounset
 # set -o xtrace
 
+# 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

@@ -58,4 +58,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

@@ -8,6 +8,11 @@ if [[ -z ${STORAGE_ACCOUNT} ]]; then
   exit 1
 fi
 
+if [[ -n ${KEYVAULT} ]]; then
+  # shellcheck disable=SC1091
+  source "$script_dir/../../../devops/scripts/kv_add_network_exception.sh"
+fi
+
 # The storage account is protected by network rules
 #
 # The rules need to be temporarily lifted so that the script can determine if the index.html file

core/version.txt

@@ -1 +1 @@
-__version__ = "0.12.1"
+__version__ = "0.12.2"

devops/scripts/destroy_env_no_terraform.sh

@@ -66,6 +66,11 @@ then
   no_wait_option="--no-wait"
 fi
 
+script_dir=$(realpath "$(dirname "${BASH_SOURCE[0]}")")
+
+# 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/kv_add_network_exception.sh

@@ -0,0 +1,131 @@
+#!/bin/bash
+
+#
+# Add an IP exception to the Key Vault firewall for deployment, and remove on script exit
+# The current machine's IP address is used, or $PUBLIC_DEPLOYMENT_IP_ADDRESS if set
+#
+# Note:  Ensure you "source" this script, or else the EXIT trap won't fire at the right time
+#
+
+
+function kv_add_network_exception() {
+
+  # set up variables
+  #
+  local KV_NAME
+  KV_NAME=$(get_kv_name)
+
+  local MY_IP
+  MY_IP=$(get_my_ip)
+
+  echo -e "\nAdding deployment network exception to key vault $KV_NAME..."
+
+  # ensure kv exists
+  #
+  if ! does_kv_exist "$KV_NAME"; then
+    return 0   # don't cause outer sourced script to fail
+  fi
+
+  # add keyvault network exception
+  #
+  az keyvault network-rule add --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
+
+}
+
+function kv_remove_network_exception() {
+
+  # set up variables
+  #
+  local KV_NAME
+  KV_NAME=$(get_kv_name)
+
+  local MY_IP
+  MY_IP=$(get_my_ip)
+
+  echo -e "\nRemoving deployment network exception to key vault $KV_NAME..."
+
+  # ensure kv exists
+  #
+  if ! does_kv_exist "$KV_NAME"; then
+    return 0   # don't cause outer sourced script to fail
+  fi
+
+  # remove keyvault network exception
+  #
+  az keyvault network-rule remove --name "$KV_NAME" --ip-address "$MY_IP" --output none
+  echo -e " Deployment network exception removed\n"
+}
+
+
+function get_kv_name() {
+
+  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/remove keyvault deployment network exception: TRE_ID is not set\nExiting...\n"
+    exit 1
+  fi
+
+  echo "kv-${TRE_ID_LOCAL}"
+}
+
+function get_my_ip() {
+
+  local MY_IP="${PUBLIC_DEPLOYMENT_IP_ADDRESS:-}"
+
+  if [[ -z "$MY_IP" ]]; then
+    MY_IP=$(curl -s "ipecho.net/plain"; echo)
+  fi
+
+  echo "$MY_IP"
+}
+
+
+function does_kv_exist() {
+
+  KV_NAME=$1
+
+  if [[ -z "$(az keyvault list --query "[?name=='$KV_NAME'].id" --output tsv)" ]]; then
+      echo -e " Core key vault $KV_NAME not found\n"
+      return 1
+  fi
+
+  return 0
+}
+
+
+# setup the trap to remove network exception on exit
+trap kv_remove_network_exception EXIT
+
+# now add the network exception
+kv_add_network_exception "$@"