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

Add support for tls certificate update #3272

Closed
wants to merge 1 commit into from

Conversation

binoue
Copy link
Contributor

@binoue binoue commented Jan 14, 2020

This pull request addresses #3198.

In this pull request, the following changes are made.

  • Add goroutine to load x509 certificate and add tlsConfig.GetCertificate handler to apply the loaded x509 certificate.
  • Add cli option to adjust the interval of updating tls certificates.

@gravitational-jenkins
Copy link

Can one of the admins verify this patch?

@webvictim
Copy link
Contributor

ok to test

@klizhentas
Copy link
Contributor

setting expectations folks, the proposed design is very risky, as there is no explicit state machine for rolling this back in this design. I would want to have FSM like approach using signals similar to what we have for binary reloads.

@binoue
Copy link
Contributor Author

binoue commented Jan 20, 2020

Thank you for reviewing!
The pull request is intending to keep updating the certificate with a certain interval.
And if the certificate is not valid, it is ignored.

Therefore, if the certificate is invalid because of some operation,
the teleport process keeps working.

Considering that, we are not sure what risk this pull request has.
If we misunderstand, please let us know.

@russjones
Copy link
Contributor

@binoue Take a look at how CA rotation is handled and has the ability to roll back to a prior state if needed. It was introduced in #1899, this is required by Teleport.

If you are interested in making the above changes let's proceed, otherwise it's best to close this for now.

@binoue
Copy link
Contributor Author

binoue commented Mar 1, 2020

@russjones

Thank you for your replay.

If you are interested in making the above changes let's proceed, otherwise it's best to close this for now

Okey, I will try to fix this pull request as soon as possible

@binoue
Copy link
Contributor Author

binoue commented Mar 16, 2020

@russjones

I checked your links, code, and documents.
I think I need to implement something like tctl proxy rotate for this issue.
This seems to become a big PR and to take much time.

In the original issue, a maintainer recognizes this as an important issue.
Is there any plan to implement this feature at some point?
If so, I will close this pull request.

@russjones
Copy link
Contributor

@binoue Let's close this PR for now then. We don't have plans to address this at the moment, but we'll keep it in mind when planning the next milestone.

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 this pull request may close these issues.

5 participants