Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix TF deployment issue / terraform-stats step #16819

Open
devopsmatt opened this issue Dec 16, 2024 · 32 comments
Open

Fix TF deployment issue / terraform-stats step #16819

devopsmatt opened this issue Dec 16, 2024 · 32 comments
Assignees
Labels
blocked Issue State label to flag PRs and issues to show they are blocked bug Issue Type label to flag an issue that is a bug DevSecOps Team Aq DevSecOps work label reportstream Stretch Bonus items brought into Sprint tech-debt Anything that is purely a technical issue and does not affect functionality terraform Pull requests that update Terraform code

Comments

@devopsmatt
Copy link
Collaborator

devopsmatt commented Dec 16, 2024

Context:
https://github.com/CDCgov/prime-reportstream/actions/runs/12359553628
https://github.com/CDCgov/prime-reportstream/actions/runs/12359553628/job/34492594460#step:4:45
https://github.com/CDCgov/prime-reportstream/actions/runs/12359553628/workflow

Ask:
Figure out why this fails, what is the value of 'terraform-stats' and consider removing that action from the TF deployment workflow if the value is low.

DoD:
TF can be deployed via https://github.com/CDCgov/prime-reportstream/actions/runs/12359553628/workflow

@devopsmatt devopsmatt added tech-debt Anything that is purely a technical issue and does not affect functionality reportstream terraform Pull requests that update Terraform code DevSecOps Team Aq DevSecOps work label labels Dec 16, 2024
@devopsmatt devopsmatt changed the title Fix TF issue Fix TF deployment issue / terraform-stats step Dec 16, 2024
@devopsmatt
Copy link
Collaborator Author

devopsmatt commented Dec 16, 2024

May resolve #16534 and #16528

@devopsmatt devopsmatt added bug Issue Type label to flag an issue that is a bug Stretch Bonus items brought into Sprint labels Dec 17, 2024
@emvaldes
Copy link
Collaborator

There are a few issues here that need to be addressed.
I will be working on this issue.

@devopsmatt devopsmatt added Spillover-Dependencies/Blockers (External) Task delayed due to dependencies on external factors and removed Spillover-Dependencies/Blockers (External) Task delayed due to dependencies on external factors labels Jan 2, 2025
@emvaldes emvaldes added the blocked Issue State label to flag PRs and issues to show they are blocked label Jan 2, 2025
@emvaldes
Copy link
Collaborator

emvaldes commented Jan 2, 2025

This issue requires for the Service Principal account (newly re-created) to have its credentials expanded to include infrastructure access.
Note: This Service Principal is a contributor and we need to ask the CDC Support team to expand on its privileges.

This is a process that will require a ticket to be created so they can track their contributions within their own processes. The issue is that we do not know which privileges are required. As it's already known, we are facing an absolute lack of documentation in what these configurations are. It will require time and a lot of effort to try and catch all issues that will surface it's this is ironed down.

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 2, 2025

@devopsmatt: Please, could you address the ticket creation issue and establish the request?
It's known that the current Service Now capabilities are limited to only grant visibility to the issue-requester and you should be the one having full visibility on this so that others at the management level can inquire you about it and you can then keep them informed in full details.

@emvaldes emvaldes assigned devopsmatt and unassigned emvaldes Jan 2, 2025
@devopsmatt
Copy link
Collaborator Author

@emvaldes please open a ticket with Cloud Services and work with them to resolution. If you need me in any working sessions, please pull me in.

@devopsmatt devopsmatt assigned emvaldes and unassigned devopsmatt Jan 3, 2025
@emvaldes
Copy link
Collaborator

emvaldes commented Jan 3, 2025

A ticket was submitted by @devopsmatt to the CDC Cloud support team.
Submitted : 01/02/2025 17:59:28
Request Number : REQ0399128

I am awaiting for updates to this project.

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 6, 2025

The execution of this process in the Deploy Terraform pipeline yields errors in the Check Terraform Stats - Staging stage in the Deploy Terraform pipeline:
Set Build Environment -> Check Terraform Stats - Staging -> Approve Deploy -> Run Deploy.

All errors are triggered by:

Error: making Read request on Azure KeyVault Secret

All errors yield:

GetSecret: Failure responding to request: StatusCode=403 + Code="Forbidden"
-- Original Error: autorest/azure: Service returned an error.

Reported Errors:

The user, group or application:
appid=***;
oid=ac21f8dc-f2ee-4a56-bd88-51a5ae230dd8;
iss=https://sts.windows.net/***/'
... does not have secrets get permission on key vault:
key vault 'pdhstaging-keyvault;location=eastus'
or
key vault 'pdhstaging-appconfig;location=eastus'

Recommendations:

For help resolving this issue, please see 
https://go.microsoft.com/fwlink/?linkid=2125287" InnerError={"code":"AccessDenied"}

Hash-Commit: 68b8cbe42c494333fbf6f8d90ac86da1fb69dcc2

Run /home/runner/work/_actions/josiahsiegel/terraform-stats/<hash-commit>/lib/tf_stats.sh \
  75: data "azurerm_key_vault_secret" "OKTA_authKey" {

tf-caller-ip-addresses: keyvault.BaseClient
  82: data "azurerm_key_vault_secret" "caller_ip_addresses" {

functionapp-RS-OKTA-ClientId: keyvault.BaseClient
  86: data "azurerm_key_vault_secret" "RS_OKTA_clientId" {
 
functionapp-RS-OKTA-authkey: keyvault.BaseClient
  92: data "azurerm_key_vault_secret" "RS_OKTA_authKey" {

functionapp-cdctiautomated: keyvault.BaseClient
  98: data "azurerm_key_vault_secret" "cdctiautomated_sa" {
 
hhsprotect-ip-ingress: keyvault.BaseClient
   3: data "azurerm_key_vault_secret" "hhsprotect_ip_ingress" {
 
cyberark-ip-ingress: keyvault.BaseClient
   8: data "azurerm_key_vault_secret" "cyberark_ip_ingress" {
Error: Process completed with exit code 1.

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 8, 2025

The issue here with the Terraform Service Principal yielding all these errors does not seem to be related at all with permissions to be granted to the PRIME-ReportStream Service Principal that was recently created because they are separate accounts for different purposes.
It has to do with the fact that the existing Terraform Service Principal certificate/secret has also expired (same problem we faced last time).

The question here is:
Should we go ahead and create a new Service Principal or should we try to fix the one that is currently setup but it has credentials that are no longer valid (expired: Certificate/Secret)?

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 8, 2025

This is my suggestion:

We should create a new Service Principal (as per the previous approach to get the issues fixed) but this time make sure we know exactly what those permissions are. If we keep the one that is now, we might be ok (short/quick fix) but it will not have a proper naming convention.

So, we could use something like: PRIME-ReportStream-Terraform

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 8, 2025

The reason for suggesting/recommending to create a new Service Principal is because of all the weird things we were able to discover the certificate that was setup for the previous iteration of the now active PRIME-ReportStream Service Principal. So, to make sure we are in compliance with what the CDC Cloud support team does, I would suggest we ask for a new one to be created.

We need to follow the process the CDC Cloud support team explained they have set to be used. For that, we will need to make a new ticket that will take care of that.

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 8, 2025

The error we get at the Azure console is:
A certificate or secret has expired. Create a new one ->

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 8, 2025

There is this awkward setup related to Terraform where three accounts have some references to it in their naming convention.

  • terraform-automation
  • terraform-service-principle
  • github-terraform

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 8, 2025

There are also other certificates (no house-keeping at all):

  • rbac -> 12/3/2021
  • rbac -> 12/3/2022
  • Renewal -> 12/5/2023
  • terraform-automation?????? -> 11/26/2024 (I am assuming this is the one that broke)
  • terraform-automation?????? -> 11/26/2025 (This one was created on November 26th 2024 but who did that?)

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 8, 2025

We need to:

  • Create a Service NOW ticket to request the creation of a new Azure Service Principal for the Terraform account.
  • Create another Service NOW ticket to ask the OMHS team to setup the Service Principal with the right roles/permissions.

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 8, 2025

The request to create the Azure Service Principal with ID PRIME-ReportStream is now completed.

Note: We are pending on the resolution for the Service NOW ticket to configure this SP with the Cloud Application Administrator role (part of the default Azure roles). Ticket ID = RITM0407552

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 9, 2025

Terraform's attribute object_id configuration will need to be changed in these files:
Note: There is an inconsistency in the properties names.

  • terraform_object_id = "4d81288c-27a3-4df8-b776-c9da8e688bc7"
./operations/app/terraform/vars/demo/locals.tf:29:
./operations/app/terraform/vars/prod/locals.tf:28:
./operations/app/terraform/vars/staging/locals.tf:28:
./operations/app/terraform/vars/test/locals.tf:28:
  • object_id = "4d81288c-27a3-4df8-b776-c9da8e688bc7"
./operations/app/terraform/modules/init/key_vault.tf:110:

Note: The Application ID will also need to be changed but it's a formality here just listed as a reference and it has no value other then informational.

  • // terraform-automation 5ab367bf-df15-45af-a027-47f95f2c75d8
./operations/app/terraform/modules/init/key_vault.tf:109:

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 9, 2025

We have an opportunity here to start remediating a significant flaw in the existing Terraform and GitHub Actions implementation. These hardcoded credentials should not be allowed or supported.

We need to start changing this approach for at least to use GitHub Environment settings.
I will work on a proposal to remediate all these issues.

Why does this matter?

  • It does matter from the security posture regarding these types of hardcoded credentials.
  • It's even more important because it creates a process dependency management that whenever a service account/principal needs to be changed, then operations and development get tied without logical reasoning.

We should be able to perform these changes without having to create builds.
As a matter of fact, this is something that should be owned by a process that will renew these credentials without having to depend on anyone. These are expiring and processes are getting affected without any warning.

This is a demonstration of very-bad practices and we need to get this fixed.

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 9, 2025

Identifying the use of hardcoded subscription ids like this is an issue:
subscription_id = "7d1e3999-6577-4cd5-b296-f518e5c8e677"

$ find . -type f -name '*.tf' | xargs -I {} egrep -Hni 'subscription_id' {} ;

./operations/app/terraform/vars/demo/azure.tf:37:
./operations/app/terraform/vars/prod/azure.tf:28:
./operations/app/terraform/vars/staging/azure.tf:28:
./operations/app/terraform/vars/test/azure.tf:30:

There are instances where this is consumed via this abstraction and we should embrace this to make the code more adaptable and capable. Like using this:
subscription_id = data.azurerm_subscription.current.subscription_id

./operations/app/terraform/modules/azure_dashboard/main.tf:13:

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 9, 2025

I have done some additional research and I have manage to get identified all the data that is hardcoded and should not be:

$ find . -type f -name '*.tf' \
  | xargs -I {} egrep -Hn \
  "^(.*)([[:alnum:]]{8})-([[:alnum:]]{4})\-([[:alnum:]]{4})\-([[:alnum:]]{4})\-([[:alnum:]]{12})(.*)" \
  {} | sort -u ;
./operations/app/terraform/modules/application_insights/alert_availability.tf:110:
"hidden-link:/subscriptions/7d1e3999-6577-4cd5-b296-f518e5c8e677/resourceGroups/${var.resource_group}/providers/Microsoft.Insights/components/${var.resource_prefix}-appinsights" = "Resource"

./operations/app/terraform/modules/application_insights/alert_availability.tf:164:
"hidden-link:/subscriptions/7d1e3999-6577-4cd5-b296-f518e5c8e677/resourceGroups/${var.resource_group}/providers/Microsoft.Insights/components/${var.resource_prefix}-appinsights" = "Resource"

./operations/app/terraform/modules/application_insights/alert_availability.tf:85:
"hidden-link:/subscriptions/7d1e3999-6577-4cd5-b296-f518e5c8e677/resourceGroups/${var.resource_group}/providers/Microsoft.Insights/components/${var.resource_prefix}-appinsights" = "Resource"

./operations/app/terraform/modules/front_door/main.tf:303:
object_id = "270e4d1a-12bd-4564-8a4b-c9de1bbdbe95"

./operations/app/terraform/modules/init/key_vault.tf:109:
// terraform-automation 5ab367bf-df15-45af-a027-47f95f2c75d8

./operations/app/terraform/modules/init/key_vault.tf:110:
object_id = "4d81288c-27a3-4df8-b776-c9da8e688bc7"

./operations/app/terraform/modules/init/key_vault.tf:77:
object_id = "52fe06f4-9717-4beb-9b7a-e05b6bdd2f0f"

./operations/app/terraform/vars/demo/azure.tf:37:
subscription_id = "7d1e3999-6577-4cd5-b296-f518e5c8e677"

./operations/app/terraform/vars/demo/locals.tf:29:
terraform_object_id = "4d81288c-27a3-4df8-b776-c9da8e688bc7"

./operations/app/terraform/vars/demo/locals.tf:30:
aad_object_keyvault_admin = "3c17896c-ff94-4298-a719-aaac248aa2c8"

./operations/app/terraform/vars/demo/locals.tf:31:
aad_group_postgres_admin = "f94409a9-12b1-4820-a1b6-e3e0a4fa282d"

./operations/app/terraform/vars/prod/azure.tf:28:
subscription_id = "7d1e3999-6577-4cd5-b296-f518e5c8e677"

./operations/app/terraform/vars/prod/locals.tf:28:
terraform_object_id = "4d81288c-27a3-4df8-b776-c9da8e688bc7"

./operations/app/terraform/vars/prod/locals.tf:29:
aad_object_keyvault_admin = "5c6a951e-a4c2-4890-b62c-0ed8179501bb"

./operations/app/terraform/vars/prod/locals.tf:30:
aad_group_postgres_admin = "c4031f1f-229c-4a8a-b3b9-23bae9dbf197"

./operations/app/terraform/vars/staging/azure.tf:28:
subscription_id = "7d1e3999-6577-4cd5-b296-f518e5c8e677"

./operations/app/terraform/vars/staging/locals.tf:28:
terraform_object_id = "4d81288c-27a3-4df8-b776-c9da8e688bc7"

./operations/app/terraform/vars/staging/locals.tf:29:
aad_object_keyvault_admin = "b35a2a63-aeb2-438c-913b-bebeb821adfe"

./operations/app/terraform/vars/staging/locals.tf:30:
aad_group_postgres_admin = "c4031f1f-229c-4a8a-b3b9-23bae9dbf197"

./operations/app/terraform/vars/test/azure.tf:30:
subscription_id = "7d1e3999-6577-4cd5-b296-f518e5c8e677"

./operations/app/terraform/vars/test/locals.tf:28:
terraform_object_id = "4d81288c-27a3-4df8-b776-c9da8e688bc7"

./operations/app/terraform/vars/test/locals.tf:29:
aad_object_keyvault_admin = "3c17896c-ff94-4298-a719-aaac248aa2c8"

./operations/app/terraform/vars/test/locals.tf:30:
aad_group_postgres_admin = "f94409a9-12b1-4820-a1b6-e3e0a4fa282d"

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 9, 2025

The Azure Service Principal "PRIME-ReportStream-Terraform" is now fully setup and I will proceed to get it updated in the Terraform configurations.

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 9, 2025

A contributor light role was added to the following resource groups for the new SP:

prime-data-hub-demo, 
prime-data-hub-demo1, 
prime-data-hub-demo2, 
prime-data-hub-demo3, 
prime-data-hub-prod, 
prime-data-hub-staging, 
prime-data-hub-test

Setup access policies for the new Service Principal on the following key vaults was completed with the same permissions as with the original terraform-automation (original Azure Service Principal):

pdhdemo1-appconfigs5m, 
pdhdemo1-keyvaults5m, 
pdhdemo2-appconfig1wu, 
pdhdemo2-keyvault1wu, 
pdhdemo3-appconfigjeo, 
pdhdemo3-keyvaultjeo, 
pdhprod-appconfig, 
pdhprod-keyvault, 
pdhstaging-appconfig, 
pdhstaging-keyvault, 
pdhtest-app-config, 
pdhtest-keyvault

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 9, 2025

Warnings: After some investigation, the original Azure Service Principal (terraform-automation) had a contributor role, but it's at the Subscription level.
The team cannot grant either the role nor the location for that without approval from higher up.
They expressed their surprise that the original team was able to have those privs within the DMZ.

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 9, 2025

The following change is required (a formality as this is just a comment):

$ find . -type f -name '*.tf' | xargs -I {} egrep -Hni '5ab367bf-df15-45af-a027-47f95f2c75d8' {} ;
./operations/app/terraform/modules/init/key_vault.tf:109:
// terraform-automation 5ab367bf-df15-45af-a027-47f95f2c75d8

With this updated information: d82b6f66-0d5f-4e70-9549-c0b072b8f93a

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 9, 2025

The following change is required (a hardcoded value):

$ find . -type f -name '*.tf' | xargs -I {} egrep -Hni '4d81288c-27a3-4df8-b776-c9da8e688bc7' {} ;
./operations/app/terraform/vars/demo/locals.tf:29:
terraform_object_id = "4d81288c-27a3-4df8-b776-c9da8e688bc7"

./operations/app/terraform/vars/prod/locals.tf:28:
terraform_object_id = "4d81288c-27a3-4df8-b776-c9da8e688bc7"

./operations/app/terraform/vars/staging/locals.tf:28:
terraform_object_id = "4d81288c-27a3-4df8-b776-c9da8e688bc7"

./operations/app/terraform/vars/test/locals.tf:28:
terraform_object_id = "4d81288c-27a3-4df8-b776-c9da8e688bc7"

./operations/app/terraform/modules/init/key_vault.tf:110:
object_id = "4d81288c-27a3-4df8-b776-c9da8e688bc7"

With this updated information: a58ee002-62c7-4a91-a2dc-4a837663aa00

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 9, 2025

I am pointing out that this information is also present in files as content outside the Terraform configurations.

Subscription ID: 7d1e3999-6577-4cd5-b296-f518e5c8e677
Service Principal: 4d81288c-27a3-4df8-b776-c9da8e688bc7

./prime-router/docs/docs-deprecated/environment-provisioning.md:87:
"/subscriptions/<subscription-id>/resourceGroups/prime-data-hub-$env/providers/Microsoft.KeyVault/vaults/pdh$env-appconfigmt8/objectId/<service-principal>"

./prime-router/docs/docs-deprecated/environment-provisioning.md:91:
"/subscriptions/<subscription-id>/resourceGroups/prime-data-hub-$env/providers/Microsoft.KeyVault/vaults/pdh$env-keyvaultmt8/objectId/<service-principal>"

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 9, 2025

$ legacy_account='4d81288c-27a3-4df8-b776-c9da8e688bc7';
$ active_account='a58ee002-62c7-4a91-a2dc-4a837663aa00';
$ oIFS="${IFS}"; IFS=$'\n';
$ declare -a targets=($(
    find . -type f | xargs -I {} egrep -Hni "${legacy_account}" {} | awk -F':' '{print $1}' | sort -u
  ));
$ for target in ${targets[@]}; do
    echo -e "\nFile: ${target}";
    sed -i '' -e "s|${legacy_account}|${active_account}|g" ${target} ;
  done;
$ IFS="${oIFS}";

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 9, 2025

devops: prime-reportstream (devsecops/emvaldes/terraform-service-principal *%) $ git diff ;
diff --git a/operations/app/terraform/modules/init/key_vault.tf b/operations/app/terraform/modules/init/key_vault.tf
index d8c1756ed..36b69673f 100644
--- a/operations/app/terraform/modules/init/key_vault.tf
+++ b/operations/app/terraform/modules/init/key_vault.tf
@@ -107,7 +107,7 @@ resource "azurerm_key_vault_access_policy" "init_tf" {
   key_vault_id = azurerm_key_vault.init[each.value].id
   tenant_id    = data.azurerm_client_config.current.tenant_id
   // terraform-automation 5ab367bf-df15-45af-a027-47f95f2c75d8
-  object_id = "4d81288c-27a3-4df8-b776-c9da8e688bc7"
+  object_id = "a58ee002-62c7-4a91-a2dc-4a837663aa00"
 
   key_permissions = [
     "Create",
diff --git a/operations/app/terraform/vars/demo/locals.tf b/operations/app/terraform/vars/demo/locals.tf
index 52e3441ef..72250bbb3 100644
--- a/operations/app/terraform/vars/demo/locals.tf
+++ b/operations/app/terraform/vars/demo/locals.tf
@@ -26,7 +26,7 @@ locals {
     tf_secrets_vault      = "pdh${local.init.environment}-keyvault${local.init.random_id}"
   }
   ad = {
-    terraform_object_id       = "4d81288c-27a3-4df8-b776-c9da8e688bc7"
+    terraform_object_id       = "a58ee002-62c7-4a91-a2dc-4a837663aa00"
     aad_object_keyvault_admin = "3c17896c-ff94-4298-a719-aaac248aa2c8"
     aad_group_postgres_admin  = "f94409a9-12b1-4820-a1b6-e3e0a4fa282d"
   }
diff --git a/operations/app/terraform/vars/prod/locals.tf b/operations/app/terraform/vars/prod/locals.tf
index a64ee7953..78df2edc7 100644
--- a/operations/app/terraform/vars/prod/locals.tf
+++ b/operations/app/terraform/vars/prod/locals.tf
@@ -25,7 +25,7 @@ locals {
     tf_secrets_vault      = "pdh${local.init.environment}-keyvault"
   }
   ad = {
-    terraform_object_id       = "4d81288c-27a3-4df8-b776-c9da8e688bc7"
+    terraform_object_id       = "a58ee002-62c7-4a91-a2dc-4a837663aa00"
     aad_object_keyvault_admin = "5c6a951e-a4c2-4890-b62c-0ed8179501bb"
     aad_group_postgres_admin  = "c4031f1f-229c-4a8a-b3b9-23bae9dbf197"
   }
diff --git a/operations/app/terraform/vars/staging/locals.tf b/operations/app/terraform/vars/staging/locals.tf
index 47066309e..3a221af96 100644
--- a/operations/app/terraform/vars/staging/locals.tf
+++ b/operations/app/terraform/vars/staging/locals.tf
@@ -25,7 +25,7 @@ locals {
     tf_secrets_vault      = "pdh${local.init.environment}-keyvault"
   }
   ad = {
-    terraform_object_id       = "4d81288c-27a3-4df8-b776-c9da8e688bc7"
+    terraform_object_id       = "a58ee002-62c7-4a91-a2dc-4a837663aa00"
     aad_object_keyvault_admin = "b35a2a63-aeb2-438c-913b-bebeb821adfe"
     aad_group_postgres_admin  = "c4031f1f-229c-4a8a-b3b9-23bae9dbf197"
   }
diff --git a/operations/app/terraform/vars/test/locals.tf b/operations/app/terraform/vars/test/locals.tf
index b229fad36..8ff6b7112 100644
--- a/operations/app/terraform/vars/test/locals.tf
+++ b/operations/app/terraform/vars/test/locals.tf
@@ -25,7 +25,7 @@ locals {
     tf_secrets_vault      = "pdh${local.init.environment}-keyvault"
   }
   ad = {
-    terraform_object_id       = "4d81288c-27a3-4df8-b776-c9da8e688bc7"
+    terraform_object_id       = "a58ee002-62c7-4a91-a2dc-4a837663aa00"
     aad_object_keyvault_admin = "3c17896c-ff94-4298-a719-aaac248aa2c8"
     aad_group_postgres_admin  = "f94409a9-12b1-4820-a1b6-e3e0a4fa282d"
   }
diff --git a/prime-router/docs/docs-deprecated/environment-provisioning.md b/prime-router/docs/docs-deprecated/environment-provisioning.md
index 83d62558c..5b9d3c602 100644
--- a/prime-router/docs/docs-deprecated/environment-provisioning.md
+++ b/prime-router/docs/docs-deprecated/environment-provisioning.md
@@ -84,11 +84,11 @@ echo "init complete"
 # Import access polices that are shared with init and key_vault modules
 terraform -chdir=$path import -var-file=$env/env.tfvars.json \
 module.key_vault.azurerm_key_vault_access_policy.terraform_app_config_access_policy[0] \
-"/subscriptions/7d1e3999-6577-4cd5-b296-f518e5c8e677/resourceGroups/prime-data-hub-$env/providers/Microsoft.KeyVault/vaults/pdh$env-appconfigmt8/objectId/4d81288c-27a3-4df8-b776-c9da8e688bc7"
+"/subscriptions/7d1e3999-6577-4cd5-b296-f518e5c8e677/resourceGroups/prime-data-hub-$env/providers/Microsoft.KeyVault/vaults/pdh$env-appconfigmt8/objectId/a58ee002-62c7-4a91-a2dc-4a837663aa00"
 
 terraform -chdir=$path import -var-file=$env/env.tfvars.json \
  module.key_vault.azurerm_key_vault_access_policy.terraform_access_policy[0] \
-"/subscriptions/7d1e3999-6577-4cd5-b296-f518e5c8e677/resourceGroups/prime-data-hub-$env/providers/Microsoft.KeyVault/vaults/pdh$env-keyvaultmt8/objectId/4d81288c-27a3-4df8-b776-c9da8e688bc7"
+"/subscriptions/7d1e3999-6577-4cd5-b296-f518e5c8e677/resourceGroups/prime-data-hub-$env/providers/Microsoft.KeyVault/vaults/pdh$env-keyvaultmt8/objectId/a58ee002-62c7-4a91-a2dc-4a837663aa00"
 
 for i in {1..3}; do \
 terraform -chdir=$path apply \

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 9, 2025

devops: prime-reportstream (devsecops/emvaldes/terraform-service-principal *%) $ git status ;
On branch devsecops/emvaldes/terraform-service-principal
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   operations/app/terraform/modules/init/key_vault.tf
	modified:   operations/app/terraform/vars/demo/locals.tf
	modified:   operations/app/terraform/vars/prod/locals.tf
	modified:   operations/app/terraform/vars/staging/locals.tf
	modified:   operations/app/terraform/vars/test/locals.tf
	modified:   prime-router/docs/docs-deprecated/environment-provisioning.md

no changes added to commit (use "git add" and/or "git commit -a")

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 9, 2025

devops: prime-reportstream (devsecops/emvaldes/terraform-service-principal *) $
git add . && git commit -m "Updating Terraform Azure Service Principal PRIME-ReportStream-Terraform";

[devsecops/emvaldes/terraform-service-principal 35d40af4a] Updating Terraform Azure Service Principal PRIME-ReportStream-Terraform
 6 files changed, 7 insertions(+), 7 deletions(-)

@emvaldes
Copy link
Collaborator

emvaldes commented Jan 9, 2025

devops: prime-reportstream (devsecops/emvaldes/terraform-service-principal) $ git push --set-upstream origin devsecops/emvaldes/terraform-service-principal ;
Enumerating objects: 41, done.
Counting objects: 100% (41/41), done.
Delta compression using up to 16 threads
Compressing objects: 100% (21/21), done.
Writing objects: 100% (21/21), 1.56 KiB | 1.56 MiB/s, done.
Total 21 (delta 18), reused 0 (delta 0), pack-reused 0 (from 0)
remote: Resolving deltas: 100% (18/18), completed with 18 local objects.
remote: 
remote: Create a pull request for 'devsecops/emvaldes/terraform-service-principal' on GitHub by visiting:
remote:      https://github.com/CDCgov/prime-reportstream/pull/new/devsecops/emvaldes/terraform-service-principal
remote: 
To https://github.com/CDCgov/prime-reportstream.git
 * [new branch]          devsecops/emvaldes/terraform-service-principal -> devsecops/emvaldes/terraform-service-principal
branch 'devsecops/emvaldes/terraform-service-principal' set up to track 'origin/devsecops/emvaldes/terraform-service-principal'.

@emvaldes
Copy link
Collaborator

An issue was identified and now corrected where the update to the Terraform Service Principal was not correct.

@emvaldes
Copy link
Collaborator

Although this PR does not address these issues, it's important to mention that there are lots of Azure account related information that is hardcoded into the code-base. Something that should eventually be remediated.

We should not be supporting or enforcing such practices but instead do all we can to bring the code-base to an example of good standards to follow. Hardcoding anything in a Software Configuration Management is just not right regardless.

Instead of maintaining a bad practice, this is what we should be doing either via secrets or environment variables:
e.g.: In your Terraform modules, define variables without assigning default values.

variable "subscription_id" {
  description = "Azure Subscription ID"
  type        = string
}

e.g.: Enforcing the scope of these values as environment values that will be available to the Terraform engine during run-time. Making it a flexible and agnostic process.

jobs:
  deploy:
    runs-on: ubuntu-latest
    env:
      ARM_SUBSCRIPTION_ID: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
      ARM_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }}
      ARM_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }}
      ARM_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }}
    steps:
      - name: Checkout code
        uses: actions/checkout@v4

      - name: Set up Terraform
        uses: hashicorp/setup-terraform@v3

      - name: Initialize Terraform
        run: terraform init

      - name: Apply Terraform configuration
        run: terraform apply -auto-approve

By not specifying the these values directly in the provider block, Terraform will reference the ARM_SUBSCRIPTION_ID environment variable during execution.
This method keeps your configuration clean and free from hardcoded sensitive data.

Important Considerations:

  • Mandatory Subscription ID: As of AzureRM provider version 4.0.0, specifying the subscription_id is mandatory. If it's not provided in the provider block, Terraform will look for the ARM_SUBSCRIPTION_ID environment variable. Ensure this environment variable is set to avoid errors during plan or apply operations.

  • Environment Variable Precedence: The AzureRM provider gives precedence to environment variables for authentication details. If both the provider block and environment variables specify a subscription_id, the provider block value will take precedence. Therefore, ensure consistency between your environment variables and provider configurations.

  • Security Best Practices: Storing sensitive information in environment variables, especially within CI/CD pipelines, is a common practice. However, always use secure storage solutions like GitHub Secrets to manage these variables, preventing unauthorized access.

By following this approach, you maintain a secure and flexible Terraform configuration, free from hardcoded sensitive information, and adaptable across different environments.


In Azure, identifiers such as subscription_id, client_id, and tenant_id are generally not considered highly sensitive on their own. However, when combined with other information, they can potentially be misused.

Potential Risks:

Information Disclosure: While these IDs alone don't grant access, they can reveal details about your Azure environment. For instance, a subscription_id can indicate the existence of a subscription, and a tenant_id can disclose your Azure Active Directory (AAD) tenant.
MICROSOFT LEARN

Targeted Attacks: Malicious actors could use these IDs to craft targeted phishing attacks or social engineering schemes, aiming to extract more sensitive information or credentials from your organization.

Credential Combination: If an attacker obtains these IDs along with other credentials, such as a client_secret or certificate, they could potentially authenticate as a service principal in your Azure environment, leading to unauthorized access.
SECUREWORKS

Recommendations:

Limit Exposure: Avoid sharing these identifiers publicly or with untrusted parties. While they are not highly sensitive, minimizing their exposure reduces the risk of them being used in conjunction with other compromised information.

Secure Associated Credentials: Ensure that any credentials associated with these IDs, such as client_secrets or certificates, are stored securely using services like Azure Key Vault. This practice helps prevent unauthorized access even if the IDs are known.
MICROSOFT LEARN

Monitor and Audit: Regularly monitor access logs and audit trails for any suspicious activities related to these IDs. Implementing Azure's monitoring tools can help detect and respond to potential threats promptly.

In summary, while subscription_id, client_id, and tenant_id are not inherently sensitive, they should still be handled with care to prevent potential misuse, especially when combined with other confidential information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Issue State label to flag PRs and issues to show they are blocked bug Issue Type label to flag an issue that is a bug DevSecOps Team Aq DevSecOps work label reportstream Stretch Bonus items brought into Sprint tech-debt Anything that is purely a technical issue and does not affect functionality terraform Pull requests that update Terraform code
Projects
None yet
Development

No branches or pull requests

2 participants