diff --git a/core/src/client.rs b/core/src/client.rs index 5a2a814..4559906 100644 --- a/core/src/client.rs +++ b/core/src/client.rs @@ -529,8 +529,13 @@ pub fn send( fields.push(("Reply-To".into(), home.uri(&me.pk()))); } } - // sec 5.8: hand an accepted correspondent the token for our own mailbox. - if let Some(identity) = store.accepted_identity(&addr.short())? { + // sec 5.8: hand an accepted correspondent the token for our own mailbox — + // once. Re-attaching it to every later message put a secret in the body of + // ordinary mail, and a recipient that republishes what it receives cannot + // carry one: a published record is verified by rebuilding the exact signed + // bytes, and the token must not be among them. + let deliver_to = store.token_to_deliver(&addr.short())?; + if let Some(identity) = deliver_to { fields.push(("Accept".into(), b32(&account.token_for(&identity)))); } let borrowed: Vec<(&str, &str)> = fields @@ -567,6 +572,12 @@ pub fn send( None }; + // Only now, with the message accepted by their server: a send that failed + // earlier leaves the token still owing, so the next attempt carries it. + if deliver_to.is_some() { + store.mark_delivered(&addr.short())?; + } + // sec 5.6: the ephemeral is gone, so keep a copy sealed to ourselves. store.store_sent(&mid, &addr.short(), &self_copy, now())?; Ok(Sent { diff --git a/core/src/store.rs b/core/src/store.rs index 284e451..c239a87 100644 --- a/core/src/store.rs +++ b/core/src/store.rs @@ -42,9 +42,16 @@ CREATE TABLE IF NOT EXISTS contacts ( -- Correspondents admitted to this mailbox's main tier (sec 5.8). The -- identity is frozen at acceptance because the token is derived from it: a -- contact's later rotation must not change the token they already hold. +-- `delivered` records that this correspondent has been handed the token, so +-- it is not re-attached to every later message (sec 5.8 says the owner +-- *delivers* each token, not that every message carries one). The token is a +-- pure function of the master and the identity frozen here, so delivery is +-- once and for all: blocking and re-accepting hands back the same value they +-- already hold. CREATE TABLE IF NOT EXISTS accepted ( address TEXT PRIMARY KEY, identity BLOB NOT NULL, - active INTEGER NOT NULL, added_at INTEGER NOT NULL); + active INTEGER NOT NULL, added_at INTEGER NOT NULL, + delivered INTEGER NOT NULL DEFAULT 0); -- Accept tokens received from correspondents, filed under the address that -- issued them: an address outlives the keys behind it, so a token keeps -- working across the issuer's rotations (sec 5.8). @@ -90,7 +97,7 @@ pub type ContactRow = (String, [u8; KEY_LEN], bool, Option); /// The store's schema version, in SQLite's `user_version`. A store written /// by a newer fumi is refused rather than misread; within a version the /// schema is stable, and a bump comes with a documented migration. -pub const SCHEMA_VERSION: i32 = 2; +pub const SCHEMA_VERSION: i32 = 3; /// The migration from schema version 1: both changes are additive, so an /// existing store is upgraded in place at open; only stores newer than this @@ -102,6 +109,17 @@ CREATE TABLE IF NOT EXISTS history ( PRIMARY KEY (address, identity)); "; +/// The migration from schema version 2, additive like the last one. +/// +/// Existing rows default to *not* delivered, so the next message to each +/// accepted correspondent carries the token exactly once more — which is what +/// every message did before this change, and therefore cannot break anything +/// that works today. Marking them delivered instead would be wrong for anyone +/// accepted but never written to, who would then never receive a token at all. +const MIGRATE_FROM_2: &str = " +ALTER TABLE accepted ADD COLUMN delivered INTEGER NOT NULL DEFAULT 0; +"; + impl Store { /// Opens the store at `path`. SQLite's own `:memory:` path works too; /// `open_in_memory` spells that case without the magic string. @@ -133,6 +151,10 @@ impl Store { db.pragma_update(None, "user_version", SCHEMA_VERSION)?; } else if version == 1 { db.execute_batch(MIGRATE_FROM_1)?; + db.execute_batch(MIGRATE_FROM_2)?; + db.pragma_update(None, "user_version", SCHEMA_VERSION)?; + } else if version == 2 { + db.execute_batch(MIGRATE_FROM_2)?; db.pragma_update(None, "user_version", SCHEMA_VERSION)?; } else if version != SCHEMA_VERSION { return Err(Error::SchemaVersion { @@ -480,6 +502,30 @@ impl Store { Ok(identity.and_then(|identity| identity.try_into().ok())) } + /// The identity to derive a token for, but only while that token still + /// needs delivering. `None` once they have it. + pub fn token_to_deliver(&self, address: &str) -> Result, Error> { + let identity: Option> = self.one( + "SELECT identity FROM accepted \ + WHERE address = ?1 AND active = 1 AND delivered = 0", + &[&address], + )?; + Ok(identity.and_then(|identity| identity.try_into().ok())) + } + + /// Records that this correspondent now holds our token, so later messages + /// carry no secret. Deliberately not undone by `block`: they keep the + /// value either way, and re-accepting restores the same one. + pub fn mark_delivered(&self, address: &str) -> Result<(), Error> { + self.db() + .execute( + "UPDATE accepted SET delivered = 1 WHERE address = ?1", + params![address], + ) + .map(|_| ())?; + Ok(()) + } + /// A token a correspondent issued us, filed under their address (sec 5.8). pub fn token_of(&self, address: &str) -> Result, Error> { let token: Option> = @@ -934,6 +980,66 @@ mod tests { ); } + #[test] + fn a_token_is_owed_until_delivered_then_never_again() { + let store = Store::open_in_memory().unwrap(); + let id = [7u8; KEY_LEN]; + store.accept("alice@example.org", &id).unwrap(); + + // Owed on acceptance. + assert_eq!(store.token_to_deliver("alice@example.org").unwrap(), Some(id)); + store.mark_delivered("alice@example.org").unwrap(); + assert_eq!(store.token_to_deliver("alice@example.org").unwrap(), None); + + // Blocking does not take the token back — they hold it either way — + // and re-accepting hands back the same value, so nothing is owed. + assert!(store.block("alice@example.org").unwrap()); + assert_eq!(store.token_to_deliver("alice@example.org").unwrap(), None); + store.accept("alice@example.org", &id).unwrap(); + assert_eq!(store.token_to_deliver("alice@example.org").unwrap(), None); + // Still accepted, so ordinary sending is unaffected. + assert_eq!(store.accepted_identity("alice@example.org").unwrap(), Some(id)); + + // A blocked correspondent is owed nothing either, delivered or not. + store.accept("bob@example.org", &[9u8; KEY_LEN]).unwrap(); + store.block("bob@example.org").unwrap(); + assert_eq!(store.token_to_deliver("bob@example.org").unwrap(), None); + + // And someone never accepted is owed nothing. + assert_eq!(store.token_to_deliver("nobody@example.org").unwrap(), None); + } + + /// A store written before `delivered` existed must open, and must treat + /// its rows as still owing: every message used to carry the token, so one + /// more cannot break anything, whereas assuming delivery would strand a + /// contact who was accepted but never written to. + #[test] + fn a_version_2_store_upgrades_and_owes_its_tokens() { + let db = rusqlite::Connection::open_in_memory().unwrap(); + db.execute_batch( + "CREATE TABLE IF NOT EXISTS accepted ( + address TEXT PRIMARY KEY, identity BLOB NOT NULL, + active INTEGER NOT NULL, added_at INTEGER NOT NULL);", + ) + .unwrap(); + db.execute( + "INSERT INTO accepted (address, identity, active, added_at) VALUES (?1, ?2, 1, 0)", + rusqlite::params!["old@example.org", &[3u8; KEY_LEN][..]], + ) + .unwrap(); + db.pragma_update(None, "user_version", 2).unwrap(); + db.execute_batch(MIGRATE_FROM_2).unwrap(); + + let owed: Option> = db + .query_row( + "SELECT identity FROM accepted WHERE delivered = 0", + [], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(owed, Some([3u8; KEY_LEN].to_vec())); + } + #[test] fn accepted_state_reads_the_row_not_a_wish() { let store = temp_store("accepted-state");