[uniffi] Make mls-rs-crypto-openssl optional #157
Conversation
For Android, we will be using a crypto provider based on BoringSSL. It
is therefore important that mls-rs-uniffi becomes agnostic to the
crypto provider.
This PR handles this very simply: by requiring two things:
- the user sets a Cargo feature to depend on the right crypto provider
implementation,
- that the crypto provider has a suitable `new()` method that can be
used to instantiate it.
A better alternative would be to make the choice of crypto provider
dynamic via a trait. Unfortunately, I don’t think this is supported
with UniFFI because it doesn’t support associated types in exported
traits and because it also doesn’t support generic exported types.
Making a single non-generic type which uses
```rust
struct CryptoProviderAdapter {
crypto_provider: Box<dyn CryptoProvider>
}
```
fail because the `CryptoProvider` trait has an associated type, which
I cannot specify at this point.
| #[cfg(not(any(feature = "openssl")))] | ||
| compile_error!( | ||
| "The crypto provider must be set by enabling exactly one of the \ | ||
| following Cargo features: [\"openssl\"]." | ||
| ); | ||
|
|
There was a problem hiding this comment.
Would it work to make it like this?
#[cfg(not(any(feature = "openssl")))]
pub type UniFFICryptoProvider = mls_rs::client_builder::Missing;
There was a problem hiding this comment.
Hey Marta, yes, we could use that type by default — when I tried it out, I ended up with more complex error messages, not fewer like I had hoped 😅
But it would be nice to use it for consistency.
| #[cfg(feature = "openssl")] | ||
| pub type UniFFICryptoProvider = mls_rs_crypto_openssl::OpensslCryptoProvider; | ||
|
|
There was a problem hiding this comment.
If we create another feature "boringssl", we won't be able to have UniFFICryptoProvider defined twice, but features should be additive. Can we achieve additive features by defining #[cfg(feature = "openssl")] UniFFIConfigOpenssl = ... and #[cfg(feature = "boringssl")] UniFFIConfigBoringssl = ...?
There was a problem hiding this comment.
If we create another feature "boringssl", we won't be able to have
UniFFICryptoProviderdefined twice, but features should be additive.
Yes, ideally... I was going to guard against having multiple features enabled by letting the build fail with compile_fail! like you saw above. It's not great, but I don't know of a way to make the crypto provider dynamic at runtime.
What we need is
- a non-generic struct which holds a
mls_rs::Clientinside it - the
mls_rs::Clientvalue should be able to hold differentCryptoProviders, implying that we should have aCryptoProviderWrappertype whichimpl CryptoProviderand which holds aBox<dyn CryptoProvider>
So I'm looking for a way to do
struct CryptoProviderWrapper {
crypto_provider: Box<dyn CryptoProvider>,
}However, this doesn't compile because I need to specify the CipherSuiteProvider associated type. I don't know how I can do this in a non-generic way?
|
Let me close this again, we're now looking at using Diplomat instead of UniFFI. |
Issues:
Addresses #81.
Description of changes:
For Android, we will be using a crypto provider based on BoringSSL. It is therefore important that mls-rs-uniffi becomes agnostic to the crypto provider.
This PR handles this very simply: by requiring two things:
new()method that can be used to instantiate it.fail because the
CryptoProvidertrait has an associated type, which I cannot specify at this point.Call-outs:
A better alternative would be to make the choice of crypto provider dynamic via a trait. Unfortunately, I don’t think this is supported with UniFFI because it doesn’t support associated types in exported traits and because it also doesn’t support generic exported types.
Making a single non-generic type which uses
doesn't compile because I need to specify the
CipherSuiteProviderassociated type. I don't know how I can do this in a non-generic way?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT license.