dalek-cryptography / curve25519-dalek

A pure-Rust implementation of group operations on Ristretto and Curve25519
Other
884 stars 448 forks source link

curve: use derive macro for zeroizing #670

Closed AaronFeickert closed 3 months ago

AaronFeickert commented 3 months ago

Several types implement Zeroize manually. It seems safer to do this via the supplied derive macro, since any field changes will be caught automatically.

This PR moves to this functionality.

tarcieri commented 3 months ago

Note: the proc macro stack is not currently a hard dependency of curve25519-dalek (this is where having Cargo.lock checked in would be useful for reviewing the effects of dependency changes, particularly in conjunction with GitHub bots that can help monitor / analyze them)

Enabling this feature, while it would save 50 lines and automatically catch field additions / removals, would also pull in the following additional dependencies which are not currently required:

Now granted, these are commonly used dependencies, but it is a pretty big increase in total dependency surface.

Regarding the problem of field additions/changes: I don't think that's a particularly big problem in practice with this set of types, which are more or less all in their "final form".

AaronFeickert commented 3 months ago

That's a fair point. Happy to close if you don't think the added baggage is worth it in this case. (Or a maintainer can close if they wish.)

tarcieri commented 3 months ago

I think the extra dependencies probably aren't worth the ~50loc it saves in this case, although that could change in the future if there are more usages of zeroize.

AaronFeickert commented 3 months ago

Noted! Closing.