Improve CRL retrieval and fix poisoned cache

CHANGELOG.md

@@ -11,12 +11,20 @@ All notable changes to this project will be documented in this file. Take a look
 
 * Converting a `Link` to a `Locator` is now synchronous: `await publication.locate(link)` becomes `publication.locator(for: link)`. The logic moved to `Manifest`, so it is also available as `manifest.locator(for: link)` without a `Publication`.
 
+#### 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
 
 * [#121](https://github.com/readium/swift-toolkit/issues/121) HTML `<audio>` and `<video>` elements are now paused when the resource moves off-screen in the EPUB navigator, rather than continuing to play in the background.
 
+#### LCP
+
+* 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.
+
 
 ## [4.0.0-alpha.1] - 2026-08-14
 

Sources/LCP/LCPService.swift

@@ -48,10 +48,18 @@ public final class LCPService: Loggable, Sendable {
             repository: passphraseRepository
         )
 
+        let crl = CRLService(httpClient: httpClient)
+
+        // Warms the CRL cache so that opening a publication does not have to
+        // wait on the network.
+        Task(priority: .utility) {
+            await crl.preload()
+        }
+
         licenses = LicensesService(
             client: client,
             licenses: licenseRepository,
-            crl: CRLService(httpClient: httpClient),
+            crl: crl,
             device: DeviceService(
                 deviceName: deviceName,
                 deviceId: deviceId,

Sources/LCP/Services/CRLService.swift

@@ -8,74 +8,162 @@ import Foundation
 import ReadiumShared
 
 /// Certificate Revocation List
-final class CRLService: Sendable {
+actor CRLService {
     /// Number of days before the CRL cache expires.
     private static let expiration = 7
 
+    private static let pemHeader = "-----BEGIN X509 CRL-----"
+    private static let pemFooter = "-----END X509 CRL-----"
+
     private static let crlKey = "org.readium.r2-lcp-swift.CRL"
     private static let dateKey = "org.readium.r2-lcp-swift.CRLDate"
 
     private let httpClient: HTTPClient
+    private let defaults: UserDefaults
+
+    /// Refresh currently in flight, if any.
+    private var refreshTask: Task<String, Error>?
 
-    init(httpClient: HTTPClient) {
+    /// - Parameter defaultsSuite: Name of the `UserDefaults` suite used to
+    ///   cache the CRL, or `nil` for the standard one. A suite name is taken
+    ///   rather than a `UserDefaults`, as the latter is not `Sendable` and
+    ///   cannot be handed over to an actor.
+    init(httpClient: HTTPClient, defaultsSuite: String? = nil) {
         self.httpClient = httpClient
+        defaults = defaultsSuite.flatMap { UserDefaults(suiteName: $0) } ?? .standard
     }
 
-    /// Retrieves the CRL either from the cache, or from EDRLab if the cache is outdated.
+    /// Warms the cache so that opening a publication does not have to wait on
+    /// the network.
+    func preload() {
+        guard readLocal()?.isExpired ?? true else {
+            return
+        }
+        _ = refresh()
+    }
+
+    /// Retrieves the CRL either from the cache, or from EDRLab if the cache is
+    /// missing or invalid.
+    ///
+    /// An expired cache is returned as is and refreshed in the background, as
+    /// waiting on the network would delay the opening of a publication.
     func retrieve() async throws -> String {
-        let localCRL = readLocal()
-        if let (crl, date) = localCRL, daysSince(date) < CRLService.expiration {
-            return crl
+        guard let (crl, isExpired) = readLocal() else {
+            return try await refresh().value
+        }
+
+        if isExpired {
+            _ = refresh()
         }
+        return crl
+    }
 
-        // Short timeout to avoid blocking the License, since we can always fall back on the cached CRL.
-        let timeout: TimeInterval? = (localCRL == nil) ? nil : 8
+    /// Starts a CRL refresh, or returns the one already in flight.
+    private func refresh() -> Task<String, Error> {
+        if let refreshTask {
+            return refreshTask
+        }
 
-        do {
-            let crl = try await fetch(timeout: timeout)
-            saveLocal(crl)
-            return crl
+        let task = Task(priority: .utility) {
+            defer { refreshTask = nil }
 
-        } catch {
-            // Fallback on the locally cached CRL if available
-            guard let (crl, _) = localCRL else {
-                throw error
-            }
+            let crl = try await fetch()
+            saveLocal(crl)
             return crl
         }
+        refreshTask = task
+        return task
     }
 
     /// Fetches the updated Certificate Revocation List from EDRLab.
-    private func fetch(timeout: TimeInterval? = nil) async throws -> String {
+    private func fetch() async throws -> String {
         let url = HTTPURL(string: "http://crl.edrlab.telesec.de/rl/EDRLab_CA.crl")!
 
-        let response = try await httpClient.fetch(HTTPRequest(url: url, timeoutInterval: timeout))
+        let response = try await httpClient.fetch(HTTPRequest(url: url))
             .mapError { _ in LCPError.crlFetching }
             .get()
 
-        guard !response.body.isEmpty else {
+        guard CRLService.isX509CRL(response.body) else {
             throw LCPError.crlFetching
         }
 
         let body = response.body.base64EncodedString()
-        return "-----BEGIN X509 CRL-----\(body)-----END X509 CRL-----"
+        return "\(CRLService.pemHeader)\(body)\(CRLService.pemFooter)"
     }
 
     /// Reads the local CRL.
-    private func readLocal() -> (String, Date)? {
-        let defaults = UserDefaults.standard
+    private func readLocal() -> (crl: String, isExpired: Bool)? {
         guard let crl = defaults.string(forKey: CRLService.crlKey),
-              let date = defaults.value(forKey: CRLService.dateKey) as? Date
+              let date = defaults.value(forKey: CRLService.dateKey) as? Date,
+              let der = CRLService.decodePEM(crl),
+              CRLService.isX509CRL(der)
         else {
             return nil
         }
 
-        return (crl, date)
+        return (crl, isExpired: daysSince(date) >= CRLService.expiration)
+    }
+
+    /// Extracts the DER payload of a PEM-encoded CRL cached by ``saveLocal(_:)``.
+    private static func decodePEM(_ crl: String) -> Data? {
+        guard crl.hasPrefix(pemHeader), crl.hasSuffix(pemFooter) else {
+            return nil
+        }
+        let base64 = crl.dropFirst(pemHeader.count).dropLast(pemFooter.count)
+        return Data(base64Encoded: String(base64))
+    }
+
+    /// Checks that `data` looks like a DER-encoded X.509 CRL.
+    ///
+    /// `CertificateList` is a `SEQUENCE` whose first element is the
+    /// `tbsCertList` `SEQUENCE`. We don't parse the whole structure: this is
+    /// only meant to reject payloads which are not DER at all, such as the HTML
+    /// login page of a captive portal, or a truncated download.
+    ///
+    /// Note that `SEQUENCE { SEQUENCE, ... }` is also the shape of an X.509
+    /// *certificate*. Telling them apart would mean walking into the
+    /// `tbsCertList` looking for a `UTCTime`/`GeneralizedTime`, which guards a
+    /// scenario – this endpoint serving the wrong DER object – with no
+    /// realistic trigger.
+    static func isX509CRL(_ data: Data) -> Bool {
+        let bytes = [UInt8](data)
+
+        // DER `SEQUENCE` tag.
+        guard bytes.count >= 2, bytes[0] == 0x30 else {
+            return false
+        }
+
+        let headerSize: Int
+        let length: Int
+
+        if bytes[1] & 0x80 == 0 {
+            // Short form: the length fits in the byte itself.
+            headerSize = 2
+            length = Int(bytes[1])
+
+        } else {
+            // Long form: the low bits give the number of length bytes. 0x80 is
+            // the indefinite form, illegal in DER, and 0xFF is reserved. We
+            // also reject lengths wider than 4 bytes, way beyond any CRL.
+            let lengthSize = Int(bytes[1] & 0x7F)
+            guard (1 ... 4).contains(lengthSize), bytes.count >= 2 + lengthSize else {
+                return false
+            }
+            headerSize = 2 + lengthSize
+            length = bytes[2 ..< headerSize].reduce(0) { $0 << 8 | Int($1) }
+        }
+
+        // Rejects both a truncated download and trailing garbage.
+        guard headerSize + length == bytes.count else {
+            return false
+        }
+
+        // `tbsCertList` `SEQUENCE`.
+        return bytes.count > headerSize && bytes[headerSize] == 0x30
     }
 
     /// Caches the given CRL.
     private func saveLocal(_ crl: String) {
-        let defaults = UserDefaults.standard
         defaults.set(crl, forKey: CRLService.crlKey)
         defaults.set(Date(), forKey: CRLService.dateKey)
     }

Tests/LCPTests/MockHTTPClient.swift

@@ -0,0 +1,68 @@
+//
+//  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.
+//
+
+import Foundation
+import ReadiumShared
+
+/// An `HTTPClient` returning a canned response to every request.
+final class MockHTTPClient: HTTPClient {
+    private let body: Data
+    private let status: HTTPStatus
+    private let mediaType: MediaType?
+
+    private let responsesStream = AsyncStream.makeStream(of: HTTPResponse.self)
+    private let _requestCount = Mutex(0)
+
+    init(body: Data, status: HTTPStatus = .ok, mediaType: MediaType? = nil) {
+        self.body = body
+        self.status = status
+        self.mediaType = mediaType
+    }
+
+    /// Yields each response returned, to await a request made in the
+    /// background.
+    var responses: AsyncStream<HTTPResponse> {
+        responsesStream.stream
+    }
+
+    /// Number of requests received.
+    var requestCount: Int {
+        _requestCount.withLock { $0 }
+    }
+
+    func stream(
+        _ request: any HTTPRequestConvertible,
+        onReceiveResponse: (@Sendable (HTTPResponse) async -> HTTPResult<Void>)?,
+        consume: @Sendable (Data, Double?) -> HTTPResult<Void>
+    ) async -> HTTPResult<HTTPResponse> {
+        let httpRequest: HTTPRequest
+        switch request.httpRequest() {
+        case let .success(request):
+            httpRequest = request
+        case let .failure(error):
+            return .failure(error)
+        }
+        _requestCount.withLock { $0 += 1 }
+
+        let response = HTTPResponse(
+            request: httpRequest,
+            url: httpRequest.url,
+            status: status,
+            headers: [:],
+            mediaType: mediaType
+        )
+        responsesStream.continuation.yield(response)
+
+        if let onReceiveResponse = onReceiveResponse, case let .failure(error) = await onReceiveResponse(response) {
+            return .failure(error)
+        }
+        if case let .failure(error) = consume(body, 1.0) {
+            return .failure(error)
+        }
+
+        return .success(response)
+    }
+}