pkg/transport: reload TLS certificates for every use #7784
Conversation
|
Caveats: This requires Go 1.8.1 |
|
Commit title should be |
This changes the baseConfig used when creating tls Configs to utilize the GetCertificate and GetClientCertificate functions to always reload the certificates from disk whenever they are needed. Always reloading the certificates allows changing the certificates via an external process without interrupting etcd. Fixes #7576
tgrosinger
changed the title
|
So the existing TLS connections will not be terminated even if the cert is changed? Is this the expected behavior? |
|
That is correct. Because the TLS negotiation occurs only when the connection is opened, ongoing connection can continue. |
|
yea. i know how tls works. my concerns is that users might expect the existing tls connections to be terminated when they switch key/cert. but i am not a security expert and do not know the common practice. it would be great if you can feed me any related info: like what other projects do online cert/key change. thanks. |
|
Not terminating existing connections is the usual case. Main reason to replace certs is local renewal. In the more rare case of moving from one trust root to another, it's a conscious act, and restarting the service would be a good way to terminate all connections. Not killing existing connections matches expectation (either automatic reload, or reloading on a SIGHUP.) |
|
|
||
| // Load the certificate from disk every time so when it is replaced we | ||
| // will be using the latest version | ||
| return tlsutil.NewCert(info.CertFile, info.KeyFile, info.parseFunc) |
There was a problem hiding this comment.
have you benchmarked the overhead of doing this?
if this brings a huge overhead, we probably need to do caching. (inotify + caching parsed result or simply pull based caching)
| ServerName: info.ServerName, | ||
| cfg.GetCertificate = func(clientHello *tls.ClientHelloInfo) ( | ||
| *tls.Certificate, error) { | ||
|
|
There was a problem hiding this comment.
remove this extra empty line at 172
| } | ||
| cfg.GetClientCertificate = func(unused *tls.CertificateRequestInfo) ( | ||
| *tls.Certificate, error) { | ||
|
|
|
|
||
| // Load the certificate from disk every time so when it is replaced we | ||
| // will be using the latest version | ||
| return tlsutil.NewCert(info.CertFile, info.KeyFile, info.parseFunc) |
|
@Spindel thanks for the explanation. make sense. |
|
For our use case we are running etcd in a Kubernetes pod, so we are using an in-memory EmptyDir volume to help with performance. I agree that caching would be desirable, however this patch was meant to be as minimal as possible. It's something I wanted to share with the community for others to build off of, not necessarily to be merged into etcd proper as it currently stands. |
|
Ok, I can confirm that this works with
Test fails because we still need Thanks! |
This changes the baseConfig used when creating tls Configs to utilize the GetCertificate and GetClientCertificate functions to always reload the certificates from disk whenever they are needed. Always reloading the certificates allows changing the certificates via an external process without interrupting etcd. Fixes etcd-io#7576 Cherry-picked by Gyu-Ho Lee <[email protected]> Original commit can be found at etcd-io#7784
This changes the baseConfig used when creating tls Configs to utilize the GetCertificate and GetClientCertificate functions to always reload the certificates from disk whenever they are needed. Always reloading the certificates allows changing the certificates via an external process without interrupting etcd. Fixes etcd-io#7576 Cherry-picked by Gyu-Ho Lee <[email protected]> Original commit can be found at etcd-io#7784
This changes the baseConfig used when creating tls Configs to utilize the GetCertificate and GetClientCertificate functions to always reload the certificates from disk whenever they are needed. Always reloading the certificates allows changing the certificates via an external process without interrupting etcd. Fixes etcd-io#7576 Cherry-picked by Gyu-Ho Lee <[email protected]> Original commit can be found at etcd-io#7784
|
Closing in favor of #7829. |
This changes the baseConfig used when creating tls Configs to utilize the GetCertificate and GetClientCertificate functions to always reload the certificates from disk whenever they are needed. Always reloading the certificates allows changing the certificates via an external process without interrupting etcd. Fixes etcd-io#7576 Cherry-picked by Gyu-Ho Lee <[email protected]> Original commit can be found at etcd-io#7784
This changes the baseConfig used when creating tls Configs to utilize the GetCertificate and GetClientCertificate functions to always reload the certificates from disk whenever they are needed. Always reloading the certificates allows changing the certificates via an external process without interrupting etcd. Fixes etcd-io#7576 Cherry-picked by Gyu-Ho Lee <[email protected]> Original commit can be found at etcd-io#7784
This changes the baseConfig used when creating tls Configs to utilize
the GetCertificate and GetClientCertificate functions to always reload
the certificates from disk whenever they are needed.
Always reloading the certificates allows changing the certificates via
an external process without interrupting etcd.
Fixes #7576