Skip to content

Commit 9a0f7e0

Browse files
authored
feat: Implement file transfer filtering infrastructure (#164)
* feat: Implement file transfer filtering infrastructure Add a complete filtering infrastructure for SFTP and SCP file transfer operations: - TransferFilter trait with check() and check_with_dest() methods - Operation enum: Upload, Download, Delete, Rename, CreateDir, ListDir, Stat, SetStat, Symlink, ReadLink - FilterResult: Allow, Deny, Log actions - FilterPolicy engine with first-match-wins rule evaluation - Matcher trait for extensible path matching Built-in matchers: - GlobMatcher: wildcard patterns (*.key, *.pem) - RegexMatcher: full regex support - PrefixMatcher: directory tree matching (/etc/*) - ExactMatcher: specific file matching - ComponentMatcher: match path components (.git, .ssh) - ExtensionMatcher: file extension matching - CombinedMatcher: OR-combine multiple matchers - NotMatcher: invert matcher results Configuration: - Extended FilterConfig with default_action, rule names, operations, and users - FilterRule supports per-user and per-operation restrictions - YAML configuration via existing config loader Closes #138 * fix(quality): resolve clippy warnings in filter module Priority: MEDIUM Issue: Three clippy warnings causing build failures with -D warnings - Derive Default instead of manual impl for FilterResult - Remove needless borrow in rule_from_config() - Rename CombinedMatcher::add() to with_matcher() to avoid confusion with std::ops::Add Review-Iteration: 1 * fix(security): add ReDoS protection to RegexMatcher Priority: HIGH Issue: RegexMatcher accepted arbitrary regex patterns without complexity limits, allowing potential CPU exhaustion via crafted patterns - Use RegexBuilder with size_limit and dfa_size_limit (1MB default) - Add with_size_limit() constructor for custom limits - Add test for size limit functionality Review-Iteration: 1 * fix(security): add GlobMatchMode for explicit control over glob matching Priority: HIGH Issue: GlobMatcher tried both full path and filename matching, which could lead to unintended matches in security-sensitive filtering - Add GlobMatchMode enum (PathOrFilename, FullPathOnly, FilenameOnly) - Add with_mode() constructor for explicit control - Document matching behavior and security implications - Add tests for each match mode Review-Iteration: 1 * fix(security): add normalize_path for path traversal protection Priority: CRITICAL Issue: Path matchers operated on raw paths without normalization, allowing bypass via path traversal sequences like ../ - Add normalize_path() function for logical path normalization - Add comprehensive security documentation - Document that callers should normalize paths before matching - Add tests demonstrating the security issue and fix Review-Iteration: 1 * test: add comprehensive test coverage for filter module - Add tests for FilterPolicy.from_config() with glob patterns and prefixes - Add tests for FilterPolicy accessor methods (rule_count, default_action) - Add tests for FilterRule.matches() with all conditions - Add tests for SharedFilterPolicy (check_with_dest, From impl) - Add tests for Operation.all() and additional parse/display tests - Add tests for matcher accessor methods (prefix, path, component, extension) - Add tests for GlobMatchMode enum and CombinedMatcher len/is_empty - Fix normalize_path tests placement (move inside test module) - Update ARCHITECTURE.md with File Transfer Filter module documentation
1 parent 4147b29 commit 9a0f7e0

7 files changed

Lines changed: 2724 additions & 1 deletion

File tree

ARCHITECTURE.md

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,107 @@ Security features for the SSH server (`src/server/security/`):
213213
- Thread-safe with fail-closed behavior on lock contention
214214
- Configuration via `allowed_ips` and `blocked_ips` in server config
215215

216+
### File Transfer Filter Module
217+
218+
Policy-based filtering infrastructure for SFTP and SCP file transfer operations (`src/server/filter/`):
219+
220+
**Structure**:
221+
- `mod.rs` - `TransferFilter` trait, `Operation` enum, `FilterResult` enum, `NoOpFilter`
222+
- `policy.rs` - `FilterPolicy` engine, `FilterRule`, `Matcher` trait, `SharedFilterPolicy`
223+
- `path.rs` - Path-based matchers: `PrefixMatcher`, `ExactMatcher`, `ComponentMatcher`, `ExtensionMatcher`
224+
- `pattern.rs` - Pattern-based matchers: `GlobMatcher`, `RegexMatcher`, `CombinedMatcher`, `NotMatcher`
225+
226+
**Key Components**:
227+
228+
- **Operation**: Enum representing file operations
229+
- `Upload`, `Download`, `Delete`, `Rename`
230+
- `CreateDir`, `ListDir`, `Stat`, `SetStat`
231+
- `Symlink`, `ReadLink`
232+
233+
- **FilterResult**: Actions to take on matched operations
234+
- `Allow` - Permit the operation (default)
235+
- `Deny` - Block the operation
236+
- `Log` - Allow but log for auditing
237+
238+
- **TransferFilter Trait**: Interface for custom filter implementations
239+
- `check(path, operation, user)` - Check single path operations
240+
- `check_with_dest(src, dest, operation, user)` - Check two-path operations (rename, symlink)
241+
- `is_enabled()` - Check if filtering is active
242+
243+
- **FilterPolicy**: First-match-wins rule evaluation engine
244+
- Ordered rule evaluation
245+
- Configurable default action
246+
- Enable/disable filtering
247+
- Create from YAML configuration via `from_config()`
248+
249+
- **FilterRule**: Combines matcher, action, and optional constraints
250+
- Path pattern matcher
251+
- Per-operation restrictions
252+
- Per-user restrictions
253+
- Named rules for debugging
254+
255+
**Built-in Matchers**:
256+
257+
| Matcher | Purpose | Example |
258+
|---------|---------|---------|
259+
| `GlobMatcher` | Wildcard patterns | `*.key`, `*.pem` |
260+
| `RegexMatcher` | Full regex support | `(?i)\.exe$` |
261+
| `PrefixMatcher` | Directory tree matching | `/etc/` |
262+
| `ExactMatcher` | Specific file matching | `/etc/shadow` |
263+
| `ComponentMatcher` | Path component matching | `.git`, `.ssh` |
264+
| `ExtensionMatcher` | File extension matching | `exe`, `key` |
265+
| `CombinedMatcher` | OR-combine matchers | Multiple patterns |
266+
| `NotMatcher` | Invert matcher results | Exclude patterns |
267+
268+
**Security Features**:
269+
- `normalize_path()` function for path traversal prevention
270+
- ReDoS protection via regex size limits
271+
- Case-insensitive extension matching
272+
273+
**Usage Example**:
274+
```rust
275+
use bssh::server::filter::{FilterPolicy, FilterResult, Operation};
276+
use bssh::server::filter::pattern::GlobMatcher;
277+
use bssh::server::filter::policy::FilterRule;
278+
use std::path::Path;
279+
280+
// Create policy that blocks *.key files
281+
let policy = FilterPolicy::new()
282+
.with_default(FilterResult::Allow)
283+
.add_rule(FilterRule::new(
284+
Box::new(GlobMatcher::new("*.key").unwrap()),
285+
FilterResult::Deny,
286+
));
287+
288+
// Check if operation is allowed
289+
let result = policy.check(
290+
Path::new("/etc/secret.key"),
291+
Operation::Download,
292+
"alice"
293+
);
294+
assert_eq!(result, FilterResult::Deny);
295+
```
296+
297+
**Configuration** (YAML):
298+
```yaml
299+
filter:
300+
enabled: true
301+
default_action: allow
302+
rules:
303+
- name: block-sensitive-keys
304+
pattern: "*.{key,pem}"
305+
action: deny
306+
operations:
307+
- download
308+
- upload
309+
- name: block-hidden-dirs
310+
path_prefix: "/home"
311+
pattern: ".*"
312+
action: deny
313+
users:
314+
- guest
315+
```
316+
216317
### Audit Logging Module
217318
218319
Comprehensive audit logging infrastructure for the SSH server (`src/server/audit/`):

src/server/config/types.rs

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,12 @@ pub struct FilterConfig {
275275
#[serde(default)]
276276
pub enabled: bool,
277277

278+
/// Default action when no rules match.
279+
///
280+
/// Default: allow
281+
#[serde(default)]
282+
pub default_action: Option<FilterAction>,
283+
278284
/// Filter rules to apply.
279285
///
280286
/// Rules are evaluated in order. First matching rule determines action.
@@ -285,25 +291,47 @@ pub struct FilterConfig {
285291
/// A single file transfer filter rule.
286292
#[derive(Debug, Clone, Deserialize, Serialize)]
287293
pub struct FilterRule {
294+
/// Rule name (for logging and debugging).
295+
///
296+
/// Example: "block-keys"
297+
#[serde(default)]
298+
pub name: Option<String>,
299+
288300
/// Glob pattern to match against file paths.
289301
///
290302
/// Example: "*.exe" matches all executable files
303+
#[serde(default)]
291304
pub pattern: Option<String>,
292305

293306
/// Path prefix to match.
294307
///
295308
/// Example: "/tmp/" matches all files in /tmp
309+
#[serde(default)]
296310
pub path_prefix: Option<String>,
297311

298312
/// Action to take when rule matches.
299313
pub action: FilterAction,
314+
315+
/// Operations this rule applies to.
316+
///
317+
/// If not specified, the rule applies to all operations.
318+
/// Valid values: upload, download, delete, rename, createdir, listdir
319+
#[serde(default)]
320+
pub operations: Option<Vec<String>>,
321+
322+
/// Users this rule applies to.
323+
///
324+
/// If not specified, the rule applies to all users.
325+
#[serde(default)]
326+
pub users: Option<Vec<String>>,
300327
}
301328

302329
/// Action to take when a filter rule matches.
303-
#[derive(Debug, Clone, Deserialize, Serialize)]
330+
#[derive(Debug, Clone, Deserialize, Serialize, Default)]
304331
#[serde(rename_all = "lowercase")]
305332
pub enum FilterAction {
306333
/// Allow the file transfer.
334+
#[default]
307335
Allow,
308336

309337
/// Deny the file transfer.

0 commit comments

Comments
 (0)