aboutsummaryrefslogtreecommitdiff
path: root/acme/autocert
diff options
context:
space:
mode:
Diffstat (limited to 'acme/autocert')
-rw-r--r--acme/autocert/autocert.go18
-rw-r--r--acme/autocert/renewal.go36
-rw-r--r--acme/autocert/renewal_test.go69
3 files changed, 76 insertions, 47 deletions
diff --git a/acme/autocert/autocert.go b/acme/autocert/autocert.go
index ccd5b7e..cde9066 100644
--- a/acme/autocert/autocert.go
+++ b/acme/autocert/autocert.go
@@ -134,7 +134,8 @@ type Manager struct {
// RenewBefore optionally specifies how early certificates should
// be renewed before they expire.
//
- // If zero, they're renewed 30 days before expiration.
+ // If zero, they're renewed at the lesser of 30 days or
+ // 1/3 of the certificate lifetime.
RenewBefore time.Duration
// Client is used to perform low-level operations, such as account registration
@@ -464,7 +465,7 @@ func (m *Manager) cert(ctx context.Context, ck certKey) (*tls.Certificate, error
leaf: cert.Leaf,
}
m.state[ck] = s
- m.startRenew(ck, s.key, s.leaf.NotAfter)
+ m.startRenew(ck, s.key, s.leaf.NotBefore, s.leaf.NotAfter)
return cert, nil
}
@@ -610,7 +611,7 @@ func (m *Manager) createCert(ctx context.Context, ck certKey) (*tls.Certificate,
}
state.cert = der
state.leaf = leaf
- m.startRenew(ck, state.key, state.leaf.NotAfter)
+ m.startRenew(ck, state.key, state.leaf.NotBefore, state.leaf.NotAfter)
return state.tlscert()
}
@@ -908,7 +909,7 @@ func httpTokenCacheKey(tokenPath string) string {
//
// The key argument is a certificate private key.
// The exp argument is the cert expiration time (NotAfter).
-func (m *Manager) startRenew(ck certKey, key crypto.Signer, exp time.Time) {
+func (m *Manager) startRenew(ck certKey, key crypto.Signer, notBefore, notAfter time.Time) {
m.renewalMu.Lock()
defer m.renewalMu.Unlock()
if m.renewal[ck] != nil {
@@ -920,7 +921,7 @@ func (m *Manager) startRenew(ck certKey, key crypto.Signer, exp time.Time) {
}
dr := &domainRenewal{m: m, ck: ck, key: key}
m.renewal[ck] = dr
- dr.start(exp)
+ dr.start(notBefore, notAfter)
}
// stopRenew stops all currently running cert renewal timers.
@@ -1028,13 +1029,6 @@ func (m *Manager) hostPolicy() HostPolicy {
return defaultHostPolicy
}
-func (m *Manager) renewBefore() time.Duration {
- if m.RenewBefore > renewJitter {
- return m.RenewBefore
- }
- return 720 * time.Hour // 30 days
-}
-
func (m *Manager) now() time.Time {
if m.nowFunc != nil {
return m.nowFunc()
diff --git a/acme/autocert/renewal.go b/acme/autocert/renewal.go
index 0df7da7..93984f3 100644
--- a/acme/autocert/renewal.go
+++ b/acme/autocert/renewal.go
@@ -11,9 +11,6 @@ import (
"time"
)
-// renewJitter is the maximum deviation from Manager.RenewBefore.
-const renewJitter = time.Hour
-
// domainRenewal tracks the state used by the periodic timers
// renewing a single domain's cert.
type domainRenewal struct {
@@ -30,13 +27,13 @@ type domainRenewal struct {
// defined by the certificate expiration time exp.
//
// If the timer is already started, calling start is a noop.
-func (dr *domainRenewal) start(exp time.Time) {
+func (dr *domainRenewal) start(notBefore, notAfter time.Time) {
dr.timerMu.Lock()
defer dr.timerMu.Unlock()
if dr.timer != nil {
return
}
- dr.timer = time.AfterFunc(dr.next(exp), dr.renew)
+ dr.timer = time.AfterFunc(dr.next(notBefore, notAfter), dr.renew)
}
// stop stops the cert renewal timer and waits for any in-flight calls to renew
@@ -79,7 +76,7 @@ func (dr *domainRenewal) renew() {
// TODO: rotate dr.key at some point?
next, err := dr.do(ctx)
if err != nil {
- next = renewJitter / 2
+ next = time.Hour / 2
next += time.Duration(pseudoRand.int63n(int64(next)))
}
testDidRenewLoop(next, err)
@@ -107,8 +104,8 @@ func (dr *domainRenewal) do(ctx context.Context) (time.Duration, error) {
// a race is likely unavoidable in a distributed environment
// but we try nonetheless
if tlscert, err := dr.m.cacheGet(ctx, dr.ck); err == nil {
- next := dr.next(tlscert.Leaf.NotAfter)
- if next > dr.m.renewBefore()+renewJitter {
+ next := dr.next(tlscert.Leaf.NotBefore, tlscert.Leaf.NotAfter)
+ if next > 0 {
signer, ok := tlscert.PrivateKey.(crypto.Signer)
if ok {
state := &certState{
@@ -139,18 +136,23 @@ func (dr *domainRenewal) do(ctx context.Context) (time.Duration, error) {
return 0, err
}
dr.updateState(state)
- return dr.next(leaf.NotAfter), nil
+ return dr.next(leaf.NotBefore, leaf.NotAfter), nil
}
-func (dr *domainRenewal) next(expiry time.Time) time.Duration {
- d := expiry.Sub(dr.m.now()) - dr.m.renewBefore()
- // add a bit of randomness to renew deadline
- n := pseudoRand.int63n(int64(renewJitter))
- d -= time.Duration(n)
- if d < 0 {
- return 0
+// next returns the wait time before the next renewal should start.
+// If manager.RenewBefore is set, it uses that capped at 30 days,
+// otherwise it uses a default of 1/3 of the cert lifetime.
+// It builds in a jitter of 10% of the renew threshold, capped at 1 hour.
+func (dr *domainRenewal) next(notBefore, notAfter time.Time) time.Duration {
+ threshold := min(notAfter.Sub(notBefore)/3, 30*24*time.Hour)
+ if dr.m.RenewBefore > 0 {
+ threshold = min(dr.m.RenewBefore, 30*24*time.Hour)
}
- return d
+ maxJitter := min(threshold/10, time.Hour)
+ jitter := pseudoRand.int63n(int64(maxJitter))
+ renewAt := notAfter.Add(-(threshold - time.Duration(jitter)))
+ renewWait := renewAt.Sub(dr.m.now())
+ return max(0, renewWait)
}
var testDidRenewLoop = func(next time.Duration, err error) {}
diff --git a/acme/autocert/renewal_test.go b/acme/autocert/renewal_test.go
index ffe4af2..67e2da2 100644
--- a/acme/autocert/renewal_test.go
+++ b/acme/autocert/renewal_test.go
@@ -17,27 +17,60 @@ import (
func TestRenewalNext(t *testing.T) {
now := time.Now()
- man := &Manager{
- RenewBefore: 7 * 24 * time.Hour,
- nowFunc: func() time.Time { return now },
- }
- defer man.stopRenew()
+ nowFn := func() time.Time { return now }
tt := []struct {
- expiry time.Time
- min, max time.Duration
+ name string
+ renewBefore time.Duration // arg to Manager
+ // leaf cert validity
+ notBefore time.Time
+ validFor time.Duration
+ // wait time
+ waitMin, waitMax time.Duration
}{
- {now.Add(90 * 24 * time.Hour), 83*24*time.Hour - renewJitter, 83 * 24 * time.Hour},
- {now.Add(time.Hour), 0, 1},
- {now, 0, 1},
- {now.Add(-time.Hour), 0, 1},
+ {"default renewal, 1h cert, valid",
+ 0, now, time.Hour, 40 * time.Minute, 50 * time.Minute},
+ {"default renewal, 1h cert, should renew",
+ 0, now.Add(-50 * time.Minute), time.Hour, 0, 0},
+ {"default renewal, 1h cert, expired",
+ 0, now.Add(-400 * 24 * time.Hour), time.Hour, 0, 0},
+ {"default renewal, 6d cert, valid",
+ 0, now, 6 * 24 * time.Hour, 4 * 24 * time.Hour, (4*24 + 1) * time.Hour},
+ {"default renewal, 6d cert, should renew",
+ 0, now.Add(-5 * 24 * time.Hour), 6 * 24 * time.Hour, 0, 0},
+ {"default renewal, 6d cert, expired",
+ 0, now.Add(-400 * 24 * time.Hour), 6 * 24 * time.Hour, 0, 0},
+ {"default renewal, 90d cert, valid",
+ 0, now, 90 * 24 * time.Hour, 60 * 24 * time.Hour, (60*24 + 1) * time.Hour},
+ {"default renewal, 90d cert, should renew",
+ 0, now.Add(-70 * 24 * time.Hour), 90 * 24 * time.Hour, 0, 0},
+ {"default renewal, 90d cert, expired",
+ 0, now.Add(-400 * 24 * time.Hour), 90 * 24 * time.Hour, 0, 0},
+ {"default renewal, 398d cert, valid",
+ 0, now, 398 * 24 * time.Hour, (368 * 24) * time.Hour, (368*24 + 1) * time.Hour},
+ {"default renewal, 398d cert, should renew",
+ 0, now.Add(-378 * 24 * time.Hour), 398 * 24 * time.Hour, 0, 0},
+ {"default renewal, 398d cert, expired",
+ 0, now.Add(-400 * 24 * time.Hour), 398 * 24 * time.Hour, 0, 0},
+ {"7d renewal, 90d cert, valid",
+ 7 * 24 * time.Hour, now, 90 * 24 * time.Hour, 83 * 24 * time.Hour, (83*24 + 1) * time.Hour},
+ {"7d renewal, 90d cert, should not renew",
+ 7 * 24 * time.Hour, now.Add(-70 * 24 * time.Hour), 90 * 24 * time.Hour, 13 * 24 * time.Hour, (13*24 + 1) * time.Hour},
+ {"7d renewal, 90d cert, should renew",
+ 7 * 24 * time.Hour, now.Add(-85 * 24 * time.Hour), 90 * 24 * time.Hour, 0, 0},
+ {"7d renewal, 90d cert, expired",
+ 7 * 24 * time.Hour, now.Add(-400 * 24 * time.Hour), 90 * 24 * time.Hour, 0, 0},
}
- dr := &domainRenewal{m: man}
- for i, test := range tt {
- next := dr.next(test.expiry)
- if next < test.min || test.max < next {
- t.Errorf("%d: next = %v; want between %v and %v", i, next, test.min, test.max)
- }
+ for _, test := range tt {
+ t.Run(test.name, func(t *testing.T) {
+ dr := &domainRenewal{m: &Manager{RenewBefore: test.renewBefore, nowFunc: nowFn}}
+ defer dr.m.stopRenew()
+
+ next := dr.next(test.notBefore, test.notBefore.Add(test.validFor))
+ if next < test.waitMin || next > test.waitMax {
+ t.Errorf("expected wait time: %v <= %v <= %v", test.waitMin, next, test.waitMax)
+ }
+ })
}
}
@@ -239,7 +272,7 @@ func TestRenewFromCacheAlreadyRenewed(t *testing.T) {
}
// trigger renew
- man.startRenew(exampleCertKey, s.key, s.leaf.NotAfter)
+ man.startRenew(exampleCertKey, s.key, s.leaf.NotBefore, s.leaf.NotAfter)
<-renewed
func() {
man.renewalMu.Lock()