rustls / rcgen

Generate X.509 certificates
Other
323 stars 100 forks source link

Expand CSR generation support #285

Open lvkv opened 1 month ago

lvkv commented 1 month ago

Hi! I'm looking for an alternative to rust-openssl's X509ReqBuilder for building and signing certificate signing requests. Ideally an alternative that supports:

I've had a great experience with other Rustls projects, so I checked here first and found rcgen. Of course, I understand if a CSR builder is out of scope here.

Motivation: I'm implementing a subset of the MS-WCCE certificate enrollment protocol, which involves crafting CSRs with a many interesting extensions and attributes. I'm also open to suggestions for other crates that might be able to help out with this!

djc commented 1 month ago

@lvkv we already have support for building CSRs, which currently works by building from a CertificateParams. Rather than supporting a random collection of custom stuff, I think we should probably go the way of defining some API that fits with the MS-WCCE standard. Given the scope here, it probably makes sense to make an exhaustive list of the exact bits you need and come up with a tentative API design that doesn't just expose a collection of customizations (extensions, EKUs and attributes) but tries to define comprehensive support for what MS-WCCE needs.

lvkv commented 1 month ago

we already have support for building CSRs, which currently works by building from a CertificateParams.

Oh interesting! I didn't catch that the way to create a CertificateSigningRequest is to first create a CertificateParams and then to serialize_request. FYI this feels non-obvious, especially given that CertificateSigningRequestParams also exists, but isn't used for that purpose.

I tried creating a dummy CSR using this method, but ran into an UnsupportedInCsr error because I tried to add key usages to the request. Looks like EKU support was recently merged, so I assume this is all a work in progress still?

https://github.com/rustls/rcgen/blob/746da00ef695873b322823ae9ff7a1a3151578ae/rcgen/src/certificate.rs#L545-L553

Given the scope here, it probably makes sense to make an exhaustive list of the exact bits you need and come up with a tentative API design that doesn't just expose a collection of customizations (extensions, EKUs and attributes) but tries to define comprehensive support for what MS-WCCE needs.

Agreed that thinking this out is a good idea. I'm also very much not against first-class support for these bits (it'd be quite nice, actually!). Comprehensive support might be a bit out of reach, given that the spec is a >250 page PDF, but I think it should be more than possible to implement enough of a skeleton so that people can easily add in whatever bits they need when they need to.

I think it'd be a good idea to figure out the boundaries of what rcgen would like to add support for. For example, adding MS-WCCE-specific bits for traditional PKCS#10 CSR generation feels like an achievable goal. However, MS-WCCE has other CSR types that encapsulate PKCS#10 CSRs with additional bundled information. E.g. 2.2.2.7 Certificate Request Attributes gives a handy list of all the different CSR flavors (truncated with added emphasis). The latter two bullets are what I'm curious to hear your thoughts on:

  • ... requests based on the PKCS#10 message format...

  • ... requests based on the CMS format... inner PKCS#10 certificate...

  • ... requests based on the CMC format... inner PKCS#10 certificate...

Personally what I'd like to support for is:

Lots of text! Let me know what you think. As always, I'm more than happy to work with you guys to create something useful!

lvkv commented 1 month ago

The big picture motivation is to use AD CS as a CA on Linux. Unfortunately, that means learning how to speak the CSR equivalent of parseltongue. On the bright side, it's a great excuse to craft a super flexible CSR generation library... 🙂

djc commented 1 month ago

Oh interesting! I didn't catch that the way to create a CertificateSigningRequest is to first create a CertificateParams and then to serialize_request. FYI this feels non-obvious, especially given that CertificateSigningRequestParams also exists, but isn't used for that purpose.

We're open to improving how that API works, if you have suggestions!

I tried creating a dummy CSR using this method, but ran into an UnsupportedInCsr error because I tried to add key usages to the request. Looks like EKU support was recently merged, so I assume this is all a work in progress still?

It's possible -- it's been a while, so it might make sense to try it against main to see what works there.

E.g. 2.2.2.7 Certificate Request Attributes gives a handy list of all the different CSR flavors (truncated with added emphasis). The latter two bullets are what I'm curious to hear your thoughts on:

Right now I think what we do for CSRs is the PKCS#10 message format, and I think deviating from that would sort of need a separate justification.

  • MS specific EKUs. Specifically XCN_OID_ENROLLMENT_AGENT. This one isn't MS-WCCE specific—it's more related to the issuing CA. In short, this denotes a certificate that allows you to request certificates be issued on others' behalf ("enroll on behalf of") (fun, I know).
  • Custom EKUs that represent user-generated (where "user" = server administrator) application policies

Adding XCN_OID_ENROLLMENT_AGENT to KeyUsagePurpose seems low-hanging fruit, though if we also do custom EKUs we might want to just support that.

  • "Parameterized" protocol-specific certificate extensions, specifically szOID_CERTIFICATE_TEMPLATE. In this case "parameterized" means that the values of of some fields will vary based on what the user is requesting, so they must be supplied by the developer.
  • "Parameterized" protocol-specific attributes (in the PKCS#10 ATTRIBUTE) sense. Specifically szOID_RENEWAL_CERTIFICATE, which is basically just an ASN.1 SET containing a certificate you'd like to renew.

For these I think it's all about coming up with a reasonably type-safe API.

(Note that these are my personal opinions, the other maintainers might feel differently.)

lvkv commented 1 month ago

Right now I think what we do for CSRs is the PKCS#10 message format, and I think deviating from that would sort of need a separate justification.

I was thinking this too!

It's possible -- it's been a while, so it might make sense to try it against main to see what works there.

Yeah, using what's in main, it looks like EKUs are supported, but key usages aren't. How does the following sound as a high-level plan? These can more or less all be worked on in parallel:

cpu commented 1 month ago

:wave: I haven't gone deep on the MS-WCCE spec or the required PKCS#10 attribute changes, but here are some initial thoughts.

At one point I had ambitions of reworking the way extensions are handled (see https://github.com/rustls/rcgen/pull/164) and one goal of that rework was to help bring parity between what's supported in certs, what's supported in CSRs, and how custom extensions are handled. Maybe there are some ideas to revive from that stale branch that could help here?

In general I'm a little bit wary about extending the existing API with new features while the underlying handling is messy/inconsistent but I also don't want to block helpful improvements on a big refactor I can't commit to seeing through myself. Maybe this concern would fall away after the diffs start to come up for review?

Rather than supporting a random collection of custom stuff, I think we should probably go the way of defining some API that fits with the MS-WCCE standard.

I'm not sure I'm convinced about this. I think in general I'd prefer to see Rcgen expose a helpful/usable API for custom EKUs, certificate extensions, and CSR attributes but not MS-WCCE specific bits (perhaps with the exception of defining well known OID constants).

The MS-WCCE standard feels pretty niche to me, and like something the majority of rcgen users wouldn't benefit from. I think it would be a better end-state if someone with the bandwidth to support the MS-WCCE use-case could do that external to Rcgen using primitives it exposes. WDYT?

lvkv commented 1 month ago

I think it would be a better end-state if someone with the bandwidth to support the MS-WCCE use-case could do that external to Rcgen using primitives it exposes. WDYT?

Yeah, FWIW I'm totally fine with MS-WCCE ideas not getting first-class API support here 🙂 having the building blocks to express them myself is just as good if not better; there are undoubtedly many other weird standards put out by big tech companies, and choosing to support this one may set an exhausting precedent.

In general I'm a little bit wary about extending the existing API with new features while the underlying handling is messy/inconsistent but I also don't want to block helpful improvements on a big refactor I can't commit to seeing through myself. Maybe this concern would fall away after the diffs start to come up for review?

Yeah, how do you feel about this revised plan, then? I can take a stab at doing all 3 of these:

  1. Add missing support for key usages (e.g. digital encipherment) in CSRs, taking ideas and code, where applicable, from #164
  2. Add support for custom PKCS#10 CSR attributes, taking ideas from #164 (Evaluate)
  3. Do whatever larger refactors/rewrites are deemed necessary, breaking them up into smaller chunks where possible

Like most users, I need this yesterday 🙂 so I'm naturally inclined to prioritize expanding the API. That being said, I'm more than happy to help you guys out with making the implementation more sustainable for the long run. Net-zero spaghetti initiative.

cpu commented 1 month ago

Yeah, how do you feel about this revised plan, then?

This sounds great to me.

@est31 Do you have any reservations here?

taking ideas and code, where applicable, from https://github.com/rustls/rcgen/pull/164

I think we're on the same page but just to be super clear don't feel obligated to try and revive all of #164 or anything :-) There's been quite a bit of API changes since then and I think the scope was ambitious on its own. I trust your judgement on what might make sense to try and shim into your overall goal.

est31 commented 1 month ago

@est31 Do you have any reservations here?

sounds fine to me.

djc commented 1 month ago

I'm not sure I'm convinced about this. I think in general I'd prefer to see Rcgen expose a helpful/usable API for custom EKUs, certificate extensions, and CSR attributes but not MS-WCCE specific bits (perhaps with the exception of defining well known OID constants).

That's fair. I thought we might want to try making sure that people don't put together potentially dangerous combinations of custom EKUs, extensions and attributes, so I figured putting some guardrails in place might help here. But I do agree that this MS-WCCE stuff might be niche (it's hard to say how niche) so we don't want to put in place too much special stuff for it.