threshold_crypto
threshold_crypto copied to clipboard
Consider zeroing and mlocking temporary values.
Our cryptographic data structures are munlocked and zeroed on drop, and mlocked on creation. In some calculations, however, we create temporary values from which secrets could be computed. Can we make sure none of this leaks, ideally without polluting the code with lots of explicit method calls for mlocking and zeroing, and without unreasonable overhead?
If I wrap a temporary value in SecretKey, it should be safe, right? Should we split that functionality out of SecretKey into a general Secret that mlocks and zeroes? We'd then have SecretKey(Secret(Fr)), but it would be less weird to use Secrets for local variables.
And we'd probably need something similar for Vecs.
Yup, as long as your temporary value is an Fr, you could pass it into a SecretKey and it will be locked, unlocked, and zeroed-on-drop because SecretKey implements ContainsSecret. That is an abuse of the SecretKey API though.
Adding a new type Secret<T> that implements ContainsSecret for Secret<Fr>, Secret<Vec<Fr>>, and Secret<Box<Fr>> would be better suited for doing arithmetic on a field element.
SecretKey, Poly, and BivarPoly implement ConatinsSecret, which means we have the lock, unlock, and zero-on-drop functionality already written for Fr, Box<Fr> and Vec<Fr>. Adding the Secret type would require some refactoring, but I don't believe that it would require any new logic.
The Safe type was added to wrap mutable references and boxes to temporary values to ensure that they are locked and cleared. Safe should be added to all instances of temporary values that are meant to be kept secret. Leaving this issue open until Safe has been added to the code accordingly.
One remaining example is the local variable g in SecretKey::decrypt. It should be zeroed, too, and we should check the code for further issues like that.