Improve CRL retrieval and fix poisoned cache

CHANGELOG.md

@@ -19,13 +19,21 @@ All notable changes to this project will be documented in this file. Take a look
 
 * :warning: The PDFium adapter now defaults to a horizontal paginated layout, instead of a vertical continuous scroll. Set `PdfiumDefaults(scroll = true)` to restore the previous behavior. See [the migration guide](docs/migration-guide.md).
 
+#### LCP
+
+* Opening an LCP publication is no longer delayed by the CRL used to validate its license. The CRL is now downloaded when creating the `LcpService`, and an expired one is refreshed in the background instead of making the user wait for the response.
+
 ### Fixed
 
 #### Navigator
 
 * Fixed the PDFium adapter reporting page positions off by one: `currentLocator` was one page ahead of the visible page, the first page was never reported, and the last page never updated `currentLocator` (contributed by [@huttarl](https://github.com/readium/kotlin-toolkit/pull/812)).
     * :warning: You must migrate the `Locator` objects created by the PDFium adapter and persisted in your database (e.g. bookmarks, reading progression), as their positions were one page too high. Use the new `Publication.migrateLegacyPdfiumLocator()` helper and take a look at [the migration guide](docs/migration-guide.md).
 
+#### LCP
+
+* [#832](https://github.com/readium/kotlin-toolkit/issues/832) The CRL used to validate LCP licenses is now checked to be a genuine X.509 CRL before being cached. Networks with a captive portal (e.g. on a plane) could return their login page with a `200 OK` status, which was then cached for seven days and prevented opening LCP publications. An invalid CRL cached by a previous version is now ignored instead of waiting for its expiration.
+
 #### Shared
 
 * EPUB HREFs that are not percent-encoded but carry a fragment or query (e.g. `chapter one.xhtml#section`, with a space in the filename) now keep their `#fragment`/`?query` instead of encoding the separators into the path. This fixes table of contents and Media Overlays links failing to resolve and navigate in poorly-authored EPUBs.

readium/lcp/src/main/java/org/readium/r2/lcp/LcpService.kt

@@ -11,6 +11,9 @@ package org.readium.r2.lcp
 
 import android.content.Context
 import java.io.File
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.SupervisorJob
 import org.readium.r2.lcp.auth.LcpDialogAuthentication
 import org.readium.r2.lcp.license.model.LicenseDocument
 import org.readium.r2.lcp.persistence.LcpDatabase
@@ -162,7 +165,14 @@ public interface LcpService {
                 httpClient = httpClient,
                 context = context
             )
-            val crl = CRLService(httpClient = httpClient, context = context)
+            val crl = CRLService(
+                httpClient = httpClient,
+                context = context,
+                coroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
+            )
+            // Warms up the CRL cache, so that opening the first publication is not delayed by it.
+            crl.preload()
+
             val passphrases = PassphrasesService(repository = passphraseRepository)
             return LicensesService(
                 licenses = licenseRepository,

readium/lcp/src/main/java/org/readium/r2/lcp/service/CRLService.kt

@@ -11,11 +11,16 @@ package org.readium.r2.lcp.service
 
 import android.content.Context
 import android.content.SharedPreferences
-import android.os.Build
 import androidx.core.content.edit
-import java.util.Base64
 import kotlin.time.Clock
 import kotlin.time.Instant
+import kotlinx.coroutines.CancellationException
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Deferred
+import kotlinx.coroutines.async
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
 import kotlinx.datetime.TimeZone
 import kotlinx.datetime.daysUntil
 import org.readium.r2.lcp.BuildConfig.DEBUG
@@ -28,62 +33,121 @@ import org.readium.r2.shared.util.http.HttpRequest
 import org.readium.r2.shared.util.http.fetch
 import timber.log.Timber
 
-internal class CRLService(val httpClient: HttpClient, val context: Context) {
+internal class CRLService(
+    private val httpClient: HttpClient,
+    private val context: Context,
+    private val coroutineScope: CoroutineScope,
+) {
+
+    companion object {
+        const val EXPIRATION = 7
+        const val CRL_KEY = "org.readium.r2-lcp-swift.CRL"
+        const val DATE_KEY = "org.readium.r2-lcp-swift.CRLDate"
+
+        private const val CRL_URL = "http://crl.edrlab.telesec.de/rl/EDRLab_CA.crl"
+    }
 
     private val preferences: SharedPreferences = context.getSharedPreferences(
         "org.readium.r2.lcp",
         Context.MODE_PRIVATE
     )
 
-    companion object {
-        const val EXPIRATION = 7
-        const val CRL_KEY = "org.readium.r2-lcp-swift.CRL"
-        const val DATE_KEY = "org.readium.r2-lcp-swift.CRLDate"
+    /**
+     * Guards [fetchJob], to make sure a single fetch is in flight at any time.
+     */
+    private val fetchMutex = Mutex()
+    private var fetchJob: Deferred<Crl>? = null
+
+    /**
+     * Warms up the cache, so that opening a publication does not have to wait for the CRL.
+     */
+    fun preload() {
+        refreshInBackground()
     }
 
     suspend fun retrieve(): String {
         val (localCRL, isExpired) = readLocal()
-        if (localCRL != null && !isExpired) {
-            return localCRL
+
+        if (localCRL != null) {
+            if (isExpired) {
+                // Refreshing in the background instead of waiting for the response, as the expired
+                // CRL is good enough to open a publication right away.
+                refreshInBackground()
+            }
+            return localCRL.pem
+        }
+
+        // Without any usable cached CRL, there is nothing to fall back on.
+        return fetchAndSave().pem
+    }
+
+    /**
+     * Fetches and caches a fresh CRL in [coroutineScope], if the cached one is missing or expired.
+     *
+     * A failed refresh is not worth reporting, as the cached CRL is used instead and the next call
+     * will try again.
+     */
+    private fun refreshInBackground() {
+        coroutineScope.launch {
+            try {
+                val (localCRL, isExpired) = readLocal()
+                if (localCRL == null || isExpired) {
+                    fetchAndSave()
+                }
+            } catch (e: CancellationException) {
+                throw e
+            } catch (e: Exception) {
+                if (DEBUG) Timber.e(e)
+            }
         }
+    }
 
-        return try {
-            fetch()
-                .also { saveLocal(it) }
-        } catch (e: Exception) {
-            if (DEBUG) Timber.e(e)
-            localCRL ?: throw e
+    /**
+     * Fetches the CRL and caches it.
+     */
+    private suspend fun fetchAndSave(): Crl {
+        val job = fetchMutex.withLock {
+            fetchJob
+                ?.takeIf { it.isActive }
+                ?: coroutineScope
+                    .async { fetch().also { saveLocal(it) } }
+                    .also { fetchJob = it }
         }
+
+        return job.await()
     }
 
-    private suspend fun fetch(): String {
-        val absoluteUrl = AbsoluteUrl(url = "http://crl.edrlab.telesec.de/rl/EDRLab_CA.crl")!!
+    private suspend fun fetch(): Crl {
+        val absoluteUrl = AbsoluteUrl(url = CRL_URL)!!
         val data = httpClient.fetch(HttpRequest(absoluteUrl))
             .map { it.body }
             .getOrElse { throw LcpException(LcpError.CrlFetching) }
 
-        return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
-            "-----BEGIN X509 CRL-----${Base64.getEncoder().encodeToString(data)}-----END X509 CRL-----"
-        } else {
-            "-----BEGIN X509 CRL-----${android.util.Base64.encodeToString(
-                data,
-                android.util.Base64.DEFAULT
-            )}-----END X509 CRL-----"
-        }
+        // The response might not be a CRL at all, for example when a captive portal returns its
+        // HTML login page with a 200 status. Caching it would break the license validation until
+        // the CRL expires, so we reject anything which is not a genuine X.509 CRL.
+        // See https://github.com/readium/kotlin-toolkit/issues/832
+        return Crl.fromDer(data)
+            ?: run {
+                if (DEBUG) Timber.e("The fetched CRL is not a valid X.509 CRL")
+                throw LcpException(LcpError.CrlFetching)
+            }
     }
 
     // Returns (CRL, expired)
-    private fun readLocal(): Pair<String?, Boolean> {
+    private fun readLocal(): Pair<Crl?, Boolean> {
         val crl = preferences.getString(CRL_KEY, null)
+            ?.let { Crl.parsePem(it) }
         val date = preferences.getString(DATE_KEY, null)?.let { Instant.parse(input = it) }
         val expired = date?.let { daysSince(date) >= EXPIRATION } ?: true
         return Pair(crl, expired)
     }
 
-    private fun saveLocal(crl: String): String {
-        preferences.edit { putString(CRL_KEY, crl) }
-        preferences.edit { putString(DATE_KEY, Clock.System.now().toString()) }
-        return crl
+    private fun saveLocal(crl: Crl) {
+        preferences.edit(commit = true) {
+            putString(CRL_KEY, crl.pem)
+            putString(DATE_KEY, Clock.System.now().toString())
+        }
     }
 
     private fun daysSince(date: Instant): Int {

readium/lcp/src/main/java/org/readium/r2/lcp/service/Crl.kt

@@ -0,0 +1,72 @@
+/*
+ * Copyright 2026 Readium Foundation. All rights reserved.
+ * Use of this source code is governed by the BSD-style license
+ * available in the top-level LICENSE file of the project.
+ */
+
+package org.readium.r2.lcp.service
+
+import java.io.ByteArrayInputStream
+import java.security.cert.CertificateFactory
+import kotlin.io.encoding.Base64
+import org.readium.r2.lcp.BuildConfig.DEBUG
+import timber.log.Timber
+
+/**
+ * A genuine X.509 certificate revocation list, in its PEM encoding.
+ *
+ * An instance can only be created from a payload which was successfully parsed as an X.509 CRL.
+ */
+@JvmInline
+internal value class Crl private constructor(val pem: String) {
+
+    companion object {
+        private const val PEM_HEADER = "-----BEGIN X509 CRL-----"
+        private const val PEM_FOOTER = "-----END X509 CRL-----"
+
+        /**
+         * Creates a [Crl] from its DER encoding, or returns null if [data] is not an X.509 CRL.
+         */
+        fun fromDer(data: ByteArray): Crl? {
+            if (!isX509Crl(data)) {
+                return null
+            }
+            return Crl("$PEM_HEADER${Base64.encode(data)}$PEM_FOOTER")
+        }
+
+        /**
+         * Parses a PEM-encoded [Crl], or returns null if [pem] is not an X.509 CRL.
+         */
+        fun parsePem(pem: String): Crl? {
+            if (!pem.startsWith(PEM_HEADER) || !pem.endsWith(PEM_FOOTER)) {
+                return null
+            }
+            val base64 = pem.substring(PEM_HEADER.length, pem.length - PEM_FOOTER.length)
+            val data = try {
+                // Decoding with [Base64.Mime] instead of [Base64.Default], as it ignores the line
+                // breaks found in the CRLs cached by previous versions of the toolkit on API level
+                // 25 and below.
+                Base64.Mime.decode(base64)
+            } catch (e: IllegalArgumentException) {
+                if (DEBUG) Timber.e(e)
+                return null
+            }
+            if (!isX509Crl(data)) {
+                return null
+            }
+            return Crl(pem)
+        }
+
+        /**
+         * Checks that [data] contains a DER-encoded X.509 CRL.
+         */
+        private fun isX509Crl(data: ByteArray): Boolean =
+            try {
+                CertificateFactory.getInstance("X.509")
+                    .generateCRL(ByteArrayInputStream(data)) != null
+            } catch (e: Exception) {
+                if (DEBUG) Timber.e(e)
+                false
+            }
+    }
+}