From 63536c53b2b2076aa79f6fd47b0940d7440d7eb2 Mon Sep 17 00:00:00 2001 From: jonnyry Date: Mon, 6 Jan 2025 11:43:21 +0000 Subject: [PATCH] Core key vault firewall should not be set to "Allow public access from all networks" #4250 Core key vault firewall should not be set to "Allow public access from all networks" Update keyvault.tf Add network exceptions via script Remove azapi use --- CHANGELOG.md | 1 + core/terraform/.terraform.lock.hcl | 19 ++++ core/terraform/deploy.sh | 7 ++ core/terraform/destroy.sh | 4 + core/terraform/keyvault.tf | 26 ++++- core/terraform/locals.tf | 4 + core/terraform/scripts/letsencrypt.sh | 7 ++ core/version.txt | 2 +- .../add_deployment_network_exceptions.sh | 95 +++++++++++++++++++ devops/scripts/destroy_env_no_terraform.sh | 8 ++ .../remove_deployment_network_exceptions.sh | 69 ++++++++++++++ 11 files changed, 240 insertions(+), 2 deletions(-) create mode 100644 devops/scripts/add_deployment_network_exceptions.sh create mode 100644 devops/scripts/remove_deployment_network_exceptions.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index df36da2a72..799ae7d74b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,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)) BUG FIXES: * Update KeyVault references in API to use the version so Terraform cascades the update ([#4112](https://github.com/microsoft/AzureTRE/pull/4112)) diff --git a/core/terraform/.terraform.lock.hcl b/core/terraform/.terraform.lock.hcl index 1c20359910..7c6b650d30 100644 --- a/core/terraform/.terraform.lock.hcl +++ b/core/terraform/.terraform.lock.hcl @@ -82,6 +82,25 @@ provider "registry.terraform.io/hashicorp/local" { ] } +provider "registry.terraform.io/hashicorp/null" { + version = "3.2.3" + hashes = [ + "h1:+AnORRgFbRO6qqcfaQyeX80W0eX3VmjadjnUFUJTiXo=", + "zh:22d062e5278d872fe7aed834f5577ba0a5afe34a3bdac2b81f828d8d3e6706d2", + "zh:23dead00493ad863729495dc212fd6c29b8293e707b055ce5ba21ee453ce552d", + "zh:28299accf21763ca1ca144d8f660688d7c2ad0b105b7202554ca60b02a3856d3", + "zh:55c9e8a9ac25a7652df8c51a8a9a422bd67d784061b1de2dc9fe6c3cb4e77f2f", + "zh:756586535d11698a216291c06b9ed8a5cc6a4ec43eee1ee09ecd5c6a9e297ac1", + "zh:78d5eefdd9e494defcb3c68d282b8f96630502cac21d1ea161f53cfe9bb483b3", + "zh:9d5eea62fdb587eeb96a8c4d782459f4e6b73baeece4d04b4a40e44faaee9301", + "zh:a6355f596a3fb8fc85c2fb054ab14e722991533f87f928e7169a486462c74670", + "zh:b5a65a789cff4ada58a5baffc76cb9767dc26ec6b45c00d2ec8b1b027f6db4ed", + "zh:db5ab669cf11d0e9f81dc380a6fdfcac437aea3d69109c7aef1a5426639d2d65", + "zh:de655d251c470197bcbb5ac45d289595295acb8f829f6c781d4a75c8c8b7c7dd", + "zh:f5c68199f2e6076bce92a12230434782bf768103a427e9bb9abee99b116af7b5", + ] +} + provider "registry.terraform.io/hashicorp/random" { version = "3.6.3" constraints = ">= 3.0.0, ~> 3.6" diff --git a/core/terraform/deploy.sh b/core/terraform/deploy.sh index e71fb14ae1..1c7a270d71 100755 --- a/core/terraform/deploy.sh +++ b/core/terraform/deploy.sh @@ -5,6 +5,12 @@ set -o pipefail set -o nounset # set -o xtrace +# add trap to remove deployment network exceptions +trap 'source "../../devops/scripts/remove_deployment_network_exceptions.sh"' EXIT + +# now add deployment network exceptions +source "../../devops/scripts/add_deployment_network_exceptions.sh" + # This is where we can migrate any Terraform before we plan and apply # For instance deprecated Terraform resources # shellcheck disable=SC1091 @@ -26,4 +32,5 @@ LOG_FILE="${TS}-tre-core.log" terraform apply -input=false -auto-approve ${PLAN_FILE} && \ terraform output -json > ../tre_output.json" + ./update_tags.sh diff --git a/core/terraform/destroy.sh b/core/terraform/destroy.sh index 92b6b75c4c..e1ad9a601a 100755 --- a/core/terraform/destroy.sh +++ b/core/terraform/destroy.sh @@ -5,6 +5,10 @@ set -o pipefail set -o nounset # set -o xtrace +# add deployment network exceptions +source "../../devops/scripts/add_deployment_network_exceptions.sh" + + # These variables are loaded in for us # shellcheck disable=SC2154 ../../devops/scripts/terraform_wrapper.sh -g "${TF_VAR_mgmt_resource_group_name}" \ diff --git a/core/terraform/keyvault.tf b/core/terraform/keyvault.tf index 5d75ae9176..05445994ec 100644 --- a/core/terraform/keyvault.tf +++ b/core/terraform/keyvault.tf @@ -8,7 +8,31 @@ 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 = true + + network_acls { + default_action = var.enable_local_debugging ? "Allow" : "Deny" + bypass = "AzureServices" + ip_rules = [ local.myip ] # exception for deployment IP, this is removed in the tf postfix step + } + + lifecycle { + ignore_changes = [access_policy, tags] + } +} + +# add a tag to the key vault denoting it requires an deployment network exception adding +# +resource "null_resource" "add_deployment_tag" { + triggers = { + always_run = timestamp() + } + + provisioner "local-exec" { + command = "az resource update --ids ${azurerm_key_vault.kv.id} --set 'tags.${local.tre_deployment_network_exception_tag}=\"true\"' --output none" + } + + depends_on = [azurerm_key_vault.kv] } resource "azurerm_role_assignment" "keyvault_deployer_role" { diff --git a/core/terraform/locals.tf b/core/terraform/locals.tf index 22d327f96f..b75ec58705 100644 --- a/core/terraform/locals.tf +++ b/core/terraform/locals.tf @@ -5,6 +5,10 @@ locals { tre_core_service_id = var.tre_id } + # used to tag storage accounts and key vaults that require a IP exception for the deployment runner + # adding to their network firewall during TRE deployment (and removing at the end of deployment) + tre_deployment_network_exception_tag = "tre_deployment_network_exception" + api_diagnostic_categories_enabled = [ "AppServiceHTTPLogs", "AppServiceConsoleLogs", "AppServiceAppLogs", "AppServiceAuditLogs", "AppServiceIPSecAuditLogs", "AppServicePlatformLogs", "AppServiceAntivirusScanAuditLogs" diff --git a/core/terraform/scripts/letsencrypt.sh b/core/terraform/scripts/letsencrypt.sh index cb1e68f4a1..a45a48f00b 100755 --- a/core/terraform/scripts/letsencrypt.sh +++ b/core/terraform/scripts/letsencrypt.sh @@ -3,6 +3,13 @@ set -e script_dir=$(realpath "$(dirname "${BASH_SOURCE[0]}")") +# add trap to remove deployment network exceptions on script exit +trap 'source "$script_dir/../../../devops/scripts/remove_deployment_network_exceptions.sh"' EXIT + +# now add deployment network exceptions +source "$script_dir/../../../devops/scripts/add_deployment_network_exceptions.sh" + + if [[ -z ${STORAGE_ACCOUNT} ]]; then echo "STORAGE_ACCOUNT not set" exit 1 diff --git a/core/version.txt b/core/version.txt index 87583f1acf..1e6e806534 100644 --- a/core/version.txt +++ b/core/version.txt @@ -1 +1 @@ -__version__ = "0.11.15" +__version__ = "0.11.16" diff --git a/devops/scripts/add_deployment_network_exceptions.sh b/devops/scripts/add_deployment_network_exceptions.sh new file mode 100644 index 0000000000..d14ae6a225 --- /dev/null +++ b/devops/scripts/add_deployment_network_exceptions.sh @@ -0,0 +1,95 @@ +#!/bin/bash + +TRE_DEPLOYMENT_NETWORK_EXCEPTION_TAG="tre_deployment_network_exception" + +function main() { + + set -o errexit + set -o pipefail + + + # parse params/set up inputs + # + if [[ -z "$TRE_ID" ]]; then + echo -e "Could not open deployment network exceptions: TRE_ID is not set\nExiting...\n" + exit 1 + fi + + local MY_IP="${PUBLIC_DEPLOYMENT_IP_ADDRESS:-}" + + if [[ -z "$MY_IP" ]]; then + MY_IP=$(curl -s "ipecho.net/plain"; echo) + fi + + local TRE_CORE_RG="rg-${TRE_ID}" + + + # find resources that require network exceptions + # + echo -e "\nQuerying resources that require network exceptions adding for deployment..." + + local AZ_IDS + AZ_IDS=$(az resource list --resource-group "$TRE_CORE_RG" --query "[?tags.${TRE_DEPLOYMENT_NETWORK_EXCEPTION_TAG}=='true'].id" --output tsv) + + if [ -z "$AZ_IDS" ]; then + echo " No resources found" + return 0 + fi + + + # add network exceptions + # + local AZ_ID + for AZ_ID in $AZ_IDS; do + + local RESOURCE_TYPE + RESOURCE_TYPE=$(az resource show --ids "${AZ_ID}" --query 'type' --output tsv) + + if [ "$RESOURCE_TYPE" == "Microsoft.KeyVault/vaults" ]; then + add_keyvault_network_exception "$AZ_ID" "$MY_IP" + fi + + done + + echo "" + +} + +function add_keyvault_network_exception() { + local AZ_ID="$1" + local MY_IP="$2" + + local KV_NAME + KV_NAME=$(basename "$AZ_ID") + + echo " Adding keyvault deployment network exception for $KV_NAME" + + 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 " Keyvault $KV_NAME is now accessible" + 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 "$@" diff --git a/devops/scripts/destroy_env_no_terraform.sh b/devops/scripts/destroy_env_no_terraform.sh index e327f313c6..a8a975aacf 100755 --- a/devops/scripts/destroy_env_no_terraform.sh +++ b/devops/scripts/destroy_env_no_terraform.sh @@ -66,6 +66,14 @@ then no_wait_option="--no-wait" fi +script_dir=$(realpath "$(dirname "${BASH_SOURCE[0]}")") + +# add trap to remove deployment network exceptions on script exit +trap 'source "$script_dir/remove_deployment_network_exceptions.sh"' EXIT + +# now add deployment network exceptions +source "$script_dir/add_deployment_network_exceptions.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" diff --git a/devops/scripts/remove_deployment_network_exceptions.sh b/devops/scripts/remove_deployment_network_exceptions.sh new file mode 100644 index 0000000000..c80517a979 --- /dev/null +++ b/devops/scripts/remove_deployment_network_exceptions.sh @@ -0,0 +1,69 @@ +#!/bin/bash + +TRE_DEPLOYER_FW_EXCEPTION_TAG="tre_deployment_network_exception" + +function main() { + + set -o errexit + set -o pipefail + + # parse params/set up inputs + # + if [[ -z "$TRE_ID" ]]; then + echo -e "Could not close deployment network exceptions: TRE_ID is not set\n" + exit 1 + fi + + local MY_IP="${PUBLIC_DEPLOYMENT_IP_ADDRESS:-}" + + if [[ -z "$MY_IP" ]]; then + MY_IP=$(curl -s "ipecho.net/plain"; echo) + fi + + local TRE_CORE_RG="rg-${TRE_ID}" + + + # find resources that require network exceptions + # + echo -e "\nQuerying resources that require network exceptions removing for deployment..." + + local AZ_IDS + AZ_IDS=$(az resource list --resource-group "$TRE_CORE_RG" --query "[?tags.${TRE_DEPLOYER_FW_EXCEPTION_TAG}=='true'].id" --output tsv) + + if [ -z "$AZ_IDS" ]; then + echo " No resources found" + return 0 + fi + + + # remove network exceptions + # + local AZ_ID + for AZ_ID in $AZ_IDS; do + + local RESOURCE_TYPE + RESOURCE_TYPE=$(az resource show --ids "${AZ_ID}" --query 'type' --output tsv) + + if [ "$RESOURCE_TYPE" == "Microsoft.KeyVault/vaults" ]; then + remove_keyvault_network_exception "$AZ_ID" "$MY_IP" + fi + + done + + echo "" + +} + +function remove_keyvault_network_exception() { + local AZ_ID="$1" + local MY_IP="$2" + + local KV_NAME + KV_NAME=$(basename "$AZ_ID") + + echo " Removing keyvault deployment network exception for $KV_NAME" + + az keyvault network-rule remove --name "$KV_NAME" --ip-address "$MY_IP" --output none +} + +main "$@"