Skip to content

Commit 3f1d0ac

Browse files
committed
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
1 parent e554bc4 commit 3f1d0ac

1 file changed

Lines changed: 89 additions & 10 deletions

File tree

src/bin/bssh_server.rs

Lines changed: 89 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,32 @@ fn gen_config(output: Option<PathBuf>) -> Result<()> {
222222
let template = generate_config_template();
223223

224224
if let Some(path) = output {
225-
fs::write(&path, &template).context("Failed to write configuration file")?;
225+
#[cfg(unix)]
226+
{
227+
use std::fs::OpenOptions;
228+
use std::os::unix::fs::OpenOptionsExt;
229+
230+
// Write config file with restrictive permissions (0600)
231+
let mut file = OpenOptions::new()
232+
.write(true)
233+
.create(true)
234+
.truncate(true)
235+
.mode(0o600)
236+
.open(&path)
237+
.context("Failed to create configuration file")?;
238+
239+
file.write_all(template.as_bytes())
240+
.context("Failed to write configuration file")?;
241+
}
242+
243+
#[cfg(not(unix))]
244+
{
245+
fs::write(&path, &template).context("Failed to write configuration file")?;
246+
}
247+
226248
println!("Configuration template written to {}", path.display());
249+
#[cfg(unix)]
250+
println!("File permissions set to 0600 (owner read/write only)");
227251
} else {
228252
print!("{}", template);
229253
}
@@ -243,6 +267,13 @@ async fn hash_password() -> Result<()> {
243267
anyhow::bail!("Password cannot be empty");
244268
}
245269

270+
// Warn about weak passwords (but still allow them)
271+
if password.len() < 8 {
272+
println!("\n⚠ Warning: Password is shorter than 8 characters.");
273+
println!(" This is considered weak and may be easily compromised.");
274+
println!(" Consider using a longer password for better security.\n");
275+
}
276+
246277
print!("Confirm password: ");
247278
io::stdout().flush()?;
248279
let confirm = read_password()?;
@@ -354,17 +385,31 @@ fn gen_host_key(key_type: &str, output: &PathBuf, _bits: u32) -> Result<()> {
354385
}
355386
};
356387

357-
// Save the key in OpenSSH format with Unix line endings
358-
key.write_openssh_file(output, LineEnding::LF)
359-
.context("Failed to write host key")?;
360-
361-
// Set restrictive permissions (0600)
388+
// Write the key atomically with correct permissions from the start (prevents race condition)
362389
#[cfg(unix)]
363390
{
364-
use std::os::unix::fs::PermissionsExt;
365-
let permissions = fs::Permissions::from_mode(0o600);
366-
fs::set_permissions(output, permissions)
367-
.context("Failed to set key file permissions")?;
391+
use std::fs::OpenOptions;
392+
use std::os::unix::fs::OpenOptionsExt;
393+
394+
let key_data = key.to_openssh(LineEnding::LF)?;
395+
396+
let mut file = OpenOptions::new()
397+
.write(true)
398+
.create(true)
399+
.truncate(true)
400+
.mode(0o600) // Set permissions atomically
401+
.open(output)
402+
.context("Failed to create host key file")?;
403+
404+
file.write_all(key_data.as_bytes())
405+
.context("Failed to write host key")?;
406+
}
407+
408+
#[cfg(not(unix))]
409+
{
410+
// On non-Unix systems, fall back to default method
411+
key.write_openssh_file(output, LineEnding::LF)
412+
.context("Failed to write host key")?;
368413
}
369414

370415
println!("✓ Host key generated: {}", output.display());
@@ -427,6 +472,40 @@ fn setup_signal_handlers() -> Result<impl std::future::Future<Output = ()>> {
427472

428473
/// Write the current process ID to a PID file
429474
fn write_pid_file(path: &PathBuf) -> Result<()> {
475+
// Check if PID file already exists and refers to a running process
476+
if path.exists() {
477+
if let Ok(existing_pid_str) = fs::read_to_string(path) {
478+
if let Ok(existing_pid) = existing_pid_str.trim().parse::<i32>() {
479+
// Check if process is still running
480+
#[cfg(unix)]
481+
{
482+
use nix::sys::signal::kill;
483+
use nix::unistd::Pid;
484+
485+
let pid = Pid::from_raw(existing_pid);
486+
// Use signal 0 (None) to check if process exists without sending actual signal
487+
if kill(pid, None).is_ok() {
488+
anyhow::bail!(
489+
"Another instance is already running with PID {}. \
490+
If this is incorrect, remove {} and try again.",
491+
existing_pid,
492+
path.display()
493+
);
494+
}
495+
}
496+
497+
#[cfg(not(unix))]
498+
{
499+
// On non-Unix systems, warn but allow overwrite
500+
tracing::warn!(
501+
"PID file exists with PID {}. Overwriting (process check not available on this platform).",
502+
existing_pid
503+
);
504+
}
505+
}
506+
}
507+
}
508+
430509
let pid = std::process::id();
431510
fs::write(path, pid.to_string()).context("Failed to write PID file")?;
432511
tracing::debug!(path = %path.display(), pid = pid, "PID file written");

0 commit comments

Comments
 (0)