Skip to content

Only indicate the replaced cert if the ARI suggested window is not in the past #364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
oliverpool wants to merge 1 commit into from
Closed

Only indicate the replaced cert if the ARI suggested window is not in the past #364

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,7 @@ func (cfg *Config) renewCert(ctx context.Context, name string, force, interactiv
}

// check if renew is still needed - might have been renewed while waiting for lock
timeLeft, leaf, needsRenew := cfg.managedCertNeedsRenewal(certRes, false)
timeLeft, replacesCert, needsRenew := cfg.managedCertNeedsRenewal(certRes, false)
if !needsRenew {
if force {
log.Info("certificate does not need to be renewed, but renewal is being forced",
Expand Down Expand Up @@ -939,11 +939,11 @@ func (cfg *Config) renewCert(ctx context.Context, name string, force, interactiv
// 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) -- TODO: change this check to whether we're using the same ACME account, not CA
if !cfg.DisableARI {
if replacesCert != nil {
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)
ctx = context.WithValue(ctx, ctxKeyARIReplaces, replacesCert)
}
}
}
Expand Down Expand Up @@ -1311,21 +1311,26 @@ func (cfg *Config) lockKey(op, domainName string) string {

// managedCertNeedsRenewal returns true if certRes is expiring soon or already expired,
// or if the process of decoding the cert and checking its expiration returned an error.
// If there wasn't an error, the leaf cert is also returned, so it can be reused if
// necessary, since we are parsing the PEM bundle anyway.
// The leaf cert is only returned if the ARI suggested window is not in the past.
func (cfg *Config) managedCertNeedsRenewal(certRes CertificateResource, emitLogs bool) (time.Duration, *x509.Certificate, bool) {
certChain, err := parseCertsFromPEMBundle(certRes.CertificatePEM)
if err != nil || len(certChain) == 0 {
return 0, nil, true
}
var replacesCert *x509.Certificate
var ari acme.RenewalInfo
if !cfg.DisableARI {
if ariPtr, err := certRes.getARI(); err == nil && ariPtr != nil {
ari = *ariPtr

// only indicate replacement if the ARI suggested window is not in the past.
if time.Now().Before(ari.SuggestedWindow.End) {
replacesCert = certChain[0]
}
}
}
remaining := time.Until(expiresAt(certChain[0]))
return remaining, certChain[0], cfg.certNeedsRenewal(certChain[0], ari, emitLogs)
return remaining, replacesCert, cfg.certNeedsRenewal(certChain[0], ari, emitLogs)
}

func (cfg *Config) emit(ctx context.Context, eventName string, data map[string]any) error {
Expand Down
Loading