From 5580948ec9f2e9a715677b9573e959bdee9d11b0 Mon Sep 17 00:00:00 2001 From: Favourice01 Date: Sat, 26 Sep 2026 13:55:44 +0000 Subject: [PATCH] fix(crypto): make key rotation actually rotate, and prove reseal is interruption-safe Audit: reseal_wallet writes all four sealed fields in one UPDATE, so a single row is atomic. The audit also found that migrate-keys could not rotate anything. It selected rows with sealed_scheme <> V1, but every record is V1 whichever key sealed it, so a MASTER_KEY -> MASTER_KEY_NEXT run migrated nothing and still reported "0 remaining". Legacy scheme-0 rows failed in open(). While MASTER_KEY_NEXT was set, the server also opened every V1 row with the next key, which broke gas tanks that had not been migrated. - migrate-keys pages over all sealed rows and skips rows that already open under the new key; others are resealed from the old key. The loop moves into a library so tests can run it. - reseal_wallet's guard is now a compare-and-swap on the old ciphertext, since the scheme does not change on a key rotation. - reseal accepts legacy scheme-0 records (same algorithm) and upgrades them to V1. - The server tries MASTER_KEY_NEXT first and falls back to MASTER_KEY when opening, and seals new gas tanks under the next key during a rotation. - Adds interruption tests that crash a run mid-batch, then assert every row is wholly old or wholly new and that a second run finishes the rest. Closes #320 --- Cargo.lock | 1 + bin/migrate-keys/Cargo.toml | 6 + bin/migrate-keys/src/lib.rs | 107 ++++++++++++ bin/migrate-keys/src/main.rs | 105 ++---------- bin/migrate-keys/tests/interruption_tests.rs | 165 +++++++++++++++++++ crates/api/src/routes/sponsor.rs | 19 +-- crates/api/src/routes/wallets.rs | 2 +- crates/api/src/state.rs | 40 +++-- crates/crypto/src/lib.rs | 27 ++- crates/store/src/lib.rs | 37 ++--- 10 files changed, 369 insertions(+), 140 deletions(-) create mode 100644 bin/migrate-keys/src/lib.rs create mode 100644 bin/migrate-keys/tests/interruption_tests.rs diff --git a/Cargo.lock b/Cargo.lock index be45e62..e70165f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1811,6 +1811,7 @@ dependencies = [ "dotenvy", "octo-crypto", "octo-store", + "sqlx", "tokio", "tracing", "tracing-subscriber", diff --git a/bin/migrate-keys/Cargo.toml b/bin/migrate-keys/Cargo.toml index 60091ea..d5e496c 100644 --- a/bin/migrate-keys/Cargo.toml +++ b/bin/migrate-keys/Cargo.toml @@ -8,6 +8,9 @@ license.workspace = true repository.workspace = true authors.workspace = true +[lib] +path = "src/lib.rs" + [[bin]] name = "octo-migrate-keys" path = "src/main.rs" @@ -22,3 +25,6 @@ tracing-subscriber.workspace = true base64.workspace = true uuid.workspace = true dotenvy = "0.15" + +[dev-dependencies] +sqlx.workspace = true diff --git a/bin/migrate-keys/src/lib.rs b/bin/migrate-keys/src/lib.rs new file mode 100644 index 0000000..d71a162 --- /dev/null +++ b/bin/migrate-keys/src/lib.rs @@ -0,0 +1,107 @@ +//! Core of `octo-migrate-keys`: re-seal every sealed wallet seed from `old_key` to `new_key`. +//! +//! Split from `main.rs` so the interruption-safety tests can drive the exact production loop. +//! +//! ## Which rows need work +//! +//! Every record is tagged `SCHEME_V1` whichever key sealed it, so the scheme tag cannot say +//! whether a row is already rotated. AES-GCM authentication can: a row that opens under +//! `new_key` is done and is skipped; anything else is opened under `old_key` and re-sealed. +//! A row that opens under neither aborts the run — it needs an operator, not a silent skip. +//! +//! ## Interruption safety +//! +//! Each row is written by one `UPDATE` of all four sealed fields (`Store::reseal_wallet`), so an +//! interrupted run leaves every row wholly old or wholly new. Re-running resumes: rotated rows +//! open under `new_key` and are skipped. + +#![forbid(unsafe_code)] + +use anyhow::{Context, Result}; +use octo_crypto::{open, reseal, SealedSeed, MASTER_KEY_LEN, SCHEME_V1}; +use octo_store::Store; +use uuid::Uuid; + +/// Totals from one run. +#[derive(Debug, Default, PartialEq, Eq)] +pub struct Summary { + /// Rows re-sealed under the new key by this run. + pub migrated: usize, + /// Rows already on the new key (or changed concurrently) and left untouched. + pub skipped: usize, +} + +/// Re-seal every sealed wallet from `old_key` to `new_key`, `batch_size` rows per page. +/// +/// `before_write` runs after a row is re-sealed in memory and before it is persisted — the +/// window a crash would hit. Production passes a no-op; tests use it to inject a failure there. +pub async fn migrate( + store: &Store, + old_key: &[u8; MASTER_KEY_LEN], + new_key: &[u8; MASTER_KEY_LEN], + batch_size: i64, + mut before_write: impl FnMut(Uuid) -> Result<()>, +) -> Result { + let mut summary = Summary::default(); + let mut after_id: Option = None; + + loop { + let batch = store + .list_sealed_wallets(batch_size, after_id) + .await + .context("list_sealed_wallets")?; + let Some(last) = batch.last() else { break }; + after_id = Some(last.id); + tracing::info!(batch_len = batch.len(), "processing batch"); + + for wallet in &batch { + // Client-custody wallets hold no server-side seed; only sealed rows are rotated. + let (Some(ciphertext), Some(nonce), Some(salt)) = ( + wallet.sealed_ciphertext.as_ref(), + wallet.sealed_nonce.as_ref(), + wallet.sealed_salt.as_ref(), + ) else { + continue; + }; + let scheme = wallet.sealed_scheme.unwrap_or(SCHEME_V1 as i16); + let sealed = SealedSeed::from_parts_with_scheme( + ciphertext.clone(), + nonce, + salt, + u8::try_from(scheme).context("sealed_scheme out of range")?, + ) + .with_context(|| format!("from_parts wallet {}", wallet.id))?; + // Context is the network string bound into the AEAD AAD (e.g. "octo:mainnet"). + let context = format!("octo:{}", wallet.network); + + // Already rotated (or old == new on a current-scheme row): nothing to do. + if open(new_key, &sealed, context.as_bytes()).is_ok() { + summary.skipped += 1; + continue; + } + + let new_sealed = reseal(old_key, new_key, &sealed, context.as_bytes()) + .with_context(|| format!("wallet {} opens under neither key", wallet.id))?; + + before_write(wallet.id)?; + + let updated = store + .reseal_wallet( + wallet.id, + &new_sealed.ciphertext, + &new_sealed.nonce, + &new_sealed.salt, + i16::from(new_sealed.scheme), + ciphertext, + ) + .await + .with_context(|| format!("reseal_wallet DB update for {}", wallet.id))?; + if updated { + summary.migrated += 1; + } else { + summary.skipped += 1; + } + } + } + Ok(summary) +} diff --git a/bin/migrate-keys/src/main.rs b/bin/migrate-keys/src/main.rs index 4631a3c..03237ae 100644 --- a/bin/migrate-keys/src/main.rs +++ b/bin/migrate-keys/src/main.rs @@ -25,17 +25,16 @@ //! //! ## Idempotency //! -//! The store method `reseal_wallet` only updates a row when its current `sealed_scheme` matches -//! the expected "old" scheme. Re-running the tool against a fully-migrated database is safe and -//! produces 0 updates. +//! A row that already opens under the new key is skipped, and `reseal_wallet` only writes when +//! the row still holds the ciphertext that was read (compare-and-swap). Re-running the tool +//! against a fully-migrated database is safe and produces 0 updates. See `lib.rs`. //! //! ## Rollback //! -//! Old-scheme and new-scheme records can coexist in the database indefinitely because every open -//! call reads the scheme tag from the row and picks the correct key. To abort a rotation, simply -//! stop the tool; already-migrated rows remain openable with the new key, un-migrated rows remain -//! openable with the old key. Rolling back a completed rotation requires running the tool again -//! with the old and new keys swapped. +//! Old-key and new-key records can coexist indefinitely: during the rotation window the server +//! tries `MASTER_KEY_NEXT` first and falls back to `MASTER_KEY` (AES-GCM authentication tells +//! them apart). To abort, stop the tool. Rolling back a completed rotation means running the tool +//! again with the old and new keys swapped. //! //! ## Usage //! @@ -53,9 +52,8 @@ use anyhow::{Context, Result}; use base64::Engine; -use octo_crypto::{master_key_from_slice, reseal, MASTER_KEY_LEN, SCHEME_V1}; +use octo_crypto::{master_key_from_slice, MASTER_KEY_LEN}; use octo_store::Store; -use uuid::Uuid; /// Maximum rows per batch (hard cap, configurable via CLI). const DEFAULT_BATCH_SIZE: i64 = 100; @@ -78,87 +76,16 @@ async fn main() -> Result<()> { .context("connect to database")?; store.migrate().await.context("run migrations")?; - let mut after_id: Option = None; - let mut total_migrated = 0usize; - let mut total_skipped = 0usize; - - loop { - let batch = store - .list_wallets_needing_reseal(SCHEME_V1 as i16, cfg.batch_size, after_id) - .await - .context("list_wallets_needing_reseal")?; - - if batch.is_empty() { - break; - } - - tracing::info!( - batch_len = batch.len(), - after_id = ?after_id, - "processing batch" - ); - - for wallet in &batch { - // Client-custody wallets hold no server-side seed (the user's key never reaches us), - // so there is nothing to reseal. Only rows that actually carry sealed material — - // legacy server-custody wallets and gas-tank fee accounts — are rotated. - let (Some(ciphertext), Some(nonce), Some(salt), Some(scheme)) = ( - wallet.sealed_ciphertext.as_ref(), - wallet.sealed_nonce.as_ref(), - wallet.sealed_salt.as_ref(), - wallet.sealed_scheme, - ) else { - tracing::debug!(wallet_id = %wallet.id, "skipping wallet with no sealed seed"); - continue; - }; - - // Build the SealedSeed from the current DB values. - let sealed = octo_crypto::SealedSeed::from_parts_with_scheme( - ciphertext.clone(), - nonce, - salt, - scheme as u8, - ) - .with_context(|| format!("from_parts wallet {}", wallet.id))?; - - // Context is the network string bound into the AEAD AAD (e.g. "octo:mainnet"). - let context = format!("octo:{}", wallet.network); - - // reseal: open under old key → re-seal under new key (Zeroizing throughout). - let new_sealed = reseal(&cfg.old_key, &cfg.new_key, &sealed, context.as_bytes()) - .with_context(|| format!("reseal wallet {}", wallet.id))?; - - // Atomically swap the DB record. The idempotency guard (expected_old_scheme) - // means a concurrent run that already migrated this wallet is a safe no-op. - let updated = store - .reseal_wallet( - wallet.id, - &new_sealed.ciphertext, - &new_sealed.nonce, - &new_sealed.salt, - SCHEME_V1 as i16, - scheme, - ) - .await - .with_context(|| format!("reseal_wallet DB update for {}", wallet.id))?; - - if updated { - total_migrated += 1; - tracing::debug!(wallet_id = %wallet.id, "migrated"); - } else { - total_skipped += 1; - tracing::debug!(wallet_id = %wallet.id, "skipped (already migrated by concurrent runner)"); - } - } - - // Advance the cursor to the last wallet in this batch (ids are ordered ASC). - after_id = batch.last().map(|w| w.id); - } + let summary = + octo_migrate_keys::migrate(&store, &cfg.old_key, &cfg.new_key, cfg.batch_size, |_| { + Ok(()) + }) + .await?; tracing::info!( - total_migrated, - total_skipped, - "migration complete — 0 wallets remaining on old scheme" + total_migrated = summary.migrated, + total_skipped = summary.skipped, + "migration complete — every sealed wallet now opens under the new key" ); Ok(()) } diff --git a/bin/migrate-keys/tests/interruption_tests.rs b/bin/migrate-keys/tests/interruption_tests.rs new file mode 100644 index 0000000..2a9266c --- /dev/null +++ b/bin/migrate-keys/tests/interruption_tests.rs @@ -0,0 +1,165 @@ +//! Interruption-safety tests for the key-rotation reseal path (`octo_migrate_keys::migrate`). +//! +//! Requires Postgres via `DATABASE_URL` (loaded from `.env`); skipped with a message otherwise. +//! Each test runs in its own throwaway database so the whole wallets table is under its control. + +use octo_crypto::{open, seal, SealedSeed, MASTER_KEY_LEN}; +use octo_migrate_keys::{migrate, Summary}; +use octo_store::{NewWallet, Store}; +use sqlx::{Connection, Executor, PgConnection}; +use uuid::Uuid; + +const OLD_KEY: [u8; MASTER_KEY_LEN] = [1u8; MASTER_KEY_LEN]; +const NEW_KEY: [u8; MASTER_KEY_LEN] = [2u8; MASTER_KEY_LEN]; +const CONTEXT: &[u8] = b"octo:testnet"; +const WALLETS: usize = 12; +/// The injected crash hits the write of the Nth wallet (0-based), so N rows are already rotated. +const CRASH_AT: usize = 5; + +/// A fresh, migrated database; returns its store and name (for dropping). +async fn fresh_store() -> Option<(Store, String, String)> { + let _ = dotenvy::dotenv(); + let Ok(url) = std::env::var("DATABASE_URL") else { + eprintln!("SKIPPED: DATABASE_URL is not set"); + return None; + }; + let name = format!("octo_mk_{}", Uuid::new_v4().simple()); + let mut admin = PgConnection::connect(&url).await.expect("connect admin"); + admin + .execute(format!(r#"CREATE DATABASE "{name}""#).as_str()) + .await + .expect("create db"); + let base = url.split('?').next().unwrap(); + let db_url = format!("{}/{name}", base.rsplit_once('/').unwrap().0); + let store = Store::connect(&db_url).await.expect("connect test db"); + store.migrate().await.expect("migrate"); + Some((store, url, name)) +} + +async fn drop_db(store: Store, admin_url: &str, name: &str) { + drop(store); + let mut admin = PgConnection::connect(admin_url) + .await + .expect("connect admin"); + let _ = admin + .execute(format!(r#"DROP DATABASE IF EXISTS "{name}" WITH (FORCE)"#).as_str()) + .await; +} + +/// Seed `WALLETS` wallets whose seeds are sealed under `OLD_KEY`; returns (id, plaintext). +async fn seed_wallets(store: &Store) -> Vec<(Uuid, Vec)> { + let mut out = Vec::new(); + for i in 0..WALLETS { + let secret = format!("seed-{i}-{}", Uuid::new_v4()).into_bytes(); + let sealed = seal(&OLD_KEY, &secret, CONTEXT).unwrap(); + let account = format!("G{}", Uuid::new_v4().simple()); + let w = store + .create_wallet(NewWallet { + network: "testnet", + stellar_account_g: &account, + sealed_ciphertext: &sealed.ciphertext, + sealed_nonce: &sealed.nonce, + sealed_salt: &sealed.salt, + sealed_scheme: i16::from(sealed.scheme), + label: None, + user_id: None, + description: None, + }) + .await + .unwrap(); + out.push((w.id, secret)); + } + out +} + +/// Which key a row's four sealed fields open under, asserting it is exactly one and that the +/// plaintext is intact — i.e. the row is wholly pre- or post-migration, never a mix. +async fn row_key(store: &Store, id: Uuid, secret: &[u8]) -> &'static str { + let w = store.get_wallet(id).await.unwrap(); + let sealed = SealedSeed::from_parts_with_scheme( + w.sealed_ciphertext.unwrap(), + &w.sealed_nonce.unwrap(), + &w.sealed_salt.unwrap(), + u8::try_from(w.sealed_scheme.unwrap()).unwrap(), + ) + .unwrap(); + let old = open(&OLD_KEY, &sealed, CONTEXT).ok(); + let new = open(&NEW_KEY, &sealed, CONTEXT).ok(); + match (old, new) { + (Some(p), None) if p.as_slice() == secret => "old", + (None, Some(p)) if p.as_slice() == secret => "new", + _ => panic!("wallet {id} has mismatched sealed fields (opens under neither/both keys)"), + } +} + +/// Run `migrate` in its own task with a hook that panics on the `CRASH_AT`-th write — the moment +/// between re-sealing in memory and persisting, i.e. a process killed mid-batch. +async fn run_and_crash(store: &Store) { + let store = store.clone(); + let handle = tokio::spawn(async move { + let mut writes = 0; + // Small batches so the crash lands mid-run, across batch boundaries. + migrate(&store, &OLD_KEY, &NEW_KEY, 4, |_| { + if writes == CRASH_AT { + panic!("simulated crash mid-batch"); + } + writes += 1; + Ok(()) + }) + .await + }); + assert!(handle.await.unwrap_err().is_panic(), "the run must crash"); +} + +#[tokio::test] +async fn interrupting_migrate_keys_mid_batch_never_leaves_a_wallet_row_with_mismatched_scheme_and_ciphertext( +) { + let Some((store, admin_url, name)) = fresh_store().await else { + return; + }; + let wallets = seed_wallets(&store).await; + + run_and_crash(&store).await; + + let mut new = 0; + for (id, secret) in &wallets { + if row_key(&store, *id, secret).await == "new" { + new += 1; + } + } + assert_eq!( + new, CRASH_AT, + "exactly the rows written before the crash are rotated" + ); + drop_db(store, &admin_url, &name).await; +} + +#[tokio::test] +async fn a_second_clean_run_after_interruption_completes_the_remaining_wallets_correctly() { + let Some((store, admin_url, name)) = fresh_store().await else { + return; + }; + let wallets = seed_wallets(&store).await; + run_and_crash(&store).await; + + let summary = migrate(&store, &OLD_KEY, &NEW_KEY, 4, |_| Ok(())) + .await + .unwrap(); + assert_eq!( + summary, + Summary { + migrated: WALLETS - CRASH_AT, + skipped: CRASH_AT + } + ); + for (id, secret) in &wallets { + assert_eq!(row_key(&store, *id, secret).await, "new"); + } + + // A third run is a no-op. + let summary = migrate(&store, &OLD_KEY, &NEW_KEY, 4, |_| Ok(())) + .await + .unwrap(); + assert_eq!(summary.migrated, 0); + drop_db(store, &admin_url, &name).await; +} diff --git a/crates/api/src/routes/sponsor.rs b/crates/api/src/routes/sponsor.rs index bfbd912..a4ddbe6 100644 --- a/crates/api/src/routes/sponsor.rs +++ b/crates/api/src/routes/sponsor.rs @@ -112,8 +112,7 @@ pub async fn sponsor( .into(), )); }; - // Keep the versioned-scheme path (PR #158) so master-key rotation keeps working. Rows - // written before the scheme tag existed fall back to V1. + // Rows written before the scheme tag existed fall back to V1. let scheme = wallet .sealed_scheme .unwrap_or(octo_crypto::SCHEME_V1 as i16); @@ -123,15 +122,13 @@ pub async fn sponsor( inner_xdr: &inner_xdr, max_base_fee_stroops: max_fee, }; - let signed = match sign_fee_bump( - state.master_key_for_scheme(scheme), - &sealed, - state.network(), - 0, - &fb, - ) { - Ok(s) => s, - Err(_) => { + // During a rotation the row may be sealed under either key; AES-GCM tells us which. + let signed = match state + .opening_keys() + .find_map(|key| sign_fee_bump(key, &sealed, state.network(), 0, &fb).ok()) + { + Some(s) => s, + None => { let _ = state .store() .finalize_sponsored_transaction(reserved.id, "failed", None, Some("signing failed")) diff --git a/crates/api/src/routes/wallets.rs b/crates/api/src/routes/wallets.rs index 9332d4a..ebd0f98 100644 --- a/crates/api/src/routes/wallets.rs +++ b/crates/api/src/routes/wallets.rs @@ -306,7 +306,7 @@ pub async fn create_gas_tank( // Provision a fresh keypair inside wallet-core. The mnemonic is deliberately dropped: the // tank is a disposable fee account, recoverable only by re-provisioning. - let provisioned = octo_wallet_core::provision_wallet(state.master_key(), state.network())?; + let provisioned = octo_wallet_core::provision_wallet(state.sealing_key(), state.network())?; let wallet = state .store() .set_gas_tank( diff --git a/crates/api/src/state.rs b/crates/api/src/state.rs index 05c9f67..cde0de2 100644 --- a/crates/api/src/state.rs +++ b/crates/api/src/state.rs @@ -24,8 +24,8 @@ struct Inner { /// AES-256 master key used to seal/open seeds. Held zeroized. master_key: Zeroizing<[u8; MASTER_KEY_LEN]>, /// Optional next master key present only during a rotation window. - /// When set, the server tries this key first (for already-migrated rows) and falls back to - /// `master_key` for rows not yet re-sealed by `octo-migrate-keys`. + /// When set, the server seals new records under it, tries it first when opening (for + /// already-migrated rows), and falls back to `master_key` for rows not yet re-sealed. master_key_next: Option>, network: StellarNetwork, horizon: Horizon, @@ -140,8 +140,8 @@ impl AppState { } /// Set the next master key for zero-downtime key rotation. - /// When set, routes select the key based on the `sealed_scheme` of each wallet row: - /// already-migrated rows use `master_key_next`; un-migrated rows use `master_key`. + /// When set, new records are sealed under it and existing records are opened by trying it + /// first, then `master_key` (see [`AppState::opening_keys`]). pub fn with_master_key_next(mut self, key: [u8; MASTER_KEY_LEN]) -> Self { let inner = Arc::make_mut(&mut self.inner); inner.master_key_next = Some(Zeroizing::new(key)); @@ -214,19 +214,27 @@ impl AppState { self.inner.master_key_next.as_deref() } - /// Select the correct master key for a wallet given its `sealed_scheme`. - /// - /// During a rotation window (`MASTER_KEY_NEXT` is set): - /// - Rows already migrated to the target scheme use `master_key_next` (the new key). - /// - Rows not yet migrated use `master_key` (the old key). + /// Key to seal *new* records under: the next key during a rotation window, so fresh rows + /// never need migrating; otherwise the current key. + pub fn sealing_key(&self) -> &[u8; MASTER_KEY_LEN] { + self.inner + .master_key_next + .as_deref() + .unwrap_or(&self.inner.master_key) + } + + /// Keys to try when opening a sealed record, newest first. /// - /// When no next key is configured, always returns `master_key`. - pub fn master_key_for_scheme(&self, sealed_scheme: i16) -> &[u8; MASTER_KEY_LEN] { - use octo_crypto::SCHEME_V1; - match &self.inner.master_key_next { - Some(next_key) if sealed_scheme == SCHEME_V1 as i16 => next_key, - _ => &self.inner.master_key, - } + /// Every V1 record carries the same scheme tag whichever key sealed it, so the tag cannot + /// pick the key. AES-GCM authentication does: a wrong key fails cleanly, and the caller + /// moves on to the next one. Migrated rows open under the next key, un-migrated rows under + /// the current one. + pub fn opening_keys(&self) -> impl Iterator { + self.inner + .master_key_next + .as_deref() + .into_iter() + .chain(std::iter::once(&*self.inner.master_key)) } pub fn jwt_secret(&self) -> &[u8] { diff --git a/crates/crypto/src/lib.rs b/crates/crypto/src/lib.rs index bad41d2..bfdc105 100644 --- a/crates/crypto/src/lib.rs +++ b/crates/crypto/src/lib.rs @@ -220,13 +220,24 @@ pub fn open( /// and the same `context`. The intermediate plaintext is wrapped in [`Zeroizing`] (as returned by /// [`open`]) and wiped on drop. The returned [`SealedSeed`] gets a fresh random nonce and salt, as /// [`seal`] always generates — it never reuses the original record's. +/// +/// `reseal` is the one place a legacy `scheme = 0` record is accepted: `0` names the same +/// algorithm as [`SCHEME_V1`], so it is opened as V1 and re-sealed with an explicit V1 tag. pub fn reseal( old_key: &[u8; MASTER_KEY_LEN], new_key: &[u8; MASTER_KEY_LEN], sealed: &SealedSeed, context: &[u8], ) -> Result { - let plaintext = open(old_key, sealed, context)?; + let plaintext = if sealed.scheme == 0 { + let as_v1 = SealedSeed { + scheme: SCHEME_V1, + ..sealed.clone() + }; + open(old_key, &as_v1, context)? + } else { + open(old_key, sealed, context)? + }; seal(new_key, plaintext.as_ref(), context) } @@ -383,6 +394,20 @@ mod tests { assert_ne!(resealed.salt, sealed.salt); } + #[test] + fn reseal_upgrades_a_legacy_scheme_0_record_to_v1() { + let (old_mk, new_mk) = (key(), key()); + let mut legacy = seal(&old_mk, b"legacy seed", CTX).unwrap(); + legacy.scheme = 0; + assert!(open(&old_mk, &legacy, CTX).is_err()); + let resealed = reseal(&old_mk, &new_mk, &legacy, CTX).unwrap(); + assert_eq!(resealed.scheme, SCHEME_V1); + assert_eq!( + open(&new_mk, &resealed, CTX).unwrap().as_slice(), + b"legacy seed" + ); + } + #[test] fn reseal_fails_cleanly_if_old_key_or_context_is_wrong() { let old_mk = key(); diff --git a/crates/store/src/lib.rs b/crates/store/src/lib.rs index 97f981a..d4da115 100644 --- a/crates/store/src/lib.rs +++ b/crates/store/src/lib.rs @@ -581,13 +581,13 @@ impl Store { /// Atomically swap the sealed seed material for a single wallet after a reseal/key-rotation. /// - /// The caller (typically `bin/migrate-keys`) opens the old seed with the old master key, - /// re-seals it with the new master key via `octo_crypto::reseal`, and then calls this method - /// to persist the result. The `expected_scheme` guard ensures idempotency: if the row was - /// already migrated (e.g. by a concurrent runner) the update is silently skipped rather than - /// overwriting a newer record. + /// All four sealed fields are written by **one** `UPDATE`, so a row can never be observed (or + /// left after a crash) with a ciphertext from one sealing and a nonce/salt/scheme from + /// another. The `expected_old_ciphertext` compare-and-swap makes it idempotent: if the row + /// changed since it was read (a concurrent runner, or a re-provisioned gas tank), nothing is + /// written. Scheme alone cannot be the guard — a key rotation keeps the scheme at V1. /// - /// Returns `true` if the row was updated, `false` if it was already on the target scheme. + /// Returns `true` if the row was updated, `false` if it no longer held the expected record. pub async fn reseal_wallet( &self, wallet_id: Uuid, @@ -595,11 +595,8 @@ impl Store { new_nonce: &[u8], new_salt: &[u8], new_scheme: i16, - expected_old_scheme: i16, + expected_old_ciphertext: &[u8], ) -> Result { - // Only update the row if it still carries the old scheme — this is the idempotency guard. - // A concurrent runner that already migrated this wallet will have set sealed_scheme to - // `new_scheme`, so the WHERE clause won't match and no double-reseal can occur. let result = sqlx::query( r#" UPDATE wallets @@ -609,7 +606,7 @@ impl Store { sealed_scheme = $5, updated_at = now() WHERE id = $1 - AND sealed_scheme = $6 + AND sealed_ciphertext = $6 "#, ) .bind(wallet_id) @@ -617,33 +614,29 @@ impl Store { .bind(new_nonce) .bind(new_salt) .bind(new_scheme) - .bind(expected_old_scheme) + .bind(expected_old_ciphertext) .execute(&self.pool) .await?; Ok(result.rows_affected() > 0) } - /// Fetch a page of wallets whose `sealed_scheme` does not equal `target_scheme`, for the - /// migration backfill job. Returns at most `batch_size` rows ordered by `id` (stable for - /// resumable cursored iteration). Pass the last returned wallet's `id` as `after_id` on - /// subsequent calls to page through the full table without re-scanning already-migrated rows. - pub async fn list_wallets_needing_reseal( + /// Fetch a page of wallets that hold sealed seed material, ordered by `id`, for the + /// key-rotation job. Pass the last returned `id` as `after_id` to page through the table. + pub async fn list_sealed_wallets( &self, - target_scheme: i16, batch_size: i64, after_id: Option, ) -> Result, StoreError> { let rows = sqlx::query_as::<_, Wallet>( r#" SELECT * FROM wallets - WHERE sealed_scheme <> $1 - AND ($2::uuid IS NULL OR id > $2) + WHERE sealed_ciphertext IS NOT NULL + AND ($1::uuid IS NULL OR id > $1) ORDER BY id - LIMIT $3 + LIMIT $2 "#, ) - .bind(target_scheme) .bind(after_id) .bind(batch_size) .fetch_all(&self.pool)