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

Incorrect behaviour for vault_approle_auth_backend_role_secret_id - Recreates even when still valid if wrapped #976

Closed
melbit-michaelw opened this issue Feb 10, 2021 · 3 comments · Fixed by #1166

Comments

@melbit-michaelw
Copy link
Contributor

Terraform Version

Terraform v0.12.29
+ provider.vault v2.18.0

Affected Resource(s)

  • vault_approle_auth_backend_role_secret_id

Terraform Configuration Files

resource "vault_approle_auth_backend_role" "role_id" {
  role_name = "approle-testing-role"
  role_id   = "approle-testing-role"
  backend   = "approle"
  token_policies  = ["default"]
}

resource "vault_approle_auth_backend_role_secret_id" "secret_id" {
  role_name = vault_approle_auth_backend_role.role_id.role_name
  backend = "approle"
  wrapping_ttl = "30m"
}

provider "vault" {
  version = "~> 2.18.0"
}

output "wrapped_secret" {
  value = vault_approle_auth_backend_role_secret_id.secret_id.wrapping_token
}

Debug Output

Gist of initial terraform apply: https://gist.github.com/melbit-michaelw/fca4e290438f060f079f3dcc1232a3c4

Gist of terraform apply after unwrapping the secret_id: https://gist.github.com/melbit-michaelw/02f2010884b69daf3b78c578f3ff8f07

Expected Behavior

If the wrapped secret_id is still valid in Vault (based on successfully being able to look up the secret_id_accessor value), then no new secret_id should be created.

Potentially a flag to control whether a new secret_id should be created based on whether the wrapped secret_id_accessor is still valid could be introduced. This would enable the current behaviour as an option (i.e. regenerate a secret-id if it has been unwrapped or can no longer be unwrapped).

A second flag to control whether to delete the old secret-id when creating a new one might also make sense (current behaviour is to completely ignore the old secret-id, so it is left as valid, which seems like a possible security issue).

Actual Behavior

A new secret-id is created every time the wrapped ttl expires, regardless of whether the old one is still active. This introduces the appearance of config drift into automated pipelines.

Additionally, the old secret-id is 'abandoned' as part of this process, and remains valid in Vault.

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:

  1. Set up a vault server (a temporary -dev one should be fine), ensure the approle backend is mounted, and set VAULT_ADDR and VAULT_TOKEN appropriately.
  2. terraform apply
  3. Unwrap the wrapped_secret output
  4. Re-run terraform apply and observe that it wants to create a fresh secret-id.

References

Are there any other GitHub issues (open or closed) or Pull Requests that should be linked here? For example:

As background, our intended usage of this functionality is to create a new wrapped secret_id which is passed to a server instance via a template in terraform that is executed as user-data (whether that server is in aws, vsphere, azure, ...) and unwrapped by that server.

Consequentially, having a new secret_id be recreated on every terraform execution is not desirable.

Please let me know if you need any additional information.

@melbit-michaelw melbit-michaelw changed the title Incorrect behaviour for vault_approle_auth_backend_role_secret_id - Recreates even when still valid when wrapped Incorrect behaviour for vault_approle_auth_backend_role_secret_id - Recreates even when still valid if wrapped Feb 10, 2021
@melbit-michaelw
Copy link
Contributor Author

Apologies for tagging you both directly, but this has been sitting dormant with no guidance for some time now.

Could either @bflad or @jasonodonnell point me at the right person to progress this issue (at least in a theoretical/discussion manner, the actual code changes can happen later, but we need direction in order to propose any code changes...) ?

@jasonodonnell
Copy link
Collaborator

@melbit-michaelw Adding a new parameter to use the wrapped token does make sense to me. Possibly something like:

resource "vault_approle_auth_backend_role_secret_id" "secret_id" {
  role_name = vault_approle_auth_backend_role.role_id.role_name
  backend = "approle"
  wrapping_ttl = "30m"
  reuse_wrapped_token = true
}

@FWest98
Copy link

FWest98 commented Aug 8, 2021

Currently, this is a breaking issue for me. I use Terraform to create a wrapped AppRole secret, and I put it in the config for a VM which will pick it up on boot, unwrap it and use the underlying periodic service token. Unfortunately, afterwards the wrapped token will no longer be valid/available, so every subsequent terraform plan will think the resource has disappeared and will attempt to recreate it - and with it, the entire VM...

I think there's two options:

  1. The most elegant way would be to implement as suggested in Return wrapped accessor when creating wrapped AppRole secret-ids vault#12173. This will make the lifetime of the resource reflect the lifetime of the actual underlying token.
  2. A less elegant, but maybe easier/quicker way would be to allow users to provide some custom "trigger" value of some sorts that would skip the validity check of the old token, and instead use that trigger to determine whether or not the resource should be recreated. This could also be a flag like @jasonodonnell proposed.

I don't know of any work-around for this issue. I have considered generating the wrapped secret manually (or well, automated in the CD pipeline) outside of Terraform, but then you just move the problem of determining when to actually recreate the resource and outside of Terraform that would be even more complicated...

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

Successfully merging a pull request may close this issue.

3 participants