From 2baf205eef4946b17b884d73a1556097bdf2ccd5 Mon Sep 17 00:00:00 2001 From: Tanner Bennett Date: Sat, 7 Mar 2026 22:56:55 -0600 Subject: [PATCH 1/3] Use serial lock queue --- TrustKit/Pinning/TSKSPKIHashCache.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TrustKit/Pinning/TSKSPKIHashCache.m b/TrustKit/Pinning/TSKSPKIHashCache.m index fac360e..fb1c63d 100644 --- a/TrustKit/Pinning/TSKSPKIHashCache.m +++ b/TrustKit/Pinning/TSKSPKIHashCache.m @@ -168,7 +168,7 @@ - (instancetype)initWithIdentifier:(NSString *)uniqueIdentifier self = [super init]; if (self) { // Initialize our locks - _lockQueue = dispatch_queue_create("TSKSPKIHashLock", DISPATCH_QUEUE_CONCURRENT); + _lockQueue = dispatch_queue_create("TSKSPKIHashLock", DISPATCH_QUEUE_SERIAL); // Ensure a non-nil identifier was provided NSAssert(uniqueIdentifier, @"TSKSPKIHashCache initializer must be passed a unique identifier"); From f97da4d3b255a88c846fb087fcc5a58c517329d6 Mon Sep 17 00:00:00 2001 From: Tanner Bennett Date: Sat, 7 Mar 2026 22:57:15 -0600 Subject: [PATCH 2/3] isProtectedDataAvailable() should block --- TrustKit/Pinning/TSKSPKIHashCache.m | 34 +++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/TrustKit/Pinning/TSKSPKIHashCache.m b/TrustKit/Pinning/TSKSPKIHashCache.m index fb1c63d..970f2dd 100644 --- a/TrustKit/Pinning/TSKSPKIHashCache.m +++ b/TrustKit/Pinning/TSKSPKIHashCache.m @@ -148,8 +148,42 @@ - (SPKICacheDictionnary *)loadSPKICacheFromFileSystem; @implementation TSKSPKIHashCache +/// Blocks to call out to the main thread if necessary static BOOL isProtectedDataAvailable(void) { + if (NSThread.isMainThread) { + return _isProtectedDataAvailable(); + } else { + __block BOOL ret = NO; + dispatch_sync(dispatch_get_main_queue(), ^{ + ret = _isProtectedDataAvailable(); + }); + + return ret; + } +} + +static void isProtectedDataAvailableAsync(dispatch_queue_t callbackQueue, void (^callback)(BOOL available)) +{ + NSCParameterAssert(callbackQueue); + NSCParameterAssert(callback); + + void (^callbackOnQueue)(BOOL) = ^(BOOL available) { + dispatch_async(callbackQueue, ^{ + callback(available); + }); + }; + + if (NSThread.isMainThread) { + callbackOnQueue(_isProtectedDataAvailable()); + } else { + dispatch_async(dispatch_get_main_queue(), ^{ + callbackOnQueue(_isProtectedDataAvailable()); + }); + } +} + +static BOOL _isProtectedDataAvailable(void) { Class uiApplicationClass = objc_getClass("UIApplication"); if (uiApplicationClass) { SEL sharedApplicationSelector = sel_registerName("sharedApplication"); From b715858d21faeb183c35baf4a75e3178aa6c94e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20M=C3=BCcke?= <317346+fabianmuecke@users.noreply.github.com> Date: Mon, 14 Jul 2025 13:02:03 +0200 Subject: [PATCH 3/3] Fix a rare crash in TSKSPKIHashCache Fix a crash in TSKSPKIHashCache caused by concurrent read write operations. Refactor hashSubjectPublicKeyInfoFromCertificate: --- TrustKit/Pinning/TSKSPKIHashCache.m | 56 ++++++++++------------ TrustKitTests/TSKPublicKeyAlgorithmTests.m | 42 ++++++++++++++++ 2 files changed, 66 insertions(+), 32 deletions(-) diff --git a/TrustKit/Pinning/TSKSPKIHashCache.m b/TrustKit/Pinning/TSKSPKIHashCache.m index 970f2dd..a4d121a 100644 --- a/TrustKit/Pinning/TSKSPKIHashCache.m +++ b/TrustKit/Pinning/TSKSPKIHashCache.m @@ -219,16 +219,21 @@ - (instancetype)initWithIdentifier:(NSString *)uniqueIdentifier return self; } -- (NSData *)hashSubjectPublicKeyInfoFromCertificate:(SecCertificateRef)certificate -{ - __block NSData *cachedSubjectPublicKeyInfo; +- (NSData *)hashSubjectPublicKeyInfoFromCertificate:(SecCertificateRef)certificate { + __block NSData *hash = nil; + + dispatch_sync(self.lockQueue, ^{ + hash = [self _hashSubjectPublicKeyInfoFromCertificate:certificate]; + }); + return hash; +} + +- (NSData *)_hashSubjectPublicKeyInfoFromCertificate:(SecCertificateRef)certificate +{ // Have we seen this certificate before? Look for the SPKI in the cache NSData *certificateData = (__bridge_transfer NSData *)(SecCertificateCopyData(certificate)); - - dispatch_sync(_lockQueue, ^{ - cachedSubjectPublicKeyInfo = self->_spkiCache[certificateData]; - }); + NSData *cachedSubjectPublicKeyInfo = self->_spkiCache[certificateData]; if (cachedSubjectPublicKeyInfo) { @@ -289,35 +294,22 @@ - (NSData *)hashSubjectPublicKeyInfoFromCertificate:(SecCertificateRef)certifica // Store the hash in our memory cache - dispatch_barrier_sync(_lockQueue, ^{ - self->_spkiCache[certificateData] = subjectPublicKeyInfoHash; - }); + self->_spkiCache[certificateData] = subjectPublicKeyInfoHash; // Update the cache on the filesystem - if (self.spkiCacheFilename.length > 0) { - - __weak typeof(self) weakSelf = self; - void (^updateCacheBlock)(void) = ^{ - - if (isProtectedDataAvailable()) { - NSData *serializedSpkiCache = [NSKeyedArchiver archivedDataWithRootObject:weakSelf.spkiCache requiringSecureCoding:YES error:nil]; - if ([serializedSpkiCache writeToURL:[weakSelf SPKICachePath] atomically:YES] == NO) { - NSAssert(false, @"Failed to write cache"); - TSKLog(@"Could not persist SPKI cache to the filesystem"); - } - } - else { - TSKLog(@"Protected data not available, skipping SPKI cache persistence"); - } - }; - - if ([NSThread isMainThread]) { - updateCacheBlock(); - } - else { - dispatch_async(dispatch_get_main_queue(), updateCacheBlock); + if (self.spkiCacheFilename.length && isProtectedDataAvailable()) { + NSData *serializedSpkiCache = [NSKeyedArchiver archivedDataWithRootObject:self.spkiCache + requiringSecureCoding:YES + error:nil]; + NSURL *cachePath = [self SPKICachePath]; + if (!cachePath || ![serializedSpkiCache writeToURL:cachePath atomically:YES]) { + NSAssert(false, @"Failed to write cache"); + TSKLog(@"Could not persist SPKI cache to the filesystem"); } } + else if (self.spkiCacheFilename.length) { + TSKLog(@"Protected data not available, skipping SPKI cache persistence"); + } return subjectPublicKeyInfoHash; } diff --git a/TrustKitTests/TSKPublicKeyAlgorithmTests.m b/TrustKitTests/TSKPublicKeyAlgorithmTests.m index 5410f84..cd944ad 100644 --- a/TrustKitTests/TSKPublicKeyAlgorithmTests.m +++ b/TrustKitTests/TSKPublicKeyAlgorithmTests.m @@ -156,4 +156,46 @@ - (void)testSPKICacheThreadSafetyAndProtectedData [self waitForExpectationsWithTimeout:5.0 handler:nil]; } +// This test hardly manages to reproduce the crash, but it does reproduce it sometimes. +- (void)testSPKICacheThreadSafetyAndProtectedDataDoesntCrash +{ + XCTestExpectation *expectation = [self expectationWithDescription:@"Cache operations completed"]; + + id mockApplication = OCMClassMock([UIApplication class]); + OCMStub([mockApplication sharedApplication]).andReturn(mockApplication); + OCMStub([mockApplication isProtectedDataAvailable]).andReturn(YES); + + // Perform multiple cache operations in parallel on a background queue + SecCertificateRef certificate = [TSKCertificateUtils createCertificateFromDer:@"www.globalsign.com"]; + dispatch_group_t group = dispatch_group_create(); + for (int i = 0; i < 100; i++) { + dispatch_group_async(group, dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + [self->spkiCache hashSubjectPublicKeyInfoFromCertificate:certificate]; + }); + } + + dispatch_group_notify(group, dispatch_get_main_queue(), ^{ + + NSDictionary *cache = [self->spkiCache getSubjectPublicKeyInfoHashesCache]; + XCTAssertEqual(cache.count, 1, @"Cache should contain one entry"); + + BOOL yes = YES; + OCMStub([mockApplication isProtectedDataAvailable]).andReturn(YES).andDo(^(NSInvocation *invocation) { + self->spkiCache = nil; + [invocation setReturnValue:(void *)&yes]; + }); + + // Perform one more cache operation to trigger filesystem write + [self->spkiCache hashSubjectPublicKeyInfoFromCertificate:certificate]; + + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ + CFRelease(certificate); + [mockApplication stopMocking]; + [expectation fulfill]; + }); + }); + + [self waitForExpectationsWithTimeout:5.0 handler:nil]; +} + @end