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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

azurerm_iothub secret spill in azurerm_iothub_shared_access_policy #17603

Open
1 task done
BeTheFlow95 opened this issue Jul 12, 2022 · 3 comments
Open
1 task done

azurerm_iothub secret spill in azurerm_iothub_shared_access_policy #17603

BeTheFlow95 opened this issue Jul 12, 2022 · 3 comments

Comments

@BeTheFlow95
Copy link
Contributor

BeTheFlow95 commented Jul 12, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Community Note

  • Please vote on this issue by adding a 馃憤 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform Version

1.2.4

AzureRM Provider Version

2.96.0

Affected Resource(s)/Data Source(s)

azurerm_iothub, azurerm_iothub_shared_access_policy

Terraform Configuration Files

	resource "azurerm_resource_group" "rg"{
		name = "rg-name"
		location = "westeurope"
	}
	
	resource "azurerm_iothub" "iothub" {
		name = "iothub-name"
		resource_group_name = azurerm_resource_group.rg.name
		location = azurerm_resource_group.rg.location
		sku {
			name = "sku-name"
			capacity = 1
		}
	}
	
	resource "azurerm_iothub_shared_access_policy" "iothub_sap"{
		name = "sap-name"
		resource_group_name = azurerm_resource_group.rg.name
		iothub_name = azurerm.iothub.name

		registry_read = true
		registry_write = true
		device_connect = true
		service_connect = true
	}

Debug Output/Panic Output

The keys of shared_access_policies aren't hidden(sensitive) in the iothub but they are hidden when they are viewed directly with "terraform state show ...".

Expected Behaviour

resource "azurerm_iothub" "iothub" {
    ...
    shared_access_policy = [
        {
	        key_name = "name"
		permissions = "..."
		primary_key = (sensitive value)
		secondary_key = (sensitive value)
        },
        {
               Similar Output in all shared_access_policies
        }
    ]
    ...
}

Actual Behaviour

resource "azurerm_iothub" "iothub" {
    ...
    shared_access_policy = [
        {
	        key_name = "name"
		permissions = "..."
		primary_key = "unsupressed Key"
		secondary_key = "unsupressed Key"
        },
        {
               Same Problem in all shared_access_policies
        }
    ]
    ...
}

Steps to Reproduce

  1. terraform apply
  2. terraform state show azurerm_iothub.iothub

Shows the secrets as described above

  1. terraform appy
  2. terraform state show azurerm_iothub_shared_access_policy.iothub_sap

Hides the keys as expected

Important Factoids

No response

References

No response

@github-actions github-actions bot removed the bug label Jul 12, 2022
@myc2h6o
Copy link
Contributor

myc2h6o commented Jul 13, 2022

Hi @BeTheFlow95 thanks for opening the issue! The root cause here is the nested sensitive property issue in Terraform SDK hashicorp/terraform-plugin-sdk#201.
Similar to #9042

@bastiandg
Copy link

Thanks for the hint @myc2h6o .

The issue shouldn't be treated like it was in #9042. It doesn't seem likely that anything is going to happen soon in the plugin sdk. From a security perspective it is better to mark the entire shared_access_policy attribute as sensitive (as done in cloudscale-ch/terraform-provider-cloudscale#60). Yes it is not a great UX, but the security concern of spilling all the saps weighs heavier. And if a user wants to see the sap config in their output they can use azurerm_iothub_shared_access_policy.

@myc2h6o
Copy link
Contributor

myc2h6o commented Jul 14, 2022

Hi @bastiandg yes it makes sense to mark the entire shared_access_policy as sensitive in terms of security.
I took a look at other similar issues, I found in azurerm_kubernetes_cluster, nested properties kube_admin_config and kube_config are switched to sensitive entirely and is controlled by an environment variable by this change and later changes to sensitive by default in 3.0 by this change. However, there are also some other occurrences where we wait for the upstream SDK to be fixed. Another example #10385.
@katbyte to possibly comment on this. Are we going to use a new environment variable to control the sensitive flag of this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants