Skip to content

Replace ring with aws-lc-rs - #2112

Merged
zonyitoo merged 1 commit into
shadowsocks:masterfrom
mullvad:replace-ring-with-aws-lc-rs
May 5, 2026
Merged

Replace ring with aws-lc-rs#2112
zonyitoo merged 1 commit into
shadowsocks:masterfrom
mullvad:replace-ring-with-aws-lc-rs

Conversation

@faern

@faern faern commented Apr 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

faern commented Apr 27, 2026

Copy link
Copy Markdown
Contributor Author

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

zonyitoo commented Apr 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

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.

@faern
faern force-pushed the replace-ring-with-aws-lc-rs branch from 691923b to 288afba Compare April 28, 2026 12:46
@faern

faern commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

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.

@faern
faern force-pushed the replace-ring-with-aws-lc-rs branch 2 times, most recently from ab76338 to 7571bb2 Compare May 5, 2026 07:43
@linus-mullvad
Replace ring with aws-lc-rs across the dependency tree
fe7538d 
Switches the cryptographic backend so that ring is no longer pulled
into shadowsocks-rust on any supported target.

* shadowsocks-crypto: use the new aws-lc-rs backend in place of
  the ring feature
* 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.

The only remaining ring path in the dependency tree 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 7571bb2 to fe7538d Compare May 5, 2026 07:45
@faern
faern marked this pull request as ready for review May 5, 2026 07:46
@faern

faern commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

Updated with the shadowsocks-crypto 0.7.0 release. So this should now be ready for review! The only error in CI is about the advisory against hickory-resolver which is not caused by this PR but simply by that advisory recently being published.

@zonyitoo
zonyitoo merged commit 9fca88b into shadowsocks:master May 5, 2026
16 of 17 checks passed
@faern faern mentioned this pull request May 5, 2026
Merged
@faern

faern commented May 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@zonyitoo Can we get a release with this change? Or do you have a plan for what will be included/when the next release will come out?

@zonyitoo

zonyitoo commented May 7, 2026

Copy link
Copy Markdown
Collaborator

I am waiting for those crypto libraries comes from RustCrypto to upgrade to their latest version.
shadowsocks/shadowsocks-crypto#25

@BrewTestBot BrewTestBot mentioned this pull request Aug 26, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@faern @zonyitoo @linus-mullvad