rcgen icon indicating copy to clipboard operation
rcgen copied to clipboard

CSR with PublicKey only

Open Virviil opened this issue 1 year ago • 12 comments

Usecase:

  • Client generates keypair
  • Sends public key to server (in form of CSR, or CSR is generated on server itself)
  • Server signs him a client cert with CA existing only on server
  • Returns this cert back to client

Mb I'm missing something, but didn't find a way to do it. The only possible flow now is:

  • Server generates both private and public key for client (KeyPair::generate)
  • Signs a cert with existing CA
  • Returns both cert and private key to a client.

Virviil avatar Jun 30 '24 08:06 Virviil

If I understand correctly, a CSR must be signed by the subject's key, which requires the private key.

djc avatar Jun 30 '24 10:06 djc

@djc Yes, for sure.

Actually i've found the possible solution: it's possible to construct rcgen::CertificateSigningRequestParams with only params and public key, and then use it to sign a client cert by CertificateSigningRequestParams::signed_by

Maybe it's worth adding this to the examples.

Virviil avatar Jun 30 '24 10:06 Virviil

Hmmm I wonder whether we could make CertificateParams::signed_by take K: PublicKeyData instead? The private key is never used.

est31 avatar Jul 01 '24 01:07 est31

Huh, yeah -- I totally missed that, sorry.

djc avatar Jul 02 '24 07:07 djc

Let me start by sketching out the workflow I'd expect to see here.

Client side:

  • Client generates key pair w/ KeyPair::generate() or another fn as appropriate.
  • Client generates rcgen::CertificateParams, customizing as required to describe attributes of the certificate it desires.
  • Client generates rcgen::CertificateSigningRequest from the rcgen::CertificateParams by calling rcgen::CertificateParams::serialize_request(subject_key) where subject_key is the public half of the client's KeyPair.
  • Client serializes the rcgen::CertificateSigningRequest to PEM or DER w/ rcgen::CertificateSigningRequest::pem()/der() and transmits this to the CA over a trusted channel.

Server side:

  • Server receives PEM/DER CSR from a trusted client.
  • Server creates rcgen::CertificateSigningRequestParams using CertificateSigningRequestParams::from_pem()|from_der(), providing the CSR data. Importantly this verifies the client's signature over the CSR and proves control of the private key corresponding to the public key that is the CSR subject.
  • Server verifies the CertificateSigningRequestParams's CertificateParams meets its profile for the client. E.g. what things is the client allow to specify be included in the generated cert? You must be very careful here or a client could request a cert with is_ca true as one example...
  • Server issues a certificate for the CertificateSigningRequestParams by calling CertificateSigningRequestParams::signed_by, providing the CA Certificate and KeyPair of the server.
  • Server serializes the Certificate it generated from the CSR to PEM/DER and sends it back to the client.

Maybe it's worth adding this to the examples.

Absolutely! I think discovering the above workflow from docs alone would be challenging.

Hmmm I wonder whether we could make CertificateParams::signed_by take K: PublicKeyData instead? The private key is never used.

I chatted about this with djc. I think the general idea of replacing the KeyPair argument in CertificateParams::signed_by makes sense to me but I think we want a new trait. PublicKeyData would bring yasna into the API.

cpu avatar Jul 03 '24 14:07 cpu

This is actually pretty straightforward, we can just extract the serialize_public_key_der() method from the trait:

https://github.com/rustls/rcgen/compare/public-key?expand=1

I think that leaves some bikeshedding on the shape of the public trait.

  • The trait's name: I guess PublicKeyData is okay? We already have a trivial implementation in csr::PublicKey.
  • The alg() method: maybe expand this to algorithm()?
  • The raw_bytes() method: maybe rename this to der_bytes()? raw by itself feels a little vague/opaque.

djc avatar Jul 03 '24 14:07 djc

Server verifies the CertificateSigningRequestParams's CertificateParams meets its profile for the client. E.g. what things is the client allow to specify be included in the generated cert? You must be very careful here or a client could request a cert with is_ca true as one example... ... I think the general idea of replacing the KeyPair argument in CertificateParams::signed_by makes sense to me

I think this change is important because it would allow the safest workflow and what I would do in the OP's shoes:

  • Server receives PEM/DER CSR from a trusted client.
  • Server creates rcgen::CertificateSigningRequestParams using CertificateSigningRequestParams::from_pem()|from_der(), providing the CSR data.
  • Server creates its own CertificateParams from scratch based on its profile for the client, perhaps lifting one or two blessed/validated fields (like SANs) out of the CertificateSigningRequestParams's CertificateParams.
  • Server uses CertificateParams::signed_by on the params it generated, providing the PublicKey from the CertificateSigningRequestParams and its own issuer Certificate and KeyPair
  • Server serializes the Certificate it generated from the CSR to PEM/DER and sends it back to the client.

This avoids the client being able to specify all sorts of stuff in its CSR parameters you might not want to allow and is a closer match to how a system like Let's Encrypt's CA software is implemented.

cpu avatar Jul 03 '24 14:07 cpu

The trait's name: I guess PublicKeyData is okay? We already have a trivial implementation in csr::PublicKey.

Seems OK to me.

The alg() method: maybe expand this to algorithm()?

Agreed.

The raw_bytes() method: maybe rename this to der_bytes()? raw by itself feels a little vague/opaque.

Agreed :+1:

Thanks for running with this!

@Virviil Would you be interested in contributing an example based on some of the discussion above? If not I will try to do it myself but it may be on a longer timescale :-)

cpu avatar Jul 03 '24 14:07 cpu

Server verifies the CertificateSigningRequestParams's CertificateParams meets its profile for the client. E.g. what things is the client allow to specify be included in the generated cert? You must be very careful here or a client could request a cert with is_ca true as one example...

...

I think the general idea of replacing the KeyPair argument in CertificateParams::signed_by makes sense to me

I think this change is important because it would allow the safest workflow and what I would do in the OP's shoes:

  • Server receives PEM/DER CSR from a trusted client.

  • Server creates rcgen::CertificateSigningRequestParams using CertificateSigningRequestParams::from_pem()|from_der(), providing the CSR data.

  • Server creates its own CertificateParams from scratch based on its profile for the client, perhaps lifting one or two blessed/validated fields (like SANs) out of the CertificateSigningRequestParams's CertificateParams.

  • Server uses CertificateParams::signed_by on the params it generated, providing the PublicKey from the CertificateSigningRequestParams and its own issuer Certificate and KeyPair

  • Server serializes the Certificate it generated from the CSR to PEM/DER and sends it back to the client.

This avoids the client being able to specify all sorts of stuff in its CSR parameters you might not want to allow and is a closer match to how a system like Let's Encrypt's CA software is implemented.

Yep, this is basically my workflow expectation.

While constructing new cert from CSR is possible to implement, I don't want neither to send any client specific attributes of the cert (via CSR) nor leverage on CSR verification mechanism.

I'm using another auth mechanism (let's say oauth) to ensure that client is trustful, and all the client cert attributes are generated on the server (or CA) side.

So I would prefer to pass only public key with my api request, and return only a cert.

Virviil avatar Jul 03 '24 15:07 Virviil

The trait's name: I guess PublicKeyData is okay? We already have a trivial implementation in csr::PublicKey.

Seems OK to me.

The alg() method: maybe expand this to algorithm()?

Agreed.

The raw_bytes() method: maybe rename this to der_bytes()? raw by itself feels a little vague/opaque.

Agreed :+1:

Thanks for running with this!

@Virviil Would you be interested in contributing an example based on some of the discussion above? If not I will try to do it myself but it may be on a longer timescale :-)

Yep, I'll try to add the most straightforward example: just accepting CSR as is and produce a cert. While it's not save - it covers the most important library apis

Virviil avatar Jul 03 '24 15:07 Virviil

Hmmm I wonder whether we could make CertificateParams::signed_by take K: PublicKeyData instead? The private key is never used.

I made a branch that does this and adds a SubjectPublicKey type that impls PublicKeyData and can be parsed from PEM or DER. Public key values output by both rcgen and the openssl CLI are serialized SubjectPublicKeyInfo structs, hence the name of the type. Parsing uses the x509_parser crate, so this type is available just when the x509-parser feature is enabled.

https://github.com/kwantam/rcgen/tree/rsw/cert-from-pk-only

Side note: it is vaguely annoying to go from x509_parser::x509::AlgorithmIdentifier to rcgen::SignatureAlgorithm. Some changes to the latter type would help here---for example, maybe it could be modified to wrap an AlgorithmIdentifier<'static> (that might take some thinking, but this crate already indirectly depends on lazy_static so there's an obvious path in any case). That would make the x509_parser dep non-optional but might let you get rid of the entire oid module in favor of the oid_registry crate, which would anyway be an indirect dep via x509_parser.

kwantam avatar Aug 17 '24 18:08 kwantam

That would make the x509_parser dep non-optional but might let you get rid of the entire oid module in favor of the oid_registry crate, which would anyway be an indirect dep via x509_parser.

Making x509-parser non-optional doesn't sound attractive.

Do you want to submit a PR?

djc avatar Aug 18 '24 09:08 djc

Sure, I'll open it now but I'm completely buried for the next few days so I may not be responsive to review comments immediately.

EDIT: #288

kwantam avatar Aug 21 '24 08:08 kwantam