-
Notifications
You must be signed in to change notification settings - Fork 11
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
Implement Zeroize
for SecretKey
#158
Conversation
3ab3073
to
cb31fa7
Compare
Resolves #155
cb31fa7
to
cba22af
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seeing all the PublicKey::from(&sk)
make me feel there should be a SecretKey::public
function. That said, not the purpose here. LGTM
impl From<SecretKey> for ViewKey { | ||
fn from(sk: SecretKey) -> Self { | ||
Self::from(&sk) | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's quite the catch on the by-copy. Nice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed! Will have some ripple effects downstream, but good ones!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/// | ||
/// To ensure that no secret information lingers in memory after the variable | ||
/// goes out of scope, we advice calling `zeroize` before the variable goes out | ||
/// of scope. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would implementing Drop
and calling zeroize
there solve this problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly not. I investigated the problem in our bls signature library: dusk-network/bls12_381-bls#6
long story short: rust still often copies values internally, so the zeroize on drop often doesn't result in actually erasing the desired values.
Resolves #155