Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider zeroing and mlocking temporary values. #24

Open
afck opened this issue Sep 3, 2018 · 3 comments
Open

Consider zeroing and mlocking temporary values. #24

afck opened this issue Sep 3, 2018 · 3 comments

Comments

@afck
Copy link
Collaborator

afck commented Sep 3, 2018

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.

@DrPeterVanNostrand
Copy link
Contributor

DrPeterVanNostrand commented Sep 3, 2018

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
Copy link
Contributor

DrPeterVanNostrand commented Sep 14, 2018

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.

@afck
Copy link
Collaborator Author

afck commented Dec 10, 2018

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.

@DrPeterVanNostrand DrPeterVanNostrand removed their assignment Dec 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants