fix(android): stop backing up credentials no key can ever open
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
This commit is contained in:
@@ -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 {
|
kotlinOptions {
|
||||||
jvmTarget = "1.8"
|
jvmTarget = "17"
|
||||||
}
|
}
|
||||||
buildFeatures {
|
buildFeatures {
|
||||||
buildConfig = true
|
buildConfig = true
|
||||||
|
|||||||
@@ -27,16 +27,50 @@
|
|||||||
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_MEDIA_PLAYBACK" />
|
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_MEDIA_PLAYBACK" />
|
||||||
<uses-permission android:name="android.permission.POST_NOTIFICATIONS" />
|
<uses-permission android:name="android.permission.POST_NOTIFICATIONS" />
|
||||||
|
|
||||||
<!-- AndroidTV support -->
|
<!--
|
||||||
<uses-feature android:name="android.software.leanback" android:required="false" />
|
Android TV is deliberately NOT declared here.
|
||||||
|
|
||||||
|
A LEANBACK_LAUNCHER category and an android.software.leanback uses-feature
|
||||||
|
used to sit in this manifest, but nothing behind them: no D-pad focus
|
||||||
|
model, no TV-sized layouts, and neither of the two declarations Play's TV
|
||||||
|
validation also requires (android.hardware.touchscreen required="false"
|
||||||
|
and an android:banner). That combination is the worst of both - it offers
|
||||||
|
the app to TV launchers while failing TV review and shipping a UI that
|
||||||
|
cannot be driven without a touchscreen.
|
||||||
|
|
||||||
|
Re-declare all four together (leanback feature, LEANBACK_LAUNCHER,
|
||||||
|
touchscreen required="false", banner) once a focus pass has actually been
|
||||||
|
done, not before.
|
||||||
|
-->
|
||||||
|
|
||||||
|
<!--
|
||||||
|
android:allowBackup / android:dataExtractionRules below:
|
||||||
|
no cloud backup, no device-to-device transfer (UR-012).
|
||||||
|
|
||||||
|
Credentials are encrypted under an Android Keystore key, and Keystore keys
|
||||||
|
are NEVER backed up. A restored install would therefore get the
|
||||||
|
jellytau_secure_prefs ciphertext with no key to open it - the app would
|
||||||
|
look signed in and silently fail every request, which is worse than a
|
||||||
|
login screen. Everything else in the data dir (the SQLite catalogue:
|
||||||
|
library metadata, watch history, download bookkeeping) is a rebuildable
|
||||||
|
mirror of the Jellyfin server, so backing it up buys nothing and exports
|
||||||
|
the user's library and viewing history to their Google account.
|
||||||
|
|
||||||
|
allowBackup covers API 24-30 completely, and kills *cloud* backup on API
|
||||||
|
31+. It does NOT stop device-to-device transfer there, so
|
||||||
|
@xml/data_extraction_rules (API 31+) excludes both channels explicitly. No
|
||||||
|
android:fullBackupContent is needed: over the API 23-30 range where it
|
||||||
|
would govern, allowBackup="false" has already turned backup off entirely.
|
||||||
|
-->
|
||||||
<application
|
<application
|
||||||
android:icon="@mipmap/ic_launcher"
|
android:icon="@mipmap/ic_launcher"
|
||||||
android:label="${appLabel}"
|
android:label="${appLabel}"
|
||||||
android:theme="@style/Theme.jellytau"
|
android:theme="@style/Theme.jellytau"
|
||||||
android:hardwareAccelerated="true"
|
android:hardwareAccelerated="true"
|
||||||
android:networkSecurityConfig="@xml/network_security_config"
|
android:networkSecurityConfig="@xml/network_security_config"
|
||||||
android:usesCleartextTraffic="${usesCleartextTraffic}">
|
android:usesCleartextTraffic="${usesCleartextTraffic}"
|
||||||
|
android:allowBackup="false"
|
||||||
|
android:dataExtractionRules="@xml/data_extraction_rules">
|
||||||
<activity
|
<activity
|
||||||
android:configChanges="orientation|keyboardHidden|keyboard|screenSize|locale|smallestScreenSize|screenLayout|uiMode|density"
|
android:configChanges="orientation|keyboardHidden|keyboard|screenSize|locale|smallestScreenSize|screenLayout|uiMode|density"
|
||||||
android:launchMode="singleTask"
|
android:launchMode="singleTask"
|
||||||
@@ -48,8 +82,7 @@
|
|||||||
<intent-filter>
|
<intent-filter>
|
||||||
<action android:name="android.intent.action.MAIN" />
|
<action android:name="android.intent.action.MAIN" />
|
||||||
<category android:name="android.intent.category.LAUNCHER" />
|
<category android:name="android.intent.category.LAUNCHER" />
|
||||||
<!-- AndroidTV support -->
|
<!-- No LEANBACK_LAUNCHER: see the Android TV note above. -->
|
||||||
<category android:name="android.intent.category.LEANBACK_LAUNCHER" />
|
|
||||||
</intent-filter>
|
</intent-filter>
|
||||||
</activity>
|
</activity>
|
||||||
|
|
||||||
|
|||||||
@@ -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? {
|
fun getCredential(key: String): String? {
|
||||||
try {
|
val encoded = prefs.getString(key, null)
|
||||||
val encoded = prefs.getString(key, null) ?: return null
|
if (encoded == null) {
|
||||||
|
Log.d(TAG, "No credential stored for: $key")
|
||||||
|
return null
|
||||||
|
}
|
||||||
|
|
||||||
|
return try {
|
||||||
val combined = Base64.decode(encoded, Base64.DEFAULT)
|
val combined = Base64.decode(encoded, Base64.DEFAULT)
|
||||||
|
|
||||||
// Extract IV (first 12 bytes for GCM)
|
// Extract IV (first 12 bytes for GCM)
|
||||||
@@ -114,10 +132,16 @@ class SecureStorage private constructor(context: Context) {
|
|||||||
cipher.init(Cipher.DECRYPT_MODE, getSecretKey(), spec)
|
cipher.init(Cipher.DECRYPT_MODE, getSecretKey(), spec)
|
||||||
|
|
||||||
val decrypted = cipher.doFinal(encrypted)
|
val decrypted = cipher.doFinal(encrypted)
|
||||||
return String(decrypted, Charsets.UTF_8)
|
String(decrypted, Charsets.UTF_8)
|
||||||
} catch (e: Exception) {
|
} catch (e: Exception) {
|
||||||
Log.e(TAG, "Failed to get credential: $key", e)
|
Log.w(
|
||||||
return null
|
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
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,45 @@
|
|||||||
|
<?xml version="1.0" encoding="utf-8"?>
|
||||||
|
<!--
|
||||||
|
Backup / transfer policy for JellyTau (API 31+; see android:allowBackup in
|
||||||
|
AndroidManifest.xml for API 24-30).
|
||||||
|
|
||||||
|
Nothing is eligible for extraction, from either channel:
|
||||||
|
|
||||||
|
* cloud-backup - already off via android:allowBackup="false".
|
||||||
|
* device-transfer - NOT covered by allowBackup on Android 12+, which is why
|
||||||
|
this file exists. A D2D transfer would otherwise copy the same data the
|
||||||
|
cloud backup used to.
|
||||||
|
|
||||||
|
Why nothing is extractable:
|
||||||
|
|
||||||
|
* Credentials are unrecoverable off-device. jellytau_secure_prefs holds
|
||||||
|
AES-GCM ciphertext encrypted under an Android Keystore key, and Keystore
|
||||||
|
keys are never backed up or transferred. Restoring the prefs without the
|
||||||
|
key produces ciphertext nothing can read - a silent auth failure that looks
|
||||||
|
like a broken app rather than a logged-out one.
|
||||||
|
* Everything else is a rebuildable cache. The SQLite catalogue is a mirror of
|
||||||
|
the Jellyfin server (library metadata, watch history, offline downloads);
|
||||||
|
signing in again reproduces it, and watch state lives on the server anyway.
|
||||||
|
Backing it up would export a user's library and viewing history to their
|
||||||
|
Google account for no gain.
|
||||||
|
|
||||||
|
Exclude rules are listed per domain rather than relying on "root" alone,
|
||||||
|
because database/, shared_prefs/, files/ and external storage are addressed
|
||||||
|
as their own domains by the extraction engine.
|
||||||
|
-->
|
||||||
|
<data-extraction-rules>
|
||||||
|
<cloud-backup>
|
||||||
|
<exclude domain="root" />
|
||||||
|
<exclude domain="file" />
|
||||||
|
<exclude domain="database" />
|
||||||
|
<exclude domain="sharedpref" />
|
||||||
|
<exclude domain="external" />
|
||||||
|
</cloud-backup>
|
||||||
|
<device-transfer>
|
||||||
|
<exclude domain="root" />
|
||||||
|
<exclude domain="file" />
|
||||||
|
<exclude domain="database" />
|
||||||
|
<exclude domain="sharedpref" />
|
||||||
|
<exclude domain="external" />
|
||||||
|
</device-transfer>
|
||||||
|
</data-extraction-rules>
|
||||||
@@ -471,6 +471,19 @@ impl CredentialStore {
|
|||||||
hasher.finalize().into()
|
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<serde_json::Value, CredentialError> {
|
fn load_credentials_file(&self) -> Result<serde_json::Value, CredentialError> {
|
||||||
if !self.credentials_path.exists() {
|
if !self.credentials_path.exists() {
|
||||||
return Ok(serde_json::json!({}));
|
return Ok(serde_json::json!({}));
|
||||||
@@ -483,8 +496,31 @@ impl CredentialStore {
|
|||||||
return Ok(serde_json::json!({}));
|
return Ok(serde_json::json!({}));
|
||||||
}
|
}
|
||||||
|
|
||||||
let decrypted = self.decrypt(&encrypted_data)?;
|
let decrypted = match self.decrypt(&encrypted_data) {
|
||||||
serde_json::from_str(&decrypted).map_err(|e| CredentialError::Encryption(e.to_string()))
|
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> {
|
fn save_credentials_file(&self, data: &serde_json::Value) -> Result<(), CredentialError> {
|
||||||
@@ -856,6 +892,73 @@ pub use android_keystore::{
|
|||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
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]
|
#[test]
|
||||||
fn test_encryption_roundtrip() {
|
fn test_encryption_roundtrip() {
|
||||||
let store = CredentialStore::new();
|
let store = CredentialStore::new();
|
||||||
|
|||||||
Reference in New Issue
Block a user