Skip to content

Replace ring with aws-lc-rs#2112

Draft
faern wants to merge 1 commit intoshadowsocks:masterfrom
mullvad:replace-ring-with-aws-lc-rs
Draft

Replace ring with aws-lc-rs#2112
faern wants to merge 1 commit intoshadowsocks:masterfrom
mullvad:replace-ring-with-aws-lc-rs

Conversation

@faern
Copy link
Copy Markdown
Contributor

@faern faern commented Apr 16, 2026

Ring is unmaintained. Even if there are no known vulnerabilities in it it's a risk to depend on it. Since it's unmaintained it probably gets fewer eyes on it studying if it's correctly implemented. If something bad is found in ring it's going to be harder to patch compared to if we use a more maintained crypto implementation.

aws-lc-rs is more maintained and seems to be what most move towards. aws-lc-rs expose more cryptographic primitives, that ring does not have. Even if shadowsocks itself does not need these newer things, downstream users of shadowsocks might (we are one of those!) and they probably don't want to have both ring and aws-lc-rs in their dependency trees as it blows up both compile time and binary size.

Luckily it seems pretty easy to migrate all of shadowsocks to aws-lc-rs, as you can see in this PR. It just depends on shadowsocks/shadowsocks-crypto#24 being merged and published first.

I have not tested this on Windows myself. I hope the CI is good enough.

This PR builds on top of #2111. Because I wanted the CI to be as clean as possible in order to catch any issues this PR might introduce. So please merge that cleanup PR first, then I can rebase this.

@faern faern marked this pull request as draft April 16, 2026 14:55
@faern faern changed the title Replace ring with aws lc rs Replace ring with aws-lc-rs Apr 16, 2026
@faern faern force-pushed the replace-ring-with-aws-lc-rs branch 2 times, most recently from 19e7cb2 to 691923b Compare April 27, 2026 09:53
@faern
Copy link
Copy Markdown
Contributor Author

faern commented Apr 27, 2026

I believe the only thing left here before marking the PR as ready for review, is a published release of shadowsocks-crypto, so we can get rid of the git dependency.

Or do you want the shadowsocks crate to also have selectable crypto backend? That is going to complicate things a bit, but is maybe doable. What's important to me as a consumer of shadowsocks as a library is that I can depend on it without having ring pulled into my dependency tree, that's it basically.

@zonyitoo
Copy link
Copy Markdown
Collaborator

zonyitoo commented Apr 28, 2026

Since I didn't make ring a configurable option in shadowsocks crate, I think it should be Ok to make aws-lc to be the default crypto backend for shadowsocks. To get rid of ring completely, we should do the same thing on the rustls features list.

I am still working on refactoring the project. After that please update this PR with the latest master.

Switches the cryptographic backend so that ring is no longer pulled
into shadowsocks-rust on any supported target.

* shadowsocks-crypto: use the new faster-crypto feature (aws-lc-rs)
  in place of the now removed ring feature, and patch crates-io to
  the local crate while a release with this change is unpublished.
* tokio-rustls: select the aws_lc_rs provider instead of ring.
* hickory-resolver: use tls-aws-lc-rs, https-aws-lc-rs and
  h3-aws-lc-rs in place of the *-ring variants.

Verified with cargo tree -i ring on x86_64-unknown-linux-gnu,
aarch64-apple-darwin and x86_64-pc-windows-msvc that ring is no
longer reachable. The only remaining ring path is via quinn-proto on
wasm32-unknown-unknown, which aws-lc-rs cannot target and which
shadowsocks does not support.
@faern faern force-pushed the replace-ring-with-aws-lc-rs branch from 691923b to 288afba Compare April 28, 2026 12:46
@faern
Copy link
Copy Markdown
Contributor Author

faern commented Apr 28, 2026

Yes, the ring features on rustls have already been addressed in this PR. With this PR applied, shadowsocks does not depend on ring.

Good luck with the refactoring. I hope you are not aiming to do too big stuff, that might stall a ring-free release 🙏. I will rebase this when I see updates.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants