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.

Sign up for GitHub

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

Jump to bottom

RUSTSEC-2023-0052: webpki: CPU denial of service in certificate path building #1342

Closed
github-actions bot opened this issue Aug 23, 2023 · 13 comments · Fixed by #1362
Closed

RUSTSEC-2023-0052: webpki: CPU denial of service in certificate path building #1342

github-actions bot opened this issue Aug 23, 2023 · 13 comments · Fixed by #1362
Assignees
@mzabaluev

Comments

@github-actions
Copy link

webpki: CPU denial of service in certificate path building

Details
Package webpki
Version 0.22.0
Date 2023-08-22

When this crate is given a pathological certificate chain to validate, it will
spend CPU time exponential with the number of candidate certificates at each
step of path building.

Both TLS clients and TLS servers that accept client certificate are affected.

This was previously reported in
<briansmith/webpki#69> and re-reported recently
by Luke Malinowski.

rustls-webpki is a fork of this crate which contains a fix for this issue
and is actively maintained.

See advisory page for additional details.
@erwanor erwanor mentioned this issue Aug 30, 2023
Merged
5 tasks
@mzabaluev mzabaluev self-assigned this Aug 31, 2023
@mzabaluev
Copy link
Contributor

It seems like cutting out hyper-proxy is the most expedient way to solve this. I'll look into available alternatives. Any suggestions on this?

@tony-iqlusion
Copy link
Collaborator

webpki v0.22.1 now contains a fix: https://rustsec.org/advisories/RUSTSEC-2023-0052.html

@mzabaluev
Copy link
Contributor

webpki v0.22.1 now contains a fix: https://rustsec.org/advisories/RUSTSEC-2023-0052.html

I see two crates through which this workspace depends on webpki 0.21.x: hyper-proxy and an outdated version of hyper-rustls. It should be easy to update the latter, but hyper-proxy does not look like it will get updated any time soon.

@mzabaluev mzabaluev mentioned this issue Sep 4, 2023
Closed
@tony-iqlusion
Copy link
Collaborator

If you update to webpki v0.22.1, the fix should work with hyper-proxy

@mzabaluev mzabaluev mentioned this issue Sep 5, 2023
Closed
5 tasks
@mzabaluev
Copy link
Contributor

If you update to webpki v0.22.1, the fix should work with hyper-proxy

Can you provide a precise way to do it?
cargo update -p [email protected] does not fail for me, but does not update the webpki dependencies either. I expected it couldn't before trying, because hyper-proxy is on webpki = ~0.21.

@mzabaluev
Copy link
Contributor

#1346 provides a surgical approach, by cutting out all proxy support.

The other alternatives to resolve this that I see:

  • Switch the network crypto to OpenSSL and native-tls, bringing in vendoring issues with a platform library.
  • Find or write a replacement for hyper-proxy.
  • Live with the vulnerability, noting the limited ways in which it can be triggered (HTTP client for RPC connecting through a malicious proxy).

@tony-iqlusion
Copy link
Collaborator

tony-iqlusion commented Sep 5, 2023 •
edited
Loading

Oh sorry, I missed that hyper-proxy was a (minor) version behind on webpki

@fmorency
Copy link
Contributor

fmorency commented Sep 5, 2023

Thanks for the fix!

Would it be possible to cut a new 0.29.x release for us still living in the TM 0.34 world when this PR gets merged?

@mzabaluev
Copy link
Contributor

mzabaluev commented Sep 5, 2023 •
edited
Loading

HTTP CONNECT support seems doable in house by following the doc on protocol upgrades in hyper.

@mzabaluev
Copy link
Contributor

mzabaluev commented Sep 5, 2023 •
edited
Loading

Thanks for the fix!

Well... we are still debating whether removing all proxy support (which was once a requested feature) is an acceptable price for this.

@tony-iqlusion
Copy link
Collaborator

FWIW we no longer run any apps which require RPC through a proxy, so if you want to rip it out that's fine with us

@mzabaluev mzabaluev mentioned this issue Sep 18, 2023
Merged
@mzabaluev
Copy link
Contributor

I have explored rewriting a proxy connector, but ran into connection lifecycle issues that are rather hard to debug, and the available examples from upstream do not fit our usage very well.
Somewhat more long-term, hyper::Client would no longer be available with the next major release of hyper.
It's better to switch to reqwest as the high-level HTTP client, since it also has proxy support built in.

@mzabaluev mzabaluev mentioned this issue Sep 28, 2023
Merged
5 tasks
mzabaluev added a commit that referenced this issue Sep 29, 2023
@mzabaluev @romac
Small rewording in changelog for #1342 (#1362)
1cb96c8 
Co-authored-by: Romain Ruetschi <[email protected]>
romac added a commit that referenced this issue Sep 29, 2023
@mzabaluev @romac
rpc: replace hyper::Client with reqwest::Client (#1362)
93877da 
* rpc: replace hyper::Client with reqwest::Client

In hyper, the high-level Client implementation is going to be removed
in the next major release. The current client lacks built-in support for
HTTP proxies, and we want to ditch hyper-proxy as it is unmaintained
and its webpki dependency has known security issues.

* Remove the proxy_client example

Oops, this was temporary and not meant to be commited.
The CLI can be used to test the proxy support.

* Bump async-tungstenite version to 0.23

* rpc: fix proxy configuration in http::Builder

* rpc: use argument type to select the dialog

Remove ugly turbofish syntax. The changes only affect internal methods,
so these stylistics do not matter a lot.

* rpc: restore LatestDialog as re-export

This solves the problem with using type alias as a constructor.

* Changelog entries for #1362

* rpc: prune dependencies for http-client feature

* rpc: demote http to dev-dependencies

* Small rewording in changelog for #1342 (#1362)

Co-authored-by: Romain Ruetschi <[email protected]>

---------

Co-authored-by: Romain Ruetschi <[email protected]>
@fmorency
Copy link
Contributor

Happy to see a fix for this issue, thanks!

Would it be possible to backport this fix to 0.29.x for TM 0.34 support?

SuperFluffy pushed a commit to astriaorg/tendermint-rs that referenced this issue Oct 12, 2023
@mzabaluev @romac @SuperFluffy
rpc: replace hyper::Client with reqwest::Client (informalsystems#…
7e4d10d 
…1362)

* rpc: replace hyper::Client with reqwest::Client

In hyper, the high-level Client implementation is going to be removed
in the next major release. The current client lacks built-in support for
HTTP proxies, and we want to ditch hyper-proxy as it is unmaintained
and its webpki dependency has known security issues.

* Remove the proxy_client example

Oops, this was temporary and not meant to be commited.
The CLI can be used to test the proxy support.

* Bump async-tungstenite version to 0.23

* rpc: fix proxy configuration in http::Builder

* rpc: use argument type to select the dialog

Remove ugly turbofish syntax. The changes only affect internal methods,
so these stylistics do not matter a lot.

* rpc: restore LatestDialog as re-export

This solves the problem with using type alias as a constructor.

* Changelog entries for informalsystems#1362

* rpc: prune dependencies for http-client feature

* rpc: demote http to dev-dependencies

* Small rewording in changelog for informalsystems#1342 (informalsystems#1362)

Co-authored-by: Romain Ruetschi <[email protected]>

---------

Co-authored-by: Romain Ruetschi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mzabaluev mzabaluev

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@fmorency @mzabaluev @tony-iqlusion