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

It is possible to DoS certificate validation with name constraints and/or a large number of candidate intermediates #69

Closed
briansmith opened this issue Jan 10, 2018 · 15 comments
Labels

Comments

@briansmith
Copy link
Owner

There is a DoS attack exploits the fact that name constraint checking, in particular, has quardratic (at least) behavior. If certificate A that chains to a certificate B and certificate A has a lot of names and certificate B has a lot of name constraints, then we do a lot of checking. BoringSSL's solution was to verify that the entire certificate chain is valid modulo name constraints, and then check name constraints. (TODO: verify that this summary of what they did is accurate.) We should do that or something better.

#68 might make this easier to do; regardless this and #68 interact with each other.

@briansmith briansmith added the bug label Jan 10, 2018
@briansmith
Copy link
Owner Author

See also golang/go#29233.

@briansmith briansmith changed the title It is possible to DoS certificate validation with name constraints It is possible to DoS certificate validation with name constraints and/or a large number of candidate intermediates Jun 3, 2020
@briansmith
Copy link
Owner Author

I generalized this to cover two DoS-related issues that interact:

  • Name constraint checking is quadratic, so a large number of names in certs and/or a large number of name constraints could cause serious issues.
  • Path building is as bad or worse in the number of candidates certificates.

Both issues are compounded by signature verification being very expensive. They are inter-related because some solutions (e.g. naively doing path building sans name constraint checks before doing name constraint checks) seem to just shift the problem around.

However, it is also the case that anti-DoS countermeasures also have significant DoS side-effects of their own: See spiffe/spire#1004, for example. This issue is especially interesting because it likely motivates the implementation of AKI/SKI matching.

This was referenced Aug 22, 2023
@Kixunil
Copy link

Kixunil commented Oct 13, 2023

Would it be possible to backport the fix for people with MSRV issues?

@briansmith
Copy link
Owner Author

Would it be possible to backport the fix for people with MSRV issues?

0.22.2 was released with all the fixes so far, without the MSRV bump. 0.22.4 is the release that has the MSRV bump, IIRC. However, 0.22.2 is using an unsupported version of ring whereas 0.22.4 is using a supported version of ring.

Rust 1.61 was released 18 months ago.

@djc
Copy link

djc commented Oct 13, 2023

@Kixunil where are your MSRV constraints coming from? I feel like we're generally pretty conservative compared to most of the ecosystem.

@Kixunil
Copy link

Kixunil commented Oct 13, 2023

The new version of rustls breaks my MSRV and it updates webpki from 0.21 to 0.22. So the problem is also related to semver breaking changes.

@briansmith
Copy link
Owner Author

The new version of rustls breaks my MSRV and it updates webpki from 0.21 to 0.22. So the problem is also related to semver breaking changes.

That's a separate issue to be discussed elsewhere.

Ultimately there's little getting around the need for MSRV 1.61 unless ring 0.17 reduces its MSRV. But at most that would be temporary, if it were to happen at all.

@Kixunil
Copy link

Kixunil commented Oct 14, 2023

1.61 is fine, rustls has 1.64, I need 1.63 (Debian stable)

@djc
Copy link

djc commented Oct 14, 2023

1.61 is fine, rustls has 1.64, I need 1.63 (Debian stable)

So why do you need to stick to Debian stable? I doubt the ecosystem will stick to 1.63 very long.

BTW, I'm not sure where you're getting that rustls needs 1.64 -- rustls on main only just bumped 1.60 to 1.61.

@Kixunil I agree with @briansmith you should open a new issue to discuss your needs.

@briansmith
Copy link
Owner Author

1.61 is fine, rustls has 1.64, I need 1.63 (Debian stable)

ring's and webpki's MSRV is 1.61. If you are running into MSRV problems, they are probably elsewhere. Thanks for letting me know that Debian Stable has 1.63, that must be the rock bottom version of Rust we need to support.

@Kixunil
Copy link

Kixunil commented Oct 14, 2023

So why do you need to stick to Debian stable?

I and other people explained this already multiple times in various places, among other things: dissatisfactory security of rustup.

rustls needs 1.64 -- rustls on main only just bumped 1.60 to 1.61.

Oh, it must be tonic then which itself depends on rustls. I will investigate and open an issue s suggested.

@djc

This comment was marked as off-topic.

@Kixunil

This comment was marked as off-topic.

@briansmith

This comment was marked as off-topic.

Pencil-Yao pushed a commit to Pencil-Yao/webpki that referenced this issue Dec 6, 2023
sprt pushed a commit to microsoft/kata-containers that referenced this issue Dec 7, 2023
The old ones have security issues.
ref: briansmith/webpki#69
briansmith/webpki#69

Signed-off-by: Peng Tao <bergwolf@hyper.sh>
xgreenx added a commit to FuelLabs/fuel-core that referenced this issue Jun 11, 2024
This PR updates `eventsource-client` dependency of `fuel-core-client`
from `0.10.2` to `0.12.2`.
Rationale: there are multiple security advisories for
`hyper-rustls`/`rustls` indirect dependencies
([RUSTSEC-2024-0336](https://rustsec.org/advisories/RUSTSEC-2024-0336),
[RUSTSEC-2023-0052](https://rustsec.org/advisories/RUSTSEC-2023-0052),
[CVE-2022-31394](GHSA-x477-xp89-wc9r)).
Found out about these by running
https://github.com/EmbarkStudios/cargo-deny on `fuels-rs`.
No breaking changes.

References:
<details>
<summary>Output of `cargo-deny`</summary>

```
error[vulnerability]: `rustls::ConnectionCommon::complete_io` could fall into an infinite loop based on network input
    ┌─ /Users/brightone/dev/github.com/FuelLabs/fuels-rs/Cargo.lock:298:1
    │
298 │ rustls 0.19.1 registry+https://github.com/rust-lang/crates.io-index
    │ ------------------------------------------------------------------- security vulnerability detected
    │
    = ID: RUSTSEC-2024-0336
    = Advisory: https://rustsec.org/advisories/RUSTSEC-2024-0336
    = If a `close_notify` alert is received during a handshake, `complete_io`
      does not terminate.

      Callers which do not call `complete_io` are not affected.

      `rustls-tokio` and `rustls-ffi` do not call `complete_io`
      and are not affected.

      `rustls::Stream` and `rustls::StreamOwned` types use
      `complete_io` and are affected.
    = Announcement: GHSA-6g7w-8wpp-frhj
    = Solution: Upgrade to >=0.23.5 OR >=0.22.4, <0.23.0 OR >=0.21.11, <0.22.0 (try `cargo update -p rustls`)
    = rustls v0.19.1
      ├── hyper-rustls v0.22.1
      │   └── eventsource-client v0.10.2
      │       └── fuel-core-client v0.28.0
      │           ├── fuels v0.63.1
      │           │   ├── (dev) e2e v0.63.1
      │           │   ├── (dev) fuels-example-codec v0.63.1
      │           │   ├── (dev) fuels-example-contracts v0.63.1
      │           │   ├── (dev) fuels-example-cookbook v0.63.1
      │           │   ├── (dev) fuels-example-debugging v0.63.1
      │           │   ├── (dev) fuels-example-macros v0.63.1
      │           │   ├── (dev) fuels-example-predicates v0.63.1
      │           │   ├── (dev) fuels-example-providers v0.63.1
      │           │   ├── (dev) fuels-example-rust-bindings v0.63.1
      │           │   ├── (dev) fuels-example-types v0.63.1
      │           │   ├── (dev) fuels-example-wallets v0.63.1
      │           │   └── (dev) wasm-tests v0.63.1
      │           ├── fuels-accounts v0.63.1
      │           │   ├── (build) e2e v0.63.1 (*)
      │           │   ├── fuel-core-version v0.63.1
      │           │   ├── fuels v0.63.1 (*)
      │           │   ├── fuels-programs v0.63.1
      │           │   │   └── fuels v0.63.1 (*)
      │           │   └── fuels-test-helpers v0.63.1
      │           │       └── fuels v0.63.1 (*)
      │           ├── fuels-core v0.63.1
      │           │   ├── fuels v0.63.1 (*)
      │           │   ├── fuels-accounts v0.63.1 (*)
      │           │   ├── fuels-programs v0.63.1 (*)
      │           │   ├── fuels-test-helpers v0.63.1 (*)
      │           │   └── (dev) wasm-tests v0.63.1 (*)
      │           └── fuels-test-helpers v0.63.1 (*)
      ├── rustls-native-certs v0.5.0
      │   └── hyper-rustls v0.22.1 (*)
      └── tokio-rustls v0.22.0
          └── hyper-rustls v0.22.1 (*)

error[vulnerability]: webpki: CPU denial of service in certificate path building
    ┌─ /Users/brightone/dev/github.com/FuelLabs/fuels-rs/Cargo.lock:426:1
    │
426 │ webpki 0.21.4 registry+https://github.com/rust-lang/crates.io-index
    │ ------------------------------------------------------------------- security vulnerability detected
    │
    = ID: RUSTSEC-2023-0052
    = Advisory: https://rustsec.org/advisories/RUSTSEC-2023-0052
    = 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.

      webpki 0.22.1 included a partial fix and webpki 0.22.2 added further fixes.
    = Solution: Upgrade to >=0.22.2 (try `cargo update -p webpki`)
    = webpki v0.21.4
      ├── hyper-rustls v0.22.1
      │   └── eventsource-client v0.10.2
      │       └── fuel-core-client v0.28.0
      │           ├── fuels v0.63.1
      │           │   ├── (dev) e2e v0.63.1
      │           │   ├── (dev) fuels-example-codec v0.63.1
      │           │   ├── (dev) fuels-example-contracts v0.63.1
      │           │   ├── (dev) fuels-example-cookbook v0.63.1
      │           │   ├── (dev) fuels-example-debugging v0.63.1
      │           │   ├── (dev) fuels-example-macros v0.63.1
      │           │   ├── (dev) fuels-example-predicates v0.63.1
      │           │   ├── (dev) fuels-example-providers v0.63.1
      │           │   ├── (dev) fuels-example-rust-bindings v0.63.1
      │           │   ├── (dev) fuels-example-types v0.63.1
      │           │   ├── (dev) fuels-example-wallets v0.63.1
      │           │   └── (dev) wasm-tests v0.63.1
      │           ├── fuels-accounts v0.63.1
      │           │   ├── (build) e2e v0.63.1 (*)
      │           │   ├── fuel-core-version v0.63.1
      │           │   ├── fuels v0.63.1 (*)
      │           │   ├── fuels-programs v0.63.1
      │           │   │   └── fuels v0.63.1 (*)
      │           │   └── fuels-test-helpers v0.63.1
      │           │       └── fuels v0.63.1 (*)
      │           ├── fuels-core v0.63.1
      │           │   ├── fuels v0.63.1 (*)
      │           │   ├── fuels-accounts v0.63.1 (*)
      │           │   ├── fuels-programs v0.63.1 (*)
      │           │   ├── fuels-test-helpers v0.63.1 (*)
      │           │   └── (dev) wasm-tests v0.63.1 (*)
      │           └── fuels-test-helpers v0.63.1 (*)
      ├── rustls v0.19.1
      │   ├── hyper-rustls v0.22.1 (*)
      │   ├── rustls-native-certs v0.5.0
      │   │   └── hyper-rustls v0.22.1 (*)
      │   └── tokio-rustls v0.22.0
      │       └── hyper-rustls v0.22.1 (*)
      └── tokio-rustls v0.22.0 (*)

 advisories FAILED: 2 errors, 0 warnings, 0 notes
```

</details>

- [release
notes](https://github.com/launchdarkly/rust-eventsource-client/releases/tag/0.12.2)
for `eventsource-client v0.12.2`

## Checklist
- [ ] Breaking changes are clearly marked as such in the PR description
and changelog
- [ ] New behavior is reflected in tests
- [ ] [The specification](https://github.com/FuelLabs/fuel-specs/)
matches the implemented behavior (link update PR if changes are needed)

### Before requesting review
- [ ] I have reviewed the code myself
- [ ] I have created follow-up issues caused by this PR and linked them
here

Co-authored-by: Green Baneling <XgreenX9999@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants