feat: abstract key storage, in-memory fallback for non-linux platforms

Cargo.toml

@@ -33,7 +33,6 @@ regex = "1.10.2"
 mime = "0.3.17"
 aes-gcm = { version = "0.10.3", features = ["zeroize", "std"] }
 constant_time_eq = "0.3.1"
-linux-keyutils = { version = "0.2.4", features = ["std"] }
 url = { version = "2.5.0", features = ["serde"] }
 openidconnect = "3.4.0"
 async-trait = "0.1.77"
@@ -43,5 +42,8 @@ futures = "0.3.31"
 futures-util = { version = "0.3.30", default-features = false, features = ["io"] }
 htpasswd-verify = { git = "https://github.com/twistedfall/htpasswd-verify", rev = "ff14703083cbd639f7d05622b398926f3e718d61" }
 
+[target.'cfg(target_os = "linux")'.dependencies]
+linux-keyutils = { version = "0.2.4", features = ["std"] }
+
 [dev-dependencies]
 mockito = "1.7.0"

src/keepass/key.rs

@@ -1,24 +1,44 @@
+use std::fmt::{Display, Formatter};
 use std::ops::Deref;
 use std::time::Duration;
 
 use anyhow::Result;
-use linux_keyutils::{Key, KeyPermissions, KeyRing, KeyRingIdentifier};
 use secrecy::{ExposeSecret, SecretBox};
 
 use crate::auth::gen_token;
 
-pub struct SecretKey {
-    pub key_id: KeyId,
-    data: SecretBox<[u8]>,
-    timeout: Duration,
-}
+#[cfg(target_os = "linux")]
+mod keyring;
+#[cfg(target_os = "linux")]
+use keyring as store;
 
-pub type KeyId = String;
+#[cfg(any(not(target_os = "linux"), test))]
+mod memory;
+#[cfg(not(target_os = "linux"))]
+use memory as store;
 
+pub type KeyId = String;
 
 const ID_LENGTH: usize = 16;
-const KEYRING_PERM: u32 = 0x3f000000;
 
+// returned when a key is missing, expired or revoked - the http layer
+// treats this as 'db closed' rather than a server fault
+#[derive(Debug, Clone)]
+pub struct KeyUnavailableError;
+
+impl Display for KeyUnavailableError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(f, "key not found, expired or revoked")
+    }
+}
+
+impl std::error::Error for KeyUnavailableError {}
+
+pub struct SecretKey {
+    pub key_id: KeyId,
+    data: SecretBox<[u8]>,
+    timeout: Duration,
+}
 
 impl SecretKey {
     pub fn new(secret: Box<[u8]>) -> Self {
@@ -30,50 +50,30 @@ impl SecretKey {
     }
 
     pub fn retrieve(key_id: &KeyId, timeout: Duration) -> Result<Self> {
-        let keyr = get_keyring()?;
-
-        let mut k = keyr.search(key_id.as_str())?;
-
-        // TODO: determine buffer size
-        let mut data = vec![0; 32].into_boxed_slice();
-        k.read(&mut data)?;
-
-        let key = Self {
-            key_id: key_id.clone(),
-            data: SecretBox::new(data),
-            timeout,
-        };
-
-        key.update_timeout(&mut k, timeout)?;
-
-        Ok(key)
+        let data = store::retrieve(key_id, timeout)?;
+
+        Ok(
+            Self {
+                key_id: key_id.clone(),
+                data: SecretBox::new(data),
+                timeout,
+            }
+        )
     }
 
     pub fn store(&mut self, timeout: Duration) -> Result<&mut Self> {
-        let keyr = get_keyring()?;
-
-        let key = keyr.add_key(self.key_id.as_str(), self.expose_secret())?;
-        key.set_timeout(timeout.as_secs() as usize)?;
-        key.set_perms(KeyPermissions::from_u32(KEYRING_PERM))?;
+        store::store(&self.key_id, self.expose_secret(), timeout)?;
 
         self.timeout = timeout;
         Ok(self)
     }
 
     // retrieve will fail after revoke
     pub fn revoke(&mut self) -> Result<&mut Self> {
-        let keyr = get_keyring()?;
-
-        let k = keyr.search(self.key_id.as_str())?;
-
-        k.revoke()?;
+        store::revoke(&self.key_id)?;
 
         Ok(self)
     }
-
-    fn update_timeout(&self, key: &mut Key, timeout: Duration) -> Result<()> {
-        Ok(key.set_timeout(timeout.as_secs() as usize)?)
-    }
 }
 
 impl Deref for SecretKey {
@@ -84,19 +84,13 @@ impl Deref for SecretKey {
     }
 }
 
-fn get_keyring() -> Result<KeyRing> {
-    // TODO: Make other keyrings available
-    // TODO: Investigate why key in Process keyring doesn't persist
-    Ok(KeyRing::from_special_id(KeyRingIdentifier::Session, false)?)
-}
-
 #[cfg(test)]
 mod tests {
     use std::time::Duration;
 
     use secrecy::ExposeSecret;
 
-    use crate::keepass::key::SecretKey;
+    use crate::keepass::key::{memory, SecretKey};
 
     #[test]
     fn key_roundtrip() {
@@ -106,4 +100,36 @@ mod tests {
 
         assert_eq!(key.expose_secret(), data.expose_secret());
     }
+
+    // regression: the store must not assume a fixed secret length
+    #[test]
+    fn key_roundtrip_other_lengths() {
+        for secret in [&b"short"[..], &[7u8; 64][..]] {
+            let mut key = SecretKey::new(secret.to_vec().into_boxed_slice());
+            key.store(Duration::from_secs(10)).unwrap();
+            let data = SecretKey::retrieve(&key.key_id, Duration::from_secs(10)).unwrap();
+
+            assert_eq!(key.expose_secret(), data.expose_secret());
+        }
+    }
+
+    #[test]
+    fn memory_store_roundtrip_and_revoke() {
+        let secret = b"0123456789abcdef0123456789abcdef";
+
+        memory::store("test-key", secret, Duration::from_secs(10)).unwrap();
+        let data = memory::retrieve("test-key", Duration::from_secs(10)).unwrap();
+        assert_eq!(data.as_ref(), secret);
+
+        memory::revoke("test-key").unwrap();
+        assert!(memory::retrieve("test-key", Duration::from_secs(10)).is_err());
+    }
+
+    #[test]
+    fn memory_store_expires() {
+        let secret = b"0123456789abcdef0123456789abcdef";
+
+        memory::store("expiring-key", secret, Duration::from_secs(0)).unwrap();
+        assert!(memory::retrieve("expiring-key", Duration::from_secs(10)).is_err());
+    }
 }

src/keepass/key/keyring.rs

@@ -0,0 +1,56 @@
+// linux kernel keyring backed key store
+
+use std::time::Duration;
+
+use anyhow::Result;
+use linux_keyutils::{KeyError, KeyPermissions, KeyRing, KeyRingIdentifier};
+
+use super::KeyUnavailableError;
+
+const KEYRING_PERM: u32 = 0x3f000000;
+
+pub(super) fn store(key_id: &str, secret: &[u8], timeout: Duration) -> Result<()> {
+    let keyr = get_keyring()?;
+
+    let key = keyr.add_key(key_id, secret).map_err(map_err)?;
+    key.set_timeout(timeout.as_secs() as usize).map_err(map_err)?;
+    key.set_perms(KeyPermissions::from_u32(KEYRING_PERM)).map_err(map_err)?;
+
+    Ok(())
+}
+
+pub(super) fn retrieve(key_id: &str, timeout: Duration) -> Result<Box<[u8]>> {
+    let keyr = get_keyring()?;
+
+    let key = keyr.search(key_id).map_err(map_err)?;
+
+    // read the actual payload length instead of assuming 32 bytes,
+    // so secrets of any length roundtrip unchanged
+    let data = key.read_to_vec().map_err(map_err)?.into_boxed_slice();
+
+    key.set_timeout(timeout.as_secs() as usize).map_err(map_err)?;
+
+    Ok(data)
+}
+
+pub(super) fn revoke(key_id: &str) -> Result<()> {
+    let keyr = get_keyring()?;
+
+    let key = keyr.search(key_id).map_err(map_err)?;
+    key.revoke().map_err(map_err)?;
+
+    Ok(())
+}
+
+fn map_err(err: KeyError) -> anyhow::Error {
+    match err {
+        KeyError::KeyDoesNotExist | KeyError::KeyExpired | KeyError::KeyRevoked => KeyUnavailableError.into(),
+        err => anyhow::Error::new(err),
+    }
+}
+
+fn get_keyring() -> Result<KeyRing> {
+    // TODO: Make other keyrings available
+    // TODO: Investigate why key in Process keyring doesn't persist
+    Ok(KeyRing::from_special_id(KeyRingIdentifier::Session, false)?)
+}

src/keepass/key/memory.rs

@@ -0,0 +1,66 @@
+// in-memory key store for platforms without kernel keyrings.
+// keys stay in (zeroized-on-drop) process memory instead of the kernel, which
+// enables native macos/windows runs but is weaker than the linux keyring: the
+// pages are not locked, so a key can reach swap or a core dump. accepted for
+// the platforms that have no kernel keyring to put it in.
+
+use std::collections::HashMap;
+use std::sync::{Mutex, OnceLock};
+use std::time::{Duration, Instant};
+
+use anyhow::Result;
+use zeroize::Zeroize;
+
+use super::KeyUnavailableError;
+
+struct Entry {
+    secret: Vec<u8>,
+    expiry: Instant,
+}
+
+impl Drop for Entry {
+    fn drop(&mut self) {
+        self.secret.zeroize();
+    }
+}
+
+fn entries() -> &'static Mutex<HashMap<String, Entry>> {
+    static STORE: OnceLock<Mutex<HashMap<String, Entry>>> = OnceLock::new();
+    STORE.get_or_init(Default::default)
+}
+
+pub(super) fn store(key_id: &str, secret: &[u8], timeout: Duration) -> Result<()> {
+    let mut store = entries().lock().expect("key store lock poisoned");
+
+    // drop expired keys so the map doesn't grow unboundedly
+    let now = Instant::now();
+    store.retain(|_, entry| now < entry.expiry);
+
+    store.insert(key_id.to_string(), Entry {
+        secret: secret.to_vec(),
+        expiry: now + timeout,
+    });
+
+    Ok(())
+}
+
+pub(super) fn retrieve(key_id: &str, timeout: Duration) -> Result<Box<[u8]>> {
+    let mut store = entries().lock().expect("key store lock poisoned");
+
+    let entry = store.get_mut(key_id).ok_or(KeyUnavailableError)?;
+    if Instant::now() >= entry.expiry {
+        store.remove(key_id);
+        return Err(KeyUnavailableError.into());
+    }
+
+    entry.expiry = Instant::now() + timeout;
+    Ok(entry.secret.clone().into_boxed_slice())
+}
+
+pub(super) fn revoke(key_id: &str) -> Result<()> {
+    entries().lock().expect("key store lock poisoned")
+        .remove(key_id)
+        .ok_or(KeyUnavailableError)?;
+
+    Ok(())
+}

src/server/route/util.rs

@@ -1,7 +1,6 @@
 use actix_session::Session;
 use actix_web::HttpResponse;
 use anyhow::{anyhow, bail};
-use linux_keyutils::KeyError;
 use log::{error, info};
 use serde_json::json;
 
@@ -10,7 +9,7 @@ use crate::auth_backend::UserInfo;
 use crate::config::config::Config;
 use crate::keepass::db_cache::{CacheExpiredError, DbCache};
 use crate::keepass::keepass::KeePass;
-use crate::keepass::key::{KeyId, SecretKey};
+use crate::keepass::key::{KeyId, KeyUnavailableError, SecretKey};
 use crate::session::AuthSession;
 
 pub const SESSION_KEY_KEY_ID: &str = "key_id";
@@ -125,7 +124,7 @@ pub(crate) async fn get_db(session: &Session, config: &Config, db_cache: &DbCach
                 }
             );
 
-            return match err.downcast_ref::<KeyError>() {
+            return match err.downcast_ref::<KeyUnavailableError>() {
                 Some(_) => {
                     _close_db(session, config, db_cache).await?;
 
@@ -197,12 +196,9 @@ pub(crate) fn revoke_key(config: &Config, session: &Session) -> anyhow::Result<(
 fn check_key_err<F>(ok: F, err: anyhow::Error) -> anyhow::Result<()>
     where F: Fn() -> anyhow::Result<()>
 {
-    match err.downcast_ref::<KeyError>() {
-        Some(e) => match e {
-            // Ignore non-existent, expired or already revoked
-            KeyError::KeyDoesNotExist | KeyError::KeyExpired | KeyError::KeyRevoked => ok(),
-            _ => Err(err),
-        }
+    // Ignore non-existent, expired or already revoked
+    match err.downcast_ref::<KeyUnavailableError>() {
+        Some(_) => ok(),
         None => Err(err),
     }
 }