diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 3dd422f..04c21fb 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -17,9 +17,9 @@ jobs: ci: runs-on: ${{ matrix.runner }} strategy: + fail-fast: false matrix: runner: [ubuntu-latest, macos-latest, windows-latest] - fail-fast: false steps: - uses: actions/checkout@v4 diff --git a/.gitignore b/.gitignore index ea8c4bf..d81f12e 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ /target +/.idea diff --git a/Cargo.toml b/Cargo.toml index cb68692..60a8761 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,6 +4,8 @@ version = "0.1.0" edition = "2021" [dependencies] -libc = "0.2.158" memsec = "0.7.0" zeroize = "1.8.1" + +[target.'cfg(unix)'.dependencies] +libc = "0.2.158" diff --git a/src/lib.rs b/src/lib.rs index b155ac4..b7feff4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,5 @@ //! This is a fork of the [secrets](https://github.com/stouset/secrets) crate. -//! This crate adds `mlock` and `mprotect` to lock the secret's page in memory -//! and read only when exposed +//! This crate adds `mlock` to lock the secret's page in memory #![cfg_attr(docsrs, feature(doc_auto_cfg))] #![warn(missing_docs, rust_2018_idioms, unused_qualifications)] @@ -9,45 +8,35 @@ use core::{ any, fmt::{self, Debug}, }; -use libc::{PROT_NONE, PROT_READ, PROT_WRITE}; -use memsec::{mlock, mprotect, munlock}; -use std::{mem, ptr::NonNull}; - -use zeroize::{Zeroize, ZeroizeOnDrop}; - +use memsec::{mlock, munlock}; +use std::ops::{Deref, DerefMut}; +use std::mem::size_of_val; pub use zeroize; +use zeroize::{Zeroize, ZeroizeOnDrop}; /// Wrapper for the inner secret. Can be exposed by [`ExposeSecret`] pub struct SecretBox { inner_secret: Box, - protected: bool, } impl Zeroize for SecretBox { fn zeroize(&mut self) { - let len = mem::size_of_val(&*self.inner_secret); + self.inner_secret.as_mut().zeroize() + } +} + +impl Drop for SecretBox { + fn drop(&mut self) { + let len = size_of_val(&*self.inner_secret); let secret_ptr = self.inner_secret.as_ref() as *const S; unsafe { if !munlock(secret_ptr as *mut u8, len) { - eprintln!("Unable to munlock variable") - } - - if !mprotect( - NonNull::new(secret_ptr as *mut S).expect("Unable to convert ptr to NonNull"), - PROT_READ | PROT_WRITE, - ) { - eprintln!("Unable to unprotect variable") + panic!("Unable to munlock variable") } } - self.inner_secret.as_mut().zeroize() - } -} - -impl Drop for SecretBox { - fn drop(&mut self) { self.zeroize() } } @@ -63,7 +52,7 @@ impl From> for SecretBox { impl SecretBox { /// Create a secret value using a pre-boxed value. pub fn new(boxed_secret: Box) -> Self { - let len = mem::size_of_val(&*boxed_secret); + let len = size_of_val(&*boxed_secret); let secret_ptr = Box::into_raw(boxed_secret); @@ -71,21 +60,11 @@ impl SecretBox { if !mlock(secret_ptr as *mut u8, len) { panic!("Unable to mlock variable ") } - - if !mprotect( - NonNull::new(secret_ptr).expect("Unable to convert box to NonNull"), - PROT_NONE, - ) { - panic!("Unable to protect secret") - } } let inner_secret = unsafe { Box::from_raw(secret_ptr) }; - Self { - inner_secret, - protected: true, - } + Self { inner_secret } } } @@ -93,7 +72,7 @@ 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().inner_secret_mut()); + ctr(&mut *secret.expose_secret_mut()); secret } } @@ -129,10 +108,8 @@ impl SecretBox { impl Default for SecretBox { fn default() -> Self { - Self { - inner_secret: Box::::default(), - protected: false, - } + let inner_secret = Box::::default(); + SecretBox::new(inner_secret) } } @@ -147,82 +124,78 @@ where S: CloneableSecret, { fn clone(&self) -> Self { - SecretBox { - inner_secret: self.inner_secret.clone(), - protected: false, - } + SecretBox::new(self.inner_secret.clone()) } } impl ExposeSecret for SecretBox { fn expose_secret(&mut self) -> SecretGuard<'_, S> { - SecretGuard::new(self.inner_secret.as_mut(), self.protected) + SecretGuard::new(&self.inner_secret) + } + + fn expose_secret_mut(&mut self) -> SecretGuardMut<'_, S> { + SecretGuardMut::new(&mut self.inner_secret) } } -/// Secret Guard that holds a mutable to reference to the secret +/// Secret Guard that holds a reference to the secret. pub struct SecretGuard<'a, S> where S: Zeroize, { - data: &'a mut S, - protected: bool, + data: &'a S, } -impl<'a, S: Zeroize> SecretGuard<'a, S> { - /// Create a new SecretGuard instance. - pub fn new(data: &'a mut S, protected: bool) -> Self { - Self { data, protected } - } - /// Get a shared reference to the inner secret - pub fn inner_secret(&mut self) -> &S { - let secret_ptr = self.data as *const S; - - if self.protected { - unsafe { - if !mprotect( - NonNull::new(secret_ptr as *mut S).expect("Unable to convert ptr to NonNull"), - PROT_READ, - ) { - panic!("Unable to protect secret_guard") - } - } - } - self.protected = false; +impl Deref for SecretGuard<'_, S> +where + S: Zeroize, +{ + type Target = S; + + fn deref(&self) -> &Self::Target { self.data } +} - /// Get an exclusive reference to the inner secret - pub fn inner_secret_mut(&mut self) -> &mut S { - let secret_ptr = self.data as *const S; +/// Secret Guard that holds a mutable to reference to the secret. +pub struct SecretGuardMut<'a, S> +where + S: Zeroize, +{ + data: &'a mut S, +} - unsafe { - if !mprotect( - NonNull::new(secret_ptr as *mut S).expect("Unable to convert ptr to NonNull"), - PROT_READ | PROT_WRITE, - ) { - panic!("Unable to protect secret_guard") - } - } - self.protected = false; +impl Deref for SecretGuardMut<'_, S> +where + S: Zeroize, +{ + type Target = S; + + fn deref(&self) -> &Self::Target { self.data } } -impl<'a, S: Zeroize> Drop for SecretGuard<'a, S> { - fn drop(&mut self) { - let secret_ptr = self.data as *const S; - - if !self.protected { - unsafe { - if !mprotect( - NonNull::new(secret_ptr as *mut S).expect("Unable to convert ptr to NonNull"), - PROT_NONE, - ) { - panic!("Unable to protect memory") - } - } - } +impl DerefMut for SecretGuardMut<'_, S> +where + S: Zeroize, +{ + fn deref_mut(&mut self) -> &mut Self::Target { + self.data + } +} + +impl<'a, S: Zeroize> SecretGuard<'a, S> { + /// Create a new SecretGuard instance. + pub fn new(data: &'a S) -> Self { + Self { data } + } +} + +impl<'a, S: Zeroize> SecretGuardMut<'a, S> { + /// Create a new SecretGuard instance. + pub fn new(data: &'a mut S) -> Self { + Self { data } } } @@ -231,13 +204,17 @@ pub trait CloneableSecret: Clone + Zeroize {} /// Create a SecretGuard that holds a reference to the secret pub trait ExposeSecret { - /// Expose secret: this is the only method providing access to a secret. + /// Expose secret as non-mutable. fn expose_secret(&mut self) -> SecretGuard<'_, S>; + + /// Expose secret as mutable. + fn expose_secret_mut(&mut self) -> SecretGuardMut<'_, S>; } #[cfg(test)] mod tests { use super::*; + #[derive(Debug, Clone, Default)] struct TestSecret { data: Vec, @@ -269,7 +246,7 @@ mod tests { fn test_secret_box_drop_zeroizes() { let secret = Box::new(TestSecret::new(10)); let mut secret_box = SecretBox::new(secret); - assert!(secret_box.expose_secret().inner_secret().check_non_zero()); + assert!((*secret_box.expose_secret()).check_non_zero()); drop(secret_box); @@ -284,17 +261,17 @@ mod tests { let secret = Box::new(TestSecret::new(10)); let mut secret_box = SecretBox::new(secret); { - let mut exposed = secret_box.expose_secret(); - exposed.inner_secret_mut().data[0] = 42; + let mut exposed = secret_box.expose_secret_mut(); + (*exposed).data[0] = 42; } - assert_eq!(secret_box.expose_secret().inner_secret().data[0], 42); + assert_eq!((*secret_box.expose_secret()).data[0], 42); } #[test] fn test_secret_box_new_with_ctr() { let mut secret_box = SecretBox::new_with_ctr(|| TestSecret::new(10)); - assert!(secret_box.expose_secret().inner_secret().check_non_zero()); + assert!((*secret_box.expose_secret()).check_non_zero()); } #[test] @@ -303,7 +280,7 @@ mod tests { SecretBox::try_new_with_ctr(|| Ok(TestSecret::new(10))); match result { - Ok(mut secret_box) => assert!(secret_box.expose_secret().inner_secret().check_non_zero()), + Ok(mut secret_box) => assert!((*secret_box.expose_secret()).check_non_zero()), Err(_) => panic!("Expected Ok variant"), } }