Box secret memory

[cheme]

This pr box memory for Secret.

[ordian]

LGTM.

[ordian]

fmt should make travis happy

[ordian]

Reading https://doc.rust-lang.org/src/alloc/boxed.rs.html#1074-1097, I'm actually not sure if Pin<Box<T>> is different from Box<T> for our use-case.

[cheme]

Yes indeed, let's remove it.

[ordian]

LGTM. Just needs a second approval.

[tomusdrw]

lgtm % TryFrom stuff

[tomusdrw]

Both TryFrom implementations can't really guarantee that the source slice is zeroized, there is no way to inform the user that it should be.
Not a security expert, but perhaps it's worth to get rid of these in favour of some more explicit methods which would allow you to add extra security notes.

[cheme]

I am more having an issue with the From implementation, try_from never zeroized the input, but yes we do not have any related doc.
I will be in favor of replacing those from and try_from implementation with specific functions, but then it would be a breaking PR, so I am thinking this PR could be parity-crypto 6.2 and another one could be a 7.0 with a more significant work for library users.

[cheme]

Actually that is not such a big change, maybe doing it now and releasing breaking 0.7.0 would be better, I will do it.

[tomusdrw]

Sounds good to me!

[drahnr]

I am a big fan of deprecation warnings, so people get a hint on how to migrate directly from the build - that could be applied to the TryFrom impl while exposing a more explicit API already

[cheme]

Could try 6.2 with dep warning indeed.

[drahnr]

From my understanding of Pin, as already stated by @ordian and @cheme, there is no advantage in using it in this context.

[arkpar]

Why does it need to be boxed? Stack memory can be zeroed in drop handler just as well.
Wouldn't type Secret = zeroize::Zeroizing<H256> work?

[cheme]

We would need to pin memory to avoid mem copy which does not looks trivial to me (I need to check again but it would mean following a &'a H256 since H256 is not Deref), box was more straightforward.

[cheme]

In 8ddd0da I am putting the 'from' 'try_from' function as deprecated, following this I switched usage in the crate and I fear there is a possible performance regression in ec_math_utils du to zeroize.
Furthermore there are part of the code that do not apply the zeroize (but it would require changing api which would not be ok for a 6.2).
maybe we should stick to only add the deprecated info and the new functions, or revert it
CC\ @ordian @tomusdrw @drahnr @arkpar

Edit : also it includes codes from @tomusdrw rustweb3 for zeroing secp secret key memory

[drahnr]

It depends on what kind of release you want to slate and what kind of implications a deprecations has in code that uses it?
Regarding perf, did you measure how much it actually costs? I think the cost is something you have to live with, security takes precedence, if perf is bad, the way of zeroing should be revisited, not the if keys should be zeroed.

[cheme]

Lastest commit I did switch back some of the previous api by simply take advantage of #[inline(always)].