From ec6625b80067afcd596811ffdb35ee9f55b20919 Mon Sep 17 00:00:00 2001 From: Maarten Billemont Date: Tue, 14 Jan 2020 15:21:56 -0500 Subject: [PATCH] Fix internal bugs. Pass masterKey data safely by ensuring the NSData holder is owned. nameOfType: threw an unrecougnized-type error always, including for recougnized types. Swizzling broke when triggered on multiple levels of the hierarchy. --- platform-darwin/External/Pearl | 2 +- platform-darwin/Source/MPAlgorithm.h | 2 +- platform-darwin/Source/MPAlgorithmV0.m | 24 ++++++++++++---------- platform-darwin/Source/MPAppDelegate_Key.m | 4 ++-- platform-darwin/Source/MPKey.h | 2 +- platform-darwin/Source/MPKey.m | 4 ++-- 6 files changed, 20 insertions(+), 18 deletions(-) diff --git a/platform-darwin/External/Pearl b/platform-darwin/External/Pearl index 4eb904f9..a0b8d6fe 160000 --- a/platform-darwin/External/Pearl +++ b/platform-darwin/External/Pearl @@ -1 +1 @@ -Subproject commit 4eb904f9b4c318da36b5071d57d137b63f8ef144 +Subproject commit a0b8d6fe4ef563579c97c70905fe0b6806e8d787 diff --git a/platform-darwin/Source/MPAlgorithm.h b/platform-darwin/Source/MPAlgorithm.h index 8b2c72f8..db8e674c 100644 --- a/platform-darwin/Source/MPAlgorithm.h +++ b/platform-darwin/Source/MPAlgorithm.h @@ -49,7 +49,7 @@ NSString *NSStringFromTimeToCrack(TimeToCrack timeToCrack); - (BOOL)tryMigrateUser:(MPUserEntity *)user inContext:(NSManagedObjectContext *)moc; - (BOOL)tryMigrateSite:(MPSiteEntity *)site explicit:(BOOL)explicit; -- (NSData *)keyIDForKey:(MPMasterKey)masterKey; +- (NSData *)keyIDForKey:(NSData *)masterKey; - (NSData *)keyDataForFullName:(NSString *)fullName withMasterPassword:(NSString *)masterPassword; - (NSString *)nameOfType:(MPResultType)type; diff --git a/platform-darwin/Source/MPAlgorithmV0.m b/platform-darwin/Source/MPAlgorithmV0.m index 4dca1b5e..b35a0999 100644 --- a/platform-darwin/Source/MPAlgorithmV0.m +++ b/platform-darwin/Source/MPAlgorithmV0.m @@ -132,7 +132,7 @@ static NSOperationQueue *_mpwQueue = nil; if (masterKey) { keyData = [NSData dataWithBytes:masterKey length:MPMasterKeySize]; trc( @"User: %@, password: %@ derives to key ID: %@ (took %0.2fs)", // - fullName, masterPassword, [self keyIDForKey:masterKey], -[start timeIntervalSinceNow] ); + fullName, masterPassword, [self keyIDForKey:keyData], -[start timeIntervalSinceNow] ); mpw_free( &masterKey, MPMasterKeySize ); } }]; @@ -140,9 +140,9 @@ static NSOperationQueue *_mpwQueue = nil; return keyData; } -- (NSData *)keyIDForKey:(MPMasterKey)masterKey { +- (NSData *)keyIDForKey:(NSData *)masterKey { - return [[NSData dataWithBytesNoCopy:(void *)masterKey length:MPMasterKeySize] hashWith:PearlHashSHA256]; + return [masterKey hashWith:PearlHashSHA256]; } - (NSString *)nameOfType:(MPResultType)type { @@ -364,8 +364,9 @@ static NSOperationQueue *_mpwQueue = nil; __block NSString *result = nil; [self mpw_perform:^{ - char const *resultBytes = mpw_siteResult( [key keyForAlgorithm:self], - name.UTF8String, counter, purpose, context.UTF8String, type, parameter.UTF8String, [self version] ); + NSData *masterKey = [key keyForAlgorithm:self]; + char const *resultBytes = mpw_siteResult( masterKey.bytes, name.UTF8String, + counter, purpose, context.UTF8String, type, parameter.UTF8String, [self version] ); if (resultBytes) { result = [NSString stringWithCString:resultBytes encoding:NSUTF8StringEncoding]; mpw_free_string( &resultBytes ); @@ -392,7 +393,8 @@ static NSOperationQueue *_mpwQueue = nil; __block NSData *state = nil; if (plainText) [self mpw_perform:^{ - char const *stateBytes = mpw_siteState( [key keyForAlgorithm:self], site.name.UTF8String, + NSData *masterKey = [key keyForAlgorithm:self]; + char const *stateBytes = mpw_siteState( masterKey.bytes, site.name.UTF8String, MPCounterValueInitial, MPKeyPurposeAuthentication, NULL, site.type, plainText.UTF8String, [self version] ); if (stateBytes) { state = [[NSString stringWithCString:stateBytes encoding:NSUTF8StringEncoding] decodeBase64]; @@ -499,7 +501,7 @@ static NSOperationQueue *_mpwQueue = nil; if (![site isKindOfClass:[MPGeneratedSiteEntity class]]) { wrn( @"Site with generated type %lu is not an MPGeneratedSiteEntity, but a %@.", (long)site.type, [site class] ); - break; + return; } MPCounterValue counter = ((MPGeneratedSiteEntity *)site).counter; @@ -507,7 +509,7 @@ static NSOperationQueue *_mpwQueue = nil; PearlNotMainQueue( ^{ resultBlock( [algorithm mpwTemplateForSiteNamed:name ofType:type withCounter:counter usingKey:key] ); } ); - break; + return; } case MPResultTypeStatefulPersonal: @@ -515,7 +517,7 @@ static NSOperationQueue *_mpwQueue = nil; if (![site isKindOfClass:[MPStoredSiteEntity class]]) { wrn( @"Site with stored type %lu is not an MPStoredSiteEntity, but a %@.", (long)site.type, [site class] ); - break; + return; } NSDictionary *siteQuery = [self queryForSite:site]; @@ -527,11 +529,11 @@ static NSOperationQueue *_mpwQueue = nil; withCounter:MPCounterValueInitial variant:MPKeyPurposeAuthentication context:nil usingKey:key] ); } ); - break; + return; } case MPResultTypeDeriveKey: - break; + return; } Throw( @"Type not supported: %lu", (long)type ); diff --git a/platform-darwin/Source/MPAppDelegate_Key.m b/platform-darwin/Source/MPAppDelegate_Key.m index 305698e3..05aeb88d 100644 --- a/platform-darwin/Source/MPAppDelegate_Key.m +++ b/platform-darwin/Source/MPAppDelegate_Key.m @@ -95,13 +95,13 @@ - (void)storeSavedKeyFor:(MPUserEntity *)user { if (user.saveKey) { - MPMasterKey masterKey = [self.key keyForAlgorithm:user.algorithm]; + NSData *masterKey = [self.key keyForAlgorithm:user.algorithm]; if (masterKey) { [self forgetSavedKeyFor:user]; inf( @"Saving key in keychain for user: %@", user.userID ); [PearlKeyChain addOrUpdateItemForQuery:[self createKeyQueryforUser:user origin:nil] withAttributes:@{ - (__bridge id)kSecValueData: [NSData dataWithBytesNoCopy:(void *)masterKey length:MPMasterKeySize] + (__bridge id)kSecValueData: masterKey }]; } } diff --git a/platform-darwin/Source/MPKey.h b/platform-darwin/Source/MPKey.h index 15d26617..7bca7fdd 100644 --- a/platform-darwin/Source/MPKey.h +++ b/platform-darwin/Source/MPKey.h @@ -38,7 +38,7 @@ typedef NS_ENUM( NSUInteger, MPKeyOrigin ) { keyOrigin:(MPKeyOrigin)origin; - (NSData *)keyIDForAlgorithm:(id)algorithm; -- (MPMasterKey)keyForAlgorithm:(id)algorithm; +- (NSData *)keyForAlgorithm:(id)algorithm; - (BOOL)isEqualToKey:(MPKey *)key; diff --git a/platform-darwin/Source/MPKey.m b/platform-darwin/Source/MPKey.m index af9b0753..5a36915b 100644 --- a/platform-darwin/Source/MPKey.m +++ b/platform-darwin/Source/MPKey.m @@ -56,7 +56,7 @@ return [algorithm keyIDForKey:[self keyForAlgorithm:algorithm]]; } -- (MPMasterKey)keyForAlgorithm:(id)algorithm { +- (NSData *)keyForAlgorithm:(id)algorithm { @synchronized (self) { NSData *keyData = [self.keyCache objectForKey:algorithm]; @@ -66,7 +66,7 @@ [self.keyCache setObject:keyData forKey:algorithm]; } - return keyData.length == MPMasterKeySize? keyData.bytes: NULL; + return keyData.length == MPMasterKeySize? keyData: NULL; } }