From ecf325ab18e43a2151db5622bb2e3e1aa806b87a Mon Sep 17 00:00:00 2001 From: benthecarman Date: Fri, 28 Aug 2026 17:57:20 -0500 Subject: [PATCH 1/8] Create private log files Create new log files without group or world access so local users cannot read node activity under the standard process umask. This commit was created with assistance from Codex. --- ldk-server/src/util/logger.rs | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/ldk-server/src/util/logger.rs b/ldk-server/src/util/logger.rs index 7a3d0527..5adb80e5 100644 --- a/ldk-server/src/util/logger.rs +++ b/ldk-server/src/util/logger.rs @@ -9,6 +9,7 @@ use std::fs::{self, File, OpenOptions}; use std::io::{self, LineWriter, Write}; +use std::os::unix::fs::OpenOptionsExt; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; use std::time::SystemTime; @@ -259,7 +260,7 @@ fn format_level(level: Level) -> &'static str { } fn open_log_file(log_file_path: &Path) -> Result { - OpenOptions::new().create(true).append(true).open(log_file_path) + OpenOptions::new().create(true).append(true).mode(0o600).open(log_file_path) } fn cleanup_old_logs(log_file_path: &Path, max_files: usize) -> io::Result<()> { @@ -319,3 +320,26 @@ impl Log for LoggerWrapper { self.0.flush() } } + +#[cfg(test)] +mod tests { + use std::os::unix::fs::PermissionsExt; + use std::time::{SystemTime, UNIX_EPOCH}; + + use super::*; + + #[test] + fn open_log_file_creates_private_file() { + let nonce = SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_nanos(); + let dir = std::env::temp_dir() + .join(format!("ldk-server-log-mode-{}-{nonce}", std::process::id())); + fs::create_dir_all(&dir).unwrap(); + let path = dir.join("ldk-server.log"); + + drop(open_log_file(&path).unwrap()); + + let mode = fs::metadata(&path).unwrap().permissions().mode(); + assert_eq!(mode & 0o077, 0); + fs::remove_dir_all(dir).unwrap(); + } +} From 45929a403402900bd96aa614b0f7006086915bb4 Mon Sep 17 00:00:00 2001 From: benthecarman Date: Fri, 28 Aug 2026 17:57:54 -0500 Subject: [PATCH 2/8] Reject malformed API key files Require existing API key files to contain the same 32-byte key material that the daemon generates. Truncated files must not enable weak HMACs. This commit was created with assistance from Codex. --- ldk-server/src/main.rs | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/ldk-server/src/main.rs b/ldk-server/src/main.rs index 28ea10de..e4981666 100644 --- a/ldk-server/src/main.rs +++ b/ldk-server/src/main.rs @@ -57,6 +57,7 @@ use crate::util::tls::get_or_generate_tls_config; use crate::util::{systemd, write_new}; const API_KEY_FILE: &str = "api_key"; +const API_KEY_LEN: usize = 32; const FULL_VERSION: &str = concat!(env!("CARGO_PKG_VERSION"), " (", env!("GIT_HASH"), ")"); pub fn get_default_data_dir() -> Option { @@ -895,13 +896,22 @@ fn load_or_generate_api_key(storage_dir: &Path) -> std::io::Result { if api_key_path.exists() { let key_bytes = fs::read(&api_key_path)?; + if key_bytes.len() != API_KEY_LEN { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!( + "API key file '{}' must contain exactly {API_KEY_LEN} bytes", + api_key_path.display() + ), + )); + } Ok(key_bytes.to_lower_hex_string()) } else { // Ensure the storage directory exists fs::create_dir_all(storage_dir)?; // Generate a 32-byte random API key - let mut key_bytes = [0u8; 32]; + let mut key_bytes = [0u8; API_KEY_LEN]; getrandom::getrandom(&mut key_bytes).map_err(std::io::Error::other)?; write_new(&api_key_path, &key_bytes, 0o400)?; @@ -929,6 +939,23 @@ mod tests { use super::*; + #[test] + fn load_api_key_rejects_invalid_lengths() { + let nonce = SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_nanos(); + let dir = std::env::temp_dir() + .join(format!("ldk-server-api-key-length-{}-{nonce}", std::process::id())); + fs::create_dir_all(&dir).unwrap(); + let path = dir.join(API_KEY_FILE); + + for len in [0, 1, API_KEY_LEN - 1, API_KEY_LEN + 1] { + fs::write(&path, vec![0x42; len]).unwrap(); + let error = load_or_generate_api_key(&dir).unwrap_err(); + assert_eq!(error.kind(), std::io::ErrorKind::InvalidData); + } + + fs::remove_dir_all(dir).unwrap(); + } + #[test] fn test_is_channel_open_failure_classification() { assert!(is_channel_open_failure(Some(&ClosureReason::FundingTimedOut))); From 572931db627d0896677f0638a76c8ec6d63a10d0 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Thu, 3 Sep 2026 16:55:25 +0000 Subject: [PATCH 3/8] Create private server data storage Create ldk_server_data.sqlite with mode 0600 and new storage and log directories with mode 0700. With the usual 0022 umask, create_dir_all made directories 0755. That let other local users list and traverse node storage, inspect metadata, and reach any file with permissive mode bits. Restricting new directories to the owner adds defense in depth for node data. Payment history should likewise not be readable by other users. This commit was created with assistance from Codex. --- ldk-server/src/io/persist/sqlite_store/mod.rs | 16 ++++++++++++++-- ldk-server/src/main.rs | 4 ++-- ldk-server/src/util/entropy.rs | 4 ++-- ldk-server/src/util/logger.rs | 4 +++- ldk-server/src/util/mod.rs | 9 +++++++-- 5 files changed, 28 insertions(+), 9 deletions(-) diff --git a/ldk-server/src/io/persist/sqlite_store/mod.rs b/ldk-server/src/io/persist/sqlite_store/mod.rs index 45e421ed..2738703d 100644 --- a/ldk-server/src/io/persist/sqlite_store/mod.rs +++ b/ldk-server/src/io/persist/sqlite_store/mod.rs @@ -7,15 +7,18 @@ // You may not use this file except in accordance with one or both of these // licenses. +use std::fs::OpenOptions; +use std::io; +use std::os::unix::fs::OpenOptionsExt; use std::path::PathBuf; use std::sync::{Arc, Mutex}; -use std::{fs, io}; use ldk_node::lightning::types::string::PrintableString; use rusqlite::{named_params, Connection}; use crate::io::persist::paginated_kv_store::{ListResponse, PaginatedKVStore}; use crate::io::utils::check_namespace_key_validity; +use crate::util::create_dir_all_private; /// The default database file name. pub const DEFAULT_SQLITE_DB_FILE_NAME: &str = "ldk_server_data.sqlite"; @@ -48,7 +51,7 @@ impl SqliteStore { let paginated_kv_table_name = paginated_kv_table_name.unwrap_or(DEFAULT_PAGINATED_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(), @@ -58,6 +61,15 @@ impl SqliteStore { })?; let mut db_file_path = data_dir; db_file_path.push(db_file_name); + match OpenOptions::new().create_new(true).write(true).mode(0o600).open(&db_file_path) { + Ok(_) => {}, + Err(e) if e.kind() == io::ErrorKind::AlreadyExists => {}, + Err(e) => { + let msg = + format!("Failed to create database file {}: {}", db_file_path.display(), e); + return Err(io::Error::other(msg)); + }, + } let connection = Connection::open(db_file_path.clone()).map_err(|e| { let msg = diff --git a/ldk-server/src/main.rs b/ldk-server/src/main.rs index e4981666..df411621 100644 --- a/ldk-server/src/main.rs +++ b/ldk-server/src/main.rs @@ -54,7 +54,7 @@ use crate::util::logger::{LogConfig, ServerLogger}; use crate::util::metrics::Metrics; use crate::util::proto_adapter::{forwarded_payment_to_proto, payment_to_proto}; use crate::util::tls::get_or_generate_tls_config; -use crate::util::{systemd, write_new}; +use crate::util::{create_dir_all_private, systemd, write_new}; const API_KEY_FILE: &str = "api_key"; const API_KEY_LEN: usize = 32; @@ -908,7 +908,7 @@ fn load_or_generate_api_key(storage_dir: &Path) -> std::io::Result { Ok(key_bytes.to_lower_hex_string()) } else { // Ensure the storage directory exists - fs::create_dir_all(storage_dir)?; + create_dir_all_private(storage_dir)?; // Generate a 32-byte random API key let mut key_bytes = [0u8; API_KEY_LEN]; diff --git a/ldk-server/src/util/entropy.rs b/ldk-server/src/util/entropy.rs index 4b73d6ee..8535c6bc 100644 --- a/ldk-server/src/util/entropy.rs +++ b/ldk-server/src/util/entropy.rs @@ -15,7 +15,7 @@ use ldk_node::bip39::Mnemonic; use ldk_node::entropy::{generate_entropy_mnemonic, NodeEntropy}; use log::info; -use crate::util::write_new; +use crate::util::{create_dir_all_private, write_new}; const DEFAULT_MNEMONIC_FILE: &str = "keys_mnemonic"; @@ -32,7 +32,7 @@ pub(crate) fn load_or_generate_node_entropy(storage_dir: &Path) -> io::Result, bytes_written: usize, @@ -68,7 +70,7 @@ impl ServerLogger { let state = if let Some(path) = &log_file_path { // Create parent directories if they don't exist if let Some(parent) = path.parent() { - fs::create_dir_all(parent)?; + create_dir_all_private(parent)?; } let file = open_log_file(path)?; diff --git a/ldk-server/src/util/mod.rs b/ldk-server/src/util/mod.rs index 7e900eb7..565f31ef 100644 --- a/ldk-server/src/util/mod.rs +++ b/ldk-server/src/util/mod.rs @@ -15,11 +15,16 @@ pub(crate) mod proto_adapter; pub(crate) mod systemd; pub(crate) mod tls; -use std::fs::{self, OpenOptions}; +use std::fs::{self, DirBuilder, OpenOptions}; use std::io::{self, Write}; -use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; +use std::os::unix::fs::{DirBuilderExt, OpenOptionsExt, PermissionsExt}; use std::path::Path; +pub(crate) fn create_dir_all_private(path: &Path) -> io::Result<()> { + let mut builder = DirBuilder::new(); + builder.recursive(true).mode(0o700).create(path) +} + pub(crate) fn write_new(path: &Path, contents: &[u8], mode: u32) -> io::Result<()> { let mut file = OpenOptions::new().create_new(true).write(true).mode(mode).open(path)?; file.write_all(contents)?; From 59643fc733ec93f48f17c53522b3958a78829ba1 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Thu, 3 Sep 2026 16:55:28 +0000 Subject: [PATCH 4/8] Bound API key file reads Read at most one byte beyond the expected API key length. This avoids unbounded memory use for oversized or special files while still rejecting contents that are not exactly 32 bytes. This commit was created with assistance from Codex. --- ldk-server/src/main.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ldk-server/src/main.rs b/ldk-server/src/main.rs index df411621..7f1d6fb0 100644 --- a/ldk-server/src/main.rs +++ b/ldk-server/src/main.rs @@ -14,6 +14,7 @@ mod util; use std::collections::HashSet; use std::fs; +use std::io::Read; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -895,7 +896,9 @@ fn load_or_generate_api_key(storage_dir: &Path) -> std::io::Result { let api_key_path = storage_dir.join(API_KEY_FILE); if api_key_path.exists() { - let key_bytes = fs::read(&api_key_path)?; + let file = fs::File::open(&api_key_path)?; + let mut key_bytes = Vec::with_capacity(API_KEY_LEN + 1); + file.take((API_KEY_LEN + 1) as u64).read_to_end(&mut key_bytes)?; if key_bytes.len() != API_KEY_LEN { return Err(std::io::Error::new( std::io::ErrorKind::InvalidData, From e02512efa1a6a2bf11352270408e4eff58af7090 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Thu, 3 Sep 2026 16:55:31 +0000 Subject: [PATCH 5/8] Bound client API key file reads API keys are exactly 32 bytes. Reading malformed or special files in full can otherwise consume unbounded memory. Read only enough bytes to detect an oversized key. Treat missing files separately from read and format errors. This keeps a malformed configured key from being silently ignored in favor of a default key. Use the same open-first handling in the daemon so only NotFound triggers key generation and all other open failures are reported. This commit was created with assistance from Codex. --- ldk-server-cli/src/main.rs | 13 +++++--- ldk-server-client/src/config.rs | 57 +++++++++++++++++++++++++-------- ldk-server-mcp/src/config.rs | 6 ++-- ldk-server/src/main.rs | 9 ++++-- 4 files changed, 63 insertions(+), 22 deletions(-) diff --git a/ldk-server-cli/src/main.rs b/ldk-server-cli/src/main.rs index 224e1d42..e31dcc3a 100644 --- a/ldk-server-cli/src/main.rs +++ b/ldk-server-cli/src/main.rs @@ -617,10 +617,15 @@ async fn main() { }, }; - let api_key = resolve_api_key(cli.api_key, config.as_ref()).unwrap_or_else(|| { - eprintln!("API key not provided. Use --api-key or ensure the api_key file exists at {DEFAULT_DIR}/[network]/api_key"); - std::process::exit(1); - }); + let api_key = resolve_api_key(cli.api_key, config.as_ref()) + .unwrap_or_else(|e| { + eprintln!("Failed to resolve API key: {e}"); + std::process::exit(1); + }) + .unwrap_or_else(|| { + eprintln!("API key not provided. Use --api-key or ensure the api_key file exists at {DEFAULT_DIR}/[network]/api_key"); + std::process::exit(1); + }); let base_url = resolve_base_url(cli.base_url, config.as_ref()); diff --git a/ldk-server-client/src/config.rs b/ldk-server-client/src/config.rs index cbe9a38c..a1a5d424 100644 --- a/ldk-server-client/src/config.rs +++ b/ldk-server-client/src/config.rs @@ -13,7 +13,8 @@ //! locating the server's TLS certificate and API key on disk, so multiple clients (CLI, MCP //! bridge, etc.) can resolve connection credentials in a consistent way. -use std::path::PathBuf; +use std::io::{ErrorKind, Read}; +use std::path::{Path, PathBuf}; use hex_conservative::DisplayHex; use serde::{Deserialize, Serialize}; @@ -21,6 +22,7 @@ use serde::{Deserialize, Serialize}; const DEFAULT_CONFIG_FILE: &str = "config.toml"; const DEFAULT_CERT_FILE: &str = "tls.crt"; const API_KEY_FILE: &str = "api_key"; +const API_KEY_LEN: usize = 32; /// Default address of the `ldk-server` gRPC endpoint when no explicit value is configured. pub const DEFAULT_GRPC_SERVICE_ADDRESS: &str = "127.0.0.1:3536"; @@ -146,18 +148,47 @@ pub fn resolve_base_url(override_url: Option, config: Option<&Config>) - /// Prefers `override_key`, falls back to reading the API key file from the configured storage /// directory, and finally from the OS-specific default data directory. The raw bytes read from /// disk are lower-hex encoded before being returned. -pub fn resolve_api_key(override_key: Option, config: Option<&Config>) -> Option { - override_key.or_else(|| { - let network = - config.and_then(|c| c.network().ok()).unwrap_or_else(|| "bitcoin".to_string()); - storage_dir(config) - .map(|dir| api_key_path_for_storage_dir(dir, &network)) - .and_then(|path| std::fs::read(&path).ok()) - .or_else(|| { - get_default_api_key_path(&network).and_then(|path| std::fs::read(&path).ok()) - }) - .map(|bytes| bytes.to_lower_hex_string()) - }) +/// +/// Returns an error if a candidate API key file exists but cannot be read or does not contain +/// exactly 32 bytes. +pub fn resolve_api_key( + override_key: Option, config: Option<&Config>, +) -> Result, String> { + if override_key.is_some() { + return Ok(override_key); + } + + let network = config.and_then(|c| c.network().ok()).unwrap_or_else(|| "bitcoin".to_string()); + if let Some(dir) = storage_dir(config) { + let path = api_key_path_for_storage_dir(dir, &network); + if let Some(api_key) = read_api_key(&path)? { + return Ok(Some(api_key)); + } + } + + match get_default_api_key_path(&network) { + Some(path) => read_api_key(&path), + None => Ok(None), + } +} + +fn read_api_key(path: &Path) -> Result, String> { + let file = match std::fs::File::open(path) { + Ok(file) => file, + Err(e) if e.kind() == ErrorKind::NotFound => return Ok(None), + Err(e) => return Err(format!("Failed to read API key file '{}': {e}", path.display())), + }; + let mut bytes = Vec::with_capacity(API_KEY_LEN + 1); + file.take((API_KEY_LEN + 1) as u64) + .read_to_end(&mut bytes) + .map_err(|e| format!("Failed to read API key file '{}': {e}", path.display()))?; + if bytes.len() != API_KEY_LEN { + return Err(format!( + "API key file '{}' must contain exactly {API_KEY_LEN} bytes", + path.display() + )); + } + Ok(Some(bytes.to_lower_hex_string())) } /// Resolves the path to the server's TLS certificate (PEM). diff --git a/ldk-server-mcp/src/config.rs b/ldk-server-mcp/src/config.rs index 6f54f60b..7bed7d26 100644 --- a/ldk-server-mcp/src/config.rs +++ b/ldk-server-mcp/src/config.rs @@ -39,7 +39,7 @@ pub fn resolve_config(config_path: Option) -> Result std::io::Result { let api_key_path = storage_dir.join(API_KEY_FILE); - if api_key_path.exists() { - let file = fs::File::open(&api_key_path)?; + let file = match fs::File::open(&api_key_path) { + Ok(file) => Some(file), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, + Err(e) => return Err(e), + }; + + if let Some(file) = file { let mut key_bytes = Vec::with_capacity(API_KEY_LEN + 1); file.take((API_KEY_LEN + 1) as u64).read_to_end(&mut key_bytes)?; if key_bytes.len() != API_KEY_LEN { From 26d59b7261df55fc3655cb53a81d7ca089895e37 Mon Sep 17 00:00:00 2001 From: benthecarman Date: Thu, 3 Sep 2026 13:38:36 -0500 Subject: [PATCH 6/8] Bound mnemonic file reads Mnemonic files are small. Limit reads to 1 KiB so an oversized or special file cannot cause unbounded memory use during startup. This commit was created with assistance from Codex. --- ldk-server/src/util/entropy.rs | 47 ++++++++++++++++++++++------------ ldk-server/src/util/mod.rs | 15 ++++++++++- 2 files changed, 44 insertions(+), 18 deletions(-) diff --git a/ldk-server/src/util/entropy.rs b/ldk-server/src/util/entropy.rs index 8535c6bc..d3299f7a 100644 --- a/ldk-server/src/util/entropy.rs +++ b/ldk-server/src/util/entropy.rs @@ -7,40 +7,42 @@ // You may not use this file except in accordance with one or both of these // licenses. +use std::io; use std::path::Path; use std::str::FromStr; -use std::{fs, io}; use ldk_node::bip39::Mnemonic; use ldk_node::entropy::{generate_entropy_mnemonic, NodeEntropy}; use log::info; -use crate::util::{create_dir_all_private, write_new}; +use crate::util::{create_dir_all_private, read_to_string_with_limit, write_new}; const DEFAULT_MNEMONIC_FILE: &str = "keys_mnemonic"; +const MNEMONIC_FILE_SIZE_LIMIT: usize = 1024; pub(crate) fn load_or_generate_node_entropy(storage_dir: &Path) -> io::Result { let mnemonic_path = storage_dir.join(DEFAULT_MNEMONIC_FILE); - let mnemonic = if mnemonic_path.exists() { - let raw = fs::read_to_string(&mnemonic_path)?; - Mnemonic::from_str(raw.trim()).map_err(|e| { + let mnemonic = match read_to_string_with_limit(&mnemonic_path, MNEMONIC_FILE_SIZE_LIMIT) { + Ok(raw) => Mnemonic::from_str(raw.trim()).map_err(|e| { io::Error::new( io::ErrorKind::InvalidData, format!("Invalid BIP39 mnemonic in {}: {}", mnemonic_path.display(), e), ) - })? - } else { - if let Some(parent) = mnemonic_path.parent() { - create_dir_all_private(parent)?; - } - let mnemonic = generate_entropy_mnemonic(None); - write_new(&mnemonic_path, format!("{}\n", mnemonic).as_bytes(), 0o600)?; - info!( - "Generated new BIP39 mnemonic at {}. Back up this file securely — it is required to recover on-chain funds.", - mnemonic_path.display() - ); - mnemonic + })?, + Err(e) if e.kind() == io::ErrorKind::NotFound => { + if let Some(parent) = mnemonic_path.parent() { + create_dir_all_private(parent)?; + } + let mnemonic = generate_entropy_mnemonic(None); + write_new(&mnemonic_path, format!("{}\n", mnemonic).as_bytes(), 0o600)?; + info!( + "Generated new BIP39 mnemonic at {}. Back up this file securely — it is required to recover on-chain funds.", + mnemonic_path.display() + ); + mnemonic + }, + Err(e) => return Err(e), }; Ok(NodeEntropy::from_bip39_mnemonic(mnemonic, None)) @@ -48,6 +50,7 @@ pub(crate) fn load_or_generate_node_entropy(storage_dir: &Path) -> io::Result io::Result<()> { builder.recursive(true).mode(0o700).create(path) } +pub(crate) fn read_to_string_with_limit(path: &Path, limit: usize) -> io::Result { + let file = fs::File::open(path)?; + let mut contents = String::new(); + file.take(limit.saturating_add(1) as u64).read_to_string(&mut contents)?; + if contents.len() > limit { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("File '{}' exceeds the {limit} byte limit", path.display()), + )); + } + Ok(contents) +} + pub(crate) fn write_new(path: &Path, contents: &[u8], mode: u32) -> io::Result<()> { let mut file = OpenOptions::new().create_new(true).write(true).mode(mode).open(path)?; file.write_all(contents)?; From a4ace8436248164c90ba0282cea75c9bee0995c7 Mon Sep 17 00:00:00 2001 From: benthecarman Date: Thu, 3 Sep 2026 13:40:41 -0500 Subject: [PATCH 7/8] Bound TLS credential file reads Limit server certificate and key reads to 1 MiB. Apply the same certificate limit to the CLI and MCP clients so local files cannot cause unbounded memory use during startup. This commit was created with assistance from Codex. --- ldk-server-cli/src/main.rs | 8 +++---- ldk-server-client/src/config.rs | 41 +++++++++++++++++++++++++++++++-- ldk-server-mcp/src/config.rs | 7 +++--- ldk-server/src/util/tls.rs | 36 ++++++++++++++++++++++++++--- 4 files changed, 79 insertions(+), 13 deletions(-) diff --git a/ldk-server-cli/src/main.rs b/ldk-server-cli/src/main.rs index e31dcc3a..f0cc8482 100644 --- a/ldk-server-cli/src/main.rs +++ b/ldk-server-cli/src/main.rs @@ -15,8 +15,8 @@ use clap_complete::{generate, Shell}; use hex_conservative::{DisplayHex, FromHex}; use ldk_server_client::client::LdkServerClient; use ldk_server_client::config::{ - get_default_config_path, load_config, resolve_api_key, resolve_base_url, resolve_cert_path, - DEFAULT_GRPC_SERVICE_ADDRESS, + get_default_config_path, load_config, read_tls_certificate, resolve_api_key, resolve_base_url, + resolve_cert_path, DEFAULT_GRPC_SERVICE_ADDRESS, }; use ldk_server_client::error::LdkServerError; use ldk_server_client::error::LdkServerErrorCode::{ @@ -635,8 +635,8 @@ async fn main() { std::process::exit(1); }); - let server_cert_pem = std::fs::read(&tls_cert_path).unwrap_or_else(|e| { - eprintln!("Failed to read server certificate file '{}': {}", tls_cert_path.display(), e); + let server_cert_pem = read_tls_certificate(&tls_cert_path).unwrap_or_else(|e| { + eprintln!("{e}"); std::process::exit(1); }); diff --git a/ldk-server-client/src/config.rs b/ldk-server-client/src/config.rs index a1a5d424..5396a8e6 100644 --- a/ldk-server-client/src/config.rs +++ b/ldk-server-client/src/config.rs @@ -13,7 +13,7 @@ //! locating the server's TLS certificate and API key on disk, so multiple clients (CLI, MCP //! bridge, etc.) can resolve connection credentials in a consistent way. -use std::io::{ErrorKind, Read}; +use std::io::{self, ErrorKind, Read}; use std::path::{Path, PathBuf}; use hex_conservative::DisplayHex; @@ -23,6 +23,7 @@ const DEFAULT_CONFIG_FILE: &str = "config.toml"; const DEFAULT_CERT_FILE: &str = "tls.crt"; const API_KEY_FILE: &str = "api_key"; const API_KEY_LEN: usize = 32; +const TLS_CERT_FILE_SIZE_LIMIT: usize = 1024 * 1024; /// Default address of the `ldk-server` gRPC endpoint when no explicit value is configured. pub const DEFAULT_GRPC_SERVICE_ADDRESS: &str = "127.0.0.1:3536"; @@ -133,6 +134,14 @@ pub fn load_config(path: &PathBuf) -> Result { .map_err(|e| format!("Failed to parse config file '{}': {}", path.display(), e)) } +/// Reads the server TLS certificate at `path`. +/// +/// Returns an error if the file exceeds 1 MiB. +pub fn read_tls_certificate(path: &Path) -> Result, String> { + read_with_limit(path, TLS_CERT_FILE_SIZE_LIMIT) + .map_err(|e| format!("Failed to read server certificate file '{}': {e}", path.display())) +} + /// Resolves the base URL of the `ldk-server` gRPC endpoint. /// /// Prefers `override_url`, falls back to the configuration file, and finally to @@ -191,6 +200,19 @@ fn read_api_key(path: &Path) -> Result, String> { Ok(Some(bytes.to_lower_hex_string())) } +fn read_with_limit(path: &Path, limit: usize) -> io::Result> { + let file = std::fs::File::open(path)?; + let mut contents = Vec::new(); + file.take(limit.saturating_add(1) as u64).read_to_end(&mut contents)?; + if contents.len() > limit { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("File '{}' exceeds the {limit} byte limit", path.display()), + )); + } + Ok(contents) +} + /// Resolves the path to the server's TLS certificate (PEM). /// /// Prefers `override_path`, falls back to `tls.cert_path` in the configuration file, then to the @@ -218,7 +240,10 @@ fn default_grpc_service_address() -> String { #[cfg(test)] mod tests { - use super::{resolve_base_url, Config, DEFAULT_GRPC_SERVICE_ADDRESS}; + use super::{ + read_tls_certificate, resolve_base_url, Config, DEFAULT_GRPC_SERVICE_ADDRESS, + TLS_CERT_FILE_SIZE_LIMIT, + }; #[test] fn config_defaults_grpc_service_address() { @@ -313,4 +338,16 @@ mod tests { fn resolve_base_url_falls_back_to_default() { assert_eq!(resolve_base_url(None, None), DEFAULT_GRPC_SERVICE_ADDRESS); } + + #[test] + fn read_tls_certificate_rejects_oversized_file() { + let path = std::env::temp_dir() + .join(format!("ldk-server-client-oversized-cert-{}", std::process::id())); + std::fs::write(&path, vec![0; TLS_CERT_FILE_SIZE_LIMIT + 1]).unwrap(); + + let error = read_tls_certificate(&path).unwrap_err(); + assert!(error.contains("exceeds")); + + std::fs::remove_file(path).unwrap(); + } } diff --git a/ldk-server-mcp/src/config.rs b/ldk-server-mcp/src/config.rs index 7bed7d26..f8c066d9 100644 --- a/ldk-server-mcp/src/config.rs +++ b/ldk-server-mcp/src/config.rs @@ -10,7 +10,8 @@ use std::path::PathBuf; use ldk_server_client::config::{ - get_default_config_path, load_config, resolve_api_key, resolve_base_url, resolve_cert_path, + get_default_config_path, load_config, read_tls_certificate, resolve_api_key, resolve_base_url, + resolve_cert_path, }; pub struct ResolvedConfig { @@ -48,9 +49,7 @@ pub fn resolve_config(config_path: Option) -> Result Vec { /// Loads TLS configuration from provided paths. fn load_tls_config(cert_path: &str, key_path: &str) -> Result { - let cert_pem = fs::read_to_string(cert_path) + let cert_pem = read_to_string_with_limit(Path::new(cert_path), TLS_FILE_SIZE_LIMIT) .map_err(|e| format!("Failed to read TLS certificate file '{cert_path}': {e}"))?; - let key_pem = fs::read_to_string(key_path) + let key_pem = read_to_string_with_limit(Path::new(key_path), TLS_FILE_SIZE_LIMIT) .map_err(|e| format!("Failed to read TLS key file '{key_path}': {e}"))?; let certs = parse_pem_certs(&cert_pem)?; @@ -496,4 +497,33 @@ mod tests { let _ = fs::remove_file(&cert_path); let _ = fs::remove_file(&key_path); } + + #[test] + fn test_load_rejects_oversized_tls_files() { + let temp_dir = std::env::temp_dir(); + let mut suffix_bytes = [0u8; 8]; + getrandom::getrandom(&mut suffix_bytes).unwrap(); + let suffix = u64::from_ne_bytes(suffix_bytes); + let cert_path = temp_dir.join(format!("oversized_tls_cert_{suffix}.pem")); + let key_path = temp_dir.join(format!("oversized_tls_key_{suffix}.pem")); + + generate_self_signed_cert(cert_path.to_str().unwrap(), key_path.to_str().unwrap(), &[]) + .unwrap(); + let valid_cert = fs::read(&cert_path).unwrap(); + + fs::write(&cert_path, vec![b'a'; TLS_FILE_SIZE_LIMIT + 1]).unwrap(); + let error = + load_tls_config(cert_path.to_str().unwrap(), key_path.to_str().unwrap()).unwrap_err(); + assert!(error.contains("exceeds")); + + fs::write(&cert_path, valid_cert).unwrap(); + fs::remove_file(&key_path).unwrap(); + fs::write(&key_path, vec![b'a'; TLS_FILE_SIZE_LIMIT + 1]).unwrap(); + let error = + load_tls_config(cert_path.to_str().unwrap(), key_path.to_str().unwrap()).unwrap_err(); + assert!(error.contains("exceeds")); + + let _ = fs::remove_file(&cert_path); + let _ = fs::remove_file(&key_path); + } } From 93e76457107d9a0ba8b9cab4edce99d2d6e60388 Mon Sep 17 00:00:00 2001 From: benthecarman Date: Thu, 3 Sep 2026 13:41:42 -0500 Subject: [PATCH 8/8] Bound configuration file reads Limit server and client configuration reads to 1 MiB so an oversized or special file cannot cause unbounded memory use during startup. This commit was created with assistance from Codex. --- ldk-server-client/src/config.rs | 26 ++++++++++++++++++++++---- ldk-server/src/util/config.rs | 23 ++++++++++++++++++++--- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/ldk-server-client/src/config.rs b/ldk-server-client/src/config.rs index 5396a8e6..243ab18a 100644 --- a/ldk-server-client/src/config.rs +++ b/ldk-server-client/src/config.rs @@ -23,6 +23,7 @@ const DEFAULT_CONFIG_FILE: &str = "config.toml"; const DEFAULT_CERT_FILE: &str = "tls.crt"; const API_KEY_FILE: &str = "api_key"; const API_KEY_LEN: usize = 32; +const CONFIG_FILE_SIZE_LIMIT: usize = 1024 * 1024; const TLS_CERT_FILE_SIZE_LIMIT: usize = 1024 * 1024; /// Default address of the `ldk-server` gRPC endpoint when no explicit value is configured. @@ -127,8 +128,8 @@ impl Config { } /// Reads and parses the `ldk-server` configuration file at `path`. -pub fn load_config(path: &PathBuf) -> Result { - let contents = std::fs::read_to_string(path) +pub fn load_config(path: &Path) -> Result { + let contents = read_to_string_with_limit(path, CONFIG_FILE_SIZE_LIMIT) .map_err(|e| format!("Failed to read config file '{}': {}", path.display(), e))?; toml::from_str(&contents) .map_err(|e| format!("Failed to parse config file '{}': {}", path.display(), e)) @@ -213,6 +214,11 @@ fn read_with_limit(path: &Path, limit: usize) -> io::Result> { Ok(contents) } +fn read_to_string_with_limit(path: &Path, limit: usize) -> io::Result { + String::from_utf8(read_with_limit(path, limit)?) + .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e)) +} + /// Resolves the path to the server's TLS certificate (PEM). /// /// Prefers `override_path`, falls back to `tls.cert_path` in the configuration file, then to the @@ -241,8 +247,8 @@ fn default_grpc_service_address() -> String { #[cfg(test)] mod tests { use super::{ - read_tls_certificate, resolve_base_url, Config, DEFAULT_GRPC_SERVICE_ADDRESS, - TLS_CERT_FILE_SIZE_LIMIT, + load_config, read_tls_certificate, resolve_base_url, Config, CONFIG_FILE_SIZE_LIMIT, + DEFAULT_GRPC_SERVICE_ADDRESS, TLS_CERT_FILE_SIZE_LIMIT, }; #[test] @@ -350,4 +356,16 @@ mod tests { std::fs::remove_file(path).unwrap(); } + + #[test] + fn load_config_rejects_oversized_file() { + let path = std::env::temp_dir() + .join(format!("ldk-server-client-oversized-config-{}", std::process::id())); + std::fs::write(&path, vec![b'a'; CONFIG_FILE_SIZE_LIMIT + 1]).unwrap(); + + let error = load_config(&path).unwrap_err(); + assert!(error.contains("exceeds")); + + std::fs::remove_file(path).unwrap(); + } } diff --git a/ldk-server/src/util/config.rs b/ldk-server/src/util/config.rs index b8028ef6..48b8de6f 100644 --- a/ldk-server/src/util/config.rs +++ b/ldk-server/src/util/config.rs @@ -7,11 +7,11 @@ // You may not use this file except in accordance with one or both of these // licenses. +use std::io; use std::net::SocketAddr; use std::path::PathBuf; use std::str::FromStr; use std::time::Duration; -use std::{fs, io}; use clap::Parser; use ldk_node::bitcoin::secp256k1::PublicKey; @@ -24,6 +24,9 @@ use ldk_node::probing::{ProbingConfig, ProbingConfigBuilder}; use log::LevelFilter; use serde::{Deserialize, Serialize}; +use crate::util::read_to_string_with_limit; + +const CONFIG_FILE_SIZE_LIMIT: usize = 1024 * 1024; const DEFAULT_GRPC_SERVICE_ADDRESS: &str = "127.0.0.1:3536"; const DEFAULT_PATHFINDING_SCORES_SOURCE_URL: &str = "https://rapidsync.lightningdevkit.org/scoring/scorer.bin"; @@ -1235,7 +1238,7 @@ pub fn load_config(args: &ArgsConfig) -> io::Result { }; if let Some(path) = config_file { - let content = fs::read_to_string(&path).map_err(|e| { + let content = read_to_string_with_limit(&path, CONFIG_FILE_SIZE_LIMIT).map_err(|e| { io::Error::new(e.kind(), format!("Failed to read config file '{:?}': {}", path, e)) })?; let toml_config: TomlConfig = toml::from_str(&content).map_err(|e| { @@ -1288,7 +1291,7 @@ fn parse_host_port(addr: &str) -> io::Result<(String, u16)> { #[cfg(test)] mod tests { - use std::str::FromStr; + use std::{fs, str::FromStr}; use clap::Parser; use ldk_node::bitcoin::secp256k1::PublicKey; @@ -1719,6 +1722,20 @@ mod tests { assert_eq!(error.to_string(), "Must set a single chain source, multiple were configured"); } + #[test] + fn test_rejects_oversized_config_file() { + let path = std::env::temp_dir() + .join(format!("ldk-server-oversized-config-{}", std::process::id())); + fs::write(&path, vec![b'a'; CONFIG_FILE_SIZE_LIMIT + 1]).unwrap(); + let mut args_config = empty_args_config(); + args_config.config_file = Some(path.to_string_lossy().to_string()); + + let error = load_config(&args_config).unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::InvalidData); + + fs::remove_file(path).unwrap(); + } + #[test] fn test_config_optional_values() { let storage_path = std::env::temp_dir();