Created
October 8, 2018 16:09
-
-
Save agunnerson-ibm/20339f05709e3e5d6ff52e8f340f556c to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| From c3494537eb37842c45da8a7ca52ce70be18b7c33 Mon Sep 17 00:00:00 2001 | |
| From: Andrew Gunnerson <andrew.gunnerson@us.ibm.com> | |
| Date: Mon, 8 Oct 2018 12:08:32 -0400 | |
| Subject: [PATCH] certificate_manager: Check that template differs from current | |
| cert before rotation | |
| Signed-off-by: Andrew Gunnerson <andrew.gunnerson@us.ibm.com> | |
| --- | |
| .../util/certificate/certificate_manager.go | 48 +++++++++++-------- | |
| 1 file changed, 29 insertions(+), 19 deletions(-) | |
| diff --git a/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go b/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go | |
| index 7b07b26a3e..c67b7a2662 100644 | |
| --- a/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go | |
| +++ b/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go | |
| @@ -274,7 +274,7 @@ func (m *manager) Start() { | |
| if m.dynamicTemplate { | |
| go wait.Forever(func() { | |
| // check if the current template matches what we last requested | |
| - if !reflect.DeepEqual(m.getLastRequest(), m.getTemplate()) { | |
| + if !m.certMatchesTemplate() && !reflect.DeepEqual(m.getLastRequest(), m.getTemplate()) { | |
| // if the template is different, queue up an interrupt of the rotation deadline loop. | |
| // if we've requested a CSR that matches the new template by the time the interrupt is handled, the interrupt is disregarded. | |
| templateChanged <- struct{}{} | |
| @@ -389,35 +389,25 @@ func (m *manager) rotateCerts() (bool, error) { | |
| return true, nil | |
| } | |
| -// nextRotationDeadline returns a value for the threshold at which the | |
| -// current certificate should be rotated, 80%+/-10% of the expiration of the | |
| -// certificate. | |
| -func (m *manager) nextRotationDeadline() time.Time { | |
| - // forceRotation is not protected by locks | |
| - if m.forceRotation { | |
| - m.forceRotation = false | |
| - return time.Now() | |
| - } | |
| - | |
| +func (m* manager) certMatchesTemplate() bool { | |
| m.certAccessLock.RLock() | |
| defer m.certAccessLock.RUnlock() | |
| if m.cert == nil { | |
| - return time.Now() | |
| + return false | |
| } | |
| - // Ensure the currently held certificate satisfies the requested subject CN and SANs | |
| if template := m.getTemplate(); template != nil { | |
| if template.Subject.CommonName != m.cert.Leaf.Subject.CommonName { | |
| - glog.V(2).Infof("Current certificate CN (%s) does not match requested CN (%s), rotating now", m.cert.Leaf.Subject.CommonName, template.Subject.CommonName) | |
| - return time.Now() | |
| + glog.V(2).Infof("Current certificate CN (%s) does not match requested CN (%s)", m.cert.Leaf.Subject.CommonName, template.Subject.CommonName) | |
| + return false | |
| } | |
| currentDNSNames := sets.NewString(m.cert.Leaf.DNSNames...) | |
| desiredDNSNames := sets.NewString(template.DNSNames...) | |
| missingDNSNames := desiredDNSNames.Difference(currentDNSNames) | |
| if len(missingDNSNames) > 0 { | |
| - glog.V(2).Infof("Current certificate is missing requested DNS names %v, rotating now", missingDNSNames.List()) | |
| - return time.Now() | |
| + glog.V(2).Infof("Current certificate is missing requested DNS names %v", missingDNSNames.List()) | |
| + return false | |
| } | |
| currentIPs := sets.NewString() | |
| @@ -430,11 +420,31 @@ func (m *manager) nextRotationDeadline() time.Time { | |
| } | |
| missingIPs := desiredIPs.Difference(currentIPs) | |
| if len(missingIPs) > 0 { | |
| - glog.V(2).Infof("Current certificate is missing requested IP addresses %v, rotating now", missingIPs.List()) | |
| - return time.Now() | |
| + glog.V(2).Infof("Current certificate is missing requested IP addresses %v", missingIPs.List()) | |
| + return false | |
| } | |
| } | |
| + return true | |
| +} | |
| + | |
| +// nextRotationDeadline returns a value for the threshold at which the | |
| +// current certificate should be rotated, 80%+/-10% of the expiration of the | |
| +// certificate. | |
| +func (m *manager) nextRotationDeadline() time.Time { | |
| + // forceRotation is not protected by locks | |
| + if m.forceRotation { | |
| + m.forceRotation = false | |
| + return time.Now() | |
| + } | |
| + | |
| + m.certAccessLock.RLock() | |
| + defer m.certAccessLock.RUnlock() | |
| + | |
| + if !m.certMatchesTemplate() { | |
| + return time.Now() | |
| + } | |
| + | |
| notAfter := m.cert.Leaf.NotAfter | |
| totalDuration := float64(notAfter.Sub(m.cert.Leaf.NotBefore)) | |
| deadline := m.cert.Leaf.NotBefore.Add(jitteryDuration(totalDuration)) | |
| -- | |
| 2.17.1 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment