Skip to content

Commit

Permalink
zeroize: impl Zeroize for MaybeUninit and remove some unsafe (#900)
Browse files Browse the repository at this point in the history
  • Loading branch information
elichai committed Jul 10, 2023
1 parent 50fd775 commit 160fa8c
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 68 deletions.
14 changes: 7 additions & 7 deletions .github/workflows/zeroize.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:
strategy:
matrix:
rust:
- 1.56.0 # MSRV
- 1.60.0 # MSRV
- stable
target:
- armv7a-none-eabi
Expand All @@ -53,7 +53,7 @@ jobs:
# 32-bit Linux
- target: i686-unknown-linux-gnu
platform: ubuntu-latest
rust: 1.56.0 # MSRV
rust: 1.60.0 # MSRV
deps: sudo apt update && sudo apt install gcc-multilib
- target: i686-unknown-linux-gnu
platform: ubuntu-latest
Expand All @@ -63,23 +63,23 @@ jobs:
# 64-bit Linux
- target: x86_64-unknown-linux-gnu
platform: ubuntu-latest
rust: 1.56.0 # MSRV
rust: 1.60.0 # MSRV
- target: x86_64-unknown-linux-gnu
platform: ubuntu-latest
rust: stable

# 64-bit macOS x86_64
- target: x86_64-apple-darwin
platform: macos-latest
rust: 1.56.0 # MSRV
rust: 1.60.0 # MSRV
- target: x86_64-apple-darwin
platform: macos-latest
rust: stable

# 64-bit Windows
- target: x86_64-pc-windows-msvc
platform: windows-latest
rust: 1.56.0 # MSRV
rust: 1.60.0 # MSRV
- target: x86_64-pc-windows-msvc
platform: windows-latest
rust: stable
Expand All @@ -102,7 +102,7 @@ jobs:
include:
# PPC32
- target: powerpc-unknown-linux-gnu
rust: 1.56.0 # MSRV
rust: 1.60.0 # MSRV
- target: powerpc-unknown-linux-gnu
rust: stable
runs-on: ubuntu-latest
Expand All @@ -122,7 +122,7 @@ jobs:
matrix:
include:
- target: aarch64-unknown-linux-gnu
rust: 1.59.0
rust: 1.60.0
- target: aarch64-unknown-linux-gnu
rust: stable
runs-on: ubuntu-latest
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ This repository contains various utility crates used in the RustCrypto project.
| [`inout`] | [![crates.io](https://img.shields.io/crates/v/inout.svg)](https://crates.io/crates/inout) | [![Documentation](https://docs.rs/inout/badge.svg)](https://docs.rs/inout) | ![MSRV 1.56][msrv-1.56] | Custom reference types for code generic over in-place and buffer-to-buffer modes of operation. |
| [`opaque-debug`] | [![crates.io](https://img.shields.io/crates/v/opaque-debug.svg)](https://crates.io/crates/opaque-debug) | [![Documentation](https://docs.rs/opaque-debug/badge.svg)](https://docs.rs/opaque-debug) | ![MSRV 1.41][msrv-1.41] | Macro for opaque `Debug` trait implementation |
| [`wycheproof2blb`] | | | | Utility for converting [Wycheproof] test vectors to the blobby format |
| [`zeroize`] | [![crates.io](https://img.shields.io/crates/v/zeroize.svg)](https://crates.io/crates/zeroize) | [![Documentation](https://docs.rs/zeroize/badge.svg)](https://docs.rs/zeroize) | ![MSRV 1.51][msrv-1.51] | Securely zero memory while avoiding compiler optimizations |
| [`zeroize`] | [![crates.io](https://img.shields.io/crates/v/zeroize.svg)](https://crates.io/crates/zeroize) | [![Documentation](https://docs.rs/zeroize/badge.svg)](https://docs.rs/zeroize) | ![MSRV 1.60][msrv-1.60] | Securely zero memory while avoiding compiler optimizations |

## License

Expand Down Expand Up @@ -52,6 +52,7 @@ Unless you explicitly state otherwise, any contribution intentionally submitted
[msrv-1.56]: https://img.shields.io/badge/rustc-1.56.0+-blue.svg
[msrv-1.57]: https://img.shields.io/badge/rustc-1.57.0+-blue.svg
[msrv-1.59]: https://img.shields.io/badge/rustc-1.59.0+-blue.svg
[msrv-1.60]: https://img.shields.io/badge/rustc-1.60.0+-blue.svg

[//]: # (crates)

Expand Down
2 changes: 1 addition & 1 deletion zeroize/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ readme = "README.md"
categories = ["cryptography", "memory-management", "no-std", "os"]
keywords = ["memory", "memset", "secure", "volatile", "zero"]
edition = "2021"
rust-version = "1.56"
rust-version = "1.60"

[dependencies]
serde = { version = "1.0", default-features = false, optional = true }
Expand Down
4 changes: 2 additions & 2 deletions zeroize/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ thereof, implemented in pure Rust with no usage of FFI or assembly.

## Minimum Supported Rust Version

Rust **1.56** or newer.
Rust **1.60** or newer.

In the future, we reserve the right to change MSRV (i.e. MSRV is out-of-scope
for this crate's SemVer guarantees), however when we do it will be accompanied by
Expand Down Expand Up @@ -64,7 +64,7 @@ dual licensed as above, without any additional terms or conditions.
[docs-image]: https://docs.rs/zeroize/badge.svg
[docs-link]: https://docs.rs/zeroize/
[license-image]: https://img.shields.io/badge/license-Apache2.0/MIT-blue.svg
[rustc-image]: https://img.shields.io/badge/rustc-1.56+-blue.svg
[rustc-image]: https://img.shields.io/badge/rustc-1.60+-blue.svg
[build-image]: https://github.com/RustCrypto/utils/actions/workflows/zeroize.yml/badge.svg
[build-link]: https://github.com/RustCrypto/utils/actions/workflows/zeroize.yml

Expand Down
2 changes: 1 addition & 1 deletion zeroize/src/aarch64.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! [`Zeroize`] impls for ARM64 SIMD registers.
//!
//! Gated behind the `aarch64` feature: MSRV 1.59
//! (the overall crate is MSRV 1.56)
//! (the overall crate is MSRV 1.60)

use crate::{atomic_fence, volatile_write, Zeroize};

Expand Down
98 changes: 42 additions & 56 deletions zeroize/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
//!
//! ## Minimum Supported Rust Version
//!
//! Requires Rust **1.56** or newer.
//! Requires Rust **1.60** or newer.
//!
//! In the future, we reserve the right to change MSRV (i.e. MSRV is out-of-scope
//! for this crate's SemVer guarantees), however when we do it will be accompanied
Expand Down Expand Up @@ -263,10 +263,7 @@ use core::{
};

#[cfg(feature = "alloc")]
use {
alloc::{boxed::Box, string::String, vec::Vec},
core::slice,
};
use alloc::{boxed::Box, string::String, vec::Vec};

#[cfg(feature = "std")]
use std::ffi::CString;
Expand Down Expand Up @@ -315,18 +312,28 @@ macro_rules! impl_zeroize_with_default {

#[rustfmt::skip]
impl_zeroize_with_default! {
bool, char,
PhantomPinned, (), bool, char,
f32, f64,
i8, i16, i32, i64, i128, isize,
u8, u16, u32, u64, u128, usize
}

/// `PhantomPinned` is zero sized so provide a ZeroizeOnDrop implementation.
impl ZeroizeOnDrop for PhantomPinned {}

/// `()` is zero sized so provide a ZeroizeOnDrop implementation.
impl ZeroizeOnDrop for () {}

macro_rules! impl_zeroize_for_non_zero {
($($type:ty),+) => {
$(
impl Zeroize for $type {
fn zeroize(&mut self) {
volatile_write(self, unsafe { <$type>::new_unchecked(1) });
const ONE: $type = match <$type>::new(1) {
Some(one) => one,
None => unreachable!(),
};
volatile_write(self, ONE);
atomic_fence();
}
}
Expand Down Expand Up @@ -371,7 +378,7 @@ where
/// Impl [`ZeroizeOnDrop`] on arrays of types that impl [`ZeroizeOnDrop`].
impl<Z, const N: usize> ZeroizeOnDrop for [Z; N] where Z: ZeroizeOnDrop {}

impl<'a, Z> Zeroize for IterMut<'a, Z>
impl<Z> Zeroize for IterMut<'_, Z>
where
Z: Zeroize,
{
Expand Down Expand Up @@ -405,25 +412,39 @@ where
// The memory pointed to by `self` is valid for `mem::size_of::<Self>()` bytes.
// It is also properly aligned, because `u8` has an alignment of `1`.
unsafe {
volatile_set(self as *mut _ as *mut u8, 0, mem::size_of::<Self>());
volatile_set((self as *mut Self).cast::<u8>(), 0, mem::size_of::<Self>());
}

// Ensures self is overwritten with the default bit pattern. volatile_write can't be
// Ensures self is overwritten with the `None` bit pattern. volatile_write can't be
// used because Option<Z> is not copy.
//
// Safety:
//
// self is safe to replace with the default, which the take() call above should have
// self is safe to replace with `None`, which the take() call above should have
// already done semantically. Any value which needed to be dropped will have been
// done so by take().
unsafe { ptr::write_volatile(self, Option::default()) }
unsafe { ptr::write_volatile(self, None) }

atomic_fence();
}
}

impl<Z> ZeroizeOnDrop for Option<Z> where Z: ZeroizeOnDrop {}

/// Impl [`Zeroize`] on [`MaybeUninit`] types.
///
/// This fills the memory with zeroes.
/// Note that this ignore invariants that `Z` might have, because
/// [`MaybeUninit`] removes all invariants.
impl<Z> Zeroize for MaybeUninit<Z> {
fn zeroize(&mut self) {
// Safety:
// `MaybeUninit` is valid for any byte pattern, including zeros.
unsafe { ptr::write_volatile(self, MaybeUninit::zeroed()) }
atomic_fence();
}
}

/// Impl [`Zeroize`] on slices of [`MaybeUninit`] types.
///
/// This impl can eventually be optimized using an memset intrinsic,
Expand All @@ -435,7 +456,7 @@ impl<Z> ZeroizeOnDrop for Option<Z> where Z: ZeroizeOnDrop {}
/// [`MaybeUninit`] removes all invariants.
impl<Z> Zeroize for [MaybeUninit<Z>] {
fn zeroize(&mut self) {
let ptr = self.as_mut_ptr() as *mut MaybeUninit<u8>;
let ptr = self.as_mut_ptr().cast::<MaybeUninit<u8>>();
let size = self.len().checked_mul(mem::size_of::<Z>()).unwrap();
assert!(size <= isize::MAX as usize);

Expand All @@ -445,7 +466,7 @@ impl<Z> Zeroize for [MaybeUninit<Z>] {
// and it is backed by a single allocated object for at least `self.len() * size_pf::<Z>()` bytes.
// and 0 is a valid value for `MaybeUninit<Z>`
// The memory of the slice should not wrap around the address space.
unsafe { volatile_set(ptr, MaybeUninit::new(0), size) }
unsafe { volatile_set(ptr, MaybeUninit::zeroed(), size) }
atomic_fence();
}
}
Expand Down Expand Up @@ -492,47 +513,22 @@ impl<Z> Zeroize for PhantomData<Z> {
/// [`PhantomData` is always zero sized so provide a ZeroizeOnDrop implementation.
impl<Z> ZeroizeOnDrop for PhantomData<Z> {}

/// `PhantomPinned` is zero sized so provide a Zeroize implementation.
impl Zeroize for PhantomPinned {
fn zeroize(&mut self) {}
}

/// `PhantomPinned` is zero sized so provide a ZeroizeOnDrop implementation.
impl ZeroizeOnDrop for PhantomPinned {}

/// `()` is zero sized so provide a Zeroize implementation.
impl Zeroize for () {
fn zeroize(&mut self) {}
}

/// `()` is zero sized so provide a ZeroizeOnDrop implementation.
impl ZeroizeOnDrop for () {}

/// Generic implementation of Zeroize for tuples up to 10 parameters.
impl<A: Zeroize> Zeroize for (A,) {
fn zeroize(&mut self) {
self.0.zeroize();
}
}

/// Generic implementation of ZeroizeOnDrop for tuples up to 10 parameters.
impl<A: ZeroizeOnDrop> ZeroizeOnDrop for (A,) {}

macro_rules! impl_zeroize_tuple {
( $( $type_name:ident ),+ ) => {
impl<$($type_name: Zeroize),+> Zeroize for ($($type_name),+) {
impl<$($type_name: Zeroize),+> Zeroize for ($($type_name,)+) {
fn zeroize(&mut self) {
#[allow(non_snake_case)]
let ($($type_name),+) = self;
let ($($type_name,)+) = self;
$($type_name.zeroize());+
}
}

impl<$($type_name: ZeroizeOnDrop),+> ZeroizeOnDrop for ($($type_name),+) { }
impl<$($type_name: ZeroizeOnDrop),+> ZeroizeOnDrop for ($($type_name,)+) { }
}
}

// Generic implementations for tuples up to 10 parameters.
impl_zeroize_tuple!(A);
impl_zeroize_tuple!(A, B);
impl_zeroize_tuple!(A, B, C);
impl_zeroize_tuple!(A, B, C, D);
Expand Down Expand Up @@ -561,17 +557,7 @@ where
self.clear();

// Zero the full capacity of `Vec`.
// Safety:
//
// This is safe, because `Vec` never allocates more than `isize::MAX` bytes.
// This exact use case is even mentioned in the documentation of `pointer::add`.
// This is safe because MaybeUninit ignores all invariants,
// so we can create a slice of MaybeUninit<Z> using the full capacity of the Vec
let uninit_slice = unsafe {
slice::from_raw_parts_mut(self.as_mut_ptr() as *mut MaybeUninit<Z>, self.capacity())
};

uninit_slice.zeroize();
self.spare_capacity_mut().zeroize();
}
}

Expand Down Expand Up @@ -621,11 +607,11 @@ impl Zeroize for CString {
// contain a trailing zero byte
let this = mem::take(self);

// - CString::into_bytes calls ::into_vec which takes ownership of the heap pointer
// - CString::into_bytes_with_nul calls ::into_vec which takes ownership of the heap pointer
// as a Vec<u8>
// - Calling .zeroize() on the resulting vector clears out the bytes
// From: https://github.com/RustCrypto/utils/pull/759#issuecomment-1087976570
let mut buf = this.into_bytes();
let mut buf = this.into_bytes_with_nul();
buf.zeroize();

// expect() should never fail, because zeroize() truncates the Vec
Expand Down

0 comments on commit 160fa8c

Please sign in to comment.