Conversation
Implement a hierarchical configuration system supporting YAML files, environment variables, and CLI argument overrides. - Add ServerFileConfig with comprehensive configuration types - Support YAML config files with default search paths - Implement environment variable overrides (BSSH_* prefix) - Add configuration validation at startup - Generate config template function - Maintain backward compatibility with existing ServerConfig API - Add ipnetwork dependency for CIDR validation - Include 25 comprehensive tests covering all features
This commit addresses HIGH and key MEDIUM severity security issues in the server configuration validation: HIGH severity fixes: - Path traversal validation for authorized_keys_pattern: Reject patterns containing ".." and enforce absolute paths to prevent directory traversal - IP address validation for bind_address: Validate that bind_address is a valid IPv4 or IPv6 address before use MEDIUM severity fixes: - Shell path validation: Verify that the configured default shell exists - Config file permissions check: On Unix systems, warn if configuration file is readable by group or others (mode & 0o077 != 0) - Align max_auth_attempts default: Changed ServerConfig.max_auth_attempts default from 6 to 5 to match SecurityConfig.max_auth_attempts All validations include comprehensive error messages and are covered by extensive unit tests. Tests verify both valid and invalid configurations, including edge cases for path traversal attempts and IP address formats.
- Add comprehensive server-configuration.md documentation - Update ARCHITECTURE.md with server config system details - Update docs/architecture/README.md with server config links - Apply cargo fmt formatting to loader.rs tests Relates to #130
Member
Author
PR Finalization ReportProject Structure Discovered
ChecklistTests
Documentation
Code Quality
Changes Made
Notes
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements #130 - Create a comprehensive configuration system for bssh-server with YAML files, environment variables, and CLI argument overrides.
Implementation Details
New Configuration System
File Structure:
src/server/config/mod.rs- Module exports and backward compatibility layersrc/server/config/types.rs- Comprehensive configuration types with serdesrc/server/config/loader.rs- Config loader with validation and env overridesKey Features:
Configuration Types
The new
ServerFileConfigincludes:Environment Variables
Supported environment variables:
BSSH_PORT- Server portBSSH_BIND_ADDRESS- Bind addressBSSH_HOST_KEY- Comma-separated host key pathsBSSH_MAX_CONNECTIONS- Maximum concurrent connectionsBSSH_KEEPALIVE_INTERVAL- Keepalive interval in secondsBSSH_AUTH_METHODS- Comma-separated auth methodsBSSH_AUTHORIZED_KEYS_DIR- Directory for authorized_keysBSSH_AUTHORIZED_KEYS_PATTERN- Pattern for authorized_keys pathsBSSH_SHELL- Default shell pathBSSH_COMMAND_TIMEOUT- Command timeout in secondsDependencies
Added
ipnetwork = "0.20"for CIDR notation validation.Testing
Backward Compatibility
The existing
ServerConfigandServerConfigBuilderAPIs remain unchanged. The new system adds:ServerFileConfigfor YAML-based configurationload_config()function to load from files/envgenerate_config_template()for generating config templatesinto_server_config()method to convert file config to builder configExisting code continues to work without changes.
Example Usage
Related Issues