diff --git a/src/builder.rs b/src/builder.rs index fbc5e53d83..c6b3bd02fc 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -9,13 +9,13 @@ use std::collections::HashMap; use std::convert::TryInto; use std::default::Default; +use std::fmt; #[cfg(feature = "unified-payments")] use std::net::ToSocketAddrs; #[cfg(feature = "storage-filesystem")] use std::path::PathBuf; use std::sync::{Arc, Mutex, Once, RwLock}; use std::time::SystemTime; -use std::{fmt, fs}; use bdk_wallet::template::Bip84; use bdk_wallet::{KeychainKind, Wallet as BdkWallet}; @@ -72,6 +72,8 @@ use crate::gossip::GossipSource; use crate::io::fs_store::open_or_migrate_fs_store; #[cfg(feature = "storage-sqlite")] use crate::io::sqlite_store::SqliteStore; +#[cfg(feature = "storage-sqlite")] +use crate::io::utils::create_dir_all_private; use crate::io::utils::{ read_all_objects, read_event_queue, read_external_pathfinding_scores_from_cache, read_n_objects, read_network_graph, read_node_metrics, read_output_sweeper, read_peer_info, @@ -692,7 +694,7 @@ impl NodeBuilder { pub fn build(&self, node_entropy: NodeEntropy) -> Result { let logger = setup_logger(&self.log_writer_config, &self.config)?; let storage_dir_path = self.config.storage_dir_path.clone(); - fs::create_dir_all(storage_dir_path.clone()) + create_dir_all_private(storage_dir_path.as_ref()) .map_err(|_| BuildError::StoragePathAccessFailed)?; let kv_store = SqliteStore::new( storage_dir_path.into(), diff --git a/src/io/fs_store.rs b/src/io/fs_store.rs index f0e28f8785..88c4749617 100644 --- a/src/io/fs_store.rs +++ b/src/io/fs_store.rs @@ -12,6 +12,7 @@ use lightning::util::persist::migrate_kv_store_data_async; use lightning_persister::fs_store::v1::FilesystemStore; use lightning_persister::fs_store::v2::{FilesystemStoreV2, FilesystemStoreV2Error}; +use crate::io::utils::create_dir_all_private; use crate::BuildError; /// Opens a [`FilesystemStoreV2`], automatically migrating from v1 format if necessary. @@ -23,10 +24,11 @@ pub(crate) async fn open_or_migrate_fs_store( storage_dir_path: PathBuf, ) -> Result { let parent_dir = storage_dir_path.parent().ok_or(BuildError::StoragePathAccessFailed)?; - fs::create_dir_all(parent_dir).map_err(|_| BuildError::StoragePathAccessFailed)?; + create_dir_all_private(parent_dir).map_err(|_| BuildError::StoragePathAccessFailed)?; recover_incomplete_fs_store_migration(&storage_dir_path)?; if !storage_dir_path.exists() { - fs::create_dir_all(&storage_dir_path).map_err(|_| BuildError::StoragePathAccessFailed)?; + create_dir_all_private(&storage_dir_path) + .map_err(|_| BuildError::StoragePathAccessFailed)?; } match FilesystemStoreV2::new(storage_dir_path.clone()) { @@ -36,7 +38,7 @@ pub(crate) async fn open_or_migrate_fs_store( let v1_store = FilesystemStore::new(storage_dir_path.clone()); let v2_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v2_migrating"); - fs::create_dir_all(&v2_dir).map_err(|_| BuildError::StoragePathAccessFailed)?; + create_dir_all_private(&v2_dir).map_err(|_| BuildError::StoragePathAccessFailed)?; let v2_store = FilesystemStoreV2::new(v2_dir.clone()) .map_err(|_| BuildError::KVStoreSetupFailed)?; diff --git a/src/io/sqlite_store/mod.rs b/src/io/sqlite_store/mod.rs index 2587220598..24e17efa61 100644 --- a/src/io/sqlite_store/mod.rs +++ b/src/io/sqlite_store/mod.rs @@ -7,8 +7,13 @@ //! Objects related to [`SqliteStore`] live here. use std::collections::HashMap; +#[cfg(test)] use std::fs; +#[cfg(unix)] +use std::fs::OpenOptions; use std::future::Future; +#[cfg(unix)] +use std::os::unix::fs::OpenOptionsExt; use std::path::PathBuf; use std::sync::atomic::{AtomicI64, AtomicU64, Ordering}; use std::sync::{Arc, Mutex}; @@ -20,7 +25,7 @@ use lightning::util::persist::{ use lightning_types::string::PrintableString; use rusqlite::{named_params, Connection}; -use crate::io::utils::check_namespace_key_validity; +use crate::io::utils::{check_namespace_key_validity, create_dir_all_private}; mod migrations; @@ -58,6 +63,8 @@ impl SqliteStore { /// If not already existing, a new SQLite database will be created in the given `data_dir` under the /// given `db_file_name` (or the default to [`DEFAULT_SQLITE_DB_FILE_NAME`] if set to `None`). /// + /// SQLite's `:memory:` database name and `file:` URI filenames are not supported. + /// /// Similarly, the given `kv_table_name` will be used or default to [`DEFAULT_KV_TABLE_NAME`]. pub fn new( data_dir: PathBuf, db_file_name: Option, kv_table_name: Option, @@ -232,9 +239,20 @@ impl SqliteStoreInner { data_dir: PathBuf, db_file_name: Option, kv_table_name: Option, ) -> io::Result { let db_file_name = db_file_name.unwrap_or(DEFAULT_SQLITE_DB_FILE_NAME.to_string()); + let mut db_file_path = data_dir.clone(); + db_file_path.push(&db_file_name); + if db_file_name == ":memory:" + || db_file_name.starts_with("file:") + || db_file_path.to_string_lossy().starts_with("file:") + { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "SQLite :memory: and file: database names are not supported", + )); + } let kv_table_name = kv_table_name.unwrap_or(DEFAULT_KV_TABLE_NAME.to_string()); - fs::create_dir_all(data_dir.clone()).map_err(|e| { + create_dir_all_private(&data_dir).map_err(|e| { let msg = format!( "Failed to create database destination directory {}: {}", data_dir.display(), @@ -242,8 +260,16 @@ impl SqliteStoreInner { ); io::Error::new(io::ErrorKind::Other, msg) })?; - let mut db_file_path = data_dir.clone(); - db_file_path.push(db_file_name); + #[cfg(unix)] + match OpenOptions::new().create_new(true).write(true).mode(0o600).open(&db_file_path) { + Ok(_) => {}, + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {}, + Err(e) => { + let msg = + format!("Failed to create database file {}: {}", db_file_path.display(), e); + return Err(io::Error::new(io::ErrorKind::Other, msg)); + }, + } let mut connection = Connection::open(db_file_path.clone()).map_err(|e| { let msg = @@ -700,6 +726,50 @@ mod tests { } } + #[cfg(unix)] + #[test] + fn creates_private_database_storage() { + use std::os::unix::fs::PermissionsExt; + + let mut data_dir = random_storage_path(); + data_dir.push("creates_private_database_storage"); + let db_file_name = "test_db"; + let db_file_path = data_dir.join(db_file_name); + let _store = SqliteStore::new( + data_dir.clone(), + Some(db_file_name.to_string()), + Some("test_table".to_string()), + ) + .unwrap(); + + let dir_mode = data_dir.metadata().unwrap().permissions().mode(); + let file_mode = db_file_path.metadata().unwrap().permissions().mode(); + assert_eq!(dir_mode & 0o077, 0); + assert_eq!(file_mode & 0o077, 0); + } + + #[test] + fn rejects_sqlite_pseudo_filenames() { + for (data_dir, db_file_name) in [ + (random_storage_path(), ":memory:"), + (random_storage_path(), "file:/tmp/node.db?mode=rwc"), + (PathBuf::from("file:."), "test_db"), + ] { + let result = SqliteStore::new( + data_dir.clone(), + Some(db_file_name.to_string()), + Some("test_table".to_string()), + ); + let error = match result { + Ok(_) => panic!("SQLite pseudo-filename was accepted"), + Err(e) => e, + }; + + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); + assert!(!data_dir.exists()); + } + } + #[tokio::test] async fn read_write_remove_list_persist() { let mut temp_path = random_storage_path(); diff --git a/src/io/utils.rs b/src/io/utils.rs index 30fc0c62d2..31f3166597 100644 --- a/src/io/utils.rs +++ b/src/io/utils.rs @@ -10,7 +10,7 @@ use std::io::Write; use std::num::NonZeroUsize; use std::ops::Deref; #[cfg(unix)] -use std::os::unix::fs::OpenOptionsExt; +use std::os::unix::fs::{DirBuilderExt, OpenOptionsExt}; use std::path::Path; use std::sync::Arc; @@ -52,6 +52,14 @@ use crate::{Error, EventQueue, NodeMetrics, PersistedNodeMetrics}; pub const EXTERNAL_PATHFINDING_SCORES_CACHE_KEY: &str = "external_pathfinding_scores_cache"; +pub(crate) fn create_dir_all_private(path: &Path) -> std::io::Result<()> { + let mut builder = fs::DirBuilder::new(); + builder.recursive(true); + #[cfg(unix)] + builder.mode(0o700); + builder.create(path) +} + pub(crate) fn read_or_generate_seed_file( keys_seed_path: &str, ) -> std::io::Result<[u8; WALLET_KEYS_SEED_LEN]> { @@ -75,7 +83,7 @@ pub(crate) fn read_or_generate_seed_file( })?; if let Some(parent_dir) = Path::new(&keys_seed_path).parent() { - fs::create_dir_all(parent_dir)?; + create_dir_all_private(parent_dir)?; } #[cfg(unix)] @@ -761,8 +769,25 @@ pub(crate) async fn read_bdk_wallet_change_set( #[cfg(test)] mod tests { - use super::read_or_generate_seed_file; use super::test_utils::random_storage_path; + use super::{create_dir_all_private, read_or_generate_seed_file}; + + #[cfg(unix)] + #[test] + fn creates_private_directories() { + use std::os::unix::fs::PermissionsExt; + + let base_path = random_storage_path(); + let nested_path = base_path.join("parent").join("child"); + create_dir_all_private(&nested_path).unwrap(); + + for path in [&base_path, &base_path.join("parent"), &nested_path] { + let mode = path.metadata().unwrap().permissions().mode(); + assert_eq!(mode & 0o077, 0); + } + + std::fs::remove_dir_all(base_path).unwrap(); + } #[test] fn generated_seed_is_readable() { diff --git a/src/logger.rs b/src/logger.rs index c5a4584a18..857b33e3a9 100644 --- a/src/logger.rs +++ b/src/logger.rs @@ -10,6 +10,8 @@ use core::fmt; use std::fs; use std::io::Write; +#[cfg(unix)] +use std::os::unix::fs::OpenOptionsExt; use std::path::Path; use std::sync::Arc; @@ -22,6 +24,16 @@ pub(crate) use lightning::util::logger::{Logger as LdkLogger, Record as LdkRecor pub(crate) use lightning::{log_bytes, log_debug, log_error, log_info, log_trace, log_warn}; use log::{Level as LogFacadeLevel, Record as LogFacadeRecord}; +use crate::io::utils::create_dir_all_private; + +fn open_log_file(file_path: &str) -> std::io::Result { + let mut options = fs::OpenOptions::new(); + options.create(true).append(true); + #[cfg(unix)] + options.mode(0o600); + options.open(file_path) +} + /// A unit of logging output with metadata to enable filtering `module_path`, /// `file`, and `line` to inform on log's source. #[cfg(not(feature = "uniffi"))] @@ -208,10 +220,7 @@ impl LogWriter for Writer { context, ); - fs::OpenOptions::new() - .create(true) - .append(true) - .open(file_path) + open_log_file(file_path) .expect("Failed to open log file") .write_all(log.as_bytes()) .expect("Failed to write to log file") @@ -261,14 +270,11 @@ impl Logger { /// are the path to the log file, and the log level. pub fn new_fs_writer(file_path: String, max_log_level: LogLevel) -> Result { if let Some(parent_dir) = Path::new(&file_path).parent() { - fs::create_dir_all(parent_dir) + create_dir_all_private(parent_dir) .map_err(|e| eprintln!("ERROR: Failed to create log parent directory: {}", e))?; // make sure the file exists. - fs::OpenOptions::new() - .create(true) - .append(true) - .open(&file_path) + open_log_file(&file_path) .map_err(|e| eprintln!("ERROR: Failed to open log file: {}", e))?; } @@ -308,6 +314,23 @@ mod tests { use std::sync::Mutex; use super::*; + #[cfg(unix)] + use crate::io::test_utils::random_storage_path; + + #[cfg(unix)] + #[test] + fn creates_private_log_file() { + use std::os::unix::fs::PermissionsExt; + + let log_dir = random_storage_path(); + let log_path = log_dir.join("ldk_node.log"); + let _logger = + Logger::new_fs_writer(log_path.to_str().unwrap().to_string(), LogLevel::Info).unwrap(); + + let mode = log_path.metadata().unwrap().permissions().mode(); + assert_eq!(mode & 0o077, 0); + fs::remove_dir_all(log_dir).unwrap(); + } /// A minimal log facade logger that captures log output for testing. struct TestLogger {