return logical and compliant ARI windows for expiring certs (#484)

While testing ARI functionality with extremely short lived certs, Pebble
returned illogical data - such as the windowEnd appearing in the future
for already expired certs, or after the cert's notAfter.

This PR adjusts `RenewalInfoSimple` to do the following:

* if the cert has expired, just return `RenewalInfoImmediate`
* if the cert will expire before the `windowEnd`, set the `windowEnd` to
the cert's `notAfter`
* cleanup: RFC requires the `windowStart` to be before `windowEnd`, so
remove a second from the `windowStart` if needed.

I also made the `RenewalInfoImmediate` window change from `-60:-30`
minutes to `-60:-59.59` minutes; the sole reason for this change is that
it is visually easier to see the change (and not need to do mental math)
and recognize this is a `RenewalInfoImmediate` from logs.
This commit is contained in:
Jonathan Vanasco 2025-02-21 12:46:22 -05:00 committed by GitHub
parent bbe7775c27
commit bc21177aca
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 24 additions and 5 deletions

View File

@ -260,14 +260,33 @@ type RenewalInfo struct {
// using a very simple renewal calculation: calculate a point 2/3rds of the way
// through the validity period, then give a 2-day window around that. Both the
// `issued` and `expires` timestamps are expected to be UTC.
func RenewalInfoSimple(issued time.Time, expires time.Time) *RenewalInfo {
func RenewalInfoSimple(issued time.Time, expires time.Time, now time.Time) *RenewalInfo {
validity := expires.Add(time.Second).Sub(issued)
renewalOffset := validity / time.Duration(3)
idealRenewal := expires.Add(-renewalOffset)
windowStart := idealRenewal.Add(-24 * time.Hour)
windowEnd := idealRenewal.Add(24 * time.Hour)
// Ensure RenewalWindow is not after the expiry
if windowEnd.After(expires) {
windowEnd = expires
}
// Ensure correct start for future issueds
if windowStart.Before(issued) {
windowStart = issued
}
// draft-ietf-acme-ari states:
// A RenewalInfo object in which the end timestamp
// equals or precedes the start timestamp is invalid.
if !windowStart.Before(windowEnd) {
windowStart = windowStart.Add(-1 * time.Second)
}
return &RenewalInfo{
SuggestedWindow: SuggestedWindow{
Start: idealRenewal.Add(-24 * time.Hour),
End: idealRenewal.Add(24 * time.Hour),
Start: windowStart,
End: windowEnd,
},
}
}
@ -282,7 +301,7 @@ func RenewalInfoImmediate(now time.Time) *RenewalInfo {
return &RenewalInfo{
SuggestedWindow: SuggestedWindow{
Start: oneHourAgo,
End: oneHourAgo.Add(time.Minute * 30),
End: oneHourAgo.Add(1 * time.Second),
},
}
}

View File

@ -1914,7 +1914,7 @@ func (wfe *WebFrontEndImpl) determineARIWindow(id *core.CertID) (*core.RenewalIn
return nil, errors.New("failed to retrieve existing certificate serial")
}
return core.RenewalInfoSimple(cert.Cert.NotBefore, cert.Cert.NotAfter), nil
return core.RenewalInfoSimple(cert.Cert.NotBefore, cert.Cert.NotAfter, time.Now().In(time.UTC)), nil
}
// parseCertID parses a unique identifier (certID) as specified in