From 160fa8c0cbc3ad1e2f272a0b07083fa41a4ded89 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Mon, 10 Jul 2023 03:22:14 +0300 Subject: [PATCH] zeroize: impl `Zeroize` for `MaybeUninit` and remove some unsafe (#900) --- .github/workflows/zeroize.yml | 14 ++--- README.md | 3 +- zeroize/Cargo.toml | 2 +- zeroize/README.md | 4 +- zeroize/src/aarch64.rs | 2 +- zeroize/src/lib.rs | 98 +++++++++++++++-------------------- 6 files changed, 55 insertions(+), 68 deletions(-) diff --git a/.github/workflows/zeroize.yml b/.github/workflows/zeroize.yml index 7f69a611..758bf9b2 100644 --- a/.github/workflows/zeroize.yml +++ b/.github/workflows/zeroize.yml @@ -26,7 +26,7 @@ jobs: strategy: matrix: rust: - - 1.56.0 # MSRV + - 1.60.0 # MSRV - stable target: - armv7a-none-eabi @@ -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 @@ -63,7 +63,7 @@ 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 @@ -71,7 +71,7 @@ jobs: # 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 @@ -79,7 +79,7 @@ jobs: # 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 @@ -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 @@ -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 diff --git a/README.md b/README.md index 0e07239c..e468028d 100644 --- a/README.md +++ b/README.md @@ -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 @@ -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) diff --git a/zeroize/Cargo.toml b/zeroize/Cargo.toml index d1bbf81b..ed6f5286 100644 --- a/zeroize/Cargo.toml +++ b/zeroize/Cargo.toml @@ -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 } diff --git a/zeroize/README.md b/zeroize/README.md index 0156ac03..03b93abb 100644 --- a/zeroize/README.md +++ b/zeroize/README.md @@ -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 @@ -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 diff --git a/zeroize/src/aarch64.rs b/zeroize/src/aarch64.rs index ee474625..07744d01 100644 --- a/zeroize/src/aarch64.rs +++ b/zeroize/src/aarch64.rs @@ -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}; diff --git a/zeroize/src/lib.rs b/zeroize/src/lib.rs index 9ca780e5..b67b5c95 100644 --- a/zeroize/src/lib.rs +++ b/zeroize/src/lib.rs @@ -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 @@ -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; @@ -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(); } } @@ -371,7 +378,7 @@ where /// Impl [`ZeroizeOnDrop`] on arrays of types that impl [`ZeroizeOnDrop`]. impl ZeroizeOnDrop for [Z; N] where Z: ZeroizeOnDrop {} -impl<'a, Z> Zeroize for IterMut<'a, Z> +impl Zeroize for IterMut<'_, Z> where Z: Zeroize, { @@ -405,18 +412,18 @@ where // The memory pointed to by `self` is valid for `mem::size_of::()` 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::()); + volatile_set((self as *mut Self).cast::(), 0, mem::size_of::()); } - // 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 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(); } @@ -424,6 +431,20 @@ where impl ZeroizeOnDrop for Option 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 Zeroize for MaybeUninit { + 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, @@ -435,7 +456,7 @@ impl ZeroizeOnDrop for Option where Z: ZeroizeOnDrop {} /// [`MaybeUninit`] removes all invariants. impl Zeroize for [MaybeUninit] { fn zeroize(&mut self) { - let ptr = self.as_mut_ptr() as *mut MaybeUninit; + let ptr = self.as_mut_ptr().cast::>(); let size = self.len().checked_mul(mem::size_of::()).unwrap(); assert!(size <= isize::MAX as usize); @@ -445,7 +466,7 @@ impl Zeroize for [MaybeUninit] { // and it is backed by a single allocated object for at least `self.len() * size_pf::()` bytes. // and 0 is a valid value for `MaybeUninit` // 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(); } } @@ -492,47 +513,22 @@ impl Zeroize for PhantomData { /// [`PhantomData` is always zero sized so provide a ZeroizeOnDrop implementation. impl ZeroizeOnDrop for PhantomData {} -/// `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 Zeroize for (A,) { - fn zeroize(&mut self) { - self.0.zeroize(); - } -} - -/// Generic implementation of ZeroizeOnDrop for tuples up to 10 parameters. -impl 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); @@ -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 using the full capacity of the Vec - let uninit_slice = unsafe { - slice::from_raw_parts_mut(self.as_mut_ptr() as *mut MaybeUninit, self.capacity()) - }; - - uninit_slice.zeroize(); + self.spare_capacity_mut().zeroize(); } } @@ -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 // - 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