lib: support X509v3 Extended Key Usage in Certificate Signing Requests

[tindzk]

Purpose

When attempting to serialise a signing request that specifies extended_key_usages on CertificateParams, currently an UnsupportedInCsr error is returned.

However, RFC 2986 does not preclude the use of X509v3 Extended Key Usage in CSRs. As a reference, OpenSSL also supports this extension:

openssl req \
  -new \
  -addext "extendedKeyUsage = serverAuth,clientAuth" \
  -out test.csr

Changes

• Allow extended_key_usages in CSRs
• Serialise "X509v3 Extended Key Usage" in CSRs

Checklist

• Self-review has been completed
• Changes are complete and ready for review
• Changes are covered by existing or new tests
• OpenSSL lists "X509v3 Extended Key Usage" in the generated CSR
  Verified by executing openssl req -inform der -in /tmp/test.der -noout -text

[codecov]

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (62f796a) 80.30% compared to head (39157cf) 72.54%.

❗ Current head 39157cf differs from pull request most recent head edebf32. Consider uploading reports for the commit edebf32 to get more accurate results

Files	Patch %	Lines
src/lib.rs	98.36%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #157      +/-   ##
==========================================
- Coverage   80.30%   72.54%   -7.77%     
==========================================
  Files           9        7       -2     
  Lines        2407     1916     -491     
==========================================
- Hits         1933     1390     -543     
- Misses        474      526      +52     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cpu]

Hi there, thanks for the PR!

This falls under a similar umbrella as #122 & #150 As you discovered right now Rcgen doesn't emit all possible extensions from the cert into the CSR and vice versa.

My personal preference is to hold fixing this until after #156 and calling it out as a breaking change. I'm nervous there are existing users of rcgen that would not expect CSRs that were created with this code to have an EKU extension and it's load bearing from a security perspective.

Beyond fixing this as a breaking change I'd also prefer to see the general extension handling improved more holistically so we don't have to handle each extension specially. There are also some related improvements to consider like #155.

I have a work in progress branch that I hope to open as a PR shortly that tackles some of that. I'm thinking that we could hold your PR until we're ready for breaking changes and then either land your quick fix, reworking it into my next set of changes afterwards, or if my changes are already ready we can roll with those.

Does that make sense?

[tindzk]

@cpu Thanks for the explanation! Alternatively, we could introduce a Cargo feature flag requiring users to explicitly opt-in if they want EKU support. That way, existing users will not be affected by the changes here.

[iamjpotts]

I thought this extended key usage was working at one point in the past, because I remember having to take it out when I did an upgrade. If you can wait a day or two, I'll try to see if I'm imagining things, or if that is what happened - in case it changes how you treat semver for this.

[iamjpotts]

It was supported in version 0.10 and broken in version 0.11 of rcgen.

Here's the snippet:

pub fn gen_cert_for_server(ca: &Certificate, ip: IpAddr) -> ServerCertificate {
    let mut dn = DistinguishedName::new();
    dn.push(DnType::CountryName, "USA");
    dn.push(DnType::CommonName, "Auto-Generated Server");

    let mut params = CertificateParams::default();

    params.is_ca = IsCa::NoCa;
    params.alg = &rcgen::PKCS_ECDSA_P256_SHA256;
    params.distinguished_name = dn;
    params.subject_alt_names = vec![SanType::IpAddress(ip)];

    // Unsupported by rcgen 0.11.x; results in RcgenError::UnsupportedInCsr
    params.extended_key_usages = vec![ExtendedKeyUsagePurpose::ServerAuth];

With version 0.11, that last line has to be commented out.

[iamjpotts]

The commit that broke being able to set extended key usages on CSRs is 7cef31e

[cpu]

Alternatively, we could introduce a Cargo feature flag requiring users to explicitly opt-in if they want EKU support.

I think feature flags are too heavy weight for this. I would still prefer to fix the lot of extension related issues as part of a breaking API change.

[artsbentley]

The CLI tool im writing is currently dependent on this feature, is there any estimate when it will be merged? Is there any support I can provide to accelerate this process?

[djc]

Depending on whether @tindzk is still interested/available to push this forward, it could help to rebase this on the main branch and address any open issues identified in the feedback so far. Current main already has breaking changes, so it would be okay to add more to that.

[tindzk]

I have rebased the PR. Please let me know if any further changes are needed.

[djc]

I think merging this would be reasonable, but I would like it to be split into smaller commits:

• One for the assert change
• One for the typo
• One for the change to write_subject_alt_names()
• One for the extraction of write_extended_key_usages()
• One for the calling write_extended_key_usages() for the CSR case

This makes it much easier to review.

[cpu]

@tindzk Do you think you'll be up for addressing the feedback suggested by @djc?

[cpu]

This ended up getting rolled into #264 Thanks for your contribution.