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();