From 301366f1f17e9da3c12505a440cf8eb5777d07ab Mon Sep 17 00:00:00 2001 From: Maarten Billemont Date: Sat, 12 May 2012 18:31:05 +0200 Subject: [PATCH] Log messages + OTP. [IMPROVED] MP-15: Audit and improve log messages. [ADDED] If an element's counter is 0, generate a time-based OTP instead. The OTP changes every 5 minutes. --- External/Pearl | 2 +- MasterPassword/MPAppDelegate_Key.m | 46 ++++++++++--------- MasterPassword/MPAppDelegate_Store.m | 18 ++++---- MasterPassword/MPElementGeneratedEntity.h | 2 +- MasterPassword/MPElementGeneratedEntity.m | 2 +- MasterPassword/MPTypes.h | 2 +- MasterPassword/MPTypes.m | 26 +++++++---- .../MasterPassword.xcdatamodel/contents | 2 +- MasterPassword/iOS/MPUnlockViewController.m | 10 ++-- 9 files changed, 61 insertions(+), 49 deletions(-) diff --git a/External/Pearl b/External/Pearl index d247edba..bbb92ad1 160000 --- a/External/Pearl +++ b/External/Pearl @@ -1 +1 @@ -Subproject commit d247edba08f70994bb8b0cb50413aedfa963cacd +Subproject commit bbb92ad1957b17d50aaa44e124fb5bacef05da10 diff --git a/MasterPassword/MPAppDelegate_Key.m b/MasterPassword/MPAppDelegate_Key.m index 24393a9d..cc7acd6e 100644 --- a/MasterPassword/MPAppDelegate_Key.m +++ b/MasterPassword/MPAppDelegate_Key.m @@ -38,7 +38,7 @@ static NSDictionary *keyHashQuery() { - (void)forgetKey { - dbg(@"Deleting key and hash from key chain."); + inf(@"Deleting key and hash from keychain."); [PearlKeyChain deleteItemForQuery:keyQuery()]; [PearlKeyChain deleteItemForQuery:keyHashQuery()]; @@ -57,13 +57,12 @@ static NSDictionary *keyHashQuery() { if ([[MPConfig get].saveKey boolValue]) { // Key is stored in keychain. Load it. - dbg(@"Loading key from key chain."); [self updateKey:[PearlKeyChain dataOfItemForQuery:keyQuery()]]; - dbg(@" -> Key %@.", self.key? @"found": @"NOT found"); + inf(@"Looking for key in keychain: %@.", self.key? @"found": @"missing"); } else { // Key should not be stored in keychain. Delete it. if ([PearlKeyChain deleteItemForQuery:keyQuery()] != errSecItemNotFound) - dbg(@"Deleted key from key chain."); + inf(@"Removed key from keychain."); #ifdef TESTFLIGHT_SDK_VERSION [TestFlight passCheckpoint:MPTestFlightCheckpointMPUnstored]; #endif @@ -72,19 +71,18 @@ static NSDictionary *keyHashQuery() { - (BOOL)tryMasterPassword:(NSString *)tryPassword { - NSData *keyHash = [PearlKeyChain dataOfItemForQuery:keyHashQuery()]; - dbg(@"Key hash %@.", keyHash? @"known": @"NOT known"); - if (![tryPassword length]) return NO; NSData *tryKey = keyForPassword(tryPassword); NSData *tryKeyHash = keyHashForKey(tryKey); + NSData *keyHash = [PearlKeyChain dataOfItemForQuery:keyHashQuery()]; + inf(@"Key hash known? %@.", keyHash? @"YES": @"NO"); if (keyHash) // A key hash is known -> a key is set. // Make sure the user's entered key matches it. if (![keyHash isEqual:tryKeyHash]) { - dbg(@"Key phrase hash mismatch. Expected: %@, answer: %@.", keyHash, tryKeyHash); + wrn(@"Key ID mismatch. Expected: %@, answer: %@.", [keyHash encodeHex], [tryKeyHash encodeHex]); #ifdef TESTFLIGHT_SDK_VERSION [TestFlight passCheckpoint:MPTestFlightCheckpointMPMismatch]; @@ -115,24 +113,30 @@ static NSDictionary *keyHashQuery() { self.keyHash = keyHashForKey(self.key); self.keyID = [self.keyHash encodeHex]; - dbg(@"Updating key ID to: %@.", self.keyID); - [PearlKeyChain addOrUpdateItemForQuery:keyHashQuery() - withAttributes:[NSDictionary dictionaryWithObjectsAndKeys: - self.keyHash, (__bridge id)kSecValueData, -#if TARGET_OS_IPHONE - kSecAttrAccessibleWhenUnlocked, (__bridge id)kSecAttrAccessible, -#endif - nil]]; - if ([[MPConfig get].saveKey boolValue]) { - dbg(@"Storing key in key chain."); - [PearlKeyChain addOrUpdateItemForQuery:keyQuery() + NSData *existingKeyHash = [PearlKeyChain dataOfItemForQuery:keyHashQuery()]; + if (![existingKeyHash isEqualToData:self.keyHash]) { + inf(@"Updating key ID in keychain."); + [PearlKeyChain addOrUpdateItemForQuery:keyHashQuery() withAttributes:[NSDictionary dictionaryWithObjectsAndKeys: - self.key, (__bridge id)kSecValueData, + self.keyHash, (__bridge id)kSecValueData, #if TARGET_OS_IPHONE - kSecAttrAccessibleWhenUnlocked, (__bridge id)kSecAttrAccessible, + kSecAttrAccessibleWhenUnlocked, (__bridge id)kSecAttrAccessible, #endif nil]]; } + if ([[MPConfig get].saveKey boolValue]) { + NSData *existingKey = [PearlKeyChain dataOfItemForQuery:keyQuery()]; + if (![existingKey isEqualToData:self.key]) { + inf(@"Updating key in keychain."); + [PearlKeyChain addOrUpdateItemForQuery:keyQuery() + withAttributes:[NSDictionary dictionaryWithObjectsAndKeys: + self.key, (__bridge id)kSecValueData, +#if TARGET_OS_IPHONE + kSecAttrAccessibleWhenUnlocked, (__bridge id)kSecAttrAccessible, +#endif + nil]]; + } + } #ifdef TESTFLIGHT_SDK_VERSION [TestFlight passCheckpoint:MPTestFlightCheckpointSetKey]; diff --git a/MasterPassword/MPAppDelegate_Store.m b/MasterPassword/MPAppDelegate_Store.m index a5eec500..d205cda8 100644 --- a/MasterPassword/MPAppDelegate_Store.m +++ b/MasterPassword/MPAppDelegate_Store.m @@ -120,7 +120,7 @@ static NSDateFormatter *rfc3339DateFormatter = nil; NSError *error = nil; if ([self.managedObjectContext hasChanges]) if (![self.managedObjectContext save:&error]) - err(@"Unresolved error %@", error); + err(@"While saving context: %@", error); }]; } @@ -178,7 +178,7 @@ static NSDateFormatter *rfc3339DateFormatter = nil; - (void)ubiquityStoreManager:(UbiquityStoreManager *)manager log:(NSString *)message { - dbg(@"StoreManager: %@", message); + dbg(@"[StoreManager] %@", message); } - (void)ubiquityStoreManager:(UbiquityStoreManager *)manager didSwitchToiCloud:(BOOL)didSwitch { @@ -192,7 +192,7 @@ static NSDateFormatter *rfc3339DateFormatter = nil; } - (void)ubiquityStoreManager:(UbiquityStoreManager *)manager didEncounterError:(NSError *)error cause:(UbiquityStoreManagerErrorCause)cause context:(id)context { - + #ifdef TESTFLIGHT_SDK_VERSION [TestFlight passCheckpoint:str(@"MPTestFlightCheckpointMPErrorUbiquity_%d", cause)]; #endif @@ -338,11 +338,11 @@ static NSDateFormatter *rfc3339DateFormatter = nil; // Delete existing sites. [elementsToDelete enumerateObjectsUsingBlock:^(id obj, BOOL *stop) { - dbg(@"Deleting: %@", [obj name]); + inf(@"Deleting site: %@, it will be replaced by an imported site.", [obj name]); [self.managedObjectContext deleteObject:obj]; }]; [self saveContext]; - + // Import new sites. for (NSArray *siteElements in importedSiteElements) { NSDate *lastUsed = [rfc3339DateFormatter dateFromString:[siteElements objectAtIndex:0]]; @@ -352,7 +352,7 @@ static NSDateFormatter *rfc3339DateFormatter = nil; NSString *exportContent = [siteElements objectAtIndex:4]; // Create new site. - dbg(@"Creating: name=%@, lastUsed=%@, uses=%d, type=%u, keyID=%@", name, lastUsed, uses, type, keyID); + inf(@"Importing site: name=%@, lastUsed=%@, uses=%d, type=%u, keyID=%@", name, lastUsed, uses, type, keyID); MPElementEntity *element = [NSEntityDescription insertNewObjectForEntityForName:ClassNameFromMPElementType(type) inManagedObjectContext:self.managedObjectContext]; element.name = name; @@ -364,11 +364,11 @@ static NSDateFormatter *rfc3339DateFormatter = nil; [element importContent:exportContent]; } [self saveContext]; - + #ifdef TESTFLIGHT_SDK_VERSION [TestFlight passCheckpoint:MPTestFlightCheckpointSitesImported]; #endif - + return MPImportResultSuccess; } @@ -428,7 +428,7 @@ static NSDateFormatter *rfc3339DateFormatter = nil; #ifdef TESTFLIGHT_SDK_VERSION [TestFlight passCheckpoint:MPTestFlightCheckpointSitesExported]; #endif - + return export; } diff --git a/MasterPassword/MPElementGeneratedEntity.h b/MasterPassword/MPElementGeneratedEntity.h index 832ca61b..1377037d 100644 --- a/MasterPassword/MPElementGeneratedEntity.h +++ b/MasterPassword/MPElementGeneratedEntity.h @@ -13,6 +13,6 @@ @interface MPElementGeneratedEntity : MPElementEntity -@property (nonatomic, assign) int16_t counter; +@property (nonatomic, assign) int32_t counter; @end diff --git a/MasterPassword/MPElementGeneratedEntity.m b/MasterPassword/MPElementGeneratedEntity.m index a4bff017..42be8759 100644 --- a/MasterPassword/MPElementGeneratedEntity.m +++ b/MasterPassword/MPElementGeneratedEntity.m @@ -18,7 +18,7 @@ - (id)content { if (!(self.type & MPElementTypeClassGenerated)) { - err(@"Corrupt element: %@, type: %d, does not match class: %@", self.name, self.type, [self class]); + err(@"Corrupt element: %@, type: %d is not in MPElementTypeClassGenerated", self.name, self.type); return nil; } diff --git a/MasterPassword/MPTypes.h b/MasterPassword/MPTypes.h index d3a5b622..86caf158 100644 --- a/MasterPassword/MPTypes.h +++ b/MasterPassword/MPTypes.h @@ -82,4 +82,4 @@ NSData *keyHashForKey(NSData *key); NSString *NSStringFromMPElementType(MPElementType type); NSString *ClassNameFromMPElementType(MPElementType type); Class ClassFromMPElementType(MPElementType type); -NSString *MPCalculateContent(MPElementType type, NSString *name, NSData *key, int16_t counter); +NSString *MPCalculateContent(MPElementType type, NSString *name, NSData *key, int32_t counter); diff --git a/MasterPassword/MPTypes.m b/MasterPassword/MPTypes.m index 0e46a6cd..714a596f 100644 --- a/MasterPassword/MPTypes.m +++ b/MasterPassword/MPTypes.m @@ -9,6 +9,8 @@ #import "MPTypes.h" #import "MPElementGeneratedEntity.h" #import "MPElementStoredEntity.h" +#include + #define MP_salt nil #define MP_N 16384 @@ -21,7 +23,8 @@ NSData *keyForPassword(NSString *password) { NSData *key = [PearlSCrypt deriveKeyWithLength:MP_dkLen fromPassword:[password dataUsingEncoding:NSUTF8StringEncoding] usingSalt:MP_salt N:MP_N r:MP_r p:MP_p]; - trc(@"password: %@ derives to key: %@", password, key); + + trc(@"Password: %@ derives to key ID: %@", password, [keyHashForKey(key) encodeHex]); return key; } NSData *keyHashForPassword(NSString *password) { @@ -102,32 +105,37 @@ NSString *ClassNameFromMPElementType(MPElementType type) { } static NSDictionary *MPTypes_ciphers = nil; -NSString *MPCalculateContent(MPElementType type, NSString *name, NSData *key, int16_t counter) { +NSString *MPCalculateContent(MPElementType type, NSString *name, NSData *key, int32_t counter) { - if (!name) { - err(@"Missing name."); - return nil; - } if (!(type & MPElementTypeClassGenerated)) { err(@"Incorrect type (is not MPElementTypeClassGenerated): %d, for: %@", type, name); return nil; } + if (!name) { + err(@"Missing name."); + return nil; + } if (!key) { err(@"Key not set."); return nil; } + uint32_t salt = (unsigned)counter; + if (!counter) + // Counter unset, go into OTP mode. + // Get the UNIX timestamp of the start of the interval of 5 minutes that the current time is in. + salt = ((uint32_t)([[NSDate date] timeIntervalSince1970] / 300)) * 300; if (MPTypes_ciphers == nil) MPTypes_ciphers = [NSDictionary dictionaryWithContentsOfURL:[[NSBundle mainBundle] URLForResource:@"ciphers" withExtension:@"plist"]]; // Determine the hash whose bytes will be used for calculating a password: md4(name-key) - uint16_t ncounter = htons(counter); - trc(@"key hash from: %@-%@-%u", name, key, ncounter); + uint32_t nsalt = htonl(salt); + trc(@"key hash from: %@-%@-%u", name, key, nsalt); NSData *keyHash = [[NSData dataByConcatenatingWithDelimitor:'-' datas: [name dataUsingEncoding:NSUTF8StringEncoding], key, - [NSData dataWithBytes:&ncounter length:sizeof(ncounter)], + [NSData dataWithBytes:&nsalt length:sizeof(nsalt)], nil] hashWith:PearlDigestSHA1]; trc(@"key hash is: %@", keyHash); const char *keyBytes = keyHash.bytes; diff --git a/MasterPassword/MasterPassword.xcdatamodeld/MasterPassword.xcdatamodel/contents b/MasterPassword/MasterPassword.xcdatamodeld/MasterPassword.xcdatamodel/contents index 2755e835..3934f1a6 100644 --- a/MasterPassword/MasterPassword.xcdatamodeld/MasterPassword.xcdatamodel/contents +++ b/MasterPassword/MasterPassword.xcdatamodeld/MasterPassword.xcdatamodel/contents @@ -8,7 +8,7 @@ - + diff --git a/MasterPassword/iOS/MPUnlockViewController.m b/MasterPassword/iOS/MPUnlockViewController.m index 292fb0fd..570ef096 100644 --- a/MasterPassword/iOS/MPUnlockViewController.m +++ b/MasterPassword/iOS/MPUnlockViewController.m @@ -98,8 +98,8 @@ typedef enum { [[NSNotificationCenter defaultCenter] addObserverForName:MPNotificationKeyForgotten object:nil queue:nil usingBlock:^(NSNotification *note) { - [self.field becomeFirstResponder]; - }]; + [self.field becomeFirstResponder]; + }]; [super viewDidLoad]; } @@ -200,7 +200,6 @@ typedef enum { - (IBAction)changeMP { - dbg(@"Forgetting key phrase."); [PearlAlert showAlertWithTitle:@"Changing Master Password" message: @"This will allow you to log in with a different master password.\n\n" @@ -210,9 +209,10 @@ typedef enum { @"Your current sites and passwords will then become available again." viewStyle:UIAlertViewStyleDefault tappedButtonBlock:^(UIAlertView *alert, NSInteger buttonIndex) { - if (buttonIndex != [alert cancelButtonIndex]) - [[MPAppDelegate get] forgetKey]; + if (buttonIndex == [alert cancelButtonIndex]) + return; + [[MPAppDelegate get] forgetKey]; [[MPAppDelegate get] loadKey:YES]; [TestFlight passCheckpoint:MPTestFlightCheckpointMPChanged];