Uh oh!
There was an error while loading. Please reload this page.
sha3: Add derive Zeroize for Sha3State - #479
Conversation
tarcieri
commented
May 19, 2023
Yes, behind a feature sounds good. If you avoid using the derive macro and write the impl by hand (which seems simple enough in this case), you can avoid the |
aewag
commented
May 19, 2023
@tarcieri Thanks for the feedback. I updated the PR and added the |
| #[cfg(feature = "zeroize")] | ||
| impl Zeroize for Sha3State { | ||
| fn zeroize(&mut self) { | ||
| self.state.zeroize(); | ||
| } | ||
| } | ||
| #[cfg(feature = "zeroize")] | ||
| impl Drop for Sha3State { | ||
| fn drop(&mut self) { | ||
| self.zeroize(); | ||
| } | ||
| } |
There was a problem hiding this comment.
I would suggest either impl'ing the Zeroize trait orDrop+ZeroizeOnDrop.
There was a problem hiding this comment.
I updated the PR with an impl of Drop + ZeroizeOnDrop.
6fe5ee3 to
631bec3CompareNot zeroizing the Sha3State allows to recover any squeezed output. This is because the `keccak` permutations can be inversed. Hence, access to the complete state allows to perform this operation. While this is security-relevant, including it would significantly increase the MSRV. Therefore, it is gated behind the `zeroize` feature.
newpavlov
left a comment
There was a problem hiding this comment.
In future we probably also should add zeroize feature to digest, which would depend on RustCrypto/utils#832, but we can do it in later PRs.
Not zeroizing the Sha3State allows to recover any squeezed output. This is because the
keccakpermutations can be inversed. Hence, access to the complete state allows to perform this operation.While this is security-relevant, including it would significantly increase the MSRV. Therefore, it is gated behind the
zeroizefeature.@tarcieri what do you think about gating the zeroizing behind a feature?
The
asconimplementation would "require" zeroizing as well. As the implementations differ, zeroizing has to be done in thespongescrate. Therefore, I would prepare a similar PR for thespongesrepository targetingascon.