diff --git a/AGENTS.md b/AGENTS.md index 96718d7..83ad33b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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 a specific test - `cargo test ::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 ` - Check specific binary - `cargo fmt` - Format code according to Rust standards - `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) - 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 ` - 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 - Use `.await` on async calls - 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 - Response format: "STATUS META\r\n" - Status 20: Success (follow with MIME type) -- Status 51: Not found -- Status 59: Bad request +- Status 41: Server unavailable (timeout, overload) +- Status 51: Not found (resource doesn't exist) +- Status 59: Bad request (malformed URL, protocol violation) - Default MIME: "text/gemini" for .gmi files - 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 - TOML config files with `serde::Deserialize` - 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 host: 0.0.0.0 for listening - 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` diff --git a/README.md b/README.md index 5f4b268..753da1c 100644 --- a/README.md +++ b/README.md @@ -22,9 +22,9 @@ Create a config file at `/etc/pollux/config.toml` or use `--config` to specify a ```toml root = "/path/to/static/files" -cert = "certs/cert.pem" -key = "certs/key.pem" -host = "gemini.jeena.net" +cert = "/path/to/cert.pem" +key = "/path/to/key.pem" +host = "gemini.example.com" port = 1965 log_level = "info" ``` @@ -33,10 +33,10 @@ log_level = "info" ### Quick Start with Self-Signed Certs ```bash -mkdir -p dev +mkdir -p tmp openssl req -x509 -newkey rsa:2048 \ - -keyout dev/key.pem \ - -out dev/cert.pem \ + -keyout tmp/key.pem \ + -out tmp/cert.pem \ -days 365 \ -nodes \ -subj "/CN=localhost" @@ -44,8 +44,8 @@ openssl req -x509 -newkey rsa:2048 \ Update `config.toml`: ```toml -cert = "dev/cert.pem" -key = "dev/key.pem" +cert = "tmp/cert.pem" +key = "tmp/key.pem" ``` Run the server: @@ -77,15 +77,12 @@ Access with a Gemini client like Lagrange at `gemini://yourdomain.com/`. - `--host`: Hostname for validation (required) - `--port`: Port to listen on (default 1965) -## Security - -Uses path validation to prevent directory traversal. Validate hostnames for production use. - ### Certificate Management + - Never commit certificate files to version control - Use development certificates only for local testing - Production certificates should be obtained via Let's Encrypt or your CA ## Testing -Run `cargo test` for unit tests. Fix warnings before commits. \ No newline at end of file +Run `cargo test` for unit tests. Fix warnings before commits. diff --git a/src/main.rs b/src/main.rs index b119c47..41f35c9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -12,13 +12,25 @@ use tokio::net::TcpListener; use tokio_rustls::TlsAcceptor; 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)] #[command(author, version, about, long_about = None)] struct Args { /// Path to config file - #[arg(short, long)] + #[arg(short = 'C', long)] config: Option, /// Directory to serve files from @@ -65,8 +77,8 @@ async fn main() { // Merge config with args (args take precedence) let root = args.root.or(config.root).expect("root is required"); - let cert = args.cert.or(config.cert).expect("cert is required"); - let key = args.key.or(config.key).expect("key is required"); + let cert_path = args.cert.or(config.cert).expect("cert 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 port = args.port.or(config.port).unwrap_or(1965); @@ -78,8 +90,8 @@ async fn main() { } // Load TLS certificates - let certs = tls::load_certs(&cert).unwrap(); - let key = tls::load_private_key(&key).unwrap(); + let certs = tls::load_certs(&cert_path).unwrap(); + let key = tls::load_private_key(&key_path).unwrap(); let config = ServerConfig::builder() .with_safe_defaults() @@ -89,13 +101,15 @@ async fn main() { let acceptor = TlsAcceptor::from(Arc::new(config)); 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 { let (stream, _) = listener.accept().await.unwrap(); let acceptor = acceptor.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 Err(e) = server::handle_connection(stream, &dir, &expected_host).await { tracing::error!("Error handling connection: {}", e); diff --git a/src/request.rs b/src/request.rs index 5379ca5..586cc73 100644 --- a/src/request.rs +++ b/src/request.rs @@ -1,6 +1,11 @@ use path_security::validate_path; use std::path::{Path, PathBuf}; +#[derive(Debug, PartialEq)] +pub enum PathResolutionError { + NotFound, +} + pub fn parse_gemini_url(request: &str, expected_host: &str) -> Result { if let Some(url) = request.strip_prefix("gemini://") { let host_end = url.find('/').unwrap_or(url.len()); @@ -15,7 +20,7 @@ pub fn parse_gemini_url(request: &str, expected_host: &str) -> Result Result { +pub fn resolve_file_path(path: &str, dir: &str) -> Result { let file_path_str = if path == "/" { "index.gmi".to_string() } else if path.ends_with('/') { @@ -25,8 +30,18 @@ pub fn resolve_file_path(path: &str, dir: &str) -> Result { }; match validate_path(Path::new(&file_path_str), Path::new(dir)) { - Ok(safe_path) => Ok(safe_path), - Err(_) => Err(()), + Ok(safe_path) => { + // 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] fn test_resolve_file_path_root() { 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()); } @@ -78,19 +95,28 @@ mod tests { fn test_resolve_file_path_directory() { let temp_dir = TempDir::new().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()); } #[test] fn test_resolve_file_path_file() { 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()); } #[test] fn test_resolve_file_path_traversal() { 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] diff --git a/src/server.rs b/src/server.rs index f23c9e1..53c3eae 100644 --- a/src/server.rs +++ b/src/server.rs @@ -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 std::fs; use std::io; @@ -21,7 +21,7 @@ pub async fn serve_file( stream.flush().await?; Ok(()) } 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 { loop { 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]; stream.read_exact(&mut byte).await?; @@ -49,43 +49,80 @@ pub async fn handle_connection( Ok(()) }; - if timeout(REQUEST_TIMEOUT, read_future).await.is_err() { - let request_str = String::from_utf8_lossy(&request_buf).trim().to_string(); - let logger = RequestLogger::new(&stream, request_str); - logger.log_error(59, "Request timeout"); - send_response(&mut stream, "59 Bad Request\r\n").await?; - return Ok(()); - } + match timeout(REQUEST_TIMEOUT, read_future).await { + Ok(Ok(())) => { + // Read successful, continue processing + let request = String::from_utf8_lossy(&request_buf).trim().to_string(); - 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 - let path = match parse_gemini_url(&request, expected_host) { - Ok(p) => p, + if request.len() > 1024 { + let logger = RequestLogger::new(&stream, request); + 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(_) => { - let logger = RequestLogger::new(&stream, request.clone()); - logger.log_error(59, "Invalid URL format"); - send_response(&mut stream, "59 Bad Request\r\n").await?; + // Timeout + let request_str = String::from_utf8_lossy(&request_buf).trim().to_string(); + 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(()); } - }; - - // 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(())