From da85da053b3e2cf0a853a0dc0c3e3d307a15883c Mon Sep 17 00:00:00 2001 From: Jannis Schlegel Date: Fri, 17 Jul 2026 18:21:16 +0200 Subject: [PATCH 01/14] fix: compression level zero silently corrupts writer output --- crates/pithos_lib/src/helpers/zstd.rs | 1 + crates/pithos_lib/src/io/pithoswriter.rs | 2 +- crates/pithos_lib/tests/writer.rs | 141 +++++++++++++++++++++++ 3 files changed, 143 insertions(+), 1 deletion(-) diff --git a/crates/pithos_lib/src/helpers/zstd.rs b/crates/pithos_lib/src/helpers/zstd.rs index c7d96d4..355219b 100644 --- a/crates/pithos_lib/src/helpers/zstd.rs +++ b/crates/pithos_lib/src/helpers/zstd.rs @@ -21,6 +21,7 @@ const _ZSTD_MAGIC_NUMBER: u32 = 0xFD2FB528; // 4 Bytes, little-endian format #[tracing::instrument(level = "trace", skip(flags))] pub fn map_to_zstd_level(flags: &ProcessingFlags) -> i32 { match flags.get_compression_level() { + 0 => 0, 1 => 1, 2 => 4, 3 => 8, diff --git a/crates/pithos_lib/src/io/pithoswriter.rs b/crates/pithos_lib/src/io/pithoswriter.rs index 38a92cc..b133108 100644 --- a/crates/pithos_lib/src/io/pithoswriter.rs +++ b/crates/pithos_lib/src/io/pithoswriter.rs @@ -281,7 +281,7 @@ impl PithosWriter { if compression_level > 0 && probe_compression_ratio(&chunk.data, Some(compression_level))? < 0.85 { - chunk.data = compress_data(chunk.data.as_slice(), None)?; + chunk.data = compress_data(chunk.data.as_slice(), Some(compression_level))?; } else { // No compression, as the input is likely to have high entropy block_index_entry.flags.set_compression_level(0); diff --git a/crates/pithos_lib/tests/writer.rs b/crates/pithos_lib/tests/writer.rs index d7d363c..64aac59 100644 --- a/crates/pithos_lib/tests/writer.rs +++ b/crates/pithos_lib/tests/writer.rs @@ -61,6 +61,147 @@ fn read_zip_member(path: &Path, name: &str) -> Vec { bytes } +fn assert_compression_round_trip( + payload: &[u8], + compression_level: u8, + encrypt: bool, + cdc: Option<(usize, usize, usize)>, +) { + let temp_dir = TempDir::new().unwrap(); + let source_path = temp_dir.path().join("input.bin"); + write(&source_path, payload).unwrap(); + + let (pithos_path, reader_key, mut writer) = create_pithos_writer(&temp_dir, cdc); + writer.write_file_header().unwrap(); + writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "input.bin".to_string(), + data: Content::File(source_path.to_string_lossy().into_owned()), + metadata: None, + encrypt, + compression_level: Some(compression_level), + }) + .unwrap(); + writer.write_directory().unwrap(); + + let (directory, _) = read_pithos_directory(&pithos_path, &reader_key).unwrap(); + for block in directory.blocks.values() { + assert_eq!(block.flags.is_encrypted(), encrypt); + let stored_level = block.flags.get_compression_level(); + if compression_level == 0 { + assert_eq!(stored_level, 0); + if !encrypt { + assert_eq!(block.stored_size, block.original_size); + } + } else { + assert!(stored_level == 0 || stored_level == compression_level); + } + } + assert_eq!( + extract_pithos_entry(&pithos_path, &reader_key, "input.bin", &temp_dir), + payload + ); +} + +fn deterministic_incompressible_payload(length: usize) -> Vec { + let mut state = 0x243f_6a88_u32; + (0..length) + .map(|index| { + state = state + .wrapping_mul(1_664_525) + .wrapping_add(1_013_904_223) + .rotate_left((index % 31) as u32); + (state ^ (index as u32)).to_le_bytes()[index % 4] + }) + .collect() +} + +#[test] +fn test_compression_levels_round_trip() { + let payloads = [ + ("repeated", vec![b'A'; 16 * 1024]), + ( + "incompressible", + deterministic_incompressible_payload(16 * 1024), + ), + ]; + + for (_, payload) in payloads { + for compression_level in 0..=7 { + for encrypt in [false, true] { + assert_compression_round_trip(&payload, compression_level, encrypt, None); + } + } + } +} + +#[test] +fn test_compression_empty_level_zero_round_trip() { + for encrypt in [false, true] { + let temp_dir = TempDir::new().unwrap(); + let source_path = temp_dir.path().join("empty.bin"); + write(&source_path, []).unwrap(); + + let (pithos_path, reader_key, mut writer) = create_pithos_writer(&temp_dir, None); + writer.write_file_header().unwrap(); + writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "empty.bin".to_string(), + data: Content::File(source_path.to_string_lossy().into_owned()), + metadata: None, + encrypt, + compression_level: Some(0), + }) + .unwrap(); + writer.write_directory().unwrap(); + + let (directory, _) = read_pithos_directory(&pithos_path, &reader_key).unwrap(); + assert!(directory.blocks.is_empty()); + assert_eq!( + extract_pithos_entry(&pithos_path, &reader_key, "empty.bin", &temp_dir), + Vec::::new() + ); + } +} + +#[test] +fn test_compression_multiblock_level_zero_round_trip() { + let payload = deterministic_incompressible_payload(16 * 1024); + for encrypt in [false, true] { + let temp_dir = TempDir::new().unwrap(); + let source_path = temp_dir.path().join("multiblock.bin"); + write(&source_path, &payload).unwrap(); + + let (pithos_path, reader_key, mut writer) = + create_pithos_writer(&temp_dir, Some((256, 512, 1024))); + writer.write_file_header().unwrap(); + writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "multiblock.bin".to_string(), + data: Content::File(source_path.to_string_lossy().into_owned()), + metadata: None, + encrypt, + compression_level: Some(0), + }) + .unwrap(); + writer.write_directory().unwrap(); + + let (directory, _) = read_pithos_directory(&pithos_path, &reader_key).unwrap(); + assert!(directory.blocks.len() > 1); + for block in directory.blocks.values() { + assert_eq!(block.flags.get_compression_level(), 0); + assert_eq!(block.flags.is_encrypted(), encrypt); + } + assert_eq!( + extract_pithos_entry(&pithos_path, &reader_key, "multiblock.bin", &temp_dir), + payload + ); + } +} + fn write_raw_zip(path: &Path, entries: &[(&str, &[u8], u32)], overlap_duplicates: bool) { let mut archive = Vec::new(); let mut central_directory = Vec::new(); From 71dadd2f992184df689a55cee8cf84c594e5b4a1 Mon Sep 17 00:00:00 2001 From: Jannis Schlegel Date: Fri, 17 Jul 2026 21:11:42 +0200 Subject: [PATCH 02/14] fix: prevent path and symlink extraction escapes --- Cargo.lock | 96 +++++ crates/pithos_lib/Cargo.toml | 1 + crates/pithos_lib/src/error.rs | 12 + crates/pithos_lib/src/helpers/archive_path.rs | 342 ++++++++++++++++++ crates/pithos_lib/src/helpers/mod.rs | 1 + crates/pithos_lib/src/io/extraction.rs | 154 ++++++++ crates/pithos_lib/src/io/mod.rs | 1 + crates/pithos_lib/src/io/pithosreader.rs | 77 ++-- crates/pithos_lib/src/io/pithoswriter.rs | 40 +- crates/pithos_lib/src/io/util.rs | 52 +-- .../pithos_lib/src/model/deserialization.rs | 6 +- crates/pithos_lib/tests/marshalling.rs | 72 +++- crates/pithos_lib/tests/reader.rs | 264 +++++++++++++- crates/pithos_lib/tests/writer.rs | 132 ++++++- deny.toml | 1 + 15 files changed, 1172 insertions(+), 79 deletions(-) create mode 100644 crates/pithos_lib/src/helpers/archive_path.rs create mode 100644 crates/pithos_lib/src/io/extraction.rs diff --git a/Cargo.lock b/Cargo.lock index 53ba149..d5b1654 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -27,6 +27,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "ambient-authority" +version = "0.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e9d4ee0d472d1cd2e28c97dfa124b3d8d992e10eb0a035f33f5d12e3a177ba3b" + [[package]] name = "android_system_properties" version = "0.1.5" @@ -192,6 +198,36 @@ version = "1.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fc652a48c352aef3ea3aed32080501cf3ef6ed5da78602a020c991775b0aff04" +[[package]] +name = "cap-primitives" +version = "4.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cdadbd7c002d3a484b35243669abdae85a0ebaded5a61117169dc3400f9a7ff0" +dependencies = [ + "ambient-authority", + "fs-set-times", + "io-extras", + "io-lifetimes 3.0.1", + "ipnet", + "maybe-owned", + "rustix", + "rustix-linux-procfs", + "windows-sys 0.61.2", + "winx", +] + +[[package]] +name = "cap-std" +version = "4.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7281235d6e96d3544ca18bba9049be92f4190f8d923e3caef1b5f66cfa752608" +dependencies = [ + "cap-primitives", + "io-extras", + "io-lifetimes 3.0.1", + "rustix", +] + [[package]] name = "cc" version = "1.2.67" @@ -577,6 +613,17 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs-set-times" +version = "0.20.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94e7099f6313ecacbe1256e8ff9d617b75d1bcb16a6fddef94866d225a01a14a" +dependencies = [ + "io-lifetimes 2.0.4", + "rustix", + "windows-sys 0.52.0", +] + [[package]] name = "fs_extra" version = "1.3.0" @@ -951,6 +998,28 @@ version = "4.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "14c00403deb17c3221a1fe4fb571b9ed0370b3dcd116553c77fa294a3d918699" +[[package]] +name = "io-extras" +version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "20fd6de4ccfcc187e38bc21cfa543cb5a302cb86a8b114eb7f0bf0dc9f8ac00f" +dependencies = [ + "io-lifetimes 3.0.1", + "windows-sys 0.52.0", +] + +[[package]] +name = "io-lifetimes" +version = "2.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06432fb54d3be7964ecd3649233cddf80db2832f47fec34c01f65b3d9d774983" + +[[package]] +name = "io-lifetimes" +version = "3.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2f0fb0570afe1fed943c5c3d4102d5358592d8625fda6a0007fdbe65a92fba96" + [[package]] name = "ipnet" version = "2.12.0" @@ -1103,6 +1172,12 @@ dependencies = [ "regex-automata", ] +[[package]] +name = "maybe-owned" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4facc753ae494aeb6e3c22f839b158aebd4f9270f55cd3c79906c45476c47ab4" + [[package]] name = "memchr" version = "2.8.3" @@ -1223,6 +1298,7 @@ version = "0.7.2" dependencies = [ "blake3", "byteorder", + "cap-std", "chacha20poly1305", "crc32fast", "digest", @@ -1529,6 +1605,16 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "rustix-linux-procfs" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2fc84bf7e9aa16c4f2c758f27412dc9841341e16aa682d9c7ac308fe3ee12056" +dependencies = [ + "once_cell", + "rustix", +] + [[package]] name = "rustls" version = "0.23.42" @@ -2449,6 +2535,16 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" +[[package]] +name = "winx" +version = "0.36.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f3fd376f71958b862e7afb20cfe5a22830e1963462f3a17f49d82a6c1d1f42d" +dependencies = [ + "bitflags", + "windows-sys 0.52.0", +] + [[package]] name = "writeable" version = "0.6.3" diff --git a/crates/pithos_lib/Cargo.toml b/crates/pithos_lib/Cargo.toml index 7c033a3..28f6cd0 100644 --- a/crates/pithos_lib/Cargo.toml +++ b/crates/pithos_lib/Cargo.toml @@ -15,6 +15,7 @@ blake3 = "1.8.2" byteorder = "1.5.0" chacha20poly1305 = "0.11.0" crc32fast = "1.5.0" +cap-std = "4.0.2" digest = "0.11.3" fastcdc = "4.0.1" indexmap = "2.11.0" diff --git a/crates/pithos_lib/src/error.rs b/crates/pithos_lib/src/error.rs index dd003f8..a2551dd 100644 --- a/crates/pithos_lib/src/error.rs +++ b/crates/pithos_lib/src/error.rs @@ -73,6 +73,18 @@ pub enum PithosError { RelationIdOccupied(u64), #[error("Path already occupied: {0}")] PathOccupied(String), + #[error("Invalid archive path {path}: {reason}")] + InvalidArchivePath { path: String, reason: String }, + #[error("Invalid symlink target {target} for {path}: {reason}")] + InvalidSymlinkTarget { + path: String, + target: String, + reason: String, + }, + #[error("Invalid symlink entry {path}: {reason}")] + InvalidSymlinkEntry { path: String, reason: String }, + #[error("Extraction collision at {path}: {reason}")] + ExtractionCollision { path: String, reason: String }, #[error("Invalid file type: {0}")] InvalidFileType(String), #[error("No recipient section found for the provided private key")] diff --git a/crates/pithos_lib/src/helpers/archive_path.rs b/crates/pithos_lib/src/helpers/archive_path.rs new file mode 100644 index 0000000..c0ebf16 --- /dev/null +++ b/crates/pithos_lib/src/helpers/archive_path.rs @@ -0,0 +1,342 @@ +use crate::error::PithosError; +use crate::helpers::file_entry_map::FileEntryMap; +use crate::model::structs::{BlockDataState, FileEntry, FileType}; + +fn invalid_path(path: &str, reason: impl Into) -> PithosError { + PithosError::InvalidArchivePath { + path: path.to_string(), + reason: reason.into(), + } +} + +pub(crate) fn validate_entry_path(path: &str) -> Result<(), PithosError> { + if path.is_empty() { + return Err(invalid_path(path, "path is empty")); + } + if path.contains('\0') { + return Err(invalid_path(path, "NUL is not allowed")); + } + if path.contains('\\') { + return Err(invalid_path(path, "backslash is not allowed")); + } + if path.starts_with('/') || path.ends_with('/') { + return Err(invalid_path(path, "path must not start or end with /")); + } + if path.as_bytes().get(1) == Some(&b':') { + return Err(invalid_path(path, "drive forms are not allowed")); + } + for component in path.split('/') { + if component.is_empty() { + return Err(invalid_path(path, "empty path components are not allowed")); + } + if component == "." || component == ".." { + return Err(invalid_path(path, "dot components are not allowed")); + } + } + Ok(()) +} + +pub(crate) fn validate_symlink_target(path: &str, target: &str) -> Result<(), PithosError> { + validate_entry_path(path).map_err(|error| PithosError::InvalidSymlinkTarget { + path: path.to_string(), + target: target.to_string(), + reason: error.to_string(), + })?; + if target.is_empty() { + return Err(PithosError::InvalidSymlinkTarget { + path: path.to_string(), + target: target.to_string(), + reason: "target is empty".into(), + }); + } + if target.contains('\0') || target.contains('\\') { + return Err(PithosError::InvalidSymlinkTarget { + path: path.to_string(), + target: target.to_string(), + reason: "invalid separator or NUL".into(), + }); + } + if target.starts_with('/') || target.as_bytes().get(1) == Some(&b':') { + return Err(PithosError::InvalidSymlinkTarget { + path: path.to_string(), + target: target.to_string(), + reason: "absolute or drive target".into(), + }); + } + let mut depth = path.split('/').count() - 1; + for component in target.split('/') { + if component.is_empty() || component == "." { + return Err(PithosError::InvalidSymlinkTarget { + path: path.to_string(), + target: target.to_string(), + reason: "empty or dot component".into(), + }); + } + if component == ".." { + if depth == 0 { + return Err(PithosError::InvalidSymlinkTarget { + path: path.to_string(), + target: target.to_string(), + reason: "target escapes archive root".into(), + }); + } + depth -= 1; + } else { + depth += 1; + } + } + Ok(()) +} + +pub(crate) fn validate_entry(path: &str, entry: &FileEntry) -> Result<(), PithosError> { + validate_entry_path(path)?; + match (&entry.file_type, &entry.symlink_target, &entry.block_data) { + (FileType::Symlink, Some(target), BlockDataState::Decrypted(blocks)) + if blocks.is_empty() => + { + validate_symlink_target(path, target) + } + (FileType::Symlink, None, _) => Err(PithosError::InvalidSymlinkEntry { + path: path.into(), + reason: "missing target".into(), + }), + (FileType::Symlink, Some(_), BlockDataState::Decrypted(blocks)) if !blocks.is_empty() => { + Err(PithosError::InvalidSymlinkEntry { + path: path.into(), + reason: "symlink has block references".into(), + }) + } + (FileType::Symlink, Some(_), BlockDataState::Encrypted(_)) => { + Err(PithosError::InvalidSymlinkEntry { + path: path.into(), + reason: "encrypted symlink block data".into(), + }) + } + (_, Some(_), _) => Err(PithosError::InvalidSymlinkEntry { + path: path.into(), + reason: "non-symlink has a target".into(), + }), + _ => Ok(()), + } +} + +pub(crate) fn validate_candidate( + map: &FileEntryMap, + path: &str, + entry: &FileEntry, +) -> Result<(), PithosError> { + validate_entry(path, entry)?; + for (_, existing, existing_entry) in map { + if path == existing { + continue; + } + if path.starts_with(existing) + && path.as_bytes().get(existing.len()) == Some(&b'/') + && existing_entry.file_type != FileType::Directory + { + return Err(PithosError::InvalidArchivePath { + path: path.into(), + reason: format!("file entry {existing} is an ancestor"), + }); + } + if existing.starts_with(path) + && existing.as_bytes().get(path.len()) == Some(&b'/') + && entry.file_type != FileType::Directory + { + return Err(PithosError::InvalidArchivePath { + path: path.into(), + reason: format!("entry is an ancestor of {existing}"), + }); + } + } + Ok(()) +} + +pub(crate) fn validate_map(map: &FileEntryMap) -> Result<(), PithosError> { + for (_, path, entry) in map { + validate_candidate(&FileEntryMap::new(), path, entry)?; + } + for (_, path, entry) in map { + for (_, other, _) in map { + if path != other + && other.starts_with(path) + && other.as_bytes().get(path.len()) == Some(&b'/') + && entry.file_type != FileType::Directory + { + return Err(PithosError::InvalidArchivePath { + path: path.into(), + reason: format!("entry is an ancestor of {other}"), + }); + } + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::helpers::file_entry_map::Key; + + fn entry(file_type: FileType, target: Option<&str>, blocks: BlockDataState) -> FileEntry { + FileEntry { + file_type, + block_data: blocks, + created: 0, + modified: 0, + file_size: 0, + permissions: 0o644, + references: vec![], + symlink_target: target.map(str::to_owned), + } + } + + #[test] + fn archive_path_valid_entry_corpus() { + for path in ["file", "nested/file", "ユニコード/file"] { + assert!(validate_entry_path(path).is_ok()); + } + } + #[test] + fn archive_path_invalid_entry_corpus() { + for path in [ + "", + "/file", + "file/", + "a//b", + "a/./b", + "a/../b", + "a\\b", + "C:", + "C:/x", + "\\\\server\\x", + "a\0b", + ] { + assert!(validate_entry_path(path).is_err(), "{path:?}"); + } + } + #[test] + fn archive_path_symlink_target_corpus() { + for target in ["target", "nested/target", "../target", "../dangling"] { + assert!(validate_symlink_target("nested/link", target).is_ok()); + } + for target in [ + "", + ".", + "target/.", + "target//child", + "target/", + "a\0b", + "a\\b", + "/absolute", + "C:", + "C:/target", + "\\\\server\\target", + "../target", + "../../target", + ] { + assert!( + validate_symlink_target("link", target).is_err(), + "{target:?}" + ); + } + assert!(validate_symlink_target("", "target").is_err()); + assert!(validate_symlink_target("bad//link", "target").is_err()); + assert!(validate_symlink_target("bad/../link", "target").is_err()); + } + + #[test] + fn archive_path_entry_invariants_corpus() { + assert!( + validate_entry( + "link", + &entry(FileType::Symlink, None, BlockDataState::Decrypted(vec![])) + ) + .is_err() + ); + assert!( + validate_entry( + "file", + &entry( + FileType::Data, + Some("target"), + BlockDataState::Decrypted(vec![]) + ) + ) + .is_err() + ); + assert!( + validate_entry( + "link", + &entry( + FileType::Symlink, + Some("target"), + BlockDataState::Decrypted(vec![([0; 32], [0; 32])]) + ) + ) + .is_err() + ); + assert!( + validate_entry( + "link", + &entry( + FileType::Symlink, + Some("target"), + BlockDataState::Encrypted(vec![]) + ) + ) + .is_err() + ); + } + + #[test] + fn archive_path_candidate_and_map_conflicts_are_order_independent() { + let file = entry(FileType::Data, None, BlockDataState::Decrypted(vec![])); + let link = entry( + FileType::Symlink, + Some("target"), + BlockDataState::Decrypted(vec![]), + ); + let directory = entry(FileType::Directory, None, BlockDataState::Decrypted(vec![])); + + for (ancestor_path, ancestor) in [("a", file.clone()), ("a", link.clone())] { + for order in [0, 1] { + let mut map = FileEntryMap::new(); + if order == 0 { + map.insert(Key::new(0, ancestor_path), ancestor.clone()) + .unwrap(); + map.insert(Key::new(1, "a/child"), file.clone()).unwrap(); + } else { + map.insert(Key::new(0, "a/child"), file.clone()).unwrap(); + map.insert(Key::new(1, ancestor_path), ancestor.clone()) + .unwrap(); + } + assert!(validate_map(&map).is_err()); + } + } + + for order in [0, 1] { + let mut map = FileEntryMap::new(); + if order == 0 { + map.insert(Key::new(0, "a/child"), file.clone()).unwrap(); + map.insert(Key::new(1, "a"), file.clone()).unwrap(); + } else { + map.insert(Key::new(0, "a"), file.clone()).unwrap(); + map.insert(Key::new(1, "a/child"), file.clone()).unwrap(); + } + assert!(validate_map(&map).is_err()); + } + + for order in [0, 1] { + let mut map = FileEntryMap::new(); + if order == 0 { + map.insert(Key::new(0, "a"), directory.clone()).unwrap(); + map.insert(Key::new(1, "a/child"), file.clone()).unwrap(); + } else { + map.insert(Key::new(0, "a/child"), file.clone()).unwrap(); + map.insert(Key::new(1, "a"), directory.clone()).unwrap(); + } + assert!(validate_map(&map).is_ok()); + } + } +} diff --git a/crates/pithos_lib/src/helpers/mod.rs b/crates/pithos_lib/src/helpers/mod.rs index 4f604a8..438b855 100644 --- a/crates/pithos_lib/src/helpers/mod.rs +++ b/crates/pithos_lib/src/helpers/mod.rs @@ -1,3 +1,4 @@ +pub(crate) mod archive_path; pub mod chacha_poly1305; pub mod crypt4gh; pub mod directory; diff --git a/crates/pithos_lib/src/io/extraction.rs b/crates/pithos_lib/src/io/extraction.rs new file mode 100644 index 0000000..e330a13 --- /dev/null +++ b/crates/pithos_lib/src/io/extraction.rs @@ -0,0 +1,154 @@ +use crate::error::PithosError; +use cap_std::fs::{Dir, OpenOptions}; +use std::io::{self, Write}; +use std::path::Path; +use std::sync::atomic::{AtomicU64, Ordering}; + +static TEMP_COUNTER: AtomicU64 = AtomicU64::new(0); + +fn collision(path: &str, reason: impl Into) -> PithosError { + PithosError::ExtractionCollision { + path: path.into(), + reason: reason.into(), + } +} + +pub(crate) struct ExtractionRoot { + root: Dir, +} + +impl ExtractionRoot { + pub(crate) fn open(path: &Path, create: bool) -> Result { + if create { + std::fs::create_dir_all(path)?; + } + Ok(Self { + root: Dir::open_ambient_dir(path, cap_std::ambient_authority())?, + }) + } + + fn parents(&self, path: &str) -> Result<(Dir, String), PithosError> { + let mut components = path.split('/').peekable(); + let final_name = components + .next_back() + .ok_or_else(|| collision(path, "empty final component"))?; + let mut dir = self.root.try_clone()?; + for component in components { + if component.is_empty() || component == "." || component == ".." { + return Err(collision(path, "invalid component")); + } + match dir.symlink_metadata(component) { + Ok(metadata) if metadata.file_type().is_symlink() => { + return Err(collision(path, "parent is a symlink")); + } + Ok(metadata) if !metadata.is_dir() => { + return Err(collision(path, "parent is not a directory")); + } + Ok(_) => dir = dir.open_dir(component)?, + Err(error) if error.kind() == io::ErrorKind::NotFound => { + dir.create_dir(component)?; + dir = dir.open_dir(component)?; + } + Err(error) => return Err(error.into()), + } + } + Ok((dir, final_name.to_string())) + } + + pub(crate) fn create_dir(&self, path: &str) -> Result<(), PithosError> { + let (parent, name) = self.parents(path)?; + match parent.symlink_metadata(&name) { + Ok(_) => Err(collision(path, "final entry already exists")), + Err(error) if error.kind() == io::ErrorKind::NotFound => { + parent.create_dir(&name).map_err(Into::into) + } + Err(error) => Err(error.into()), + } + } + + pub(crate) fn create_symlink(&self, path: &str, target: &str) -> Result<(), PithosError> { + let (parent, name) = self.parents(path)?; + match parent.symlink_metadata(&name) { + Ok(_) => return Err(collision(path, "final entry already exists")), + Err(error) if error.kind() == io::ErrorKind::NotFound => {} + Err(error) => return Err(error.into()), + } + parent.symlink(target, &name).map_err(Into::into) + } + + pub(crate) fn pending_file(&self, path: &str) -> Result { + let (parent, name) = self.parents(path)?; + match parent.symlink_metadata(&name) { + Ok(_) => return Err(collision(path, "final entry already exists")), + Err(error) if error.kind() == io::ErrorKind::NotFound => {} + Err(error) => return Err(error.into()), + } + let pid = std::process::id(); + loop { + let temp = + format! {".pithos-tmp-{pid}-{}", TEMP_COUNTER.fetch_add(1, Ordering::Relaxed)}; + let mut options = OpenOptions::new(); + options.write(true).create_new(true); + match parent.open_with(&temp, &options) { + Ok(file) => { + return Ok(PendingFile { + parent, + temp, + final_name: name, + file, + }); + } + Err(error) if error.kind() == io::ErrorKind::AlreadyExists => continue, + Err(error) => return Err(error.into()), + } + } + } +} + +pub(crate) struct PendingFile { + parent: Dir, + temp: String, + final_name: String, + file: cap_std::fs::File, +} + +impl PendingFile { + pub(crate) fn writer(&self) -> Result { + Ok(self.file.try_clone()?) + } + + pub(crate) fn commit(mut self) -> Result<(), PithosError> { + self.file.sync_all()?; + match self + .parent + .hard_link(&self.temp, &self.parent, &self.final_name) + { + Ok(()) => { + self.parent.remove_file(&self.temp)?; + self.temp.clear(); + Ok(()) + } + Err(error) if error.kind() == io::ErrorKind::AlreadyExists => { + Err(collision(&self.final_name, "final entry already exists")) + } + Err(error) => Err(error.into()), + } + } +} + +impl Write for PendingFile { + fn write(&mut self, buf: &[u8]) -> io::Result { + self.file.write(buf) + } + fn flush(&mut self) -> io::Result<()> { + self.file.flush() + } +} + +impl Drop for PendingFile { + fn drop(&mut self) { + if !self.temp.is_empty() { + let _ = self.parent.remove_file(&self.temp); + } + } +} diff --git a/crates/pithos_lib/src/io/mod.rs b/crates/pithos_lib/src/io/mod.rs index 4fe93b8..4e238b2 100644 --- a/crates/pithos_lib/src/io/mod.rs +++ b/crates/pithos_lib/src/io/mod.rs @@ -1,3 +1,4 @@ +pub(crate) mod extraction; pub mod pithosreader; pub mod pithoswriter; pub mod util; diff --git a/crates/pithos_lib/src/io/pithosreader.rs b/crates/pithos_lib/src/io/pithosreader.rs index f898f9d..79a322a 100644 --- a/crates/pithos_lib/src/io/pithosreader.rs +++ b/crates/pithos_lib/src/io/pithosreader.rs @@ -1,10 +1,11 @@ use crate::error::PithosError; +use crate::helpers::archive_path::{validate_entry, validate_map}; use crate::helpers::chacha_poly1305::{decrypt_chunk, encrypt_chunk}; use crate::helpers::crypt4gh::{CRYPT4GH_BLOCK_SIZE, Crypt4GHHeader, HeaderPacket}; use crate::helpers::file_entry_map::KeyQuery; use crate::helpers::x25519_keys::private_key_from_pem_bytes; use crate::helpers::zstd::decompress_data; -use crate::io::util::{create_dir, create_symlink}; +use crate::io::extraction::ExtractionRoot; use crate::model::structs::{ BlockDataState, BlockHeader, BlockIndexEntry, BlockLocation, Directory, FileEntry, FileType, }; @@ -126,6 +127,7 @@ impl PithosReaderSimple { None => false, }, })?; + validate_map(&directory.files)?; Ok((directory, (parent_dir_start, parent_dir_len))) } @@ -153,6 +155,14 @@ impl PithosReaderSimple { output_path: Option<&PathBuf>, ranges: Option>>, ) -> Result<(), PithosError> { + validate_map(&directory.files)?; + validate_entry( + inner_path, + directory + .files + .get(&KeyQuery::Path(inner_path.to_string())) + .ok_or(PithosError::FileNotFound(inner_path.to_string()))?, + )?; let file_entry = directory .files .get(&KeyQuery::Path(inner_path.to_string())) @@ -160,19 +170,38 @@ impl PithosReaderSimple { match &file_entry.file_type { FileType::Data | FileType::Metadata => { - // Write output - let mut output_target: Box = if let Some(dest) = output_path { - let target = if dest.is_dir() { - &dest.join(inner_path) + if output_path.is_none() { + let mut output_target: Box = Box::new(io::stdout()); + if let Some(ranges) = ranges { + for range in ranges { + self.read_data_range_to_sink( + range, + file_entry, + &directory.blocks, + &mut output_target, + )?; + } } else { - dest - }; - - Box::new(File::create(target).map_err(PithosError::Io)?) + self.read_data_to_sink(file_entry, &directory.blocks, output_target)?; + } + return Ok(()); + } + let dest = output_path.unwrap(); + let (root_path, final_path, create_root) = if dest.is_dir() { + (dest.as_path(), inner_path.to_string(), false) } else { - Box::new(io::stdout()) + let parent = dest.parent().unwrap_or_else(|| Path::new(".")); + let name = + dest.file_name() + .and_then(|name| name.to_str()) + .ok_or_else(|| { + PithosError::Conversion("Invalid output file name".into()) + })?; + (parent, name.to_string(), false) }; - + let root = ExtractionRoot::open(root_path, create_root)?; + let pending = root.pending_file(&final_path)?; + let mut output_target: Box = Box::new(pending.writer()?); if let Some(ranges) = ranges { for range in ranges { self.read_data_range_to_sink( @@ -185,21 +214,25 @@ impl PithosReaderSimple { } else { self.read_data_to_sink(file_entry, &directory.blocks, output_target)?; } + pending.commit()?; } FileType::Directory => { - // Create directory (parent?) - create_dir(inner_path, output_path)?; + let root_path = output_path + .map(PathBuf::as_path) + .unwrap_or_else(|| Path::new(".")); + ExtractionRoot::open(root_path, true)?.create_dir(inner_path)?; } FileType::Symlink => { - // Create symlink (UNIX only) - create_symlink( - inner_path, - file_entry - .symlink_target - .as_ref() - .expect("Symlink has no target"), - output_path, - )?; + let target = file_entry.symlink_target.as_deref().ok_or_else(|| { + PithosError::InvalidSymlinkEntry { + path: inner_path.into(), + reason: "missing target".into(), + } + })?; + let root_path = output_path + .map(PathBuf::as_path) + .unwrap_or_else(|| Path::new(".")); + ExtractionRoot::open(root_path, true)?.create_symlink(inner_path, target)?; } } diff --git a/crates/pithos_lib/src/io/pithoswriter.rs b/crates/pithos_lib/src/io/pithoswriter.rs index b133108..d2195ab 100644 --- a/crates/pithos_lib/src/io/pithoswriter.rs +++ b/crates/pithos_lib/src/io/pithoswriter.rs @@ -1,4 +1,5 @@ use crate::error::PithosError; +use crate::helpers::archive_path::{validate_candidate, validate_map}; use crate::helpers::directory::DirectoryBuilder; use crate::helpers::file_entry_map::{FileEntryMap, Key}; use crate::helpers::hash::{Hasher, Hashes}; @@ -51,8 +52,7 @@ impl TryFrom<&PathBuf> for InputFile { type Error = PithosError; fn try_from(file_path: &PathBuf) -> Result { - let file = File::open(file_path)?; - let metadata = file.metadata()?; + let metadata = symlink_metadata(file_path)?; let file_type = FileType::try_from(&metadata)?; let file_path_str = file_path .to_str() @@ -68,7 +68,10 @@ impl TryFrom<&PathBuf> for InputFile { Ok(InputFile { file_type, inner_path: inner_path.to_string(), - data: if file_type == FileType::Data { + data: if file_type == FileType::Data + || file_type == FileType::Directory + || file_type == FileType::Symlink + { Content::File(file_path_str) } else { Content::Raw("".to_string()) @@ -323,6 +326,7 @@ impl PithosWriter { processing_flags: &ProcessingFlags, content: R, ) -> Result { + validate_candidate(&self.directory.files, entry_path, file_entry)?; // Directory or Symlink FileEntry are just added to Pithos directory let file_entry_key = Key::new( self.directory.next_free_file_index(), @@ -372,8 +376,25 @@ impl PithosWriter { #[tracing::instrument(level = "trace", skip(self, input))] pub fn process_input(&mut self, input: InputFile) -> Result { + let data_check = FileEntry::new_from_content(input.file_type, &input.data)?; + validate_candidate(&self.directory.files, &input.inner_path, &data_check)?; + let mut preflight = self.directory.files.clone(); + preflight.insert( + Key::new(preflight.next_free_id(false), input.inner_path.clone()), + data_check.clone(), + )?; + if let Some(metadata) = &input.metadata + && !matches!(metadata, Content::Reference(_)) + { + let metadata_check = FileEntry::new_from_content(FileType::Metadata, metadata)?; + validate_candidate( + &preflight, + &format!("{}.meta", input.inner_path), + &metadata_check, + )?; + } // Create FileEntry with its ProcessingFlags from data file input - let mut data_file = FileEntry::new_from_content(input.file_type, &input.data)?; + let mut data_file = data_check; let processing_flags = ProcessingFlags::new(input.encrypt, input.compression_level); // First process metadata to add reference @@ -409,7 +430,9 @@ impl PithosWriter { // Process data FileEntry let data_reference = match input.data { - Content::File(disk_path) => { + Content::File(disk_path) + if [FileType::Data, FileType::Metadata].contains(&input.file_type) => + { let handle = File::open(disk_path)?; self.process_file_entry( &input.inner_path, @@ -418,6 +441,12 @@ impl PithosWriter { handle, )? } + Content::File(_) => self.process_file_entry( + &input.inner_path, + &mut data_file, + &processing_flags, + Cursor::new(Vec::::new()), + )?, Content::Raw(raw_content) => { let handle = Cursor::new(raw_content.into_bytes()); self.process_file_entry( @@ -540,6 +569,7 @@ impl PithosWriter { #[tracing::instrument(level = "trace", skip(self))] pub fn write_directory(&mut self) -> Result<(), PithosError> { + validate_map(&self.directory.files)?; // Encrypt recipients of writer section self.directory.encrypt_recipients(&self.writer_key)?; diff --git a/crates/pithos_lib/src/io/util.rs b/crates/pithos_lib/src/io/util.rs index a249d8b..1194c73 100644 --- a/crates/pithos_lib/src/io/util.rs +++ b/crates/pithos_lib/src/io/util.rs @@ -1,4 +1,6 @@ use crate::error::PithosError; +use crate::helpers::archive_path::{validate_entry_path, validate_symlink_target}; +use crate::io::extraction::ExtractionRoot; use fastcdc::v2020::{Normalization, StreamCDC}; use std::env::current_dir; use std::io::Read; @@ -27,15 +29,9 @@ pub fn get_symlink_target(file: &std::fs::File) -> Result { #[tracing::instrument(level = "trace", skip(path, base_dir))] pub fn create_dir(path: &str, base_dir: Option<&PathBuf>) -> Result<(), PithosError> { - // If no base dir provided create directory hierarchy in current working directory - let path = if let Some(base_dir) = base_dir { - base_dir.join(path) - } else { - current_dir()?.join(path) - }; - std::fs::create_dir_all(path)?; - - Ok(()) + validate_entry_path(path)?; + let root = base_dir.cloned().unwrap_or(current_dir()?); + ExtractionRoot::open(&root, true)?.create_dir(path) } #[tracing::instrument(level = "trace", skip(path, target, base_dir))] @@ -44,16 +40,10 @@ pub fn create_symlink( target: &str, base_dir: Option<&PathBuf>, ) -> Result<(), PithosError> { - // Resolve only the link location. The target is stored verbatim so relative - // targets remain relative to the created symlink. - let path = if let Some(base_dir) = base_dir { - base_dir.join(path) - } else { - current_dir()?.join(path) - }; - std::os::unix::fs::symlink(target, path)?; - - Ok(()) + validate_entry_path(path)?; + validate_symlink_target(path, target)?; + let root = base_dir.cloned().unwrap_or(current_dir()?); + ExtractionRoot::open(&root, true)?.create_symlink(path, target) } #[tracing::instrument(level = "trace", skip(content, cdc))] @@ -115,15 +105,11 @@ mod tests { } #[test] - fn create_dir_creates_nested_paths_with_and_without_a_base_directory() { + fn create_dir_creates_nested_paths_with_a_base_directory() { let temp_dir = TempDir::new().unwrap(); let base_dir = temp_dir.path().join("base"); create_dir("nested/path", Some(&base_dir)).unwrap(); assert!(base_dir.join("nested/path").is_dir()); - - let absolute_path = temp_dir.path().join("absolute/path"); - create_dir(absolute_path.to_str().unwrap(), None).unwrap(); - assert!(absolute_path.is_dir()); } #[test] @@ -132,9 +118,10 @@ mod tests { let base_dir = temp_dir.path().join("output"); fs::create_dir(&base_dir).unwrap(); - create_symlink("link", "../target.txt", Some(&base_dir)).unwrap(); + create_dir("nested", Some(&base_dir)).unwrap(); + create_symlink("nested/link", "../target.txt", Some(&base_dir)).unwrap(); - let link = base_dir.join("link"); + let link = base_dir.join("nested/link"); assert!( fs::symlink_metadata(&link) .unwrap() @@ -145,22 +132,13 @@ mod tests { } #[test] - fn create_symlink_preserves_absolute_targets_without_creating_them() { + fn create_symlink_rejects_absolute_targets() { let temp_dir = TempDir::new().unwrap(); let base_dir = temp_dir.path().join("output"); let target = temp_dir.path().join("outside-target"); fs::create_dir(&base_dir).unwrap(); - create_symlink("link", target.to_str().unwrap(), Some(&base_dir)).unwrap(); - - let link = base_dir.join("link"); - assert!( - fs::symlink_metadata(&link) - .unwrap() - .file_type() - .is_symlink() - ); - assert_eq!(fs::read_link(link).unwrap(), target); + assert!(create_symlink("link", target.to_str().unwrap(), Some(&base_dir)).is_err()); assert!(fs::symlink_metadata(temp_dir.path().join("outside-target")).is_err()); } diff --git a/crates/pithos_lib/src/model/deserialization.rs b/crates/pithos_lib/src/model/deserialization.rs index 8a5b6ed..d3ee6cf 100644 --- a/crates/pithos_lib/src/model/deserialization.rs +++ b/crates/pithos_lib/src/model/deserialization.rs @@ -7,6 +7,7 @@ // - Error handling via DeserializationError use crate::error::PithosError; +use crate::helpers::archive_path::{validate_entry, validate_map}; use crate::helpers::file_entry_map::{FileEntryMap, Key}; use crate::model::structs::*; use byteorder::{BigEndian, ReadBytesExt}; @@ -163,8 +164,11 @@ impl Directory { for _ in 0..files_len { let id = reader.read_varint::()?; let path = decode_string(reader)?; - files.insert(Key::new(id, path), FileEntry::deserialize(reader)?)?; + let entry = FileEntry::deserialize(reader)?; + validate_entry(&path, &entry)?; + files.insert(Key::new(id, path), entry)?; } + validate_map(&files)?; let blocks_len = reader.read_varint::()?; let mut blocks = IndexMap::new(); diff --git a/crates/pithos_lib/tests/marshalling.rs b/crates/pithos_lib/tests/marshalling.rs index 9e5cf2d..224dbfd 100644 --- a/crates/pithos_lib/tests/marshalling.rs +++ b/crates/pithos_lib/tests/marshalling.rs @@ -107,7 +107,7 @@ fn directory_roundtrip() { target_file_id: 2, relationship: 3, }], - symlink_target: Some("target".to_string()), + symlink_target: None, }; let block_index = BlockIndexEntry { offset: 4, @@ -182,6 +182,76 @@ fn directory_builder_crc_matches_recalculation() { assert_eq!(directory.crc32, recalculated.crc32); } +fn serialized_directory(path: &str, entry: FileEntry) -> Vec { + let mut files = FileEntryMap::new(); + files.insert(Key::new(0, path), entry).unwrap(); + let directory = DirectoryBuilder::new().files(files).build().unwrap(); + let mut bytes = Vec::new(); + directory.serialize(&mut bytes).unwrap(); + bytes +} + +fn plain_entry(target: Option<&str>) -> FileEntry { + FileEntry { + file_type: FileType::Data, + block_data: BlockDataState::Decrypted(vec![]), + created: 0, + modified: 0, + file_size: 0, + permissions: 0o644, + references: vec![], + symlink_target: target.map(str::to_owned), + } +} + +#[test] +fn directory_deserialization_rejects_unsafe_entry_paths() { + for path in ["../outside", "/absolute", "nested//file", "C:/file"] { + assert!( + Directory::deserialize(&mut Cursor::new(serialized_directory( + path, + plain_entry(None) + ))) + .is_err() + ); + } +} + +#[test] +fn directory_deserialization_rejects_missing_inconsistent_symlink_targets() { + let mut missing = plain_entry(None); + missing.file_type = FileType::Symlink; + assert!( + Directory::deserialize(&mut Cursor::new(serialized_directory("link", missing))).is_err() + ); + assert!( + Directory::deserialize(&mut Cursor::new(serialized_directory( + "file", + plain_entry(Some("target")) + ))) + .is_err() + ); + let mut blocks = plain_entry(Some("target")); + blocks.file_type = FileType::Symlink; + blocks.block_data = BlockDataState::Decrypted(vec![([0; 32], [0; 32])]); + assert!( + Directory::deserialize(&mut Cursor::new(serialized_directory("link", blocks))).is_err() + ); + let mut encrypted = plain_entry(Some("target")); + encrypted.file_type = FileType::Symlink; + encrypted.block_data = BlockDataState::Encrypted(vec![]); + assert!( + Directory::deserialize(&mut Cursor::new(serialized_directory("link", encrypted))).is_err() + ); +} + +#[test] +fn directory_deserialization_rejects_unsafe_symlink_targets() { + let mut link = plain_entry(Some("../outside")); + link.file_type = FileType::Symlink; + assert!(Directory::deserialize(&mut Cursor::new(serialized_directory("link", link))).is_err()); +} + #[test] fn file_type_roundtrip() { for ft in [ diff --git a/crates/pithos_lib/tests/reader.rs b/crates/pithos_lib/tests/reader.rs index b1e946c..7c3170b 100644 --- a/crates/pithos_lib/tests/reader.rs +++ b/crates/pithos_lib/tests/reader.rs @@ -1,15 +1,17 @@ pub mod common; -use crate::common::util::write_dummy_pithos; +use crate::common::util::{create_pithos_writer, write_dummy_pithos}; use pithos_lib::error::PithosError; use pithos_lib::helpers::chacha_poly1305::decrypt_chunk; use pithos_lib::helpers::crypt4gh::{ CRYPT4GH_ENCRYPTED_BLOCK_SIZE, Crypt4GHHeader, Packet, PacketData, }; +use pithos_lib::helpers::file_entry_map::Key; use pithos_lib::helpers::ro_crate::{RoCrateSource, read_ro_crate_directory, read_ro_crate_zip}; use pithos_lib::helpers::x25519_keys::{private_key_from_pem_bytes, public_key_from_pem_bytes}; use pithos_lib::io::pithosreader::PithosReaderSimple; -use pithos_lib::model::structs::{FileType, Reference}; +use pithos_lib::io::pithoswriter::{Content, InputFile}; +use pithos_lib::model::structs::{BlockDataState, FileEntry, FileType, Reference}; use rocraters::ro_crate::graph_vector::GraphVector; use rocraters::ro_crate::read::CrateReadError; use rocraters::ro_crate::schema::RoCrateSchemaVersion; @@ -19,6 +21,43 @@ use std::path::{Path, PathBuf}; use tempfile::TempDir; use x25519_dalek::StaticSecret; +fn reader_key() -> StaticSecret { + private_key_from_pem_bytes( + std::fs::read("tests/data/keys/recipient1_private.pem") + .unwrap() + .as_slice(), + ) + .unwrap() +} + +fn empty_entry(file_type: FileType, target: Option<&str>) -> FileEntry { + FileEntry { + file_type, + block_data: BlockDataState::Decrypted(vec![]), + created: 0, + modified: 0, + file_size: 0, + permissions: 0o644, + references: vec![], + symlink_target: target.map(str::to_owned), + } +} + +fn caller_directory( + archive: &Path, + entry_path: &str, + file_type: FileType, + target: Option<&str>, +) -> (PithosReaderSimple, pithos_lib::model::structs::Directory) { + let mut reader = PithosReaderSimple::new_with_key(archive, reader_key()).unwrap(); + let (mut directory, _) = reader.read_directory().unwrap(); + directory + .files + .insert(Key::new(9000, entry_path), empty_entry(file_type, target)) + .unwrap(); + (reader, directory) +} + fn write_zip_entry(path: &Path, name: &str, content: &[u8]) { let file = File::create(path).unwrap(); let mut archive = zip::ZipWriter::new(file); @@ -45,6 +84,227 @@ fn test_reader_single_file() { assert_eq!(inner_paths[0].1, "t8.shakespeare.txt"); } +#[test] +fn test_safe_extraction_rejects_caller_constructed_unsafe_paths() { + let temp_dir = TempDir::new().unwrap(); + let pithos_file = write_dummy_pithos(&temp_dir, false, false); + let key = private_key_from_pem_bytes( + std::fs::read("tests/data/keys/recipient1_private.pem") + .unwrap() + .as_slice(), + ) + .unwrap(); + let mut reader = PithosReaderSimple::new_with_key(&pithos_file, key).unwrap(); + let (mut directory, _) = reader.read_directory().unwrap(); + let entry = FileEntry { + file_type: FileType::Data, + block_data: BlockDataState::Decrypted(vec![]), + created: 0, + modified: 0, + file_size: 0, + permissions: 0o644, + references: vec![], + symlink_target: None, + }; + directory + .files + .insert(Key::new(999, "../outside"), entry.clone()) + .unwrap(); + directory + .files + .insert(Key::new(1000, "/absolute"), entry) + .unwrap(); + let output = temp_dir.path().join("output"); + std::fs::create_dir(&output).unwrap(); + assert!( + reader + .read_file("../outside", &directory, Some(&output), None) + .is_err() + ); + assert!( + reader + .read_file("/absolute", &directory, Some(&output), None) + .is_err() + ); + assert!(!temp_dir.path().join("outside").exists()); +} + +#[test] +fn test_safe_extraction_rejects_preexisting_parent_symlink() { + let temp = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp, false, false); + let outside = temp.path().join("outside"); + std::fs::create_dir(&outside).unwrap(); + let output = temp.path().join("output"); + std::fs::create_dir(&output).unwrap(); + std::os::unix::fs::symlink(&outside, output.join("nested")).unwrap(); + let (mut reader, directory) = caller_directory(&archive, "nested/file", FileType::Data, None); + assert!( + reader + .read_file("nested/file", &directory, Some(&output), None) + .is_err() + ); + assert!(!outside.join("file").exists()); +} + +#[test] +fn test_safe_extraction_rejects_existing_final_symlink() { + let temp = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp, false, false); + let outside = temp.path().join("outside"); + std::fs::write(&outside, b"unchanged").unwrap(); + let output = temp.path().join("output"); + std::fs::create_dir(&output).unwrap(); + std::os::unix::fs::symlink(&outside, output.join("file")).unwrap(); + let (mut reader, directory) = caller_directory(&archive, "file", FileType::Data, None); + assert!( + reader + .read_file("file", &directory, Some(&output), None) + .is_err() + ); + assert_eq!(std::fs::read(&outside).unwrap(), b"unchanged"); +} + +#[test] +fn test_safe_extraction_rejects_existing_final_regular_file() { + let temp = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp, false, false); + let output = temp.path().join("output"); + std::fs::create_dir(&output).unwrap(); + std::fs::write(output.join("file"), b"unchanged").unwrap(); + let (mut reader, directory) = caller_directory(&archive, "file", FileType::Data, None); + assert!( + reader + .read_file("file", &directory, Some(&output), None) + .is_err() + ); + assert_eq!(std::fs::read(output.join("file")).unwrap(), b"unchanged"); +} + +#[test] +fn test_safe_extraction_rejects_existing_final_directory_for_data() { + let temp = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp, false, false); + let output = temp.path().join("output"); + std::fs::create_dir_all(output.join("file")).unwrap(); + let (mut reader, directory) = caller_directory(&archive, "file", FileType::Data, None); + assert!( + reader + .read_file("file", &directory, Some(&output), None) + .is_err() + ); + assert!(output.join("file").is_dir()); +} + +#[test] +fn test_safe_extraction_cleans_pending_file_after_read_failure() { + let temp = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp, false, false); + let output = temp.path().join("output"); + std::fs::create_dir(&output).unwrap(); + let (mut reader, mut directory) = caller_directory(&archive, "failed", FileType::Data, None); + for (_, path, entry) in directory.files.iter_mut() { + if path == "failed" { + entry.block_data = BlockDataState::Decrypted(vec![([9; 32], [0; 32])]); + } + } + assert!( + reader + .read_file("failed", &directory, Some(&output), None) + .is_err() + ); + assert!(!output.join("failed").exists()); + assert!(!std::fs::read_dir(&output).unwrap().any(|item| { + item.unwrap() + .file_name() + .to_string_lossy() + .starts_with(".pithos-tmp-") + })); +} + +#[test] +fn test_safe_extraction_extracts_safe_nested_regular_file() { + let temp = TempDir::new().unwrap(); + let (archive, key, mut writer) = create_pithos_writer(&temp, None); + writer.write_file_header().unwrap(); + writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "nested/file".into(), + data: Content::Raw("nested payload".into()), + metadata: None, + encrypt: false, + compression_level: Some(0), + }) + .unwrap(); + writer.write_directory().unwrap(); + let mut reader = PithosReaderSimple::new_with_key(&archive, key).unwrap(); + let (directory, _) = reader.read_directory().unwrap(); + let output = temp.path().join("output"); + std::fs::create_dir(&output).unwrap(); + reader + .read_file("nested/file", &directory, Some(&output), None) + .unwrap(); + assert_eq!( + std::fs::read(output.join("nested/file")).unwrap(), + b"nested payload" + ); +} + +#[test] +fn test_safe_extraction_creates_contained_and_dangling_symlinks() { + let temp = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp, false, false); + let output = temp.path().join("output"); + std::fs::create_dir_all(output.join("nested")).unwrap(); + let (mut reader, mut directory) = caller_directory( + &archive, + "nested/link", + FileType::Symlink, + Some("../target"), + ); + directory + .files + .insert( + Key::new(9001, "dangling"), + empty_entry(FileType::Symlink, Some("missing/target")), + ) + .unwrap(); + reader + .read_file("nested/link", &directory, Some(&output), None) + .unwrap(); + reader + .read_file("dangling", &directory, Some(&output), None) + .unwrap(); + assert_eq!( + std::fs::read_link(output.join("nested/link")).unwrap(), + PathBuf::from("../target") + ); + assert_eq!( + std::fs::read_link(output.join("dangling")).unwrap(), + PathBuf::from("missing/target") + ); +} + +#[test] +fn test_safe_extraction_rejects_caller_constructed_ancestor_conflict() { + let temp = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp, false, false); + let output = temp.path().join("output"); + std::fs::create_dir(&output).unwrap(); + let (mut reader, mut directory) = caller_directory(&archive, "a", FileType::Data, None); + directory + .files + .insert(Key::new(9001, "a/child"), empty_entry(FileType::Data, None)) + .unwrap(); + assert!( + reader + .read_file("a", &directory, Some(&output), None) + .is_err() + ); + assert!(!output.join("a").exists()); +} + #[test] fn test_reader_hides_files_without_keys() { let temp_dir = TempDir::new().unwrap(); diff --git a/crates/pithos_lib/tests/writer.rs b/crates/pithos_lib/tests/writer.rs index 64aac59..6af7531 100644 --- a/crates/pithos_lib/tests/writer.rs +++ b/crates/pithos_lib/tests/writer.rs @@ -12,8 +12,7 @@ use pithos_lib::io::pithosreader::PithosReaderSimple; use pithos_lib::io::pithoswriter::{Content, InputFile, PithosWriter}; use pithos_lib::model::structs::{FileType, Reference}; use std::fs::{ - File, copy, create_dir_all, read, read_dir, read_link, read_to_string, remove_file, - symlink_metadata, write, + File, copy, create_dir_all, read, read_dir, read_link, read_to_string, remove_file, write, }; use std::io::Read; use std::path::{Path, PathBuf}; @@ -805,8 +804,8 @@ fn test_rocrate_zip_synthesizes_parent_directories() { fn test_rocrate_zip_preserves_and_extracts_symlink() { let temp_dir = TempDir::new().unwrap(); let zip_path = temp_dir.path().join("symlink.zip"); - let target = temp_dir.path().join("outside-target"); - let target_string = target.to_string_lossy().into_owned(); + let target = PathBuf::from("target"); + let target_string = "target"; let metadata = minimal_ro_crate_metadata(&[]); write_raw_zip( &zip_path, @@ -823,10 +822,7 @@ fn test_rocrate_zip_preserves_and_extracts_symlink() { let entry = directory.get_file_by_path("link").unwrap(); assert_eq!(entry.file_type, FileType::Symlink); - assert_eq!( - entry.symlink_target.as_deref(), - Some(target_string.as_str()) - ); + assert_eq!(entry.symlink_target.as_deref(), Some(target_string)); assert!(entry.references.is_empty()); let output_dir = temp_dir.path().join("extracted"); @@ -838,7 +834,119 @@ fn test_rocrate_zip_preserves_and_extracts_symlink() { .unwrap(); assert_eq!(read_link(output_dir.join("link")).unwrap(), target); - assert!(symlink_metadata(temp_dir.path().join("outside-target")).is_err()); +} + +#[test] +fn test_writer_rejects_unsafe_paths_before_block_output() { + let temp_dir = TempDir::new().unwrap(); + let (path, _key, mut writer) = create_pithos_writer(&temp_dir, None); + writer.write_file_header().unwrap(); + let before = std::fs::metadata(&path).unwrap().len(); + let result = writer.process_input(InputFile { + file_type: FileType::Data, + inner_path: "../outside".into(), + data: Content::Raw("payload".into()), + metadata: None, + encrypt: false, + compression_level: Some(0), + }); + assert!(matches!( + result, + Err(PithosError::InvalidArchivePath { .. }) + )); + assert_eq!(std::fs::metadata(&path).unwrap().len(), before); +} + +#[test] +fn test_writer_rejects_candidate_ancestor_before_metadata_output() { + let temp_dir = TempDir::new().unwrap(); + let (path, _key, mut writer) = create_pithos_writer(&temp_dir, None); + writer.write_file_header().unwrap(); + writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "a".into(), + data: Content::Raw("existing".into()), + metadata: None, + encrypt: false, + compression_level: Some(0), + }) + .unwrap(); + let before = std::fs::metadata(&path).unwrap().len(); + assert!( + writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "a/child".into(), + data: Content::Raw("child".into()), + metadata: Some(Content::Raw("metadata".into())), + encrypt: false, + compression_level: Some(0), + }) + .is_err() + ); + assert_eq!(std::fs::metadata(&path).unwrap().len(), before); +} + +#[test] +fn test_writer_rejects_unsafe_symlink_targets() { + let temp_dir = TempDir::new().unwrap(); + let (_path, _key, mut writer) = create_pithos_writer(&temp_dir, None); + let mut entry = pithos_lib::model::structs::FileEntry { + file_type: FileType::Symlink, + block_data: pithos_lib::model::structs::BlockDataState::Decrypted(vec![]), + created: 0, + modified: 0, + file_size: 0, + permissions: 0o777, + references: vec![], + symlink_target: Some("/outside".into()), + }; + assert!(matches!( + writer.process_file_entry( + "link", + &mut entry, + &pithos_lib::model::structs::ProcessingFlags(0), + std::io::Cursor::new(Vec::::new()) + ), + Err(PithosError::InvalidSymlinkTarget { .. }) + )); +} + +#[test] +fn test_writer_rejects_unsafe_rocrate_absolute_symlink_target() { + let temp_dir = TempDir::new().unwrap(); + let zip_path = temp_dir.path().join("unsafe-link.zip"); + let metadata = minimal_ro_crate_metadata(&[]); + write_raw_zip( + &zip_path, + &[ + ("ro-crate-metadata.json", metadata.as_bytes(), 0), + ("link", b"/outside", 0o120777), + ], + false, + ); + let loaded = read_ro_crate_zip(&zip_path).unwrap(); + let (_path, _key, mut writer) = create_pithos_writer(&temp_dir, None); + assert!(writer.process_ro_crate(&loaded).is_err()); +} + +#[test] +fn test_writer_rejects_unsafe_rocrate_root_escaping_symlink_target() { + let temp_dir = TempDir::new().unwrap(); + let zip_path = temp_dir.path().join("unsafe-link.zip"); + let metadata = minimal_ro_crate_metadata(&[]); + write_raw_zip( + &zip_path, + &[ + ("ro-crate-metadata.json", metadata.as_bytes(), 0), + ("link", b"../outside", 0o120777), + ], + false, + ); + let loaded = read_ro_crate_zip(&zip_path).unwrap(); + let (_path, _key, mut writer) = create_pithos_writer(&temp_dir, None); + assert!(writer.process_ro_crate(&loaded).is_err()); } #[test] @@ -952,9 +1060,11 @@ fn test_rocrate_directory_zip_parity() { assert_eq!(directory_entries, zip_entries); for path in ["ro-crate-metadata.json", "nested/file.txt", "unlisted.txt"] { + let directory_output = TempDir::new().unwrap(); + let zip_output = TempDir::new().unwrap(); let directory_bytes = - extract_pithos_entry(&directory_pithos, &directory_key, path, &temp_dir); - let zip_bytes = extract_pithos_entry(&zip_pithos, &zip_key, path, &temp_dir); + extract_pithos_entry(&directory_pithos, &directory_key, path, &directory_output); + let zip_bytes = extract_pithos_entry(&zip_pithos, &zip_key, path, &zip_output); assert_eq!(directory_bytes, zip_bytes, "{path}"); } assert_eq!( diff --git a/deny.toml b/deny.toml index 422b404..a99eca2 100644 --- a/deny.toml +++ b/deny.toml @@ -107,6 +107,7 @@ exceptions = [ { allow = ["CDLA-Permissive-2.0"], crate = "webpki-root-certs" }, { allow = ["MPL-2.0"], crate = "option-ext" }, { allow = ["Zlib"], crate = "zlib-rs" }, + { allow = ["Apache-2.0 WITH LLVM-exception"], crate = "winx" } ] # Some crates don't have (easily) machine readable licensing information, From a4e297328f461ccf781d8c76ae988a1a01e1949b Mon Sep 17 00:00:00 2001 From: Jannis Schlegel Date: Fri, 17 Jul 2026 22:22:38 +0200 Subject: [PATCH 03/14] fix: add strict directory integrity checks (marker, length, CRC) through normative specification details --- crates/pithos_lib/src/error.rs | 8 ++ crates/pithos_lib/src/helpers/directory.rs | 30 +---- crates/pithos_lib/src/io/pithosreader.rs | 52 ++++++-- .../pithos_lib/src/model/deserialization.rs | 8 ++ crates/pithos_lib/tests/marshalling.rs | 54 +++++++- crates/pithos_lib/tests/reader.rs | 120 +++++++++++++++++- spec/PITHOS_1.0.0_draft.md | 12 +- 7 files changed, 238 insertions(+), 46 deletions(-) diff --git a/crates/pithos_lib/src/error.rs b/crates/pithos_lib/src/error.rs index a2551dd..4675696 100644 --- a/crates/pithos_lib/src/error.rs +++ b/crates/pithos_lib/src/error.rs @@ -30,6 +30,14 @@ pub enum PithosError { Serialization(#[from] SerializationError), #[error("Deserialization error: {0:?}")] Deserialization(#[from] DeserializationError), + #[error("Invalid directory marker: expected {expected:?}, got {actual:?}")] + InvalidDirectoryMarker { expected: [u8; 8], actual: [u8; 8] }, + #[error("Directory length mismatch: expected {expected}, got {actual}")] + DirectoryLengthMismatch { expected: u64, actual: u64 }, + #[error("Directory checksum mismatch: expected {expected:#010x}, got {actual:#010x}")] + DirectoryChecksumMismatch { expected: u32, actual: u32 }, + #[error("Directory parser consumption mismatch: expected {expected}, got {actual}")] + DirectoryConsumptionMismatch { expected: u64, actual: u64 }, #[error("Crypt error: {0}")] Crypt(#[from] CryptError), #[error("Crypt4GH error: {0}")] diff --git a/crates/pithos_lib/src/helpers/directory.rs b/crates/pithos_lib/src/helpers/directory.rs index 2078a68..9166a99 100644 --- a/crates/pithos_lib/src/helpers/directory.rs +++ b/crates/pithos_lib/src/helpers/directory.rs @@ -1,6 +1,5 @@ use crate::error::PithosError; use crate::helpers::file_entry_map::{FileEntryMap, Key, KeyQuery}; -use crate::model::serialization::encode_string; use crate::model::structs::{RecipientData, RecipientSection}; use crate::model::{ serialization::SerializationError, @@ -9,8 +8,6 @@ use crate::model::{ use crc32fast::Hasher; use indexmap::IndexMap; use indexmap::map::Entry; -use integer_encoding::VarIntWriter; -use std::io::Write; use x25519_dalek::{PublicKey, StaticSecret}; pub struct DirectoryBuilder { @@ -435,34 +432,11 @@ impl Directory { #[tracing::instrument(level = "trace", skip(self))] pub fn update_crc32(&mut self) -> Result<(), SerializationError> { let mut buf = Vec::new(); - if let Some((start, len)) = self.parent_directory_offset { - buf.extend(&[1u8]); - buf.write_varint(start)?; - buf.write_varint(len)?; - } - for (id, path, file) in &self.files { - buf.write_varint(id)?; - encode_string(&mut buf, path)?; - file.serialize(&mut buf)? - } - for (hash, block) in &self.blocks { - buf.write_all(hash)?; - block.serialize(&mut buf)? - } - buf.write_varint(self.relations.len() as u64)?; - for (idx, name) in &self.relations { - buf.write_varint(*idx)?; - encode_string(&mut buf, name)?; - } - for (key, enc) in &self.encryption { - buf.write_all(key)?; - enc.serialize(&mut buf)? - } - buf.write_varint(self.dir_len)?; + self.serialize(&mut buf)?; // Calculate CRC32 checksum let mut hasher = Hasher::new(); - hasher.update(&buf); + hasher.update(&buf[..buf.len() - 4]); self.crc32 = hasher.finalize(); Ok(()) diff --git a/crates/pithos_lib/src/io/pithosreader.rs b/crates/pithos_lib/src/io/pithosreader.rs index 79a322a..7812451 100644 --- a/crates/pithos_lib/src/io/pithosreader.rs +++ b/crates/pithos_lib/src/io/pithosreader.rs @@ -9,10 +9,11 @@ use crate::io::extraction::ExtractionRoot; use crate::model::structs::{ BlockDataState, BlockHeader, BlockIndexEntry, BlockLocation, Directory, FileEntry, FileType, }; +use crc32fast::hash; use indexmap::IndexMap; use std::collections::HashMap; use std::fs::File; -use std::io::{self, Read, Seek, SeekFrom, Write}; +use std::io::{self, Cursor, Read, Seek, SeekFrom, Write}; use std::ops::Range; use std::path::{Path, PathBuf}; use x25519_dalek::{PublicKey, StaticSecret}; @@ -25,6 +26,46 @@ pub struct PithosReaderSimple { } impl PithosReaderSimple { + fn parse_directory(bytes: &[u8]) -> Result { + if bytes.len() < 25 { + return Err(PithosError::DirectoryLengthMismatch { + expected: 25, + actual: bytes.len() as u64, + }); + } + if bytes[..8] != Directory::DIRECTORY_MARKER { + return Err(PithosError::InvalidDirectoryMarker { + expected: Directory::DIRECTORY_MARKER, + actual: bytes[..8].try_into().unwrap(), + }); + } + let expected_len = + u64::from_be_bytes(bytes[bytes.len() - 12..bytes.len() - 4].try_into().unwrap()); + if expected_len != bytes.len() as u64 { + return Err(PithosError::DirectoryLengthMismatch { + expected: bytes.len() as u64, + actual: expected_len, + }); + } + let expected_crc = u32::from_be_bytes(bytes[bytes.len() - 4..].try_into().unwrap()); + let actual_crc = hash(&bytes[..bytes.len() - 4]); + if expected_crc != actual_crc { + return Err(PithosError::DirectoryChecksumMismatch { + expected: actual_crc, + actual: expected_crc, + }); + } + let mut cursor = Cursor::new(bytes); + let directory = Directory::deserialize(&mut cursor)?; + if cursor.position() != bytes.len() as u64 { + return Err(PithosError::DirectoryConsumptionMismatch { + expected: bytes.len() as u64, + actual: cursor.position(), + }); + } + Ok(directory) + } + /// Open a Pithos archive and prepare for reading #[tracing::instrument(level = "trace", skip(pithos_path, private_key_pem_path))] pub fn new>( @@ -80,18 +121,13 @@ impl PithosReaderSimple { })?); let parent_dir_start = file_len - parent_dir_len; - // Last 4 bytes: crc32, next 8 bytes: directory length (u64, BE) - let _crc32 = u32::from_be_bytes(footer[8..12].try_into().map_err(|_| { - PithosError::Conversion("Failed to convert crc32 checksum bytes to u32".to_string()) - })?); - self.file.seek(SeekFrom::End(0 - parent_dir_len as i64))?; let mut dir_buf = vec![0u8; parent_dir_len as usize]; self.file.read_exact(&mut dir_buf)?; // Deserialize full directory let mut available_file_keys = HashMap::new(); - let mut directory = Directory::deserialize(&mut dir_buf.as_slice())?; + let mut directory = Self::parse_directory(&dir_buf)?; available_file_keys.extend(directory.decrypt_recipient(&self.private_key)?); // Merge with parent directories @@ -100,7 +136,7 @@ impl PithosReaderSimple { self.file.seek(SeekFrom::Start(start))?; let mut dir_buf = vec![0u8; len as usize]; self.file.read_exact(&mut dir_buf)?; - let mut older_directory = Directory::deserialize(&mut dir_buf.as_slice())?; + let mut older_directory = Self::parse_directory(&dir_buf)?; available_file_keys.extend(older_directory.decrypt_recipient(&self.private_key)?); // Merge directories and swap diff --git a/crates/pithos_lib/src/model/deserialization.rs b/crates/pithos_lib/src/model/deserialization.rs index d3ee6cf..fa692be 100644 --- a/crates/pithos_lib/src/model/deserialization.rs +++ b/crates/pithos_lib/src/model/deserialization.rs @@ -139,11 +139,19 @@ impl BlockIndexEntry { // Directory impl Directory { + pub(crate) const DIRECTORY_MARKER: [u8; 8] = *b"PITHOSDR"; + #[tracing::instrument(level = "trace", skip(reader))] pub fn deserialize(reader: &mut R) -> Result { // Read static directory identifier let mut identifier = [0u8; 8]; reader.read_exact(&mut identifier)?; + if identifier != Self::DIRECTORY_MARKER { + return Err(PithosError::InvalidDirectoryMarker { + expected: Self::DIRECTORY_MARKER, + actual: identifier, + }); + } // Read parent directory offset let mut tag = [0u8; 1]; diff --git a/crates/pithos_lib/tests/marshalling.rs b/crates/pithos_lib/tests/marshalling.rs index 224dbfd..e6b67eb 100644 --- a/crates/pithos_lib/tests/marshalling.rs +++ b/crates/pithos_lib/tests/marshalling.rs @@ -1,4 +1,5 @@ use indexmap::IndexMap; +use pithos_lib::error::PithosError; use pithos_lib::helpers::directory::DirectoryBuilder; use pithos_lib::helpers::file_entry_map::{FileEntryMap, Key}; use pithos_lib::helpers::x25519_keys::generate_private_key; @@ -149,7 +150,7 @@ fn directory_roundtrip() { } #[test] -fn directory_builder_crc_matches_recalculation() { +fn directory_integrity_crc_matches_serialized_prefix() { let file_entry = FileEntry { file_type: FileType::Data, block_data: BlockDataState::Decrypted(vec![]), @@ -176,10 +177,55 @@ fn directory_builder_crc_matches_recalculation() { .blocks(blocks) .build() .unwrap(); - let mut recalculated = directory.clone(); - recalculated.update_crc32().unwrap(); + let mut serialized = Vec::new(); + directory.serialize(&mut serialized).unwrap(); + assert_eq!( + directory.crc32, + crc32fast::hash(&serialized[..serialized.len() - 4]) + ); +} + +#[test] +fn directory_integrity_minimal_vector_matches_specification() { + let directory = Directory { + identifier: *b"PITHOSDR", + parent_directory_offset: None, + files: FileEntryMap::new(), + blocks: IndexMap::new(), + relations: vec![], + encryption: IndexMap::new(), + dir_len: 25, + crc32: 0, + }; + let mut prefix = Vec::new(); + directory.serialize(&mut prefix).unwrap(); + let crc = crc32fast::hash(&prefix[..prefix.len() - 4]); + let mut complete = prefix.clone(); + let crc_start = complete.len() - 4; + complete[crc_start..].copy_from_slice(&crc.to_be_bytes()); + + assert_eq!(crc, 0xb1674081); + assert_eq!( + complete, + hex_bytes("504954484f53445200000000000000000000000019b1674081") + ); +} + +#[test] +fn directory_integrity_deserialization_rejects_invalid_marker() { + let mut bytes = hex_bytes("504954484f53445200000000000000000000000019b1674081"); + bytes[..8].copy_from_slice(b"INVALID!"); + let error = Directory::deserialize(&mut Cursor::new(bytes)).unwrap_err(); + + assert!(matches!(error, PithosError::InvalidDirectoryMarker { .. })); +} - assert_eq!(directory.crc32, recalculated.crc32); +fn hex_bytes(value: &str) -> Vec { + value + .as_bytes() + .chunks_exact(2) + .map(|pair| u8::from_str_radix(std::str::from_utf8(pair).unwrap(), 16).unwrap()) + .collect() } fn serialized_directory(path: &str, entry: FileEntry) -> Vec { diff --git a/crates/pithos_lib/tests/reader.rs b/crates/pithos_lib/tests/reader.rs index 7c3170b..57e1d2c 100644 --- a/crates/pithos_lib/tests/reader.rs +++ b/crates/pithos_lib/tests/reader.rs @@ -1,6 +1,6 @@ pub mod common; -use crate::common::util::{create_pithos_writer, write_dummy_pithos}; +use crate::common::util::{create_pithos_writer, load_test_keys, write_dummy_pithos}; use pithos_lib::error::PithosError; use pithos_lib::helpers::chacha_poly1305::decrypt_chunk; use pithos_lib::helpers::crypt4gh::{ @@ -10,12 +10,12 @@ use pithos_lib::helpers::file_entry_map::Key; use pithos_lib::helpers::ro_crate::{RoCrateSource, read_ro_crate_directory, read_ro_crate_zip}; use pithos_lib::helpers::x25519_keys::{private_key_from_pem_bytes, public_key_from_pem_bytes}; use pithos_lib::io::pithosreader::PithosReaderSimple; -use pithos_lib::io::pithoswriter::{Content, InputFile}; +use pithos_lib::io::pithoswriter::{Content, InputFile, PithosWriter}; use pithos_lib::model::structs::{BlockDataState, FileEntry, FileType, Reference}; use rocraters::ro_crate::graph_vector::GraphVector; use rocraters::ro_crate::read::CrateReadError; use rocraters::ro_crate::schema::RoCrateSchemaVersion; -use std::fs::{File, OpenOptions, read_to_string}; +use std::fs::{File, OpenOptions, read, read_to_string, write}; use std::io::{Read, Seek, SeekFrom, Write}; use std::path::{Path, PathBuf}; use tempfile::TempDir; @@ -68,6 +68,120 @@ fn write_zip_entry(path: &Path, name: &str, content: &[u8]) { archive.finish().unwrap(); } +fn append_empty_directory(path: &Path) { + let (writer_key, _, reader_key) = load_test_keys(); + let mut writer = PithosWriter::new_from_file(writer_key, vec![reader_key], None, path).unwrap(); + writer.write_directory().unwrap(); + drop(writer); +} + +fn directory_bounds(bytes: &[u8]) -> (usize, usize) { + let length = + u64::from_be_bytes(bytes[bytes.len() - 12..bytes.len() - 4].try_into().unwrap()) as usize; + (bytes.len() - length, length) +} + +#[test] +fn test_directory_integrity_valid_terminal_archive() { + let temp_dir = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp_dir, false, false); + let mut reader = PithosReaderSimple::new_with_key(&archive, reader_key()).unwrap(); + + assert!(reader.read_directory().is_ok()); +} + +#[test] +fn test_directory_integrity_terminal_marker_mutation() { + let temp_dir = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp_dir, false, false); + let mut bytes = read(&archive).unwrap(); + let (start, _) = directory_bounds(&bytes); + bytes[start] ^= 1; + write(&archive, bytes).unwrap(); + + let mut reader = PithosReaderSimple::new_with_key(&archive, reader_key()).unwrap(); + assert!(matches!( + reader.read_directory(), + Err(PithosError::InvalidDirectoryMarker { .. }) + )); +} + +#[test] +fn test_directory_integrity_terminal_crc_mutation() { + let temp_dir = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp_dir, false, false); + let mut bytes = read(&archive).unwrap(); + *bytes.last_mut().unwrap() ^= 1; + write(&archive, bytes).unwrap(); + + let mut reader = PithosReaderSimple::new_with_key(&archive, reader_key()).unwrap(); + assert!(matches!( + reader.read_directory(), + Err(PithosError::DirectoryChecksumMismatch { .. }) + )); +} + +#[test] +fn test_directory_integrity_rejects_extra_unconsumed_byte() { + let temp_dir = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp_dir, false, false); + let mut bytes = read(&archive).unwrap(); + let (start, length) = directory_bounds(&bytes); + let mut directory = bytes.split_off(start); + directory.insert(directory.len() - 12, 0); + let new_length = (length + 1) as u64; + let footer_start = directory.len() - 12; + directory[footer_start..footer_start + 8].copy_from_slice(&new_length.to_be_bytes()); + let crc_start = directory.len() - 4; + let crc = crc32fast::hash(&directory[..crc_start]); + directory[crc_start..].copy_from_slice(&crc.to_be_bytes()); + bytes.extend_from_slice(&directory); + write(&archive, bytes).unwrap(); + + let mut reader = PithosReaderSimple::new_with_key(&archive, reader_key()).unwrap(); + assert!(matches!( + reader.read_directory(), + Err(PithosError::DirectoryConsumptionMismatch { .. }) + )); +} + +#[test] +fn test_directory_integrity_valid_append_chain() { + let temp_dir = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp_dir, false, false); + append_empty_directory(&archive); + + let mut reader = PithosReaderSimple::new_with_key(&archive, reader_key()).unwrap(); + assert!(reader.read_directory().is_ok()); +} + +#[test] +fn test_directory_integrity_parent_embedded_length_mismatch() { + let temp_dir = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp_dir, false, false); + append_empty_directory(&archive); + let mut bytes = read(&archive).unwrap(); + let (final_start, final_length) = directory_bounds(&bytes); + let final_directory = &bytes[final_start..final_start + final_length]; + let mut cursor = std::io::Cursor::new(final_directory); + let directory = pithos_lib::model::structs::Directory::deserialize(&mut cursor).unwrap(); + let (parent_start, parent_length) = directory.parent_directory_offset.unwrap(); + let parent_start = parent_start as usize; + let parent_length = parent_length as usize; + let parent = &mut bytes[parent_start..parent_start + parent_length]; + parent[parent_length - 12..parent_length - 4] + .copy_from_slice(&((parent_length + 1) as u64).to_be_bytes()); + let crc = crc32fast::hash(&parent[..parent_length - 4]); + parent[parent_length - 4..].copy_from_slice(&crc.to_be_bytes()); + write(&archive, bytes).unwrap(); + + let mut reader = PithosReaderSimple::new_with_key(&archive, reader_key()).unwrap(); + assert!(matches!( + reader.read_directory(), + Err(PithosError::DirectoryLengthMismatch { .. }) + )); +} + #[test] fn test_reader_single_file() { let temp_dir = TempDir::new().unwrap(); diff --git a/spec/PITHOS_1.0.0_draft.md b/spec/PITHOS_1.0.0_draft.md index 458da5a..e7bcf90 100644 --- a/spec/PITHOS_1.0.0_draft.md +++ b/spec/PITHOS_1.0.0_draft.md @@ -2,7 +2,7 @@ **Version:** 1.0 **Status:** Draft -**Date:** September 2025 +**Date:** July 2026 **Purpose:** Next-generation file format for scientific data management, optimized for object storage with built-in deduplication, encryption, and metadata support ## 1. Introduction @@ -118,17 +118,23 @@ The directory MUST contain all file and block metadata: /// Directory - lists all files and blocks in this segment #[derive(Debug, Clone, PartialEq, Eq)] pub struct Directory { - pub identifier: [u8; 8], // MUST be b"PITHOSDR" + pub identifier: [u8; 8], // MUST be exactly ASCII b"PITHOSDR" pub parent_directory_offset: Option<(u64, u64)>, // Previous directory (start, len) (varint, backwards chain) pub files: Vec, // Files in this segment pub blocks: Vec, // Blocks in this segment pub relations: Vec<(u64, String)>, // Relation idx, relationname / id pub encryption: Vec, pub dir_len: u64, - pub crc32: u32, // CRC32 of all preceding fields + pub crc32: u32, // CRC-32/ISO-HDLC of serialized bytes through dir_len } ``` +The directory marker MUST be exactly the eight ASCII bytes `PITHOSDR`. +The final 12 directory bytes MUST be `dir_len:u64be || crc32:u32be`, where `dir_len` is the complete directory length from the marker through the CRC, inclusive. +The CRC MUST be CRC-32/ISO-HDLC with width 32, polynomial `0x04C11DB7`, initial value`0xFFFFFFFF`, reflected input and output, and final XOR `0xFFFFFFFF` (the check value for ASCII `123456789` is `0xCBF43926`). +It MUST cover every exact serialized byte from the marker through the fixed-width `dir_len`, excluding only the stored final CRC. +Readers MUST validate the marker, embedded length, CRC, and exact parser consumption for the terminal directory and independently for every parent directory before decrypting, merging, or otherwise using its metadata. Invalid directories MUST be rejected. + ### 4.4 File Representation #### 4.4.1 File Types From 872a554418ca33c1c53e716d113520f06178d811 Mon Sep 17 00:00:00 2001 From: Jannis Schlegel Date: Sat, 18 Jul 2026 22:55:23 +0200 Subject: [PATCH 04/14] fix: enforce strict block integrity checks and validation --- crates/pithos_lib/src/error.rs | 7 ++ crates/pithos_lib/src/io/pithosreader.rs | 59 ++++++---- crates/pithos_lib/tests/reader.rs | 140 +++++++++++++++++++++++ spec/PITHOS_1.0.0_draft.md | 10 +- 4 files changed, 192 insertions(+), 24 deletions(-) diff --git a/crates/pithos_lib/src/error.rs b/crates/pithos_lib/src/error.rs index 4675696..3ddaa5f 100644 --- a/crates/pithos_lib/src/error.rs +++ b/crates/pithos_lib/src/error.rs @@ -73,6 +73,13 @@ pub enum PithosError { InvalidBlockDataState(String), #[error("Block hash not found: {0:?}")] BlockHashNotFound([u8; 32]), + #[error("Block size mismatch: expected {expected}, got {actual}")] + BlockSizeMismatch { expected: u64, actual: u64 }, + #[error("Block hash mismatch: expected {expected:?}, got {actual:?}")] + BlockHashMismatch { + expected: [u8; 32], + actual: [u8; 32], + }, #[error("File not found: {0}")] FileNotFound(String), #[error("File already exists: {0}")] diff --git a/crates/pithos_lib/src/io/pithosreader.rs b/crates/pithos_lib/src/io/pithosreader.rs index 7812451..61dd5f5 100644 --- a/crates/pithos_lib/src/io/pithosreader.rs +++ b/crates/pithos_lib/src/io/pithosreader.rs @@ -26,6 +26,40 @@ pub struct PithosReaderSimple { } impl PithosReaderSimple { + fn decode_and_verify_block( + stored_bytes: Vec, + key: &[u8; 32], + expected_hash: &[u8; 32], + block_meta: &BlockIndexEntry, + ) -> Result, PithosError> { + let mut plaintext = if block_meta.flags.is_encrypted() { + decrypt_chunk(&stored_bytes, key)? + } else { + stored_bytes + }; + if block_meta.flags.get_compression_level() > 0 { + plaintext = decompress_data(&plaintext, block_meta.original_size)?; + } + + let actual_size = plaintext.len() as u64; + if actual_size != block_meta.original_size { + return Err(PithosError::BlockSizeMismatch { + expected: block_meta.original_size, + actual: actual_size, + }); + } + + let actual_hash = *blake3::hash(&plaintext).as_bytes(); + if actual_hash != *expected_hash { + return Err(PithosError::BlockHashMismatch { + expected: *expected_hash, + actual: actual_hash, + }); + } + + Ok(plaintext) + } + fn parse_directory(bytes: &[u8]) -> Result { if bytes.len() < 25 { return Err(PithosError::DirectoryLengthMismatch { @@ -351,13 +385,7 @@ impl PithosReaderSimple { let mut block_buf = vec![0u8; block_meta.stored_size as usize]; self.file.read_exact(&mut block_buf)?; - // Decrypt and decompress according to ProcessingFlags - if block_meta.flags.is_encrypted() { - block_buf = decrypt_chunk(&block_buf, key)?; - } - if block_meta.flags.get_compression_level() > 0 { - block_buf = decompress_data(&block_buf, block_meta.original_size)?; - } + block_buf = Self::decode_and_verify_block(block_buf, key, hash, block_meta)?; // Write chunk data in 64KiB ChaCha20Poly1305 encrypted blocks let mut chunk_offset = 0; @@ -452,13 +480,7 @@ impl PithosReaderSimple { } } - // Decrypt and decompress according to ProcessingFlags - if block_meta.flags.is_encrypted() { - block_data = decrypt_chunk(&block_data, key)?; - } - if block_meta.flags.get_compression_level() > 0 { - block_data = decompress_data(&block_data, block_meta.original_size)?; - } + block_data = Self::decode_and_verify_block(block_data, key, hash, block_meta)?; sink.write_all(&block_data)?; } @@ -515,14 +537,7 @@ impl PithosReaderSimple { let mut block_buf = vec![0u8; block_meta.stored_size as usize]; self.file.read_exact(&mut block_buf)?; - // Decrypt and decompress according to ProcessingFlags - if block_meta.flags.is_encrypted() { - block_buf = decrypt_chunk(&block_buf, key)?; - } - - if block_meta.flags.get_compression_level() > 0 { - block_buf = decompress_data(&block_buf, block_meta.original_size)?; - } + block_buf = Self::decode_and_verify_block(block_buf, key, hash, block_meta)?; // Calculate the range within this block to write let write_start = if byte_range.start > block_start { diff --git a/crates/pithos_lib/tests/reader.rs b/crates/pithos_lib/tests/reader.rs index 57e1d2c..65ec63e 100644 --- a/crates/pithos_lib/tests/reader.rs +++ b/crates/pithos_lib/tests/reader.rs @@ -81,6 +81,146 @@ fn directory_bounds(bytes: &[u8]) -> (usize, usize) { (bytes.len() - length, length) } +fn write_identity_integrity_archive( + temp_dir: &TempDir, +) -> ( + PathBuf, + StaticSecret, + pithos_lib::model::structs::Directory, + [u8; 32], +) { + let (archive, key, mut writer) = create_pithos_writer(temp_dir, None); + writer.write_file_header().unwrap(); + writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "integrity.txt".into(), + data: Content::Raw("deterministic integrity payload".into()), + metadata: None, + encrypt: false, + compression_level: Some(0), + }) + .unwrap(); + writer.write_directory().unwrap(); + drop(writer); + + let mut reader = PithosReaderSimple::new_with_key(&archive, key.clone()).unwrap(); + let (directory, _) = reader.read_directory().unwrap(); + let file_entry = directory.get_file_by_path("integrity.txt").unwrap(); + let hash = match &file_entry.block_data { + BlockDataState::Decrypted(blocks) => blocks[0].0, + BlockDataState::Encrypted(_) => panic!("identity archive has encrypted block data"), + }; + (archive, key, directory, hash) +} + +fn mutate_block_payload( + archive: &Path, + directory: &pithos_lib::model::structs::Directory, + hash: [u8; 32], +) { + let block = directory.blocks.get(&hash).unwrap(); + let mut bytes = read(archive).unwrap(); + let offset = block.offset as usize; + assert_eq!(&bytes[offset..offset + 4], b"BLCK"); + bytes[offset + 4] ^= 1; + write(archive, bytes).unwrap(); +} + +#[test] +fn test_block_integrity_valid_identity_read() { + let temp_dir = TempDir::new().unwrap(); + let (archive, key, directory, _) = write_identity_integrity_archive(&temp_dir); + let output = temp_dir.path().join("valid.txt"); + let mut reader = PithosReaderSimple::new_with_key(&archive, key).unwrap(); + + reader + .read_file("integrity.txt", &directory, Some(&output), None) + .unwrap(); + assert_eq!(read(&output).unwrap(), b"deterministic integrity payload"); +} + +#[test] +fn test_block_integrity_full_read_rejects_corruption_without_commit() { + let temp_dir = TempDir::new().unwrap(); + let (archive, key, directory, hash) = write_identity_integrity_archive(&temp_dir); + mutate_block_payload(&archive, &directory, hash); + let output = temp_dir.path().join("corrupt-full.txt"); + let mut reader = PithosReaderSimple::new_with_key(&archive, key).unwrap(); + + assert!(matches!( + reader.read_file("integrity.txt", &directory, Some(&output), None), + Err(PithosError::BlockHashMismatch { .. }) + )); + assert!(!output.exists()); +} + +#[test] +fn test_block_integrity_range_read_rejects_corruption_without_commit() { + let temp_dir = TempDir::new().unwrap(); + let (archive, key, directory, hash) = write_identity_integrity_archive(&temp_dir); + mutate_block_payload(&archive, &directory, hash); + let output = temp_dir.path().join("corrupt-range.txt"); + let mut reader = PithosReaderSimple::new_with_key(&archive, key).unwrap(); + + assert!(matches!( + reader.read_file( + "integrity.txt", + &directory, + Some(&output), + #[allow(clippy::single_range_in_vec_init)] + Some(vec![0..1]), + ), + Err(PithosError::BlockHashMismatch { .. }) + )); + assert!(!output.exists()); +} + +#[test] +fn test_block_integrity_crypt4gh_rejects_corruption_before_block_output() { + let temp_dir = TempDir::new().unwrap(); + let (archive, key, directory, hash) = write_identity_integrity_archive(&temp_dir); + mutate_block_payload(&archive, &directory, hash); + let recipient = public_key_from_pem_bytes( + read_to_string("tests/data/keys/recipient2_public.pem") + .unwrap() + .as_bytes(), + ) + .unwrap(); + let output = temp_dir.path().join("corrupt.crypt4gh"); + let mut reader = PithosReaderSimple::new_with_key(&archive, key).unwrap(); + let sink = Box::new(File::create(&output).unwrap()); + + assert!(matches!( + reader.read_file_to_crypt4gh("integrity.txt", &directory, vec![recipient], Some(sink)), + Err(PithosError::BlockHashMismatch { .. }) + )); + let output_bytes = read(&output).unwrap(); + let header = Crypt4GHHeader::try_from(output_bytes.as_slice()).unwrap(); + let header_len = 16 + + header + .header_packets + .iter() + .map(|packet| packet.length as usize) + .sum::(); + assert_eq!(output_bytes.len(), header_len); +} + +#[test] +fn test_block_integrity_size_mismatch_precedes_output() { + let temp_dir = TempDir::new().unwrap(); + let (archive, key, mut directory, hash) = write_identity_integrity_archive(&temp_dir); + directory.blocks.get_mut(&hash).unwrap().original_size += 1; + let output = temp_dir.path().join("wrong-size.txt"); + let mut reader = PithosReaderSimple::new_with_key(&archive, key).unwrap(); + + assert!(matches!( + reader.read_file("integrity.txt", &directory, Some(&output), None), + Err(PithosError::BlockSizeMismatch { .. }) + )); + assert!(!output.exists()); +} + #[test] fn test_directory_integrity_valid_terminal_archive() { let temp_dir = TempDir::new().unwrap(); diff --git a/spec/PITHOS_1.0.0_draft.md b/spec/PITHOS_1.0.0_draft.md index e7bcf90..8ebc82e 100644 --- a/spec/PITHOS_1.0.0_draft.md +++ b/spec/PITHOS_1.0.0_draft.md @@ -320,7 +320,12 @@ Implementations SHOULD use content-defined chunking with recommended parameters: ### 6.2 Block Hashing -All block hashes MUST use Blake3. +Each block identifier MUST be the full 32-byte default unkeyed BLAKE3 digest of the exact +plaintext chunk before compression or encryption. Readers MUST retrieve the stored block, +authenticate and decrypt it when encrypted, decompress it when compressed using the recorded +original size as the output bound, require the resulting plaintext length to equal the recorded +original size, compute the complete plaintext digest, and compare it with the block identifier +before releasing any output derived from that block. ### 6.3 Convergent Encryption @@ -363,7 +368,8 @@ When archiving directory trees: ## 8. Security Considerations -1. Implementations MUST verify block hashes before decompression/decryption +1. Implementations MUST verify the complete plaintext block size and hash after authenticated + decryption and decompression, and before releasing output derived from the block 2. CRC32 values MUST be validated for directories and encryption sections 3. Convergent encryption reveals when identical files exist (accepted trade-off) 4. External block URLs MUST use HTTPS in production environments From 758f3c369bc85f158e4e878d1ce84da9e2569fb8 Mon Sep 17 00:00:00 2001 From: Jannis Schlegel Date: Mon, 20 Jul 2026 15:27:23 +0200 Subject: [PATCH 05/14] chore: bump integer-encoding dependency to 4.1.0 --- crates/pithos_lib/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/pithos_lib/Cargo.toml b/crates/pithos_lib/Cargo.toml index 28f6cd0..46f6e19 100644 --- a/crates/pithos_lib/Cargo.toml +++ b/crates/pithos_lib/Cargo.toml @@ -19,7 +19,7 @@ cap-std = "4.0.2" digest = "0.11.3" fastcdc = "4.0.1" indexmap = "2.11.0" -integer-encoding = "4.0.2" +integer-encoding = "4.1.0" pkcs8 = { version = "0.11.0", features = ["pem", "alloc"] } reqwest = { version = "0.13.4", features = ["blocking"] } ro-crate-rs = { version = "0.5.1", default-features = false } From 929f09104f70ca088b1b2f48b80140f90d4f011a Mon Sep 17 00:00:00 2001 From: Jannis Schlegel Date: Mon, 20 Jul 2026 15:28:25 +0200 Subject: [PATCH 06/14] fix: add configurable deserialization limits and harden length checks --- crates/pithos_lib/src/error.rs | 12 + crates/pithos_lib/src/helpers/crypt4gh.rs | 79 ++++- crates/pithos_lib/src/helpers/directory.rs | 19 +- crates/pithos_lib/src/helpers/zstd.rs | 7 +- crates/pithos_lib/src/io/pithosreader.rs | 250 ++++++++++++++-- crates/pithos_lib/src/io/pithoswriter.rs | 25 +- .../pithos_lib/src/model/deserialization.rs | 263 ++++++++++++++--- crates/pithos_lib/src/model/serialization.rs | 32 +- crates/pithos_lib/src/model/structs.rs | 33 ++- crates/pithos_lib/tests/marshalling.rs | 219 ++++++++++++++ crates/pithos_lib/tests/reader.rs | 276 +++++++++++++++++- crates/pithos_lib/tests/writer.rs | 57 +++- 12 files changed, 1178 insertions(+), 94 deletions(-) diff --git a/crates/pithos_lib/src/error.rs b/crates/pithos_lib/src/error.rs index 3ddaa5f..ea3163b 100644 --- a/crates/pithos_lib/src/error.rs +++ b/crates/pithos_lib/src/error.rs @@ -38,6 +38,18 @@ pub enum PithosError { DirectoryChecksumMismatch { expected: u32, actual: u32 }, #[error("Directory parser consumption mismatch: expected {expected}, got {actual}")] DirectoryConsumptionMismatch { expected: u64, actual: u64 }, + #[error("{field} exceeds limit {limit}: {actual}")] + LimitExceeded { + field: &'static str, + limit: u64, + actual: u64, + }, + #[error("allocation failed for {field}: {size}")] + AllocationFailed { field: &'static str, size: u64 }, + #[error("invalid directory range: {0}")] + InvalidDirectoryRange(String), + #[error("invalid directory chain: {0}")] + InvalidDirectoryChain(String), #[error("Crypt error: {0}")] Crypt(#[from] CryptError), #[error("Crypt4GH error: {0}")] diff --git a/crates/pithos_lib/src/helpers/crypt4gh.rs b/crates/pithos_lib/src/helpers/crypt4gh.rs index 083b55c..e52af34 100644 --- a/crates/pithos_lib/src/helpers/crypt4gh.rs +++ b/crates/pithos_lib/src/helpers/crypt4gh.rs @@ -10,6 +10,9 @@ pub const CRYPT4GH_HEADER_MAGIC: [u8; 8] = [0x63, 0x72, 0x79, 0x70, 0x74, 0x34, pub const CRYPT4GH_HEADER_VERSION: u32 = 1; pub const CRYPT4GH_BLOCK_SIZE: usize = 65536; pub const CRYPT4GH_ENCRYPTED_BLOCK_SIZE: usize = 65564; +const PACKET_LENGTH_FIELD_SIZE: usize = size_of::(); +const MIN_PACKET_BODY_LENGTH: usize = size_of::() + 32 + 12 + 16; +const MIN_PACKET_LENGTH: usize = PACKET_LENGTH_FIELD_SIZE + MIN_PACKET_BODY_LENGTH; #[derive(Debug, Error)] pub enum Crypt4GHError { @@ -84,7 +87,9 @@ impl HeaderPacket { })]); let mac = packet_data.encrypt(session_key.as_bytes(), &nonce)?; let header_packet = HeaderPacket { - length: 4 + 4 + 32 + 12 + packet_data.get_len() as u32 + 16, + length: u32::try_from(MIN_PACKET_LENGTH + packet_data.get_len()).map_err(|_| { + Crypt4GHError::EncryptionError("header packet length".to_string()) + })?, encryption_method: 0, writers_pubkey: sender_pubkey.to_bytes(), nonce: nonce.into(), @@ -200,19 +205,55 @@ impl TryFrom<&[u8]> for Crypt4GHHeader { header.packet_count = cursor .read_u32::() .map_err(|_| Crypt4GHError::FromBytesError("header size".to_string()))?; - //dbg!(&header.packet_count, &cursor.position()); - - for _ in 0..header.packet_count { - let len = cursor - .read_u32::() - .map_err(|_| Crypt4GHError::FromBytesError("packet length".to_string()))?; - let mut buf = vec![0; len as usize - 4]; // u32 of length already read + let packet_count = usize::try_from(header.packet_count) + .map_err(|_| Crypt4GHError::FromBytesError("packet count".to_string()))?; + let position = usize::try_from(cursor.position()) + .map_err(|_| Crypt4GHError::FromBytesError("packet position".to_string()))?; + let remaining = bytes.len().saturating_sub(position); + let minimum_packet_bytes = packet_count + .checked_mul(MIN_PACKET_LENGTH) + .ok_or_else(|| Crypt4GHError::FromBytesError("packet count".to_string()))?; + if minimum_packet_bytes > remaining { + return Err(Crypt4GHError::FromBytesError( + "packet count exceeds header".to_string(), + )); + } + header + .header_packets + .try_reserve_exact(packet_count) + .map_err(|_| Crypt4GHError::FromBytesError("packet allocation".to_string()))?; + + for _ in 0..packet_count { + let len = usize::try_from( + cursor + .read_u32::() + .map_err(|_| Crypt4GHError::FromBytesError("packet length".to_string()))?, + ) + .map_err(|_| Crypt4GHError::FromBytesError("packet length".to_string()))?; + if len < MIN_PACKET_LENGTH { + return Err(Crypt4GHError::FromBytesError( + "packet length below minimum".to_string(), + )); + } + let packet_bytes = len - PACKET_LENGTH_FIELD_SIZE; + let position = usize::try_from(cursor.position()) + .map_err(|_| Crypt4GHError::FromBytesError("packet position".to_string()))?; + let remaining = bytes.len().saturating_sub(position); + if packet_bytes > remaining { + return Err(Crypt4GHError::FromBytesError( + "packet exceeds header".to_string(), + )); + } + let mut buf = Vec::new(); + buf.try_reserve_exact(packet_bytes) + .map_err(|_| Crypt4GHError::FromBytesError("packet allocation".to_string()))?; + buf.resize(packet_bytes, 0); cursor .read_exact(&mut buf) .map_err(|_| Crypt4GHError::FromBytesError("packet data".to_string()))?; header .header_packets - .push(HeaderPacket::from_buf(buf, len as usize)?); + .push(HeaderPacket::from_buf(buf, len)?); } Ok(header) @@ -262,6 +303,11 @@ impl HeaderPacket { #[tracing::instrument(level = "trace", skip(bytes, len))] pub fn from_buf(bytes: Vec, len: usize) -> Result { + if len < MIN_PACKET_LENGTH || bytes.len() < MIN_PACKET_BODY_LENGTH { + return Err(Crypt4GHError::FromBytesError( + "packet is too short".to_string(), + )); + } let mut bytes = Cursor::new(bytes); let encryption_method = bytes .read_u32::() @@ -279,8 +325,17 @@ impl HeaderPacket { bytes .read_to_end(&mut packet_data) .map_err(|_| Crypt4GHError::FromBytesError("packet data and mac".to_string()))?; - let (enc, mac) = packet_data.split_at(packet_data.len() - 16); - let encrypted_packet_data = PacketData::Encrypted(enc.to_vec()); + let mac_start = packet_data + .len() + .checked_sub(16) + .ok_or_else(|| Crypt4GHError::FromBytesError("packet MAC".to_string()))?; + let (enc, mac) = packet_data.split_at(mac_start); + let mut encrypted_data = Vec::new(); + encrypted_data + .try_reserve_exact(enc.len()) + .map_err(|_| Crypt4GHError::FromBytesError("packet data allocation".to_string()))?; + encrypted_data.extend_from_slice(enc); + let encrypted_packet_data = PacketData::Encrypted(encrypted_data); Ok(HeaderPacket { length: u32::try_from(len) @@ -318,7 +373,7 @@ impl HeaderPacket { self.mac = self.packet_data.encrypt(session_key.as_bytes(), &nonce)?; self.writers_pubkey = PublicKey::from(&sender_key).to_bytes(); self.nonce = nonce.into(); - self.length = (4 + 4 + 32 + 12 + self.packet_data.get_len() + 16) + self.length = (MIN_PACKET_LENGTH + self.packet_data.get_len()) .try_into() .map_err(|_| Crypt4GHError::EncryptionError("header packet length".to_string()))?; Ok(()) diff --git a/crates/pithos_lib/src/helpers/directory.rs b/crates/pithos_lib/src/helpers/directory.rs index 9166a99..e5528f7 100644 --- a/crates/pithos_lib/src/helpers/directory.rs +++ b/crates/pithos_lib/src/helpers/directory.rs @@ -2,6 +2,7 @@ use crate::error::PithosError; use crate::helpers::file_entry_map::{FileEntryMap, Key, KeyQuery}; use crate::model::structs::{RecipientData, RecipientSection}; use crate::model::{ + deserialization::DeserializationLimits, serialization::SerializationError, structs::{BlockIndexEntry, Directory, EncryptionSection, FileEntry}, }; @@ -370,6 +371,15 @@ impl Directory { pub fn decrypt_recipient( &mut self, reader_key: &StaticSecret, + ) -> Result, PithosError> { + self.decrypt_recipient_with_limits(reader_key, &DeserializationLimits::default()) + } + + #[tracing::instrument(level = "trace", skip(self, reader_key, limits))] + pub fn decrypt_recipient_with_limits( + &mut self, + reader_key: &StaticSecret, + limits: &DeserializationLimits, ) -> Result, PithosError> { // Store for decrypted sections let mut available_file_indices = Vec::<(u64, [u8; 32])>::new(); @@ -381,7 +391,9 @@ impl Directory { let shared_key = reader_key.diffie_hellman(&PublicKey::from(*key)); match &r_section.recipient_data { RecipientData::Encrypted(_) => { - let entries = r_section.recipient_data.decrypt(&shared_key)?; + let entries = r_section + .recipient_data + .decrypt_with_limits(&shared_key, limits)?; available_file_indices.extend(entries); } RecipientData::Decrypted(entries) => { @@ -405,7 +417,10 @@ impl Directory { match e_section.recipients.entry(*reader_pubkey.as_bytes()) { Entry::Occupied(ref mut entry) => match &entry.get().recipient_data { RecipientData::Encrypted(_) => { - let entries = entry.get_mut().recipient_data.decrypt(&shared_key)?; + let entries = entry + .get_mut() + .recipient_data + .decrypt_with_limits(&shared_key, limits)?; available_file_indices.extend(entries); } RecipientData::Decrypted(entries) => { diff --git a/crates/pithos_lib/src/helpers/zstd.rs b/crates/pithos_lib/src/helpers/zstd.rs index 355219b..abe2c1c 100644 --- a/crates/pithos_lib/src/helpers/zstd.rs +++ b/crates/pithos_lib/src/helpers/zstd.rs @@ -14,6 +14,8 @@ pub enum ZstdError { /// Compression failure #[error("Decompression error: {0}")] DecompressionError(String), + #[error("decompressed size does not fit platform: {0}")] + SizeOverflow(u64), } const _ZSTD_MAGIC_NUMBER: u32 = 0xFD2FB528; // 4 Bytes, little-endian format @@ -61,6 +63,7 @@ pub fn compress_data(input: &[u8], level: Option) -> Result, ZstdEr #[tracing::instrument(level = "trace", skip(input, decompressed_size))] pub fn decompress_data(input: &[u8], decompressed_size: u64) -> Result, ZstdError> { - bulk::decompress(input, decompressed_size as usize) - .map_err(|e| ZstdError::DecompressionError(e.to_string())) + let size = usize::try_from(decompressed_size) + .map_err(|_| ZstdError::SizeOverflow(decompressed_size))?; + bulk::decompress(input, size).map_err(|e| ZstdError::DecompressionError(e.to_string())) } diff --git a/crates/pithos_lib/src/io/pithosreader.rs b/crates/pithos_lib/src/io/pithosreader.rs index 61dd5f5..bde3ff4 100644 --- a/crates/pithos_lib/src/io/pithosreader.rs +++ b/crates/pithos_lib/src/io/pithosreader.rs @@ -6,6 +6,7 @@ use crate::helpers::file_entry_map::KeyQuery; use crate::helpers::x25519_keys::private_key_from_pem_bytes; use crate::helpers::zstd::decompress_data; use crate::io::extraction::ExtractionRoot; +use crate::model::deserialization::DeserializationLimits; use crate::model::structs::{ BlockDataState, BlockHeader, BlockIndexEntry, BlockLocation, Directory, FileEntry, FileType, }; @@ -18,11 +19,33 @@ use std::ops::Range; use std::path::{Path, PathBuf}; use x25519_dalek::{PublicKey, StaticSecret}; +#[derive(Clone, Copy, Debug)] +pub struct ReaderLimits { + pub max_directory_bytes: u64, + pub max_parent_directories: u64, + pub max_stored_block_bytes: u64, + pub max_decoded_block_bytes: u64, + pub deserialization: DeserializationLimits, +} + +impl Default for ReaderLimits { + fn default() -> Self { + Self { + max_directory_bytes: 64 * 1024 * 1024, + max_parent_directories: 1024, + max_stored_block_bytes: 64 * 1024 * 1024, + max_decoded_block_bytes: 64 * 1024 * 1024, + deserialization: DeserializationLimits::default(), + } + } +} + pub struct PithosReaderSimple { /// Underlying file handle for the Pithos archive file: File, /// User's private key private_key: StaticSecret, + limits: ReaderLimits, } impl PithosReaderSimple { @@ -31,7 +54,15 @@ impl PithosReaderSimple { key: &[u8; 32], expected_hash: &[u8; 32], block_meta: &BlockIndexEntry, + limits: &ReaderLimits, ) -> Result, PithosError> { + if block_meta.original_size > limits.max_decoded_block_bytes { + return Err(PithosError::LimitExceeded { + field: "decoded block", + limit: limits.max_decoded_block_bytes, + actual: block_meta.original_size, + }); + } let mut plaintext = if block_meta.flags.is_encrypted() { decrypt_chunk(&stored_bytes, key)? } else { @@ -60,7 +91,56 @@ impl PithosReaderSimple { Ok(plaintext) } - fn parse_directory(bytes: &[u8]) -> Result { + fn checked_stored_size( + meta: &BlockIndexEntry, + limits: &ReaderLimits, + ) -> Result { + if meta.stored_size > limits.max_stored_block_bytes { + return Err(PithosError::LimitExceeded { + field: "stored block", + limit: limits.max_stored_block_bytes, + actual: meta.stored_size, + }); + } + if meta.original_size > limits.max_decoded_block_bytes { + return Err(PithosError::LimitExceeded { + field: "decoded block", + limit: limits.max_decoded_block_bytes, + actual: meta.original_size, + }); + } + usize::try_from(meta.stored_size).map_err(|_| { + PithosError::InvalidDirectoryRange("stored block size does not fit platform".into()) + }) + } + + fn zeroed_buffer(size: usize, field: &'static str) -> Result, PithosError> { + let mut buffer = Vec::new(); + buffer + .try_reserve_exact(size) + .map_err(|_| PithosError::AllocationFailed { + field, + size: size as u64, + })?; + buffer.resize(size, 0); + Ok(buffer) + } + + fn validate_local_block_range(&self, meta: &BlockIndexEntry) -> Result<(), PithosError> { + let end = meta + .offset + .checked_add(4) + .and_then(|value| value.checked_add(meta.stored_size)) + .ok_or_else(|| PithosError::InvalidDirectoryRange("block range overflow".into()))?; + if end > self.file.metadata()?.len() { + return Err(PithosError::InvalidDirectoryRange( + "block range exceeds archive".into(), + )); + } + Ok(()) + } + + fn parse_directory(bytes: &[u8], limits: &ReaderLimits) -> Result { if bytes.len() < 25 { return Err(PithosError::DirectoryLengthMismatch { expected: 25, @@ -90,7 +170,7 @@ impl PithosReaderSimple { }); } let mut cursor = Cursor::new(bytes); - let directory = Directory::deserialize(&mut cursor)?; + let directory = Directory::deserialize_with_limits(&mut cursor, &limits.deserialization)?; if cursor.position() != bytes.len() as u64 { return Err(PithosError::DirectoryConsumptionMismatch { expected: bytes.len() as u64, @@ -113,7 +193,11 @@ impl PithosReaderSimple { let pem_content = std::fs::read_to_string(private_key_pem_path)?; let private_key = private_key_from_pem_bytes(pem_content.as_bytes())?; - Ok(Self { file, private_key }) + Ok(Self { + file, + private_key, + limits: ReaderLimits::default(), + }) } /// Init a simple Pithos reader @@ -125,7 +209,16 @@ impl PithosReaderSimple { // Open the Pithos file let file = File::open(&pithos_path)?; - Ok(Self { file, private_key }) + Ok(Self { + file, + private_key, + limits: ReaderLimits::default(), + }) + } + + pub fn with_limits(mut self, limits: ReaderLimits) -> Self { + self.limits = limits; + self } /// Init a simple Pithos reader @@ -153,25 +246,97 @@ impl PithosReaderSimple { let parent_dir_len = u64::from_be_bytes(footer[..8].try_into().map_err(|_| { PithosError::Conversion("Failed to convert directory length bytes to u64".to_string()) })?); - let parent_dir_start = file_len - parent_dir_len; + if parent_dir_len < 25 { + return Err(PithosError::DirectoryLengthMismatch { + expected: 25, + actual: parent_dir_len, + }); + } + if parent_dir_len > self.limits.max_directory_bytes { + return Err(PithosError::LimitExceeded { + field: "directory", + limit: self.limits.max_directory_bytes, + actual: parent_dir_len, + }); + } + let parent_dir_start = file_len.checked_sub(parent_dir_len).ok_or_else(|| { + PithosError::InvalidDirectoryRange("terminal directory exceeds archive".into()) + })?; + let parent_dir_len_usize = usize::try_from(parent_dir_len).map_err(|_| { + PithosError::InvalidDirectoryRange("directory length does not fit platform".into()) + })?; - self.file.seek(SeekFrom::End(0 - parent_dir_len as i64))?; - let mut dir_buf = vec![0u8; parent_dir_len as usize]; + self.file.seek(SeekFrom::Start(parent_dir_start))?; + let mut dir_buf = Vec::new(); + dir_buf + .try_reserve_exact(parent_dir_len_usize) + .map_err(|_| PithosError::LimitExceeded { + field: "directory allocation", + limit: self.limits.max_directory_bytes, + actual: parent_dir_len, + })?; + dir_buf.resize(parent_dir_len_usize, 0); self.file.read_exact(&mut dir_buf)?; // Deserialize full directory let mut available_file_keys = HashMap::new(); - let mut directory = Self::parse_directory(&dir_buf)?; - available_file_keys.extend(directory.decrypt_recipient(&self.private_key)?); + let mut directory = Self::parse_directory(&dir_buf, &self.limits)?; + available_file_keys.extend( + directory + .decrypt_recipient_with_limits(&self.private_key, &self.limits.deserialization)?, + ); // Merge with parent directories + let mut visited = std::collections::HashSet::new(); + let mut depth = 0u64; + let mut child_start = parent_dir_start; while let Some((start, len)) = directory.parent_directory_offset { + depth = depth + .checked_add(1) + .ok_or_else(|| PithosError::InvalidDirectoryChain("depth overflow".into()))?; + if depth > self.limits.max_parent_directories { + return Err(PithosError::LimitExceeded { + field: "parent directories", + limit: self.limits.max_parent_directories, + actual: depth, + }); + } + if len < 25 || len > self.limits.max_directory_bytes { + return Err(PithosError::InvalidDirectoryRange( + "parent length out of bounds".into(), + )); + } + let end = start.checked_add(len).ok_or_else(|| { + PithosError::InvalidDirectoryRange("parent range overflow".into()) + })?; + if end > child_start || !visited.insert((start, len)) { + return Err(PithosError::InvalidDirectoryChain( + "parent must be backward and nonoverlapping".into(), + )); + } + let len_usize = usize::try_from(len).map_err(|_| { + PithosError::InvalidDirectoryRange("parent length does not fit platform".into()) + })?; // Read parent directory self.file.seek(SeekFrom::Start(start))?; - let mut dir_buf = vec![0u8; len as usize]; + let mut dir_buf = Vec::new(); + dir_buf + .try_reserve_exact(len_usize) + .map_err(|_| PithosError::LimitExceeded { + field: "directory allocation", + limit: self.limits.max_directory_bytes, + actual: len, + })?; + dir_buf.resize(len_usize, 0); self.file.read_exact(&mut dir_buf)?; - let mut older_directory = Self::parse_directory(&dir_buf)?; - available_file_keys.extend(older_directory.decrypt_recipient(&self.private_key)?); + let mut older_directory = Self::parse_directory(&dir_buf, &self.limits)?; + available_file_keys.extend( + older_directory.decrypt_recipient_with_limits( + &self.private_key, + &self.limits.deserialization, + )?, + ); + child_start = start; // Merge directories and swap older_directory.merge(directory)?; @@ -184,7 +349,10 @@ impl PithosReaderSimple { .retain_mut(|id, path, file| match &mut file.block_data { BlockDataState::Decrypted(_) => true, BlockDataState::Encrypted(_) => match available_file_keys.get(&id) { - Some(block_key) => match file.block_data.decrypt(block_key) { + Some(block_key) => match file + .block_data + .decrypt_with_limits(block_key, &self.limits.deserialization) + { Ok(_) => { tracing::info!("Successfully decrypted {path}"); true @@ -374,6 +542,8 @@ impl PithosReaderSimple { .ok_or(PithosError::BlockHashNotFound(*hash))?; // Jump to begin of block in file + let stored_size = Self::checked_stored_size(block_meta, &self.limits)?; + self.validate_local_block_range(block_meta)?; self.file.seek(SeekFrom::Start(block_meta.offset))?; // Read block header for block start validation @@ -382,10 +552,16 @@ impl PithosReaderSimple { BlockHeader::deserialize(&mut block_header.as_slice())?; // Read block data - let mut block_buf = vec![0u8; block_meta.stored_size as usize]; + let mut block_buf = Self::zeroed_buffer(stored_size, "stored block")?; self.file.read_exact(&mut block_buf)?; - block_buf = Self::decode_and_verify_block(block_buf, key, hash, block_meta)?; + block_buf = Self::decode_and_verify_block( + block_buf, + key, + hash, + block_meta, + &self.limits, + )?; // Write chunk data in 64KiB ChaCha20Poly1305 encrypted blocks let mut chunk_offset = 0; @@ -460,9 +636,11 @@ impl PithosReaderSimple { .ok_or(PithosError::BlockHashNotFound(*hash))?; let mut block_header = [0u8; 4]; - let mut block_data = vec![0u8; block_meta.stored_size as usize]; + let stored_size = Self::checked_stored_size(block_meta, &self.limits)?; + let mut block_data = Self::zeroed_buffer(stored_size, "stored block")?; match &block_meta.location { BlockLocation::Local => { + self.validate_local_block_range(block_meta)?; self.file.seek(SeekFrom::Start(block_meta.offset))?; // Read block header for block start validation self.file.read_exact(&mut block_header)?; @@ -480,7 +658,13 @@ impl PithosReaderSimple { } } - block_data = Self::decode_and_verify_block(block_data, key, hash, block_meta)?; + block_data = Self::decode_and_verify_block( + block_data, + key, + hash, + block_meta, + &self.limits, + )?; sink.write_all(&block_data)?; } @@ -498,7 +682,7 @@ impl PithosReaderSimple { block_index: &IndexMap<[u8; 32], BlockIndexEntry>, sink: &mut Box, ) -> Result<(), PithosError> { - let mut block_byte_sum = 0; + let mut block_byte_sum: u64 = 0; match &file_entry.block_data { BlockDataState::Encrypted(_) => { @@ -513,8 +697,13 @@ impl PithosReaderSimple { .get(hash) .ok_or(PithosError::BlockHashNotFound(*hash))?; + let stored_size = Self::checked_stored_size(block_meta, &self.limits)?; let block_start = block_byte_sum; - let block_end = block_byte_sum + block_meta.original_size; + let block_end = block_byte_sum + .checked_add(block_meta.original_size) + .ok_or_else(|| { + PithosError::InvalidDirectoryRange("range block size overflow".into()) + })?; block_byte_sum = block_end; // If block ends before start of range; Discard block @@ -528,25 +717,40 @@ impl PithosReaderSimple { } // Read block header for block start validation + self.validate_local_block_range(block_meta)?; self.file.seek(SeekFrom::Start(block_meta.offset))?; let mut block_header = [0u8; 4]; self.file.read_exact(&mut block_header)?; BlockHeader::deserialize(&mut block_header.as_slice())?; // Read block data - let mut block_buf = vec![0u8; block_meta.stored_size as usize]; + let mut block_buf = Self::zeroed_buffer(stored_size, "stored block")?; self.file.read_exact(&mut block_buf)?; - block_buf = Self::decode_and_verify_block(block_buf, key, hash, block_meta)?; + block_buf = Self::decode_and_verify_block( + block_buf, + key, + hash, + block_meta, + &self.limits, + )?; // Calculate the range within this block to write let write_start = if byte_range.start > block_start { - (byte_range.start - block_start) as usize + usize::try_from(byte_range.start - block_start).map_err(|_| { + PithosError::InvalidDirectoryRange( + "range index does not fit platform".into(), + ) + })? } else { 0 }; let write_end = if byte_range.end < block_end { - (byte_range.end - block_start) as usize + usize::try_from(byte_range.end - block_start).map_err(|_| { + PithosError::InvalidDirectoryRange( + "range index does not fit platform".into(), + ) + })? } else { block_buf.len() }; diff --git a/crates/pithos_lib/src/io/pithoswriter.rs b/crates/pithos_lib/src/io/pithoswriter.rs index d2195ab..740b67a 100644 --- a/crates/pithos_lib/src/io/pithoswriter.rs +++ b/crates/pithos_lib/src/io/pithoswriter.rs @@ -8,7 +8,7 @@ use crate::helpers::ro_crate::{ inspect_ro_crate_zip_manifest, }; use crate::helpers::zstd::{ZstdError, map_to_zstd_level}; -use crate::io::pithosreader::PithosReaderSimple; +use crate::io::pithosreader::{PithosReaderSimple, ReaderLimits}; use crate::io::util::{create_stream_cdc, extract_filename}; use crate::model::serialization::SerializationError; use crate::model::structs::{ @@ -204,9 +204,30 @@ impl PithosWriter { reader_keys: Vec, cdc: Option<(usize, usize, usize)>, pithos_file: P, + ) -> Result { + Self::new_from_file_with_limits( + writer_key, + reader_keys, + cdc, + pithos_file, + ReaderLimits::default(), + ) + } + + #[tracing::instrument( + level = "trace", + skip(writer_key, reader_keys, cdc, pithos_file, limits) + )] + pub fn new_from_file_with_limits>( + writer_key: StaticSecret, + reader_keys: Vec, + cdc: Option<(usize, usize, usize)>, + pithos_file: P, + limits: ReaderLimits, ) -> Result { // Read existing directories - let mut reader = PithosReaderSimple::new_with_key(&pithos_file, writer_key.clone())?; + let mut reader = + PithosReaderSimple::new_with_key(&pithos_file, writer_key.clone())?.with_limits(limits); let (directory, offset) = reader.read_directory()?; // Open Pithos file in append write mode diff --git a/crates/pithos_lib/src/model/deserialization.rs b/crates/pithos_lib/src/model/deserialization.rs index fa692be..33add6b 100644 --- a/crates/pithos_lib/src/model/deserialization.rs +++ b/crates/pithos_lib/src/model/deserialization.rs @@ -37,13 +37,74 @@ pub enum DeserializationError { /// Invalid length encountered #[error("Invalid length")] InvalidLength, + #[error("{field} exceeds limit {limit}: {actual}")] + LimitExceeded { + field: &'static str, + limit: u64, + actual: u64, + }, + #[error("allocation failed for {field}: {size}")] + AllocationFailed { field: &'static str, size: u64 }, +} + +#[derive(Clone, Copy, Debug)] +pub struct DeserializationLimits { + pub max_string_bytes: u64, + pub max_opaque_bytes: u64, + pub max_collection_entries: u64, +} + +impl Default for DeserializationLimits { + fn default() -> Self { + Self { + max_string_bytes: 1024 * 1024, + max_opaque_bytes: 64 * 1024 * 1024, + max_collection_entries: 1_000_000, + } + } +} + +fn bounded_len(value: u64, limit: u64, field: &'static str) -> Result { + if value > limit { + return Err(DeserializationError::LimitExceeded { + field, + limit, + actual: value, + }); + } + usize::try_from(value).map_err(|_| DeserializationError::InvalidLength) +} + +fn reserve( + vec: &mut Vec, + count: usize, + field: &'static str, +) -> Result<(), DeserializationError> { + vec.try_reserve(count) + .map_err(|_| DeserializationError::AllocationFailed { + field, + size: count as u64, + }) } // Helper: decode string (UTF-8 with varint length prefix) #[tracing::instrument(level = "trace", skip(reader))] pub fn decode_string(reader: &mut R) -> Result { - let len = reader.read_varint()?; - let mut buf = vec![0u8; len]; + decode_string_with_limits(reader, &DeserializationLimits::default()) +} + +pub fn decode_string_with_limits( + reader: &mut R, + limits: &DeserializationLimits, +) -> Result { + let len = bounded_len( + reader.read_varint::()?, + limits.max_string_bytes, + "string", + )?; + let mut buf = Vec::new(); + reserve(&mut buf, len, "string")?; + buf.resize(len, 0); reader.read_exact(&mut buf)?; Ok(String::from_utf8(buf)?) } @@ -105,12 +166,19 @@ impl ProcessingFlags { impl BlockLocation { #[tracing::instrument(level = "trace", skip(reader))] pub fn deserialize(reader: &mut R) -> Result { + Self::deserialize_with_limits(reader, &DeserializationLimits::default()) + } + + pub fn deserialize_with_limits( + reader: &mut R, + limits: &DeserializationLimits, + ) -> Result { let mut tag = [0u8; 1]; reader.read_exact(&mut tag)?; match tag[0] { 0 => Ok(BlockLocation::Local), 1 => { - let url = decode_string(reader)?; + let url = decode_string_with_limits(reader, limits)?; Ok(BlockLocation::External { url }) } v => Err(DeserializationError::InvalidEnumValue(v)), @@ -122,11 +190,18 @@ impl BlockLocation { impl BlockIndexEntry { #[tracing::instrument(level = "trace", skip(reader))] pub fn deserialize(reader: &mut R) -> Result { + Self::deserialize_with_limits(reader, &DeserializationLimits::default()) + } + + pub fn deserialize_with_limits( + reader: &mut R, + limits: &DeserializationLimits, + ) -> Result { let offset: u64 = reader.read_varint()?; let stored_size: u64 = reader.read_varint()?; let original_size: u64 = reader.read_varint()?; let flags = ProcessingFlags::deserialize(reader)?; - let location = BlockLocation::deserialize(reader)?; + let location = BlockLocation::deserialize_with_limits(reader, limits)?; Ok(BlockIndexEntry { offset, stored_size, @@ -143,6 +218,13 @@ impl Directory { #[tracing::instrument(level = "trace", skip(reader))] pub fn deserialize(reader: &mut R) -> Result { + Self::deserialize_with_limits(reader, &DeserializationLimits::default()) + } + + pub fn deserialize_with_limits( + reader: &mut R, + limits: &DeserializationLimits, + ) -> Result { // Read static directory identifier let mut identifier = [0u8; 8]; reader.read_exact(&mut identifier)?; @@ -168,39 +250,59 @@ impl Directory { // Read file entries let mut files = FileEntryMap::new(); - let files_len = reader.read_varint::()?; + let files_len = bounded_len( + reader.read_varint::()?, + limits.max_collection_entries, + "files", + )?; for _ in 0..files_len { let id = reader.read_varint::()?; - let path = decode_string(reader)?; - let entry = FileEntry::deserialize(reader)?; + let path = decode_string_with_limits(reader, limits)?; + let entry = FileEntry::deserialize_with_limits(reader, limits)?; validate_entry(&path, &entry)?; files.insert(Key::new(id, path), entry)?; } validate_map(&files)?; - let blocks_len = reader.read_varint::()?; + let blocks_len = bounded_len( + reader.read_varint::()?, + limits.max_collection_entries, + "blocks", + )?; let mut blocks = IndexMap::new(); for _ in 0..blocks_len { let mut hash = [0u8; 32]; reader.read_exact(&mut hash)?; - let block = BlockIndexEntry::deserialize(reader)?; + let block = BlockIndexEntry::deserialize_with_limits(reader, limits)?; blocks.insert(hash, block); } - let relations_len = reader.read_varint()?; - let mut relations = Vec::with_capacity(relations_len); + let relations_len = bounded_len( + reader.read_varint::()?, + limits.max_collection_entries, + "relations", + )?; + let mut relations = Vec::new(); + reserve(&mut relations, relations_len, "relations")?; for _ in 0..relations_len { let idx = reader.read_varint::()?; - let name = decode_string(reader)?; + let name = decode_string_with_limits(reader, limits)?; relations.push((idx, name)); } - let encryption_len = reader.read_varint::()?; + let encryption_len = bounded_len( + reader.read_varint::()?, + limits.max_collection_entries, + "encryption", + )?; let mut encryption = IndexMap::new(); for _ in 0..encryption_len { let mut key = [0u8; 32]; reader.read_exact(&mut key)?; - encryption.insert(key, EncryptionSection::deserialize(reader)?); + encryption.insert( + key, + EncryptionSection::deserialize_with_limits(reader, limits)?, + ); } let dir_len = reader.read_u64::()?; @@ -239,18 +341,36 @@ impl FileType { impl BlockDataState { #[tracing::instrument(level = "trace", skip(reader))] pub fn deserialize(reader: &mut R) -> Result { + Self::deserialize_with_limits(reader, &DeserializationLimits::default()) + } + + pub fn deserialize_with_limits( + reader: &mut R, + limits: &DeserializationLimits, + ) -> Result { let mut tag = [0u8; 1]; reader.read_exact(&mut tag)?; match tag[0] { 0 => { - let len = reader.read_varint()?; - let mut buf = vec![0u8; len]; + let len = bounded_len( + reader.read_varint::()?, + limits.max_opaque_bytes, + "encrypted block", + )?; + let mut buf = Vec::new(); + reserve(&mut buf, len, "encrypted block")?; + buf.resize(len, 0); reader.read_exact(&mut buf)?; Ok(BlockDataState::Encrypted(buf)) } 1 => { - let list_len = reader.read_varint()?; - let mut list = Vec::with_capacity(list_len); + let list_len = bounded_len( + reader.read_varint::()?, + limits.max_collection_entries, + "block keys", + )?; + let mut list = Vec::new(); + reserve(&mut list, list_len, "block keys")?; for _ in 0..list_len { let mut hash = [0u8; 32]; reader.read_exact(&mut hash)?; @@ -269,8 +389,21 @@ impl BlockDataState { &self, reader: &mut R, ) -> Result, DeserializationError> { - let list_len = reader.read_varint()?; - let mut list = Vec::with_capacity(list_len); + self.deserialize_block_index_with_limits(reader, &DeserializationLimits::default()) + } + + pub fn deserialize_block_index_with_limits( + &self, + reader: &mut R, + limits: &DeserializationLimits, + ) -> Result, DeserializationError> { + let list_len = bounded_len( + reader.read_varint::()?, + limits.max_collection_entries, + "block index", + )?; + let mut list = Vec::new(); + reserve(&mut list, list_len, "block index")?; for _ in 0..list_len { let mut hash = [0u8; 32]; reader.read_exact(&mut hash)?; @@ -286,14 +419,26 @@ impl BlockDataState { impl FileEntry { #[tracing::instrument(level = "trace", skip(reader))] pub fn deserialize(reader: &mut R) -> Result { + Self::deserialize_with_limits(reader, &DeserializationLimits::default()) + } + + pub fn deserialize_with_limits( + reader: &mut R, + limits: &DeserializationLimits, + ) -> Result { let file_type = FileType::deserialize(reader)?; - let block_data = BlockDataState::deserialize(reader)?; + let block_data = BlockDataState::deserialize_with_limits(reader, limits)?; let created: u64 = reader.read_varint()?; let modified: u64 = reader.read_varint()?; let file_size: u64 = reader.read_varint()?; let permissions: u32 = reader.read_varint()?; - let refs_len = reader.read_varint()?; - let mut references = Vec::with_capacity(refs_len); + let refs_len = bounded_len( + reader.read_varint::()?, + limits.max_collection_entries, + "references", + )?; + let mut references = Vec::new(); + reserve(&mut references, refs_len, "references")?; for _ in 0..refs_len { references.push(Reference::deserialize(reader)?); } @@ -301,7 +446,7 @@ impl FileEntry { reader.read_exact(&mut tag)?; let symlink_target = match tag[0] { 0 => None, - 1 => Some(decode_string(reader)?), + 1 => Some(decode_string_with_limits(reader, limits)?), _ => return Err(DeserializationError::InvalidOption), }; Ok(FileEntry { @@ -334,12 +479,26 @@ impl Reference { impl EncryptionSection { #[tracing::instrument(level = "trace", skip(reader))] pub fn deserialize(reader: &mut R) -> Result { - let recipients_len = reader.read_varint::()?; + Self::deserialize_with_limits(reader, &DeserializationLimits::default()) + } + + pub fn deserialize_with_limits( + reader: &mut R, + limits: &DeserializationLimits, + ) -> Result { + let recipients_len = bounded_len( + reader.read_varint::()?, + limits.max_collection_entries, + "recipients", + )?; let mut recipients = IndexMap::new(); for _ in 0..recipients_len { let mut key = [0u8; 32]; reader.read_exact(&mut key)?; - recipients.insert(key, RecipientSection::deserialize(reader)?); + recipients.insert( + key, + RecipientSection::deserialize_with_limits(reader, limits)?, + ); } Ok(EncryptionSection { recipients }) } @@ -349,19 +508,37 @@ impl EncryptionSection { impl RecipientData { #[tracing::instrument(level = "trace", skip(reader))] pub fn deserialize(reader: &mut R) -> Result { + Self::deserialize_with_limits(reader, &DeserializationLimits::default()) + } + + pub fn deserialize_with_limits( + reader: &mut R, + limits: &DeserializationLimits, + ) -> Result { let mut tag = [0u8; 1]; reader.read_exact(&mut tag)?; match tag[0] { 0 => { - let len = reader.read_varint()?; - let mut buf = vec![0u8; len]; + let len = bounded_len( + reader.read_varint::()?, + limits.max_opaque_bytes, + "encrypted recipient data", + )?; + let mut buf = Vec::new(); + reserve(&mut buf, len, "encrypted recipient data")?; + buf.resize(len, 0); reader.read_exact(&mut buf)?; Ok(RecipientData::Encrypted(buf)) } 1 => { - let list_len = reader.read_varint()?; - let mut list = Vec::with_capacity(list_len); + let list_len = bounded_len( + reader.read_varint::()?, + limits.max_collection_entries, + "recipient keys", + )?; + let mut list = Vec::new(); + reserve(&mut list, list_len, "recipient keys")?; for _ in 0..list_len { let idx: u64 = reader.read_varint()?; let mut key = [0u8; 32]; @@ -379,8 +556,21 @@ impl RecipientData { &self, reader: &mut R, ) -> Result, DeserializationError> { - let list_len = reader.read_varint()?; - let mut list = Vec::with_capacity(list_len); + self.deserialize_decrypted_list_with_limits(reader, &DeserializationLimits::default()) + } + + pub fn deserialize_decrypted_list_with_limits( + &self, + reader: &mut R, + limits: &DeserializationLimits, + ) -> Result, DeserializationError> { + let list_len = bounded_len( + reader.read_varint::()?, + limits.max_collection_entries, + "recipient keys", + )?; + let mut list = Vec::new(); + reserve(&mut list, list_len, "recipient keys")?; for _ in 0..list_len { let idx: u64 = reader.read_varint()?; let mut key = [0u8; 32]; @@ -396,7 +586,14 @@ impl RecipientData { impl RecipientSection { #[tracing::instrument(level = "trace", skip(reader))] pub fn deserialize(reader: &mut R) -> Result { - let recipient_data = RecipientData::deserialize(reader)?; + Self::deserialize_with_limits(reader, &DeserializationLimits::default()) + } + + pub fn deserialize_with_limits( + reader: &mut R, + limits: &DeserializationLimits, + ) -> Result { + let recipient_data = RecipientData::deserialize_with_limits(reader, limits)?; Ok(RecipientSection { recipient_data }) } } diff --git a/crates/pithos_lib/src/model/serialization.rs b/crates/pithos_lib/src/model/serialization.rs index 82a3db0..17ba762 100644 --- a/crates/pithos_lib/src/model/serialization.rs +++ b/crates/pithos_lib/src/model/serialization.rs @@ -21,10 +21,20 @@ pub enum SerializationError { Other(String), } +pub(crate) fn write_len_prefix( + writer: &mut W, + len: usize, +) -> Result<(), SerializationError> { + let len = u64::try_from(len) + .map_err(|_| SerializationError::Other("length does not fit in u64".to_string()))?; + writer.write_varint(len)?; + Ok(()) +} + // Helper: encode string (UTF-8 with varint length prefix) #[tracing::instrument(level = "trace", skip(writer, s))] pub fn encode_string(writer: &mut W, s: &str) -> Result<(), SerializationError> { - writer.write_varint(s.len())?; + write_len_prefix(writer, s.len())?; writer.write_all(s.as_bytes())?; Ok(()) } @@ -106,23 +116,23 @@ impl Directory { } // Write file entries - writer.write_varint(self.files.len() as u64)?; + write_len_prefix(writer, self.files.len())?; for (id, path, file) in &self.files { writer.write_varint::(id)?; encode_string(writer, path)?; file.serialize(writer)?; } - writer.write_varint(self.blocks.len() as u64)?; + write_len_prefix(writer, self.blocks.len())?; for (hash, block) in &self.blocks { writer.write_all(hash)?; block.serialize(writer)?; } - writer.write_varint(self.relations.len())?; + write_len_prefix(writer, self.relations.len())?; for (idx, name) in &self.relations { writer.write_varint(*idx)?; encode_string(writer, name)?; } - writer.write_varint(self.encryption.len() as u64)?; + write_len_prefix(writer, self.encryption.len())?; for (key, enc) in &self.encryption { writer.write_all(key)?; enc.serialize(writer)?; @@ -148,12 +158,12 @@ impl BlockDataState { match self { BlockDataState::Encrypted(data) => { writer.write_all(&[0u8])?; - writer.write_varint(data.len())?; + write_len_prefix(writer, data.len())?; writer.write_all(data)?; } BlockDataState::Decrypted(list) => { writer.write_all(&[1u8])?; - writer.write_varint(list.len())?; + write_len_prefix(writer, list.len())?; for (hash, key) in list { writer.write_all(hash)?; writer.write_all(key)?; @@ -180,7 +190,7 @@ impl FileEntry { writer.write_varint(self.modified)?; writer.write_varint(self.file_size)?; writer.write_varint(self.permissions)?; - writer.write_varint(self.references.len())?; + write_len_prefix(writer, self.references.len())?; for r in &self.references { r.serialize(writer)?; } @@ -214,7 +224,7 @@ impl Reference { impl EncryptionSection { #[tracing::instrument(level = "trace", skip(self, writer))] pub fn serialize(&self, writer: &mut W) -> Result<(), SerializationError> { - writer.write_varint(self.recipients.len() as u64)?; + write_len_prefix(writer, self.recipients.len())?; for (key, recipient) in &self.recipients { writer.write_all(key)?; recipient.serialize(writer)?; @@ -229,12 +239,12 @@ impl RecipientData { match self { RecipientData::Encrypted(data) => { writer.write_all(&[0u8])?; - writer.write_varint(data.len())?; + write_len_prefix(writer, data.len())?; writer.write_all(data)?; } RecipientData::Decrypted(list) => { writer.write_all(&[1u8])?; - writer.write_varint(list.len())?; + write_len_prefix(writer, list.len())?; for (idx, hash) in list { writer.write_varint(*idx)?; writer.write_all(hash)?; diff --git a/crates/pithos_lib/src/model/structs.rs b/crates/pithos_lib/src/model/structs.rs index 23e728e..2365f16 100644 --- a/crates/pithos_lib/src/model/structs.rs +++ b/crates/pithos_lib/src/model/structs.rs @@ -5,6 +5,8 @@ use std::fmt::{Display, Formatter}; use crate::error::PithosError; use crate::helpers::file_entry_map::{FileEntryMap, Key}; +use crate::model::deserialization::DeserializationLimits; +use crate::model::serialization::write_len_prefix; use indexmap::IndexMap; use integer_encoding::VarIntWriter; use std::fs::{Metadata, symlink_metadata}; @@ -198,7 +200,7 @@ impl BlockDataState { } BlockDataState::Decrypted(entries) => { let mut data_bytes = Vec::new(); - data_bytes.write_varint(entries.len())?; + write_len_prefix(&mut data_bytes, entries.len())?; for (hash, key) in entries { data_bytes.write_all(hash)?; data_bytes.write_all(key)?; @@ -214,11 +216,20 @@ impl BlockDataState { #[tracing::instrument(level = "trace", skip(self, key))] pub fn decrypt(&mut self, key: &[u8; 32]) -> Result<(), PithosError> { + self.decrypt_with_limits(key, &DeserializationLimits::default()) + } + + #[tracing::instrument(level = "trace", skip(self, key, limits))] + pub fn decrypt_with_limits( + &mut self, + key: &[u8; 32], + limits: &DeserializationLimits, + ) -> Result<(), PithosError> { match &self { BlockDataState::Encrypted(data) => { let decrypted_bytes = decrypt_chunk(data, key)?; - let block_data_entries = - self.deserialize_block_index(&mut decrypted_bytes.as_slice())?; + let block_data_entries = self + .deserialize_block_index_with_limits(&mut decrypted_bytes.as_slice(), limits)?; *self = BlockDataState::Decrypted(block_data_entries); } @@ -557,7 +568,7 @@ impl RecipientSection { } RecipientData::Decrypted(entries) => { let mut data_bytes = Vec::new(); - data_bytes.write_varint(entries.len())?; + write_len_prefix(&mut data_bytes, entries.len())?; for (idx, key) in entries { data_bytes.write_varint(*idx)?; data_bytes.write_all(key)?; @@ -591,7 +602,7 @@ impl RecipientData { } RecipientData::Decrypted(entries) => { let mut data_bytes = Vec::new(); - data_bytes.write_varint(entries.len())?; + write_len_prefix(&mut data_bytes, entries.len())?; for (idx, key) in entries { data_bytes.write_varint(*idx)?; data_bytes.write_all(key)?; @@ -611,12 +622,22 @@ impl RecipientData { pub fn decrypt( &mut self, shared_key: &SharedSecret, + ) -> Result, PithosError> { + self.decrypt_with_limits(shared_key, &DeserializationLimits::default()) + } + + #[tracing::instrument(level = "trace", skip(self, shared_key, limits))] + pub fn decrypt_with_limits( + &mut self, + shared_key: &SharedSecret, + limits: &DeserializationLimits, ) -> Result, PithosError> { let entries = match &self { RecipientData::Decrypted(entries) => entries.clone(), RecipientData::Encrypted(enc_data) => { let dec_data = decrypt_chunk(enc_data, shared_key.as_bytes())?; - let entries = self.deserialize_decrypted_list(&mut dec_data.as_slice())?; + let entries = + self.deserialize_decrypted_list_with_limits(&mut dec_data.as_slice(), limits)?; *self = RecipientData::Decrypted(entries.clone()); entries diff --git a/crates/pithos_lib/tests/marshalling.rs b/crates/pithos_lib/tests/marshalling.rs index e6b67eb..80292e0 100644 --- a/crates/pithos_lib/tests/marshalling.rs +++ b/crates/pithos_lib/tests/marshalling.rs @@ -3,11 +3,123 @@ use pithos_lib::error::PithosError; use pithos_lib::helpers::directory::DirectoryBuilder; use pithos_lib::helpers::file_entry_map::{FileEntryMap, Key}; use pithos_lib::helpers::x25519_keys::generate_private_key; +use pithos_lib::model::deserialization::{DeserializationError, DeserializationLimits}; use pithos_lib::model::serialization::encode_string; use pithos_lib::model::structs::*; use std::io::Cursor; use x25519_dalek::PublicKey; +#[test] +fn robust_overlong_zero_file_count_is_accepted() { + let mut bytes = hex_bytes("504954484f53445200000000000000000000000019b1674081"); + bytes.splice(9..10, [0x80, 0x00]); + let length = bytes.len() as u64; + let footer_start = bytes.len() - 12; + bytes[footer_start..footer_start + 8].copy_from_slice(&length.to_be_bytes()); + let crc_start = bytes.len() - 4; + let crc = crc32fast::hash(&bytes[..crc_start]); + bytes[crc_start..].copy_from_slice(&crc.to_be_bytes()); + + let directory = Directory::deserialize(&mut Cursor::new(bytes)).unwrap(); + assert!(directory.files.is_empty()); +} + +#[test] +fn robust_typed_narrow_overflow_is_rejected() { + let mut bytes = b"PITH".to_vec(); + use integer_encoding::VarIntWriter; + bytes.write_varint(u64::from(u16::MAX) + 1).unwrap(); + + assert!(FileHeader::deserialize(&mut Cursor::new(bytes)).is_err()); +} + +#[test] +fn robust_u64_tenth_byte_overflow_is_rejected() { + let bytes = [0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x02]; + + assert!(BlockIndexEntry::deserialize(&mut Cursor::new(bytes)).is_err()); +} + +#[test] +fn robust_unterminated_varint_is_rejected() { + let bytes = [b'P', b'I', b'T', b'H', 0x80]; + + assert!(FileHeader::deserialize(&mut Cursor::new(bytes)).is_err()); +} + +#[test] +fn robust_maximum_typed_values_roundtrip() { + let header = FileHeader { + magic: *b"PITH", + version: u16::MAX, + }; + let mut header_bytes = Vec::new(); + header.serialize(&mut header_bytes).unwrap(); + assert_eq!( + FileHeader::deserialize(&mut Cursor::new(header_bytes)).unwrap(), + header + ); + + let entry = FileEntry { + file_type: FileType::Data, + block_data: BlockDataState::Decrypted(vec![]), + created: u64::MAX, + modified: u64::MAX, + file_size: u64::MAX, + permissions: u32::MAX, + references: vec![], + symlink_target: None, + }; + let mut entry_bytes = Vec::new(); + entry.serialize(&mut entry_bytes).unwrap(); + assert_eq!( + FileEntry::deserialize(&mut Cursor::new(entry_bytes)).unwrap(), + entry + ); +} + +#[test] +fn robust_over_policy_file_count_is_rejected_before_entry_parsing() { + let mut bytes = hex_bytes("504954484f53445200"); + use integer_encoding::VarIntWriter; + bytes.write_varint(1_000_001u64).unwrap(); + + let error = Directory::deserialize(&mut Cursor::new(bytes)).unwrap_err(); + assert!(error.to_string().contains("limit")); +} + +#[test] +fn robust_custom_collection_limit_applies_to_blocks() { + let directory = Directory { + identifier: *b"PITHOSDR", + parent_directory_offset: None, + files: FileEntryMap::new(), + blocks: IndexMap::from_iter([( + [1u8; 32], + BlockIndexEntry { + offset: 0, + stored_size: 0, + original_size: 0, + flags: ProcessingFlags(0), + location: BlockLocation::Local, + }, + )]), + relations: vec![], + encryption: IndexMap::new(), + dir_len: 0, + crc32: 0, + }; + let mut bytes = Vec::new(); + directory.serialize(&mut bytes).unwrap(); + let limits = DeserializationLimits { + max_collection_entries: 0, + ..DeserializationLimits::default() + }; + + let error = Directory::deserialize_with_limits(&mut Cursor::new(bytes), &limits).unwrap_err(); + assert!(error.to_string().contains("blocks")); +} + #[test] fn test_varint_encoding() { use integer_encoding::VarIntWriter; @@ -400,6 +512,113 @@ fn recipient_data_roundtrip() { } } +#[test] +fn block_data_state_decrypt_with_limits_rejects_oversized_block_index() { + let key = [7u8; 32]; + let mut block_data = + BlockDataState::Decrypted(vec![([1u8; 32], [2u8; 32]), ([3u8; 32], [4u8; 32])]); + block_data.encrypt(key).unwrap(); + let limits = DeserializationLimits { + max_collection_entries: 1, + ..DeserializationLimits::default() + }; + + let error = block_data.decrypt_with_limits(&key, &limits).unwrap_err(); + assert!(matches!( + error, + PithosError::Deserialization(DeserializationError::LimitExceeded { + field: "block index", + limit: 1, + actual: 2, + }) + )); +} + +#[test] +fn recipient_data_decrypt_with_limits_rejects_oversized_recipient_keys() { + let sender_key = generate_private_key().unwrap(); + let recipient_key = generate_private_key().unwrap(); + let shared_key = sender_key.diffie_hellman(&PublicKey::from(&recipient_key)); + let mut recipient_data = RecipientData::Decrypted(vec![(1, [2u8; 32]), (3, [4u8; 32])]); + recipient_data.encrypt(&shared_key).unwrap(); + let limits = DeserializationLimits { + max_collection_entries: 1, + ..DeserializationLimits::default() + }; + + let error = recipient_data + .decrypt_with_limits(&shared_key, &limits) + .unwrap_err(); + assert!(matches!( + error, + PithosError::Deserialization(DeserializationError::LimitExceeded { + field: "recipient keys", + limit: 1, + actual: 2, + }) + )); +} + +#[test] +fn directory_decrypt_with_limits_recipient_path_propagates_recipient_limit() { + let sender_key = generate_private_key().unwrap(); + let recipient_key = generate_private_key().unwrap(); + let shared_key = sender_key.diffie_hellman(&PublicKey::from(&recipient_key)); + let mut recipient_data = RecipientData::Decrypted(vec![(1, [2u8; 32]), (3, [4u8; 32])]); + recipient_data.encrypt(&shared_key).unwrap(); + let mut directory = Directory { + identifier: *b"PITHOSDR", + parent_directory_offset: None, + files: FileEntryMap::new(), + blocks: IndexMap::new(), + relations: vec![], + encryption: IndexMap::from_iter([( + PublicKey::from(&sender_key).to_bytes(), + EncryptionSection { + recipients: IndexMap::from_iter([( + PublicKey::from(&recipient_key).to_bytes(), + RecipientSection { recipient_data }, + )]), + }, + )]), + dir_len: 0, + crc32: 0, + }; + let limits = DeserializationLimits { + max_collection_entries: 1, + ..DeserializationLimits::default() + }; + + let error = directory + .decrypt_recipient_with_limits(&sender_key, &limits) + .unwrap_err(); + assert!(matches!( + error, + PithosError::Deserialization(DeserializationError::LimitExceeded { + field: "recipient keys", + limit: 1, + actual: 2, + }) + )); +} + +#[test] +fn decrypt_with_limits_default_wrappers_decrypt_small_payloads() { + let block_key = [7u8; 32]; + let mut block_data = BlockDataState::Decrypted(vec![([1u8; 32], [2u8; 32])]); + block_data.encrypt(block_key).unwrap(); + block_data.decrypt(&block_key).unwrap(); + assert!(matches!(block_data, BlockDataState::Decrypted(entries) if entries.len() == 1)); + + let sender_key = generate_private_key().unwrap(); + let recipient_key = generate_private_key().unwrap(); + let shared_key = sender_key.diffie_hellman(&PublicKey::from(&recipient_key)); + let mut recipient_data = RecipientData::Decrypted(vec![(1, [2u8; 32])]); + recipient_data.encrypt(&shared_key).unwrap(); + recipient_data.decrypt(&shared_key).unwrap(); + assert!(matches!(recipient_data, RecipientData::Decrypted(entries) if entries.len() == 1)); +} + #[test] fn recipient_section_roundtrip() { let original = RecipientSection { diff --git a/crates/pithos_lib/tests/reader.rs b/crates/pithos_lib/tests/reader.rs index 65ec63e..0542f11 100644 --- a/crates/pithos_lib/tests/reader.rs +++ b/crates/pithos_lib/tests/reader.rs @@ -4,12 +4,12 @@ use crate::common::util::{create_pithos_writer, load_test_keys, write_dummy_pith use pithos_lib::error::PithosError; use pithos_lib::helpers::chacha_poly1305::decrypt_chunk; use pithos_lib::helpers::crypt4gh::{ - CRYPT4GH_ENCRYPTED_BLOCK_SIZE, Crypt4GHHeader, Packet, PacketData, + CRYPT4GH_ENCRYPTED_BLOCK_SIZE, Crypt4GHError, Crypt4GHHeader, Packet, PacketData, }; use pithos_lib::helpers::file_entry_map::Key; use pithos_lib::helpers::ro_crate::{RoCrateSource, read_ro_crate_directory, read_ro_crate_zip}; use pithos_lib::helpers::x25519_keys::{private_key_from_pem_bytes, public_key_from_pem_bytes}; -use pithos_lib::io::pithosreader::PithosReaderSimple; +use pithos_lib::io::pithosreader::{PithosReaderSimple, ReaderLimits}; use pithos_lib::io::pithoswriter::{Content, InputFile, PithosWriter}; use pithos_lib::model::structs::{BlockDataState, FileEntry, FileType, Reference}; use rocraters::ro_crate::graph_vector::GraphVector; @@ -75,6 +75,28 @@ fn append_empty_directory(path: &Path) { drop(writer); } +fn complete_test_directory(parent: Option<(u64, u64)>) -> Vec { + let mut directory = pithos_lib::model::structs::Directory { + identifier: *b"PITHOSDR", + parent_directory_offset: parent, + files: pithos_lib::helpers::file_entry_map::FileEntryMap::new(), + blocks: indexmap::IndexMap::new(), + relations: vec![], + encryption: indexmap::IndexMap::new(), + dir_len: 0, + crc32: 0, + }; + let mut bytes = Vec::new(); + directory.serialize(&mut bytes).unwrap(); + directory.dir_len = bytes.len() as u64; + bytes.clear(); + directory.serialize(&mut bytes).unwrap(); + let crc_start = bytes.len() - 4; + let crc = crc32fast::hash(&bytes[..crc_start]); + bytes[crc_start..].copy_from_slice(&crc.to_be_bytes()); + bytes +} + fn directory_bounds(bytes: &[u8]) -> (usize, usize) { let length = u64::from_be_bytes(bytes[bytes.len() - 12..bytes.len() - 4].try_into().unwrap()) as usize; @@ -295,6 +317,256 @@ fn test_directory_integrity_valid_append_chain() { assert!(reader.read_directory().is_ok()); } +#[test] +fn test_robust_terminal_footer_max_length_returns_error() { + let temp_dir = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp_dir, false, false); + let mut bytes = read(&archive).unwrap(); + let footer_start = bytes.len() - 12; + bytes[footer_start..footer_start + 8].copy_from_slice(&u64::MAX.to_be_bytes()); + write(&archive, bytes).unwrap(); + + let result = PithosReaderSimple::new_with_key(&archive, reader_key()) + .unwrap() + .read_directory(); + assert!(matches!( + result, + Err(PithosError::LimitExceeded { + field: "directory", + .. + }) + )); +} + +#[test] +fn test_robust_overlapping_parent_is_rejected_as_invalid_chain() { + let temp_dir = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp_dir, false, false); + append_empty_directory(&archive); + let bytes = read(&archive).unwrap(); + let (final_start, final_length) = directory_bounds(&bytes); + let mut final_directory = pithos_lib::model::structs::Directory::deserialize( + &mut std::io::Cursor::new(&bytes[final_start..]), + ) + .unwrap(); + final_directory.parent_directory_offset = Some(((final_start + 1) as u64, final_length as u64)); + final_directory.dir_len = 0; + final_directory.crc32 = 0; + let mut replacement = Vec::new(); + final_directory.serialize(&mut replacement).unwrap(); + final_directory.dir_len = replacement.len() as u64; + replacement.clear(); + final_directory.serialize(&mut replacement).unwrap(); + let crc = crc32fast::hash(&replacement[..replacement.len() - 4]); + let crc_start = replacement.len() - 4; + replacement[crc_start..].copy_from_slice(&crc.to_be_bytes()); + let mut archive_bytes = bytes[..final_start].to_vec(); + archive_bytes.extend_from_slice(&replacement); + write(&archive, archive_bytes).unwrap(); + + let mut reader = PithosReaderSimple::new_with_key(&archive, reader_key()).unwrap(); + let error = reader.read_directory().unwrap_err(); + assert!(error.to_string().contains("chain")); +} + +#[test] +fn test_robust_parent_boundary_uses_immediate_child() { + let temp_dir = TempDir::new().unwrap(); + let middle = complete_test_directory(None); + let oldest = complete_test_directory(Some((27, middle.len() as u64))); + let terminal = complete_test_directory(Some((0, oldest.len() as u64))); + assert_eq!(oldest.len(), 27); + assert_eq!(middle.len(), 25); + let archive = temp_dir.path().join("three-directories.pithos"); + let mut bytes = oldest; + bytes.extend_from_slice(&middle); + bytes.extend_from_slice(&terminal); + write(&archive, bytes).unwrap(); + + let mut reader = PithosReaderSimple::new_with_key(&archive, reader_key()).unwrap(); + let error = reader.read_directory().unwrap_err(); + assert!( + matches!(error, PithosError::InvalidDirectoryChain(_)), + "unexpected chain fixture error: {error:?}" + ); +} + +#[test] +fn test_robust_oversized_stored_block_returns_error_without_commit() { + let temp_dir = TempDir::new().unwrap(); + let (archive, key, mut directory, _) = write_identity_integrity_archive(&temp_dir); + let hash = match directory + .get_file_by_path("integrity.txt") + .unwrap() + .block_data + .clone() + { + BlockDataState::Decrypted(blocks) => blocks[0].0, + BlockDataState::Encrypted(_) => unreachable!(), + }; + directory.blocks.get_mut(&hash).unwrap().stored_size = u64::MAX; + let output = temp_dir.path().join("oversized.txt"); + let result = PithosReaderSimple::new_with_key(&archive, key) + .unwrap() + .read_file("integrity.txt", &directory, Some(&output), None); + assert!(matches!( + result, + Err(PithosError::LimitExceeded { + field: "stored block", + .. + }) + )); + assert!(!output.exists()); +} + +#[test] +fn test_robust_zero_crypt4gh_packet_length_returns_error() { + let mut bytes = Vec::from(b"crypt4gh".as_slice()); + bytes.extend_from_slice(&1u32.to_le_bytes()); + bytes.extend_from_slice(&1u32.to_le_bytes()); + bytes.extend_from_slice(&0u32.to_le_bytes()); + bytes.extend_from_slice(&[0u8; 68]); + let result = Crypt4GHHeader::try_from(bytes.as_slice()); + assert!(matches!(result, Err(Crypt4GHError::FromBytesError(_)))); +} + +#[test] +fn test_robust_skipped_range_enforces_decoded_block_limit() { + let temp_dir = TempDir::new().unwrap(); + let (archive, key, directory, hash) = write_identity_integrity_archive(&temp_dir); + let file_entry = directory.get_file_by_path("integrity.txt").unwrap(); + let block_end = directory.blocks.get(&hash).unwrap().original_size; + let limits = ReaderLimits { + max_decoded_block_bytes: 0, + ..ReaderLimits::default() + }; + let mut reader = PithosReaderSimple::new_with_key(&archive, key) + .unwrap() + .with_limits(limits); + let mut sink: Box = Box::new(Vec::::new()); + assert!(matches!( + reader.read_data_range_to_sink( + block_end..block_end + 1, + file_entry, + &directory.blocks, + &mut sink, + ), + Err(PithosError::LimitExceeded { + field: "decoded block", + .. + }) + )); +} + +#[test] +fn test_robust_skipped_range_enforces_stored_block_limit() { + let temp_dir = TempDir::new().unwrap(); + let (archive, key, directory, hash) = write_identity_integrity_archive(&temp_dir); + let file_entry = directory.get_file_by_path("integrity.txt").unwrap(); + let block_end = directory.blocks.get(&hash).unwrap().original_size; + let limits = ReaderLimits { + max_stored_block_bytes: 0, + ..ReaderLimits::default() + }; + let mut reader = PithosReaderSimple::new_with_key(&archive, key) + .unwrap() + .with_limits(limits); + let mut sink: Box = Box::new(Vec::::new()); + assert!(matches!( + reader.read_data_range_to_sink( + block_end..block_end + 1, + file_entry, + &directory.blocks, + &mut sink, + ), + Err(PithosError::LimitExceeded { + field: "stored block", + .. + }) + )); +} + +#[test] +fn test_robust_configured_directory_limit_returns_limit_error() { + let temp_dir = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp_dir, false, false); + let limits = ReaderLimits { + max_directory_bytes: 24, + ..ReaderLimits::default() + }; + let mut reader = PithosReaderSimple::new_with_key(&archive, reader_key()) + .unwrap() + .with_limits(limits); + assert!(matches!( + reader.read_directory(), + Err(PithosError::LimitExceeded { + field: "directory", + .. + }) + )); +} + +#[test] +fn test_robust_configured_stored_block_limit_returns_limit_error() { + let temp_dir = TempDir::new().unwrap(); + let (archive, key, directory, _) = write_identity_integrity_archive(&temp_dir); + let limits = ReaderLimits { + max_stored_block_bytes: 0, + ..ReaderLimits::default() + }; + let mut reader = PithosReaderSimple::new_with_key(&archive, key) + .unwrap() + .with_limits(limits); + assert!(matches!( + reader.read_file("integrity.txt", &directory, None, None), + Err(PithosError::LimitExceeded { + field: "stored block", + .. + }) + )); +} + +#[test] +fn test_robust_configured_decoded_block_limit_returns_limit_error() { + let temp_dir = TempDir::new().unwrap(); + let (archive, key, directory, _) = write_identity_integrity_archive(&temp_dir); + let limits = ReaderLimits { + max_decoded_block_bytes: 0, + ..ReaderLimits::default() + }; + let mut reader = PithosReaderSimple::new_with_key(&archive, key) + .unwrap() + .with_limits(limits); + assert!(matches!( + reader.read_file("integrity.txt", &directory, None, None), + Err(PithosError::LimitExceeded { + field: "decoded block", + .. + }) + )); +} + +#[test] +fn test_robust_configured_parent_depth_returns_limit_error() { + let temp_dir = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp_dir, false, false); + append_empty_directory(&archive); + let limits = ReaderLimits { + max_parent_directories: 0, + ..ReaderLimits::default() + }; + let mut reader = PithosReaderSimple::new_with_key(&archive, reader_key()) + .unwrap() + .with_limits(limits); + assert!(matches!( + reader.read_directory(), + Err(PithosError::LimitExceeded { + field: "parent directories", + .. + }) + )); +} + #[test] fn test_directory_integrity_parent_embedded_length_mismatch() { let temp_dir = TempDir::new().unwrap(); diff --git a/crates/pithos_lib/tests/writer.rs b/crates/pithos_lib/tests/writer.rs index 6af7531..f9fe71b 100644 --- a/crates/pithos_lib/tests/writer.rs +++ b/crates/pithos_lib/tests/writer.rs @@ -8,7 +8,7 @@ use pithos_lib::error::PithosError; use pithos_lib::helpers::file_entry_map::KeyQuery; use pithos_lib::helpers::ro_crate::{LoadedRoCrate, read_ro_crate_directory, read_ro_crate_zip}; use pithos_lib::helpers::x25519_keys::private_key_from_pem_bytes; -use pithos_lib::io::pithosreader::PithosReaderSimple; +use pithos_lib::io::pithosreader::{PithosReaderSimple, ReaderLimits}; use pithos_lib::io::pithoswriter::{Content, InputFile, PithosWriter}; use pithos_lib::model::structs::{FileType, Reference}; use std::fs::{ @@ -377,6 +377,61 @@ fn test_append_single_file() { assert_eq!(entry.file_size, 342848); } +#[test] +fn new_from_file_with_limits_accepts_exact_terminal_directory_length() { + let temp_dir = TempDir::new().unwrap(); + let (path, sender_key, mut writer) = create_pithos_writer(&temp_dir, None); + writer.write_file_header().unwrap(); + writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "file.txt".into(), + data: Content::Raw("payload".into()), + metadata: None, + encrypt: true, + compression_level: Some(0), + }) + .unwrap(); + writer.write_directory().unwrap(); + + let bytes = read(&path).unwrap(); + let directory_length = + u64::from_be_bytes(bytes[bytes.len() - 12..bytes.len() - 4].try_into().unwrap()); + let limits = ReaderLimits { + max_directory_bytes: directory_length, + ..ReaderLimits::default() + }; + PithosWriter::new_from_file_with_limits(sender_key, vec![], None, &path, limits).unwrap(); +} + +#[test] +fn new_from_file_with_limits_rejects_smaller_terminal_directory_length() { + let temp_dir = TempDir::new().unwrap(); + let (path, sender_key, mut writer) = create_pithos_writer(&temp_dir, None); + writer.write_file_header().unwrap(); + writer.write_directory().unwrap(); + + let bytes = read(&path).unwrap(); + let directory_length = + u64::from_be_bytes(bytes[bytes.len() - 12..bytes.len() - 4].try_into().unwrap()); + let limits = ReaderLimits { + max_directory_bytes: directory_length - 1, + ..ReaderLimits::default() + }; + let error = + match PithosWriter::new_from_file_with_limits(sender_key, vec![], None, &path, limits) { + Ok(_) => panic!("directory limit should reject the archive"), + Err(error) => error, + }; + assert!(matches!( + error, + PithosError::LimitExceeded { + field: "directory", + .. + } + )); +} + #[test] fn test_multiple_files() { // Dummy files with metadata From a7e5711337bc35cb88ba91b95ccf7dc34eb8e51a Mon Sep 17 00:00:00 2001 From: Jannis Schlegel Date: Mon, 20 Jul 2026 16:08:25 +0200 Subject: [PATCH 07/14] fix: add support for external block sources and refactor block loading logic --- Cargo.lock | 518 ----------------------- crates/pithos_lib/Cargo.toml | 1 - crates/pithos_lib/src/error.rs | 4 + crates/pithos_lib/src/io/pithosreader.rs | 108 +++-- crates/pithos_lib/tests/reader.rs | 224 +++++++++- 5 files changed, 287 insertions(+), 568 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d5b1654..df65ccd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -116,29 +116,6 @@ version = "1.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" -[[package]] -name = "aws-lc-rs" -version = "1.17.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4342d8937fc7e5dd9b1c60292261c0670c882a2cd1719cfc11b1af41731e32ad" -dependencies = [ - "aws-lc-sys", - "zeroize", -] - -[[package]] -name = "aws-lc-sys" -version = "0.42.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6d9ceb1da931507a12f4fccea479dccd00da1943e1b4ae72d8e502d707361444" -dependencies = [ - "cc", - "cmake", - "dunce", - "fs_extra", - "pkg-config", -] - [[package]] name = "base64" version = "0.22.1" @@ -246,12 +223,6 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" -[[package]] -name = "cfg_aliases" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" - [[package]] name = "chacha20" version = "0.10.1" @@ -341,15 +312,6 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c8d4a3bb8b1e0c1050499d1815f5ab16d04f0959b233085fb31653fbfc9d98f9" -[[package]] -name = "cmake" -version = "0.1.58" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c0f78a02292a74a88ac736019ab962ece0bc380e3f977bf72e376c5d78ff0678" -dependencies = [ - "cc", -] - [[package]] name = "cmov" version = "0.5.4" @@ -362,16 +324,6 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" -[[package]] -name = "combine" -version = "4.6.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba5a308b75df32fe02788e748662718f03fde005016435c444eea572398219fd" -dependencies = [ - "bytes", - "memchr", -] - [[package]] name = "const-oid" version = "0.10.2" @@ -384,26 +336,6 @@ version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3d52eff69cd5e647efe296129160853a42795992097e8af39800e1060caeea9b" -[[package]] -name = "core-foundation" -version = "0.9.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "91e195e091a93c46f7102ec7818a2aa394e1e1771c3ab4825963fa03e45afb8f" -dependencies = [ - "core-foundation-sys", - "libc", -] - -[[package]] -name = "core-foundation" -version = "0.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b2a6cd9ae233e7f62ba4e9353e81a88df7fc8a5987b8d445b4d90c879bd156f6" -dependencies = [ - "core-foundation-sys", - "libc", -] - [[package]] name = "core-foundation-sys" version = "0.8.7" @@ -533,21 +465,6 @@ dependencies = [ "syn", ] -[[package]] -name = "dunce" -version = "1.0.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "92773504d58c093f6de2459af4af33faa518c13451eb8f2b5698ed3d36e7c813" - -[[package]] -name = "encoding_rs" -version = "0.8.35" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "75030f3c4f45dafd7586dd6780965a8c7e8e285a5ecb86713e63a79c5b2766f3" -dependencies = [ - "cfg-if", -] - [[package]] name = "equivalent" version = "1.0.2" @@ -598,12 +515,6 @@ dependencies = [ "zlib-rs", ] -[[package]] -name = "fnv" -version = "1.0.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" - [[package]] name = "form_urlencoded" version = "1.2.2" @@ -624,12 +535,6 @@ dependencies = [ "windows-sys 0.52.0", ] -[[package]] -name = "fs_extra" -version = "1.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "42703706b716c37f96a77aea830392ad231f44c9e9a67872fa5548707e11b11c" - [[package]] name = "futures-channel" version = "0.3.32" @@ -686,10 +591,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ff2abc00be7fca6ebc474524697ae276ad847ad0a6b3faa4bcb027e9a4614ad0" dependencies = [ "cfg-if", - "js-sys", "libc", "wasi", - "wasm-bindgen", ] [[package]] @@ -699,30 +602,9 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "300e883d756b2e4ec94e02791f39b04b522276138852cfc41d9fb7e904106099" dependencies = [ "cfg-if", - "js-sys", "libc", "r-efi", "rand_core", - "wasm-bindgen", -] - -[[package]] -name = "h2" -version = "0.4.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6cb093c84e8bd9b188d4c4a8cb6579fc016968d14c99882163cd3ff402a4f155" -dependencies = [ - "atomic-waker", - "bytes", - "fnv", - "futures-core", - "futures-sink", - "http", - "indexmap", - "slab", - "tokio", - "tokio-util", - "tracing", ] [[package]] @@ -795,7 +677,6 @@ dependencies = [ "bytes", "futures-channel", "futures-core", - "h2", "http", "http-body", "httparse", @@ -806,21 +687,6 @@ dependencies = [ "want", ] -[[package]] -name = "hyper-rustls" -version = "0.27.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "33ca68d021ef39cf6463ab54c1d0f5daf03377b70561305bb89a8f83aab66e0f" -dependencies = [ - "http", - "hyper", - "hyper-util", - "rustls", - "tokio", - "tokio-rustls", - "tower-service", -] - [[package]] name = "hyper-util" version = "0.1.20" @@ -839,11 +705,9 @@ dependencies = [ "percent-encoding", "pin-project-lite", "socket2", - "system-configuration", "tokio", "tower-service", "tracing", - "windows-registry", ] [[package]] @@ -1038,55 +902,6 @@ version = "1.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" -[[package]] -name = "jni" -version = "0.22.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5efd9a482cf3a427f00d6b35f14332adc7902ce91efb778580e180ff90fa3498" -dependencies = [ - "cfg-if", - "combine", - "jni-macros", - "jni-sys", - "log", - "simd_cesu8", - "thiserror", - "walkdir", - "windows-link", -] - -[[package]] -name = "jni-macros" -version = "0.22.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a00109accc170f0bdb141fed3e393c565b6f5e072365c3bd58f5b062591560a3" -dependencies = [ - "proc-macro2", - "quote", - "rustc_version", - "simd_cesu8", - "syn", -] - -[[package]] -name = "jni-sys" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c6377a88cb3910bee9b0fa88d4f42e1d2da8e79915598f65fb0c7ee14c878af2" -dependencies = [ - "jni-sys-macros", -] - -[[package]] -name = "jni-sys-macros" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "38c0b942f458fe50cdac086d2f946512305e5631e720728f2a61aabcd47a6264" -dependencies = [ - "quote", - "syn", -] - [[package]] name = "jobserver" version = "0.1.35" @@ -1157,12 +972,6 @@ version = "0.4.33" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0ceec5bc11778974d1bcb055b18002eba7f4b3518b6a0081b3af5f21666da9ad" -[[package]] -name = "lru-slab" -version = "0.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "112b39cec0b298b6c1999fee3e31427f74f676e4cb9879ed1a121b43661a4154" - [[package]] name = "matchers" version = "0.2.0" @@ -1184,12 +993,6 @@ version = "2.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98" -[[package]] -name = "mime" -version = "0.3.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6877bb514081ee2a7ff5ef9de3281f14a4dd4bceac4c09388074a6b5df8a139a" - [[package]] name = "miniz_oxide" version = "0.8.9" @@ -1247,12 +1050,6 @@ version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" -[[package]] -name = "openssl-probe" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7c87def4c32ab89d880effc9e097653c8da5d6ef28e6b539d313baaacfbafcbe" - [[package]] name = "option-ext" version = "0.2.0" @@ -1306,7 +1103,6 @@ dependencies = [ "indexmap", "integer-encoding", "pkcs8", - "reqwest", "ro-crate-rs", "shake", "tempfile", @@ -1375,63 +1171,6 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "quinn" -version = "0.11.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c1a41e437b6bbd489372cd4971de128e85c855f56c57f283d20ff016cf7c0a8" -dependencies = [ - "bytes", - "cfg_aliases", - "pin-project-lite", - "quinn-proto", - "quinn-udp", - "rustc-hash", - "rustls", - "socket2", - "thiserror", - "tokio", - "tracing", - "web-time", -] - -[[package]] -name = "quinn-proto" -version = "0.11.16" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2f4bfc015262b9df63c8845072ce59068853ff5872180c2ce2f13038b970e560" -dependencies = [ - "aws-lc-rs", - "bytes", - "getrandom 0.4.3", - "lru-slab", - "rand", - "rand_pcg", - "ring", - "rustc-hash", - "rustls", - "rustls-pki-types", - "slab", - "thiserror", - "tinyvec", - "tracing", - "web-time", -] - -[[package]] -name = "quinn-udp" -version = "0.5.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "35a133f956daabe89a61a685c2649f13d82d5aa4bd5d12d1277e1072a21c0694" -dependencies = [ - "cfg_aliases", - "libc", - "once_cell", - "socket2", - "tracing", - "windows-sys 0.61.2", -] - [[package]] name = "quote" version = "1.0.46" @@ -1464,15 +1203,6 @@ version = "0.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "63b8176103e19a2643978565ca18b50549f6101881c443590420e4dc998a3c69" -[[package]] -name = "rand_pcg" -version = "0.10.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "caa0f4137e1c0a72f4c651489402276c8e8e1cf081f3b0ba156d2cbeef09e86a" -dependencies = [ - "rand_core", -] - [[package]] name = "redox_users" version = "0.5.2" @@ -1509,31 +1239,22 @@ checksum = "219c5811de6525e5416c7d5d53bb656d3afdbc6c5af816e0802bcfa42dbdc1c3" dependencies = [ "base64", "bytes", - "encoding_rs", "futures-channel", "futures-core", "futures-util", - "h2", "http", "http-body", "http-body-util", "hyper", - "hyper-rustls", "hyper-util", "js-sys", "log", - "mime", "percent-encoding", "pin-project-lite", - "quinn", - "rustls", - "rustls-pki-types", - "rustls-platform-verifier", "serde", "serde_json", "sync_wrapper", "tokio", - "tokio-rustls", "tower", "tower-http", "tower-service", @@ -1543,20 +1264,6 @@ dependencies = [ "web-sys", ] -[[package]] -name = "ring" -version = "0.17.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4689e6c2294d81e88dc6261c768b63bc4fcdb852be6d1352498b114f61383b7" -dependencies = [ - "cc", - "cfg-if", - "getrandom 0.2.17", - "libc", - "untrusted", - "windows-sys 0.52.0", -] - [[package]] name = "ro-crate-rs" version = "0.5.1" @@ -1577,12 +1284,6 @@ dependencies = [ "zip", ] -[[package]] -name = "rustc-hash" -version = "2.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6b1e7f9a428571be2dc5bc0505c13fb6bf936822b894ec87abf8a08a4e51742d" - [[package]] name = "rustc_version" version = "0.4.1" @@ -1615,81 +1316,6 @@ dependencies = [ "rustix", ] -[[package]] -name = "rustls" -version = "0.23.42" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3c54fcab019b409d04215d3a17cb438fd7fbf192ee61461f20f4fe18704bc138" -dependencies = [ - "aws-lc-rs", - "once_cell", - "rustls-pki-types", - "rustls-webpki", - "subtle", - "zeroize", -] - -[[package]] -name = "rustls-native-certs" -version = "0.8.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dab5152771c58876a2146916e53e35057e1a4dfa2b9df0f0305b07f611fdea4d" -dependencies = [ - "openssl-probe", - "rustls-pki-types", - "schannel", - "security-framework", -] - -[[package]] -name = "rustls-pki-types" -version = "1.15.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "764899a24af3980067ee14bc143654f297b22eaebfe3c7b6b211920a5a59b046" -dependencies = [ - "web-time", - "zeroize", -] - -[[package]] -name = "rustls-platform-verifier" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "26d1e2536ce4f35f4846aa13bff16bd0ff40157cdb14cc056c7b14ba41233ba0" -dependencies = [ - "core-foundation 0.10.1", - "core-foundation-sys", - "jni", - "log", - "once_cell", - "rustls", - "rustls-native-certs", - "rustls-platform-verifier-android", - "rustls-webpki", - "security-framework", - "security-framework-sys", - "webpki-root-certs", - "windows-sys 0.61.2", -] - -[[package]] -name = "rustls-platform-verifier-android" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f87165f0995f63a9fbeea62b64d10b4d9d8e78ec6d7d51fb2125fda7bb36788f" - -[[package]] -name = "rustls-webpki" -version = "0.103.13" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "61c429a8649f110dddef65e2a5ad240f747e85f7758a6bccc7e5777bd33f756e" -dependencies = [ - "aws-lc-rs", - "ring", - "rustls-pki-types", - "untrusted", -] - [[package]] name = "rustversion" version = "1.0.23" @@ -1705,38 +1331,6 @@ dependencies = [ "winapi-util", ] -[[package]] -name = "schannel" -version = "0.1.29" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "91c1b7e4904c873ef0710c1f407dde2e6287de2bebc1bbbf7d430bb7cbffd939" -dependencies = [ - "windows-sys 0.61.2", -] - -[[package]] -name = "security-framework" -version = "3.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b7f4bc775c73d9a02cde8bf7b2ec4c9d12743edf609006c7facc23998404cd1d" -dependencies = [ - "bitflags", - "core-foundation 0.10.1", - "core-foundation-sys", - "libc", - "security-framework-sys", -] - -[[package]] -name = "security-framework-sys" -version = "2.17.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6ce2691df843ecc5d231c0b14ece2acc3efb62c0a398c7e1d875f3983ce020e3" -dependencies = [ - "core-foundation-sys", - "libc", -] - [[package]] name = "semver" version = "1.0.28" @@ -1818,22 +1412,6 @@ version = "0.3.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a219298ac11a56ea9a6d2120044824d6f01aeb034955e7af7bc16858527deea" -[[package]] -name = "simd_cesu8" -version = "1.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "11031e251abf8611c80f460e19dbdeb54a66db918e49c65a7065b46ac7aec520" -dependencies = [ - "rustc_version", - "simdutf8", -] - -[[package]] -name = "simdutf8" -version = "0.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e3a9fe34e3e7a50316060351f37187a3f546bce95496156754b601a5fa71b76e" - [[package]] name = "slab" version = "0.4.12" @@ -1921,27 +1499,6 @@ dependencies = [ "syn", ] -[[package]] -name = "system-configuration" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a13f3d0daba03132c0aa9767f98351b3488edc2c100cda2d2ec2b04f3d8d3c8b" -dependencies = [ - "bitflags", - "core-foundation 0.9.4", - "system-configuration-sys", -] - -[[package]] -name = "system-configuration-sys" -version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8e1d1b10ced5ca923a1fcb8d03e96b8d3268065d724548c0211415ff6ac6bac4" -dependencies = [ - "core-foundation-sys", - "libc", -] - [[package]] name = "tempfile" version = "3.27.0" @@ -2025,28 +1582,12 @@ dependencies = [ "zerovec", ] -[[package]] -name = "tinyvec" -version = "1.12.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bb4ebadaa0af04fab11ae01eb5f9fdb5f9c5b875506e210e71c07873528baa7f" -dependencies = [ - "tinyvec_macros", -] - -[[package]] -name = "tinyvec_macros" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" - [[package]] name = "tokio" version = "1.52.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8fc7f01b389ac15039e4dc9531aa973a135d7a4135281b12d7c1bc79fd57fffe" dependencies = [ - "bytes", "libc", "mio", "pin-project-lite", @@ -2054,29 +1595,6 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "tokio-rustls" -version = "0.26.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1729aa945f29d91ba541258c8df89027d5792d85a8841fb65e8bf0f4ede4ef61" -dependencies = [ - "rustls", - "tokio", -] - -[[package]] -name = "tokio-util" -version = "0.7.18" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9ae9cec805b01e8fc3fd2fe289f89149a9b66dd16786abd8b19cfa7b48cb0098" -dependencies = [ - "bytes", - "futures-core", - "futures-sink", - "pin-project-lite", - "tokio", -] - [[package]] name = "tower" version = "0.5.3" @@ -2218,12 +1736,6 @@ dependencies = [ "ctutils", ] -[[package]] -name = "untrusted" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ecb6da28b8a351d773b68d5825ac39017e680750f980f3a1a85cd8dd28a47c1" - [[package]] name = "url" version = "2.5.8" @@ -2355,25 +1867,6 @@ dependencies = [ "wasm-bindgen", ] -[[package]] -name = "web-time" -version = "1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a6580f308b1fad9207618087a65c04e7a10bc77e02c8e84e9b00dd4b12fa0bb" -dependencies = [ - "js-sys", - "wasm-bindgen", -] - -[[package]] -name = "webpki-root-certs" -version = "1.0.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0d46a5a140e6f7afeccd8eae97eff335163939eac8b929834875168b29b3d267" -dependencies = [ - "rustls-pki-types", -] - [[package]] name = "winapi-util" version = "0.1.11" @@ -2424,17 +1917,6 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" -[[package]] -name = "windows-registry" -version = "0.6.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "02752bf7fbdcce7f2a27a742f798510f3e5ad88dbe84871e5168e2120c3d5720" -dependencies = [ - "windows-link", - "windows-result", - "windows-strings", -] - [[package]] name = "windows-result" version = "0.4.1" diff --git a/crates/pithos_lib/Cargo.toml b/crates/pithos_lib/Cargo.toml index 46f6e19..f1e44e5 100644 --- a/crates/pithos_lib/Cargo.toml +++ b/crates/pithos_lib/Cargo.toml @@ -21,7 +21,6 @@ fastcdc = "4.0.1" indexmap = "2.11.0" integer-encoding = "4.1.0" pkcs8 = { version = "0.11.0", features = ["pem", "alloc"] } -reqwest = { version = "0.13.4", features = ["blocking"] } ro-crate-rs = { version = "0.5.1", default-features = false } shake = "0.1.0" thiserror = { workspace = true } diff --git a/crates/pithos_lib/src/error.rs b/crates/pithos_lib/src/error.rs index ea3163b..c927908 100644 --- a/crates/pithos_lib/src/error.rs +++ b/crates/pithos_lib/src/error.rs @@ -83,6 +83,10 @@ pub enum PithosError { UnsupportedZipEntry(String), #[error("Invalid block data state: {0}")] InvalidBlockDataState(String), + #[error("external block source required")] + ExternalBlockSourceRequired, + #[error("external block framing error: {0}")] + ExternalBlockFraming(String), #[error("Block hash not found: {0:?}")] BlockHashNotFound([u8; 32]), #[error("Block size mismatch: expected {expected}, got {actual}")] diff --git a/crates/pithos_lib/src/io/pithosreader.rs b/crates/pithos_lib/src/io/pithosreader.rs index bde3ff4..56deaa4 100644 --- a/crates/pithos_lib/src/io/pithosreader.rs +++ b/crates/pithos_lib/src/io/pithosreader.rs @@ -19,6 +19,10 @@ use std::ops::Range; use std::path::{Path, PathBuf}; use x25519_dalek::{PublicKey, StaticSecret}; +pub trait ExternalBlockSource { + fn open(&self, url: &str, max_response_size: u64) -> Result, PithosError>; +} + #[derive(Clone, Copy, Debug)] pub struct ReaderLimits { pub max_directory_bytes: u64, @@ -46,6 +50,7 @@ pub struct PithosReaderSimple { /// User's private key private_key: StaticSecret, limits: ReaderLimits, + external_block_source: Option>, } impl PithosReaderSimple { @@ -197,6 +202,7 @@ impl PithosReaderSimple { file, private_key, limits: ReaderLimits::default(), + external_block_source: None, }) } @@ -213,6 +219,7 @@ impl PithosReaderSimple { file, private_key, limits: ReaderLimits::default(), + external_block_source: None, }) } @@ -221,6 +228,57 @@ impl PithosReaderSimple { self } + pub fn with_external_block_source(mut self, source: Box) -> Self { + self.external_block_source = Some(source); + self + } + + fn load_stored_block(&mut self, meta: &BlockIndexEntry) -> Result, PithosError> { + let stored_size = Self::checked_stored_size(meta, &self.limits)?; + let mut block_data = Self::zeroed_buffer(stored_size, "stored block")?; + let mut block_header = [0u8; 4]; + + match &meta.location { + BlockLocation::Local => { + self.validate_local_block_range(meta)?; + self.file.seek(SeekFrom::Start(meta.offset))?; + self.file.read_exact(&mut block_header)?; + BlockHeader::deserialize(&mut block_header.as_slice())?; + self.file.read_exact(&mut block_data)?; + } + BlockLocation::External { url } => { + let max_response_size = meta.stored_size.checked_add(5).ok_or_else(|| { + PithosError::ExternalBlockFraming("response size overflow".into()) + })?; + let source = self + .external_block_source + .as_ref() + .ok_or(PithosError::ExternalBlockSourceRequired)?; + let mut response = source.open(url, max_response_size)?; + response.read_exact(&mut block_header).map_err(|error| { + PithosError::ExternalBlockFraming(format!("short block marker: {error}")) + })?; + BlockHeader::deserialize(&mut block_header.as_slice()).map_err(|error| { + PithosError::ExternalBlockFraming(format!("invalid block marker: {error}")) + })?; + response.read_exact(&mut block_data).map_err(|error| { + PithosError::ExternalBlockFraming(format!("short block payload: {error}")) + })?; + let mut extra = [0u8; 1]; + if response.read(&mut extra).map_err(|error| { + PithosError::ExternalBlockFraming(format!("reading block boundary: {error}")) + })? != 0 + { + return Err(PithosError::ExternalBlockFraming( + "response exceeds expected size".into(), + )); + } + } + } + + Ok(block_data) + } + /// Init a simple Pithos reader #[tracing::instrument(level = "trace", skip(_pithos_path, _private_key))] pub fn new_with_keys>( @@ -541,19 +599,7 @@ impl PithosReaderSimple { .get(hash) .ok_or(PithosError::BlockHashNotFound(*hash))?; - // Jump to begin of block in file - let stored_size = Self::checked_stored_size(block_meta, &self.limits)?; - self.validate_local_block_range(block_meta)?; - self.file.seek(SeekFrom::Start(block_meta.offset))?; - - // Read block header for block start validation - let mut block_header = [0u8; 4]; - self.file.read_exact(&mut block_header)?; - BlockHeader::deserialize(&mut block_header.as_slice())?; - - // Read block data - let mut block_buf = Self::zeroed_buffer(stored_size, "stored block")?; - self.file.read_exact(&mut block_buf)?; + let mut block_buf = self.load_stored_block(block_meta)?; block_buf = Self::decode_and_verify_block( block_buf, @@ -635,28 +681,7 @@ impl PithosReaderSimple { .get(hash) .ok_or(PithosError::BlockHashNotFound(*hash))?; - let mut block_header = [0u8; 4]; - let stored_size = Self::checked_stored_size(block_meta, &self.limits)?; - let mut block_data = Self::zeroed_buffer(stored_size, "stored block")?; - match &block_meta.location { - BlockLocation::Local => { - self.validate_local_block_range(block_meta)?; - self.file.seek(SeekFrom::Start(block_meta.offset))?; - // Read block header for block start validation - self.file.read_exact(&mut block_header)?; - BlockHeader::deserialize(&mut block_header.as_slice())?; - // Read block data - self.file.read_exact(&mut block_data)?; - } - BlockLocation::External { url } => { - let mut response = reqwest::blocking::get(url).unwrap(); - // Read block header for block start validation - response.read_exact(&mut block_header)?; - BlockHeader::deserialize(&mut block_header.as_slice())?; - // Read block data - self.file.read_exact(&mut block_data)?; - } - } + let mut block_data = self.load_stored_block(block_meta)?; block_data = Self::decode_and_verify_block( block_data, @@ -697,7 +722,7 @@ impl PithosReaderSimple { .get(hash) .ok_or(PithosError::BlockHashNotFound(*hash))?; - let stored_size = Self::checked_stored_size(block_meta, &self.limits)?; + Self::checked_stored_size(block_meta, &self.limits)?; let block_start = block_byte_sum; let block_end = block_byte_sum .checked_add(block_meta.original_size) @@ -716,16 +741,7 @@ impl PithosReaderSimple { break; } - // Read block header for block start validation - self.validate_local_block_range(block_meta)?; - self.file.seek(SeekFrom::Start(block_meta.offset))?; - let mut block_header = [0u8; 4]; - self.file.read_exact(&mut block_header)?; - BlockHeader::deserialize(&mut block_header.as_slice())?; - - // Read block data - let mut block_buf = Self::zeroed_buffer(stored_size, "stored block")?; - self.file.read_exact(&mut block_buf)?; + let mut block_buf = self.load_stored_block(block_meta)?; block_buf = Self::decode_and_verify_block( block_buf, diff --git a/crates/pithos_lib/tests/reader.rs b/crates/pithos_lib/tests/reader.rs index 0542f11..1c208c9 100644 --- a/crates/pithos_lib/tests/reader.rs +++ b/crates/pithos_lib/tests/reader.rs @@ -9,15 +9,16 @@ use pithos_lib::helpers::crypt4gh::{ use pithos_lib::helpers::file_entry_map::Key; use pithos_lib::helpers::ro_crate::{RoCrateSource, read_ro_crate_directory, read_ro_crate_zip}; use pithos_lib::helpers::x25519_keys::{private_key_from_pem_bytes, public_key_from_pem_bytes}; -use pithos_lib::io::pithosreader::{PithosReaderSimple, ReaderLimits}; +use pithos_lib::io::pithosreader::{ExternalBlockSource, PithosReaderSimple, ReaderLimits}; use pithos_lib::io::pithoswriter::{Content, InputFile, PithosWriter}; -use pithos_lib::model::structs::{BlockDataState, FileEntry, FileType, Reference}; +use pithos_lib::model::structs::{BlockDataState, BlockLocation, FileEntry, FileType, Reference}; use rocraters::ro_crate::graph_vector::GraphVector; use rocraters::ro_crate::read::CrateReadError; use rocraters::ro_crate::schema::RoCrateSchemaVersion; use std::fs::{File, OpenOptions, read, read_to_string, write}; -use std::io::{Read, Seek, SeekFrom, Write}; +use std::io::{Cursor, Read, Seek, SeekFrom, Write}; use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex}; use tempfile::TempDir; use x25519_dalek::StaticSecret; @@ -149,6 +150,223 @@ fn mutate_block_payload( write(archive, bytes).unwrap(); } +fn external_directory( + mut directory: pithos_lib::model::structs::Directory, + hash: [u8; 32], +) -> pithos_lib::model::structs::Directory { + let block = directory.blocks.get_mut(&hash).unwrap(); + block.location = BlockLocation::External { + url: "https://invalid.invalid/external-block".into(), + }; + block.offset = u64::MAX; + directory +} + +struct RecordingExternalSource { + response: Vec, + requested_url: Arc>>, + requested_max: Arc>>, +} + +struct SharedWriter(Arc>>); + +impl Write for SharedWriter { + fn write(&mut self, bytes: &[u8]) -> std::io::Result { + self.0.lock().unwrap().extend_from_slice(bytes); + Ok(bytes.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } +} + +impl ExternalBlockSource for RecordingExternalSource { + fn open(&self, url: &str, max_response_size: u64) -> Result, PithosError> { + *self.requested_url.lock().unwrap() = Some(url.to_owned()); + *self.requested_max.lock().unwrap() = Some(max_response_size); + Ok(Box::new(Cursor::new(self.response.clone()))) + } +} + +fn external_block_bytes( + archive: &Path, + directory: &pithos_lib::model::structs::Directory, + hash: [u8; 32], +) -> Vec { + let block = directory.blocks.get(&hash).unwrap(); + let bytes = read(archive).unwrap(); + let start = block.offset as usize; + let end = start + 4 + block.stored_size as usize; + bytes[start..end].to_vec() +} + +#[test] +fn test_external_blocks_use_injected_source_for_all_paths() { + let temp_dir = TempDir::new().unwrap(); + let (archive, key, directory, hash) = write_identity_integrity_archive(&temp_dir); + let valid_response = external_block_bytes(&archive, &directory, hash); + let directory = external_directory(directory, hash); + let requested_url = Arc::new(Mutex::new(None)); + let requested_max = Arc::new(Mutex::new(None)); + let source = RecordingExternalSource { + response: valid_response.clone(), + requested_url: requested_url.clone(), + requested_max: requested_max.clone(), + }; + let mut reader = PithosReaderSimple::new_with_key(&archive, key.clone()) + .unwrap() + .with_external_block_source(Box::new(source)); + let output = temp_dir.path().join("external-full-success.txt"); + reader + .read_file("integrity.txt", &directory, Some(&output), None) + .unwrap(); + assert_eq!(read(&output).unwrap(), b"deterministic integrity payload"); + assert_eq!( + requested_url.lock().unwrap().as_deref(), + Some("https://invalid.invalid/external-block") + ); + assert_eq!( + *requested_max.lock().unwrap(), + Some(4 + directory.blocks.get(&hash).unwrap().stored_size + 1) + ); + + let source = RecordingExternalSource { + response: valid_response.clone(), + requested_url: Arc::new(Mutex::new(None)), + requested_max: Arc::new(Mutex::new(None)), + }; + let mut reader = PithosReaderSimple::new_with_key(&archive, key.clone()) + .unwrap() + .with_external_block_source(Box::new(source)); + let range_bytes = Arc::new(Mutex::new(Vec::new())); + let mut range_output: Box = Box::new(SharedWriter(range_bytes.clone())); + let file_entry = directory.get_file_by_path("integrity.txt").unwrap(); + reader + .read_data_range_to_sink(2..10, file_entry, &directory.blocks, &mut range_output) + .unwrap(); + assert_eq!(*range_bytes.lock().unwrap(), b"terminis"); + + let recipient = public_key_from_pem_bytes( + read_to_string("tests/data/keys/recipient2_public.pem") + .unwrap() + .as_bytes(), + ) + .unwrap(); + let source = RecordingExternalSource { + response: valid_response, + requested_url: Arc::new(Mutex::new(None)), + requested_max: Arc::new(Mutex::new(None)), + }; + let mut reader = PithosReaderSimple::new_with_key(&archive, key) + .unwrap() + .with_external_block_source(Box::new(source)); + let crypt4gh_output = Arc::new(Mutex::new(Vec::new())); + reader + .read_file_to_crypt4gh( + "integrity.txt", + &directory, + vec![recipient], + Some(Box::new(SharedWriter(crypt4gh_output.clone()))), + ) + .unwrap(); + assert!(!crypt4gh_output.lock().unwrap().is_empty()); +} + +#[test] +fn test_external_blocks_reject_invalid_framing_without_plaintext() { + let cases = [ + ( + b"BLCK".to_vec(), + "external block framing error: short block payload", + ), + ( + [b"BLCK".as_slice(), b"deterministic integrity payload", b"x"].concat(), + "external block framing error: response exceeds expected size", + ), + ( + [b"NOPE".as_slice(), b"deterministic integrity payload"].concat(), + "external block framing error: invalid block marker", + ), + ]; + for (response, expected_error) in cases { + let temp_dir = TempDir::new().unwrap(); + let (archive, key, directory, hash) = write_identity_integrity_archive(&temp_dir); + let directory = external_directory(directory, hash); + let source = RecordingExternalSource { + response, + requested_url: Arc::new(Mutex::new(None)), + requested_max: Arc::new(Mutex::new(None)), + }; + let output = temp_dir.path().join("invalid-external.txt"); + let mut reader = PithosReaderSimple::new_with_key(&archive, key) + .unwrap() + .with_external_block_source(Box::new(source)); + let result = reader.read_file("integrity.txt", &directory, Some(&output), None); + assert!(result.unwrap_err().to_string().contains(expected_error)); + assert!(!output.exists()); + } +} + +#[test] +fn test_external_blocks_fail_closed_without_source() { + let temp_dir = TempDir::new().unwrap(); + let (archive, key, directory, hash) = write_identity_integrity_archive(&temp_dir); + let directory = external_directory(directory, hash); + + let full_output = temp_dir.path().join("external-full.txt"); + let mut reader = PithosReaderSimple::new_with_key(&archive, key.clone()).unwrap(); + let full_result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + reader.read_file("integrity.txt", &directory, Some(&full_output), None) + })) + .map_err(|panic| { + let message = panic + .downcast_ref::() + .cloned() + .or_else(|| panic.downcast_ref::<&str>().map(ToString::to_string)) + .unwrap_or_else(|| "unknown panic".into()); + PithosError::Other(message) + }) + .and_then(|result| result); + assert_eq!( + full_result.unwrap_err().to_string(), + "external block source required" + ); + assert!(!full_output.exists()); + + let range_output = temp_dir.path().join("external-range.txt"); + let mut reader = PithosReaderSimple::new_with_key(&archive, key.clone()).unwrap(); + let range_result = reader.read_file( + "integrity.txt", + &directory, + Some(&range_output), + Some(vec![0..1]), + ); + assert_eq!( + range_result.unwrap_err().to_string(), + "external block source required" + ); + assert!(!range_output.exists()); + + let recipient = public_key_from_pem_bytes( + read_to_string("tests/data/keys/recipient2_public.pem") + .unwrap() + .as_bytes(), + ) + .unwrap(); + let mut reader = PithosReaderSimple::new_with_key(&archive, key).unwrap(); + let crypt4gh_result = reader.read_file_to_crypt4gh( + "integrity.txt", + &directory, + vec![recipient], + Some(Box::new(Vec::::new())), + ); + assert_eq!( + crypt4gh_result.unwrap_err().to_string(), + "external block source required" + ); +} + #[test] fn test_block_integrity_valid_identity_read() { let temp_dir = TempDir::new().unwrap(); From 5c6f86c1273166b8b5bb9342f31290cc7c89aa90 Mon Sep 17 00:00:00 2001 From: Jannis Schlegel Date: Mon, 20 Jul 2026 16:31:35 +0200 Subject: [PATCH 08/14] fix: validate file header version and add tests for unsupported/invalid headers --- crates/pithos_lib/src/error.rs | 2 + crates/pithos_lib/src/io/pithosreader.rs | 21 ++++++--- crates/pithos_lib/src/model/structs.rs | 6 ++- crates/pithos_lib/tests/reader.rs | 54 ++++++++++++++++++++++-- 4 files changed, 73 insertions(+), 10 deletions(-) diff --git a/crates/pithos_lib/src/error.rs b/crates/pithos_lib/src/error.rs index c927908..ed83b51 100644 --- a/crates/pithos_lib/src/error.rs +++ b/crates/pithos_lib/src/error.rs @@ -30,6 +30,8 @@ pub enum PithosError { Serialization(#[from] SerializationError), #[error("Deserialization error: {0:?}")] Deserialization(#[from] DeserializationError), + #[error("Unsupported file version: supported {supported:#06x}, actual {actual:#06x}")] + UnsupportedFileVersion { supported: u16, actual: u16 }, #[error("Invalid directory marker: expected {expected:?}, got {actual:?}")] InvalidDirectoryMarker { expected: [u8; 8], actual: [u8; 8] }, #[error("Directory length mismatch: expected {expected}, got {actual}")] diff --git a/crates/pithos_lib/src/io/pithosreader.rs b/crates/pithos_lib/src/io/pithosreader.rs index 56deaa4..a699339 100644 --- a/crates/pithos_lib/src/io/pithosreader.rs +++ b/crates/pithos_lib/src/io/pithosreader.rs @@ -8,7 +8,8 @@ use crate::helpers::zstd::decompress_data; use crate::io::extraction::ExtractionRoot; use crate::model::deserialization::DeserializationLimits; use crate::model::structs::{ - BlockDataState, BlockHeader, BlockIndexEntry, BlockLocation, Directory, FileEntry, FileType, + BlockDataState, BlockHeader, BlockIndexEntry, BlockLocation, Directory, FileEntry, FileHeader, + FileType, }; use crc32fast::hash; use indexmap::IndexMap; @@ -54,6 +55,18 @@ pub struct PithosReaderSimple { } impl PithosReaderSimple { + fn open_archive>(pithos_path: P) -> Result { + let mut file = File::open(pithos_path)?; + let header = FileHeader::deserialize(&mut file)?; + if header.version != FileHeader::SUPPORTED_VERSION { + return Err(PithosError::UnsupportedFileVersion { + supported: FileHeader::SUPPORTED_VERSION, + actual: header.version, + }); + } + Ok(file) + } + fn decode_and_verify_block( stored_bytes: Vec, key: &[u8; 32], @@ -191,8 +204,7 @@ impl PithosReaderSimple { pithos_path: P, private_key_pem_path: P, ) -> Result { - // Open the Pithos file - let file = File::open(&pithos_path)?; + let file = Self::open_archive(&pithos_path)?; // Read and parse the PEM-encoded private key let pem_content = std::fs::read_to_string(private_key_pem_path)?; @@ -212,8 +224,7 @@ impl PithosReaderSimple { pithos_path: P, private_key: StaticSecret, ) -> Result { - // Open the Pithos file - let file = File::open(&pithos_path)?; + let file = Self::open_archive(&pithos_path)?; Ok(Self { file, diff --git a/crates/pithos_lib/src/model/structs.rs b/crates/pithos_lib/src/model/structs.rs index 2365f16..3dd09a0 100644 --- a/crates/pithos_lib/src/model/structs.rs +++ b/crates/pithos_lib/src/model/structs.rs @@ -25,11 +25,15 @@ impl Default for FileHeader { fn default() -> Self { FileHeader { magic: *b"PITH", - version: 0x0100, + version: Self::SUPPORTED_VERSION, } } } +impl FileHeader { + pub const SUPPORTED_VERSION: u16 = 0x0100; +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct BlockHeader { pub marker: [u8; 4], // MUST be b"BLCK" diff --git a/crates/pithos_lib/tests/reader.rs b/crates/pithos_lib/tests/reader.rs index 1c208c9..b2c524e 100644 --- a/crates/pithos_lib/tests/reader.rs +++ b/crates/pithos_lib/tests/reader.rs @@ -11,7 +11,10 @@ use pithos_lib::helpers::ro_crate::{RoCrateSource, read_ro_crate_directory, read use pithos_lib::helpers::x25519_keys::{private_key_from_pem_bytes, public_key_from_pem_bytes}; use pithos_lib::io::pithosreader::{ExternalBlockSource, PithosReaderSimple, ReaderLimits}; use pithos_lib::io::pithoswriter::{Content, InputFile, PithosWriter}; -use pithos_lib::model::structs::{BlockDataState, BlockLocation, FileEntry, FileType, Reference}; +use pithos_lib::model::deserialization::DeserializationError; +use pithos_lib::model::structs::{ + BlockDataState, BlockLocation, FileEntry, FileHeader, FileType, Reference, +}; use rocraters::ro_crate::graph_vector::GraphVector; use rocraters::ro_crate::read::CrateReadError; use rocraters::ro_crate::schema::RoCrateSchemaVersion; @@ -59,6 +62,47 @@ fn caller_directory( (reader, directory) } +#[test] +fn test_reader_rejects_invalid_header_magic_during_construction() { + let temp_dir = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp_dir, false, false); + let mut bytes = read(&archive).unwrap(); + bytes[0] = b'X'; + write(&archive, bytes).unwrap(); + + let error = match PithosReaderSimple::new_with_key(&archive, reader_key()) { + Ok(_) => panic!("reader construction must reject an invalid header magic"), + Err(error) => error, + }; + assert!(matches!( + error, + PithosError::Deserialization(DeserializationError::InvalidMarker(_)) + )); +} + +#[test] +fn test_reader_rejects_invalid_header_version_during_construction() { + let temp_dir = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp_dir, false, false); + let mut bytes = read(&archive).unwrap(); + bytes[4..6].copy_from_slice(&[0x80, 0x04]); + write(&archive, bytes).unwrap(); + + let error = match PithosReaderSimple::new_with_key(&archive, reader_key()) { + Ok(_) => panic!("reader construction must reject an unsupported header version"), + Err(error) => error, + }; + let message = error.to_string(); + assert!( + message.contains("0x0100"), + "missing supported version: {message}" + ); + assert!( + message.contains("0x0200"), + "missing actual version: {message}" + ); +} + fn write_zip_entry(path: &Path, name: &str, content: &[u8]) { let file = File::create(path).unwrap(); let mut archive = zip::ZipWriter::new(file); @@ -340,6 +384,7 @@ fn test_external_blocks_fail_closed_without_source() { "integrity.txt", &directory, Some(&range_output), + #[allow(clippy::single_range_in_vec_init)] Some(vec![0..1]), ); assert_eq!( @@ -591,12 +636,13 @@ fn test_robust_overlapping_parent_is_rejected_as_invalid_chain() { fn test_robust_parent_boundary_uses_immediate_child() { let temp_dir = TempDir::new().unwrap(); let middle = complete_test_directory(None); - let oldest = complete_test_directory(Some((27, middle.len() as u64))); - let terminal = complete_test_directory(Some((0, oldest.len() as u64))); + let oldest = complete_test_directory(Some((33, middle.len() as u64))); + let terminal = complete_test_directory(Some((6, oldest.len() as u64))); assert_eq!(oldest.len(), 27); assert_eq!(middle.len(), 25); let archive = temp_dir.path().join("three-directories.pithos"); - let mut bytes = oldest; + let mut bytes = FileHeader::default().serialize_to_bytes().unwrap(); + bytes.extend_from_slice(&oldest); bytes.extend_from_slice(&middle); bytes.extend_from_slice(&terminal); write(&archive, bytes).unwrap(); From 2bfbfa2978a3a056699dc7b4b04945a97dc1592f Mon Sep 17 00:00:00 2001 From: Jannis Schlegel Date: Tue, 21 Jul 2026 11:21:59 +0200 Subject: [PATCH 09/14] fix: correct ancestor block reuse and conflict resolution --- crates/pithos_lib/src/error.rs | 8 ++ crates/pithos_lib/src/helpers/directory.rs | 15 ++- crates/pithos_lib/src/io/pithoswriter.rs | 6 + crates/pithos_lib/tests/writer.rs | 145 ++++++++++++++++++++- 4 files changed, 172 insertions(+), 2 deletions(-) diff --git a/crates/pithos_lib/src/error.rs b/crates/pithos_lib/src/error.rs index ed83b51..d844f33 100644 --- a/crates/pithos_lib/src/error.rs +++ b/crates/pithos_lib/src/error.rs @@ -98,6 +98,14 @@ pub enum PithosError { expected: [u8; 32], actual: [u8; 32], }, + #[error( + "Block index conflict for hash {hash:?}: existing original size {existing_original_size}, new original size {new_original_size}" + )] + BlockIndexConflict { + hash: [u8; 32], + existing_original_size: u64, + new_original_size: u64, + }, #[error("File not found: {0}")] FileNotFound(String), #[error("File already exists: {0}")] diff --git a/crates/pithos_lib/src/helpers/directory.rs b/crates/pithos_lib/src/helpers/directory.rs index e5528f7..3d99c5e 100644 --- a/crates/pithos_lib/src/helpers/directory.rs +++ b/crates/pithos_lib/src/helpers/directory.rs @@ -193,7 +193,20 @@ impl Directory { block_hash: [u8; 32], block_entry: BlockIndexEntry, ) -> Result<(), PithosError> { - self.blocks.insert(block_hash, block_entry); + match self.blocks.entry(block_hash) { + Entry::Occupied(existing) => { + if existing.get().original_size != block_entry.original_size { + return Err(PithosError::BlockIndexConflict { + hash: block_hash, + existing_original_size: existing.get().original_size, + new_original_size: block_entry.original_size, + }); + } + } + Entry::Vacant(entry) => { + entry.insert(block_entry); + } + } Ok(()) } diff --git a/crates/pithos_lib/src/io/pithoswriter.rs b/crates/pithos_lib/src/io/pithoswriter.rs index 740b67a..4d01179 100644 --- a/crates/pithos_lib/src/io/pithoswriter.rs +++ b/crates/pithos_lib/src/io/pithoswriter.rs @@ -170,6 +170,7 @@ pub struct PithosWriter { // Processing directory: Directory, // Single or merged from multiple + ancestor_blocks: IndexMap<[u8; 32], BlockIndexEntry>, written_bytes: u64, } @@ -194,6 +195,7 @@ impl PithosWriter { directory: DirectoryBuilder::new() .encryption(encryption_sections) .build()?, + ancestor_blocks: IndexMap::new(), written_bytes: 0, }) } @@ -249,6 +251,7 @@ impl PithosWriter { EncryptionSection::new(&reader_keys), )])) .build()?, + ancestor_blocks: directory.blocks.clone(), written_bytes, writer_key, cdc, @@ -290,6 +293,9 @@ impl PithosWriter { // Return already existing block entry return Ok((entry, hashes, true)); } + if let Some(entry) = self.ancestor_blocks.get(hashes.blake3.as_bytes()) { + return Ok((entry.clone(), hashes, true)); + } // Init BlockIndexEntry let mut block_index_entry = BlockIndexEntry { diff --git a/crates/pithos_lib/tests/writer.rs b/crates/pithos_lib/tests/writer.rs index f9fe71b..051d02a 100644 --- a/crates/pithos_lib/tests/writer.rs +++ b/crates/pithos_lib/tests/writer.rs @@ -4,13 +4,17 @@ use crate::common::util::{ create_pithos_writer, extract_pithos_entry, load_test_keys, minimal_ro_crate_metadata, read_pithos_directory, write_zip_entries, }; +use indexmap::IndexMap; use pithos_lib::error::PithosError; +use pithos_lib::helpers::directory::DirectoryBuilder; use pithos_lib::helpers::file_entry_map::KeyQuery; use pithos_lib::helpers::ro_crate::{LoadedRoCrate, read_ro_crate_directory, read_ro_crate_zip}; use pithos_lib::helpers::x25519_keys::private_key_from_pem_bytes; use pithos_lib::io::pithosreader::{PithosReaderSimple, ReaderLimits}; use pithos_lib::io::pithoswriter::{Content, InputFile, PithosWriter}; -use pithos_lib::model::structs::{FileType, Reference}; +use pithos_lib::model::structs::{ + BlockIndexEntry, BlockLocation, FileType, ProcessingFlags, Reference, +}; use std::fs::{ File, copy, create_dir_all, read, read_dir, read_link, read_to_string, remove_file, write, }; @@ -377,6 +381,145 @@ fn test_append_single_file() { assert_eq!(entry.file_size, 342848); } +#[test] +fn format_006_append_reuses_ancestor_block_without_repointing() { + let temp_dir = TempDir::new().unwrap(); + let payload = "format-006 ancestor block reuse\n".repeat(64).into_bytes(); + let (path, reader_key, mut writer) = create_pithos_writer(&temp_dir, None); + writer.write_file_header().unwrap(); + writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "base.bin".into(), + data: Content::Raw(String::from_utf8(payload.clone()).unwrap()), + metadata: None, + encrypt: false, + compression_level: Some(0), + }) + .unwrap(); + writer.write_directory().unwrap(); + drop(writer); + + let base_bytes = read(&path).unwrap(); + let (base_directory, _) = read_pithos_directory(&path, &reader_key).unwrap(); + assert_eq!(base_directory.blocks.len(), 1); + let (hash, original_block) = base_directory.blocks.first().unwrap(); + let hash = *hash; + let original_block = original_block.clone(); + + let (writer_key, reader_public_key, _) = load_test_keys(); + let mut writer = + PithosWriter::new_from_file(writer_key, vec![reader_public_key], None, &path).unwrap(); + writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "child.bin".into(), + data: Content::Raw(String::from_utf8(payload.clone()).unwrap()), + metadata: None, + encrypt: false, + compression_level: Some(0), + }) + .unwrap(); + + assert_eq!( + std::fs::metadata(&path).unwrap().len(), + base_bytes.len() as u64 + ); + assert!(writer.get_directory_mut().blocks.is_empty()); + writer.write_directory().unwrap(); + drop(writer); + + let final_bytes = read(&path).unwrap(); + assert!(final_bytes.starts_with(&base_bytes)); + let (directory, _) = read_pithos_directory(&path, &reader_key).unwrap(); + assert_eq!(directory.blocks.len(), 1); + assert_eq!(directory.blocks.get(&hash), Some(&original_block)); + assert!(directory.get_file_by_path("base.bin").is_some()); + assert!(directory.get_file_by_path("child.bin").is_some()); + assert_eq!( + extract_pithos_entry(&path, &reader_key, "base.bin", &temp_dir), + payload + ); + remove_file(temp_dir.path().join("extracted-entry")).unwrap(); + assert_eq!( + extract_pithos_entry(&path, &reader_key, "child.bin", &temp_dir), + payload + ); +} + +#[test] +fn format_006_merge_preserves_older_compatible_block_record() { + let hash = [7; 32]; + let older = BlockIndexEntry { + offset: 10, + stored_size: 20, + original_size: 30, + flags: ProcessingFlags::new(false, Some(0)), + location: BlockLocation::Local, + }; + let newer = BlockIndexEntry { + offset: 40, + stored_size: 50, + original_size: 30, + flags: ProcessingFlags::new(true, Some(7)), + location: BlockLocation::External { + url: "https://example.invalid/block".into(), + }, + }; + let mut older_directory = DirectoryBuilder::new() + .blocks(IndexMap::from_iter([(hash, older.clone())])) + .build() + .unwrap(); + let newer_directory = DirectoryBuilder::new() + .blocks(IndexMap::from_iter([(hash, newer)])) + .build() + .unwrap(); + + older_directory.merge(newer_directory).unwrap(); + + assert_eq!(older_directory.blocks.get(&hash), Some(&older)); +} + +#[test] +fn format_006_merge_rejects_conflicting_original_size() { + let hash = [9; 32]; + let mut older_directory = DirectoryBuilder::new() + .blocks(IndexMap::from_iter([( + hash, + BlockIndexEntry { + offset: 10, + stored_size: 20, + original_size: 30, + flags: ProcessingFlags::new(false, Some(0)), + location: BlockLocation::Local, + }, + )])) + .build() + .unwrap(); + let newer_directory = DirectoryBuilder::new() + .blocks(IndexMap::from_iter([( + hash, + BlockIndexEntry { + offset: 40, + stored_size: 50, + original_size: 31, + flags: ProcessingFlags::new(true, Some(7)), + location: BlockLocation::Local, + }, + )])) + .build() + .unwrap(); + + assert!(matches!( + older_directory.merge(newer_directory), + Err(PithosError::BlockIndexConflict { + hash: actual_hash, + existing_original_size: 30, + new_original_size: 31, + }) if actual_hash == hash + )); +} + #[test] fn new_from_file_with_limits_accepts_exact_terminal_directory_length() { let temp_dir = TempDir::new().unwrap(); From c126795f0175b22cd4a2ab65dadbffe25cb5bcff Mon Sep 17 00:00:00 2001 From: Jannis Schlegel Date: Tue, 21 Jul 2026 14:13:38 +0200 Subject: [PATCH 10/14] feat: improve error handling for unsupported content, file ID exhaustion, and input validation --- crates/pithos_lib/src/error.rs | 6 ++ crates/pithos_lib/src/helpers/directory.rs | 4 +- .../pithos_lib/src/helpers/file_entry_map.rs | 10 ++-- crates/pithos_lib/src/io/pithosreader.rs | 2 +- crates/pithos_lib/src/io/pithoswriter.rs | 10 ++-- crates/pithos_lib/src/model/structs.rs | 2 +- crates/pithos_lib/tests/robustness.rs | 58 +++++++++++++++++++ 7 files changed, 80 insertions(+), 12 deletions(-) create mode 100644 crates/pithos_lib/tests/robustness.rs diff --git a/crates/pithos_lib/src/error.rs b/crates/pithos_lib/src/error.rs index d844f33..786cfaf 100644 --- a/crates/pithos_lib/src/error.rs +++ b/crates/pithos_lib/src/error.rs @@ -32,6 +32,10 @@ pub enum PithosError { Deserialization(#[from] DeserializationError), #[error("Unsupported file version: supported {supported:#06x}, actual {actual:#06x}")] UnsupportedFileVersion { supported: u16, actual: u16 }, + #[error("Multiple reader keys are not supported")] + UnsupportedMultipleReaderKeys, + #[error("Reference content is not supported for file entry construction")] + UnsupportedReferenceContent, #[error("Invalid directory marker: expected {expected:?}, got {actual:?}")] InvalidDirectoryMarker { expected: [u8; 8], actual: [u8; 8] }, #[error("Directory length mismatch: expected {expected}, got {actual}")] @@ -110,6 +114,8 @@ pub enum PithosError { FileNotFound(String), #[error("File already exists: {0}")] DuplicateFileId(String), + #[error("File ID allocation is exhausted")] + FileIdExhausted, #[error("Relation id already occupied: {0}")] RelationIdOccupied(u64), #[error("Path already occupied: {0}")] diff --git a/crates/pithos_lib/src/helpers/directory.rs b/crates/pithos_lib/src/helpers/directory.rs index 3d99c5e..84e43eb 100644 --- a/crates/pithos_lib/src/helpers/directory.rs +++ b/crates/pithos_lib/src/helpers/directory.rs @@ -214,7 +214,7 @@ impl Directory { pub fn add_file(&mut self, path: &str, file_entry: &FileEntry) -> Result<(), PithosError> { let key = Key::new( self.files - .next_free_id(self.parent_directory_offset.is_some()), + .next_free_id(self.parent_directory_offset.is_some())?, path.to_owned(), ); self.files.insert(key, file_entry.clone()) @@ -326,7 +326,7 @@ impl Directory { } #[tracing::instrument(level = "trace", skip(self))] - pub fn next_free_file_index(&self) -> u64 { + pub fn next_free_file_index(&self) -> Result { self.files .next_free_id(self.parent_directory_offset.is_some()) /* diff --git a/crates/pithos_lib/src/helpers/file_entry_map.rs b/crates/pithos_lib/src/helpers/file_entry_map.rs index 29563f7..5f93a60 100644 --- a/crates/pithos_lib/src/helpers/file_entry_map.rs +++ b/crates/pithos_lib/src/helpers/file_entry_map.rs @@ -271,13 +271,15 @@ impl FileEntryMap { self.current_max_id } - pub fn next_free_id(&self, has_parent: bool) -> u64 { + pub fn next_free_id(&self, has_parent: bool) -> Result { if self.current_max_id == 0 && self.values.is_empty() && has_parent { - 1 + Ok(1) } else if self.current_max_id == 0 && self.values.is_empty() { - 0 + Ok(0) } else { - self.current_max_id + 1 + self.current_max_id + .checked_add(1) + .ok_or(PithosError::FileIdExhausted) } } } diff --git a/crates/pithos_lib/src/io/pithosreader.rs b/crates/pithos_lib/src/io/pithosreader.rs index a699339..0a315aa 100644 --- a/crates/pithos_lib/src/io/pithosreader.rs +++ b/crates/pithos_lib/src/io/pithosreader.rs @@ -296,7 +296,7 @@ impl PithosReaderSimple { _pithos_path: P, _private_key: Vec, ) -> Result { - unimplemented!("Multiple reader keys"); + Err(PithosError::UnsupportedMultipleReaderKeys) } #[tracing::instrument(level = "trace", skip(self))] diff --git a/crates/pithos_lib/src/io/pithoswriter.rs b/crates/pithos_lib/src/io/pithoswriter.rs index 4d01179..233e21a 100644 --- a/crates/pithos_lib/src/io/pithoswriter.rs +++ b/crates/pithos_lib/src/io/pithoswriter.rs @@ -62,7 +62,9 @@ impl TryFrom<&PathBuf> for InputFile { .to_string(); let inner_path = match &file_type { FileType::Directory => file_path_str.as_str(), - _ => extract_filename(&file_path_str).expect("Input file is missing file name."), + _ => extract_filename(&file_path_str).ok_or(PithosError::Conversion( + "Input file is missing file name.".to_string(), + ))?, }; Ok(InputFile { @@ -356,7 +358,7 @@ impl PithosWriter { validate_candidate(&self.directory.files, entry_path, file_entry)?; // Directory or Symlink FileEntry are just added to Pithos directory let file_entry_key = Key::new( - self.directory.next_free_file_index(), + self.directory.next_free_file_index()?, entry_path.to_string(), ); @@ -407,7 +409,7 @@ impl PithosWriter { validate_candidate(&self.directory.files, &input.inner_path, &data_check)?; let mut preflight = self.directory.files.clone(); preflight.insert( - Key::new(preflight.next_free_id(false), input.inner_path.clone()), + Key::new(preflight.next_free_id(false)?, input.inner_path.clone()), data_check.clone(), )?; if let Some(metadata) = &input.metadata @@ -501,7 +503,7 @@ impl PithosWriter { .get_file_encryption_key(reference.target_file_id) { let file_entry_key = - Key::new(self.directory.next_free_file_index(), &input.inner_path); + Key::new(self.directory.next_free_file_index()?, &input.inner_path); self.directory.add_file(&input.inner_path, &data_file)?; self.directory .add_file_to_all_recipients((file_entry_key.id(), enc_key)); diff --git a/crates/pithos_lib/src/model/structs.rs b/crates/pithos_lib/src/model/structs.rs index 3dd09a0..1ce8f1e 100644 --- a/crates/pithos_lib/src/model/structs.rs +++ b/crates/pithos_lib/src/model/structs.rs @@ -397,7 +397,7 @@ impl FileEntry { } } Content::Reference(_) => { - unimplemented!("Currently FileEntry cannot be created from Content::Reference") + return Err(PithosError::UnsupportedReferenceContent); } }) } diff --git a/crates/pithos_lib/tests/robustness.rs b/crates/pithos_lib/tests/robustness.rs new file mode 100644 index 0000000..5649ad6 --- /dev/null +++ b/crates/pithos_lib/tests/robustness.rs @@ -0,0 +1,58 @@ +use pithos_lib::error::PithosError; +use pithos_lib::helpers::directory::DirectoryBuilder; +use pithos_lib::helpers::file_entry_map::FileEntryMap; +use pithos_lib::io::pithosreader::PithosReaderSimple; +use pithos_lib::io::pithoswriter::Content; +use pithos_lib::model::structs::{BlockDataState, FileEntry, FileType, Reference}; +use std::error::Error; +use x25519_dalek::StaticSecret; + +#[test] +fn new_with_keys_returns_error_instead_of_panicking() { + let result = PithosReaderSimple::new_with_keys( + "/nonexistent-pithos-robustness-test", + Vec::::new(), + ); + + assert!(matches!( + result, + Err(PithosError::UnsupportedMultipleReaderKeys) + )); +} + +#[test] +fn reference_content_returns_error_instead_of_panicking() { + let reference = Reference { + target_file_id: 0, + relationship: 0, + }; + + let result = FileEntry::new_from_content(FileType::Data, &Content::Reference(reference)); + + assert!(matches!( + result, + Err(PithosError::UnsupportedReferenceContent) + )); +} + +#[test] +fn exhausted_file_ids_return_error_instead_of_panicking() -> Result<(), Box> { + let mut directory = DirectoryBuilder::new() + .files(FileEntryMap::new_with_max(u64::MAX)) + .build()?; + let file_entry = FileEntry { + file_type: FileType::Data, + block_data: BlockDataState::Decrypted(Vec::new()), + created: 0, + modified: 0, + file_size: 0, + permissions: 0o644, + references: Vec::new(), + symlink_target: None, + }; + + let result = directory.add_file("exhausted", &file_entry); + + assert!(matches!(result, Err(PithosError::FileIdExhausted))); + Ok(()) +} From 92646f38297e6f8c3b48cadf6c5080ebc9771594 Mon Sep 17 00:00:00 2001 From: Jannis Schlegel Date: Tue, 21 Jul 2026 14:41:16 +0200 Subject: [PATCH 11/14] chore: remove deprecated license exception --- deny.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/deny.toml b/deny.toml index a99eca2..d03b311 100644 --- a/deny.toml +++ b/deny.toml @@ -104,7 +104,6 @@ confidence-threshold = 0.8 # Allow 1 or more licenses on a per-crate basis, so that particular licenses # aren't accepted for every possible crate as with the normal allow list exceptions = [ - { allow = ["CDLA-Permissive-2.0"], crate = "webpki-root-certs" }, { allow = ["MPL-2.0"], crate = "option-ext" }, { allow = ["Zlib"], crate = "zlib-rs" }, { allow = ["Apache-2.0 WITH LLVM-exception"], crate = "winx" } From b2c5e16977e198fc072426271f0ddce46efb2a94 Mon Sep 17 00:00:00 2001 From: Jannis Schlegel Date: Tue, 21 Jul 2026 14:41:43 +0200 Subject: [PATCH 12/14] chore: Update version to 0.7.3 --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- crates/pithos/Cargo.toml | 4 ++-- crates/pithos_lib/Cargo.toml | 2 +- crates/pithos_pyo3/Cargo.toml | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index df65ccd..a5d4cc0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1079,7 +1079,7 @@ checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" [[package]] name = "pithos" -version = "0.7.2" +version = "0.7.3" dependencies = [ "clap", "pithos_lib", @@ -1091,7 +1091,7 @@ dependencies = [ [[package]] name = "pithos_lib" -version = "0.7.2" +version = "0.7.3" dependencies = [ "blake3", "byteorder", diff --git a/Cargo.toml b/Cargo.toml index 87d77ad..a2e664e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ members = [ resolver = "2" [workspace.package] -version = "0.7.2" +version = "0.7.3" authors = ["Jannis Schlegel ", "Sebastian Beyvers "] edition = "2024" rust-version = "1.88" diff --git a/crates/pithos/Cargo.toml b/crates/pithos/Cargo.toml index aa32f48..5349aef 100644 --- a/crates/pithos/Cargo.toml +++ b/crates/pithos/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "pithos" description = "CLI for the Pithos container file format" -version = "0.7.2" +version = "0.7.3" authors.workspace = true edition.workspace = true rust-version.workspace = true @@ -10,7 +10,7 @@ license.workspace = true [dependencies] clap = { version = "4.6.1", features = ["derive"] } -pithos_lib = { version = "0.7.2", path = "../pithos_lib" } +pithos_lib = { version = "0.7.3", path = "../pithos_lib" } thiserror = { workspace = true } tracing.workspace = true tracing-subscriber.workspace = true diff --git a/crates/pithos_lib/Cargo.toml b/crates/pithos_lib/Cargo.toml index f1e44e5..cd0f4ec 100644 --- a/crates/pithos_lib/Cargo.toml +++ b/crates/pithos_lib/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "pithos_lib" description = "Library and components for encrypting / compressing pithos (.pith) files, including specification" -version = "0.7.2" +version = "0.7.3" authors.workspace = true edition.workspace = true rust-version.workspace = true diff --git a/crates/pithos_pyo3/Cargo.toml b/crates/pithos_pyo3/Cargo.toml index a1d6a22..fcfd9f7 100644 --- a/crates/pithos_pyo3/Cargo.toml +++ b/crates/pithos_pyo3/Cargo.toml @@ -10,4 +10,4 @@ repository.workspace = true license.workspace = true [dependencies] -pithos_lib = { version = "0.7.2", path = "../pithos_lib" } +pithos_lib = { version = "0.7.3", path = "../pithos_lib" } From a06e2806fa2012df67ab7d403cd33d7f5872cac4 Mon Sep 17 00:00:00 2001 From: Jannis Schlegel Date: Tue, 21 Jul 2026 15:48:19 +0200 Subject: [PATCH 13/14] chore: update licenses and badges in README files --- LICENSE-APACHE | 2 +- LICENSE-MIT | 2 +- README.md | 5 +++-- crates/pithos/README.md | 3 ++- crates/pithos_lib/README.md | 4 ++-- crates/pithos_pyo3/README.md | 20 ++++---------------- 6 files changed, 13 insertions(+), 23 deletions(-) diff --git a/LICENSE-APACHE b/LICENSE-APACHE index 211727c..5372d84 100644 --- a/LICENSE-APACHE +++ b/LICENSE-APACHE @@ -186,7 +186,7 @@ same "printed page" as the copyright notice for easier identification within third-party archives. - Copyright [2024] [ArunaStorage Team] + Copyright [2026] [ArunaEngine Team] Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/LICENSE-MIT b/LICENSE-MIT index 894d702..deb2b2d 100644 --- a/LICENSE-MIT +++ b/LICENSE-MIT @@ -1,6 +1,6 @@ The MIT License (MIT) -Copyright (c) 2024 ArunaStorage Team +Copyright (c) 2024 ArunaEngine Team Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/README.md b/README.md index ff5fac9..ec2c681 100644 --- a/README.md +++ b/README.md @@ -12,8 +12,9 @@

Language: Rust - License: MIT - License: MIT + License: MIT + License: Apache 2.0 + Codecov

🔑 A secure, fast and versatile package file format for object storage focused data management 📦

diff --git a/crates/pithos/README.md b/crates/pithos/README.md index 9744fb5..6db7f87 100644 --- a/crates/pithos/README.md +++ b/crates/pithos/README.md @@ -1,7 +1,8 @@ # Pithos CLI [![Rust](https://img.shields.io/badge/built_with-Rust-dca282.svg)](https://www.rust-lang.org/) -[![License](https://img.shields.io/badge/License-MIT-brightgreen.svg)](https://github.com/ArunaStorage/aruna-file/blob/main/LICENSE) +[![License](https://img.shields.io/badge/License-MIT-brightgreen.svg)](https://github.com/arunaengine/aruna-file/blob/main/LICENSE-MIT) +[![License](https://img.shields.io/badge/License-APACHE-brightgreen.svg)](https://github.com/arunaengine/aruna-file/blob/main/LICENSE-APACHE) CLI application for handling Pithos (.pto) files. diff --git a/crates/pithos_lib/README.md b/crates/pithos_lib/README.md index a8201fe..5c397a1 100644 --- a/crates/pithos_lib/README.md +++ b/crates/pithos_lib/README.md @@ -1,6 +1,6 @@ [![Rust](https://img.shields.io/badge/built_with-Rust-dca282.svg)](https://www.rust-lang.org/) -[![License](https://img.shields.io/badge/License-MIT-brightgreen.svg)](https://github.com/ArunaStorage/aruna-file/blob/main/LICENSE) -![CI](https://github.com/ArunaStorage/aruna-file/actions/workflows/push.yaml/badge.svg) +[![License](https://img.shields.io/badge/License-MIT-brightgreen.svg)](https://github.com/arunaengine/aruna-file/blob/main/LICENSE-MIT) +[![License](https://img.shields.io/badge/License-APACHE-brightgreen.svg)](https://github.com/arunaengine/aruna-file/blob/main/LICENSE-APACHE) [![Codecov](https://codecov.io/github/ArunaStorage/aruna-file/coverage.svg?branch=main)](https://codecov.io/gh/ArunaStorage/aruna-file) [![Dependency status](https://deps.rs/repo/github/ArunaStorage/aruna-file/status.svg)](https://deps.rs/repo/github/ArunaStorage/aruna-file) ___ diff --git a/crates/pithos_pyo3/README.md b/crates/pithos_pyo3/README.md index 9744fb5..9a3ee6b 100644 --- a/crates/pithos_pyo3/README.md +++ b/crates/pithos_pyo3/README.md @@ -1,19 +1,7 @@ -# Pithos CLI +# Pithos Python bindings [![Rust](https://img.shields.io/badge/built_with-Rust-dca282.svg)](https://www.rust-lang.org/) -[![License](https://img.shields.io/badge/License-MIT-brightgreen.svg)](https://github.com/ArunaStorage/aruna-file/blob/main/LICENSE) - - -CLI application for handling Pithos (.pto) files. - - -## Installation - -A release is published via [crates.io](crates.io) (you need to have `cargo` and the `rust-toolchain` installed see [rustup.rs](rustup.rs) for more info) - -```command -cargo install pithos -``` - -### Usage +[![License](https://img.shields.io/badge/License-MIT-brightgreen.svg)](https://github.com/arunaengine/aruna-file/blob/main/LICENSE-MIT) +[![License](https://img.shields.io/badge/License-APACHE-brightgreen.svg)](https://github.com/arunaengine/aruna-file/blob/main/LICENSE-APACHE) +`pithos_pyo3` is the planned Python interface for Pithos. This crate is a work in progress and will be implemented once `pithos_lib` has reached a stable state. From 6f8bd2651ef8e2c6094d0de8a9e7eb31f1425140 Mon Sep 17 00:00:00 2001 From: Jannis Schlegel Date: Wed, 22 Jul 2026 10:08:20 +0200 Subject: [PATCH 14/14] fix: optimize archive path validation and directory integrity --- crates/pithos_lib/src/helpers/archive_path.rs | 166 +++++++++--- crates/pithos_lib/src/helpers/directory.rs | 14 +- .../pithos_lib/src/helpers/file_entry_map.rs | 222 ++++++++++++++-- crates/pithos_lib/src/io/pithosreader.rs | 14 +- crates/pithos_lib/src/io/pithoswriter.rs | 35 +-- .../pithos_lib/src/model/deserialization.rs | 4 +- crates/pithos_lib/tests/marshalling.rs | 105 ++++++++ crates/pithos_lib/tests/reader.rs | 66 +++++ crates/pithos_lib/tests/writer.rs | 238 +++++++++++++++++- 9 files changed, 778 insertions(+), 86 deletions(-) diff --git a/crates/pithos_lib/src/helpers/archive_path.rs b/crates/pithos_lib/src/helpers/archive_path.rs index c0ebf16..b838ac4 100644 --- a/crates/pithos_lib/src/helpers/archive_path.rs +++ b/crates/pithos_lib/src/helpers/archive_path.rs @@ -120,56 +120,90 @@ pub(crate) fn validate_entry(path: &str, entry: &FileEntry) -> Result<(), Pithos } } -pub(crate) fn validate_candidate( +fn validate_candidate_hierarchy( map: &FileEntryMap, path: &str, entry: &FileEntry, ) -> Result<(), PithosError> { - validate_entry(path, entry)?; - for (_, existing, existing_entry) in map { - if path == existing { - continue; - } - if path.starts_with(existing) - && path.as_bytes().get(existing.len()) == Some(&b'/') - && existing_entry.file_type != FileType::Directory - { - return Err(PithosError::InvalidArchivePath { - path: path.into(), - reason: format!("file entry {existing} is an ancestor"), - }); - } - if existing.starts_with(path) - && existing.as_bytes().get(path.len()) == Some(&b'/') - && entry.file_type != FileType::Directory + for (index, _) in path.match_indices('/') { + let ancestor = &path[..index]; + if map + .get_by_path(ancestor) + .is_some_and(|existing| existing.file_type != FileType::Directory) { return Err(PithosError::InvalidArchivePath { path: path.into(), - reason: format!("entry is an ancestor of {existing}"), + reason: format!("file entry {ancestor} is an ancestor"), }); } } + + if entry.file_type != FileType::Directory + && let Some(successor) = map.first_path_after(path) + && successor.starts_with(path) + && successor.as_bytes().get(path.len()) == Some(&b'/') + { + return Err(PithosError::InvalidArchivePath { + path: path.into(), + reason: format!("entry is an ancestor of {successor}"), + }); + } + Ok(()) } +pub(crate) fn validate_existing_candidate( + map: &FileEntryMap, + path: &str, + entry: &FileEntry, +) -> Result<(), PithosError> { + validate_entry(path, entry)?; + validate_candidate_hierarchy(map, path, entry) +} + +pub(crate) fn validate_new_candidate( + map: &FileEntryMap, + path: &str, + entry: &FileEntry, +) -> Result<(), PithosError> { + validate_entry(path, entry)?; + if map.get_by_path(path).is_some() { + return Err(PithosError::PathOccupied(format!( + "File path already occupied: {path}" + ))); + } + validate_candidate_hierarchy(map, path, entry) +} + pub(crate) fn validate_map(map: &FileEntryMap) -> Result<(), PithosError> { for (_, path, entry) in map { - validate_candidate(&FileEntryMap::new(), path, entry)?; + validate_entry(path, entry)?; } - for (_, path, entry) in map { - for (_, other, _) in map { - if path != other - && other.starts_with(path) - && other.as_bytes().get(path.len()) == Some(&b'/') - && entry.file_type != FileType::Directory - { - return Err(PithosError::InvalidArchivePath { - path: path.into(), - reason: format!("entry is an ancestor of {other}"), - }); - } + + validate_hierarchy(map) +} + +pub(crate) fn validate_hierarchy(map: &FileEntryMap) -> Result<(), PithosError> { + let mut entries = map.iter_ordered(); + let Some((mut previous_path, mut previous_entry)) = entries.next() else { + return Ok(()); + }; + + for (path, entry) in entries { + if previous_entry.file_type != FileType::Directory + && path.starts_with(previous_path) + && path.as_bytes().get(previous_path.len()) == Some(&b'/') + { + return Err(PithosError::InvalidArchivePath { + path: previous_path.into(), + reason: format!("entry is an ancestor of {path}"), + }); } + + previous_path = path; + previous_entry = entry; } + Ok(()) } @@ -339,4 +373,72 @@ mod tests { assert!(validate_map(&map).is_ok()); } } + + #[test] + fn archive_path_candidate_validation_handles_component_boundaries_and_depth() { + let file = entry(FileType::Data, None, BlockDataState::Decrypted(vec![])); + let directory = entry(FileType::Directory, None, BlockDataState::Decrypted(vec![])); + + let mut map = FileEntryMap::new(); + map.insert(Key::new(0, "a"), file.clone()).unwrap(); + assert!(validate_new_candidate(&map, "a/child", &file).is_err()); + + let mut map = FileEntryMap::new(); + map.insert(Key::new(0, "a/child"), file.clone()).unwrap(); + assert!(validate_new_candidate(&map, "a", &file).is_err()); + + let mut map = FileEntryMap::new(); + map.insert(Key::new(0, "a"), directory.clone()).unwrap(); + assert!(validate_new_candidate(&map, "a/child", &file).is_ok()); + + let mut map = FileEntryMap::new(); + for (id, path) in [(0, "ab"), (1, "a!"), (2, "a.b"), (3, "a/child")] { + map.insert(Key::new(id, path), file.clone()).unwrap(); + } + assert!(validate_new_candidate(&map, "a", &file).is_err()); + assert!(validate_new_candidate(&map, "a!x", &file).is_ok()); + assert!(validate_new_candidate(&map, "a.bx", &file).is_ok()); + assert!(validate_new_candidate(&map, "abx", &file).is_ok()); + + let mut map = FileEntryMap::new(); + map.insert(Key::new(0, "ユニコード"), file.clone()).unwrap(); + assert!(validate_new_candidate(&map, "ユニコード/子", &file).is_err()); + + let deep = (0..64) + .map(|part| format!("part{part}")) + .collect::>(); + let ancestor = deep.join("/"); + let descendant = format!("{ancestor}/leaf"); + let mut map = FileEntryMap::new(); + map.insert(Key::new(0, descendant), file.clone()).unwrap(); + assert!(validate_new_candidate(&map, &ancestor, &file).is_err()); + } + + #[test] + fn archive_path_existing_and_new_candidate_exact_path_semantics() { + let file = entry(FileType::Data, None, BlockDataState::Decrypted(vec![])); + let mut map = FileEntryMap::new(); + map.insert(Key::new(0, "occupied"), file.clone()).unwrap(); + + assert!(validate_existing_candidate(&map, "occupied", &file).is_ok()); + assert!(matches!( + validate_new_candidate(&map, "occupied", &file), + Err(PithosError::PathOccupied(message)) if message == "File path already occupied: occupied" + )); + } + + #[test] + fn archive_path_component_order_keeps_descendants_adjacent() { + let file = entry(FileType::Data, None, BlockDataState::Decrypted(vec![])); + let mut map = FileEntryMap::new(); + for (id, path) in [(0, "a"), (1, "a!"), (2, "a/child")] { + map.insert(Key::new(id, path), file.clone()).unwrap(); + } + + assert_eq!( + map.iter_ordered().map(|(path, _)| path).collect::>(), + ["a", "a/child", "a!"] + ); + assert!(validate_map(&map).is_err()); + } } diff --git a/crates/pithos_lib/src/helpers/directory.rs b/crates/pithos_lib/src/helpers/directory.rs index 84e43eb..a4cfd98 100644 --- a/crates/pithos_lib/src/helpers/directory.rs +++ b/crates/pithos_lib/src/helpers/directory.rs @@ -18,7 +18,6 @@ pub struct DirectoryBuilder { blocks: IndexMap<[u8; 32], BlockIndexEntry>, relations: Vec<(u64, String)>, encryption: IndexMap<[u8; 32], EncryptionSection>, - dir_len: u64, } impl Default for DirectoryBuilder { @@ -37,7 +36,6 @@ impl DirectoryBuilder { blocks: IndexMap::new(), relations: Self::default_relations(), encryption: IndexMap::new(), - dir_len: 25, } } @@ -92,12 +90,6 @@ impl DirectoryBuilder { self } - #[tracing::instrument(level = "trace", skip(self, dir_len))] - pub fn dir_len(mut self, dir_len: u64) -> Self { - self.dir_len = dir_len; - self - } - #[tracing::instrument(level = "trace", skip(self))] pub fn build(self) -> Result { let mut directory = Directory { @@ -107,9 +99,10 @@ impl DirectoryBuilder { blocks: self.blocks, relations: self.relations, encryption: self.encryption, - dir_len: self.dir_len, + dir_len: 0, crc32: 0, }; + directory.update_len()?; directory.update_crc32()?; Ok(directory) } @@ -453,7 +446,8 @@ impl Directory { pub fn update_len(&mut self) -> Result<(), SerializationError> { let mut buf = Vec::new(); self.serialize(&mut buf)?; - self.dir_len = buf.len() as u64; + self.dir_len = u64::try_from(buf.len()) + .map_err(|_| SerializationError::Other("length does not fit in u64".to_string()))?; Ok(()) } diff --git a/crates/pithos_lib/src/helpers/file_entry_map.rs b/crates/pithos_lib/src/helpers/file_entry_map.rs index 5f93a60..4bc0531 100644 --- a/crates/pithos_lib/src/helpers/file_entry_map.rs +++ b/crates/pithos_lib/src/helpers/file_entry_map.rs @@ -1,21 +1,25 @@ use crate::error::PithosError; use crate::model::structs::FileEntry; use indexmap::IndexMap; -use indexmap::map::{Entry, Keys}; +use indexmap::map::Entry; +use std::cmp::Ordering; +use std::collections::BTreeMap; use std::fmt::{Debug, Display, Formatter}; +use std::ops::Bound::{Excluded, Unbounded}; +use std::sync::Arc; /// Values used as keys in a Map, e.g. `HashMap`. #[derive(Clone, Debug, Eq, Hash, PartialEq)] pub struct Key { id: u64, - path: String, + path: Arc, } impl Key { pub fn new(id: u64, path: impl Into) -> Key { Key { id, - path: path.into(), + path: Arc::from(path.into()), } } @@ -36,7 +40,23 @@ impl Key { } pub fn path_query(&self) -> KeyQuery { - KeyQuery::Path(self.path.clone()) + KeyQuery::Path(self.path.to_string()) + } +} + +/// An archive path ordered by slash-delimited components rather than raw bytes. +#[derive(Clone, Debug, Eq, PartialEq)] +struct ArchivePathKey(Arc); + +impl Ord for ArchivePathKey { + fn cmp(&self, other: &Self) -> Ordering { + self.0.split('/').cmp(other.0.split('/')) + } +} + +impl PartialOrd for ArchivePathKey { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) } } @@ -61,7 +81,8 @@ impl Display for KeyQuery { #[derive(Clone, Debug, Eq, PartialEq)] pub struct FileEntryMap { id_map: IndexMap, - path_map: IndexMap, + path_map: IndexMap, usize>, + ordered_path_map: BTreeMap, values: Vec, current_max_id: u64, } @@ -71,6 +92,7 @@ impl FileEntryMap { Self { id_map: IndexMap::new(), path_map: IndexMap::new(), + ordered_path_map: BTreeMap::new(), values: Vec::new(), current_max_id: 0, } @@ -80,6 +102,7 @@ impl FileEntryMap { Self { id_map: IndexMap::new(), path_map: IndexMap::new(), + ordered_path_map: BTreeMap::new(), values: Vec::new(), current_max_id: max_id, } @@ -89,8 +112,8 @@ impl FileEntryMap { &self.id_map } - pub fn get_paths_ref(&'_ self) -> Keys<'_, String, usize> { - self.path_map.keys() + pub fn get_paths_ref(&self) -> impl Iterator { + self.path_map.keys().map(|path| path.as_ref()) } pub fn get_id_by_path(&self, path: &str) -> Option { @@ -105,6 +128,35 @@ impl FileEntryMap { } } + pub(crate) fn get_by_path(&self, path: &str) -> Option<&FileEntry> { + self.path_map + .get(path) + .and_then(|idx| self.values.get(*idx)) + } + + pub(crate) fn first_path_after(&self, path: &str) -> Option<&str> { + let query = self + .path_map + .get_key_value(path) + .map(|(stored, _)| ArchivePathKey(Arc::clone(stored))) + .unwrap_or_else(|| ArchivePathKey(Arc::from(path))); + self.ordered_path_map + .range((Excluded(query), Unbounded)) + .next() + .map(|(path, _)| path.0.as_ref()) + } + + pub(crate) fn iter_ordered(&self) -> impl Iterator { + self.ordered_path_map.iter().map(|(path, idx)| { + ( + path.0.as_ref(), + self.values + .get(*idx) + .expect("ordered path index must reference an entry"), + ) + }) + } + pub fn get_path_by_id(&self, id: &u64) -> Option<&str> { if let Some(idx) = self.id_map.get(id) { if let Some((path, _)) = self.path_map.get_index(*idx) { @@ -207,7 +259,9 @@ impl FileEntryMap { ))), Entry::Vacant(vacant_id_entry) => { // Also check if path is occupied - let vacant_path_entry = match self.path_map.entry(key.path) { + let path = key.path; + let ordered_path = ArchivePathKey(path.clone()); + let vacant_path_entry = match self.path_map.entry(path) { Entry::Occupied(entry) => { return Err(PithosError::PathOccupied(format!( "File path already occupied: {}", @@ -222,6 +276,7 @@ impl FileEntryMap { let idx = self.values.len() - 1; vacant_id_entry.insert(idx); vacant_path_entry.insert(idx); + self.ordered_path_map.insert(ordered_path, idx); // Set current max if key.id > self.current_max_id { @@ -236,7 +291,7 @@ impl FileEntryMap { pub fn get(&self, kq: &KeyQuery) -> Option<&FileEntry> { let idx = match kq { KeyQuery::Id(file_id) => self.id_map.get(file_id), - KeyQuery::Path(file_path) => self.path_map.get(file_path), + KeyQuery::Path(file_path) => self.path_map.get(file_path.as_str()), }; match idx { @@ -311,7 +366,7 @@ impl<'a> Iterator for Iter<'a> { self.pos += 1; - Some((*id, path.as_str(), value)) + Some((*id, path.as_ref(), value)) } fn size_hint(&self) -> (usize, Option) { @@ -325,7 +380,7 @@ impl<'a> ExactSizeIterator for Iter<'a> {} /// Iterator that yields mutable references to (id, path, FileEntry) tuples pub struct IterMut<'a> { id_map: &'a IndexMap, - path_map: &'a IndexMap, + path_map: &'a IndexMap, usize>, values: std::slice::IterMut<'a, FileEntry>, pos: usize, } @@ -342,7 +397,7 @@ impl<'a> Iterator for IterMut<'a> { self.pos += 1; - Some((*id, path.as_str(), value)) + Some((*id, path.as_ref(), value)) } fn size_hint(&self) -> (usize, Option) { @@ -355,7 +410,7 @@ impl<'a> ExactSizeIterator for IterMut<'a> {} /// Iterator that yields owned (id, path, FileEntry) tuples pub struct IntoIter { id_map: IndexMap, - path_map: IndexMap, + path_map: IndexMap, usize>, values: Vec, pos: usize, } @@ -374,7 +429,7 @@ impl Iterator for IntoIter { self.pos += 1; - Some((*id, path.clone(), value)) + Some((*id, path.to_string(), value)) } fn size_hint(&self) -> (usize, Option) { @@ -409,3 +464,142 @@ impl<'a> IntoIterator for &'a FileEntryMap { self.iter() } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::model::structs::{BlockDataState, Directory, FileType}; + use indexmap::IndexMap; + + fn entry() -> FileEntry { + FileEntry { + file_type: FileType::Data, + block_data: BlockDataState::Decrypted(vec![]), + created: 0, + modified: 0, + file_size: 0, + permissions: 0o644, + references: vec![], + symlink_target: None, + } + } + + fn assert_indexes_consistent(map: &FileEntryMap) { + assert_eq!(map.id_map.len(), map.values.len()); + assert_eq!(map.path_map.len(), map.values.len()); + assert_eq!(map.ordered_path_map.len(), map.values.len()); + for (index, ((id, id_index), (path, path_index))) in + map.id_map.iter().zip(map.path_map.iter()).enumerate() + { + assert_eq!(*id, map.get_id_by_path(path).unwrap()); + assert_eq!(index, *id_index); + assert_eq!(index, *path_index); + } + for ordered_index in map.ordered_path_map.values() { + assert!(map.values.get(*ordered_index).is_some()); + } + } + + #[test] + fn file_entry_map_indexes_survive_clone_extend_and_retain() { + let mut map = FileEntryMap::new(); + for (id, path) in [(4, "a!"), (2, "a/child"), (7, "a")] { + map.insert(Key::new(id, path), entry()).unwrap(); + } + assert_indexes_consistent(&map); + + let clone = map.clone(); + assert_indexes_consistent(&clone); + + let mut extended = FileEntryMap::new(); + extended.extend(clone).unwrap(); + assert_indexes_consistent(&extended); + extended.retain_mut(|id, _, _| id != 2).unwrap(); + assert_indexes_consistent(&extended); + assert_eq!( + extended + .iter() + .map(|(id, path, _)| (id, path)) + .collect::>(), + [(4, "a!"), (7, "a")] + ); + } + + #[test] + fn file_entry_map_duplicate_errors_do_not_mutate_indexes() { + let mut map = FileEntryMap::new(); + map.insert(Key::new(1, "first"), entry()).unwrap(); + let expected = map.clone(); + + assert!(matches!( + map.insert(Key::new(1, "second"), entry()), + Err(PithosError::DuplicateFileId(_)) + )); + assert_eq!(map, expected); + assert!(matches!( + map.insert(Key::new(2, "first"), entry()), + Err(PithosError::PathOccupied(_)) + )); + assert_eq!(map, expected); + assert_indexes_consistent(&map); + } + + #[test] + fn file_entry_map_preserves_insertion_order_and_shares_path_storage() { + let mut map = FileEntryMap::new(); + for (id, path) in [(3, "third"), (1, "first"), (2, "second")] { + map.insert(Key::new(id, path), entry()).unwrap(); + } + + assert_eq!( + map.iter() + .map(|(id, path, _)| (id, path)) + .collect::>(), + [(3, "third"), (1, "first"), (2, "second")] + ); + let directory = Directory { + identifier: *b"PITHOSDR", + parent_directory_offset: None, + files: map.clone(), + blocks: IndexMap::new(), + relations: vec![], + encryption: IndexMap::new(), + dir_len: 0, + crc32: 0, + }; + let mut serialized = Vec::new(); + directory.serialize(&mut serialized).unwrap(); + let path_position = |path: &[u8]| { + serialized + .windows(path.len()) + .position(|window| window == path) + .unwrap() + }; + assert!( + path_position(b"\x05third") < path_position(b"\x05first") + && path_position(b"\x05first") < path_position(b"\x06second") + ); + + assert_eq!( + map.clone() + .into_iter() + .map(|(id, path, _)| (id, path)) + .collect::>(), + [ + (3, "third".to_string()), + (1, "first".to_string()), + (2, "second".to_string()) + ] + ); + for path in map.path_map.keys() { + let ordered = map + .ordered_path_map + .keys() + .find(|ordered| ordered.0 == *path) + .unwrap(); + assert!(Arc::ptr_eq(path, &ordered.0)); + assert!(map.get_by_path(path).is_some()); + assert!(map.get_id_by_path(path).is_some()); + } + } +} diff --git a/crates/pithos_lib/src/io/pithosreader.rs b/crates/pithos_lib/src/io/pithosreader.rs index 0a315aa..058687c 100644 --- a/crates/pithos_lib/src/io/pithosreader.rs +++ b/crates/pithos_lib/src/io/pithosreader.rs @@ -1,8 +1,7 @@ use crate::error::PithosError; -use crate::helpers::archive_path::{validate_entry, validate_map}; +use crate::helpers::archive_path::{validate_existing_candidate, validate_map}; use crate::helpers::chacha_poly1305::{decrypt_chunk, encrypt_chunk}; use crate::helpers::crypt4gh::{CRYPT4GH_BLOCK_SIZE, Crypt4GHHeader, HeaderPacket}; -use crate::helpers::file_entry_map::KeyQuery; use crate::helpers::x25519_keys::private_key_from_pem_bytes; use crate::helpers::zstd::decompress_data; use crate::io::extraction::ExtractionRoot; @@ -462,18 +461,11 @@ impl PithosReaderSimple { output_path: Option<&PathBuf>, ranges: Option>>, ) -> Result<(), PithosError> { - validate_map(&directory.files)?; - validate_entry( - inner_path, - directory - .files - .get(&KeyQuery::Path(inner_path.to_string())) - .ok_or(PithosError::FileNotFound(inner_path.to_string()))?, - )?; let file_entry = directory .files - .get(&KeyQuery::Path(inner_path.to_string())) + .get_by_path(inner_path) .ok_or(PithosError::FileNotFound(inner_path.to_string()))?; + validate_existing_candidate(&directory.files, inner_path, file_entry)?; match &file_entry.file_type { FileType::Data | FileType::Metadata => { diff --git a/crates/pithos_lib/src/io/pithoswriter.rs b/crates/pithos_lib/src/io/pithoswriter.rs index 233e21a..2e9ac4a 100644 --- a/crates/pithos_lib/src/io/pithoswriter.rs +++ b/crates/pithos_lib/src/io/pithoswriter.rs @@ -1,5 +1,5 @@ use crate::error::PithosError; -use crate::helpers::archive_path::{validate_candidate, validate_map}; +use crate::helpers::archive_path::{validate_map, validate_new_candidate}; use crate::helpers::directory::DirectoryBuilder; use crate::helpers::file_entry_map::{FileEntryMap, Key}; use crate::helpers::hash::{Hasher, Hashes}; @@ -355,7 +355,17 @@ impl PithosWriter { processing_flags: &ProcessingFlags, content: R, ) -> Result { - validate_candidate(&self.directory.files, entry_path, file_entry)?; + validate_new_candidate(&self.directory.files, entry_path, file_entry)?; + self.process_file_entry_prevalidated(entry_path, file_entry, processing_flags, content) + } + + fn process_file_entry_prevalidated( + &mut self, + entry_path: &str, + file_entry: &mut FileEntry, + processing_flags: &ProcessingFlags, + content: R, + ) -> Result { // Directory or Symlink FileEntry are just added to Pithos directory let file_entry_key = Key::new( self.directory.next_free_file_index()?, @@ -406,18 +416,13 @@ impl PithosWriter { #[tracing::instrument(level = "trace", skip(self, input))] pub fn process_input(&mut self, input: InputFile) -> Result { let data_check = FileEntry::new_from_content(input.file_type, &input.data)?; - validate_candidate(&self.directory.files, &input.inner_path, &data_check)?; - let mut preflight = self.directory.files.clone(); - preflight.insert( - Key::new(preflight.next_free_id(false)?, input.inner_path.clone()), - data_check.clone(), - )?; + validate_new_candidate(&self.directory.files, &input.inner_path, &data_check)?; if let Some(metadata) = &input.metadata && !matches!(metadata, Content::Reference(_)) { let metadata_check = FileEntry::new_from_content(FileType::Metadata, metadata)?; - validate_candidate( - &preflight, + validate_new_candidate( + &self.directory.files, &format!("{}.meta", input.inner_path), &metadata_check, )?; @@ -434,7 +439,7 @@ impl PithosWriter { Content::File(disk_path) => { let mut meta_file = FileEntry::new_from_content(FileType::Metadata, &metadata)?; let handle = File::open(disk_path)?; - self.process_file_entry( + self.process_file_entry_prevalidated( meta_file_path, &mut meta_file, &processing_flags, @@ -444,7 +449,7 @@ impl PithosWriter { Content::Raw(raw_content) => { let mut meta_file = FileEntry::new_from_content(FileType::Metadata, &metadata)?; let handle = Cursor::new(raw_content.clone().into_bytes()); - self.process_file_entry( + self.process_file_entry_prevalidated( meta_file_path, &mut meta_file, &processing_flags, @@ -463,14 +468,14 @@ impl PithosWriter { if [FileType::Data, FileType::Metadata].contains(&input.file_type) => { let handle = File::open(disk_path)?; - self.process_file_entry( + self.process_file_entry_prevalidated( &input.inner_path, &mut data_file, &processing_flags, handle, )? } - Content::File(_) => self.process_file_entry( + Content::File(_) => self.process_file_entry_prevalidated( &input.inner_path, &mut data_file, &processing_flags, @@ -478,7 +483,7 @@ impl PithosWriter { )?, Content::Raw(raw_content) => { let handle = Cursor::new(raw_content.into_bytes()); - self.process_file_entry( + self.process_file_entry_prevalidated( &input.inner_path, &mut data_file, &processing_flags, diff --git a/crates/pithos_lib/src/model/deserialization.rs b/crates/pithos_lib/src/model/deserialization.rs index 33add6b..a748167 100644 --- a/crates/pithos_lib/src/model/deserialization.rs +++ b/crates/pithos_lib/src/model/deserialization.rs @@ -7,7 +7,7 @@ // - Error handling via DeserializationError use crate::error::PithosError; -use crate::helpers::archive_path::{validate_entry, validate_map}; +use crate::helpers::archive_path::{validate_entry, validate_hierarchy}; use crate::helpers::file_entry_map::{FileEntryMap, Key}; use crate::model::structs::*; use byteorder::{BigEndian, ReadBytesExt}; @@ -262,7 +262,7 @@ impl Directory { validate_entry(&path, &entry)?; files.insert(Key::new(id, path), entry)?; } - validate_map(&files)?; + validate_hierarchy(&files)?; let blocks_len = bounded_len( reader.read_varint::()?, diff --git a/crates/pithos_lib/tests/marshalling.rs b/crates/pithos_lib/tests/marshalling.rs index 80292e0..bc9578c 100644 --- a/crates/pithos_lib/tests/marshalling.rs +++ b/crates/pithos_lib/tests/marshalling.rs @@ -297,6 +297,111 @@ fn directory_integrity_crc_matches_serialized_prefix() { ); } +fn assert_builder_directory_integrity(directory: &Directory) { + let mut serialized = Vec::new(); + directory.serialize(&mut serialized).unwrap(); + + assert_eq!(directory.dir_len, serialized.len() as u64); + assert_eq!( + u64::from_be_bytes( + serialized[serialized.len() - 12..serialized.len() - 4] + .try_into() + .unwrap() + ), + directory.dir_len + ); + assert_eq!( + directory.crc32, + crc32fast::hash(&serialized[..serialized.len() - 4]) + ); +} + +#[test] +fn directory_builder_sets_integrity_for_empty_directory() { + let directory = DirectoryBuilder::new() + .set_relations(vec![]) + .build() + .unwrap(); + + assert!(directory.files.is_empty()); + assert!(directory.relations.is_empty()); + assert_builder_directory_integrity(&directory); +} + +#[test] +fn directory_builder_sets_integrity_for_default_relations() { + let directory = DirectoryBuilder::new().build().unwrap(); + + assert_eq!(directory.relations.len(), 10); + assert_builder_directory_integrity(&directory); +} + +#[test] +fn directory_builder_sets_integrity_for_populated_directory() { + let mut files = FileEntryMap::new(); + files + .insert(Key::new(7, "file.txt"), plain_entry(None)) + .unwrap(); + let blocks = IndexMap::from_iter([( + [1u8; 32], + BlockIndexEntry { + offset: 4, + stored_size: 5, + original_size: 6, + flags: ProcessingFlags(0), + location: BlockLocation::Local, + }, + )]); + let directory = DirectoryBuilder::new() + .files(files) + .blocks(blocks) + .set_relations(vec![(42, "Related".into())]) + .build() + .unwrap(); + + assert_eq!(directory.files.len(), 1); + assert_eq!(directory.blocks.len(), 1); + assert_eq!(directory.relations, vec![(42, "Related".into())]); + assert_builder_directory_integrity(&directory); +} + +#[test] +fn directory_builder_sets_integrity_for_encrypted_directory() { + let sender_key = generate_private_key().unwrap(); + let recipient_key = generate_private_key().unwrap(); + let mut recipient_data = RecipientData::Decrypted(vec![(7, [3u8; 32])]); + recipient_data + .encrypt(&sender_key.diffie_hellman(&PublicKey::from(&recipient_key))) + .unwrap(); + let directory = DirectoryBuilder::new() + .encryption(IndexMap::from_iter([( + PublicKey::from(&sender_key).to_bytes(), + EncryptionSection { + recipients: IndexMap::from_iter([( + PublicKey::from(&recipient_key).to_bytes(), + RecipientSection { recipient_data }, + )]), + }, + )])) + .build() + .unwrap(); + + assert!(matches!( + directory + .encryption + .values() + .next() + .unwrap() + .recipients + .values() + .next() + .unwrap() + .recipient_data, + RecipientData::Encrypted(_) + )); + assert_builder_directory_integrity(&directory); +} + #[test] fn directory_integrity_minimal_vector_matches_specification() { let directory = Directory { diff --git a/crates/pithos_lib/tests/reader.rs b/crates/pithos_lib/tests/reader.rs index b2c524e..774eab2 100644 --- a/crates/pithos_lib/tests/reader.rs +++ b/crates/pithos_lib/tests/reader.rs @@ -6,6 +6,7 @@ use pithos_lib::helpers::chacha_poly1305::decrypt_chunk; use pithos_lib::helpers::crypt4gh::{ CRYPT4GH_ENCRYPTED_BLOCK_SIZE, Crypt4GHError, Crypt4GHHeader, Packet, PacketData, }; +use pithos_lib::helpers::directory::DirectoryBuilder; use pithos_lib::helpers::file_entry_map::Key; use pithos_lib::helpers::ro_crate::{RoCrateSource, read_ro_crate_directory, read_ro_crate_zip}; use pithos_lib::helpers::x25519_keys::{private_key_from_pem_bytes, public_key_from_pem_bytes}; @@ -515,6 +516,24 @@ fn test_directory_integrity_valid_terminal_archive() { assert!(reader.read_directory().is_ok()); } +#[test] +fn test_reader_reads_directory_built_without_manual_length_update() { + let temp_dir = TempDir::new().unwrap(); + let directory = DirectoryBuilder::new() + .set_relations(vec![]) + .build() + .unwrap(); + let archive = temp_dir.path().join("builder-directory.pith"); + let mut bytes = FileHeader::default().serialize_to_bytes().unwrap(); + directory.serialize(&mut bytes).unwrap(); + write(&archive, bytes).unwrap(); + + let mut reader = PithosReaderSimple::new_with_key(&archive, reader_key()).unwrap(); + let (read_directory, _) = reader.read_directory().unwrap(); + + assert_eq!(read_directory, directory); +} + #[test] fn test_directory_integrity_terminal_marker_mutation() { let temp_dir = TempDir::new().unwrap(); @@ -1095,6 +1114,53 @@ fn test_safe_extraction_rejects_caller_constructed_ancestor_conflict() { assert!(!output.join("a").exists()); } +#[test] +fn test_reader_targeted_validation_rejects_reverse_caller_ancestor_conflict() { + let temp = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp, false, false); + let output = temp.path().join("output"); + std::fs::create_dir(&output).unwrap(); + let (mut reader, mut directory) = caller_directory(&archive, "a/child", FileType::Data, None); + directory + .files + .insert(Key::new(9001, "a"), empty_entry(FileType::Data, None)) + .unwrap(); + assert!( + reader + .read_file("a/child", &directory, Some(&output), None) + .is_err() + ); + assert!(!output.join("a/child").exists()); +} + +#[test] +fn test_reader_reads_unrelated_entries_without_full_map_revalidation() { + let temp = TempDir::new().unwrap(); + let archive = write_dummy_pithos(&temp, false, false); + let output = temp.path().join("output"); + std::fs::create_dir(&output).unwrap(); + let (mut reader, mut directory) = caller_directory(&archive, "safe", FileType::Data, None); + directory + .files + .insert( + Key::new(9001, "conflict"), + empty_entry(FileType::Data, None), + ) + .unwrap(); + directory + .files + .insert( + Key::new(9002, "conflict/child"), + empty_entry(FileType::Data, None), + ) + .unwrap(); + + reader + .read_file("safe", &directory, Some(&output), None) + .unwrap(); + assert!(output.join("safe").exists()); +} + #[test] fn test_reader_hides_files_without_keys() { let temp_dir = TempDir::new().unwrap(); diff --git a/crates/pithos_lib/tests/writer.rs b/crates/pithos_lib/tests/writer.rs index 051d02a..d348f0a 100644 --- a/crates/pithos_lib/tests/writer.rs +++ b/crates/pithos_lib/tests/writer.rs @@ -13,12 +13,13 @@ use pithos_lib::helpers::x25519_keys::private_key_from_pem_bytes; use pithos_lib::io::pithosreader::{PithosReaderSimple, ReaderLimits}; use pithos_lib::io::pithoswriter::{Content, InputFile, PithosWriter}; use pithos_lib::model::structs::{ - BlockIndexEntry, BlockLocation, FileType, ProcessingFlags, Reference, + BlockIndexEntry, BlockLocation, FileEntry, FileType, ProcessingFlags, Reference, }; +use std::cell::Cell; use std::fs::{ File, copy, create_dir_all, read, read_dir, read_link, read_to_string, remove_file, write, }; -use std::io::Read; +use std::io::{Cursor, Read}; use std::path::{Path, PathBuf}; use tempfile::TempDir; use x25519_dalek::StaticSecret; @@ -56,6 +57,18 @@ fn metadata_reference() -> Reference { } } +struct CountingReader { + cursor: Cursor>, + reads: std::rc::Rc>, +} + +impl Read for CountingReader { + fn read(&mut self, buffer: &mut [u8]) -> std::io::Result { + self.reads.set(self.reads.get() + 1); + self.cursor.read(buffer) + } +} + fn read_zip_member(path: &Path, name: &str) -> Vec { let mut archive = zip::ZipArchive::new(File::open(path).unwrap()).unwrap(); let mut member = archive.by_name(name).unwrap(); @@ -1086,6 +1099,227 @@ fn test_writer_rejects_candidate_ancestor_before_metadata_output() { assert_eq!(std::fs::metadata(&path).unwrap().len(), before); } +#[test] +fn test_writer_duplicate_main_preflight_is_atomic() { + let temp_dir = TempDir::new().unwrap(); + let (path, _key, mut writer) = create_pithos_writer(&temp_dir, None); + writer.write_file_header().unwrap(); + writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "duplicate".into(), + data: Content::Raw("existing".into()), + metadata: None, + encrypt: false, + compression_level: Some(0), + }) + .unwrap(); + let before_bytes = read(&path).unwrap(); + let before_directory = writer.get_directory_mut().clone(); + + let error = writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "duplicate".into(), + data: Content::Raw("non-empty data".into()), + metadata: Some(Content::Raw("metadata".into())), + encrypt: false, + compression_level: Some(0), + }) + .unwrap_err(); + + assert!(matches!(error, PithosError::PathOccupied(_))); + assert_eq!(read(&path).unwrap(), before_bytes); + assert_eq!(*writer.get_directory_mut(), before_directory); +} + +#[test] +fn test_writer_generated_metadata_duplicate_preflight_is_atomic() { + let temp_dir = TempDir::new().unwrap(); + let (path, _key, mut writer) = create_pithos_writer(&temp_dir, None); + writer.write_file_header().unwrap(); + writer + .process_input(InputFile { + file_type: FileType::Metadata, + inner_path: "generated.meta".into(), + data: Content::Raw("existing metadata".into()), + metadata: None, + encrypt: false, + compression_level: Some(0), + }) + .unwrap(); + let before_bytes = read(&path).unwrap(); + let before_directory = writer.get_directory_mut().clone(); + + let error = writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "generated".into(), + data: Content::Raw("data".into()), + metadata: Some(Content::Raw("new metadata".into())), + encrypt: false, + compression_level: Some(0), + }) + .unwrap_err(); + + assert!(matches!(error, PithosError::PathOccupied(_))); + assert_eq!(read(&path).unwrap(), before_bytes); + assert_eq!(*writer.get_directory_mut(), before_directory); +} + +#[test] +fn test_writer_process_file_entry_duplicate_preflight_is_atomic() { + let temp_dir = TempDir::new().unwrap(); + let (path, _key, mut writer) = create_pithos_writer(&temp_dir, None); + writer.write_file_header().unwrap(); + writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "duplicate".into(), + data: Content::Raw("existing".into()), + metadata: None, + encrypt: false, + compression_level: Some(0), + }) + .unwrap(); + let before_bytes = read(&path).unwrap(); + let before_directory = writer.get_directory_mut().clone(); + let mut entry = + FileEntry::new_from_content(FileType::Data, &Content::Raw("data".into())).unwrap(); + let original_entry = entry.clone(); + let reads = std::rc::Rc::new(Cell::new(0)); + + let error = writer + .process_file_entry( + "duplicate", + &mut entry, + &ProcessingFlags::new(false, Some(0)), + CountingReader { + cursor: Cursor::new(b"unconsumed data".to_vec()), + reads: reads.clone(), + }, + ) + .unwrap_err(); + + assert!(matches!(error, PithosError::PathOccupied(_))); + assert_eq!(reads.get(), 0); + assert_eq!(entry, original_entry); + assert_eq!(read(&path).unwrap(), before_bytes); + assert_eq!(*writer.get_directory_mut(), before_directory); +} + +#[test] +fn test_writer_append_active_segment_duplicate_preflight_preserves_base_archive() { + let temp_dir = TempDir::new().unwrap(); + let (path, reader_key, mut writer) = create_pithos_writer(&temp_dir, None); + writer.write_file_header().unwrap(); + writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "base".into(), + data: Content::Raw("base data".into()), + metadata: None, + encrypt: false, + compression_level: Some(0), + }) + .unwrap(); + writer.write_directory().unwrap(); + drop(writer); + + let before_bytes = read(&path).unwrap(); + let (writer_key, reader_public_key, _) = load_test_keys(); + let mut writer = + PithosWriter::new_from_file(writer_key, vec![reader_public_key], None, &path).unwrap(); + writer + .process_input(InputFile { + file_type: FileType::Directory, + inner_path: "duplicate".into(), + data: Content::Raw(String::new()), + metadata: None, + encrypt: false, + compression_level: Some(0), + }) + .unwrap(); + let staged_directory = writer.get_directory_mut().clone(); + + let error = writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "duplicate".into(), + data: Content::Raw("non-empty data".into()), + metadata: Some(Content::Raw("metadata".into())), + encrypt: false, + compression_level: Some(0), + }) + .unwrap_err(); + assert!(matches!(error, PithosError::PathOccupied(_))); + assert_eq!(read(&path).unwrap(), before_bytes); + assert_eq!(*writer.get_directory_mut(), staged_directory); + drop(writer); + + assert_eq!(read(&path).unwrap(), before_bytes); + assert_eq!( + extract_pithos_entry(&path, &reader_key, "base", &temp_dir), + b"base data" + ); +} + +#[test] +fn test_writer_rejects_reverse_ancestor_and_metadata_conflicts_before_output() { + let temp_dir = TempDir::new().unwrap(); + let (path, _key, mut writer) = create_pithos_writer(&temp_dir, None); + writer.write_file_header().unwrap(); + writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "a/child".into(), + data: Content::Raw("child".into()), + metadata: None, + encrypt: false, + compression_level: Some(0), + }) + .unwrap(); + let before_ancestor = std::fs::metadata(&path).unwrap().len(); + assert!( + writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "a".into(), + data: Content::Raw("ancestor".into()), + metadata: None, + encrypt: false, + compression_level: Some(0), + }) + .is_err() + ); + assert_eq!(std::fs::metadata(&path).unwrap().len(), before_ancestor); + + writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "metadata.meta/child".into(), + data: Content::Raw("existing metadata".into()), + metadata: None, + encrypt: false, + compression_level: Some(0), + }) + .unwrap(); + let before_metadata = std::fs::metadata(&path).unwrap().len(); + assert!( + writer + .process_input(InputFile { + file_type: FileType::Data, + inner_path: "metadata".into(), + data: Content::Raw("data".into()), + metadata: Some(Content::Raw("metadata".into())), + encrypt: false, + compression_level: Some(0), + }) + .is_err() + ); + assert_eq!(std::fs::metadata(&path).unwrap().len(), before_metadata); +} + #[test] fn test_writer_rejects_unsafe_symlink_targets() { let temp_dir = TempDir::new().unwrap();