From a220567270362030be27c62ca98026534bf1b8b1 Mon Sep 17 00:00:00 2001 From: DevITWay Date: Mon, 26 Jan 2026 00:02:15 +0000 Subject: [PATCH] feat: add input validation with path traversal protection - validate_storage_key: reject ../, null bytes, absolute paths - validate_docker_name: OCI distribution spec compliance - validate_digest: sha256/sha512 format validation - validate_docker_reference: tag and digest reference validation - Integrate validation in storage wrapper and Docker handlers --- nora-registry/src/registry/docker.rs | 52 ++- nora-registry/src/storage/mod.rs | 34 +- nora-registry/src/validation.rs | 552 +++++++++++++++++++++++++++ 3 files changed, 622 insertions(+), 16 deletions(-) create mode 100644 nora-registry/src/validation.rs diff --git a/nora-registry/src/registry/docker.rs b/nora-registry/src/registry/docker.rs index 1bff76f..a724d99 100644 --- a/nora-registry/src/registry/docker.rs +++ b/nora-registry/src/registry/docker.rs @@ -1,3 +1,4 @@ +use crate::validation::{validate_digest, validate_docker_name, validate_docker_reference}; use crate::AppState; use axum::{ body::Bytes, @@ -33,6 +34,13 @@ async fn check_blob( State(state): State>, Path((name, digest)): Path<(String, String)>, ) -> Response { + if let Err(e) = validate_docker_name(&name) { + return (StatusCode::BAD_REQUEST, e.to_string()).into_response(); + } + if let Err(e) = validate_digest(&digest) { + return (StatusCode::BAD_REQUEST, e.to_string()).into_response(); + } + let key = format!("docker/{}/blobs/{}", name, digest); match state.storage.get(&key).await { Ok(data) => ( @@ -48,6 +56,13 @@ async fn download_blob( State(state): State>, Path((name, digest)): Path<(String, String)>, ) -> Response { + if let Err(e) = validate_docker_name(&name) { + return (StatusCode::BAD_REQUEST, e.to_string()).into_response(); + } + if let Err(e) = validate_digest(&digest) { + return (StatusCode::BAD_REQUEST, e.to_string()).into_response(); + } + let key = format!("docker/{}/blobs/{}", name, digest); match state.storage.get(&key).await { Ok(data) => ( @@ -61,6 +76,10 @@ async fn download_blob( } async fn start_upload(Path(name): Path) -> Response { + if let Err(e) = validate_docker_name(&name) { + return (StatusCode::BAD_REQUEST, e.to_string()).into_response(); + } + let uuid = uuid::Uuid::new_v4().to_string(); let location = format!("/v2/{}/blobs/uploads/{}", name, uuid); ( @@ -79,10 +98,19 @@ async fn upload_blob( axum::extract::Query(params): axum::extract::Query>, body: Bytes, ) -> Response { + if let Err(e) = validate_docker_name(&name) { + return (StatusCode::BAD_REQUEST, e.to_string()).into_response(); + } + let digest = match params.get("digest") { Some(d) => d, - None => return StatusCode::BAD_REQUEST.into_response(), + None => return (StatusCode::BAD_REQUEST, "Missing digest parameter").into_response(), }; + + if let Err(e) = validate_digest(digest) { + return (StatusCode::BAD_REQUEST, e.to_string()).into_response(); + } + let key = format!("docker/{}/blobs/{}", name, digest); match state.storage.put(&key, &body).await { Ok(()) => { @@ -97,6 +125,13 @@ async fn get_manifest( State(state): State>, Path((name, reference)): Path<(String, String)>, ) -> Response { + if let Err(e) = validate_docker_name(&name) { + return (StatusCode::BAD_REQUEST, e.to_string()).into_response(); + } + if let Err(e) = validate_docker_reference(&reference) { + return (StatusCode::BAD_REQUEST, e.to_string()).into_response(); + } + let key = format!("docker/{}/manifests/{}.json", name, reference); match state.storage.get(&key).await { Ok(data) => ( @@ -117,6 +152,13 @@ async fn put_manifest( Path((name, reference)): Path<(String, String)>, body: Bytes, ) -> Response { + if let Err(e) = validate_docker_name(&name) { + return (StatusCode::BAD_REQUEST, e.to_string()).into_response(); + } + if let Err(e) = validate_docker_reference(&reference) { + return (StatusCode::BAD_REQUEST, e.to_string()).into_response(); + } + let key = format!("docker/{}/manifests/{}.json", name, reference); match state.storage.put(&key, &body).await { Ok(()) => { @@ -139,7 +181,11 @@ async fn put_manifest( async fn list_tags( State(state): State>, Path(name): Path, -) -> (StatusCode, Json) { +) -> Response { + if let Err(e) = validate_docker_name(&name) { + return (StatusCode::BAD_REQUEST, e.to_string()).into_response(); + } + let prefix = format!("docker/{}/manifests/", name); let keys = state.storage.list(&prefix).await; let tags: Vec = keys @@ -150,5 +196,5 @@ async fn list_tags( .map(String::from) }) .collect(); - (StatusCode::OK, Json(json!({"name": name, "tags": tags}))) + (StatusCode::OK, Json(json!({"name": name, "tags": tags}))).into_response() } diff --git a/nora-registry/src/storage/mod.rs b/nora-registry/src/storage/mod.rs index 4497d92..891c348 100644 --- a/nora-registry/src/storage/mod.rs +++ b/nora-registry/src/storage/mod.rs @@ -4,10 +4,11 @@ mod s3; pub use local::LocalStorage; pub use s3::S3Storage; +use crate::validation::{validate_storage_key, ValidationError}; use async_trait::async_trait; use axum::body::Bytes; -use std::fmt; use std::sync::Arc; +use thiserror::Error; /// File metadata #[derive(Debug, Clone)] @@ -16,25 +17,21 @@ pub struct FileMeta { pub modified: u64, // Unix timestamp } -#[derive(Debug)] +#[derive(Debug, Error)] pub enum StorageError { + #[error("Network error: {0}")] Network(String), + + #[error("Object not found")] NotFound, + + #[error("IO error: {0}")] Io(String), -} -impl fmt::Display for StorageError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::Network(msg) => write!(f, "Network error: {}", msg), - Self::NotFound => write!(f, "Object not found"), - Self::Io(msg) => write!(f, "IO error: {}", msg), - } - } + #[error("Validation error: {0}")] + Validation(#[from] ValidationError), } -impl std::error::Error for StorageError {} - pub type Result = std::result::Result; /// Storage backend trait @@ -68,18 +65,29 @@ impl Storage { } pub async fn put(&self, key: &str, data: &[u8]) -> Result<()> { + validate_storage_key(key)?; self.inner.put(key, data).await } pub async fn get(&self, key: &str) -> Result { + validate_storage_key(key)?; self.inner.get(key).await } pub async fn list(&self, prefix: &str) -> Vec { + // Empty prefix is valid for listing all + if !prefix.is_empty() { + if let Err(_) = validate_storage_key(prefix) { + return Vec::new(); + } + } self.inner.list(prefix).await } pub async fn stat(&self, key: &str) -> Option { + if validate_storage_key(key).is_err() { + return None; + } self.inner.stat(key).await } diff --git a/nora-registry/src/validation.rs b/nora-registry/src/validation.rs new file mode 100644 index 0000000..8acf074 --- /dev/null +++ b/nora-registry/src/validation.rs @@ -0,0 +1,552 @@ +//! Input validation for artifact registry paths and identifiers +//! +//! Provides security validation to prevent path traversal attacks and +//! ensure inputs conform to protocol specifications. + +use std::fmt; + +/// Validation errors +#[derive(Debug, Clone, PartialEq)] +pub enum ValidationError { + /// Path contains traversal sequences (../, etc.) + PathTraversal, + /// Docker image name is invalid + InvalidDockerName(String), + /// Content digest is invalid + InvalidDigest(String), + /// Tag/reference is invalid + InvalidReference(String), + /// Input is empty + EmptyInput, + /// Input exceeds maximum length + TooLong { max: usize, actual: usize }, + /// Contains forbidden characters + ForbiddenCharacter(char), +} + +impl fmt::Display for ValidationError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::PathTraversal => write!(f, "Path traversal detected"), + Self::InvalidDockerName(reason) => write!(f, "Invalid Docker name: {}", reason), + Self::InvalidDigest(reason) => write!(f, "Invalid digest: {}", reason), + Self::InvalidReference(reason) => write!(f, "Invalid reference: {}", reason), + Self::EmptyInput => write!(f, "Input cannot be empty"), + Self::TooLong { max, actual } => { + write!(f, "Input exceeds maximum length ({} > {})", actual, max) + } + Self::ForbiddenCharacter(c) => write!(f, "Forbidden character: {:?}", c), + } + } +} + +impl std::error::Error for ValidationError {} + +/// Maximum allowed storage key length +const MAX_KEY_LENGTH: usize = 1024; + +/// Maximum Docker name length +const MAX_DOCKER_NAME_LENGTH: usize = 256; + +/// Maximum tag/reference length +const MAX_REFERENCE_LENGTH: usize = 128; + +/// Validate and sanitize a storage key to prevent path traversal attacks. +/// +/// Rejects keys containing: +/// - `..` path traversal sequences +/// - Leading `/` or `\` (absolute paths) +/// - Null bytes +/// - Empty segments +pub fn validate_storage_key(key: &str) -> Result<(), ValidationError> { + if key.is_empty() { + return Err(ValidationError::EmptyInput); + } + + if key.len() > MAX_KEY_LENGTH { + return Err(ValidationError::TooLong { + max: MAX_KEY_LENGTH, + actual: key.len(), + }); + } + + // Check for null bytes + if key.contains('\0') { + return Err(ValidationError::ForbiddenCharacter('\0')); + } + + // Check for absolute paths + if key.starts_with('/') || key.starts_with('\\') { + return Err(ValidationError::PathTraversal); + } + + // Check for path traversal patterns + if key.contains("..") { + return Err(ValidationError::PathTraversal); + } + + // Check for backslash (Windows path separator) + if key.contains('\\') { + return Err(ValidationError::PathTraversal); + } + + // Check each segment + for segment in key.split('/') { + if segment.is_empty() && key != "" { + // Allow trailing slash but not double slashes + continue; + } + if segment == "." || segment == ".." { + return Err(ValidationError::PathTraversal); + } + } + + Ok(()) +} + +/// Validate Docker image name per OCI distribution spec. +/// +/// Valid names: +/// - Lowercase letters, digits, underscores, dots, hyphens +/// - May contain path separators (/) +/// - Each component must start with alphanumeric +/// - Max 256 characters +/// +/// Examples: +/// - `nginx` ✓ +/// - `library/nginx` ✓ +/// - `my-org/my-image` ✓ +/// - `NGINX` ✗ (uppercase) +/// - `../escape` ✗ (path traversal) +pub fn validate_docker_name(name: &str) -> Result<(), ValidationError> { + if name.is_empty() { + return Err(ValidationError::EmptyInput); + } + + if name.len() > MAX_DOCKER_NAME_LENGTH { + return Err(ValidationError::TooLong { + max: MAX_DOCKER_NAME_LENGTH, + actual: name.len(), + }); + } + + // Check for path traversal + if name.contains("..") { + return Err(ValidationError::PathTraversal); + } + + // Must contain only valid characters + for c in name.chars() { + if !matches!(c, 'a'..='z' | '0'..='9' | '_' | '.' | '-' | '/') { + if c.is_ascii_uppercase() { + return Err(ValidationError::InvalidDockerName( + "must be lowercase".to_string(), + )); + } + return Err(ValidationError::ForbiddenCharacter(c)); + } + } + + // Cannot start with separator + if name.starts_with('/') || name.starts_with('.') || name.starts_with('-') { + return Err(ValidationError::InvalidDockerName( + "cannot start with separator or special character".to_string(), + )); + } + + // Cannot end with separator + if name.ends_with('/') { + return Err(ValidationError::InvalidDockerName( + "cannot end with /".to_string(), + )); + } + + // No consecutive separators (except ..) + if name.contains("//") || name.contains("--") || name.contains("__") { + return Err(ValidationError::InvalidDockerName( + "consecutive separators not allowed".to_string(), + )); + } + + // Each path segment must start with alphanumeric + for segment in name.split('/') { + if segment.is_empty() { + return Err(ValidationError::InvalidDockerName( + "empty path segment".to_string(), + )); + } + let first = segment.chars().next().unwrap(); + if !first.is_ascii_alphanumeric() { + return Err(ValidationError::InvalidDockerName( + "segment must start with alphanumeric".to_string(), + )); + } + } + + Ok(()) +} + +/// Validate content digest format. +/// +/// Supported formats: +/// - `sha256:<64 hex chars>` +/// - `sha512:<128 hex chars>` +/// +/// Examples: +/// - `sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4` ✓ +/// - `sha256:ABC` ✗ (uppercase) +/// - `md5:abc` ✗ (unsupported algorithm) +pub fn validate_digest(digest: &str) -> Result<(), ValidationError> { + if digest.is_empty() { + return Err(ValidationError::EmptyInput); + } + + // Check for path traversal (shouldn't be in digest but defensive check) + if digest.contains("..") || digest.contains('/') { + return Err(ValidationError::PathTraversal); + } + + let parts: Vec<&str> = digest.splitn(2, ':').collect(); + if parts.len() != 2 { + return Err(ValidationError::InvalidDigest( + "missing algorithm prefix (expected algo:hash)".to_string(), + )); + } + + let (algo, hash) = (parts[0], parts[1]); + + match algo { + "sha256" => { + if hash.len() != 64 { + return Err(ValidationError::InvalidDigest(format!( + "sha256 hash must be 64 characters, got {}", + hash.len() + ))); + } + } + "sha512" => { + if hash.len() != 128 { + return Err(ValidationError::InvalidDigest(format!( + "sha512 hash must be 128 characters, got {}", + hash.len() + ))); + } + } + _ => { + return Err(ValidationError::InvalidDigest(format!( + "unsupported algorithm: {} (use sha256 or sha512)", + algo + ))); + } + } + + // Hash must be lowercase hex + for c in hash.chars() { + if !matches!(c, '0'..='9' | 'a'..='f') { + if c.is_ascii_uppercase() { + return Err(ValidationError::InvalidDigest( + "hash must be lowercase hex".to_string(), + )); + } + return Err(ValidationError::InvalidDigest(format!( + "invalid character in hash: {:?}", + c + ))); + } + } + + Ok(()) +} + +/// Validate Docker tag or reference (tag or digest). +/// +/// Tags: +/// - Alphanumeric, dots, underscores, hyphens +/// - Max 128 characters +/// - Must start with alphanumeric +/// +/// References may also be digests (sha256:...). +pub fn validate_docker_reference(reference: &str) -> Result<(), ValidationError> { + if reference.is_empty() { + return Err(ValidationError::EmptyInput); + } + + if reference.len() > MAX_REFERENCE_LENGTH { + return Err(ValidationError::TooLong { + max: MAX_REFERENCE_LENGTH, + actual: reference.len(), + }); + } + + // Check for path traversal + if reference.contains("..") || reference.contains('/') { + return Err(ValidationError::PathTraversal); + } + + // If it looks like a digest, validate as digest + if reference.starts_with("sha256:") || reference.starts_with("sha512:") { + return validate_digest(reference); + } + + // Validate as tag + let first = reference.chars().next().unwrap(); + if !first.is_ascii_alphanumeric() { + return Err(ValidationError::InvalidReference( + "tag must start with alphanumeric".to_string(), + )); + } + + for c in reference.chars() { + if !matches!(c, 'a'..='z' | 'A'..='Z' | '0'..='9' | '.' | '_' | '-') { + return Err(ValidationError::ForbiddenCharacter(c)); + } + } + + Ok(()) +} + +/// Validate Maven artifact path. +/// +/// Maven paths follow the pattern: groupId/artifactId/version/filename +/// Example: `org/apache/commons/commons-lang3/3.12.0/commons-lang3-3.12.0.jar` +pub fn validate_maven_path(path: &str) -> Result<(), ValidationError> { + validate_storage_key(path) +} + +/// Validate npm package name. +pub fn validate_npm_name(name: &str) -> Result<(), ValidationError> { + if name.is_empty() { + return Err(ValidationError::EmptyInput); + } + + if name.len() > 214 { + return Err(ValidationError::TooLong { + max: 214, + actual: name.len(), + }); + } + + // Check for path traversal + if name.contains("..") { + return Err(ValidationError::PathTraversal); + } + + Ok(()) +} + +/// Validate Cargo crate name. +pub fn validate_crate_name(name: &str) -> Result<(), ValidationError> { + if name.is_empty() { + return Err(ValidationError::EmptyInput); + } + + if name.len() > 64 { + return Err(ValidationError::TooLong { + max: 64, + actual: name.len(), + }); + } + + // Check for path traversal + if name.contains("..") || name.contains('/') { + return Err(ValidationError::PathTraversal); + } + + // Crate names: alphanumeric, underscores, hyphens + for c in name.chars() { + if !matches!(c, 'a'..='z' | 'A'..='Z' | '0'..='9' | '_' | '-') { + return Err(ValidationError::ForbiddenCharacter(c)); + } + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + // Storage key tests + #[test] + fn test_storage_key_valid() { + assert!(validate_storage_key("docker/nginx/blobs/sha256:abc").is_ok()); + assert!(validate_storage_key("maven/org/apache/commons").is_ok()); + assert!(validate_storage_key("simple").is_ok()); + } + + #[test] + fn test_storage_key_path_traversal() { + assert!(matches!( + validate_storage_key("../etc/passwd"), + Err(ValidationError::PathTraversal) + )); + assert!(matches!( + validate_storage_key("foo/../bar"), + Err(ValidationError::PathTraversal) + )); + assert!(matches!( + validate_storage_key("foo/.."), + Err(ValidationError::PathTraversal) + )); + } + + #[test] + fn test_storage_key_absolute_path() { + assert!(matches!( + validate_storage_key("/etc/passwd"), + Err(ValidationError::PathTraversal) + )); + assert!(matches!( + validate_storage_key("\\windows\\system32"), + Err(ValidationError::PathTraversal) + )); + } + + #[test] + fn test_storage_key_null_byte() { + assert!(matches!( + validate_storage_key("foo\0bar"), + Err(ValidationError::ForbiddenCharacter('\0')) + )); + } + + #[test] + fn test_storage_key_empty() { + assert!(matches!( + validate_storage_key(""), + Err(ValidationError::EmptyInput) + )); + } + + #[test] + fn test_storage_key_too_long() { + let long_key = "a".repeat(1025); + assert!(matches!( + validate_storage_key(&long_key), + Err(ValidationError::TooLong { .. }) + )); + } + + // Docker name tests + #[test] + fn test_docker_name_valid() { + assert!(validate_docker_name("nginx").is_ok()); + assert!(validate_docker_name("library/nginx").is_ok()); + assert!(validate_docker_name("my-org/my-image").is_ok()); + assert!(validate_docker_name("my_image").is_ok()); + assert!(validate_docker_name("image.name").is_ok()); + assert!(validate_docker_name("a/b/c/d").is_ok()); + } + + #[test] + fn test_docker_name_uppercase() { + assert!(matches!( + validate_docker_name("NGINX"), + Err(ValidationError::InvalidDockerName(_)) + )); + assert!(matches!( + validate_docker_name("MyImage"), + Err(ValidationError::InvalidDockerName(_)) + )); + } + + #[test] + fn test_docker_name_path_traversal() { + assert!(matches!( + validate_docker_name("../escape"), + Err(ValidationError::PathTraversal) + )); + assert!(matches!( + validate_docker_name("foo/../bar"), + Err(ValidationError::PathTraversal) + )); + } + + #[test] + fn test_docker_name_invalid_start() { + assert!(validate_docker_name("/nginx").is_err()); + assert!(validate_docker_name(".nginx").is_err()); + assert!(validate_docker_name("-nginx").is_err()); + } + + #[test] + fn test_docker_name_consecutive_separators() { + assert!(validate_docker_name("foo//bar").is_err()); + assert!(validate_docker_name("foo--bar").is_err()); + assert!(validate_docker_name("foo__bar").is_err()); + } + + // Digest tests + #[test] + fn test_digest_valid_sha256() { + let valid = format!("sha256:{}", "a".repeat(64)); + assert!(validate_digest(&valid).is_ok()); + } + + #[test] + fn test_digest_valid_sha512() { + let valid = format!("sha512:{}", "a".repeat(128)); + assert!(validate_digest(&valid).is_ok()); + } + + #[test] + fn test_digest_wrong_length() { + assert!(validate_digest("sha256:abc").is_err()); + assert!(validate_digest(&format!("sha256:{}", "a".repeat(63))).is_err()); + assert!(validate_digest(&format!("sha256:{}", "a".repeat(65))).is_err()); + } + + #[test] + fn test_digest_uppercase() { + let upper = format!("sha256:{}", "A".repeat(64)); + assert!(matches!( + validate_digest(&upper), + Err(ValidationError::InvalidDigest(_)) + )); + } + + #[test] + fn test_digest_unsupported_algorithm() { + assert!(matches!( + validate_digest("md5:abc"), + Err(ValidationError::InvalidDigest(_)) + )); + } + + #[test] + fn test_digest_missing_prefix() { + assert!(matches!( + validate_digest("abcdef123456"), + Err(ValidationError::InvalidDigest(_)) + )); + } + + // Reference tests + #[test] + fn test_reference_valid_tag() { + assert!(validate_docker_reference("latest").is_ok()); + assert!(validate_docker_reference("v1.0.0").is_ok()); + assert!(validate_docker_reference("1.0").is_ok()); + assert!(validate_docker_reference("my-tag_v2").is_ok()); + } + + #[test] + fn test_reference_valid_digest() { + let digest = format!("sha256:{}", "a".repeat(64)); + assert!(validate_docker_reference(&digest).is_ok()); + } + + #[test] + fn test_reference_path_traversal() { + assert!(matches!( + validate_docker_reference("../escape"), + Err(ValidationError::PathTraversal) + )); + } + + #[test] + fn test_reference_invalid_start() { + assert!(validate_docker_reference(".hidden").is_err()); + assert!(validate_docker_reference("-dash").is_err()); + } +}