diff --git a/cmd/passless/src/storage/local/init.rs b/cmd/passless/src/storage/local/init.rs index 371b6ba..5b6d3b1 100644 --- a/cmd/passless/src/storage/local/init.rs +++ b/cmd/passless/src/storage/local/init.rs @@ -5,9 +5,9 @@ use crate::notification::{ YesNoResult, show_error_notification, show_info_notification, show_yes_no_notification, }; +use crate::util::create_secure_dir_all; use passless_core::error::{Error, Result}; -use std::fs; use std::path::Path; use log::{info, warn}; @@ -53,7 +53,7 @@ pub fn ensure_initialized(storage_path: &Path) -> Result<()> { } } - fs::create_dir_all(storage_path).map_err(|e| { + create_secure_dir_all(storage_path).map_err(|e| { let msg = format!("Failed to create storage directory: {}", e); let _ = show_error_notification("Initialization Failed", &msg); Error::Storage(msg) diff --git a/cmd/passless/src/storage/local/mod.rs b/cmd/passless/src/storage/local/mod.rs index c9ff881..ac48ee6 100644 --- a/cmd/passless/src/storage/local/mod.rs +++ b/cmd/passless/src/storage/local/mod.rs @@ -8,10 +8,11 @@ use crate::storage::index::{ update_indexes_on_write, }; use crate::storage::{CredentialFilter, CredentialStorage}; +use crate::util::{create_secure_dir_all, create_secure_file}; use soft_fido2::Result; -use std::fs::{self, File}; +use std::fs; use std::io::{Read, Write}; use std::path::{Path, PathBuf}; @@ -64,7 +65,7 @@ impl LocalStorageAdapter { /// Load a credential from a file path fn load_credential_from_path(&self, path: &Path) -> Result { debug!("Loading credential from: {:?}", path); - let mut file = File::open(path).map_err(|_| soft_fido2::Error::DoesNotExist)?; + let mut file = fs::File::open(path).map_err(|_| soft_fido2::Error::DoesNotExist)?; let mut contents = Vec::new(); file.read_to_end(&mut contents) .map_err(|_| soft_fido2::Error::Other)?; @@ -85,15 +86,15 @@ impl LocalStorageAdapter { let path = path_info.to_path(&self.storage_dir); - // Ensure the RP directory exists + // Ensure the RP directory exists with secure permissions if let Some(parent) = path.parent() { - fs::create_dir_all(parent).map_err(|_| soft_fido2::Error::Other)?; + create_secure_dir_all(parent).map_err(|_| soft_fido2::Error::Other)?; } // Use Zeroizing to ensure credential bytes are cleared from memory after use let bytes = Zeroizing::new(our_cred.to_bytes()?); - let mut file = File::create(&path).map_err(|_| soft_fido2::Error::Other)?; + let mut file = create_secure_file(&path).map_err(|_| soft_fido2::Error::Other)?; file.write_all(&bytes) .map_err(|_| soft_fido2::Error::Other)?; diff --git a/cmd/passless/src/storage/pass/init/uninitialized.rs b/cmd/passless/src/storage/pass/init/uninitialized.rs index 2db40cb..53cb1d8 100644 --- a/cmd/passless/src/storage/pass/init/uninitialized.rs +++ b/cmd/passless/src/storage/pass/init/uninitialized.rs @@ -4,10 +4,10 @@ use super::directory_created::DirectoryCreated; use crate::notification::{YesNoResult, show_info_notification, show_yes_no_notification}; use crate::storage::pass::GpgBackend; +use crate::util::create_secure_dir_all; use passless_core::error::{Error, Result}; -use std::fs; use std::path::PathBuf; use log::{debug, info, warn}; @@ -65,7 +65,7 @@ impl Uninitialized { } if !self.store_path.exists() { - fs::create_dir_all(&self.store_path).map_err(|e| { + create_secure_dir_all(&self.store_path).map_err(|e| { let msg = format!("Failed to create store directory: {}", e); let _ = crate::notification::show_error_notification("Initialization Failed", &msg); Error::Storage(msg) diff --git a/cmd/passless/src/storage/pass/mod.rs b/cmd/passless/src/storage/pass/mod.rs index 27e2bf0..aacf672 100644 --- a/cmd/passless/src/storage/pass/mod.rs +++ b/cmd/passless/src/storage/pass/mod.rs @@ -8,8 +8,7 @@ use crate::storage::index::{ load_credential_paths, update_indexes_on_delete, update_indexes_on_write, }; use crate::storage::{CredentialFilter, CredentialStorage}; -use crate::util::bytes_to_hex; - +use crate::util::{bytes_to_hex, create_secure_dir_all}; use passless_core::error::{Error, Result}; use std::fmt::Display; @@ -373,9 +372,9 @@ impl PassStorageAdapter { let path = get_credential_path(&self.get_fido2_path(), &cred.rp.id, &cred.id, "gpg"); debug!("Writing credential to: {:?}", path); - // Ensure parent directory exists + // Ensure parent directory exists with secure permissions if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent).map_err(|e| { + create_secure_dir_all(parent).map_err(|e| { debug!("Failed to create directory: {}", e); Error::Storage(format!("Failed to create directory: {}", e)) })?; diff --git a/cmd/passless/src/storage/tpm/init.rs b/cmd/passless/src/storage/tpm/init.rs index 098be2e..60b4c4c 100644 --- a/cmd/passless/src/storage/tpm/init.rs +++ b/cmd/passless/src/storage/tpm/init.rs @@ -5,9 +5,9 @@ use crate::notification::{ YesNoResult, show_error_notification, show_info_notification, show_yes_no_notification, }; +use crate::util::create_secure_dir_all; use passless_core::error::{Error, Result}; -use std::fs; use std::path::Path; use log::{info, warn}; @@ -50,7 +50,7 @@ pub fn ensure_initialized(storage_path: &Path) -> Result<()> { } } - fs::create_dir_all(storage_path).map_err(|e| { + create_secure_dir_all(storage_path).map_err(|e| { let msg = format!("Failed to create storage directory: {}", e); let _ = show_error_notification("Initialization Failed", &msg); Error::Storage(msg) diff --git a/cmd/passless/src/storage/tpm/mod.rs b/cmd/passless/src/storage/tpm/mod.rs index 26fa20a..235937d 100644 --- a/cmd/passless/src/storage/tpm/mod.rs +++ b/cmd/passless/src/storage/tpm/mod.rs @@ -8,7 +8,7 @@ use crate::storage::index::{ load_credential_paths, update_indexes_on_delete, update_indexes_on_write, }; use crate::storage::{CredentialFilter, CredentialStorage}; -use crate::util::bytes_to_hex; +use crate::util::{bytes_to_hex, create_secure_dir_all, create_secure_file}; use soft_fido2::Result; @@ -553,9 +553,9 @@ impl TpmStorageAdapter { let path = get_credential_path(&self.storage_dir, &cred.rp.id, &cred.id, "tpm"); debug!("Writing credential to: {:?}", path); - // Ensure parent directory exists + // Ensure parent directory exists with secure permissions if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent).map_err(|e| { + create_secure_dir_all(parent).map_err(|e| { debug!("Failed to create directory: {}", e); soft_fido2::Error::Other })?; @@ -569,7 +569,7 @@ impl TpmStorageAdapter { // Seal the data using TPM let sealed_data = self.seal_data(&bytes)?; - let mut file = File::create(&path).map_err(|e| { + let mut file = create_secure_file(&path).map_err(|e| { log::error!("Failed to create credential file {}: {}", path.display(), e); soft_fido2::Error::Other })?; diff --git a/cmd/passless/src/util.rs b/cmd/passless/src/util.rs index 6106998..a3abf84 100644 --- a/cmd/passless/src/util.rs +++ b/cmd/passless/src/util.rs @@ -1,11 +1,60 @@ +use std::fs::{self, File, OpenOptions}; +use std::io::{self, Write}; +use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; +use std::path::Path; + /// Convert bytes to lowercase hexadecimal string pub fn bytes_to_hex(bytes: &[u8]) -> String { bytes.iter().map(|b| format!("{:02x}", b)).collect() } +/// Create a file with secure permissions (0o600: owner read/write only) +/// +/// This function creates a file with user-only permissions to prevent +/// unauthorized access to sensitive credential data. +pub fn create_secure_file>(path: P) -> io::Result { + OpenOptions::new() + .create(true) + .write(true) + .truncate(true) + .mode(0o600) + .open(path.as_ref()) +} + +/// Write data to a file with secure permissions (0o600: owner read/write only) +/// +/// This is a convenience function that creates the file with secure permissions +/// and writes the data in one operation. +#[allow(dead_code)] +pub fn write_secure_file>(path: P, data: &[u8]) -> io::Result<()> { + let mut file = create_secure_file(path)?; + file.write_all(data) +} + +/// Create a directory with secure permissions (0o700: owner read/write/execute only) +/// +/// This function creates a directory and all its parents with user-only permissions. +/// If the directory already exists, it will set the permissions to 0o700. +pub fn create_secure_dir_all>(path: P) -> io::Result<()> { + let path = path.as_ref(); + + if path.exists() { + fs::set_permissions(path, fs::Permissions::from_mode(0o700))?; + return Ok(()); + } + + fs::create_dir_all(path)?; + + fs::set_permissions(path, fs::Permissions::from_mode(0o700))?; + + Ok(()) +} + #[cfg(test)] mod tests { use super::*; + use std::fs; + use tempfile::tempdir; #[test] fn test_bytes_to_hex() { @@ -15,4 +64,75 @@ mod tests { assert_eq!(bytes_to_hex(&[0x01, 0x23, 0x45, 0x67]), "01234567"); assert_eq!(bytes_to_hex(&[0xab, 0xcd, 0xef]), "abcdef"); } + + #[test] + fn test_create_secure_file() { + let dir = tempdir().expect("Failed to create temp dir"); + let file_path = dir.path().join("test_file"); + + let file = create_secure_file(&file_path).expect("Failed to create secure file"); + drop(file); + + assert!(file_path.exists()); + + let metadata = fs::metadata(&file_path).expect("Failed to get metadata"); + let mode = metadata.permissions().mode(); + assert_eq!(mode & 0o777, 0o600, "File should have 0o600 permissions"); + } + + #[test] + fn test_write_secure_file() { + let dir = tempdir().expect("Failed to create temp dir"); + let file_path = dir.path().join("test_file"); + + write_secure_file(&file_path, b"test data").expect("Failed to write secure file"); + + assert!(file_path.exists()); + + let metadata = fs::metadata(&file_path).expect("Failed to get metadata"); + let mode = metadata.permissions().mode(); + assert_eq!(mode & 0o777, 0o600, "File should have 0o600 permissions"); + + let contents = fs::read(&file_path).expect("Failed to read file"); + assert_eq!(contents, b"test data"); + } + + #[test] + fn test_create_secure_dir_all() { + let dir = tempdir().expect("Failed to create temp dir"); + let nested_path = dir.path().join("a/b/c"); + + create_secure_dir_all(&nested_path).expect("Failed to create secure directories"); + + assert!(nested_path.exists()); + + let metadata = fs::metadata(&nested_path).expect("Failed to get metadata"); + assert!(metadata.is_dir()); + let mode = metadata.permissions().mode(); + assert_eq!( + mode & 0o777, + 0o700, + "Directory a/b/c should have 0o700 permissions" + ); + } + + #[test] + fn test_create_secure_dir_all_existing() { + let dir = tempdir().expect("Failed to create temp dir"); + let path = dir.path().join("existing_dir"); + + fs::create_dir_all(&path).expect("Failed to create directory"); + fs::set_permissions(&path, fs::Permissions::from_mode(0o755)) + .expect("Failed to set permissions"); + + create_secure_dir_all(&path).expect("Failed to update permissions"); + + let metadata = fs::metadata(&path).expect("Failed to get metadata"); + let mode = metadata.permissions().mode(); + assert_eq!( + mode & 0o777, + 0o700, + "Directory should have 0o700 permissions" + ); + } }