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

lib: add cert and CRL distribution point extension support. #136

Merged
merged 2 commits into from
Aug 22, 2023

Conversation

cpu
Copy link
Member

@cpu cpu commented Aug 16, 2023

Description

This branch adds support for certificate CRL distribution points extensions, and the corresponding CRL issuing distribution point extension.

Initially I opened this PR with just the former, but the latter is so closely related it felt more appropriate to make one branch with two commits.

Test coverage is better for the certificate extension compared to the CRL extension, the upstream library support for the latter is quite thin.

I haven't updated the webpki CRL revocation checking tests to demonstrate the new extensions at this time, because it is only available in an alpha release. I can either do this as a follow up once the v/0.102.0 release is cut (likely in O(weeks)) or we could update the dev-dependency to the alpha now and add coverage in this branch. LMK what you would prefer :-)

lib: add cert CRL distribution points ext. support.

This commit extends rcgen to allow generating certificates that contain an RFC 5280 certificate revocation list (CRL) distribution points extension. This is a useful mechanism for helping ensure CRL coverage when performing revocation checks, and is newly supported by rustls/webpki. See this upstream webpki issue and RFC 5280 §4.2.1.13 for more background.

Using the new crl_distribution_points field of the CertificateParams struct it's possible to encode one or more distribution points specifying URI general names where up-to-date CRL information for the certificate can be found.

Similar to existing rcgen CRL generation, the support for this extension is not extensive, but instead tailored towards usage in the web PKI with a RFC 5280 profile.

Notably this means:

  • There's no support for specifying the reasons flag - RFC 5280 "RECOMMENDS against segmenting CRLs by reason code".
  • There's no support for specifying a cRLIssuer in the DP - this is specific to indirect CRLs, and neither rcgen's CRL generation code or webpki's parsing/validation support these.
  • There's no support for specifying a nameRelativeToCrlIssuer in the DP name instead of a sequence of general names for similar reasons as above: 5280 says: "Conforming CAs SHOULD NOT use nameRelativeToCRLIssuer to specify distribution point names."
  • There's no support for specifying general names of type other than URI within a DP name's full name. Other name types either don't make sense in the context of this extension, or are rarely useful in practice (e.g. directory name).

Test coverage is mixed based on the support of the relevant third party libraries. OpenSSL (openssl-rs) and x509-parser both parse this extension well, and so the openssl.rs and generic.rs test coverage is the most extensive. Webpki (v/0.102.0-alpha.0) recognizes this extension for use during revocation checking, but doesn't expose it externally so a simple parse test is added. Botan's rust bindings do not recognize the extension or offer a way to pull out arbitrary extensions, so no test coverage is added there.

lib: add CRL issuing distribution point ext. support.

This commit extends rcgen to allow generating certificate revocation lists (CRLs) that contain an RFC 5280 CRL issuing distribution point extension. This is a useful mechanism for helping ensure CRL coverage when performing revocation checks, and is newly supported by rustls/webpki. See this upstream webpki issue and RFC 5280 §5.2.5 for more background.

Using the new optional issuing_distribution_point field of the CertificateRevocationListParams struct it's possible to encode a
issuing distribution point specifying URI general names where up-to-date CRL information for the CRL can be found.

Similar to existing rcgen CRL generation, the support for this extension is not extensive, but instead tailored towards usage in the web PKI with a RFC 5280 profile.

Notably this means:

  • There's no support for specifying the indirectCRL bool - neither rcgen's existing CRL generation code or webpki's parsing/validation supports these.
  • There's no support for specifying the onlySomeReasons field - RFC 5280 "RECOMMENDS against segmenting CRLs by reason code".
  • There's no support for specifying a onlyContainsAttributeCerts bool - RFC 5280 says "Conforming CRLs issuers MUST set the onlyContainsAttributeCerts boolean to FALSE." and the DER encoding of 'false' requires eliding the value.
  • There's no support for specifying a 'nameRelativeToCrlIssuer' in the DP name instead of a sequence of general names for similar reasons as above: 5280 says: "Conforming CAs SHOULD NOT use nameRelativeToCRLIssuer to specify distribution point names."
  • There's no support for specifying general names of type other than URI within a DP name's full name. Other name types either don't make sense in the context of this extension, or are rarely useful in practice (e.g. directory name).

Compared to test coverage of the certificate CRL distribution points extension this commit can't offer too much. OpenSSL (openssl-rs) doesn't expose arbitrary CRL extensions, or the issuing distribution point. The x509-parser crate can pull out the extension, but doesn't decompose the value (I may attempt to land code for this upstream in the future, stay tuned). Webpki (v/0.102.0-alpha.0) recognizes this extension for use during revocation checking, but doesn't expose it externally. Botan's rust bindings do not recognize the extension or offer a way to pull out arbitrary extensions, so no test coverage is added there.

tests/generic.rs Outdated Show resolved Hide resolved
@cpu cpu force-pushed the cpu-cert-crl-distrib-points branch from 9e5f117 to 96bf5f8 Compare August 17, 2023 18:01
@cpu cpu changed the title lib: add cert CRL distribution points ext. support. lib: add cert and CRL distribution point extension support. Aug 17, 2023
@cpu
Copy link
Member Author

cpu commented Aug 17, 2023

cpu force-pushed the cpu-cert-crl-distrib-points branch from 9e5f117 to 96bf5f8

  • I updated the cert distribution points extension serialization to pull out the code for writing a sequence of URI general names since this can be shared between the cert + CRL extensions.
  • I fixed the x509-parser test coverage for the cert CRL distribution points extension after realizing x509-parser does have support for fully parsing this extension (it is however missing the same for the CRL extension - I will follow-up on this upstream).
  • I rolled the CRL issuing distribution point support into this branch as a 2nd commit instead of making a follow-up PR. I think the changes are so closely related it will be easier to review it all in one go.

I also called out one question in the updated PR desc about whether it makes sense to pull in the alpha webpki release that has support for these extensions in order to get better test coverage.

Thanks!

src/lib.rs Outdated Show resolved Hide resolved
tests/generic.rs Outdated Show resolved Hide resolved
tests/generic.rs Outdated Show resolved Hide resolved
This commit extends rcgen to allow generating certificates that contain
an RFC 5280 certificate revocation list (CRL) distribution points
extension. This is a useful mechanism for helping ensure CRL coverage
when performing revocation checks, and is newly supported by
rustls/webpki. See this upstream webpki issue[0] and RFC 5280
§4.2.1.13[1] for more background.

Using the new `crl_distribution_points` field of the `CertificateParams`
struct it's possible to encode one or more distribution points
specifying URI general names where up-to-date CRL information for the
certificate can be found.

Similar to existing rcgen CRL generation, the support for this extension
is not extensive, but instead tailored towards usage in the web PKI with
a RFC 5280 profile.

Notably this means:
* There's no support for specifying the 'reasons' flag - RFC 5280
  "RECOMMENDS against segmenting CRLs by reason code".
* There's no support for specifying a 'cRLIssuer' in the DP - this is
  specific to indirect CRLs, and neither rcgen's CRL generation code or
  webpki's parsing/validation support these.
* There's no support for specifying a 'nameRelativeToCrlIssuer' in the
  DP name instead of a sequence of general names for similar reasons as
  above: 5280 says: "Conforming CAs SHOULD NOT use
  nameRelativeToCRLIssuer to specify distribution point names."
* There's no support for specifying general names of type other than URI
  within a DP name's full name. Other name types either don't make sense
  in the context of this extension, or are rarely useful in practice
  (e.g. directory name).

Test coverage is mixed based on the support of the relevant third party
libraries. OpenSSL (openssl-rs) and x509-parser both parse this
extension well, and so the `openssl.rs` and `generic.rs` test coverage
is the most extensive. Webpki (v/0.102.0-alpha.0) recognizes this
extension for use during revocation checking, but doesn't expose it
externally so a simple parse test is added. Botan's rust bindings do not
recognize the extension or offer a way to pull out arbitrary extensions,
so no test coverage is added there.

[0] rustls/webpki#121
[1] https://www.rfc-editor.org/rfc/rfc5280#section-4.2.1.13
This commit extends rcgen to allow generating certificate revocation
lists (CRLs) that contain an RFC 5280 CRL issuing distribution point
extension. This is a useful mechanism for helping ensure CRL coverage
when performing revocation checks, and is newly supported by
rustls/webpki. See this upstream webpki issue[0] and RFC 5280
§5.2.5[1] for more background.

Using the new optional `issuing_distribution_point` field of the
`CertificateRevocationListParams` struct it's possible to encode a
issuing distribution point specifying URI general names where up-to-date
CRL information for the CRL can be found.

Similar to existing rcgen CRL generation, the support for this extension
is not extensive, but instead tailored towards usage in the web PKI with
a RFC 5280 profile.

Notably this means:
* There's no support for specifying the `indirectCRL` bool - neither
  rcgen's existing CRL generation code or webpki's parsing/validation
  supports these.
* There's no support for specifying the `onlySomeReasons` field - RFC
  5280 "RECOMMENDS against segmenting CRLs by reason code".
* There's no support for specifying a `onlyContainsAttributeCerts` bool
  - RFC 5280 says "Conforming CRLs issuers MUST set the
  onlyContainsAttributeCerts boolean to FALSE." and the DER encoding of
  'false' requires eliding the value.
* There's no support for specifying a 'nameRelativeToCrlIssuer' in the
  DP name instead of a sequence of general names for similar reasons as
  above: 5280 says: "Conforming CAs SHOULD NOT use
  nameRelativeToCRLIssuer to specify distribution point names."
* There's no support for specifying general names of type other than URI
  within a DP name's full name. Other name types either don't make sense
  in the context of this extension, or are rarely useful in practice
  (e.g. directory name).

Compared to test coverage of the certificate CRL distribution points
extension this commit can't offer too much. OpenSSL (openssl-rs) doesn't
expose arbitrary CRL extensions, or the issuing distribution point. The
`x509-parser` crate can pull out the extension, but doesn't decompose
the value (I may attempt to land code for this upstream in the future,
stay tuned). Webpki (v/0.102.0-alpha.0) recognizes this extension for
use during revocation checking, but doesn't expose it externally.
Botan's rust bindings do not recognize the extension or offer a way to
pull out arbitrary extensions, so no test coverage is added there.

[0] rustls/webpki#121
[1] https://www.rfc-editor.org/rfc/rfc5280#section-5.2.5
@cpu cpu force-pushed the cpu-cert-crl-distrib-points branch from 96bf5f8 to c422302 Compare August 21, 2023 14:36
@est31 est31 merged commit 041d4cb into rustls:master Aug 22, 2023
6 checks passed
@est31
Copy link
Member

est31 commented Aug 22, 2023

I've merged this. I will work on a PR to split up rcgen into multiple files soon.

@cpu cpu deleted the cpu-cert-crl-distrib-points branch August 24, 2023 13:31
@cpu
Copy link
Member Author

cpu commented Aug 24, 2023

Ty!

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