Skip to content

crl-updater: Fix lookback period for GetRevokedCertsByShard#8071

Closed
aarongable wants to merge 1 commit intomainfrom
fix-crl-lookback
Closed

crl-updater: Fix lookback period for GetRevokedCertsByShard#8071
aarongable wants to merge 1 commit intomainfrom
fix-crl-lookback

Conversation

@aarongable
Copy link
Copy Markdown
Contributor

@aarongable aarongable commented Mar 18, 2025

The purpose of the lookback period is so that entries are not removed from a CRL until well after they have expired, to ensure we comply with RFC 5280, Section 3.3, which says "An entry MUST NOT be removed from the CRL until it appears on one regularly scheduled CRL issued beyond the revoked certificate's validity period."

Setting the expiresAfter time to be in the future means that certificates will be omitted from the query before they expire, rather than after. Change the sign of the lookbackPeriod so that the expiresAfter timestamp is in the past.

Copy link
Copy Markdown
Contributor

@jsha jsha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Syntactically,-cu.lookbackPeriod also works, which is what we use elsewhere. But okay to merge as-is. I'll work on a unittest update.

@jsha
Copy link
Copy Markdown
Contributor

jsha commented Mar 18, 2025

Followup: https://github.com/letsencrypt/boulder/compare/fix-crl-updater-lookback?expand=1

@aarongable aarongable marked this pull request as ready for review March 18, 2025 17:13
@aarongable aarongable requested a review from a team as a code owner March 18, 2025 17:13
@aarongable
Copy link
Copy Markdown
Contributor Author

Obviated by the change above.

@aarongable aarongable closed this Mar 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants