hashes
hashes copied to clipboard
RustCrypto
Zero memory on drop
I have a question about the hash memory management but also for other related repositories. As it seems RustCrypto use GenericArray and/or raw arrays for sensitive data like keys and internal buffers but the memory is not overwritten/zeroed on reset or when the buffer/struct is dropped for most crates, right? Are there any plans to change this?
Yes, you are right, currently most crates do not zeroize inner state on drop. IIUC to implement it without hacks we will need either generic-array support (e.g. by implementing Zeroize trait.) or const generics.
@newpavlov you can do a Zeroize impl without explicit support for generic-array by borrowing the contents of the latter as a byte slice and calling zeroize() on that. IterMut will also work.
@tarcieri Maybe we can transform the whole hash state into byte slice (using unsafe code) and call zeroize on it?
@newpavlov or you can just call zeroize on each value in its state in a Drop handler. zeroize is designed to support structured data. If they're GenericArray you just need to call as_ref() (or as_slice()?) first iirc
When const generics land it should be possible to derive a Zeroize impl and drop handler using zeroize_derive (the same thing would be possible with first-class generic-array support for zeroize, but that doesn't exist).
@fxha is there a particular hash function or set of them you'd specifically like to have this in?
We can add optional zeroize features to high-priority crates.
I think it's good practice to (optionally) erase secret data and don't leak keys in memory.
I agree (in fact I wrote the most popular crate for it).
However hash functions are a bit of an odd fish in this regard. Ones based on the Merkle–Damgård construction (which constitute a large number of the ones in this repo, including the popular sha2) effectively output their internal state upon finalization (which is what enables length extension attacks). If that internal state were reversible to a secret, that would be a preimage attack against the underlying hash function.
I think the hash functions where this would be relevant are, as you pointed out, ones with keys, i.e. ones which provide PRFs.
As far as I know the only one in this particular repo that applies to is blake2.
Zeroize cannot prevent the type from being used after zeroing. Internal states often include constants, so zeroizing can leave an internal state that functions but provides less security.
@tarcieri I don't quite remember, but I was using Blake2 and noticed that my key was copied or something like this. Cleaning the internal state isn't a high priority but basic cleanup should be provided.
I wonder if we should make the reset capability optional... It would allow us to remove the caching of initial state and thus secret key inside hasher state will be overwritten as soon as first block of data has been processed. How often in your opinion users need to reset MAC states?
How about adding the Zeroize trait for blake2 at least? I am trying to implement a noise-like protocol where hmac-blake2 is used as a KDF. Right now I am using libsodium including sodium_malloc/free to make sure ephemeral secrecy is guaranteed.
The lack of well thought-through zeroization in RustCrypto is a major blocker for moving away from sodium.
I think an optional zeroize feature which adds a Drop impl that zeroizes the relevant state is probably the best approach, along with a ZeroizeOnDrop impl.
The problem with Zeroize is it might leave the hasher in an invalid state without consuming it.
Hi! bumping this again because a lot of project effectively need digest::Digest to implement zeroize::Zeroize. Do you need help with this?
- Impl
Zeroizefor Digest such that afterzeroize(), it writes the valid state. If theZeroizeimpl is ordered with respect to the post-write, then this will leave the hasher in a valid state. While this post-write may be optimized out if unused, if reused, it should be valid. No?
(I'm not sure this is valid due to the ordering pre-condition. Doesn't Zeroize use a volatile only guaranteed to be ordered with regard to other volatiles? If so, I think that'd make all use of Zeroize unsafe, so I doubt it, but I obviously may be missing something...)
- I would love
Zeroizeon all items satisfyingDigestbecause the resulting state is a secret. It's used as a nonce in a variety of systems. This leaves me to floodDigestwithblock sizeof data (if a block based hash), attempting to overwrite the prior state, and then get the resulting Digest marked used (to prevent the block of data written from being optimized out).
Doesn't Zeroize use a volatile only guaranteed to be ordered with regard to other volatiles?
You are thinking of some outdated guidelines. Please read this section of the zeroize documentation:
https://docs.rs/zeroize/latest/zeroize/#what-guarantees-does-this-crate-provide
In that case, the sole question is if a further write is guaranteed to be ordered, and if not, how to trigger a read it is ordered in regard to (not my field of expertise). Regardless, sounds like my proposed solution could/should work?
I would probably suggest just adding a Drop impl, rather than a Zeroize impl (you could also add a ZeroizeOnDrop impl).
The problem with adding a Zeroize impl is there's a potential "use-after-zeroize" condition that can occur, which can't occur with a Drop impl.
Edit: oops, I already said this
Right, I read the history, yet if after the zeroize we re-write the starting state... it's use-after-init-after-zeroize. That shouldn't introduce any unsafety.
It isn't a "safety" issue in the Rust sense, but the problem is zeroize might be called on it, then it could be reused in a context where the zeroization was unexpected.
Though I suppose there's an argument to be made that it's no different from Reset::reset, and adding a Zeroize impl would not penalize users who don't care about zeroization.
If a user explicitly calls Zeroize, which is just a stronger reset in this context, and then it's reused 'unexpectedly', I'd argue that's the users fault, yes.
it's use-after-init-after-zeroize. That shouldn't introduce any unsafety.
It's a logical bug. Zeroized state may not be valid for a given hash function. In the worst case scenario, it may even break safety assumptions used by implementation.