-
-
Notifications
You must be signed in to change notification settings - Fork 128
Bump aes-gcm
to v0.9.1 and enable force-soft
; MSRV 1.41+
#176
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
Conversation
I released a new version of `aes-gcm` with a `force-soft` feature: RustCrypto/AEADs#306 This disables autodetection of AES-NI, falling back on a portable constant-time software implementation. However, in this configuration `aes-gcm` still supports an MSRV of 1.41+, as opposed to 1.49+ required by the autodetection feature. A comment in Cargo.toml notes `force-soft` should be removed when MSRV is bumped to at least 1.49.
Did |
@SergioBenitez it previously would use AES-NI only if the requisite target flags were explicitly enabled with:
The v0.9 release adds runtime autodetection on Unfortunately, the strategy we used to implement this depends on some features introduced in Rust 1.49 (namely support for non- Unless the |
Got it. I suspect |
Yes, if the requisite target features are enabled explicitly, the runtime check compiles down to a noop and only the AES-NI implementation will be linked into the resulting executable. Unfortunately, though that's a nice optimization which we get for free today, the code is still written in a way that requires Rust 1.49+. |
Am I understanding correctly that this is a monotonic regression? Even if I compile with |
Yes, using If you'd like to support AES-NI on MSRV 1.41+, I'd suggest sticking with |
This seems like an ill-fitted, perhaps incorrect use of features. Features should only be used to add capabilities, but I would suggest what I would consider to be the "correct" approach: always enable AES-NI if it is known to statically exist, and otherwise, add a feature that adds the ability to dynamically detect its existence, which of course becomes a no-op if the first condition is met. Ideally, the MSRV is only bumped if the feature is enabled, resolving two issues in one. |
We've tried an RustCrypto/block-ciphers@0fb7bb5 Here's some of the past discussion about it: RustCrypto/block-ciphers#22 We settled on RustCrypto/block-ciphers#208 A big part of why we use the current strategy is it provides an abstraction where
We do all of this, with the caveat that autodetection is enabled by default, which is a very desirable thing to have so long as it is also easy to shut off. The problem is this:
Unless we lean on the autodetection codepaths to do this, we now have two cases to deal with:
In our current scheme, we don't deal with the first case. We used to, and our code used to be riddled with a ton of We're also in the process of expanding runtime feature support to ARM, which further complicates all of this gating. Feel free to jump in on the past discussion, but I ask that you please note the large outpouring of people concerned about having an easy way to regain MSRV 1.41+ support, and I'd also note this thread as precedent. Making it as simple as the code in this PR to reliably the crate still works on Rust 1.41+ while still allowing us to ship on-by-default CPU feature autodetection and without littering our code with a ton of There is perhaps a case to be made for using something like: [features]
default = ["autodetect"] ...but any Rust Embedded user can tell you about their fraught endeavors trying to chase down every Eventually the MSRV 1.41+ requirement will go away and we can assume MSRV 1.49+ at a baseline, which will simplify things. However, I still see trying to go back to an
|
I've taken a look at the issue you've linked, and I've read and reread your post, but I still don't understand where the technical difficulty lies in enabling AES-NI if either it is statically known to be available or it can be detected to be available at runtime. Every other option feels remarkably wrong to me. For AES and SHA256, especially on x86, I always expect hardware supported operations if they are available. If the library supports it, it should not be possible for some transitive dependency to disable such support, and ideally, I shouldn't need to upgrade compilers to obtain this support, either. I consider either a huge misstep. The fact that a version upgrade results in losing important functionality makes the situation even less ideal.
Somewhere in the codebase, you already have 2 versions of the code. Let's call them Effectively, we want this: fn aes() {
#[cfg(have_aesni)]
return aes::ni();
#[cfg(feature(autodetect)]
if detect(aes-ni) { return aes::ni() }
aes::soft()
} This structure makes it clear that
Aren't you enabling auto-detection by default now? I don't see what the problem is either way. It's a simple line or two of documentation to say, "hey, this is how you use this crate in a |
It's a lot more complicated than that... https://github.com/RustCrypto/block-ciphers/blob/master/aes/src/autodetect.rs Anyway, I'd suggest opening an upstream issue on the |
This is now in 0.16. |
I released a new version of
aes-gcm
with aforce-soft
feature:RustCrypto/AEADs#306
This disables autodetection of AES-NI, falling back on a portable constant-time software implementation.
However, in this configuration
aes-gcm
still supports an MSRV of 1.41+, as opposed to 1.49+ required by the autodetection feature.A comment in Cargo.toml notes
force-soft
should be removed when MSRV is bumped to at least 1.49.