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.18" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| #!/bin/bash | ||
|
|
||
| function main() { | ||
|
|
||
| set -o errexit | ||
| set -o pipefail | ||
|
|
||
| # parse params/set up inputs | ||
| # | ||
| if [[ -z "$TRE_ID" ]]; then | ||
| echo -e "Could not add keyvault deployment network exception: TRE_ID is not set\nExiting...\n" | ||
| exit 1 | ||
| fi | ||
|
|
||
| local RG_NAME="rg-${TRE_ID}" | ||
| local KV_NAME="kv-${TRE_ID}" | ||
| 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 "$@" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| #!/bin/bash | ||
|
|
||
| function main() { | ||
|
|
||
| set -o errexit | ||
| set -o pipefail | ||
|
|
||
| # parse params/set up inputs | ||
| # | ||
| if [[ -z "$TRE_ID" ]]; then | ||
| echo -e "Could not remove keyvault deployment network exception: TRE_ID is not set\nExiting...\n" | ||
| exit 1 | ||
| fi | ||
|
|
||
| local RG_NAME="rg-${TRE_ID}" | ||
| local KV_NAME="kv-${TRE_ID}" | ||
| 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 "$@" |
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