Add use-rustls-ring feature

[thunderbiscuit]

This PR adds the ability to build the client using the ring dependency for rustls instead of the new default aws-lc-rs.

As of the 0.23.0 release, rustls changed its default cryptography provider to aws-lc-rs. This new library is actually a set of bindings to a C library maintained by AWS, and they provide prebuilt bindings for some platforms but not all. On these other platforms, the compilation step will attempt to build the bindings, requiring extra dependencies (CMake, libclang and others depending on the platform). This compilation step is what is currently breaking our Android and Swift builds for bdk-ffi. It is certainly possible to build the bindings (and the AWS docs on it are very nice), but for some reason I have not been able to make it work everywhere yet (local, CI, Windows).

This PR enables us to use the previous default ring library for rustls. I basically have to turn off the default features on rustls and re-enable all of them except for the aws_lc_rs. We also have a few feature-gated constructs in the library, for which I needed to add the new proposed use-rustls-ring feature in order to make all of this work for us. Let me know if there are maybe better ways to achieve this!

[LLFourn]

Can we just expose the rustls provider API here: https://docs.rs/rustls/latest/rustls/crypto/struct.CryptoProvider.html so I can pass in my own provider? We can have a rustls-aws-lc feature that adds a constructor that uses that. Can do the same for rustls-ring. But I'd like to able to fix this kind of issue in the future without having to wait for a merge.

[oleonardolima]

Can we just expose the rustls provider API here: https://docs.rs/rustls/latest/rustls/crypto/struct.CryptoProvider.html so I can pass in my own provider? We can have a rustls-aws-lc feature that adds a constructor that uses that. Can do the same for rustls-ring. But I'd like to able to fix this kind of issue in the future without having to wait for a merge.

That's a good question! I'm not sure and would need to take a look at the docs and give it a shot, but if that's feasible, I agree that could be a better approach.

[oleonardolima]

Can we just expose the rustls provider API here: https://docs.rs/rustls/latest/rustls/crypto/struct.CryptoProvider.html so I can pass in my own provider? We can have a rustls-aws-lc feature that adds a constructor that uses that. Can do the same for rustls-ring. But I'd like to able to fix this kind of issue in the future without having to wait for a merge.

That's a good question! I'm not sure and would need to take a look at the docs and give it a shot, but if that's feasible, I agree that could be a better approach.

@thunderbiscuit Taking a look at the documentation it looks like really simple to follow Lloyd's suggestion.

You'd need to use the proper .builder_with_provider() on ClientConfig, probably on the lines:

https://github.com/bitcoindevkit/rust-electrum-client/blob/64c77ee1bc90c5e19ec0d401b97e420d495d7c6d/src/raw_client.rs#L387-L410

[thunderbiscuit]

@LLFourn @oleonardolima

Can we just expose the rustls provider API here: https://docs.rs/rustls/latest/rustls/crypto/struct.CryptoProvider.html so I can pass in my own provider? We can have a rustls-aws-lc feature that adds a constructor that uses that. Can do the same for rustls-ring. But I'd like to able to fix this kind of issue in the future without having to wait for a merge.

This is interesting and might be a good way to go long term, but is outside the scope of this PR IMO, which is just a flag fix for rustls changing their default provider. I'll open an issue so we can tackle this approach in a different PR which should get its own review.

The builds are currently breaking for 3 known projects, and I want to make sure we help them move forward. If this non-intrusive flag allows users to fix their build in the short run, let's get it in.

Unless there are oppositions to this, consider merging @notmandatory.

Interested parties: @oleonardolima @ValuedMammal @LLFourn @notmandatory @praveenperera

[LLFourn]

ACK b415b5cf826c799febeefa3c04ff2e093554f3ff

[praveenperera]

@thunderbiscuit got error.

44 | extern crate webpki_roots;
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^ can't find crate

use-rustls-ring does not enable the webpki_roots feature

use-rustls-ring = ["rustls/ring", "rustls/logging", "rustls/std", "rustls/tls12"]

But it is imported here

#[cfg(any(
    feature = "default",
    feature = "use-rustls",
    feature = "use-rustls-ring"
))]
extern crate webpki_roots;

its working and compiling after adding the webpki_roots to the deps for the new feature: https://github.com/bitcoinppl/rust-electrum-client/commit/e3314aa5720653f110b579fe80085cc52e611189

[oleonardolima]

@thunderbiscuit got error.

44 | extern crate webpki_roots;
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^ can't find crate

use-rustls-ring does not enable the webpki_roots feature

use-rustls-ring = ["rustls/ring", "rustls/logging", "rustls/std", "rustls/tls12"]

But it is imported here

#[cfg(any(
    feature = "default",
    feature = "use-rustls",
    feature = "use-rustls-ring"
))]
extern crate webpki_roots;

its working and compiling after adding the webpki_roots to the deps for the new feature: bitcoinppl@e3314aa

Yes, you're right it's missing as it's being used here:

https://github.com/bitcoindevkit/rust-electrum-client/blob/64c77ee1bc90c5e19ec0d401b97e420d495d7c6d/src/raw_client.rs#L392-L399

It got my attention now, that our CI is not properly covering these tests, which is some sort of complicated because of TLS stuff 🤔

[thunderbiscuit]

Note that I updated the CI to run the use-rustls-ring feature. Thanks @oleonardolima!