vancluever / terraform-provider-acme

Terraform ACME provider
https://registry.terraform.io/providers/vancluever/acme/latest
Mozilla Public License 2.0
213 stars 75 forks source link

rotating ACME credentials #285

Closed okruga closed 2 weeks ago

okruga commented 1 year ago

Hi It will be helpful if acme_registration provides an option to generate an account private key within the resource itself. The use case is when we need to rotate ACME credentials just by changing key_id and hmac_base64, and for now, apart from changing the above parameters we also need to trigger the recreation of tls_private_key

The second question is related to acme_certificate. Updating account_key_pem causes resource recreation, but can't it just be updated if it's used only to authenticate to the ACME server?

thanks in advance.

vancluever commented 1 year ago

Hey @okruga,

RFC 8555 does seems to indicate that externalAccountBinding is immutable: https://www.rfc-editor.org/rfc/rfc8555.html#section-7.1.2

Do you know for sure that your provider supports updating them?

I don't really know if we can support his if it's immutable in Boulder and Pebble as we are not going to be able to test it if they don't support EAB updates.

WRT public key updates though - we might be able to do something here - I have to check though! Will update once I know.

okruga commented 1 year ago

Thank you, @vancluever for the shift response. Yes, we can't change creds in the existing externalAccountBinding but let me be more clear here. By rotation, I meant not updating them within the same registration, but replacing everything (key_id, hmac_base64, account_key_pem) by recreating one resource - acme_registration Example: acme_registration will be recreated once key_id and hmac_base64 are changed. So, basically, registration of the account_key_pem will be removed from the ACME server and then created again (the same account_key_pem) with the new creds. I'm playing with ssl.com provider. Step1. Create ACME registration.

provider "acme" {
  alias      = "prd"
  server_url = "https://acme.ssl.com/sslcom-dv-rsa"
}

resource "tls_private_key" "reg_private_key" {
  algorithm = "RSA"
  rsa_bits  = 4096
}

resource "acme_registration" "reg" {
  provider        = acme.prd
  account_key_pem = tls_private_key.reg_private_key.private_key_pem
  email_address   = "some@mail.com"
  external_account_binding {
    key_id      = "keyid"
    hmac_base64 = "key"
  }
}

Apply

Step2. Generate new credentials through the ssl.com console, change external_account_binding only, and apply. tls_private_key.reg_private_key.private_key_pem left unchanged.

acme_registration.reg: Destroying... [id=https://acme.ssl.com/ejbca/acme/sslcom-dv-rsa/acct/42a-_0xxxx]
acme_registration.reg: Destruction complete after 4s
acme_registration.reg: Creating...
acme_registration.reg: Creation complete after 6s [id=https://acme.ssl.com/ejbca/acme/sslcom-dv-rsa/acct/42a-_0xxxx]

Step3. Issuing certificate. Adding acme_certificate to the config.

resource "acme_certificate" "certificate" {
  provider                     = acme.prd
  common_name                  = "test.doman.com"
  account_key_pem              = acme_registration.reg.account_key_pem
...

acme_certificate.certificate: Creating...
Error: error creating certificate: acme: error: 0 :: POST :: https://acme.ssl.com/ejbca/acme/sslcom-dv-rsa/newOrder :: urn:ietf:params:acme:error:unauthorized :: This account has been deactivated in response to an deactivation request.

So, this happened because we created acme_registration.reg in Step2 using old tls_private_key.reg_private_key which was previously deactivated on the server after we destroy acme_registration.reg. To fix this we need to trigger tls_private_key.reg_private_key recreation in Step2 when changing external_account_binding:

terraform apply -replace "tls_private_key.reg_private_key"

acme_registration.reg: Destruction complete after 6s
tls_private_key.reg_private_key: Destruction complete after 0s
tls_private_key.reg_private_key: Creation complete after 2s
acme_registration.reg: Creation complete after 6s
acme_certificate.certificate: Creation complete after 27s

That's why it will be nice to have account_key_pem managed within acme_registration resource.

vancluever commented 1 year ago

@okruga so you just need the ability to update the key, right?

That makes sense and is supported in the spec: https://www.rfc-editor.org/rfc/rfc8555.html#section-7.3.5

What might not be would be is the ability to re-activate a deactivated account - which it almost sounds like you did when you destroyed the resource: https://www.rfc-editor.org/rfc/rfc8555.html#section-7.3.6

Given that you're using a platform that uses EAB I'd imagine it's not a full deactivation in that regard, but you may want to see what the process is for re-activation, if any. Of course, if this is just about ensuring that you don't have to fully tear down and re-create the registration every time that you need to rotate the key, that makes sense, and you can disregard this particular paragraph 🙂

One complication that might arise if we do manage the key just straight in the resource that I didn't bring up though. If the resource ultimately fully manages account_key_pem then it might be tough to force an update of the resource as there will be nowhere to source the input from and thus no way for Terraform to determine if there's anything that needs to be updated. I'm loath to introduce a dummy attribute to force the recreation as that's not necessarily the Terraform way. I almost think that continuing to use tls_private_key is the right way to do that, and we just remove ForceNew from the schema for the account_key_pem attribute, then rotating the key would be as easy as just tainting the tls_private_key resource.

okruga commented 1 year ago

@vancluever I meant that accout_ley_pem recreation should be triggered by changing key_id and hmac_base64 in the external_account_binding. In that case, a new account with new values of key_id, hmac_base64, and accout_key_pem will be registered, and the old accout_key_pem deactivated.

Just my thoughts: A bool option like accout_key_pem_managed could be added, probably. In case someone needs to roll over accout_key_pem this option can be set to false and accout_key_pem become managed outside the acme_registration resource as it works now.

vancluever commented 1 year ago

@okruga:

"Magic" variables like the suggested accout_key_pem_managed don't really work in Terraform because you still need to either update configuration or a variable to flip them, which can lead to infinite drift until you do so.

A better alternative would instead be a bring-your-own-key setup where we mark account_key_pem as optional or computed (so it either uses the one you supply or creates a new one if empty). We would have to block changes for external keys if you were changed EAB attributes only (ie: you did not also taint a tls_private_key resource or change the supplied private key otherwise), but that might actually be a UX improvement anyway.

Let me sit with it for a bit and I'll see what I can come up with.

vancluever commented 1 year ago

Just noting here that this will probably be a feature we'll need to earmark for a future major version (read: 3.0). Main reason is that we would need to remove the requirement for a key being necessary for acme_certificate.

This will give us the opportunity to update nomenclature as well ("registration" as a term to refer to account objects was actually dropped in draft 5, a very long time ago now).

We will probably drop the account detail requirements in acme_certificate altogether so we will need to deprecate the field in version 2.x and I need to figure out a way to allow folks to manage registrations with the provider still while requiring accounts for certificates.

vancluever commented 2 weeks ago

Hey folks, this is finally landing in #423.

After quite a bit of deliberating along with examination of what we could reasonably do here within the constraints of the lego API, I've settled on a pretty rudimentary solution, which is probably all that was needed in the first place. :wink:

No nomenclature is changing, and in fact registrations will be much simpler to use in standard workflows going forward as you will not need to bring your own key (but you still can if you want!).

vancluever commented 2 weeks ago

Hey folks, this is now completed and will be in v2.24.0!