From fa1b48fcc7357ffcefbb622f3e73447494526db3 Mon Sep 17 00:00:00 2001 From: benma's agent Date: Sat, 18 Jul 2026 09:22:21 +0000 Subject: [PATCH] noise: restrict persisted config permissions Create new Noise config files with mode 0600 on Unix and tighten permissions on existing files. Leave symlinks and their targets untouched. Document that callers must create the config directory themselves, with mode 0700 recommended on Unix. --- CHANGELOG-rust.md | 1 + src/noise.rs | 135 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 132 insertions(+), 4 deletions(-) diff --git a/CHANGELOG-rust.md b/CHANGELOG-rust.md index 926fdb9..b0977c5 100644 --- a/CHANGELOG-rust.md +++ b/CHANGELOG-rust.md @@ -1,6 +1,7 @@ # Changelog ## [Unreleased] +- Restrict persisted Noise config files to private permissions on Unix. ## 0.13.0 - Add `BitBox::from_transport()` diff --git a/src/noise.rs b/src/noise.rs index b927fec..397c5b0 100644 --- a/src/noise.rs +++ b/src/noise.rs @@ -1,6 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 use crate::util::Threading; +#[cfg(unix)] +use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; use thiserror::Error; #[derive(Error, Debug)] @@ -55,6 +57,19 @@ pub struct NoiseConfigNoCache; impl NoiseConfig for NoiseConfigNoCache {} impl Threading for NoiseConfigNoCache {} +#[cfg(unix)] +fn ensure_private_file_permissions(path: &std::path::Path) -> Result<(), ConfigError> { + let metadata = std::fs::symlink_metadata(path).map_err(|e| ConfigError(e.to_string()))?; + + // A symlink can be part of an intentional setup. Do not change its target's permissions. + if metadata.file_type().is_symlink() { + return Ok(()); + } + + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600)) + .map_err(|e| ConfigError(e.to_string())) +} + pub struct PersistedNoiseConfig { config_dir: String, } @@ -63,7 +78,8 @@ impl Threading for PersistedNoiseConfig {} impl PersistedNoiseConfig { /// Creates a new persisting noise config, which stores the pairing information in "bitbox.json" - /// in the provided directory. + /// in the provided directory. The directory must already exist and should be created with + /// `0700` permissions on Unix. pub fn new(config_dir: &str) -> PersistedNoiseConfig { PersistedNoiseConfig { config_dir: config_dir.into(), @@ -81,7 +97,10 @@ impl NoiseConfig for PersistedNoiseConfig { return Ok(NoiseConfigData::default()); } - let mut file = std::fs::File::open(config_path).map_err(|e| ConfigError(e.to_string()))?; + let mut file = std::fs::File::open(&config_path).map_err(|e| ConfigError(e.to_string()))?; + + #[cfg(unix)] + ensure_private_file_permissions(&config_path)?; let mut contents = String::new(); file.read_to_string(&mut contents) @@ -95,8 +114,18 @@ impl NoiseConfig for PersistedNoiseConfig { let config_path = std::path::Path::new(&self.config_dir).join("bitbox.json"); - let mut file = - std::fs::File::create(config_path).map_err(|e| ConfigError(e.to_string()))?; + let mut options = std::fs::File::options(); + options.write(true).create(true).truncate(true); + + #[cfg(unix)] + options.mode(0o600); + + let mut file = options + .open(&config_path) + .map_err(|e| ConfigError(e.to_string()))?; + + #[cfg(unix)] + ensure_private_file_permissions(&config_path)?; let data = serde_json::to_string(conf).map_err(|e| ConfigError(e.to_string()))?; @@ -104,3 +133,101 @@ impl NoiseConfig for PersistedNoiseConfig { .map_err(|e| ConfigError(e.to_string())) } } + +#[cfg(all(test, unix))] +mod tests { + use super::*; + use std::os::unix::fs::PermissionsExt; + + struct TempDir(std::path::PathBuf); + + impl TempDir { + fn new() -> Self { + let unique = format!( + "bitbox-api-noise-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + ); + Self(std::env::temp_dir().join(unique)) + } + } + + impl Drop for TempDir { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + fn mode(path: &std::path::Path) -> u32 { + std::fs::metadata(path).unwrap().permissions().mode() & 0o777 + } + + #[test] + fn store_config_uses_private_file_permissions() { + let dir = TempDir::new(); + std::fs::create_dir(&dir.0).unwrap(); + std::fs::set_permissions(&dir.0, std::fs::Permissions::from_mode(0o755)).unwrap(); + let config = PersistedNoiseConfig::new(dir.0.to_str().unwrap()); + + config.store_config(&NoiseConfigData::default()).unwrap(); + + assert_eq!(mode(&dir.0), 0o755); + assert_eq!(mode(&dir.0.join("bitbox.json")), 0o600); + } + + #[test] + fn store_config_does_not_create_config_directory() { + let dir = TempDir::new(); + let config = PersistedNoiseConfig::new(dir.0.to_str().unwrap()); + + assert!(config.store_config(&NoiseConfigData::default()).is_err()); + assert!(!dir.0.exists()); + } + + #[test] + fn read_config_repairs_permissive_permissions() { + let dir = TempDir::new(); + std::fs::create_dir(&dir.0).unwrap(); + std::fs::write( + dir.0.join("bitbox.json"), + r#"{"app_static_privkey":null,"device_static_pubkeys":[]}"#, + ) + .unwrap(); + std::fs::set_permissions(&dir.0, std::fs::Permissions::from_mode(0o755)).unwrap(); + std::fs::set_permissions( + dir.0.join("bitbox.json"), + std::fs::Permissions::from_mode(0o644), + ) + .unwrap(); + let config = PersistedNoiseConfig::new(dir.0.to_str().unwrap()); + + config.read_config().unwrap(); + + assert_eq!(mode(&dir.0), 0o755); + assert_eq!(mode(&dir.0.join("bitbox.json")), 0o600); + } + + #[test] + fn store_config_does_not_change_file_symlink_target_permissions() { + let dir = TempDir::new(); + std::fs::create_dir(&dir.0).unwrap(); + let target = dir.0.join("target.json"); + std::fs::write(&target, "{}").unwrap(); + std::fs::set_permissions(&target, std::fs::Permissions::from_mode(0o644)).unwrap(); + let config_dir = dir.0.join("config"); + std::fs::create_dir(&config_dir).unwrap(); + std::os::unix::fs::symlink(&target, config_dir.join("bitbox.json")).unwrap(); + let config = PersistedNoiseConfig::new(config_dir.to_str().unwrap()); + + config.store_config(&NoiseConfigData::default()).unwrap(); + + assert!(std::fs::symlink_metadata(config_dir.join("bitbox.json")) + .unwrap() + .file_type() + .is_symlink()); + assert_eq!(mode(&target), 0o644); + } +}