From 8592b835385c36c4c0bfecebd59a2a046c9bb960 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 8 Oct 2026 10:23:22 -0400 Subject: [PATCH] android: the device token is stored sealed under a Keystore key Family idea #5105, practice 12, as the operator chose on 2026-10-08: the token is encrypted, and Android backup stays on. The core: - Adds a TokenSeal trait in sync/state.rs, with set_sealed_link and open_token. - A sealed token is stored as "sealed:". - A plain token, stored before this change or while sealing failed, is sealed in place on its next read. - A sealed token that won't open is dropped, and the server address and cursor are kept, so the app reads as unlinked and asks to sign in again. That is what happens after Android restores the app onto another phone. - The desktop passes no seal and keeps storing the token as before. The FFI: - Exports TokenSeal as a uniffi foreign trait (seal_token / open_token, null rather than an exception). - Requires it in Inkwell's constructor, so there is no moment a token could be stored unsealed. - Routes credentials(), unlink() and store_link() through it. Kotlin: - KeystoreTokenSeal is AES-GCM under an Android Keystore key, using the SealedBox framing from Minstrel's KeystoreSessionVault (Scribe snippet #5025), with no new dependency. - SealedBoxTest checks the framing on the JVM. allowBackup stays true, and the manifest says why. An unlinked phone's notes exist only on the phone, and the backup is their one other copy. The backup carries a token nothing can open. Co-Authored-By: Claude Opus 5.5 --- android/app/src/main/AndroidManifest.xml | 8 + .../fabledsword/inkwell/InkwellApplication.kt | 2 +- .../fabledsword/inkwell/KeystoreTokenSeal.kt | 98 ++++++++++++ .../com/fabledsword/inkwell/SealedBoxTest.kt | 38 +++++ android/ffi/src/lib.rs | 113 ++++++++++---- core/src/sync/state.rs | 144 ++++++++++++++++++ 6 files changed, 376 insertions(+), 27 deletions(-) create mode 100644 android/app/src/main/java/com/fabledsword/inkwell/KeystoreTokenSeal.kt create mode 100644 android/app/src/test/java/com/fabledsword/inkwell/SealedBoxTest.kt diff --git a/android/app/src/main/AndroidManifest.xml b/android/app/src/main/AndroidManifest.xml index d3b079f..33abf1b 100644 --- a/android/app/src/main/AndroidManifest.xml +++ b/android/app/src/main/AndroidManifest.xml @@ -90,6 +90,14 @@ + IV_BYTES) { "sealed value too short" } + val cipher = Cipher.getInstance(TRANSFORMATION) + cipher.init(Cipher.DECRYPT_MODE, key, GCMParameterSpec(TAG_BITS, bytes, 0, IV_BYTES)) + cipher.updateAAD(AAD) + return String(cipher.doFinal(bytes, IV_BYTES, bytes.size - IV_BYTES), Charsets.UTF_8) + } +} diff --git a/android/app/src/test/java/com/fabledsword/inkwell/SealedBoxTest.kt b/android/app/src/test/java/com/fabledsword/inkwell/SealedBoxTest.kt new file mode 100644 index 0000000..9d22126 --- /dev/null +++ b/android/app/src/test/java/com/fabledsword/inkwell/SealedBoxTest.kt @@ -0,0 +1,38 @@ +package com.fabledsword.inkwell + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertThrows +import org.junit.Test +import javax.crypto.KeyGenerator +import javax.crypto.SecretKey + +class SealedBoxTest { + private fun aesKey(): SecretKey = KeyGenerator.getInstance("AES").apply { init(256) }.generateKey() + + @Test + fun `a sealed token opens with the key that sealed it`() { + val key = aesKey() + val sealed = SealedBox.seal(key, "tok-1") + assertNotEquals("tok-1", sealed) + assertEquals("tok-1", SealedBox.open(key, sealed)) + } + + @Test + fun `sealing twice gives two different values`() { + // A fresh IV each time, so equal tokens can't be spotted in the file. + val key = aesKey() + assertNotEquals(SealedBox.seal(key, "tok-1"), SealedBox.seal(key, "tok-1")) + } + + @Test + fun `another key can't open it, as on a phone the backup was restored to`() { + val sealed = SealedBox.seal(aesKey(), "tok-1") + assertThrows(Exception::class.java) { SealedBox.open(aesKey(), sealed) } + } + + @Test + fun `a value that was never sealed is refused`() { + assertThrows(Exception::class.java) { SealedBox.open(aesKey(), "dG9vIHNob3J0") } + } +} diff --git a/android/ffi/src/lib.rs b/android/ffi/src/lib.rs index 7ee734e..e7a2aad 100644 --- a/android/ffi/src/lib.rs +++ b/android/ffi/src/lib.rs @@ -104,15 +104,46 @@ impl CoreError { } } +/// Seals the device token before it is stored, and opens it when it is read: Kotlin's +/// half of [`state::TokenSeal`] (family idea #5105, practice 12). +/// +/// Implemented with a key in the Android Keystore. The key never leaves the phone, +/// so a copy of the app's files carries a token nothing else can open. That includes +/// Android's own backup, which stays on because an unlinked phone's notes exist only +/// there. Restored onto another phone, the app asks to sign in again. +/// +/// Return null rather than throw: an exception escaping into the core would be a +/// panic there. +#[uniffi::export(with_foreign)] +pub trait TokenSeal: Send + Sync { + /// The token sealed for storage, or null when that isn't possible right now. + fn seal_token(&self, token: String) -> Option; + /// The sealed token opened, or null when this phone can't open it. + fn open_token(&self, sealed: String) -> Option; +} + +/// Kotlin's seal, as the core asks for one. +struct ForeignSeal(Arc); + +impl state::TokenSeal for ForeignSeal { + fn seal(&self, token: &str) -> Option { + self.0.seal_token(token.to_string()) + } + fn open(&self, sealed: &str) -> Option { + self.0.open_token(sealed.to_string()) + } +} + /// The client handle: the on-device store plus the attachment directory beside it. /// -/// Held by Kotlin for the process lifetime. Both halves are `Send + Sync` — the store -/// behind its mutex, the blob store being a path — which is what lets uniffi share -/// one instance across coroutines. +/// Held by Kotlin for the process lifetime. Every part is `Send + Sync` (the store +/// behind its mutex, the blob store being a path, the seal by its trait bound), +/// which is what lets uniffi share one instance across coroutines. #[derive(uniffi::Object)] pub struct Inkwell { db: Db, blobs: BlobStore, + seal: ForeignSeal, } #[uniffi::export] @@ -124,8 +155,11 @@ impl Inkwell { /// storage is; the core must not guess at a platform path. The layout inside is /// the core's business and matches the desktop's exactly — `inkwell.db` and /// `blobs/` — so a store is readable by any client that opens it. + /// + /// `seal` is required rather than set later, so there is no moment in which a + /// token could be stored unsealed by a caller that forgot. #[uniffi::constructor] - pub fn new(data_dir: String) -> Result, CoreError> { + pub fn new(data_dir: String, seal: Arc) -> Result, CoreError> { let dir = PathBuf::from(data_dir); std::fs::create_dir_all(&dir).map_err(CoreError::store)?; @@ -133,7 +167,11 @@ impl Inkwell { log::info!("local store ready — {}", local::summary(&db)); let blobs = BlobStore::new(dir.join("blobs")).map_err(CoreError::store)?; - Ok(Arc::new(Inkwell { db, blobs })) + Ok(Arc::new(Inkwell { + db, + blobs, + seal: ForeignSeal(seal), + })) } /// A one-line count summary, for the boot log. @@ -506,11 +544,12 @@ impl Inkwell { pub async fn unlink(&self) -> Result { // Read and release before the network call: a std MutexGuard isn't Send, so // it cannot be held across an await, and holding the store through a - // round-trip would freeze every note operation in the UI. - let link = { - let conn = self.db.conn().map_err(CoreError::store)?; - let current = state::read(&conn).map_err(CoreError::store)?; - current.server_url.zip(current.device_token) + // round-trip would freeze every note operation in the UI. A token that won't + // open on this phone can't be revoked from here, so it is skipped. + let link = match self.credentials() { + Ok(link) => Some(link), + Err(CoreError::NotLinked) => None, + Err(e) => return Err(e), }; let revoked = match &link { Some((base_url, token)) => client::revoke_self(base_url, token).await, @@ -699,12 +738,18 @@ pub fn body_tags(body: String) -> Vec { impl Inkwell { /// The server URL + token, or the `NotLinked` state. Every networked call needs /// exactly this, and none of them may hold the lock past it. + /// + /// `NotLinked` also when the stored token won't open on this phone, which drops + /// it (`state::open_token`), so the app asks to sign in again. fn credentials(&self) -> Result<(String, String), CoreError> { let conn = self.db.conn().map_err(CoreError::store)?; let current = state::read(&conn).map_err(CoreError::store)?; - match (current.server_url, current.device_token) { - (Some(url), Some(token)) => Ok((url, token)), - _ => Err(CoreError::NotLinked), + let (Some(url), Some(stored)) = (current.server_url, current.device_token) else { + return Err(CoreError::NotLinked); + }; + match state::open_token(&conn, &stored, &self.seal).map_err(CoreError::store)? { + Some(token) => Ok((url, token)), + None => Err(CoreError::NotLinked), } } @@ -718,7 +763,7 @@ impl Inkwell { retention_days: Option, ) -> Result<(), CoreError> { let conn = self.db.conn().map_err(CoreError::store)?; - state::set_link(&conn, base_url, token).map_err(CoreError::store)?; + state::set_sealed_link(&conn, base_url, token, &self.seal).map_err(CoreError::store)?; if let Some(days) = retention_days { state::set_server_retention(&conn, days as i64).map_err(CoreError::store)?; } @@ -736,6 +781,22 @@ mod tests { /// Process id + a counter rather than a uuid dependency: the FFI crate has no /// business pulling one in to name a temp folder, and this is the same approach /// the desktop's updater tests settled on. + /// A seal that can't seal: tokens are stored as they are, as on the desktop. + struct Plain; + + impl TokenSeal for Plain { + fn seal_token(&self, _token: String) -> Option { + None + } + fn open_token(&self, _sealed: String) -> Option { + None + } + } + + fn plain() -> Arc { + Arc::new(Plain) + } + fn scratch_dir() -> String { use std::sync::atomic::{AtomicU32, Ordering}; static NEXT: AtomicU32 = AtomicU32::new(0); @@ -761,7 +822,7 @@ mod tests { #[test] fn creates_a_store_and_round_trips_a_note() { let dir = scratch_dir(); - let app = Inkwell::new(dir.clone()).expect("a fresh data dir should open"); + let app = Inkwell::new(dir.clone(), plain()).expect("a fresh data dir should open"); let created = app .create_note(draft("Groceries\nmilk")) @@ -783,7 +844,7 @@ mod tests { #[test] fn a_note_is_named_by_its_first_line() { let dir = scratch_dir(); - let app = Inkwell::new(dir.clone()).expect("a fresh data dir should open"); + let app = Inkwell::new(dir.clone(), plain()).expect("a fresh data dir should open"); let created = app .create_note(draft("just a thought")) @@ -798,7 +859,7 @@ mod tests { #[test] fn a_note_with_only_items_is_named_by_its_first_item() { let dir = scratch_dir(); - let app = Inkwell::new(dir.clone()).expect("a fresh data dir should open"); + let app = Inkwell::new(dir.clone(), plain()).expect("a fresh data dir should open"); let created = app .create_note(NoteDraft { @@ -817,7 +878,7 @@ mod tests { #[test] fn syncing_unlinked_reports_not_linked() { let dir = scratch_dir(); - let app = Inkwell::new(dir.clone()).expect("a fresh data dir should open"); + let app = Inkwell::new(dir.clone(), plain()).expect("a fresh data dir should open"); let status = app.sync_status().expect("status should read"); assert!(!status.linked); @@ -833,7 +894,7 @@ mod tests { #[test] fn ticking_an_item_rewrites_only_its_box() { let dir = scratch_dir(); - let app = Inkwell::new(dir.clone()).expect("a fresh data dir should open"); + let app = Inkwell::new(dir.clone(), plain()).expect("a fresh data dir should open"); let note = app .create_note(NoteDraft { body: "Packing".to_string(), @@ -865,7 +926,7 @@ mod tests { #[test] fn setting_labels_leaves_tag_derived_ones_alone() { let dir = scratch_dir(); - let app = Inkwell::new(dir.clone()).expect("a fresh data dir should open"); + let app = Inkwell::new(dir.clone(), plain()).expect("a fresh data dir should open"); let note = app .create_note(draft("Trip\nbook the ferry #travel")) @@ -906,7 +967,7 @@ mod tests { #[test] fn deleting_forever_removes_the_note() { let dir = scratch_dir(); - let app = Inkwell::new(dir.clone()).expect("a fresh data dir should open"); + let app = Inkwell::new(dir.clone(), plain()).expect("a fresh data dir should open"); let note = app.create_note(draft("Ephemeral\nbody")).expect("create"); app.delete_note_forever(note.id.clone()) @@ -924,7 +985,7 @@ mod tests { #[test] fn an_attached_file_is_stored_found_and_removable() { let dir = scratch_dir(); - let app = Inkwell::new(dir.clone()).expect("a fresh data dir should open"); + let app = Inkwell::new(dir.clone(), plain()).expect("a fresh data dir should open"); let note = app.create_note(draft("Receipt")).expect("create"); let attached = app @@ -973,7 +1034,7 @@ mod tests { #[test] fn reminders_can_be_snoozed_and_completed() { let dir = scratch_dir(); - let app = Inkwell::new(dir.clone()).expect("a fresh data dir should open"); + let app = Inkwell::new(dir.clone(), plain()).expect("a fresh data dir should open"); let note = app.create_note(draft("Call back")).expect("create"); assert_eq!(note.remind_at, None); @@ -999,7 +1060,7 @@ mod tests { #[test] fn completing_a_recurring_reminder_moves_it_rather_than_ending_it() { let dir = scratch_dir(); - let app = Inkwell::new(dir.clone()).expect("a fresh data dir should open"); + let app = Inkwell::new(dir.clone(), plain()).expect("a fresh data dir should open"); let note = app.create_note(draft("Water the plants")).expect("create"); let armed = app @@ -1061,7 +1122,7 @@ mod tests { #[test] fn renaming_onto_an_existing_tag_merges_into_the_older_one() { let dir = scratch_dir(); - let app = Inkwell::new(dir.clone()).expect("a fresh data dir should open"); + let app = Inkwell::new(dir.clone(), plain()).expect("a fresh data dir should open"); let older = app.create_label("grocery".to_string()).expect("older"); // `created_at` is RFC3339 to the MILLISECOND. Without a gap the two rows can @@ -1106,7 +1167,7 @@ mod tests { #[test] fn the_rename_merge_survivor_does_not_depend_on_the_direction() { let dir = scratch_dir(); - let app = Inkwell::new(dir.clone()).expect("a fresh data dir should open"); + let app = Inkwell::new(dir.clone(), plain()).expect("a fresh data dir should open"); let older = app.create_label("grocery".to_string()).expect("older"); std::thread::sleep(std::time::Duration::from_millis(5)); diff --git a/core/src/sync/state.rs b/core/src/sync/state.rs index 47db739..05e7bc0 100644 --- a/core/src/sync/state.rs +++ b/core/src/sync/state.rs @@ -8,6 +8,10 @@ //! the `keyring` crate needs libsecret/DBus on Linux, which adds a C dependency to a //! binary that has to cross-compile, and fails outright on headless or minimal-WM //! setups. Protecting the database file is the portable trade. +//! +//! A client that has somewhere better to keep a key passes a [`TokenSeal`], and the +//! token is stored sealed. Android does, with a key in the Keystore (family idea +//! #5105, practice 12); the desktop does not. use rusqlite::{params, Connection}; use serde::Serialize; @@ -107,6 +111,79 @@ pub fn set_link(conn: &Connection, server_url: &str, device_token: &str) -> rusq Ok(()) } +/// Seals the device token for storage, and opens it again. +/// +/// Neither method fails loudly: a client's key store can be briefly unavailable, and +/// a token sealed on another device can never be opened here. `None` says so, and +/// [`set_sealed_link`] and [`open_token`] decide what happens next. +pub trait TokenSeal { + /// The token sealed for storage, or None when that isn't possible right now. + fn seal(&self, token: &str) -> Option; + /// The sealed token opened, or None when this device can't open it. + fn open(&self, sealed: &str) -> Option; +} + +/// Marks a stored token as sealed. A device token is URL-safe base64, which has no +/// colon, so a plain one can't be mistaken for a sealed one. +const SEALED: &str = "sealed:"; + +/// [`set_link`] for a client that seals its token. +/// +/// When sealing fails, the token is stored as it is and [`open_token`] seals it on +/// its next read. Refusing the link instead would leave someone unable to sync +/// over a passing key-store error. +pub fn set_sealed_link( + conn: &Connection, + server_url: &str, + device_token: &str, + seal: &dyn TokenSeal, +) -> rusqlite::Result<()> { + let stored = match seal.seal(device_token) { + Some(sealed) => format!("{SEALED}{sealed}"), + None => { + log::warn!("couldn't seal the device token; it is stored as is until the next read"); + device_token.to_string() + } + }; + set_link(conn, server_url, &stored) +} + +/// The device token the server expects, from the `stored` one. +/// +/// - A sealed token is opened. If it won't open, it is dropped, so the app reads as +/// unlinked and asks to sign in again. That is what happens when Android restores +/// the app's files onto another phone, whose Keystore never held the key. The +/// server address and cursor stay, for the sign-in form and the same server. +/// - A plain token, stored before tokens were sealed or when sealing failed, is +/// sealed in place and returned. +pub fn open_token( + conn: &Connection, + stored: &str, + seal: &dyn TokenSeal, +) -> rusqlite::Result> { + if let Some(sealed) = stored.strip_prefix(SEALED) { + let token = seal.open(sealed); + if token.is_none() { + log::warn!("the stored device token won't open on this device; sign in again"); + store_token(conn, None)?; + } + return Ok(token); + } + if let Some(sealed) = seal.seal(stored) { + store_token(conn, Some(&format!("{SEALED}{sealed}")))?; + } + Ok(Some(stored.to_string())) +} + +/// Replace the stored token alone, leaving the server and cursor as they are. +fn store_token(conn: &Connection, token: Option<&str>) -> rusqlite::Result<()> { + conn.execute( + "UPDATE sync_state SET device_token = ?1 WHERE id = 1", + params![token], + )?; + Ok(()) +} + /// Drop the notes other people shared with the account this device was linked to. /// They were only ever here through that link: kept after it ends, they would sit /// on the board as notes nobody here can edit and nothing would ever update. @@ -240,6 +317,73 @@ mod tests { assert_eq!(state.device_token.as_deref(), Some("tok-1")); } + /// Reverses the token: enough to tell sealed from plain. `broken` stands in for a + /// key store that is unavailable, or a key this device never held. + struct Reverse { + broken: bool, + } + + impl TokenSeal for Reverse { + fn seal(&self, token: &str) -> Option { + (!self.broken).then(|| token.chars().rev().collect()) + } + fn open(&self, sealed: &str) -> Option { + (!self.broken).then(|| sealed.chars().rev().collect()) + } + } + + const WORKING: Reverse = Reverse { broken: false }; + const BROKEN: Reverse = Reverse { broken: true }; + + fn stored_token(conn: &Connection) -> Option { + read(conn).expect("read").device_token + } + + #[test] + fn a_sealed_token_is_stored_sealed_and_opens() { + let conn = db(); + set_sealed_link(&conn, "https://notes.example.com", "tok-1", &WORKING).expect("link"); + assert_eq!(stored_token(&conn).as_deref(), Some("sealed:1-kot")); + + let opened = open_token(&conn, "sealed:1-kot", &WORKING).expect("open"); + assert_eq!(opened.as_deref(), Some("tok-1")); + } + + #[test] + fn a_plain_token_is_sealed_on_its_next_read() { + let conn = db(); + set_link(&conn, "https://notes.example.com", "tok-1").expect("link"); + + let opened = open_token(&conn, "tok-1", &WORKING).expect("open"); + assert_eq!(opened.as_deref(), Some("tok-1")); + assert_eq!(stored_token(&conn).as_deref(), Some("sealed:1-kot")); + } + + #[test] + fn a_token_that_cant_be_sealed_is_kept_plain_and_still_works() { + let conn = db(); + set_sealed_link(&conn, "https://notes.example.com", "tok-1", &BROKEN).expect("link"); + assert_eq!(stored_token(&conn).as_deref(), Some("tok-1")); + let opened = open_token(&conn, "tok-1", &BROKEN).expect("open"); + assert_eq!(opened.as_deref(), Some("tok-1")); + } + + #[test] + fn a_token_that_wont_open_here_is_dropped_but_the_server_is_kept() { + let conn = db(); + set_sealed_link(&conn, "https://notes.example.com", "tok-1", &WORKING).expect("link"); + + let opened = open_token(&conn, "sealed:1-kot", &BROKEN).expect("open"); + assert_eq!(opened, None); + let state = read(&conn).expect("read"); + assert!(!state.is_linked()); + assert_eq!(state.device_token, None); + assert_eq!( + state.server_url.as_deref(), + Some("https://notes.example.com") + ); + } + #[test] fn an_unlinked_device_uses_its_own_retention_window() { let conn = db();