Allow library users to bring their own crypto providers by jcgruenhage · Pull Request #268 · djc/instant-acme

jcgruenhage · 2026-02-25T20:49:44Z

Implements base for #267, allowing users to bring their own crypto.

Marked as draft for now, will un-draft once I've implemented (and tested) backends with other crypto libraries, like openssl and rustcrypto to validate this approach is viable.

djc · 2026-02-25T20:56:11Z

To pass the semver check, just add a commit that bumps the version to 0.9.0, please.

djc

Here's some early feedback. I like the direction overall!

src/account.rs

src/crypto.rs

src/order.rs

src/types.rs

jcgruenhage · 2026-02-26T12:25:03Z

@djc do you have a preference regarding commits? Should I address the feedback in new commits, or are you fine with force pushes that amend existing commits?

djc · 2026-02-26T12:41:24Z

I generally prefer clean commits that do one thing, so would definitely prefer force pushes over addressing feedback in separate commits.

src/account.rs

jcgruenhage · 2026-02-26T22:48:11Z

I have locally tested this with an openssl provider and a CLI that creates and deactivates accounts on letsencrypt staging, with 2k/3k/4k RSA and P-256/P-384 key types, including cross key rollover, so I'm removing the draft state.

I'll push another time for working on the remaining unaddressed comments and splitting up the changes a bit further to help with reviewing the code.

djc

Looking good, appreciate the clean commit history!

src/types.rs

src/account.rs

djc · 2026-02-27T11:05:20Z

src/crypto.rs

+}
+
+#[cfg(feature = "ring")]
+mod backend_ring {


Nit: rename these to ring_crypto and aws_lc_rs_crypto maybe?

I've instead moved them to modules called ring and aws_lc_rs, nested beneath crypto. The benefit is that it allows easily diffing the two fairly similar modules.

tests/pebble.rs

Cargo.toml

src/lib.rs

src/types.rs

cpu · 2026-02-27T14:21:33Z

I'd like to give this branch a review as well, but won't have a chance for a couple days. Probably best to address djc's feedback up-front anyway :-)

djc · 2026-02-27T14:30:39Z

I'd like to give this branch a review as well, but won't have a chance for a couple days. Probably best to address djc's feedback up-front anyway :-)

Yup, won't merge without your review.

This allows providers of custom `CryptoProvider`s to also support other algorithms, like EdDSA and RSA.

cpu

This is really nice work, thank you!

I had a fair amount of feedback but I don't think any of it is super substantial.

src/account.rs

cpu · 2026-03-03T21:00:31Z

src/types.rs

-impl Jwk {
-    pub(crate) fn new(key: &crypto::EcdsaKeyPair) -> Self {
+impl Jwk<'_> {
+    pub(crate) fn new(key: &crypto::EcdsaKeyPair) -> Jwk<'_> {


Does this need to change from Self because of the lifetime?

src/account.rs

src/lib.rs

cpu · 2026-03-08T16:35:10Z

src/crypto.rs

+#[cfg(feature = "ring")]
+mod ring;
+
+/// Cryptographic provider for ACME operations.


It feels like it might be worth a sentence or two describing how this differs from a Rustls cryptography provider used for the TLS operations required for the ACME server HTTPS API, as opposed to the crypto operations required for the ACME protocol. WDYT?

cpu · 2026-03-08T16:54:13Z

src/crypto/aws_lc_rs.rs

+    }
+}
+
+struct BuiltinSha256;


nit: the "Builtin" part of this type name (and the HMAC) one seems a bit odd to me. Could we drop it? (Ditto for the ring mod equivalents)

cpu · 2026-03-08T16:55:24Z

tests/pebble.rs

 static NEXT_PORT: AtomicU16 = AtomicU16::new(5555);
 const RETRY_POLICY: RetryPolicy = RetryPolicy::new().backoff(1.0);
+
+fn test_provider() -> &'static CryptoProvider {


nit: put this above the static/consts?

src/lib.rs

cpu · 2026-03-08T16:57:06Z

src/order.rs

-pub struct KeyAuthorization(String);
+pub struct KeyAuthorization {
+    inner: String,
+    sha256: &'static dyn Sha256,


Would it be feasible/better to pre-compute the SHA256 in new() instead of holding on to the dyn Sha256 ref?

cpu · 2026-03-08T16:58:44Z

src/types.rs

+        /// Key type, must be `"OKP"`
+        kty: &'static str,


If the field must always be a certain value, should we maybe handle it ourselves in the Jwk.thumb_sha256 based on self.alg or the variant of self.key? I think it means we'd have to have an internal type that has the kty field for the purposes of serde_json, but it removes one potential source of error from a buggy crypto provider. That seems lightly worth it to me, especially given how many footguns exist in JWS w.r.t "alg: none" and similar :-/

Same feedback applies to the Rsa.kty and Ec.kty fields if you think it's worth considering.

Yeah, that sounds like an improvement.

djc reviewed Feb 25, 2026

View reviewed changes

jcgruenhage force-pushed the crypto-providers branch from e3e6ffe to f91a0d8 Compare February 26, 2026 17:45

jcgruenhage commented Feb 26, 2026

View reviewed changes

src/account.rs Show resolved Hide resolved

jcgruenhage marked this pull request as ready for review February 26, 2026 22:44

jcgruenhage force-pushed the crypto-providers branch 2 times, most recently from 06fc20c to 0013fea Compare February 27, 2026 00:33

djc reviewed Feb 27, 2026

View reviewed changes

src/types.rs Show resolved Hide resolved

jcgruenhage force-pushed the crypto-providers branch from 0013fea to 5ea6432 Compare March 1, 2026 01:06

jcgruenhage added 9 commits March 1, 2026 02:14

Require rustls CryptoProvider for instantiating with DefaultClient

59521d1

Refactor Jwk in preparation of supporting more key types

7f872c3

Replace deprecated Key::generate function with Key::generate_pkcs8

367d86c

Make SigningAlgorithm non_exhaustive and extend docs

080ae6b

Allow library users to bring their own crypto providers

ffac6f3

Add examples/certgen to Cargo.toml

69fd8bb

Extend SigningAlgorithm and JwkInner

8d2b820

This allows providers of custom `CryptoProvider`s to also support other algorithms, like EdDSA and RSA.

Support switching account keys from one provider to another

9d0dfec

Cargo: version 0.8.5 -> 0.9.0

b8299f2

jcgruenhage force-pushed the crypto-providers branch from 5ea6432 to b8299f2 Compare March 1, 2026 01:14

cpu reviewed Mar 8, 2026

View reviewed changes

Uh oh!

Conversation

jcgruenhage commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

djc commented Feb 25, 2026

Uh oh!

djc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jcgruenhage commented Feb 26, 2026

Uh oh!

djc commented Feb 26, 2026

Uh oh!

Uh oh!

jcgruenhage commented Feb 26, 2026

Uh oh!

djc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cpu commented Feb 27, 2026

Uh oh!

djc commented Feb 27, 2026

Uh oh!

cpu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cpu Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jcgruenhage commented Feb 25, 2026 •

edited

Loading

djc left a comment •

edited

Loading

cpu Mar 8, 2026 •

edited

Loading