From e14dd097e5619792f450f6b5675eb3a5cb216c41 Mon Sep 17 00:00:00 2001 From: lexi <173241226+cutieway@users.noreply.github.com> Date: Mon, 26 Oct 2026 02:16:39 +0200 Subject: [PATCH 1/3] Seal the token with Android Keystore and redact credentials Store the Discord token as an AES-256-GCM blob under an Android Keystore key, migrate the legacy plaintext preference, and delete the key whenever the credential is cleared so an escaped blob cannot be reopened. Read the token on demand instead of caching plaintext. Redact authorization headers, cookies, token-shaped strings, signed URLs, and credential fields from logs and crash reports. Clear WebView storage and the sealed token before a sign-out wipe, and stop boot-start failures from logging raw exception text. Add instrumentation coverage for migration, isolated test storage, key deletion, corrupted blobs, and legacy cleanup. --- app/build.gradle.kts | 3 + .../discordrp/settings/TokenStorageTest.kt | 140 ++++++++++++++++++ .../spacealtctrl/discordrp/di/AppProviders.kt | 6 +- .../discordrp/discord/DiscordApi.kt | 5 + .../net/spacealtctrl/discordrp/log/AppLog.kt | 8 +- .../spacealtctrl/discordrp/log/Redactor.kt | 47 ++++++ .../discordrp/service/StartOnBootReceiver.kt | 12 +- .../spacealtctrl/discordrp/settings/Stash.kt | 70 ++++++++- .../discordrp/settings/TokenVault.kt | 120 +++++++++++++++ .../discordrp/ui/crash/CrashGuard.kt | 4 +- .../discordrp/ui/screens/you/YouViewModel.kt | 11 ++ .../discordrp/log/RedactorTest.kt | 131 ++++++++++++++++ 12 files changed, 542 insertions(+), 15 deletions(-) create mode 100644 app/src/androidTest/java/net/spacealtctrl/discordrp/settings/TokenStorageTest.kt create mode 100644 app/src/main/java/net/spacealtctrl/discordrp/log/Redactor.kt create mode 100644 app/src/main/java/net/spacealtctrl/discordrp/settings/TokenVault.kt create mode 100644 app/src/test/java/net/spacealtctrl/discordrp/log/RedactorTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index b4f8f9b..e1b1635 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -23,6 +23,7 @@ android { defaultConfig { applicationId = "net.spacealtctrl.discordrp" + testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" minSdk = 26 targetSdk = 36 versionCode = (findProperty("versionCodeOverride") as String?)?.toInt() @@ -110,4 +111,6 @@ dependencies { ksp(libs.hilt.compiler) testImplementation(libs.junit) + androidTestImplementation("androidx.test.ext:junit:1.3.0") + androidTestImplementation("androidx.test:runner:1.7.0") } diff --git a/app/src/androidTest/java/net/spacealtctrl/discordrp/settings/TokenStorageTest.kt b/app/src/androidTest/java/net/spacealtctrl/discordrp/settings/TokenStorageTest.kt new file mode 100644 index 0000000..2852831 --- /dev/null +++ b/app/src/androidTest/java/net/spacealtctrl/discordrp/settings/TokenStorageTest.kt @@ -0,0 +1,140 @@ +package net.spacealtctrl.discordrp.settings + +import android.content.Context +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith + +/** + * Exercises the storage rules against a dedicated preferences file and key + * alias. The live session in "stash" is never written or re-keyed, so a killed + * test process cannot sign the developer out. + */ +@RunWith(AndroidJUnit4::class) +class TokenStorageTest { + private val context: Context + get() = InstrumentationRegistry.getInstrumentation().targetContext + + private val prefs + get() = context.getSharedPreferences(TEST_PREFS, Context.MODE_PRIVATE) + + private fun stash() = Stash.isolated(context, TEST_PREFS) + + @Before + fun resetStore() { + context.deleteSharedPreferences(TEST_PREFS) + } + + @After + fun removeStore() { + // Flush any apply() still queued, then drop the file so test tokens do + // not linger between runs. + prefs.edit().clear().commit() + context.deleteSharedPreferences(TEST_PREFS) + } + + @Test + fun legacyTokenIsMigrated() { + prefs.edit().putString("discord.token", "legacy-token").commit() + + val stash = stash() + assertEquals("legacy-token", stash.authToken) + assertNull(prefs.getString("discord.token", null)) + assertNotNull(prefs.getString("discord.token.enc", null)) + } + + @Test + fun tokenIsSealedAndNeverLeftInPlaintext() { + val stash = stash() + stash.authToken = "smoke-token" + + assertEquals("smoke-token", stash.authToken) + assertNull(prefs.getString("discord.token", null)) + assertNotNull(prefs.getString("discord.token.enc", null)) + } + + @Test + fun clearingTokenRemovesBothStoredKeys() { + val stash = stash() + stash.authToken = "clear-me" + stash.authToken = "" + + assertFalse(stash.signedIn) + assertNull(prefs.getString("discord.token", null)) + assertNull(prefs.getString("discord.token.enc", null)) + } + + @Test + fun tokenIsReadFromStorageOnEveryAccess() { + val stash = stash() + stash.authToken = "first-token" + + // A second store instance rewrites the sealed blob; the first reads it + // fresh instead of serving a cached plaintext. + stash().authToken = "second-token" + + assertEquals("second-token", stash.authToken) + } + + @Test + fun signedInFollowsStoredBlobPresence() { + val stash = stash() + assertFalse(stash.signedIn) + + stash.authToken = "probe-token" + assertTrue(stash.signedIn) + + stash.clearToken() + assertFalse(stash.signedIn) + } + + @Test + fun clearTokenDeletesTheKeySoAnEscapedBlobStaysUnreadable() { + val stash = stash() + stash.authToken = "clear-me" + val sealed = prefs.getString("discord.token.enc", null)!! + + stash.clearToken() + + // Simulate a blob that escaped the sandbox and is put back later. + // Reading runs TokenVault.decrypt for this store's alias; it must fail + // because clearToken removed that key. + prefs.edit().putString("discord.token.enc", sealed).commit() + + assertEquals("", stash.authToken) + } + + @Test + fun corruptedBlobIsRejectedWithoutPlaintextFallback() { + assertNull(TokenVault.decrypt("not-a-blob")) + assertNull(TokenVault.decrypt("iv:not-base64!!")) + + prefs.edit().putString("discord.token.enc", "not-a-blob").commit() + val stash = stash() + + assertEquals("", stash.authToken) + assertNull(prefs.getString("discord.token", null)) + } + + @Test + fun isolatedStoreLeavesTheLiveSessionAlone() { + val live = context.getSharedPreferences("stash", Context.MODE_PRIVATE) + val sealed = live.getString("discord.token.enc", null) + + stash().authToken = "isolated-token" + + assertEquals(sealed, live.getString("discord.token.enc", null)) + } + + private companion object { + const val TEST_PREFS = "stash-test" + } +} diff --git a/app/src/main/java/net/spacealtctrl/discordrp/di/AppProviders.kt b/app/src/main/java/net/spacealtctrl/discordrp/di/AppProviders.kt index 54bc914..9c22d87 100644 --- a/app/src/main/java/net/spacealtctrl/discordrp/di/AppProviders.kt +++ b/app/src/main/java/net/spacealtctrl/discordrp/di/AppProviders.kt @@ -55,7 +55,11 @@ object AppProviders { } install(Logging) { level = if (BuildConfig.DEBUG) LogLevel.HEADERS else LogLevel.NONE - sanitizeHeader { it == HttpHeaders.Authorization } + sanitizeHeader { + it == HttpHeaders.Authorization || + it == HttpHeaders.Cookie || + it == HttpHeaders.SetCookie + } logger = object : Logger { override fun log(message: String) = log.debug("Http", message) } diff --git a/app/src/main/java/net/spacealtctrl/discordrp/discord/DiscordApi.kt b/app/src/main/java/net/spacealtctrl/discordrp/discord/DiscordApi.kt index 2164cb5..4361f70 100644 --- a/app/src/main/java/net/spacealtctrl/discordrp/discord/DiscordApi.kt +++ b/app/src/main/java/net/spacealtctrl/discordrp/discord/DiscordApi.kt @@ -122,6 +122,11 @@ class DiscordApi @Inject constructor( } private fun HttpResponse.raiseForStatus() { + if (status.value == 401) { + // Unauthorized is permanent for this credential; drop it so the + // service cannot redial with a token Discord already rejected. + stash.clearToken() + } if (status.value !in 200..299) throw ApiRefusal(status.value) } diff --git a/app/src/main/java/net/spacealtctrl/discordrp/log/AppLog.kt b/app/src/main/java/net/spacealtctrl/discordrp/log/AppLog.kt index b01149d..b5dbb79 100644 --- a/app/src/main/java/net/spacealtctrl/discordrp/log/AppLog.kt +++ b/app/src/main/java/net/spacealtctrl/discordrp/log/AppLog.kt @@ -14,19 +14,19 @@ object AndroidAppLog : AppLog { private const val PREFIX = "DiscordRP/" override fun debug(tag: String, message: String) { - if (BuildConfig.DEBUG) Log.d(PREFIX + tag, message) + if (BuildConfig.DEBUG) Log.d(PREFIX + tag, Redactor.redact(message)) } override fun info(tag: String, message: String) { - if (BuildConfig.DEBUG) Log.i(PREFIX + tag, message) + if (BuildConfig.DEBUG) Log.i(PREFIX + tag, Redactor.redact(message)) } override fun warn(tag: String, message: String) { - Log.w(PREFIX + tag, message) + Log.w(PREFIX + tag, Redactor.redact(message)) } override fun error(tag: String, message: String) { - Log.e(PREFIX + tag, message) + Log.e(PREFIX + tag, Redactor.redact(message)) } } diff --git a/app/src/main/java/net/spacealtctrl/discordrp/log/Redactor.kt b/app/src/main/java/net/spacealtctrl/discordrp/log/Redactor.kt new file mode 100644 index 0000000..2b30303 --- /dev/null +++ b/app/src/main/java/net/spacealtctrl/discordrp/log/Redactor.kt @@ -0,0 +1,47 @@ +package net.spacealtctrl.discordrp.log + +/** + * Every log line and crash report passes through here. Exception messages and + * headers can quote credentials or signed URLs, so the text is stripped before + * it leaves the process -- including debug builds. + */ +internal object Redactor { + private const val REPLACEMENT = "[redacted]" + + private val HEADER = Regex( + "(?i)\\b(authorization|proxy-authorization|cookie|set-cookie)\\s*:\\s*[^\\r\\n]+", + ) + private val AUTH_SCHEME = Regex("(?i)\\b(bearer|bot)\\s+[A-Za-z0-9._~+/=-]{8,}") + private val TOKEN = Regex( + "\\b(?:mfa\\.[A-Za-z0-9_-]{20,}" + + "|[A-Za-z0-9_-]{23,28}\\.[A-Za-z0-9_-]{6,7}\\.[A-Za-z0-9_-]{27,40})\\b", + ) + // Opaque credentials are long and base64-like; no message the app logs + // legitimately needs a run that long. + private val BASE64 = Regex("[A-Za-z0-9+/_-]{60,}={0,2}") + private val QUERY = Regex( + "(?i)([?&](?:" + + "token|access_token|refresh_token|id_token|client_secret|api[_-]?key|" + + "password|passwd|secret|session|session_id|code|hm|sig|signature|key|" + + "x-amz-signature|x-amz-credential|x-amz-security-token|" + + "x-goog-signature|x-goog-credential|googleaccessid|awsaccesskeyid" + + ")=)[^&#\\s'\"]+", + ) + private val FIELD = Regex( + "(?i)([\"']?(?:password|passwd|pwd|secret|token|cookie|set[_-]?cookie|" + + "authorization|client[_-]?secret|api[_-]?key|" + + "access[_-]?token|refresh[_-]?token|id[_-]?token|auth[_-]?token)" + + "[\"']?\\s*[=:]\\s*)" + + "(\"[^\"]*\"|'[^']*'|[^\\s,;&}\\)]+)", + ) + private val USERINFO = Regex("([A-Za-z][A-Za-z0-9+.-]*://)[^/?#\\s@]+@") + + fun redact(text: String): String = text + .replace(HEADER) { match -> "${match.groupValues[1]}: $REPLACEMENT" } + .replace(AUTH_SCHEME) { match -> "${match.groupValues[1]} $REPLACEMENT" } + .replace(TOKEN, "[redacted-token]") + .replace(BASE64, REPLACEMENT) + .replace(QUERY) { match -> "${match.groupValues[1]}$REPLACEMENT" } + .replace(FIELD) { match -> "${match.groupValues[1]}$REPLACEMENT" } + .replace(USERINFO) { match -> "${match.groupValues[1]}$REPLACEMENT@" } +} diff --git a/app/src/main/java/net/spacealtctrl/discordrp/service/StartOnBootReceiver.kt b/app/src/main/java/net/spacealtctrl/discordrp/service/StartOnBootReceiver.kt index 177cd5d..45e6706 100644 --- a/app/src/main/java/net/spacealtctrl/discordrp/service/StartOnBootReceiver.kt +++ b/app/src/main/java/net/spacealtctrl/discordrp/service/StartOnBootReceiver.kt @@ -5,7 +5,7 @@ import android.content.BroadcastReceiver import android.content.Context import android.content.Intent import android.os.Build -import android.util.Log +import net.spacealtctrl.discordrp.log.AndroidAppLog import net.spacealtctrl.discordrp.settings.Stash class StartOnBootReceiver : BroadcastReceiver() { @@ -17,16 +17,20 @@ class StartOnBootReceiver : BroadcastReceiver() { try { context.startForegroundService(Intent(context, BridgeService::class.java)) - Log.i(TAG, "Bridge relaunched after ${intent.action}") + AndroidAppLog.info(TAG, "Bridge relaunched after ${intent.action}") } catch (e: Exception) { val vetoed = Build.VERSION.SDK_INT >= Build.VERSION_CODES.S && e is ForegroundServiceStartNotAllowedException - Log.w(TAG, if (vetoed) "System vetoed the relaunch" else "Relaunch failed", e) + AndroidAppLog.warn( + TAG, + if (vetoed) "System vetoed the relaunch" + else "Relaunch failed: ${e::class.java.simpleName}", + ) } } private companion object { - const val TAG = "DiscordRP/BootWake" + const val TAG = "BootWake" val WAKE_ACTIONS = setOf( Intent.ACTION_BOOT_COMPLETED, diff --git a/app/src/main/java/net/spacealtctrl/discordrp/settings/Stash.kt b/app/src/main/java/net/spacealtctrl/discordrp/settings/Stash.kt index f8d2c04..c351de1 100644 --- a/app/src/main/java/net/spacealtctrl/discordrp/settings/Stash.kt +++ b/app/src/main/java/net/spacealtctrl/discordrp/settings/Stash.kt @@ -10,9 +10,13 @@ import kotlinx.serialization.encodeToString import kotlinx.serialization.json.Json import kotlin.time.Duration.Companion.hours -class Stash private constructor(context: Context) { +class Stash private constructor( + context: Context, + prefsName: String = PREFS_NAME, + private val tokenAlias: String = TokenVault.ALIAS, +) { private val prefs: SharedPreferences = - context.applicationContext.getSharedPreferences("stash", Context.MODE_PRIVATE) + context.applicationContext.getSharedPreferences(prefsName, Context.MODE_PRIVATE) private val json = Json { ignoreUnknownKeys = true } @@ -27,8 +31,51 @@ class Stash private constructor(context: Context) { } var authToken: String - get() = prefs.getString(KEY_TOKEN, null).orEmpty() - set(value) = write { putString(KEY_TOKEN, value) } + // Decrypt on demand: the plaintext must not survive in the process + // beyond the call that needs it. + get() = readToken() + set(value) { + val token = value.trim() + if (token.isEmpty()) { + clearToken() + } else { + val sealed = TokenVault.encrypt(token, tokenAlias) + write { + putString(KEY_TOKEN_ENC, sealed) + remove(KEY_TOKEN) + } + } + } + + /** + * Drops the stored credential and its Keystore key. Deleting the key first + * makes an escaped copy of the sealed blob useless even if the process dies + * before the preference write lands. + */ + fun clearToken() { + TokenVault.forget(tokenAlias) + write { + remove(KEY_TOKEN) + remove(KEY_TOKEN_ENC) + } + } + + private fun readToken(): String { + prefs.getString(KEY_TOKEN_ENC, null)?.let { sealed -> + TokenVault.decrypt(sealed, tokenAlias)?.let { return it } + } + val legacy = prefs.getString(KEY_TOKEN, null).orEmpty() + if (legacy.isBlank()) return "" + + // One-time migration from the pre-Keystore format. If sealing fails the + // plaintext copy is still dropped; a token must not stay readable on disk. + val sealed = runCatching { TokenVault.encrypt(legacy, tokenAlias) }.getOrNull() + write { + if (sealed != null) putString(KEY_TOKEN_ENC, sealed) + remove(KEY_TOKEN) + } + return if (sealed != null) legacy else "" + } var selfId: String get() = prefs.getString(KEY_SELF_ID, null).orEmpty() @@ -46,7 +93,11 @@ class Stash private constructor(context: Context) { get() = prefs.getString(KEY_PROFILE_CACHE, null).orEmpty() set(value) = write { putString(KEY_PROFILE_CACHE, value) } - val signedIn: Boolean get() = authToken.isNotBlank() + // Presence check only: reading the sealed blob is enough to tell the UI + // whether a session exists, and avoids decrypting on every recomposition. + val signedIn: Boolean + get() = prefs.contains(KEY_TOKEN_ENC) || + !prefs.getString(KEY_TOKEN, null).isNullOrBlank() var presenceEnabled: Boolean get() = prefs.getBoolean(KEY_PRESENCE_ON, true) @@ -205,7 +256,16 @@ class Stash private constructor(context: Context) { shared ?: Stash(context).also { shared = it } } + /** + * A store that does not share preferences with the signed-in app. Tests + * use it so a run can never clobber the developer's live session. + */ + internal fun isolated(context: Context, prefsName: String): Stash = + Stash(context, prefsName, "${TokenVault.ALIAS}.$prefsName") + + private const val PREFS_NAME = "stash" private const val KEY_TOKEN = "discord.token" + private const val KEY_TOKEN_ENC = "discord.token.enc" private const val KEY_SELF_ID = "discord.user.id" private const val KEY_SELF_BIO = "discord.user.bio" private const val KEY_SELF_NITRO = "discord.user.nitro" diff --git a/app/src/main/java/net/spacealtctrl/discordrp/settings/TokenVault.kt b/app/src/main/java/net/spacealtctrl/discordrp/settings/TokenVault.kt new file mode 100644 index 0000000..e5d0eff --- /dev/null +++ b/app/src/main/java/net/spacealtctrl/discordrp/settings/TokenVault.kt @@ -0,0 +1,120 @@ +package net.spacealtctrl.discordrp.settings + +import android.os.Build +import android.security.keystore.KeyGenParameterSpec +import android.security.keystore.KeyProperties +import android.util.Base64 +import java.security.KeyStore +import javax.crypto.Cipher +import javax.crypto.KeyGenerator +import javax.crypto.SecretKey +import javax.crypto.spec.GCMParameterSpec + +/** + * Seals the Discord token with an Android Keystore key. + * + * The key is deliberately not user-authentication bound and not tied to the + * unlocked-device state: the gateway service has to read the token while the + * screen is off. `EncryptedSharedPreferences` would add a second wrapping layer + * without changing that property, so the vault is the single, documented story + * for token storage. The sealed blob lives in private preferences, which the + * backup rules exclude. + * + * Plaintext is held only for the duration of one call. Byte buffers used to + * move it through the cipher are zeroed afterwards; Kotlin strings themselves + * cannot be wiped, so the callers keep the returned value short-lived. + */ +internal object TokenVault { + /** Alias used by the signed-in app. Test stores derive their own alias. */ + const val ALIAS = "discordrp.token.v1" + + private const val KEYSTORE = "AndroidKeyStore" + private const val TRANSFORMATION = "AES/GCM/NoPadding" + private const val TAG_BITS = 128 + private const val KEY_BITS = 256 + + @Synchronized + fun encrypt(plaintext: String, alias: String = ALIAS): String { + val bytes = plaintext.toByteArray(Charsets.UTF_8) + try { + val cipher = Cipher.getInstance(TRANSFORMATION) + cipher.init(Cipher.ENCRYPT_MODE, key(alias)) + val iv = Base64.encodeToString(cipher.iv, Base64.NO_WRAP) + val body = Base64.encodeToString(cipher.doFinal(bytes), Base64.NO_WRAP) + return "$iv:$body" + } finally { + bytes.fill(0) + } + } + + @Synchronized + fun decrypt(sealed: String, alias: String = ALIAS): String? = try { + val parts = sealed.split(':', limit = 2) + if (parts.size != 2) { + null + } else { + val cipher = Cipher.getInstance(TRANSFORMATION) + cipher.init( + Cipher.DECRYPT_MODE, + key(alias), + GCMParameterSpec(TAG_BITS, Base64.decode(parts[0], Base64.NO_WRAP)), + ) + val plain = cipher.doFinal(Base64.decode(parts[1], Base64.NO_WRAP)) + try { + String(plain, Charsets.UTF_8) + } finally { + plain.fill(0) + } + } + } catch (_: Exception) { + // A missing or invalidated key means the stored token is unusable; the + // user has to sign in again rather than fall back to plaintext. + null + } + + /** + * Drops the key. Called whenever the stored credential is discarded, so + * even a sealed blob that escaped the app sandbox (backup, extracted file) + * cannot be opened again. + */ + @Synchronized + fun forget(alias: String = ALIAS) { + runCatching { + KeyStore.getInstance(KEYSTORE).apply { load(null) }.deleteEntry(alias) + } + } + + private fun key(alias: String): SecretKey { + val store = KeyStore.getInstance(KEYSTORE).apply { load(null) } + (store.getKey(alias, null) as? SecretKey)?.let { return it } + return generate(alias) + } + + private fun generate(alias: String): SecretKey { + // Hardware-backed keys when the device offers them; the fallback keeps + // the app working where StrongBox is absent or already occupied. + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { + runCatching { generate(alias, strongBox = true) }.getOrNull()?.let { return it } + } + return generate(alias, strongBox = false) + } + + private fun generate(alias: String, strongBox: Boolean): SecretKey { + val builder = KeyGenParameterSpec.Builder( + alias, + KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT, + ) + .setBlockModes(KeyProperties.BLOCK_MODE_GCM) + .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE) + .setKeySize(KEY_BITS) + .setUserAuthenticationRequired(false) + .setRandomizedEncryptionRequired(true) + if (strongBox && Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { + builder.setIsStrongBoxBacked(true) + } + + val generator = KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, KEYSTORE) + generator.init(builder.build()) + return generator.generateKey() + } +} diff --git a/app/src/main/java/net/spacealtctrl/discordrp/ui/crash/CrashGuard.kt b/app/src/main/java/net/spacealtctrl/discordrp/ui/crash/CrashGuard.kt index c14416f..c55abad 100644 --- a/app/src/main/java/net/spacealtctrl/discordrp/ui/crash/CrashGuard.kt +++ b/app/src/main/java/net/spacealtctrl/discordrp/ui/crash/CrashGuard.kt @@ -3,6 +3,7 @@ package net.spacealtctrl.discordrp.ui.crash import android.content.Context import android.content.Intent import android.os.Build +import net.spacealtctrl.discordrp.log.Redactor import java.io.PrintWriter import java.io.StringWriter import kotlin.system.exitProcess @@ -27,6 +28,7 @@ object CrashGuard { private fun describe(context: Context, error: Throwable): String { val trace = StringWriter().also { error.printStackTrace(PrintWriter(it)) }.toString() + val redacted = Redactor.redact(trace) val version = runCatching { context.packageManager.getPackageInfo(context.packageName, 0).versionName }.getOrNull() ?: "?" @@ -36,7 +38,7 @@ object CrashGuard { |Android: ${Build.VERSION.RELEASE} (API ${Build.VERSION.SDK_INT}) |App version: $version | - |$trace + |$redacted """.trimMargin().take(REPORT_CAP) } diff --git a/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/you/YouViewModel.kt b/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/you/YouViewModel.kt index 1376962..262d4ba 100644 --- a/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/you/YouViewModel.kt +++ b/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/you/YouViewModel.kt @@ -2,6 +2,8 @@ package net.spacealtctrl.discordrp.ui.screens.you import android.app.ActivityManager import android.content.Context +import android.webkit.CookieManager +import android.webkit.WebStorage import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel @@ -66,6 +68,15 @@ class YouViewModel @Inject constructor( }.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), YouUiState()) fun signOut() { + // clearApplicationUserData removes the sealed blob with the rest of the + // sandbox; clearToken drops the Keystore key and WebView copies first so + // neither survives even if the data wipe is interrupted. + runCatching { + CookieManager.getInstance().removeAllCookies(null) + CookieManager.getInstance().flush() + WebStorage.getInstance().deleteAllData() + } + stash.clearToken() appContext.getSystemService(ActivityManager::class.java) ?.clearApplicationUserData() } diff --git a/app/src/test/java/net/spacealtctrl/discordrp/log/RedactorTest.kt b/app/src/test/java/net/spacealtctrl/discordrp/log/RedactorTest.kt new file mode 100644 index 0000000..18e3f82 --- /dev/null +++ b/app/src/test/java/net/spacealtctrl/discordrp/log/RedactorTest.kt @@ -0,0 +1,131 @@ +package net.spacealtctrl.discordrp.log + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class RedactorTest { + @Test + fun removesTokenShapedValues() { + val token = "${"M".repeat(24)}.${"Ga4BcD"}.${"a".repeat(38)}" + val mfa = "mfa.${"A".repeat(84)}" + + val redacted = Redactor.redact("token=$token\nmfa=$mfa") + assertFalse(redacted.contains(token)) + assertFalse(redacted.contains(mfa)) + assertTrue(redacted.contains("[redacted-token]")) + } + + @Test + fun removesCredentialHeaders() { + val report = """ + Authorization: Bearer eyJhbGciOiJIUzI1NiJ9.payload.signature + Cookie: session=abc123; discord_token=secret + Accept: application/json + """.trimIndent() + + val redacted = Redactor.redact(report) + assertFalse(redacted.contains("eyJhbGciOiJIUzI1NiJ9")) + assertFalse(redacted.contains("session=abc123")) + assertTrue(redacted.contains("Authorization: [redacted]")) + assertTrue(redacted.contains("Cookie: [redacted]")) + assertTrue(redacted.contains("Accept: application/json")) + } + + @Test + fun removesAuthenticatedUrlParts() { + val redacted = Redactor.redact( + "GET https://cdn.discordapp.com/a/b.png?ex=1&is=2&hm=deadbeef " + + "https://user:pass@discord.com/app?token=abc", + ) + assertFalse(redacted.contains("deadbeef")) + assertFalse(redacted.contains("token=abc")) + assertFalse(redacted.contains("user:pass")) + assertTrue(redacted.contains("ex=1&is=2")) + assertTrue(redacted.contains("https://[redacted]@discord.com/app")) + } + + @Test + fun removesAwsAndGoogleSignedUrlSecrets() { + val report = + "GET https://bucket.s3.amazonaws.com/a?X-Amz-Signature=deadbeef" + + "&X-Amz-Credential=AKIAEXAMPLE&X-Amz-Security-Token=secretvalue" + + "&X-Amz-Date=20260101T000000Z\n" + + "GET https://storage.googleapis.com/b?X-Goog-Signature=cafe" + + "&X-Goog-Credential=svc%2F20260101&GoogleAccessId=svc@example.test&Expires=1" + + val redacted = Redactor.redact(report) + assertFalse(redacted.contains("deadbeef")) + assertFalse(redacted.contains("AKIAEXAMPLE")) + assertFalse(redacted.contains("secretvalue")) + assertFalse(redacted.contains("cafe")) + assertFalse(redacted.contains("svc@example.test")) + assertTrue(redacted.contains("X-Amz-Date=20260101T000000Z")) + assertTrue(redacted.contains("Expires=1")) + } + + @Test + fun redactsStructuredCredentialFields() { + val report = """ + {"password":"hunter2","clientSecret":"abc123","api_key":"k-1"} + client_secret=shh + PASSWORD: swordfish + """.trimIndent() + + val redacted = Redactor.redact(report) + assertFalse(redacted.contains("hunter2")) + assertFalse(redacted.contains("abc123")) + assertFalse(redacted.contains("k-1")) + assertFalse(redacted.contains("shh")) + assertFalse(redacted.contains("swordfish")) + } + + @Test + fun redactsGenericStructuredTokensAndCookies() { + val report = """ + {"token":"short-test-secret"} + {"Cookie":"session=example-secret"} + {"set-cookie":"session=json-cookie"} + {"Authorization":"Basic dXNlcjpwYXNz"} + token=plainvalue + cookie=plaincookie + set_cookie=kv-cookie + """.trimIndent() + + val redacted = Redactor.redact(report) + assertFalse(redacted.contains("short-test-secret")) + assertFalse(redacted.contains("example-secret")) + assertFalse(redacted.contains("json-cookie")) + assertFalse(redacted.contains("dXNlcjpwYXNz")) + assertFalse(redacted.contains("plainvalue")) + assertFalse(redacted.contains("plaincookie")) + assertFalse(redacted.contains("kv-cookie")) + assertTrue(redacted.contains("\"token\"")) + assertTrue(redacted.contains("\"Cookie\"")) + assertTrue(redacted.contains("\"set-cookie\"")) + assertTrue(redacted.contains("\"Authorization\"")) + assertTrue(redacted.contains("set_cookie=")) + } + + @Test + fun redactsLongOpaqueRuns() { + val blob = "A".repeat(64) + val redacted = Redactor.redact("material=$blob setting=ok") + assertFalse(redacted.contains(blob)) + assertTrue(redacted.contains("[redacted]")) + assertTrue(redacted.contains("setting=ok")) + } + + @Test + fun leavesOrdinaryTextAlone() { + val report = """ + java.lang.IllegalStateException: update signature mismatch + at net.spacealtctrl.discordrp.update.UpdateIdentity.verify(UpdateIdentity.kt:31) + App version: 1.5.3 + See https://discord.com/app + """.trimIndent() + + assertEquals(report, Redactor.redact(report)) + } +} From 4ccb49d3f865f581c13a1c7cee2765f79f77556a Mon Sep 17 00:00:00 2001 From: lexi <173241226+cutieway@users.noreply.github.com> Date: Mon, 26 Oct 2026 02:17:30 +0200 Subject: [PATCH 2/3] Constrain the Discord login WebView to its trust boundary Load and navigate only canonical https://discord.com origins, and extract the token only from /app. Read storage through a same-origin iframe because Discord removes the storage API from the top window, then remove the iframe in the same turn. Tear the login WebView down in an order that cannot leave a live document behind: stop loading, replace it with about:blank, wait for the commit, clear storage, and only then flush cookies. Fail closed and destroy the view if the blank page never commits. Validate pasted token shape before the handshake, reject failed handshakes instead of crashing, and keep the manual-token and repaint fallbacks for WebViews that cannot render the login page. Add JVM coverage for origin matching, token shape, and cleanup ordering/timeout. --- .../discordrp/ui/screens/login/LoginScreen.kt | 142 ++++++++++- .../ui/screens/login/LoginWebPolicy.kt | 29 +++ .../ui/screens/login/SessionCleanup.kt | 149 ++++++++++++ .../ui/screens/login/TokenCapture.kt | 223 ++++++++++++++---- .../discordrp/ui/screens/login/TokenShape.kt | 15 ++ app/src/main/res/values/strings.xml | 7 + .../ui/screens/login/LoginWebPolicyTest.kt | 42 ++++ .../ui/screens/login/SessionCleanupTest.kt | 156 ++++++++++++ .../ui/screens/login/TokenShapeTest.kt | 25 ++ 9 files changed, 747 insertions(+), 41 deletions(-) create mode 100644 app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/LoginWebPolicy.kt create mode 100644 app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/SessionCleanup.kt create mode 100644 app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/TokenShape.kt create mode 100644 app/src/test/java/net/spacealtctrl/discordrp/ui/screens/login/LoginWebPolicyTest.kt create mode 100644 app/src/test/java/net/spacealtctrl/discordrp/ui/screens/login/SessionCleanupTest.kt create mode 100644 app/src/test/java/net/spacealtctrl/discordrp/ui/screens/login/TokenShapeTest.kt diff --git a/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/LoginScreen.kt b/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/LoginScreen.kt index 42c4056..7761fc0 100644 --- a/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/LoginScreen.kt +++ b/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/LoginScreen.kt @@ -10,6 +10,10 @@ import androidx.compose.animation.core.tween import androidx.compose.animation.fadeIn import androidx.compose.animation.fadeOut import androidx.compose.animation.togetherWith +import android.content.ClipData +import android.content.ClipboardManager +import android.content.Context +import android.os.Build import androidx.compose.foundation.background import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -20,25 +24,38 @@ import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.shape.CircleShape +import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.rounded.ArrowBack +import androidx.compose.material.icons.rounded.Visibility +import androidx.compose.material.icons.rounded.VisibilityOff +import androidx.compose.material3.AlertDialog import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme import androidx.compose.material3.ModalBottomSheet +import androidx.compose.material3.OutlinedTextField import androidx.compose.material3.Text +import androidx.compose.material3.TextButton import androidx.compose.material3.rememberModalBottomSheetState import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.graphics.graphicsLayer +import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.input.KeyboardType +import androidx.compose.ui.text.input.PasswordVisualTransformation +import androidx.compose.ui.text.input.VisualTransformation import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.IntSize import androidx.compose.ui.unit.dp @@ -88,9 +105,14 @@ class LoginViewModel @Inject constructor( fun onToken(token: String) { if (_stage.value == LoginStage.Verifying) return + if (!TokenShape.isPlausible(token)) { + _stage.value = LoginStage.Failed + return + } _stage.value = LoginStage.Verifying viewModelScope.launch { - _stage.value = if (handshake.run(token)) LoginStage.Done else LoginStage.Failed + val accepted = runCatching { handshake.run(token) }.getOrDefault(false) + _stage.value = if (accepted) LoginStage.Done else LoginStage.Failed } } } @@ -103,7 +125,17 @@ fun LoginScreen( viewModel: LoginViewModel = hiltViewModel(), ) { val stage by viewModel.stage.collectAsState() + val context = LocalContext.current val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) + var manualTokenOpen by remember { mutableStateOf(false) } + var manualToken by remember { mutableStateOf("") } + var manualTokenVisible by remember { mutableStateOf(false) } + + fun closeManualToken() { + manualTokenOpen = false + manualToken = "" + manualTokenVisible = false + } LaunchedEffect(stage) { if (stage == LoginStage.Done) onDone() @@ -231,6 +263,21 @@ fun LoginScreen( ) } } + + TextButton( + onClick = { + manualToken = "" + manualTokenVisible = false + manualTokenOpen = true + }, + enabled = stage != LoginStage.Verifying, + ) { + Text( + text = stringResource(R.string.login_token_manual), + style = MaterialTheme.typography.labelLarge, + color = Look.palette.inkFaint, + ) + } } } @@ -243,5 +290,98 @@ fun LoginScreen( TokenCapture(onToken = viewModel::onToken) } } + + if (manualTokenOpen) { + val cleanToken = manualToken.trim().removeSurrounding("\"").trim() + AlertDialog( + onDismissRequest = ::closeManualToken, + title = { Text(stringResource(R.string.login_token_manual_title)) }, + text = { + Column { + Text( + text = stringResource(R.string.login_token_manual_help), + style = MaterialTheme.typography.bodySmall, + color = Look.palette.inkFaint, + ) + OutlinedTextField( + value = manualToken, + onValueChange = { manualToken = it.trim() }, + singleLine = true, + keyboardOptions = KeyboardOptions( + keyboardType = KeyboardType.Password, + autoCorrectEnabled = false, + ), + label = { Text(stringResource(R.string.login_token_field_label)) }, + visualTransformation = if (manualTokenVisible) { + VisualTransformation.None + } else { + PasswordVisualTransformation() + }, + trailingIcon = { + IconButton( + onClick = { manualTokenVisible = !manualTokenVisible }, + ) { + Icon( + imageVector = if (manualTokenVisible) { + Icons.Rounded.VisibilityOff + } else { + Icons.Rounded.Visibility + }, + contentDescription = stringResource( + if (manualTokenVisible) { + R.string.login_token_hide + } else { + R.string.login_token_show + }, + ), + ) + } + }, + modifier = Modifier + .fillMaxWidth() + .padding(top = Look.gaps.snug), + ) + } + }, + confirmButton = { + TextButton( + enabled = cleanToken.isNotEmpty() && stage != LoginStage.Verifying, + onClick = { + closeManualToken() + clearClipboardIfMatches(context, cleanToken) + viewModel.onToken(cleanToken) + }, + ) { + Text(stringResource(R.string.login_token_manual_submit)) + } + }, + dismissButton = { + TextButton(onClick = ::closeManualToken) { + Text(stringResource(R.string.common_cancel)) + } + }, + ) + } + } +} + +/** + * A pasted token is a credential left behind in the clipboard. Clear it once + * the app has its copy, but only when the clipboard still holds exactly what + * was pasted. + */ +private fun clearClipboardIfMatches(context: Context, token: String) { + val clipboard = context.getSystemService(ClipboardManager::class.java) ?: return + val clip = clipboard.primaryClip ?: return + if (clip.itemCount == 0) return + val copied = clip.getItemAt(0).text?.toString() + ?.trim()?.removeSurrounding("\"")?.trim() + if (copied != token) return + runCatching { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { + clipboard.clearPrimaryClip() + } else { + clipboard.setPrimaryClip(ClipData.newPlainText("", "")) + } } } diff --git a/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/LoginWebPolicy.kt b/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/LoginWebPolicy.kt new file mode 100644 index 0000000..c0ec63f --- /dev/null +++ b/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/LoginWebPolicy.kt @@ -0,0 +1,29 @@ +package net.spacealtctrl.discordrp.ui.screens.login + +import java.net.URI + +/** + * Trust boundary for the embedded Discord login page. Only canonical HTTPS + * Discord origins can load in the login WebView, and only /app may trigger + * token extraction. + */ +internal object LoginWebPolicy { + private const val DEFAULT_HTTPS_PORT = 443 + + fun isDiscordUrl(raw: String?): Boolean { + val uri = raw?.let { runCatching { URI(it) }.getOrNull() } ?: return false + if (!"https".equals(uri.scheme, ignoreCase = true)) return false + if (uri.userInfo != null) return false + if (uri.port != -1 && uri.port != DEFAULT_HTTPS_PORT) return false + val host = uri.host?.lowercase() ?: return false + // Only the origin that serves the login application. Legacy + // discordapp.com links 301 to the same place, and neither the login page nor + // extraction needs any other Discord origin. + return host == "discord.com" + } + + fun isDiscordAppUrl(raw: String?): Boolean { + val uri = raw?.let { runCatching { URI(it) }.getOrNull() } ?: return false + return isDiscordUrl(raw) && uri.path?.trimEnd('/') == "/app" + } +} diff --git a/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/SessionCleanup.kt b/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/SessionCleanup.kt new file mode 100644 index 0000000..3703610 --- /dev/null +++ b/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/SessionCleanup.kt @@ -0,0 +1,149 @@ +package net.spacealtctrl.discordrp.ui.screens.login + +import java.util.concurrent.atomic.AtomicBoolean + +/** + * The operations a login WebView needs for teardown, split out so the ordering + * can be exercised without an Android runtime. + */ +internal interface SessionOps { + fun stopPage() + + fun loadBlank() + + /** Detaches the view; [destroyPage] refuses to run while it is attached. */ + fun detach() + + fun clearStorage() + + /** Must call [onDone] only after the cookie store is actually gone. */ + fun removeCookies(onDone: () -> Unit) + + fun flushCookies() + + fun clearCache() + + fun destroyPage() +} + +/** + * Tears a login WebView down in an order that cannot leak the document back: + * stop loading, replace the document with about:blank, wait until that + * navigation has finished (the old document is gone once it commits), wipe + * storage, and only after [SessionOps.removeCookies] reports completion flush + * the cookie store and consider the session cleaned. + * + * If the blank navigation never commits, the timeout path fails closed instead + * of pretending the commit happened: the view is detached and destroyed (which + * stops the still-live document) and only then are storage and cookies wiped. + * That path never delivers a capture callback. + * + * Both the successful capture path and the cancel path go through the same + * instance, so the work happens once. Destruction is deferred until cleanup is + * complete even when the composable is released first. + */ +internal class SessionCleanup( + private val ops: SessionOps, + private val scheduleTimeout: (action: () -> Unit) -> Unit, +) { + private val started = AtomicBoolean(false) + private var clearing = false + private var complete = false + private var destroyWhenComplete = false + private var destroyed = false + private var cancelled = false + private var onCleaned: (() -> Unit)? = null + + /** True once capture is cancelled or the fail-closed timeout has run. */ + val isCancelled: Boolean get() = cancelled + + fun start(onCleaned: (() -> Unit)? = null) { + // A capture callback arriving after cancellation must never be accepted; + // the token may have been read from a document that was never torn down. + if (cancelled) return + if (onCleaned != null) this.onCleaned = onCleaned + if (complete) { + deliver() + return + } + begin() + } + + fun cancel() { + cancelled = true + onCleaned = null + release() + } + + fun release() { + destroyWhenComplete = true + if (complete) { + destroy() + return + } + begin() + } + + /** Called for every page finish; only the blank document starts cleanup. */ + fun onPageFinished(url: String) { + if (url == BLANK_URL) clear() + } + + private fun begin() { + if (!started.compareAndSet(false, true)) return + ops.stopPage() + ops.loadBlank() + scheduleTimeout { failClosed() } + } + + private fun clear() { + if (clearing || complete) return + clearing = true + ops.clearStorage() + ops.removeCookies { + if (complete) return@removeCookies + ops.flushCookies() + complete = true + ops.clearCache() + deliver() + if (destroyWhenComplete) destroy() + } + } + + /** + * The blank document never committed, so the previous document may still be + * executing and could write the token back after a wipe. Destroying the view + * stops it; only then is storage safe to clear. + */ + private fun failClosed() { + if (complete || clearing) return + clearing = true + cancelled = true + onCleaned = null + ops.detach() + destroy() + ops.clearStorage() + ops.removeCookies { + if (complete) return@removeCookies + ops.flushCookies() + complete = true + } + } + + private fun deliver() { + if (cancelled) return + val callback = onCleaned + onCleaned = null + callback?.invoke() + } + + private fun destroy() { + if (destroyed) return + destroyed = true + ops.destroyPage() + } + + companion object { + const val BLANK_URL = "about:blank" + } +} diff --git a/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/TokenCapture.kt b/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/TokenCapture.kt index 93392a0..f11b9de 100644 --- a/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/TokenCapture.kt +++ b/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/TokenCapture.kt @@ -2,70 +2,213 @@ package net.spacealtctrl.discordrp.ui.screens.login import android.annotation.SuppressLint import android.os.Build +import android.os.Handler +import android.os.Looper import android.view.View import android.view.ViewGroup -import android.webkit.JsResult -import android.webkit.WebChromeClient +import android.webkit.CookieManager +import android.webkit.WebResourceRequest +import android.webkit.WebSettings +import android.webkit.WebStorage import android.webkit.WebView import android.webkit.WebViewClient import androidx.compose.runtime.Composable +import androidx.compose.runtime.remember import androidx.compose.ui.viewinterop.AndroidView +import org.json.JSONTokener +import java.util.concurrent.atomic.AtomicReference private const val LOGIN_URL = "https://discord.com/login" -private const val TOKEN_HOOK = - "javascript:(function()%7Bvar%20i%3Ddocument.createElement('iframe')%3B" + - "document.body.appendChild(i)%3B" + - "alert(i.contentWindow.localStorage.token.slice(1,-1))%7D)()" +// Discord's login page removes the storage API from the top window once its +// scripts run, but a same-origin iframe still reaches the same storage area. +// Read through the iframe and drop it again in the same turn. +private const val TOKEN_READ = + "(function(){var f=null;try{" + + "f=document.createElement('iframe');" + + "f.style.display='none';" + + "(document.documentElement||document.body).appendChild(f);" + + "var t=f.contentWindow.localStorage.getItem('token');" + + "if(!t)return '';" + + "try{return JSON.parse(t)}catch(e){return t}" + + "}catch(e){return ''}" + + "finally{if(f&&f.parentNode)f.parentNode.removeChild(f)}" + + "})()" + +private const val CAPTURE_INTERVAL_MS = 500L +private const val CAPTURE_WINDOW_MS = 20_000L + +// A blank document commits in milliseconds; the timeout only exists so a +// wedged renderer cannot keep the storage wipe from ever running. +private const val CLEANUP_TIMEOUT_MS = 3_000L private const val FALLBACK_UA = "Mozilla/5.0 (Linux; Android 14; SM-S921U; Build/UP1A.231005.007) " + "AppleWebKit/537.36 (KHTML, like Gecko) Chrome/129.0.0.0 Mobile Safari/537.363" +private fun tokenFromEvaluation(raw: String?): String? { + if (raw.isNullOrBlank() || raw == "null") return null + val value = runCatching { JSONTokener(raw).nextValue() as? String }.getOrNull() + ?: return null + return value.trim().removeSurrounding("\"").trim().takeIf { it.isNotEmpty() } +} + +private class WebViewSessionOps( + private val webView: WebView, + private val handler: Handler, +) : SessionOps { + override fun stopPage() { + webView.stopLoading() + } + + override fun loadBlank() { + webView.loadUrl(SessionCleanup.BLANK_URL) + } + + override fun detach() { + (webView.parent as? ViewGroup)?.removeView(webView) + } + + override fun clearStorage() { + // The WebView keeps a second copy of the token in localStorage while the + // user is signed in. The blank document has committed by now, so no + // Discord script is left to write it back. + WebStorage.getInstance().deleteAllData() + } + + override fun removeCookies(onDone: () -> Unit) { + CookieManager.getInstance().removeAllCookies { handler.post(onDone) } + } + + override fun flushCookies() { + CookieManager.getInstance().flush() + } + + override fun clearCache() { + webView.clearCache(true) + webView.clearHistory() + webView.clearFormData() + } + + override fun destroyPage() { + webView.destroy() + } +} + @SuppressLint("SetJavaScriptEnabled") @Composable fun TokenCapture(onToken: (String) -> Unit) { + val sessionRef = remember { AtomicReference(null) } AndroidView( onRelease = { webView -> - webView.stopLoading() - webView.destroy() + // The composable owns the view until here; detaching first keeps a + // deferred destroy legal. + (webView.parent as? ViewGroup)?.removeView(webView) + sessionRef.get()?.cancel() }, factory = { context -> - WebView(context).apply { - layoutParams = ViewGroup.LayoutParams( - ViewGroup.LayoutParams.MATCH_PARENT, - ViewGroup.LayoutParams.MATCH_PARENT, + val handler = Handler(Looper.getMainLooper()) + val webView = WebView(context) + val session = SessionCleanup( + ops = WebViewSessionOps(webView, handler), + scheduleTimeout = { action -> handler.postDelayed(action, CLEANUP_TIMEOUT_MS) }, ) - settings.javaScriptEnabled = true - settings.domStorageEnabled = true - if (Build.MANUFACTURER.equals("motorola", ignoreCase = true)) { - settings.userAgentString = FALLBACK_UA - } + sessionRef.set(session) + webView.apply { + layoutParams = ViewGroup.LayoutParams( + ViewGroup.LayoutParams.MATCH_PARENT, + ViewGroup.LayoutParams.MATCH_PARENT, + ) + settings.javaScriptEnabled = true + settings.domStorageEnabled = true + settings.allowFileAccess = false + settings.allowContentAccess = false + settings.javaScriptCanOpenWindowsAutomatically = false + settings.setSupportMultipleWindows(false) + settings.setGeolocationEnabled(false) + settings.mediaPlaybackRequiresUserGesture = true + settings.mixedContentMode = WebSettings.MIXED_CONTENT_NEVER_ALLOW + if (Build.MANUFACTURER.equals("motorola", ignoreCase = true)) { + settings.userAgentString = FALLBACK_UA + } + + webViewClient = object : WebViewClient() { + private var capturing = false + + private fun capture(view: WebView, url: String?) { + if (session.isCancelled) return + if (capturing || !LoginWebPolicy.isDiscordAppUrl(url)) return + capturing = true + val deadline = System.currentTimeMillis() + CAPTURE_WINDOW_MS + + // The token lands in storage a moment after Discord + // starts navigating to /app, so keep reading until the + // deadline instead of sampling a fixed number of times. + fun read() { + view.evaluateJavascript(TOKEN_READ) { result -> + if (session.isCancelled) return@evaluateJavascript + val token = tokenFromEvaluation(result) + if (token != null && !session.isCancelled && + LoginWebPolicy.isDiscordUrl(view.url) + ) { + // Hide the page and hand it to the cleanup + // sequence: it stops the page, swaps in + // about:blank, waits for that commit, then + // wipes storage and awaits cookie removal + // before the token is accepted. + view.visibility = View.GONE + session.start { onToken(token) } + return@evaluateJavascript + } + if (System.currentTimeMillis() < deadline && + LoginWebPolicy.isDiscordUrl(view.url) + ) { + view.postDelayed({ + read() + }, CAPTURE_INTERVAL_MS) + } else { + capturing = false + } + } + } - webViewClient = object : WebViewClient() { - @Deprecated("Deprecated in Java") - override fun shouldOverrideUrlLoading(view: WebView, url: String): Boolean { - stopLoading() - if (url.endsWith("/app")) { - loadUrl(TOKEN_HOOK) - visibility = View.GONE + read() + } + + override fun shouldOverrideUrlLoading( + view: WebView, + request: WebResourceRequest, + ): Boolean { + if (!request.isForMainFrame) return false + val url = request.url.toString() + if (!LoginWebPolicy.isDiscordUrl(url)) return true + if (LoginWebPolicy.isDiscordAppUrl(url)) { + view.stopLoading() + capture(view, url) + return true + } + return false + } + + // The first composited frame can stay white while the bottom + // sheet animates in; repaint once the page is committed. + override fun onPageCommitVisible(view: WebView, url: String) { + super.onPageCommitVisible(view, url) + view.postInvalidateOnAnimation() + } + + override fun onPageFinished(view: WebView, url: String) { + super.onPageFinished(view, url) + view.post { + view.requestLayout() + view.invalidate() + } + session.onPageFinished(url) + capture(view, url) } - return false - } - } - webChromeClient = object : WebChromeClient() { - override fun onJsAlert( - view: WebView, - url: String, - message: String, - result: JsResult, - ): Boolean { - onToken(message) - visibility = View.GONE - return true } + loadUrl(LOGIN_URL) } - loadUrl(LOGIN_URL) - } - }) + }, + ) } diff --git a/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/TokenShape.kt b/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/TokenShape.kt new file mode 100644 index 0000000..4783625 --- /dev/null +++ b/app/src/main/java/net/spacealtctrl/discordrp/ui/screens/login/TokenShape.kt @@ -0,0 +1,15 @@ +package net.spacealtctrl.discordrp.ui.screens.login + +/** + * Discord tokens are opaque, but they are always a compact run of URL-safe + * characters. Rejecting obvious garbage before a handshake keeps pasted text + * from being sent to Discord or stored. + */ +internal object TokenShape { + private val SHAPE = Regex("[A-Za-z0-9._-]{20,200}") + + fun isPlausible(raw: String?): Boolean { + val value = raw?.trim()?.removeSurrounding("\"")?.trim() ?: return false + return SHAPE.matches(value) + } +} diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 74c04e4..bf587da 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -134,6 +134,13 @@ You sign in on Discord\'s own page. Your login is stored only on this device. Sign in failed Discord did not accept the login. Try again. + Use an existing token + Use an existing token + On a computer signed in to Discord, open DevTools → Application → Local Storage → discord.com and copy the token value here. It stays on this device. + Token + Show token + Hide token + Sign in Back Cancel OK diff --git a/app/src/test/java/net/spacealtctrl/discordrp/ui/screens/login/LoginWebPolicyTest.kt b/app/src/test/java/net/spacealtctrl/discordrp/ui/screens/login/LoginWebPolicyTest.kt new file mode 100644 index 0000000..7deb742 --- /dev/null +++ b/app/src/test/java/net/spacealtctrl/discordrp/ui/screens/login/LoginWebPolicyTest.kt @@ -0,0 +1,42 @@ +package net.spacealtctrl.discordrp.ui.screens.login + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class LoginWebPolicyTest { + @Test + fun acceptsCanonicalDiscordHttpsOrigins() { + assertTrue(LoginWebPolicy.isDiscordUrl("https://discord.com/login")) + assertTrue(LoginWebPolicy.isDiscordUrl("https://discord.com/app")) + assertTrue(LoginWebPolicy.isDiscordAppUrl("https://discord.com/app")) + assertTrue(LoginWebPolicy.isDiscordAppUrl("https://discord.com/app/")) + assertTrue(LoginWebPolicy.isDiscordAppUrl("https://discord.com/app?from=login")) + } + + @Test + fun onlyAppPathTriggersTokenExtraction() { + assertFalse(LoginWebPolicy.isDiscordAppUrl("https://discord.com/login")) + assertFalse(LoginWebPolicy.isDiscordAppUrl("https://discord.com/app/messages")) + } + + @Test + fun onlyTheLoginOriginsAreTrusted() { + assertFalse(LoginWebPolicy.isDiscordUrl("https://canary.discord.com/app")) + assertFalse(LoginWebPolicy.isDiscordUrl("https://cdn.discord.com/app")) + assertFalse(LoginWebPolicy.isDiscordUrl("https://www.discordapp.com/app")) + assertFalse(LoginWebPolicy.isDiscordUrl("https://discordapp.com/login")) + assertFalse(LoginWebPolicy.isDiscordAppUrl("https://discordapp.com/app")) + assertFalse(LoginWebPolicy.isDiscordAppUrl("https://canary.discord.com/app")) + } + + @Test + fun rejectsLookalikeAndInsecureOrigins() { + assertFalse(LoginWebPolicy.isDiscordUrl("http://discord.com/app")) + assertFalse(LoginWebPolicy.isDiscordUrl("https://discord.com.evil.test/app")) + assertFalse(LoginWebPolicy.isDiscordUrl("https://notdiscord.com/app")) + assertFalse(LoginWebPolicy.isDiscordUrl("https://user@discord.com/app")) + assertFalse(LoginWebPolicy.isDiscordUrl("https://discord.com:8443/app")) + assertFalse(LoginWebPolicy.isDiscordAppUrl("https://evil.test/app")) + } +} diff --git a/app/src/test/java/net/spacealtctrl/discordrp/ui/screens/login/SessionCleanupTest.kt b/app/src/test/java/net/spacealtctrl/discordrp/ui/screens/login/SessionCleanupTest.kt new file mode 100644 index 0000000..4c3f66a --- /dev/null +++ b/app/src/test/java/net/spacealtctrl/discordrp/ui/screens/login/SessionCleanupTest.kt @@ -0,0 +1,156 @@ +package net.spacealtctrl.discordrp.ui.screens.login + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +class SessionCleanupTest { + private val ops = FakeOps() + private var timeout: (() -> Unit)? = null + private val cleanup = SessionCleanup(ops) { timeout = it } + + @Test + fun waitsForTheBlankDocumentBeforeWiping() { + var cleaned = 0 + cleanup.start { cleaned++ } + + assertEquals(listOf("stop", "blank"), ops.calls) + assertNull(ops.cookieCallback) + + cleanup.onPageFinished("https://discord.com/app") + assertEquals(listOf("stop", "blank"), ops.calls) + + cleanup.onPageFinished(SessionCleanup.BLANK_URL) + assertEquals(listOf("stop", "blank", "storage", "cookies"), ops.calls) + assertEquals(0, cleaned) + + ops.cookieCallback!!.invoke() + assertEquals(listOf("stop", "blank", "storage", "cookies", "flush", "cache"), ops.calls) + assertEquals(1, cleaned) + } + + @Test + fun releaseDefersDestructionUntilCookiesAreGone() { + cleanup.start() + cleanup.onPageFinished(SessionCleanup.BLANK_URL) + cleanup.release() + + assertEquals(false, ops.calls.contains("destroy")) + + ops.cookieCallback!!.invoke() + assertEquals(listOf("stop", "blank", "storage", "cookies", "flush", "cache", "destroy"), ops.calls) + + cleanup.release() + assertEquals(1, ops.calls.count { it == "destroy" }) + } + + @Test + fun bothPathsShareOneCleanup() { + var cleaned = 0 + cleanup.start { cleaned++ } + cleanup.release() + cleanup.start() + cleanup.onPageFinished(SessionCleanup.BLANK_URL) + cleanup.onPageFinished(SessionCleanup.BLANK_URL) + + assertEquals(1, ops.calls.count { it == "stop" }) + assertEquals(1, ops.calls.count { it == "storage" }) + assertEquals(1, ops.calls.count { it == "cookies" }) + + ops.cookieCallback!!.invoke() + assertEquals(1, cleaned) + assertEquals(1, ops.calls.count { it == "destroy" }) + } + + @Test + fun timeoutFailsClosedInsteadOfWipingALivePage() { + var cleaned = 0 + cleanup.start { cleaned++ } + timeout!!.invoke() + + assertEquals(listOf("stop", "blank", "detach", "destroy", "storage", "cookies"), ops.calls) + assertTrue(cleanup.isCancelled) + + ops.cookieCallback!!.invoke() + assertEquals( + listOf("stop", "blank", "detach", "destroy", "storage", "cookies", "flush"), + ops.calls, + ) + assertEquals(0, cleaned) + + // A late blank commit or capture result must not restart the normal path. + cleanup.onPageFinished(SessionCleanup.BLANK_URL) + cleanup.start { cleaned++ } + assertEquals( + listOf("stop", "blank", "detach", "destroy", "storage", "cookies", "flush"), + ops.calls, + ) + assertEquals(0, cleaned) + } + + @Test + fun lateCaptureResultAfterCancelIsNotDelivered() { + var cleaned = 0 + cleanup.start { cleaned++ } + cleanup.cancel() + cleanup.onPageFinished(SessionCleanup.BLANK_URL) + + ops.cookieCallback!!.invoke() + assertTrue(cleanup.isCancelled) + + // The pending evaluateJavascript callback returns after cleanup finished. + cleanup.start { cleaned++ } + assertEquals(0, cleaned) + } + + @Test + fun cancellingWithoutACaptureStillCleansUp() { + cleanup.release() + assertEquals(listOf("stop", "blank"), ops.calls) + + cleanup.onPageFinished(SessionCleanup.BLANK_URL) + assertEquals(listOf("stop", "blank", "storage", "cookies"), ops.calls) + + ops.cookieCallback!!.invoke() + assertEquals("destroy", ops.calls.last()) + } + + private class FakeOps : SessionOps { + val calls = mutableListOf() + var cookieCallback: (() -> Unit)? = null + + override fun stopPage() { + calls += "stop" + } + + override fun loadBlank() { + calls += "blank" + } + + override fun detach() { + calls += "detach" + } + + override fun clearStorage() { + calls += "storage" + } + + override fun removeCookies(onDone: () -> Unit) { + calls += "cookies" + cookieCallback = onDone + } + + override fun flushCookies() { + calls += "flush" + } + + override fun clearCache() { + calls += "cache" + } + + override fun destroyPage() { + calls += "destroy" + } + } +} diff --git a/app/src/test/java/net/spacealtctrl/discordrp/ui/screens/login/TokenShapeTest.kt b/app/src/test/java/net/spacealtctrl/discordrp/ui/screens/login/TokenShapeTest.kt new file mode 100644 index 0000000..07a45d8 --- /dev/null +++ b/app/src/test/java/net/spacealtctrl/discordrp/ui/screens/login/TokenShapeTest.kt @@ -0,0 +1,25 @@ +package net.spacealtctrl.discordrp.ui.screens.login + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class TokenShapeTest { + @Test + fun acceptsDiscordStyleTokens() { + assertTrue(TokenShape.isPlausible("A".repeat(24) + "." + "B".repeat(6) + "." + "C".repeat(38))) + assertTrue(TokenShape.isPlausible("mfa." + "A".repeat(84))) + assertTrue(TokenShape.isPlausible("\"" + "a".repeat(40) + "\"")) + assertTrue(TokenShape.isPlausible(" " + "a".repeat(40) + " ")) + } + + @Test + fun rejectsObviousGarbage() { + assertFalse(TokenShape.isPlausible(null)) + assertFalse(TokenShape.isPlausible("")) + assertFalse(TokenShape.isPlausible("short")) + assertFalse(TokenShape.isPlausible("token with spaces and enough length")) + assertFalse(TokenShape.isPlausible("A".repeat(201))) + assertFalse(TokenShape.isPlausible("line\nbreak")) + } +} From aaf89265fc32f41b2b1d7a47d38b579c466d6aec Mon Sep 17 00:00:00 2001 From: lexi <173241226+cutieway@users.noreply.github.com> Date: Mon, 26 Oct 2026 02:19:03 +0200 Subject: [PATCH 3/3] Validate gateway resume URLs and update artifacts Accept only Discord gateway hosts for resume endpoints and re-append the gateway protocol query, since the endpoint arrives over the gateway itself. Read the token through a provider so the engine does not retain a plaintext copy, clear the credential on terminal auth rejection or an unreadable blob, and drop it on an unauthorized API response. Require update releases from the project's GitHub feed, validate the asset URL at parse and download time, cap the transfer size, verify the published SHA-256 digest, and check the archive package id and signing certificate against the installed app before invoking the installer. Disable cleartext traffic, drop the privileged media permission, and stop exporting the boot receiver. Add JVM tests for gateway endpoints and update URL/digest policy, plus instrumentation fixtures for foreign, multi-signer, and unsigned update APKs. --- .../assets/update-foreign-signer.apk | Bin 0 -> 8536 bytes .../assets/update-multi-signer.apk | Bin 0 -> 8665 bytes .../androidTest/assets/update-unsigned.apk | Bin 0 -> 631 bytes .../discordrp/update/UpdateIdentityTest.kt | 83 ++++++++++++++ app/src/main/AndroidManifest.xml | 14 ++- .../discordrp/discord/FirstHandshake.kt | 2 +- .../discordrp/gateway/GatewayEndpoint.kt | 41 +++++++ .../discordrp/gateway/GatewayEngine.kt | 13 ++- .../discordrp/service/BridgeService.kt | 26 ++++- .../discordrp/update/UpdateHub.kt | 98 +++++++++++++---- .../discordrp/update/UpdateIdentity.kt | 74 +++++++++++++ .../discordrp/update/UpdatePolicy.kt | 57 ++++++++++ .../main/res/xml/network_security_config.xml | 11 ++ .../discordrp/gateway/GatewayEndpointTest.kt | 48 ++++++++ .../discordrp/update/UpdatePolicyTest.kt | 103 ++++++++++++++++++ 15 files changed, 534 insertions(+), 36 deletions(-) create mode 100644 app/src/androidTest/assets/update-foreign-signer.apk create mode 100644 app/src/androidTest/assets/update-multi-signer.apk create mode 100644 app/src/androidTest/assets/update-unsigned.apk create mode 100644 app/src/androidTest/java/net/spacealtctrl/discordrp/update/UpdateIdentityTest.kt create mode 100644 app/src/main/java/net/spacealtctrl/discordrp/gateway/GatewayEndpoint.kt create mode 100644 app/src/main/java/net/spacealtctrl/discordrp/update/UpdateIdentity.kt create mode 100644 app/src/main/java/net/spacealtctrl/discordrp/update/UpdatePolicy.kt create mode 100644 app/src/main/res/xml/network_security_config.xml create mode 100644 app/src/test/java/net/spacealtctrl/discordrp/gateway/GatewayEndpointTest.kt create mode 100644 app/src/test/java/net/spacealtctrl/discordrp/update/UpdatePolicyTest.kt diff --git a/app/src/androidTest/assets/update-foreign-signer.apk b/app/src/androidTest/assets/update-foreign-signer.apk new file mode 100644 index 0000000000000000000000000000000000000000..421d0dc08e125b08f0d992138c2cc994e22bc5b8 GIT binary patch literal 8536 zcmeHNc|6qX_n(C!OQ!5uqhvQ`P`2z-ni+#hQlT(}7)E5tdJ#!NN|YsAkz{8mlC2DC z?0XU=JJ-H_KW?|z?RLMvxRg}!tt6`!$M5VPSVG4 zSnRF1rTW@h9#x1qBme!1w%(|VPM3y{yqH}dpXt=w%NRaAHbs5qg*sZ{lXOaD@laZ* zhZ`*+a|Ag;3KVr=wy2i!WJpeNNEdK{3Rd$y`KoWOceA7GY+#F-Dk379xmckfn$n99 z4bcrnnA)&hRZqL8w4IW0U1lYBYTOVf@NsnZcD|k!IGfQ)NNOr4E1372?s3!@M!n#K zxOIWuBChKOn-DD!pB~X-`Q&^7C&c60ndt6ymRa`fTAR`*9YleeBCX)AL!~4ioY9~L$@1e$Hj;9P!ZEr31_);cpEa5NHYc&e#UZXeQx|!&zTHNUFJ-#^tcimOvcEB zs)XTh_POP5*fm8=%QCCspW)aE$w3gap%S||(A&|ypgkZm(T(IzwkEpCULw0$EB4TX z{)im}#PY|K9V0M^A2@yzAeukMJI*7Ou=C7{h)J~#&3kn(9!VVXOR3}+jDcDx1 zDl2aufW3De>FMd8>!u*{m4~FpEMxF(% z-;}<^g&W_NK40~sbIk96sp_g(mfJ0|cN3vxHKoUpNkn)WCAnNHr_It zTrj+^Q|{c+V72S-td7pttks$H4Vei(Sbg*1Vm4Z+`q+iUHyR>AWnJ^W1U78p{o@O- zVmkUtqznqBT)}_GV8vCU2o7Lq2PlBdKQS;SXrV4!>D!L9J1ksYZ)s|5DM|@HTnlJG zc=U%meF3qNCW%oG^l!$!4&X}-bJb#RGyhF2GZBWn#lVd~$s^@m8wb2)-&8p^h(ImM zRM3JcO7=SXP*ibsO|IZB2Z;l3sIB~RFt5Dv4%nm#a%6| z;a!aH>hj9NjrQ~)(Pv;|VrV~D$}w&NyQt`!d@tr6`p#uZC)Htgcz@j7z(K_3o7=KO z<4u;%`K=nH3JJvXlC@IBt}YAhh%nFb;o|4W0-E2#br1Ly5` z^^bV*gZxdL)Slc9<(@d5iVxjo6Sj#nw*~LjEMWFaC&|etZg;HSSqTX_`t0S5X?GDB zL!44s@Tkd+=}@KKZqRF7ay;k1DdE%U$L-TGTC{q;$GP=nVdNv*4_D6}ws~r^)=a^e z%oTEMNeU`YH*#$EE{ugZYM~`guv8t9kx>!Z*fhT9n@X&|-L)V9eP?BdXHT%ra4lNG ze12`}nllL=r?byr8ZWVo&BIWsL90> zTyS05N?Js&#pkVE-Epi*&sm{oH7#j{1)lHhi+HSIA?7Fgn6vKk>1F1=BuLMTF&GpP z%hx8|iXC)H@O?;u5#9#TyFnPDn6%`}dEE;(c}u~?4w3$Q3*rae9j#)9R5rsrb7unm zu@DlrE4$r^t>*wom144KEn1g?U73klx;nadeIqN;Vj3ASajWl|GAg#|a!6L34sTS) z)AhH_Im+|Y z*6vkD+e2w@o2pqU+HZ&4N_|_BODE&EGM;Mr&jcIH&#ilo)y2X~^iWx{D8&VhCO41P zQ2njtLBE1#6z!bX!SF9xRa9z1GGPQyigx#0s@oXNznO+g5Uku+^*Y14yY?#s_M}Di zg^MY-kMfFSTu-M4CCHj3@bJZYu3@%Df5 z#Y;OJuP7H88tHjhzC7La5V4q4OYk*#um65Kb7P9imo2Dvp2S~SMtJEpU|JrDwlwQH zZkZ}LKw<;%|&-Z!Y^^HrwJ_dXuH=~g1MAeeH`zHFt>>l{JUPD5l-f!@dp;f4U8P}7W%=?m93nmX zTky8o>CT&La&qL?I*Hbf=cgw()#_~XY>Q9cmth9|ok1>d!DYe#=12$hZw!LdGSEls z5l+kE&8u1tFweS|&5;n3Gb&y%G#BTAGRm3q_yo$0m6cu*s>CUo7CD(y1~Q!( zX-NY#szzS@Xa=GVrQO$Db*T9;s!J-Xv9Gp=hD=BBUHRj$0=UXu*+pO%fn5Z45!gjw z7lHqF1XkF-Bj85=y*+SakEXm95%Y)tg9Ctf3x5rW*TNF{j$Qc+$Rg+M5s z<7P$e43U2h|99gcu*lCJ2Brr=gTQRSb%B{6L0~YbJ}H#}o);-n(ug%A+`PQ3X=FEB zb`y6$YrIH<)T&-!WZ~3_K=7Q!9r*WRObSL$oqKYMpww8Y*8nEm-w5 zu>A7p$$F=KS1KgKWAzxN0#T@&J)yX=TXe3i=@*ZENmoi<)T!ZqpW3a88S+#MVLsqM zp-$cDy_!>|)L?N|AWA}YaaEvMgTmn5W}93L>x;KlH@|l*sOiw@3|_k)Kbp5?g7D_S z+6FC%<%E9E`%Yt|O|AU0RR+11eVOK<;i`POY$939(8E0O-u`)d+BV%}#gNFm$DCu0qSfg-#uj`I2RLFw zGo?}S4QE9N?)-NiySIPl=6ES}Cs!U zSxjQ(P~+h9VBbu?VJ8}rzly3TfAzsw`W2=(O1#*U{QHDWc^zXqw~?x3z^a$ zi>0JZp8=2}LqB6MUVw2T&AKDzL}-D@`o36_6u;2n&!PjDYSW3a|?UlDkAU zKJiHvZkjxCjd|R1uH2EV&Yf3qPCQa{j)I&Y3VVSoVjAGjE>w^^UOMrX;yU=W|LwzM z-;)nrj1*;#}3+v_ExA-I|drH((D8Q-QWn zw?1s5@%&@r0MleLx>L(2kRT4nQ-!_A9LM=u^iU!tGHLNnKHM|IAz5HK!HJxGk1C0! zlP<^XX{aPp`URH+wgY+gXwD&}Li$QIMheQ??+kD6>COPhV{P9ykl!Aifl=cZ>cjvHfMnZE z!v8Y~e^77rWy0(a>OHtiy}Q)AOTGW3ULJs5x3{-)2E0C+7*=el%s&&P}&a^G* z#Kf9Ql2AZ>rsYE&+xw5lqZ|%Cf@CPe(?Dla>5S{fx;rD*ANP9d=D!T}(qt-3)UT`) zDEC<~iDwBh`;13lZ}8#zRT7Wv;b2R7gtf1btR00FkcJpAuv*`d?^WSnp8)v&-jCOR z^E|*`4t6H}TC@IPxYI@fD}ujQuy<@ZcFHFILrMBC>q)@&-xR2Sb?|FF_b&&tz_r|OO&h90&viyk<%fy)F5?~MKh=$Vgn literal 0 HcmV?d00001 diff --git a/app/src/androidTest/assets/update-multi-signer.apk b/app/src/androidTest/assets/update-multi-signer.apk new file mode 100644 index 0000000000000000000000000000000000000000..8e550d9de744b1dfa517a454e0c73f666763e379 GIT binary patch literal 8665 zcmeHNcRXC%_8%mO5~BCcRR%K#QG!T9!px{qQV_-(G*)TUhy|a+Wk?c@~SQ^@N4)9f=yqQWg z7lX$ z@upDhA-+evr$m-UG??{R7aqMfj%5`N?H4Rm9 zRZSfgwW}Ipx~fKF6(X0wO zq$BDB#nG0guz2W*Dv?5fbg_;ivwRxUX_m^<%1`XjTtu0Hh3VVx1&vC;My=#bP(-15 zH>F}oJKUEBzTjJAZ){r7@m3}`axE{CkqNo+Gz&P?Qz~+Ll9rJ-+-;vs(KFQ~)ft_D zf{!xh8;yMi6yHfywU;^bH6{;-E&)o0DP4V#!;@#XMh^rslZoNsB#a;kSOV9wJ#sV_wK*B3|9gPy`WCiJ+q~ zf_i|ot+A*oMElH;gsMLAtbpo0;^#7qy0pB!AKrM951szBC2S`@NlX1Xesz=^bnqck zYysu$@ z)Yg>3tU1QkPLCNX=$9Rezg*KjSh;ACyb{TgP`?hP0j7$BlaKm$qPK&B&gZ{dF&Hd$ zf+DeU>u&W0vHkLx$Yzxmob64&10nBE_ZYqVXG(W&4cT{HD~U+4=ytiuZ=Pem+ffbG zTP>kG6y}g!ZlOCGUY`xJRaO<|3D|f_XV;ka$udEsLjk$lQzjB!?6?x zovt&mb4J0j43A8yz0kS!nbq!Qt|;%!xEaisUfW7PHEH1O&qgf`n3ngIhTRv(E;29$ z7W)9rfIQ-z-mR96iZHz)L(csA&Q~SH9&Og8ObFs2qpKt2S#hi?nFa53folRJ8l}jp z-6iKTo6F^zBNFUW%+0)U0Z^m>Q25>2mD}b;xWF=-2*0z%iDRy|X0hXP2ah}oR_^=3 z2vM+s+&(+1Ar?BcRGLA9>XmBP_DU?yW#;VOepa&avP9To_=vkKdB6R=lkuUg>exWzMW|mONoaWIm&)3ff{S8dmgimkEzR5AX#D%sdc4xXSc3k zwZpBkd&M1)o|~TBA#+)13??Z}cS;i#`JnB1wr)0A%Uv<1(%g3J&E4zRxhlOV9b*%?VXb*g;zw|;=Y8)Pk*%eJ zk(Bh>Wwk{_xuEv)#A!P=#s0=D0Wn7*)2_EeYo1+0XSxbWQ-plZCBJU9spu$UUCBzL zGDSX1v8~3ned5gxBg1qVjN`!5hCY$L#&g0UGs%0M%pu@qzs9x+AHT0tPf#_+^aVSc zqE|lHWPS0<^fwPXq(9Q!JH5BCk=Ffe{=?)rcn;S@NRZ)?%9=c9v2K}_Qh=?go>%d% za}NicUBv)0aY17caFoQd&S{46X%CkYUg*-cltw80m7r+1abJZ$D~NwxP^%A=KZ!mbg7$KJD!+&EoK&hQ^14zwJhJwq_NgF5LEZ zjjzFcv3@LGl58EQq+z>v(8)6)Q?VeaXbVlv67k?AaXAaaUvJbIf{L0a zj`{~>f`Q{xN6QDY+fOdz4#QA2v6+(j4vrIQ(KSl$&2>e@1v-?} z?4|`-q(Ys^3d2x^=%WLH#MF)H?YVaGy_@uwcD3MDrvl4islJ5GwghIU@0vS>WEo?GblPDsn<7w)j?u|MG11S=tASr)%_Qn zh*5$VB5;h;sJi^o80z!ro_FiW2zId|^}3quJ1_m=T{SeUx)x3&eoowTqpBGwqLNeh zR^2i*dNbt19C>P`_>t>}h53ZbBG7vUH+Nh3A5XyQk08J234Q7TgvaFr&lJq>VSX&QUcsxBMivx~#Rfulh-L*3o~Vb}u_iPd;l|J{3H@eA{RML?MT$coU7A!E z)&kPL$jjhTrgUfbN@wy8H@9EV)9A`Nn zMef}Nuku~Pf~TDA?-?*#xT1Gny8A3n!%r}%)giU$BRq4a)73XUK34l6|hN zFcLCDA-X`OJx}xBg7oy|Jc(5q=vgYO^{Wz-svmA|5A$Gz)I~eRMb$M#KS6=Q8mf?b zu)_HaP$T5hNC%p)gC8;=lGQTOFw`TF4r0BF=eS?x7Xtrn2y9b*A5jC@_nPJ58OOKy z^IN9@0CxN?01$uqc=<_$UnO$k>yj^Kxjyxv+KnFO%pgW$ zBE3^YwA897C#1EdJ(mvB%^K;BbOLcOQc6gJ@y~OxECd9SzR5@lIc`e)X#RWU00N$$ ze~jQXfGB`~3V&Y&WP||(1c0X07bMh05j^EBFfH9stF4RLmftEv;ZL#_N|jJu3dP#S zzTSKCWSczzqBSQHCcSa&Hh*^ps@ye5Idei;y=^BLT(Zo8&beP@HNDhickXVDaL8j7 zQjz-*Na#>7yfXZhV^{henYnc7w9U))j2~YN%0tIJE(DRY*i>V%(ZeozmD0_|hU`&7 z@|!#C9ZJH-;;ys23kpS>0c>a?4pp#TkM4XKMW9=A)Q~k5+-u(Wyu;5G}B(d3$N7t1t`lV~% zG$Y+Rk>~<}`}=jLq()T(2sz81G%)(Y-yafEFG&^f-K)4>rejiuqNKY($LdCVkp9q; z_mL86rg))R+Bn3xw%6L4Od?xN?xGuoT-!L}K_037`I+09K)1~b)PeUXK#JruX`m)M z>8fyx!*wYiA`%ct>~Qqr0o6diA0$7+sY*A(m}gmXTcQS4~4J@wg%H)$_rzc{;bexb>u zF)?6biE)tZI3|8C6!@SZ#mjMU;CEgC9!;rb(w`Y!q+~_aceY6_L8lZ^_BS>KRF*{@ zUMZCI(yB5P_!Bq#s@hIlxPCd)U`2BY$&HY_IIT>C!fMqsUftfH5=f&EnklNKKc~UO zYrf^jg(Y~da7jnm-Q$jC;`FGJ$;Zi;{&et}O8J4%kw$J?^isWPt9Yi6%8NHEr{J+Y zjRGA@b_k&;GY$%b`JG2%4m8C}osUSvVu`h{Dr&PfYdr|Cj<32TLngd5u{_z%UN2@^8vHPF#){XfJ^mH!$S0&h+IkhfJ(M!8{+5i5me+ zkG$>UJ~}5{j|TdGTinR0ZG&cLgZ%{X7~li!SK$4}z`KE`SJwXny^iClb>us}4&aeS z|DadFO+$!q;G_E|f)&I9W&$yMr&lUnBm!k`{;$~e{9oDi{LO!>`=5MVoy3lN=C@b3 z>fS!WPQPN7iIm7iC)`GP@O*^Set$NCvr6$d;}X|Rw*z_p^B=EgPIteS zPCS#%VvtoQDwu>>zG~9*kZvy6MWfse1$#wZ-cOMoEGT=+Z~sg?3zYy;!5_DS8Xh~8 zM^xfA#^15=JBN01&(KA%>dW1GaS$2BcQ!aGzRB`+-~mysJ~p9XMdjHo-3L!04y%_6 zm=wG}IkTsIy;45>HIV*mtLWk|4skT_ttecaH|$?NUg@_Ue5$pU^Tj4+4kOaL`e|kuksq-h4Q= zUw1&tK0c4DHjDGmzH*V`4##*(W!)wyh{d2I&XgFd4*bA*BOg0bB+aE9uDdzvq!j$z z2zSoXY`S^9cOf!}JE>$arq@@^;+d+ z2@X613eI=E%K?+AVGoF>2VpaKMf^*(kPmnJdv2?33vU;`r+a_<21giA)xr4vnuxu0 z?8w8SvQ|Fx_PivEqS#%@P+Tmd3!}@I^>Aq$a~}WmUcfjS`IZk|4%1h&b_Xvtme%LE z?B?doBz;NV`AWtl6(SP;gIfV7$3Q{e)LHerr7s;TD)!8ad>MS$k zLj8+r4Hk-iU+rb{TPmto6)ry8MbFs1?-paBB$XR3^6-)xPhr9U$hRLcKR_f{X zG0w6=JE%T(N>SSpDg@CH3xlxIsKU0ZnIdW&}o!^1Tz(_kR|xS z`#NjbCQ|WrwQ1#cv6(HU{%tyzvj>85XdTkq>{Oo2cF$rP5-^p$O8YOJ-H|Wp;mDMW zU#OY%76j?Um;^^;)8ixN$$Se%82RcP&n3K}*`k@@b9;H#$_#gILWGp!0&6BsHR-AB z)2ak_)|DQGLkAtFUPyO4G&|Ka{;$aOSK#4iHTY@D?*%`Hy!6Mv=~w*=fnNyxLg4=^ z0)MRYm9?N;y4IFlm+qnvwp@}DT2KP~bn>`-&kCP>1K|62e|-MU`v89$IPP>ZXZ?rb zaUF$U9sD(eeXL7&yzlofQqq5FFX46nCP97Dz{#BMPX_R#{3iqW1jR`UCsR6qS`a7s z$-+MpJSX)}=0yI~7sCVmk30A?Lvqr<$sN%@4LDN#WZwf@@DmcCX literal 0 HcmV?d00001 diff --git a/app/src/androidTest/assets/update-unsigned.apk b/app/src/androidTest/assets/update-unsigned.apk new file mode 100644 index 0000000000000000000000000000000000000000..ea3940e14162a7981a9434b2f04c696937a19b04 GIT binary patch literal 631 zcmWIWW@cdk0uCTnWN?%J>J!Jvz+l77z#t41am-68%Fj&kP0Y(oOD!(ZtH{lnI?3Lj z$x-0w{UQa084gJg<+-?Xbu4^+e0rvSJ9gxVgf)`}hs^Zdmv8kaFN!Ripc3?c0|DzOoOV ze(2lin#8z@BUfJg?ds(%lCNCsjGuYiZgSU1-jx2;_j)_O{G~?!CEe%ZS43|TWhyHV zn|R|l&j;aEM^d)0xSA?^Xp3XZb(QV;)e@WiU$<_1E&sLQ>%Dn7%xNd|7FMm2dFj*t zEzin3@2S1z?DNai``mZl z0BCf8qC}B_JHSj>1IPklJ_bgHqSWI2(xT+lV!gzo;$#LU1|F~!NHHUW3=kp#U@XG; y0p5&E5E<;TjHC<5#1iKS13*gA-G!{ZA80MAD+9b)*+43ofUp)wKLM&>U;qGz`qVxE literal 0 HcmV?d00001 diff --git a/app/src/androidTest/java/net/spacealtctrl/discordrp/update/UpdateIdentityTest.kt b/app/src/androidTest/java/net/spacealtctrl/discordrp/update/UpdateIdentityTest.kt new file mode 100644 index 0000000..476a65f --- /dev/null +++ b/app/src/androidTest/java/net/spacealtctrl/discordrp/update/UpdateIdentityTest.kt @@ -0,0 +1,83 @@ +package net.spacealtctrl.discordrp.update + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Assert.fail +import org.junit.Test +import org.junit.runner.RunWith +import java.io.File + +@RunWith(AndroidJUnit4::class) +class UpdateIdentityTest { + private val context get() = InstrumentationRegistry.getInstrumentation().targetContext + private val testContext get() = InstrumentationRegistry.getInstrumentation().context + + @After + fun removeFixtures() { + File(context.cacheDir, FIXTURE_DIR).deleteRecursively() + } + + @Test + fun acceptsTheInstalledApk() { + UpdateIdentity.verify(context, File(context.applicationInfo.sourceDir)) + } + + @Test + fun rejectsAnApkForAnotherPackage() { + expectRejected( + File(testContext.applicationInfo.sourceDir), + "package id", + ) + } + + /** + * The critical negative: the archive carries our package id but was signed + * by someone else. Rotation is deliberately not trusted here either; since + * no APK signing lineage is verified, a rotated key is indistinguishable + * from a foreign one and must fail closed. + */ + @Test + fun rejectsTheSamePackageSignedByAnotherOrRotatedKey() { + val apk = asset("update-foreign-signer.apk") + assertEquals( + context.packageName, + context.packageManager.getPackageArchiveInfo(apk.absolutePath, 0)?.packageName, + ) + expectRejected(apk, "signature") + } + + @Test + fun rejectsTheSamePackageWithoutAnySigner() { + expectRejected(asset("update-unsigned.apk"), "signature") + } + + @Test + fun rejectsTheSamePackageWithExtraSigners() { + expectRejected(asset("update-multi-signer.apk"), "signature") + } + + private fun expectRejected(apk: File, messagePart: String) { + try { + UpdateIdentity.verify(context, apk) + fail("${apk.name} must not pass update verification") + } catch (e: IllegalStateException) { + assertTrue("unexpected reason: ${e.message}", e.message.orEmpty().contains(messagePart)) + } + } + + private fun asset(name: String): File { + val dir = File(context.cacheDir, FIXTURE_DIR).apply { mkdirs() } + val file = File(dir, name) + testContext.assets.open(name).use { input -> + file.outputStream().use { output -> input.copyTo(output) } + } + return file + } + + private companion object { + const val FIXTURE_DIR = "identity-fixtures" + } +} diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 6f68969..1038e93 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -6,14 +6,14 @@ + - + @@ -26,6 +26,7 @@ android:icon="@mipmap/ic_launcher" android:label="@string/app_name_short" android:localeConfig="@xml/locales" + android:networkSecurityConfig="@xml/network_security_config" android:roundIcon="@mipmap/ic_launcher" android:supportsRtl="true" android:theme="@style/Theme.DiscordRP" @@ -58,6 +59,8 @@ android:name=".ui.crash.CrashActivity" android:exported="false" android:process=":crash" /> + + + android:exported="false"> diff --git a/app/src/main/java/net/spacealtctrl/discordrp/discord/FirstHandshake.kt b/app/src/main/java/net/spacealtctrl/discordrp/discord/FirstHandshake.kt index d5badd8..5fc7bc9 100644 --- a/app/src/main/java/net/spacealtctrl/discordrp/discord/FirstHandshake.kt +++ b/app/src/main/java/net/spacealtctrl/discordrp/discord/FirstHandshake.kt @@ -13,7 +13,7 @@ class FirstHandshake @Inject constructor( private val stash: Stash, ) { suspend fun run(token: String): Boolean { - val probe = GatewayEngine(token = token, mobileBadge = false, autoRedial = false) + val probe = GatewayEngine(tokenProvider = { token }, mobileBadge = false, autoRedial = false) return try { val outcome = coroutineScope { val firstAnswer = async { diff --git a/app/src/main/java/net/spacealtctrl/discordrp/gateway/GatewayEndpoint.kt b/app/src/main/java/net/spacealtctrl/discordrp/gateway/GatewayEndpoint.kt new file mode 100644 index 0000000..562d0d6 --- /dev/null +++ b/app/src/main/java/net/spacealtctrl/discordrp/gateway/GatewayEndpoint.kt @@ -0,0 +1,41 @@ +package net.spacealtctrl.discordrp.gateway + +import java.net.URI + +/** + * Resume endpoints arrive over the gateway itself, so they are validated + * before a credential-bearing reconnect can use them. + */ +internal object GatewayEndpoint { + const val DEFAULT_URL = "wss://gateway.discord.gg/?v=10&encoding=json" + + private const val QUERY = "?v=10&encoding=json" + private const val DEFAULT_WSS_PORT = 443 + + fun resumeUrl(raw: String?): String? { + val uri = raw?.let { runCatching { URI(it) }.getOrNull() } ?: return null + if (!"wss".equals(uri.scheme, ignoreCase = true)) return null + if (uri.userInfo != null) return null + if (uri.port != -1 && uri.port != DEFAULT_WSS_PORT) return null + if (uri.rawQuery != null || uri.fragment != null) return null + + val path = uri.path.orEmpty() + if (path.isNotEmpty() && path != "/") return null + + val host = uri.host?.lowercase() ?: return null + if (!isGatewayHost(host)) return null + return "wss://$host/$QUERY" + } + + private fun isGatewayHost(host: String): Boolean { + // Anchored to Discord's own three-label hosts: gateway.discord.gg or a + // sharded name such as gateway-us-east1-b.discord.gg. Suffix checks + // alone would also accept crafted names beneath discord.gg. + val labels = host.split('.') + if (labels.size != 3) return false + if (labels[1] != "discord" || labels[2] != "gg") return false + return labels[0] == "gateway" || SHARDED_LABEL.matches(labels[0]) + } + + private val SHARDED_LABEL = Regex("gateway-[a-z0-9]+(?:-[a-z0-9]+)*") +} diff --git a/app/src/main/java/net/spacealtctrl/discordrp/gateway/GatewayEngine.kt b/app/src/main/java/net/spacealtctrl/discordrp/gateway/GatewayEngine.kt index 6087c2c..950f7ef 100644 --- a/app/src/main/java/net/spacealtctrl/discordrp/gateway/GatewayEngine.kt +++ b/app/src/main/java/net/spacealtctrl/discordrp/gateway/GatewayEngine.kt @@ -32,7 +32,7 @@ import net.spacealtctrl.discordrp.log.SilentLog import kotlin.random.Random class GatewayEngine( - private val token: String, + private val tokenProvider: () -> String, private val mobileBadge: Boolean, private val autoRedial: Boolean = true, fallbackSelfId: String? = null, @@ -231,7 +231,10 @@ class GatewayEngine( "READY" -> { val ready = GatewayCodec.read(frame) ?: return sessionId = ready.sessionId - resumeUrl = ready.resumeUrl?.let { "$it/?v=10&encoding=json" } + resumeUrl = GatewayEndpoint.resumeUrl(ready.resumeUrl) + if (ready.resumeUrl != null && resumeUrl == null) { + log.warn(TAG, "Ignoring a resume endpoint outside Discord's gateway hosts") + } ready.user?.id?.let { selfId = it } sessionUp = true _linked.value = true @@ -260,7 +263,7 @@ class GatewayEngine( transmit( Op.IDENTIFY, GatewayCodec.encode( - IdentifyRequest(token = token, properties = ClientStamp.forMobileBadge(mobileBadge)) + IdentifyRequest(token = tokenProvider(), properties = ClientStamp.forMobileBadge(mobileBadge)) ), ) } @@ -270,7 +273,7 @@ class GatewayEngine( transmit( Op.RESUME, GatewayCodec.encode( - ResumeRequest(token = token, sessionId = sessionId.orEmpty(), seq = seq) + ResumeRequest(token = tokenProvider(), sessionId = sessionId.orEmpty(), seq = seq) ), ) } @@ -324,7 +327,7 @@ class GatewayEngine( private companion object { const val TAG = "Gateway" - const val GATEWAY_URL = "wss://gateway.discord.gg/?v=10&encoding=json" + const val GATEWAY_URL = GatewayEndpoint.DEFAULT_URL const val APP_CLOSE_CODE: Short = 4000 diff --git a/app/src/main/java/net/spacealtctrl/discordrp/service/BridgeService.kt b/app/src/main/java/net/spacealtctrl/discordrp/service/BridgeService.kt index ad3e822..da49284 100644 --- a/app/src/main/java/net/spacealtctrl/discordrp/service/BridgeService.kt +++ b/app/src/main/java/net/spacealtctrl/discordrp/service/BridgeService.kt @@ -17,6 +17,7 @@ import android.media.session.MediaController import android.media.session.MediaSessionManager import android.media.session.PlaybackState import android.os.Build +import android.os.Debug import android.os.Handler import android.os.IBinder import android.os.Looper @@ -33,6 +34,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.launch +import net.spacealtctrl.discordrp.BuildConfig import net.spacealtctrl.discordrp.R import net.spacealtctrl.discordrp.gateway.Gateway import net.spacealtctrl.discordrp.gateway.GatewayEngine @@ -82,11 +84,27 @@ class BridgeService : Service() { ensureStatusChannel() goForeground(statusNotification(track = null)) + // Every start path goes through startForegroundService, so the + // foreground obligation must be met before this early stop. + if (BuildConfig.DEBUG && Debug.isDebuggerConnected()) { + log.error(TAG, "Debugger attached; refusing to run the bridge") + stopSelf() + return + } + if (!stash.signedIn) { log.error(TAG, "No account on file; the bridge cannot start") stopSelf() return } + if (stash.authToken.isBlank()) { + // The sealed blob is present but cannot be opened (key lost or + // invalidated); drop it instead of dialing with an empty token. + log.error(TAG, "Stored login is unreadable; clearing it") + stash.clearToken() + stopSelf() + return + } _alive.value = true lastUpAt = System.currentTimeMillis() @@ -140,7 +158,7 @@ class BridgeService : Service() { private fun openGateway() { val engine = GatewayEngine( - token = stash.authToken, + tokenProvider = stash::authToken, mobileBadge = stash.mobileBadge, fallbackSelfId = stash.selfId.takeIf { it.isNotBlank() }, log = AndroidAppLog, @@ -173,7 +191,11 @@ class BridgeService : Service() { } private fun onAuthRejected() { - log.error(TAG, "Discord no longer accepts the stored login; stopping") + log.error(TAG, "Discord no longer accepts the stored login; clearing it and stopping") + // The close code is terminal, so the credential will never work again. + // Dropping the sealed blob keeps the app from redialing a dead token and + // makes the next launch land on the setup screen. + stash.clearToken() notificationManager().notify( STATUS_NOTICE_ID, statusBuilder() diff --git a/app/src/main/java/net/spacealtctrl/discordrp/update/UpdateHub.kt b/app/src/main/java/net/spacealtctrl/discordrp/update/UpdateHub.kt index 4e5c687..19dbb19 100644 --- a/app/src/main/java/net/spacealtctrl/discordrp/update/UpdateHub.kt +++ b/app/src/main/java/net/spacealtctrl/discordrp/update/UpdateHub.kt @@ -12,6 +12,7 @@ import io.ktor.client.request.header import io.ktor.client.request.prepareGet import io.ktor.client.statement.bodyAsChannel import io.ktor.http.HttpHeaders +import io.ktor.http.HttpStatusCode import io.ktor.utils.io.readAvailable import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope @@ -29,10 +30,11 @@ import kotlinx.serialization.Serializable import net.spacealtctrl.discordrp.BuildConfig import net.spacealtctrl.discordrp.log.AppLog import java.io.File +import java.security.MessageDigest import javax.inject.Inject import javax.inject.Singleton -data class AppUpdate(val version: String, val apkUrl: String) +data class AppUpdate(val version: String, val apkUrl: String, val sha256: String) sealed interface UpdateState { data object Idle : UpdateState @@ -95,15 +97,33 @@ class UpdateHub @Inject constructor( } private suspend fun fetchLatest(): UpdateState = try { - val release: FeedRelease = http.get(FEED_URL) { + val response = http.get(UpdatePolicy.FEED_URL) { header(HttpHeaders.Accept, "application/vnd.github+json") - }.body() + } + if (response.status == HttpStatusCode.NotFound) return UpdateState.UpToDate + val release: FeedRelease = response.body() val version = release.tagName.removePrefix("v") val apk = release.assets.firstOrNull { it.name.endsWith(".apk") } + ?: return UpdateState.UpToDate + if (!UpdatePolicy.isTrustedApkUrl(apk.downloadUrl)) { + log.warn(TAG, "Release asset is not on the expected GitHub path") + return UpdateState.Failed + } when { - apk == null -> UpdateState.UpToDate - VersionOrder.isNewer(version, BuildConfig.VERSION_NAME) -> - UpdateState.Available(AppUpdate(version, apk.downloadUrl)) + VersionOrder.isNewer(version, BuildConfig.VERSION_NAME) -> { + val digest = UpdatePolicy.sha256(apk.digest) + if (digest == null) { + log.warn(TAG, "Release asset has no valid SHA-256 digest") + return UpdateState.Failed + } + UpdateState.Available( + AppUpdate( + version = version, + apkUrl = apk.downloadUrl, + sha256 = digest, + ), + ) + } else -> UpdateState.UpToDate } } catch (e: CancellationException) { @@ -114,25 +134,56 @@ class UpdateHub @Inject constructor( } private suspend fun download(update: AppUpdate): File { + // Rechecked here as well as when the feed was parsed: this is the point + // where the URL turns into a network request. + if (!UpdatePolicy.isTrustedApkUrl(update.apkUrl)) error("untrusted update url") val dir = File(context.cacheDir, UPDATES_DIR).apply { mkdirs() } - val file = File(dir, "DiscordRP-${update.version}.apk") - http.prepareGet(update.apkUrl) { - timeout { requestTimeoutMillis = DOWNLOAD_TIMEOUT_MS } - }.execute { response -> - if (response.status.value !in 200..299) { - error("download answered ${response.status.value}") - } - file.outputStream().use { out -> - val channel = response.bodyAsChannel() - val buffer = ByteArray(DOWNLOAD_CHUNK_BYTES) - while (true) { - val read = channel.readAvailable(buffer, 0, buffer.size) - if (read == -1) break - if (read > 0) out.write(buffer, 0, read) + val safeVersion = update.version.replace(Regex("[^A-Za-z0-9._-]"), "_") + val file = File(dir, "DiscordRP-$safeVersion.apk") + try { + val digest = MessageDigest.getInstance("SHA-256") + var written = 0L + http.prepareGet(update.apkUrl) { + timeout { requestTimeoutMillis = DOWNLOAD_TIMEOUT_MS } + }.execute { response -> + if (response.status.value !in 200..299) { + error("download answered ${response.status.value}") + } + val resolved = response.call.request.url.toString() + if (!UpdatePolicy.isTrustedDownloadUrl(update.apkUrl, resolved)) { + error("download redirected to an untrusted host") + } + val declared = response.headers[HttpHeaders.ContentLength]?.toLongOrNull() + if (declared != null && (declared < 0 || declared > MAX_APK_BYTES)) { + error("update size is outside the accepted range") + } + file.outputStream().use { out -> + val channel = response.bodyAsChannel() + val buffer = ByteArray(DOWNLOAD_CHUNK_BYTES) + while (true) { + val read = channel.readAvailable(buffer, 0, buffer.size) + if (read == -1) break + if (read > 0) { + written += read + if (written > MAX_APK_BYTES) { + error("update size is outside the accepted range") + } + out.write(buffer, 0, read) + digest.update(buffer, 0, read) + } + } } } + val expected = update.sha256 + val actual = digest.digest().joinToString("") { "%02x".format(it.toInt() and 0xff) } + if (actual != expected) error("download digest mismatch") + UpdateIdentity.verify(context, file) + return file + } catch (e: Throwable) { + // No partial file may survive a failed or cancelled download. + file.delete() + throw e } - return file } private fun launchInstaller(apk: File) { @@ -148,6 +199,7 @@ class UpdateHub @Inject constructor( private data class FeedAsset( val name: String, @SerialName("browser_download_url") val downloadUrl: String, + val digest: String? = null, ) @Serializable @@ -159,12 +211,10 @@ class UpdateHub @Inject constructor( private companion object { const val TAG = "UpdateHub" - const val FEED_URL = - "https://api.github.com/repos/spacealtctrl/DiscordRP/releases/latest" - const val UPDATES_DIR = "updates" const val AUTO_CHECK_COOLDOWN_MS = 6 * 60 * 60 * 1000L const val DOWNLOAD_TIMEOUT_MS = 10 * 60 * 1000L const val DOWNLOAD_CHUNK_BYTES = 64 * 1024 + const val MAX_APK_BYTES = 100L * 1024 * 1024 } } diff --git a/app/src/main/java/net/spacealtctrl/discordrp/update/UpdateIdentity.kt b/app/src/main/java/net/spacealtctrl/discordrp/update/UpdateIdentity.kt new file mode 100644 index 0000000..24cddd0 --- /dev/null +++ b/app/src/main/java/net/spacealtctrl/discordrp/update/UpdateIdentity.kt @@ -0,0 +1,74 @@ +package net.spacealtctrl.discordrp.update + +import android.content.Context +import android.content.pm.PackageInfo +import android.content.pm.PackageManager +import android.os.Build +import androidx.annotation.RequiresApi +import java.io.File +import java.security.MessageDigest + +/** + * An update must be the same app, signed by the same key that installed the + * running build. Android enforces that at install time; checking it before + * handing the file to the installer keeps a foreign or re-signed package from + * ever being presented as an update, and turns the failure into a clear one. + */ +internal object UpdateIdentity { + fun verify(context: Context, apk: File) { + val pm = context.packageManager + val archive = pm.getPackageArchiveInfo(apk.absolutePath, signingFlags) + if (archive == null) { + // Some devices refuse to parse an archive at all when signatures are + // requested and the APK has none. Confirm the package if the file is + // otherwise readable, then fail on identity. + val readable = pm.getPackageArchiveInfo(apk.absolutePath, 0) + ?: error("unreadable update package") + if (readable.packageName != context.packageName) error("update package id mismatch") + error("update signature mismatch") + } + if (archive.packageName != context.packageName) { + error("update package id mismatch") + } + + val candidate = signerDigests(archive) + val installed = signerDigests(pm.getPackageInfo(context.packageName, signingFlags)) + // Signing-key rotation is intentionally not accepted. Verifying a + // lineage would let a rotated key pass, but without checking the APK's + // lineage against the installed certificate history a rotated key looks + // exactly like a foreign one, so identity failures stay fail-closed. + if (candidate.isEmpty() || installed.isEmpty() || candidate != installed) { + error("update signature mismatch") + } + } + + private val signingFlags: Int + get() = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { + // Some devices leave signingInfo unset when parsing an archive, so + // ask for the legacy field as well and fall back to it below. + PackageManager.GET_SIGNING_CERTIFICATES or PackageManager.GET_SIGNATURES + } else { + @Suppress("DEPRECATION") + PackageManager.GET_SIGNATURES + } + + private fun signerDigests(info: PackageInfo): Set { + val signers = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { + signersP(info) + } else { + legacySignatures(info) + }.orEmpty() + if (signers.isEmpty()) return emptySet() + return signers.mapTo(mutableSetOf()) { signature -> + MessageDigest.getInstance("SHA-256") + .digest(signature.toByteArray()) + .joinToString("") { "%02x".format(it.toInt() and 0xff) } + } + } + + @RequiresApi(Build.VERSION_CODES.P) + private fun signersP(info: PackageInfo) = info.signingInfo?.apkContentsSigners + + @Suppress("DEPRECATION") + private fun legacySignatures(info: PackageInfo) = info.signatures +} diff --git a/app/src/main/java/net/spacealtctrl/discordrp/update/UpdatePolicy.kt b/app/src/main/java/net/spacealtctrl/discordrp/update/UpdatePolicy.kt new file mode 100644 index 0000000..d723746 --- /dev/null +++ b/app/src/main/java/net/spacealtctrl/discordrp/update/UpdatePolicy.kt @@ -0,0 +1,57 @@ +package net.spacealtctrl.discordrp.update + +import java.net.URI + +/** + * Updates are distributed from the project's GitHub releases. The downloaded + * APK must also carry the SHA-256 digest published with the asset. + */ +internal object UpdatePolicy { + const val REPO = "spacealtctrl/DiscordRP" + const val FEED_URL = "https://api.github.com/repos/$REPO/releases/latest" + + private const val ASSET_PREFIX = "/$REPO/releases/download/" + private const val DEFAULT_HTTPS_PORT = 443 + private val SHA256 = Regex("[0-9a-f]{64}") + + // GitHub answers release-asset downloads with a redirect to a signed, + // short-lived object URL. Both hosts are first-party. + private val REDIRECT_HOSTS = setOf( + "objects.githubusercontent.com", + "release-assets.githubusercontent.com", + ) + + fun isTrustedApkUrl(raw: String?): Boolean { + val uri = raw?.let { runCatching { URI(it) }.getOrNull() } ?: return false + if (!"https".equals(uri.scheme, ignoreCase = true)) return false + if (!"github.com".equals(uri.host, ignoreCase = true)) return false + if (uri.userInfo != null) return false + if (uri.port != -1 && uri.port != DEFAULT_HTTPS_PORT) return false + if (uri.rawQuery != null || uri.fragment != null) return false + return uri.path?.startsWith(ASSET_PREFIX) == true + } + + /** + * Rechecked at download time. [resolved] is the URL the HTTP client actually + * ends up on after redirects; a redirect may only land on GitHub's signed + * asset host, and a direct answer must still be the trusted asset URL. + */ + fun isTrustedDownloadUrl(initial: String?, resolved: String?): Boolean { + if (!isTrustedApkUrl(initial)) return false + val uri = resolved?.let { runCatching { URI(it) }.getOrNull() } ?: return false + if (!"https".equals(uri.scheme, ignoreCase = true)) return false + if (uri.userInfo != null) return false + if (uri.port != -1 && uri.port != DEFAULT_HTTPS_PORT) return false + if (uri.fragment != null) return false + return when (uri.host?.lowercase()) { + "github.com" -> isTrustedApkUrl(resolved) + in REDIRECT_HOSTS -> true + else -> false + } + } + + fun sha256(raw: String?): String? { + val value = raw?.removePrefix("sha256:")?.trim()?.lowercase() ?: return null + return value.takeIf { SHA256.matches(it) } + } +} diff --git a/app/src/main/res/xml/network_security_config.xml b/app/src/main/res/xml/network_security_config.xml new file mode 100644 index 0000000..845156f --- /dev/null +++ b/app/src/main/res/xml/network_security_config.xml @@ -0,0 +1,11 @@ + + + + + + + + + diff --git a/app/src/test/java/net/spacealtctrl/discordrp/gateway/GatewayEndpointTest.kt b/app/src/test/java/net/spacealtctrl/discordrp/gateway/GatewayEndpointTest.kt new file mode 100644 index 0000000..4c2cce4 --- /dev/null +++ b/app/src/test/java/net/spacealtctrl/discordrp/gateway/GatewayEndpointTest.kt @@ -0,0 +1,48 @@ +package net.spacealtctrl.discordrp.gateway + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +class GatewayEndpointTest { + @Test + fun acceptsDiscordGatewayHostsAndAddsTheProtocolQuery() { + assertEquals( + "wss://gateway.discord.gg/?v=10&encoding=json", + GatewayEndpoint.resumeUrl("wss://gateway.discord.gg"), + ) + assertEquals( + "wss://gateway.discord.gg/?v=10&encoding=json", + GatewayEndpoint.resumeUrl("wss://gateway.discord.gg/"), + ) + assertEquals( + "wss://gateway-us-east1-b.discord.gg/?v=10&encoding=json", + GatewayEndpoint.resumeUrl("wss://gateway-us-east1-b.discord.gg"), + ) + assertEquals( + "wss://gateway.discord.gg/?v=10&encoding=json", + GatewayEndpoint.resumeUrl("wss://GATEWAY.DISCORD.GG:443"), + ) + } + + @Test + fun rejectsEndpointsThatCouldReceiveTheCredential() { + assertNull(GatewayEndpoint.resumeUrl(null)) + assertNull(GatewayEndpoint.resumeUrl("")) + assertNull(GatewayEndpoint.resumeUrl("ws://gateway.discord.gg")) + assertNull(GatewayEndpoint.resumeUrl("https://gateway.discord.gg")) + assertNull(GatewayEndpoint.resumeUrl("wss://evil.test")) + assertNull(GatewayEndpoint.resumeUrl("wss://discord.gg")) + assertNull(GatewayEndpoint.resumeUrl("wss://gateway.discord.gg.evil.test")) + assertNull(GatewayEndpoint.resumeUrl("wss://gateway.discord.gg.evil.discord.gg")) + assertNull(GatewayEndpoint.resumeUrl("wss://gateway.attacker.discord.gg")) + assertNull(GatewayEndpoint.resumeUrl("wss://gatewayevil.discord.gg")) + assertNull(GatewayEndpoint.resumeUrl("wss://gateway-.discord.gg")) + assertNull(GatewayEndpoint.resumeUrl("wss://gateway.discord.gg.")) + assertNull(GatewayEndpoint.resumeUrl("wss://user@gateway.discord.gg")) + assertNull(GatewayEndpoint.resumeUrl("wss://gateway.discord.gg:8443")) + assertNull(GatewayEndpoint.resumeUrl("wss://gateway.discord.gg/socket")) + assertNull(GatewayEndpoint.resumeUrl("wss://gateway.discord.gg?v=10")) + assertNull(GatewayEndpoint.resumeUrl("wss://gateway.discord.gg/#fragment")) + } +} diff --git a/app/src/test/java/net/spacealtctrl/discordrp/update/UpdatePolicyTest.kt b/app/src/test/java/net/spacealtctrl/discordrp/update/UpdatePolicyTest.kt new file mode 100644 index 0000000..59fe03f --- /dev/null +++ b/app/src/test/java/net/spacealtctrl/discordrp/update/UpdatePolicyTest.kt @@ -0,0 +1,103 @@ +package net.spacealtctrl.discordrp.update + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +class UpdatePolicyTest { + private val digest = "a".repeat(64) + + @Test + fun acceptsOnlyUpstreamReleaseAssets() { + assertTrue( + UpdatePolicy.isTrustedApkUrl( + "https://github.com/spacealtctrl/DiscordRP/releases/download/v1.6.0/app.apk", + ), + ) + assertFalse( + UpdatePolicy.isTrustedApkUrl( + "https://github.com/cutieway/DiscordRP/releases/download/v1.6.0/app.apk", + ), + ) + assertFalse( + UpdatePolicy.isTrustedApkUrl( + "http://github.com/spacealtctrl/DiscordRP/releases/download/v1.6.0/app.apk", + ), + ) + assertFalse( + UpdatePolicy.isTrustedApkUrl( + "https://github.com.evil.test/spacealtctrl/DiscordRP/releases/download/v1.6.0/app.apk", + ), + ) + assertFalse( + UpdatePolicy.isTrustedApkUrl( + "https://user@github.com/spacealtctrl/DiscordRP/releases/download/v1.6.0/app.apk", + ), + ) + assertFalse( + UpdatePolicy.isTrustedApkUrl( + "https://github.com:8443/spacealtctrl/DiscordRP/releases/download/v1.6.0/app.apk", + ), + ) + } + + @Test + fun requiresACompleteSha256Digest() { + assertEquals(digest, UpdatePolicy.sha256("sha256:$digest")) + assertEquals(digest, UpdatePolicy.sha256("sha256:${digest.uppercase()}")) + assertNull(UpdatePolicy.sha256(null)) + assertNull(UpdatePolicy.sha256("")) + assertNull(UpdatePolicy.sha256("sha256:${"a".repeat(63)}")) + assertNull(UpdatePolicy.sha256("sha256:${"a".repeat(65)}")) + assertNull(UpdatePolicy.sha256("md5:$digest")) + } + + @Test + fun acceptsOnlyGitHubRedirectTargets() { + val asset = "https://github.com/spacealtctrl/DiscordRP/releases/download/v1.6.0/app.apk" + assertTrue(UpdatePolicy.isTrustedDownloadUrl(asset, asset)) + assertTrue( + UpdatePolicy.isTrustedDownloadUrl( + asset, + "https://objects.githubusercontent.com/github-production-release-asset-2e65be/1?X-Amz-Signature=abc", + ), + ) + assertTrue( + UpdatePolicy.isTrustedDownloadUrl( + asset, + "https://release-assets.githubusercontent.com/github-production-release-asset/2?X-Amz-Signature=def", + ), + ) + + assertFalse(UpdatePolicy.isTrustedDownloadUrl(asset, "https://evil.test/app.apk")) + assertFalse( + UpdatePolicy.isTrustedDownloadUrl( + asset, + "https://objects.githubusercontent.com.evil.test/app.apk", + ), + ) + assertFalse( + UpdatePolicy.isTrustedDownloadUrl( + asset, + "http://objects.githubusercontent.com/app.apk", + ), + ) + assertFalse( + UpdatePolicy.isTrustedDownloadUrl( + asset, + "https://user@objects.githubusercontent.com/app.apk", + ), + ) + assertFalse( + UpdatePolicy.isTrustedDownloadUrl( + asset, + "https://objects.githubusercontent.com:8443/app.apk", + ), + ) + assertFalse(UpdatePolicy.isTrustedDownloadUrl(asset, "https://github.com/spacealtctrl")) + assertFalse(UpdatePolicy.isTrustedDownloadUrl("https://evil.test/app.apk", asset)) + assertFalse(UpdatePolicy.isTrustedDownloadUrl(asset, null)) + } +}