From 5ee48a310846cbbd42c3c156db9aaf8a72b18c51 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Thu, 8 Aug 2024 08:08:29 -0600 Subject: [PATCH] Add config option to disable ARI This may be temporary until ARI is more mature --- acmeissuer.go | 2 +- certificates.go | 89 +++++++++++++++++++++++++------------------------ config.go | 24 ++++++++----- handshake.go | 2 +- maintain.go | 2 +- 5 files changed, 64 insertions(+), 55 deletions(-) diff --git a/acmeissuer.go b/acmeissuer.go index 87fa5ff5..e010f087 100644 --- a/acmeissuer.go +++ b/acmeissuer.go @@ -461,7 +461,7 @@ func (am *ACMEIssuer) doIssue(ctx context.Context, csr *x509.CertificateRequest, // between client and server or some sort of bookkeeping error with regards to the certID // and the server is rejecting the ARI certID. In any case, an invalid certID may cause // orders to fail. So try once without setting it. - if !usingTestCA && attempts != 2 { + if !am.config.DisableARI && !usingTestCA && attempts != 2 { if replacing, ok := ctx.Value(ctxKeyARIReplaces).(*x509.Certificate); ok { params.Replaces = replacing } diff --git a/certificates.go b/certificates.go index a5147a2d..2bdc07bd 100644 --- a/certificates.go +++ b/certificates.go @@ -103,53 +103,54 @@ func (cfg *Config) certNeedsRenewal(leaf *x509.Certificate, ari acme.RenewalInfo logger = zap.NewNop() } - // first check ARI: if it says it's time to renew, it's time to renew - // (notice that we don't strictly require an ARI window to also exist; we presume - // that if a time has been selected, a window does or did exist, even if it didn't - // get stored/encoded for some reason - but also: this allows administrators to - // manually or explicitly schedule a renewal time indepedently of ARI which could - // be useful) - selectedTime := ari.SelectedTime - - // if, for some reason a random time in the window hasn't been selected yet, but an ARI - // window does exist, we can always improvise one... even if this is called repeatedly, - // a random time is a random time, whether you generate it once or more :D - // (code borrowed from our acme package) - if selectedTime.IsZero() && - (!ari.SuggestedWindow.Start.IsZero() && !ari.SuggestedWindow.End.IsZero()) { - start, end := ari.SuggestedWindow.Start.Unix()+1, ari.SuggestedWindow.End.Unix() - selectedTime = time.Unix(rand.Int63n(end-start)+start, 0).UTC() - logger.Warn("no renewal time had been selected with ARI; chose an ephemeral one for now", - zap.Time("ephemeral_selected_time", selectedTime)) - } - - // if a renewal time has been selected, start with that - if !selectedTime.IsZero() { - // ARI spec recommends an algorithm that renews after the randomly-selected - // time OR just before it if the next waking time would be after it; this - // cutoff can actually be before the start of the renewal window, but the spec - // author says that's OK: https://github.com/aarongable/draft-acme-ari/issues/71 - cutoff := ari.SelectedTime.Add(-cfg.certCache.options.RenewCheckInterval) - if time.Now().After(cutoff) { - logger.Info("certificate needs renewal based on ARI window", - zap.Time("selected_time", selectedTime), - zap.Time("renewal_cutoff", cutoff)) - return true + if !cfg.DisableARI { + // first check ARI: if it says it's time to renew, it's time to renew + // (notice that we don't strictly require an ARI window to also exist; we presume + // that if a time has been selected, a window does or did exist, even if it didn't + // get stored/encoded for some reason - but also: this allows administrators to + // manually or explicitly schedule a renewal time indepedently of ARI which could + // be useful) + selectedTime := ari.SelectedTime + + // if, for some reason a random time in the window hasn't been selected yet, but an ARI + // window does exist, we can always improvise one... even if this is called repeatedly, + // a random time is a random time, whether you generate it once or more :D + // (code borrowed from our acme package) + if selectedTime.IsZero() && + (!ari.SuggestedWindow.Start.IsZero() && !ari.SuggestedWindow.End.IsZero()) { + start, end := ari.SuggestedWindow.Start.Unix()+1, ari.SuggestedWindow.End.Unix() + selectedTime = time.Unix(rand.Int63n(end-start)+start, 0).UTC() + logger.Warn("no renewal time had been selected with ARI; chose an ephemeral one for now", + zap.Time("ephemeral_selected_time", selectedTime)) } - // according to ARI, we are not ready to renew; however, we do not rely solely on - // ARI calculations... what if there is a bug in our implementation, or in the - // server's, or the stored metadata? for redundancy, give credence to the expiration - // date; ignore ARI if we are past a "dangerously close" limit, to avoid any - // possibility of a bug in ARI compromising a site's uptime: we should always always - // always give heed to actual validity period - if currentlyInRenewalWindow(leaf.NotBefore, expiration, 1.0/20.0) { - logger.Warn("certificate is in emergency renewal window; superceding ARI", - zap.Duration("remaining", time.Until(expiration)), - zap.Time("renewal_cutoff", cutoff)) - return true + // if a renewal time has been selected, start with that + if !selectedTime.IsZero() { + // ARI spec recommends an algorithm that renews after the randomly-selected + // time OR just before it if the next waking time would be after it; this + // cutoff can actually be before the start of the renewal window, but the spec + // author says that's OK: https://github.com/aarongable/draft-acme-ari/issues/71 + cutoff := ari.SelectedTime.Add(-cfg.certCache.options.RenewCheckInterval) + if time.Now().After(cutoff) { + logger.Info("certificate needs renewal based on ARI window", + zap.Time("selected_time", selectedTime), + zap.Time("renewal_cutoff", cutoff)) + return true + } + + // according to ARI, we are not ready to renew; however, we do not rely solely on + // ARI calculations... what if there is a bug in our implementation, or in the + // server's, or the stored metadata? for redundancy, give credence to the expiration + // date; ignore ARI if we are past a "dangerously close" limit, to avoid any + // possibility of a bug in ARI compromising a site's uptime: we should always always + // always give heed to actual validity period + if currentlyInRenewalWindow(leaf.NotBefore, expiration, 1.0/20.0) { + logger.Warn("certificate is in emergency renewal window; superceding ARI", + zap.Duration("remaining", time.Until(expiration)), + zap.Time("renewal_cutoff", cutoff)) + return true + } } - } // the normal check, in the absence of ARI, is to determine if we're near enough (or past) diff --git a/config.go b/config.go index 37529798..4910fac3 100644 --- a/config.go +++ b/config.go @@ -149,6 +149,10 @@ type Config struct { // EXPERIMENTAL: Subject to change or removal. SubjectTransformer func(ctx context.Context, domain string) string + // Disables both ARI fetching and the use of ARI for renewal decisions. + // TEMPORARY: Will likely be removed in the future. + DisableARI bool + // Set a logger to enable logging. If not set, // a default logger will be created. Logger *zap.Logger @@ -451,7 +455,7 @@ func (cfg *Config) manageOne(ctx context.Context, domainName string, async bool) // ensure ARI is updated before we check whether the cert needs renewing // (we ignore the second return value because we already check if needs renewing anyway) - if cert.ari.NeedsRefresh() { + if !cfg.DisableARI && cert.ari.NeedsRefresh() { cert, _, err = cfg.updateARI(ctx, cert, cfg.Logger) if err != nil { 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 // if we're renewing with the same ACME CA as before, have the ACME // client tell the server we are replacing a certificate (but doing // this on the wrong CA, or when the CA doesn't recognize the certID, - // can fail the order) - if acmeData, err := certRes.getACMEData(); err == nil && acmeData.CA != "" { - if acmeIss, ok := issuer.(*ACMEIssuer); ok { - if acmeIss.CA == acmeData.CA { - ctx = context.WithValue(ctx, ctxKeyARIReplaces, leaf) + // can fail the order) -- TODO: change this check to whether we're using the same ACME account, not CA + if !cfg.DisableARI { + if acmeData, err := certRes.getACMEData(); err == nil && acmeData.CA != "" { + if acmeIss, ok := issuer.(*ACMEIssuer); ok { + if acmeIss.CA == acmeData.CA { + ctx = context.WithValue(ctx, ctxKeyARIReplaces, leaf) + } } } } @@ -1246,8 +1252,10 @@ func (cfg *Config) managedCertNeedsRenewal(certRes CertificateResource, emitLogs return 0, nil, true } var ari acme.RenewalInfo - if ariPtr, err := certRes.getARI(); err == nil && ariPtr != nil { - ari = *ariPtr + if !cfg.DisableARI { + if ariPtr, err := certRes.getARI(); err == nil && ariPtr != nil { + ari = *ariPtr + } } remaining := time.Until(expiresAt(certChain[0])) return remaining, certChain[0], cfg.certNeedsRenewal(certChain[0], ari, emitLogs) diff --git a/handshake.go b/handshake.go index fd576995..1ff0ce27 100644 --- a/handshake.go +++ b/handshake.go @@ -582,7 +582,7 @@ func (cfg *Config) handshakeMaintenance(ctx context.Context, hello *tls.ClientHe } // Check ARI status - if cert.ari.NeedsRefresh() { + if !cfg.DisableARI && cert.ari.NeedsRefresh() { // we ignore the second return value here because we go on to check renewal status below regardless var err error cert, _, err = cfg.updateARI(ctx, cert, logger) diff --git a/maintain.go b/maintain.go index 477289c8..28ded5d8 100644 --- a/maintain.go +++ b/maintain.go @@ -136,7 +136,7 @@ func (certCache *Cache) RenewManagedCertificates(ctx context.Context) error { } // ACME-specific: see if if ACME Renewal Info (ARI) window needs refreshing - if cert.ari.NeedsRefresh() { + if !cfg.DisableARI && cert.ari.NeedsRefresh() { configs[cert.hash] = cfg ariQueue = append(ariQueue, cert) }