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

Enable periodic, automatic rebuilding of CRLs #16762

Merged
merged 9 commits into from Aug 23, 2022

Conversation

cipherboy
Copy link
Contributor

@cipherboy cipherboy commented Aug 16, 2022

This enables automatic, periodic rebuilding of CRLs. When periodic rebuilding of CRLs is enabled, we'll decline to rebuild the CRL on every revocation and instead wait until the next update (or some precipitating event -- revocation of an issuer, manual rebuild, &c). This ensures that there's not a lot of load on the CRL building nodes (also allowing "batch" revocations as a side effect) between expected CRL updates. This also fixes a long-standing bug, wherein the CRL would expire but if an operator does not issue a new revocation (or does not manually trigger a rebuild), we'd let the CRL lapse.


@cipherboy cipherboy added this to the 1.12.0-rc1 milestone Aug 16, 2022
@stevendpclark stevendpclark force-pushed the stevendpclark/pki-ocsp-responder branch from ad2e82e to 2296498 Compare August 19, 2022 19:33
@cipherboy cipherboy force-pushed the cipherboy-auto-rebuild-crls branch 3 times, most recently from b35bc20 to 2d2d398 Compare August 22, 2022 17:11
Base automatically changed from stevendpclark/pki-ocsp-responder to main August 22, 2022 18:06
When enabled, periodic rebuilding of CRLs will improve PKI mounts in two
way:

 1. Reduced load during periods of high (new) revocations, as the CRL
    isn't rebuilt after each revocation but instead on a fixed schedule.
 2. Ensuring the CRL is never stale as long as the cluster remains up,
    by checking for next CRL expiry and regenerating CRLs before that
    happens. This may increase cluster load when operators have large
    CRLs that they'd prefer to let go stale, rather than regenerating
    fresh copies.

In particular, we set a grace period before expiration of CRLs where,
when the periodic function triggers (about once a minute), we check
upcoming CRL expirations and check if we need to rebuild the CRLs.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
When testing backends that use the periodic func, and specifically,
testing the behavior of that periodic func, waiting for the usual 1m
interval can lead to excessively long test execution. By switching to a
shorter period--strictly for testing--we can make these tests execute
faster.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
@cipherboy cipherboy marked this pull request as ready for review August 22, 2022 18:12
@cipherboy cipherboy requested a review from a team August 22, 2022 18:13
Copy link
Contributor

@stevendpclark stevendpclark left a comment

Choose a reason for hiding this comment

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

Overall 👍 a few things or possible misunderstandings on my side.

builtin/logical/pki/crl_util.go Outdated Show resolved Hide resolved
builtin/logical/pki/crl_util.go Show resolved Hide resolved
website/content/api-docs/secret/pki.mdx Outdated Show resolved Hide resolved
website/content/api-docs/secret/pki.mdx Show resolved Hide resolved
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Copy link
Contributor

@kitography kitography left a comment

Choose a reason for hiding this comment

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

This looks great! I have one tiny nit, and that is all

website/content/api-docs/secret/pki.mdx Outdated Show resolved Hide resolved
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants