security(bootstrap): use SafeCell for token storage instead of zeroize

server/crates/arbiter-server/src/actors/bootstrap.rs

@@ -1,4 +1,5 @@
 use crate::db::{self, DatabasePool, schema};
+use arbiter_crypto::safecell::{SafeCell, SafeCellHandle as _};
 use arbiter_proto::{BOOTSTRAP_PATH, home_path};
 
 use diesel::QueryDsl;
@@ -10,7 +11,6 @@ use std::path::{Path, PathBuf};
 use subtle::ConstantTimeEq as _;
 use thiserror::Error;
 use tracing::warn;
-use zeroize::Zeroizing;
 
 const TOKEN_LENGTH: usize = 64;
 
@@ -26,18 +26,23 @@ async fn write_token_file(path: &Path, content: &str) -> Result<(), std::io::Err
     Ok(())
 }
 
-async fn generate_token(path: &Path) -> Result<String, std::io::Error> {
-    let token = UnwrapErr(SysRng)
-        .sample_iter(Alphanumeric)
-        .take(TOKEN_LENGTH)
-        .fold(String::default(), |mut accum, char| {
-            accum += char.to_string().as_str();
-            accum
-        });
+async fn generate_token(path: &Path) -> Result<SafeCell<[u8; TOKEN_LENGTH]>, std::io::Error> {
+    let mut cell = SafeCell::new([0u8; TOKEN_LENGTH]);
+    {
+        let mut buf = cell.write();
+        for (slot, b) in buf
+            .iter_mut()
+            .zip(UnwrapErr(SysRng).sample_iter(Alphanumeric))
+        {
+            *slot = b;
+        }
+    }
 
-    write_token_file(path, &token).await?;
+    let token_str = cell.read_inline(|buf| String::from_utf8_lossy(buf.as_ref()).into_owned());
 
-    Ok(token)
+    write_token_file(path, &token_str).await?;
+
+    Ok(cell)
 }
 
 #[derive(Error, Debug)]
@@ -54,7 +59,7 @@ pub enum Error {
 
 #[derive(Actor)]
 pub struct Bootstrapper {
-    token: Option<Zeroizing<String>>,
+    token: Option<SafeCell<[u8; TOKEN_LENGTH]>>,
     token_path: Option<PathBuf>,
 }
 
@@ -72,7 +77,7 @@ impl Bootstrapper {
         let (token, token_path) = if row_count == 0 {
             let path = home_path()?.join(BOOTSTRAP_PATH);
             let token = generate_token(&path).await?;
-            (Some(Zeroizing::new(token)), Some(path))
+            (Some(token), Some(path))
         } else {
             (None, None)
         };
@@ -82,13 +87,9 @@ impl Bootstrapper {
 }
 
 impl Bootstrapper {
-    fn is_correct_token(&self, token: &str) -> bool {
-        self.token.as_ref().is_some_and(|expected| {
-            let expected_bytes = expected.as_bytes();
-            let token_bytes = token.as_bytes();
-
-            let choice = expected_bytes.ct_eq(token_bytes);
-            bool::from(choice)
+    fn is_correct_token(&mut self, token: &[u8]) -> bool {
+        self.token.as_mut().is_some_and(|expected| {
+            expected.read_inline(|exp| bool::from(exp.as_ref().ct_eq(token)))
         })
     }
 }
@@ -96,13 +97,13 @@ impl Bootstrapper {
 #[messages]
 impl Bootstrapper {
     #[message]
-    pub async fn consume_token(&mut self, token: Zeroizing<String>) -> bool {
+    pub async fn consume_token(&mut self, token: Vec<u8>) -> bool {
         if self.is_correct_token(&token) {
             self.token = None;
-            if let Some(path) = self.token_path.take() {
-                if let Err(e) = tokio::fs::remove_file(&path).await {
-                    warn!(error = ?e, path = ?path, "Failed to delete bootstrap token file after consumption");
-                }
+            if let Some(path) = self.token_path.take()
+                && let Err(e) = tokio::fs::remove_file(&path).await
+            {
+                warn!(error = ?e, path = ?path, "Failed to delete bootstrap token file after consumption");
             }
             true
         } else {
@@ -114,7 +115,9 @@ impl Bootstrapper {
 #[messages]
 impl Bootstrapper {
     #[message]
-    pub fn get_token(&self) -> Option<String> {
-        self.token.as_ref().map(|token| token.to_string())
+    pub fn get_token(&mut self) -> Option<String> {
+        self.token
+            .as_mut()
+            .map(|cell| cell.read_inline(|buf| String::from_utf8_lossy(buf.as_ref()).into_owned()))
     }
 }