fix(security): harden token file permissions (closes #2)
- save_restore_token: use create_new(true) + mode(0o600) for exclusive atomic file creation, preventing symlink attacks and predictable temp file exploitation - token_path: return Option, eliminate insecure /tmp fallback - load_restore_token: reject insecure files (symlinks, wrong owner, group/world-readable permissions) - Directory creation uses DirBuilderExt::mode(0o700) bypassing umask - Added verify_secure_dir and ensure_secure_parent with full metadata validation (owner, permissions, symlink rejection) - Added 11 regression tests covering all security scenarios
This commit is contained in:
1
Cargo.lock
generated
1
Cargo.lock
generated
@@ -2505,6 +2505,7 @@ dependencies = [
|
||||
"signal-hook",
|
||||
"signal-hook-mio",
|
||||
"str0m",
|
||||
"tempfile",
|
||||
"tokio",
|
||||
"tracing",
|
||||
"tracing-subscriber",
|
||||
|
||||
@@ -28,3 +28,6 @@ crossbeam-channel = "0.5"
|
||||
str0m = "0.20"
|
||||
serde_json = "1"
|
||||
dirs = "6"
|
||||
|
||||
[dev-dependencies]
|
||||
tempfile = "3.27.0"
|
||||
|
||||
@@ -246,27 +246,162 @@ impl CapPortal {
|
||||
}
|
||||
}
|
||||
|
||||
fn token_path() -> PathBuf {
|
||||
let base = dirs::cache_dir()
|
||||
.unwrap_or_else(|| PathBuf::from("/tmp"));
|
||||
base.join("wl-webrtc").join("portal-restore-token")
|
||||
fn token_path() -> Option<PathBuf> {
|
||||
dirs::cache_dir().map(|base| base.join("wl-webrtc").join("portal-restore-token"))
|
||||
}
|
||||
|
||||
/// Verify that `path` is a directory owned by the current user with no group/other permissions.
|
||||
/// Rejects symlinks at the path itself (but allows the resolved target to be a real dir).
|
||||
fn verify_secure_dir(path: &std::path::Path) -> bool {
|
||||
use std::os::unix::fs::{MetadataExt, PermissionsExt};
|
||||
|
||||
match std::fs::symlink_metadata(path) {
|
||||
Ok(meta) => {
|
||||
if meta.file_type().is_symlink() {
|
||||
tracing::warn!("Token parent dir is a symlink, rejecting: {}", path.display());
|
||||
return false;
|
||||
}
|
||||
// Must be a directory
|
||||
if !meta.is_dir() {
|
||||
tracing::warn!("Token parent path is not a directory: {}", path.display());
|
||||
return false;
|
||||
}
|
||||
// Must be owned by current user
|
||||
if meta.uid() != unsafe { libc::getuid() } {
|
||||
tracing::warn!("Token parent dir not owned by current user: {}", path.display());
|
||||
return false;
|
||||
}
|
||||
// No group or other permissions (mode must be 0o700 exactly within the 0o777 mask)
|
||||
let mode = meta.permissions().mode() & 0o777;
|
||||
if mode != 0o700 {
|
||||
tracing::warn!(
|
||||
"Token parent dir has insecure permissions {:o}, expected 0700: {}",
|
||||
mode,
|
||||
path.display()
|
||||
);
|
||||
return false;
|
||||
}
|
||||
true
|
||||
}
|
||||
Err(e) => {
|
||||
tracing::warn!("Failed to stat token parent dir: {e}");
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Ensure the parent directory exists with restrictive permissions (0o700).
|
||||
/// Returns false if the directory could not be created or is insecure.
|
||||
fn ensure_secure_parent(parent: &std::path::Path) -> bool {
|
||||
use std::os::unix::fs::{DirBuilderExt, PermissionsExt};
|
||||
|
||||
if parent.exists() {
|
||||
// Directory exists — try to tighten permissions, then verify.
|
||||
// set_permissions follows symlinks, which is fine here since
|
||||
// we verify with symlink_metadata in verify_secure_dir.
|
||||
if let Err(e) = std::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700)) {
|
||||
tracing::warn!("Failed to set directory permissions: {e}");
|
||||
return false;
|
||||
}
|
||||
return verify_secure_dir(parent);
|
||||
}
|
||||
|
||||
// Create with restrictive mode — DirBuilderExt::mode bypasses umask.
|
||||
let mut builder = std::fs::DirBuilder::new();
|
||||
builder.recursive(true);
|
||||
builder.mode(0o700);
|
||||
if let Err(e) = builder.create(parent) {
|
||||
tracing::warn!("Failed to create token directory: {e}");
|
||||
return false;
|
||||
}
|
||||
|
||||
// Verify after creation (belt-and-suspenders)
|
||||
verify_secure_dir(parent)
|
||||
}
|
||||
|
||||
fn load_restore_token() -> Option<String> {
|
||||
let path = token_path();
|
||||
load_restore_token_from(token_path()?)
|
||||
}
|
||||
|
||||
fn load_restore_token_from(path: PathBuf) -> Option<String> {
|
||||
use std::os::unix::fs::{MetadataExt, PermissionsExt};
|
||||
|
||||
let meta = match std::fs::symlink_metadata(&path) {
|
||||
Ok(m) => m,
|
||||
Err(_) => return None,
|
||||
};
|
||||
|
||||
if meta.file_type().is_symlink() {
|
||||
tracing::warn!("Token file is a symlink, refusing to read: {}", path.display());
|
||||
return None;
|
||||
}
|
||||
if !meta.is_file() {
|
||||
tracing::warn!("Token path is not a regular file: {}", path.display());
|
||||
return None;
|
||||
}
|
||||
if meta.uid() != unsafe { libc::getuid() } {
|
||||
tracing::warn!("Token file not owned by current user: {}", path.display());
|
||||
return None;
|
||||
}
|
||||
let mode = meta.permissions().mode() & 0o777;
|
||||
if mode & 0o077 != 0 {
|
||||
tracing::warn!(
|
||||
"Token file has insecure permissions {:o}, refusing to read: {}",
|
||||
mode,
|
||||
path.display()
|
||||
);
|
||||
return None;
|
||||
}
|
||||
|
||||
let token = std::fs::read_to_string(&path).ok()?;
|
||||
let trimmed = token.trim().to_string();
|
||||
if trimmed.is_empty() { None } else { Some(trimmed) }
|
||||
}
|
||||
|
||||
fn save_restore_token(token: &str) {
|
||||
let path = token_path();
|
||||
if let Some(parent) = path.parent() {
|
||||
let _ = std::fs::create_dir_all(parent);
|
||||
let Some(path) = token_path() else {
|
||||
tracing::warn!("No secure cache directory available, skipping token save");
|
||||
return;
|
||||
};
|
||||
save_restore_token_to(token, &path);
|
||||
}
|
||||
|
||||
fn save_restore_token_to(token: &str, path: &std::path::Path) {
|
||||
use std::fs::OpenOptions;
|
||||
use std::io::Write;
|
||||
use std::os::unix::fs::OpenOptionsExt;
|
||||
|
||||
let Some(parent) = path.parent() else {
|
||||
tracing::warn!("Token path has no parent directory");
|
||||
return;
|
||||
};
|
||||
|
||||
if !ensure_secure_parent(parent) {
|
||||
tracing::warn!("Parent directory is insecure, refusing to save token");
|
||||
return;
|
||||
}
|
||||
match std::fs::write(&path, token) {
|
||||
|
||||
// Use a unique temp file to prevent symlink attacks.
|
||||
// create_new(true) guarantees exclusive creation — fails if file already exists,
|
||||
// and does NOT follow existing symlinks.
|
||||
let tmp_path = path.with_extension(format!("{}.tmp", std::process::id()));
|
||||
let result = (|| -> std::io::Result<()> {
|
||||
let mut f = OpenOptions::new()
|
||||
.write(true)
|
||||
.create_new(true)
|
||||
.mode(0o600)
|
||||
.open(&tmp_path)?;
|
||||
f.write_all(token.as_bytes())?;
|
||||
f.sync_all()?;
|
||||
std::fs::rename(&tmp_path, path)?;
|
||||
Ok(())
|
||||
})();
|
||||
match result {
|
||||
Ok(()) => tracing::info!("Saved portal restore token"),
|
||||
Err(e) => tracing::warn!("Failed to save restore token: {e}"),
|
||||
Err(e) => {
|
||||
let _ = std::fs::remove_file(&tmp_path);
|
||||
tracing::warn!("Failed to save restore token: {e}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -645,6 +780,7 @@ fn spa_to_drm_fourcc(format: libspa::param::video::VideoFormat) -> u32 {
|
||||
mod tests {
|
||||
use super::*;
|
||||
use drm_fourcc::DrmFourcc;
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
#[test]
|
||||
fn spa_to_drm_fourcc_all_32bit() {
|
||||
@@ -688,4 +824,160 @@ mod tests {
|
||||
use libspa::param::video::VideoFormat;
|
||||
assert_eq!(spa_to_drm_fourcc(VideoFormat::NV12), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn token_path_never_uses_tmp() {
|
||||
assert!(token_path().is_some(), "token_path should resolve on Linux");
|
||||
let path = token_path().unwrap();
|
||||
assert!(!path.starts_with("/tmp"), "must not fallback to /tmp");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn verify_secure_dir_rejects_wrong_permissions() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path();
|
||||
|
||||
// 0o700 should pass
|
||||
std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o700)).unwrap();
|
||||
assert!(verify_secure_dir(path));
|
||||
|
||||
// 0o755 should fail
|
||||
std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o755)).unwrap();
|
||||
assert!(!verify_secure_dir(path));
|
||||
|
||||
// 0o777 should fail
|
||||
std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o777)).unwrap();
|
||||
assert!(!verify_secure_dir(path));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn verify_secure_dir_rejects_non_directory() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let file_path = dir.path().join("not-a-dir");
|
||||
std::fs::write(&file_path, b"test").unwrap();
|
||||
assert!(!verify_secure_dir(&file_path));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ensure_secure_parent_creates_with_0700() {
|
||||
let base = tempfile::tempdir().unwrap();
|
||||
let new_dir = base.path().join("wl-test-new-dir");
|
||||
assert!(!new_dir.exists());
|
||||
|
||||
assert!(ensure_secure_parent(&new_dir));
|
||||
assert!(new_dir.is_dir());
|
||||
|
||||
let meta = std::fs::symlink_metadata(&new_dir).unwrap();
|
||||
let mode = meta.permissions().mode() & 0o777;
|
||||
assert_eq!(mode, 0o700, "created directory should be 0700, got {mode:o}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ensure_secure_parent_tightens_existing_dir() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path();
|
||||
|
||||
// Simulate an existing directory with loose permissions
|
||||
std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o755)).unwrap();
|
||||
|
||||
assert!(ensure_secure_parent(path));
|
||||
|
||||
let meta = std::fs::symlink_metadata(path).unwrap();
|
||||
let mode = meta.permissions().mode() & 0o777;
|
||||
assert_eq!(mode, 0o700, "tightened directory should be 0700, got {mode:o}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn save_creates_file_with_0600() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let token_path = dir.path().join("portal-restore-token");
|
||||
|
||||
save_restore_token_to("secret-token-123", &token_path);
|
||||
|
||||
assert!(token_path.exists());
|
||||
let meta = std::fs::symlink_metadata(&token_path).unwrap();
|
||||
let mode = meta.permissions().mode() & 0o777;
|
||||
assert_eq!(mode, 0o600, "token file should be 0600, got {mode:o}");
|
||||
assert_eq!(std::fs::read_to_string(&token_path).unwrap(), "secret-token-123");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_reads_secure_file() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let token_path = dir.path().join("portal-restore-token");
|
||||
|
||||
// Write a valid 0o600 token file
|
||||
use std::os::unix::fs::OpenOptionsExt;
|
||||
let mut f = std::fs::OpenOptions::new()
|
||||
.write(true)
|
||||
.create_new(true)
|
||||
.mode(0o600)
|
||||
.open(&token_path)
|
||||
.unwrap();
|
||||
std::io::Write::write_all(&mut f, b"my-secret\n").unwrap();
|
||||
|
||||
let result = load_restore_token_from(token_path);
|
||||
assert_eq!(result, Some("my-secret".to_string()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_rejects_group_readable_file() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let token_path = dir.path().join("portal-restore-token");
|
||||
|
||||
// Write with 0o640 (group readable) — should be rejected
|
||||
use std::os::unix::fs::OpenOptionsExt;
|
||||
let mut f = std::fs::OpenOptions::new()
|
||||
.write(true)
|
||||
.create_new(true)
|
||||
.mode(0o640)
|
||||
.open(&token_path)
|
||||
.unwrap();
|
||||
std::io::Write::write_all(&mut f, b"leaked-token\n").unwrap();
|
||||
|
||||
let result = load_restore_token_from(token_path);
|
||||
assert!(result.is_none(), "should reject group-readable token file");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_rejects_world_readable_file() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let token_path = dir.path().join("portal-restore-token");
|
||||
|
||||
use std::os::unix::fs::OpenOptionsExt;
|
||||
let mut f = std::fs::OpenOptions::new()
|
||||
.write(true)
|
||||
.create_new(true)
|
||||
.mode(0o604)
|
||||
.open(&token_path)
|
||||
.unwrap();
|
||||
std::io::Write::write_all(&mut f, b"leaked-token\n").unwrap();
|
||||
|
||||
let result = load_restore_token_from(token_path);
|
||||
assert!(result.is_none(), "should reject world-readable token file");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_rejects_symlink() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let real_path = dir.path().join("real-file");
|
||||
let link_path = dir.path().join("portal-restore-token");
|
||||
|
||||
std::fs::write(&real_path, b"target-content\n").unwrap();
|
||||
std::os::unix::fs::symlink(&real_path, &link_path).unwrap();
|
||||
|
||||
let result = load_restore_token_from(link_path);
|
||||
assert!(result.is_none(), "should reject symlinked token file");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn save_then_load_roundtrip() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let token_path = dir.path().join("portal-restore-token");
|
||||
|
||||
save_restore_token_to("roundtrip-token", &token_path);
|
||||
let loaded = load_restore_token_from(token_path);
|
||||
|
||||
assert_eq!(loaded, Some("roundtrip-token".to_string()));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user