Skip to content

Conversation

@blheatwole
Copy link

There was some previous, but incomplete work to simply the parameters need for signing operations. See previous pull request for Issuer API. This work was stopped due to KeyPair ownership vs borrow issues.

Changes

This PR fixes the KeyPair ownership issue by make KeyPair cheap to clone through the use of a reference-counted inner implementation. I used std::rc::Rc but this could be easily swapped for std::sync::Arc if multi-threading is deemed higher importance than low runtime impact.

Fixing the KeyPair ownership question then enables the Issuer type to encapsulate all information needed for signing operations while still allowing the KeyPair to be used by other code.

Versioning

This PR does change the published signature of rcgen functions that sign code, so that needs to be tracked with appropriate semver versioning for rcgen at least. The public API of rustls-cert-gen is not affected.

Background

I am working on a library that allows an AzureKeyVault to be used for PKI storage. The RemoteKeyPair works well for this, allowing the private key to never exist anywhere but locked inside the KV.

However signing using rcgen has proved impossible due to the signing functions requiring a full Certificate even though it was only used as a wrapper for the params field and then only to construct the Issuer. The construction of a Certificate was also locked inside the library.

My first thought was to simply provide public methods of creating a Certificate, but in reviewing the issues and pull requests, I determined the project wanted an update to Issuer, so I implemented that change instead.

This PR resolves the problem of requiring a Certificate that I can't create by making the Issuer the thing that performs signing. The issuer requires access to the KeyPair (which can still reference a remote key) and some of the parameters to the certificate, though it does not require a full Certificate. An Issuer can be constructed from a KeyPair and either CertificateParams or CertificateDer if the x509-parser features is enabled.

@djc
Copy link
Member

djc commented Apr 13, 2025

Thanks for your work on this! There are indeed some long-standing issues here that I've been wanting to fix but hadn't gotten to yet. I don't think this is quite the right approach but I've made a start in a similar direction in #328.

@blheatwole
Copy link
Author

#328 removes the need for this

@blheatwole blheatwole closed this Apr 24, 2025
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