Skip to content

Commit fa28bbd

Browse files
committed
fix(txn): scope spill keys to the open handle and reclaim orphaned scratch
The transaction sequence restarts at 1 on every open and doubles as the spill nonce, so a spill key derived only from the durable file id and that sequence would repeat across two opens of one store at the same sequence, and the nonce built from it would repeat with it. Mix a random per-open epoch into the derivation so the key is distinct per open, leaving the sequence free to serve as the nonce. Pair this with reclaiming the scratch such keys ever protected: a transaction dropped without commit or abort cannot await the removal, so it now hands its exact path to the next begin_write, and whatever still survives at the next open is swept outright, since a spill file is never read back across handles.
1 parent 5f1a431 commit fa28bbd

13 files changed

Lines changed: 257 additions & 34 deletions

File tree

src/crypto/kdf.rs

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,35 @@ pub fn derive_hk(mk: &MasterKey) -> Result<DerivedKey> {
5757

5858
/// Derive a transient per-WriteTxn spill key from the master key.
5959
///
60-
/// `info` = `"pagedb/spill/" ‖ file_id[16] ‖ txn_seq.to_le_bytes()[8]` (13 + 16 + 8 = 37 bytes).
61-
/// The key is transient: the tmp file it protects is discarded at commit/abort.
62-
pub fn derive_spill_key(mk: &MasterKey, file_id: &[u8; 16], txn_seq: u64) -> Result<DerivedKey> {
60+
/// `info` = `"pagedb/spill/" ‖ file_id[16] ‖ spill_epoch[16] ‖ txn_seq.to_le_bytes()[8]`
61+
/// (13 + 16 + 16 + 8 = 53 bytes).
62+
///
63+
/// `spill_epoch` is fresh per open handle. `file_id` and the master key are
64+
/// durable and `txn_seq` restarts at 1 on every open, so those three alone
65+
/// would repeat the same key across two opens of one store — and the spill
66+
/// nonce, built from that same restarting sequence, would repeat with it. The
67+
/// epoch is what keeps each open's spill key distinct, and therefore what makes
68+
/// the sequence a sound nonce source without a durable anchor.
69+
///
70+
/// Nothing reads a spill file written by an earlier open, so there is no
71+
/// durability cost to a per-open key: the scratch a crash leaves behind is
72+
/// swept at the next open, never decrypted.
73+
pub fn derive_spill_key(
74+
mk: &MasterKey,
75+
file_id: &[u8; 16],
76+
spill_epoch: &[u8; 16],
77+
txn_seq: u64,
78+
) -> Result<DerivedKey> {
6379
const PREFIX: &[u8] = b"pagedb/spill/";
64-
let mut info = [0u8; PREFIX.len() + 16 + 8];
65-
info[..PREFIX.len()].copy_from_slice(PREFIX);
66-
info[PREFIX.len()..PREFIX.len() + 16].copy_from_slice(file_id);
67-
info[PREFIX.len() + 16..].copy_from_slice(&txn_seq.to_le_bytes());
80+
let mut info = [0u8; PREFIX.len() + 16 + 16 + 8];
81+
let mut off = 0;
82+
info[off..off + PREFIX.len()].copy_from_slice(PREFIX);
83+
off += PREFIX.len();
84+
info[off..off + 16].copy_from_slice(file_id);
85+
off += 16;
86+
info[off..off + 16].copy_from_slice(spill_epoch);
87+
off += 16;
88+
info[off..off + 8].copy_from_slice(&txn_seq.to_le_bytes());
6889
expand(mk.as_bytes(), &info)
6990
}
7091

@@ -114,6 +135,36 @@ mod tests {
114135
assert_ne!(a.as_bytes(), b.as_bytes());
115136
}
116137

138+
#[test]
139+
fn spill_key_isolates_per_open_at_the_same_sequence() {
140+
// The transaction sequence restarts at 1 on every open and also builds
141+
// the spill nonce, so two opens of one store must not share a key at
142+
// one sequence.
143+
let mk = derive_mk(&fixed_kek(), &[0; 16], 0).unwrap();
144+
let file_id = [0x5A; 16];
145+
let a = derive_spill_key(&mk, &file_id, &[0x01; 16], 1).unwrap();
146+
let b = derive_spill_key(&mk, &file_id, &[0x02; 16], 1).unwrap();
147+
assert_ne!(a.as_bytes(), b.as_bytes());
148+
}
149+
150+
#[test]
151+
fn spill_key_isolates_per_sequence_within_one_open() {
152+
let mk = derive_mk(&fixed_kek(), &[0; 16], 0).unwrap();
153+
let epoch = [0x01; 16];
154+
let a = derive_spill_key(&mk, &[0x5A; 16], &epoch, 1).unwrap();
155+
let b = derive_spill_key(&mk, &[0x5A; 16], &epoch, 2).unwrap();
156+
assert_ne!(a.as_bytes(), b.as_bytes());
157+
}
158+
159+
#[test]
160+
fn spill_key_isolates_per_store() {
161+
let mk = derive_mk(&fixed_kek(), &[0; 16], 0).unwrap();
162+
let epoch = [0x01; 16];
163+
let a = derive_spill_key(&mk, &[0x5A; 16], &epoch, 1).unwrap();
164+
let b = derive_spill_key(&mk, &[0xA5; 16], &epoch, 1).unwrap();
165+
assert_ne!(a.as_bytes(), b.as_bytes());
166+
}
167+
117168
#[test]
118169
fn dek_isolates_per_realm() {
119170
let mk = derive_mk(&fixed_kek(), &[0; 16], 0).unwrap();

src/crypto/random/identity.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,18 @@ pub(crate) fn segment_id() -> Result<[u8; 16]> {
2525
random_nonzero_identity()
2626
}
2727

28+
/// Identity partitioning the spill nonce space of one open handle.
29+
///
30+
/// The per-transaction spill key is derived from the durable `file_id` and the
31+
/// transaction sequence, and the spill nonce is built from that same sequence.
32+
/// The sequence restarts at 1 on every open, so without a per-open component
33+
/// two handles over the same store would encrypt different scratch payloads
34+
/// under one key at one nonce. Mixing this in makes the key distinct per open,
35+
/// which is what leaves the sequence free to serve as the nonce.
36+
pub(crate) fn spill_epoch() -> Result<[u8; 16]> {
37+
random_nonzero_identity()
38+
}
39+
2840
// Sole caller is `txn::db::snapshot::apply_incremental`, which is native-only
2941
// (`#![cfg(not(target_arch = "wasm32"))]`); gate the function the same way.
3042
#[cfg(not(target_arch = "wasm32"))]
@@ -41,5 +53,11 @@ mod tests {
4153
assert_ne!(database_identity().unwrap().0, [0u8; 16]);
4254
assert_ne!(segment_id().unwrap(), [0u8; 16]);
4355
assert_ne!(journal_id().unwrap(), [0u8; 16]);
56+
assert_ne!(spill_epoch().unwrap(), [0u8; 16]);
57+
}
58+
59+
#[test]
60+
fn spill_epochs_do_not_repeat_across_opens() {
61+
assert_ne!(spill_epoch().unwrap(), spill_epoch().unwrap());
4462
}
4563
}

src/crypto/random/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@ mod identity;
44

55
#[cfg(not(target_arch = "wasm32"))]
66
pub(crate) use identity::journal_id;
7-
pub(crate) use identity::{database_identity, segment_id};
7+
pub(crate) use identity::{database_identity, segment_id, spill_epoch};

src/recovery/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
//! Open-flow recovery: apply-journal replay, catalog reconciliation,
2-
//! tombstone GC.
2+
//! tombstone GC, spill-scratch reclamation.
33
44
pub(crate) mod deep_walk;
55
pub(crate) mod gc;
66
pub(crate) mod journal;
77
pub(crate) mod reconcile;
8+
pub(crate) mod scratch;
89
#[cfg(test)]
910
mod tests;
1011

src/recovery/scratch.rs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
//! Reclamation of write-transaction spill scratch.
2+
//!
3+
//! A spill file is scratch for one live write transaction and is never read
4+
//! back by a later one: its key is scoped to the open handle that wrote it. So
5+
//! every scratch file present at open belongs to a handle that is already gone,
6+
//! and is reclaimable without inspection.
7+
8+
use crate::Result;
9+
use crate::vfs::Vfs;
10+
11+
/// Directory holding per-transaction spill scratch.
12+
const SCRATCH_DIR: &str = "tmp";
13+
14+
/// Prefix of a spill scratch file name, completed by the transaction sequence.
15+
const SCRATCH_PREFIX: &str = "scratch-";
16+
17+
/// Delete the spill scratch left by handles that closed without cleaning up.
18+
///
19+
/// Callers must hold write authority over the store: this removes files, and a
20+
/// handle that only observes must not. Returns the number of files deleted.
21+
pub async fn delete_orphaned_scratch<V: Vfs + Clone>(vfs: &V) -> Result<u64> {
22+
let entries = vfs.list_dir(SCRATCH_DIR).await?;
23+
let mut count: u64 = 0;
24+
for name in entries {
25+
// Only the names this crate writes. `tmp/` is a shared directory in the
26+
// store layout, and a sweep that removed everything under it would
27+
// reclaim files belonging to whatever is added there next.
28+
if !name.starts_with(SCRATCH_PREFIX) {
29+
continue;
30+
}
31+
vfs.remove(&format!("{SCRATCH_DIR}/{name}")).await?;
32+
count += 1;
33+
}
34+
if count > 0 {
35+
// Durable directory entries: a crash between here and the next open
36+
// would otherwise present the same scratch to be swept again.
37+
vfs.sync_dir(SCRATCH_DIR).await?;
38+
}
39+
Ok(count)
40+
}
41+
42+
#[cfg(test)]
43+
mod tests {
44+
use super::*;
45+
use crate::vfs::VfsFile;
46+
use crate::vfs::memory::MemVfs;
47+
use crate::vfs::types::OpenMode;
48+
49+
async fn write(vfs: &MemVfs, path: &str) {
50+
vfs.mkdir_all(SCRATCH_DIR).await.unwrap();
51+
let mut f = vfs.open(path, OpenMode::CreateNew).await.unwrap();
52+
f.write_at(0, b"scratch").await.unwrap();
53+
}
54+
55+
#[tokio::test(flavor = "current_thread")]
56+
async fn sweeps_scratch_and_spares_unrelated_names() {
57+
let vfs = MemVfs::new();
58+
write(&vfs, "tmp/scratch-1").await;
59+
write(&vfs, "tmp/scratch-9").await;
60+
write(&vfs, "tmp/unrelated").await;
61+
62+
assert_eq!(delete_orphaned_scratch(&vfs).await.unwrap(), 2);
63+
64+
assert!(vfs.open("tmp/scratch-1", OpenMode::Read).await.is_err());
65+
assert!(vfs.open("tmp/scratch-9", OpenMode::Read).await.is_err());
66+
assert!(vfs.open("tmp/unrelated", OpenMode::Read).await.is_ok());
67+
}
68+
69+
#[tokio::test(flavor = "current_thread")]
70+
async fn absent_scratch_directory_is_not_an_error() {
71+
let vfs = MemVfs::new();
72+
assert_eq!(delete_orphaned_scratch(&vfs).await.unwrap(), 0);
73+
}
74+
}

src/txn/db/core.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,17 @@ pub struct Db<V: Vfs + Clone> {
177177
/// Each txn gets `txn_seq.fetch_add(1, Relaxed)` (first txn gets 1 since
178178
/// we start at 0 and add-then-use the pre-increment value + 1).
179179
pub(crate) txn_seq: AtomicU64,
180+
/// Random per-open identity mixed into every spill key derivation. The
181+
/// counter above restarts at 1 on each open and also supplies the spill
182+
/// nonce, so this is what stops two opens of one store from encrypting
183+
/// different scratch under the same key at the same nonce.
184+
pub(crate) spill_epoch: [u8; 16],
185+
/// Scratch paths belonging to write transactions that were dropped without
186+
/// `commit` or `abort`. `Drop` cannot await the VFS removal, so it records
187+
/// the exact path here and the next `begin_write` — which holds the writer
188+
/// lock and can await — removes it. Anything still listed when the handle
189+
/// closes is swept by the next open.
190+
pub(crate) orphaned_spill_paths: parking_lot::Mutex<Vec<String>>,
180191
/// The mode this handle was opened with (Standalone, Follower, `ReadOnly`, Observer).
181192
pub(crate) mode: DbMode,
182193
/// Set of reader `entry_id`s that have been aborted by `AbortOldest` stall

src/txn/db/open/create.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ struct FreshDbState<V: Vfs + Clone> {
4242
}
4343

4444
impl<V: Vfs + Clone> Db<V> {
45-
fn assemble_fresh(state: FreshDbState<V>) -> Self {
45+
fn assemble_fresh(state: FreshDbState<V>) -> Result<Self> {
4646
let FreshDbState {
4747
pager,
4848
realm_id,
@@ -83,7 +83,7 @@ impl<V: Vfs + Clone> Db<V> {
8383
seq: initial.seq,
8484
},
8585
);
86-
Self {
86+
Ok(Self {
8787
pager: Arc::new(pager),
8888
realm_id,
8989
page_size,
@@ -109,6 +109,8 @@ impl<V: Vfs + Clone> Db<V> {
109109
mmap_bytes_in_use: Arc::new(AtomicU64::new(0)),
110110
spill_bytes_in_use: AtomicU64::new(0),
111111
txn_seq: AtomicU64::new(0),
112+
spill_epoch: crate::crypto::random::spill_epoch()?,
113+
orphaned_spill_paths: parking_lot::Mutex::new(Vec::new()),
112114
mode: DbMode::Standalone,
113115
aborted_readers: parking_lot::Mutex::new(std::collections::HashSet::new()),
114116
sentinel_locks: Vec::new(),
@@ -132,7 +134,7 @@ impl<V: Vfs + Clone> Db<V> {
132134
visibility_test_hook: parking_lot::Mutex::new(None),
133135
#[cfg(test)]
134136
rekey_test_fault: parking_lot::Mutex::new(None),
135-
}
137+
})
136138
}
137139

138140
/// Bootstrap a fresh database. Creates `main.db`, writes an initial A/B
@@ -278,7 +280,7 @@ impl<V: Vfs + Clone> Db<V> {
278280
let vfs_for_pager = V::clone(&*vfs_arc);
279281
let pager = Pager::open(vfs_for_pager, mk, cfg).await?;
280282

281-
Ok(Self::assemble_fresh(FreshDbState {
283+
Self::assemble_fresh(FreshDbState {
282284
pager,
283285
realm_id: realm,
284286
page_size,
@@ -291,6 +293,6 @@ impl<V: Vfs + Clone> Db<V> {
291293
file_id,
292294
kek_salt,
293295
initial,
294-
}))
296+
})
295297
}
296298
}

src/txn/db/open/existing.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,8 @@ impl<V: Vfs + Clone> Db<V> {
274274
mmap_bytes_in_use: Arc::new(AtomicU64::new(0)),
275275
spill_bytes_in_use: AtomicU64::new(0),
276276
txn_seq: AtomicU64::new(0),
277+
spill_epoch: crate::crypto::random::spill_epoch()?,
278+
orphaned_spill_paths: parking_lot::Mutex::new(Vec::new()),
277279
mode,
278280
aborted_readers: parking_lot::Mutex::new(std::collections::HashSet::new()),
279281
sentinel_locks: Vec::new(),

src/txn/db/open/recovery.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,14 @@ pub(super) async fn recover_open_state<V: Vfs + Clone>(
3232
Err(PagedbError::Io(error)) if error.kind() == std::io::ErrorKind::NotFound => {}
3333
Err(error) => return Err(error),
3434
}
35+
36+
// Write-transaction spill scratch is keyed to the handle that wrote it,
37+
// so anything still present belongs to a handle that is gone and is
38+
// reclaimable outright. Without this the scratch of every transaction
39+
// that died with its process would accumulate for the life of the
40+
// store. Gated with the removals above because it needs write
41+
// authority.
42+
crate::recovery::scratch::delete_orphaned_scratch(&*db.vfs).await?;
3543
}
3644

3745
// An incremental apply assembles its target beside `main.db` and renames it

src/txn/write/spill.rs

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,17 +39,22 @@ pub(crate) struct SpillSegmentMeta {
3939
/// storage in a per-transaction tmp file.
4040
///
4141
/// The tmp file lives at `tmp/scratch-<txn_seq>`. It is created lazily on the
42-
/// first `append` call and removed at `commit` or `abort` (best-effort).
42+
/// first `append` call and removed at `commit` or `abort`. A transaction
43+
/// dropped without either records its path for the next `begin_write` to
44+
/// remove, and whatever a crash leaves behind is swept at the next open.
4345
///
4446
/// Nonce scheme: `txn_seq_le6 ‖ segment_index_le6`.
4547
/// - First 6 bytes: the low 6 bytes of `txn_seq` in little-endian order.
4648
/// - Last 6 bytes: the low 6 bytes of the per-append segment index in
4749
/// little-endian order.
4850
///
49-
/// This guarantees uniqueness across all appends within a txn and across
50-
/// independent txns (different `txn_seq`), without requiring a durable
51-
/// nonce anchor (the tmp file is discarded before any subsequent txn can
52-
/// reuse the same `txn_seq`).
51+
/// Uniqueness under one key comes from `txn_seq` being strictly increasing
52+
/// within an open handle and `segment_index` within a transaction. It does not
53+
/// come from the file being deleted: `txn_seq` restarts at 1 on every open, so
54+
/// deletion alone would leave two opens writing one nonce. The spill key is
55+
/// instead derived per open, from a random epoch this handle generates and
56+
/// never persists, so a repeated sequence lands under a different key and no
57+
/// durable nonce anchor is needed.
5358
pub struct SpillScope<'scope, 'db, V: Vfs + Clone> {
5459
pub(super) txn: &'scope mut WriteTxn<'db, V>,
5560
}
@@ -212,7 +217,12 @@ impl<'db, V: Vfs + Clone> WriteTxn<'db, V> {
212217
pub(crate) fn ensure_spill_cipher(&mut self) -> Result<&Cipher> {
213218
if self.spill_cipher.is_none() {
214219
let pager_mk = self.db.pager.mk()?;
215-
let key = derive_spill_key(&pager_mk, &self.db.file_id, self.txn_seq)?;
220+
let key = derive_spill_key(
221+
&pager_mk,
222+
&self.db.file_id,
223+
&self.db.spill_epoch,
224+
self.txn_seq,
225+
)?;
216226
self.spill_cipher = Some(Cipher::new_aes_gcm(&key));
217227
}
218228
self.spill_cipher.as_ref().ok_or(PagedbError::NotFound)
@@ -238,10 +248,10 @@ impl<'db, V: Vfs + Clone> WriteTxn<'db, V> {
238248
/// Build the per-append nonce deterministically from `(txn_seq, segment_index)`.
239249
///
240250
/// Layout: `txn_seq_le6 ‖ segment_index_le6` (6 + 6 = 12 bytes).
241-
/// Uniqueness: nonces are unique per-txn (distinct `segment_index`) and
242-
/// across txns (distinct `txn_seq` in first 6 bytes). No durable anchor
243-
/// is needed because the tmp file is discarded before any key reuse
244-
/// could matter.
251+
/// Uniqueness within one key: distinct `segment_index` within a
252+
/// transaction, distinct `txn_seq` across the transactions of one open
253+
/// handle. `txn_seq` repeats across opens, which is why the key is scoped
254+
/// to an open rather than to the store; see the type-level docs.
245255
pub(crate) fn next_spill_nonce(&self, segment_index: u64) -> Nonce {
246256
let mut bytes = [0u8; 12];
247257
let seq_le = self.txn_seq.to_le_bytes();

0 commit comments

Comments
 (0)