chore: Move rest of coloring.rs into secret-memory crate

Also removes the StoreSecret trait from cli.rs as it was
redundant.

Cargo.lock

@@ -1064,7 +1064,6 @@ dependencies = [
  "clap 4.4.8",
  "criterion",
  "env_logger",
- "lazy_static",
  "libsodium-sys-stable",
  "log",
  "memoffset",
@@ -1122,6 +1121,8 @@ name = "rosenpass-secret-memory"
 version = "0.1.0"
 dependencies = [
  "anyhow",
+ "lazy_static",
+ "libsodium-sys-stable",
  "rosenpass-sodium",
  "rosenpass-to",
  "rosenpass-util",

fuzz/Cargo.toml

@@ -11,6 +11,7 @@ cargo-fuzz = true
 arbitrary = { workspace = true }
 libfuzzer-sys = { workspace = true }
 stacker = { workspace = true }
+rosenpass-secret-memory = { workspace = true }
 rosenpass-sodium = { workspace = true }
 rosenpass-ciphers = { workspace = true }
 rosenpass-to = { workspace = true }

fuzz/fuzz_targets/handle_msg.rs

@@ -3,8 +3,8 @@ extern crate rosenpass;
 
 use libfuzzer_sys::fuzz_target;
 
-use rosenpass::coloring::Secret;
 use rosenpass::protocol::CryptoServer;
+use rosenpass_secret_memory::Secret;
 use rosenpass_sodium::init as sodium_init;
 
 fuzz_target!(|rx_buf: &[u8]| {

rosenpass/Cargo.toml

@@ -25,7 +25,6 @@ static_assertions = { workspace = true }
 memoffset = { workspace = true }
 libsodium-sys-stable = { workspace = true }
 oqs-sys = { workspace = true }
-lazy_static = { workspace = true }
 thiserror = { workspace = true }
 paste = { workspace = true }
 log = { workspace = true }

rosenpass/src/cli.rs

@@ -1,13 +1,12 @@
 use anyhow::{bail, ensure};
 use clap::Parser;
+use rosenpass_secret_memory::file::StoreSecret;
 use rosenpass_util::file::{LoadValue, LoadValueB64};
-use std::path::{Path, PathBuf};
+use std::path::PathBuf;
 
 use crate::app_server;
 use crate::app_server::AppServer;
 use crate::{
-    // app_server::{AppServer, LoadValue, LoadValueB64},
-    coloring::Secret,
     pqkem::{StaticKEM, KEM},
     protocol::{SPk, SSk, SymKey},
 };
@@ -248,14 +247,3 @@ impl Cli {
         srv.event_loop()
     }
 }
-
-trait StoreSecret {
-    fn store_secret<P: AsRef<Path>>(&self, path: P) -> anyhow::Result<()>;
-}
-
-impl<const N: usize> StoreSecret for Secret<N> {
-    fn store_secret<P: AsRef<Path>>(&self, path: P) -> anyhow::Result<()> {
-        std::fs::write(path, self.secret())?;
-        Ok(())
-    }
-}

rosenpass/src/lib.rs

@@ -1,4 +1,3 @@
-pub mod coloring;
 #[rustfmt::skip]
 pub mod labeled_prf;
 pub mod app_server;

rosenpass/src/prftree.rs

@@ -1,5 +1,5 @@
 //! Implementation of the tree-like structure used for the label derivation in [labeled_prf](crate::labeled_prf)
-use crate::coloring::Secret;
+use rosenpass_secret_memory::Secret;
 
 use anyhow::Result;
 use rosenpass_ciphers::{hash, KEY_LEN};

rosenpass/src/protocol.rs

@@ -68,7 +68,6 @@
 //! ```
 
 use crate::{
-    coloring::*,
     labeled_prf as lprf,
     msgs::*,
     pqkem::*,
@@ -76,7 +75,7 @@ use crate::{
 };
 use anyhow::{bail, ensure, Context, Result};
 use rosenpass_ciphers::{aead, xaead, KEY_LEN};
-use rosenpass_secret_memory::Public;
+use rosenpass_secret_memory::{Public, Secret};
 use rosenpass_util::{cat, mem::cpy_min, ord::max_usize, time::Timebase};
 use std::collections::hash_map::{
     Entry::{Occupied, Vacant},

secret-memory/Cargo.toml

@@ -14,3 +14,5 @@ anyhow = { workspace = true }
 rosenpass-to = { workspace = true }
 rosenpass-sodium = { workspace = true }
 rosenpass-util = { workspace = true }
+libsodium-sys-stable = { workspace = true }
+lazy_static = { workspace = true }

secret-memory/src/file.rs

@@ -0,0 +1,7 @@
+use std::path::Path;
+
+pub trait StoreSecret {
+    type Error;
+
+    fn store_secret<P: AsRef<Path>>(&self, path: P) -> Result<(), Self::Error>;
+}

secret-memory/src/lib.rs

@@ -1,4 +1,8 @@
+pub mod debug;
+pub mod file;
+
 mod public;
 pub use crate::public::Public;
 
-pub mod debug;
+mod secret;
+pub use crate::secret::Secret;

secret-memory/src/secret.rs

@@ -3,15 +3,17 @@ use lazy_static::lazy_static;
 use libsodium_sys as libsodium;
 use rosenpass_util::{
     b64::b64_reader,
-    file::{fopen_r, LoadValue, LoadValueB64, ReadExactToEnd, StoreValue},
+    file::{fopen_r, LoadValue, LoadValueB64, ReadExactToEnd},
     functional::mutating,
 };
-use std::result::Result;
+
 use std::{
     collections::HashMap, convert::TryInto, fmt, os::raw::c_void, path::Path, ptr::null_mut,
     sync::Mutex,
 };
 
+use crate::file::StoreSecret;
+
 // This might become a problem in library usage; it's effectively a memory
 // leak which probably isn't a problem right now because most memory will
 // be reused…
@@ -28,7 +30,7 @@ lazy_static! {
 /// Further information about the protection in place can be found in in the
 /// [libsodium documentation](https://libsodium.gitbook.io/doc/memory_management#guarded-heap-allocations)
 #[derive(Debug)] // TODO check on Debug derive, is that clever
-pub struct SecretMemoryPool {
+struct SecretMemoryPool {
     pool: HashMap<usize, Vec<*mut c_void>>,
 }
 
@@ -44,6 +46,7 @@ impl SecretMemoryPool {
     /// Return secrete back to the pool for future re-use
     ///
     /// This consumes the [Secret], but its memory is re-used.
+    #[allow(dead_code)]
     pub fn release<const N: usize>(&mut self, mut s: Secret<N>) {
         unsafe {
             self.release_by_ref(&mut s);
@@ -201,6 +204,50 @@ impl<const N: usize> fmt::Debug for Secret<N> {
     }
 }
 
+impl<const N: usize> LoadValue for Secret<N> {
+    type Error = anyhow::Error;
+
+    fn load<P: AsRef<Path>>(path: P) -> anyhow::Result<Self> {
+        let mut v = Self::random();
+        let p = path.as_ref();
+        fopen_r(p)?
+            .read_exact_to_end(v.secret_mut())
+            .with_context(|| format!("Could not load file {p:?}"))?;
+        Ok(v)
+    }
+}
+
+impl<const N: usize> LoadValueB64 for Secret<N> {
+    type Error = anyhow::Error;
+
+    fn load_b64<P: AsRef<Path>>(path: P) -> anyhow::Result<Self> {
+        use std::io::Read;
+
+        let mut v = Self::random();
+        let p = path.as_ref();
+        // This might leave some fragments of the secret on the stack;
+        // in practice this is likely not a problem because the stack likely
+        // will be overwritten by something else soon but this is not exactly
+        // guaranteed. It would be possible to remedy this, but since the secret
+        // data will linger in the Linux page cache anyways with the current
+        // implementation, going to great length to erase the secret here is
+        // not worth it right now.
+        b64_reader(&mut fopen_r(p)?)
+            .read_exact(v.secret_mut())
+            .with_context(|| format!("Could not load base64 file {p:?}"))?;
+        Ok(v)
+    }
+}
+
+impl<const N: usize> StoreSecret for Secret<N> {
+    type Error = anyhow::Error;
+
+    fn store_secret<P: AsRef<Path>>(&self, path: P) -> anyhow::Result<()> {
+        std::fs::write(path, self.secret())?;
+        Ok(())
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;
@@ -274,61 +321,3 @@ mod test {
         assert_eq!(new_secret.secret(), &[0; N]);
     }
 }
-
-trait StoreSecret {
-    type Error;
-
-    fn store_secret<P: AsRef<Path>>(&self, path: P) -> Result<(), Self::Error>;
-}
-
-impl<T: StoreValue> StoreSecret for T {
-    type Error = <T as StoreValue>::Error;
-
-    fn store_secret<P: AsRef<Path>>(&self, path: P) -> Result<(), Self::Error> {
-        self.store(path)
-    }
-}
-
-impl<const N: usize> LoadValue for Secret<N> {
-    type Error = anyhow::Error;
-
-    fn load<P: AsRef<Path>>(path: P) -> anyhow::Result<Self> {
-        let mut v = Self::random();
-        let p = path.as_ref();
-        fopen_r(p)?
-            .read_exact_to_end(v.secret_mut())
-            .with_context(|| format!("Could not load file {p:?}"))?;
-        Ok(v)
-    }
-}
-
-impl<const N: usize> LoadValueB64 for Secret<N> {
-    type Error = anyhow::Error;
-
-    fn load_b64<P: AsRef<Path>>(path: P) -> anyhow::Result<Self> {
-        use std::io::Read;
-
-        let mut v = Self::random();
-        let p = path.as_ref();
-        // This might leave some fragments of the secret on the stack;
-        // in practice this is likely not a problem because the stack likely
-        // will be overwritten by something else soon but this is not exactly
-        // guaranteed. It would be possible to remedy this, but since the secret
-        // data will linger in the Linux page cache anyways with the current
-        // implementation, going to great length to erase the secret here is
-        // not worth it right now.
-        b64_reader(&mut fopen_r(p)?)
-            .read_exact(v.secret_mut())
-            .with_context(|| format!("Could not load base64 file {p:?}"))?;
-        Ok(v)
-    }
-}
-
-impl<const N: usize> StoreSecret for Secret<N> {
-    type Error = anyhow::Error;
-
-    fn store_secret<P: AsRef<Path>>(&self, path: P) -> anyhow::Result<()> {
-        std::fs::write(path, self.secret())?;
-        Ok(())
-    }
-}