doc: which traits must be implemented for a custom crypto provider

[jsha]

I was looking up how to implement a custom crypto provider. It looks like it's not currently documented in a consolidated place. These are all the traits found in modules under rustls::crypto, plus some in rustls::quic. Could someone confirm whether this is the full set? Also, does each provider need to implement all of the ones not marked Optional, or is it expected that some providers would delegate to an existing default implementation for some of them?

• CryptoProvider
• ActiveKeyExchange
• SupportedKxGroup
• cipher::MessageDecrypter
• cipher::MessageEncrypter
• cipher::Tls12AeadAlgorithm
• cipher::Tls13AeadAlgorithm
• signer::Signer
• signer::SigningKey
• tls12::Prf
• hash::Hash
• hash::Context
• tls13::Hkdf
• tls13::HkdfExpander
• hmac::Hmac
• hmac::Key

Optional: QUIC

• quick::Algorithm
• quic::HeaderProtectionKey
• quic::PacketKey

Optional: HPKE

• hpke::Hpke
• hpke::HpkeProvider

[ctz]

the intention is that CryptoProvider is the only mandatory trait that needs implementing -- everything else is optional. there's no requirement that one CryptoProvider provides everything -- eg, it's absolutely reasonable for your impl of CryptoProvider::default_cipher_suites() to just be rustls::crypto::ring::RING.default_cipher_suites()

i think we have some docs work to do here. will prepare a PR for that in the morning (and use that to close the fledgling RFC PR).

[cpu]

Optional: HPKE

hpke::Hpke
hpke::HpkeProvider

Those aren't useful anywhere yet and shouldn't have been included in the docs. Addressed by #1604

[jsha]

Thanks! And yeah, that makes sense. But say someone wants to not depend on ring or aws-lc-rs at all, which traits would they have to implement?

While you're working on documentation in this area, load_private_key should probably specify which private key formats an implementation should minimally accept.

[djc]

@jsha what use case/audience do you have in mind here?

[jsha]

My initial motivation was that @cpu had mentioned on rustls-ffi wanting to export the CryptoProvider API in FFI, so I wanted to understand the API upstream.

For audiences, I've drawn from use cases I've seen mentioned on this repo:

• someone who needs all crypto provided by a FIPS-approved library
• someone who wants all crypto implemented natively in Rust
• someone who wants all crypto implemented by a library built into their platform
• someone who wants to store their private keys in an HSM, TPM, etc.

Thanks for #1608, @ctz. And thanks for the clarification that CryptoProvider is the only "mandatory" touchpoint for implementing some of these. I see now how the "private keys in an HSM" use case implicates only SigningKey and Signer.

But it seems like the other major use cases require implementing all the traits I listed up top, and it would be useful to list them out in one place or put them all in one module. That's particularly the case because some of the extension traits are located at one remove. For instance, CryptoProvider links to SupportedKxGroup, but that in turn links to ActiveKeyExchange.

Some questions I still have after reading the updated docs:

• Must all CryptoProvider implementations be instantiated as a static? Looks like yes; it would be good to document that on CryptoProvider.
• Is it allowable to supply a different CryptoProvider for a {Client,Server}Config vs the WebPki{Client,Server}Verifier included in that config?

Also, the concept of delegating to an existing provider makes me think: a custom CryptoProvider is an alternate way of configuring cipher suites and kx groups, instead of ConfigBuilder. You could define a CryptoProvider that returns your desired values as the defaults, and then not override them at all with ConfigBuilder. This could be a possible future improvement to the ergonomics of ConfigBuilder if it became the recommended way to choose those values. Relatedly: should a CryptoProvider have a method default_protocols? A CryptoProvider that implements only TLS 1.3 ciphersuites would cause problems because ConfigBuilder would by default try to configure both TLS 1.2 and 1.3, and would error because there were no TLS 1.2 ciphersuites available.

Anyhow I'm happy to take my own stab at writing the additional docs I'd like to see, if folks feel generally aligned on what I'm saying? I'd also like to add text to each CryptoProvider-related trait linking it back to CryptoProvider itself.

[djc]

You could define a CryptoProvider that returns your desired values as the defaults, and then not override them at all with ConfigBuilder. This could be a possible future improvement to the ergonomics of ConfigBuilder if it became the recommended way to choose those values.

Sounds like you may have missed #1372 (comment). 👍

[ctz]

wanting to export the CryptoProvider API in FFI

So I don't think we should provide FFI APIs that allow custom CryptoProviders to be written. I think for most existing crypto libraries written in not-rust, there exist usable rust bindings and it probably makes more sense to do it that way around.

What I think we should do in rustls-ffi, is allow the caller to specify which crypto provider they want to use. That could be at runtime (con: the library contains all the options) or compile/link time (con: there are many .a libraries to manage).

[cpu]

What I think we should do in rustls-ffi, is allow the caller to specify which crypto provider they want to use.

That's more of what I had in mind 👍 Figuring out the right way to express the API for choosing aws-lc-rs as the provider, or another provider that lives outside of the Rustls tree.

[jsha]

I don't think we should provide FFI APIs that allow custom CryptoProviders to be written.

Yep, that's the conclusion I came to as well, in rustls/rustls-ffi#366.

[jsha]

Sounds like you may have missed #1372 (comment). 👍

Aha, thanks for the link. Yes I saw that comment and then immediately forgot the specific recommendation of using CryptoProvider as the config surface here. Great, seems like we are all on the same page!

[jsha]

It seems like rustls_pki_types::SignatureVerificationAlgorithm is part of the extension surface exposed by CryptoProvider, by way of rustls::WebPkiSupportedAlgorithms, right? Do we have a semver hazard here because that trait is in another crate?

https://docs.rs/rustls-pki-types/0.2.1/rustls_pki_types/trait.SignatureVerificationAlgorithm.html

[djc]

The goal is to version pki-types (which we control) to 1.0 before the release, and hopefully keep it at 1.x for a long time (although we might want to bump to 2.0 when IpAddress moves from std to core so we can use it in no_std scenarios directly).

[jsha]

The mix-and-match nature of the CryptoProvider API leads to this potentially surprising bit of code, if both ring and aws_lc_rs are configured on:

use rustls::crypto::aws_lc_rs::cipher_suite::TLS13_CHACHA20_POLY1305_SHA256;
use rustls::crypto::aws_lc_rs::kx_group::X25519;
use rustls::crypto::ring::RING;
...

fn main() {
    ...
    let config = rustls::ClientConfig::builder_with_provider(RING)
        .with_cipher_suites(&[TLS13_CHACHA20_POLY1305_SHA256])
        .with_kx_groups(&[X25519])
        .with_protocol_versions(&[&rustls::version::TLS13])
        .unwrap()
        .with_root_certificates(root_store)
        .with_no_client_auth();

This is a ClientConfig made with the ring provider, where none of the cryptography actually used is from ring. It all comes from aws_lc_rs instead (except for randomness).

This is particularly confusing because TLS13_CHACHA20_POLY1305_SHA256 looks like it would be a constant that references an entry in the IANA cipher suite registry. And in fact there's a same-named enum variant, CipherSuite::TLS13_AES_128_GCM_SHA256. But the structs by that name in ring and aws_lc_rs actually carry implementation, not just a constant. And it's possible for the implementation to be different from the CryptoProvider passed to builder_with_provider.

I'll see if I can write up some documentation to explain this nicely. It would be great if we could detect this situation and error. For instance, at the end of config building, if the cipher suites or kx groups aren't in the list provided by CryptoProvider::default_cipher_suites / CryptoProvider::default_kx_groups, error out. But of course that doesn't work because a provider can provide cipher suites that aren't the default. And there's no CryptoProvider::all_cipher_suites.

[djc]

Ugh, that's a fair point. I feel like the better way forward here would be to follow through on #1372 (comment) in 0.22 instead of postponing it out to 0.23.

[cpu]

I feel like the better way forward here would be to follow through on #1372 (comment)

I'm not sure I understand how that proposal improves this situation. Can you expand on where the improvement is in your eyes?

If I write:

use rustls::crypto::aws_lc_rs::cipher_suite::TLS13_CHACHA20_POLY1305_SHA256;
use rustls::crypto::aws_lc_rs::kx_group::X25519;
use rustls::crypto::ring::RING;

let provider = CryptoProvider {
  cipher_suites: &[TLS13_CHACHA20_POLY1305_SHA256],
  kx_groups: &[X25519],
  .. RING
};
let config = rustls::ClientConfig::builder_with_provider(provider)
        .with_protocol_versions(&[&rustls::version::TLS13])
        .unwrap()
        .with_root_certificates(root_store)
        .with_no_client_auth();

We arrive at what feels like a very similar state, having only traded whether struct fields are used or builder methods. In both cases RING is only providing randomness and not much else. Am I overlooking something?

[djc]

That's fair. I guess I feel like (a) the CryptoProvider construction would be new, so people might be less likely to port existing code where they might get this wrong and (b) the CryptoProvider construction API feels like a separate API surface, which is more clearly specialized compared to the config builder API.

[cpu]

the CryptoProvider construction API feels like a separate API surface, which is more clearly specialized compared to the config builder API.

I can see what you mean & that does seem like an advantage.

[cpu]

I feel like the better way forward here would be to follow through on #1372 (comment) in 0.22 instead of postponing it out to 0.23.

Here's a PR implementing the changes: #1628

[djc]

Have all of these concerns been addressed?

[jsha]

Mostly yes. I understand the mix-and-match nature, but I think it would still be useful to have a list of what needs implementing for the use case "I want to write a CryptoProvider that does not rely on any other CryptoProvider. Perhaps that is just "the contents of the rustls::crypto module."

Since this would be a doc-only change it doesn't need to block an 0.22 release; we can always follow up with a patchlevel release containing doc improvements.

[pinkforest]

I've struggled with rustls::SecureRandom - see

• API for providing Rng RustCrypto/rustls-rustcrypto#41 (comment)

Which may seem to be missing ?

Ideally would love to see rng::CryptoRng supported but that is a feature req:

• rand_core::RngCore & CryptoRng support for CryptoProvider #1853