Implement Zeroize

[vlad9486]

The type ChaCha20Poly1305 should implement Zeroize. Also, I do not sure if AesGcm should implement Zeroize too. It will allow the user to implement Zeroize for types containing them.

[tarcieri]

Just a quick note on this: will revisit it soon. It's a bit tricky as both of those ciphers have SIMD backends, and right now there are no Zeroize impls for the SIMD register types, so doing this properly needs support for zeroizing SIMD types in zeroize first.

[kurnevsky]

Looks like NewAead::new should take key by value, otherwise it won't be zeroed properly.

[tarcieri]

Other way around: taking it by value can make a copy via a move, making the old version impossible to zero out.

[kurnevsky]

Yeah, looks like you are right. So this means currently there is no way to use aead ciphers safely if you don't zero memory manually...

[tarcieri]

FWIW, aead 1.0 took the key by value, however pretty much everything that key would get passed to next (e.g. block cipher or stream cipher) performs some sort of key expansion / round key setup / state initialization which accepts the key by reference.

It felt much cleaner to accept a key by reference.

No matter what, the caller needs to deal with the key somehow. In some of the other crates (notably elliptic-curve) there are key types which handle loading keys from on disk (from e.g. PKCS#8) as well as zeroization. But note this is still all best effort and unless pinned or stored on the heap it's possible for copies of these keys to be made unintentionally right now.

[kurnevsky]

I actually had in mind a case when we need to create aead from rng, with no need to store the key anywhere except memory. The only way to do it now is to generate a new key, wrap it to aead and then zero key buffer. Not sure though how common this case is, or if it indeed requires memory zeroing, but still it feels like there should be an easy way to wrap a newly generated key. Probably key fields in aeads can be made public so we can just create it as a struct?

[kurnevsky]

On the second thought if a key is stored in a stack then it'll likely be moved, and drop won't help in this case. So probably we shouldn't care that much about stack, and if we do we can use something like https://docs.rs/clear_on_drop/0.2.4/clear_on_drop/fn.clear_stack_on_return.html

[tarcieri]

Probably key fields in aeads can be made public so we can just create it as a struct?

That would still be a move which would copy the key to within the struct. Also as I mentioned earlier, often the key is transformed in some way prior to use, e.g. expanded into per-round keys as in AES.

Since the NewAead API takes it by reference, it won't be moved, so you can create value on the stack and use it to initialize the AEAD, then zero it out with zeroize().

[kurnevsky]

That would still be a move which would copy the key to within the struct.

Actually it won't, because this struct is a simple wrapper. The only case I can imagine of is when key is stored in a Box or similar and being moved to a stack. But again, this can happen with aead struct itself and Drop trait won't help here.

I also checked a case when we create a random buffer and pass it to a constructor by reference where we store it - rustc/llvm can optimize it to not to copy the buffer. Here what I wrote: https://play.rust-lang.org/?version=stable&mode=release&edition=2018&gist=81662cc073f5ea50483d9e08f99826ae - rustc generates the same assembly when we pass parameter by value or by reference. So it means we don't need to zero key manually in such case. But still it can be moved later, if we store it in a heap, for example.

Update: no, rust can't optimize it and copies memory :(
https://play.rust-lang.org/?version=stable&mode=release&edition=2018&gist=8fbba37c53741ec4ec1484bf8d6631ab

[kurnevsky]

Here found a blogpost on this: https://benma.github.io/2020/10/16/rust-zeroize-move.html
And since aead stores data in stack it applicable to it as well.

[tarcieri]

Actually it won't, because this struct is a simple wrapper.

It depends. The struct could be stored in a Box. With a non-opaque struct, It could also be initialized with values returned from another function, which would definitely incur a copy until placement-by-return lands.

This is why using references provides better guarantees. You know any data passed by reference won't be copied.