From f98d4ccf1e81d2fb9daaa7a5458f8064ecb563a2 Mon Sep 17 00:00:00 2001 From: Bogdan Opanchuk Date: Fri, 19 Jan 2024 12:12:57 -0800 Subject: [PATCH] secrecy: rework the `SecretBox` type (#1140) Redefines `SecretBox` as a struct containing `Box` with the same impls as `Secret`. --- Cargo.lock | 1 - secrecy/Cargo.toml | 7 +- secrecy/src/boxed.rs | 10 -- secrecy/src/bytes.rs | 95 --------------- secrecy/src/lib.rs | 272 +++++++++++++++++------------------------- secrecy/src/string.rs | 19 --- secrecy/src/vec.rs | 11 -- 7 files changed, 109 insertions(+), 306 deletions(-) delete mode 100644 secrecy/src/boxed.rs delete mode 100644 secrecy/src/bytes.rs delete mode 100644 secrecy/src/string.rs delete mode 100644 secrecy/src/vec.rs diff --git a/Cargo.lock b/Cargo.lock index 6ec39a31..16b1132b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1074,7 +1074,6 @@ dependencies = [ name = "secrecy" version = "0.9.0-pre" dependencies = [ - "bytes", "serde", "zeroize", ] diff --git a/secrecy/Cargo.toml b/secrecy/Cargo.toml index 492daecd..26d15d6f 100644 --- a/secrecy/Cargo.toml +++ b/secrecy/Cargo.toml @@ -18,16 +18,11 @@ edition = "2021" rust-version = "1.60" [dependencies] -zeroize = { version = "1.6", default-features = false } +zeroize = { version = "1.6", default-features = false, features = ["alloc"] } # optional dependencies -bytes = { version = "1", optional = true } serde = { version = "1", optional = true } -[features] -default = ["alloc"] -alloc = ["zeroize/alloc"] - [package.metadata.docs.rs] all-features = true rustdoc-args = ["--cfg", "docsrs"] diff --git a/secrecy/src/boxed.rs b/secrecy/src/boxed.rs deleted file mode 100644 index 6cdf80cc..00000000 --- a/secrecy/src/boxed.rs +++ /dev/null @@ -1,10 +0,0 @@ -//! `Box` types containing secrets - -use super::{DebugSecret, Secret}; -use alloc::boxed::Box; -use zeroize::Zeroize; - -/// `Box` types containing a secret value -pub type SecretBox = Secret>; - -impl DebugSecret for Box {} diff --git a/secrecy/src/bytes.rs b/secrecy/src/bytes.rs deleted file mode 100644 index 9791e3eb..00000000 --- a/secrecy/src/bytes.rs +++ /dev/null @@ -1,95 +0,0 @@ -//! Optional `Secret` wrapper type for the `bytes::BytesMut` crate. - -use super::ExposeSecret; -use bytes::BytesMut; -use core::fmt; -use zeroize::Zeroize; - -#[cfg(all(feature = "bytes", feature = "serde"))] -use serde::de::{self, Deserialize}; - -/// Instance of [`BytesMut`] protected by a type that impls the [`ExposeSecret`] -/// trait like `Secret`. -/// -/// Because of the nature of how the `BytesMut` type works, it needs some special -/// care in order to have a proper zeroizing drop handler. -#[derive(Clone)] -pub struct SecretBytesMut(BytesMut); - -impl SecretBytesMut { - /// Wrap bytes in `SecretBytesMut` - pub fn new(bytes: impl Into) -> SecretBytesMut { - SecretBytesMut(bytes.into()) - } -} - -impl ExposeSecret for SecretBytesMut { - fn expose_secret(&self) -> &BytesMut { - &self.0 - } -} - -impl fmt::Debug for SecretBytesMut { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "SecretBytesMut([REDACTED])") - } -} - -impl From for SecretBytesMut { - fn from(bytes: BytesMut) -> SecretBytesMut { - SecretBytesMut::new(bytes) - } -} - -impl Drop for SecretBytesMut { - fn drop(&mut self) { - self.0.resize(self.0.capacity(), 0); - self.0.as_mut().zeroize(); - debug_assert!(self.0.as_ref().iter().all(|b| *b == 0)); - } -} - -#[cfg(all(feature = "bytes", feature = "serde"))] -impl<'de> Deserialize<'de> for SecretBytesMut { - fn deserialize>(deserializer: D) -> Result { - struct SecretBytesVisitor; - - impl<'de> de::Visitor<'de> for SecretBytesVisitor { - type Value = SecretBytesMut; - - fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - formatter.write_str("byte array") - } - - #[inline] - fn visit_bytes(self, v: &[u8]) -> Result - where - E: de::Error, - { - let mut bytes = BytesMut::with_capacity(v.len()); - bytes.extend_from_slice(v); - Ok(SecretBytesMut(bytes)) - } - - #[inline] - fn visit_seq(self, mut seq: V) -> Result - where - V: de::SeqAccess<'de>, - { - // 4096 is cargo culted from upstream - let len = core::cmp::min(seq.size_hint().unwrap_or(0), 4096); - let mut bytes = BytesMut::with_capacity(len); - - use bytes::BufMut; - - while let Some(value) = seq.next_element()? { - bytes.put_u8(value); - } - - Ok(SecretBytesMut(bytes)) - } - } - - deserializer.deserialize_bytes(SecretBytesVisitor) - } -} diff --git a/secrecy/src/lib.rs b/secrecy/src/lib.rs index c1872eb9..d64d1540 100644 --- a/secrecy/src/lib.rs +++ b/secrecy/src/lib.rs @@ -1,12 +1,10 @@ -//! [`Secret`] wrapper type for more carefully handling secret values +//! [`SecretBox`] wrapper type for more carefully handling secret values //! (e.g. passwords, cryptographic keys, access tokens or other credentials) //! //! # Goals //! //! - Make secret access explicit and easy-to-audit via the -//! [`ExposeSecret`] trait. This also makes secret values immutable which -//! helps avoid making accidental copies (e.g. reallocating the backing -//! buffer for a `Vec`) +//! [`ExposeSecret`] and [`ExposeSecretMut`] traits. //! - Prevent accidental leakage of secrets via channels like debug logging //! - Ensure secrets are wiped from memory on drop securely //! (using the [`zeroize`] crate) @@ -15,59 +13,20 @@ //! `forbid(unsafe_code)`-based implementation and does not provide more advanced //! memory protection mechanisms e.g. ones based on `mlock(2)`/`mprotect(2)`. //! We may explore more advanced protection mechanisms in the future. -//! -//! # `Box`, `String`, and `Vec` wrappers -//! -//! Most users of this crate will simply want [`Secret`] wrappers around Rust's -//! core collection types: i.e. `Box`, `String`, and `Vec`. -//! -//! When the `alloc` feature of this crate is enabled (which it is by default), -//! [`SecretBox`], [`SecretString`], and [`SecretVec`] type aliases are -//! available. -//! -//! There's nothing particularly fancy about these: they're just the simple -//! composition of `Secret>`, `Secret`, and `Secret>`! -//! However, in many cases they're all you will need. -//! -//! # Advanced usage -//! -//! If you are hitting limitations on what's possible with the collection type -//! wrappers, you'll want to define your own newtype which lets you customize -//! the implementation: -//! -//! ```rust -//! use secrecy::{CloneableSecret, DebugSecret, Secret, Zeroize}; -//! -//! #[derive(Clone)] -//! pub struct AccountNumber(String); -//! -//! impl Zeroize for AccountNumber { -//! fn zeroize(&mut self) { -//! self.0.zeroize(); -//! } -//! } -//! -//! /// Permits cloning -//! impl CloneableSecret for AccountNumber {} -//! -//! /// Provides a `Debug` impl (by default `[[REDACTED]]`) -//! impl DebugSecret for AccountNumber {} -//! -//! /// Use this alias when storing secret values -//! pub type SecretAccountNumber = Secret; -//! ``` +//! Those who don't mind `std` and `libc` dependencies should consider using +//! the [`secrets`](https://crates.io/crates/secrets) crate. //! //! # `serde` support //! -//! When the `serde` feature of this crate is enabled, the [`Secret`] type will -//! receive a [`Deserialize`] impl for all `Secret` types where +//! When the `serde` feature of this crate is enabled, the [`SecretBox`] type will +//! receive a [`Deserialize`] impl for all `SecretBox` types where //! `T: DeserializeOwned`. This allows *loading* secret values from data //! deserialized from `serde` (be careful to clean up any intermediate secrets //! when doing this, e.g. the unparsed input!) //! -//! To prevent exfiltration of secret values via `serde`, by default `Secret` +//! To prevent exfiltration of secret values via `serde`, by default `SecretBox` //! does *not* receive a corresponding [`Serialize`] impl. If you would like -//! types of `Secret` to be serializable with `serde`, you will need to impl +//! types of `SecretBox` to be serializable with `serde`, you will need to impl //! the [`SerializableSecret`] marker trait on `T`. #![no_std] @@ -75,179 +34,164 @@ #![forbid(unsafe_code)] #![warn(missing_docs, rust_2018_idioms, unused_qualifications)] -#[cfg(feature = "alloc")] extern crate alloc; -#[cfg(feature = "alloc")] -mod boxed; -#[cfg(feature = "bytes")] -mod bytes; -#[cfg(feature = "alloc")] -mod string; -#[cfg(feature = "alloc")] -mod vec; - -pub use zeroize::{self, Zeroize}; - -#[cfg(feature = "alloc")] -pub use self::{boxed::SecretBox, string::SecretString, vec::SecretVec}; - -#[cfg(feature = "bytes")] -pub use self::bytes::SecretBytesMut; - +use alloc::boxed::Box; use core::{ any, fmt::{self, Debug}, }; +use zeroize::{Zeroize, ZeroizeOnDrop}; + #[cfg(feature = "serde")] use serde::{de, ser, Deserialize, Serialize}; +pub use zeroize; + /// Wrapper type for values that contains secrets, which attempts to limit /// accidental exposure and ensure secrets are wiped from memory when dropped. /// (e.g. passwords, cryptographic keys, access tokens or other credentials) /// -/// Access to the secret inner value occurs through the [`ExposeSecret`] trait, -/// which provides an `expose_secret()` method for accessing the inner secret -/// value. -pub struct Secret -where - S: Zeroize, -{ - /// Inner secret value - inner_secret: S, +/// Access to the secret inner value occurs through the [`ExposeSecret`] +/// or [`ExposeSecretMut`] traits, which provide methods for accessing the inner secret value. +pub struct SecretBox { + inner_secret: Box, } -impl Secret -where - S: Zeroize, -{ - /// Take ownership of a secret value - pub fn new(secret: S) -> Self { - Secret { - inner_secret: secret, +impl Zeroize for SecretBox { + fn zeroize(&mut self) { + self.inner_secret.as_mut().zeroize() + } +} + +impl Drop for SecretBox { + fn drop(&mut self) { + self.zeroize() + } +} + +impl ZeroizeOnDrop for SecretBox {} + +impl From> for SecretBox { + fn from(source: Box) -> Self { + Self::new(source) + } +} + +impl SecretBox { + /// Create a secret value using a pre-boxed value. + pub fn new(boxed_secret: Box) -> Self { + Self { + inner_secret: boxed_secret, } } } -impl ExposeSecret for Secret -where - S: Zeroize, -{ - fn expose_secret(&self) -> &S { - &self.inner_secret +impl SecretBox { + /// Create a secret value using a function that can initialize the vale in-place. + pub fn new_with_mut(ctr: impl FnOnce(&mut S)) -> Self { + let mut secret = Self::default(); + ctr(secret.expose_secret_mut()); + secret } } -impl From for Secret -where - S: Zeroize, -{ - fn from(secret: S) -> Self { - Self::new(secret) +impl SecretBox { + /// Create a secret value using the provided function as a constructor. + /// + /// The implementation makes an effort to zeroize the locally constructed value + /// before it is copied to the heap, and constructing it inside the closure minimizes + /// the possibility of it being accidentally copied by other code. + /// + /// **Note:** using [`Self::new`] or [`Self::new_with_mut`] is preferable when possible, + /// since this method's safety relies on empyric evidence and may be violated on some targets. + pub fn new_with_ctr(ctr: impl FnOnce() -> S) -> Self { + let mut data = ctr(); + let secret = Self { + inner_secret: Box::new(data.clone()), + }; + data.zeroize(); + secret + } + + /// Same as [`Self::new_with_ctr`], but the constructor can be fallible. + /// + /// + /// **Note:** using [`Self::new`] or [`Self::new_with_mut`] is preferable when possible, + /// since this method's safety relies on empyric evidence and may be violated on some targets. + pub fn try_new_with_ctr(ctr: impl FnOnce() -> Result) -> Result { + let mut data = ctr()?; + let secret = Self { + inner_secret: Box::new(data.clone()), + }; + data.zeroize(); + Ok(secret) + } +} + +impl Default for SecretBox { + fn default() -> Self { + Self { + inner_secret: Box::::default(), + } + } +} + +impl Debug for SecretBox { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "SecretBox<{}>([REDACTED])", any::type_name::()) } } -impl Clone for Secret +impl Clone for SecretBox where S: CloneableSecret, { fn clone(&self) -> Self { - Secret { + SecretBox { inner_secret: self.inner_secret.clone(), } } } -impl Debug for Secret -where - S: Zeroize + DebugSecret, -{ - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str("Secret(")?; - S::debug_secret(f)?; - f.write_str(")") +impl ExposeSecret for SecretBox { + fn expose_secret(&self) -> &S { + self.inner_secret.as_ref() } } -impl Drop for Secret -where - S: Zeroize, -{ - fn drop(&mut self) { - // Zero the secret out from memory - self.inner_secret.zeroize(); +impl ExposeSecretMut for SecretBox { + fn expose_secret_mut(&mut self) -> &mut S { + self.inner_secret.as_mut() } } /// Marker trait for secrets which are allowed to be cloned pub trait CloneableSecret: Clone + Zeroize {} -/// Implement `CloneableSecret` on arrays of types that impl `Clone` and -/// `Zeroize`. -macro_rules! impl_cloneable_secret_for_array { - ($($size:expr),+) => { - $( - impl CloneableSecret for [T; $size] {} - )+ - }; -} - -// TODO(tarcieri): const generics -impl_cloneable_secret_for_array!( - 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, - 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, - 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64 -); - /// Expose a reference to an inner secret pub trait ExposeSecret { /// Expose secret: this is the only method providing access to a secret. fn expose_secret(&self) -> &S; } -/// Debugging trait which is specialized for handling secret values -pub trait DebugSecret { - /// Format information about the secret's type. - /// - /// This can be thought of as an equivalent to [`Debug::fmt`], but one - /// which by design does not permit access to the secret value. - fn debug_secret(f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { - f.write_str("[REDACTED ")?; - f.write_str(any::type_name::())?; - f.write_str("]") - } -} - -/// Implement `DebugSecret` on arrays of types that impl `Debug`. -macro_rules! impl_debug_secret_for_array { - ($($size:expr),+) => { - $( - impl DebugSecret for [T; $size] {} - )+ - }; +/// Expose a mutable reference to an inner secret +pub trait ExposeSecretMut { + /// Expose secret: this is the only method providing access to a secret. + fn expose_secret_mut(&mut self) -> &mut S; } -// TODO(tarcieri): const generics -impl_debug_secret_for_array!( - 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, - 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, - 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64 -); - /// Marker trait for secret types which can be [`Serialize`]-d by [`serde`]. /// /// When the `serde` feature of this crate is enabled and types are marked with -/// this trait, they receive a [`Serialize` impl][1] for `Secret`. +/// this trait, they receive a [`Serialize` impl][1] for `SecretBox`. /// (NOTE: all types which impl `DeserializeOwned` receive a [`Deserialize`] /// impl) /// /// This is done deliberately to prevent accidental exfiltration of secrets /// via `serde` serialization. /// -/// If you are working with [`SecretString`] or [`SecretVec`], not that -/// by design these types do *NOT* impl this trait. -/// /// If you really want to have `serde` serialize those types, use the /// [`serialize_with`][2] attribute to specify a serializer that exposes the secret. /// @@ -257,7 +201,7 @@ impl_debug_secret_for_array!( pub trait SerializableSecret: Serialize {} #[cfg(feature = "serde")] -impl<'de, T> Deserialize<'de> for Secret +impl<'de, T> Deserialize<'de> for SecretBox where T: Zeroize + Clone + de::DeserializeOwned + Sized, { @@ -265,12 +209,12 @@ where where D: de::Deserializer<'de>, { - T::deserialize(deserializer).map(Secret::new) + Self::try_new_with_ctr(|| T::deserialize(deserializer)) } } #[cfg(feature = "serde")] -impl Serialize for Secret +impl Serialize for SecretBox where T: Zeroize + SerializableSecret + Serialize + Sized, { diff --git a/secrecy/src/string.rs b/secrecy/src/string.rs deleted file mode 100644 index a54cd0b1..00000000 --- a/secrecy/src/string.rs +++ /dev/null @@ -1,19 +0,0 @@ -//! Secret strings - -use super::{CloneableSecret, DebugSecret, Secret}; -use alloc::str::FromStr; -use alloc::string::{String, ToString}; - -/// Secret strings -pub type SecretString = Secret; - -impl DebugSecret for String {} -impl CloneableSecret for String {} - -impl FromStr for SecretString { - type Err = core::convert::Infallible; - - fn from_str(src: &str) -> Result { - Ok(SecretString::new(src.to_string())) - } -} diff --git a/secrecy/src/vec.rs b/secrecy/src/vec.rs deleted file mode 100644 index 5fe95467..00000000 --- a/secrecy/src/vec.rs +++ /dev/null @@ -1,11 +0,0 @@ -//! Secret `Vec` types - -use super::{CloneableSecret, DebugSecret, Secret}; -use alloc::vec::Vec; -use zeroize::Zeroize; - -/// `Vec` types containing secret value -pub type SecretVec = Secret>; - -impl CloneableSecret for Vec {} -impl DebugSecret for Vec {}