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

Box zeroize to prevent leaving copies on move. #358

Merged
merged 2 commits into from Jun 22, 2020
Merged

Conversation

tomusdrw
Copy link
Owner

CC @cheme

@tomusdrw tomusdrw merged commit e98780b into master Jun 22, 2020
@tomusdrw tomusdrw deleted the td-zeroize-boxe branch June 22, 2020 15:39
Copy link
Contributor

@That3Percent That3Percent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good step to fix the leaking of private data, but is not yet foolproof.

Consider for example that the API:

pub fn boxed(key: SecretKey) -> Box<Self> {
         Box::new(Self(key))
     }

May create copies of the SecretKey on the stack. The optimizer might remove some of these copies, but that's depending on whether the function is inlined which is not deterministic when there are multiple codegen units.

I've implemented a version which copies from the reference after the memory is allocated here:
https://github.com/graphprotocol/solidity-bindgen/blob/master/solidity-bindgen/src/secrets.rs

The relevant section is in SafeSecretKey::new(secret: &SecretKey) which allocates first, then copies from a reference into the allocation. In that implementation, it goes the extra mile to mlock the memory to avoid paging to disk as well.

If you like I can pull SafeSecretKey out of solidity-bindgen and into a crate for use in web3. Just say the word. Or, feel free to copy any code out of it into here.

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

Successfully merging this pull request may close these issues.

None yet

2 participants