From 42483640fabda997803a2cbb980b791bc1668ca3 Mon Sep 17 00:00:00 2001 From: rxwatcher Date: Mon, 27 Jul 2026 09:34:58 +0200 Subject: [PATCH 01/17] fix(network): centralize authenticated origin policy --- .../silo/network/HttpOriginPolicy.kt | 146 ++++++++++++++++++ .../silo/network/HttpOriginPolicyTest.kt | 143 +++++++++++++++++ 2 files changed, 289 insertions(+) create mode 100644 shared/src/commonMain/kotlin/org/siloserver/silo/network/HttpOriginPolicy.kt create mode 100644 shared/src/commonTest/kotlin/org/siloserver/silo/network/HttpOriginPolicyTest.kt diff --git a/shared/src/commonMain/kotlin/org/siloserver/silo/network/HttpOriginPolicy.kt b/shared/src/commonMain/kotlin/org/siloserver/silo/network/HttpOriginPolicy.kt new file mode 100644 index 000000000..fbfe6c095 --- /dev/null +++ b/shared/src/commonMain/kotlin/org/siloserver/silo/network/HttpOriginPolicy.kt @@ -0,0 +1,146 @@ +package org.siloserver.silo.network + +import io.ktor.http.Url + +data class HttpOrigin( + val scheme: String, + val host: String, + val port: Int, +) + +fun httpOrigin(raw: String): HttpOrigin? = runCatching { + val scheme = raw.httpSchemeOrNull() ?: return null + val authorityHost = raw.authorityHostOrNull(scheme) ?: return null + + val url = Url(raw) + if (url.user != null || url.password != null) return null + if (url.protocol.name.lowercase() != scheme) return null + + val parsedHost = url.host + if (!parsedHost.isValidHttpHost()) return null + if (!authorityHost.equals(parsedHost, ignoreCase = true)) return null + + HttpOrigin( + scheme = scheme, + host = parsedHost.lowercase(), + port = url.port, + ) +}.getOrNull() + +fun isSameHttpOrigin(serverUrl: String, requestUrl: String): Boolean { + val serverOrigin = httpOrigin(serverUrl) ?: return false + val requestOrigin = httpOrigin(requestUrl) ?: return false + return serverOrigin == requestOrigin +} + +private fun String.httpSchemeOrNull(): String? = when { + startsWith("https://", ignoreCase = true) -> "https" + startsWith("http://", ignoreCase = true) -> "http" + else -> null +} + +private fun String.authorityHostOrNull(scheme: String): String? { + val prefix = "$scheme://" + val authority = drop(prefix.length).takeWhile { it != '/' && it != '?' && it != '#' } + if (authority.isBlank() || authority.any(Char::isUnsafeAuthorityCharacter)) return null + if ('@' in authority) return null + + return if (authority.startsWith('[')) { + authority.bracketedIpv6HostOrNull() + } else { + authority.unbracketedHostOrNull() + } +} + +private fun String.bracketedIpv6HostOrNull(): String? { + val closingBracket = indexOf(']') + if (closingBracket <= 1) return null + if (indexOf('[', startIndex = 1) >= 0) return null + if (indexOf(']', startIndex = closingBracket + 1) >= 0) return null + + val literal = substring(1, closingBracket) + if (!literal.isValidIpv6Literal()) return null + + val suffix = substring(closingBracket + 1) + if (suffix.isNotEmpty()) { + if (!suffix.startsWith(':')) return null + val port = suffix.drop(1) + if (port.isEmpty() || !port.all(Char::isAsciiDigit)) return null + } + return substring(0, closingBracket + 1) +} + +private fun String.unbracketedHostOrNull(): String? { + if ('[' in this || ']' in this) return null + + val firstColon = indexOf(':') + val lastColon = lastIndexOf(':') + if (firstColon != lastColon) return null + + val host = if (lastColon >= 0) substring(0, lastColon) else this + if (!host.isValidRawHttpHost()) return null + + if (lastColon >= 0) { + val port = substring(lastColon + 1) + if (port.isEmpty() || !port.all(Char::isAsciiDigit)) return null + } + return host +} + +private fun String.isValidRawHttpHost(): Boolean = + isNotBlank() && + !startsWith('.') && + !contains("..") && + all { character -> + character.isLetterOrDigit() || + character == '.' || + character == '-' || + character == '_' + } + +private fun String.isValidIpv6Literal(): Boolean { + if (isEmpty() || any { !it.isAsciiHexDigit() && it != ':' }) return false + + val compressionIndex = indexOf("::") + if (compressionIndex != lastIndexOf("::")) return false + + if (compressionIndex < 0) { + val groups = split(':') + return groups.size == IPV6_GROUP_COUNT && groups.all(String::isValidIpv6Group) + } + + val leftGroups = substring(0, compressionIndex).ipv6GroupsOrNull() ?: return false + val rightGroups = substring(compressionIndex + 2).ipv6GroupsOrNull() ?: return false + return leftGroups.size + rightGroups.size < IPV6_GROUP_COUNT +} + +private fun String.ipv6GroupsOrNull(): List? { + if (isEmpty()) return emptyList() + return split(':').takeIf { groups -> groups.all(String::isValidIpv6Group) } +} + +private fun String.isValidIpv6Group(): Boolean = + length in 1..IPV6_GROUP_HEX_LENGTH && all(Char::isAsciiHexDigit) + +private fun Char.isUnsafeAuthorityCharacter(): Boolean = + isWhitespace() || + code <= 0x1f || + code == 0x7f || + this == '\\' + +private fun Char.isAsciiDigit(): Boolean = this in '0'..'9' + +private fun Char.isAsciiHexDigit(): Boolean = + isAsciiDigit() || this in 'a'..'f' || this in 'A'..'F' + +private fun String.isValidHttpHost(): Boolean = + isNotBlank() && + none { character -> + character.isWhitespace() || + character.code <= 0x1f || + character.code == 0x7f || + character in "/?#@\\" + } + +private const val IPV6_GROUP_COUNT = 8 +private const val IPV6_GROUP_HEX_LENGTH = 4 diff --git a/shared/src/commonTest/kotlin/org/siloserver/silo/network/HttpOriginPolicyTest.kt b/shared/src/commonTest/kotlin/org/siloserver/silo/network/HttpOriginPolicyTest.kt new file mode 100644 index 000000000..d71bc9a43 --- /dev/null +++ b/shared/src/commonTest/kotlin/org/siloserver/silo/network/HttpOriginPolicyTest.kt @@ -0,0 +1,143 @@ +package org.siloserver.silo.network + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull +import kotlin.test.assertTrue + +class HttpOriginPolicyTest { + @Test + fun defaultPortsAndCaseNormalize() { + assertTrue(isSameHttpOrigin("HTTPS://Silo.Example", "https://silo.example:443/a")) + assertTrue(isSameHttpOrigin("http://silo.example", "http://SILO.EXAMPLE:80/a")) + assertEquals( + HttpOrigin(scheme = "https", host = "silo.example", port = 443), + httpOrigin("HTTPS://Silo.Example"), + ) + } + + @Test + fun pathQueryAndFragmentDoNotAffectOrigin() { + assertTrue( + isSameHttpOrigin( + "https://silo.example/base?server=true#settings", + "https://SILO.EXAMPLE/library/items?offset=10#details", + ), + ) + } + + @Test + fun matchingNonDefaultPortsArePreserved() { + assertEquals( + HttpOrigin(scheme = "https", host = "silo.example", port = 8443), + httpOrigin("https://silo.example:8443/a"), + ) + assertTrue( + isSameHttpOrigin( + "https://silo.example:8443", + "https://SILO.EXAMPLE:8443/a", + ), + ) + } + + @Test + fun schemeHostPortAndUserInfoMismatchesFailClosed() { + assertFalse(isSameHttpOrigin("https://silo.example", "http://silo.example/a")) + assertFalse(isSameHttpOrigin("https://silo.example", "https://cdn.silo.example/a")) + assertFalse(isSameHttpOrigin("https://silo.example", "https://silo.example:444/a")) + assertNull(httpOrigin("https://user@silo.example/a")) + assertNull(httpOrigin("file:///tmp/a")) + } + + @Test + fun everyUserInfoFormIsRejected() { + assertNull(httpOrigin("https://user:password@silo.example/a")) + assertNull(httpOrigin("https://:password@silo.example/a")) + assertNull(httpOrigin("https://@silo.example/a")) + assertNull(httpOrigin("https://user:@silo.example/a")) + } + + @Test + fun relativeAndMissingAuthorityUrlsAreRejected() { + listOf( + "", + " ", + "/relative/path", + "silo.example/path", + "https://", + "https:///path", + "https://?query", + "https://#fragment", + "https://:443/path", + ).forEach { raw -> + assertNull(httpOrigin(raw), "Expected a missing authority to be rejected: '$raw'") + } + } + + @Test + fun malformedHostsAndPortsAreRejected() { + assertNull(httpOrigin("https://silo example/path")) + assertNull(httpOrigin("https://silo.example:65536/path")) + assertNull(httpOrigin("https://silo.example:-1/path")) + } + + @Test + fun backslashAuthorityConfusionFailsClosed() { + listOf( + "https://\\attacker.example", + "https://attacker.example\\path", + "https://localhost\\@attacker.example", + "https://attacker.example\u0001.evil", + ).forEach { raw -> + assertNull(httpOrigin(raw), "Expected a confused authority to be rejected: '$raw'") + } + assertFalse( + isSameHttpOrigin( + serverUrl = "https://localhost", + requestUrl = "https://\\attacker.example", + ), + ) + } + + @Test + fun malformedBracketedAuthoritiesFailClosed() { + listOf( + "https://[]/", + "https://[x]/", + "https://[::1/", + "https://[[::1]]/", + "https://[:::1]/", + "https://[::1]extra/", + ).forEach { raw -> + assertNull(httpOrigin(raw), "Expected an invalid bracketed host to be rejected: '$raw'") + } + } + + @Test + fun validatedIpv6AuthoritiesArePreserved() { + assertEquals( + HttpOrigin(scheme = "https", host = "[::1]", port = 443), + httpOrigin("HTTPS://[::1]"), + ) + assertTrue(isSameHttpOrigin("https://[::1]", "https://[::1]:443/a")) + assertTrue( + isSameHttpOrigin( + "http://[2001:db8:0:1::1]:8080", + "http://[2001:DB8:0:1::1]:8080/a", + ), + ) + } + + @Test + fun invalidOriginsNeverCompareEqual() { + assertFalse(isSameHttpOrigin("", "")) + assertFalse(isSameHttpOrigin("/server", "/request")) + assertFalse( + isSameHttpOrigin( + "https://user@silo.example", + "https://user@silo.example/path", + ), + ) + } +} From e8c030f0a4a4b39216f29793c1a902cc3cc6be2b Mon Sep 17 00:00:00 2001 From: rxwatcher Date: Mon, 27 Jul 2026 09:38:38 +0200 Subject: [PATCH 02/17] fix(network): keep credentials on the silo origin --- android-shared/build.gradle.kts | 1 + .../player/AuthenticatedDataSourceFactory.kt | 41 +- .../common/player/MediaAuthInterceptor.kt | 27 +- .../silo/common/player/MediaAuthSession.kt | 41 +- .../silo/common/player/PlayerOkHttpClient.kt | 24 + .../AuthenticatedDataSourceFactoryTest.kt | 99 ++- .../common/player/MediaAuthInterceptorTest.kt | 208 ++++++ .../screens/reader/ReaderFileResolverTest.kt | 25 + .../silo/network/EncryptedTokenManagerImpl.kt | 86 +++ ...ncryptedTokenManagerScopeGenerationTest.kt | 98 +++ .../silo/network/AuthInterceptorImpl.kt | 334 +++++++--- .../silo/network/AuthScopeSnapshot.kt | 22 + .../siloserver/silo/network/TokenManager.kt | 16 + .../silo/network/SiloAuthPluginPinTest.kt | 603 ++++++++++++++++++ 14 files changed, 1539 insertions(+), 86 deletions(-) create mode 100644 shared/src/androidUnitTest/kotlin/org/siloserver/silo/network/EncryptedTokenManagerScopeGenerationTest.kt diff --git a/android-shared/build.gradle.kts b/android-shared/build.gradle.kts index cfe5e475f..01a819e56 100644 --- a/android-shared/build.gradle.kts +++ b/android-shared/build.gradle.kts @@ -121,6 +121,7 @@ kotlin { implementation(libs.ktor.client.mock) implementation(libs.ktor.client.content.negotiation) implementation(libs.ktor.serialization.json) + implementation(libs.okhttp.mockwebserver) } } } diff --git a/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/AuthenticatedDataSourceFactory.kt b/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/AuthenticatedDataSourceFactory.kt index edeefb7bb..9589d19b3 100644 --- a/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/AuthenticatedDataSourceFactory.kt +++ b/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/AuthenticatedDataSourceFactory.kt @@ -14,6 +14,7 @@ import org.siloserver.silo.common.player.subtitle.normalizeSubripPayloadIfNeeded import java.io.ByteArrayOutputStream import java.io.IOException import kotlinx.coroutines.runBlocking +import org.siloserver.silo.network.isSameHttpOrigin /** * DataSource.Factory that resolves relative stream URLs against the server @@ -88,7 +89,14 @@ internal class RefreshingHttpDataSource( return try { first.openWithGuards(dataSpec, failedSnapshot, guardEnabled) } catch (error: HttpDataSource.InvalidResponseCodeException) { - if (error.responseCode != 401 || !runBlocking { authSession.refreshIfStale(failedSnapshot) }) { + if ( + !shouldRefreshMediaRequest( + serverUrl = failedSnapshot.serverUrl, + requestUrl = dataSpec.uri.toString(), + responseCode = error.responseCode, + ) || + !runBlocking { authSession.refreshIfStale(failedSnapshot) } + ) { throw error } first.close() @@ -189,7 +197,12 @@ internal class RefreshingHttpDataSource( // headers as authoritative while filling only missing auth/profile // headers from the refreshable Silo session. .setHttpRequestHeaders( - mergeSessionAuthHeaders(snapshot.asRequestHeaders(), httpRequestHeaders), + authenticatedHeadersFor( + serverUrl = snapshot.serverUrl, + requestUrl = uri.toString(), + sessionHeaders = snapshot.asRequestHeaders(), + explicitHeaders = httpRequestHeaders, + ), ) .build() } @@ -225,6 +238,29 @@ internal fun mergeSessionAuthHeaders( } } +internal fun authenticatedHeadersFor( + serverUrl: String, + requestUrl: String, + sessionHeaders: Map, + explicitHeaders: Map, +): Map { + val resolvedRequestUrl = resolveRoutedDataSourceUrl(serverUrl, requestUrl) + val scopedSessionHeaders = if (isSameHttpOrigin(serverUrl, resolvedRequestUrl)) { + sessionHeaders + } else { + emptyMap() + } + return mergeSessionAuthHeaders(scopedSessionHeaders, explicitHeaders) +} + +internal fun shouldRefreshMediaRequest( + serverUrl: String, + requestUrl: String, + responseCode: Int, +): Boolean = + responseCode == 401 && + isSameHttpOrigin(serverUrl, resolveRoutedDataSourceUrl(serverUrl, requestUrl)) + /** * Picks between a [FileDataSource] (offline media playback) and the shared * [OkHttpDataSource] (every other scheme) based on the DataSpec's URI. Also @@ -310,6 +346,7 @@ internal fun resolveRoutedDataSourceUrl(serverUrl: String, rawUri: String): Stri trimmed.startsWith("https://", ignoreCase = true) || trimmed.startsWith("file://", ignoreCase = true) || trimmed.startsWith("content://", ignoreCase = true) -> trimmed + "://" in trimmed -> trimmed trimmed.startsWith("/") -> resolvePlaybackStreamUrl(serverUrl, trimmed) else -> "${serverUrl.trimEnd('/')}/${trimmed.trimStart('/')}" } diff --git a/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/MediaAuthInterceptor.kt b/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/MediaAuthInterceptor.kt index fd8ba1b82..da09df161 100644 --- a/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/MediaAuthInterceptor.kt +++ b/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/MediaAuthInterceptor.kt @@ -7,6 +7,7 @@ import okhttp3.Interceptor import okhttp3.OkHttpClient import okhttp3.Request import okhttp3.Response +import org.siloserver.silo.network.isSameHttpOrigin /** * OkHttp interceptor that mirrors [org.siloserver.silo.network.SiloAuthPlugin] @@ -36,6 +37,9 @@ class MediaAuthInterceptor( override fun intercept(chain: Interceptor.Chain): Response { val original = chain.request() val failedSnapshot = runBlocking { authSession.snapshot() } + if (!isSameHttpOrigin(failedSnapshot.serverUrl, original.url.toString())) { + return chain.proceed(original.withoutSiloCredentials()) + } val authed = original.newBuilder() .applyAuthHeaders(failedSnapshot) @@ -55,9 +59,14 @@ class MediaAuthInterceptor( return chain.proceed(authed) } - val retried = original.newBuilder() - .applyAuthHeaders(runBlocking { authSession.snapshot() }) - .build() + val retrySnapshot = runBlocking { authSession.snapshot() } + val retried = if (isSameHttpOrigin(retrySnapshot.serverUrl, original.url.toString())) { + original.newBuilder() + .applyAuthHeaders(retrySnapshot) + .build() + } else { + original.withoutSiloCredentials() + } return chain.proceed(retried) } @@ -66,3 +75,15 @@ class MediaAuthInterceptor( return this } } + +private fun Request.withoutSiloCredentials(): Request = + newBuilder() + .removeHeader("Authorization") + .removeHeader("X-Profile-Id") + .removeHeader("X-Profile-Token") + .apply { + headers.names() + .filter { name -> name.startsWith("X-Silo-", ignoreCase = true) } + .forEach(::removeHeader) + } + .build() diff --git a/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/MediaAuthSession.kt b/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/MediaAuthSession.kt index 4f4bfd570..cb245e38f 100644 --- a/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/MediaAuthSession.kt +++ b/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/MediaAuthSession.kt @@ -11,6 +11,7 @@ import okhttp3.RequestBody.Companion.toRequestBody import org.siloserver.silo.model.auth.RefreshRequest import org.siloserver.silo.model.auth.RefreshResponse import org.siloserver.silo.network.TokenManager +import org.siloserver.silo.network.isSameHttpOrigin import java.io.IOException /** @@ -28,12 +29,39 @@ class MediaAuthSession( ) { private val refreshMutex = Mutex() - suspend fun snapshot(): MediaAuthSnapshot = MediaAuthSnapshot( - accessToken = tokenManager.getAccessToken(), - profileId = tokenManager.getProfileId(), - profileToken = tokenManager.getProfileToken(), - serverId = tokenManager.getCurrentServerId(), - ) + suspend fun snapshot(): MediaAuthSnapshot { + val serverIdBefore = tokenManager.getCurrentServerId() + val serverUrl = tokenManager.getServerUrl() + val accessToken = tokenManager.getAccessToken() + val profileId = tokenManager.getProfileId() + val profileToken = tokenManager.getProfileToken() + val serverIdAfter = tokenManager.getCurrentServerId() + val serverUrlAfter = tokenManager.getServerUrl() + + // TokenManager exposes the active fields separately. If the user + // switches servers while they are read, never combine one server's + // credentials with another server's URL. An empty URL makes every + // transport's origin check fail closed and also suppresses refresh. + if ( + serverIdBefore != serverIdAfter || + !isSameHttpOrigin(serverUrl, serverUrlAfter) + ) { + return MediaAuthSnapshot( + accessToken = null, + profileId = null, + profileToken = null, + serverId = serverIdAfter, + serverUrl = "", + ) + } + return MediaAuthSnapshot( + accessToken = accessToken, + profileId = profileId, + profileToken = profileToken, + serverId = serverIdAfter, + serverUrl = serverUrlAfter, + ) + } suspend fun refreshIfStale(failedSnapshot: MediaAuthSnapshot): Boolean = refreshMutex.withLock { val current = snapshot() @@ -101,6 +129,7 @@ data class MediaAuthSnapshot( val profileId: String?, val profileToken: String?, val serverId: String?, + val serverUrl: String, ) { fun asRequestHeaders(): Map = buildMap { accessToken?.takeIf { it.isNotBlank() }?.let { put("Authorization", "Bearer $it") } diff --git a/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/PlayerOkHttpClient.kt b/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/PlayerOkHttpClient.kt index 3d7634df5..853e426fd 100644 --- a/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/PlayerOkHttpClient.kt +++ b/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/PlayerOkHttpClient.kt @@ -4,6 +4,8 @@ import okhttp3.ConnectionPool import okhttp3.Dispatcher import okhttp3.OkHttpClient import okhttp3.Protocol +import okhttp3.Request +import org.siloserver.silo.network.isSameHttpOrigin import java.util.concurrent.TimeUnit /** @@ -22,11 +24,33 @@ internal fun buildPlayerOkHttpClient(): OkHttpClient = .writeTimeout(30, TimeUnit.SECONDS) .followRedirects(true) .followSslRedirects(true) + .addNetworkInterceptor { chain -> + val initialTarget = chain.call().request().url.toString() + val networkRequest = chain.request() + val safeRequest = if (isSameHttpOrigin(initialTarget, networkRequest.url.toString())) { + networkRequest + } else { + networkRequest.withoutCrossOriginCredentials() + } + chain.proceed(safeRequest) + } .protocols(listOf(Protocol.HTTP_2, Protocol.HTTP_1_1)) .connectionPool(ConnectionPool(maxIdleConnections = 5, keepAliveDuration = 5, timeUnit = TimeUnit.MINUTES)) .dispatcher(Dispatcher()) .build() +private fun Request.withoutCrossOriginCredentials(): Request = + newBuilder() + .removeHeader("Authorization") + .removeHeader("X-Profile-Id") + .removeHeader("X-Profile-Token") + .apply { + headers.names() + .filter { name -> name.startsWith("X-Silo-", ignoreCase = true) } + .forEach(::removeHeader) + } + .build() + /** * Bounded bootstrap client for the token-refresh RPC. It intentionally has no * auth interceptor so a rejected refresh cannot recurse through the media diff --git a/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/AuthenticatedDataSourceFactoryTest.kt b/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/AuthenticatedDataSourceFactoryTest.kt index 1edd566d1..45d29dcd1 100644 --- a/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/AuthenticatedDataSourceFactoryTest.kt +++ b/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/AuthenticatedDataSourceFactoryTest.kt @@ -58,7 +58,9 @@ class AuthenticatedDataSourceFactoryTest { @Test fun explicitPlanHeadersOverrideSessionAuthCaseInsensitively() { - val merged = mergeSessionAuthHeaders( + val merged = authenticatedHeadersFor( + serverUrl = "https://silo.example", + requestUrl = "https://silo.example/video", sessionHeaders = mapOf( "Authorization" to "Bearer silo-session", "X-Profile-Id" to "profile-1", @@ -75,6 +77,101 @@ class AuthenticatedDataSourceFactoryTest { assertEquals("route-7", merged["X-Stream-Scope"]) } + @Test + fun foreignOriginReceivesOnlyExplicitTargetHeaders() { + assertEquals( + mapOf("X-Stream-Scope" to "route-7"), + authenticatedHeadersFor( + serverUrl = "https://silo.example", + requestUrl = "https://cdn.example/video", + sessionHeaders = mapOf( + "Authorization" to "Bearer silo-session", + "X-Profile-Id" to "profile-1", + "X-Profile-Token" to "profile-token", + ), + explicitHeaders = mapOf("X-Stream-Scope" to "route-7"), + ), + ) + } + + @Test + fun explicitAuthorizationIsPreservedOnlyForItsIssuedTarget() { + assertEquals( + mapOf("Authorization" to "Signed cdn-route-7"), + authenticatedHeadersFor( + serverUrl = "https://silo.example", + requestUrl = "https://cdn.example/video", + sessionHeaders = mapOf("Authorization" to "Bearer silo-session"), + explicitHeaders = mapOf("Authorization" to "Signed cdn-route-7"), + ), + ) + } + + @Test + fun sessionHeadersRequireExactSchemeHostAndPort() { + val sessionHeaders = mapOf( + "Authorization" to "Bearer silo-session", + "X-Profile-Id" to "profile-1", + "X-Profile-Token" to "profile-token", + ) + listOf( + "https://cdn.silo.example/video", + "https://silo.example:444/video", + "http://silo.example/video", + "file:///tmp/video", + "://malformed", + ).forEach { requestUrl -> + assertEquals( + emptyMap(), + authenticatedHeadersFor( + serverUrl = "https://silo.example", + requestUrl = requestUrl, + sessionHeaders = sessionHeaders, + explicitHeaders = emptyMap(), + ), + "Session credentials leaked to $requestUrl", + ) + } + } + + @Test + fun relativeMediaAndSubtitleUrlsResolveBeforeOriginPolicy() { + val sessionHeaders = mapOf("Authorization" to "Bearer silo-session") + + listOf( + "/api/v1/stream/session-1", + "api/v1/stream/session-1/subtitles/4.srt", + ).forEach { requestUrl -> + assertEquals( + sessionHeaders, + authenticatedHeadersFor( + serverUrl = "https://silo.example", + requestUrl = requestUrl, + sessionHeaders = sessionHeaders, + explicitHeaders = emptyMap(), + ), + ) + } + } + + @Test + fun foreignUnauthorizedMediaResponseDoesNotRefresh() { + assertFalse( + shouldRefreshMediaRequest( + serverUrl = "https://silo.example", + requestUrl = "https://cdn.example/video", + responseCode = 401, + ), + ) + assertTrue( + shouldRefreshMediaRequest( + serverUrl = "https://silo.example", + requestUrl = "https://silo.example/video", + responseCode = 401, + ), + ) + } + @Test fun srtSubtitlePayloadBytesAreNormalizedBeforeMedia3ParsesThem() { val loose = """ diff --git a/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/MediaAuthInterceptorTest.kt b/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/MediaAuthInterceptorTest.kt index 3a2f0a7ee..e8c22bf79 100644 --- a/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/MediaAuthInterceptorTest.kt +++ b/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/MediaAuthInterceptorTest.kt @@ -16,6 +16,8 @@ import okhttp3.Protocol import okhttp3.Request import okhttp3.Response import okhttp3.ResponseBody.Companion.toResponseBody +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer import java.util.concurrent.TimeUnit import kotlin.test.Test import kotlin.test.assertEquals @@ -29,6 +31,7 @@ class MediaAuthInterceptorTest { fun `adds auth and active profile headers to media and reader requests`() { val tokenManager = TokenManagerImpl() runBlocking { + tokenManager.setServerUrl("https://lib.strm.cafe") tokenManager.saveTokens("access-token", "refresh-token", expiresIn = 3600) tokenManager.setProfileId("profile-1") tokenManager.setProfileToken("profile-token") @@ -53,6 +56,194 @@ class MediaAuthInterceptorTest { } } + @Test + fun `foreign origins receive no auth or profile headers`() { + val tokenManager = TokenManagerImpl() + runBlocking { + tokenManager.setServerUrl("https://lib.strm.cafe") + tokenManager.saveTokens("access-token", "refresh-token", expiresIn = 3600) + tokenManager.setProfileId("profile-1") + tokenManager.setProfileToken("profile-token") + } + + listOf( + "https://cdn.lib.strm.cafe/video", + "https://lib.strm.cafe:444/video", + "http://lib.strm.cafe/video", + ).forEach { url -> + val chain = CapturingChain( + Request.Builder() + .url(url) + .header("Authorization", "Signed explicit-credential") + .header("X-Profile-Id", "explicit-profile") + .header("X-Profile-Token", "explicit-profile-token") + .build(), + ) + + MediaAuthInterceptor(tokenManager, refreshClient = OkHttpClient()).intercept(chain) + + val request = chain.capturedRequest ?: error("request was not captured") + assertNull(request.header("Authorization"), "Authorization leaked to $url") + assertNull(request.header("X-Profile-Id"), "X-Profile-Id leaked to $url") + assertNull(request.header("X-Profile-Token"), "X-Profile-Token leaked to $url") + } + } + + @Test + fun `foreign unauthorized response causes no refresh attempt`() { + val tokenManager = FakeTokenManager( + accessToken = "expired-access", + refreshToken = "refresh-token", + serverUrl = "https://lib.strm.cafe", + serverId = "server-a", + ) + var refreshRequests = 0 + val refreshClient = OkHttpClient.Builder() + .addInterceptor { chain -> + refreshRequests += 1 + responseFor(chain.request(), code = 500) + } + .build() + val chain = SequenceChain( + Request.Builder() + .url("https://cdn.example/video") + .build(), + responseCodes = listOf(401, 401), + ) + + MediaAuthInterceptor(tokenManager, refreshClient = refreshClient).intercept(chain).close() + + assertEquals(0, refreshRequests) + assertEquals(1, chain.proceedCalls) + assertFalse(tokenManager.invalidatedSession) + } + + @Test + fun `server switch while snapshot is read cannot pair old credentials with new origin`() { + val tokenManager = FakeTokenManager( + accessToken = "server-a-access", + refreshToken = "server-a-refresh", + serverUrl = "https://server-a.example", + serverId = "server-a", + ).apply { + runBlocking { + setProfileId("server-a-profile") + setProfileToken("server-a-profile-token") + } + onGetServerUrl = { + serverId = "server-b" + serverUrl = "https://server-b.example" + } + } + val chain = CapturingChain( + Request.Builder() + .url("https://server-b.example/video") + .build(), + ) + + MediaAuthInterceptor(tokenManager, refreshClient = OkHttpClient()).intercept(chain) + + val request = chain.capturedRequest ?: error("request was not captured") + assertNull(request.header("Authorization")) + assertNull(request.header("X-Profile-Id")) + assertNull(request.header("X-Profile-Token")) + } + + @Test + fun `cross origin redirect strips silo and explicit authorization headers`() { + val origin = MockWebServer() + val downstream = MockWebServer() + origin.start() + downstream.start() + try { + origin.enqueue( + MockResponse() + .setResponseCode(302) + .setHeader("Location", downstream.url("/asset")), + ) + downstream.enqueue(MockResponse().setResponseCode(200).setBody("ok")) + val tokenManager = FakeTokenManager( + accessToken = null, + refreshToken = "refresh-token", + serverUrl = origin.url("/").toString(), + serverId = "server-a", + ).apply { + runBlocking { + setProfileId("profile-1") + setProfileToken("profile-token") + } + } + val client = buildPlayerOkHttpClient() + .newBuilder() + .addInterceptor(MediaAuthInterceptor(tokenManager, refreshClient = OkHttpClient())) + .build() + val request = Request.Builder() + .url(origin.url("/redirect")) + .header("Authorization", "Signed explicit-target-credential") + .header("X-Silo-Device-Id", "device-1") + .build() + + client.newCall(request).execute().use { response -> + assertEquals(200, response.code) + } + + val received = downstream.takeRequest() + assertNull(received.getHeader("Authorization")) + assertNull(received.getHeader("X-Profile-Id")) + assertNull(received.getHeader("X-Profile-Token")) + assertNull(received.getHeader("X-Silo-Device-Id")) + } finally { + origin.shutdown() + downstream.shutdown() + } + } + + @Test + fun `retry never applies credentials from a newly active foreign server`() { + val tokenManager = FakeTokenManager( + accessToken = "server-a-expired", + refreshToken = "server-a-refresh", + serverUrl = "https://server-a.example", + serverId = "server-a", + ) + val refreshClient = OkHttpClient.Builder() + .addInterceptor { chain -> + responseFor( + chain.request(), + code = 200, + body = """ + { + "access_token": "server-a-fresh", + "refresh_token": "server-a-fresh-refresh", + "expires_in": 3600 + } + """.trimIndent(), + ) + } + .build() + tokenManager.onSaveTokens = { + tokenManager.switchScope( + accessToken = "server-b-access", + refreshToken = "server-b-refresh", + serverUrl = "https://server-b.example", + serverId = "server-b", + ) + } + val chain = SequenceChain( + Request.Builder() + .url("https://server-a.example/video") + .build(), + responseCodes = listOf(401, 401), + ) + + MediaAuthInterceptor(tokenManager, refreshClient = refreshClient).intercept(chain).close() + + val retry = chain.capturedRequests.last() + assertNull(retry.header("Authorization")) + assertNull(retry.header("X-Profile-Id")) + assertNull(retry.header("X-Profile-Token")) + } + @Test fun `failed refresh invalidates session`() { val tokenManager = FakeTokenManager( @@ -276,10 +467,13 @@ class MediaAuthInterceptorTest { private val responseCodes: List, ) : Interceptor.Chain { private var calls = 0 + val proceedCalls: Int get() = calls + val capturedRequests = mutableListOf() override fun request(): Request = request override fun proceed(request: Request): Response { + capturedRequests += request val code = responseCodes.getOrElse(calls) { responseCodes.last() } calls += 1 return mediaAuthTestResponseFor(request, code) @@ -302,6 +496,7 @@ class MediaAuthInterceptorTest { var serverId: String?, ) : TokenManager { var onGetServerUrl: (() -> Unit)? = null + var onSaveTokens: (() -> Unit)? = null var invalidatedSession = false private set var savedTokens = false @@ -320,6 +515,7 @@ class MediaAuthInterceptorTest { savedTokens = true this.accessToken = accessToken this.refreshToken = refreshToken + onSaveTokens?.invoke() } override suspend fun clearTokens() { @@ -364,6 +560,18 @@ class MediaAuthInterceptorTest { override suspend fun signOutCurrentServer() { clearTokens() } + + fun switchScope( + accessToken: String?, + refreshToken: String?, + serverUrl: String, + serverId: String?, + ) { + this.accessToken = accessToken + this.refreshToken = refreshToken + this.serverUrl = serverUrl + this.serverId = serverId + } } private fun responseFor( diff --git a/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/ReaderFileResolverTest.kt b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/ReaderFileResolverTest.kt index 45ddfae36..9c1a6b70f 100644 --- a/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/ReaderFileResolverTest.kt +++ b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/ReaderFileResolverTest.kt @@ -2,6 +2,7 @@ package org.siloserver.silo.android.ui.screens.reader import java.io.File import java.net.URI +import org.siloserver.silo.network.isSameHttpOrigin import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotEquals @@ -17,6 +18,23 @@ class ReaderFileResolverTest { serverUrl = "https://lib.strm.cafe/", ), ) + assertEquals( + "https://lib.strm.cafe/api/v1/ebooks/book-1/files/7/read", + resolveReaderRequestUrl( + url = "api/v1/ebooks/book-1/files/7/read", + serverUrl = "https://lib.strm.cafe/", + ), + ) + assertEquals( + true, + isSameHttpOrigin( + "https://lib.strm.cafe", + resolveReaderRequestUrl( + url = "/api/v1/ebooks/book-1/files/7/read", + serverUrl = "https://lib.strm.cafe/", + ), + ), + ) } @Test @@ -37,6 +55,13 @@ class ReaderFileResolverTest { "content://media/external/downloads/12", resolveReaderRequestUrl("content://media/external/downloads/12", "https://lib.strm.cafe"), ) + assertEquals( + false, + isSameHttpOrigin( + "https://lib.strm.cafe", + resolveReaderRequestUrl("https://cdn.example.test/book.epub", "https://lib.strm.cafe"), + ), + ) } @Test diff --git a/shared/src/androidMain/kotlin/org/siloserver/silo/network/EncryptedTokenManagerImpl.kt b/shared/src/androidMain/kotlin/org/siloserver/silo/network/EncryptedTokenManagerImpl.kt index 230344ae9..9a0c51268 100644 --- a/shared/src/androidMain/kotlin/org/siloserver/silo/network/EncryptedTokenManagerImpl.kt +++ b/shared/src/androidMain/kotlin/org/siloserver/silo/network/EncryptedTokenManagerImpl.kt @@ -50,6 +50,14 @@ class EncryptedTokenManagerImpl( private var profileToken: String? = null private var temporaryScope: TemporaryAuthScope? = null + /** + * Incremented whenever this manager writes or clears PERSISTENT credentials. + * Stamped onto snapshots so a scope captured before a sign-out cannot read or + * overwrite the credentials of the login that replaced it. Overlay begin/end + * deliberately does not move it — see [AuthScopeSnapshot.credentialEpoch]. + */ + private var persistentCredentialEpoch: Long = 0L + private val _sessionExpired = MutableSharedFlow( replay = 0, extraBufferCapacity = 1, @@ -138,6 +146,7 @@ class EncryptedTokenManagerImpl( this.refreshToken = refreshToken val expiryEpochMs = System.currentTimeMillis() + expiresIn * 1000L this.tokenExpiryEpochMs = expiryEpochMs + persistentCredentialEpoch += 1 prefs.edit() .putString(serverScopedKey(serverId, KEY_ACCESS_TOKEN), accessToken) .putString(serverScopedKey(serverId, KEY_REFRESH_TOKEN), refreshToken) @@ -175,6 +184,7 @@ class EncryptedTokenManagerImpl( } private fun clearPersistentTokensLocked() { + persistentCredentialEpoch += 1 val serverId = activeServerId accessToken = null refreshToken = null @@ -305,6 +315,7 @@ class EncryptedTokenManagerImpl( serverUrl = scope.serverUrl, profileToken = scope.profileToken, credentialGenerationId = scope.generationId, + identityGeneration = identityTransitions.generation.value, ) } val serverId = activeServerId ?: return@withLock null @@ -320,6 +331,8 @@ class EncryptedTokenManagerImpl( profileId = profileId, serverUrl = url, profileToken = profileToken, + identityGeneration = identityTransitions.generation.value, + credentialEpoch = persistentCredentialEpoch, ) } @@ -333,9 +346,27 @@ class EncryptedTokenManagerImpl( else prefs.getString(serverScopedKey(serverId, KEY_REFRESH_TOKEN), null) } + /** + * True when an identity transition has happened since [this] was captured. + * + * The persistent path is keyed by serverId alone, so a snapshot taken before + * a sign-out could still read — and overwrite — the credentials issued by the + * NEXT login on that same server. Comparing the captured identity generation + * closes that: [saveTokens] and [clearTokens]/[invalidateSession] both run + * inside `identityTransitions.changing`, which increments it. + * + * `0L` means "not captured from a live snapshot" — several call sites build a + * scope by hand (the interceptor's refresh fallback, companion pairing, + * remote-playback identity) and carry the default. Those keep the old + * behaviour rather than failing closed on a generation they never recorded. + */ + private fun AuthScopeSnapshot.credentialsReplaced(): Boolean = + credentialEpoch != 0L && credentialEpoch != persistentCredentialEpoch + override suspend fun getAccessTokenForScope(scope: AuthScopeSnapshot): String? = mutex.withLock { val generationId = scope.credentialGenerationId if (generationId == null) { + if (scope.credentialsReplaced()) return@withLock null persistentAccessToken(scope.serverId) } else { temporaryScope @@ -347,6 +378,7 @@ class EncryptedTokenManagerImpl( override suspend fun getRefreshTokenForScope(scope: AuthScopeSnapshot): String? = mutex.withLock { val generationId = scope.credentialGenerationId if (generationId == null) { + if (scope.credentialsReplaced()) return@withLock null persistentRefreshToken(scope.serverId) } else { temporaryScope @@ -377,6 +409,9 @@ class EncryptedTokenManagerImpl( val expiryEpochMs = System.currentTimeMillis() + expiresIn * 1000L val generationId = scope.credentialGenerationId if (generationId == null) { + // A stale scope must not overwrite the credentials of the login + // that replaced it. + if (scope.credentialsReplaced()) return@withLock savePersistentTokens(scope.serverId, accessToken, refreshToken, expiryEpochMs) return@withLock } @@ -451,4 +486,55 @@ class EncryptedTokenManagerImpl( // strip the pre-multi-server unprefixed key off disk. const val KEY_SERVER_URL = "server_url" } + + private fun currentScopeMatchesLocked( + scope: AuthScopeSnapshot, + expectedGeneration: Long, + ): Boolean { + if (identityTransitions.generation.value != expectedGeneration) return false + val temporary = temporaryScope + val generationId = scope.credentialGenerationId + if (generationId != null) { + return temporary?.generationId == generationId && + temporary.serverId == scope.serverId && + temporary.serverUrl == scope.serverUrl + } + if (temporary != null || activeServerId != scope.serverId) return false + return registry.entries.value + .firstOrNull { it.id == scope.serverId } + ?.url == scope.serverUrl + } + + override suspend fun invalidateSessionForScope(scope: AuthScopeSnapshot): Boolean = + tokenWriteMutex.withLock { + val matchesBeforeTransition = mutex.withLock { + ensureCacheMatchesRegistryLocked() + currentScopeMatchesLocked( + scope = scope, + expectedGeneration = scope.identityGeneration, + ) + } + if (!matchesBeforeTransition) return@withLock false + + identityTransitions.changing(IdentityTransitionKind.SIGN_OUT) { + mutex.withLock { + ensureCacheMatchesRegistryLocked() + // `changing` increments the generation before entering this + // block. Any intervening identity transition therefore + // makes this value larger and the mutation fails closed. + if ( + !currentScopeMatchesLocked( + scope = scope, + expectedGeneration = scope.identityGeneration + 1, + ) + ) { + return@withLock false + } + val wasTemporary = temporaryScope != null + clearCurrentScopeLocked() + if (!wasTemporary) _sessionExpired.tryEmit(Unit) + true + } + } + } } diff --git a/shared/src/androidUnitTest/kotlin/org/siloserver/silo/network/EncryptedTokenManagerScopeGenerationTest.kt b/shared/src/androidUnitTest/kotlin/org/siloserver/silo/network/EncryptedTokenManagerScopeGenerationTest.kt new file mode 100644 index 000000000..a3e06ce64 --- /dev/null +++ b/shared/src/androidUnitTest/kotlin/org/siloserver/silo/network/EncryptedTokenManagerScopeGenerationTest.kt @@ -0,0 +1,98 @@ +package org.siloserver.silo.network + +import android.content.SharedPreferences +import java.lang.reflect.Proxy +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.test.runTest +import org.siloserver.silo.model.server.ServerEntry +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +class EncryptedTokenManagerScopeGenerationTest { + + @Test + fun staleSameServerScopeCannotReadOrRestoreReloggedCredentials() = runTest { + val registry = FakeServerRegistry() + val manager = EncryptedTokenManagerImpl( + prefs = inMemoryPreferences(), + registry = registry, + ) + manager.saveTokens("old-access", "old-refresh", 3600) + val staleScope = checkNotNull(manager.snapshotCurrentScope()) + + manager.clearTokens() + manager.saveTokens("new-access", "new-refresh", 3600) + + assertNull(manager.getAccessTokenForScope(staleScope)) + assertNull(manager.getRefreshTokenForScope(staleScope)) + + manager.saveTokensForScope(staleScope, "stale-access", "stale-refresh", 3600) + + assertEquals("new-access", manager.getAccessToken()) + assertEquals("new-refresh", manager.getRefreshToken()) + } + + private class FakeServerRegistry : ServerRegistry { + private val entry = ServerEntry(id = "server-a", url = "https://server-a.example") + override val entries: StateFlow> = MutableStateFlow(listOf(entry)) + override val activeServerId: StateFlow = MutableStateFlow(entry.id) + override val activeEntry: StateFlow = MutableStateFlow(entry) + override suspend fun addOrUpdate(url: String, fetchedName: String?): String = entry.id + override suspend fun rename(serverId: String, userOverrideName: String?) = Unit + override suspend fun setFetchedName(serverId: String, fetchedName: String?) = Unit + override suspend fun setProfileId(serverId: String, profileId: String?) = Unit + override suspend fun remove(serverId: String) = Unit + override suspend fun signOut(serverId: String) = Unit + override suspend fun switchTo(serverId: String) = Unit + override suspend fun touchActive() = Unit + } + + private fun inMemoryPreferences(): SharedPreferences { + val values = mutableMapOf() + lateinit var preferences: SharedPreferences + preferences = Proxy.newProxyInstance( + SharedPreferences::class.java.classLoader, + arrayOf(SharedPreferences::class.java), + ) { _, method, args -> + when (method.name) { + "getString" -> values[args!![0]] as? String ?: args[1] + "getLong" -> values[args!![0]] as? Long ?: args[1] + "contains" -> values.containsKey(args!![0]) + "getAll" -> values.toMap() + "edit" -> editor(values) + "registerOnSharedPreferenceChangeListener", + "unregisterOnSharedPreferenceChangeListener" -> Unit + else -> method.defaultValue() + } + } as SharedPreferences + return preferences + } + + private fun editor(values: MutableMap): SharedPreferences.Editor { + lateinit var editor: SharedPreferences.Editor + editor = Proxy.newProxyInstance( + SharedPreferences.Editor::class.java.classLoader, + arrayOf(SharedPreferences.Editor::class.java), + ) { _, method, args -> + when (method.name) { + "putString", "putLong" -> editor.also { values[args!![0] as String] = args[1] } + "remove" -> editor.also { values.remove(args!![0] as String) } + "clear" -> editor.also { values.clear() } + "apply" -> Unit + "commit" -> true + else -> editor + } + } as SharedPreferences.Editor + return editor + } + + private fun java.lang.reflect.Method.defaultValue(): Any? = when (returnType) { + java.lang.Boolean.TYPE -> false + java.lang.Integer.TYPE -> 0 + java.lang.Long.TYPE -> 0L + java.lang.Float.TYPE -> 0f + else -> null + } +} diff --git a/shared/src/commonMain/kotlin/org/siloserver/silo/network/AuthInterceptorImpl.kt b/shared/src/commonMain/kotlin/org/siloserver/silo/network/AuthInterceptorImpl.kt index 5a61c0432..c3201e9b0 100644 --- a/shared/src/commonMain/kotlin/org/siloserver/silo/network/AuthInterceptorImpl.kt +++ b/shared/src/commonMain/kotlin/org/siloserver/silo/network/AuthInterceptorImpl.kt @@ -6,6 +6,8 @@ import io.ktor.client.plugins.api.* import io.ktor.client.plugins.* import io.ktor.client.request.* import io.ktor.http.* +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.update import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import org.siloserver.silo.model.auth.RefreshRequest @@ -46,28 +48,51 @@ val SiloAuthPlugin = createClientPlugin("SiloAuthPlugin", ::SiloAuthConfig) { val refreshMutex = Mutex() + // Temporary credential generations (remote-playback overlays) whose refresh + // the server has definitively rejected. Such an overlay is deliberately left + // INSTALLED — clearing it would make every later token read fall through to + // the saved owner's account, so the guest session would silently continue as + // the owner. Flagging the generation here is what stops the plugin from + // refreshing dead credentials again on every subsequent 401. + val deadCredentialGenerations = MutableStateFlow>(emptySet()) + onRequest { request, _ -> val skipAuth = request.attributes.getOrNull(SkipSiloAuthAttributeKey) == true val diagnosticsScope = request.attributes.getOrNull(DiagnosticsRequestScopeKey) + val pinned = request.attributes.getOrNull(AuthScopeAttributeKey) + val activeServerIdBefore = if (pinned == null) tokenManager.getCurrentServerId() else null + val trustedServerUrl = pinned?.serverUrl ?: tokenManager.getServerUrl() + + // Shared calls are normally relative. Resolve those against the exact + // server that owns the credential scope before deciding whether any + // Silo header may be attached. + if ( + request.url.encodedPath.startsWith("/api/") && + (request.url.host.isBlank() || request.url.host == "localhost") && + trustedServerUrl.isNotBlank() + ) { + request.url.rebaseRelativeApiUrl(trustedServerUrl) + } + + val sameOrigin = isSameSiloHttpOrigin(trustedServerUrl, request.url) + if (skipAuth) { + request.removeSiloCredentialHeaders() + if (sameOrigin) { + request.attachSiloDeviceMetadataHeaders(deviceMetadataProvider) + } + return@onRequest + } + + if (!sameOrigin) { + request.removeSiloCredentialHeaders() + return@onRequest + } // Pinned (Track B outbox replay): bind this request to a captured scope // regardless of the globally-active server/profile, so a mid-drain switch // can't send it to the wrong account. Uses the snapshot's URL/profile and // the *live* per-server access token (handles rotation). - val pinned = request.attributes.getOrNull(AuthScopeAttributeKey) - if (pinned != null && !skipAuth) { - if (request.url.encodedPath.startsWith("/api/") && pinned.serverUrl.isNotBlank()) { - val originalPath = request.url.encodedPath - val originalParameters = request.url.parameters.build() - val originalFragment = request.url.fragment - val originalProtocol = request.url.protocol - request.url.takeFrom(pinned.serverUrl) - request.url.restoreWebSocketProtocol(originalProtocol) - request.url.encodedPath = originalPath - request.url.parameters.clear() - request.url.parameters.appendAll(originalParameters) - request.url.fragment = originalFragment - } + if (pinned != null) { // Replace (never append) the scoped headers; clear the profile token // header when the snapshot has none. request.headers.remove(HttpHeaders.Authorization) @@ -85,65 +110,83 @@ val SiloAuthPlugin = createClientPlugin("SiloAuthPlugin", ::SiloAuthConfig) { return@onRequest } - // Shared API calls use relative paths; bind them to the configured server URL - // before the request is sent so Ktor doesn't fall back to localhost on iOS. - if (request.url.encodedPath.startsWith("/api/") && request.url.host == "localhost") { - val serverUrl = tokenManager.getServerUrl() - if (serverUrl.isNotBlank()) { - val originalPath = request.url.encodedPath - val originalParameters = request.url.parameters.build() - val originalFragment = request.url.fragment - val originalProtocol = request.url.protocol - - request.url.takeFrom(serverUrl) - request.url.restoreWebSocketProtocol(originalProtocol) - request.url.encodedPath = originalPath - request.url.parameters.clear() - request.url.parameters.appendAll(originalParameters) - request.url.fragment = originalFragment - } - } - - if (skipAuth) { - request.headers.remove(HttpHeaders.Authorization) - request.headers.remove("X-Profile-Id") - request.headers.remove("X-Profile-Token") - request.attachSiloDeviceMetadataHeaders(deviceMetadataProvider) - return@onRequest - } - // Skip auth headers for the refresh endpoint itself to avoid recursion val isRefreshRequest = request.url.encodedPath.endsWith("/auth/refresh") if (isRefreshRequest) return@onRequest - tokenManager.getAccessToken()?.let { token -> + val accessToken = tokenManager.getAccessToken() + val profileId = tokenManager.getProfileId() + val profileToken = tokenManager.getProfileToken() + val activeServerIdAfter = tokenManager.getCurrentServerId() + val activeServerUrlAfter = tokenManager.getServerUrl() + if ( + activeServerIdBefore != activeServerIdAfter || + !isSameHttpOrigin(trustedServerUrl, activeServerUrlAfter) + ) { + request.removeSiloCredentialHeaders() + return@onRequest + } + + accessToken?.let { token -> request.header(HttpHeaders.Authorization, "Bearer $token") } request.applyProfileHeaders( diagnosticsScope = diagnosticsScope, - activeProfileId = tokenManager.getProfileId(), - activeProfileToken = tokenManager.getProfileToken(), + activeProfileId = profileId, + activeProfileToken = profileToken, ) request.attachSiloDeviceMetadataHeaders(deviceMetadataProvider) } on(Send) { request -> - if (request.attributes.getOrNull(SkipSiloAuthAttributeKey) == true) { - return@on proceed(request) - } - // Pinned scope (Track B): refresh against the *captured* scope, never the // active one, and never invalidate the active UI session — a failed // pinned refresh just surfaces the 401 so the outbox keeps the op. val pinnedScope = request.attributes.getOrNull(AuthScopeAttributeKey) + val normalScope = + if (pinnedScope == null) tokenManager.snapshotCurrentScope() else null + val activeServerIdBeforeUrl = + if (pinnedScope == null) normalScope?.serverId ?: tokenManager.getCurrentServerId() else null + val trustedServerUrl = + pinnedScope?.serverUrl ?: normalScope?.serverUrl ?: tokenManager.getServerUrl() + val activeServerIdBeforeRequest = + if (pinnedScope == null) tokenManager.getCurrentServerId() else null + if ( + pinnedScope == null && + activeServerIdBeforeUrl != activeServerIdBeforeRequest + ) { + request.removeSiloCredentialHeaders() + return@on proceed(request) + } + if (request.attributes.getOrNull(SkipSiloAuthAttributeKey) == true) { + if (!isSameSiloHttpOrigin(trustedServerUrl, request.url)) { + request.removeSiloCredentialHeaders() + } + return@on proceed(request) + } + if (!isSameSiloHttpOrigin(trustedServerUrl, request.url)) { + request.removeSiloCredentialHeaders() + return@on proceed(request) + } if (pinnedScope != null) { val sentAuth = request.headers[HttpHeaders.Authorization] val originalCall = proceed(request) if (originalCall.response.status != HttpStatusCode.Unauthorized) { return@on originalCall } + val pinnedGeneration = pinnedScope.credentialGenerationId + if (pinnedGeneration != null && pinnedGeneration in deadCredentialGenerations.value) { + // Already-rejected temporary credentials: surface the 401 instead of + // re-refreshing them for every pinned op (progress ticks, teardown). + return@on originalCall + } + // A redirect can carry the pinned call off the Silo origin; refreshing + // then would hand this scope's credentials to whatever answered. + if (!isSameSiloHttpOrigin(pinnedScope.serverUrl, originalCall.request.url)) { + return@on originalCall + } diagnosticsObserver.safeAuthRefresh("required") val refreshed = refreshMutex.withLock { // Another path may have refreshed this scope while we waited. @@ -179,6 +222,11 @@ val SiloAuthPlugin = createClientPlugin("SiloAuthPlugin", ::SiloAuthConfig) { after != null && after != sentAuth } else { diagnosticsObserver.safeAuthRefresh("failed") + if (pinnedGeneration != null && + refreshResponse.status.shouldInvalidateSessionAfterRefreshFailure() + ) { + deadCredentialGenerations.update { it + pinnedGeneration } + } // Don't invalidate the active session for a background scope. // Re-check in case a concurrent path refreshed it in flight. val after = tokenManager.getAccessTokenForScope(pinnedScope)?.let { "Bearer $it" } @@ -200,11 +248,19 @@ val SiloAuthPlugin = createClientPlugin("SiloAuthPlugin", ::SiloAuthConfig) { } } - // Capture the access token we will actually SEND with this request. - // If the response comes back 401, we compare against this snapshot + val refreshScope = normalScope ?: AuthScopeSnapshot( + serverId = activeServerIdBeforeRequest.orEmpty(), + profileId = null, + serverUrl = trustedServerUrl, + profileToken = null, + ) + + // Capture the authorization value we will actually SEND, together with + // the server identity and origin that own it. If the response comes back + // 401, we compare against this snapshot // inside the refresh mutex to detect a concurrent refresh that // already happened — so N parallel 401s collapse into ONE refresh. - val tokenBeforeRequest = tokenManager.getAccessToken() + val authorizationBeforeRequest = request.headers[HttpHeaders.Authorization] val originalCall = proceed(request) @@ -212,6 +268,15 @@ val SiloAuthPlugin = createClientPlugin("SiloAuthPlugin", ::SiloAuthConfig) { if (originalCall.response.status != HttpStatusCode.Unauthorized) { return@on originalCall } + if (!isSameSiloHttpOrigin(trustedServerUrl, originalCall.request.url)) { + return@on originalCall + } + if ( + tokenManager.getCurrentServerId() != activeServerIdBeforeRequest || + !isSameHttpOrigin(trustedServerUrl, tokenManager.getServerUrl()) + ) { + return@on originalCall + } diagnosticsObserver.safeAuthRefresh("required") val requestPath = originalCall.request.url.encodedPath @@ -219,6 +284,18 @@ val SiloAuthPlugin = createClientPlugin("SiloAuthPlugin", ::SiloAuthConfig) { return@on originalCall } + // Identity of the temporary overlay (remote playback) this request ran + // under, if any. Null means the request ran on a saved account. + val temporaryGeneration = tokenManager.temporaryGenerationId() + if (temporaryGeneration != null && + temporaryGeneration in deadCredentialGenerations.value + ) { + // The server already rejected these credentials. Refreshing again would + // storm it once per request for the rest of the handoff; the overlay stays + // installed so the guest cannot fall back onto the owner's account. + return@on originalCall + } + // Capture the server id as well so we can detect a mid-refresh server // switch — without this, a 401-refresh kicked off against server A // could land after the user has switched to server B and write A's @@ -236,30 +313,50 @@ val SiloAuthPlugin = createClientPlugin("SiloAuthPlugin", ::SiloAuthConfig) { // "refresh" — the refresh token wouldn't be valid for the new // server anyway, and we'd risk persisting cross-server tokens. val serverIdNow = tokenManager.getCurrentServerId() - if (serverIdNow != serverIdBeforeRequest) { + val serverUrlNow = tokenManager.getServerUrl() + if ( + serverIdNow != activeServerIdBeforeRequest || + !isSameHttpOrigin(trustedServerUrl, serverUrlNow) + ) { return@withLock false } - val tokenNow = tokenManager.getAccessToken() - if (tokenNow != null && tokenNow != tokenBeforeRequest) { + val tokenNow = tokenManager.getAccessTokenForScope(refreshScope) + if ( + tokenManager.getCurrentServerId() != activeServerIdBeforeRequest || + !isSameHttpOrigin(trustedServerUrl, tokenManager.getServerUrl()) + ) { + return@withLock false + } + if (tokenNow != null && "Bearer $tokenNow" != authorizationBeforeRequest) { // Another coroutine already refreshed while we were waiting — // just retry the original request with the new token. return@withLock true } - val refreshToken = tokenManager.getRefreshToken() + if (temporaryGeneration != null && + temporaryGeneration in deadCredentialGenerations.value + ) { + // A 401 that won the race already proved these temporary credentials + // are dead; the token is unchanged, so without this every waiter would + // repeat the same doomed refresh. + return@withLock false + } + + // Scope-bound, not global: a refresh must spend the token belonging to + // the scope this request ran under. + val refreshToken = tokenManager.getRefreshTokenForScope(refreshScope) if (refreshToken.isNullOrBlank()) { return@withLock false } try { diagnosticsObserver.safeAuthRefresh("started") - val serverUrl = tokenManager.getServerUrl() - if (serverUrl.isBlank()) { + if (trustedServerUrl.isBlank()) { return@withLock false } - val refreshResponse = client.post("$serverUrl/api/v1/auth/refresh") { + val refreshResponse = client.post("$trustedServerUrl/api/v1/auth/refresh") { contentType(ContentType.Application.Json) setBody(RefreshRequest(refreshToken)) } @@ -270,7 +367,11 @@ val SiloAuthPlugin = createClientPlugin("SiloAuthPlugin", ::SiloAuthConfig) { // save time, so a mismatch here means we'd write to the wrong // slot. val serverIdAfterCall = tokenManager.getCurrentServerId() - if (serverIdAfterCall != serverIdBeforeRequest) { + val serverUrlAfterCall = tokenManager.getServerUrl() + if ( + serverIdAfterCall != activeServerIdBeforeRequest || + !isSameHttpOrigin(trustedServerUrl, serverUrlAfterCall) + ) { return@withLock false } @@ -280,32 +381,53 @@ val SiloAuthPlugin = createClientPlugin("SiloAuthPlugin", ::SiloAuthConfig) { // start a refresh with the still-valid refresh token; without // this guard the refresh response lands after clearTokens() // and saveTokens() silently signs the user back in. - if (tokenManager.getRefreshToken().isNullOrBlank()) { + if (tokenManager.getRefreshTokenForScope(refreshScope).isNullOrBlank()) { return@withLock false } if (refreshResponse.status.isSuccess()) { diagnosticsObserver.safeAuthRefresh("succeeded") val tokens = refreshResponse.body() - tokenManager.saveTokens( + tokenManager.saveTokensForScope( + scope = refreshScope, accessToken = tokens.accessToken, refreshToken = tokens.refreshToken, - expiresIn = tokens.expiresIn + expiresIn = tokens.expiresIn, ) - true + val after = tokenManager.getAccessTokenForScope(refreshScope) + after != null && "Bearer $after" != authorizationBeforeRequest } else { diagnosticsObserver.safeAuthRefresh("failed") // Only auth rejection proves the refresh token is bad. // Gateway/proxy/server failures should keep the session so // a temporary outage does not sign the user out. if (refreshResponse.status.shouldInvalidateSessionAfterRefreshFailure()) { - // The [TokenManager.sessionExpired] event emitted by - // this call is what the root NavHost observer uses to - // route the user back to the login screen; without it, - // the UI would stay on Home and keep rendering - // "Failed to load..." for every subsequent API call - // that now has no credentials. - tokenManager.invalidateSession() + val generationNow = tokenManager.temporaryGenerationId() + when { + // The identity changed while the refresh was in flight + // (overlay began or ended): the rejection belongs to a + // credential set that is no longer installed, so it must + // not tear down whatever is installed now. + generationNow != temporaryGeneration -> Unit + + // Remote playback: the rejected credentials are a + // temporary overlay. invalidateSession() would drop that + // overlay, and every later read would fall through to the + // saved OWNER's account — the guest would keep browsing + // and writing history as the owner. Flag the generation + // dead and leave the overlay installed instead; the cast + // teardown path is what removes it. + temporaryGeneration != null -> + deadCredentialGenerations.update { it + temporaryGeneration } + + // The [TokenManager.sessionExpired] event emitted by + // this call is what the root NavHost observer uses to + // route the user back to the login screen; without it, + // the UI would stay on Home and keep rendering + // "Failed to load..." for every subsequent API call + // that now has no credentials. + else -> tokenManager.invalidateSessionForScope(refreshScope) + } } false } @@ -322,11 +444,23 @@ val SiloAuthPlugin = createClientPlugin("SiloAuthPlugin", ::SiloAuthConfig) { // NOT run a second time, so if we don't update the header here the // retry gets sent with the expired Bearer token and the server // returns another 401. - val newAccessToken = tokenManager.getAccessToken() - if (newAccessToken != null) { - request.headers.remove(HttpHeaders.Authorization) - request.header(HttpHeaders.Authorization, "Bearer $newAccessToken") + val retryServerIdBeforeToken = tokenManager.getCurrentServerId() + val retryServerUrlBeforeToken = tokenManager.getServerUrl() + val newAccessToken = tokenManager.getAccessTokenForScope(refreshScope) + val retryServerIdAfterToken = tokenManager.getCurrentServerId() + val retryServerUrlAfterToken = tokenManager.getServerUrl() + if ( + retryServerIdBeforeToken != activeServerIdBeforeRequest || + retryServerIdAfterToken != activeServerIdBeforeRequest || + !isSameHttpOrigin(trustedServerUrl, retryServerUrlBeforeToken) || + !isSameHttpOrigin(trustedServerUrl, retryServerUrlAfterToken) || + !isSameSiloHttpOrigin(trustedServerUrl, request.url) || + newAccessToken == null + ) { + return@on originalCall } + request.headers.remove(HttpHeaders.Authorization) + request.header(HttpHeaders.Authorization, "Bearer $newAccessToken") proceed(request) } else { originalCall @@ -361,6 +495,14 @@ private fun HttpRequestBuilder.applyProfileHeaders( } } +/** + * Generation id of the temporary credential overlay currently installed (remote + * playback), or null when the active identity is a saved account. Managers that + * don't model overlays report null, which keeps the saved-account behaviour. + */ +private suspend fun TokenManager.temporaryGenerationId(): String? = + snapshotCurrentScope()?.credentialGenerationId + private fun HttpStatusCode.shouldInvalidateSessionAfterRefreshFailure(): Boolean = this == HttpStatusCode.BadRequest || this == HttpStatusCode.Unauthorized || @@ -381,6 +523,50 @@ private suspend fun HttpRequestBuilder.attachSiloDeviceMetadataHeaders( device.clientVersion?.takeIf { it.isNotBlank() }?.let { header("X-Silo-Client-Version", it) } } +private fun URLBuilder.rebaseRelativeApiUrl(serverUrl: String) { + val originalPath = encodedPath + val originalParameters = parameters.build() + val originalFragment = fragment + val originalProtocol = protocol + + takeFrom(serverUrl) + restoreWebSocketProtocol(originalProtocol) + encodedPath = originalPath + parameters.clear() + parameters.appendAll(originalParameters) + fragment = originalFragment +} + +private fun HttpRequestBuilder.removeSiloCredentialHeaders() { + headers.remove(HttpHeaders.Authorization) + headers.remove("X-Profile-Id") + headers.remove("X-Profile-Token") + headers.names() + .filter { name -> name.startsWith("X-Silo-", ignoreCase = true) } + .forEach(headers::remove) +} + +private fun isSameSiloHttpOrigin(serverUrl: String, requestUrl: URLBuilder): Boolean { + val httpRequestUrl = when (requestUrl.protocol) { + URLProtocol.WS -> requestUrl.toString().replaceSchemeForOriginCheck("http") + URLProtocol.WSS -> requestUrl.toString().replaceSchemeForOriginCheck("https") + else -> requestUrl.toString() + } + return isSameHttpOrigin(serverUrl, httpRequestUrl) +} + +private fun isSameSiloHttpOrigin(serverUrl: String, requestUrl: Url): Boolean { + val httpRequestUrl = when (requestUrl.protocol) { + URLProtocol.WS -> requestUrl.toString().replaceSchemeForOriginCheck("http") + URLProtocol.WSS -> requestUrl.toString().replaceSchemeForOriginCheck("https") + else -> requestUrl.toString() + } + return isSameHttpOrigin(serverUrl, httpRequestUrl) +} + +private fun String.replaceSchemeForOriginCheck(scheme: String): String = + "$scheme://${substringAfter("://")}" + /** * Re-applies the websocket protocol after a `takeFrom(serverUrl)` rebase. * diff --git a/shared/src/commonMain/kotlin/org/siloserver/silo/network/AuthScopeSnapshot.kt b/shared/src/commonMain/kotlin/org/siloserver/silo/network/AuthScopeSnapshot.kt index 8c1aa32dc..f09059e25 100644 --- a/shared/src/commonMain/kotlin/org/siloserver/silo/network/AuthScopeSnapshot.kt +++ b/shared/src/commonMain/kotlin/org/siloserver/silo/network/AuthScopeSnapshot.kt @@ -20,6 +20,11 @@ import io.ktor.util.AttributeKey * refresh tokens are read live from the captured credential slot at send time * because they rotate on refresh; freezing their values would break the second * op in a drain after the first triggers a token rotation. + * + * [identityGeneration] changes before every server, account, profile, or + * temporary-scope mutation. It distinguishes a later login that happens to + * reuse the same server/profile identifiers from the credential identity that + * was active when this snapshot was captured. */ data class AuthScopeSnapshot( val serverId: String, @@ -27,6 +32,23 @@ data class AuthScopeSnapshot( val serverUrl: String, val profileToken: String?, val credentialGenerationId: String? = null, + val identityGeneration: Long = 0L, + /** + * Bumped every time this server's PERSISTENT credentials are written or + * cleared — i.e. by sign-in and sign-out, but deliberately NOT by a + * remote-playback overlay beginning or ending. + * + * Persistent scopes are keyed by serverId alone, so without this a snapshot + * captured before a sign-out would still read — and overwrite — whatever the + * next login stored for that same server. [identityGeneration] cannot serve: + * it moves for overlays too, and an overlay must leave a pinned persistent + * scope working. + * + * `0L` means "not stamped from a live snapshot"; hand-built scopes keep the + * pre-existing behaviour rather than failing closed on a value they never + * recorded. + */ + val credentialEpoch: Long = 0L, ) /** Attribute carrying the [AuthScopeSnapshot] that [SiloAuthPlugin] honors. */ diff --git a/shared/src/commonMain/kotlin/org/siloserver/silo/network/TokenManager.kt b/shared/src/commonMain/kotlin/org/siloserver/silo/network/TokenManager.kt index 17226a6a6..ed8de9456 100644 --- a/shared/src/commonMain/kotlin/org/siloserver/silo/network/TokenManager.kt +++ b/shared/src/commonMain/kotlin/org/siloserver/silo/network/TokenManager.kt @@ -43,6 +43,22 @@ interface TokenManager { */ suspend fun invalidateSession() + /** + * Invalidates [scope] without ever clearing a different active identity. + * + * Single-scope implementations may use this default. Implementations that + * support server switching must override it with an atomic scope check and + * mutation. + * + * @return true only when the captured scope was invalidated. + */ + suspend fun invalidateSessionForScope(scope: AuthScopeSnapshot): Boolean { + val current = snapshotCurrentScope() + if (current != null && current != scope) return false + invalidateSession() + return true + } + /** * Emits [Unit] each time [invalidateSession] runs. Does NOT fire for * plain [clearTokens] calls — manual signout flows own their own nav. diff --git a/shared/src/commonTest/kotlin/org/siloserver/silo/network/SiloAuthPluginPinTest.kt b/shared/src/commonTest/kotlin/org/siloserver/silo/network/SiloAuthPluginPinTest.kt index 06086402e..976c6b970 100644 --- a/shared/src/commonTest/kotlin/org/siloserver/silo/network/SiloAuthPluginPinTest.kt +++ b/shared/src/commonTest/kotlin/org/siloserver/silo/network/SiloAuthPluginPinTest.kt @@ -3,13 +3,19 @@ package org.siloserver.silo.network import io.ktor.client.HttpClient import io.ktor.client.engine.mock.MockEngine import io.ktor.client.engine.mock.respond +import io.ktor.client.plugins.contentnegotiation.ContentNegotiation +import io.ktor.client.request.get +import io.ktor.client.request.header import io.ktor.client.request.post import io.ktor.http.HttpHeaders import io.ktor.http.HttpStatusCode import io.ktor.http.headersOf +import io.ktor.http.isSuccess +import io.ktor.serialization.kotlinx.json.json import kotlinx.coroutines.test.runTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNull /** * Verifies [SiloAuthPlugin] honors an [AuthScopeSnapshot] pin: a request @@ -114,4 +120,601 @@ class SiloAuthPluginPinTest { assertEquals("Silo Android TV", captured.siloClient) assertEquals("0.2.3", captured.siloClientVersion) } + + @Test + fun normalRequestsUseCredentialsOnlyOnConfiguredOrigin() = runTest { + val tokenManager = TokenManagerImpl().apply { + setServerUrl("https://silo.example:8443") + setProfileId("profile-a") + setProfileToken("profile-token-a") + saveTokens(accessToken = "ACCESS-A", refreshToken = "REFRESH-A", expiresIn = 3600) + } + val captured = mutableListOf() + val client = capturingClient(tokenManager, captured) + + client.get("/api/v1/downloads/download-1/file") + client.get("https://silo.example:8443/api/v1/items") + client.get("https://cdn.silo.example:8443/video") + client.get("https://silo.example:9443/video") + client.get("http://silo.example:8443/video") + + assertEquals( + listOf( + "https://silo.example:8443/api/v1/downloads/download-1/file", + "https://silo.example:8443/api/v1/items", + "https://cdn.silo.example:8443/video", + "https://silo.example:9443/video", + "http://silo.example:8443/video", + ), + captured.map(CapturedRequest::url), + ) + captured.take(2).forEach { request -> + assertEquals("Bearer ACCESS-A", request.authorization) + assertEquals("profile-a", request.profileId) + assertEquals("profile-token-a", request.profileToken) + } + captured.drop(2).forEach { request -> + assertNull(request.authorization, "Authorization leaked to ${request.url}") + assertNull(request.profileId, "X-Profile-Id leaked to ${request.url}") + assertNull(request.profileToken, "X-Profile-Token leaked to ${request.url}") + } + } + + @Test + fun pinnedForeignRequestRemovesSensitiveExplicitHeaders() = runTest { + val tokenManager = TokenManagerImpl().apply { + setServerUrl("https://active.example") + saveTokens(accessToken = "ACCESS-A", refreshToken = "REFRESH-A", expiresIn = 3600) + } + val captured = mutableListOf() + val snapshot = AuthScopeSnapshot( + serverId = "server-a", + profileId = "profile-a", + serverUrl = "https://silo.example", + profileToken = "profile-token-a", + ) + + capturingClient(tokenManager, captured).get("https://cdn.example/video") { + authScope(snapshot) + header(HttpHeaders.Authorization, "Signed explicit-cdn-credential") + header("X-Profile-Id", "explicit-profile") + header("X-Profile-Token", "explicit-profile-token") + } + + val request = captured.single() + assertNull(request.authorization) + assertNull(request.profileId) + assertNull(request.profileToken) + } + + @Test + fun foreignUnauthorizedResponseDoesNotRefresh() = runTest { + val tokenManager = TokenManagerImpl().apply { + setServerUrl("https://silo.example") + setProfileId("profile-a") + setProfileToken("profile-token-a") + saveTokens(accessToken = "ACCESS-A", refreshToken = "REFRESH-A", expiresIn = 3600) + } + val captured = mutableListOf() + val engine = MockEngine { request -> + captured += request.capture() + respond( + content = "{}", + status = if (request.url.host == "cdn.example") { + HttpStatusCode.Unauthorized + } else { + HttpStatusCode.InternalServerError + }, + headers = headersOf(HttpHeaders.ContentType, "application/json"), + ) + } + val client = HttpClient(engine) { + install(SiloAuthPlugin) { this.tokenManager = tokenManager } + } + + client.get("https://cdn.example/video") + + assertEquals(listOf("https://cdn.example/video"), captured.map(CapturedRequest::url)) + assertNull(captured.single().authorization) + assertNull(captured.single().profileId) + assertNull(captured.single().profileToken) + } + + @Test + fun crossOriginRedirectDoesNotForwardSiloOrExplicitAuthorizationHeaders() = runTest { + val tokenManager = TokenManagerImpl().apply { + setServerUrl("https://silo.example") + setProfileId("profile-a") + setProfileToken("profile-token-a") + saveTokens(accessToken = "ACCESS-A", refreshToken = "REFRESH-A", expiresIn = 3600) + } + val captured = mutableListOf() + val provider = object : DeviceMetadataProvider { + override suspend fun current(): SiloDeviceMetadata = + SiloDeviceMetadata( + id = "device-1", + name = "Phone", + platform = "android", + clientName = "Silo Android", + clientVersion = "1.0", + ) + } + val engine = MockEngine { request -> + captured += request.capture() + if (request.url.host == "silo.example") { + respond( + content = "", + status = HttpStatusCode.Found, + headers = headersOf(HttpHeaders.Location, "https://cdn.example/video"), + ) + } else { + respond("ok", HttpStatusCode.OK) + } + } + val client = HttpClient(engine) { + followRedirects = true + install(SiloAuthPlugin) { + this.tokenManager = tokenManager + deviceMetadataProvider = provider + } + } + + client.get("https://silo.example/redirect") { + header(HttpHeaders.Authorization, "Signed explicit-target-credential") + } + + val downstream = captured.single { it.url == "https://cdn.example/video" } + assertNull(downstream.authorization) + assertNull(downstream.profileId) + assertNull(downstream.profileToken) + assertEquals(emptyMap(), downstream.siloHeaders) + } + + @Test + fun activeServerSwitchDuringCredentialReadFailsClosed() = runTest { + val tokenManager = SwitchingTokenManager() + val captured = mutableListOf() + + capturingClient(tokenManager, captured).get("/api/v1/items") + + val request = captured.single() + assertEquals("https://server-a.example/api/v1/items", request.url) + assertNull(request.authorization) + assertNull(request.profileId) + assertNull(request.profileToken) + } + + @Test + fun foreignSkipAuthRequestDoesNotReceiveDeviceMetadata() = runTest { + val tokenManager = TokenManagerImpl().apply { + setServerUrl("https://silo.example") + } + val captured = mutableListOf() + val provider = object : DeviceMetadataProvider { + override suspend fun current(): SiloDeviceMetadata = + SiloDeviceMetadata( + id = "device-1", + name = "Phone", + platform = "android", + clientName = "Silo Android", + clientVersion = "1.0", + ) + } + + capturingClient( + tokenManager = tokenManager, + captured = captured, + deviceMetadataProvider = provider, + ).get("https://candidate.example/api/v1/auth/device/start") { + skipSiloAuth() + } + + assertEquals(emptyMap(), captured.single().siloHeaders) + } + + @Test + fun sameOriginSkipAuthRequestKeepsDeviceMetadata() = runTest { + val tokenManager = TokenManagerImpl().apply { + setServerUrl("https://silo.example") + } + val captured = mutableListOf() + val provider = deviceMetadataProvider() + + capturingClient( + tokenManager = tokenManager, + captured = captured, + deviceMetadataProvider = provider, + ).get("https://silo.example/api/v1/auth/login") { + skipSiloAuth() + } + + assertEquals("device-1", captured.single().siloHeaders["X-Silo-Device-Id"]) + } + + @Test + fun crossOriginSkipAuthRedirectRemovesCopiedDeviceMetadata() = runTest { + val tokenManager = TokenManagerImpl().apply { + setServerUrl("https://silo.example") + } + val captured = mutableListOf() + val engine = MockEngine { request -> + captured += request.capture() + if (request.url.host == "silo.example") { + respond( + content = "", + status = HttpStatusCode.Found, + headers = headersOf(HttpHeaders.Location, "https://candidate.example/device"), + ) + } else { + respond("ok", HttpStatusCode.OK) + } + } + val client = HttpClient(engine) { + followRedirects = true + install(SiloAuthPlugin) { + this.tokenManager = tokenManager + deviceMetadataProvider = deviceMetadataProvider() + } + } + + client.get("https://silo.example/api/v1/auth/login") { + skipSiloAuth() + } + + assertEquals( + emptyMap(), + captured.single { it.url == "https://candidate.example/device" }.siloHeaders, + ) + } + + @Test + fun serverSwitchWhileRequestIsInFlightDoesNotRetryWithNewServerCredentials() = runTest { + val tokenManager = InFlightSwitchingTokenManager() + val captured = mutableListOf() + val engine = MockEngine { request -> + captured += request.capture() + if (captured.size == 1) { + tokenManager.switchToServerB() + respond("{}", HttpStatusCode.Unauthorized) + } else { + respond("{}", HttpStatusCode.OK) + } + } + val client = HttpClient(engine) { + install(SiloAuthPlugin) { this.tokenManager = tokenManager } + } + + client.get("https://server-a.example/api/v1/items") + + assertEquals(1, captured.size) + assertEquals("Bearer server-a-access", captured.single().authorization) + assertEquals(0, tokenManager.invalidations) + } + + @Test + fun normalUnauthorizedRefreshUsesOnlyCapturedScopeOperations() = runTest { + val tokenManager = ScopedRefreshTokenManager() + val paths = mutableListOf() + val client = HttpClient( + MockEngine { request -> + paths += request.url.encodedPath + when { + request.url.encodedPath.endsWith("/auth/refresh") -> + respond( + content = """ + { + "access_token": "server-a-fresh-access", + "refresh_token": "server-a-fresh-refresh", + "expires_in": 3600 + } + """.trimIndent(), + status = HttpStatusCode.OK, + headers = headersOf(HttpHeaders.ContentType, "application/json"), + ) + paths.count { it.endsWith("/catalog/home") } == 1 -> + respond("{}", HttpStatusCode.Unauthorized) + else -> respond("{}", HttpStatusCode.OK) + } + }, + ) { + install(ContentNegotiation) { json(SiloJson) } + install(SiloAuthPlugin) { this.tokenManager = tokenManager } + } + + client.get("https://server-a.example/api/v1/catalog/home") + + assertEquals( + listOf( + "/api/v1/catalog/home", + "/api/v1/auth/refresh", + "/api/v1/catalog/home", + ), + paths, + ) + assertEquals(0, tokenManager.activeRefreshReads) + assertEquals(0, tokenManager.activeSaves) + assertEquals(2, tokenManager.scopedRefreshReads) + assertEquals(1, tokenManager.scopedSaves) + } + + @Test + fun switchBeforeRefreshTokenReadNeverReadsNewActiveServerToken() = runTest { + val tokenManager = ScopedRefreshTokenManager(switchBeforeRefreshTokenRead = true) + val paths = executeRefreshScenario(tokenManager, HttpStatusCode.OK) + + assertEquals(listOf("/api/v1/catalog/home", "/api/v1/auth/refresh"), paths) + assertEquals(0, tokenManager.activeRefreshReads) + assertEquals(1, tokenManager.scopedRefreshReads) + assertEquals(0, tokenManager.activeSaves) + } + + @Test + fun switchDuringPostResponseCredentialReadSavesOnlyCapturedScope() = runTest { + val tokenManager = ScopedRefreshTokenManager(switchDuringPostResponseRead = true) + executeRefreshScenario(tokenManager, HttpStatusCode.OK) + + assertEquals(0, tokenManager.activeSaves) + assertEquals(1, tokenManager.scopedSaves) + assertEquals("server-a-fresh-access", tokenManager.scopedAccessToken) + } + + @Test + fun switchAtRefreshRejectionInvalidatesOnlyCapturedScope() = runTest { + val tokenManager = ScopedRefreshTokenManager(switchDuringInvalidation = true) + executeRefreshScenario(tokenManager, HttpStatusCode.Unauthorized) + + assertEquals(0, tokenManager.activeInvalidations) + assertEquals(1, tokenManager.scopedInvalidations) + assertEquals("server-a", tokenManager.invalidatedScope?.serverId) + assertEquals("server-b-access", tokenManager.activeAccessToken) + } + + private suspend fun executeRefreshScenario( + tokenManager: ScopedRefreshTokenManager, + refreshStatus: HttpStatusCode, + ): List { + val paths = mutableListOf() + val client = HttpClient( + MockEngine { request -> + paths += request.url.encodedPath + when { + request.url.encodedPath.endsWith("/auth/refresh") -> + respond( + content = if (refreshStatus.isSuccess()) { + """ + { + "access_token": "server-a-fresh-access", + "refresh_token": "server-a-fresh-refresh", + "expires_in": 3600 + } + """.trimIndent() + } else { + """{"error":"unauthorized","message":"expired"}""" + }, + status = refreshStatus, + headers = headersOf(HttpHeaders.ContentType, "application/json"), + ) + paths.count { it.endsWith("/catalog/home") } == 1 -> + respond("{}", HttpStatusCode.Unauthorized) + else -> respond("{}", HttpStatusCode.OK) + } + }, + ) { + install(ContentNegotiation) { json(SiloJson) } + install(SiloAuthPlugin) { this.tokenManager = tokenManager } + } + client.get("https://server-a.example/api/v1/catalog/home") + return paths + } + + private data class CapturedRequest( + val url: String, + val authorization: String?, + val profileId: String?, + val profileToken: String?, + val siloHeaders: Map, + ) + + private fun io.ktor.client.request.HttpRequestData.capture(): CapturedRequest = + CapturedRequest( + url = url.toString(), + authorization = headers[HttpHeaders.Authorization], + profileId = headers["X-Profile-Id"], + profileToken = headers["X-Profile-Token"], + siloHeaders = headers.entries() + .filter { (name, _) -> name.startsWith("X-Silo-", ignoreCase = true) } + .associate { (name, values) -> name to values.joinToString(",") }, + ) + + private fun capturingClient( + tokenManager: TokenManager, + captured: MutableList, + deviceMetadataProvider: DeviceMetadataProvider? = null, + ): HttpClient = + HttpClient( + MockEngine { request -> + captured += request.capture() + respond("{}", HttpStatusCode.OK, headersOf(HttpHeaders.ContentType, "application/json")) + }, + ) { + install(SiloAuthPlugin) { + this.tokenManager = tokenManager + this.deviceMetadataProvider = deviceMetadataProvider + } + } + + private fun deviceMetadataProvider(): DeviceMetadataProvider = + object : DeviceMetadataProvider { + override suspend fun current(): SiloDeviceMetadata = + SiloDeviceMetadata( + id = "device-1", + name = "Phone", + platform = "android", + clientName = "Silo Android", + clientVersion = "1.0", + ) + } + + private class SwitchingTokenManager( + private val delegate: TokenManager = TokenManagerImpl(), + ) : TokenManager by delegate { + private var switched = false + private var serverUrlReads = 0 + + override suspend fun getServerUrl(): String { + serverUrlReads += 1 + if (serverUrlReads > 1) switched = false + return if (switched) "https://server-b.example" else "https://server-a.example" + } + + override suspend fun getCurrentServerId(): String = + if (switched) "server-b" else "server-a" + + override suspend fun getAccessToken(): String { + switched = true + return "server-b-access" + } + + override suspend fun getProfileId(): String = "server-b-profile" + + override suspend fun getProfileToken(): String = "server-b-profile-token" + } + + private class InFlightSwitchingTokenManager( + private val delegate: TokenManager = TokenManagerImpl(), + ) : TokenManager by delegate { + private var activeServer = "server-a" + var invalidations = 0 + private set + + fun switchToServerB() { + activeServer = "server-b" + } + + override suspend fun getCurrentServerId(): String = activeServer + + override suspend fun getServerUrl(): String = "https://$activeServer.example" + + override suspend fun getAccessToken(): String = "$activeServer-access" + + override suspend fun getRefreshToken(): String = "$activeServer-refresh" + + override suspend fun getProfileId(): String = "$activeServer-profile" + + override suspend fun getProfileToken(): String = "$activeServer-profile-token" + + override suspend fun invalidateSession() { + invalidations += 1 + } + } + + private class ScopedRefreshTokenManager( + private val switchBeforeRefreshTokenRead: Boolean = false, + private val switchDuringPostResponseRead: Boolean = false, + private val switchDuringInvalidation: Boolean = false, + private val delegate: TokenManager = TokenManagerImpl(), + ) : TokenManager by delegate { + private val scope = AuthScopeSnapshot( + serverId = "server-a", + profileId = null, + serverUrl = "https://server-a.example", + profileToken = null, + identityGeneration = 7L, + ) + private var activeServer = "server-a" + private var accessToken = "server-a-expired-access" + private var armSwitchAfterServerUrlRead = false + private var activeAccessReads = 0 + var activeRefreshReads = 0 + private set + var activeSaves = 0 + private set + var scopedRefreshReads = 0 + private set + var scopedSaves = 0 + private set + var activeInvalidations = 0 + private set + var scopedInvalidations = 0 + private set + var invalidatedScope: AuthScopeSnapshot? = null + private set + val scopedAccessToken: String get() = accessToken + val activeAccessToken: String + get() = if (activeServer == "server-a") accessToken else "server-b-access" + + override suspend fun snapshotCurrentScope(): AuthScopeSnapshot = scope + + override suspend fun getCurrentServerId(): String = activeServer + + override suspend fun getServerUrl(): String { + val url = "https://$activeServer.example" + if (armSwitchAfterServerUrlRead) { + armSwitchAfterServerUrlRead = false + activeServer = "server-b" + } + return url + } + + override suspend fun getAccessToken(): String { + activeAccessReads += 1 + if (switchBeforeRefreshTokenRead && activeAccessReads > 1) { + armSwitchAfterServerUrlRead = true + } + return activeAccessToken + } + + override suspend fun getRefreshToken(): String { + activeRefreshReads += 1 + val token = "$activeServer-refresh" + if (switchDuringPostResponseRead && activeRefreshReads == 2) { + activeServer = "server-b" + } + return token + } + + override suspend fun saveTokens( + accessToken: String, + refreshToken: String, + expiresIn: Long, + ) { + activeSaves += 1 + this.accessToken = accessToken + } + + override suspend fun getAccessTokenForScope(scope: AuthScopeSnapshot): String { + if (switchBeforeRefreshTokenRead) armSwitchAfterServerUrlRead = true + return accessToken + } + + override suspend fun getRefreshTokenForScope(scope: AuthScopeSnapshot): String { + scopedRefreshReads += 1 + if (switchDuringPostResponseRead && scopedRefreshReads == 2) { + activeServer = "server-b" + } + return "server-a-refresh" + } + + override suspend fun saveTokensForScope( + scope: AuthScopeSnapshot, + accessToken: String, + refreshToken: String, + expiresIn: Long, + ) { + scopedSaves += 1 + this.accessToken = accessToken + } + + override suspend fun invalidateSession() { + if (switchDuringInvalidation) activeServer = "server-b" + activeInvalidations += 1 + } + + override suspend fun invalidateSessionForScope(scope: AuthScopeSnapshot): Boolean { + if (switchDuringInvalidation) activeServer = "server-b" + scopedInvalidations += 1 + invalidatedScope = scope + return true + } + } } From ccb84620227abc982f58dea7c131c2f243a9c4d4 Mon Sep 17 00:00:00 2001 From: rxwatcher Date: Mon, 27 Jul 2026 09:39:51 +0200 Subject: [PATCH 03/17] fix(auth): require consent before cleartext login --- .../common/network/CleartextConsentStore.kt | 61 ++++++ .../network/CleartextConsentStoreTest.kt | 59 ++++++ .../silo/android/di/AndroidModule.kt | 5 +- .../ui/screens/auth/ServerSetupScreen.kt | 31 +++ .../ui/screens/auth/ServerSetupViewModel.kt | 127 ++++++++++-- .../auth/ServerSetupPersistenceTest.kt | 190 +++++++++++++++++- .../siloserver/silo/tv/di/AndroidTvModule.kt | 5 +- .../tv/ui/screens/auth/TvServerSetupScreen.kt | 31 +++ .../ui/screens/auth/TvServerSetupViewModel.kt | 107 +++++++++- .../auth/TvServerSetupPersistenceTest.kt | 180 ++++++++++++++++- .../silo/network/EncryptedTokenManagerImpl.kt | 6 +- ...ncryptedTokenManagerScopeGenerationTest.kt | 38 +++- 12 files changed, 795 insertions(+), 45 deletions(-) create mode 100644 android-shared/src/androidMain/kotlin/org/siloserver/silo/common/network/CleartextConsentStore.kt create mode 100644 android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/network/CleartextConsentStoreTest.kt diff --git a/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/network/CleartextConsentStore.kt b/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/network/CleartextConsentStore.kt new file mode 100644 index 000000000..6132f74e3 --- /dev/null +++ b/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/network/CleartextConsentStore.kt @@ -0,0 +1,61 @@ +package org.siloserver.silo.common.network + +import android.content.Context +import androidx.datastore.core.DataStore +import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.PreferenceDataStoreFactory +import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.stringSetPreferencesKey +import androidx.datastore.preferences.preferencesDataStoreFile +import java.net.URI +import java.security.MessageDigest +import kotlinx.coroutines.flow.first + +interface CleartextConsentStore { + suspend fun isApproved(origin: String): Boolean + suspend fun approve(origin: String) +} + +class DataStoreCleartextConsentStore( + private val dataStore: DataStore, +) : CleartextConsentStore { + constructor(context: Context) : this( + PreferenceDataStoreFactory.create { + context.preferencesDataStoreFile(DATA_STORE_NAME) + }, + ) + + override suspend fun isApproved(origin: String): Boolean { + val normalized = cleartextOrigin(origin) ?: return false + return originDigest(normalized) in dataStore.data.first()[APPROVED_ORIGIN_DIGESTS].orEmpty() + } + + override suspend fun approve(origin: String) { + val normalized = requireNotNull(cleartextOrigin(origin)) { + "Cleartext approval requires a valid HTTP origin" + } + val digest = originDigest(normalized) + dataStore.edit { preferences -> + preferences[APPROVED_ORIGIN_DIGESTS] = + preferences[APPROVED_ORIGIN_DIGESTS].orEmpty() + digest + } + } + + private companion object { + private const val DATA_STORE_NAME = "silo_cleartext_consent" + private val APPROVED_ORIGIN_DIGESTS = stringSetPreferencesKey("approved_origin_sha256") + + private fun originDigest(origin: String): String = + MessageDigest.getInstance("SHA-256") + .digest(origin.encodeToByteArray()) + .joinToString("") { byte -> "%02x".format(byte) } + } +} + +fun cleartextOrigin(url: String): String? = runCatching { + val uri = URI(url.trim()) + if (!uri.scheme.equals("http", ignoreCase = true)) return null + val host = uri.host?.lowercase()?.takeIf(String::isNotBlank) ?: return null + val port = if (uri.port == 80) -1 else uri.port + URI("http", null, host, port, null, null, null).toASCIIString() +}.getOrNull() diff --git a/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/network/CleartextConsentStoreTest.kt b/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/network/CleartextConsentStoreTest.kt new file mode 100644 index 000000000..04316f85a --- /dev/null +++ b/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/network/CleartextConsentStoreTest.kt @@ -0,0 +1,59 @@ +package org.siloserver.silo.common.network + +import androidx.datastore.preferences.core.PreferenceDataStoreFactory +import java.nio.file.Files +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.runTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class CleartextConsentStoreTest { + + @Test + fun approvalPersistsOnlyNormalizedOriginDigest() = runTest { + val file = Files.createTempDirectory("cleartext-consent").resolve("preferences.preferences_pb").toFile() + val dataStore = PreferenceDataStoreFactory.create(scope = backgroundScope) { file } + val store = DataStoreCleartextConsentStore(dataStore) + val supplied = "HTTP://User:secret@SILO.LAN:80/private?token=credential#fragment" + + store.approve(supplied) + + assertTrue(store.isApproved("http://silo.lan/another-path")) + val persisted = dataStore.data.first().asMap() + assertEquals(1, persisted.size) + val digests = persisted.values.single() as Set<*> + assertEquals(1, digests.size) + assertTrue((digests.single() as String).matches(Regex("[0-9a-f]{64}"))) + val serialized = persisted.toString() + assertFalse(serialized.contains("silo.lan", ignoreCase = true)) + assertFalse(serialized.contains("secret", ignoreCase = true)) + assertFalse(serialized.contains("token", ignoreCase = true)) + assertFalse(serialized.contains("credential", ignoreCase = true)) + } + + @Test + fun approvalsAreSpecificToNormalizedSchemeHostAndPort() = runTest { + val file = Files.createTempDirectory("cleartext-origin").resolve("preferences.preferences_pb").toFile() + val store = DataStoreCleartextConsentStore( + PreferenceDataStoreFactory.create(scope = backgroundScope) { file }, + ) + + store.approve("http://SILO.LAN:8090/path") + + assertTrue(store.isApproved("http://silo.lan:8090/other")) + assertFalse(store.isApproved("http://silo.lan:8091")) + assertFalse(store.isApproved("http://other.lan:8090")) + assertFalse(store.isApproved("https://silo.lan:8090")) + } + + @Test + fun normalizedOriginDropsCredentialsPathQueryFragmentAndDefaultPort() { + assertEquals( + "http://silo.lan", + cleartextOrigin("HTTP://user:password@SILO.LAN:80/path?q=token#fragment"), + ) + assertEquals("http://silo.lan:8090", cleartextOrigin("http://SILO.LAN:8090/path")) + } +} diff --git a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/di/AndroidModule.kt b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/di/AndroidModule.kt index 1c64c4bec..f599dd591 100644 --- a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/di/AndroidModule.kt +++ b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/di/AndroidModule.kt @@ -29,6 +29,8 @@ import org.siloserver.silo.common.player.backend.VideoPlaybackBackendFactory import org.siloserver.silo.common.player.video.VideoPlaybackSessionCoordinator import org.siloserver.silo.common.player.video.VideoPlaybackStarter import org.siloserver.silo.common.network.AndroidDeviceMetadataProvider +import org.siloserver.silo.common.network.CleartextConsentStore +import org.siloserver.silo.common.network.DataStoreCleartextConsentStore import org.siloserver.silo.common.settings.AndroidServerSettingsCache import android.content.SharedPreferences import org.siloserver.silo.network.AndroidServerRegistry @@ -100,6 +102,7 @@ import org.koin.dsl.module * the viewModel DSL for proper lifecycle integration. */ val androidModule = module { + single { DataStoreCleartextConsentStore(androidContext()) } // Single encrypted prefs handle shared between the server registry and the // token manager — opening it twice means two MasterKey lookups + decryption // passes on cold start. @@ -402,7 +405,7 @@ val androidModule = module { viewModel { AdminScansViewModel(get(), get()) } viewModel { DownloadsViewModel(get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get()) } viewModel { org.siloserver.silo.android.ui.screens.pairing.CompanionPairingViewModel(get(), get()) } - viewModel { ServerSetupViewModel(get()) } + viewModel { ServerSetupViewModel(get(), get()) } viewModel { LoginViewModel(get()) } viewModel { SetupViewModel(get()) } viewModel { SignupViewModel(get()) } diff --git a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/auth/ServerSetupScreen.kt b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/auth/ServerSetupScreen.kt index 362981be9..f2696ba18 100644 --- a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/auth/ServerSetupScreen.kt +++ b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/auth/ServerSetupScreen.kt @@ -15,6 +15,7 @@ import androidx.compose.foundation.layout.width import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.KeyboardArrowDown import androidx.compose.material3.Icon +import androidx.compose.material3.AlertDialog import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect @@ -88,6 +89,36 @@ fun ServerSetupScreen( var showAdvanced by rememberSaveable { mutableStateOf(false) } + state.pendingCleartextUrl?.let { origin -> + AlertDialog( + onDismissRequest = viewModel::cancelCleartextConnection, + title = { Text("Use unencrypted HTTP?") }, + text = { + Column(verticalArrangement = Arrangement.spacedBy(12.dp)) { + Text(origin, fontWeight = FontWeight.SemiBold) + Text( + "This connection is not encrypted. Anyone on the network may see or change " + + "traffic, including your sign-in. Continue only on a network you trust.", + ) + } + }, + confirmButton = { + AuroraPrimaryButton( + label = "Use HTTP", + onClick = viewModel::confirmCleartextConnection, + modifier = Modifier.width(140.dp), + isLoading = state.isLoading, + ) + }, + dismissButton = { + AuroraGhostButton( + label = "Cancel", + onClick = viewModel::cancelCleartextConnection, + ) + }, + ) + } + AuroraScreen(variant = AuroraVariant.Server) { // Silo wordmark (iOS width 132). Image( diff --git a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/auth/ServerSetupViewModel.kt b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/auth/ServerSetupViewModel.kt index 5f7d82e3b..d017da833 100644 --- a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/auth/ServerSetupViewModel.kt +++ b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/auth/ServerSetupViewModel.kt @@ -2,6 +2,10 @@ package org.siloserver.silo.android.ui.screens.auth import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import org.siloserver.silo.common.network.CleartextConsentStore +import org.siloserver.silo.common.network.cleartextOrigin +import org.siloserver.silo.model.auth.SetupStatusResponse +import org.siloserver.silo.model.auth.SignupStatusResponse import org.siloserver.silo.network.AndroidServerRegistry import org.siloserver.silo.network.ApiResult import org.siloserver.silo.repository.AuthRepository @@ -20,6 +24,7 @@ data class ServerSetupUiState( val error: String? = null, /** Destination after successful server validation. */ val navigateTo: ServerSetupDestination? = null, + val pendingCleartextUrl: String? = null, ) { /** True when the entered address will connect over unencrypted HTTP. * Informational only — does not block connecting to LAN/IP servers. */ @@ -45,10 +50,21 @@ sealed class ServerSetupDestination { class ServerSetupViewModel( private val authRepository: AuthRepository, + private val cleartextConsentStore: CleartextConsentStore, + private val getSetupStatus: suspend (String) -> ApiResult = { + authRepository.getSetupStatus(it) + }, + private val getSignupStatus: suspend (String) -> ApiResult = { + authRepository.getSignupStatus(it) + }, + private val candidateUrls: (ServerSetupUiState) -> List = { + buildServerSetupCandidateUrls(it.serverUrl, it.selectedScheme, it.port) + }, ) : ViewModel() { private val _uiState = MutableStateFlow(ServerSetupUiState()) val uiState: StateFlow = _uiState.asStateFlow() + private var pendingCleartextConnection: PendingCleartextConnection? = null init { // Pre-populate with previously saved server URL, if any. @@ -82,31 +98,24 @@ class ServerSetupViewModel( if (_uiState.value.isLoading) return val current = _uiState.value val candidates = try { - buildServerSetupCandidateUrls( - rawInput = current.serverUrl, - selectedScheme = current.selectedScheme, - port = current.port, - ) + candidateUrls(current) } catch (error: ServerSetupValidationException) { _uiState.update { it.copy(error = error.message) } return } viewModelScope.launch { - _uiState.update { it.copy(isLoading = true, error = null) } + pendingCleartextConnection = null + _uiState.update { + it.copy(isLoading = true, error = null, pendingCleartextUrl = null) + } var lastError: String? = null for (candidate in candidates) { - when (val setupResult = authRepository.getSetupStatus(candidate)) { + when (val setupResult = getSetupStatus(candidate)) { is ApiResult.Success -> { if (setupResult.data.needsSetup) { - authRepository.setServerUrl(candidate) - _uiState.update { - it.copy( - isLoading = false, - navigateTo = ServerSetupDestination.Setup, - ) - } + handleSuccessfulConnection(candidate, ServerSetupDestination.Setup) return@launch } } @@ -122,18 +131,15 @@ class ServerSetupViewModel( } } - val signupEnabled = when (val signupResult = authRepository.getSignupStatus(candidate)) { + val signupEnabled = when (val signupResult = getSignupStatus(candidate)) { is ApiResult.Success -> signupResult.data.enabled else -> false // If we can't determine, default to no signup. } - authRepository.setServerUrl(candidate) - _uiState.update { - it.copy( - isLoading = false, - navigateTo = ServerSetupDestination.Login(signupEnabled = signupEnabled), - ) - } + handleSuccessfulConnection( + candidate, + ServerSetupDestination.Login(signupEnabled = signupEnabled), + ) return@launch } @@ -149,12 +155,89 @@ class ServerSetupViewModel( } } + fun confirmCleartextConnection() { + if (_uiState.value.isLoading) return + val pending = pendingCleartextConnection ?: return + _uiState.update { it.copy(isLoading = true, error = null) } + viewModelScope.launch { + cleartextConsentStore.approve(pending.origin) + if (pendingCleartextConnection !== pending) return@launch + pendingCleartextConnection = null + persistAndNavigate(pending.serverUrl, pending.destination) + } + } + + fun cancelCleartextConnection() { + pendingCleartextConnection = null + _uiState.update { + it.copy(pendingCleartextUrl = null, isLoading = false) + } + } + /** Resets navigation state after the UI has consumed the event. */ fun onNavigationConsumed() { _uiState.update { it.copy(navigateTo = null) } } + + private suspend fun handleSuccessfulConnection( + serverUrl: String, + destination: ServerSetupDestination, + ) { + if (serverUrl.startsWith("http://", ignoreCase = true)) { + val origin = cleartextOrigin(serverUrl) + if (origin == null) { + _uiState.update { + it.copy( + isLoading = false, + error = "Could not safely identify this HTTP server.", + pendingCleartextUrl = null, + navigateTo = null, + ) + } + return + } + if (cleartextConsentStore.isApproved(origin)) { + persistAndNavigate(serverUrl, destination) + return + } + pendingCleartextConnection = PendingCleartextConnection( + serverUrl = serverUrl, + origin = origin, + destination = destination, + ) + _uiState.update { + it.copy( + isLoading = false, + pendingCleartextUrl = origin, + navigateTo = null, + ) + } + return + } + persistAndNavigate(serverUrl, destination) + } + + private suspend fun persistAndNavigate( + serverUrl: String, + destination: ServerSetupDestination, + ) { + authRepository.setServerUrl(serverUrl) + _uiState.update { + it.copy( + isLoading = false, + pendingCleartextUrl = null, + navigateTo = destination, + ) + } + } } +private data class PendingCleartextConnection( + val serverUrl: String, + val origin: String, + val destination: ServerSetupDestination, +) + /** * True when the address WILL connect over plaintext HTTP: an explicit `http` * scheme selection, or an `http://` prefix in the input. Auto mode is not diff --git a/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/auth/ServerSetupPersistenceTest.kt b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/auth/ServerSetupPersistenceTest.kt index be1fa2298..4279d89a6 100644 --- a/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/auth/ServerSetupPersistenceTest.kt +++ b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/auth/ServerSetupPersistenceTest.kt @@ -7,21 +7,31 @@ import io.ktor.serialization.kotlinx.json.json import java.io.IOException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.setMain +import kotlinx.coroutines.withTimeout import org.siloserver.silo.network.SiloAuthPlugin import org.siloserver.silo.network.SiloJson import org.siloserver.silo.network.TokenManager +import org.siloserver.silo.network.ApiResult import org.siloserver.silo.network.api.AuthApi +import org.siloserver.silo.common.network.CleartextConsentStore +import org.siloserver.silo.model.auth.SetupStatusResponse +import org.siloserver.silo.model.auth.SignupStatusResponse import org.siloserver.silo.repository.AuthRepository import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNull @OptIn(ExperimentalCoroutinesApi::class) class ServerSetupPersistenceTest { @@ -54,7 +64,7 @@ class ServerSetupPersistenceTest { ), tokenManager = tokenManager, ) - val viewModel = ServerSetupViewModel(repository) + val viewModel = ServerSetupViewModel(repository, FakeCleartextConsentStore()) viewModel.onServerUrlChanged("bad.silo") viewModel.onConnectClick() @@ -67,10 +77,165 @@ class ServerSetupPersistenceTest { "Failed probes must not persist candidate URLs.", ) } + + @Test + fun successfulHttpFallbackStopsForConfirmationBeforePersistence() = runTest(dispatcher) { + val tokenManager = RecordingTokenManager() + val viewModel = viewModelFor(tokenManager) + + viewModel.onServerUrlChanged("silo.lan") + viewModel.onConnectClick() + awaitSettled(viewModel) + + assertEquals( + "http://silo.lan", + viewModel.uiState.value.pendingCleartextUrl, + viewModel.uiState.value.toString(), + ) + assertEquals(emptyList(), tokenManager.serverUrlWrites) + assertNull(viewModel.uiState.value.navigateTo) + + viewModel.confirmCleartextConnection() + awaitSettled(viewModel) + + assertEquals(listOf("http://silo.lan"), tokenManager.serverUrlWrites) + assertEquals(ServerSetupDestination.Login(signupEnabled = true), viewModel.uiState.value.navigateTo) + } + + @Test + fun cancelCleartextConnectionClearsPendingWithoutPersistence() = runTest(dispatcher) { + val tokenManager = RecordingTokenManager() + val viewModel = viewModelFor(tokenManager) + + viewModel.onServerUrlChanged("silo.lan") + viewModel.onConnectClick() + awaitSettled(viewModel) + viewModel.cancelCleartextConnection() + + assertNull(viewModel.uiState.value.pendingCleartextUrl) + assertEquals(emptyList(), tokenManager.serverUrlWrites) + assertNull(viewModel.uiState.value.navigateTo) + } + + @Test + fun cleartextApprovalIsOriginSpecific() = runTest(dispatcher) { + val tokenManager = RecordingTokenManager() + val approvals = FakeCleartextConsentStore().apply { approve("http://other.lan") } + val viewModel = viewModelFor(tokenManager, approvals) + + viewModel.onServerUrlChanged("silo.lan") + viewModel.onConnectClick() + awaitSettled(viewModel) + + assertEquals("http://silo.lan", viewModel.uiState.value.pendingCleartextUrl) + assertEquals(emptyList(), tokenManager.serverUrlWrites) + } + + @Test + fun priorOriginApprovalAndHttpsBypassConfirmation() = runTest(dispatcher) { + val approvedTokens = RecordingTokenManager() + val approvals = FakeCleartextConsentStore().apply { approve("http://silo.lan") } + val approvedViewModel = viewModelFor(approvedTokens, approvals) + + approvedViewModel.onServerUrlChanged("silo.lan") + approvedViewModel.onConnectClick() + awaitSettled(approvedViewModel) + + assertNull(approvedViewModel.uiState.value.pendingCleartextUrl) + assertEquals(listOf("http://silo.lan"), approvedTokens.serverUrlWrites) + + val httpsTokens = RecordingTokenManager() + val httpsViewModel = viewModelFor(httpsTokens, httpsSucceeds = true) + httpsViewModel.onServerUrlChanged("secure.silo") + httpsViewModel.onConnectClick() + awaitSettled(httpsViewModel) + + assertNull(httpsViewModel.uiState.value.pendingCleartextUrl) + assertEquals(listOf("https://secure.silo"), httpsTokens.serverUrlWrites) + } + + @Test + fun cancelWinsRaceWithInFlightCleartextApproval() = runTest(dispatcher) { + val tokenManager = RecordingTokenManager() + val approvals = BlockingCleartextConsentStore() + val viewModel = viewModelFor(tokenManager, approvals) + viewModel.onServerUrlChanged("silo.lan") + viewModel.onConnectClick() + awaitSettled(viewModel) + + viewModel.confirmCleartextConnection() + runCurrent() + approvals.approveStarted.await() + viewModel.cancelCleartextConnection() + approvals.releaseApproval.complete(Unit) + advanceUntilIdle() + + assertNull(viewModel.uiState.value.pendingCleartextUrl) + assertNull(viewModel.uiState.value.navigateTo) + assertEquals(emptyList(), tokenManager.serverUrlWrites) + } + + @Test + fun successfulUnnormalizableHttpCandidateFailsClosed() = runTest(dispatcher) { + val tokenManager = RecordingTokenManager() + val viewModel = ServerSetupViewModel( + authRepository = repositoryFor(tokenManager), + cleartextConsentStore = FakeCleartextConsentStore(), + candidateUrls = { listOf("http://silo_lan") }, + getSetupStatus = { ApiResult.Success(SetupStatusResponse(needsSetup = false)) }, + getSignupStatus = { ApiResult.Success(SignupStatusResponse(enabled = true)) }, + ) + + viewModel.onServerUrlChanged("silo_lan") + viewModel.onConnectClick() + awaitSettled(viewModel) + + assertEquals(emptyList(), tokenManager.serverUrlWrites) + assertNull(viewModel.uiState.value.navigateTo) + assertEquals("Could not safely identify this HTTP server.", viewModel.uiState.value.error) + } + + private fun viewModelFor( + tokenManager: RecordingTokenManager, + approvals: FakeCleartextConsentStore = FakeCleartextConsentStore(), + httpsSucceeds: Boolean = false, + ) = ServerSetupViewModel( + authRepository = repositoryFor(tokenManager), + cleartextConsentStore = approvals, + getSetupStatus = { candidate -> + if (candidate.startsWith("https://") && !httpsSucceeds) { + ApiResult.NetworkError(IOException("TLS unavailable")) + } else { + ApiResult.Success(SetupStatusResponse(needsSetup = false)) + } + }, + getSignupStatus = { + ApiResult.Success(SignupStatusResponse(enabled = true)) + }, + ) + + private fun repositoryFor( + tokenManager: RecordingTokenManager, + ): AuthRepository = AuthRepository( + authApi = AuthApi( + HttpClient(MockEngine { throw IOException("Unexpected network request") }) { + install(ContentNegotiation) { json(SiloJson) } + install(SiloAuthPlugin) { this.tokenManager = tokenManager } + }, + ), + tokenManager = tokenManager, + ) + + private suspend fun TestScope.awaitSettled(viewModel: ServerSetupViewModel) { + runCurrent() + withTimeout(5_000) { + viewModel.uiState.first { !it.isLoading } + } + } } private class RecordingTokenManager( - private var serverUrl: String, + private var serverUrl: String = "", ) : TokenManager { val serverUrlWrites = mutableListOf() override val sessionExpired = MutableSharedFlow() @@ -92,3 +257,24 @@ private class RecordingTokenManager( override suspend fun switchActiveServer(serverId: String?) = Unit override suspend fun signOutCurrentServer() = Unit } + +private open class FakeCleartextConsentStore : CleartextConsentStore { + private val approved = mutableSetOf() + + override suspend fun isApproved(origin: String): Boolean = origin in approved + + override suspend fun approve(origin: String) { + approved += origin + } +} + +private class BlockingCleartextConsentStore : FakeCleartextConsentStore() { + val approveStarted = CompletableDeferred() + val releaseApproval = CompletableDeferred() + + override suspend fun approve(origin: String) { + approveStarted.complete(Unit) + releaseApproval.await() + super.approve(origin) + } +} diff --git a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/di/AndroidTvModule.kt b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/di/AndroidTvModule.kt index d116409e0..f0b25e7c2 100644 --- a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/di/AndroidTvModule.kt +++ b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/di/AndroidTvModule.kt @@ -7,6 +7,8 @@ import org.siloserver.silo.repository.SettingsRepository import org.siloserver.silo.tv.data.preferences.LegacyTvPrefsMigration import org.siloserver.silo.tv.data.preferences.TvLibrarySelectionStore import org.siloserver.silo.common.network.AndroidDeviceMetadataProvider +import org.siloserver.silo.common.network.CleartextConsentStore +import org.siloserver.silo.common.network.DataStoreCleartextConsentStore import org.siloserver.silo.common.settings.AndroidServerSettingsCache import android.content.SharedPreferences import org.siloserver.silo.network.AndroidServerRegistry @@ -71,6 +73,7 @@ import org.koin.dsl.module * [org.siloserver.silo.tv.SiloTvApplication]. */ val androidTvModule = module { + single { DataStoreCleartextConsentStore(androidContext()) } // Single encrypted prefs handle shared between the server registry and // the token manager — see the phone module for rationale. single { createSecureSharedPrefs(androidContext()) } @@ -319,7 +322,7 @@ val androidTvModule = module { } // Auth ViewModels - viewModel { TvServerSetupViewModel(get()) } + viewModel { TvServerSetupViewModel(get(), get()) } viewModel { org.siloserver.silo.tv.ui.screens.auth.TvSetupViewModel(get()) } viewModel { org.siloserver.silo.tv.ui.screens.auth.TvSignupViewModel(get()) } viewModel { TvLoginViewModel(get(), get(), get()) } diff --git a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/auth/TvServerSetupScreen.kt b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/auth/TvServerSetupScreen.kt index a154fe858..048d7a109 100644 --- a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/auth/TvServerSetupScreen.kt +++ b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/auth/TvServerSetupScreen.kt @@ -75,6 +75,7 @@ import androidx.compose.ui.unit.sp import androidx.compose.foundation.shape.CircleShape import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.OutlinedTextField +import androidx.compose.material3.AlertDialog import androidx.tv.material3.ExperimentalTvMaterial3Api import androidx.tv.material3.Icon import androidx.tv.material3.MaterialTheme @@ -169,6 +170,36 @@ fun TvServerSetupScreen( } } + state.pendingCleartextUrl?.let { origin -> + AlertDialog( + onDismissRequest = viewModel::cancelCleartextConnection, + title = { Text("Use unencrypted HTTP?") }, + text = { + Column(verticalArrangement = Arrangement.spacedBy(12.dp)) { + Text(origin, fontWeight = FontWeight.SemiBold) + Text( + "This connection is not encrypted. Anyone on the network may see or change " + + "traffic, including your sign-in. Continue only on a network you trust.", + ) + } + }, + confirmButton = { + AuroraPrimaryButton( + label = "Use HTTP", + onClick = viewModel::confirmCleartextConnection, + modifier = Modifier.width(180.dp), + enabled = !state.isLoading, + ) + }, + dismissButton = { + AuroraGhostButton( + label = "Cancel", + onClick = viewModel::cancelCleartextConnection, + ) + }, + ) + } + Box( modifier = Modifier .fillMaxSize() diff --git a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/auth/TvServerSetupViewModel.kt b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/auth/TvServerSetupViewModel.kt index 82fac3404..c8abe6aa1 100644 --- a/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/auth/TvServerSetupViewModel.kt +++ b/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/auth/TvServerSetupViewModel.kt @@ -2,6 +2,8 @@ package org.siloserver.silo.tv.ui.screens.auth import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import org.siloserver.silo.common.network.CleartextConsentStore +import org.siloserver.silo.common.network.cleartextOrigin import org.siloserver.silo.model.auth.SetupStatusResponse import org.siloserver.silo.model.auth.SignupStatusResponse import org.siloserver.silo.network.AndroidServerRegistry @@ -19,6 +21,7 @@ data class TvServerSetupUiState( val error: String? = null, /** Destination after a successful server probe; consumed by the screen. */ val navigateTo: TvServerSetupDestination? = null, + val pendingCleartextUrl: String? = null, ) { /** True when the entered address will connect over unencrypted HTTP. * Informational only — does not block LAN/IP connections. */ @@ -64,10 +67,18 @@ internal sealed class TvServerSetupProbeResult { */ class TvServerSetupViewModel( private val authRepository: AuthRepository, + private val cleartextConsentStore: CleartextConsentStore, + private val getSetupStatus: suspend (String) -> ApiResult = { + authRepository.getSetupStatus(it) + }, + private val getSignupStatus: suspend (String) -> ApiResult = { + authRepository.getSignupStatus(it) + }, ) : ViewModel() { private val _uiState = MutableStateFlow(TvServerSetupUiState()) val uiState: StateFlow = _uiState.asStateFlow() + private var pendingCleartextConnection: PendingTvCleartextConnection? = null init { viewModelScope.launch { @@ -100,22 +111,18 @@ class TvServerSetupViewModel( val candidates = serverSetupUrlProbeCandidates(raw) viewModelScope.launch { - _uiState.update { it.copy(isLoading = true, error = null) } + pendingCleartextConnection = null + _uiState.update { + it.copy(isLoading = true, error = null, pendingCleartextUrl = null) + } when (val result = probeTvServerSetupCandidates( candidates = candidates, - getSetupStatus = { candidate -> authRepository.getSetupStatus(candidate) }, - getSignupStatus = { candidate -> authRepository.getSignupStatus(candidate) }, + getSetupStatus = getSetupStatus, + getSignupStatus = getSignupStatus, )) { is TvServerSetupProbeResult.Success -> { - authRepository.setServerUrl(result.serverUrl) - _uiState.update { - it.copy( - serverUrl = result.serverUrl, - isLoading = false, - navigateTo = result.destination, - ) - } + handleSuccessfulConnection(result.serverUrl, result.destination) } is TvServerSetupProbeResult.Failure -> { _uiState.update { @@ -130,11 +137,89 @@ class TvServerSetupViewModel( } } + fun confirmCleartextConnection() { + if (_uiState.value.isLoading) return + val pending = pendingCleartextConnection ?: return + _uiState.update { it.copy(isLoading = true, error = null) } + viewModelScope.launch { + cleartextConsentStore.approve(pending.origin) + if (pendingCleartextConnection !== pending) return@launch + pendingCleartextConnection = null + persistAndNavigate(pending.serverUrl, pending.destination) + } + } + + fun cancelCleartextConnection() { + pendingCleartextConnection = null + _uiState.update { + it.copy(pendingCleartextUrl = null, isLoading = false) + } + } + fun onNavigationConsumed() { _uiState.update { it.copy(navigateTo = null) } } + + private suspend fun handleSuccessfulConnection( + serverUrl: String, + destination: TvServerSetupDestination, + ) { + if (serverUrl.startsWith("http://", ignoreCase = true)) { + val origin = cleartextOrigin(serverUrl) + if (origin == null) { + _uiState.update { + it.copy( + isLoading = false, + error = "Could not safely identify this HTTP server.", + pendingCleartextUrl = null, + navigateTo = null, + ) + } + return + } + if (cleartextConsentStore.isApproved(origin)) { + persistAndNavigate(serverUrl, destination) + return + } + pendingCleartextConnection = PendingTvCleartextConnection( + serverUrl = serverUrl, + origin = origin, + destination = destination, + ) + _uiState.update { + it.copy( + isLoading = false, + pendingCleartextUrl = origin, + navigateTo = null, + ) + } + return + } + persistAndNavigate(serverUrl, destination) + } + + private suspend fun persistAndNavigate( + serverUrl: String, + destination: TvServerSetupDestination, + ) { + authRepository.setServerUrl(serverUrl) + _uiState.update { + it.copy( + serverUrl = serverUrl, + isLoading = false, + pendingCleartextUrl = null, + navigateTo = destination, + ) + } + } } +private data class PendingTvCleartextConnection( + val serverUrl: String, + val origin: String, + val destination: TvServerSetupDestination, +) + internal suspend fun probeTvServerSetupCandidates( candidates: List, getSetupStatus: suspend (String) -> ApiResult, diff --git a/androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/auth/TvServerSetupPersistenceTest.kt b/androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/auth/TvServerSetupPersistenceTest.kt index 410ff6ecc..c82f3aca2 100644 --- a/androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/auth/TvServerSetupPersistenceTest.kt +++ b/androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/auth/TvServerSetupPersistenceTest.kt @@ -7,23 +7,33 @@ import io.ktor.serialization.kotlinx.json.json import java.io.IOException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runCurrent import androidx.lifecycle.viewModelScope import kotlinx.coroutines.cancel import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.setMain +import kotlinx.coroutines.withTimeout import org.siloserver.silo.network.SiloAuthPlugin import org.siloserver.silo.network.SiloJson import org.siloserver.silo.network.TokenManager +import org.siloserver.silo.network.ApiResult import org.siloserver.silo.network.api.AuthApi +import org.siloserver.silo.common.network.CleartextConsentStore +import org.siloserver.silo.model.auth.SetupStatusResponse +import org.siloserver.silo.model.auth.SignupStatusResponse import org.siloserver.silo.repository.AuthRepository import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNull @OptIn(ExperimentalCoroutinesApi::class) class TvServerSetupPersistenceTest { @@ -70,7 +80,7 @@ class TvServerSetupPersistenceTest { ), tokenManager = tokenManager, ) - val viewModel = track(TvServerSetupViewModel(repository)) + val viewModel = track(TvServerSetupViewModel(repository, FakeCleartextConsentStore())) viewModel.onServerUrlChanged("bad.silo") viewModel.onConnectClick() @@ -83,10 +93,155 @@ class TvServerSetupPersistenceTest { "Failed probes must not persist candidate URLs.", ) } + + @Test + fun successfulHttpFallbackStopsForConfirmationBeforePersistence() = runTest(dispatcher) { + val tokenManager = RecordingTokenManager() + val viewModel = track(viewModelFor(tokenManager)) + + viewModel.onServerUrlChanged("silo.lan") + viewModel.onConnectClick() + awaitSettled(viewModel) + + assertEquals("http://silo.lan", viewModel.uiState.value.pendingCleartextUrl) + assertEquals(emptyList(), tokenManager.serverUrlWrites) + assertNull(viewModel.uiState.value.navigateTo) + + viewModel.confirmCleartextConnection() + awaitSettled(viewModel) + + assertEquals(listOf("http://silo.lan"), tokenManager.serverUrlWrites) + assertEquals(TvServerSetupDestination.Login(signupEnabled = true), viewModel.uiState.value.navigateTo) + } + + @Test + fun cancelCleartextConnectionClearsPendingWithoutPersistence() = runTest(dispatcher) { + val tokenManager = RecordingTokenManager() + val viewModel = track(viewModelFor(tokenManager)) + + viewModel.onServerUrlChanged("silo.lan") + viewModel.onConnectClick() + awaitSettled(viewModel) + viewModel.cancelCleartextConnection() + + assertNull(viewModel.uiState.value.pendingCleartextUrl) + assertEquals(emptyList(), tokenManager.serverUrlWrites) + assertNull(viewModel.uiState.value.navigateTo) + } + + @Test + fun cleartextApprovalIsOriginSpecific() = runTest(dispatcher) { + val tokenManager = RecordingTokenManager() + val approvals = FakeCleartextConsentStore().apply { approve("http://other.lan") } + val viewModel = track(viewModelFor(tokenManager, approvals)) + + viewModel.onServerUrlChanged("silo.lan") + viewModel.onConnectClick() + awaitSettled(viewModel) + + assertEquals("http://silo.lan", viewModel.uiState.value.pendingCleartextUrl) + assertEquals(emptyList(), tokenManager.serverUrlWrites) + } + + @Test + fun priorOriginApprovalAndHttpsBypassConfirmation() = runTest(dispatcher) { + val approvedTokens = RecordingTokenManager() + val approvals = FakeCleartextConsentStore().apply { approve("http://silo.lan") } + val approvedViewModel = track(viewModelFor(approvedTokens, approvals)) + + approvedViewModel.onServerUrlChanged("silo.lan") + approvedViewModel.onConnectClick() + awaitSettled(approvedViewModel) + + assertNull(approvedViewModel.uiState.value.pendingCleartextUrl) + assertEquals(listOf("http://silo.lan"), approvedTokens.serverUrlWrites) + + val httpsTokens = RecordingTokenManager() + val httpsViewModel = track(viewModelFor(httpsTokens, httpsSucceeds = true)) + httpsViewModel.onServerUrlChanged("secure.silo") + httpsViewModel.onConnectClick() + awaitSettled(httpsViewModel) + + assertNull(httpsViewModel.uiState.value.pendingCleartextUrl) + assertEquals(listOf("https://secure.silo"), httpsTokens.serverUrlWrites) + } + + @Test + fun cancelWinsRaceWithInFlightCleartextApproval() = runTest(dispatcher) { + val tokenManager = RecordingTokenManager() + val approvals = BlockingCleartextConsentStore() + val viewModel = track(viewModelFor(tokenManager, approvals)) + viewModel.onServerUrlChanged("silo.lan") + viewModel.onConnectClick() + awaitSettled(viewModel) + + viewModel.confirmCleartextConnection() + runCurrent() + approvals.approveStarted.await() + viewModel.cancelCleartextConnection() + approvals.releaseApproval.complete(Unit) + advanceUntilIdle() + + assertNull(viewModel.uiState.value.pendingCleartextUrl) + assertNull(viewModel.uiState.value.navigateTo) + assertEquals(emptyList(), tokenManager.serverUrlWrites) + } + + @Test + fun successfulUnnormalizableHttpCandidateFailsClosed() = runTest(dispatcher) { + val tokenManager = RecordingTokenManager() + val viewModel = track(viewModelFor(tokenManager)) + + viewModel.onServerUrlChanged("http://silo_lan") + viewModel.onConnectClick() + awaitSettled(viewModel) + + assertEquals(emptyList(), tokenManager.serverUrlWrites) + assertNull(viewModel.uiState.value.navigateTo) + assertEquals("Could not safely identify this HTTP server.", viewModel.uiState.value.error) + } + + private fun viewModelFor( + tokenManager: RecordingTokenManager, + approvals: FakeCleartextConsentStore = FakeCleartextConsentStore(), + httpsSucceeds: Boolean = false, + ) = TvServerSetupViewModel( + authRepository = repositoryFor(tokenManager), + cleartextConsentStore = approvals, + getSetupStatus = { candidate -> + if (candidate.startsWith("https://") && !httpsSucceeds) { + ApiResult.NetworkError(IOException("TLS unavailable")) + } else { + ApiResult.Success(SetupStatusResponse(needsSetup = false)) + } + }, + getSignupStatus = { + ApiResult.Success(SignupStatusResponse(enabled = true)) + }, + ) + + private fun repositoryFor( + tokenManager: RecordingTokenManager, + ): AuthRepository = AuthRepository( + authApi = AuthApi( + HttpClient(MockEngine { throw IOException("Unexpected network request") }) { + install(ContentNegotiation) { json(SiloJson) } + install(SiloAuthPlugin) { this.tokenManager = tokenManager } + }, + ), + tokenManager = tokenManager, + ) + + private suspend fun TestScope.awaitSettled(viewModel: TvServerSetupViewModel) { + runCurrent() + withTimeout(5_000) { + viewModel.uiState.first { !it.isLoading } + } + } } private class RecordingTokenManager( - private var serverUrl: String, + private var serverUrl: String = "", ) : TokenManager { val serverUrlWrites = mutableListOf() override val sessionExpired = MutableSharedFlow() @@ -108,3 +263,24 @@ private class RecordingTokenManager( override suspend fun switchActiveServer(serverId: String?) = Unit override suspend fun signOutCurrentServer() = Unit } + +private open class FakeCleartextConsentStore : CleartextConsentStore { + private val approved = mutableSetOf() + + override suspend fun isApproved(origin: String): Boolean = origin in approved + + override suspend fun approve(origin: String) { + approved += origin + } +} + +private class BlockingCleartextConsentStore : FakeCleartextConsentStore() { + val approveStarted = CompletableDeferred() + val releaseApproval = CompletableDeferred() + + override suspend fun approve(origin: String) { + approveStarted.complete(Unit) + releaseApproval.await() + super.approve(origin) + } +} diff --git a/shared/src/androidMain/kotlin/org/siloserver/silo/network/EncryptedTokenManagerImpl.kt b/shared/src/androidMain/kotlin/org/siloserver/silo/network/EncryptedTokenManagerImpl.kt index 9a0c51268..c74f65e5f 100644 --- a/shared/src/androidMain/kotlin/org/siloserver/silo/network/EncryptedTokenManagerImpl.kt +++ b/shared/src/androidMain/kotlin/org/siloserver/silo/network/EncryptedTokenManagerImpl.kt @@ -51,7 +51,8 @@ class EncryptedTokenManagerImpl( private var temporaryScope: TemporaryAuthScope? = null /** - * Incremented whenever this manager writes or clears PERSISTENT credentials. + * Incremented whenever the persistent identity moves: writing or clearing + * persistent credentials, and switching the active server. * Stamped onto snapshots so a scope captured before a sign-out cannot read or * overwrite the credentials of the login that replaced it. Overlay begin/end * deliberately does not move it — see [AuthScopeSnapshot.credentialEpoch]. @@ -79,6 +80,7 @@ class EncryptedTokenManagerImpl( mutex.withLock { if (id != activeServerId) { activeServerId = id + persistentCredentialEpoch += 1 reloadCacheUnsynchronized() } } @@ -110,6 +112,7 @@ class EncryptedTokenManagerImpl( val liveId = registry.activeServerId.value if (liveId != activeServerId) { activeServerId = liveId + persistentCredentialEpoch += 1 reloadCacheUnsynchronized() } } @@ -275,6 +278,7 @@ class EncryptedTokenManagerImpl( mutex.withLock { if (activeServerId == serverId) return@withLock activeServerId = serverId + persistentCredentialEpoch += 1 reloadCacheUnsynchronized() } } diff --git a/shared/src/androidUnitTest/kotlin/org/siloserver/silo/network/EncryptedTokenManagerScopeGenerationTest.kt b/shared/src/androidUnitTest/kotlin/org/siloserver/silo/network/EncryptedTokenManagerScopeGenerationTest.kt index a3e06ce64..0c6dc6f55 100644 --- a/shared/src/androidUnitTest/kotlin/org/siloserver/silo/network/EncryptedTokenManagerScopeGenerationTest.kt +++ b/shared/src/androidUnitTest/kotlin/org/siloserver/silo/network/EncryptedTokenManagerScopeGenerationTest.kt @@ -34,12 +34,35 @@ class EncryptedTokenManagerScopeGenerationTest { assertEquals("new-refresh", manager.getRefreshToken()) } + @Test + fun registryFirstSwitchInvalidatesSnapshotBeforeManagerSwitchCall() = runTest { + val registry = FakeServerRegistry() + val manager = EncryptedTokenManagerImpl( + prefs = inMemoryPreferences(), + registry = registry, + ) + manager.saveTokens("server-a-access", "server-a-refresh", 3600) + val staleScope = checkNotNull(manager.snapshotCurrentScope()) + + registry.switchExternally("server-b") + manager.getAccessToken() + manager.switchActiveServer("server-b") + + assertNull(manager.getAccessTokenForScope(staleScope)) + manager.saveTokensForScope(staleScope, "stale-access", "stale-refresh", 3600) + assertNull(manager.getAccessTokenForScope(staleScope)) + } + private class FakeServerRegistry : ServerRegistry { - private val entry = ServerEntry(id = "server-a", url = "https://server-a.example") - override val entries: StateFlow> = MutableStateFlow(listOf(entry)) - override val activeServerId: StateFlow = MutableStateFlow(entry.id) - override val activeEntry: StateFlow = MutableStateFlow(entry) - override suspend fun addOrUpdate(url: String, fetchedName: String?): String = entry.id + private val serverA = ServerEntry(id = "server-a", url = "https://server-a.example") + private val serverB = ServerEntry(id = "server-b", url = "https://server-b.example") + private val entriesFlow = MutableStateFlow(listOf(serverA, serverB)) + private val activeServerIdFlow = MutableStateFlow(serverA.id) + private val activeEntryFlow = MutableStateFlow(serverA) + override val entries: StateFlow> = entriesFlow + override val activeServerId: StateFlow = activeServerIdFlow + override val activeEntry: StateFlow = activeEntryFlow + override suspend fun addOrUpdate(url: String, fetchedName: String?): String = serverA.id override suspend fun rename(serverId: String, userOverrideName: String?) = Unit override suspend fun setFetchedName(serverId: String, fetchedName: String?) = Unit override suspend fun setProfileId(serverId: String, profileId: String?) = Unit @@ -47,6 +70,11 @@ class EncryptedTokenManagerScopeGenerationTest { override suspend fun signOut(serverId: String) = Unit override suspend fun switchTo(serverId: String) = Unit override suspend fun touchActive() = Unit + + fun switchExternally(serverId: String) { + activeServerIdFlow.value = serverId + activeEntryFlow.value = entriesFlow.value.first { it.id == serverId } + } } private fun inMemoryPreferences(): SharedPreferences { From 07eb7954e223c62d1d13556a21cdca7365558a39 Mon Sep 17 00:00:00 2001 From: rxwatcher Date: Thu, 23 Jul 2026 11:07:55 +0200 Subject: [PATCH 04/17] fix(reader): sanitize epub with parsed allowlist (cherry picked from commit 85890c6d970a858385072a8c6f1fe260690d2b8e) --- androidApp/build.gradle.kts | 1 + .../reader/reflow/EpubHtmlSanitizer.kt | 421 ++++++++++++++---- .../reader/reflow/EpubHtmlSanitizerTest.kt | 143 +++++- gradle/libs.versions.toml | 3 + 4 files changed, 474 insertions(+), 94 deletions(-) diff --git a/androidApp/build.gradle.kts b/androidApp/build.gradle.kts index f659fcbfb..0f28973ab 100644 --- a/androidApp/build.gradle.kts +++ b/androidApp/build.gradle.kts @@ -77,6 +77,7 @@ kotlin { implementation(libs.koin.compose.viewmodel) implementation(libs.coil.compose) implementation(libs.coil.network.ktor) + implementation(libs.jsoup) implementation(libs.media3.exoplayer) implementation(libs.media3.exoplayer.hls) implementation(libs.media3.datasource.okhttp) diff --git a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubHtmlSanitizer.kt b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubHtmlSanitizer.kt index 24e46320b..05ff1d063 100644 --- a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubHtmlSanitizer.kt +++ b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubHtmlSanitizer.kt @@ -1,112 +1,347 @@ package org.siloserver.silo.android.ui.screens.reader.reflow -internal fun sanitizeEpubChapterHtml(html: String): String = - html - .replace(BLOCKED_ELEMENT_WITH_BODY_REGEX, "") - .replace(BLOCKED_ELEMENT_REGEX, "") - .replace(EVENT_HANDLER_ATTR_REGEX, "") - .replace(STYLE_ATTR_REGEX, "") - .replace(SRCDOC_ATTR_REGEX, "") - .replace(SRCSET_ATTR_REGEX, "") - .replace(RESOURCE_ATTR_REGEX) { match -> - val name = match.groupValues[1] - val quote = match.groupValues[2] - val value = match.groupValues[3] - if (isUnsafeEpubResourceUrl(value)) "" else " $name=$quote$value$quote" - } - .replace(UNQUOTED_RESOURCE_ATTR_REGEX) { match -> - val name = match.groupValues[1] - val value = match.groupValues[2] - if (isUnsafeEpubResourceUrl(value)) "" else " $name=$value" - } +import org.jsoup.Jsoup +import org.jsoup.nodes.Element +import org.jsoup.safety.Cleaner +import org.jsoup.safety.Safelist -private fun isUnsafeEpubResourceUrl(value: String): Boolean { - val normalized = value - .decodeHtmlCharacterReferences() - .trim() - .filterNot { it.isIgnoredUrlPolicyCharacter() } - .lowercase() - return normalized.startsWith("javascript:") || - normalized.startsWith("vbscript:") || - normalized.startsWith("data:") || - ABSOLUTE_URL_SCHEME_REGEX.containsMatchIn(normalized) || - normalized.startsWith("//") +internal fun sanitizeEpubChapterHtml(html: String): String { + val parsed = Jsoup.parseBodyFragment(html) + parsed.outputSettings().prettyPrint(false) + + val cleaned = EPUB_HTML_CLEANER.clean(parsed) + cleaned.body().getAllElements().forEach(Element::removeUnsafeResourceAttributes) + return cleaned.body().html() } -private fun String.decodeHtmlCharacterReferences(): String = - HTML_CHARACTER_REFERENCE_REGEX.replace(this) { match -> - decodeHtmlCharacterReference(match.groupValues[1]) ?: match.value +private fun Element.removeUnsafeResourceAttributes() { + RESOURCE_ATTRIBUTES.forEach { attribute -> + if (hasAttr(attribute) && !isSafeRelativeEpubResourceUrl(attr(attribute))) { + removeAttr(attribute) + } } +} -private fun decodeHtmlCharacterReference(reference: String): String? { - val value = reference.removeSuffix(";") - val codePoint = when { - value.startsWith("#x", ignoreCase = true) -> - value.drop(2).toIntOrNull(radix = 16) - value.startsWith("#") -> - value.drop(1).toIntOrNull(radix = 10) - else -> - return namedHtmlCharacterReference(value) - } ?: return null +private fun isSafeRelativeEpubResourceUrl(value: String): Boolean { + val decoded = value.decodePercentEncodedAsciiRecursively() ?: return false + val normalized = decoded + .trim() + .filterNot(Char::isIgnoredUrlPolicyCharacter) - return when (codePoint) { - in 0..Char.MAX_VALUE.code -> codePoint.toChar().toString() - in 0..0x10FFFF -> String(Character.toChars(codePoint)) - else -> null + return normalized.isEmpty() || + ( + !normalized.startsWith("/") && + !normalized.startsWith("\\") && + !normalized.contains('\\') && + !ABSOLUTE_URL_SCHEME_REGEX.containsMatchIn(normalized) + ) +} + +private fun String.decodePercentEncodedAsciiRecursively(): String? { + var decoded = this + repeat(length.coerceAtMost(MAX_PERCENT_DECODING_PASSES)) { + val next = decoded.decodePercentEncodedAscii() + if (next == decoded) return decoded + decoded = next } + return if (decoded.decodePercentEncodedAscii() == decoded) decoded else null } -private fun namedHtmlCharacterReference(name: String): String? = - when (name.lowercase()) { - "amp" -> "&" - "apos" -> "'" - "colon" -> ":" - "gt" -> ">" - "lt" -> "<" - "newline" -> "\n" - "quot" -> "\"" - "sol" -> "/" - "tab" -> "\t" - else -> null +private fun String.decodePercentEncodedAscii(): String { + val decoded = StringBuilder(length) + var index = 0 + while (index < length) { + if (this[index] == '%' && index + 2 < length) { + val high = this[index + 1].digitToIntOrNull(16) + val low = this[index + 2].digitToIntOrNull(16) + val byte = if (high != null && low != null) (high shl 4) or low else null + if (byte != null && byte <= Char.MAX_VALUE.code && byte < 0x80) { + decoded.append(byte.toChar()) + index += 3 + continue + } + } + decoded.append(this[index]) + index += 1 } + return decoded.toString() +} private fun Char.isIgnoredUrlPolicyCharacter(): Boolean = code <= 0x20 || code == 0x7F || isWhitespace() -private val HTML_CHARACTER_REFERENCE_REGEX = Regex( - """&(#x[0-9a-fA-F]+;?|#[0-9]+;?|[a-zA-Z][a-zA-Z0-9]+;)""", +private val ABSOLUTE_URL_SCHEME_REGEX = Regex( + pattern = """^[a-z][a-z0-9+.-]*:""", + option = RegexOption.IGNORE_CASE, ) -private val ABSOLUTE_URL_SCHEME_REGEX = Regex("""^[a-z][a-z0-9+.-]*:""") -private val BLOCKED_ELEMENT_WITH_BODY_REGEX = Regex( - """<\s*(script|iframe|object|embed|form|textarea|select|button)\b[^>]*>.*?<\s*/\s*\1\s*>""", - setOf(RegexOption.IGNORE_CASE, RegexOption.DOT_MATCHES_ALL), -) -private val BLOCKED_ELEMENT_REGEX = Regex( - """<\s*/?\s*(script|iframe|object|embed|form|input|textarea|select|option|button|meta|link|base)\b[^>]*>""", - RegexOption.IGNORE_CASE, -) -private val EVENT_HANDLER_ATTR_REGEX = Regex( - """\s+on[a-zA-Z0-9_-]+\s*=\s*("[^"]*"|'[^']*'|[^\s>]+)""", - RegexOption.IGNORE_CASE, -) -private val STYLE_ATTR_REGEX = Regex( - """\s+style\s*=\s*("[^"]*"|'[^']*'|[^\s>]+)""", - RegexOption.IGNORE_CASE, -) -private val SRCDOC_ATTR_REGEX = Regex( - """\s+srcdoc\s*=\s*("[^"]*"|'[^']*'|[^\s>]+)""", - RegexOption.IGNORE_CASE, -) -private val SRCSET_ATTR_REGEX = Regex( - """\s+srcset\s*=\s*("[^"]*"|'[^']*'|[^\s>]+)""", - RegexOption.IGNORE_CASE, -) -private val RESOURCE_ATTR_REGEX = Regex( - """\s+(href|src|xlink:href)\s*=\s*(["'])(.*?)\2""", - setOf(RegexOption.IGNORE_CASE, RegexOption.DOT_MATCHES_ALL), -) -private val UNQUOTED_RESOURCE_ATTR_REGEX = Regex( - """\s+(href|src|xlink:href)\s*=\s*([^\s>]+)""", - RegexOption.IGNORE_CASE, +private const val MAX_PERCENT_DECODING_PASSES = 64 + +private val RESOURCE_ATTRIBUTES = arrayOf("href", "src", "xlink:href") + +private val EPUB_HTML_CLEANER = Cleaner( + Safelist.none() + .addTags( + "a", + "abbr", + "address", + "article", + "aside", + "b", + "bdi", + "bdo", + "blockquote", + "br", + "caption", + "cite", + "code", + "col", + "colgroup", + "data", + "dd", + "del", + "details", + "dfn", + "div", + "dl", + "dt", + "em", + "figcaption", + "figure", + "footer", + "h1", + "h2", + "h3", + "h4", + "h5", + "h6", + "header", + "hr", + "i", + "img", + "ins", + "kbd", + "li", + "main", + "mark", + "nav", + "ol", + "p", + "pre", + "q", + "rp", + "rt", + "ruby", + "s", + "samp", + "section", + "small", + "span", + "strong", + "sub", + "summary", + "sup", + "table", + "tbody", + "td", + "tfoot", + "th", + "thead", + "time", + "tr", + "u", + "ul", + "var", + "wbr", + // SVG presentation elements. Active animation and foreign-content + // elements are intentionally absent. + "svg", + "g", + "defs", + "symbol", + "use", + "path", + "circle", + "ellipse", + "line", + "polyline", + "polygon", + "rect", + "image", + "text", + "tspan", + "textpath", + "clippath", + "mask", + "pattern", + "lineargradient", + "radialgradient", + "stop", + "marker", + "switch", + "desc", + "title", + // MathML presentation elements. Annotation XML is intentionally + // absent because it can switch back into active HTML/SVG content. + "math", + "mrow", + "mi", + "mn", + "mo", + "ms", + "mtext", + "mspace", + "mfrac", + "msqrt", + "mroot", + "mstyle", + "merror", + "mpadded", + "mphantom", + "mfenced", + "menclose", + "msub", + "msup", + "msubsup", + "munder", + "mover", + "munderover", + "mmultiscripts", + "mprescripts", + "none", + "mtable", + "mtr", + "mtd", + "semantics", + "annotation", + ) + .addAttributes( + ":all", + "id", + "class", + "title", + "lang", + "xml:lang", + "dir", + "role", + "aria-label", + "aria-labelledby", + "aria-describedby", + "aria-hidden", + "epub:type", + "href", + "src", + "xlink:href", + ) + .addAttributes("a", "name", "rel") + .addAttributes("blockquote", "cite") + .addAttributes("col", "span", "width") + .addAttributes("colgroup", "span", "width") + .addAttributes("data", "value") + .addAttributes("del", "cite", "datetime") + .addAttributes("details", "open") + .addAttributes("img", "alt", "width", "height", "loading", "decoding") + .addAttributes("ins", "cite", "datetime") + .addAttributes("li", "value") + .addAttributes("ol", "start", "reversed", "type") + .addAttributes("q", "cite") + .addAttributes("td", "abbr", "colspan", "headers", "rowspan") + .addAttributes("th", "abbr", "colspan", "headers", "rowspan", "scope") + .addAttributes("time", "datetime") + .addAttributes( + "svg", + "viewbox", + "preserveaspectratio", + "version", + "xmlns", + "xmlns:xlink", + "width", + "height", + ) + .addAttributes( + "g", + "transform", + "fill", + "fill-opacity", + "fill-rule", + "stroke", + "stroke-width", + "stroke-linecap", + "stroke-linejoin", + "stroke-miterlimit", + "stroke-dasharray", + "stroke-dashoffset", + "stroke-opacity", + "opacity", + ) + .addAttributes("defs", "transform") + .addAttributes("symbol", "viewbox", "preserveaspectratio") + .addAttributes("use", "x", "y", "width", "height", "transform") + .addAttributes( + "path", + "d", + "pathlength", + "transform", + "fill", + "fill-opacity", + "fill-rule", + "stroke", + "stroke-width", + "stroke-linecap", + "stroke-linejoin", + "stroke-miterlimit", + "stroke-dasharray", + "stroke-dashoffset", + "stroke-opacity", + "opacity", + ) + .addAttributes("circle", "cx", "cy", "r", "transform", "fill", "stroke", "opacity") + .addAttributes("ellipse", "cx", "cy", "rx", "ry", "transform", "fill", "stroke", "opacity") + .addAttributes("line", "x1", "y1", "x2", "y2", "transform", "stroke", "opacity") + .addAttributes("polyline", "points", "transform", "fill", "stroke", "opacity") + .addAttributes("polygon", "points", "transform", "fill", "stroke", "opacity") + .addAttributes("rect", "x", "y", "width", "height", "rx", "ry", "transform", "fill", "stroke", "opacity") + .addAttributes("image", "x", "y", "width", "height", "preserveaspectratio", "transform") + .addAttributes("text", "x", "y", "dx", "dy", "text-anchor", "transform", "fill", "stroke") + .addAttributes("tspan", "x", "y", "dx", "dy") + .addAttributes("textpath", "startoffset", "method", "spacing") + .addAttributes("clippath", "clippathunits", "transform") + .addAttributes("mask", "x", "y", "width", "height", "maskunits", "maskcontentunits") + .addAttributes("pattern", "x", "y", "width", "height", "patternunits", "patterncontentunits", "patterntransform") + .addAttributes("lineargradient", "x1", "y1", "x2", "y2", "gradientunits", "gradienttransform", "spreadmethod") + .addAttributes("radialgradient", "cx", "cy", "r", "fx", "fy", "gradientunits", "gradienttransform", "spreadmethod") + .addAttributes("stop", "offset", "stop-color", "stop-opacity") + .addAttributes("marker", "refx", "refy", "markerwidth", "markerheight", "orient", "markerunits", "viewbox") + .addAttributes("math", "display", "mathvariant", "mathsize", "mathcolor", "mathbackground") + .addAttributes( + "mstyle", + "displaystyle", + "scriptlevel", + "scriptsizemultiplier", + "scriptminsize", + "mathvariant", + "mathsize", + "mathcolor", + "mathbackground", + ) + .addAttributes( + "mo", + "form", + "fence", + "separator", + "stretchy", + "symmetric", + "maxsize", + "minsize", + "largeop", + "movablelimits", + "accent", + "lspace", + "rspace", + ) + .addAttributes("mfrac", "linethickness", "bevelled") + .addAttributes("menclose", "notation") + .addAttributes("mtable", "rowalign", "columnalign", "rowspacing", "columnspacing") + .addAttributes("mtr", "rowalign", "columnalign") + .addAttributes("mtd", "rowspan", "columnspan", "rowalign", "columnalign"), ) diff --git a/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubHtmlSanitizerTest.kt b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubHtmlSanitizerTest.kt index 0d06b97d3..f1c5660d4 100644 --- a/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubHtmlSanitizerTest.kt +++ b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubHtmlSanitizerTest.kt @@ -1,8 +1,10 @@ package org.siloserver.silo.android.ui.screens.reader.reflow import kotlin.test.Test +import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertTrue +import org.jsoup.Jsoup class EpubHtmlSanitizerTest { @Test @@ -47,7 +49,8 @@ class EpubHtmlSanitizerTest { assertFalse(sanitized.contains("java script", ignoreCase = true)) assertFalse(sanitized.contains("https", ignoreCase = true)) assertFalse(sanitized.contains("//example.invalid", ignoreCase = true)) - assertTrue(sanitized.contains("chapter-1.xhtml")) + val sanitizedDocument = Jsoup.parseBodyFragment(sanitized) + assertEquals("chapter-1.xhtml", sanitizedDocument.selectFirst("a[href]")?.attr("href")) } @Test @@ -70,4 +73,142 @@ class EpubHtmlSanitizerTest { assertFalse(sanitized.contains("ftp://example.invalid", ignoreCase = true)) assertTrue(sanitized.contains("images/local-cover.jpg")) } + + @Test + fun sanitizerCleansMalformedSvgAndMathMlMutationPayloads() { + val html = """ + + + + + + + + """.trimIndent() + + val sanitized = sanitizeEpubChapterHtml(html) + val sanitizedDocument = Jsoup.parseBodyFragment(sanitized) + + assertTrue(sanitizedDocument.select("style, script").isEmpty()) + assertTrue(sanitizedDocument.select("[onerror]").isEmpty()) + assertFalse(sanitized.contains("svg-owned", ignoreCase = true)) + } + + @Test + fun sanitizerRemovesActiveDocumentsAndExternalStyleResources() { + val html = """ + + + + +
+ + + + + +

Readable text

+ """.trimIndent() + + val sanitized = sanitizeEpubChapterHtml(html) + + listOf( + "base", + "link", + "style", + "script", + "form", + "input", + "button", + "iframe", + "frame", + "frameset", + "object", + "embed", + ).forEach { tag -> + assertFalse(sanitized.contains("<$tag", ignoreCase = true), "kept <$tag>") + } + assertFalse(sanitized.contains("srcdoc", ignoreCase = true)) + assertFalse(sanitized.contains("onanimationstart", ignoreCase = true)) + assertFalse(sanitized.contains("style=", ignoreCase = true)) + assertFalse(sanitized.contains("tracker.css", ignoreCase = true)) + assertFalse(sanitized.contains("chapter.js", ignoreCase = true)) + assertTrue(sanitized.contains("Readable text")) + } + + @Test + fun sanitizerRejectsEncodedAndNonRelativeResourceUrls() { + val html = """ + percent encoded script + double encoded script + + + + + entity encoded script + email + """.trimIndent() + + val sanitized = sanitizeEpubChapterHtml(html) + val sanitizedDocument = Jsoup.parseBodyFragment(sanitized) + + assertTrue(sanitizedDocument.select("[href], [src], [xlink:href]").isEmpty()) + } + + @Test + fun sanitizerPreservesRichEpubMarkupAndRelativeResources() { + val html = """ +
+

Chapter 一(one)

+

Diagram

+ + Safe diagram + + + + + + x=12 + +
+ + + +
Values
Name
Example
+ Next chapter + Chapter start + Cover + + """.trimIndent() + + val sanitized = sanitizeEpubChapterHtml(html) + + listOf( + "", + "", + " + assertTrue(sanitized.contains(element, ignoreCase = true), "removed $element") + } + assertTrue(sanitized.contains("../images/shapes.svg#shape")) + assertTrue(sanitized.contains("../text/chapter-2.xhtml#start")) + assertTrue(sanitized.contains("href=\"#chapter-one\"")) + assertTrue(sanitized.contains("../images/cover.jpg")) + } } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 57c22da63..af80b19f1 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -35,6 +35,7 @@ firebase-messaging = "25.1.0" google-services = "4.5.0" junit4 = "4.13.2" okhttp = "4.12.0" +jsoup = "1.22.2" # Google Cast (Chromecast, phone app only) + its MediaRouter dependency. play-services-cast-framework = "21.5.0" androidx-mediarouter = "1.7.0" @@ -125,6 +126,8 @@ androidx-test-ext-junit = { module = "androidx.test.ext:junit", version.ref = "a androidx-uiautomator = { module = "androidx.test.uiautomator:uiautomator", version.ref = "uiautomator" } firebase-messaging = { module = "com.google.firebase:firebase-messaging", version.ref = "firebase-messaging" } junit4 = { module = "junit:junit", version.ref = "junit4" } +okhttp-mockwebserver = { module = "com.squareup.okhttp3:mockwebserver", version.ref = "okhttp" } +jsoup = { module = "org.jsoup:jsoup", version.ref = "jsoup" } # Google Cast (Chromecast) — phone app only. Tier 2 casting starts a separate # cast-capability playback session; the raw phone stream is never cast. From cb1d8a85f905722f582a838701c686621e6e0ae0 Mon Sep 17 00:00:00 2001 From: rxwatcher Date: Thu, 23 Jul 2026 11:16:54 +0200 Subject: [PATCH 05/17] fix(reader): validate svg paint references (cherry picked from commit 4af0f4db759519032d00559e8334dc3a25300a5e) (cherry picked from commit 5db84af8842ad28705c74e4b886d44ae18f461fe) --- .../reader/reflow/EpubHtmlSanitizer.kt | 39 +++++++++++- .../reader/reflow/EpubHtmlSanitizerTest.kt | 62 +++++++++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubHtmlSanitizer.kt b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubHtmlSanitizer.kt index 05ff1d063..2f25d958a 100644 --- a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubHtmlSanitizer.kt +++ b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubHtmlSanitizer.kt @@ -10,7 +10,10 @@ internal fun sanitizeEpubChapterHtml(html: String): String { parsed.outputSettings().prettyPrint(false) val cleaned = EPUB_HTML_CLEANER.clean(parsed) - cleaned.body().getAllElements().forEach(Element::removeUnsafeResourceAttributes) + cleaned.body().getAllElements().forEach { element -> + element.removeUnsafeResourceAttributes() + element.removeUnsafeSvgPaintAttributes() + } return cleaned.body().html() } @@ -22,6 +25,14 @@ private fun Element.removeUnsafeResourceAttributes() { } } +private fun Element.removeUnsafeSvgPaintAttributes() { + SVG_PAINT_ATTRIBUTES.forEach { attribute -> + if (hasAttr(attribute) && !isSafeSvgPaintValue(attr(attribute))) { + removeAttr(attribute) + } + } +} + private fun isSafeRelativeEpubResourceUrl(value: String): Boolean { val decoded = value.decodePercentEncodedAsciiRecursively() ?: return false val normalized = decoded @@ -37,6 +48,17 @@ private fun isSafeRelativeEpubResourceUrl(value: String): Boolean { ) } +private fun isSafeSvgPaintValue(value: String): Boolean { + val decoded = value.decodePercentEncodedAsciiRecursively() ?: return false + if (decoded.any(Char::isUnsafeSvgPaintCharacter)) return false + + val normalized = decoded.trim() + return SAFE_SVG_PAINT_KEYWORD_REGEX.matches(normalized) || + SAFE_SVG_HEX_COLOR_REGEX.matches(normalized) || + SAFE_SVG_COLOR_FUNCTION_REGEX.matches(normalized) || + SAFE_LOCAL_SVG_PAINT_URL_REGEX.matches(normalized) +} + private fun String.decodePercentEncodedAsciiRecursively(): String? { var decoded = this repeat(length.coerceAtMost(MAX_PERCENT_DECODING_PASSES)) { @@ -70,14 +92,29 @@ private fun String.decodePercentEncodedAscii(): String { private fun Char.isIgnoredUrlPolicyCharacter(): Boolean = code <= 0x20 || code == 0x7F || isWhitespace() +private fun Char.isUnsafeSvgPaintCharacter(): Boolean = + this == '\\' || code < 0x20 || code == 0x7F || (isWhitespace() && this != ' ') + private val ABSOLUTE_URL_SCHEME_REGEX = Regex( pattern = """^[a-z][a-z0-9+.-]*:""", option = RegexOption.IGNORE_CASE, ) +private val SAFE_SVG_PAINT_KEYWORD_REGEX = Regex("""^[a-z]+(?:-[a-z]+)*$""", RegexOption.IGNORE_CASE) +private val SAFE_SVG_HEX_COLOR_REGEX = Regex("""^#(?:[0-9a-f]{3,4}|[0-9a-f]{6}|[0-9a-f]{8})$""", RegexOption.IGNORE_CASE) +private val SAFE_SVG_COLOR_FUNCTION_REGEX = Regex( + pattern = """^(?:rgb|rgba|hsl|hsla)\([0-9+\-.,%/ ]*(?:(?:deg|grad|rad|turn)[0-9+\-.,%/ ]*)?\)$""", + option = RegexOption.IGNORE_CASE, +) +private val SAFE_LOCAL_SVG_PAINT_URL_REGEX = Regex( + pattern = """^url\(\s*(["']?)#[a-z0-9_.:-]+\1\s*\)$""", + option = RegexOption.IGNORE_CASE, +) + private const val MAX_PERCENT_DECODING_PASSES = 64 private val RESOURCE_ATTRIBUTES = arrayOf("href", "src", "xlink:href") +private val SVG_PAINT_ATTRIBUTES = arrayOf("fill", "stroke") private val EPUB_HTML_CLEANER = Cleaner( Safelist.none() diff --git a/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubHtmlSanitizerTest.kt b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubHtmlSanitizerTest.kt index f1c5660d4..e4b34def1 100644 --- a/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubHtmlSanitizerTest.kt +++ b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubHtmlSanitizerTest.kt @@ -211,4 +211,66 @@ class EpubHtmlSanitizerTest { assertTrue(sanitized.contains("href=\"#chapter-one\"")) assertTrue(sanitized.contains("../images/cover.jpg")) } + + @Test + fun sanitizerRejectsNonLocalSvgPaintUrls() { + val html = """ + + + + + + text + + """.trimIndent() + + val sanitizedDocument = Jsoup.parseBodyFragment(sanitizeEpubChapterHtml(html)) + + assertTrue(sanitizedDocument.select("[fill], [stroke]").isEmpty()) + } + + @Test + fun sanitizerPreservesSafeSvgPaintValues() { + val html = """ + + + + + + + + + + text + + """.trimIndent() + + val sanitizedDocument = Jsoup.parseBodyFragment(sanitizeEpubChapterHtml(html)) + + assertEquals("url(#gradient)", sanitizedDocument.getElementById("local-paint")?.attr("fill")) + assertEquals("url('#outline')", sanitizedDocument.getElementById("local-paint")?.attr("stroke")) + assertEquals("#336699", sanitizedDocument.getElementById("colors")?.attr("fill")) + assertEquals("currentColor", sanitizedDocument.getElementById("colors")?.attr("stroke")) + assertEquals("red", sanitizedDocument.getElementById("keywords")?.attr("fill")) + assertEquals("none", sanitizedDocument.getElementById("keywords")?.attr("stroke")) + assertEquals( + "rgb(10 20 30 / 50%)", + sanitizedDocument.getElementById("functional-colors")?.attr("fill"), + ) + assertEquals( + "hsl(120 100% 50%)", + sanitizedDocument.getElementById("functional-colors")?.attr("stroke"), + ) + } } From 003a7c705d13da965d9d1f2501cdc319f914597f Mon Sep 17 00:00:00 2001 From: rxwatcher Date: Thu, 23 Jul 2026 11:27:35 +0200 Subject: [PATCH 06/17] fix(reader): bound remote and archive content (cherry picked from commit 517cc323d818caef86baf8cfac6921892c3fefd4) (cherry picked from commit 14b41b7f9f54aa7274cafdfbe1f6afe557f3c920) --- .../silo/common/io/LimitedStreams.kt | 49 +++++ .../player/AuthenticatedDataSourceFactory.kt | 30 ++- .../silo/common/io/LimitedStreamsTest.kt | 52 +++++ .../AuthenticatedDataSourceFactoryTest.kt | 69 +++++- .../android/ui/screens/reader/EpubBook.kt | 193 ++++++++++++++--- .../ui/screens/reader/ReaderFileCache.kt | 58 ++++- .../android/ui/screens/reader/EpubBookTest.kt | 204 +++++++++++++++++- .../ui/screens/reader/ReaderFileCacheTest.kt | 81 +++++++ 8 files changed, 689 insertions(+), 47 deletions(-) create mode 100644 android-shared/src/androidMain/kotlin/org/siloserver/silo/common/io/LimitedStreams.kt create mode 100644 android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/io/LimitedStreamsTest.kt diff --git a/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/io/LimitedStreams.kt b/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/io/LimitedStreams.kt new file mode 100644 index 000000000..7c086c484 --- /dev/null +++ b/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/io/LimitedStreams.kt @@ -0,0 +1,49 @@ +package org.siloserver.silo.common.io + +import java.io.IOException +import java.io.InputStream +import java.io.OutputStream + +class ContentLimitExceeded( + val maxBytes: Long, + limitName: String = "content", +) : IOException("$limitName exceeds the allowed limit of $maxBytes") + +fun checkedLimitedByteCount( + currentBytes: Long, + additionalBytes: Long, + maxBytes: Long, + limitName: String = "content", +): Long { + require(currentBytes >= 0) { "currentBytes must not be negative" } + require(additionalBytes >= 0) { "additionalBytes must not be negative" } + require(maxBytes >= 0) { "maxBytes must not be negative" } + if (currentBytes > maxBytes || additionalBytes > maxBytes - currentBytes) { + throw ContentLimitExceeded(maxBytes, limitName) + } + return currentBytes + additionalBytes +} + +fun InputStream.copyToLimited( + out: OutputStream, + maxBytes: Long, + bufferSize: Int = DEFAULT_BUFFER_SIZE, +): Long { + require(maxBytes >= 0) { "maxBytes must not be negative" } + require(bufferSize > 0) { "bufferSize must be positive" } + var total = 0L + val buffer = ByteArray(bufferSize) + while (true) { + val read = read(buffer) + if (read < 0) return total + if (read == 0) { + val byte = read() + if (byte < 0) return total + total = checkedLimitedByteCount(total, 1, maxBytes) + out.write(byte) + continue + } + total = checkedLimitedByteCount(total, read.toLong(), maxBytes) + out.write(buffer, 0, read) + } +} diff --git a/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/AuthenticatedDataSourceFactory.kt b/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/AuthenticatedDataSourceFactory.kt index 9589d19b3..3680b3aa5 100644 --- a/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/AuthenticatedDataSourceFactory.kt +++ b/android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/AuthenticatedDataSourceFactory.kt @@ -10,6 +10,7 @@ import androidx.media3.datasource.DataSpec import androidx.media3.datasource.FileDataSource import androidx.media3.datasource.HttpDataSource import androidx.media3.datasource.TransferListener +import org.siloserver.silo.common.io.checkedLimitedByteCount import org.siloserver.silo.common.player.subtitle.normalizeSubripPayloadIfNeeded import java.io.ByteArrayOutputStream import java.io.IOException @@ -355,6 +356,7 @@ internal fun resolveRoutedDataSourceUrl(serverUrl: String, rawUri: String): Stri @UnstableApi internal class SubripNormalizingDataSource( private val upstream: DataSource, + private val maxBytes: Long = MAX_SUBTITLE_BYTES, ) : DataSource { private var normalizedData: ByteArray? = null private var normalizedPosition: Int = 0 @@ -373,14 +375,28 @@ internal class SubripNormalizingDataSource( return upstream.open(dataSpec) } - upstream.open(dataSpec) + val declaredLength = upstream.open(dataSpec) uri = upstream.uri ?: dataSpec.uri val raw = try { + if (declaredLength >= 0) { + checkedLimitedByteCount( + currentBytes = 0, + additionalBytes = declaredLength, + maxBytes = maxBytes, + limitName = "subtitle", + ) + } readAllFromUpstream() } finally { upstream.close() } val normalized = normalizeSubripDataIfNeeded(raw) + checkedLimitedByteCount( + currentBytes = 0, + additionalBytes = normalized.size.toLong(), + maxBytes = maxBytes, + limitName = "normalized subtitle", + ) normalizedData = normalized return normalized.size.toLong() } @@ -409,10 +425,19 @@ internal class SubripNormalizingDataSource( private fun readAllFromUpstream(): ByteArray { val out = ByteArrayOutputStream() val buffer = ByteArray(DEFAULT_SUBRIP_READ_BUFFER_SIZE) + var total = 0L while (true) { val read = upstream.read(buffer, 0, buffer.size) if (read == C.RESULT_END_OF_INPUT) break - if (read > 0) out.write(buffer, 0, read) + if (read > 0) { + total = checkedLimitedByteCount( + currentBytes = total, + additionalBytes = read.toLong(), + maxBytes = maxBytes, + limitName = "subtitle", + ) + out.write(buffer, 0, read) + } } return out.toByteArray() } @@ -427,4 +452,5 @@ internal fun shouldNormalizeSubripPath(path: String?, position: Long): Boolean = internal fun normalizeSubripDataIfNeeded(raw: ByteArray): ByteArray = normalizeSubripPayloadIfNeeded(raw, 0, raw.size) ?: raw +internal const val MAX_SUBTITLE_BYTES = 32L * 1024 * 1024 private const val DEFAULT_SUBRIP_READ_BUFFER_SIZE = 16 * 1024 diff --git a/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/io/LimitedStreamsTest.kt b/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/io/LimitedStreamsTest.kt new file mode 100644 index 000000000..453f4a6c2 --- /dev/null +++ b/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/io/LimitedStreamsTest.kt @@ -0,0 +1,52 @@ +package org.siloserver.silo.common.io + +import java.io.ByteArrayInputStream +import java.io.ByteArrayOutputStream +import kotlin.test.Test +import kotlin.test.assertContentEquals +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith + +class LimitedStreamsTest { + @Test + fun `copy accepts exactly the byte limit`() { + val output = ByteArrayOutputStream() + + val copied = ByteArrayInputStream(byteArrayOf(1, 2, 3, 4)) + .copyToLimited(output, maxBytes = 4) + + assertEquals(4, copied) + assertContentEquals(byteArrayOf(1, 2, 3, 4), output.toByteArray()) + } + + @Test + fun `copy rejects limit plus one without writing the overflowing chunk`() { + val output = ByteArrayOutputStream() + + val error = assertFailsWith { + ByteArrayInputStream(byteArrayOf(1, 2, 3, 4, 5)) + .copyToLimited(output, maxBytes = 4, bufferSize = 2) + } + + assertEquals(4, error.maxBytes) + assertContentEquals(byteArrayOf(1, 2, 3, 4), output.toByteArray()) + } + + @Test + fun `checked byte count rejects overflow instead of wrapping`() { + assertFailsWith { + checkedLimitedByteCount( + currentBytes = Long.MAX_VALUE - 1, + additionalBytes = 2, + maxBytes = Long.MAX_VALUE, + ) + } + } + + @Test + fun `negative limits are rejected`() { + assertFailsWith { + ByteArrayInputStream(byteArrayOf()).copyToLimited(ByteArrayOutputStream(), -1) + } + } +} diff --git a/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/AuthenticatedDataSourceFactoryTest.kt b/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/AuthenticatedDataSourceFactoryTest.kt index 45d29dcd1..34b9b79ff 100644 --- a/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/AuthenticatedDataSourceFactoryTest.kt +++ b/android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/AuthenticatedDataSourceFactoryTest.kt @@ -6,6 +6,10 @@ import androidx.media3.datasource.DataSpec import androidx.media3.datasource.HttpDataSource import androidx.media3.datasource.TransferListener import kotlin.test.AfterTest +import androidx.media3.datasource.DataSource +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.siloserver.silo.common.io.ContentLimitExceeded import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -16,8 +20,6 @@ import okhttp3.OkHttpClient import okhttp3.Protocol import okhttp3.Response import okhttp3.ResponseBody.Companion.toResponseBody -import org.junit.runner.RunWith -import org.robolectric.RobolectricTestRunner import org.siloserver.silo.network.TokenManagerImpl @RunWith(RobolectricTestRunner::class) @@ -421,6 +423,57 @@ class AuthenticatedDataSourceFactoryTest { private val onOpen: (DataSpec) -> Long = { C.LENGTH_UNSET.toLong() }, ) : HttpDataSource { val openedDataSpecs = mutableListOf() + fun subtitleLimitIs32MiB() { + assertEquals(32L * 1024 * 1024, MAX_SUBTITLE_BYTES) + } + + @Test + fun subripNormalizationAcceptsExactlyTheStreamLimit() { + val upstream = ByteArrayDataSource(byteArrayOf(1, 2, 3, 4)) + val source = SubripNormalizingDataSource(upstream, maxBytes = 4) + + assertEquals(4, source.open(subripDataSpec())) + assertTrue(upstream.closed) + } + + @Test + fun subripNormalizationRejectsDeclaredLimitPlusOneBeforeReading() { + val upstream = ByteArrayDataSource( + bytes = byteArrayOf(1), + declaredLength = 5, + ) + val source = SubripNormalizingDataSource(upstream, maxBytes = 4) + + assertFailsWith { + source.open(subripDataSpec()) + } + assertEquals(0, upstream.readCalls) + assertTrue(upstream.closed) + } + + @Test + fun subripNormalizationRejectsStreamedLimitPlusOneAndClosesUpstream() { + val upstream = ByteArrayDataSource( + bytes = byteArrayOf(1, 2, 3, 4, 5), + declaredLength = C.LENGTH_UNSET.toLong(), + ) + val source = SubripNormalizingDataSource(upstream, maxBytes = 4) + + assertFailsWith { + source.open(subripDataSpec()) + } + assertTrue(upstream.closed) + } + + private fun subripDataSpec(): DataSpec = + DataSpec(Uri.parse("https://silo.example/subtitles/1.srt")) + + private class ByteArrayDataSource( + private val bytes: ByteArray, + private val declaredLength: Long = bytes.size.toLong(), + ) : DataSource { + private var position = 0 + var readCalls = 0 var closed = false override fun addTransferListener(transferListener: TransferListener) = Unit @@ -444,6 +497,18 @@ class AuthenticatedDataSourceFactoryTest { override fun clearRequestProperty(name: String) = Unit override fun clearAllRequestProperties() = Unit + override fun open(dataSpec: DataSpec): Long = declaredLength + + override fun read(buffer: ByteArray, offset: Int, length: Int): Int { + readCalls += 1 + if (position == bytes.size) return C.RESULT_END_OF_INPUT + val count = minOf(length, bytes.size - position) + bytes.copyInto(buffer, offset, position, position + count) + position += count + return count + } + + override fun getUri(): Uri = Uri.parse("https://silo.example/subtitles/1.srt") override fun close() { closed = true diff --git a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/reader/EpubBook.kt b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/reader/EpubBook.kt index 67ce8683c..5421d5d4f 100644 --- a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/reader/EpubBook.kt +++ b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/reader/EpubBook.kt @@ -1,11 +1,35 @@ package org.siloserver.silo.android.ui.screens.reader +import org.siloserver.silo.common.io.ContentLimitExceeded +import org.siloserver.silo.common.io.checkedLimitedByteCount +import org.siloserver.silo.common.io.copyToLimited import java.io.File import java.io.FileOutputStream import java.net.URI import java.security.MessageDigest import java.util.zip.ZipFile +internal data class EpubExtractionLimits( + val maxEntries: Int, + val maxEntryBytes: Long, + val maxTotalBytes: Long, + val maxCompressionRatio: Long, +) { + init { + require(maxEntries >= 0) { "maxEntries must not be negative" } + require(maxEntryBytes >= 0) { "maxEntryBytes must not be negative" } + require(maxTotalBytes >= 0) { "maxTotalBytes must not be negative" } + require(maxCompressionRatio > 0) { "maxCompressionRatio must be positive" } + } +} + +internal val DEFAULT_EPUB_EXTRACTION_LIMITS = EpubExtractionLimits( + maxEntries = 20_000, + maxEntryBytes = 512L * 1024 * 1024, + maxTotalBytes = 2L * 1024 * 1024 * 1024, + maxCompressionRatio = 200, +) + /** * Minimal EPUB parser. Holds the unpacked archive on disk so the * WebView can resolve relative URLs naturally; nothing else in the app @@ -40,55 +64,126 @@ internal class EpubBook private constructor( } companion object { - fun open(epub: File, cacheRoot: File): EpubBook { + fun open( + epub: File, + cacheRoot: File, + limits: EpubExtractionLimits = DEFAULT_EPUB_EXTRACTION_LIMITS, + ): EpubBook { val key = fileContentCacheKey(epub) val cacheDir = File(cacheRoot, "readers").apply { mkdirs() } val unpacked = File(cacheDir, "epub-$key") - if (unpacked.listFiles().isNullOrEmpty()) { - unpackAtomically(epub, unpacked) + try { + validateArchiveMetadata(epub, limits) + if (unpacked.listFiles().isNullOrEmpty()) { + unpackAtomically(epub, unpacked, limits) + } + + // container.xml points to the OPF package. + val containerXml = safeChild(unpacked, "META-INF/container.xml", unpacked).readText() + val opfHref = ROOTFILE_TAG_REGEX.findAll(containerXml) + .mapNotNull { it.value.xmlAttribute("full-path") } + .firstOrNull() + ?: error("EPUB missing OPF rootfile") + val opfFile = safeChild(unpacked, opfHref, unpacked) + val opfDir = opfFile.parentFile!! + val opfXml = opfFile.readText() + + // Spine entries reference manifest items by idref. Manifest + // items carry the actual href. Build the chapter list by + // joining the two. + val manifest = ITEM_TAG_REGEX.findAll(opfXml).mapNotNull { + val tag = it.value + val id = tag.xmlAttribute("id") + val href = tag.xmlAttribute("href") + if (id != null && href != null) id to href else null + }.toMap() + val spine = ITEMREF_TAG_REGEX.findAll(opfXml) + .mapNotNull { it.value.xmlAttribute("idref") } + .mapNotNull { manifest[it] } + .toList() + + return EpubBook(unpacked, opfDir, spine) + } catch (throwable: Throwable) { + unpacked.deleteRecursively() + throw throwable } + } - // container.xml points to the OPF package. - val containerXml = safeChild(unpacked, "META-INF/container.xml", unpacked).readText() - val opfHref = ROOTFILE_TAG_REGEX.findAll(containerXml) - .mapNotNull { it.value.xmlAttribute("full-path") } - .firstOrNull() - ?: error("EPUB missing OPF rootfile") - val opfFile = safeChild(unpacked, opfHref, unpacked) - val opfDir = opfFile.parentFile!! - val opfXml = opfFile.readText() - - // Spine entries reference manifest items by idref. Manifest - // items carry the actual href. Build the chapter list by - // joining the two. - val manifest = ITEM_TAG_REGEX.findAll(opfXml).mapNotNull { - val tag = it.value - val id = tag.xmlAttribute("id") - val href = tag.xmlAttribute("href") - if (id != null && href != null) id to href else null - }.toMap() - val spine = ITEMREF_TAG_REGEX.findAll(opfXml) - .mapNotNull { it.value.xmlAttribute("idref") } - .mapNotNull { manifest[it] } - .toList() - - return EpubBook(unpacked, opfDir, spine) + private fun validateArchiveMetadata(epub: File, limits: EpubExtractionLimits) { + ZipFile(epub).use { zip -> + val entries = zip.entries() + var entryCount = 0 + var declaredTotal = 0L + while (entries.hasMoreElements()) { + val entry = entries.nextElement() + entryCount += 1 + if (entryCount > limits.maxEntries) { + throw ContentLimitExceeded( + limits.maxEntries.toLong(), + "EPUB entry count", + ) + } + if (entry.size >= 0) { + checkedLimitedByteCount( + currentBytes = 0, + additionalBytes = entry.size, + maxBytes = limits.maxEntryBytes, + limitName = "EPUB entry", + ) + declaredTotal = checkedLimitedByteCount( + currentBytes = declaredTotal, + additionalBytes = entry.size, + maxBytes = limits.maxTotalBytes, + limitName = "EPUB total output", + ) + validateCompressionRatio( + uncompressedBytes = entry.size, + compressedBytes = entry.compressedSize, + limits = limits, + ) + } + } + } } - private fun unpackAtomically(epub: File, unpacked: File) { + private fun unpackAtomically( + epub: File, + unpacked: File, + limits: EpubExtractionLimits, + ) { val tmp = File(unpacked.parentFile, "${unpacked.name}.tmp-${System.nanoTime()}") tmp.deleteRecursively() tmp.mkdirs() try { ZipFile(epub).use { zip -> - zip.entries().toList().forEach { entry -> + val entries = zip.entries() + var streamedTotal = 0L + while (entries.hasMoreElements()) { + val entry = entries.nextElement() val out = safeChild(tmp, entry.name, tmp) if (entry.isDirectory) { out.mkdirs() } else { out.parentFile?.mkdirs() + val totalRemaining = limits.maxTotalBytes - streamedTotal zip.getInputStream(entry).use { input -> - FileOutputStream(out).use { output -> input.copyTo(output) } + FileOutputStream(out).use { output -> + val entryBytes = input.copyToLimited( + out = output, + maxBytes = minOf(limits.maxEntryBytes, totalRemaining), + ) + streamedTotal = checkedLimitedByteCount( + currentBytes = streamedTotal, + additionalBytes = entryBytes, + maxBytes = limits.maxTotalBytes, + limitName = "EPUB total output", + ) + validateCompressionRatio( + uncompressedBytes = entryBytes, + compressedBytes = entry.compressedSize, + limits = limits, + ) + } } } } @@ -100,10 +195,46 @@ internal class EpubBook private constructor( } } catch (throwable: Throwable) { tmp.deleteRecursively() + unpacked.deleteRecursively() throw throwable } } + private fun validateCompressionRatio( + uncompressedBytes: Long, + compressedBytes: Long, + limits: EpubExtractionLimits, + ) { + if (compressedBytes < 0) { + throw ContentLimitExceeded( + limits.maxCompressionRatio, + "EPUB compression ratio with unknown compressed size", + ) + } + if (compressedBytes == 0L) { + if (uncompressedBytes > 0) { + throw ContentLimitExceeded( + limits.maxCompressionRatio, + "EPUB compression ratio", + ) + } + return + } + val maxUncompressed = if ( + compressedBytes > Long.MAX_VALUE / limits.maxCompressionRatio + ) { + Long.MAX_VALUE + } else { + compressedBytes * limits.maxCompressionRatio + } + if (uncompressedBytes > maxUncompressed) { + throw ContentLimitExceeded( + limits.maxCompressionRatio, + "EPUB compression ratio", + ) + } + } + private fun safeChild(parent: File, name: String, root: File): File { val canonicalRoot = root.canonicalFile val out = File(parent, name).canonicalFile diff --git a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/reader/ReaderFileCache.kt b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/reader/ReaderFileCache.kt index 3748aa37c..7e0f67f6d 100644 --- a/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/reader/ReaderFileCache.kt +++ b/androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/reader/ReaderFileCache.kt @@ -6,9 +6,12 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import okhttp3.OkHttpClient import okhttp3.Request +import org.siloserver.silo.common.io.checkedLimitedByteCount +import org.siloserver.silo.common.io.copyToLimited import java.io.File import java.io.FileOutputStream import java.io.IOException +import java.io.InputStream import java.io.OutputStream import java.net.URI import java.security.MessageDigest @@ -17,6 +20,7 @@ import java.security.MessageDigest * fallback body, so the reader must not cache it as the converted format. */ private const val EBOOK_CONVERSION_HEADER = "X-Silo-Ebook-Conversion" private const val EBOOK_CONVERSION_FAILED = "failed" +internal const val MAX_READER_INPUT_BYTES = 2L * 1024 * 1024 * 1024 /** SHA-1 cache key for a reader URL — the one shared copy of the helper * the readers previously duplicated five times. */ @@ -65,9 +69,15 @@ internal fun cacheReaderFile( tmp.delete() } } - if (validate != null && !validate(target)) { - target.delete() - throw IOException("Cached reader file failed validation") + if (validate != null) { + try { + if (!validate(target)) { + throw IOException("Cached reader file failed validation") + } + } catch (throwable: Throwable) { + target.delete() + throw throwable + } } return target } @@ -91,17 +101,31 @@ internal suspend fun resolveReaderFile( ): File = withContext(Dispatchers.IO) { val requestUrl = resolveReaderRequestUrl(url, serverUrl) when (readerRequestKind(url, serverUrl)) { - ReaderRequestKind.File -> return@withContext readerFileFromFileUrl(requestUrl) + ReaderRequestKind.File -> { + val file = readerFileFromFileUrl(requestUrl) + validateReaderDeclaredLength(file.length()) + return@withContext file + } ReaderRequestKind.Content, ReaderRequestKind.Remote -> Unit } val cacheDir = File(context.cacheDir, "readers") val fileName = readerCacheFileName(url, serverUrl, extension) - val validate = readerCacheValidatorFor(extension) + val formatValidator = readerCacheValidatorFor(extension) + val validate: (File) -> Boolean = { file -> + file.length() <= MAX_READER_INPUT_BYTES && + (formatValidator == null || formatValidator(file)) + } if (requestUrl.startsWith("content://")) { + val declaredLength = runCatching { + context.contentResolver.openAssetFileDescriptor(Uri.parse(requestUrl), "r")?.use { descriptor -> + descriptor.length + } + }.getOrNull() + if (declaredLength != null) validateReaderDeclaredLength(declaredLength) return@withContext cacheReaderFile(cacheDir, fileName, validate) { out -> context.contentResolver.openInputStream(Uri.parse(requestUrl))?.use { input -> - input.copyTo(out) + input.copyReaderInputTo(out) } ?: error("Could not open content reader file") } } @@ -115,11 +139,31 @@ internal suspend fun resolveReaderFile( error("Server could not convert this book for in-app reading") } val body = resp.body ?: error("Empty body fetching reader file") - body.byteStream().copyTo(out) + validateReaderDeclaredLength(body.contentLength()) + body.byteStream().use { input -> input.copyReaderInputTo(out) } } } } +internal fun InputStream.copyReaderInputTo( + output: OutputStream, + maxBytes: Long = MAX_READER_INPUT_BYTES, +): Long = copyToLimited(output, maxBytes) + +internal fun validateReaderDeclaredLength( + length: Long, + maxBytes: Long = MAX_READER_INPUT_BYTES, +) { + if (length >= 0) { + checkedLimitedByteCount( + currentBytes = 0, + additionalBytes = length, + maxBytes = maxBytes, + limitName = "reader input", + ) + } +} + private fun readerCacheValidatorFor(extension: String): ((File) -> Boolean)? = when (extension.trim().lowercase().removePrefix(".")) { "epub" -> ::hasZipMagic diff --git a/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/EpubBookTest.kt b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/EpubBookTest.kt index e7d7168cd..1d8208e8a 100644 --- a/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/EpubBookTest.kt +++ b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/EpubBookTest.kt @@ -1,14 +1,18 @@ package org.siloserver.silo.android.ui.screens.reader +import org.siloserver.silo.common.io.ContentLimitExceeded import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder import java.io.File +import java.util.zip.CRC32 import java.util.zip.ZipEntry +import java.util.zip.ZipFile import java.util.zip.ZipOutputStream import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertFailsWith +import kotlin.test.assertTrue class EpubBookTest { @get:Rule @@ -27,6 +31,7 @@ class EpubBookTest { EpubBook.open(epub, tmp.root) } assertFalse(File(tmp.root, "escaped.txt").exists()) + assertNoPartialExtraction() } @Test @@ -56,11 +61,166 @@ class EpubBookTest { assertEquals("Chapter", book.readChapterHtml("chapter.xhtml")) } + @Test + fun `production epub limits match the security policy`() { + assertEquals(20_000, DEFAULT_EPUB_EXTRACTION_LIMITS.maxEntries) + assertEquals(512L * 1024 * 1024, DEFAULT_EPUB_EXTRACTION_LIMITS.maxEntryBytes) + assertEquals(2L * 1024 * 1024 * 1024, DEFAULT_EPUB_EXTRACTION_LIMITS.maxTotalBytes) + assertEquals(200L, DEFAULT_EPUB_EXTRACTION_LIMITS.maxCompressionRatio) + } + + @Test + fun `open accepts exact entry count boundary`() { + val epub = tmp.newFile("exact-entries.epub") + writeEpub(epub, chapters = mapOf("OEBPS/chapter.xhtml" to "chapter")) + + val book = EpubBook.open( + epub, + tmp.root, + limitsFor(epub).copy(maxEntries = zipEntries(epub).size), + ) + + assertEquals("chapter", book.readChapterHtml("chapter.xhtml")) + } + + @Test + fun `open rejects entry count limit plus one and removes partial directory`() { + val epub = tmp.newFile("too-many-entries.epub") + writeEpub( + epub, + chapters = mapOf("OEBPS/chapter.xhtml" to "chapter"), + extraEntries = mapOf("extra.txt" to "extra"), + ) + + assertFailsWith { + EpubBook.open( + epub, + tmp.root, + limitsFor(epub).copy(maxEntries = zipEntries(epub).size - 1), + ) + } + + assertNoPartialExtraction() + } + + @Test + fun `open accepts exact per entry byte boundary`() { + val epub = tmp.newFile("exact-entry-bytes.epub") + writeEpub(epub, chapters = mapOf("OEBPS/chapter.xhtml" to "chapter")) + val maxEntryBytes = zipEntries(epub).maxOf { it.size } + + EpubBook.open( + epub, + tmp.root, + limitsFor(epub).copy(maxEntryBytes = maxEntryBytes), + ) + } + + @Test + fun `open rejects per entry byte limit plus one and removes partial directory`() { + val epub = tmp.newFile("large-entry.epub") + writeEpub(epub, chapters = mapOf("OEBPS/chapter.xhtml" to "chapter")) + val maxEntryBytes = zipEntries(epub).maxOf { it.size } + + assertFailsWith { + EpubBook.open( + epub, + tmp.root, + limitsFor(epub).copy(maxEntryBytes = maxEntryBytes - 1), + ) + } + + assertNoPartialExtraction() + } + + @Test + fun `open accepts exact total byte boundary`() { + val epub = tmp.newFile("exact-total.epub") + writeEpub(epub, chapters = mapOf("OEBPS/chapter.xhtml" to "chapter")) + val totalBytes = zipEntries(epub).sumOf { it.size } + + EpubBook.open( + epub, + tmp.root, + limitsFor(epub).copy(maxTotalBytes = totalBytes), + ) + } + + @Test + fun `open rejects total byte limit plus one and removes partial directory`() { + val epub = tmp.newFile("large-total.epub") + writeEpub(epub, chapters = mapOf("OEBPS/chapter.xhtml" to "chapter")) + val totalBytes = zipEntries(epub).sumOf { it.size } + + assertFailsWith { + EpubBook.open( + epub, + tmp.root, + limitsFor(epub).copy(maxTotalBytes = totalBytes - 1), + ) + } + + assertNoPartialExtraction() + } + + @Test + fun `open accepts exact one to one compression ratio boundary`() { + val epub = tmp.newFile("exact-ratio.epub") + writeEpub( + epub, + chapters = mapOf("OEBPS/chapter.xhtml" to "chapter"), + stored = true, + ) + + EpubBook.open( + epub, + tmp.root, + limitsFor(epub).copy(maxCompressionRatio = 1), + ) + } + + @Test + fun `open rejects compression ratio over limit and removes partial directory`() { + val epub = tmp.newFile("ratio-bomb.epub") + writeEpub( + epub, + chapters = mapOf("OEBPS/chapter.xhtml" to "A".repeat(4096)), + ) + + assertFailsWith { + EpubBook.open( + epub, + tmp.root, + limitsFor(epub).copy(maxCompressionRatio = 1), + ) + } + + assertNoPartialExtraction() + } + + @Test + fun `open removes extracted directory when epub metadata parsing fails`() { + val epub = tmp.newFile("missing-opf.epub") + ZipOutputStream(epub.outputStream()).use { zip -> + zip.writeTextEntry( + "META-INF/container.xml", + "", + ) + } + + assertFailsWith { + EpubBook.open(epub, tmp.root) + } + + assertNoPartialExtraction() + } + private fun writeEpub( target: File, chapters: Map, extraEntries: Map = emptyMap(), selfClosingManifestItem: Boolean = true, + stored: Boolean = false, ) { val manifestItem = if (selfClosingManifestItem) { """""" @@ -71,6 +231,7 @@ class EpubBookTest { zip.writeTextEntry( "META-INF/container.xml", """""", + stored, ) zip.writeTextEntry( "OEBPS/content.opf", @@ -84,15 +245,48 @@ class EpubBookTest { """.trimIndent(), + stored, ) - chapters.forEach { (name, body) -> zip.writeTextEntry(name, body) } - extraEntries.forEach { (name, body) -> zip.writeTextEntry(name, body) } + chapters.forEach { (name, body) -> zip.writeTextEntry(name, body, stored) } + extraEntries.forEach { (name, body) -> zip.writeTextEntry(name, body, stored) } } } - private fun ZipOutputStream.writeTextEntry(name: String, body: String) { - putNextEntry(ZipEntry(name)) - write(body.toByteArray()) + private fun ZipOutputStream.writeTextEntry(name: String, body: String, stored: Boolean = false) { + val bytes = body.toByteArray() + val entry = ZipEntry(name) + if (stored) { + val crc = CRC32().apply { update(bytes) } + entry.method = ZipEntry.STORED + entry.size = bytes.size.toLong() + entry.compressedSize = bytes.size.toLong() + entry.crc = crc.value + } + putNextEntry(entry) + write(bytes) closeEntry() } + + private fun zipEntries(epub: File): List = + ZipFile(epub).use { it.entries().toList() } + + private fun limitsFor(epub: File): EpubExtractionLimits { + val entries = zipEntries(epub) + return EpubExtractionLimits( + maxEntries = entries.size + 1, + maxEntryBytes = entries.maxOf { it.size } + 1, + maxTotalBytes = entries.sumOf { it.size } + 1, + maxCompressionRatio = Long.MAX_VALUE, + ) + } + + private fun assertNoPartialExtraction() { + val readerCache = File(tmp.root, "readers") + assertTrue( + readerCache.listFiles().orEmpty().none { + it.name.startsWith("epub-") || ".tmp-" in it.name + }, + "no partial EPUB extraction expected", + ) + } } diff --git a/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/ReaderFileCacheTest.kt b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/ReaderFileCacheTest.kt index c54ce8839..a56f0d486 100644 --- a/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/ReaderFileCacheTest.kt +++ b/androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/ReaderFileCacheTest.kt @@ -1,5 +1,8 @@ package org.siloserver.silo.android.ui.screens.reader +import kotlinx.coroutines.CancellationException +import org.siloserver.silo.common.io.ContentLimitExceeded +import java.io.ByteArrayInputStream import java.io.File import java.io.IOException import java.nio.file.Files @@ -45,6 +48,84 @@ class ReaderFileCacheTest { assertTrue(cacheDir.listFiles()?.none { it.name.endsWith(".tmp") } ?: true, "no .tmp residue expected") } + @Test + fun `reader input limit is 2GiB`() { + assertEquals(2L * 1024 * 1024 * 1024, MAX_READER_INPUT_BYTES) + } + + @Test + fun `reader copy accepts exactly the byte limit`() { + val cacheDir = newCacheDir() + + val result = cacheReaderFile(cacheDir, "exact.pdf") { out -> + ByteArrayInputStream(byteArrayOf(1, 2, 3, 4)) + .copyReaderInputTo(out, maxBytes = 4) + } + + assertEquals(4, result.length()) + } + + @Test + fun `reader declared length accepts exact limit and rejects limit plus one`() { + validateReaderDeclaredLength(length = 4, maxBytes = 4) + + assertFailsWith { + validateReaderDeclaredLength(length = 5, maxBytes = 4) + } + } + + @Test + fun `reader unknown declared length defers to streamed byte accounting`() { + validateReaderDeclaredLength(length = -1, maxBytes = 0) + } + + @Test + fun `reader copy rejects limit plus one and removes partial files`() { + val cacheDir = newCacheDir() + + assertFailsWith { + cacheReaderFile(cacheDir, "oversize.pdf") { out -> + ByteArrayInputStream(byteArrayOf(1, 2, 3, 4, 5)) + .copyReaderInputTo(out, maxBytes = 4) + } + } + + assertFalse(File(cacheDir, "oversize.pdf").exists()) + assertTrue(cacheDir.listFiles().isNullOrEmpty(), "no partial reader files expected") + } + + @Test + fun `cancelled fetch removes partial files`() { + val cacheDir = newCacheDir() + + assertFailsWith { + cacheReaderFile(cacheDir, "cancelled.pdf") { out -> + out.write(byteArrayOf(1, 2, 3)) + throw CancellationException("cancelled") + } + } + + assertFalse(File(cacheDir, "cancelled.pdf").exists()) + assertTrue(cacheDir.listFiles().isNullOrEmpty(), "no partial reader files expected") + } + + @Test + fun `cancelled validation removes the newly cached file`() { + val cacheDir = newCacheDir() + + assertFailsWith { + cacheReaderFile( + cacheDir = cacheDir, + fileName = "cancelled-validation.epub", + validate = { throw CancellationException("cancelled") }, + ) { out -> + out.write(byteArrayOf(1, 2, 3)) + } + } + + assertTrue(cacheDir.listFiles().isNullOrEmpty(), "no partial reader files expected") + } + @Test fun `existing non-empty cache entry short-circuits without fetching`() { val cacheDir = newCacheDir() From 7acfebb97d6c2d22b7d9bfc6126c20decfe8ec55 Mon Sep 17 00:00:00 2001 From: rxwatcher Date: Thu, 23 Jul 2026 11:37:17 +0200 Subject: [PATCH 07/17] fix(reader): isolate epub resources in webview (cherry picked from commit 63cdcaddd5af12256e9ee72fae913c5e3d0e701c) (cherry picked from commit 5a040e0d3bb9b5210c3f4bdde0c0679b88f9d81e) --- androidApp/build.gradle.kts | 1 + .../assets/reader/reflow/paginator.js | 41 +++++- .../assets/reader/reflow/reader.html | 1 + .../android/ui/screens/reader/EpubBook.kt | 14 +- .../reader/reflow/EpubResourcePathHandler.kt | 120 ++++++++++++++++++ .../ui/screens/reader/reflow/ReflowWebView.kt | 41 ++++-- .../reader/ReflowWebViewEpubResourceTest.kt | 67 +++++++++- .../reader/reflow/EpubReflowSourceTest.kt | 8 +- .../reflow/EpubResourcePathHandlerTest.kt | 85 +++++++++++++ gradle/libs.versions.toml | 2 + 10 files changed, 358 insertions(+), 22 deletions(-) create mode 100644 androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubResourcePathHandler.kt create mode 100644 androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/reader/reflow/EpubResourcePathHandlerTest.kt diff --git a/androidApp/build.gradle.kts b/androidApp/build.gradle.kts index 0f28973ab..8511c0b1c 100644 --- a/androidApp/build.gradle.kts +++ b/androidApp/build.gradle.kts @@ -87,6 +87,7 @@ kotlin { implementation(libs.media3.ui.compose) implementation(libs.kotlinx.coroutines.android) implementation(libs.androidx.work.runtime.ktx) + implementation(libs.androidx.webkit) implementation(libs.koin.androidx.workmanager) implementation(libs.firebase.messaging) // Google Cast (Chromecast) — phone app only. TV app must not depend diff --git a/androidApp/src/androidMain/assets/reader/reflow/paginator.js b/androidApp/src/androidMain/assets/reader/reflow/paginator.js index ca2839119..6a0e7e2fc 100644 --- a/androidApp/src/androidMain/assets/reader/reflow/paginator.js +++ b/androidApp/src/androidMain/assets/reader/reflow/paginator.js @@ -2,6 +2,7 @@ var root = document.getElementById('reflow-root'); var styleEl = document.getElementById('reflow-style'); var page = 0, pageCount = 1; + var privateOrigin = 'https://appassets.androidplatform.net'; // Viewport in CSS px, pushed from the Android side (Compose-measured size). // Android WebView resolves `vh`/`vw` to 0 when the page loads before the view // is laid out, so we never rely on viewport units for the page box. @@ -46,15 +47,49 @@ img.addEventListener('load', onLoad); }); } + function rewriteResourceUrls(html, baseUrl){ + var template = document.createElement('template'); + template.innerHTML = html; + if (!baseUrl) return template.innerHTML; + + var base; + try { + base = new URL(baseUrl, privateOrigin); + if (base.origin !== privateOrigin || base.pathname.indexOf('/epub/') !== 0) { + return template.innerHTML; + } + } catch (e) { + return template.innerHTML; + } + + var attributes = ['href', 'src', 'xlink:href']; + Array.prototype.forEach.call(template.content.querySelectorAll('*'), function(element){ + attributes.forEach(function(attribute){ + if (!element.hasAttribute(attribute)) return; + var value = element.getAttribute(attribute); + if (!value || value.charAt(0) === '#') return; + try { + var resolved = new URL(value, base); + if (resolved.origin === privateOrigin && resolved.pathname.indexOf('/epub/') === 0) { + element.setAttribute(attribute, resolved.href); + } else { + element.removeAttribute(attribute); + } + } catch (e) { + element.removeAttribute(attribute); + } + }); + }); + return template.innerHTML; + } window.ReflowApi = { setViewport: function(w, h){ vpW = w; vpH = h; applyViewport(); requestAnimationFrame(remeasureKeepingProgress); }, load: function(html, baseUrl){ - var b = document.querySelector('base'); if(!b){ b=document.createElement('base'); document.head.appendChild(b);} - if(baseUrl) b.href = baseUrl; - root.innerHTML = html; remeasureAfterPendingImages(); page = 0; applyViewport(); apply(); + root.innerHTML = rewriteResourceUrls(html, baseUrl); + remeasureAfterPendingImages(); page = 0; applyViewport(); apply(); requestAnimationFrame(function(){ requestAnimationFrame(function(){ measure(); apply(); relocate(); }); }); }, goToPage: function(n){ page = Math.min(Math.max(0, n), pageCount-1); apply(); relocate(); }, diff --git a/androidApp/src/androidMain/assets/reader/reflow/reader.html b/androidApp/src/androidMain/assets/reader/reflow/reader.html index 3037b29e4..0fbcb114b 100644 --- a/androidApp/src/androidMain/assets/reader/reflow/reader.html +++ b/androidApp/src/androidMain/assets/reader/reflow/reader.html @@ -1,5 +1,6 @@ +