Let PKI tidy associate revoked certs with their issuers #16871
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cipherboy
force-pushed
the
cipherboy-tidy-associate-revoked-certs
branch
from
678cabe to
4460d34
Compare
cipherboy
force-pushed
the
cipherboy-tidy-associate-revoked-certs
branch
from
4460d34 to
ff73fa1
Compare
cipherboy
changed the title
stevendpclark
approved these changes
Aug 26, 2022
This refactors the tidy go routine into two separate helpers, making it clear where the boundaries of each are: variables are passed into these method and concerns are separated. As more operations are rolled into tidy, we can continue adding more helpers as appropriate. Additionally, as we move to make auto-tidy occur, we can use these as points to hook into periodic tidying. Signed-off-by: Alexander Scheel <[email protected]>
This allows us to validate whether or not a revInfo entry contains a presently valid issuer, from the existing mapping. Coupled with the changeset to identify the issuer on revocation, we can begin adding capabilities to tidy to update this association, decreasing CRL build time and increasing the performance of OCSP. Signed-off-by: Alexander Scheel <[email protected]>
Revocation needs to gracefully handle using the old legacy cert bundle, so fetching issuers (and parsing them) needs to be done slightly differently than other places. Refactor this from revokeCert into a common helper that can be used by tidy. Signed-off-by: Alexander Scheel <[email protected]>
When revoking a certificate, we need to associate the issuer that signed its certificate back to the revInfo entry. Historically this was performed during CRL building (and still remains so), but when running without CRL building and with only OCSP, performance will degrade as the issuer needs to be found each time. Instead, allow the tidy operation to take over this role, allowing us to increase the performance of OCSP and CRL in this scenario, by decoupling issuer identification from CRL building in the ideal case. Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
Signed-off-by: Alexander Scheel <[email protected]>
Finish adding metrics, status messages about new tidy operation. Signed-off-by: Alexander Scheel <[email protected]>
cipherboy
force-pushed
the
cipherboy-tidy-associate-revoked-certs
branch
from
7d50d0c to
78d9b3d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This updates PKI's tidy operation to optionally associate certificates with their issuers.
As noted in a previous PR about OCSP, this allows OCSP to behave faster on existing revocation entries (without this mapping) if CRL building is disabled.