Merge pull request #2 from radumarias/master

added concept of expose_mut. reference to protected

.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
 

.gitignore

@@ -1 +1,2 @@
 /target
+/.idea

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"

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<S: Zeroize> {
     inner_secret: Box<S>,
-    protected: bool,
 }
 
 impl<S: Zeroize> Zeroize for SecretBox<S> {
     fn zeroize(&mut self) {
-        let len = mem::size_of_val(&*self.inner_secret);
+        self.inner_secret.as_mut().zeroize()
+    }
+}
+
+impl<S: Zeroize> Drop for SecretBox<S> {
+    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<S: Zeroize> Drop for SecretBox<S> {
-    fn drop(&mut self) {
         self.zeroize()
     }
 }
@@ -63,37 +52,27 @@ impl<S: Zeroize> From<Box<S>> for SecretBox<S> {
 impl<S: Zeroize> SecretBox<S> {
     /// Create a secret value using a pre-boxed value.
     pub fn new(boxed_secret: Box<S>) -> Self {
-        let len = mem::size_of_val(&*boxed_secret);
+        let len = size_of_val(&*boxed_secret);
 
         let secret_ptr = Box::into_raw(boxed_secret);
 
         unsafe {
             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 }
     }
 }
 
 impl<S: Zeroize + Default> SecretBox<S> {
     /// 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<S: Zeroize + Clone> SecretBox<S> {
 
 impl<S: Zeroize + Default> Default for SecretBox<S> {
     fn default() -> Self {
-        Self {
-            inner_secret: Box::<S>::default(),
-            protected: false,
-        }
+        let inner_secret = Box::<S>::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<S: Zeroize> ExposeSecret<S> for SecretBox<S> {
     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<S> 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<S> 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<S> 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<S: Zeroize> {
-    /// 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<u8>,
@@ -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"),
         }
     }