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

Don't mark non SecureString SSM parameters as sensitive #9090

Closed
tdmalone opened this issue Jun 22, 2019 · 14 comments · Fixed by #25721
Closed

Don't mark non SecureString SSM parameters as sensitive #9090

tdmalone opened this issue Jun 22, 2019 · 14 comments · Fixed by #25721
Labels
bug Addresses a defect in current functionality. service/ssm Issues and PRs that pertain to the ssm service.
Milestone

Comments

@tdmalone
Copy link
Contributor

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

Description

Plans including changes to aws_ssm_parameter values currently mask the changes, due to the value field being defined as sensitive.

SSM parameters support a type param to define whether a value is a String or a SecureString.

Would it be possible for value to only be considered sensitive if type == "SecureString"? This would make it easier to grok planned changes of non-secret configuration values without needing to manually verify the changes outside of Terraform.

Terraform Version

Terraform v0.11.14
+ provider.aws v2.16.0

Affected Resource(s)

Terraform Configuration Files

resource "aws_ssm_parameter" "test_normal" {
  name  = "/test/normal"
  type  = "String"
  value = "public info"
}

resource "aws_ssm_parameter" "test_secure" {
  name  = "/test/secure"
  type  = "SecureString"
  value = "something really secret"
}

Expected Behavior

Terraform will perform the following actions:

  ~ aws_ssm_parameter.test_normal
      value: "public information" => "public info"

  ~ aws_ssm_parameter.test_secure
      value: <sensitive> => <sensitive> (attribute changed)

Plan: 0 to add, 2 to change, 0 to destroy.

Actual Behavior

Terraform will perform the following actions:

  ~ aws_ssm_parameter.test_normal
      value: <sensitive> => <sensitive> (attribute changed)

  ~ aws_ssm_parameter.test_secure
      value: <sensitive> => <sensitive> (attribute changed)

Plan: 0 to add, 2 to change, 0 to destroy.

Steps to Reproduce

  1. Set up the config as provided above
  2. terraform apply
  3. Make changes to the values in each parameter
  4. terraform plan
  5. Observe that the value for the type = String parameter is masked
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Jun 22, 2019
@aeschright aeschright added the service/ssm Issues and PRs that pertain to the ssm service. label Jul 3, 2019
@ryndaniels ryndaniels added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Nov 26, 2019
@tdmalone
Copy link
Contributor Author

tdmalone commented Jun 13, 2020

Possible workaround (won't be suitable for all circumstances): duplicate the value as a tag.

eg.

locals {
  test_normal_value = "public info"
}

resource "aws_ssm_parameter" "test_normal" {
  name  = "/test/normal"
  type  = "String"
  value = local.test_normal_value

  tags = {
    value = local.test_normal_value
  }
}

While value is still marked <sensitive> in the plan, tags.value is not, and thus you can still see the change you will be applying.

@mrwacky42
Copy link
Contributor

I see the value is unconditionally flagged as Sensitive: true. Is it possible to change the schema to have non-secret Parameters to Sensitive: false? I do not see anywhere in the provider where the value for Sensitive is conditional on other aspects of the schema.

https://github.com/terraform-providers/terraform-provider-aws/blob/master/aws/resource_aws_ssm_parameter.go#L63-L66

@robinjhector
Copy link

Bumping this, since it seems like nothing has happened for quite a while. I was surprised to see regular string parameters marked as "sensitive" today. They shouldn't be. The idea of parameter store is to store variables that are different per account, environment, or region. If you need to store secrets, it should be using SecureString

@zachwhaley
Copy link

zachwhaley commented Apr 8, 2021

This is causing an error with Terraform 0.15

I have a "String" SSM parameter whose value is used in an output value. With Terraform 0.14 this worked, but with Terraform 0.15 it fails with the following message:

╷
│ Error: Output refers to sensitive values
│
│   on outputs.tf line 1:
│    1: output "deployment_color" {
│
│ Expressions used in outputs can only refer to sensitive values if the
│ sensitive attribute is true.
╵

Edit: I added sensitive = true as suggested, but now the output does not show the value. Instead it shows <sensitive>, which is not helpful:

Outputs:

deployment_color = <sensitive>

@bflad
Copy link
Contributor

bflad commented Apr 12, 2021

Hi @zachwhaley 👋 A fix to the nonsensitive() function has been merged upstream in Terraform CLI and will release with version 0.15.0 to fix this use case. You can bypass the sensitivity, when you know it is safe, by explicitly removing the value marking such as:

output "example" {
  value = nonsensitive(aws_ssm_parameter.example.value)
}

@cilindrox
Copy link

This also impacts the data source - had to flag the whole thing when reading a remote value, eg:

data "aws_ssm_parameter" "users" {
  name     = "/foo/users"
}

locals {
    users = split(",", nonsensitive(data.aws_ssm_parameter.users.value))
}

was having some random dynamic blocks fail due to these being flagged as sensitive on 0.15

@igreg
Copy link

igreg commented May 16, 2021

Having looked into the code of the aws provider plugin and the SDK module it depends on, this is caused by a limitation of the SDK which does not allow the sensitivity of data & resource attributes (like in our case, the value attribute of both the data.aws_ssm_parameter and resource.aws_ssm_parameter) to be set dynamically.

So you can either have the values of all the parameters treated as a sensitive value, or none of them. It is not possible to set sensitivity dynamically (e.g. depending on the type of the parameter) which would require the Terraform SDK to be modified for this to be possible.

@WhyNotHugo
Copy link

How about having entirely different resources for SecretString and String?

While that may require a one-time refactor for most of us, it does allow to continue using SSM without so many hacks.

Keep in mind that AWS uses SSM for public information, like the ID of the AMI recommended for ECS. SSM is far from being all secrets: it's used to publish data too.

@igreg
Copy link

igreg commented May 19, 2021

Alternatively, I think we could also use a separate attribute (e.g. secure_value) when working with SSM parameters of the type SecureString. I think this would also work and avoid duplicating the entire code for data.aws_ssm_parameter and resource.aws_ssm_parameter.

@srinivas-vangari
Copy link

Unnecessarily marking all ssm parameters as secure is giving me below error when I tried to use a complex structure containing ssm parameter as for_each expression for a module.


Sensitive values, or values derived from sensitive values, cannot be used as for_each arguments. If used, the sensitive value could be exposed as a resource instance key.

@WhyNotHugo
Copy link

@srinivas-vangari You have to add nonsensitive() to every single usage of all SSM params.

This regression has been terrible, but I guess nobody has the expertise to do something no-that-complex like #9090 (comment).

@IrfanAnsari
Copy link

IrfanAnsari commented Sep 30, 2021

Same issue as @srinivas-vangari and marking password as nonsensitive means it's part of plan and state, not acceptable to my TL.
Here is the error with for_each where one of the value in the option setting is password, If I make it nonsensitive then works but prints the password in the plan.

Error: Invalid dynamic for_each value

│ on modules\option_groups\main.tf line 27, in resource "aws_db_option_group" "this":
│ 27: for_each = lookup(option.value, "option_settings", [])
│ ├────────────────
│ │ option.value is object with 6 attributes

│ Cannot use a tuple value in for_each. An iterable collection is required.

Update: - I upgraded to 0.15.5 and seems to be fine for me. Looked at the release notes and the issue I am seeing is fixed in 0.15.4

sakisv added a commit to alphagov/notifications-cf-monitoring that referenced this issue May 4, 2022
This was preventing `for_each` to use the list of objects in a `dynamic`
block.

This seems to be a (maybe intentional) bug [[1][]], [[2][]], [[3][]],
though the error message wasn't particularly helpful

[1]: hashicorp/terraform#28384
[2]: hashicorp/terraform-provider-aws#9090
[3]: hashicorp/terraform-provider-aws#9090 (comment)
@github-actions github-actions bot added this to the v4.22.0 milestone Jul 7, 2022
@github-actions
Copy link

github-actions bot commented Jul 8, 2022

This functionality has been released in v4.22.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

github-actions bot commented Aug 7, 2022

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/ssm Issues and PRs that pertain to the ssm service.
Projects
None yet
Development

Successfully merging a pull request may close this issue.