From ae7d3c750ed83c329092d60cef3c1eb784224614 Mon Sep 17 00:00:00 2001 From: otsmr Date: Tue, 1 Sep 2026 15:51:27 +0200 Subject: [PATCH] improve startup --- lib/locator.dart | 4 + lib/main.dart | 333 ++++++++++++++-------- lib/src/utils/exclusive_access.utils.dart | 29 +- rust/Cargo.toml | 17 ++ rust/cargokit.yaml | 12 + rust/src/bridge/api.rs | 7 +- rust/src/context.rs | 49 ++-- rust/src/database/app/mod.rs | 5 +- rust/src/database/cipher.rs | 183 ++++++++++++ rust/src/database/mod.rs | 1 + rust/src/database/signal/mod.rs | 6 +- rust/tests/database_cipher_migration.rs | 269 +++++++++++++++++ 12 files changed, 775 insertions(+), 140 deletions(-) create mode 100644 rust/cargokit.yaml create mode 100644 rust/src/database/cipher.rs create mode 100644 rust/tests/database_cipher_migration.rs diff --git a/lib/locator.dart b/lib/locator.dart index b91512c5..c2f7d8c7 100644 --- a/lib/locator.dart +++ b/lib/locator.dart @@ -10,6 +10,10 @@ export 'package:twonly/src/services/api/rust_api_result.dart'; final GetIt locator = GetIt.instance; void setupLocator() { + // Called both from `main` (before the first frame, so the theme can be read) + // and from `twonlyMinimumInitialization`, which background entry points use + // on their own. Registering twice throws, so the second call is a no-op. + if (locator.isRegistered()) return; locator ..registerLazySingleton(UserService.new) ..registerLazySingleton(ApiService.new) diff --git a/lib/main.dart b/lib/main.dart index d0ea2628..8a659648 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -27,9 +27,9 @@ import 'package:twonly/src/services/migrations.service.dart'; import 'package:twonly/src/services/notifications/fcm.notifications.dart'; import 'package:twonly/src/services/notifications/native.notifications.dart'; import 'package:twonly/src/services/notifications/setup.notifications.dart'; -import 'package:twonly/src/utils/exclusive_access.utils.dart'; import 'package:twonly/src/utils/log.dart'; import 'package:twonly/src/utils/startup_guard.dart'; +import 'package:twonly/src/visual/themes/light.dart'; final _initMutex = Mutex(); @@ -37,134 +37,82 @@ final _initMutex = Mutex(); /// can also be used by the backend without the UI was loaded. Future twonlyMinimumInitialization() async { Log.info('twonlyMinimumInitialization: called'); - final hasStorageError = await exclusiveAccess( - lockName: 'init', - mutex: _initMutex, - action: () async { - Log.info('twonlyMinimumInitialization started'); - setupLocator(); + final hasStorageError = await _initMutex.protect(() async { + Log.info('twonlyMinimumInitialization started'); + setupLocator(); - await RustLib.init(); + await RustLib.init(); - await initFlutterCallbacksForRust(); + await initFlutterCallbacksForRust(); - try { - await bridge.initializeTwonlyFlutter( - config: bridge.InitConfig( - databaseDir: AppEnvironment.supportDir, - dataDir: AppEnvironment.supportDir, - ), + try { + await bridge.initializeTwonlyFlutter( + config: bridge.InitConfig( + databaseDir: AppEnvironment.supportDir, + dataDir: AppEnvironment.supportDir, + ), + ); + Log.enableRustSink(); + if (!await RustAppDatabase.legacyImportComplete()) { + final legacyFile = File( + '${AppEnvironment.supportDir}/twonly.sqlite', ); - Log.enableRustSink(); - if (!await RustAppDatabase.legacyImportComplete()) { - final legacyFile = File( - '${AppEnvironment.supportDir}/twonly.sqlite', - ); - if (legacyFile.existsSync()) { - final legacyDatabase = TwonlyDB(NativeDatabase(legacyFile)); - // Opening the database applies every existing Drift migration up - // to v25 before Rust copies the application tables. - await legacyDatabase - .customSelect('PRAGMA user_version') - .getSingle(); - await legacyDatabase.close(); - } - await RustAppDatabase.migrateLegacyDatabase(); + if (legacyFile.existsSync()) { + final legacyDatabase = TwonlyDB(NativeDatabase(legacyFile)); + // Opening the database applies every existing Drift migration up + // to v25 before Rust copies the application tables. + await legacyDatabase.customSelect('PRAGMA user_version').getSingle(); + await legacyDatabase.close(); } - } catch (e) { - // Tracing is initialized before the rest of the Rust context, so even - // failed initialization can persist the buffered startup diagnostics. - Log.enableRustSink(); - Log.error(e); - return true; + await RustAppDatabase.migrateLegacyDatabase(); } - Log.info('twonlyMinimumInitialization: finished'); - return false; - }, - ); + } catch (e) { + // Tracing is initialized before the rest of the Rust context, so even + // failed initialization can persist the buffered startup diagnostics. + Log.enableRustSink(); + Log.error(e); + return true; + } + Log.info('twonlyMinimumInitialization: finished'); + return false; + }); return hasStorageError; } -void main() async { - final binding = SentryWidgetsFlutterBinding.ensureInitialized(); - await AppEnvironment.init(); +/// What the UI needs to know before it can pick a route. +class StartupResult { + const StartupResult({ + required this.storageError, + required this.recoveryPossible, + }); + final bool storageError; + final bool recoveryPossible; +} - // Preload available cameras in the background to speed up camera tab startup - unawaited( - availableCameras().then((cameras) { - AppEnvironment.cameras = cameras; - }), - ); +void main() { + SentryWidgetsFlutterBinding.ensureInitialized(); - final stopwatch = Stopwatch()..start(); - - unawaited(StartupGuard.markAppStartup()); - - var storageError = await twonlyMinimumInitialization(); - await FcmNotificationService.initStartup(); - await setupPushNotification(); - NativeNotificationService.init(); - - var userExists = false; - - var recoveryPossible = false; - - if (!storageError) { - try { - userExists = await userService.tryInit(); - } catch (e) { - Log.error('Failed to initialize user session due to storage error: $e'); - storageError = true; - } - } - - if (!userExists && !storageError) { - try { - final userId = await RustKeyManager.getUserId(); - if (userId != null) { - recoveryPossible = true; - } - } catch (e) { - Log.error('Could not check KeyManager userId for iOS recovery: $e'); - } - } - - Log.info('User loaded.'); + // `App` used to set this as it mounted, which was at the same moment the + // engine started. It now mounts only once startup finishes, and the API can + // authenticate before that — `ApiService.onAuthenticated` skips its + // foreground work while this is true, so it has to be correct from the start. + AppState.isAppInBackground = false; + // Registering the services is a few map insertions and needs nothing from + // storage, but it has to happen before the settings provider can read the + // user's theme. + setupLocator(); final settingsController = SettingsChangeProvider()..loadSettings(); - await SystemChrome.setPreferredOrientations([DeviceOrientation.portraitUp]); - if (userExists) { - unawaited(FcmNotificationService.initAfterUserLoaded()); - - if (userService.currentUser.allowErrorTrackingViaSentry) { - AppState.allowErrorTrackingViaSentry = true; - await SentryFlutter.init( - (options) => options - ..dsn = - 'https://6b24a012c85144c9b522440a1d17d01c@glitchtip.twonly.eu/4' - ..tracesSampleRate = 0.1 - ..enableAutoSessionTracking = false, - ); - } - - await runMigrations(); - // We wait for the first frame to be rendered before starting heavy tasks. - // This ensures the splash screen is dismissed on Android immediately. - binding.addPostFrameCallback((_) async { - await Future.delayed(const Duration(seconds: 1)); - unawaited(postStartupTasks()); - }); - } - - await apiService.listenToNetworkChanges(); - - stopwatch.stop(); - - Log.info( - 'Initialization finished after ${stopwatch.elapsed}. Calling runApp...', + // A platform call that nothing below depends on. + unawaited( + SystemChrome.setPreferredOrientations([DeviceOrientation.portraitUp]), ); + // Storage, Rust, Firebase and the network are all reached from `_startup`, + // and the app renders a splash until it completes. Nothing here is awaited, + // so the first frame is drawn while that work is still running instead of + // after it — the native splash used to stay up for the whole chain. runApp( MultiProvider( providers: [ @@ -173,14 +121,165 @@ void main() async { ChangeNotifierProvider(create: (_) => ImageEditorProvider()), ChangeNotifierProvider(create: (_) => PurchasesProvider()), ], - child: App( - storageError: storageError, - recoveryPossible: recoveryPossible, - ), + child: TwonlyBootstrap(startup: _startup(settingsController)), ), ); } +/// Everything the app cannot run without, off the first-frame path. +/// +/// Never completes with an error: a failure here has to reach the UI as a +/// storage error, because a rejected future would leave the splash up forever. +Future _startup(SettingsChangeProvider settings) async { + final stopwatch = Stopwatch()..start(); + try { + await AppEnvironment.init(); + + // Preload available cameras in the background to speed up camera tab startup + unawaited( + availableCameras().then((cameras) { + AppEnvironment.cameras = cameras; + }), + ); + unawaited(StartupGuard.markAppStartup()); + + // Firebase and the local notification plugin are platform-side setup that + // touches neither storage nor Rust, so they run alongside the Rust + // initialization rather than queueing behind it. Both are awaited before + // this returns, because the home view subscribes to them as it mounts. + // Failing to set up notifications must not stop the rest of the app. + final notificationSetup = + Future.wait([ + FcmNotificationService.initStartup(), + setupPushNotification(), + ]).then((_) {}).catchError((Object error, StackTrace stackTrace) { + Log.error( + 'Notification setup failed', + error: error, + stackTrace: stackTrace, + ); + }); + + var storageError = await twonlyMinimumInitialization(); + + var userExists = false; + var recoveryPossible = false; + + if (!storageError) { + try { + userExists = await userService.tryInit(); + } catch (e) { + Log.error('Failed to initialize user session due to storage error: $e'); + storageError = true; + } + } + + if (!userExists && !storageError) { + try { + final userId = await RustKeyManager.getUserId(); + if (userId != null) { + recoveryPossible = true; + } + } catch (e) { + Log.error('Could not check KeyManager userId for iOS recovery: $e'); + } + } + + Log.info('User loaded.'); + + await notificationSetup; + NativeNotificationService.init(); + + // The theme was read before the user was known, so re-read it now that it + // is. Repainting the splash costs nothing; the app UI is built after this. + settings.loadSettings(); + + if (userExists) { + unawaited(FcmNotificationService.initAfterUserLoaded()); + + if (userService.currentUser.allowErrorTrackingViaSentry) { + AppState.allowErrorTrackingViaSentry = true; + // Not awaited: error reporting does not have to be live before the + // first screen is. + unawaited( + SentryFlutter.init( + (options) => options + ..dsn = + 'https://6b24a012c85144c9b522440a1d17d01c@glitchtip.twonly.eu/4' + ..tracesSampleRate = 0.1 + ..enableAutoSessionTracking = false, + ), + ); + } + + // Data migrations have to finish before any screen reads the database. + await runMigrations(); + + // Heavy background work waits for the app to settle rather than + // competing with the first frames of the real UI. + unawaited( + Future.delayed( + const Duration(seconds: 1), + ).then((_) => postStartupTasks()), + ); + } + + unawaited(apiService.listenToNetworkChanges()); + + stopwatch.stop(); + Log.info('Startup finished after ${stopwatch.elapsed}.'); + + return StartupResult( + storageError: storageError, + recoveryPossible: recoveryPossible, + ); + } catch (error, stackTrace) { + Log.error('Startup failed', error: error, stackTrace: stackTrace); + return const StartupResult(storageError: true, recoveryPossible: false); + } +} + +/// Shows the splash until [startup] resolves, then hands over to [App]. +class TwonlyBootstrap extends StatelessWidget { + const TwonlyBootstrap({required this.startup, super.key}); + + final Future startup; + + @override + Widget build(BuildContext context) { + return FutureBuilder( + future: startup, + builder: (context, snapshot) { + final result = snapshot.data; + if (result == null) return const _SplashView(); + return App( + storageError: result.storageError, + recoveryPossible: result.recoveryPossible, + ); + }, + ); + } +} + +/// The first frame. Painted in the same color as the Android launch theme's +/// `windowSplashScreenBackground` and the iOS launch screen, so handing over +/// from the OS splash is not visible. +class _SplashView extends StatelessWidget { + const _SplashView(); + + @override + Widget build(BuildContext context) { + return const MaterialApp( + debugShowCheckedModeBanner: false, + title: 'twonly', + home: ColoredBox( + color: defaultPrimaryColor, + child: SizedBox.expand(), + ), + ); + } +} + Future postStartupTasks() async { Log.info('Post startup started.'); unawaited(MemoriesService.prewarmCache()); diff --git a/lib/src/utils/exclusive_access.utils.dart b/lib/src/utils/exclusive_access.utils.dart index f39ce62e..5c661e23 100644 --- a/lib/src/utils/exclusive_access.utils.dart +++ b/lib/src/utils/exclusive_access.utils.dart @@ -3,6 +3,14 @@ import 'dart:io'; import 'package:mutex/mutex.dart'; import 'package:twonly/globals.dart'; +/// A lock file whose timestamp has not moved for this long is assumed to belong +/// to a process that is gone. The holder refreshes it while it works, so this +/// only ever breaks an abandoned lock — never a slow one. +const _staleAfter = Duration(seconds: 3); + +/// How often the holder proves it is still alive. +const _heartbeat = Duration(seconds: 1); + Future exclusiveAccess({ required String lockName, required Future Function() action, @@ -24,8 +32,11 @@ Future exclusiveAccess({ try { final stat = lockFile.statSync(); if (stat.type != FileSystemEntityType.notFound) { - final age = DateTime.now().difference(stat.modified).inSeconds; - if (age > 10) { + // A process killed mid-initialization leaves its lock behind. The + // holder keeps the timestamp fresh, so a stale one means nobody is + // working on it any more. Everything the lock guards is idempotent + // by itself, so breaking an abandoned lock is safe. + if (DateTime.now().difference(stat.modified) > _staleAfter) { lockFile.deleteSync(); continue; } @@ -36,9 +47,23 @@ Future exclusiveAccess({ break; } } + + // Keep the lock visibly alive for as long as the work takes. Without this, + // an initialization slower than `_staleAfter` — the legacy database import, + // for one — would have its lock broken by the next waiter. + Timer? keepAlive; + if (lockAcquired) { + keepAlive = Timer.periodic(_heartbeat, (_) { + try { + lockFile.setLastModifiedSync(DateTime.now()); + } catch (_) {} + }); + } + try { return await action(); } finally { + keepAlive?.cancel(); if (lockAcquired) { try { if (lockFile.existsSync()) { diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 4cc03848..96c76532 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -9,6 +9,14 @@ crate-type = ["cdylib", "staticlib", "rlib"] [lints.rust] unexpected_cfgs = { level = "warn", check-cfg = ['cfg(frb_expand)'] } +[features] +default = [] +# Points every API URL at the development server. Flutter profile builds +# compile with `--release`, so `debug_assertions` cannot distinguish them from +# a production release; cargokit enables this feature for the profile +# configuration instead (see `cargokit.yaml`). +dev-api = [] + [dependencies] flutter_rust_bridge = { version = "=2.12.0", features = ["chrono"] } thiserror = "2.0.18" @@ -87,3 +95,12 @@ tempfile = "3.27.0" [build-dependencies] prost-build = "0.14.1" + +[profile.release] +# The default release profile leaves LTO off, which for a crate this size means +# a bigger `.so` for the dynamic linker to map and relocate before the first +# Rust call. `thin` keeps build times reasonable; `panic = "abort"` is +# deliberately absent because flutter_rust_bridge turns Rust panics into Dart +# exceptions by unwinding, and stripping is left to the platform builds so +# native crash reports keep their symbols. +lto = "thin" diff --git a/rust/cargokit.yaml b/rust/cargokit.yaml new file mode 100644 index 00000000..d8626beb --- /dev/null +++ b/rust/cargokit.yaml @@ -0,0 +1,12 @@ +# Read by cargokit (rust_builder/cargokit) when it invokes cargo. +# +# Flutter's profile configuration builds Rust with `--release`, which turns off +# `debug_assertions`. Without this, a profile build would be indistinguishable +# from a store build and would talk to the production API. Enabling `dev-api` +# here keeps profile builds pointed at dev-api.twonly.eu; only `release` (debug +# assertions off, feature off) reaches the production server. +cargo: + profile: + extra_flags: + - --features + - dev-api diff --git a/rust/src/bridge/api.rs b/rust/src/bridge/api.rs index 9e9d4ede..66a796f6 100644 --- a/rust/src/bridge/api.rs +++ b/rust/src/bridge/api.rs @@ -372,7 +372,12 @@ impl RustApi { #[frb(sync)] pub fn api_base_url(protocol: String) -> String { - if cfg!(debug_assertions) { + // Every non-production build talks to the development server. A Flutter + // profile build compiles Rust with `--release`, so `debug_assertions` + // is off and cannot tell it apart from a store build; cargokit enables + // the `dev-api` feature for that configuration instead (see + // `rust/cargokit.yaml`). + if cfg!(debug_assertions) || cfg!(feature = "dev-api") { format!("{}://dev-api.twonly.eu/api/", protocol) } else { format!("{}://api.twonly.eu/api/", protocol) diff --git a/rust/src/context.rs b/rust/src/context.rs index 5af7fc55..bc1d4f01 100644 --- a/rust/src/context.rs +++ b/rust/src/context.rs @@ -218,6 +218,10 @@ impl Context { return Ok(()); } + // Only the foreground app re-encrypts a database still on the old + // SQLCipher key; see `database::cipher`. + crate::database::cipher::set_migration_enabled(runtime_mode == RuntimeMode::Flutter); + SecureStorage::init()?; let secure_storage = SecureStorage::new("eu.twonly"); @@ -244,25 +248,34 @@ impl Context { }; let mut rust_db_key = key_manager.main_key.get_database_key(DatabaseKey::RustDb); - - let rust_db = Database::new( - &rust_db_path.display().to_string(), - Some(rust_db_key.as_str()), - false, - ) - .await?; - rust_db.run_migrations().await?; - let rust_db = Arc::new(rust_db); - let rust_db_handle = Arc::new(RwLock::new(rust_db)); - let mut app_db_key = key_manager.main_key.get_database_key(DatabaseKey::AppDb); - let app_db = AppDatabase::new( - &app_db_path.display().to_string(), - Some(app_db_key.as_str()), - false, - ) - .await?; - app_db.run_migrations().await?; + + // The two files are independent, and opening one is mostly + // waiting on the filesystem, so they are opened concurrently + // rather than one after the other. + let (rust_db, app_db) = tokio::try_join!( + async { + let database = Database::new( + &rust_db_path.display().to_string(), + Some(rust_db_key.as_str()), + false, + ) + .await?; + database.run_migrations().await?; + Ok::<_, TwonlyError>(database) + }, + async { + let database = AppDatabase::new( + &app_db_path.display().to_string(), + Some(app_db_key.as_str()), + false, + ) + .await?; + database.run_migrations().await?; + Ok::<_, TwonlyError>(database) + }, + )?; + let rust_db_handle = Arc::new(RwLock::new(Arc::new(rust_db))); let app_db = Arc::new(RwLock::new(Arc::new(app_db))); app_db_key.zeroize(); rust_db_key.zeroize(); diff --git a/rust/src/database/app/mod.rs b/rust/src/database/app/mod.rs index beebbb20..4a6e4c7e 100644 --- a/rust/src/database/app/mod.rs +++ b/rust/src/database/app/mod.rs @@ -70,7 +70,10 @@ impl AppDatabase { .log_statements(tracing::log::LevelFilter::Off) .log_slow_statements(tracing::log::LevelFilter::Warn, Duration::from_millis(500)); if let Some(key) = encryption_key { - options = options.pragma("key", format!("'{key}'")); + // Migrates a database still encrypted with the old passphrase-derived + // key before the pool opens it. See `database::cipher`. + let key_pragma = crate::database::cipher::key_pragma(db_path, key, read_only).await?; + options = options.pragma("key", key_pragma); } let (changes, _) = broadcast::channel(256); diff --git a/rust/src/database/cipher.rs b/rust/src/database/cipher.rs new file mode 100644 index 00000000..34b5bcd8 --- /dev/null +++ b/rust/src/database/cipher.rs @@ -0,0 +1,183 @@ +/* + * Copyright (c) 2026, Tobias Müller git@tsmr.eu + * + */ + +//! How the SQLCipher key is handed to `PRAGMA key`, and the one-time migration +//! between the two forms. +//! +//! Both database keys are 32-byte HKDF outputs of the main key +//! (`MainKey::get_database_key`), hex encoded. They already have full entropy, +//! so nothing is gained by stretching them — but passing them to `PRAGMA key` +//! as a *passphrase*, which is what earlier versions did, makes SQLCipher run +//! PBKDF2-HMAC-SHA512 over 256000 iterations before it can touch the file. +//! That costs roughly 45 ms per connection on a desktop and several times that +//! on a phone, and it is paid by every connection a pool ever opens, not just +//! the first. +//! +//! SQLCipher's raw key form (`x''`) uses the bytes as the AES key +//! directly and skips the derivation entirely. The two forms produce different +//! encryption keys, so a database written by an older version has to be +//! re-encrypted once. [`prepare`] does that transparently on the first +//! read-write open and is a no-op afterwards. + +use crate::error::Result; +use sqlx::sqlite::SqliteConnectOptions; +use sqlx::{AssertSqlSafe, ConnectOptions, Connection, Executor, SqliteConnection}; +use std::path::Path; +use std::str::FromStr; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::time::Duration; + +/// Whether this process may re-encrypt a database it finds on the old key. +/// +/// Only the foreground app does. The iOS notification service extension opens +/// the same files out of the shared app group, and it is short-lived and killed +/// aggressively by the OS — a rekey interrupted there would be retried on every +/// push instead of once, and any connection another process holds open across +/// the switch would start failing. Leaving the rewrite to the app means it +/// happens once, while the user is looking at it, and a background process +/// simply keeps opening the database on the key it already has. +static MIGRATION_ENABLED: AtomicBool = AtomicBool::new(true); + +/// Set from `Context::init_common` once the runtime mode is known. +pub(crate) fn set_migration_enabled(enabled: bool) { + MIGRATION_ENABLED.store(enabled, Ordering::Release); +} + +/// How a key string is spelled in `PRAGMA key`. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum KeyForm { + /// `x''`. The 32 bytes become the AES key with no derivation. + Raw, + /// `''`. SQLCipher stretches the text with PBKDF2-HMAC-SHA512. + Passphrase, +} + +impl KeyForm { + /// The right-hand side of `PRAGMA key = …` for this form. + pub(crate) fn pragma_value(self, key: &str) -> String { + match self { + // The double quotes are part of SQLCipher's documented raw key + // syntax: PRAGMA key = "x'2DD29CA8…'". + Self::Raw => format!("\"x'{key}'\""), + Self::Passphrase => format!("'{key}'"), + } + } +} + +/// Only a 64 character hex string is exactly the 32 bytes SQLCipher wants for a +/// raw key. Anything else (test fixtures, a user-chosen password) stays on the +/// passphrase path, where SQLCipher derives a key of the right size itself. +fn can_use_raw_key(key: &str) -> bool { + key.len() == 64 && key.bytes().all(|byte| byte.is_ascii_hexdigit()) +} + +/// A path SQLCipher will open as a real file on disk. In-memory databases are +/// created fresh every time and never need migrating. +fn is_file_backed(db_path: &str) -> bool { + !db_path.contains(":memory:") +} + +/// True when the file already holds an encrypted database. A missing or empty +/// file is about to be created, and gets the raw key from the start. +fn has_existing_database(db_path: &str) -> bool { + Path::new(db_path) + .metadata() + .is_ok_and(|metadata| metadata.is_file() && metadata.len() > 0) +} + +/// Opens a bare connection to an existing database with `key` in `form`. +async fn connect(db_path: &str, key: &str, form: KeyForm) -> Result { + let options = SqliteConnectOptions::from_str(&format!("sqlite://{db_path}"))? + .create_if_missing(false) + // Both databases run in DELETE mode. sqlx would otherwise apply its own + // WAL default here, and journal_mode is persistent — a probe must not + // leave the file in a different mode than the pool expects. + .journal_mode(sqlx::sqlite::SqliteJournalMode::Delete) + .busy_timeout(Duration::from_secs(30)) + .log_statements(tracing::log::LevelFilter::Off) + .pragma("key", form.pragma_value(key)); + Ok(options.connect().await?) +} + +/// Whether `key` in `form` actually decrypts the database. Reading the schema +/// is the cheapest statement that has to touch the (encrypted) first page. +async fn decrypts_with(db_path: &str, key: &str, form: KeyForm) -> bool { + let Ok(mut connection) = connect(db_path, key, form).await else { + return false; + }; + let readable = connection + .execute(AssertSqlSafe("SELECT count(*) FROM sqlite_master")) + .await + .is_ok(); + let _ = connection.close().await; + readable +} + +/// Re-encrypts the database in place with the raw key. SQLCipher performs the +/// rekey inside a transaction, so an interrupted run rolls back to the +/// passphrase key and [`prepare`] simply tries again on the next start. +async fn rekey_to_raw(db_path: &str, key: &str) -> Result<()> { + let mut connection = connect(db_path, key, KeyForm::Passphrase).await?; + // `key` is our own hex string, checked by `can_use_raw_key`, so there is + // nothing here that could carry SQL. + let statement = format!("PRAGMA rekey = {};", KeyForm::Raw.pragma_value(key)); + let result = connection.execute(AssertSqlSafe(statement)).await; + let closed = connection.close().await; + result?; + closed?; + Ok(()) +} + +/// Decides which key form to open `db_path` with, migrating a database still +/// encrypted with the passphrase-derived key when it can be opened read-write. +/// +/// Never fails because of a key it cannot place: if neither form decrypts the +/// file, the caller's own connection reports the real error with its own +/// context instead. +pub(crate) async fn prepare(db_path: &str, key: &str, read_only: bool) -> Result { + if !can_use_raw_key(key) || !is_file_backed(db_path) { + return Ok(KeyForm::Passphrase); + } + if !has_existing_database(db_path) { + return Ok(KeyForm::Raw); + } + if decrypts_with(db_path, key, KeyForm::Raw).await { + return Ok(KeyForm::Raw); + } + if !decrypts_with(db_path, key, KeyForm::Passphrase).await { + return Ok(KeyForm::Raw); + } + if read_only { + // Staged backup archives are verified read-only before they replace + // anything. They get migrated when they are opened for writing. + return Ok(KeyForm::Passphrase); + } + if !MIGRATION_ENABLED.load(Ordering::Acquire) { + return Ok(KeyForm::Passphrase); + } + + tracing::info!( + database = %Path::new(db_path) + .file_name() + .unwrap_or_default() + .to_string_lossy(), + "re-encrypting the database with the raw key form" + ); + rekey_to_raw(db_path, key).await?; + + if !decrypts_with(db_path, key, KeyForm::Raw).await { + // The rekey rolled back. Keep the app usable on the old key and retry + // on the next start rather than failing to open the database at all. + tracing::error!("re-encryption did not take effect; staying on the passphrase key"); + return Ok(KeyForm::Passphrase); + } + tracing::info!("database re-encrypted with the raw key form"); + Ok(KeyForm::Raw) +} + +/// Builds the `PRAGMA key` value for a database, migrating it first if needed. +pub(crate) async fn key_pragma(db_path: &str, key: &str, read_only: bool) -> Result { + Ok(prepare(db_path, key, read_only).await?.pragma_value(key)) +} diff --git a/rust/src/database/mod.rs b/rust/src/database/mod.rs index 24d7c7e6..7be97972 100644 --- a/rust/src/database/mod.rs +++ b/rust/src/database/mod.rs @@ -4,4 +4,5 @@ */ pub mod app; +pub(crate) mod cipher; pub mod signal; diff --git a/rust/src/database/signal/mod.rs b/rust/src/database/signal/mod.rs index 95bb20e6..caa06bb6 100644 --- a/rust/src/database/signal/mod.rs +++ b/rust/src/database/signal/mod.rs @@ -38,7 +38,11 @@ impl Database { .log_slow_statements(tracing::log::LevelFilter::Warn, Duration::from_millis(500)); if let Some(encryption_key) = encryption_key { - connect_options = connect_options.pragma("key", format!("'{}'", encryption_key)); + // Migrates a database still encrypted with the old passphrase-derived + // key before the pool opens it. See `database::cipher`. + let key_pragma = + crate::database::cipher::key_pragma(db_path, encryption_key, read_only).await?; + connect_options = connect_options.pragma("key", key_pragma); } let pool = SqlitePoolOptions::new() diff --git a/rust/tests/database_cipher_migration.rs b/rust/tests/database_cipher_migration.rs new file mode 100644 index 00000000..c45b99e6 --- /dev/null +++ b/rust/tests/database_cipher_migration.rs @@ -0,0 +1,269 @@ +//! An installed app has its databases encrypted with the PBKDF2-derived key +//! that older versions used. Opening one read-write now re-encrypts it with the +//! raw key form; everything in it has to survive that untouched. + +use rust_lib_twonly::database::app::AppDatabase; +use rust_lib_twonly::database::signal::Database; +use sqlx::sqlite::SqliteConnectOptions; +use sqlx::{ConnectOptions, Connection, Executor, Row}; +use std::path::Path; +use std::str::FromStr; +use std::time::Instant; + +/// A database key is always `hex::encode` of a 32-byte HKDF output, so it is +/// exactly 64 hex characters. `database::cipher` only uses the raw key form for +/// keys of that shape, so a fixture of the wrong length would silently test the +/// passphrase path instead. +const KEY: &str = "5b1f8c2d3e4a596877a8b9c0d1e2f30415263748596a7b8c9d0e1f2a3b4c5d6e"; + +/// Writes a database keyed the way every released version keyed it: the hex +/// string handed to `PRAGMA key` as a passphrase. +async fn create_legacy_database(path: &Path) { + let mut connection = SqliteConnectOptions::from_str(&format!("sqlite://{}", path.display())) + .unwrap() + .create_if_missing(true) + .journal_mode(sqlx::sqlite::SqliteJournalMode::Delete) + .pragma("key", format!("'{KEY}'")) + .connect() + .await + .unwrap(); + connection + .execute("CREATE TABLE notes(id INTEGER PRIMARY KEY, body TEXT)") + .await + .unwrap(); + connection + .execute("INSERT INTO notes(id, body) VALUES (1, 'kept'), (2, 'also kept')") + .await + .unwrap(); + connection + .execute("PRAGMA user_version = 42") + .await + .unwrap(); + connection.close().await.unwrap(); +} + +/// True when the file opens with the raw key form and no key derivation. +async fn opens_with_raw_key(path: &Path) -> bool { + let Ok(mut connection) = + SqliteConnectOptions::from_str(&format!("sqlite://{}", path.display())) + .unwrap() + .create_if_missing(false) + .journal_mode(sqlx::sqlite::SqliteJournalMode::Delete) + .pragma("key", format!("\"x'{KEY}'\"")) + .connect() + .await + else { + return false; + }; + let readable = connection + .execute("SELECT count(*) FROM sqlite_master") + .await + .is_ok(); + let _ = connection.close().await; + readable +} + +#[tokio::test] +async fn legacy_signal_database_is_migrated_and_keeps_its_contents() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("rust_db.sqlite"); + create_legacy_database(&path).await; + assert!( + !opens_with_raw_key(&path).await, + "the fixture must start out passphrase-keyed" + ); + + let database = Database::new(&path.display().to_string(), Some(KEY), false) + .await + .expect("a legacy database must still open"); + + let bodies: Vec = sqlx::query("SELECT body FROM notes ORDER BY id") + .fetch_all(&database.pool) + .await + .unwrap() + .into_iter() + .map(|row| row.get::("body")) + .collect(); + assert_eq!(bodies, vec!["kept", "also kept"]); + + let user_version: i64 = sqlx::query("PRAGMA user_version") + .fetch_one(&database.pool) + .await + .unwrap() + .get(0); + assert_eq!(user_version, 42, "user_version must survive the rekey"); + + // Migrations still apply on top of the re-encrypted file. + database.run_migrations().await.unwrap(); + database.pool.close().await; + + assert!( + opens_with_raw_key(&path).await, + "the database must be re-encrypted with the raw key" + ); +} + +#[tokio::test] +async fn legacy_app_database_is_migrated_and_keeps_its_contents() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("app_db.sqlite"); + create_legacy_database(&path).await; + + let database = AppDatabase::new(&path.display().to_string(), Some(KEY), false) + .await + .expect("a legacy database must still open"); + let count: i64 = sqlx::query("SELECT count(*) FROM notes") + .fetch_one(&database.pool) + .await + .unwrap() + .get(0); + assert_eq!(count, 2); + database.run_migrations().await.unwrap(); + database.pool.close().await; + + assert!(opens_with_raw_key(&path).await); +} + +#[tokio::test] +async fn migrating_twice_is_a_no_op() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("rust_db.sqlite"); + create_legacy_database(&path).await; + + for _ in 0..3 { + let database = Database::new(&path.display().to_string(), Some(KEY), false) + .await + .unwrap(); + let count: i64 = sqlx::query("SELECT count(*) FROM notes") + .fetch_one(&database.pool) + .await + .unwrap() + .get(0); + assert_eq!(count, 2); + database.pool.close().await; + } + assert!(opens_with_raw_key(&path).await); +} + +/// A staged backup archive is verified read-only before it replaces anything. +/// It cannot be rewritten at that point, so it has to stay readable as is. +#[tokio::test] +async fn a_legacy_database_opened_read_only_is_not_migrated() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("archive.sqlite"); + create_legacy_database(&path).await; + + let database = Database::new(&path.display().to_string(), Some(KEY), true) + .await + .expect("a legacy archive must open read-only"); + let count: i64 = sqlx::query("SELECT count(*) FROM notes") + .fetch_one(&database.pool) + .await + .unwrap() + .get(0); + assert_eq!(count, 2); + database.pool.close().await; + + assert!( + !opens_with_raw_key(&path).await, + "a read-only open must leave the archive's encryption alone" + ); + + // Opening the same file for writing later does migrate it. + let database = Database::new(&path.display().to_string(), Some(KEY), false) + .await + .unwrap(); + database.pool.close().await; + assert!(opens_with_raw_key(&path).await); +} + +/// A key that is not 32 bytes of hex cannot be a raw key. Those callers have to +/// keep working on the passphrase path. +#[tokio::test] +async fn a_non_hex_key_keeps_working() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("passworded.sqlite"); + let database = Database::new(&path.display().to_string(), Some("not-a-hex-key"), false) + .await + .unwrap(); + database + .pool + .execute("CREATE TABLE t(a); INSERT INTO t VALUES (1)") + .await + .unwrap(); + database.pool.close().await; + + let reopened = Database::new(&path.display().to_string(), Some("not-a-hex-key"), false) + .await + .unwrap(); + let count: i64 = sqlx::query("SELECT count(*) FROM t") + .fetch_one(&reopened.pool) + .await + .unwrap() + .get(0); + assert_eq!(count, 1); + reopened.pool.close().await; +} + +#[tokio::test] +async fn the_wrong_key_still_fails() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("rust_db.sqlite"); + create_legacy_database(&path).await; + + let wrong = "0".repeat(64); + let result = Database::new(&path.display().to_string(), Some(&wrong), false).await; + let opened_anything = match result { + Err(_) => false, + Ok(database) => { + let readable = sqlx::query("SELECT count(*) FROM notes") + .fetch_one(&database.pool) + .await + .is_ok(); + database.pool.close().await; + readable + } + }; + assert!(!opened_anything, "a wrong key must not read the database"); +} + +/// The whole point of the change: a fresh database opens without spending a +/// quarter of a second on PBKDF2, and so does every further connection. +#[tokio::test] +async fn opening_a_migrated_database_skips_key_derivation() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("rust_db.sqlite"); + create_legacy_database(&path).await; + + let database = Database::new(&path.display().to_string(), Some(KEY), false) + .await + .unwrap(); + database.pool.close().await; + + let started = Instant::now(); + let database = Database::new(&path.display().to_string(), Some(KEY), false) + .await + .unwrap(); + // Force the pool to grow past its first connection. + let mut held = Vec::new(); + for _ in 0..8 { + held.push(database.pool.acquire().await.unwrap()); + } + let elapsed = started.elapsed(); + drop(held); + database.pool.close().await; + + // Eight passphrase-keyed connections cost ~350 ms on a desktop and far more + // on a phone. The bound is loose so a slow CI machine cannot flake it while + // still failing loudly if key derivation comes back. + assert!( + elapsed.as_millis() < 150, + "opening 8 connections took {elapsed:?}, which suggests key derivation is still running" + ); +} + +#[test] +fn the_fixture_key_has_the_shape_of_a_real_database_key() { + assert_eq!(KEY.len(), 64); + assert!(KEY.bytes().all(|byte| byte.is_ascii_hexdigit())); +}