Skip to content

AutoTLS CA rotation #93

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

Closed
Jimvin opened this issue Mar 24, 2022 · 12 comments · Fixed by #350
Closed

AutoTLS CA rotation #93

Jimvin opened this issue Mar 24, 2022 · 12 comments · Fixed by #350
Assignees
Labels
release/24.3.0 release-note Denotes a PR that will be considered when it comes time to generate release notes. type/internal-debt type/internal-improvement

Comments

@Jimvin
Copy link
Member

Jimvin commented Mar 24, 2022

Currently the CA certificates create by secret-operator are valid for two years and signed service certificates for 1 day. We should improve the lifecycle handling for TLS certificates to ensure that new certificates are minted and rotated in when required. We should also generate metrics or alerts for certificate expiration, especially for the CA.

### Must have
- [x] New CA certificate should be generated some time in advance of the current one expiring
- [x] After a new CA is provisioned, old CA should still be used for Some Time(tm) to ensure that old peers will trust them (at least until those peers' own certs expire)
- [x] All valid CA certificates must be added to truststores (ca.crt and truststore.p12)
### Nice to have
- [ ] Clean up expired CA certificates?
@nightkr nightkr closed this as completed Mar 24, 2022
@nightkr nightkr reopened this Mar 24, 2022
@soenkeliebau
Copy link
Member

ping @lfrancke

It would be nice to get this in the refinement and implementation queue. Not super urgent, but a low hanging fruit that I am 100% sure will become super-urgent due to customer demands at some point in time if we don't fix it before then.

@nightkr nightkr changed the title Improve handling for TLS certificate lifecycle AutoTLS CA rotation Jan 23, 2024
@nightkr nightkr moved this to Development: In Progress in Stackable Engineering Jan 25, 2024
@sbernauer sbernauer moved this from Development: In Progress to Development: Waiting for Review in Stackable Engineering Jan 29, 2024
@sbernauer sbernauer moved this from Development: Waiting for Review to Development: In Review in Stackable Engineering Jan 29, 2024
@sbernauer sbernauer self-assigned this Jan 29, 2024
@sbernauer sbernauer added release-note Denotes a PR that will be considered when it comes time to generate release notes. release/24.3.0 labels Jan 29, 2024
@sbernauer sbernauer moved this from Development: In Review to Development: Done in Stackable Engineering Jan 29, 2024
@lfrancke
Copy link
Member

For anyone reading this later, these are the docs (nightly right now): https://docs.stackable.tech/home/nightly/secret-operator/secretclass.html#_certificate_authority_rotation

@lfrancke lfrancke moved this from Development: Done to Acceptance: In Progress in Stackable Engineering Jan 30, 2024
@lfrancke
Copy link
Member

If configured not to provision its own CA, a warning will be issued when there is less than 1 year remaining.

Where is that warning generated?

Expired certificates will currently not be deleted automatically, and should be cleaned up manually.

It is under the CA heading but does this apply to all certificates?
(Or do they go away with the pod they are attached to anyway?)

I think this could use an admonition of some sorts instead of being in-line.
@fhennig Thoughts?

@fhennig
Copy link
Contributor

fhennig commented Jan 30, 2024

yes it could be a "NOTE". We don't really have any guidelines for when to use an admonition or when not to though

@soenkeliebau
Copy link
Member

It is under the CA heading but does this apply to all certificates?
(Or do they go away with the pod they are attached to anyway?)

Normal certificates die with the pods they were created for. Unless running in rootless mode they are only ever "stored" in ram.

@lfrancke lfrancke moved this from In Progress to Done in Stackable End-to-End Coordination Jan 31, 2024
@nightkr
Copy link
Member

nightkr commented Feb 1, 2024

Where is that warning generated?

In the operator logs, currently. I can understand if we don't want to mention it to avoid overpromising, until we expose it better.

@nightkr
Copy link
Member

nightkr commented Feb 1, 2024

It is under the CA heading but does this apply to all certificates?
(Or do they go away with the pod they are attached to anyway?)

As @soenkeliebau mentioned, we don't store pod certificates at all, outside of the temporary pod volume.

I think this could use an admonition of some sorts instead of being in-line.
@fhennig Thoughts?

It could be, but I think it's also a relatively low priority problem for people to worry about. I don't want to overuse admonitions to the point where people become blind to them.

@lfrancke
Copy link
Member

lfrancke commented Feb 1, 2024

Thanks!
Could you please do a follow-up PR with these two changes:

  1. Either remove the information about the warning or mention that it can be found in the operator logs
  2. Make the deletion thing a NOTE admonition

I understand that you have a different opinion on the second but I find those things to actually help because they break up the "monotony" of long texts.

@nightkr
Copy link
Member

nightkr commented Feb 1, 2024

Sure.

nightkr added a commit that referenced this issue Feb 1, 2024
github-merge-queue bot pushed a commit that referenced this issue Feb 1, 2024
@lfrancke lfrancke moved this from Acceptance: In Progress to Done in Stackable Engineering Feb 1, 2024
@soenkeliebau
Copy link
Member

Where is that warning generated?

In the operator logs, currently. I can understand if we don't want to mention it to avoid overpromising, until we expose it better.

What would a better exposure look like here? I'll be happy to create an issue for it.
In the status of the SecretClass?

Random thougth that was sparked by this, should we start a list of "status stuff you should add to your alerting if you want to be informed about what we think you should do" page in the docs?

@nightkr
Copy link
Member

nightkr commented Feb 6, 2024

@soenkeliebau
I was primarily thinking of a warning Event, but writing it into the status also sounds like a good idea.

@soenkeliebau
Copy link
Member

Events are really short lived and would die pretty soon, so unless someone would alert on that event they might miss it..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/24.3.0 release-note Denotes a PR that will be considered when it comes time to generate release notes. type/internal-debt type/internal-improvement
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants