You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
There was a long discussion regarding the requirement of implementing zeroize for the Share struct in sta-rs in #24. Overall, we decided that it wasn't urgent to address this for now since secret shares are uniformly random until paired with k other shares (at which point the secret is regarded as known). However, we may want to come up with a more solid strategy for addressing this. @rillian provided more context in the following comment:
If we're not treating Share as sensitive, perhaps there's nothing to do in this particular library. At best the caller might benefit from a zeroize() method implementation as you had before so they don't have to wrap it.
Rather, I'd look at where we might have k shares collected together. if they're passed by reference or returned by value, that's fine, but the idea of ZeroizeOnDrop is to not leave any intermediate copies laying around in de-allocated memory. It's possible interpolate is doing that, but hopefully not. I'd have to step through it to be sure.
More clear is Sharks::recover() which copies the x values from the shares into a hash set and clones a matching set of Share structs into a local Vec for interpolation. When that's dropped, a complete set of data for recovering the key is left sitting on the heap. If we're serious about implementing Zeroize internally, those two variables would be a good candidates.
There was a long discussion regarding the requirement of implementing
zeroize
for theShare
struct in sta-rs in #24. Overall, we decided that it wasn't urgent to address this for now since secret shares are uniformly random until paired withk
other shares (at which point the secret is regarded as known). However, we may want to come up with a more solid strategy for addressing this. @rillian provided more context in the following comment:See the merged PR for the full discussion.
The text was updated successfully, but these errors were encountered: