diff --git a/README.md b/README.md index 0a51b1d..eb22e0b 100644 --- a/README.md +++ b/README.md @@ -360,7 +360,7 @@ Encryption introduces the second major: an encrypted database is stamped MAJOR = **Page level** — each non-superblock page carries a one-byte `page_format_version` in its header, letting individual page layouts evolve within a major without a file-wide format bump. The post-1.0 upgrade story is lazy migration: on read, the page-type module dispatches on its page's declared version; on write, it always produces the current version; cold pages stay in the old layout until an opt-in `db.upgrade()` sweep rewrites them. An additional 8 bytes are reserved in every non-superblock page header for future common-header fields. -Write safety across minors is a narrower guarantee: a binary at MINOR = *m* opening a file at MINOR = *m' > m* cannot safely commit without risking overwriting fields it doesn't know about. The open gate is MAJOR-only by design, so minor variants coexist — same-major files of any minor open successfully, and the chunk-tags MINOR = 1 variant is the first such case. The write-refusal arm (refuse writes when file MINOR > binary MINOR, leaving the newer-minor file read-only) is not yet wired up; it lands with the first post-1.0 minor bump that makes the direction observable. The post-1.0 cross-minor read-compatibility guarantee is absolute; write-compatibility requires binary MINOR ≥ file MINOR. +Write safety across minors is a narrower guarantee: a binary at MINOR = *m* opening a file at MINOR = *m' > m* cannot safely commit without risking overwriting fields it doesn't know about. The open gate is MAJOR-only by design, so minor variants coexist — same-major files of any minor open successfully, and the chunk-tags MINOR = 1 variant is the first such case. The write-refusal arm (refuse writes when file MINOR > binary MINOR) is implemented (I29): opening a newer-minor file forces the handle read-only, so any mutation returns `ReadOnlyMode` rather than risking a write that clobbers fields the binary doesn't know about. The post-1.0 cross-minor read-compatibility guarantee is absolute; write-compatibility requires binary MINOR ≥ file MINOR. ### Pre-1.0 caveat diff --git a/src/lib.rs b/src/lib.rs index f6bcb0e..13d86bb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -373,6 +373,15 @@ impl Chisel { } let io = PageIo::open(path, options.read_only)?; + // I143: decide create-vs-open from the file length observed AFTER the + // flock is held (page_count() returns the count cached from the post-lock + // length), NOT from the pre-lock `file_exists` stat. The pre-lock stat + // races a concurrent creator: another process can create + commit + + // release the lock between our stat and our lock, and the stale boolean + // would then run create_new over its just-committed data. `file_exists` + // stays above only for the create_if_missing gate, which must remain + // pre-lock so a refused open never materializes an empty file. + let existed = io.page_count()? > 0; let cache = PageCache::new( io, options.cache_max_bytes, @@ -381,7 +390,7 @@ impl Chisel { SpillwayLocation::Path(path.to_path_buf()), ); - let txm = if file_exists { + let txm = if existed { // Existing database: N is discovered from the on-disk // superblock. options.superblock_count is ignored here. TransactionManager::open_existing(cache, options.encryption_key.clone())? diff --git a/src/recovery_tests.rs b/src/recovery_tests.rs index e15e17a..4602160 100644 --- a/src/recovery_tests.rs +++ b/src/recovery_tests.rs @@ -9,10 +9,11 @@ use crate::page::{self, PageType, PAGE_SIZE}; use crate::superblock::Superblock; -use crate::{Chisel, ChiselError, Options}; +use crate::{Chisel, ChiselError, Key, Options}; use std::fs; use std::io::{Read as _, Seek, SeekFrom, Write}; use tempfile::{NamedTempFile, TempDir}; +use zeroize::Zeroizing; /// Scan the file for the first page whose type tag matches `want` and /// return its page id. Panics if no such page exists; test helper only. @@ -1623,3 +1624,29 @@ fn test_open_rejects_page_size_mismatch() { Ok(_) => panic!("Chisel::open accepted a file with a mismatched page_size"), } } + +// I144 regression: a forged crypto-header stride must fail the open with a typed +// CorruptSuperblock, never a division-by-zero panic in set_stride (the stride is +// plaintext, guarded only by the forgeable XXH3 checksum). Page 0 is always read +// first to learn the stride, so we scribble byte 325 (the u32 stride field at +// CRYPTO_HEADER_OFFSET+1) to 0 and re-stamp the checksum so deserialize accepts it. +#[test] +fn corrupt_crypto_header_stride_errors_not_panic() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("enc.db"); + let opts = || Options::default().encryption_key(Key::Raw(Zeroizing::new(vec![0x11u8; 32]))); + { + let mut db = Chisel::open(&path, opts()).unwrap(); + db.begin().unwrap(); + db.allocate(b"payload").unwrap(); + db.commit().unwrap(); + } + rewrite_page_with_valid_checksum(&path, 0, |buf| { + buf[325..329].copy_from_slice(&0u32.to_le_bytes()); + }); + match Chisel::open(&path, opts()) { + Err(ChiselError::CorruptSuperblock { .. }) => {} + Err(other) => panic!("expected CorruptSuperblock for a forged stride, got {other:?}"), + Ok(_) => panic!("forged-stride open unexpectedly succeeded (should have failed)"), + } +} diff --git a/src/transaction/freemap.rs b/src/transaction/freemap.rs index eebbe2b..54bba32 100644 --- a/src/transaction/freemap.rs +++ b/src/transaction/freemap.rs @@ -663,12 +663,22 @@ impl TransactionManager { pub(crate) fn reclaim_freemap_orphans(&mut self) -> Result { let savepoint_active = !self.savepoints.is_empty(); let superblock_count = self.superblock_count; - let mut cache = self.cache.borrow_mut(); - self.freemap.reclaim_orphans( - &mut cache, - &mut self.current_roots, - savepoint_active, - superblock_count, - ) + // I145: wrap the sweep in poison_on_fatal like every other TM entry point. + // A fatal error mid-sweep (an IoError/CorruptPage from a page read or a + // live-page cache.get) otherwise returns un-poisoned, and reclaim_orphans + // writes the partially-advanced freemap root back into current_roots even + // on its error path — leaving a usable manager holding an indeterminate + // freemap. The inner borrow is scoped so the cache is released before + // poison_on_fatal takes &self. + let result = { + let mut cache = self.cache.borrow_mut(); + self.freemap.reclaim_orphans( + &mut cache, + &mut self.current_roots, + savepoint_active, + superblock_count, + ) + }; + self.poison_on_fatal(result) } } diff --git a/src/transaction/recovery.rs b/src/transaction/recovery.rs index 42d43c3..81c3cec 100644 --- a/src/transaction/recovery.rs +++ b/src/transaction/recovery.rs @@ -255,6 +255,18 @@ impl TransactionManager { if let Some(stride) = Superblock::deserialize(&page0).and_then(|sb| sb.encryption.map(|h| h.stride as usize)) { + // I144: the stride comes from the plaintext crypto-header, guarded + // only by the forgeable (non-cryptographic) XXH3 page checksum — the + // same trust boundary decrypt_body already bounds-checks ct_len at. + // ENC_PAGE_SIZE is the only value ever written; reject anything else + // BEFORE set_stride, which divides the file length by stride (a forged + // 0 is a division-by-zero panic; a huge value drives multi-GiB read + // allocations). Fail as CorruptSuperblock, honoring poison-not-panic. + if stride != crate::crypto::ENC_PAGE_SIZE { + return Err(ChiselError::CorruptSuperblock { + defects: Vec::new(), + }); + } cache.io_mut().set_stride(stride); } // With an intact page 0 the stride is already correct here, so this