AES implementation should be chosen at runtime rather than compile time

[upsuper]

AES implementation should be chosen at runtime rather than compile time, otherwise it makes people very hard to ship products with this crate, because they cannot choose the environment a product would be run on.

[newpavlov]

Switch between implementations should be done as high-level as possible, not at the block cipher level. There is a nocapture feature for this use-case, but you will need to be sure to enable necessary target features for proper inlining.

[newpavlov]

I believe ideally we need a support for this at the language level. I've described my view here.

[tarcieri]

What I'd like in the short term:

• On platforms that have no aesni impl, use the software fallback
• On platforms that do (presently x86/x86_64) require the appropriate target features are enabled (i.e. +aes,+ssse3) and refuse to compile if they are not. Use runtime detection to determine if AES-NI is available, and if not, fall back to the software implementation

[tarcieri]

Why not [target_feature(enable = "aes")]

Because it silently falls back to only using the software implementation of AES. In fact, if you compiled with RUSTFLAGS=-Ctarget-feature=+aes today, that is exactly what will happen, because using aesni requires +aes,+ssse3, and +aes alone will silently fall back to aes-soft as the one and only option. It's the kind of "gotcha" users of this crate shouldn't have to worry about, and I think the best way to surface that at present is through a compile error.

I have plastered Miscreant with instructions to use RUSTFLAGS or ~/.cargo/config to enable the appropriate target features, but even then they changed, and I found myself having trouble configuring them to enable aesni and reconfiguring all of the places which were using just +aes instead of +aes,+ssse3. In absence of that, I am pretty sure nearly everyone using the aes crate is using aes-soft instead of aesni.

It would be nice if the ergonomics of target features were better, but failing that, if the aes crate silently falls back to aes-soft with no runtime AES-NI detection unless the target features are correctly configured, then nobody is going to bother to configure their builds correctly, and everything will use the software implementation. I would consider that a bad outcome, as runtime AES-NI detection is the norm for crypto libraries outside the Rust ecosystem.

[kazcw]

On platforms that do (presently x86/x86_64) require the appropriate target features are enabled (i.e. +aes,+ssse3) and refuse to compile if they are not. Use runtime detection to determine if AES-NI is available, and if not, fall back to the software implementation

I don't think that's sound. Once you enable +ssse3 for the whole crate, the compiler could choose to use ssse3 anywhere.

For groestl-aesni, I used a combination of:

• #[inline(always)] all the implementations
• sets of wrappers for different #[target_feature(enable = ...)]
• lazy_static selection between wrappers

It works fine; the version with aes detected at runtime runs just as fast as compiling with --target-feature=+aes, and much faster than the soft-aes path.
kazcw/groestl-aesni@d9fd06f

[tarcieri]

That's better than what this crate presently does, however unless I'm mistaken it still appears to require the crate's consumer to enable the +aes target feature, and if they don't do that, it will omit AES-NI support. Is that the case?

I think all builds for all x86(_64) targets should always attempt to detect and use AES-NI, and if the required target features aren't enabled, the crate should refuse to compile. That could potentially have a way to opt out for users who really don't want AES-NI, but the default should be to attempt to use it, IMO.

Barring that, I think no one will enable the target features and nothing will even attempt to use AES-NI.

[kazcw]

That's better than what this crate presently does, however unless I'm mistaken it still appears to require the crate's consumer to enable the +aes target feature, and if they don't do that, it will omit AES-NI support. Is that the case?

No, that's what #[target_feature(enable...)] does. If it doesn't work it's because the functions with the intrinsics didn't get inlined into the target_feature fn; to be sure it's effective you have to use #[inline(always)] all the way down the call stack.

[kazcw]

(Btw, the #[cfg(target_feature...)] stuff in groestl is kind of a red herring: I did that so if you do set --target-cpu and --target-feature for a custom build, it'll skip building impls inferior to whatever is configured at compile time; it's just a compile speed optimization)

[tarcieri]

Interesting. I’ll have to look into it more.

[kazcw]

Gah. What I've been doing doesn't actually work. I was fooled by rust-lang/stdarch#323, and the functions I thought were LLVM's fallbacks are actually just Rust's pointless wrappers. The bug is here: cryptocorrosion/cryptocorrosion#7

[tarcieri]

@kazcw oof, sorry to hear, almost thought you had this one licked!

Any plans for how to address it?

[kazcw]

@tarcieri Yeah, it won't be too bad. The upshot is this is forcing me to put everything through traits, which was on the roadmap anyway (cryptocorrosion/cryptocorrosion#6) but I'd been putting it off because ad-hoc polymorphism by switching out imports took zero work. But it's not turning out to be as hard as I thought, and the new API is cleaner (the user parameterizes code that will be compiled with different features sets by a Machine trait, whose impls are ZSTs that define a set of types indicating what kind of registers to use, what cpu flags are enabled, etc; possessing a particular Machine instance is used as a marker that its associated types can safely be created). This also removes the barrier to AVX2, or rather what I needed to do to get AVX2 working turns out to also be what I need to make everything else work anyway.

I already have ports of JH and BLAKE working on the new API. I can probably do ChaCha later today, and then I'll release the new versions, with packed_simd disabled for now. Putting packed_simd through the traits will be about as much work as implementing them on coresimd has been, and packed_simd is less crucial now that ppv-lite86 can do AVX2.