Skip to content
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.

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

crl-updater: split temporal/explicit sharding by serial #7990

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jsha
Copy link
Contributor

@jsha jsha commented Jan 30, 2025

When we turn on explicit sharding, we'll change the CA serial prefix, so we can know that all issuance from the new prefixes uses explicit sharding, and all issuance from the old prefixes uses temporal sharding. This lets us avoid putting a revoked cert in two different CRL shards (the temporal one and the explicit one).

To achieve this, the crl-updater gets a list of temporally sharded serial prefixes. When it queries the certificateStatus table by date (GetRevokedCerts), it will filter out explicitly sharded certificates: those that don't have their prefix on the list.

Part of #7094

Warning

The integration test case TestRevocation currently fails because the 6e-prefixed certificates don't show up on any CRL. That will be addressed in #7985 and tests will pass. However, it's okay to review this PR as it stands.

@jsha jsha marked this pull request as ready for review January 30, 2025 19:28
@jsha jsha requested a review from a team as a code owner January 30, 2025 19:28
@jsha jsha requested a review from aarongable January 30, 2025 19:28
Copy link
Contributor

@jsha, this PR appears to contain configuration and/or SQL schema changes. Please ensure that a corresponding deployment ticket has been filed with the new values.

@jsha jsha force-pushed the serial-prefix-shard-assignment branch from c116dae to 6147101 Compare January 30, 2025 23:26
Copy link
Contributor

@aarongable aarongable left a comment

Choose a reason for hiding this comment

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

Code and docs LGTM, just some comments on the tests

test/integration/crl_test.go Outdated Show resolved Hide resolved
test/integration/crl_test.go Outdated Show resolved Hide resolved
test/integration/crl_test.go Outdated Show resolved Hide resolved
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