diff --git a/CHANGELOG.md b/CHANGELOG.md index fcc8d8b3f6..dbc341587c 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/core/terraform/deploy.sh b/core/terraform/deploy.sh index 148cf1aca4..a622576b5c 100755 --- a/core/terraform/deploy.sh +++ b/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 diff --git a/core/terraform/destroy.sh b/core/terraform/destroy.sh index 92b6b75c4c..e3782c391e 100755 --- a/core/terraform/destroy.sh +++ b/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}" \ diff --git a/core/terraform/keyvault.tf b/core/terraform/keyvault.tf index 5d75ae9176..c491a09517 100644 --- a/core/terraform/keyvault.tf +++ b/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 = < /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/key_vault_list.sh b/devops/scripts/key_vault_list.sh index faa1aa9384..ab65ede789 100755 --- a/devops/scripts/key_vault_list.sh +++ b/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}" diff --git a/devops/scripts/kv_add_network_exception.sh b/devops/scripts/kv_add_network_exception.sh new file mode 100755 index 0000000000..90786ab2f3 --- /dev/null +++ b/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 "$@" diff --git a/devops/scripts/kv_remove_network_exception.sh b/devops/scripts/kv_remove_network_exception.sh new file mode 100755 index 0000000000..d46e6e1fd6 --- /dev/null +++ b/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 "$@" diff --git a/devops/scripts/set_contributor_sp_secrets.sh b/devops/scripts/set_contributor_sp_secrets.sh index 95a07da877..838263c3e0 100755 --- a/devops/scripts/set_contributor_sp_secrets.sh +++ b/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