threshold_crypto icon indicating copy to clipboard operation
threshold_crypto copied to clipboard

Consider zeroing and mlocking temporary values.

Open afck opened this issue 7 years ago • 3 comments
trafficstars

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.

afck avatar Sep 03 '18 12:09 afck

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.

DrPeterVanNostrand avatar Sep 03 '18 13:09 DrPeterVanNostrand

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.

DrPeterVanNostrand avatar Sep 14 '18 20:09 DrPeterVanNostrand

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.

afck avatar Dec 10 '18 09:12 afck