Fill BlindingFactor with zeros on Drop

[eupn]

This is a first (and probably naïve) step towards security of sensitive data such as BlindingFactor.

In response to #2218. Cleaning of secp's SecretKey and wallet's mnemonic should be done in corresponding modules.

This PR:

• Adds zeroize crate as a dependency for keychain
• Adds Zeroize derive for BlindingFactor in a way that underlying byte array is filled with zeroes
• Removes Copy derive from BlindingFactor since it is impossible to implement both Drop and Copy at the same time. All implicit copying were replaced with more explicit .clone()
• Adds unit test for BlindingFactor zeroing

Some references:
dalek-cryptography/curve25519-dalek#11
rust-lang/rfcs#2533
https://www.youtube.com/watch?v=cQ9wTyYCdNU

Feedback is highly appreciated!

[hashmap]

We use zeroize crate in some parts of grin, would you consider using it?

[elichai]

FYI,
Every time you pass this struct by value it's get copied in memory without calling the Drop trait.

The better way to implement it is using the Pin struct or just putting it in a smart pointer(i.e Box) so that only the pointer will be copied and not the data itself

[hashmap]

@elichai could you elaborate on why drop is not called when a cloned value gets out of its scope?

[elichai]

@hashmap because semantically when you pass something by value it's not dropped.
In reality the compiler will prefer to copy the data when passing by value (especially if it's on the stack)

Example https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=edc392a036c0d880224f0a94d6336f69

[garyyu]

@elichai 👍
and that's exact what I'm worrying, as comments in #2851 (comment). thanks for a nice playground example to show that.

The better way to implement it is using the Pin struct or just putting it in a smart pointer(i.e Box) so that only the pointer will be copied and not the data itself

Could you please also demo this in your example?
And another method to solve this is always use the explicit b = a.clone() instead of b = a.

[elichai]

@garyyu With Box: https://play.rust-lang.org/?gist=26f5b9609348f4b6e287bf808ab0161a

The problem is that even though it works it's not guaranteed, Wrapping the Box in Pin guarantees it (e.g. https://play.rust-lang.org/?gist=3a3fabc5c44b3017b64dee1d22169d45 )

[cadmuspeverell]

Maybe we could also use mlock to prevent paging to disk?

[elichai]

I think that paging is a whole different conversation that deserves its own issue

[garyyu]

@elichai

The problem is that even though it works it's not guaranteed, Wrapping the Box in Pin guarantees it

👍 Thanks, now I understand. Would you mind to submit PRs to fix this?