From 4c9361d0200f48853fb26fbfd5f3023bd4ace755 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sun, 16 Aug 2026 22:56:32 +0200 Subject: [PATCH] fix(android): stop backing up credentials no key can ever open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The app's data dir was eligible for Google cloud backup: the manifest set neither allowBackup nor any extraction rules, so the SQLite catalogue (library metadata, watch history) and the jellytau_secure_prefs credential blob were shipped to the user's Google account. Restoring that is worse than not having it — SecureStorage encrypts under an Android Keystore key, and Keystore keys are never backed up, so a restored install gets ciphertext with nothing to open it and fails auth silently while looking signed in. Backup and device-to-device transfer are both turned off. allowBackup ="false" covers API 24-30 outright and kills cloud backup on 31+; it does NOT stop D2D there, so @xml/data_extraction_rules excludes every domain from both channels. Nothing is lost: the catalogue is a rebuildable mirror of the Jellyfin server, and watch state lives on the server. The credential-load path degrades instead of erroring, because a device can still arrive at undecryptable ciphertext (an older install's backup, a Keystore key invalidated by a lockscreen change). Both backends now distinguish "nothing stored" from "stored but unreadable" and answer the second as the first: CredentialStore::load_credentials_file logs and returns an empty map rather than CredentialError::Encryption — which storage_get_access_token was turning into a hard Err and storage_get_active_session into a warning — and SecureStorage.getCredential discards the dead blob so it cannot fail every subsequent read. The result is a login screen rather than a broken session, and the next successful sign-in rewrites the store. Also removes the half-declared Android TV support: the manifest offered LEANBACK_LAUNCHER and the leanback uses-feature with no D-pad focus model, no TV layouts, and neither of the two declarations Play's TV validation also requires (touchscreen required="false", android:banner). That fails review while advertising the app to TV launchers. All four go back together when a focus pass is actually done. And raises jvmTarget from 1.8 to 17 under compileSdk 36, with matching compileOptions — AGP 8.11 already requires a JDK 17 toolchain, so 1.8 was only capping emitted bytecode. Nothing else in the build assumed 1.8. TRACES: UR-012 | IR-014 --- src-tauri/android/app/build.gradle.kts | 11 +- .../android/src/main/AndroidManifest.xml | 43 ++++++- .../jellytau/security/SecureStorage.kt | 34 +++++- .../main/res/xml/data_extraction_rules.xml | 45 ++++++++ src-tauri/src/credentials.rs | 107 +++++++++++++++++- 5 files changed, 227 insertions(+), 13 deletions(-) create mode 100644 src-tauri/android/src/main/res/xml/data_extraction_rules.xml diff --git a/src-tauri/android/app/build.gradle.kts b/src-tauri/android/app/build.gradle.kts index b6cc2de8..f8d46518 100644 --- a/src-tauri/android/app/build.gradle.kts +++ b/src-tauri/android/app/build.gradle.kts @@ -107,8 +107,17 @@ android { ) } } + // Java 17 bytecode. AGP 8.11 already requires a JDK 17 toolchain to run + // (the builder image ships openjdk-17), so "1.8" was only capping the + // bytecode we emit, not the JDK in use. Kotlin's jvmTarget and javac's + // source/targetCompatibility must agree or AGP 8 fails the build, so all + // three move together. + compileOptions { + sourceCompatibility = JavaVersion.VERSION_17 + targetCompatibility = JavaVersion.VERSION_17 + } kotlinOptions { - jvmTarget = "1.8" + jvmTarget = "17" } buildFeatures { buildConfig = true diff --git a/src-tauri/android/src/main/AndroidManifest.xml b/src-tauri/android/src/main/AndroidManifest.xml index 6eb892ac..bd473e21 100644 --- a/src-tauri/android/src/main/AndroidManifest.xml +++ b/src-tauri/android/src/main/AndroidManifest.xml @@ -27,16 +27,50 @@ - - + + + + android:usesCleartextTraffic="${usesCleartextTraffic}" + android:allowBackup="false" + android:dataExtractionRules="@xml/data_extraction_rules"> - - + diff --git a/src-tauri/android/src/main/java/com/dtourolle/jellytau/security/SecureStorage.kt b/src-tauri/android/src/main/java/com/dtourolle/jellytau/security/SecureStorage.kt index da18a39f..23c77e5e 100644 --- a/src-tauri/android/src/main/java/com/dtourolle/jellytau/security/SecureStorage.kt +++ b/src-tauri/android/src/main/java/com/dtourolle/jellytau/security/SecureStorage.kt @@ -100,9 +100,27 @@ class SecureStorage private constructor(context: Context) { } } + /** + * Read a credential. + * + * Returns null for both "nothing stored" and "stored but undecryptable", but + * treats them as distinct events. The second happens after a backup restore + * or a device-to-device transfer: SharedPreferences travel, the Android + * Keystore key that encrypted them never does, so the ciphertext can never + * be read again on this install. That blob is discarded here rather than + * left to fail on every subsequent read, which turns a permanently broken + * credential into a clean logged-out state. (The app also declares + * allowBackup="false" plus data-extraction rules so this should no longer + * arise - this is the belt to that manifest's braces.) + */ fun getCredential(key: String): String? { - try { - val encoded = prefs.getString(key, null) ?: return null + val encoded = prefs.getString(key, null) + if (encoded == null) { + Log.d(TAG, "No credential stored for: $key") + return null + } + + return try { val combined = Base64.decode(encoded, Base64.DEFAULT) // Extract IV (first 12 bytes for GCM) @@ -114,10 +132,16 @@ class SecureStorage private constructor(context: Context) { cipher.init(Cipher.DECRYPT_MODE, getSecretKey(), spec) val decrypted = cipher.doFinal(encrypted) - return String(decrypted, Charsets.UTF_8) + String(decrypted, Charsets.UTF_8) } catch (e: Exception) { - Log.e(TAG, "Failed to get credential: $key", e) - return null + Log.w( + TAG, + "Credential '$key' is present but cannot be decrypted; discarding it and " + + "reporting no credential. Signing in again will store a fresh one.", + e + ) + prefs.edit().remove(key).apply() + null } } diff --git a/src-tauri/android/src/main/res/xml/data_extraction_rules.xml b/src-tauri/android/src/main/res/xml/data_extraction_rules.xml new file mode 100644 index 00000000..ed5d19db --- /dev/null +++ b/src-tauri/android/src/main/res/xml/data_extraction_rules.xml @@ -0,0 +1,45 @@ + + + + + + + + + + + + + + + + + + diff --git a/src-tauri/src/credentials.rs b/src-tauri/src/credentials.rs index 38585776..0032dd20 100644 --- a/src-tauri/src/credentials.rs +++ b/src-tauri/src/credentials.rs @@ -471,6 +471,19 @@ impl CredentialStore { hasher.finalize().into() } + /// Load and decrypt the credential map. + /// + /// A file that is present but **undecryptable** is deliberately reported as + /// an *empty* credential set rather than as an error. The key never leaves + /// the device it was derived on (Android Keystore keys are never backed up, + /// and the file fallback's key is derived from machine identifiers), so a + /// restored/transferred install gets ciphertext with no key and every read + /// would fail *permanently*. Surfacing that as an error made session restore + /// throw instead of falling back to the login screen: an unrecoverable app + /// rather than a clean logged-out one. The next successful login re-encrypts + /// the file with the current key, so the state self-heals. + /// + /// TRACES: UR-012 | IR-014 fn load_credentials_file(&self) -> Result { if !self.credentials_path.exists() { return Ok(serde_json::json!({})); @@ -483,8 +496,31 @@ impl CredentialStore { return Ok(serde_json::json!({})); } - let decrypted = self.decrypt(&encrypted_data)?; - serde_json::from_str(&decrypted).map_err(|e| CredentialError::Encryption(e.to_string())) + let decrypted = match self.decrypt(&encrypted_data) { + Ok(decrypted) => decrypted, + Err(e) => { + warn!( + "Credentials file at {:?} exists but cannot be decrypted ({}); \ + treating as no stored credentials. This is expected after a \ + backup restore or device transfer - the encryption key does \ + not travel with the data. Signing in again will rewrite it.", + self.credentials_path, e + ); + return Ok(serde_json::json!({})); + } + }; + + match serde_json::from_str(&decrypted) { + Ok(value) => Ok(value), + Err(e) => { + warn!( + "Credentials file at {:?} decrypted to invalid JSON ({}); \ + treating as no stored credentials.", + self.credentials_path, e + ); + Ok(serde_json::json!({})) + } + } } fn save_credentials_file(&self, data: &serde_json::Value) -> Result<(), CredentialError> { @@ -856,6 +892,73 @@ pub use android_keystore::{ mod tests { use super::*; + /// Build a store pinned to the encrypted-file backend with an explicit key, + /// so a test can simulate "same file, different machine key" (which is what + /// a restored backup looks like). + fn file_backed_store(credentials_path: PathBuf, encryption_key: [u8; 32]) -> CredentialStore { + CredentialStore { + using_keyring: false, + credentials_path, + encryption_key, + } + } + + /// A credentials file we cannot decrypt must read as *no credentials stored*, + /// not as a hard error. This is the restored-backup case: the ciphertext comes + /// back but the key that encrypted it (Android Keystore / the machine-derived + /// key) does not, so every read fails forever. + /// + /// TRACES: UR-012 | IR-014 + #[test] + fn undecryptable_credentials_file_reads_as_not_found() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join(CREDENTIALS_FILENAME); + + let original = file_backed_store(path.clone(), [1u8; 32]); + original.save_to_file("user-1", "token-abc").unwrap(); + + // Restored onto a device whose derived key differs: same bytes, no key. + let restored = file_backed_store(path.clone(), [2u8; 32]); + match restored.get_token("user-1") { + Err(CredentialError::NotFound) => {} + other => panic!("expected NotFound for undecryptable ciphertext, got {other:?}"), + } + } + + /// Garbage in the file (truncation, partial restore) is the same story. + /// + /// TRACES: UR-012 | IR-014 + #[test] + fn corrupt_credentials_file_reads_as_not_found() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join(CREDENTIALS_FILENAME); + fs::write(&path, "not base64 at all !!!").unwrap(); + + let store = file_backed_store(path, [3u8; 32]); + match store.get_token("user-1") { + Err(CredentialError::NotFound) => {} + other => panic!("expected NotFound for corrupt file, got {other:?}"), + } + } + + /// …and the logged-out state must be recoverable: signing in again has to be + /// able to write over the unreadable file rather than failing on load. + /// + /// TRACES: UR-012 | IR-014 + #[test] + fn login_after_undecryptable_file_rewrites_it() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join(CREDENTIALS_FILENAME); + + let original = file_backed_store(path.clone(), [1u8; 32]); + original.save_to_file("user-1", "token-abc").unwrap(); + + let restored = file_backed_store(path.clone(), [2u8; 32]); + restored.save_to_file("user-1", "token-fresh").unwrap(); + + assert_eq!(restored.get_from_file("user-1").unwrap(), "token-fresh"); + } + #[test] fn test_encryption_roundtrip() { let store = CredentialStore::new();