zeroize_derive: Require zeroize(drop) or zeroize(no_drop) #212
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The previous version of `zeroize` (v0.8) automatically derived a `Drop` impl unless `zeroize(no_drop)` was passed explicitly. This felt like the right thing to do at first, however it turns out there are a lot of cases in practice where it's undesirable to derive `Drop` by default, e.g. on any `Copy` type. This was encountered in-practice retrofitting `zeroize` into the `curve25519-dalek` crate. This changes the custom derive impl to require an explicit decision in the form of an attribute as to whether a `Drop` impl should be derived or not. Going forward (i.e. in `zeroize` 1.0) deriving `Drop` will need to be explicitly declared as `zeroize(drop)`, and the need to explicitly specify `zeroize(no_drop)` can be removed, meaning no `Drop` impl will be derived by default. However, since previous users of `zeroize` v0.8 will be expecting `Drop` by default, this change makes it an explicit decision, so as to avoid people expecting to get a `Drop` impl by default not actually receiving one. Once crates.io stats show `zeroize` v0.8 is no longer in use (or in `zeroize` 1.0, whichever comes first), the need to specify explicitly that no `Drop` handler should be derived via `zeroize(no_drop)` can be removed, however until then this change does the safely-explicit-but-annoying thing by default so as to correct for previous API mistakes.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The previous version of
zeroize(v0.8) automatically derived aDropimpl unlesszeroize(no_drop)was passed explicitly.This felt like the right thing to do at first, however it turns out there are a lot of cases in practice where it's undesirable to derive
Dropby default, e.g. on anyCopytype. This was encounteredin-practice retrofitting
zeroizeinto thecurve25519-dalekcrate.This changes the custom derive impl to require an explicit decision in the form of an attribute as to whether a
Dropimpl should be derived or not.Going forward (i.e. in
zeroize1.0) derivingDropwill need to be explicitly declared aszeroize(drop), and the need to explicitly specifyzeroize(no_drop)can be removed, meaning noDropimpl will be derived by default. However, since previous users ofzeroizev0.8 will be expectingDropby default, this change makes it an explicit decision, so as to avoid people expecting to get aDropimpl by default not actually receiving one.Once crates.io stats show
zeroizev0.8 is no longer in use (or inzeroize1.0, whichever comes first), the need to specify explicitly that noDrophandler should be derived viazeroize(no_drop)can be removed, however until then this change does the safely-explicit-but-annoying thing by default so as to correct for previous API mistakes.