From 37a6156596a242978b182b38cdcf70a5de80814a Mon Sep 17 00:00:00 2001 From: SiteRelEnby <125829806+SiteRelEnby@users.noreply.github.com> Date: Tue, 14 Jul 2026 22:52:29 -0400 Subject: [PATCH] fix(security): give the image loader its own credential-free client Confirmed against the server (sheaf) and infra (cf-image-worker): served images are authorised entirely by an HMAC signature already in the URL (?token=&expires=), verified by the API's /v1/files serve path or by the Cloudflare image worker on the CDN host. They never need, and the worker never checks, an Authorization header. So instead of scoping the shared client's credentials to include the CDN origin, the image loader now uses a standalone OkHttp client with no auth stack at all: no AuthInterceptor, no CF-Access headers, no trusted-device cookie jar, no 401 token-refresh authenticator. It carries only the user-agent and the debug TLS trust for local dev servers (extracted to a shared applyDebugTls helper). This also stops the user's session transiting the CDN's Cloudflare edge for no reason. With images off the shared client, AuthInterceptor no longer needs the CDN in its trusted set, so it's tightened to the API origin only (defence in depth for the API client itself). Dropped the now-unused isTrustedCredentialOrigin helper and updated the tests to match: the API client sends nothing to the CDN or any external host. --- CHANGELOG.md | 7 ++- .../lupine/sheaf/data/api/AuthInterceptor.kt | 15 ++--- .../lupine/sheaf/data/api/CredentialOrigin.kt | 17 ++---- .../systems/lupine/sheaf/di/NetworkModule.kt | 56 ++++++++++++------- .../sheaf/data/api/AuthInterceptorTest.kt | 11 ++-- .../sheaf/data/api/CredentialOriginTest.kt | 55 ++++++++---------- 6 files changed, 77 insertions(+), 84 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a820ed4..4bca360 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,9 +11,10 @@ uses semantic versioning (`MAJOR.MINOR.PATCH`). - **Your login credentials only ever go to your own server now.** The app used to attach your session token (and any Cloudflare Access secrets) to every network request, including image loads. An avatar or an image embedded in a bio that was - hosted on some other server would therefore receive your live credentials. They - are now sent only to your configured instance and its image host, never to any - other server. The "remember this device" cookie is scoped the same way. + hosted on another server would therefore receive your live credentials. They are + now sent only to your configured server's API. Images are loaded with no + credentials at all (they don't need any), and the "remember this device" cookie + is limited to your server too. - **Switching servers, or being signed out, fully clears the previous account.** Changing the server address while signed in now ends the old session and clears diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/AuthInterceptor.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/AuthInterceptor.kt index dbadcef..045443e 100644 --- a/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/AuthInterceptor.kt +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/AuthInterceptor.kt @@ -32,17 +32,12 @@ class AuthInterceptor @Inject constructor( val builder = request.newBuilder() .addHeader("X-Sheaf-Client", clientHeader) - // Credentials go only to the instance's own origins. The Coil image - // client shares this interceptor, and image URLs can point at an - // external host (a remote avatar, an image embedded in a bio), so an - // unconditional bearer / CF-Access header would hand the user's live - // session to whatever server that image lives on. + // Credentials go only to the configured API origin. Requests to any + // other host (an external URL, or the image CDN) must not carry the + // user's session. The image loader uses its own credential-free client, + // so this is defence in depth for the API client itself. val trusted = runBlocking { - isTrustedCredentialOrigin( - request.url, - prefs.baseUrl.firstOrNull(), - prefs.fileCdnBase.firstOrNull(), - ) + originMatches(request.url, prefs.baseUrl.firstOrNull()) } if (trusted) { val token = pendingToken ?: runBlocking { prefs.accessToken.firstOrNull() } diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/CredentialOrigin.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/CredentialOrigin.kt index a99fcdd..5f7a2bd 100644 --- a/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/CredentialOrigin.kt +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/CredentialOrigin.kt @@ -7,11 +7,10 @@ import okhttp3.HttpUrl.Companion.toHttpUrlOrNull * Origin matching for deciding where credentials may be sent. * * The API auth stack (bearer token, Cloudflare Access secrets, the - * trusted-device cookie) used to be attached host-blind, and the Coil image - * client is cloned from the API client, so a member avatar or a bio-embedded - * image hosted on an external server received the user's live credentials. The - * interceptors now gate on origin: credentials go only to the instance's own - * hosts, never to whatever host an image URL happens to point at. + * trusted-device cookie) used to be attached host-blind. The interceptors now + * gate on origin so the session reaches only the configured API host. (Images + * carry no credentials at all - they use their own client and are authorised by + * an in-URL HMAC signature - so the CDN host is not part of this.) */ private fun normalizedOrigin(configured: String?): HttpUrl? { @@ -30,14 +29,6 @@ internal fun originMatches(url: HttpUrl, configured: String?): Boolean { url.port == base.port } -/** - * Credentials may be sent to the API base origin and to the instance's - * configured file CDN origin (both are infrastructure the user configured or - * received from the instance's own config), and to nothing else. - */ -internal fun isTrustedCredentialOrigin(url: HttpUrl, baseUrl: String?, fileCdnBase: String?): Boolean = - originMatches(url, baseUrl) || originMatches(url, fileCdnBase) - /** * True when two configured base URLs point at the same origin. Used to decide * whether changing the server URL is actually switching instances (and so must diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/di/NetworkModule.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/di/NetworkModule.kt index 365f67e..4fa8361 100644 --- a/sheaf/app/src/main/java/systems/lupine/sheaf/di/NetworkModule.kt +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/di/NetworkModule.kt @@ -88,23 +88,30 @@ object NetworkModule { } ) - if (BuildConfig.DEBUG) { - val trustAll = object : X509TrustManager { - override fun checkClientTrusted(chain: Array, authType: String) = Unit - override fun checkServerTrusted(chain: Array, authType: String) = Unit - override fun getAcceptedIssuers(): Array = emptyArray() - } - val sslContext = SSLContext.getInstance("TLS").apply { - init(null, arrayOf(trustAll), null) - } - builder - .sslSocketFactory(sslContext.socketFactory, trustAll) - .hostnameVerifier { _, _ -> true } - } - + applyDebugTls(builder) return builder.build() } + /** + * In debug builds, trust any certificate and host so a locally-run instance + * on a self-signed cert works. No-op in release. Shared by the API and image + * clients so both reach a local dev server. + */ + private fun applyDebugTls(builder: OkHttpClient.Builder) { + if (!BuildConfig.DEBUG) return + val trustAll = object : X509TrustManager { + override fun checkClientTrusted(chain: Array, authType: String) = Unit + override fun checkServerTrusted(chain: Array, authType: String) = Unit + override fun getAcceptedIssuers(): Array = emptyArray() + } + val sslContext = SSLContext.getInstance("TLS").apply { + init(null, arrayOf(trustAll), null) + } + builder + .sslSocketFactory(sslContext.socketFactory, trustAll) + .hostnameVerifier { _, _ -> true } + } + @Provides @Singleton fun provideRetrofit(okHttpClient: OkHttpClient, moshi: Moshi): Retrofit = @@ -124,15 +131,22 @@ object NetworkModule { @Singleton fun provideImageLoader( @ApplicationContext context: Context, - okHttpClient: OkHttpClient, relativeUrlInterceptor: RelativeUrlInterceptor, + userAgentInterceptor: UserAgentInterceptor, ): ImageLoader { - // Cloned from the API client so it keeps the debug SSL-trust config for - // local dev servers. It also inherits AuthInterceptor, but that only - // attaches credentials to the instance's own origins now, so an image - // fetched from an external host carries none. (User-Agent and the - // cookie jar are inherited from the clone, so we don't re-add them.) - val imageClient = okHttpClient.newBuilder().build() + // A standalone client with NO auth stack, deliberately not cloned from + // the API client. Served images are authorised entirely by the HMAC + // signature already in their URL (?token=&expires=, verified by the API + // or the CDN worker), so they never need the bearer, the CF-Access + // secrets, the trusted-device cookie, or the 401 token-refresh + // authenticator. Cloning the API client would drag all of that onto + // every avatar fetch, including ones to the CDN host, sending the user's + // session through Cloudflare's edge for nothing. This carries only the + // user-agent and the debug TLS trust for local dev servers. + val imageClient = OkHttpClient.Builder() + .addInterceptor(userAgentInterceptor) + .also { applyDebugTls(it) } + .build() return ImageLoader.Builder(context) .okHttpClient(imageClient) .components { diff --git a/sheaf/app/src/test/java/systems/lupine/sheaf/data/api/AuthInterceptorTest.kt b/sheaf/app/src/test/java/systems/lupine/sheaf/data/api/AuthInterceptorTest.kt index ac600ce..2224c35 100644 --- a/sheaf/app/src/test/java/systems/lupine/sheaf/data/api/AuthInterceptorTest.kt +++ b/sheaf/app/src/test/java/systems/lupine/sheaf/data/api/AuthInterceptorTest.kt @@ -22,7 +22,6 @@ class AuthInterceptorTest { private val prefs = mockk().apply { every { baseUrl } returns flowOf("https://app.sheaf.sh") - every { fileCdnBase } returns flowOf("https://cdn.sheaf.sh") every { accessToken } returns flowOf("access-tok") every { cfClientId } returns flowOf("cf-id") every { cfClientSecret } returns flowOf("cf-secret") @@ -53,13 +52,15 @@ class AuthInterceptorTest { assertEquals("cf-secret", sent.header("CF-Access-Client-Secret")) } - @Test fun `CDN image requests are still credentialed`() { + @Test fun `the image CDN host receives no credentials from the API client`() { + // Served images are authorised by their in-URL HMAC signature, so even + // the instance's own CDN host must not get the session token. val sent = send("https://cdn.sheaf.sh/avatars/x.png") - assertEquals("Bearer access-tok", sent.header("Authorization")) - assertEquals("cf-id", sent.header("CF-Access-Client-Id")) + assertNull(sent.header("Authorization")) + assertNull(sent.header("CF-Access-Client-Id")) } - @Test fun `an external image host receives no credentials`() { + @Test fun `an external host receives no credentials`() { val sent = send("https://images.example.com/remote-avatar.png") assertNull(sent.header("Authorization")) assertNull(sent.header("CF-Access-Client-Id")) diff --git a/sheaf/app/src/test/java/systems/lupine/sheaf/data/api/CredentialOriginTest.kt b/sheaf/app/src/test/java/systems/lupine/sheaf/data/api/CredentialOriginTest.kt index fedc549..907485e 100644 --- a/sheaf/app/src/test/java/systems/lupine/sheaf/data/api/CredentialOriginTest.kt +++ b/sheaf/app/src/test/java/systems/lupine/sheaf/data/api/CredentialOriginTest.kt @@ -7,66 +7,57 @@ import kotlin.test.assertTrue /** * Credentials (bearer, CF-Access secrets, the trusted-device cookie) may reach - * only the instance's own origins. This is what stops the shared image client - * from handing the user's session to whatever host an avatar or bio-embedded - * image points at. + * only the configured API origin. Images use a separate credential-free client, + * so the CDN is deliberately not trusted here. */ class CredentialOriginTest { private fun url(u: String) = u.toHttpUrl() private val api = "https://app.sheaf.sh" - private val cdn = "https://cdn.sheaf.sh" - @Test fun `a request to the API origin is trusted`() { - assertTrue(isTrustedCredentialOrigin(url("https://app.sheaf.sh/v1/members"), api, cdn)) + @Test fun `a request to the API origin matches`() { + assertTrue(originMatches(url("https://app.sheaf.sh/v1/members"), api)) } - @Test fun `a request to the configured CDN origin is trusted`() { - assertTrue(isTrustedCredentialOrigin(url("https://cdn.sheaf.sh/avatars/x.png"), api, cdn)) + @Test fun `an external host never matches`() { + assertFalse(originMatches(url("https://evil.example.com/x.png"), api)) + assertFalse(originMatches(url("https://imgur.com/a.png"), api)) } - @Test fun `an external host is never trusted`() { - assertFalse(isTrustedCredentialOrigin(url("https://evil.example.com/x.png"), api, cdn)) - assertFalse(isTrustedCredentialOrigin(url("https://imgur.com/a.png"), api, cdn)) + @Test fun `the image CDN host is not the API origin`() { + // The API client must not send the session to the CDN; images fetch it + // via their own client with no credentials anyway. + assertFalse(originMatches(url("https://cdn.sheaf.sh/avatars/x.png"), api)) } - @Test fun `a lookalike host is not trusted`() { - assertFalse(isTrustedCredentialOrigin(url("https://app.sheaf.sh.evil.com/x"), api, cdn)) - assertFalse(isTrustedCredentialOrigin(url("https://notapp.sheaf.sh/x"), api, cdn)) + @Test fun `a lookalike host does not match`() { + assertFalse(originMatches(url("https://app.sheaf.sh.evil.com/x"), api)) + assertFalse(originMatches(url("https://notapp.sheaf.sh/x"), api)) } @Test fun `scheme must match`() { // http vs https is a different origin; a downgrade must not carry creds. - assertFalse(isTrustedCredentialOrigin(url("http://app.sheaf.sh/v1/members"), api, cdn)) + assertFalse(originMatches(url("http://app.sheaf.sh/v1/members"), api)) } @Test fun `port must match`() { - assertFalse(isTrustedCredentialOrigin(url("https://app.sheaf.sh:8443/v1/members"), api, cdn)) + assertFalse(originMatches(url("https://app.sheaf.sh:8443/v1/members"), api)) } - @Test fun `the path prefix on the base URL is ignored for origin matching`() { + @Test fun `the path prefix on the base URL is ignored`() { val based = "https://example.org/sheaf" - assertTrue(isTrustedCredentialOrigin(url("https://example.org/v1/members"), based, null)) - assertTrue(isTrustedCredentialOrigin(url("https://example.org/sheaf/v1/members"), based, null)) + assertTrue(originMatches(url("https://example.org/v1/members"), based)) + assertTrue(originMatches(url("https://example.org/sheaf/v1/members"), based)) } @Test fun `host comparison is case-insensitive`() { - assertTrue(isTrustedCredentialOrigin(url("https://APP.sheaf.sh/v1/members"), api, cdn)) + assertTrue(originMatches(url("https://APP.sheaf.sh/v1/members"), api)) } - @Test fun `no CDN configured means only the API origin is trusted`() { - assertTrue(isTrustedCredentialOrigin(url("https://app.sheaf.sh/v1/members"), api, null)) - assertFalse(isTrustedCredentialOrigin(url("https://cdn.sheaf.sh/x"), api, null)) - } - - @Test fun `a scheme-less CDN base is assumed https`() { - assertTrue(isTrustedCredentialOrigin(url("https://cdn.sheaf.sh/x"), api, "cdn.sheaf.sh")) - } - - @Test fun `nothing is trusted when no base is configured`() { - assertFalse(isTrustedCredentialOrigin(url("https://app.sheaf.sh/v1/members"), null, null)) - assertFalse(isTrustedCredentialOrigin(url("https://app.sheaf.sh/v1/members"), "", null)) + @Test fun `nothing matches when no base is configured`() { + assertFalse(originMatches(url("https://app.sheaf.sh/v1/members"), null)) + assertFalse(originMatches(url("https://app.sheaf.sh/v1/members"), "")) } // ── Server-change detection ─────────────────────────────────────────────