Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lack of support for p521 signatures with the ring-based reqwest/rustls backend #3820

Open
Tracked by #3806
rami3l opened this issue May 11, 2024 · 11 comments
Open
Tracked by #3806
Milestone

Comments

@rami3l
Copy link
Member

rami3l commented May 11, 2024

Rustls is completely unusable with the WARP Gateway (a corporate VPN) due to lack of support for p521 signatures.

RUSTUP_USE_RUSTLS=1 rustup update
info: syncing channel updates for 'stable-aarch64-apple-darwin'
error: could not download file from 'https://static.rust-lang.org/dist/channel-rust-stable.toml.sha256' to '~/.rustup/tmp/pnvxaiia4u2hcr_n_file'
info: syncing channel updates for 'nightly-aarch64-apple-darwin'
error: could not download file from 'https://static.rust-lang.org/dist/channel-rust-nightly.toml.sha256' to '~/.rustup/tmp/ay1l00g5xg91pnuc_file'
info: syncing channel updates for '1.63-aarch64-apple-darwin'
error: could not download file from 'https://static.rust-lang.org/dist/channel-rust-1.63.toml.sha256' to '~/.rustup/tmp/0oaqi61f4mgwqa4n_file'
info: syncing channel updates for '1.64-aarch64-apple-darwin'
error: could not download file from 'https://static.rust-lang.org/dist/channel-rust-1.64.toml.sha256' to '~/.rustup/tmp/9rhc8csclaotwleh_file'
info: syncing channel updates for '1.65-aarch64-apple-darwin'
error: could not download file from 'https://static.rust-lang.org/dist/channel-rust-1.65.toml.sha256' to '~/.rustup/tmp/26d6fm0my9i9sgvg_file'
info: checking for self-update
error: could not download file from 'https://static.rust-lang.org/rustup/release-stable.toml' to '/var/folders/lq/fqqfw_z50v96h8tlkj56c8wc0000gn/T/rustup-update5PMZuE/release-stable.toml'

Caused by:
    0: failed to make network request
    1: error sending request for url (https://static.rust-lang.org/rustup/release-stable.toml): error trying to connect: invalid peer certificate: BadSignature
    2: error trying to connect: invalid peer certificate: BadSignature
    3: invalid peer certificate: BadSignature

The curl backend has no problems with it.

Originally posted by @kornelski in #3806 (comment)

@rami3l rami3l added this to the 1.28.0 milestone May 11, 2024
@rami3l
Copy link
Member Author

rami3l commented May 11, 2024

@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 (dfd71c0) 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.

@rami3l rami3l changed the title Lack of support for p521 signatures with the reqwest/rustls backend Lack of support for p521 signatures with the ring-based reqwest/rustls backend May 11, 2024
@djc
Copy link
Contributor

djc commented May 11, 2024

Yes -- rustls should support P521 since https://github.com/rustls/rustls/releases/tag/v%2F0.22.2 if we switch to aws-lc-rs.

@rami3l
Copy link
Member Author

rami3l commented May 11, 2024

@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 seanmonstar/reqwest#2225 (comment).

Am I doing anything wrong, or we need to wait for something like seanmonstar/reqwest#2136?

@kornelski
Copy link
Contributor

@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.

@djc
Copy link
Contributor

djc commented May 11, 2024

@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!

@djc
Copy link
Contributor

djc commented May 11, 2024

@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
Copy link
Member Author

rami3l commented May 11, 2024

@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?

@djc
Copy link
Contributor

djc commented May 12, 2024

@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.

@kornelski
Copy link
Contributor

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.

@djc
Copy link
Contributor

djc commented May 13, 2024

@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.)

@kornelski
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants