From fd35fea8cfb1cc1273e5abce69c97e4175ac7fb1 Mon Sep 17 00:00:00 2001 From: Maarten Billemont Date: Wed, 11 Mar 2015 17:30:34 -0400 Subject: [PATCH] mpw C code is not thread-safe + bad performance long site queries. --- External/Pearl | 2 +- MasterPassword/C/mpw-util.c | 27 ++++---- MasterPassword/ObjC/MPAlgorithmV0.m | 64 ++++++++++++++----- MasterPassword/ObjC/MPAppDelegate_Store.m | 31 +++++---- .../ObjC/Mac/MPPasswordWindowController.m | 23 +++++-- MasterPassword/ObjC/Mac/MPSiteModel.h | 3 +- MasterPassword/ObjC/Mac/MPSiteModel.m | 8 +-- .../ObjC/iOS/MPPasswordsViewController.m | 7 +- Site/mpw-js/js/mpw-js | 2 +- 9 files changed, 111 insertions(+), 56 deletions(-) diff --git a/External/Pearl b/External/Pearl index 08c42ba0..65b4e3d9 160000 --- a/External/Pearl +++ b/External/Pearl @@ -1 +1 @@ -Subproject commit 08c42ba0f96a95703ec67cbf5846bfe6680dd0a6 +Subproject commit 65b4e3d9984d077f66e6ab15f2ffcc4965d07825 diff --git a/MasterPassword/C/mpw-util.c b/MasterPassword/C/mpw-util.c index 347afe70..49236a39 100644 --- a/MasterPassword/C/mpw-util.c +++ b/MasterPassword/C/mpw-util.c @@ -96,25 +96,24 @@ const char *mpw_idForBuf(const void *buf, size_t length) { return mpw_hex( hash, 32 ); } -//static char **mpw_hex_buf = NULL; -//static unsigned int mpw_hex_buf_i = 0; +static char **mpw_hex_buf = NULL; +static unsigned int mpw_hex_buf_i = 0; const char *mpw_hex(const void *buf, size_t length) { // FIXME -// if (!mpw_hex_buf) { -// mpw_hex_buf = malloc( 10 * sizeof( char * ) ); -// for (uint8_t i = 0; i < 10; ++i) -// mpw_hex_buf[i] = NULL; -// } -// mpw_hex_buf_i = (mpw_hex_buf_i + 1) % 10; -// -// mpw_hex_buf[mpw_hex_buf_i] = realloc( mpw_hex_buf[mpw_hex_buf_i], length * 2 + 1 ); -// for (size_t kH = 0; kH < length; kH++) -// sprintf( &(mpw_hex_buf[mpw_hex_buf_i][kH * 2]), "%02X", ((const uint8_t *)buf)[kH] ); + if (!mpw_hex_buf) { + mpw_hex_buf = malloc( 10 * sizeof( char * ) ); + for (uint8_t i = 0; i < 10; ++i) + mpw_hex_buf[i] = NULL; + } + mpw_hex_buf_i = (mpw_hex_buf_i + 1) % 10; -// return mpw_hex_buf[mpw_hex_buf_i]; - return NULL; + mpw_hex_buf[mpw_hex_buf_i] = realloc( mpw_hex_buf[mpw_hex_buf_i], length * 2 + 1 ); + for (size_t kH = 0; kH < length; kH++) + sprintf( &(mpw_hex_buf[mpw_hex_buf_i][kH * 2]), "%02X", ((const uint8_t *)buf)[kH] ); + + return mpw_hex_buf[mpw_hex_buf_i]; } const char *mpw_hex_l(uint32_t number) { diff --git a/MasterPassword/ObjC/MPAlgorithmV0.m b/MasterPassword/ObjC/MPAlgorithmV0.m index bf5a7f46..f25c2c93 100644 --- a/MasterPassword/ObjC/MPAlgorithmV0.m +++ b/MasterPassword/ObjC/MPAlgorithmV0.m @@ -28,8 +28,10 @@ #define CRACKING_PER_SECOND 2495000000UL #define CRACKING_PRICE 350 +NSOperationQueue *_mpwQueue = nil; + @implementation MPAlgorithmV0 { - BN_CTX *ctx; + BN_CTX *_ctx; } - (id)init { @@ -37,15 +39,22 @@ if (!(self = [super init])) return nil; - ctx = BN_CTX_new(); + _ctx = BN_CTX_new(); + + static dispatch_once_t once = 0; + dispatch_once( &once, ^{ + _mpwQueue = [NSOperationQueue new]; + _mpwQueue.maxConcurrentOperationCount = 1; + _mpwQueue.name = @"mpw queue"; + } ); return self; } - (void)dealloc { - BN_CTX_free( ctx ); - ctx = NULL; + BN_CTX_free( _ctx ); + _ctx = NULL; } - (MPAlgorithmVersion)version { @@ -68,6 +77,19 @@ return [(id)other version] == [self version]; } +- (void)mpw_perform:(void ( ^ )(void))operationBlock { + + if ([NSOperationQueue currentQueue] == _mpwQueue) { + operationBlock(); + return; + } + + NSOperation *operation = [NSBlockOperation blockOperationWithBlock:operationBlock]; + if ([operation respondsToSelector:@selector( qualityOfService )]) + operation.qualityOfService = NSQualityOfServiceUserInitiated; + [_mpwQueue addOperations:@[ operation ] waitUntilFinished:YES]; +} + - (BOOL)tryMigrateUser:(MPUserEntity *)user inContext:(NSManagedObjectContext *)moc { NSError *error = nil; @@ -107,12 +129,16 @@ - (NSData *)keyDataForFullName:(NSString *)fullName withMasterPassword:(NSString *)masterPassword { - NSDate *start = [NSDate date]; - uint8_t const *masterKeyBytes = mpw_masterKeyForUser( fullName.UTF8String, masterPassword.UTF8String, [self version] ); - NSData *keyData = [NSData dataWithBytes:masterKeyBytes length:MP_dkLen]; - trc( @"User: %@, password: %@ derives to key ID: %@ (took %0.2fs)", // - fullName, masterPassword, [self keyIDForKeyData:keyData], -[start timeIntervalSinceNow] ); - mpw_free( masterKeyBytes, MP_dkLen ); + __block NSData *keyData; + [self mpw_perform:^{ + NSDate *start = [NSDate date]; + uint8_t const *masterKeyBytes = mpw_masterKeyForUser( fullName.UTF8String, masterPassword.UTF8String, [self version] ); + keyData = [NSData dataWithBytes:masterKeyBytes length:MP_dkLen]; + trc( @"User: %@, password: %@ derives to key ID: %@ (took %0.2fs)", // + fullName, masterPassword, [self keyIDForKeyData:keyData], -[start timeIntervalSinceNow] ); + mpw_free( masterKeyBytes, MP_dkLen ); + }]; + return keyData; } @@ -317,10 +343,13 @@ - (NSString *)generateContentForSiteNamed:(NSString *)name ofType:(MPSiteType)type withCounter:(NSUInteger)counter variant:(MPSiteVariant)variant context:(NSString *)context usingKey:(MPKey *)key { - char const *contentBytes = mpw_passwordForSite( [key keyDataForAlgorithm:self].bytes, - name.UTF8String, type, (uint32_t)counter, variant, context.UTF8String, [self version] ); - NSString *content = [NSString stringWithCString:contentBytes encoding:NSUTF8StringEncoding]; - mpw_freeString( contentBytes ); + __block NSString *content; + [self mpw_perform:^{ + char const *contentBytes = mpw_passwordForSite( [key keyDataForAlgorithm:self].bytes, + name.UTF8String, type, (uint32_t)counter, variant, context.UTF8String, [self version] ); + content = [NSString stringWithCString:contentBytes encoding:NSUTF8StringEncoding]; + mpw_freeString( contentBytes ); + }]; return content; } @@ -382,7 +411,7 @@ [PearlKeyChain deleteItemForQuery:siteQuery]; else [PearlKeyChain addOrUpdateItemForQuery:siteQuery withAttributes:@{ - (__bridge id)kSecValueData : encryptedContent, + (__bridge id)kSecValueData : encryptedContent, #if TARGET_OS_IPHONE (__bridge id)kSecAttrAccessible : (__bridge id)kSecAttrAccessibleWhenUnlockedThisDeviceOnly, #endif @@ -562,7 +591,8 @@ - (void)resolveAnswerForQuestion:(MPSiteQuestionEntity *)question usingKey:(MPKey *)siteKey result:(void ( ^ )(NSString *result))resultBlock { - NSAssert( [[siteKey keyIDForAlgorithm:question.site.user.algorithm] isEqualToData:question.site.user.keyID], @"Site does not belong to current user." ); + NSAssert( [[siteKey keyIDForAlgorithm:question.site.user.algorithm] isEqualToData:question.site.user.keyID], + @"Site does not belong to current user." ); NSString *name = question.site.name; NSString *keyword = question.keyword; id algorithm = nil; @@ -748,7 +778,7 @@ if (strchr( charactersForClass, passwordCharacter )) { // Found class for password character. - characterEntropy = (BN_ULONG)strlen(charactersForClass); + characterEntropy = (BN_ULONG)strlen( charactersForClass ); break; } } diff --git a/MasterPassword/ObjC/MPAppDelegate_Store.m b/MasterPassword/ObjC/MPAppDelegate_Store.m index 730f8622..8eba5670 100644 --- a/MasterPassword/ObjC/MPAppDelegate_Store.m +++ b/MasterPassword/ObjC/MPAppDelegate_Store.m @@ -190,14 +190,14 @@ PearlAssociatedObjectProperty( NSNumber*, StoreCorrupted, storeCorrupted ); // When privateManagedObjectContext is saved, import the changes into mainManagedObjectContext. PearlAddNotificationObserverTo( self.mainManagedObjectContext, NSManagedObjectContextDidSaveNotification, self.privateManagedObjectContext, nil, ^(NSManagedObjectContext *mainManagedObjectContext, NSNotification *note) { - [mainManagedObjectContext performBlock:^{ - @try { - [mainManagedObjectContext mergeChangesFromContextDidSaveNotification:note]; - } - @catch (NSException *exception) { - err( @"While merging changes:\n%@",[exception fullDescription] ); - } - }]; + [mainManagedObjectContext performBlock:^{ + @try { + [mainManagedObjectContext mergeChangesFromContextDidSaveNotification:note]; + } + @catch (NSException *exception) { + err( @"While merging changes:\n%@", [exception fullDescription] ); + } + }]; } ); @@ -821,10 +821,17 @@ PearlAssociatedObjectProperty( NSNumber*, StoreCorrupted, storeCorrupted ); content = [site.algorithm exportPasswordForSite:site usingKey:self.key]; } - [export appendFormat:@"%@ %8ld %8s %25s\t%25s\t%@\n", - [[NSDateFormatter rfc3339DateFormatter] stringFromDate:lastUsed], (long)uses, - [strf( @"%lu:%lu:%lu", (long)type, (long)[algorithm version], (long)counter ) UTF8String], - [(loginName?: @"") UTF8String], [siteName UTF8String], content?: @""]; + NSString *lastUsedExport = [[NSDateFormatter rfc3339DateFormatter] stringFromDate:lastUsed]; + long usesExport = (long)uses; + NSString *typeExport = strf( @"%lu:%lu:%lu", (long)type, (long)[algorithm version], (long)counter ); + NSString *loginNameExport = loginName?: @""; + NSString *contentExport = content?: @""; + [export appendFormat:@"%@ %8ld %8S %25S\t%25S\t%@\n", + lastUsedExport, usesExport, + (const unichar *)[typeExport cStringUsingEncoding:NSUTF16StringEncoding], + (const unichar *)[loginNameExport cStringUsingEncoding:NSUTF16StringEncoding], + (const unichar *)[siteName cStringUsingEncoding:NSUTF16StringEncoding], + contentExport]; } return export; diff --git a/MasterPassword/ObjC/Mac/MPPasswordWindowController.m b/MasterPassword/ObjC/Mac/MPPasswordWindowController.m index 3fd217c9..d80f8cb0 100644 --- a/MasterPassword/ObjC/Mac/MPPasswordWindowController.m +++ b/MasterPassword/ObjC/Mac/MPPasswordWindowController.m @@ -17,6 +17,7 @@ // #import +#import #import "MPPasswordWindowController.h" #import "MPMacAppDelegate.h" #import "MPAppDelegate_Store.h" @@ -518,6 +519,7 @@ - (void)updateSites { + NSAssert( [NSOperationQueue currentQueue] == [NSOperationQueue mainQueue], @"updateSites should be called on the main queue." ); if (![MPMacAppDelegate get].key) { self.sites = nil; return; @@ -530,13 +532,18 @@ } ); NSString *queryString = self.siteField.stringValue; - NSString *queryPattern = [queryString stringByReplacingMatchesOfExpression:fuzzyRE withTemplate:@"*$1*"]; + NSString *queryPattern; + if ([queryString length] < 13) + queryPattern = [queryString stringByReplacingMatchesOfExpression:fuzzyRE withTemplate:@"*$1*"]; + else + // If query is too long, a wildcard per character makes the CoreData fetch take excessively long. + queryPattern = strf( @"*%@*", queryString ); NSMutableArray *fuzzyGroups = [NSMutableArray new]; [fuzzyRE enumerateMatchesInString:queryString options:0 range:NSMakeRange( 0, queryString.length ) usingBlock:^(NSTextCheckingResult *result, NSMatchingFlags flags, BOOL *stop) { [fuzzyGroups addObject:[queryString substringWithRange:result.range]]; }]; - [MPMacAppDelegate managedObjectContextPerformBlockAndWait:^(NSManagedObjectContext *context) { + [MPMacAppDelegate managedObjectContextPerformBlock:^(NSManagedObjectContext *context) { NSFetchRequest *fetchRequest = [NSFetchRequest fetchRequestWithEntityName:NSStringFromClass( [MPSiteEntity class] )]; fetchRequest.sortDescriptors = @[ [[NSSortDescriptor alloc] initWithKey:@"lastUsed" ascending:NO] ]; fetchRequest.predicate = [NSPredicate predicateWithFormat:@"(%@ == '' OR name LIKE[cd] %@) AND user == %@", @@ -555,10 +562,16 @@ [newSites addObject:[[MPSiteModel alloc] initWithEntity:site fuzzyGroups:fuzzyGroups]]; exact |= [site.name isEqualToString:queryString]; } - if (!exact && [queryString length]) - [newSites addObject:[[MPSiteModel alloc] initWithName:queryString]]; + if (!exact && [queryString length]) { + MPUserEntity *activeUser = [[MPAppDelegate_Shared get] activeUserInContext:context]; + [newSites addObject:[[MPSiteModel alloc] initWithName:queryString forUser:activeUser]]; + } + dbg( @"newSites: %@", newSites ); - self.sites = newSites; + if (![newSites isEqualToArray:self.sites]) + PearlMainQueue( ^{ + self.sites = newSites; + } ); }]; } diff --git a/MasterPassword/ObjC/Mac/MPSiteModel.h b/MasterPassword/ObjC/Mac/MPSiteModel.h index 2581dc19..ae29dc04 100644 --- a/MasterPassword/ObjC/Mac/MPSiteModel.h +++ b/MasterPassword/ObjC/Mac/MPSiteModel.h @@ -19,6 +19,7 @@ #import #import "MPSiteEntity.h" #import "MPAlgorithm.h" +#import "MPUserEntity.h" @class MPSiteEntity; @@ -40,7 +41,7 @@ @property (nonatomic, readonly) BOOL transient; - (instancetype)initWithEntity:(MPSiteEntity *)entity fuzzyGroups:(NSArray *)fuzzyGroups; -- (instancetype)initWithName:(NSString *)siteName; +- (instancetype)initWithName:(NSString *)siteName forUser:(MPUserEntity *)user; - (MPSiteEntity *)entityInContext:(NSManagedObjectContext *)moc; - (void)updateContent; diff --git a/MasterPassword/ObjC/Mac/MPSiteModel.m b/MasterPassword/ObjC/Mac/MPSiteModel.m index 56c7e6df..ba9f8ace 100644 --- a/MasterPassword/ObjC/Mac/MPSiteModel.m +++ b/MasterPassword/ObjC/Mac/MPSiteModel.m @@ -39,12 +39,12 @@ return self; } -- (instancetype)initWithName:(NSString *)siteName { +- (instancetype)initWithName:(NSString *)siteName forUser:(MPUserEntity *)user { if (!(self = [super init])) return nil; - [self setTransientSiteName:siteName]; + [self setTransientSiteName:siteName forUser:user]; _initialized = YES; return self; @@ -84,7 +84,7 @@ [self updateContent:entity]; } -- (void)setTransientSiteName:(NSString *)siteName { +- (void)setTransientSiteName:(NSString *)siteName forUser:(MPUserEntity *)user { _entityOID = nil; @@ -97,7 +97,7 @@ self.name = siteName; self.algorithm = MPAlgorithmDefault; self.lastUsed = nil; - self.type = [MPAppDelegate_Shared get].activeUserForMainThread.defaultType; + self.type = user.defaultType; self.typeName = [self.algorithm nameOfType:self.type]; self.uses = @0; self.counter = 1; diff --git a/MasterPassword/ObjC/iOS/MPPasswordsViewController.m b/MasterPassword/ObjC/iOS/MPPasswordsViewController.m index 3a75b112..9929e94a 100644 --- a/MasterPassword/ObjC/iOS/MPPasswordsViewController.m +++ b/MasterPassword/ObjC/iOS/MPPasswordsViewController.m @@ -392,7 +392,12 @@ typedef NS_OPTIONS( NSUInteger, MPPasswordsTips ) { } ); NSString *queryString = self.query; - NSString *queryPattern = [queryString stringByReplacingMatchesOfExpression:fuzzyRE withTemplate:@"*$1*"]; + NSString *queryPattern; + if ([queryString length] < 13) + queryPattern = [queryString stringByReplacingMatchesOfExpression:fuzzyRE withTemplate:@"*$1*"]; + else + // If query is too long, a wildcard per character makes the CoreData fetch take excessively long. + queryPattern = strf( @"*%@*", queryString ); NSMutableArray *fuzzyGroups = [NSMutableArray arrayWithCapacity:[queryString length]]; [fuzzyRE enumerateMatchesInString:queryString options:0 range:NSMakeRange( 0, queryString.length ) usingBlock:^(NSTextCheckingResult *result, NSMatchingFlags flags, BOOL *stop) { diff --git a/Site/mpw-js/js/mpw-js b/Site/mpw-js/js/mpw-js index 442e4189..28a988eb 160000 --- a/Site/mpw-js/js/mpw-js +++ b/Site/mpw-js/js/mpw-js @@ -1 +1 @@ -Subproject commit 442e41896998e06e850b42f9f8ea4b33bb237bf1 +Subproject commit 28a988ebe6d1b8052ecc5190b5f9a1fd658b6cf8