Open rami3l opened 1 month ago
@djc Can we use aws-lc
instead of ring
for rustls
?
aws-lc
seems to have p521 support already; OTOH comparing aws-lc
's platform support and that of ring
I can see that the former lacks mips*
, but since we don't do mips*
anymore (dfd71c024530779aab3665286a162b30367dfb52) this shouldn't be a problem...
I'm still not very familiar to the subject so please feel free to correct me if I'm wrong.
Yes -- rustls should support P521 since https://github.com/rustls/rustls/releases/tag/v%2F0.22.2 if we switch to aws-lc-rs.
@djc (A newbie question:) I tried adding
[dependencies]
rustls = { version = "0.22", optional = true, default-features = false, features = ["logging", "aws_lc_rs", "tls12"] }
... to our Cargo.toml
but looks like ring
is still there in the lockfile. Actually, both aws-lc-rs
and ring
are in the dependencies now, which may cause problems as described in https://github.com/seanmonstar/reqwest/pull/2225#issuecomment-2031126910.
Am I doing anything wrong, or we need to wait for something like https://github.com/seanmonstar/reqwest/issues/2136?
@djc I'll try to find the reason for the p521 curve, but I'm pretty sure the choice is quite deliberate. WARP is written in Rust, so the engineers developing it feel themselves the pain of ring
being incompatible with it. The ring PR is by a Cloudflare engineer.
@kornelski I think we'll be able to use aws-lc-rs but would still be interested in hearing the reasons that it's used!
@rami3l reqwest allows configuring the ClientBuilder with a pre-built ClientConfig (of the matching Rustls release), so I think we can build a rustls 0.22 ClientConfig and configure reqwest to use this.
@rami3l reqwest allows configuring the ClientBuilder with a pre-built ClientConfig (of the matching Rustls release), so I think we can build a rustls 0.22 ClientConfig and configure reqwest to use this.
@djc That's the way to do manual resolution at runtime right? Shouldn't there be a way to simply remove ring
if we're not using it?
@rami3l reqwest allows configuring the ClientBuilder with a pre-built ClientConfig (of the matching Rustls release), so I think we can build a rustls 0.22 ClientConfig and configure reqwest to use this.
@djc That's the way to do manual resolution at runtime right? Shouldn't there be a way to simply remove
ring
if we're not using it?
Unfortunately it doesn't look like that exists in reqwest right now. Let's see if I can move that forward.
In WARP, the p521 curve has been chosen as the best algorithm with FIPS compliance.
The p521 signature is necessary "only" to validate the root CA certificate used by WARP MITM. At the same time this is the hardest thing to change in this setup, so it's very unlikely to be changed anytime soon.
@kornelski but in terms of impact: this specifically impacts Cloudflare's WARP deployment, right, not the default deployment one would get when setting up WARP for their organization?
(I revised https://github.com/seanmonstar/reqwest/pull/2225 yesterday to try to make progress on this.)
It impacts more than just internal Cloudflare deployment. Customers have an option to upload their own CA cert (which can use any signature algorithm), but if they don't, the default Cloudflare cert is used. I don't have data on how many deployments use the incompatible cert.
@kornelski but in terms of impact: this specifically impacts Cloudflare's WARP deployment, right, not the default deployment one would get when setting up WARP for their organization?
(I revised seanmonstar/reqwest#2225 yesterday to try to make progress on this.)
@djc Oops, looks like https://github.com/seanmonstar/reqwest/pull/2225 has been rejected? Should we resolve aws-lc
at runtime? And if we're shipping with ring
anyways, does it make sense to allow experiments on both backends? (Now it's backends relying on backends relying on backends (x_x))
I don't think we'll need to support both aws-lc-rs and ring in rustup. I think we should use reqwest's use_preconfigured_tls()
API to pass in our own ClientConfig
that uses aws-lc-rs and the rustls-platform-verifier.
I don't think we'll need to support both aws-lc-rs and ring in rustup. I think we should use reqwest's
use_preconfigured_tls()
API to pass in our ownClientConfig
that uses aws-lc-rs and the rustls-platform-verifier.
@djc There's still hope that we might remove ring
from the binary one day?
There's a bunch of dead-code elimination at several stages, it's possible that ring stuff gets removed at some point anyway if we avoid using it in practice.
There's a bunch of dead-code elimination at several stages, it's possible that ring stuff gets removed at some point anyway if we avoid using it in practice.
Update: Actually https://github.com/rust-lang/rustup/pull/3898 is no longer depending on ring
even at compile time now.
Originally posted by @kornelski in https://github.com/rust-lang/rustup/issues/3806#issuecomment-2105434221