Skip to content

Commit 382f506

Browse files
authored
feat: Implement command execution handler for server (#148)
* feat: Implement command execution handler for server Implement the exec_request handler for bssh-server, enabling clients to execute remote commands via SSH. Features include: - CommandExecutor with configurable shell and timeout - Stdout/stderr streaming to SSH channel - Command allow/block list validation for security - Exit code propagation to client - Environment variable configuration (HOME, USER, SHELL, PATH) - Working directory configuration - Proper channel lifecycle (success, eof, close) Closes #128 * security: Fix critical and high security issues in command execution This commit addresses all CRITICAL and HIGH severity security vulnerabilities identified in PR #148 code review. CRITICAL Issues Fixed: 1. Command Injection via Blocklist Bypass - Implement normalized validation (lowercase, whitespace collapse) - Add regex-based pattern detection for variable expansion - Detect all shell metacharacters (;, &&, ||, |, $(), etc.) - Case-insensitive and normalized blocklist matching 2. Allowlist Validation Bypass - Detect and reject command chaining in allowlist mode - Change to exact match only (no prefix matching) - Prevent "ls; rm -rf /" style bypasses 3. Process Privilege Control - Add process group creation for better isolation - Document OS-level privilege control requirements - Use kill_on_drop for automatic cleanup HIGH Issues Fixed: 4. Zombie Process Risk on Timeout - Create new process group with process_group(0) - Kill entire process group on timeout using kill(-pid, SIGKILL) - Implement fallback to immediate child kill 5. Resource Limits - Add documentation for OS-level resource limits - Recommend systemd service limits and ulimit configuration 6. Environment Variable Security - Block dangerous environment variables (LD_PRELOAD, LD_LIBRARY_PATH, etc.) - Add runtime filtering with warning logs - Prevent library injection and environment-based attacks Implementation Details: - Enhanced validation with multiple defense layers - Comprehensive test suite with 17 passing tests - Updated default blocklist to block command categories - Added dangerous environment variable blocklist - Process group management for Unix systems - Updated dependencies: nix (process, signal features), libc Breaking Changes: - Default blocklist now blocks command names instead of full patterns - Command chaining strictly blocked when allowlist is configured - Case-insensitive command blocking (RM, Rm blocked along with rm) - Dangerous environment variables are now filtered Testing: - All 17 exec module tests pass - Comprehensive injection prevention tests added - Blocklist normalization tests - Allowlist bypass prevention tests - Environment variable filtering tests * chore: Fix formatting and dead code warning in exec handler Apply cargo fmt formatting and add #[allow(dead_code)] for test helper
1 parent cce72c6 commit 382f506

File tree

6 files changed

+1031
-10
lines changed

6 files changed

+1031
-10
lines changed

ARCHITECTURE.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ SSH server implementation using the russh library for accepting incoming connect
198198
- `config.rs` - `ServerConfig` with builder pattern for server settings
199199
- `handler.rs` - `SshHandler` implementing `russh::server::Handler` trait
200200
- `session.rs` - Session state management (`SessionManager`, `SessionInfo`, `ChannelState`)
201+
- `exec.rs` - Command execution for SSH exec requests
201202
- `auth/` - Authentication provider infrastructure
202203

203204
**Key Components**:
@@ -213,12 +214,22 @@ SSH server implementation using the russh library for accepting incoming connect
213214
- Connection limits and timeouts
214215
- Authentication method toggles (password, publickey, keyboard-interactive)
215216
- Public key authentication configuration (authorized_keys location)
217+
- Command execution configuration (shell, timeout, allowed/blocked commands)
216218

217219
- **SshHandler**: Per-connection handler for SSH protocol events
218220
- Public key authentication via AuthProvider trait
219221
- Rate limiting for authentication attempts
220222
- Channel operations (open, close, EOF, data)
221223
- PTY, exec, shell, and subsystem request handling
224+
- Command execution with stdout/stderr streaming
225+
226+
- **CommandExecutor**: Executes commands requested by SSH clients
227+
- Shell-based command execution with `-c` flag
228+
- Environment variable configuration (HOME, USER, SHELL, PATH)
229+
- Stdout/stderr streaming to SSH channel
230+
- Command timeout with graceful process termination
231+
- Command allow/block list validation for security
232+
- Exit code propagation to client
222233

223234
- **SessionManager**: Tracks active sessions with configurable capacity
224235
- Session creation and cleanup

Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ regex = "1.12.2"
4343
lazy_static = "1.5"
4444
ctrlc = "3.5.1"
4545
signal-hook = "0.4.1"
46-
nix = { version = "0.30", features = ["poll"] }
46+
nix = { version = "0.30", features = ["poll", "process", "signal"] }
4747
atty = "0.2.14"
4848
arrayvec = "0.7.6"
4949
smallvec = "1.15.1"
@@ -52,10 +52,10 @@ uuid = { version = "1.19.0", features = ["v4"] }
5252
fastrand = "2.3.0"
5353
tokio-util = "0.7.17"
5454
shell-words = "1.1.1"
55+
libc = "0.2"
5556

5657
[target.'cfg(target_os = "macos")'.dependencies]
5758
security-framework = "3.5.1"
58-
libc = "0.2"
5959

6060
[dev-dependencies]
6161
tempfile = "3.23.0"

src/server/config.rs

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use std::time::Duration;
2323
use serde::{Deserialize, Serialize};
2424

2525
use super::auth::{AuthProvider, PublicKeyAuthConfig, PublicKeyVerifier};
26+
use super::exec::ExecConfig;
2627

2728
/// Configuration for the SSH server.
2829
///
@@ -72,6 +73,10 @@ pub struct ServerConfig {
7273
/// Configuration for public key authentication.
7374
#[serde(default)]
7475
pub publickey_auth: PublicKeyAuthConfigSerde,
76+
77+
/// Configuration for command execution.
78+
#[serde(default)]
79+
pub exec: ExecConfig,
7580
}
7681

7782
/// Serializable configuration for public key authentication.
@@ -139,6 +144,7 @@ impl Default for ServerConfig {
139144
allow_keyboard_interactive: false,
140145
banner: None,
141146
publickey_auth: PublicKeyAuthConfigSerde::default(),
147+
exec: ExecConfig::default(),
142148
}
143149
}
144150
}
@@ -282,6 +288,24 @@ impl ServerConfigBuilder {
282288
self
283289
}
284290

291+
/// Set the exec configuration.
292+
pub fn exec(mut self, exec_config: ExecConfig) -> Self {
293+
self.config.exec = exec_config;
294+
self
295+
}
296+
297+
/// Set the command execution timeout in seconds.
298+
pub fn exec_timeout_secs(mut self, secs: u64) -> Self {
299+
self.config.exec.timeout_secs = secs;
300+
self
301+
}
302+
303+
/// Set the default shell for command execution.
304+
pub fn exec_shell(mut self, shell: impl Into<PathBuf>) -> Self {
305+
self.config.exec.default_shell = shell.into();
306+
self
307+
}
308+
285309
/// Build the ServerConfig.
286310
pub fn build(self) -> ServerConfig {
287311
self.config
@@ -413,4 +437,37 @@ mod tests {
413437
// Provider should be created successfully (verifies no panic)
414438
let _provider = config.create_auth_provider();
415439
}
440+
441+
#[test]
442+
fn test_exec_config_default() {
443+
let config = ServerConfig::default();
444+
assert_eq!(config.exec.default_shell, PathBuf::from("/bin/sh"));
445+
assert_eq!(config.exec.timeout_secs, 3600);
446+
}
447+
448+
#[test]
449+
fn test_builder_exec_config() {
450+
let exec_config = ExecConfig::new()
451+
.with_shell("/bin/bash")
452+
.with_timeout_secs(1800);
453+
454+
let config = ServerConfig::builder().exec(exec_config).build();
455+
456+
assert_eq!(config.exec.default_shell, PathBuf::from("/bin/bash"));
457+
assert_eq!(config.exec.timeout_secs, 1800);
458+
}
459+
460+
#[test]
461+
fn test_builder_exec_timeout() {
462+
let config = ServerConfig::builder().exec_timeout_secs(600).build();
463+
464+
assert_eq!(config.exec.timeout_secs, 600);
465+
}
466+
467+
#[test]
468+
fn test_builder_exec_shell() {
469+
let config = ServerConfig::builder().exec_shell("/bin/zsh").build();
470+
471+
assert_eq!(config.exec.default_shell, PathBuf::from("/bin/zsh"));
472+
}
416473
}

0 commit comments

Comments
 (0)