Merge branch 'main' into marrobi/issue4396

CHANGELOG.md

@@ -3,6 +3,7 @@
 **BREAKING CHANGES & MIGRATIONS**:
 
 ENHANCEMENTS:
+* Deny public access to TRE management storage account, and add private endpoint for TRE core [#4353](https://github.com/microsoft/AzureTRE/issues/4353)
 
 BUG FIXES:
 

core/terraform/data.tf

@@ -37,4 +37,4 @@ data "azurerm_monitor_diagnostic_categories" "sb" {
   depends_on = [
     azurerm_servicebus_namespace.sb
   ]
-}
+}

core/terraform/main.tf

@@ -165,6 +165,7 @@ module "resource_processor_vmss_porter" {
   acr_id                                           = data.azurerm_container_registry.mgmt_acr.id
   app_insights_connection_string                   = module.azure_monitor.app_insights_connection_string
   resource_processor_subnet_id                     = module.network.resource_processor_subnet_id
+  blob_core_dns_zone_id                            = module.network.blob_core_dns_zone_id
   docker_registry_server                           = local.docker_registry_server
   resource_processor_vmss_porter_image_repository  = var.resource_processor_vmss_porter_image_repository
   service_bus_namespace_id                         = azurerm_servicebus_namespace.sb.id

core/terraform/migrate.sh

@@ -5,6 +5,9 @@ set -o pipefail
 set -o nounset
 # set -o xtrace
 
+# shellcheck disable=SC1091
+source ../../devops/scripts/mgmtstorage_enable_public_access.sh
+
 get_resource_id() {
   local json_data="$1"
   local resource_addr="$2"

core/terraform/outputs.sh

@@ -1,6 +1,9 @@
 #!/bin/bash
 set -e
 
+# shellcheck disable=SC1091
+source ../../devops/scripts/mgmtstorage_enable_public_access.sh
+
 if [ ! -f ../tre_output.json ] || [ ! -s ../tre_output.json ]; then
   # Connect to the remote backend of Terraform
   export TF_LOG=""

core/terraform/resource_processor/vmss_porter/main.tf

@@ -224,6 +224,27 @@ resource "azurerm_role_assignment" "vmss_kv_encryption_key_user" {
   principal_id         = azurerm_user_assigned_identity.vmss_msi.principal_id
 }
 
+resource "azurerm_private_endpoint" "mgmtblobpe" {
+  name                = "pe-mgmt-blob-${var.tre_id}"
+  location            = var.location
+  resource_group_name = var.mgmt_resource_group_name
+  subnet_id           = var.resource_processor_subnet_id
+  tags                = local.tre_core_tags
+  lifecycle { ignore_changes = [tags] }
+
+  private_dns_zone_group {
+    name                 = "private-dns-zone-group-blobcore"
+    private_dns_zone_ids = [var.blob_core_dns_zone_id]
+  }
+
+  private_service_connection {
+    name                           = "psc-mgmt-${var.tre_id}"
+    private_connection_resource_id = data.azurerm_storage_account.mgmt_storage.id
+    is_manual_connection           = false
+    subresource_names              = ["Blob"]
+  }
+}
+
 module "terraform_azurerm_environment_configuration" {
   source          = "git::https://github.com/microsoft/terraform-azurerm-environment-configuration.git?ref=0.2.0"
   arm_environment = var.arm_environment

core/terraform/resource_processor/vmss_porter/variables.tf

@@ -13,6 +13,9 @@ variable "resource_group_name" {
 variable "resource_processor_subnet_id" {
   type = string
 }
+variable "blob_core_dns_zone_id" {
+  type = string
+}
 variable "resource_processor_vmss_porter_image_repository" {
   type = string
 }

core/terraform/update_tags.sh

@@ -5,6 +5,9 @@ set -o pipefail
 set -o nounset
 # set -o xtrace
 
+# shellcheck disable=SC1091
+source ../../devops/scripts/mgmtstorage_enable_public_access.sh
+
 script_dir=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
 
 core_rg_rid=$(terraform show -json | jq -r '.values.root_module.resources[] | select(.address=="azurerm_resource_group.core") | .values.id')

core/version.txt

@@ -1 +1 @@
-__version__ = "0.12.3"
+__version__ = "0.12.4"

devops/scripts/mgmtstorage_enable_public_access.sh

@@ -0,0 +1,130 @@
+#!/bin/bash
+
+#
+# Add an exception to the TRE management storage account by making it public for deployment, and remove it on script exit.
+#
+# Note: Ensure you "source" this script, or else the EXIT trap won't fire at the right time.
+#
+
+function mgmtstorage_enable_public_access() {
+  local RESOURCE_GROUP
+  RESOURCE_GROUP=$(get_resource_group_name)
+
+  local SA_NAME
+  SA_NAME=$(get_storage_account_name)
+
+  # Check that the storage account exists before making changes
+  if ! does_storage_account_exist "$SA_NAME"; then
+    echo -e "Error: Storage account $SA_NAME does not exist.\n" >&2
+    exit 1
+  fi
+
+  # Pre-check: if public access is already enabled, no need to update.
+  if is_public_access_enabled "$RESOURCE_GROUP" "$SA_NAME"; then
+    echo -e " Storage account $SA_NAME is already publicly accessible\n"
+    return
+  fi
+
+  echo -e "\nEnabling public access on storage account $SA_NAME"
+
+  # Enable public network access with explicit default action allow
+  az storage account update --resource-group "$RESOURCE_GROUP" --name "$SA_NAME" --public-network-access Enabled --default-action Allow --output none
+
+  for ATTEMPT in {1..10}; do
+    if is_public_access_enabled "$RESOURCE_GROUP" "$SA_NAME"; then
+      echo -e " Storage account $SA_NAME is now publicly accessible\n"
+      return
+    fi
+
+    echo " Unable to confirm public access on storage account $SA_NAME after $ATTEMPT/10. Waiting for update to take effect..."
+    sleep 10
+  done
+
+  echo -e "Error: Could not enable public access for $SA_NAME after 10 attempts.\n"
+  exit 1
+}
+
+function mgmtstorage_disable_public_access() {
+  local RESOURCE_GROUP
+  RESOURCE_GROUP=$(get_resource_group_name)
+
+  local SA_NAME
+  SA_NAME=$(get_storage_account_name)
+
+  # Check that the storage account exists before making changes
+  if ! does_storage_account_exist "$SA_NAME"; then
+    echo -e "Error: Storage account $SA_NAME does not exist.\n" >&2
+    exit 1
+  fi
+
+  # Pre-check: if public access is already disabled, no need to update.
+  if ! is_public_access_enabled "$RESOURCE_GROUP" "$SA_NAME"; then
+    echo -e " Storage account $SA_NAME is already not publicly accessible\n"
+    return
+  fi
+
+  echo -e "\nDisabling public access on storage account $SA_NAME"
+
+  # Disable public network access with explicit default action deny
+  az storage account update --resource-group "$RESOURCE_GROUP" --name "$SA_NAME" --public-network-access Disabled --default-action Deny --output none
+
+  for ATTEMPT in {1..10}; do
+    if ! is_public_access_enabled "$RESOURCE_GROUP" "$SA_NAME"; then
+      echo -e " Public access has been disabled successfully\n"
+      return
+    fi
+
+    echo " Unable to confirm public access is disabled on storage account $SA_NAME after $ATTEMPT/10. Waiting for update to take effect..."
+    sleep 10
+  done
+
+  echo -e "Error: Could not disable public access for $SA_NAME after 10 attempts.\n"
+  exit 1
+}
+
+function get_resource_group_name() {
+  if [[ -z "${TF_VAR_mgmt_resource_group_name:-}" ]]; then
+    echo -e "Error: TF_VAR_mgmt_resource_group_name is not set\nExiting...\n" >&2
+    exit 1
+  fi
+  echo "$TF_VAR_mgmt_resource_group_name"
+}
+
+function get_storage_account_name() {
+  if [[ -z "${TF_VAR_mgmt_storage_account_name:-}" ]]; then
+    echo -e "Error: TF_VAR_mgmt_storage_account_name is not set\nExiting...\n" >&2
+    exit 1
+  fi
+  echo "$TF_VAR_mgmt_storage_account_name"
+}
+
+function does_storage_account_exist() {
+  [[ -n "$(az storage account show --name "$1" --query "id" --output tsv)" ]]
+}
+
+function is_public_access_enabled() {
+  local RESOURCE_GROUP="$1"
+  local SA_NAME="$2"
+
+  # Try listing containers
+  local containers
+  if ! containers=$(az storage container list --account-name "$SA_NAME" --auth-mode login --query "[].name" --output tsv); then
+    return 1
+  fi
+
+  # For each container found, check blob listing
+  for container in $containers; do
+    if ! az storage blob list --container-name "$container" --account-name "$SA_NAME" --auth-mode login --output none; then
+      return 1
+    fi
+  done
+
+  # If container list succeeded (even if empty) and blob list (if any) succeeded, public access is enabled
+  return 0
+}
+
+# Setup the trap to disable public access on exit
+trap mgmtstorage_disable_public_access EXIT
+
+# Enable public access for deployment
+mgmtstorage_enable_public_access "$@"

devops/scripts/terraform_wrapper.sh

@@ -91,6 +91,9 @@ if [[ -z ${tf_logfile+x} ]]; then
     echo -e "No logfile provided, using ${tf_logfile}\n"
 fi
 
+# shellcheck disable=SC1091
+source "$(dirname "$0")/mgmtstorage_enable_public_access.sh"
+
 terraform init -input=false -backend=true -reconfigure \
     -backend-config="resource_group_name=${mgmt_resource_group_name}" \
     -backend-config="storage_account_name=${mgmt_storage_account_name}" \

devops/scripts/upgrade.sh

@@ -53,6 +53,9 @@ else
   KEY="${TRE_ID?}_${PARENT_DIR}"
 fi
 
+# shellcheck disable=SC1091
+source "$(dirname "$0")/mgmtstorage_enable_public_access.sh"
+
 # Run terraform init with upgrade and reconfigure options
 terraform -chdir="$DIR/terraform" init -upgrade -reconfigure -input=false -backend=true \
     -backend-config="resource_group_name=${TF_VAR_mgmt_resource_group_name}" \

devops/terraform/bootstrap.sh

@@ -28,6 +28,9 @@ else
   az storage account show --resource-group "$TF_VAR_mgmt_resource_group_name" --name "$TF_VAR_mgmt_storage_account_name" --output table
 fi
 
+# shellcheck disable=SC1091
+source ../scripts/mgmtstorage_enable_public_access.sh
+
 # Grant user blob data contributor permissions
 echo -e "\n\e[34m»»» 🔑 \e[96mGranting Storage Blob Data Contributor role to the current user\e[0m..."
 if [ -n "${ARM_CLIENT_ID:-}" ]; then

devops/terraform/deploy.sh

@@ -5,6 +5,9 @@ set -o pipefail
 set -o nounset
 # set -o xtrace
 
+# shellcheck disable=SC1091
+source ../scripts/mgmtstorage_enable_public_access.sh
+
 PLAN_FILE="devops.tfplan"
 
 terraform init -input=false -backend=true -reconfigure

devops/terraform/destroy.sh

@@ -1 +1,11 @@
-terraform destroy -auto-approve
+#!/bin/bash
+
+set -o errexit
+set -o pipefail
+set -o nounset
+# set -o xtrace
+
+# shellcheck disable=SC1091
+source ../scripts/mgmtstorage_enable_public_access.sh
+
+terraform destroy -auto-approve

devops/version.txt

@@ -1 +1 @@
-__version__ = "0.5.5"
+__version__ = "0.5.6"