From 175d889798a1550973ae1a0f040820ea83f740e6 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Thu, 22 Jan 2026 17:35:47 +0900 Subject: [PATCH 1/3] feat: Add comprehensive YAML-based configuration system for bssh-server Implement a hierarchical configuration system supporting YAML files, environment variables, and CLI argument overrides. - Add ServerFileConfig with comprehensive configuration types - Support YAML config files with default search paths - Implement environment variable overrides (BSSH_* prefix) - Add configuration validation at startup - Generate config template function - Maintain backward compatibility with existing ServerConfig API - Add ipnetwork dependency for CIDR validation - Include 25 comprehensive tests covering all features --- Cargo.lock | 10 + Cargo.toml | 1 + src/server/config/loader.rs | 526 ++++++++++++++++++++ src/server/{config.rs => config/mod.rs} | 275 ++++++----- src/server/config/types.rs | 630 ++++++++++++++++++++++++ 5 files changed, 1320 insertions(+), 122 deletions(-) create mode 100644 src/server/config/loader.rs rename src/server/{config.rs => config/mod.rs} (68%) create mode 100644 src/server/config/types.rs diff --git a/Cargo.lock b/Cargo.lock index e095623a..7a0f240a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -385,6 +385,7 @@ dependencies = [ "glob", "indicatif", "insta", + "ipnetwork", "lazy_static", "libc", "lru", @@ -1936,6 +1937,15 @@ dependencies = [ "zeroize", ] +[[package]] +name = "ipnetwork" +version = "0.20.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf466541e9d546596ee94f9f69590f89473455f88372423e0008fc1a7daf100e" +dependencies = [ + "serde", +] + [[package]] name = "is_terminal_polyfill" version = "1.70.2" diff --git a/Cargo.toml b/Cargo.toml index 8b2694c5..471a6a77 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,6 +53,7 @@ fastrand = "2.3.0" tokio-util = "0.7.17" shell-words = "1.1.1" libc = "0.2" +ipnetwork = "0.20" [target.'cfg(target_os = "macos")'.dependencies] security-framework = "3.5.1" diff --git a/src/server/config/loader.rs b/src/server/config/loader.rs new file mode 100644 index 00000000..a40a5cef --- /dev/null +++ b/src/server/config/loader.rs @@ -0,0 +1,526 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Configuration loader for bssh-server. +//! +//! This module handles loading configuration from multiple sources with +//! the following precedence (highest to lowest): +//! 1. CLI arguments +//! 2. Environment variables +//! 3. Configuration file (YAML) +//! 4. Default values + +use super::types::ServerFileConfig; +use anyhow::{Context, Result}; +use std::path::{Path, PathBuf}; + +/// Load configuration from file, environment, and CLI arguments. +/// +/// # Configuration Precedence +/// +/// Configuration is loaded with the following precedence (highest to lowest): +/// 1. CLI arguments (when supported) +/// 2. Environment variables (BSSH_* prefix) +/// 3. Configuration file (YAML) +/// 4. Default values +/// +/// # Arguments +/// +/// * `config_path` - Optional path to configuration file. If None, searches default locations. +/// +/// # Default Locations +/// +/// If no config path is specified, searches in order: +/// 1. `./bssh-server.yaml` (current directory) +/// 2. `/etc/bssh/server.yaml` (system-wide) +/// 3. `$XDG_CONFIG_HOME/bssh/server.yaml` or `~/.config/bssh/server.yaml` (user-specific) +/// +/// # Environment Variables +/// +/// The following environment variables can override config file settings: +/// +/// - `BSSH_PORT` - Server port (e.g., "2222") +/// - `BSSH_BIND_ADDRESS` - Bind address (e.g., "0.0.0.0") +/// - `BSSH_HOST_KEY` - Comma-separated host key paths +/// - `BSSH_MAX_CONNECTIONS` - Maximum concurrent connections +/// - `BSSH_KEEPALIVE_INTERVAL` - Keepalive interval in seconds +/// - `BSSH_AUTH_METHODS` - Comma-separated auth methods (e.g., "publickey,password") +/// - `BSSH_AUTHORIZED_KEYS_DIR` - Directory for authorized_keys files +/// - `BSSH_AUTHORIZED_KEYS_PATTERN` - Pattern for authorized_keys paths +/// - `BSSH_SHELL` - Default shell path +/// - `BSSH_COMMAND_TIMEOUT` - Command timeout in seconds +/// +/// # Example +/// +/// ```no_run +/// use bssh::server::config::load_config; +/// +/// # fn main() -> anyhow::Result<()> { +/// // Load from default locations +/// let config = load_config(None)?; +/// +/// // Load from specific file +/// let config = load_config(Some("/etc/bssh/custom.yaml".as_ref()))?; +/// # Ok(()) +/// # } +/// ``` +/// +/// # Errors +/// +/// Returns an error if: +/// - Configuration file cannot be read or parsed +/// - Environment variables have invalid values +/// - Configuration validation fails +pub fn load_config(config_path: Option<&Path>) -> Result { + // Start with defaults + let mut config = ServerFileConfig::default(); + + // Load from file if specified or found in default locations + if let Some(path) = config_path { + config = load_config_file(path).context("Failed to load configuration file")?; + tracing::info!(path = %path.display(), "Loaded configuration from file"); + } else { + // Try default locations + for path in default_config_paths() { + if path.exists() { + config = load_config_file(&path).context("Failed to load configuration file")?; + tracing::info!(path = %path.display(), "Loaded configuration from file"); + break; + } + } + } + + // Apply environment variable overrides + config = apply_env_overrides(config)?; + + // Validate configuration + validate_config(&config)?; + + Ok(config) +} + +/// Generate a configuration template as YAML string. +/// +/// This generates a fully documented configuration file template with +/// all available options and their default values. +/// +/// # Example +/// +/// ``` +/// use bssh::server::config::generate_config_template; +/// +/// let template = generate_config_template(); +/// std::fs::write("bssh-server.yaml", template).unwrap(); +/// ``` +pub fn generate_config_template() -> String { + let config = ServerFileConfig::default(); + let mut yaml = String::new(); + + // Add header comment + yaml.push_str("# bssh-server configuration file\n"); + yaml.push_str("#\n"); + yaml.push_str( + "# This is a comprehensive configuration template showing all available options.\n", + ); + yaml.push_str("# Uncomment and modify options as needed.\n"); + yaml.push_str("#\n"); + yaml.push_str("# Configuration hierarchy (highest to lowest precedence):\n"); + yaml.push_str("# 1. CLI arguments\n"); + yaml.push_str("# 2. Environment variables (BSSH_* prefix)\n"); + yaml.push_str("# 3. This configuration file\n"); + yaml.push_str("# 4. Default values\n\n"); + + // Serialize config with defaults + yaml.push_str(&serde_yaml::to_string(&config).unwrap_or_default()); + + yaml +} + +/// Load configuration from a YAML file. +fn load_config_file(path: &Path) -> Result { + let content = + std::fs::read_to_string(path).context(format!("Failed to read {}", path.display()))?; + + serde_yaml::from_str(&content).context(format!("Failed to parse {}", path.display())) +} + +/// Get default configuration file search paths. +fn default_config_paths() -> Vec { + let mut paths = Vec::new(); + + // Current directory + paths.push(PathBuf::from("./bssh-server.yaml")); + + // System-wide config + paths.push(PathBuf::from("/etc/bssh/server.yaml")); + + // User config directory + if let Some(config_dir) = dirs::config_dir() { + paths.push(config_dir.join("bssh/server.yaml")); + } + + paths +} + +/// Apply environment variable overrides to configuration. +fn apply_env_overrides(mut config: ServerFileConfig) -> Result { + // BSSH_PORT + if let Ok(port_str) = std::env::var("BSSH_PORT") { + config.server.port = port_str + .parse() + .context(format!("Invalid BSSH_PORT value: {port_str}"))?; + tracing::debug!(port = config.server.port, "Applied BSSH_PORT override"); + } + + // BSSH_BIND_ADDRESS + if let Ok(addr) = std::env::var("BSSH_BIND_ADDRESS") { + config.server.bind_address = addr.clone(); + tracing::debug!(address = %addr, "Applied BSSH_BIND_ADDRESS override"); + } + + // BSSH_HOST_KEY (comma-separated list) + if let Ok(keys) = std::env::var("BSSH_HOST_KEY") { + config.server.host_keys = keys.split(',').map(|s| PathBuf::from(s.trim())).collect(); + tracing::debug!( + key_count = config.server.host_keys.len(), + "Applied BSSH_HOST_KEY override" + ); + } + + // BSSH_MAX_CONNECTIONS + if let Ok(max_str) = std::env::var("BSSH_MAX_CONNECTIONS") { + config.server.max_connections = max_str + .parse() + .context(format!("Invalid BSSH_MAX_CONNECTIONS value: {max_str}"))?; + tracing::debug!( + max = config.server.max_connections, + "Applied BSSH_MAX_CONNECTIONS override" + ); + } + + // BSSH_KEEPALIVE_INTERVAL + if let Ok(interval_str) = std::env::var("BSSH_KEEPALIVE_INTERVAL") { + config.server.keepalive_interval = interval_str.parse().context(format!( + "Invalid BSSH_KEEPALIVE_INTERVAL value: {interval_str}" + ))?; + tracing::debug!( + interval = config.server.keepalive_interval, + "Applied BSSH_KEEPALIVE_INTERVAL override" + ); + } + + // BSSH_AUTH_METHODS (comma-separated: "publickey,password") + if let Ok(methods_str) = std::env::var("BSSH_AUTH_METHODS") { + use super::types::AuthMethod; + let mut methods = Vec::new(); + for method in methods_str.split(',') { + let method = method.trim().to_lowercase(); + match method.as_str() { + "publickey" => methods.push(AuthMethod::PublicKey), + "password" => methods.push(AuthMethod::Password), + _ => { + anyhow::bail!("Unknown auth method in BSSH_AUTH_METHODS: {}", method); + } + } + } + config.auth.methods = methods; + tracing::debug!( + methods = ?config.auth.methods, + "Applied BSSH_AUTH_METHODS override" + ); + } + + // BSSH_AUTHORIZED_KEYS_DIR + if let Ok(dir) = std::env::var("BSSH_AUTHORIZED_KEYS_DIR") { + config.auth.publickey.authorized_keys_dir = Some(PathBuf::from(dir.clone())); + config.auth.publickey.authorized_keys_pattern = None; + tracing::debug!(dir = %dir, "Applied BSSH_AUTHORIZED_KEYS_DIR override"); + } + + // BSSH_AUTHORIZED_KEYS_PATTERN + if let Ok(pattern) = std::env::var("BSSH_AUTHORIZED_KEYS_PATTERN") { + config.auth.publickey.authorized_keys_pattern = Some(pattern.clone()); + config.auth.publickey.authorized_keys_dir = None; + tracing::debug!( + pattern = %pattern, + "Applied BSSH_AUTHORIZED_KEYS_PATTERN override" + ); + } + + // BSSH_SHELL + if let Ok(shell) = std::env::var("BSSH_SHELL") { + config.shell.default = PathBuf::from(shell.clone()); + tracing::debug!(shell = %shell, "Applied BSSH_SHELL override"); + } + + // BSSH_COMMAND_TIMEOUT + if let Ok(timeout_str) = std::env::var("BSSH_COMMAND_TIMEOUT") { + config.shell.command_timeout = timeout_str + .parse() + .context(format!("Invalid BSSH_COMMAND_TIMEOUT value: {timeout_str}"))?; + tracing::debug!( + timeout = config.shell.command_timeout, + "Applied BSSH_COMMAND_TIMEOUT override" + ); + } + + Ok(config) +} + +/// Validate configuration for correctness. +fn validate_config(config: &ServerFileConfig) -> Result<()> { + // Validate host keys exist + if config.server.host_keys.is_empty() { + anyhow::bail!( + "At least one host key must be configured (server.host_keys or BSSH_HOST_KEY)" + ); + } + + for key_path in &config.server.host_keys { + if !key_path.exists() { + anyhow::bail!("Host key file not found: {}", key_path.display()); + } + } + + // Validate authentication configuration + if config.auth.methods.is_empty() { + anyhow::bail!("At least one authentication method must be enabled (auth.methods)"); + } + + // Validate IP ranges (CIDR notation) + for cidr in &config.security.allowed_ips { + cidr.parse::() + .context(format!("Invalid CIDR notation in allowed_ips: {cidr}"))?; + } + + for cidr in &config.security.blocked_ips { + cidr.parse::() + .context(format!("Invalid CIDR notation in blocked_ips: {cidr}"))?; + } + + // Validate port number + if config.server.port == 0 { + anyhow::bail!("Server port cannot be 0"); + } + + // Validate max connections + if config.server.max_connections == 0 { + anyhow::bail!("max_connections must be greater than 0"); + } + + tracing::info!("Configuration validation passed"); + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::io::Write; + use tempfile::NamedTempFile; + + #[test] + fn test_generate_config_template() { + let template = generate_config_template(); + assert!(template.contains("bssh-server configuration")); + assert!(template.contains("server:")); + assert!(template.contains("auth:")); + assert!(template.contains("shell:")); + + // Template should be valid YAML + let parsed: Result = serde_yaml::from_str(&template); + assert!(parsed.is_ok()); + } + + #[test] + fn test_load_config_from_file() { + let yaml_content = r#" +server: + port: 2223 + bind_address: "127.0.0.1" + host_keys: + - /tmp/test_key +auth: + methods: + - publickey +"#; + + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(yaml_content.as_bytes()).unwrap(); + temp_file.flush().unwrap(); + + let config = load_config_file(temp_file.path()).unwrap(); + assert_eq!(config.server.port, 2223); + assert_eq!(config.server.bind_address, "127.0.0.1"); + assert_eq!(config.server.host_keys.len(), 1); + } + + #[test] + #[serial_test::serial] + fn test_env_override_port() { + // Clear any existing env vars + std::env::remove_var("BSSH_PORT"); + + std::env::set_var("BSSH_PORT", "3333"); + let config = apply_env_overrides(ServerFileConfig::default()).unwrap(); + assert_eq!(config.server.port, 3333); + std::env::remove_var("BSSH_PORT"); + } + + #[test] + #[serial_test::serial] + fn test_env_override_bind_address() { + // Clear any existing env vars + std::env::remove_var("BSSH_PORT"); + + std::env::set_var("BSSH_BIND_ADDRESS", "192.168.1.1"); + let config = apply_env_overrides(ServerFileConfig::default()).unwrap(); + assert_eq!(config.server.bind_address, "192.168.1.1"); + std::env::remove_var("BSSH_BIND_ADDRESS"); + } + + #[test] + #[serial_test::serial] + fn test_env_override_host_keys() { + // Clear any existing env vars + std::env::remove_var("BSSH_PORT"); + + std::env::set_var("BSSH_HOST_KEY", "/key1,/key2,/key3"); + let config = apply_env_overrides(ServerFileConfig::default()).unwrap(); + assert_eq!(config.server.host_keys.len(), 3); + assert_eq!(config.server.host_keys[0], PathBuf::from("/key1")); + std::env::remove_var("BSSH_HOST_KEY"); + } + + #[test] + #[serial_test::serial] + fn test_env_override_auth_methods() { + // Clear any existing env vars + std::env::remove_var("BSSH_PORT"); + + std::env::set_var("BSSH_AUTH_METHODS", "publickey,password"); + let config = apply_env_overrides(ServerFileConfig::default()).unwrap(); + assert_eq!(config.auth.methods.len(), 2); + std::env::remove_var("BSSH_AUTH_METHODS"); + } + + #[test] + #[serial_test::serial] + fn test_env_override_invalid_port() { + // Clear any existing env vars first + std::env::remove_var("BSSH_PORT"); + + std::env::set_var("BSSH_PORT", "invalid"); + let result = apply_env_overrides(ServerFileConfig::default()); + assert!(result.is_err()); + std::env::remove_var("BSSH_PORT"); + } + + #[test] + fn test_validate_config_no_host_keys() { + let mut config = ServerFileConfig::default(); + config.server.host_keys.clear(); + + let result = validate_config(&config); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("At least one host key")); + } + + #[test] + fn test_validate_config_no_auth_methods() { + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(b"fake host key").unwrap(); + temp_file.flush().unwrap(); + + let mut config = ServerFileConfig::default(); + config.server.host_keys.push(temp_file.path().to_path_buf()); + config.auth.methods.clear(); + + let result = validate_config(&config); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("authentication method")); + } + + #[test] + fn test_validate_config_invalid_cidr() { + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(b"fake host key").unwrap(); + temp_file.flush().unwrap(); + + let mut config = ServerFileConfig::default(); + config.server.host_keys.push(temp_file.path().to_path_buf()); + config.security.allowed_ips.push("invalid-cidr".to_string()); + + let result = validate_config(&config); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("CIDR")); + } + + #[test] + fn test_validate_config_valid_cidr() { + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(b"fake host key").unwrap(); + temp_file.flush().unwrap(); + + let mut config = ServerFileConfig::default(); + config.server.host_keys.push(temp_file.path().to_path_buf()); + config + .security + .allowed_ips + .push("192.168.1.0/24".to_string()); + config.security.blocked_ips.push("10.0.0.0/8".to_string()); + + // Should pass validation with valid CIDR + let result = validate_config(&config); + assert!(result.is_ok()); + } + + #[test] + fn test_validate_config_zero_port() { + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(b"fake host key").unwrap(); + temp_file.flush().unwrap(); + + let mut config = ServerFileConfig::default(); + config.server.host_keys.push(temp_file.path().to_path_buf()); + config.server.port = 0; + + let result = validate_config(&config); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("port cannot be 0")); + } + + #[test] + fn test_validate_config_zero_max_connections() { + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(b"fake host key").unwrap(); + temp_file.flush().unwrap(); + + let mut config = ServerFileConfig::default(); + config.server.host_keys.push(temp_file.path().to_path_buf()); + config.server.max_connections = 0; + + let result = validate_config(&config); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("max_connections must be greater than 0")); + } +} diff --git a/src/server/config.rs b/src/server/config/mod.rs similarity index 68% rename from src/server/config.rs rename to src/server/config/mod.rs index f370438f..1274c42b 100644 --- a/src/server/config.rs +++ b/src/server/config/mod.rs @@ -12,22 +12,88 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Server configuration types. +//! Server configuration module. //! -//! This module defines configuration options for the SSH server. +//! This module provides both the original builder-based configuration API +//! and a new comprehensive YAML file-based configuration system. +//! +//! # Configuration Systems +//! +//! ## Builder-Based Configuration (Original) +//! +//! The [`ServerConfig`] and [`ServerConfigBuilder`] provide a programmatic +//! way to configure the server: +//! +//! ``` +//! use bssh::server::config::{ServerConfig, ServerConfigBuilder}; +//! +//! let config = ServerConfig::builder() +//! .host_key("/etc/ssh/ssh_host_ed25519_key") +//! .listen_address("0.0.0.0:2222") +//! .max_connections(100) +//! .build(); +//! ``` +//! +//! ## File-Based Configuration (New) +//! +//! The new configuration system supports loading from YAML files with +//! environment variable and CLI argument overrides: +//! +//! ```no_run +//! use bssh::server::config::load_config; +//! +//! # fn main() -> anyhow::Result<()> { +//! // Load from default locations or environment +//! let file_config = load_config(None)?; +//! +//! // Convert to ServerConfig for use with BsshServer +//! let server_config = file_config.into_server_config(); +//! # Ok(()) +//! # } +//! ``` +//! +//! # Configuration File Format +//! +//! Configuration files use YAML format. Generate a template with: +//! +//! ``` +//! use bssh::server::config::generate_config_template; +//! +//! let template = generate_config_template(); +//! println!("{}", template); +//! ``` -use std::path::PathBuf; -use std::sync::Arc; -use std::time::Duration; +mod loader; +mod types; -use serde::{Deserialize, Serialize}; +// Re-export the new configuration types and functions +pub use loader::{generate_config_template, load_config}; +pub use types::*; +// Re-export the original config types for backward compatibility use super::auth::{AuthProvider, PublicKeyAuthConfig, PublicKeyVerifier}; use super::exec::ExecConfig; +use serde::{Deserialize, Serialize}; +use std::path::PathBuf; +use std::sync::Arc; +use std::time::Duration; -/// Configuration for the SSH server. +/// Configuration for the SSH server (original builder-based API). +/// +/// This is the original configuration type used by [`BsshServer`]. +/// For file-based configuration, use [`ServerFileConfig`] and convert +/// it using [`into_server_config()`](ServerFileConfig::into_server_config). +/// +/// # Example /// -/// Contains all settings needed to initialize and run the SSH server. +/// ``` +/// use bssh::server::config::ServerConfig; +/// +/// let config = ServerConfig::builder() +/// .host_key("/etc/ssh/ssh_host_ed25519_key") +/// .listen_address("0.0.0.0:2222") +/// .build(); +/// ``` #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ServerConfig { /// Paths to host key files (e.g., SSH private keys). @@ -80,17 +146,12 @@ pub struct ServerConfig { } /// Serializable configuration for public key authentication. -/// -/// This is a separate type from `PublicKeyAuthConfig` to support -/// serde serialization while keeping the actual config flexible. #[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct PublicKeyAuthConfigSerde { /// Directory containing authorized_keys files. - /// Structure: `{dir}/{username}/authorized_keys` pub authorized_keys_dir: Option, /// Alternative: file path pattern with `{user}` placeholder. - /// Example: `/home/{user}/.ssh/authorized_keys` pub authorized_keys_pattern: Option, } @@ -187,8 +248,6 @@ impl ServerConfig { } /// Create an auth provider based on the configuration. - /// - /// Returns a `PublicKeyVerifier` configured according to the server settings. pub fn create_auth_provider(&self) -> Arc { let config: PublicKeyAuthConfig = self.publickey_auth.clone().into(); Arc::new(PublicKeyVerifier::new(config)) @@ -269,9 +328,6 @@ impl ServerConfigBuilder { } /// Set the authorized_keys directory. - /// - /// The directory should contain subdirectories for each user, - /// with an `authorized_keys` file in each. pub fn authorized_keys_dir(mut self, dir: impl Into) -> Self { self.config.publickey_auth.authorized_keys_dir = Some(dir.into()); self.config.publickey_auth.authorized_keys_pattern = None; @@ -279,9 +335,6 @@ impl ServerConfigBuilder { } /// Set the authorized_keys file pattern. - /// - /// The pattern should contain `{user}` which will be replaced - /// with the username. pub fn authorized_keys_pattern(mut self, pattern: impl Into) -> Self { self.config.publickey_auth.authorized_keys_pattern = Some(pattern.into()); self.config.publickey_auth.authorized_keys_dir = None; @@ -312,6 +365,58 @@ impl ServerConfigBuilder { } } +/// Extension trait for converting file-based config to builder-based config. +impl ServerFileConfig { + /// Convert a file-based configuration to a builder-based ServerConfig. + /// + /// This enables using file-based configuration with the existing BsshServer API. + /// + /// # Example + /// + /// ```no_run + /// use bssh::server::{BsshServer, config::load_config}; + /// + /// # fn main() -> anyhow::Result<()> { + /// let file_config = load_config(None)?; + /// let server_config = file_config.into_server_config(); + /// let server = BsshServer::new(server_config); + /// # Ok(()) + /// # } + /// ``` + pub fn into_server_config(self) -> ServerConfig { + let listen_address = format!("{}:{}", self.server.bind_address, self.server.port); + + // Determine which auth methods are enabled + let allow_publickey = self.auth.methods.contains(&AuthMethod::PublicKey); + let allow_password = self.auth.methods.contains(&AuthMethod::Password); + + ServerConfig { + host_keys: self.server.host_keys, + listen_address, + max_connections: self.server.max_connections, + max_auth_attempts: self.security.max_auth_attempts, + auth_timeout_secs: self.server.timeout, + idle_timeout_secs: self.security.idle_timeout, + allow_password_auth: allow_password, + allow_publickey_auth: allow_publickey, + allow_keyboard_interactive: false, + banner: None, + publickey_auth: PublicKeyAuthConfigSerde { + authorized_keys_dir: self.auth.publickey.authorized_keys_dir, + authorized_keys_pattern: self.auth.publickey.authorized_keys_pattern, + }, + exec: ExecConfig { + default_shell: self.shell.default, + timeout_secs: self.shell.command_timeout, + env: self.shell.env, + working_dir: None, + allowed_commands: None, + blocked_commands: Vec::new(), + }, + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -347,27 +452,23 @@ mod tests { } #[test] - fn test_auth_timeout() { - let config = ServerConfig::default(); - assert_eq!(config.auth_timeout(), Duration::from_secs(120)); - } + fn test_file_config_to_server_config_conversion() { + let mut file_config = ServerFileConfig::default(); + file_config.server.bind_address = "127.0.0.1".to_string(); + file_config.server.port = 2223; + file_config.server.host_keys = vec![PathBuf::from("/test/key")]; + file_config.auth.methods = vec![AuthMethod::PublicKey, AuthMethod::Password]; + file_config.security.max_auth_attempts = 3; + file_config.security.idle_timeout = 600; - #[test] - fn test_idle_timeout() { - let mut config = ServerConfig::default(); - assert!(config.idle_timeout().is_none()); + let server_config = file_config.into_server_config(); - config.idle_timeout_secs = 300; - assert_eq!(config.idle_timeout(), Some(Duration::from_secs(300))); - } - - #[test] - fn test_has_host_keys() { - let mut config = ServerConfig::default(); - assert!(!config.has_host_keys()); - - config.add_host_key("/path/to/key"); - assert!(config.has_host_keys()); + assert_eq!(server_config.listen_address, "127.0.0.1:2223"); + assert_eq!(server_config.host_keys.len(), 1); + assert_eq!(server_config.max_auth_attempts, 3); + assert_eq!(server_config.idle_timeout_secs, 600); + assert!(server_config.allow_publickey_auth); + assert!(server_config.allow_password_auth); } #[test] @@ -378,96 +479,26 @@ mod tests { } #[test] - fn test_builder_host_keys_vec() { - let config = ServerConfig::builder() - .host_keys(vec!["/path/to/key1".into(), "/path/to/key2".into()]) - .build(); - - assert_eq!(config.host_keys.len(), 2); - } - - #[test] - fn test_builder_auth_timeout() { - let config = ServerConfig::builder().auth_timeout_secs(60).build(); - - assert_eq!(config.auth_timeout_secs, 60); - assert_eq!(config.auth_timeout(), Duration::from_secs(60)); - } - - #[test] - fn test_builder_idle_timeout() { - let config = ServerConfig::builder().idle_timeout_secs(600).build(); - - assert_eq!(config.idle_timeout_secs, 600); - assert_eq!(config.idle_timeout(), Some(Duration::from_secs(600))); - } - - #[test] - fn test_builder_authorized_keys_dir() { - let config = ServerConfig::builder() - .authorized_keys_dir("/etc/bssh/authorized_keys") - .build(); - - assert_eq!( - config.publickey_auth.authorized_keys_dir, - Some(PathBuf::from("/etc/bssh/authorized_keys")) - ); - assert!(config.publickey_auth.authorized_keys_pattern.is_none()); - } - - #[test] - fn test_builder_authorized_keys_pattern() { - let config = ServerConfig::builder() - .authorized_keys_pattern("/home/{user}/.ssh/authorized_keys") - .build(); - - assert!(config.publickey_auth.authorized_keys_dir.is_none()); - assert_eq!( - config.publickey_auth.authorized_keys_pattern, - Some("/home/{user}/.ssh/authorized_keys".to_string()) - ); - } - - #[test] - fn test_create_auth_provider() { - let config = ServerConfig::builder() - .authorized_keys_dir("/etc/bssh/keys") - .build(); - - // Provider should be created successfully (verifies no panic) - let _provider = config.create_auth_provider(); - } - - #[test] - fn test_exec_config_default() { + fn test_auth_timeout() { let config = ServerConfig::default(); - assert_eq!(config.exec.default_shell, PathBuf::from("/bin/sh")); - assert_eq!(config.exec.timeout_secs, 3600); - } - - #[test] - fn test_builder_exec_config() { - let exec_config = ExecConfig::new() - .with_shell("/bin/bash") - .with_timeout_secs(1800); - - let config = ServerConfig::builder().exec(exec_config).build(); - - assert_eq!(config.exec.default_shell, PathBuf::from("/bin/bash")); - assert_eq!(config.exec.timeout_secs, 1800); + assert_eq!(config.auth_timeout(), Duration::from_secs(120)); } #[test] - fn test_builder_exec_timeout() { - let config = ServerConfig::builder().exec_timeout_secs(600).build(); + fn test_idle_timeout() { + let mut config = ServerConfig::default(); + assert!(config.idle_timeout().is_none()); - assert_eq!(config.exec.timeout_secs, 600); + config.idle_timeout_secs = 300; + assert_eq!(config.idle_timeout(), Some(Duration::from_secs(300))); } #[test] - fn test_builder_exec_shell() { - let config = ServerConfig::builder().exec_shell("/bin/zsh").build(); + fn test_has_host_keys() { + let mut config = ServerConfig::default(); + assert!(!config.has_host_keys()); - assert_eq!(config.exec.default_shell, PathBuf::from("/bin/zsh")); + config.add_host_key("/path/to/key"); + assert!(config.has_host_keys()); } } diff --git a/src/server/config/types.rs b/src/server/config/types.rs new file mode 100644 index 00000000..0c6dbd25 --- /dev/null +++ b/src/server/config/types.rs @@ -0,0 +1,630 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Configuration types for bssh-server. +//! +//! This module defines the complete configuration schema for YAML file-based +//! server configuration. All types support serde serialization/deserialization. + +use serde::{Deserialize, Serialize}; +use std::collections::HashMap; +use std::path::PathBuf; + +/// Main server configuration loaded from YAML files. +/// +/// This is the root configuration structure that encompasses all server settings. +/// It supports hierarchical configuration from multiple sources: +/// - YAML configuration files +/// - Environment variables +/// - CLI arguments +/// +/// # Example YAML +/// +/// ```yaml +/// server: +/// bind_address: "0.0.0.0" +/// port: 2222 +/// host_keys: +/// - /etc/bssh/ssh_host_ed25519_key +/// max_connections: 100 +/// +/// auth: +/// methods: +/// - publickey +/// publickey: +/// authorized_keys_pattern: "/home/{user}/.ssh/authorized_keys" +/// ``` +#[derive(Debug, Clone, Deserialize, Serialize, Default)] +#[serde(default)] +pub struct ServerFileConfig { + /// Server network and connection settings. + pub server: ServerSettings, + + /// Authentication configuration. + pub auth: AuthConfig, + + /// Shell execution configuration. + pub shell: ShellConfig, + + /// SFTP subsystem configuration. + pub sftp: SftpConfig, + + /// SCP protocol configuration. + pub scp: ScpConfig, + + /// File transfer filtering rules. + pub filter: FilterConfig, + + /// Audit logging configuration. + pub audit: AuditConfig, + + /// Security and access control settings. + pub security: SecurityConfig, +} + +/// Server network and connection settings. +#[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(default)] +pub struct ServerSettings { + /// Address to bind to (e.g., "0.0.0.0" or "127.0.0.1"). + /// + /// Default: "0.0.0.0" + #[serde(default = "default_bind_address")] + pub bind_address: String, + + /// Port to listen on. + /// + /// Default: 2222 (to avoid conflicts with system SSH on port 22) + #[serde(default = "default_port")] + pub port: u16, + + /// Paths to SSH host private key files. + /// + /// At least one host key must be configured. Supports multiple key types: + /// - Ed25519 (recommended) + /// - RSA + /// - ECDSA + #[serde(default)] + pub host_keys: Vec, + + /// Maximum number of concurrent connections. + /// + /// Default: 100 + #[serde(default = "default_max_connections")] + pub max_connections: usize, + + /// Connection timeout in seconds. + /// + /// Connections idle for longer than this will be closed. + /// Set to 0 to disable timeout. + /// + /// Default: 300 (5 minutes) + #[serde(default = "default_timeout")] + pub timeout: u64, + + /// SSH keepalive interval in seconds. + /// + /// Send keepalive messages at this interval to detect broken connections. + /// Set to 0 to disable keepalives. + /// + /// Default: 60 (1 minute) + #[serde(default = "default_keepalive")] + pub keepalive_interval: u64, +} + +/// Authentication configuration. +#[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(default)] +pub struct AuthConfig { + /// List of enabled authentication methods. + /// + /// Default: ["publickey"] + #[serde(default = "default_auth_methods")] + pub methods: Vec, + + /// Public key authentication settings. + #[serde(default)] + pub publickey: PublicKeyAuthSettings, + + /// Password authentication settings. + #[serde(default)] + pub password: PasswordAuthSettings, +} + +/// Authentication method types. +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] +#[serde(rename_all = "lowercase")] +pub enum AuthMethod { + /// Public key authentication (recommended). + PublicKey, + + /// Password authentication. + Password, +} + +/// Public key authentication settings. +#[derive(Debug, Clone, Deserialize, Serialize, Default)] +#[serde(default)] +pub struct PublicKeyAuthSettings { + /// Directory containing per-user authorized_keys files. + /// + /// Structure: `{dir}/{username}/authorized_keys` + /// + /// Example: `/etc/bssh/authorized_keys` + /// would look for `/etc/bssh/alice/authorized_keys` for user "alice" + pub authorized_keys_dir: Option, + + /// Pattern for authorized_keys file path. + /// + /// Supports `{user}` placeholder which will be replaced with username. + /// + /// Example: `/home/{user}/.ssh/authorized_keys` + pub authorized_keys_pattern: Option, +} + +/// Password authentication settings. +#[derive(Debug, Clone, Deserialize, Serialize, Default)] +#[serde(default)] +pub struct PasswordAuthSettings { + /// Path to YAML file containing user definitions. + /// + /// The file should contain a list of UserDefinition entries. + pub users_file: Option, + + /// Inline user definitions. + /// + /// Users can be defined directly in the configuration file. + #[serde(default)] + pub users: Vec, +} + +/// User definition for password authentication. +#[derive(Debug, Clone, Deserialize, Serialize)] +pub struct UserDefinition { + /// Username. + pub name: String, + + /// Password hash (bcrypt or similar). + /// + /// Generate with: `openssl passwd -6` + pub password_hash: String, + + /// Override shell for this user. + #[serde(default)] + pub shell: Option, + + /// Override home directory for this user. + #[serde(default)] + pub home: Option, + + /// Additional environment variables for this user. + #[serde(default)] + pub env: HashMap, +} + +/// Shell execution configuration. +#[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(default)] +pub struct ShellConfig { + /// Default shell for command execution. + /// + /// Default: "/bin/sh" + #[serde(default = "default_shell")] + pub default: PathBuf, + + /// Global environment variables to set for all sessions. + #[serde(default)] + pub env: HashMap, + + /// Command execution timeout in seconds. + /// + /// Commands running longer than this will be terminated. + /// Set to 0 for no timeout. + /// + /// Default: 3600 (1 hour) + #[serde(default = "default_command_timeout")] + pub command_timeout: u64, +} + +/// SFTP subsystem configuration. +#[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(default)] +pub struct SftpConfig { + /// Enable SFTP subsystem. + /// + /// Default: true + #[serde(default = "default_true")] + pub enabled: bool, + + /// Root directory for SFTP operations. + /// + /// If set, SFTP clients will be chrooted to this directory. + /// If None, users have access to the entire filesystem (subject to permissions). + pub root: Option, +} + +/// SCP protocol configuration. +#[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(default)] +pub struct ScpConfig { + /// Enable SCP protocol support. + /// + /// Default: true + #[serde(default = "default_true")] + pub enabled: bool, +} + +/// File transfer filtering configuration. +#[derive(Debug, Clone, Deserialize, Serialize, Default)] +#[serde(default)] +pub struct FilterConfig { + /// Enable file transfer filtering. + /// + /// Default: false + #[serde(default)] + pub enabled: bool, + + /// Filter rules to apply. + /// + /// Rules are evaluated in order. First matching rule determines action. + #[serde(default)] + pub rules: Vec, +} + +/// A single file transfer filter rule. +#[derive(Debug, Clone, Deserialize, Serialize)] +pub struct FilterRule { + /// Glob pattern to match against file paths. + /// + /// Example: "*.exe" matches all executable files + pub pattern: Option, + + /// Path prefix to match. + /// + /// Example: "/tmp/" matches all files in /tmp + pub path_prefix: Option, + + /// Action to take when rule matches. + pub action: FilterAction, +} + +/// Action to take when a filter rule matches. +#[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum FilterAction { + /// Allow the file transfer. + Allow, + + /// Deny the file transfer. + Deny, + + /// Log the file transfer but allow it. + Log, +} + +/// Audit logging configuration. +#[derive(Debug, Clone, Deserialize, Serialize, Default)] +#[serde(default)] +pub struct AuditConfig { + /// Enable audit logging. + /// + /// Default: false + #[serde(default)] + pub enabled: bool, + + /// Audit log exporters. + /// + /// Multiple exporters can be configured to send logs to different destinations. + #[serde(default)] + pub exporters: Vec, +} + +/// Audit log exporter configuration. +#[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(tag = "type")] +pub enum AuditExporterConfig { + /// Export audit logs to a file. + #[serde(rename = "file")] + File { + /// Path to the audit log file. + path: PathBuf, + }, + + /// Export audit logs to OpenTelemetry. + #[serde(rename = "otel")] + Otel { + /// OpenTelemetry collector endpoint. + endpoint: String, + }, + + /// Export audit logs to Logstash. + #[serde(rename = "logstash")] + Logstash { + /// Logstash host. + host: String, + /// Logstash port. + port: u16, + }, +} + +/// Security and access control configuration. +#[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(default)] +pub struct SecurityConfig { + /// Maximum authentication attempts before banning IP. + /// + /// Default: 5 + #[serde(default = "default_max_auth_attempts")] + pub max_auth_attempts: u32, + + /// Ban duration in seconds after exceeding max auth attempts. + /// + /// Default: 300 (5 minutes) + #[serde(default = "default_ban_time")] + pub ban_time: u64, + + /// Maximum number of concurrent sessions per user. + /// + /// Default: 10 + #[serde(default = "default_max_sessions")] + pub max_sessions_per_user: usize, + + /// Idle session timeout in seconds. + /// + /// Sessions idle for longer than this will be terminated. + /// Set to 0 to disable. + /// + /// Default: 3600 (1 hour) + #[serde(default = "default_idle_timeout")] + pub idle_timeout: u64, + + /// Allowed IP ranges in CIDR notation. + /// + /// If non-empty, only connections from these ranges are allowed. + /// Empty list means all IPs are allowed (subject to blocked_ips). + /// + /// Example: ["192.168.1.0/24", "10.0.0.0/8"] + #[serde(default)] + pub allowed_ips: Vec, + + /// Blocked IP ranges in CIDR notation. + /// + /// Connections from these ranges are always denied. + /// + /// Example: ["203.0.113.0/24"] + #[serde(default)] + pub blocked_ips: Vec, +} + +// Default value functions + +fn default_bind_address() -> String { + "0.0.0.0".to_string() +} + +fn default_port() -> u16 { + 2222 +} + +fn default_max_connections() -> usize { + 100 +} + +fn default_timeout() -> u64 { + 300 +} + +fn default_keepalive() -> u64 { + 60 +} + +fn default_auth_methods() -> Vec { + vec![AuthMethod::PublicKey] +} + +fn default_shell() -> PathBuf { + PathBuf::from("/bin/sh") +} + +fn default_command_timeout() -> u64 { + 3600 +} + +fn default_true() -> bool { + true +} + +fn default_max_auth_attempts() -> u32 { + 5 +} + +fn default_ban_time() -> u64 { + 300 +} + +fn default_max_sessions() -> usize { + 10 +} + +fn default_idle_timeout() -> u64 { + 3600 +} + +// Default implementations + +impl Default for ServerSettings { + fn default() -> Self { + Self { + bind_address: default_bind_address(), + port: default_port(), + host_keys: Vec::new(), + max_connections: default_max_connections(), + timeout: default_timeout(), + keepalive_interval: default_keepalive(), + } + } +} + +impl Default for AuthConfig { + fn default() -> Self { + Self { + methods: default_auth_methods(), + publickey: PublicKeyAuthSettings::default(), + password: PasswordAuthSettings::default(), + } + } +} + +impl Default for ShellConfig { + fn default() -> Self { + Self { + default: default_shell(), + env: HashMap::new(), + command_timeout: default_command_timeout(), + } + } +} + +impl Default for SftpConfig { + fn default() -> Self { + Self { + enabled: default_true(), + root: None, + } + } +} + +impl Default for ScpConfig { + fn default() -> Self { + Self { + enabled: default_true(), + } + } +} + +impl Default for SecurityConfig { + fn default() -> Self { + Self { + max_auth_attempts: default_max_auth_attempts(), + ban_time: default_ban_time(), + max_sessions_per_user: default_max_sessions(), + idle_timeout: default_idle_timeout(), + allowed_ips: Vec::new(), + blocked_ips: Vec::new(), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_default_config() { + let config = ServerFileConfig::default(); + assert_eq!(config.server.bind_address, "0.0.0.0"); + assert_eq!(config.server.port, 2222); + assert_eq!(config.server.max_connections, 100); + assert!(config.sftp.enabled); + assert!(config.scp.enabled); + assert!(!config.filter.enabled); + assert!(!config.audit.enabled); + } + + #[test] + fn test_auth_method_serialization() { + let yaml = "publickey"; + let method: AuthMethod = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(method, AuthMethod::PublicKey); + + let yaml = "password"; + let method: AuthMethod = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(method, AuthMethod::Password); + } + + #[test] + fn test_filter_action_serialization() { + let yaml = "allow"; + let action: FilterAction = serde_yaml::from_str(yaml).unwrap(); + matches!(action, FilterAction::Allow); + + let yaml = "deny"; + let action: FilterAction = serde_yaml::from_str(yaml).unwrap(); + matches!(action, FilterAction::Deny); + + let yaml = "log"; + let action: FilterAction = serde_yaml::from_str(yaml).unwrap(); + matches!(action, FilterAction::Log); + } + + #[test] + fn test_yaml_parsing_minimal() { + let yaml = r#" +server: + port: 2222 + host_keys: + - /etc/bssh/host_key +"#; + let config: ServerFileConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.server.port, 2222); + assert_eq!(config.server.host_keys.len(), 1); + } + + #[test] + fn test_yaml_parsing_comprehensive() { + let yaml = r#" +server: + bind_address: "127.0.0.1" + port: 2223 + host_keys: + - /etc/bssh/ssh_host_ed25519_key + - /etc/bssh/ssh_host_rsa_key + max_connections: 50 + timeout: 600 + keepalive_interval: 30 + +auth: + methods: + - publickey + - password + publickey: + authorized_keys_pattern: "/home/{user}/.ssh/authorized_keys" + password: + users: + - name: testuser + password_hash: "$6$rounds=656000$..." + shell: /bin/bash + +shell: + default: /bin/bash + command_timeout: 7200 + env: + LANG: en_US.UTF-8 + +security: + max_auth_attempts: 3 + ban_time: 600 + allowed_ips: + - "192.168.1.0/24" +"#; + let config: ServerFileConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.server.bind_address, "127.0.0.1"); + assert_eq!(config.server.port, 2223); + assert_eq!(config.server.host_keys.len(), 2); + assert_eq!(config.auth.methods.len(), 2); + assert_eq!(config.shell.default, PathBuf::from("/bin/bash")); + assert_eq!(config.security.max_auth_attempts, 3); + assert_eq!(config.security.allowed_ips.len(), 1); + } +} From 7dc3a039c70d12ad8f792238ff9596a2d7fa1d4f Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Thu, 22 Jan 2026 17:42:41 +0900 Subject: [PATCH 2/3] fix: Add security validations for server configuration This commit addresses HIGH and key MEDIUM severity security issues in the server configuration validation: HIGH severity fixes: - Path traversal validation for authorized_keys_pattern: Reject patterns containing ".." and enforce absolute paths to prevent directory traversal - IP address validation for bind_address: Validate that bind_address is a valid IPv4 or IPv6 address before use MEDIUM severity fixes: - Shell path validation: Verify that the configured default shell exists - Config file permissions check: On Unix systems, warn if configuration file is readable by group or others (mode & 0o077 != 0) - Align max_auth_attempts default: Changed ServerConfig.max_auth_attempts default from 6 to 5 to match SecurityConfig.max_auth_attempts All validations include comprehensive error messages and are covered by extensive unit tests. Tests verify both valid and invalid configurations, including edge cases for path traversal attempts and IP address formats. --- src/server/config/loader.rs | 227 ++++++++++++++++++++++++++++++++++++ src/server/config/mod.rs | 4 +- 2 files changed, 229 insertions(+), 2 deletions(-) diff --git a/src/server/config/loader.rs b/src/server/config/loader.rs index a40a5cef..d87351a0 100644 --- a/src/server/config/loader.rs +++ b/src/server/config/loader.rs @@ -149,12 +149,43 @@ pub fn generate_config_template() -> String { /// Load configuration from a YAML file. fn load_config_file(path: &Path) -> Result { + // MEDIUM: Check config file permissions on Unix + #[cfg(unix)] + check_config_file_permissions(path)?; + let content = std::fs::read_to_string(path).context(format!("Failed to read {}", path.display()))?; serde_yaml::from_str(&content).context(format!("Failed to parse {}", path.display())) } +/// Check configuration file permissions on Unix systems. +/// +/// Warns if the config file is readable by group or others, as configuration +/// files may contain sensitive information. +#[cfg(unix)] +fn check_config_file_permissions(path: &Path) -> Result<()> { + use std::os::unix::fs::PermissionsExt; + + let metadata = std::fs::metadata(path) + .context(format!("Failed to read metadata for {}", path.display()))?; + let permissions = metadata.permissions(); + let mode = permissions.mode(); + + // Check if file is readable by group or others (mode & 0o077 != 0) + if mode & 0o077 != 0 { + tracing::warn!( + path = %path.display(), + mode = format!("{:o}", mode & 0o777), + "Configuration file is readable by group or others. \ + Consider using 'chmod 600 {}' to restrict access.", + path.display() + ); + } + + Ok(()) +} + /// Get default configuration file search paths. fn default_config_paths() -> Vec { let mut paths = Vec::new(); @@ -298,6 +329,53 @@ fn validate_config(config: &ServerFileConfig) -> Result<()> { anyhow::bail!("At least one authentication method must be enabled (auth.methods)"); } + // HIGH: Validate bind_address is a valid IP address + config + .server + .bind_address + .parse::() + .context(format!( + "Invalid bind_address: {}", + config.server.bind_address + ))?; + + // HIGH: Validate authorized_keys_pattern for path traversal + if let Some(ref pattern) = config.auth.publickey.authorized_keys_pattern { + // Ensure pattern doesn't contain ".." after substitution + if pattern.contains("..") { + anyhow::bail!( + "authorized_keys_pattern contains '..' which could lead to path traversal: {}", + pattern + ); + } + + // For patterns with {user} placeholder, validate structure + if pattern.contains("{user}") { + // After removing {user}, the pattern should start with absolute path + let without_placeholder = pattern.replace("{user}", ""); + if !without_placeholder.starts_with('/') && !without_placeholder.starts_with("./") { + anyhow::bail!( + "authorized_keys_pattern must use absolute paths: {}", + pattern + ); + } + } else if !pattern.starts_with('/') { + // If no placeholder, must be absolute path + anyhow::bail!( + "authorized_keys_pattern must use absolute paths: {}", + pattern + ); + } + } + + // MEDIUM: Validate shell path exists + if !config.shell.default.exists() { + anyhow::bail!( + "Default shell does not exist: {}", + config.shell.default.display() + ); + } + // Validate IP ranges (CIDR notation) for cidr in &config.security.allowed_ips { cidr.parse::() @@ -523,4 +601,153 @@ auth: .to_string() .contains("max_connections must be greater than 0")); } + + #[test] + fn test_validate_config_invalid_bind_address() { + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(b"fake host key").unwrap(); + temp_file.flush().unwrap(); + + let mut config = ServerFileConfig::default(); + config.server.host_keys.push(temp_file.path().to_path_buf()); + config.server.bind_address = "not-an-ip-address".to_string(); + + let result = validate_config(&config); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("Invalid bind_address")); + } + + #[test] + fn test_validate_config_valid_bind_address() { + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(b"fake host key").unwrap(); + temp_file.flush().unwrap(); + + let mut config = ServerFileConfig::default(); + config.server.host_keys.push(temp_file.path().to_path_buf()); + + // Test IPv4 + config.server.bind_address = "127.0.0.1".to_string(); + assert!(validate_config(&config).is_ok()); + + // Test IPv6 + config.server.bind_address = "::1".to_string(); + assert!(validate_config(&config).is_ok()); + + // Test wildcard + config.server.bind_address = "0.0.0.0".to_string(); + assert!(validate_config(&config).is_ok()); + } + + #[test] + fn test_validate_config_authorized_keys_pattern_path_traversal() { + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(b"fake host key").unwrap(); + temp_file.flush().unwrap(); + + let mut config = ServerFileConfig::default(); + config.server.host_keys.push(temp_file.path().to_path_buf()); + + // Test pattern with ".." + config.auth.publickey.authorized_keys_pattern = Some("/home/../etc/passwd".to_string()); + let result = validate_config(&config); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("path traversal")); + } + + #[test] + fn test_validate_config_authorized_keys_pattern_relative_path() { + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(b"fake host key").unwrap(); + temp_file.flush().unwrap(); + + let mut config = ServerFileConfig::default(); + config.server.host_keys.push(temp_file.path().to_path_buf()); + + // Test relative path without placeholder + config.auth.publickey.authorized_keys_pattern = Some("relative/path".to_string()); + let result = validate_config(&config); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("absolute paths")); + } + + #[test] + fn test_validate_config_authorized_keys_pattern_valid() { + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(b"fake host key").unwrap(); + temp_file.flush().unwrap(); + + let mut config = ServerFileConfig::default(); + config.server.host_keys.push(temp_file.path().to_path_buf()); + + // Test valid absolute path with placeholder + config.auth.publickey.authorized_keys_pattern = + Some("/home/{user}/.ssh/authorized_keys".to_string()); + assert!(validate_config(&config).is_ok()); + + // Test valid absolute path without placeholder + config.auth.publickey.authorized_keys_pattern = + Some("/etc/bssh/authorized_keys".to_string()); + assert!(validate_config(&config).is_ok()); + } + + #[test] + fn test_validate_config_shell_not_exists() { + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(b"fake host key").unwrap(); + temp_file.flush().unwrap(); + + let mut config = ServerFileConfig::default(); + config.server.host_keys.push(temp_file.path().to_path_buf()); + config.shell.default = PathBuf::from("/nonexistent/shell"); + + let result = validate_config(&config); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("Default shell does not exist")); + } + + #[cfg(unix)] + #[test] + fn test_config_file_permissions_warning() { + use std::os::unix::fs::PermissionsExt; + + let yaml_content = r#" +server: + port: 2222 + host_keys: + - /tmp/test_key +"#; + + // Create temp file with world-readable permissions + let mut temp_file = NamedTempFile::new().unwrap(); + temp_file.write_all(yaml_content.as_bytes()).unwrap(); + temp_file.flush().unwrap(); + + // Set permissions to 0644 (world-readable) + let mut permissions = temp_file.as_file().metadata().unwrap().permissions(); + permissions.set_mode(0o644); + std::fs::set_permissions(temp_file.path(), permissions).unwrap(); + + // This should succeed but log a warning + let result = check_config_file_permissions(temp_file.path()); + assert!(result.is_ok()); + + // Set permissions to 0600 (owner only) + let mut permissions = temp_file.as_file().metadata().unwrap().permissions(); + permissions.set_mode(0o600); + std::fs::set_permissions(temp_file.path(), permissions).unwrap(); + + // This should succeed without warning + let result = check_config_file_permissions(temp_file.path()); + assert!(result.is_ok()); + } } diff --git a/src/server/config/mod.rs b/src/server/config/mod.rs index 1274c42b..f75bbce2 100644 --- a/src/server/config/mod.rs +++ b/src/server/config/mod.rs @@ -176,7 +176,7 @@ fn default_max_connections() -> usize { } fn default_max_auth_attempts() -> u32 { - 6 + 5 } fn default_auth_timeout_secs() -> u64 { @@ -427,7 +427,7 @@ mod tests { assert!(config.host_keys.is_empty()); assert_eq!(config.listen_address, "0.0.0.0:2222"); assert_eq!(config.max_connections, 100); - assert_eq!(config.max_auth_attempts, 6); + assert_eq!(config.max_auth_attempts, 5); assert!(!config.allow_password_auth); assert!(config.allow_publickey_auth); } From f6f7099f0a6bcd5941fc76b61ad58ae69f369ffb Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Thu, 22 Jan 2026 17:48:10 +0900 Subject: [PATCH 3/3] docs: Add server configuration documentation and apply formatting fixes - Add comprehensive server-configuration.md documentation - Update ARCHITECTURE.md with server config system details - Update docs/architecture/README.md with server config links - Apply cargo fmt formatting to loader.rs tests Relates to #130 --- ARCHITECTURE.md | 21 +- docs/architecture/README.md | 6 +- docs/architecture/server-configuration.md | 344 ++++++++++++++++++++++ src/server/config/loader.rs | 15 +- 4 files changed, 374 insertions(+), 12 deletions(-) create mode 100644 docs/architecture/server-configuration.md diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index e8415436..3b3d6d04 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -190,12 +190,15 @@ Common utilities for code reuse between bssh client and server implementations: The `security` and `jump::rate_limiter` modules re-export from shared for backward compatibility. ### SSH Server Module +**Documentation**: [docs/architecture/server-configuration.md](./docs/architecture/server-configuration.md) SSH server implementation using the russh library for accepting incoming connections: **Structure** (`src/server/`): - `mod.rs` - `BsshServer` struct and `russh::server::Server` trait implementation -- `config.rs` - `ServerConfig` with builder pattern for server settings +- `config/mod.rs` - Module exports and backward compatibility layer +- `config/types.rs` - Comprehensive configuration types with serde +- `config/loader.rs` - Config loader with validation and environment overrides - `handler.rs` - `SshHandler` implementing `russh::server::Handler` trait - `session.rs` - Session state management (`SessionManager`, `SessionInfo`, `ChannelState`) - `exec.rs` - Command execution for SSH exec requests @@ -209,6 +212,13 @@ SSH server implementation using the russh library for accepting incoming connect - Configures russh with authentication settings - Creates shared rate limiter for authentication attempts +- **Server Configuration System**: Dual configuration system for flexibility + - **Builder API** (`ServerConfig`): Programmatic configuration for embedded use + - **File-Based** (`ServerFileConfig`): YAML configuration with environment overrides + - Configuration precedence: CLI > Environment > File > Defaults + - Configuration validation at startup (host keys, CIDR ranges, paths) + - Support for BSSH_* environment variable overrides + - **ServerConfig**: Configuration options with builder pattern - Host key paths and listen address - Connection limits and timeouts @@ -216,6 +226,15 @@ SSH server implementation using the russh library for accepting incoming connect - Public key authentication configuration (authorized_keys location) - Command execution configuration (shell, timeout, allowed/blocked commands) +- **ServerFileConfig**: Comprehensive YAML file configuration + - Server settings (bind address, port, host keys, keepalive) + - Authentication (public key, password with inline or file-based users) + - Shell configuration (default shell, environment, command timeout) + - SFTP/SCP enablement with optional chroot + - File transfer filtering rules + - Audit logging (file, OpenTelemetry, Logstash exporters) + - Security settings (auth attempts, bans, session limits, IP allowlist/blocklist) + - **SshHandler**: Per-connection handler for SSH protocol events - Public key authentication via AuthProvider trait - Rate limiting for authentication attempts diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 6a091bbf..dda26d33 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -32,6 +32,7 @@ bssh is a high-performance parallel SSH command execution tool with SSH-compatib ### Server Components +- **[Server Configuration](./server-configuration.md)** - YAML-based server configuration, environment overrides, validation - **SSH Server Module** - SSH server implementation using russh (see main ARCHITECTURE.md) - **Server Authentication** - Authentication providers including public key verification (see main ARCHITECTURE.md) @@ -56,7 +57,8 @@ Each component document includes: ### Finding Information - **CLI options and modes** → [CLI Interface](./cli-interface.md) -- **Configuration file format** → [Configuration Management](./configuration.md) +- **Client configuration file format** → [Configuration Management](./configuration.md) +- **Server configuration file format** → [Server Configuration](./server-configuration.md) - **Parallel execution behavior** → [Parallel Executor](./executor.md) - **SSH connection details** → [SSH Client](./ssh-client.md) - **Interactive terminal usage** → [TUI](./tui.md) or [Interactive Mode](./interactive-mode.md) @@ -76,7 +78,7 @@ src/ ├── interactive/ → Interactive Mode ├── jump/ → Jump Host Support ├── forward/ → Port Forwarding -├── server/ → SSH Server (handler, session, config) +├── server/ → SSH Server (handler, session, config/, auth/) ├── shared/ → Shared utilities (validation, rate limiting, auth types, errors) ├── security/ → Security utilities (re-exports from shared for compatibility) └── commands/ → Command Implementations diff --git a/docs/architecture/server-configuration.md b/docs/architecture/server-configuration.md new file mode 100644 index 00000000..ea476b95 --- /dev/null +++ b/docs/architecture/server-configuration.md @@ -0,0 +1,344 @@ +# Server Configuration Architecture + +[Back to Main Architecture](../../ARCHITECTURE.md) + +## Table of Contents +- [Overview](#overview) +- [Configuration Systems](#configuration-systems) +- [File-Based Configuration](#file-based-configuration) +- [Environment Variable Overrides](#environment-variable-overrides) +- [Configuration Validation](#configuration-validation) +- [Data Model](#data-model) +- [Usage Examples](#usage-examples) + +## Overview + +The bssh-server configuration system provides a comprehensive way to configure the SSH server through YAML files, environment variables, and CLI arguments. The system supports a hierarchical configuration model where more specific settings override more general ones. + +### Configuration Precedence + +Settings are applied in the following order (highest to lowest priority): + +1. **CLI arguments** - Command-line options +2. **Environment variables** - BSSH_* prefixed variables +3. **Configuration file** - YAML file settings +4. **Default values** - Built-in defaults + +## Configuration Systems + +### Builder-Based Configuration (Programmatic) + +The original `ServerConfig` and `ServerConfigBuilder` provide a programmatic way to configure the server: + +```rust +use bssh::server::config::{ServerConfig, ServerConfigBuilder}; + +let config = ServerConfig::builder() + .host_key("/etc/ssh/ssh_host_ed25519_key") + .listen_address("0.0.0.0:2222") + .max_connections(100) + .build(); +``` + +### File-Based Configuration (YAML) + +The new `ServerFileConfig` supports YAML file configuration with environment variable overrides: + +```rust +use bssh::server::config::load_config; + +// Load from default locations or environment +let file_config = load_config(None)?; + +// Convert to ServerConfig for use with BsshServer +let server_config = file_config.into_server_config(); +``` + +## File-Based Configuration + +### Default Search Paths + +When no config path is specified, the system searches in order: + +1. `./bssh-server.yaml` (current directory) +2. `/etc/bssh/server.yaml` (system-wide) +3. `$XDG_CONFIG_HOME/bssh/server.yaml` or `~/.config/bssh/server.yaml` (user-specific) + +### Configuration File Permissions + +On Unix systems, the loader checks file permissions and warns if the configuration file is readable by group or others, as configuration files may contain sensitive information. + +### Complete Configuration Schema + +```yaml +# Server network and connection settings +server: + # Address to bind to + bind_address: "0.0.0.0" # Default: "0.0.0.0" + + # Port to listen on + port: 2222 # Default: 2222 + + # Paths to SSH host private key files + host_keys: + - /etc/bssh/ssh_host_ed25519_key + - /etc/bssh/ssh_host_rsa_key + + # Maximum concurrent connections + max_connections: 100 # Default: 100 + + # Connection timeout in seconds (0 to disable) + timeout: 300 # Default: 300 (5 minutes) + + # SSH keepalive interval in seconds (0 to disable) + keepalive_interval: 60 # Default: 60 (1 minute) + +# Authentication configuration +auth: + # Enabled authentication methods + methods: + - publickey # Default: [publickey] + - password + + # Public key authentication settings + publickey: + # Directory containing per-user authorized_keys + # Structure: {dir}/{username}/authorized_keys + authorized_keys_dir: /etc/bssh/authorized_keys + + # OR: Pattern for authorized_keys file path + # {user} placeholder replaced with username + authorized_keys_pattern: "/home/{user}/.ssh/authorized_keys" + + # Password authentication settings + password: + # Path to YAML file with user definitions + users_file: /etc/bssh/users.yaml + + # Inline user definitions + users: + - name: testuser + password_hash: "$6$rounds=656000$..." # openssl passwd -6 + shell: /bin/bash + home: /home/testuser + env: + LANG: en_US.UTF-8 + +# Shell execution configuration +shell: + # Default shell for command execution + default: /bin/sh # Default: /bin/sh + + # Command execution timeout in seconds (0 for no timeout) + command_timeout: 3600 # Default: 3600 (1 hour) + + # Global environment variables + env: + LANG: en_US.UTF-8 + PATH: /usr/local/bin:/usr/bin:/bin + +# SFTP subsystem configuration +sftp: + enabled: true # Default: true + # Optional chroot directory + root: /data/sftp + +# SCP protocol configuration +scp: + enabled: true # Default: true + +# File transfer filtering +filter: + enabled: false # Default: false + rules: + - pattern: "*.exe" + action: deny + - path_prefix: "/tmp/" + action: log + +# Audit logging configuration +audit: + enabled: false # Default: false + exporters: + - type: file + path: /var/log/bssh/audit.log + - type: otel + endpoint: http://otel-collector:4317 + - type: logstash + host: logstash.example.com + port: 5044 + +# Security and access control +security: + # Max auth attempts before banning IP + max_auth_attempts: 5 # Default: 5 + + # Ban duration after exceeding max attempts (seconds) + ban_time: 300 # Default: 300 (5 minutes) + + # Max concurrent sessions per user + max_sessions_per_user: 10 # Default: 10 + + # Idle session timeout (seconds, 0 to disable) + idle_timeout: 3600 # Default: 3600 (1 hour) + + # IP allowlist (CIDR notation, empty = allow all) + allowed_ips: + - "192.168.1.0/24" + - "10.0.0.0/8" + + # IP blocklist (CIDR notation) + blocked_ips: + - "203.0.113.0/24" +``` + +## Environment Variable Overrides + +The following environment variables can override configuration file settings: + +| Variable | Description | Example | +|----------|-------------|---------| +| `BSSH_PORT` | Server port | `2222` | +| `BSSH_BIND_ADDRESS` | Bind address | `0.0.0.0` | +| `BSSH_HOST_KEY` | Comma-separated host key paths | `/etc/ssh/key1,/etc/ssh/key2` | +| `BSSH_MAX_CONNECTIONS` | Maximum concurrent connections | `100` | +| `BSSH_KEEPALIVE_INTERVAL` | Keepalive interval in seconds | `60` | +| `BSSH_AUTH_METHODS` | Comma-separated auth methods | `publickey,password` | +| `BSSH_AUTHORIZED_KEYS_DIR` | Directory for authorized_keys | `/etc/bssh/keys` | +| `BSSH_AUTHORIZED_KEYS_PATTERN` | Pattern for authorized_keys paths | `/home/{user}/.ssh/authorized_keys` | +| `BSSH_SHELL` | Default shell path | `/bin/bash` | +| `BSSH_COMMAND_TIMEOUT` | Command timeout in seconds | `3600` | + +## Configuration Validation + +The configuration system validates settings at load time: + +### Required Validations +- At least one host key must be configured +- At least one authentication method must be enabled +- Host key files must exist + +### Network Validations +- `bind_address` must be a valid IP address (IPv4 or IPv6) +- `port` cannot be 0 +- `max_connections` must be greater than 0 + +### Security Validations +- `authorized_keys_pattern` must not contain `..` (path traversal prevention) +- `authorized_keys_pattern` must use absolute paths +- IP ranges in `allowed_ips` and `blocked_ips` must be valid CIDR notation + +### Shell Validations +- Default shell path must exist on the filesystem + +## Data Model + +### Core Types + +```rust +/// Main server configuration loaded from YAML files +pub struct ServerFileConfig { + pub server: ServerSettings, + pub auth: AuthConfig, + pub shell: ShellConfig, + pub sftp: SftpConfig, + pub scp: ScpConfig, + pub filter: FilterConfig, + pub audit: AuditConfig, + pub security: SecurityConfig, +} + +/// Authentication methods +pub enum AuthMethod { + PublicKey, + Password, +} + +/// Filter actions +pub enum FilterAction { + Allow, + Deny, + Log, +} + +/// Audit exporter types +pub enum AuditExporterConfig { + File { path: PathBuf }, + Otel { endpoint: String }, + Logstash { host: String, port: u16 }, +} +``` + +### Conversion to ServerConfig + +The `ServerFileConfig` can be converted to the builder-based `ServerConfig` for use with `BsshServer`: + +```rust +impl ServerFileConfig { + pub fn into_server_config(self) -> ServerConfig { + // Converts file-based config to runtime config + } +} +``` + +## Usage Examples + +### Basic Server Setup + +```rust +use bssh::server::{BsshServer, config::load_config}; + +#[tokio::main] +async fn main() -> anyhow::Result<()> { + // Load configuration from default locations + let file_config = load_config(None)?; + + // Convert to runtime config + let server_config = file_config.into_server_config(); + + // Create and run server + let server = BsshServer::new(server_config); + server.run().await?; + + Ok(()) +} +``` + +### Custom Configuration Path + +```rust +use bssh::server::config::load_config; +use std::path::Path; + +let config = load_config(Some(Path::new("/custom/path/server.yaml")))?; +``` + +### Generate Configuration Template + +```rust +use bssh::server::config::generate_config_template; + +let template = generate_config_template(); +std::fs::write("bssh-server.yaml", template)?; +``` + +### Environment-Based Configuration + +```bash +# Set environment variables +export BSSH_PORT=2222 +export BSSH_BIND_ADDRESS=0.0.0.0 +export BSSH_HOST_KEY=/etc/bssh/ssh_host_ed25519_key +export BSSH_AUTH_METHODS=publickey,password + +# Run server - will use environment variables +bssh-server +``` + +--- + +**Related Documentation:** +- [SSH Server Module](../../ARCHITECTURE.md#ssh-server-module) +- [Server Authentication](../../ARCHITECTURE.md#server-authentication-module) +- [Client Configuration Management](./configuration.md) +- [Main Architecture](../../ARCHITECTURE.md) diff --git a/src/server/config/loader.rs b/src/server/config/loader.rs index d87351a0..ca5e6f64 100644 --- a/src/server/config/loader.rs +++ b/src/server/config/loader.rs @@ -614,7 +614,10 @@ auth: let result = validate_config(&config); assert!(result.is_err()); - assert!(result.unwrap_err().to_string().contains("Invalid bind_address")); + assert!(result + .unwrap_err() + .to_string() + .contains("Invalid bind_address")); } #[test] @@ -652,10 +655,7 @@ auth: config.auth.publickey.authorized_keys_pattern = Some("/home/../etc/passwd".to_string()); let result = validate_config(&config); assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("path traversal")); + assert!(result.unwrap_err().to_string().contains("path traversal")); } #[test] @@ -671,10 +671,7 @@ auth: config.auth.publickey.authorized_keys_pattern = Some("relative/path".to_string()); let result = validate_config(&config); assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("absolute paths")); + assert!(result.unwrap_err().to_string().contains("absolute paths")); } #[test]