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

[💡 FEATURE REQUEST]: Enable TLS client certificate rotation #522

Closed
benkelukas opened this issue May 29, 2024 · 2 comments · Fixed by #523
Closed

[💡 FEATURE REQUEST]: Enable TLS client certificate rotation #522

benkelukas opened this issue May 29, 2024 · 2 comments · Fixed by #523
Assignees
Labels
C-feature-request Category: feature requested, but need to be discussed

Comments

@benkelukas
Copy link
Contributor

Plugin

Temporal

I have an idea!

Hi,

we have an infrastructure issuing client certificates for mTLS connection.

Currently, when those certificates expire and they are rotated and re-mounted into the application, the connection does not know about this and does not use the rotated certificates, leading to a connection error and a need to restart the application, which is not ideal as it introduces some additional operational overhead and uses resources to restart the application - k8s pods in our case.

We've solved it in our Go apps using Temporal by using tls.Config::getClientCertificate method instead of passing the already built certificates into the structs Certificates field. The solution is inspired by this article

For our PHP apps, we'd like to emulate the same functionality in this plugin.

I'm attaching a pull request that does that and would appreciate any feedback, if you are interested in incorporating this into the codebase :)

Thanks!

@rustatian
Copy link
Collaborator

Hey @benkelukas 👋
Thanks for the PR 👍
Yes, this is true, there are two methods to use certificates, I implemented a similar thing for the RR http plugin: link. I think in the future it might be worth implementing ACME support as well.

@benkelukas
Copy link
Contributor Author

Hi @rustatian 👋

thanks for reviewing the PR! Happy to contribute :)

ACME support sounds like something people might use for Temporal. I was not aware of ACME before, so would have to do some studying on how the implementation for Temporal might look like, but sounds like a great addition 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: feature requested, but need to be discussed
Projects
None yet
2 participants