fix(secret-store): use safe file locking

Signed-off-by: Tom Brow <tomb@block.xyz>
This commit is contained in:
Tom Brow
2026-08-14 09:20:15 -07:00
parent a01e2afafd
commit 80a5535dc3
3 changed files with 41 additions and 124 deletions
Generated
+24 -14
View File
@@ -1373,12 +1373,12 @@ dependencies = [
name = "buzz-secret-store"
version = "0.1.0"
dependencies = [
"fs4",
"keyring",
"libc",
"rustix 1.1.4",
"security-framework 3.7.0",
"serde_json",
"thiserror 2.0.18",
"windows-sys 0.61.2",
"zeroize",
]
@@ -1727,7 +1727,7 @@ version = "3.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34"
dependencies = [
"windows-sys 0.59.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -2348,7 +2348,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ccc2776f0c61eca1ca32528f85548abd1a4be8fb53d1b21c013e4f18da1e7090"
dependencies = [
"data-encoding",
"syn 1.0.109",
"syn 2.0.117",
]
[[package]]
@@ -2559,7 +2559,7 @@ dependencies = [
"libc",
"option-ext",
"redox_users",
"windows-sys 0.59.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -2782,7 +2782,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb"
dependencies = [
"libc",
"windows-sys 0.59.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -3047,6 +3047,16 @@ dependencies = [
"percent-encoding",
]
[[package]]
name = "fs4"
version = "1.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7e72ed92b67c146290f88e9c89d60ca163ea417a446f61ffd7b72df3e7f1dfd5"
dependencies = [
"rustix 1.1.4",
"windows-sys 0.61.2",
]
[[package]]
name = "fs_extra"
version = "1.3.0"
@@ -3198,7 +3208,7 @@ dependencies = [
"libc",
"log",
"rustversion",
"windows-link 0.1.3",
"windows-link 0.2.1",
"windows-result 0.4.1",
]
@@ -6043,7 +6053,7 @@ version = "0.50.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5"
dependencies = [
"windows-sys 0.59.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -7516,7 +7526,7 @@ dependencies = [
"once_cell",
"socket2",
"tracing",
"windows-sys 0.59.0",
"windows-sys 0.60.2",
]
[[package]]
@@ -8200,7 +8210,7 @@ dependencies = [
"errno",
"libc",
"linux-raw-sys 0.12.1",
"windows-sys 0.59.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -8259,7 +8269,7 @@ dependencies = [
"security-framework 3.7.0",
"security-framework-sys",
"webpki-root-certs",
"windows-sys 0.59.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -8542,7 +8552,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5b55fb86dfd3a2f5f76ea78310a88f96c4ea21a3031f8d212443d56123fd0521"
dependencies = [
"libc",
"windows-sys 0.59.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -9667,7 +9677,7 @@ dependencies = [
"getrandom 0.4.3",
"once_cell",
"rustix 1.1.4",
"windows-sys 0.59.0",
"windows-sys 0.61.2",
]
[[package]]
@@ -11034,7 +11044,7 @@ version = "0.1.11"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22"
dependencies = [
"windows-sys 0.59.0",
"windows-sys 0.61.2",
]
[[package]]
+3 -3
View File
@@ -9,15 +9,16 @@ description = "Platform secure storage shared by Buzz clients"
[features]
default = ["system-keyring"]
system-keyring = ["dep:keyring", "dep:security-framework"]
system-keyring = ["dep:fs4", "dep:keyring", "dep:rustix", "dep:security-framework"]
[dependencies]
serde_json = { workspace = true }
thiserror = { workspace = true }
zeroize = { workspace = true }
fs4 = { version = "1.1.0", optional = true }
[target.'cfg(unix)'.dependencies]
libc = "0.2"
rustix = { version = "1.1.4", features = ["process"], optional = true }
[target.'cfg(target_os = "linux")'.dependencies]
keyring = { version = "3.6.3", default-features = false, features = ["sync-secret-service", "vendored"], optional = true }
@@ -28,4 +29,3 @@ security-framework = { version = "3.7.0", features = ["OSX_10_15"], optional = t
[target.'cfg(windows)'.dependencies]
keyring = { version = "3.6.3", default-features = false, features = ["windows-native", "vendored"], optional = true }
windows-sys = { version = "0.61", features = ["Win32_Foundation", "Win32_Security", "Win32_System_Threading"] }
+14 -107
View File
@@ -179,21 +179,17 @@ pub fn load_desktop_release_identity() -> Result<Option<String>, ReadonlySecretE
/// terminal-launched dev build resolve `/tmp` to the same inode, so they
/// contend on the same lockfile and achieve mutual exclusion.
///
/// On Windows the same name used for the kernel mutex is derived from the
/// lockfile path, so the service-keyed uniqueness is preserved.
/// On Windows the lockfile lives in the per-user temporary directory.
#[cfg(feature = "system-keyring")]
fn blob_lockfile_path(service: &str) -> PathBuf {
#[cfg(unix)]
{
// Use the real UID so distinct users get distinct lockfiles.
// SAFETY: getuid() is always safe on Unix — it never fails.
let uid = unsafe { libc::getuid() };
let uid = rustix::process::getuid().as_raw();
PathBuf::from(format!("/tmp/buzz-keychain-{uid}-{service}.lock"))
}
#[cfg(not(unix))]
{
// Windows: no lockfile used (named mutex instead); this path is only
// used to derive the mutex name and for test assertions.
std::env::temp_dir().join(format!("buzz-keychain-{service}.lock"))
}
}
@@ -203,7 +199,6 @@ fn blob_lockfile_path(service: &str) -> PathBuf {
/// Opens (or creates) the lockfile and blocks until the lock is acquired.
/// Returns the open `File`; the lock is released when the file is dropped.
///
/// On non-Unix/non-Windows platforms this is a no-op that returns a stub.
#[cfg(feature = "system-keyring")]
fn acquire_blob_lock(service: &str) -> Result<BlobLockGuard, String> {
let path = blob_lockfile_path(service);
@@ -212,114 +207,26 @@ fn acquire_blob_lock(service: &str) -> Result<BlobLockGuard, String> {
/// RAII guard that holds an exclusive advisory file lock.
///
/// On Unix, implemented via `flock(2)` on a lockfile in the system temp dir.
/// On Windows, implemented via a named kernel mutex (cross-process, no file I/O
/// needed). The Windows mutex handle is released on drop.
/// Implemented by `fs4`, which uses `flock(2)` on Unix and `LockFileEx` on
/// Windows. Closing the file releases the lock on both platforms.
#[cfg(feature = "system-keyring")]
struct BlobLockGuard {
/// The open lockfile. Never read held purely for RAII: closing the fd
/// releases the `flock(LOCK_EX)` on Unix.
#[cfg(unix)]
/// The open lockfile. Never read; held purely for RAII.
#[allow(dead_code)]
file: std::fs::File,
#[cfg(windows)]
mutex_handle: windows_sys::Win32::Foundation::HANDLE,
}
#[cfg(feature = "system-keyring")]
impl BlobLockGuard {
fn acquire(path: &std::path::Path) -> Result<Self, String> {
#[cfg(unix)]
{
let file = std::fs::OpenOptions::new()
.create(true)
.truncate(false)
.write(true)
.open(path)
.map_err(|e| format!("blob lock open {}: {e}", path.display()))?;
use std::os::unix::io::AsRawFd;
// LOCK_EX blocks until the lock is acquired (no LOCK_NB).
let ret = unsafe { libc::flock(file.as_raw_fd(), libc::LOCK_EX) };
if ret != 0 {
let err = std::io::Error::last_os_error();
return Err(format!("blob lock flock: {err}"));
}
return Ok(BlobLockGuard { file });
}
#[cfg(windows)]
{
// Named kernel mutexes are cross-process on Windows — no lockfile
// needed. Derive a unique mutex name from the lockfile path so
// distinct services get distinct mutexes.
let name_str = format!(
"Local\\BuzzKeychain-{}",
path.file_stem()
.and_then(|s| s.to_str())
.unwrap_or("default")
);
// Encode as null-terminated UTF-16.
let name_wide: Vec<u16> = name_str
.encode_utf16()
.chain(std::iter::once(0u16))
.collect();
use windows_sys::Win32::Foundation::WAIT_OBJECT_0;
use windows_sys::Win32::Security::SECURITY_ATTRIBUTES;
use windows_sys::Win32::System::Threading::{
CreateMutexW, WaitForSingleObject, INFINITE,
};
// CreateMutexW: lpMutexAttributes = null (default security),
// bInitialOwner = FALSE (0), lpName = our mutex name.
let handle = unsafe {
CreateMutexW(
std::ptr::null::<SECURITY_ATTRIBUTES>(),
0,
name_wide.as_ptr(),
)
};
// HANDLE = *mut c_void; null means creation failed.
if handle.is_null() {
let err = std::io::Error::last_os_error();
return Err(format!("blob lock CreateMutexW: {err}"));
}
let wait_result = unsafe { WaitForSingleObject(handle, INFINITE) };
if wait_result != WAIT_OBJECT_0 {
// Also accept WAIT_ABANDONED (0x80) — previous holder crashed;
// the mutex is still acquired and we own it.
if wait_result != windows_sys::Win32::Foundation::WAIT_ABANDONED {
let err = std::io::Error::last_os_error();
unsafe { windows_sys::Win32::Foundation::CloseHandle(handle) };
return Err(format!(
"blob lock WaitForSingleObject: {wait_result} / {err}"
));
}
}
return Ok(BlobLockGuard {
mutex_handle: handle,
});
}
// Fallback for exotic platforms: no-op lock (only Unix/Windows ship).
#[allow(unreachable_code)]
Err("blob lock: unsupported platform".to_string())
}
}
#[cfg(feature = "system-keyring")]
impl Drop for BlobLockGuard {
fn drop(&mut self) {
#[cfg(unix)]
{
// Dropping `self.file` closes the fd, which releases flock on Unix.
// Nothing explicit needed.
}
#[cfg(windows)]
{
unsafe {
windows_sys::Win32::System::Threading::ReleaseMutex(self.mutex_handle);
windows_sys::Win32::Foundation::CloseHandle(self.mutex_handle);
}
}
let file = std::fs::OpenOptions::new()
.create(true)
.truncate(false)
.write(true)
.open(path)
.map_err(|e| format!("blob lock open {}: {e}", path.display()))?;
fs4::FileExt::lock(&file).map_err(|e| format!("blob lock acquire: {e}"))?;
Ok(BlobLockGuard { file })
}
}
@@ -1233,7 +1140,7 @@ mod tests {
let path = blob_lockfile_path("buzz-desktop");
#[cfg(unix)]
{
let uid = unsafe { libc::getuid() };
let uid = rustix::process::getuid().as_raw();
assert!(
path.starts_with("/tmp"),
"lockfile {path:?} must start with /tmp (not $TMPDIR)"