TLS renew #807
TLS renew #807
Conversation
Ticker in cert generation poll loop was never stopped. Signed-off-by: Seán C McCord <ulexus@gmail.com>
- Implements part of #67 - Create option functions for gRPC TLS config - Create CertificateProvider interface - Create (legacy) userdata CertificateProvider - Create cert renewing, file-caching CertificateProvider - Create tls.NewConfigWithOpts() to specify custom gRPC TLS options - Adapt existing tls.NewConfig to use options - Update osd to use dynamic CertificateProvider - Add a constant for location of Talos node certificate Signed-off-by: Seán C McCord <ulexus@gmail.com>
Using TODO context for now. Signal-bound root context? Signed-off-by: Seán C McCord <ulexus@gmail.com>
Signed-off-by: Seán C McCord <ulexus@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #807 +/- ##
==========================================
- Coverage 31.46% 30.79% -0.68%
==========================================
Files 84 85 +1
Lines 4846 4946 +100
==========================================
- Hits 1525 1523 -2
- Misses 3182 3285 +103
+ Partials 139 138 -1
Continue to review full report at Codecov.
|
|
Closes #67. |
|
Well, the cert rotation definitely works. It's now rotating as fast as it possibly can, so every web request is seeing an updated cert. :( |
The current TLS certs do not store the pre-parsed form in cert.Leaf; thus, we have to parse the certificate first, to get the expiry. This also limits the maximum TLS certificate renewal period to a global constant, presently set to one hour. Signed-off-by: Seán C McCord <ulexus@gmail.com>
internal/pkg/grpc/tls/cert.go
Outdated
| "github.com/talos-systems/talos/pkg/userdata" | ||
| ) | ||
|
|
||
| /* |
There was a problem hiding this comment.
Are these commented out intentionally?
There was a problem hiding this comment.
Yeah... it was a draft thought, and I didn't want to get rid of the code entirely. I should probably just delete it.
internal/pkg/grpc/tls/cert.go
Outdated
| return nil | ||
| } | ||
|
|
||
| /* |
There was a problem hiding this comment.
Is this comment intentional?
There was a problem hiding this comment.
Same thing; old code that I wasn't quite ready to throw away.
| if ok := certPool.AppendCertsFromPEM(data.CA.Crt); !ok { | ||
| return nil, fmt.Errorf("failed to append client certs") | ||
| // ConfigOptionFunc describes a configuration option function for the TLS config | ||
| type ConfigOptionFunc func(*tls.Config) error |
There was a problem hiding this comment.
This is actually a little silly, right now, since it returns a standard public object which can then be directly modified. However, I suspect that may change in the future, or that we may want to add more project-specific options.
Signed-off-by: Seán C McCord <ulexus@gmail.com>
internal/pkg/constants/constants.go
Outdated
| NodeCertFile = "/var/talos-node.crt" | ||
|
|
||
| // NodeCertRenewalInterval is the default interval at which Talos Node Certifications should be renewed | ||
| NodeCertRenewalInterval = time.Minute |
There was a problem hiding this comment.
Is defaulting to a minute intentional?
There was a problem hiding this comment.
Oops. That's a bit aggressive!
| if len(c.Certificate) > 0 { | ||
| cert, err := x509.ParseCertificate(c.Certificate[0]) | ||
| if err == nil { | ||
| nextRenewal = time.Until(cert.NotAfter) / 2 |
There was a problem hiding this comment.
Ahh I think I understand. We default to a minute, but then transition to half the expiry date.
There was a problem hiding this comment.
Yes, but even then, 1 minute is too aggressive. That was just for testing.
Signed-off-by: Seán C McCord <ulexus@gmail.com>
|
@Ulexus It looks like we have a couple place we need to set the (not osctl though) |
|
Yeah, I wasn't planning to get into the certificate production (and hence expiration) on this run; all I'm doing is checking when it expires, and setting the renewal to be before then. |
Works for me. |
Implement TLS certificate renewal for OSD.
This is definitely a WIP and needs architectural review. As discussed, this is a temporary option until we can move to an ephemeral-by-default system.