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

(CA Issuer) Root CA rotation without downtime when using keystores and truststores #4344

Closed
Miles-Garnsey opened this issue Aug 12, 2021 · 11 comments
Labels
Epic Indicates an issues that is meant for tracking a collection of other issues. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@Miles-Garnsey
Copy link

Miles-Garnsey commented Aug 12, 2021

Is your feature request related to a problem? Please describe.

We love cert-manager and I’ve been a heavy user for some time. Cassandra and the K8ssandra project both rely upon the availability of keystores and trust stores, and we’re considering how to use cert-manager to provide these for k8s deployments.

However AFAIK there is currently no clean way to update a certificate in a running cluster of machines without downtime.
For example,

  1. When using a CA Issuer, the CA cert expires, and the certificates that depend upon it are no longer valid and are re-issued using the new CA cert.
  2. We want to refresh these certs in the application and therefore initiate a rolling restart.
  3. But when the first new pod comes online its truststore no longer recognises the certs presented by the existing pods.
  4. Concurrently, the existing pods no longer recognise the keystore being used by the new pod.
  5. At this point people typically find that their deployments deadlock as the new pods cannot speak to the old pods (which they need to contact at initialisation to obtain an initial state) maxUnavailability and PodDisruptionBudget limits are quickly reached and things go downhill from there.

Describe the solution you’d like

We have a mechanism to avoid the above failure mode, but we need some support from cert-manager to make it work. I think we can offer to contribute some PRs ourselves but we might need help from the community with finding our way around.

We’re asking for the inclusion of a grace period in which - for some configurable period following the expiry of a root CA (as held by an Issuer), all truststores held within the secrets descended from the Certificate resources hold both the old and new root CA certificate.

See below for how we can then use this to avoid downtime.

Describe alternatives you’ve considered

We could write custom scripts to somehow cache the certs when we know they are about to expire, but it feels like orchestrating certificate rotation belongs with cert-manager.

There is a question as to whether this should apply to all Issuer types or just the CA Issuer. I am open to options here, as it may be counterintuitive to allow for a grace period on certs issued by LetsEncrypt or similar.

Additional context

We can use the requested feature to rotate certificates without downtime. My colleague @ossarga has put together a great blog post which describes our procedure to achieve this.

To summarise, his script takes these steps;

  1. Add new root CA to the truststore on the nodes.
  2. Rolling restart. All pods now recognise certs issued by both the old root CA and the new root CA.
  3. Replace the keystores in the pods.
  4. Rolling restart. All pods are now presenting the new certs.
  5. Replace the truststores with a truststore containing only the new root CA.
  6. Rolling restart. CA certs and pod certs are now fully updated.

/kind feature

@jetstack-bot jetstack-bot added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 12, 2021
@JoshVanL
Copy link
Collaborator

Hi @Miles-Garnsey,

Thanks very much for your detailed ticket and kind words! 😄

Trust distribution and the related features in that space is something that the cert-manager project is keen to address and provide solutions for. It is still early days but we are currently experimenting with trust which effectively attempts to solve the base case for trust distribution. You can read the design for it as it stands here. I see CA rotation as a feature that sits on top of trust distribution- whatever that will look like.

Personally, I don't want to touch the ca.crt. I think this is a bad way for distributing trust which ideally should be shared out of band to clients requesting their actual certificates.. I wish that this field was never added 😅 So I am against adding any features around it. To me, this seems like we need to add some kind of machinery to cert-manager itself (maybe?), as well as some new CRDs to trust.

Possible solutions for this could include some kind of new CRD. This resource would understand what a CA is, know how to distribute it and to where. It should then also understand when a rotation should happen, how to do that, and execute it in a controlled way. Some kind of staged process which can be moved along by manual intervention could work here:

-> See new CA -> Set condition to stage 1 -> Stage 2 gets applied -> Put the new CA everywhere -> Set to stage 3 -> Stage 4 gets applied -> Remove old CA everywhere -> Set condition stage 5 END

In my opinion an operator, whether that be cert-manager itself or trust, should never do things like an automatic roll-out restart. We could never catch every scenario for every weird and wacky deployment, so at the least some manual intervention or custom user automation will be required.

We'd love to have your involvement in this area to give real user scenarios as we move forward.

@JoshVanL JoshVanL added Epic Indicates an issues that is meant for tracking a collection of other issues. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Aug 12, 2021
@JoshVanL JoshVanL added this to the 1.6 milestone Aug 12, 2021
@Miles-Garnsey
Copy link
Author

Miles-Garnsey commented Aug 13, 2021

@JoshVanL you're most welcome and thanks for the quick reply! It would be my pleasure to share what experience I have.

A few clarifications here:

Personally, I don't want to touch the ca.crt. I think this is a bad way for distributing trust which ideally should be shared out of band to clients requesting their actual certificates.. I wish that this field was never added 😅 So I am against adding any features around it. To me, this seems like we need to add some kind of machinery to cert-manager itself (maybe?), as well as some new CRDs to trust.

EDIT: Having read the trust design, I can see why you say this. I think the design you're proposing with trust will work.

My only note is don't forget the JKS keystore/truststore when thinking about this stuff! The ca.crt isn't the only format for distributing trust info.

In my opinion an operator, whether that be cert-manager itself or trust, should never do things like an automatic roll-out restart. We could never catch every scenario for every weird and wacky deployment, so at the least some manual intervention or custom user automation will be required.

I 100% agree with this. I only mention the restarts to provide context on the overall process we follow. Our operators should orchestrate the restarts.

The only thing we strictly need is to keep the previous CA cert in the truststore for a user-definable grace period. We can handle the rest 👍

@Miles-Garnsey
Copy link
Author

Miles-Garnsey commented Aug 13, 2021

Actually, having just read through your doc on trust, I think you've answered most of my questions there!

Editing my above comment.

Notably, these fields in practice only ever contain one CA certificate, which makes their direct use as trust bundles dangerous; without the ability to have multiple certificates in a trust store, rotation cannot be reliably and safely carried out without downtime.

Clearly you already knew about this problem! I can see the justification for having a trust resource.

One question here is just whether you can have multiple different trust bundles exposed to different namespaces for multi-tenancy. I'd suggest maybe using an annotation in the namespace to define which namespaces receive which trust bundles? Distributing the same trust bundle to every namespace may be a problem in some cases.

On namespacing, can I also suggest that you consider having both ClusterBundle and Bundle resources (after ClusterRole and Role) because in some instances, we want to delegate creation of the trust bundle to a user in a namespace and allow them to manage it. This is particularly true for the Cassandra case where all nodes should be in the same namespace and don't need to (and shouldn't) trust anything outside it.

Finally, I can't see much mention of rotation in the trust design doc. Would the grace period approach I've suggested above be an appropriate way to make this work?

@bradfordcp
Copy link

@JoshVanL

-> See new CA -> Set condition to stage 1 -> Stage 2 gets applied -> Put the new CA everywhere -> Set to stage 3 -> Stage 4 gets applied -> Remove old CA everywhere -> Set condition stage 5 END

Given the workflow here is the transition from Stage 3 and Stage 4 automated or is there an expectation that an external operator (human or otherwise) indicates when the old CA is no longer needed and the transition may occur?

I ask as that would remove the requirement of a grace period. Although you may still want a timeout to trigger this transition after a set period. That may be a separate feature request.

@Miles-Garnsey

This is particularly true for the Cassandra case where all nodes should be in the same namespace.

This holds true for a single logical DC, but may not hold for multi DC deployments. I'm not sure if this distinction changes the meaning of your point, but think we should be clear here.

@Miles-Garnsey
Copy link
Author

@bradfordcp, RE logical DCs - that's a good point. I would be inclined to suggest that where a Cassandra cluster spans multiple namespaces, we could consider propagating trust bundles ourselves rather than relying on cert-manager/trust operator for this.

Although if the trust operator enables this use case by allowing us to disseminate trust amongst multiple namespaces then so much the better.

NB: good point on the grace period I've proposed being equivalent to a timeout on some of the transitions. My only consideration on that part of the design is ensuring that we are keeping the API declarative and not slipping into any imperative thinking.

@JoshVanL are you open to talking about having a namespaced trust bundle in addition to the current cluster-wide proposal?

@JoshVanL
Copy link
Collaborator

JoshVanL commented Aug 16, 2021

One question here is just whether you can have multiple different trust bundles exposed to different namespaces for multi-tenancy. I'd suggest maybe using an annotation in the namespace to define which namespaces receive which trust bundles? Distributing the same trust bundle to every namespace may be a problem in some cases.

Yep, there is an open issue about adding a namespace selector to Bundle.

On namespacing, can I also suggest that you consider having both ClusterBundle and Bundle resources (after ClusterRole and Role) because in some instances, we want to delegate creation of the trust bundle to a user in a namespace and allow them to manage it. This is particularly true for the Cassandra case where all nodes should be in the same namespace and don't need to (and shouldn't) trust anything outside it.

This is trickier. The idea of a trust bundle is that it will propagate trusted data from a ring fenced trusted area into targets for them to be consumable. Will need some thoughts on how to do this or what the right abstraction is for the same namespace. Like you say, maybe it's some kind of namespaced Bundle.

Given the workflow here is the transition from Stage 3 and Stage 4 automated or is there an expectation that an external operator (human or otherwise) indicates when the old CA is no longer needed and the transition may occur?
I ask as that would remove the requirement of a grace period. Although you may still want a timeout to trigger this transition after a set period. That may be a separate feature request.

That's right, it would be some condition that needs to be set either by a human or a context aware operator that isn't trust. In the realm of: cmctl trust rotate my-ca-rotation-resource --continue.

@JoshVanL are you open to talking about having a namespaced trust bundle in addition to the current cluster-wide proposal?

Definitely, there is a clear real world use case for it and we should aim to meet that- This area is brand new and we're still figuring out what it is going to look like so your inputs are invaluable :)

I'm quite caught up with policy-approver atm, but I would like to take a stab at a CA rotation design document later in the week. Will try to tackle this namespacing issue as well- we might find that Bundle isn't the right abstraction for this use-case, or Bundle itself is just not the API we should be building.

@jetstack-bot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle stale

@jetstack-bot jetstack-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 14, 2021
@jetstack-bot
Copy link
Collaborator

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle rotten
/remove-lifecycle stale

@jetstack-bot jetstack-bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Dec 14, 2021
@jetstack-bot
Copy link
Collaborator

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/close

@jetstack-bot
Copy link
Collaborator

@jetstack-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.
Send feedback to jetstack.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@Miles-Garnsey
Copy link
Author

@JoshVanL I am hoping to put some time into this ticket soon, can we possibly re-open it?

Is there a Slack/Discord/mailing list for cert-manager devs? I have some basic questions (e.g. I'm having an issue with missing Gateway APIs when running the install and e2e testing steps described here). I don't know if I should raise a ticket/PR for it since the bug I'm seeing isn't in a release (I'm working on master).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic Indicates an issues that is meant for tracking a collection of other issues. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

No branches or pull requests

4 participants