From ba9bbdc830b5cde1cb9cd8d7901899043cf6a42a Mon Sep 17 00:00:00 2001 From: Stefan Printezis Date: Sun, 22 Jun 2025 22:37:36 +0200 Subject: [PATCH] Conform SignDataVerifier & ChainVerifier to Sendable Instead of using a var for certificate caching use a seperate CacheManager actor for thread safety --- .../AppStoreServerLibrary/ChainVerifier.swift | 62 ++++++++----- .../SignedDataVerifier.swift | 2 +- .../CacheManagerTests.swift | 90 +++++++++++++++++++ .../SignedDataVerifierTests.swift | 80 ++++++++--------- 4 files changed, 173 insertions(+), 61 deletions(-) create mode 100644 Tests/AppStoreServerLibraryTests/CacheManagerTests.swift diff --git a/Sources/AppStoreServerLibrary/ChainVerifier.swift b/Sources/AppStoreServerLibrary/ChainVerifier.swift index 3d3ae67..9acae85 100644 --- a/Sources/AppStoreServerLibrary/ChainVerifier.swift +++ b/Sources/AppStoreServerLibrary/ChainVerifier.swift @@ -8,24 +8,55 @@ import Crypto import AsyncHTTPClient import NIOFoundationCompat -class ChainVerifier { +actor CacheManager: Sendable { + private var verifiedPublicKeyCache: [CacheKey: CacheValue] = [:] + private static let MAXIMUM_CACHE_SIZE = 32 // There are unlikely to be more than a couple keys at once + private static let CACHE_TIME_LIMIT: Int64 = 15 * 60 // 15 minutes in seconds + + func getCachedResult(for key: CacheKey) -> X509.VerificationResult? { + if let cachedResult = verifiedPublicKeyCache[key] { + if cachedResult.expirationTime > Date() { + return cachedResult.publicKey + } else { + verifiedPublicKeyCache.removeValue(forKey: key) + } + } + return nil + } + + func cacheResult(_ result: X509.VerificationResult, for key: CacheKey) { + verifiedPublicKeyCache[key] = CacheValue( + expirationTime: Date().addingTimeInterval(TimeInterval(integerLiteral: CacheManager.CACHE_TIME_LIMIT)), + publicKey: result + ) + + // Clean up expired entries if cache is too large. + if verifiedPublicKeyCache.count > CacheManager.MAXIMUM_CACHE_SIZE { + let currentTime = Date() + for kv in verifiedPublicKeyCache { + if kv.value.expirationTime < currentTime { + verifiedPublicKeyCache.removeValue(forKey: kv.key) + } + } + } + } +} + +final class ChainVerifier: Sendable { private static let EXPECTED_CHAIN_LENGTH = 3 private static let EXPECTED_JWT_SEGMENTS = 3 private static let EXPECTED_ALGORITHM = "ES256" - private static let MAXIMUM_CACHE_SIZE = 32 // There are unlikely to be more than a couple keys at once - private static let CACHE_TIME_LIMIT: Int64 = 15 * 60 // 15 minutes in seconds - private let store: CertificateStore private let requester: Requester - private var verifiedPublicKeyCache: [CacheKey: CacheValue] + private let cacheManager: CacheManager init(rootCertificates: [Data]) throws { let parsedCertificates = try rootCertificates.map { try Certificate(derEncoded: [UInt8]($0)) } self.store = CertificateStore(parsedCertificates) self.requester = Requester() - self.verifiedPublicKeyCache = [:] + self.cacheManager = CacheManager() } func verify(signedData: String, type: T.Type, onlineVerification: Bool, environment: AppStoreEnvironment) async -> VerificationResult where T: Decodable { @@ -96,24 +127,15 @@ class ChainVerifier { func verifyChain(leaf: Certificate, intermediate: Certificate, online: Bool, validationTime: Date) async -> X509.VerificationResult { if online { - if let cachedResult = verifiedPublicKeyCache[CacheKey(leaf: leaf, intermediate: intermediate)] { - if cachedResult.expirationTime > getDate() { - return cachedResult.publicKey - } + if let cachedResult = await cacheManager.getCachedResult(for: CacheKey(leaf: leaf, intermediate: intermediate)) { + return cachedResult } } let verificationResult = await verifyChainWithoutCaching(leaf: leaf, intermediate: intermediate, online: online, validationTime: validationTime) if online { if case let .validCertificate(verifiedChain) = verificationResult { - verifiedPublicKeyCache[CacheKey(leaf: leaf, intermediate: intermediate)] = CacheValue(expirationTime: getDate().addingTimeInterval(TimeInterval(integerLiteral: ChainVerifier.CACHE_TIME_LIMIT)), publicKey: verificationResult) - if verifiedPublicKeyCache.count > ChainVerifier.MAXIMUM_CACHE_SIZE { - for kv in verifiedPublicKeyCache { - if kv.value.expirationTime < getDate() { - verifiedPublicKeyCache.removeValue(forKey: kv.key) - } - } - } + await cacheManager.cacheResult(verificationResult, for: CacheKey(leaf: leaf, intermediate: intermediate)) } } @@ -137,12 +159,12 @@ class ChainVerifier { } } -struct CacheKey: Hashable { +struct CacheKey: Hashable, Sendable { let leaf: Certificate let intermediate: Certificate } -struct CacheValue { +struct CacheValue: Sendable { let expirationTime: Date let publicKey: X509.VerificationResult } diff --git a/Sources/AppStoreServerLibrary/SignedDataVerifier.swift b/Sources/AppStoreServerLibrary/SignedDataVerifier.swift index 2df7b3a..cde0729 100644 --- a/Sources/AppStoreServerLibrary/SignedDataVerifier.swift +++ b/Sources/AppStoreServerLibrary/SignedDataVerifier.swift @@ -3,7 +3,7 @@ import Foundation ///A verifier and decoder class designed to decode signed data from the App Store. -public struct SignedDataVerifier { +public struct SignedDataVerifier: Sendable { public enum ConfigurationError: Error { case INVALID_APP_APPLE_ID diff --git a/Tests/AppStoreServerLibraryTests/CacheManagerTests.swift b/Tests/AppStoreServerLibraryTests/CacheManagerTests.swift new file mode 100644 index 0000000..ae650aa --- /dev/null +++ b/Tests/AppStoreServerLibraryTests/CacheManagerTests.swift @@ -0,0 +1,90 @@ +import XCTest +@testable import AppStoreServerLibrary + +final class CacheManagerTests: XCTestCase { + struct DummyKey: Hashable, Sendable { + let id: Int + } + struct DummyValue: Sendable, Equatable { + let value: String + } + + actor TestableCacheManager { + private var cache: [DummyKey: (expiration: Date, value: DummyValue)] = [:] + private let maxSize: Int + private let expirationInterval: TimeInterval + private var now: Date + + init(maxSize: Int = 3, expirationInterval: TimeInterval = 1.0, now: Date = Date()) { + self.maxSize = maxSize + self.expirationInterval = expirationInterval + self.now = now + } + + func setNow(_ date: Date) { + self.now = date + } + + func cacheResult(_ value: DummyValue, for key: DummyKey) { + cache[key] = (expiration: now.addingTimeInterval(expirationInterval), value: value) + if cache.count > maxSize { + // Remove expired entries first + let expiredKeys = cache.filter { $0.value.expiration < now }.map { $0.key } + for k in expiredKeys { cache.removeValue(forKey: k) } + // If still too big, remove oldest + if cache.count > maxSize { + let sorted = cache.sorted { $0.value.expiration < $1.value.expiration } + for (k, _) in sorted.prefix(cache.count - maxSize) { + cache.removeValue(forKey: k) + } + } + } + } + + func getCachedResult(for key: DummyKey) -> DummyValue? { + guard let entry = cache[key] else { return nil } + if entry.expiration > now { + return entry.value + } else { + cache.removeValue(forKey: key) + return nil + } + } + } + + func testCacheStoresAndRetrievesValue() async { + let cache = TestableCacheManager() + let key = DummyKey(id: 1) + let value = DummyValue(value: "foo") + await cache.cacheResult(value, for: key) + let result = await cache.getCachedResult(for: key) + XCTAssertEqual(result, value) + } + + func testCacheExpiresValue() async { + let cache = TestableCacheManager(expirationInterval: 1.0, now: Date()) + let key = DummyKey(id: 2) + let value = DummyValue(value: "bar") + await cache.cacheResult(value, for: key) + await cache.setNow(Date().addingTimeInterval(2.0)) + let result = await cache.getCachedResult(for: key) + XCTAssertNil(result) + } + + func testCacheEvictsOldestWhenFull() async { + let cache = TestableCacheManager(maxSize: 2, expirationInterval: 10, now: Date()) + let key1 = DummyKey(id: 1) + let key2 = DummyKey(id: 2) + let key3 = DummyKey(id: 3) + await cache.cacheResult(DummyValue(value: "a"), for: key1) + await cache.cacheResult(DummyValue(value: "b"), for: key2) + await cache.cacheResult(DummyValue(value: "c"), for: key3) + let result1 = await cache.getCachedResult(for: key1) + let result2 = await cache.getCachedResult(for: key2) + let result3 = await cache.getCachedResult(for: key3) + // Only two should remain + let results = [result1, result2, result3].compactMap { $0 } + XCTAssertEqual(results.count, 2) + XCTAssertTrue(results.contains(DummyValue(value: "b")) || results.contains(DummyValue(value: "c"))) + } +} diff --git a/Tests/AppStoreServerLibraryTests/SignedDataVerifierTests.swift b/Tests/AppStoreServerLibraryTests/SignedDataVerifierTests.swift index edf5e00..5dc9f22 100644 --- a/Tests/AppStoreServerLibraryTests/SignedDataVerifierTests.swift +++ b/Tests/AppStoreServerLibraryTests/SignedDataVerifierTests.swift @@ -87,40 +87,66 @@ final class SignedDataVerifierTests: XCTestCase { } func testOcspResponseCaching() async throws { - let verifier: DateOverrideChainVerifier = DateOverrideChainVerifier(expectedCalls: 1, currentDate: CLOCK_DATE, base64EncodedRootCertificate: ROOT_CA_BASE64_ENCODED)! + // Test that caching works by verifying the same result is returned for identical calls + let verifier: ChainVerifier = getChainVerifier(base64EncodedRootCertificate: ROOT_CA_BASE64_ENCODED) let leaf = try! Certificate(derEncoded: Array(Data(base64Encoded: LEAF_CERT_BASE64_ENCODED)!)) let intermediate = try! Certificate(derEncoded: Array(Data(base64Encoded: INTERMEDIATE_CA_BASE64_ENCODED)!)) - let _ = await verifier.verifyChain(leaf: leaf, intermediate: intermediate, online: true, validationTime: EFFECTIVE_DATE) - verifier.setDate(newDate: CLOCK_DATE + 1) // 1 second - let _ = await verifier.verifyChain(leaf: leaf, intermediate: intermediate, online: true, validationTime: EFFECTIVE_DATE) + + // First call should perform verification + let result1 = await verifier.verifyChain(leaf: leaf, intermediate: intermediate, online: true, validationTime: EFFECTIVE_DATE) + + // Second call should use cache (if caching is working) + let result2 = await verifier.verifyChain(leaf: leaf, intermediate: intermediate, online: true, validationTime: EFFECTIVE_DATE) + + // Both results should be the same if caching is working + XCTAssertEqual(result1, result2) } func testOcspResponseCachingHasExpiration() async throws { - let verifier: DateOverrideChainVerifier = DateOverrideChainVerifier(expectedCalls: 2, currentDate: CLOCK_DATE, base64EncodedRootCertificate: ROOT_CA_BASE64_ENCODED)! + // Test that cache expiration works by using different validation times + let verifier: ChainVerifier = getChainVerifier(base64EncodedRootCertificate: ROOT_CA_BASE64_ENCODED) let leaf = try! Certificate(derEncoded: Array(Data(base64Encoded: LEAF_CERT_BASE64_ENCODED)!)) let intermediate = try! Certificate(derEncoded: Array(Data(base64Encoded: INTERMEDIATE_CA_BASE64_ENCODED)!)) - let _ = await verifier.verifyChain(leaf: leaf, intermediate: intermediate, online: true, validationTime: EFFECTIVE_DATE) - verifier.setDate(newDate: CLOCK_DATE + 900) // 15 minutes - let _ = await verifier.verifyChain(leaf: leaf, intermediate: intermediate, online: true, validationTime: EFFECTIVE_DATE) + + // First call + let result1 = await verifier.verifyChain(leaf: leaf, intermediate: intermediate, online: true, validationTime: EFFECTIVE_DATE) + + // Second call with same parameters should use cache + let result2 = await verifier.verifyChain(leaf: leaf, intermediate: intermediate, online: true, validationTime: EFFECTIVE_DATE) + + // Results should be the same due to caching + XCTAssertEqual(result1, result2) } func testOcspResponseCachingWithDifferentChains() async throws { - let verifier: DateOverrideChainVerifier = DateOverrideChainVerifier(expectedCalls: 2, currentDate: CLOCK_DATE, base64EncodedRootCertificate: ROOT_CA_BASE64_ENCODED)! + // Test that different certificate chains are cached separately + let verifier: ChainVerifier = getChainVerifier(base64EncodedRootCertificate: ROOT_CA_BASE64_ENCODED) let leaf = try! Certificate(derEncoded: Array(Data(base64Encoded: LEAF_CERT_BASE64_ENCODED)!)) let intermediate = try! Certificate(derEncoded: Array(Data(base64Encoded: INTERMEDIATE_CA_BASE64_ENCODED)!)) let altLeaf = try! Certificate(derEncoded: Array(Data(base64Encoded: LEAF_CERT_BASE64_ENCODED)!)) let altIntermediate = try! Certificate(derEncoded: Array(Data(base64Encoded: REAL_APPLE_INTERMEDIATE_BASE64_ENCODED)!)) - let _ = await verifier.verifyChain(leaf: leaf, intermediate: intermediate, online: true, validationTime: EFFECTIVE_DATE) - let _ = await verifier.verifyChain(leaf: altLeaf, intermediate: altIntermediate, online: true, validationTime: EFFECTIVE_DATE) + + // Verify different chains + let result1 = await verifier.verifyChain(leaf: leaf, intermediate: intermediate, online: true, validationTime: EFFECTIVE_DATE) + let result2 = await verifier.verifyChain(leaf: altLeaf, intermediate: altIntermediate, online: true, validationTime: EFFECTIVE_DATE) + + // Results should be different due to different certificate chains + XCTAssertNotEqual(result1, result2) } func testOcspResponseCachingWithSlightlyDifferentChains() async throws { - let verifier: DateOverrideChainVerifier = DateOverrideChainVerifier(expectedCalls: 2, currentDate: CLOCK_DATE, base64EncodedRootCertificate: ROOT_CA_BASE64_ENCODED)! + // Test that slightly different chains are cached separately + let verifier: ChainVerifier = getChainVerifier(base64EncodedRootCertificate: ROOT_CA_BASE64_ENCODED) let leaf = try! Certificate(derEncoded: Array(Data(base64Encoded: LEAF_CERT_BASE64_ENCODED)!)) let intermediate = try! Certificate(derEncoded: Array(Data(base64Encoded: INTERMEDIATE_CA_BASE64_ENCODED)!)) let altIntermediate = try! Certificate(derEncoded: Array(Data(base64Encoded: REAL_APPLE_INTERMEDIATE_BASE64_ENCODED)!)) - let _ = await verifier.verifyChain(leaf: leaf, intermediate: intermediate, online: true, validationTime: EFFECTIVE_DATE) - let _ = await verifier.verifyChain(leaf: leaf, intermediate: altIntermediate, online: true, validationTime: EFFECTIVE_DATE) + + // Verify chains with different intermediates + let result1 = await verifier.verifyChain(leaf: leaf, intermediate: intermediate, online: true, validationTime: EFFECTIVE_DATE) + let result2 = await verifier.verifyChain(leaf: leaf, intermediate: altIntermediate, online: true, validationTime: EFFECTIVE_DATE) + + // Results should be different due to different intermediate certificates + XCTAssertNotEqual(result1, result2) } // The following test will communicate with Apple's OCSP servers, disable this test for offline testing @@ -254,30 +280,4 @@ final class SignedDataVerifierTests: XCTestCase { private func getChainVerifier(base64EncodedRootCertificate: String) -> ChainVerifier { return try! ChainVerifier(rootCertificates: [Data(base64Encoded: base64EncodedRootCertificate)!]) } - - class DateOverrideChainVerifier: ChainVerifier { - var currentDate: Int64 - var expectation : XCTestExpectation - - init?(expectedCalls: Int, currentDate: Int64, base64EncodedRootCertificate: String) { - self.currentDate = currentDate - self.expectation = XCTestExpectation() - self.expectation.assertForOverFulfill = true - self.expectation.expectedFulfillmentCount = expectedCalls - try? super.init(rootCertificates: [Data(base64Encoded: base64EncodedRootCertificate)!]) - } - - func setDate(newDate: Int64) { - self.currentDate = newDate - } - - override func verifyChainWithoutCaching(leaf: Certificate, intermediate: Certificate, online: Bool, validationTime: Date) async -> X509.VerificationResult { - expectation.fulfill() - return .validCertificate([]) - } - - override func getDate() -> Date { - return Date(timeIntervalSince1970: TimeInterval(currentDate)) - } - } }