Skip to content

Commit 5ee48a3

Browse files
committed
Add config option to disable ARI
This may be temporary until ARI is more mature
1 parent 1a2275d commit 5ee48a3

File tree

5 files changed

+64
-55
lines changed

5 files changed

+64
-55
lines changed

acmeissuer.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ func (am *ACMEIssuer) doIssue(ctx context.Context, csr *x509.CertificateRequest,
461461
// between client and server or some sort of bookkeeping error with regards to the certID
462462
// and the server is rejecting the ARI certID. In any case, an invalid certID may cause
463463
// orders to fail. So try once without setting it.
464-
if !usingTestCA && attempts != 2 {
464+
if !am.config.DisableARI && !usingTestCA && attempts != 2 {
465465
if replacing, ok := ctx.Value(ctxKeyARIReplaces).(*x509.Certificate); ok {
466466
params.Replaces = replacing
467467
}

certificates.go

+45-44
Original file line numberDiff line numberDiff line change
@@ -103,53 +103,54 @@ func (cfg *Config) certNeedsRenewal(leaf *x509.Certificate, ari acme.RenewalInfo
103103
logger = zap.NewNop()
104104
}
105105

106-
// first check ARI: if it says it's time to renew, it's time to renew
107-
// (notice that we don't strictly require an ARI window to also exist; we presume
108-
// that if a time has been selected, a window does or did exist, even if it didn't
109-
// get stored/encoded for some reason - but also: this allows administrators to
110-
// manually or explicitly schedule a renewal time indepedently of ARI which could
111-
// be useful)
112-
selectedTime := ari.SelectedTime
113-
114-
// if, for some reason a random time in the window hasn't been selected yet, but an ARI
115-
// window does exist, we can always improvise one... even if this is called repeatedly,
116-
// a random time is a random time, whether you generate it once or more :D
117-
// (code borrowed from our acme package)
118-
if selectedTime.IsZero() &&
119-
(!ari.SuggestedWindow.Start.IsZero() && !ari.SuggestedWindow.End.IsZero()) {
120-
start, end := ari.SuggestedWindow.Start.Unix()+1, ari.SuggestedWindow.End.Unix()
121-
selectedTime = time.Unix(rand.Int63n(end-start)+start, 0).UTC()
122-
logger.Warn("no renewal time had been selected with ARI; chose an ephemeral one for now",
123-
zap.Time("ephemeral_selected_time", selectedTime))
124-
}
125-
126-
// if a renewal time has been selected, start with that
127-
if !selectedTime.IsZero() {
128-
// ARI spec recommends an algorithm that renews after the randomly-selected
129-
// time OR just before it if the next waking time would be after it; this
130-
// cutoff can actually be before the start of the renewal window, but the spec
131-
// author says that's OK: https://github.com/aarongable/draft-acme-ari/issues/71
132-
cutoff := ari.SelectedTime.Add(-cfg.certCache.options.RenewCheckInterval)
133-
if time.Now().After(cutoff) {
134-
logger.Info("certificate needs renewal based on ARI window",
135-
zap.Time("selected_time", selectedTime),
136-
zap.Time("renewal_cutoff", cutoff))
137-
return true
106+
if !cfg.DisableARI {
107+
// first check ARI: if it says it's time to renew, it's time to renew
108+
// (notice that we don't strictly require an ARI window to also exist; we presume
109+
// that if a time has been selected, a window does or did exist, even if it didn't
110+
// get stored/encoded for some reason - but also: this allows administrators to
111+
// manually or explicitly schedule a renewal time indepedently of ARI which could
112+
// be useful)
113+
selectedTime := ari.SelectedTime
114+
115+
// if, for some reason a random time in the window hasn't been selected yet, but an ARI
116+
// window does exist, we can always improvise one... even if this is called repeatedly,
117+
// a random time is a random time, whether you generate it once or more :D
118+
// (code borrowed from our acme package)
119+
if selectedTime.IsZero() &&
120+
(!ari.SuggestedWindow.Start.IsZero() && !ari.SuggestedWindow.End.IsZero()) {
121+
start, end := ari.SuggestedWindow.Start.Unix()+1, ari.SuggestedWindow.End.Unix()
122+
selectedTime = time.Unix(rand.Int63n(end-start)+start, 0).UTC()
123+
logger.Warn("no renewal time had been selected with ARI; chose an ephemeral one for now",
124+
zap.Time("ephemeral_selected_time", selectedTime))
138125
}
139126

140-
// according to ARI, we are not ready to renew; however, we do not rely solely on
141-
// ARI calculations... what if there is a bug in our implementation, or in the
142-
// server's, or the stored metadata? for redundancy, give credence to the expiration
143-
// date; ignore ARI if we are past a "dangerously close" limit, to avoid any
144-
// possibility of a bug in ARI compromising a site's uptime: we should always always
145-
// always give heed to actual validity period
146-
if currentlyInRenewalWindow(leaf.NotBefore, expiration, 1.0/20.0) {
147-
logger.Warn("certificate is in emergency renewal window; superceding ARI",
148-
zap.Duration("remaining", time.Until(expiration)),
149-
zap.Time("renewal_cutoff", cutoff))
150-
return true
127+
// if a renewal time has been selected, start with that
128+
if !selectedTime.IsZero() {
129+
// ARI spec recommends an algorithm that renews after the randomly-selected
130+
// time OR just before it if the next waking time would be after it; this
131+
// cutoff can actually be before the start of the renewal window, but the spec
132+
// author says that's OK: https://github.com/aarongable/draft-acme-ari/issues/71
133+
cutoff := ari.SelectedTime.Add(-cfg.certCache.options.RenewCheckInterval)
134+
if time.Now().After(cutoff) {
135+
logger.Info("certificate needs renewal based on ARI window",
136+
zap.Time("selected_time", selectedTime),
137+
zap.Time("renewal_cutoff", cutoff))
138+
return true
139+
}
140+
141+
// according to ARI, we are not ready to renew; however, we do not rely solely on
142+
// ARI calculations... what if there is a bug in our implementation, or in the
143+
// server's, or the stored metadata? for redundancy, give credence to the expiration
144+
// date; ignore ARI if we are past a "dangerously close" limit, to avoid any
145+
// possibility of a bug in ARI compromising a site's uptime: we should always always
146+
// always give heed to actual validity period
147+
if currentlyInRenewalWindow(leaf.NotBefore, expiration, 1.0/20.0) {
148+
logger.Warn("certificate is in emergency renewal window; superceding ARI",
149+
zap.Duration("remaining", time.Until(expiration)),
150+
zap.Time("renewal_cutoff", cutoff))
151+
return true
152+
}
151153
}
152-
153154
}
154155

155156
// the normal check, in the absence of ARI, is to determine if we're near enough (or past)

config.go

+16-8
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,10 @@ type Config struct {
149149
// EXPERIMENTAL: Subject to change or removal.
150150
SubjectTransformer func(ctx context.Context, domain string) string
151151

152+
// Disables both ARI fetching and the use of ARI for renewal decisions.
153+
// TEMPORARY: Will likely be removed in the future.
154+
DisableARI bool
155+
152156
// Set a logger to enable logging. If not set,
153157
// a default logger will be created.
154158
Logger *zap.Logger
@@ -451,7 +455,7 @@ func (cfg *Config) manageOne(ctx context.Context, domainName string, async bool)
451455

452456
// ensure ARI is updated before we check whether the cert needs renewing
453457
// (we ignore the second return value because we already check if needs renewing anyway)
454-
if cert.ari.NeedsRefresh() {
458+
if !cfg.DisableARI && cert.ari.NeedsRefresh() {
455459
cert, _, err = cfg.updateARI(ctx, cert, cfg.Logger)
456460
if err != nil {
457461
cfg.Logger.Error("updating ARI upon managing", zap.Error(err))
@@ -888,11 +892,13 @@ func (cfg *Config) renewCert(ctx context.Context, name string, force, interactiv
888892
// if we're renewing with the same ACME CA as before, have the ACME
889893
// client tell the server we are replacing a certificate (but doing
890894
// this on the wrong CA, or when the CA doesn't recognize the certID,
891-
// can fail the order)
892-
if acmeData, err := certRes.getACMEData(); err == nil && acmeData.CA != "" {
893-
if acmeIss, ok := issuer.(*ACMEIssuer); ok {
894-
if acmeIss.CA == acmeData.CA {
895-
ctx = context.WithValue(ctx, ctxKeyARIReplaces, leaf)
895+
// can fail the order) -- TODO: change this check to whether we're using the same ACME account, not CA
896+
if !cfg.DisableARI {
897+
if acmeData, err := certRes.getACMEData(); err == nil && acmeData.CA != "" {
898+
if acmeIss, ok := issuer.(*ACMEIssuer); ok {
899+
if acmeIss.CA == acmeData.CA {
900+
ctx = context.WithValue(ctx, ctxKeyARIReplaces, leaf)
901+
}
896902
}
897903
}
898904
}
@@ -1246,8 +1252,10 @@ func (cfg *Config) managedCertNeedsRenewal(certRes CertificateResource, emitLogs
12461252
return 0, nil, true
12471253
}
12481254
var ari acme.RenewalInfo
1249-
if ariPtr, err := certRes.getARI(); err == nil && ariPtr != nil {
1250-
ari = *ariPtr
1255+
if !cfg.DisableARI {
1256+
if ariPtr, err := certRes.getARI(); err == nil && ariPtr != nil {
1257+
ari = *ariPtr
1258+
}
12511259
}
12521260
remaining := time.Until(expiresAt(certChain[0]))
12531261
return remaining, certChain[0], cfg.certNeedsRenewal(certChain[0], ari, emitLogs)

handshake.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,7 @@ func (cfg *Config) handshakeMaintenance(ctx context.Context, hello *tls.ClientHe
582582
}
583583

584584
// Check ARI status
585-
if cert.ari.NeedsRefresh() {
585+
if !cfg.DisableARI && cert.ari.NeedsRefresh() {
586586
// we ignore the second return value here because we go on to check renewal status below regardless
587587
var err error
588588
cert, _, err = cfg.updateARI(ctx, cert, logger)

maintain.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ func (certCache *Cache) RenewManagedCertificates(ctx context.Context) error {
136136
}
137137

138138
// ACME-specific: see if if ACME Renewal Info (ARI) window needs refreshing
139-
if cert.ari.NeedsRefresh() {
139+
if !cfg.DisableARI && cert.ari.NeedsRefresh() {
140140
configs[cert.hash] = cfg
141141
ariQueue = append(ariQueue, cert)
142142
}

0 commit comments

Comments
 (0)