RustCrypto / utils

Utility crates used in RustCrypto
439 stars 129 forks source link

Zeroize: Discrepency between `[Z; N]`, ``[Z]`, `Vec<Z>`, `Box<[Z]>` #899

Closed elichai closed 1 year ago

elichai commented 1 year ago

Right now [Z] only implements Zeroize if Z: DefaultIsZeroes[0]. While Vec<Z>[1], Box<[Z]>[2], and even [Z; N][3] all implement Zeroize if Z: Zeroize.

The justification for impl<Z: DefaultIsZeros> Zeroize for [Z] is given here: https://github.com/RustCrypto/utils/blob/017165f4c509ed137f0afbafac59215cb20490c7/zeroize/src/lib.rs#L455-L457 To me it feels like they should all behave ~the same with regard to zeroing

[0] https://docs.rs/zeroize/1.6.0/zeroize/trait.Zeroize.html#impl-Zeroize-for-%5BZ%5D [1] https://docs.rs/zeroize/1.6.0/zeroize/trait.Zeroize.html#impl-Zeroize-for-Vec%3CZ%3E [2] https://docs.rs/zeroize/1.6.0/zeroize/trait.Zeroize.html#impl-Zeroize-for-Box%3C%5BZ%5D%3E [3] https://docs.rs/zeroize/1.6.0/zeroize/trait.Zeroize.html#impl-Zeroize-for-%5BZ;+N%5D

elichai commented 1 year ago

Maybe one day we could use specialization for these kinds of stuff

tarcieri commented 1 year ago

That stated reason for the slice impl is very important. It's what permits the sort of optimizations we're exploring in #841 without specialization.

It's deliberately inconsistent with the owned types in that regard, and also if you wanted to use the optimized implementation to zero the others, you can always obtain a slice.

elichai commented 1 year ago

It's deliberately inconsistent with the owned types in that regard

Could you expand on that?

tarcieri commented 1 year ago

...to permit vectorized optimizations, as you already identified

elichai commented 1 year ago

...to permit vectorized optimizations, as you already identified

Wouldn't we also want to do vectorized optimizations for the owned variants?

elichai commented 1 year ago

Maybe DefaultIsZeroes should be an associated const bool in Zeroize and then we can "branch" over it? (which will be optimized away post-monomorphization)

tarcieri commented 1 year ago

As I just mentioned, if you want to take advantage of that optimization for the owned variants in the case the type is DefaultIsZeros, you can call .as_mut().zeroize().

Likewise, if you have a mut slice you want to zero for a type which is Z: Zeroize but !DefaultIsZeros, you can do slice.iter_mut().zeroize(). So it's possible to go either way while still permitting an optimized implementation.

Without specialization I don't think you can do much better than this. Also zeroize has been 1.0 since 2019, so it's a little late for breaking changes.

elichai commented 1 year ago

As I just mentioned, if you want to take advantage of that optimization for the owned variants in the case the type is DefaultIsZeros, you can call .as_mut().zeroize().

My main concern with that is that at least I usually use this crate via either the derive macros or the derive macro combined with zeroize::Zeroizing so if I do #[derive(Zeroize, ZeroizeOnDrop)] struct MySecret(Box<[u8]) then it won't get the optimized version

tarcieri commented 1 year ago

We can potentially change the derive macro to support calling some sort of mutable slice accessor like as_mut when a given attribute is provided.

Other than that I'm not sure there's a solution other than specialization.

(also note we haven't landed any optimized implementation yet, so you're not missing anything)

tarcieri commented 1 year ago

I opened #901 to track custom derive support for AsMut.

Otherwise this is working as intended, and probably won't be revisited until specialization is stable.