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

Add CRL capabilities to issuance package #7300

Merged
merged 9 commits into from
Feb 13, 2024
Merged

Add CRL capabilities to issuance package #7300

merged 9 commits into from
Feb 13, 2024

Conversation

aarongable
Copy link
Contributor

@aarongable aarongable commented Feb 2, 2024

Move the CRL issuance logic -- building an x509.RevocationList template, populating it with correctly-built extensions, linting it, and actually signing it -- out of the //ca package and into the //issuance package. This means that the CA's CRL code no longer needs to be able to reach inside the issuance package to access its issuers and certificates (and those fields will be able to be made private after the same is done for OCSP issuance).

Additionally, improve the configuration of CRL issuance, create additional checks on CRL's ThisUpdate and NextUpdate fields, and make it possible for a CRL to contain two IssuingDistributionPoint URIs so that we can migrate to shorter addresses.

Fixes #7159
Part of #7296
Part of #7294
Part of #7094
Part of #7100

IN-10045 tracks the corresponding production changes.

DO NOT MERGE before #7285
DO NOT MERGE before #7299

@aarongable aarongable requested a review from a team as a code owner February 2, 2024 02:28
@aarongable aarongable requested review from jsha and removed request for a team February 2, 2024 02:28
Copy link
Contributor

github-actions bot commented Feb 2, 2024

@aarongable, this PR appears to contain configuration changes. Please ensure that a corresponding deployment ticket has been filed with the new configuration values.

Copy link
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.

Generally looks great! Some minor proposals.

boulderProfile.Lints,
time.Hour,
issuance.CRLProfileConfig{
ValidityInterval: config.Duration{Duration: 216 * time.Hour},
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it was @beautifulentropy who provided a nice distinction:

  • period: a span of time, unanchored from any particular instance (i.e. a duration)
  • interval: a span of time anchored to a specific beginning and end time

So ValidityPeriod would be nice here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, except that "Validity Period" is defined by RFC 5280 as being a thing that applies to certificates only (i.e. between NotBefore and NotAfter), while "Validity Interval" is the term used by the BRs to speak about CRL and OCSP responses (i.e. between ThisUpdate and NextUpdate): https://github.com/cabforum/servercert/blob/main/docs/BR.md#4910-on-line-revocation-checking-requirements

issuance/crl.go Outdated Show resolved Hide resolved
issuance/crl.go Outdated Show resolved Hide resolved
@jsha jsha requested a review from a team February 6, 2024 01:29
@aarongable aarongable requested a review from jsha February 7, 2024 01:00
Base automatically changed from improve-crl-idp-lint to main February 8, 2024 23:14
jsha
jsha previously approved these changes Feb 8, 2024
@aarongable aarongable requested a review from a team February 8, 2024 23:55
@aarongable aarongable dismissed stale reviews from beautifulentropy and jsha via 89861bd February 9, 2024 20:25
issuance/issuer.go Outdated Show resolved Hide resolved
@aarongable aarongable merged commit ad699af into main Feb 13, 2024
19 checks passed
@aarongable aarongable deleted the issuance-crls branch February 13, 2024 17:13
maksimsavrilov pushed a commit to plesk/boulder that referenced this pull request Feb 14, 2024
Move the CRL issuance logic -- building an x509.RevocationList template,
populating it with correctly-built extensions, linting it, and actually
signing it -- out of the //ca package and into the //issuance package.
This means that the CA's CRL code no longer needs to be able to reach
inside the issuance package to access its issuers and certificates (and
those fields will be able to be made private after the same is done for
OCSP issuance).

Additionally, improve the configuration of CRL issuance, create
additional checks on CRL's ThisUpdate and NextUpdate fields, and make it
possible for a CRL to contain two IssuingDistributionPoint URIs so that
we can migrate to shorter addresses.

IN-10045 tracks the corresponding production changes.

Fixes letsencrypt#7159
Part of letsencrypt#7296
Part of letsencrypt#7294
Part of letsencrypt#7094
Part of letsencrypt#7100
aarongable added a commit that referenced this pull request Apr 10, 2024
aarongable added a commit that referenced this pull request Apr 12, 2024
vbaranovskiy-plesk pushed a commit to plesk/boulder that referenced this pull request May 30, 2024
vbaranovskiy-plesk pushed a commit to plesk/boulder that referenced this pull request May 30, 2024
AlinaADmi pushed a commit to plesk/boulder that referenced this pull request Jul 29, 2024
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.

Build CRL profiles into //issuance package
4 participants