Skip to content

Commit c3251de

Browse files
committed
chore: finalize PR with lint fixes and documentation
- Fix clippy while_let_loop warning in SCP handler - Apply cargo fmt formatting - Add SCP handler documentation to ARCHITECTURE.md - Add SCP protocol handler section to server-configuration.md - Update architecture README to reference SCP handler
1 parent 3f751a3 commit c3251de

5 files changed

Lines changed: 164 additions & 27 deletions

File tree

ARCHITECTURE.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,7 @@ SSH server implementation using the russh library for accepting incoming connect
249249
- `session.rs` - Session state management (`SessionManager`, `SessionInfo`, `ChannelState`)
250250
- `exec.rs` - Command execution for SSH exec requests
251251
- `sftp.rs` - SFTP subsystem handler with path traversal prevention
252+
- `scp.rs` - SCP protocol handler with sink/source modes
252253
- `auth/` - Authentication provider infrastructure
253254

254255
**Key Components**:
@@ -328,6 +329,22 @@ SSH server implementation using the russh library for accepting incoming connect
328329
- Handle limit enforcement to prevent resource exhaustion
329330
- Read size capping to prevent memory exhaustion
330331

332+
- **ScpHandler**: SCP protocol handler (`src/server/scp.rs`)
333+
- Implements SCP server protocol for file transfers via the `scp` command
334+
- Sink mode (`-t` flag): receives files from client (upload)
335+
- Source mode (`-f` flag): sends files to client (download)
336+
- Recursive transfer support (`-r` flag) for directories
337+
- Time preservation (`-p` flag) for file modification times
338+
- Security features:
339+
- Path traversal prevention with normalized path resolution
340+
- Symlink escape prevention via canonicalization
341+
- Filename validation (rejects `/`, `..`, `.`)
342+
- File size limit (10 GB maximum)
343+
- Mode permission masking (strips setuid/setgid/sticky bits)
344+
- Line length limits to prevent DoS via buffer exhaustion
345+
- Automatic SCP command detection in exec_request handler
346+
- Configurable via `scp_enabled` setting
347+
331348
### Server Authentication Module
332349

333350
The authentication subsystem (`src/server/auth/`) provides extensible authentication for the SSH server:

docs/architecture/README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,12 @@ bssh is a high-performance parallel SSH command execution tool with SSH-compatib
3232

3333
### Server Components
3434

35-
- **[Server Configuration](./server-configuration.md)** - YAML-based server configuration, environment overrides, validation
35+
- **[Server Configuration](./server-configuration.md)** - YAML-based server configuration, environment overrides, validation, SCP protocol handler
3636
- **Server CLI (`bssh-server`)** - Server management commands including host key generation, password hashing, config validation (see main ARCHITECTURE.md)
3737
- **SSH Server Module** - SSH server implementation using russh (see main ARCHITECTURE.md)
3838
- **Server Authentication** - Authentication providers including public key verification (see main ARCHITECTURE.md)
3939
- **SFTP Handler** - SFTP subsystem with path traversal prevention and chroot-like isolation (see main ARCHITECTURE.md)
40+
- **SCP Handler** - SCP protocol with sink/source modes and security controls (see main ARCHITECTURE.md)
4041

4142
## Navigation
4243

docs/architecture/server-configuration.md

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,120 @@ SSH Client Request Flow:
486486
└───────────────────┘ └─────────────────┘ └──────────────┘
487487
```
488488

489+
## SCP Protocol Handler
490+
491+
The bssh-server supports file transfers via the SCP (Secure Copy Protocol) command. Unlike SFTP which uses a dedicated subsystem, SCP operates through SSH exec requests.
492+
493+
### Protocol Overview
494+
495+
SCP is not a standalone protocol but a command-line tool that communicates over SSH. When a client runs `scp file user@host:path`:
496+
1. The SSH client establishes a connection to the server
497+
2. The server receives an exec request for `scp -t path` (upload) or `scp -f path` (download)
498+
3. The server spawns the SCP handler to manage the file transfer
499+
500+
### Operation Modes
501+
502+
**Sink Mode (`-t` flag)**: Server receives files from client (upload)
503+
```bash
504+
# Client uploads file.txt to server's /tmp directory
505+
scp file.txt user@server:/tmp/
506+
```
507+
508+
**Source Mode (`-f` flag)**: Server sends files to client (download)
509+
```bash
510+
# Client downloads file.txt from server
511+
scp user@server:/home/user/file.txt ./
512+
```
513+
514+
### SCP Command Flags
515+
516+
| Flag | Description |
517+
|------|-------------|
518+
| `-t` | Sink mode (target/upload) |
519+
| `-f` | Source mode (from/download) |
520+
| `-r` | Recursive transfer for directories |
521+
| `-p` | Preserve file modification times |
522+
| `-d` | Target is expected to be a directory |
523+
| `-v` | Verbose mode |
524+
525+
### Security Features
526+
527+
The SCP handler implements multiple security measures:
528+
529+
**Path Traversal Prevention:**
530+
- All paths are normalized before processing
531+
- `..` components are resolved without escaping the root directory
532+
- Absolute paths are stripped and joined with the user's root directory
533+
534+
**Symlink Escape Prevention:**
535+
- Existing paths are canonicalized to resolve symlinks
536+
- If the canonical path is outside the root directory, access is denied
537+
- Symlinks in recursive transfers are skipped for security
538+
539+
**Input Validation:**
540+
- Filenames cannot contain `/`, `..`, or `.`
541+
- File size is limited to 10 GB maximum
542+
- Permission mode bits are masked to strip setuid/setgid/sticky bits (only 0o777 allowed)
543+
- Protocol line length is limited to prevent DoS via buffer exhaustion
544+
545+
### Configuration
546+
547+
SCP is enabled by default. To disable it:
548+
549+
**YAML Configuration:**
550+
```yaml
551+
scp:
552+
enabled: false
553+
```
554+
555+
**Builder API:**
556+
```rust
557+
let config = ServerConfig::builder()
558+
.scp_enabled(false)
559+
.build();
560+
```
561+
562+
### Handler Architecture
563+
564+
```
565+
SCP Request Flow:
566+
┌───────────────┐ ┌──────────────────┐ ┌────────────────┐
567+
│ exec_request │ --> │ Parse SCP cmd │ --> │ Create Handler │
568+
│ "scp -t /tmp"│ │ mode, path, flags│ │ with root_dir │
569+
└───────────────┘ └──────────────────┘ └────────────────┘
570+
571+
v
572+
┌───────────────┐ ┌──────────────────┐ ┌────────────────┐
573+
│ Spawn task │ --> │ SCP I/O loop │ --> │ File transfer │
574+
│ (async) │ │ protocol messages│ │ operations │
575+
└───────────────┘ └──────────────────┘ └────────────────┘
576+
577+
v
578+
┌───────────────┐ ┌──────────────────┐ ┌────────────────┐
579+
│ Send status │ --> │ EOF & close │ --> │ Channel done │
580+
│ exit code │ │ channel │ │ │
581+
└───────────────┘ └──────────────────┘ └────────────────┘
582+
```
583+
584+
### Usage Examples
585+
586+
```bash
587+
# Upload a single file
588+
scp local_file.txt user@bssh-server:/home/user/
589+
590+
# Download a file
591+
scp user@bssh-server:/home/user/file.txt ./
592+
593+
# Recursive directory upload
594+
scp -r ./project/ user@bssh-server:/home/user/projects/
595+
596+
# Preserve timestamps
597+
scp -p important.doc user@bssh-server:/backup/
598+
599+
# Recursive with timestamps
600+
scp -rp ./data/ user@bssh-server:/storage/backup/
601+
```
602+
489603
---
490604

491605
**Related Documentation:**

src/server/handler.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -732,7 +732,9 @@ impl russh::server::Handler for SshHandler {
732732
// The data() handler will forward data to shell_data_tx which the SCP handler
733733
// will receive via data_rx
734734
tokio::spawn(async move {
735-
let exit_code = scp_handler.run(channel_id, handle_clone.clone(), data_rx).await;
735+
let exit_code = scp_handler
736+
.run(channel_id, handle_clone.clone(), data_rx)
737+
.await;
736738

737739
// Send exit status, EOF, and close channel
738740
let _ = handle_clone

src/server/scp.rs

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -257,11 +257,7 @@ impl ScpHandler {
257257
}
258258

259259
/// Create a handler from a parsed SCP command.
260-
pub fn from_command(
261-
cmd: &ScpCommand,
262-
user_info: UserInfo,
263-
root_dir: Option<PathBuf>,
264-
) -> Self {
260+
pub fn from_command(cmd: &ScpCommand, user_info: UserInfo, root_dir: Option<PathBuf>) -> Self {
265261
let mut handler = Self::new(cmd.mode, cmd.path.clone(), user_info, root_dir);
266262
handler.recursive = cmd.recursive;
267263
handler.preserve_times = cmd.preserve_times;
@@ -463,7 +459,14 @@ impl ScpHandler {
463459
b'C' => {
464460
// File: C<mode> <size> <filename>
465461
if let Err(e) = self
466-
.receive_file(&line, &current_dir, channel_id, &handle, &mut buffer, data_rx)
462+
.receive_file(
463+
&line,
464+
&current_dir,
465+
channel_id,
466+
&handle,
467+
&mut buffer,
468+
data_rx,
469+
)
467470
.await
468471
{
469472
tracing::error!("Error receiving file: {}", e);
@@ -653,24 +656,19 @@ impl ScpHandler {
653656
buffer.remove(0);
654657
} else {
655658
// Wait for the null byte
656-
loop {
657-
match data_rx.recv().await {
658-
Some(data) => {
659-
if !data.is_empty() {
660-
if data[0] == 0 {
661-
// Store any remaining data
662-
if data.len() > 1 {
663-
buffer.extend_from_slice(&data[1..]);
664-
}
665-
break;
666-
} else {
667-
// Unexpected data
668-
buffer.extend_from_slice(&data);
669-
}
670-
}
659+
while let Some(data) = data_rx.recv().await {
660+
if data.is_empty() {
661+
continue;
662+
}
663+
if data[0] == 0 {
664+
// Store any remaining data
665+
if data.len() > 1 {
666+
buffer.extend_from_slice(&data[1..]);
671667
}
672-
None => break,
668+
break;
673669
}
670+
// Unexpected data
671+
buffer.extend_from_slice(&data);
674672
}
675673
}
676674

@@ -782,7 +780,8 @@ impl ScpHandler {
782780
anyhow::bail!("Source is a directory but recursive mode not enabled");
783781
}
784782
} else if metadata.is_file() {
785-
self.send_file(channel_id, &handle, &source, data_rx).await?;
783+
self.send_file(channel_id, &handle, &source, data_rx)
784+
.await?;
786785
} else {
787786
self.send_error(channel_id, &handle, "Not a regular file")
788787
.await?;
@@ -1138,7 +1137,9 @@ mod tests {
11381137
Some(PathBuf::from("/home/testuser")),
11391138
);
11401139

1141-
let result = handler.resolve_path(Path::new("documents/file.txt")).unwrap();
1140+
let result = handler
1141+
.resolve_path(Path::new("documents/file.txt"))
1142+
.unwrap();
11421143
assert_eq!(result, PathBuf::from("/home/testuser/documents/file.txt"));
11431144
}
11441145

@@ -1152,7 +1153,9 @@ mod tests {
11521153
Some(PathBuf::from("/home/testuser")),
11531154
);
11541155

1155-
let result = handler.resolve_path(Path::new("/documents/file.txt")).unwrap();
1156+
let result = handler
1157+
.resolve_path(Path::new("/documents/file.txt"))
1158+
.unwrap();
11561159
assert_eq!(result, PathBuf::from("/home/testuser/documents/file.txt"));
11571160
}
11581161

0 commit comments

Comments
 (0)