fix: scrub the revoked key-slot table from every superblock slot (#103)

src/lib.rs

@@ -1060,6 +1060,27 @@ impl Chisel {
     /// database and `new` does. O(1) — the data key is unchanged, no page is
     /// re-encrypted.
     ///
+    /// # This is revocation, not cryptographic erasure
+    ///
+    /// The data-encryption key does not change, so `old` is being denied a way
+    /// IN — it is not being cut off from data it has already seen. Two limits
+    /// follow, and neither is a defect:
+    ///
+    /// * Anyone who captured the DEK while `old` was valid (a process memory
+    ///   dump, say) keeps the ability to read every page, including pages
+    ///   written afterwards. Recovering from a compromised DEK needs a full
+    ///   re-encryption pass, which is not implemented — see the tracking issue
+    ///   for bulk DEK rotation.
+    /// * An OLD COPY of the file — a backup taken before the revocation — is
+    ///   still fully readable with `old`. Revocation rewrites the live file, not
+    ///   copies of it.
+    ///
+    /// What this DOES guarantee, as of the fix for CRYPTO-1, is that no slot of
+    /// the live file still carries `old`'s wrapped DEK. Previously the
+    /// pre-revocation key-slot table survived verbatim in the sibling superblock
+    /// slots, so the revoked credential could recover the current DEK from the
+    /// current file with nothing but read access.
+    ///
     /// # Errors
     /// `EncryptionNotSupported` if the database has no encryption;
     /// `InvalidEncryptionKey` if `old` unlocks no slot; `NoFreeKeySlot` if all 8
@@ -1074,6 +1095,10 @@ impl Chisel {
     /// the only remaining credential. O(1) — the data key is unchanged, no
     /// page is re-encrypted.
     ///
+    /// Read the "This is revocation, not cryptographic erasure" section on
+    /// [`Chisel::rotate_key`] before relying on this for incident response: the
+    /// DEK is unchanged, so this closes a door rather than re-keying the data.
+    ///
     /// # Errors
     /// `EncryptionNotSupported` if the database has no encryption;
     /// `InvalidEncryptionKey` if `key` unlocks no slot; `LastKeySlot` if

src/recovery_tests.rs

@@ -2397,3 +2397,81 @@ fn a_raw_key_database_writes_zero_argon2_params_as_the_format_documents() {
         "expected to inspect both key slots; only saw {checked}"
     );
 }
+
+#[test]
+fn a_revoked_credentials_wrapped_dek_survives_in_no_slot() {
+    // CRYPTO-1. `rewrite_crypto_header` wrote the new key-slot table into
+    // exactly ONE superblock slot; the other N-1 kept their own state, table
+    // included. Because the table is CLEARTEXT (bytes 332..1356 of every
+    // superblock page) and the per-DB DEK never changes, the revoked
+    // credential's wrapped DEK stayed sitting in the LIVE file.
+    //
+    // The attack needed no tampering, no older file image and no rollback:
+    // an adversary holding the revoked credential and mere READ access parses
+    // the sibling slot's 128-byte record, runs derive_kek + unwrap_dek with
+    // slot.aad(), and recovers the DEK that still seals every current page.
+    // `Chisel::open` refusing the old key is not protection when the material
+    // to bypass it is in the same file. And the residue only cleared when N-1
+    // further commits happened to overwrite every sibling — for an idle
+    // database, never.
+    //
+    // This asserts the property directly on the bytes: after revocation, the
+    // old credential's wrap_tag must appear in NO slot of the file.
+    use crate::superblock::{CRYPTO_HEADER_OFFSET, KEY_SLOT_SIZE};
+
+    let tmp = NamedTempFile::new().unwrap();
+    let old = || Key::Raw(Zeroizing::new(vec![0xA1u8; 32]));
+    let new = || Key::Raw(Zeroizing::new(vec![0xB2u8; 32]));
+
+    let mut db = Chisel::open(tmp.path(), Options::default().encryption_key(old())).unwrap();
+    db.begin().unwrap();
+    db.allocate(b"sealed under a DEK that never changes")
+        .unwrap();
+    db.commit().unwrap();
+
+    // Capture the old credential's wrapped DEK + tag before revoking, so we can
+    // look for those exact bytes afterwards.
+    let slot0_record = {
+        let mut f = fs::File::open(tmp.path()).unwrap();
+        let mut unit = [0u8; crate::crypto::ENC_PAGE_SIZE];
+        f.read_exact(&mut unit).unwrap();
+        let base = CRYPTO_HEADER_OFFSET + 8;
+        let mut rec = [0u8; KEY_SLOT_SIZE];
+        rec.copy_from_slice(&unit[base..base + KEY_SLOT_SIZE]);
+        rec
+    };
+    // wrapped_dek is at 54..86 within the record — the bytes that must vanish.
+    let old_wrapped: Vec<u8> = slot0_record[54..86].to_vec();
+    assert_ne!(old_wrapped, vec![0u8; 32], "fixture must have a real wrap");
+
+    db.rotate_key(&old(), &new()).unwrap();
+    drop(db);
+
+    // Scan EVERY superblock slot's EVERY key slot.
+    let mut f = fs::File::open(tmp.path()).unwrap();
+    let mut unit = [0u8; crate::crypto::ENC_PAGE_SIZE];
+    for sb_slot in 0..crate::DEFAULT_SUPERBLOCK_COUNT as u64 {
+        f.seek(SeekFrom::Start(
+            sb_slot * crate::crypto::ENC_PAGE_SIZE as u64,
+        ))
+        .unwrap();
+        f.read_exact(&mut unit).unwrap();
+        for key_slot in 0..crate::superblock::KEY_SLOT_COUNT {
+            let base = CRYPTO_HEADER_OFFSET + 8 + key_slot * KEY_SLOT_SIZE;
+            assert_ne!(
+                &unit[base + 54..base + 86],
+                old_wrapped.as_slice(),
+                "the revoked credential's wrapped DEK is still readable in \
+                 superblock slot {sb_slot}, key slot {key_slot}"
+            );
+        }
+    }
+
+    // And the documented contract still holds in both directions.
+    assert!(
+        Chisel::open(tmp.path(), Options::default().encryption_key(old())).is_err(),
+        "the revoked credential must not open the database"
+    );
+    let db = Chisel::open(tmp.path(), Options::default().encryption_key(new())).unwrap();
+    drop(db);
+}

src/superblock/crypto_header.rs

@@ -164,6 +164,43 @@ impl CryptoHeader {
         })
     }
 
+    /// Overwrite ONLY the key-slot table inside an already-serialized superblock
+    /// image, leaving every other field — counter, roots, sealed body — exactly
+    /// as it was, and re-stamp the page checksum.
+    ///
+    /// This exists for credential revocation. `rewrite_crypto_header` writes a
+    /// full new superblock into one slot; the other N-1 slots keep their own
+    /// (older, still-valid) state, and that state includes their copy of the
+    /// key-slot table. Since the table is CLEARTEXT and the DEK never changes,
+    /// a revoked credential's wrapped DEK stayed readable in a sibling slot of
+    /// the live file: an adversary holding the revoked key needed only read
+    /// access to recover the DEK that still seals every current page. No
+    /// tampering, no older file image, no rollback — the stale credential lived
+    /// inside the current file, and for an idle database it lived there forever,
+    /// since the residue only clears when N-1 further commits happen to
+    /// overwrite every sibling.
+    ///
+    /// Patching just the table is what makes this safe to apply to a sibling:
+    /// the body's AAD (`sb_identity_aad`) covers magic, format_version,
+    /// txn_counter and superblock_count — NOT the crypto header — so the
+    /// sibling's sealed body still authenticates afterwards, and its value as a
+    /// shadow-paging fallback is untouched.
+    ///
+    /// Returns false if `unit` does not currently hold a parseable encrypted
+    /// superblock (a torn or never-written slot), in which case there is no
+    /// stale table in it to scrub.
+    pub fn overwrite_slot_table(&self, unit: &mut [u8; PAGE_SIZE]) -> bool {
+        if super::Superblock::deserialize(unit).is_none() {
+            return false;
+        }
+        for (i, slot) in self.slots.iter().enumerate() {
+            let base = SLOT_TABLE_OFFSET + i * KEY_SLOT_SIZE;
+            slot.write_into(&mut unit[base..base + KEY_SLOT_SIZE]);
+        }
+        page::stamp_checksum(unit);
+        true
+    }
+
     /// Count how many slots currently hold a wrapped DEK (state == active).
     /// Used by `remove_key` (Task 5.4) to guard against removing the last
     /// credential and locking the caller out of their own database.

src/transaction/keys.rs

@@ -113,6 +113,42 @@ impl TransactionManager {
             cache.io_mut().write_page_unit(inactive, &unit)?;
         }
 
+        // CRYPTO-1: scrub the key-slot table out of every OTHER slot too.
+        //
+        // Writing only the target slot left the PRE-revocation table intact in
+        // its siblings. The table is cleartext and the DEK never changes, so a
+        // revoked credential's wrapped DEK stayed recoverable from the LIVE file
+        // by anyone with read access — `Chisel::open` correctly refused the old
+        // key, but the bytes needed to bypass that were still sitting at offset
+        // 332 of a sibling slot. The public contract says the opposite: "After
+        // this returns, `key` no longer opens the database."
+        //
+        // Only the table region is patched (see `overwrite_slot_table`), so each
+        // sibling keeps its own counter, roots and sealed body and remains a
+        // valid shadow-paging fallback.
+        //
+        // Crash window: these writes share the target slot's fsync below, so a
+        // crash mid-way can leave a sibling un-scrubbed. That is no worse than
+        // the steady state this replaces, and the next key operation clears it.
+        // Ordering the scrubs after the target write means the revocation itself
+        // is never the thing lost to a partial write.
+        {
+            use crate::crypto::ENC_PAGE_SIZE;
+            let mut unit = [0u8; ENC_PAGE_SIZE];
+            for slot in 0..self.superblock_count as u64 {
+                if slot == inactive {
+                    continue;
+                }
+                cache.io_mut().read_page_unit_into(slot, &mut unit)?;
+                let mut image = [0u8; PAGE_SIZE];
+                image.copy_from_slice(&unit[..PAGE_SIZE]);
+                if new_header.overwrite_slot_table(&mut image) {
+                    unit[..PAGE_SIZE].copy_from_slice(&image);
+                    cache.io_mut().write_page_unit(slot, &unit)?;
+                }
+            }
+        }
+
         // Durability linearization point: the rewrite is crash-safe only after
         // this fsync returns. A crash before this leaves the old superblock
         // intact in the other slot; recovery picks it by highest txn_counter.