From e554bc47c4c894658b1b3275c1ac09afd618ae10 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Thu, 22 Jan 2026 17:55:24 +0900 Subject: [PATCH 1/3] feat: Implement server CLI interface (bssh-server binary) (#131) Implement the bssh-server binary with comprehensive CLI interface for managing the SSH server. Features: - Main commands: run, gen-config, hash-password, check-config, gen-host-key, version - Global CLI arguments: -c/--config, -b/--bind-address, -p/--port, -k/--host-key, -v/--verbose, -D/--foreground, --pid-file - Configuration file loading with CLI overrides - Signal handling for graceful shutdown (SIGTERM, SIGINT) - Password hashing with bcrypt (cost factor 12) - SSH host key generation (Ed25519/RSA) - Configuration validation and checking - Proper error handling and exit codes Technical details: - Uses clap for CLI parsing with derive API - Integrates with existing ServerFileConfig from issue #130 - Supports both file-based and CLI-based configuration - Added dependencies: bcrypt 0.16, rand 0.8, ssh-key 0.6 - All clippy checks pass with -D warnings Resolves #131 --- .gitignore | 1 + Cargo.lock | 78 ++++++- Cargo.toml | 7 + src/bin/bssh_server.rs | 466 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 548 insertions(+), 4 deletions(-) create mode 100644 src/bin/bssh_server.rs diff --git a/.gitignore b/.gitignore index a6ceea96..d3a2ccd3 100644 --- a/.gitignore +++ b/.gitignore @@ -25,3 +25,4 @@ Thumbs.db .gemini/ references/ vendor/ +bssh-server.yaml diff --git a/Cargo.lock b/Cargo.lock index 7a0f240a..d0073e0e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -269,6 +269,19 @@ version = "1.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7d809780667f4410e7c41b07f52439b94d2bdf8528eeedc287fa38d3b7f95d82" +[[package]] +name = "bcrypt" +version = "0.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b1866ecef4f2d06a0bb77880015fdf2b89e25a1c2e5addacb87e459c86dc67e" +dependencies = [ + "base64", + "blowfish", + "getrandom 0.2.16", + "subtle", + "zeroize", +] + [[package]] name = "bcrypt-pbkdf" version = "0.10.0" @@ -373,6 +386,7 @@ dependencies = [ "arrayvec", "async-trait", "atty", + "bcrypt", "chrono", "clap", "criterion", @@ -394,6 +408,7 @@ dependencies = [ "nix 0.30.1", "once_cell", "owo-colors", + "rand 0.8.5", "ratatui", "regex", "rpassword", @@ -408,6 +423,7 @@ dependencies = [ "shell-words", "signal-hook 0.4.1", "smallvec", + "ssh-key", "tempfile", "terminal_size", "thiserror 2.0.17", @@ -1924,7 +1940,7 @@ dependencies = [ "p384", "p521", "rand_core 0.6.4", - "rsa", + "rsa 0.10.0-rc.11", "sec1", "sha1 0.10.6", "sha1 0.11.0-rc.3", @@ -2358,6 +2374,7 @@ dependencies = [ "rand 0.8.5", "serde", "smallvec", + "zeroize", ] [[package]] @@ -2404,6 +2421,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841" dependencies = [ "autocfg", + "libm", ] [[package]] @@ -2733,6 +2751,17 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" +[[package]] +name = "pkcs1" +version = "0.7.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8ffb9f10fa047879315e6625af03c164b16962a5368d724ed16323b68ace47f" +dependencies = [ + "der 0.7.10", + "pkcs8 0.10.2", + "spki 0.7.3", +] + [[package]] name = "pkcs1" version = "0.8.0-rc.4" @@ -3183,6 +3212,27 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "rsa" +version = "0.9.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b8573f03f5883dcaebdfcf4725caa1ecb9c15b2ef50c43a07b816e06799bb12d" +dependencies = [ + "const-oid 0.9.6", + "digest 0.10.7", + "num-bigint-dig", + "num-integer", + "num-traits", + "pkcs1 0.7.5", + "pkcs8 0.10.2", + "rand_core 0.6.4", + "sha2 0.10.9", + "signature 2.2.0", + "spki 0.7.3", + "subtle", + "zeroize", +] + [[package]] name = "rsa" version = "0.10.0-rc.11" @@ -3193,7 +3243,7 @@ dependencies = [ "crypto-bigint 0.7.0-rc.15", "crypto-primes", "digest 0.11.0-rc.5", - "pkcs1", + "pkcs1 0.8.0-rc.4", "pkcs8 0.11.0-rc.8", "rand_core 0.10.0-rc-3", "sha2 0.11.0-rc.3", @@ -3253,12 +3303,12 @@ dependencies = [ "p521", "pageant", "pbkdf2", - "pkcs1", + "pkcs1 0.8.0-rc.4", "pkcs5", "pkcs8 0.10.2", "rand 0.8.5", "rand_core 0.6.4", - "rsa", + "rsa 0.10.0-rc.11", "russh-cryptovec", "russh-util", "sec1", @@ -3806,6 +3856,26 @@ dependencies = [ "sha2 0.10.9", ] +[[package]] +name = "ssh-key" +version = "0.6.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b86f5297f0f04d08cabaa0f6bff7cb6aec4d9c3b49d87990d63da9d9156a8c3" +dependencies = [ + "p256", + "p384", + "p521", + "rand_core 0.6.4", + "rsa 0.9.10", + "sec1", + "sha2 0.10.9", + "signature 2.2.0", + "ssh-cipher", + "ssh-encoding", + "subtle", + "zeroize", +] + [[package]] name = "static_assertions" version = "1.1.0" diff --git a/Cargo.toml b/Cargo.toml index 471a6a77..26911039 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,6 +54,9 @@ tokio-util = "0.7.17" shell-words = "1.1.1" libc = "0.2" ipnetwork = "0.20" +bcrypt = "0.16" +rand = "0.8" +ssh-key = { version = "0.6", features = ["std"] } [target.'cfg(target_os = "macos")'.dependencies] security-framework = "3.5.1" @@ -72,3 +75,7 @@ mockall = "0.14" name = "large_output_benchmark" harness = false +[[bin]] +name = "bssh-server" +path = "src/bin/bssh_server.rs" + diff --git a/src/bin/bssh_server.rs b/src/bin/bssh_server.rs new file mode 100644 index 00000000..e5dbe3a4 --- /dev/null +++ b/src/bin/bssh_server.rs @@ -0,0 +1,466 @@ +// 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. + +//! bssh-server binary - SSH server for containers +//! +//! This binary provides a command-line interface for managing the bssh SSH server. + +use anyhow::{Context, Result}; +use bssh::server::config::{generate_config_template, load_config, ServerFileConfig}; +use bssh::server::BsshServer; +use bssh::utils::logging; +use clap::{ArgAction, Parser, Subcommand}; +use std::fs; +use std::io::{self, Write}; +use std::path::PathBuf; + +/// Backend.AI SSH Server - A lightweight SSH server for containers +#[derive(Parser, Debug)] +#[command(name = "bssh-server")] +#[command(version)] +#[command(about = "Backend.AI SSH Server - A lightweight SSH server for containers", long_about = None)] +struct Cli { + /// Subcommand to execute + #[command(subcommand)] + command: Option, + + /// Configuration file path + #[arg(short, long, global = true, value_name = "FILE")] + config: Option, + + /// Bind address + #[arg(short = 'b', long, global = true, value_name = "ADDR")] + bind_address: Option, + + /// Port to listen on + #[arg(short, long, global = true, value_name = "PORT")] + port: Option, + + /// Host key file(s) + #[arg(short = 'k', long = "host-key", global = true, value_name = "FILE")] + host_keys: Vec, + + /// Verbosity level (-v, -vv, -vvv) + #[arg(short, long, action = ArgAction::Count, global = true)] + verbose: u8, + + /// Run in foreground (don't daemonize) + #[arg(short = 'D', long, global = true)] + foreground: bool, + + /// PID file path + #[arg(long, global = true, value_name = "FILE")] + pid_file: Option, +} + +/// Available subcommands +#[derive(Subcommand, Debug)] +enum Commands { + /// Start the SSH server (default) + Run, + + /// Generate a configuration file template + GenConfig { + /// Output path (stdout if not specified) + #[arg(short, long, value_name = "FILE")] + output: Option, + }, + + /// Hash a password for configuration + HashPassword, + + /// Check configuration file for errors + CheckConfig, + + /// Generate host keys + GenHostKey { + /// Key type (ed25519 or rsa) + #[arg(short = 't', long, default_value = "ed25519", value_name = "TYPE")] + key_type: String, + + /// Output file path + #[arg(short, long, value_name = "FILE")] + output: PathBuf, + + /// RSA key bits (only for rsa type) + #[arg(long, default_value = "4096", value_name = "BITS")] + bits: u32, + }, + + /// Show version and build information + Version, +} + +/// CLI arguments for configuration overrides +#[derive(Debug, Clone)] +pub struct CliArgs { + pub bind_address: Option, + pub port: Option, + pub host_keys: Vec, +} + +impl From<&Cli> for CliArgs { + fn from(cli: &Cli) -> Self { + Self { + bind_address: cli.bind_address.clone(), + port: cli.port, + host_keys: cli.host_keys.clone(), + } + } +} + +#[tokio::main] +async fn main() -> Result<()> { + let cli = Cli::parse(); + + // Initialize logging based on verbosity + logging::init_logging_console_only(cli.verbose); + + // Execute the appropriate command + match cli.command { + None | Some(Commands::Run) => run_server(&cli).await, + Some(Commands::GenConfig { output }) => gen_config(output), + Some(Commands::HashPassword) => hash_password().await, + Some(Commands::CheckConfig) => check_config(&cli), + Some(Commands::GenHostKey { + key_type, + output, + bits, + }) => gen_host_key(&key_type, &output, bits), + Some(Commands::Version) => show_version(), + } +} + +/// Run the SSH server +async fn run_server(cli: &Cli) -> Result<()> { + tracing::info!("Starting bssh-server"); + + // Load configuration from file + let mut file_config = if let Some(config_path) = &cli.config { + load_config(Some(config_path)) + .with_context(|| format!("Failed to load config from {}", config_path.display()))? + } else { + load_config(None).unwrap_or_else(|_| { + tracing::warn!("No configuration file found, using defaults"); + ServerFileConfig::default() + }) + }; + + // Apply CLI overrides + if let Some(bind_address) = &cli.bind_address { + file_config.server.bind_address = bind_address.clone(); + } + if let Some(port) = cli.port { + file_config.server.port = port; + } + if !cli.host_keys.is_empty() { + file_config.server.host_keys = cli.host_keys.clone(); + } + + // Convert to ServerConfig + let config = file_config.into_server_config(); + + tracing::info!( + address = %config.listen_address, + host_keys = %config.host_keys.len(), + "Server configuration loaded" + ); + + // Validate that we have at least one host key + if !config.has_host_keys() { + anyhow::bail!( + "No host keys configured. Use -k/--host-key or configure in config file. \ + Generate keys with: bssh-server gen-host-key -o /path/to/key" + ); + } + + // Write PID file if requested + if let Some(pid_file) = &cli.pid_file { + write_pid_file(pid_file)?; + } + + // Create and run server + let server = BsshServer::new(config); + + // Setup signal handlers for graceful shutdown + let shutdown_signal = setup_signal_handlers()?; + + tracing::info!("SSH server started successfully"); + + // Run server with graceful shutdown + tokio::select! { + result = server.run() => { + result.context("Server error")?; + } + _ = shutdown_signal => { + tracing::info!("Received shutdown signal"); + } + } + + // Cleanup PID file + if let Some(pid_file) = &cli.pid_file { + let _ = fs::remove_file(pid_file); + } + + tracing::info!("Server stopped"); + Ok(()) +} + +/// Generate a configuration file template +fn gen_config(output: Option) -> Result<()> { + let template = generate_config_template(); + + if let Some(path) = output { + fs::write(&path, &template).context("Failed to write configuration file")?; + println!("Configuration template written to {}", path.display()); + } else { + print!("{}", template); + } + + Ok(()) +} + +/// Hash a password for configuration +async fn hash_password() -> Result<()> { + use rpassword::read_password; + + print!("Enter password: "); + io::stdout().flush()?; + let password = read_password()?; + + if password.is_empty() { + anyhow::bail!("Password cannot be empty"); + } + + print!("Confirm password: "); + io::stdout().flush()?; + let confirm = read_password()?; + + if password != confirm { + anyhow::bail!("Passwords do not match"); + } + + // Use bcrypt for password hashing (cost factor 12) + let hash = bcrypt::hash(&password, 12).context("Failed to hash password")?; + + println!("\nPassword hash (use in configuration):"); + println!("{}", hash); + println!("\nExample configuration:"); + println!("auth:"); + println!(" methods:"); + println!(" - password"); + println!(" password:"); + println!(" users:"); + println!(" - name: username"); + println!(" password_hash: \"{}\"", hash); + + Ok(()) +} + +/// Check configuration file for errors +fn check_config(cli: &Cli) -> Result<()> { + let config = if let Some(config_path) = &cli.config { + load_config(Some(config_path)) + .with_context(|| format!("Failed to load config from {}", config_path.display()))? + } else { + load_config(None).context("Failed to load configuration")? + }; + + println!("✓ Configuration is valid\n"); + println!("Server Configuration:"); + println!(" Bind address: {}", config.server.bind_address); + println!(" Port: {}", config.server.port); + println!(" Host keys: {}", config.server.host_keys.len()); + for key in &config.server.host_keys { + println!(" - {}", key.display()); + } + println!(" Max connections: {}", config.server.max_connections); + println!(" Timeout: {}s", config.server.timeout); + println!(" Keepalive: {}s", config.server.keepalive_interval); + + println!("\nAuthentication:"); + println!(" Methods: {:?}", config.auth.methods); + if let Some(pattern) = &config.auth.publickey.authorized_keys_pattern { + println!(" Authorized keys pattern: {}", pattern); + } + if let Some(dir) = &config.auth.publickey.authorized_keys_dir { + println!(" Authorized keys directory: {}", dir.display()); + } + + println!("\nShell Configuration:"); + println!(" Default shell: {}", config.shell.default.display()); + println!(" Command timeout: {}s", config.shell.command_timeout); + println!(" Environment variables: {}", config.shell.env.len()); + + println!("\nSecurity:"); + println!( + " Max auth attempts: {}", + config.security.max_auth_attempts + ); + println!(" Ban time: {}s", config.security.ban_time); + println!( + " Max sessions per user: {}", + config.security.max_sessions_per_user + ); + println!(" Idle timeout: {}s", config.security.idle_timeout); + + if !config.security.allowed_ips.is_empty() { + println!(" Allowed IPs: {:?}", config.security.allowed_ips); + } + if !config.security.blocked_ips.is_empty() { + println!(" Blocked IPs: {:?}", config.security.blocked_ips); + } + + Ok(()) +} + +/// Generate SSH host keys +fn gen_host_key(key_type: &str, output: &PathBuf, _bits: u32) -> Result<()> { + use russh::keys::PrivateKey; + use ssh_key::LineEnding; + + let key = match key_type.to_lowercase().as_str() { + "ed25519" => { + tracing::info!("Generating Ed25519 host key"); + PrivateKey::random(&mut rand::thread_rng(), russh::keys::Algorithm::Ed25519) + .context("Failed to generate Ed25519 key")? + } + "rsa" => { + if _bits < 2048 { + anyhow::bail!("RSA key size must be at least 2048 bits"); + } + tracing::info!(bits = _bits, "Generating RSA host key"); + PrivateKey::random( + &mut rand::thread_rng(), + russh::keys::Algorithm::Rsa { + hash: Some(russh::keys::HashAlg::Sha256), + }, + ) + .context("Failed to generate RSA key")? + } + _ => { + anyhow::bail!("Unknown key type: {}. Use 'ed25519' or 'rsa'", key_type); + } + }; + + // Save the key in OpenSSH format with Unix line endings + key.write_openssh_file(output, LineEnding::LF) + .context("Failed to write host key")?; + + // Set restrictive permissions (0600) + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let permissions = fs::Permissions::from_mode(0o600); + fs::set_permissions(output, permissions) + .context("Failed to set key file permissions")?; + } + + println!("✓ Host key generated: {}", output.display()); + println!( + "\nAdd this to your configuration file or use -k/--host-key argument:" + ); + println!(" --host-key {}", output.display()); + println!("\nOr in YAML config:"); + println!("server:"); + println!(" host_keys:"); + println!(" - {}", output.display()); + + Ok(()) +} + +/// Show version and build information +fn show_version() -> Result<()> { + println!("bssh-server {}", env!("CARGO_PKG_VERSION")); + println!("Backend.AI SSH Server"); + println!(); + println!("A lightweight SSH server for containers"); + println!(); + println!("Homepage: {}", env!("CARGO_PKG_REPOSITORY")); + + Ok(()) +} + +/// Setup signal handlers for graceful shutdown +fn setup_signal_handlers() -> Result> { + use tokio::signal; + + let ctrl_c = async { + signal::ctrl_c() + .await + .expect("Failed to install Ctrl+C handler"); + }; + + #[cfg(unix)] + let terminate = async { + signal::unix::signal(signal::unix::SignalKind::terminate()) + .expect("Failed to install SIGTERM handler") + .recv() + .await; + }; + + #[cfg(not(unix))] + let terminate = std::future::pending::<()>(); + + Ok(async move { + tokio::select! { + _ = ctrl_c => { + tracing::info!("Received SIGINT (Ctrl+C)"); + } + _ = terminate => { + tracing::info!("Received SIGTERM"); + } + } + }) +} + +/// Write the current process ID to a PID file +fn write_pid_file(path: &PathBuf) -> Result<()> { + let pid = std::process::id(); + fs::write(path, pid.to_string()).context("Failed to write PID file")?; + tracing::debug!(path = %path.display(), pid = pid, "PID file written"); + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_cli_parsing() { + use clap::CommandFactory; + + // Verify CLI structure is valid + Cli::command().debug_assert(); + } + + #[test] + fn test_cli_args_conversion() { + let cli = Cli { + command: None, + config: None, + bind_address: Some("127.0.0.1".to_string()), + port: Some(2222), + host_keys: vec![PathBuf::from("/test/key")], + verbose: 1, + foreground: true, + pid_file: None, + }; + + let args: CliArgs = (&cli).into(); + assert_eq!(args.bind_address, Some("127.0.0.1".to_string())); + assert_eq!(args.port, Some(2222)); + assert_eq!(args.host_keys.len(), 1); + } +} From 3f1d0ac0da49311ffebdb32d39120f39f99197a7 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Thu, 22 Jan 2026 18:01:39 +0900 Subject: [PATCH 2/3] fix: Address critical and high security issues in server CLI - Fix host key file race condition by using atomic file creation with mode 0o600 - Add exclusive PID file lock check to prevent multiple server instances - Add password complexity warning for passwords shorter than 8 characters - Set restrictive permissions (0600) on generated config files --- src/bin/bssh_server.rs | 99 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 89 insertions(+), 10 deletions(-) diff --git a/src/bin/bssh_server.rs b/src/bin/bssh_server.rs index e5dbe3a4..969278f9 100644 --- a/src/bin/bssh_server.rs +++ b/src/bin/bssh_server.rs @@ -222,8 +222,32 @@ fn gen_config(output: Option) -> Result<()> { let template = generate_config_template(); if let Some(path) = output { - fs::write(&path, &template).context("Failed to write configuration file")?; + #[cfg(unix)] + { + use std::fs::OpenOptions; + use std::os::unix::fs::OpenOptionsExt; + + // Write config file with restrictive permissions (0600) + let mut file = OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .mode(0o600) + .open(&path) + .context("Failed to create configuration file")?; + + file.write_all(template.as_bytes()) + .context("Failed to write configuration file")?; + } + + #[cfg(not(unix))] + { + fs::write(&path, &template).context("Failed to write configuration file")?; + } + println!("Configuration template written to {}", path.display()); + #[cfg(unix)] + println!("File permissions set to 0600 (owner read/write only)"); } else { print!("{}", template); } @@ -243,6 +267,13 @@ async fn hash_password() -> Result<()> { anyhow::bail!("Password cannot be empty"); } + // Warn about weak passwords (but still allow them) + if password.len() < 8 { + println!("\n⚠ Warning: Password is shorter than 8 characters."); + println!(" This is considered weak and may be easily compromised."); + println!(" Consider using a longer password for better security.\n"); + } + print!("Confirm password: "); io::stdout().flush()?; let confirm = read_password()?; @@ -354,17 +385,31 @@ fn gen_host_key(key_type: &str, output: &PathBuf, _bits: u32) -> Result<()> { } }; - // Save the key in OpenSSH format with Unix line endings - key.write_openssh_file(output, LineEnding::LF) - .context("Failed to write host key")?; - - // Set restrictive permissions (0600) + // Write the key atomically with correct permissions from the start (prevents race condition) #[cfg(unix)] { - use std::os::unix::fs::PermissionsExt; - let permissions = fs::Permissions::from_mode(0o600); - fs::set_permissions(output, permissions) - .context("Failed to set key file permissions")?; + use std::fs::OpenOptions; + use std::os::unix::fs::OpenOptionsExt; + + let key_data = key.to_openssh(LineEnding::LF)?; + + let mut file = OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .mode(0o600) // Set permissions atomically + .open(output) + .context("Failed to create host key file")?; + + file.write_all(key_data.as_bytes()) + .context("Failed to write host key")?; + } + + #[cfg(not(unix))] + { + // On non-Unix systems, fall back to default method + key.write_openssh_file(output, LineEnding::LF) + .context("Failed to write host key")?; } println!("✓ Host key generated: {}", output.display()); @@ -427,6 +472,40 @@ fn setup_signal_handlers() -> Result> { /// Write the current process ID to a PID file fn write_pid_file(path: &PathBuf) -> Result<()> { + // Check if PID file already exists and refers to a running process + if path.exists() { + if let Ok(existing_pid_str) = fs::read_to_string(path) { + if let Ok(existing_pid) = existing_pid_str.trim().parse::() { + // Check if process is still running + #[cfg(unix)] + { + use nix::sys::signal::kill; + use nix::unistd::Pid; + + let pid = Pid::from_raw(existing_pid); + // Use signal 0 (None) to check if process exists without sending actual signal + if kill(pid, None).is_ok() { + anyhow::bail!( + "Another instance is already running with PID {}. \ + If this is incorrect, remove {} and try again.", + existing_pid, + path.display() + ); + } + } + + #[cfg(not(unix))] + { + // On non-Unix systems, warn but allow overwrite + tracing::warn!( + "PID file exists with PID {}. Overwriting (process check not available on this platform).", + existing_pid + ); + } + } + } + } + let pid = std::process::id(); fs::write(path, pid.to_string()).context("Failed to write PID file")?; tracing::debug!(path = %path.display(), pid = pid, "PID file written"); From 6bf1d9b7c406107de564d889cfd263e2e9588a5f Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Thu, 22 Jan 2026 18:06:26 +0900 Subject: [PATCH 3/3] test: Add comprehensive tests for server CLI binary - Add 19 new unit tests covering CLI parsing, subcommands, and options - Test gen-config with file output and permissions validation - Test gen-host-key for Ed25519/RSA with permission checks - Test write_pid_file including stale PID handling - Test all CLI parsing scenarios and global options - Update ARCHITECTURE.md with Server CLI Binary documentation - Update docs/architecture/README.md with server CLI references - Update docs/architecture/server-configuration.md with CLI commands section - Fix formatting issues in bssh_server.rs (cargo fmt) --- ARCHITECTURE.md | 46 +++ docs/architecture/README.md | 4 + docs/architecture/server-configuration.md | 61 ++++ src/bin/bssh_server.rs | 365 +++++++++++++++++++++- 4 files changed, 469 insertions(+), 7 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 3b3d6d04..4e843da9 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -189,6 +189,52 @@ 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. +### Server CLI Binary +**Binary**: `bssh-server` + +The `bssh-server` binary provides a command-line interface for managing and operating the SSH server: + +**Subcommands**: +- **run** - Start the SSH server (default when no subcommand specified) +- **gen-config** - Generate a configuration file template with secure defaults +- **hash-password** - Hash passwords for configuration using bcrypt +- **check-config** - Validate configuration files and display settings +- **gen-host-key** - Generate SSH host keys (Ed25519 or RSA) +- **version** - Show version and build information + +**Global Options**: +- `-c, --config ` - Configuration file path +- `-b, --bind-address ` - Override bind address +- `-p, --port ` - Override listen port +- `-k, --host-key ` - Host key file(s) (can be repeated) +- `-v, --verbose` - Verbosity level (repeatable: -v, -vv, -vvv) +- `-D, --foreground` - Run in foreground (don't daemonize) +- `--pid-file ` - PID file path + +**Usage Examples**: +```bash +# Generate configuration template +bssh-server gen-config -o /etc/bssh/server.yaml + +# Generate Ed25519 host key (recommended) +bssh-server gen-host-key -t ed25519 -o /etc/bssh/ssh_host_ed25519_key + +# Generate RSA host key (for compatibility) +bssh-server gen-host-key -t rsa -o /etc/bssh/ssh_host_rsa_key --bits 4096 + +# Hash a password for configuration +bssh-server hash-password + +# Validate configuration +bssh-server check-config -c /etc/bssh/server.yaml + +# Start server with configuration file +bssh-server -c /etc/bssh/server.yaml + +# Start server with CLI overrides +bssh-server -c /etc/bssh/server.yaml -p 2222 -b 0.0.0.0 -k /path/to/key +``` + ### SSH Server Module **Documentation**: [docs/architecture/server-configuration.md](./docs/architecture/server-configuration.md) diff --git a/docs/architecture/README.md b/docs/architecture/README.md index dda26d33..8f15384a 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -33,6 +33,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 +- **Server CLI (`bssh-server`)** - Server management commands including host key generation, password hashing, config validation (see main ARCHITECTURE.md) - **SSH Server Module** - SSH server implementation using russh (see main ARCHITECTURE.md) - **Server Authentication** - Authentication providers including public key verification (see main ARCHITECTURE.md) @@ -59,6 +60,7 @@ Each component document includes: - **CLI options and modes** → [CLI Interface](./cli-interface.md) - **Client configuration file format** → [Configuration Management](./configuration.md) - **Server configuration file format** → [Server Configuration](./server-configuration.md) +- **Server CLI commands** → Main ARCHITECTURE.md (Server CLI Binary section) - **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) @@ -70,6 +72,8 @@ Each component document includes: ``` src/ +├── bin/ +│ └── bssh_server.rs → Server CLI Binary (bssh-server) ├── cli/ → CLI Interface ├── config/ → Configuration Management ├── executor/ → Parallel Executor diff --git a/docs/architecture/server-configuration.md b/docs/architecture/server-configuration.md index ea476b95..445b1cbb 100644 --- a/docs/architecture/server-configuration.md +++ b/docs/architecture/server-configuration.md @@ -335,9 +335,70 @@ export BSSH_AUTH_METHODS=publickey,password bssh-server ``` +## Server CLI Commands + +The `bssh-server` binary provides several management commands: + +### Generate Configuration Template + +```bash +# Output to stdout +bssh-server gen-config + +# Write to file with secure permissions (0600) +bssh-server gen-config -o /etc/bssh/server.yaml +``` + +### Generate Host Keys + +```bash +# Generate Ed25519 key (recommended, fast, secure) +bssh-server gen-host-key -t ed25519 -o /etc/bssh/ssh_host_ed25519_key + +# Generate RSA key with custom size +bssh-server gen-host-key -t rsa -o /etc/bssh/ssh_host_rsa_key --bits 4096 +``` + +Generated keys have secure permissions (0600) and are in OpenSSH format. + +### Hash Passwords + +```bash +# Interactive password hashing with bcrypt +bssh-server hash-password +``` + +This prompts for a password, confirms it, and outputs a bcrypt hash suitable for use in the configuration file. + +### Validate Configuration + +```bash +# Check default config locations +bssh-server check-config + +# Check specific config file +bssh-server check-config -c /etc/bssh/server.yaml +``` + +Displays all configuration settings and validates the file format. + +### Start Server + +```bash +# Start with config file +bssh-server -c /etc/bssh/server.yaml + +# Start with CLI overrides +bssh-server -c /etc/bssh/server.yaml -p 2222 -b 0.0.0.0 + +# Run in foreground with verbose logging +bssh-server -c /etc/bssh/server.yaml -D -vvv +``` + --- **Related Documentation:** +- [Server CLI Binary](../../ARCHITECTURE.md#server-cli-binary) - [SSH Server Module](../../ARCHITECTURE.md#ssh-server-module) - [Server Authentication](../../ARCHITECTURE.md#server-authentication-module) - [Client Configuration Management](./configuration.md) diff --git a/src/bin/bssh_server.rs b/src/bin/bssh_server.rs index 969278f9..afc74e91 100644 --- a/src/bin/bssh_server.rs +++ b/src/bin/bssh_server.rs @@ -335,10 +335,7 @@ fn check_config(cli: &Cli) -> Result<()> { println!(" Environment variables: {}", config.shell.env.len()); println!("\nSecurity:"); - println!( - " Max auth attempts: {}", - config.security.max_auth_attempts - ); + println!(" Max auth attempts: {}", config.security.max_auth_attempts); println!(" Ban time: {}s", config.security.ban_time); println!( " Max sessions per user: {}", @@ -413,9 +410,7 @@ fn gen_host_key(key_type: &str, output: &PathBuf, _bits: u32) -> Result<()> { } println!("✓ Host key generated: {}", output.display()); - println!( - "\nAdd this to your configuration file or use -k/--host-key argument:" - ); + println!("\nAdd this to your configuration file or use -k/--host-key argument:"); println!(" --host-key {}", output.display()); println!("\nOr in YAML config:"); println!("server:"); @@ -515,6 +510,7 @@ fn write_pid_file(path: &PathBuf) -> Result<()> { #[cfg(test)] mod tests { use super::*; + use tempfile::tempdir; #[test] fn test_cli_parsing() { @@ -542,4 +538,359 @@ mod tests { assert_eq!(args.port, Some(2222)); assert_eq!(args.host_keys.len(), 1); } + + #[test] + fn test_cli_args_conversion_empty() { + let cli = Cli { + command: None, + config: None, + bind_address: None, + port: None, + host_keys: vec![], + verbose: 0, + foreground: false, + pid_file: None, + }; + + let args: CliArgs = (&cli).into(); + assert_eq!(args.bind_address, None); + assert_eq!(args.port, None); + assert!(args.host_keys.is_empty()); + } + + #[test] + fn test_cli_args_conversion_multiple_host_keys() { + let cli = Cli { + command: None, + config: None, + bind_address: None, + port: None, + host_keys: vec![ + PathBuf::from("/test/key1"), + PathBuf::from("/test/key2"), + PathBuf::from("/test/key3"), + ], + verbose: 0, + foreground: false, + pid_file: None, + }; + + let args: CliArgs = (&cli).into(); + assert_eq!(args.host_keys.len(), 3); + } + + #[test] + fn test_cli_parsing_with_subcommand() { + use clap::Parser; + + // Test run subcommand + let args = Cli::try_parse_from(["bssh-server", "run"]).unwrap(); + assert!(matches!(args.command, Some(Commands::Run))); + + // Test gen-config subcommand + let args = Cli::try_parse_from(["bssh-server", "gen-config"]).unwrap(); + assert!(matches!( + args.command, + Some(Commands::GenConfig { output: None }) + )); + + // Test gen-config with output + let args = + Cli::try_parse_from(["bssh-server", "gen-config", "-o", "/tmp/config.yaml"]).unwrap(); + if let Some(Commands::GenConfig { output }) = args.command { + assert_eq!(output, Some(PathBuf::from("/tmp/config.yaml"))); + } else { + panic!("Expected GenConfig command"); + } + + // Test hash-password subcommand + let args = Cli::try_parse_from(["bssh-server", "hash-password"]).unwrap(); + assert!(matches!(args.command, Some(Commands::HashPassword))); + + // Test check-config subcommand + let args = Cli::try_parse_from(["bssh-server", "check-config"]).unwrap(); + assert!(matches!(args.command, Some(Commands::CheckConfig))); + + // Test version subcommand + let args = Cli::try_parse_from(["bssh-server", "version"]).unwrap(); + assert!(matches!(args.command, Some(Commands::Version))); + } + + #[test] + fn test_cli_parsing_gen_host_key() { + use clap::Parser; + + // Test gen-host-key with required output + let args = + Cli::try_parse_from(["bssh-server", "gen-host-key", "-o", "/tmp/host_key"]).unwrap(); + if let Some(Commands::GenHostKey { + key_type, + output, + bits, + }) = args.command + { + assert_eq!(key_type, "ed25519"); // default + assert_eq!(output, PathBuf::from("/tmp/host_key")); + assert_eq!(bits, 4096); // default + } else { + panic!("Expected GenHostKey command"); + } + + // Test gen-host-key with all options + let args = Cli::try_parse_from([ + "bssh-server", + "gen-host-key", + "-t", + "rsa", + "-o", + "/tmp/host_key", + "--bits", + "2048", + ]) + .unwrap(); + if let Some(Commands::GenHostKey { + key_type, + output, + bits, + }) = args.command + { + assert_eq!(key_type, "rsa"); + assert_eq!(output, PathBuf::from("/tmp/host_key")); + assert_eq!(bits, 2048); + } else { + panic!("Expected GenHostKey command"); + } + } + + #[test] + fn test_cli_global_options() { + use clap::Parser; + + // Test global options + let args = Cli::try_parse_from([ + "bssh-server", + "-c", + "/etc/bssh/config.yaml", + "-b", + "192.168.1.1", + "-p", + "2222", + "-k", + "/etc/bssh/key1", + "-k", + "/etc/bssh/key2", + "-vvv", + "-D", + "--pid-file", + "/var/run/bssh.pid", + "run", + ]) + .unwrap(); + + assert_eq!(args.config, Some(PathBuf::from("/etc/bssh/config.yaml"))); + assert_eq!(args.bind_address, Some("192.168.1.1".to_string())); + assert_eq!(args.port, Some(2222)); + assert_eq!(args.host_keys.len(), 2); + assert_eq!(args.verbose, 3); + assert!(args.foreground); + assert_eq!(args.pid_file, Some(PathBuf::from("/var/run/bssh.pid"))); + } + + #[test] + fn test_gen_config_to_stdout() { + // gen_config with None output should print to stdout (captured by the function) + let result = gen_config(None); + assert!(result.is_ok()); + } + + #[test] + fn test_gen_config_to_file() { + let temp_dir = tempdir().unwrap(); + let config_path = temp_dir.path().join("server.yaml"); + + let result = gen_config(Some(config_path.clone())); + assert!(result.is_ok()); + + // Verify file was created + assert!(config_path.exists()); + + // Verify file has content + let content = fs::read_to_string(&config_path).unwrap(); + assert!(!content.is_empty()); + assert!(content.contains("server:")); + assert!(content.contains("bind_address")); + } + + #[test] + #[cfg(unix)] + fn test_gen_config_file_permissions() { + use std::os::unix::fs::PermissionsExt; + + let temp_dir = tempdir().unwrap(); + let config_path = temp_dir.path().join("server.yaml"); + + let result = gen_config(Some(config_path.clone())); + assert!(result.is_ok()); + + // Verify file permissions are 0600 + let metadata = fs::metadata(&config_path).unwrap(); + let permissions = metadata.permissions(); + assert_eq!(permissions.mode() & 0o777, 0o600); + } + + #[test] + fn test_show_version() { + let result = show_version(); + assert!(result.is_ok()); + } + + #[test] + fn test_gen_host_key_ed25519() { + let temp_dir = tempdir().unwrap(); + let key_path = temp_dir.path().join("host_key_ed25519"); + + let result = gen_host_key("ed25519", &key_path, 4096); + assert!(result.is_ok()); + + // Verify file was created + assert!(key_path.exists()); + + // Verify file has OpenSSH key format + let content = fs::read_to_string(&key_path).unwrap(); + assert!(content.contains("-----BEGIN OPENSSH PRIVATE KEY-----")); + assert!(content.contains("-----END OPENSSH PRIVATE KEY-----")); + } + + #[test] + fn test_gen_host_key_rsa() { + let temp_dir = tempdir().unwrap(); + let key_path = temp_dir.path().join("host_key_rsa"); + + let result = gen_host_key("rsa", &key_path, 2048); + assert!(result.is_ok()); + + // Verify file was created + assert!(key_path.exists()); + + // Verify file has OpenSSH key format + let content = fs::read_to_string(&key_path).unwrap(); + assert!(content.contains("-----BEGIN OPENSSH PRIVATE KEY-----")); + assert!(content.contains("-----END OPENSSH PRIVATE KEY-----")); + } + + #[test] + fn test_gen_host_key_invalid_type() { + let temp_dir = tempdir().unwrap(); + let key_path = temp_dir.path().join("host_key_invalid"); + + let result = gen_host_key("dsa", &key_path, 4096); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("Unknown key type")); + } + + #[test] + fn test_gen_host_key_rsa_small_bits() { + let temp_dir = tempdir().unwrap(); + let key_path = temp_dir.path().join("host_key_rsa_small"); + + let result = gen_host_key("rsa", &key_path, 1024); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("RSA key size must be at least 2048")); + } + + #[test] + #[cfg(unix)] + fn test_gen_host_key_file_permissions() { + use std::os::unix::fs::PermissionsExt; + + let temp_dir = tempdir().unwrap(); + let key_path = temp_dir.path().join("host_key_ed25519"); + + let result = gen_host_key("ed25519", &key_path, 4096); + assert!(result.is_ok()); + + // Verify file permissions are 0600 + let metadata = fs::metadata(&key_path).unwrap(); + let permissions = metadata.permissions(); + assert_eq!(permissions.mode() & 0o777, 0o600); + } + + #[test] + fn test_gen_host_key_case_insensitive() { + let temp_dir = tempdir().unwrap(); + + // Test uppercase + let key_path = temp_dir.path().join("host_key_ED25519"); + let result = gen_host_key("ED25519", &key_path, 4096); + assert!(result.is_ok()); + + // Test mixed case + let key_path = temp_dir.path().join("host_key_Ed25519"); + let result = gen_host_key("Ed25519", &key_path, 4096); + assert!(result.is_ok()); + + // Test RSA uppercase + let key_path = temp_dir.path().join("host_key_RSA"); + let result = gen_host_key("RSA", &key_path, 2048); + assert!(result.is_ok()); + } + + #[test] + fn test_write_pid_file() { + let temp_dir = tempdir().unwrap(); + let pid_path = temp_dir.path().join("test.pid"); + + let result = write_pid_file(&pid_path); + assert!(result.is_ok()); + + // Verify file was created with current PID + let content = fs::read_to_string(&pid_path).unwrap(); + let pid: u32 = content.parse().unwrap(); + assert_eq!(pid, std::process::id()); + } + + #[test] + fn test_write_pid_file_overwrite_stale() { + let temp_dir = tempdir().unwrap(); + let pid_path = temp_dir.path().join("test.pid"); + + // Write a stale PID (non-existent process) + // PID 1 is init and always exists, so use a very high unlikely PID + fs::write(&pid_path, "999999999").unwrap(); + + // Should succeed because the process doesn't exist + let result = write_pid_file(&pid_path); + assert!(result.is_ok()); + + // Verify file was updated with current PID + let content = fs::read_to_string(&pid_path).unwrap(); + let pid: u32 = content.parse().unwrap(); + assert_eq!(pid, std::process::id()); + } + + #[test] + fn test_write_pid_file_invalid_content() { + let temp_dir = tempdir().unwrap(); + let pid_path = temp_dir.path().join("test.pid"); + + // Write invalid content + fs::write(&pid_path, "not-a-pid").unwrap(); + + // Should succeed because the content is not a valid PID + let result = write_pid_file(&pid_path); + assert!(result.is_ok()); + } + + #[test] + fn test_default_command_is_run() { + use clap::Parser; + + // No subcommand should default to run behavior + let args = Cli::try_parse_from(["bssh-server"]).unwrap(); + assert!(args.command.is_none()); + // In main(), None is treated the same as Some(Commands::Run) + } }