bitcoindevkit / rust-electrum-client

Bitcoin Electrum client library. Supports plaintext, TLS and Onion servers.
MIT License
80 stars 62 forks source link

Upgrade rustls to 0.23 #132

Closed nickfarrow closed 6 months ago

nickfarrow commented 7 months ago

With rustls 0.23 there is no longer a dependency on ring, allowing easier compilation for various targets.

Not super confident with my updates to ServerCertVerifier and Der of certificates (is this being tested?), needs review.

RCasatta commented 7 months ago

Version 0.21 has also other issues...

Crate:     rustls
Version:   0.21.7
Title:     `rustls::ConnectionCommon::complete_io` could fall into an infinite loop based on network input
Date:      2024-04-19
ID:        RUSTSEC-2024-0336
URL:       https://rustsec.org/advisories/RUSTSEC-2024-0336
Severity:  7.5 (high)
Solution:  Upgrade to >=0.23.5 OR >=0.22.4, <0.23.0 OR >=0.21.11, <0.22.0
nickfarrow commented 4 months ago

Apologies for the substantial mess this caused!

When making this PR it allowed some android target to build where I was having substantial trouble with ring. It has broken many other targets however and this was not readily apparent.

I think in the future I should take more time to test across other targets?

Big thanks for your work resolving this @thunderbiscuit

thunderbiscuit commented 4 months ago

Yeah ring has caused me problems in the past too with the Android NDK, so I totally understand.

I don't think we could have predicted this, and certainly testing with all possible architecture would be insane. I'm puzzled by the decision in rustls to straight replace the default provider for something that doesn't build everywhere, but then again their docs (aws-lc-rs and co.) are very good and should in theory allow us to build for all platforms; it's just been harder than advertised IMO 😆.