From dd1aa26827a3934acbe715c7c6c7c78f58df8d74 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 23 Jan 2017 17:15:40 -0500 Subject: [PATCH 1/3] Prevent destroying user database after resetting device. // FREEBIE --- src/Storage/TSStorageManager.m | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/Storage/TSStorageManager.m b/src/Storage/TSStorageManager.m index 82bc6729a..534b27b63 100644 --- a/src/Storage/TSStorageManager.m +++ b/src/Storage/TSStorageManager.m @@ -23,6 +23,7 @@ NSString *const TSUIDatabaseConnectionDidUpdateNotification = @"TSUIDatabaseConnectionDidUpdateNotification"; NSString *const TSStorageManagerExceptionNameDatabasePasswordInaccessible = @"TSStorageManagerExceptionNameDatabasePasswordInaccessible"; +NSString *const TSStorageManagerExceptionNameDatabasePasswordInaccessibleWhileBackgrounded = @"TSStorageManagerExceptionNameDatabasePasswordInaccessibleWhileBackgrounded"; NSString *const TSStorageManagerExceptionNameDatabasePasswordUnwritable = @"TSStorageManagerExceptionNameDatabasePasswordUnwritable"; NSString *const TSStorageManagerExceptionNameNoDatabase = @"TSStorageManagerExceptionNameNoDatabase"; @@ -287,6 +288,17 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; return NO; } +- (void)backgroundedAppDatabasePasswordInaccessibleWithError:(NSError *)error +{ + OWSAssert([UIApplication sharedApplication].applicationState == UIApplicationStateBackground); + + // Presumably this happened in response to a push notification. It's possible that the keychain is corrupted + // but it could also just be that the user hasn't yet unlocked their device since our password is + // kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly + [NSException raise:TSStorageManagerExceptionNameDatabasePasswordInaccessibleWhileBackgrounded + format:@"Unable to access database password. No unlock since device restart? Error: %@", error]; +} + - (NSData *)databasePassword { [SAMKeychain setAccessibilityType:kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly]; @@ -296,7 +308,14 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; [SAMKeychain passwordForService:keychainService account:keychainDBPassAccount error:&keyFetchError]; if (keyFetchError) { - // Either this is a new install so there's no existing password to retrieve + if ([UIApplication sharedApplication].applicationState == UIApplicationStateBackground) { + // TODO: Rather than crash here, we should detect the situation earlier + // and exit gracefully - (in the app delegate?). See the ` + // This is a last ditch effort to avoid blowing away the user's database. + [self backgroundedAppDatabasePasswordInaccessibleWithError:keyFetchError]; + } + + // At this point, either this is a new install so there's no existing password to retrieve // or the keychain has become corrupt. Either way, we want to get back to a // "known good state" and behave like a new install. From a45ab9fe40a8b9c3741b2ae70916c0731eae0f3d Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 23 Jan 2017 17:23:12 -0500 Subject: [PATCH 2/3] We need to know if the DB password is accessible *before* we init the db So method can't be *on* the instance. // FREEBIE --- src/Storage/TSStorageManager.h | 16 ++++++++++------ src/Storage/TSStorageManager.m | 7 +++---- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/Storage/TSStorageManager.h b/src/Storage/TSStorageManager.h index 21195cd21..46d0727fe 100644 --- a/src/Storage/TSStorageManager.h +++ b/src/Storage/TSStorageManager.h @@ -1,9 +1,5 @@ // -// TSStorageManager.h -// TextSecureKit -// -// Created by Frederic Jacobs on 27/10/14. -// Copyright (c) 2014 Open Whisper Systems. All rights reserved. +// Copyright (c) 2017 Open Whisper Systems. All rights reserved. // #import "TSStorageKeys.h" @@ -21,9 +17,17 @@ extern NSString *const TSUIDatabaseConnectionDidUpdateNotification; @interface TSStorageManager : NSObject + (instancetype)sharedManager; + +/** + * Returns NO if: + * + * - Keychain is locked because device has just been restarted. + * - Password could not be retrieved because of a keychain error. + */ ++ (BOOL)isDatabasePasswordAccessible; + - (void)setupDatabase; - (void)deleteThreadsAndMessages; -- (BOOL)databasePasswordAccessible; - (void)resetSignalStorage; - (YapDatabase *)database; diff --git a/src/Storage/TSStorageManager.m b/src/Storage/TSStorageManager.m index 534b27b63..3ccdae575 100644 --- a/src/Storage/TSStorageManager.m +++ b/src/Storage/TSStorageManager.m @@ -1,6 +1,4 @@ // -// TSStorageManager.m -// // Copyright (c) 2017 Open Whisper Systems. All rights reserved. // @@ -23,7 +21,8 @@ NSString *const TSUIDatabaseConnectionDidUpdateNotification = @"TSUIDatabaseConnectionDidUpdateNotification"; NSString *const TSStorageManagerExceptionNameDatabasePasswordInaccessible = @"TSStorageManagerExceptionNameDatabasePasswordInaccessible"; -NSString *const TSStorageManagerExceptionNameDatabasePasswordInaccessibleWhileBackgrounded = @"TSStorageManagerExceptionNameDatabasePasswordInaccessibleWhileBackgrounded"; +NSString *const TSStorageManagerExceptionNameDatabasePasswordInaccessibleWhileBackgrounded = + @"TSStorageManagerExceptionNameDatabasePasswordInaccessibleWhileBackgrounded"; NSString *const TSStorageManagerExceptionNameDatabasePasswordUnwritable = @"TSStorageManagerExceptionNameDatabasePasswordUnwritable"; NSString *const TSStorageManagerExceptionNameNoDatabase = @"TSStorageManagerExceptionNameNoDatabase"; @@ -271,7 +270,7 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; return databasePath; } -- (BOOL)databasePasswordAccessible ++ (BOOL)isDatabasePasswordAccessible { [SAMKeychain setAccessibilityType:kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly]; NSError *error; From b5429595acd15f66c76c5d2805bc49cc7cadebd2 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 23 Jan 2017 18:00:34 -0500 Subject: [PATCH 3/3] Better logging per CR // FREEBIE --- src/Storage/TSStorageManager.m | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Storage/TSStorageManager.m b/src/Storage/TSStorageManager.m index 3ccdae575..49bba31ed 100644 --- a/src/Storage/TSStorageManager.m +++ b/src/Storage/TSStorageManager.m @@ -287,7 +287,7 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; return NO; } -- (void)backgroundedAppDatabasePasswordInaccessibleWithError:(NSError *)error +- (void)backgroundedAppDatabasePasswordInaccessibleWithErrorDescription:(NSString *)errorDescription { OWSAssert([UIApplication sharedApplication].applicationState == UIApplicationStateBackground); @@ -295,7 +295,7 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; // but it could also just be that the user hasn't yet unlocked their device since our password is // kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly [NSException raise:TSStorageManagerExceptionNameDatabasePasswordInaccessibleWhileBackgrounded - format:@"Unable to access database password. No unlock since device restart? Error: %@", error]; + format:@"%@", errorDescription]; } - (NSData *)databasePassword @@ -307,11 +307,16 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; [SAMKeychain passwordForService:keychainService account:keychainDBPassAccount error:&keyFetchError]; if (keyFetchError) { + UIApplicationState applicationState = [UIApplication sharedApplication].applicationState; + NSString *errorDescription = [NSString stringWithFormat:@"Database password inaccessible. No unlock since device restart? Error: %@ ApplicationState: %d", keyFetchError, (int)applicationState]; + DDLogError(@"%@ %@", self.tag, errorDescription); + [DDLog flushLog]; + if ([UIApplication sharedApplication].applicationState == UIApplicationStateBackground) { // TODO: Rather than crash here, we should detect the situation earlier // and exit gracefully - (in the app delegate?). See the ` // This is a last ditch effort to avoid blowing away the user's database. - [self backgroundedAppDatabasePasswordInaccessibleWithError:keyFetchError]; + [self backgroundedAppDatabasePasswordInaccessibleWithErrorDescription:errorDescription]; } // At this point, either this is a new install so there's no existing password to retrieve