-
Notifications
You must be signed in to change notification settings - Fork 626
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
Rustls w/ aws-lc-rs on Windows requires NASM #1913
Comments
Which version of aws-lc-rs do you have in your tree? AIUI aws-lc-rs 1.7 requires fewer extra dependencies than before on the most common platforms at least. |
1.7 |
This is on 64-bit windows using MSVC toolchain, so a fairly standard platform, and NASM is not listed as a required dependency anywhere that I could see. |
https://aws.github.io/aws-lc-rs/requirements/ Heard you on the frustration with additional dependencies, especially on Windows. See aws/aws-lc-rs#364 upstream for the specific NASM requirement. |
Indeed, but I think there's still a ways to go for Windows in particular w.r.t the Hopefully these improvements continue!
The 0.23.0 release notes that brought the change to aws-lc-rs as default mentions it: "Note that this has some implications on platform support and build-time tool requirements such as cmake on all platforms and nasm on Windows." Since then the |
Please also consider users who are installing tools via This change has a huge negative impact across the whole Rust ecosystem. Please revert to using ring, or other pure rust implementation by default. |
Agreed that this is a negative change, though in the meantime for my use case at least I was able to get around this with |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Unfortunately that doesn't help when intermediate dependencies don't specify this which is the case for eg. |
FWIW, I don't really know how to deal with this situation in ureq. I absolutely need to support windows users and rustls has so far been brilliant in how little dependencies and complexity it introduced. In ureq we also want as few dependencies as possible, however For now I hope I can just stall on upgrading rustls until the dust settles, but so far life seemed simpler when it was just ring… |
Why do you have a direct dependency on rustls-webpki? In either case, it seems like you can simply use |
LOL. I don't know! Just realize it seems to work without it 🤦 … Thanks!
Sure, I can do that, but that also means I take a stand for/against some default choices rustls has made for me, and you are the experts on this. From my perspective FIPS is an enterprise compliance requirement – the kind of entities that can afford to spend more time tinkering with their builds. I understand aws-lc-rs is required for FIPS, and there is a small performance gain, but are there more reasons rustls decided to change the default? Maybe there's a discussion somewhere I missed? |
We have found aws-lc-rs has a number of features that ring does not support, and upstream is generally more responsive/moving faster. Things like P-521 support, RSA keygen support, better support for SEC-1 keys, support for the PQC KYBER key exchange mechanism, support for HPKE (which is a prerequisite for Encrypted ClientHello support) are now supported in aws-lc-rs but not in ring (even though there have been long-standing feature requests). We've also seen fairly rapid support in the build system requirement from aws-lc-rs (and note that ring effectively ships pre-built binary code for Windows to avoid these issues -- which has other problems). Due to this issue we've been having a discussion about how to better communicate the advantages of the aws-lc-rs choice, and this is something we expect to work on soon, but please assume the choice is well-motivated. That said, the more cumbersome build process is a downside for the time being. IMO for your minimal-dependencies use case it is fine to stick with ring for the time being -- and I don't think you're the only library making that choice. |
@djc thank you for elaborating!
I apologize if I came across as questioning your motives. That was not my intent.
Sounds good! I'll make a PR to that today. Thank you! |
That's great as a motivation for users to enable the
That's great, and when the build system requirements are as low as ring, then it might make sense to switch.
It's fairly normal to ship binary code on Windows. The source can still be reviewed and the build can be reproduced, so I don't see a huge difference from a security perspective (and most people aren't reviewing the source code before running |
Diggsey: It sounds like you're prioritizing minimal build time requirements as the most important factor in a cryptography provider dependency. That's reasonable and we hear you, but I don't think you've made a case for why that should be the default prioritization for everyone. As @djc outlined there are several concurrent factors that make aws-lc-rs appealing as a default even though for Windows users it requires an additional build dependency. The fact that several classes of users all have their own justifiable prioritizations (build reqs, performance, post-quantum ciphersuites, FIPS mode, national ciphersuites, a pure Rust impl (note: not ring), etc) is why the backend is now selectable. There's no one provider that could make everyone happy. I think it's expected there will be some bumps in the road short-term that make it harder to opt-in to using
This crate has had default features for a very long time. At least back to 0.10.0. |
Rustls is not just any cryptography provider - we already had the openssl crate which worked just fine. What made rustls special was precisely the minimal build dependencies.
It seems pretty clear to me: opting into aws-lc is easy, but opting out is essentially impossible when it's provided under a default feature. Therefore the question is not whether to prioritise build requirements vs say performance, but whether to prioritize giving people a choice vs not giving them the choice. |
I hardly think that's the only thing (or even necessarily most important) thing that makes rustls an attractive option.
It's not "essentially impossible" -- get the intermediate libraries to fix their feature usage, and/or advocate for/contribute to Cargo efforts to improve general handling of the feature situation. |
Have you surveyed your users? I promise you, the only thing most people care about is having a TLS library that just works. To quote someone else, who phrases it better than I can:
This is the way a lot of people feel, myself included, and the way rustls has been portrayed. Also
As a user, those things are not in my control. hence "essentially impossible". Sure, I can go and fork every intermediate crate to fix the feature usage - is that practical? No. |
If your goal is a true "pure Rust" TLS implementation top-to-bottom then it seems as though the recent work has improved Rustls for you. Unlike before when the cryptography provider was not a choice you can now build rustls without In general it seems like this is becoming an unproductive conversation. I would characterize your attitude as rather combative from the outset and the goal posts keep being moved as we explain our decisions. It's clear you're unhappy with the decision and you've made the reasons why understood. Perhaps we should call that a conclusion and let this thread lie unless there are new perspectives to weigh? |
That's not my goal, I'm saying that the name "rustls" has given users the (apparantly false) impression that this library was striving to provide a TLS implementation with fewer external dependencies that what we had before with openssl, and that this is the main reason this library ever became popular with users.
Yes, I'm upset that yet again someone has decided to break the world for no reason (the new features would work perfectly well under a feature flag) and expects everyone else to fix it.
I don't think I've moved the goalposts at all? Prior versions of this crate just worked with a standard Rust install, new versions don't. That's bad.
Sure, at least until the maintainers of all 728 direct dependencies of this crate - https://crates.io/crates/rustls/reverse_dependencies - start receiving complaints from all the users of those crates asking why the new version no longer builds correctly. Or when the person who's not even a Rust programmer tries to |
Why use Or even more portabley, using inline asm? |
@gimbling-away this is a question for the aws-lc-rs folks, please follow-up with them upstream: |
This comment was marked as off-topic.
This comment was marked as off-topic.
I've also just hit this issue trying to |
Here's what fixed the nasm errors for me, in case this helps anyone else:
|
@clairewood I think you fixed it because on version 0.22.4 aws-lc is not used I have this on my cargo.toml:
However,
Anyone know why? |
Other crates in your dependency graph might be pulling in rustls with default features? Or maybe rust-lang/cargo#10801? |
This is an issue that is being discussed, but we want to be cautious. Ring seems to get around this requirement by pregenerating/embedding the NASM object files into the crate:
|
This comment has been minimized.
This comment has been minimized.
I have a sub-crate which uses rustls and specifies aws-lc-rs. I cannot get it to build on android. |
@narodnik Since your problem is unrelated to Windows/NASM I think it deserves a separate discussion. Please open a new issue, filling out the issue template with as much information as you have. We would need many more details than you've provided in order to help. |
Folks following this issue may be interested in providing feedback on this upstream PR that offers a route to using pre-generated assembly artifacts for |
What was the reasoning of changing the default that was working fine to something that is LESS compatible? I also never had any issues with building rustls on GitHub actions before, that is no longer the case: https://github.com/avencera/rustywind/actions/runs/10760933878/job/29839537060 I'm sure I could fix that but disabling |
Performance, maintainability, support for future features. I think those were covered above, but happy to restate them for your benefit. |
Note that aws-lc-rs 1.9.0 was released 3 days ago, which includes support for using prebuilt NASM objects. |
The entire reason half the ecosystem is using rustls rather than openssl is because it "just works" without any dependencies.
Having upgraded to the latest version, I get the following error:
It's very frustrating that
aws-lc
has been added to the default dependency list as many intermediate dependencies forget to add "default-features = false", making it in practice a hard dependency of rustls.Please reconsider making
aws-lc
the default backend. The default backend should require fewer dependencies, not more. Any minor performance difference is a secondary concern.The text was updated successfully, but these errors were encountered: