Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Implement ZeroizeOnDrop #699

Merged
merged 1 commit into from Jan 5, 2022
Merged

Implement ZeroizeOnDrop #699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
125 changes: 109 additions & 16 deletions zeroize/derive/src/lib.rs
View file
Open in desktop
Expand Up @@ -22,7 +22,7 @@ decl_derive!(
/// Supports the following attributes:
///
/// On the item level:
/// - `#[zeroize(drop)]`: call `zeroize()` when this item is dropped
/// - `#[zeroize(drop)]`: *deprecated* use `ZeroizeOnDrop` instead
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To properly deprecate this I think it'd be good to emit a #[deprecated(...)] attribute in the generated code, although I'm fine with addressing that in a follow-up PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am looking into it.

/// - `#[zeroize(bound = "T: MyTrait")]`: this replaces any trait bounds
/// inferred by zeroize-derive
///
Expand All @@ -31,6 +31,18 @@ decl_derive!(
derive_zeroize
);

decl_derive!(
[ZeroizeOnDrop, attributes(zeroize)] =>

/// Derive the `ZeroizeOnDrop` trait.
///
/// Supports the following attributes:
///
/// On the field level:
/// - `#[zeroize(skip)]`: skips this field or variant when calling `zeroize()`
derive_zeroize_on_drop
);

/// Name of zeroize-related attributes
const ZEROIZE_ATTR: &str = "zeroize";

Expand All @@ -55,6 +67,29 @@ fn derive_zeroize(mut s: synstructure::Structure<'_>) -> TokenStream {
}
}

/// Custom derive for `ZeroizeOnDrop`
fn derive_zeroize_on_drop(mut s: synstructure::Structure<'_>) -> TokenStream {
let zeroizers = generate_fields(&mut s);

let drop_impl = s.gen_impl(quote! {
gen impl Drop for @Self {
fn drop(&mut self) {
match self {
#zeroizers
}
}
}
});

let zeroize_on_drop_impl = impl_zeroize_on_drop(&s);

quote! {
#drop_impl

#zeroize_on_drop_impl
}
}

/// Custom derive attributes for `Zeroize`
#[derive(Default)]
struct ZeroizeAttrs {
Expand Down Expand Up @@ -216,6 +251,23 @@ impl ZeroizeAttrs {
}
}

fn generate_fields(s: &mut synstructure::Structure<'_>) -> TokenStream {
s.bind_with(|_| BindStyle::RefMut);

s.filter_variants(|vi| {
let result = filter_skip(vi.ast().attrs, true);

// check for duplicate `#[zeroize(skip)]` attributes in nested variants
for field in vi.ast().fields {
filter_skip(&field.attrs, result);
}

result
})
.filter(|bi| filter_skip(&bi.ast().attrs, true))
.each(|bi| quote! { #bi.zeroize(); })
}

fn filter_skip(attrs: &[Attribute], start: bool) -> bool {
let mut result = start;

Expand All @@ -239,21 +291,7 @@ fn filter_skip(attrs: &[Attribute], start: bool) -> bool {

/// Custom derive for `Zeroize` (without `Drop`)
fn derive_zeroize_without_drop(mut s: synstructure::Structure<'_>) -> TokenStream {
s.bind_with(|_| BindStyle::RefMut);

let zeroizers = s
.filter_variants(|vi| {
let result = filter_skip(vi.ast().attrs, true);

// check for duplicate `#[zeroize(skip)]` attributes in nested variants
for field in vi.ast().fields {
filter_skip(&field.attrs, result);
}

result
})
.filter(|bi| filter_skip(&bi.ast().attrs, true))
.each(|bi| quote! { #bi.zeroize(); });
let zeroizers = generate_fields(&mut s);

s.bound_impl(
quote!(zeroize::Zeroize),
Expand All @@ -277,16 +315,25 @@ fn derive_zeroize_with_drop(s: synstructure::Structure<'_>) -> TokenStream {
}
});

let zeroize_on_drop_impl = impl_zeroize_on_drop(&s);

let zeroize_impl = derive_zeroize_without_drop(s);

quote! {
#zeroize_impl

#[doc(hidden)]
#drop_impl

#zeroize_on_drop_impl
}
}

fn impl_zeroize_on_drop(s: &synstructure::Structure<'_>) -> TokenStream {
#[allow(unused_qualifications)]
s.bound_impl(quote!(zeroize::ZeroizeOnDrop), Option::<TokenStream>::None)
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -369,6 +416,12 @@ mod tests {
}
}
};
#[allow(non_upper_case_globals)]
#[doc(hidden)]
const _DERIVE_zeroize_ZeroizeOnDrop_FOR_Z: () = {
extern crate zeroize;
impl zeroize::ZeroizeOnDrop for Z {}
};
}
no_build // tests the code compiles are in the `zeroize` crate
}
Expand Down Expand Up @@ -439,6 +492,46 @@ mod tests {
}
}

#[test]
fn zeroize_only_drop() {
test_derive! {
derive_zeroize_on_drop {
struct Z {
a: String,
b: Vec<u8>,
c: [u8; 3],
}
}
expands to {
#[allow(non_upper_case_globals)]
const _DERIVE_Drop_FOR_Z: () = {
impl Drop for Z {
fn drop(&mut self) {
match self {
Z {
a: ref mut __binding_0,
b: ref mut __binding_1,
c: ref mut __binding_2,
} => {
{ __binding_0.zeroize(); }
{ __binding_1.zeroize(); }
{ __binding_2.zeroize(); }
}
}
}
}
};
#[allow(non_upper_case_globals)]
#[doc(hidden)]
const _DERIVE_zeroize_ZeroizeOnDrop_FOR_Z: () = {
extern crate zeroize;
impl zeroize::ZeroizeOnDrop for Z {}
};
}
no_build // tests the code compiles are in the `zeroize` crate
}
}

#[test]
fn zeroize_on_struct() {
parse_zeroize_test(stringify!(
Expand Down
35 changes: 29 additions & 6 deletions zeroize/src/lib.rs
View file
Open in desktop
Expand Up @@ -66,26 +66,30 @@
//! which automatically calls `zeroize()` on all members of a struct
//! or tuple struct.
//!
//! Additionally it supports the following attributes:
//! Attributes supported for `Zeroize`:
//!
//! On the item level:
//! - `#[zeroize(drop)]`: call `zeroize()` when this item is dropped
//! - `#[zeroize(drop)]`: *deprecated* use `ZeroizeOnDrop` instead
//! - `#[zeroize(bound = "T: MyTrait")]`: this replaces any trait bounds
//! inferred by zeroize
//!
//! On the field level:
//! - `#[zeroize(skip)]`: skips this field or variant when calling `zeroize()`
//!
//! Attributes supported for `ZeroizeOnDrop`:
//!
//! On the field level:
//! - `#[zeroize(skip)]`: skips this field or variant when calling `zeroize()`
//!
//! Example which derives `Drop`:
//!
//! ```
//! # #[cfg(feature = "derive")]
//! # {
//! use zeroize::Zeroize;
//! use zeroize::{Zeroize, ZeroizeDrop};
//!
//! // This struct will be zeroized on drop
//! #[derive(Zeroize)]
//! #[zeroize(drop)]
//! #[derive(Zeroize, ZeroizeDrop)]
//! struct MyStruct([u8; 32]);
//! # }
//! ```
Expand All @@ -103,6 +107,19 @@
//! # }
//! ```
//!
//! Example which only derives `Drop`:
//!
//! ```
//! # #[cfg(feature = "derive")]
//! # {
//! use zeroize::ZeroizeDrop;
//!
//! // This struct will be zeroized on drop
//! #[derive(ZeroizeDrop)]
//! struct MyStruct([u8; 32]);
//! # }
//! ```
//!
//! ## `Zeroizing<Z>`: wrapper for zeroizing arbitrary values on drop
//!
//! `Zeroizing<Z: Zeroize>` is a generic wrapper type that impls `Deref`
Expand Down Expand Up @@ -219,7 +236,7 @@ extern crate alloc;

#[cfg(feature = "zeroize_derive")]
#[cfg_attr(docsrs, doc(cfg(feature = "zeroize_derive")))]
pub use zeroize_derive::Zeroize;
pub use zeroize_derive::{Zeroize, ZeroizeOnDrop};

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
mod x86;
Expand All @@ -242,6 +259,10 @@ pub trait Zeroize {
fn zeroize(&mut self);
}

/// Marker trait signifying that this type will [`zeroize`](Zeroize::zeroize) itself on [`Drop`].
#[allow(drop_bounds)]
pub trait ZeroizeOnDrop: Drop {}

/// Marker trait for types whose `Default` is the desired zeroization result
pub trait DefaultIsZeroes: Copy + Default + Sized {}

Expand Down Expand Up @@ -531,6 +552,8 @@ where
}
}

impl<Z> ZeroizeOnDrop for Zeroizing<Z> where Z: Zeroize {}

impl<Z> Drop for Zeroizing<Z>
where
Z: Zeroize,
Expand Down
45 changes: 30 additions & 15 deletions zeroize/tests/zeroize_derive.rs
View file
Open in desktop
Expand Up @@ -2,12 +2,11 @@

#[cfg(feature = "zeroize_derive")]
mod custom_derive_tests {
use zeroize::Zeroize;
use zeroize::{Zeroize, ZeroizeOnDrop};

#[test]
fn derive_tuple_struct_test() {
#[derive(Zeroize)]
#[zeroize(drop)]
#[derive(Zeroize, ZeroizeOnDrop)]
struct Z([u8; 3]);

let mut value = Z([1, 2, 3]);
Expand All @@ -17,8 +16,7 @@ mod custom_derive_tests {

#[test]
fn derive_struct_test() {
#[derive(Zeroize)]
#[zeroize(drop)]
#[derive(Zeroize, ZeroizeOnDrop)]
struct Z {
string: String,
vec: Vec<u8>,
Expand Down Expand Up @@ -46,8 +44,7 @@ mod custom_derive_tests {

#[test]
fn derive_enum_test() {
#[derive(Zeroize)]
#[zeroize(drop)]
#[derive(Zeroize, ZeroizeOnDrop)]
enum Z {
#[allow(dead_code)]
Variant1,
Expand All @@ -64,8 +61,7 @@ mod custom_derive_tests {
/// Test that the custom macro actually derived `Drop` for `Z`
#[test]
fn derive_struct_drop() {
#[derive(Zeroize)]
#[zeroize(drop)]
#[derive(Zeroize, ZeroizeOnDrop)]
struct Z([u8; 3]);

assert!(std::mem::needs_drop::<Z>());
Expand All @@ -75,8 +71,29 @@ mod custom_derive_tests {
#[test]
fn derive_enum_drop() {
#[allow(dead_code)]
#[derive(Zeroize)]
#[zeroize(drop)]
#[derive(Zeroize, ZeroizeOnDrop)]
enum Z {
Variant1,
Variant2(usize),
}

assert!(std::mem::needs_drop::<Z>());
}

/// Test that the custom macro actually derived `Drop` for `Z`
#[test]
fn derive_struct_only_drop() {
#[derive(ZeroizeOnDrop)]
struct Z([u8; 3]);

assert!(std::mem::needs_drop::<Z>());
}

/// Test that the custom macro actually derived `Drop` for `Z`
#[test]
fn derive_enum_only_drop() {
#[allow(dead_code)]
#[derive(ZeroizeOnDrop)]
enum Z {
Variant1,
Variant2(usize),
Expand Down Expand Up @@ -107,8 +124,7 @@ mod custom_derive_tests {

#[test]
fn derive_struct_skip() {
#[derive(Zeroize)]
#[zeroize(drop)]
#[derive(Zeroize, ZeroizeOnDrop)]
struct Z {
string: String,
vec: Vec<u8>,
Expand Down Expand Up @@ -137,8 +153,7 @@ mod custom_derive_tests {

#[test]
fn derive_enum_skip() {
#[derive(Zeroize)]
#[zeroize(drop)]
#[derive(Zeroize, ZeroizeOnDrop)]
enum Z {
#[allow(dead_code)]
Variant1,
Expand Down