Bump `aes-gcm` to v0.9.1 and enable `force-soft`; MSRV 1.41+

[tarcieri]

I released a new version of aes-gcm with a force-soft feature:

https://github.com/RustCrypto/AEADs/pull/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.

[SergioBenitez]

This disables autodetection of AES-NI, falling back on a portable constant-time software implementation.

Did aes take advantage of AES-NI before on stable? If so, is there any way to regain this support?

[tarcieri]

@SergioBenitez it previously would use AES-NI only if the requisite target flags were explicitly enabled with:

RUSTFLAGS=-Ctarget-feature=+aes,+ssse3

The v0.9 release adds runtime autodetection on i686/x86-64 targets, checking CPUID for AES-NI (and CLMUL) availability and automatically selecting the AES-NI/CLMUL backends with no explicit configuration by the user.

Unfortunately, the strategy we used to implement this depends on some features introduced in Rust 1.49 (namely support for non-Copy unions using ManuallyDrop).

Unless the RUSTFLAGS were explicitly configured to enable these target features, however, there will be no user-facing change to using the force-soft feature.

[SergioBenitez]

Got it. I suspect -C target-cpu=native is a much more common setting that would have enabled AES-NI previously. Does static detection supersede dynamic detection even now? That is, if I compile with -C target-cpu=native, can I avoid the cost of dynamically checking for AES-NI on 0.9 and still use AES-NI? This seems like a good compromise to me.

[tarcieri]

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+.

[SergioBenitez]

Am I understanding correctly that this is a monotonic regression? Even if I compile with -C target-cpu=native, setting force-soft will result in AES-NI not being used, even though its presence can be statically determined, is that correct? If so, this feels rather unfortunate.

[tarcieri]

Yes, using force-soft will disable AES-NI support completely. I would only suggest it as a temporary stopgap until you can bump MSRV to 1.49+.

If you'd like to support AES-NI on MSRV 1.41+, I'd suggest sticking with aes-gcm v0.8 until you can bump MSRV.

[SergioBenitez]

Yes, using force-soft will disable AES-NI support completely.

This seems like an ill-fitted, perhaps incorrect use of features. Features should only be used to add capabilities, but force-soft removes the ability to make use of AES-NI. The consequence is that a consumer of aes has little control over whether AES-NI is actually used: all it takes is a single dependency, direct or indirect, enabling force-soft to disable AES-NI completely, without recourse.

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.

[tarcieri]

We've tried an autodetect feature:

https://github.com/RustCrypto/block-ciphers/commit/0fb7bb55fc851db10cade44de9eaf11b08441046

Here's some of the past discussion about it:

https://github.com/RustCrypto/block-ciphers/pull/22 https://github.com/RustCrypto/block-ciphers/issues/25

We settled on force-soft over autodetect as at least a temporary a compromise which allows feature autodetection to be enabled by-default, but with a clear and convenient way to opt out, after a lot of people were vocal about having a way to do so in order to, among other reasons, continue to support MSRV 1.41+:

https://github.com/RustCrypto/block-ciphers/pull/208 https://github.com/RustCrypto/block-ciphers/pull/216 https://github.com/RustCrypto/block-ciphers/pull/221 https://github.com/RustCrypto/block-ciphers/pull/220

A big part of why we use the current strategy is it provides an abstraction where target-feature overrides autodetection via cpufeatures and optimizes it away. This lets us solve the problem of "force AES-NI" in one place in the cpufeatures crate as a generally useful abstraction, as opposed to riddling our code with a bunch of #[cfg(target-feature = "...")] gating which I assure you we've done quite a bit of in the past.

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.

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:

always enable AES-NI if it is known to statically exist

Unless we lean on the autodetection codepaths to do this, we now have two cases to deal with:

1. target-feature=+aes is enabled, and autodetect is not
2. target-feature=+aes is enabled, and autodetect is also enabled

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 #[cfg(target_feature = "...")] gating. If you look at the commits/PRs I linked with an autodetect feature they were using a lot of complex cfg-if driven gating for this purpose. Now all of that has been abstracted away into the cpufeatures crate.

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 #[cfg(target_feature = "...")] gating is why it is the way it is today.

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 default = ["std"] feature activation directive in all of their transitive dependencies.

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 autodetect "feature" as a bit fraught, as it introduces a sort of quadrangle of feature activation states as permuted between target-feature and crate features, when really there only three effective states we actually care about:

1. soft
2. autodetect
3. AES-NI always on

[SergioBenitez]

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.

Unless we lean on the autodetection codepaths to do this, we now have two cases to deal with:

1. target-feature=+aes is enabled, and autodetect is not
2. target-feature=+aes is enabled, and autodetect is also enabled

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 #[cfg(target_feature = "...")] gating. If you look at the commits/PRs I linked with an autodetect feature they were using a lot of complex cfg-if driven gating for this purpose. Now all of that has been abstracted away into the cpufeatures crate.

Somewhere in the codebase, you already have 2 versions of the code. Let's call them aes::ni and aes::soft.

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 autodetect is additive; it adds the ability to dynamically detect AES-NI and removes nothing otherwise; there is no not(feature(autodetect)). This is the only fully correct use of features. If the code that requires 1.49 is restricted to detect, then MSRV can remain at 1.41 as long as autodetect is not enabled.

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 default = ["std"] feature activation directive in all of their transitive dependencies.

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 no-std environment, and here's what you'll miss out on."

[tarcieri]

Somewhere in the codebase, you already have 2 versions of the code. Let's call them aes::ni and aes::soft.

Effectively, we want this:

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 block-ciphers repo about this so yet another round of discussion about it is captured there.

[SergioBenitez]

This is now in 0.16.