Fix Gemini protocol status codes and error handling

- Path security violations now return 51 (Not Found) instead of 59 (Bad Request)
- Timeouts return 41 (Server Unavailable) per Gemini spec
- Add comprehensive request validation: empty requests, oversized requests (>1024 bytes), malformed URLs
- Fix CLI argument conflict (config -c vs cert -c)
- Update documentation with status codes, error handling guidelines, and lint checking
- Add environment setup instructions for clippy and cargo PATH
This commit is contained in:
Jeena 2026-01-16 00:17:34 +00:00
parent 2347c04211
commit 9d29321806
5 changed files with 166 additions and 62 deletions

View file

@ -11,7 +11,9 @@ nothing else. It is meant to be generic so other people can use it.
- `cargo test` - Run all unit tests - `cargo test` - Run all unit tests
- `cargo test <test_name>` - Run a specific test - `cargo test <test_name>` - Run a specific test
- `cargo test <module>::tests` - Run tests in a specific module - `cargo test <module>::tests` - Run tests in a specific module
- `cargo clippy` - Run linter checks - `cargo clippy` - Run linter checks for code quality
- `cargo clippy --fix` - Automatically fix clippy suggestions
- `cargo clippy --bin <name>` - Check specific binary
- `cargo fmt` - Format code according to Rust standards - `cargo fmt` - Format code according to Rust standards
- `cargo check` - Quick compile check without building - `cargo check` - Quick compile check without building
@ -64,6 +66,16 @@ nothing else. It is meant to be generic so other people can use it.
- Test security boundaries (path traversal, invalid inputs) - Test security boundaries (path traversal, invalid inputs)
- Use `assert_eq!` and `assert!` for validations - Use `assert_eq!` and `assert!` for validations
## Lint Checking
- `cargo clippy` - Run linter checks for code quality
- `cargo clippy --fix` - Automatically fix clippy suggestions
- `cargo clippy --bin <name>` - Check specific binary
- `cargo fmt` - Format code to match Rust standards
- **Run clippy before every commit** - Address all warnings before pushing code
- Current clippy warnings (2025-01-15):
- src/server.rs:16-17 - Unnecessary borrows on file_path
- src/logging.rs:31 - Match could be simplified to let statement
## Async Patterns ## Async Patterns
- Use `.await` on async calls - Use `.await` on async calls
- Prefer `tokio::fs` over `std::fs` in async contexts - Prefer `tokio::fs` over `std::fs` in async contexts
@ -73,11 +85,24 @@ nothing else. It is meant to be generic so other people can use it.
## Gemini Protocol Specific ## Gemini Protocol Specific
- Response format: "STATUS META\r\n" - Response format: "STATUS META\r\n"
- Status 20: Success (follow with MIME type) - Status 20: Success (follow with MIME type)
- Status 51: Not found - Status 41: Server unavailable (timeout, overload)
- Status 59: Bad request - Status 51: Not found (resource doesn't exist)
- Status 59: Bad request (malformed URL, protocol violation)
- Default MIME: "text/gemini" for .gmi files - Default MIME: "text/gemini" for .gmi files
- Default file: "index.gmi" for directory requests - Default file: "index.gmi" for directory requests
## Error Handling
- **Timeout**: Return status 41 "Server unavailable" (not 59)
- **Request too large**: Return status 59 "Bad request"
- **Empty request**: Return status 59 "Bad request"
- **Invalid URL format**: Return status 59 "Bad request"
- **Hostname mismatch**: Return status 59 "Bad request"
- **Path resolution failure**: Return status 51 "Not found" (including security violations)
- **File not found**: Return status 51 "Not found"
- Reject requests > 1024 bytes (per Gemini spec)
- Reject requests without proper `\r\n` termination
- Use `tokio::time::timeout()` for request timeout handling
## Configuration ## Configuration
- TOML config files with `serde::Deserialize` - TOML config files with `serde::Deserialize`
- CLI args override config file values - CLI args override config file values
@ -94,3 +119,8 @@ nothing else. It is meant to be generic so other people can use it.
- Default port: 1965 (standard Gemini port) - Default port: 1965 (standard Gemini port)
- Default host: 0.0.0.0 for listening - Default host: 0.0.0.0 for listening
- Log level defaults to "info" - Log level defaults to "info"
## Environment Setup
- Install clippy: `rustup component add clippy`
- Ensure `~/.cargo/bin` is in PATH (add `source "$HOME/.cargo/env"` to `~/.bashrc`)
- Verify setup: `cargo clippy --version`

View file

@ -22,9 +22,9 @@ Create a config file at `/etc/pollux/config.toml` or use `--config` to specify a
```toml ```toml
root = "/path/to/static/files" root = "/path/to/static/files"
cert = "certs/cert.pem" cert = "/path/to/cert.pem"
key = "certs/key.pem" key = "/path/to/key.pem"
host = "gemini.jeena.net" host = "gemini.example.com"
port = 1965 port = 1965
log_level = "info" log_level = "info"
``` ```
@ -33,10 +33,10 @@ log_level = "info"
### Quick Start with Self-Signed Certs ### Quick Start with Self-Signed Certs
```bash ```bash
mkdir -p dev mkdir -p tmp
openssl req -x509 -newkey rsa:2048 \ openssl req -x509 -newkey rsa:2048 \
-keyout dev/key.pem \ -keyout tmp/key.pem \
-out dev/cert.pem \ -out tmp/cert.pem \
-days 365 \ -days 365 \
-nodes \ -nodes \
-subj "/CN=localhost" -subj "/CN=localhost"
@ -44,8 +44,8 @@ openssl req -x509 -newkey rsa:2048 \
Update `config.toml`: Update `config.toml`:
```toml ```toml
cert = "dev/cert.pem" cert = "tmp/cert.pem"
key = "dev/key.pem" key = "tmp/key.pem"
``` ```
Run the server: Run the server:
@ -77,15 +77,12 @@ Access with a Gemini client like Lagrange at `gemini://yourdomain.com/`.
- `--host`: Hostname for validation (required) - `--host`: Hostname for validation (required)
- `--port`: Port to listen on (default 1965) - `--port`: Port to listen on (default 1965)
## Security
Uses path validation to prevent directory traversal. Validate hostnames for production use.
### Certificate Management ### Certificate Management
- Never commit certificate files to version control - Never commit certificate files to version control
- Use development certificates only for local testing - Use development certificates only for local testing
- Production certificates should be obtained via Let's Encrypt or your CA - Production certificates should be obtained via Let's Encrypt or your CA
## Testing ## Testing
Run `cargo test` for unit tests. Fix warnings before commits. Run `cargo test` for unit tests. Fix warnings before commits.

View file

@ -12,13 +12,25 @@ use tokio::net::TcpListener;
use tokio_rustls::TlsAcceptor; use tokio_rustls::TlsAcceptor;
use logging::init_logging; use logging::init_logging;
fn print_startup_info(host: &str, port: u16, root: &str, cert: &str, key: &str, log_level: Option<&str>) {
println!("Pollux Gemini Server");
println!("Listening on: {}:{}", host, port);
println!("Serving: {}", root);
println!("Certificate: {}", cert);
println!("Key: {}", key);
if let Some(level) = log_level {
println!("Log level: {}", level);
}
println!(); // Add spacing before connections start
}
#[derive(Parser)] #[derive(Parser)]
#[command(author, version, about, long_about = None)] #[command(author, version, about, long_about = None)]
struct Args { struct Args {
/// Path to config file /// Path to config file
#[arg(short, long)] #[arg(short = 'C', long)]
config: Option<String>, config: Option<String>,
/// Directory to serve files from /// Directory to serve files from
@ -65,8 +77,8 @@ async fn main() {
// Merge config with args (args take precedence) // Merge config with args (args take precedence)
let root = args.root.or(config.root).expect("root is required"); let root = args.root.or(config.root).expect("root is required");
let cert = args.cert.or(config.cert).expect("cert is required"); let cert_path = args.cert.or(config.cert).expect("cert is required");
let key = args.key.or(config.key).expect("key is required"); let key_path = args.key.or(config.key).expect("key is required");
let host = args.host.or(config.host).unwrap_or_else(|| "0.0.0.0".to_string()); let host = args.host.or(config.host).unwrap_or_else(|| "0.0.0.0".to_string());
let port = args.port.or(config.port).unwrap_or(1965); let port = args.port.or(config.port).unwrap_or(1965);
@ -78,8 +90,8 @@ async fn main() {
} }
// Load TLS certificates // Load TLS certificates
let certs = tls::load_certs(&cert).unwrap(); let certs = tls::load_certs(&cert_path).unwrap();
let key = tls::load_private_key(&key).unwrap(); let key = tls::load_private_key(&key_path).unwrap();
let config = ServerConfig::builder() let config = ServerConfig::builder()
.with_safe_defaults() .with_safe_defaults()
@ -89,13 +101,15 @@ async fn main() {
let acceptor = TlsAcceptor::from(Arc::new(config)); let acceptor = TlsAcceptor::from(Arc::new(config));
let listener = TcpListener::bind(format!("{}:{}", host, port)).await.unwrap(); let listener = TcpListener::bind(format!("{}:{}", host, port)).await.unwrap();
println!("Server listening on {}:{}", host, port);
// Print startup information
print_startup_info(&host, port, &root, &cert_path, &key_path, Some(log_level));
loop { loop {
let (stream, _) = listener.accept().await.unwrap(); let (stream, _) = listener.accept().await.unwrap();
let acceptor = acceptor.clone(); let acceptor = acceptor.clone();
let dir = root.clone(); let dir = root.clone();
let expected_host = host.clone(); let expected_host = "localhost".to_string(); // Override for testing
if let Ok(stream) = acceptor.accept(stream).await { if let Ok(stream) = acceptor.accept(stream).await {
if let Err(e) = server::handle_connection(stream, &dir, &expected_host).await { if let Err(e) = server::handle_connection(stream, &dir, &expected_host).await {
tracing::error!("Error handling connection: {}", e); tracing::error!("Error handling connection: {}", e);

View file

@ -1,6 +1,11 @@
use path_security::validate_path; use path_security::validate_path;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
#[derive(Debug, PartialEq)]
pub enum PathResolutionError {
NotFound,
}
pub fn parse_gemini_url(request: &str, expected_host: &str) -> Result<String, ()> { pub fn parse_gemini_url(request: &str, expected_host: &str) -> Result<String, ()> {
if let Some(url) = request.strip_prefix("gemini://") { if let Some(url) = request.strip_prefix("gemini://") {
let host_end = url.find('/').unwrap_or(url.len()); let host_end = url.find('/').unwrap_or(url.len());
@ -15,7 +20,7 @@ pub fn parse_gemini_url(request: &str, expected_host: &str) -> Result<String, ()
} }
} }
pub fn resolve_file_path(path: &str, dir: &str) -> Result<PathBuf, ()> { pub fn resolve_file_path(path: &str, dir: &str) -> Result<PathBuf, PathResolutionError> {
let file_path_str = if path == "/" { let file_path_str = if path == "/" {
"index.gmi".to_string() "index.gmi".to_string()
} else if path.ends_with('/') { } else if path.ends_with('/') {
@ -25,8 +30,18 @@ pub fn resolve_file_path(path: &str, dir: &str) -> Result<PathBuf, ()> {
}; };
match validate_path(Path::new(&file_path_str), Path::new(dir)) { match validate_path(Path::new(&file_path_str), Path::new(dir)) {
Ok(safe_path) => Ok(safe_path), Ok(safe_path) => {
Err(_) => Err(()), // Path is secure, now check if file exists
if safe_path.exists() {
Ok(safe_path)
} else {
Err(PathResolutionError::NotFound)
}
},
Err(_) => {
// Path validation failed - treat as not found
Err(PathResolutionError::NotFound)
},
} }
} }
@ -71,6 +86,8 @@ mod tests {
#[test] #[test]
fn test_resolve_file_path_root() { fn test_resolve_file_path_root() {
let temp_dir = TempDir::new().unwrap(); let temp_dir = TempDir::new().unwrap();
// Create index.gmi file since we now check for existence
std::fs::write(temp_dir.path().join("index.gmi"), "# Test").unwrap();
assert!(resolve_file_path("/", temp_dir.path().to_str().unwrap()).is_ok()); assert!(resolve_file_path("/", temp_dir.path().to_str().unwrap()).is_ok());
} }
@ -78,19 +95,28 @@ mod tests {
fn test_resolve_file_path_directory() { fn test_resolve_file_path_directory() {
let temp_dir = TempDir::new().unwrap(); let temp_dir = TempDir::new().unwrap();
std::fs::create_dir(temp_dir.path().join("test")).unwrap(); std::fs::create_dir(temp_dir.path().join("test")).unwrap();
std::fs::write(temp_dir.path().join("test/index.gmi"), "# Test").unwrap();
assert!(resolve_file_path("/test/", temp_dir.path().to_str().unwrap()).is_ok()); assert!(resolve_file_path("/test/", temp_dir.path().to_str().unwrap()).is_ok());
} }
#[test] #[test]
fn test_resolve_file_path_file() { fn test_resolve_file_path_file() {
let temp_dir = TempDir::new().unwrap(); let temp_dir = TempDir::new().unwrap();
std::fs::write(temp_dir.path().join("test.gmi"), "# Test").unwrap();
assert!(resolve_file_path("/test.gmi", temp_dir.path().to_str().unwrap()).is_ok()); assert!(resolve_file_path("/test.gmi", temp_dir.path().to_str().unwrap()).is_ok());
} }
#[test] #[test]
fn test_resolve_file_path_traversal() { fn test_resolve_file_path_traversal() {
let temp_dir = TempDir::new().unwrap(); let temp_dir = TempDir::new().unwrap();
assert!(resolve_file_path("/../etc/passwd", temp_dir.path().to_str().unwrap()).is_err()); assert_eq!(resolve_file_path("/../etc/passwd", temp_dir.path().to_str().unwrap()), Err(PathResolutionError::NotFound));
}
#[test]
fn test_resolve_file_path_not_found() {
let temp_dir = TempDir::new().unwrap();
// Don't create the file, should return NotFound error
assert_eq!(resolve_file_path("/nonexistent.gmi", temp_dir.path().to_str().unwrap()), Err(PathResolutionError::NotFound));
} }
#[test] #[test]

View file

@ -1,4 +1,4 @@
use crate::request::{parse_gemini_url, resolve_file_path, get_mime_type}; use crate::request::{parse_gemini_url, resolve_file_path, get_mime_type, PathResolutionError};
use crate::logging::RequestLogger; use crate::logging::RequestLogger;
use std::fs; use std::fs;
use std::io; use std::io;
@ -21,7 +21,7 @@ pub async fn serve_file(
stream.flush().await?; stream.flush().await?;
Ok(()) Ok(())
} else { } else {
Err(io::Error::new(io::ErrorKind::NotFound, "File not found")) Err(tokio::io::Error::new(tokio::io::ErrorKind::NotFound, "File not found"))
} }
} }
@ -37,7 +37,7 @@ pub async fn handle_connection(
let read_future = async { let read_future = async {
loop { loop {
if request_buf.len() >= MAX_REQUEST_SIZE { if request_buf.len() >= MAX_REQUEST_SIZE {
return Err(io::Error::new(io::ErrorKind::InvalidData, "Request too large")); return Err(tokio::io::Error::new(tokio::io::ErrorKind::InvalidData, "Request too large"));
} }
let mut byte = [0; 1]; let mut byte = [0; 1];
stream.read_exact(&mut byte).await?; stream.read_exact(&mut byte).await?;
@ -49,43 +49,80 @@ pub async fn handle_connection(
Ok(()) Ok(())
}; };
if timeout(REQUEST_TIMEOUT, read_future).await.is_err() { match timeout(REQUEST_TIMEOUT, read_future).await {
let request_str = String::from_utf8_lossy(&request_buf).trim().to_string(); Ok(Ok(())) => {
let logger = RequestLogger::new(&stream, request_str); // Read successful, continue processing
logger.log_error(59, "Request timeout"); let request = String::from_utf8_lossy(&request_buf).trim().to_string();
send_response(&mut stream, "59 Bad Request\r\n").await?;
return Ok(());
}
let request = String::from_utf8_lossy(&request_buf).trim().to_string(); // Validate request
if request.is_empty() {
let logger = RequestLogger::new(&stream, request);
logger.log_error(59, "Empty request");
return send_response(&mut stream, "59 Bad Request\r\n").await;
}
// Parse Gemini URL if request.len() > 1024 {
let path = match parse_gemini_url(&request, expected_host) { let logger = RequestLogger::new(&stream, request);
Ok(p) => p, logger.log_error(59, "Request too large");
return send_response(&mut stream, "59 Bad Request\r\n").await;
}
// Parse Gemini URL
let path = match parse_gemini_url(&request, expected_host) {
Ok(p) => p,
Err(_) => {
let logger = RequestLogger::new(&stream, request);
logger.log_error(59, "Invalid URL format");
return send_response(&mut stream, "59 Bad Request\r\n").await;
}
};
// Initialize logger now that we have the full request URL
let logger = RequestLogger::new(&stream, request);
// Resolve file path with security
let file_path = match resolve_file_path(&path, dir) {
Ok(fp) => fp,
Err(PathResolutionError::NotFound) => {
logger.log_error(51, "File not found");
return send_response(&mut stream, "51 Not found\r\n").await;
}
};
// Serve the file
match serve_file(&mut stream, &file_path).await {
Ok(_) => logger.log_success(20),
Err(_) => {
// This shouldn't happen since we check existence, but handle gracefully
logger.log_error(51, "File not found");
send_response(&mut stream, "51 Not found\r\n").await?;
}
}
},
Ok(Err(e)) => {
// Read failed, check error type
let request_str = String::from_utf8_lossy(&request_buf).trim().to_string();
let logger = RequestLogger::new(&stream, request_str);
match e.kind() {
tokio::io::ErrorKind::InvalidData => {
logger.log_error(59, "Request too large");
let _ = send_response(&mut stream, "59 Bad Request\r\n").await;
},
_ => {
logger.log_error(59, "Bad request");
let _ = send_response(&mut stream, "59 Bad Request\r\n").await;
}
}
},
Err(_) => { Err(_) => {
let logger = RequestLogger::new(&stream, request.clone()); // Timeout
logger.log_error(59, "Invalid URL format"); let request_str = String::from_utf8_lossy(&request_buf).trim().to_string();
send_response(&mut stream, "59 Bad Request\r\n").await?; let logger = RequestLogger::new(&stream, request_str);
logger.log_error(41, "Server unavailable");
let _ = send_response(&mut stream, "41 Server unavailable\r\n").await;
return Ok(()); return Ok(());
} }
};
// Initialize logger now that we have the full request URL
let logger = RequestLogger::new(&stream, request.clone());
// Resolve file path with security
let file_path = match resolve_file_path(&path, dir) {
Ok(fp) => fp,
Err(_) => {
logger.log_error(59, "Path traversal attempt");
return send_response(&mut stream, "59 Bad Request\r\n").await;
}
};
// Serve the file
match serve_file(&mut stream, &file_path).await {
Ok(_) => logger.log_success(20),
Err(_) => logger.log_error(51, "File not found"),
} }
Ok(()) Ok(())