Core key vault firewall should not be set to "Allow public access from all networks" #4260
There are no files selected for viewing
|
There was a problem hiding this comment. There's a pending ask to enable the deployer to access resources such as these keyvaults over private network only.
There was a problem hiding this comment.
I guess it depends on whether implementing keyvaults private networking only means switching off the ability to use Github hosted runners - is that the plan? There was a problem hiding this comment. The goal I'm referring to that resources will be accessed via private network only which means private runners. The question for you, is weather this PR comes from a similar place but doesn't go as far yet and just limits which public IPs can access. Or, in a situation where private agents / network is done, will you still need this method of limiting public IPs There was a problem hiding this comment. Yep when the codebase switches to deployment from private runners ONLY, then this change won't be needed anymore. There was a problem hiding this comment. To clarify, in your deployment/usecase you would also like to use private runners? There was a problem hiding this comment. I'm just considering most orgs that are concerned with this might want to go all the way. This solution might not be enough in those cases. There was a problem hiding this comment. Agree private runners should be the objective, but when are they scheduled to be implemented? Is this a reasonable stop gap until then? There was a problem hiding this comment. I think we need to ensure the solution can still be deployed without private runners. Most orgs start out testing out the solution etc, and don't have the infrastructure for private runners. There was a problem hiding this comment. My point is that we might not want to offer this middle way of opening the deployer IP in all currently defined public resources due to the complexities of supporting all the resources I mentioned above. There was a problem hiding this comment. Supporting all resources might be too much (e.g. the ACR) but the KV is one that stands out as worthy of tightening network access on; though I understand that private runners will supersede this once implemented. |
|
There was a problem hiding this comment. Can the Storage Account rules in this script be handles the same way? There was a problem hiding this comment. Yes certainly - planning to have a look at storage accounts after this. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| __version__ = "0.11.17" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| #!/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..." | ||
|
|
||
| if [[ -z "$(az group list --query "[?name=='$TRE_CORE_RG']" --output tsv)" ]]; then | ||
| echo -e " Core resource group $TRE_CORE_RG not found\n" | ||
| return 0 | ||
| fi | ||
|
|
||
| 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 -e " No resources found\n" | ||
| 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 "$@" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| #!/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 close 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 removing for deployment..." | ||
|
|
||
| if [[ -z "$(az group list --query "[?name=='$TRE_CORE_RG']" --output tsv)" ]]; then | ||
| echo -e " Core resource group $TRE_CORE_RG not found\n" | ||
| return 0 | ||
| fi | ||
|
|
||
| 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 -e " No resources found\n" | ||
| 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 "$@" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have time to go over this the next few days and guess @marrobi is the same. Just wanted to point out we now have 2 vaults being used from the deployer point of view.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When CMK is enabled another vault is created in the mgmt resource group