diff --git a/CHANGELOG.md b/CHANGELOG.md index 7497ff2..ee479ad 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 ─────────────────────────────────────────────