commit dcc6042618354018d0a268501fb31273d4c05030
parent 1cedb4fca18143215a569ab8bce78824d7193686
Author: triesap <tyson@radroots.org>
Date: Mon, 3 Aug 2026 23:13:08 +0000
security: harden direct credential custody
- remove debug formatting from secret-bearing domain wrappers
- serialize operating-system keyring access behind one adapter lock
- zeroize temporary credential reads and preserve typed failures
- keep credential storage free of fallback persistence paths
Diffstat:
7 files changed, 40 insertions(+), 34 deletions(-)
diff --git a/core/Cargo.lock b/core/Cargo.lock
@@ -1778,6 +1778,7 @@ dependencies = [
"rusqlite",
"tempfile",
"tokio",
+ "zeroize",
]
[[package]]
diff --git a/core/crates/application/src/secrets.rs b/core/crates/application/src/secrets.rs
@@ -222,7 +222,6 @@ mod tests {
assert!(store.contains(public_key).expect("contains"));
let loaded = store.load(public_key).expect("load");
assert_eq!(loaded.with_exposed_secret(str::len), 64);
- assert!(!format!("{loaded:?}").contains(SECRET));
store.delete(public_key).expect("delete");
assert!(!store.contains(public_key).expect("contains"));
}
@@ -231,7 +230,9 @@ mod tests {
fn secret_store_rejects_duplicates_and_reports_missing_credentials() {
let store = InMemorySecretStore::default();
let public_key = PublicKey::from_bytes([2; 32]);
- let missing = store.load(public_key).expect_err("missing");
+ let Err(missing) = store.load(public_key) else {
+ panic!("missing credential was returned");
+ };
assert_eq!(missing.code(), SafeErrorCode::CredentialMissing);
store
.put(
diff --git a/core/crates/domain/src/key.rs b/core/crates/domain/src/key.rs
@@ -94,12 +94,6 @@ impl Nsec {
}
}
-impl fmt::Debug for Nsec {
- fn fmt(&self, formatter: &mut Formatter<'_>) -> fmt::Result {
- formatter.write_str("Nsec([REDACTED])")
- }
-}
-
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum SecretKeyInputKind {
Nsec,
@@ -150,16 +144,6 @@ impl SecretKeyInput {
}
}
-impl fmt::Debug for SecretKeyInput {
- fn fmt(&self, formatter: &mut Formatter<'_>) -> fmt::Result {
- formatter
- .debug_struct("SecretKeyInput")
- .field("value", &"[REDACTED]")
- .field("kind", &self.kind)
- .finish()
- }
-}
-
#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
pub struct PublicKey([u8; PUBLIC_KEY_BYTE_LENGTH]);
@@ -319,11 +303,7 @@ mod tests {
assert_eq!(input.kind(), SecretKeyInputKind::Hex);
assert_eq!(input.with_exposed_secret(str::len), secret.len());
- assert_eq!(
- format!("{input:?}"),
- "SecretKeyInput { value: \"[REDACTED]\", kind: Hex }"
- );
- assert!(!format!("{input:?}").contains(&secret));
+ assert_eq!(input.with_exposed_secret(str::len), 64);
}
#[test]
@@ -332,7 +312,7 @@ mod tests {
let input = SecretKeyInput::parse(secret.clone()).expect("nsec-shaped input");
assert_eq!(input.kind(), SecretKeyInputKind::Nsec);
- assert!(!format!("{input:?}").contains(&secret));
+ assert_eq!(input.with_exposed_secret(str::len), secret.len());
}
#[test]
@@ -342,7 +322,9 @@ mod tests {
"very-sensitive-input",
&"GG".repeat(PUBLIC_KEY_BYTE_LENGTH),
] {
- let error = SecretKeyInput::parse(value.to_owned()).expect_err("invalid secret");
+ let Err(error) = SecretKeyInput::parse(value.to_owned()) else {
+ panic!("invalid secret accepted");
+ };
assert_eq!(error.code(), SafeErrorCode::InvalidSecretKey);
if !value.is_empty() {
assert!(!format!("{error:?}").contains(value));
@@ -362,8 +344,7 @@ mod tests {
fn nsec_is_redacted_and_exposed_only_to_a_scoped_operation() {
let nsec = Nsec::from_encoded(NSEC.to_owned()).expect("valid nsec shape");
- assert_eq!(format!("{nsec:?}"), "Nsec([REDACTED])");
- assert!(!format!("{nsec:?}").contains(NSEC));
+ assert_eq!(nsec.with_exposed_secret(str::len), NSEC.len());
assert_eq!(nsec.with_exposed_secret(str::len), NSEC.len());
}
diff --git a/core/crates/ffi/src/commands.rs b/core/crates/ffi/src/commands.rs
@@ -271,7 +271,7 @@ impl StudioAppCore {
let actor = RuntimeActorHandle::open(
path,
relays,
- Arc::new(OsKeyringSecretStore),
+ Arc::new(OsKeyringSecretStore::default()),
Arc::new(SystemClock),
Arc::new(SdkNostrClient::new(Duration::from_secs(5))),
NonZeroUsize::new(ACTOR_MAILBOX_CAPACITY).expect("nonzero actor mailbox capacity"),
diff --git a/core/crates/nostr/src/keys.rs b/core/crates/nostr/src/keys.rs
@@ -122,7 +122,8 @@ mod tests {
assert!(npub.as_str().starts_with("npub1"));
assert_eq!(secret.with_exposed_secret(str::len), 64);
assert_eq!(nsec.with_exposed_secret(str::len), 63);
- assert!(!format!("{secret:?} {nsec:?}").contains("nsec1"));
+ assert_eq!(secret.with_exposed_secret(str::len), 64);
+ assert_eq!(nsec.with_exposed_secret(str::len), 63);
}
#[test]
diff --git a/core/crates/storage/Cargo.toml b/core/crates/storage/Cargo.toml
@@ -11,6 +11,7 @@ radroots-studio-application = { path = "../application" }
radroots-studio-domain = { path = "../domain" }
fs2.workspace = true
keyring.workspace = true
+zeroize.workspace = true
refinery.workspace = true
rusqlite.workspace = true
tokio.workspace = true
diff --git a/core/crates/storage/src/os_keyring.rs b/core/crates/storage/src/os_keyring.rs
@@ -1,23 +1,38 @@
+use std::sync::{Mutex, MutexGuard};
+
use keyring::{Entry, Error as KeyringError};
use radroots_studio_application::SecretStore;
use radroots_studio_domain::{PublicKey, SafeError, SafeErrorCode, SafeMessage, SecretKeyInput};
+use zeroize::Zeroizing;
pub const CREDENTIAL_SERVICE: &str = "org.radroots.studio.nostr";
-#[derive(Clone, Copy, Debug, Default)]
-pub struct OsKeyringSecretStore;
+#[derive(Default)]
+pub struct OsKeyringSecretStore {
+ operation_lock: Mutex<()>,
+}
impl OsKeyringSecretStore {
fn entry(public_key: PublicKey) -> Result<Entry, SafeError> {
Entry::new(CREDENTIAL_SERVICE, &public_key.to_hex()).map_err(|_| keyring_unavailable())
}
+
+ fn operation(&self) -> MutexGuard<'_, ()> {
+ self.operation_lock
+ .lock()
+ .unwrap_or_else(std::sync::PoisonError::into_inner)
+ }
}
impl SecretStore for OsKeyringSecretStore {
fn put(&self, public_key: PublicKey, secret: SecretKeyInput) -> Result<(), SafeError> {
+ let _operation = self.operation();
let entry = Self::entry(public_key)?;
match entry.get_password() {
- Ok(_) => return Err(credential_exists()),
+ Ok(password) => {
+ drop(Zeroizing::new(password));
+ return Err(credential_exists());
+ }
Err(KeyringError::NoEntry) => {}
Err(_) => return Err(keyring_unavailable()),
}
@@ -27,6 +42,7 @@ impl SecretStore for OsKeyringSecretStore {
}
fn load(&self, public_key: PublicKey) -> Result<SecretKeyInput, SafeError> {
+ let _operation = self.operation();
let password = Self::entry(public_key)?
.get_password()
.map_err(|error| map_read_error(&error))?;
@@ -34,14 +50,19 @@ impl SecretStore for OsKeyringSecretStore {
}
fn contains(&self, public_key: PublicKey) -> Result<bool, SafeError> {
+ let _operation = self.operation();
match Self::entry(public_key)?.get_password() {
- Ok(_) => Ok(true),
+ Ok(password) => {
+ drop(Zeroizing::new(password));
+ Ok(true)
+ }
Err(KeyringError::NoEntry) => Ok(false),
Err(_) => Err(keyring_unavailable()),
}
}
fn delete(&self, public_key: PublicKey) -> Result<(), SafeError> {
+ let _operation = self.operation();
Self::entry(public_key)?
.delete_credential()
.map_err(|error| map_read_error(&error))
@@ -93,7 +114,7 @@ mod tests {
#[test]
#[ignore = "mutates the current user's operating-system credential store"]
fn real_keyring_smoke_round_trips_and_deletes() {
- let store = OsKeyringSecretStore;
+ let store = OsKeyringSecretStore::default();
let public_key = PublicKey::from_bytes([0xcd; 32]);
let _ = store.delete(public_key);
store
