fix: deliver an accept token once, not on every message

send attached the token to every message addressed to an accepted
correspondent, so a secret rode in the body of ordinary mail indefinitely.
sec 5.8 says the owner delivers each token to its correspondent; it does not
say every message carries one.

Harmless between people, and fatal for any recipient that republishes what it
receives: a published record is verified by rebuilding the exact bytes the
author signed, and those cannot include a token without disclosing it. An
author who accepted such a service could never post to it again — and
accepting a correspondent is the behaviour the rest of the system teaches.

Schema 3 adds `delivered`, set only once the recipient's server has taken the
message, so a failed send leaves the token still owing. Not cleared by block:
the token is a pure function of the master and the identity frozen at
acceptance, so they keep the value either way and re-accepting hands back the
same one.

Existing rows migrate as still owing rather than delivered. One more message
carries the token, which is what every message did before, so nothing that
works today can break; assuming delivery would instead strand anyone accepted
but never written to.

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 {

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<i64>);
 /// 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<Option<[u8; KEY_LEN]>, Error> {
+        let identity: Option<Vec<u8>> = 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<Option<[u8; 32]>, Error> {
         let token: Option<Vec<u8>> =
@@ -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<Vec<u8>> = 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");