diff --git a/Podfile.lock b/Podfile.lock index e2c1142f2..c2906f6ef 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -193,7 +193,7 @@ CHECKOUT OPTIONS: :commit: d5c2bec :git: https://github.com/sqlcipher/sqlcipher.git YapDatabase: - :commit: eaff655ebc774105e83f835ead71f8b7a02e4ac1 + :commit: bdd7409de45f9e38b9144adba3b38d74ca48ea77 :git: https://github.com/WhisperSystems/YapDatabase.git SPEC CHECKSUMS: diff --git a/Signal.xcodeproj/xcshareddata/xcbaselines/D221A0A9169C9E5F00537ABF.xcbaseline/8A553EB1-B9DF-4DDE-8F93-10474ECF05C2.plist b/Signal.xcodeproj/xcshareddata/xcbaselines/D221A0A9169C9E5F00537ABF.xcbaseline/8A553EB1-B9DF-4DDE-8F93-10474ECF05C2.plist new file mode 100644 index 000000000..39650aa08 --- /dev/null +++ b/Signal.xcodeproj/xcshareddata/xcbaselines/D221A0A9169C9E5F00537ABF.xcbaseline/8A553EB1-B9DF-4DDE-8F93-10474ECF05C2.plist @@ -0,0 +1,52 @@ + + + + + classNames + + OWSDatabaseConverterTest + + testGranularKeySpecFetchingStrategy + + com.apple.XCTPerformanceMetric_WallClockTime + + baselineAverage + 0.039171 + baselineIntegrationDisplayName + Local Baseline + + + testGranularPassphraseFetchingStrategy + + com.apple.XCTPerformanceMetric_WallClockTime + + baselineAverage + 0.22846 + baselineIntegrationDisplayName + Local Baseline + + + testWideKeyFetchingStrategy + + com.apple.XCTPerformanceMetric_WallClockTime + + baselineAverage + 0.039649 + baselineIntegrationDisplayName + Local Baseline + + + testWidePassphraseFetchingStrategy + + com.apple.XCTPerformanceMetric_WallClockTime + + baselineAverage + 0.21819 + baselineIntegrationDisplayName + Local Baseline + + + + + + diff --git a/Signal.xcodeproj/xcshareddata/xcbaselines/D221A0A9169C9E5F00537ABF.xcbaseline/Info.plist b/Signal.xcodeproj/xcshareddata/xcbaselines/D221A0A9169C9E5F00537ABF.xcbaseline/Info.plist new file mode 100644 index 000000000..78d6282c7 --- /dev/null +++ b/Signal.xcodeproj/xcshareddata/xcbaselines/D221A0A9169C9E5F00537ABF.xcbaseline/Info.plist @@ -0,0 +1,40 @@ + + + + + runDestinationsByUUID + + 8A553EB1-B9DF-4DDE-8F93-10474ECF05C2 + + localComputer + + busSpeedInMHz + 100 + cpuCount + 1 + cpuKind + Intel Core i7 + cpuSpeedInMHz + 2900 + logicalCPUCoresPerPackage + 8 + modelCode + MacBookPro13,3 + physicalCPUCoresPerPackage + 4 + platformIdentifier + com.apple.platform.macosx + + targetArchitecture + x86_64 + targetDevice + + modelCode + iPhone8,1 + platformIdentifier + com.apple.platform.iphonesimulator + + + + + diff --git a/Signal/src/AppDelegate.m b/Signal/src/AppDelegate.m index 0f8d406d6..4bbd3ca9f 100644 --- a/Signal/src/AppDelegate.m +++ b/Signal/src/AppDelegate.m @@ -251,28 +251,40 @@ static NSString *const kURLHostVerifyPrefix = @"verify"; - (nullable NSError *)convertDatabaseIfNecessary { NSString *databaseFilePath = [TSStorageManager legacyDatabaseFilePath]; + if (![[NSFileManager defaultManager] fileExistsAtPath:databaseFilePath]) { + DDLogVerbose(@"%@ no legacy database file found", self.logTag); + return nil; + } NSError *error; - NSData *_Nullable databasePassword = [OWSStorage tryToLoadDatabasePassword:&error]; + NSData *_Nullable databasePassword = [OWSStorage tryToLoadDatabaseLegacyPassphrase:&error]; if (!databasePassword || error) { return (error ?: OWSErrorWithCodeDescription( OWSErrorCodeDatabaseConversionFatalError, @"Failed to load database password")); } - YapDatabaseSaltBlock saltBlock = ^(NSData *saltData) { + YapRecordDatabaseSaltBlock recordSaltBlock = ^(NSData *saltData) { DDLogVerbose(@"%@ saltData: %@", self.logTag, saltData.hexadecimalString); - [OWSStorage storeDatabaseSalt:saltData]; - }; - YapDatabaseKeySpecBlock keySpecBlock = ^(NSData *keySpecData) { - DDLogVerbose(@"%@ keySpecData: %@", self.logTag, keySpecData.hexadecimalString); - [OWSStorage storeDatabaseKeySpec:keySpecData]; + + // Derive and store the raw cipher key spec, to avoid the ongoing tax of future KDF + NSData *_Nullable keySpecData = + [YapDatabaseCryptoUtils deriveDatabaseKeySpecForPassword:databasePassword saltData:saltData]; + + if (!keySpecData) { + DDLogError(@"%@ Failed to derive key spec.", self.logTag); + return NO; + } + + [OWSStorage storeDatabaseCipherKeySpec:keySpecData]; + [OWSStorage removeLegacyPassphrase]; + + return YES; }; return [YapDatabaseCryptoUtils convertDatabaseIfNecessary:databaseFilePath databasePassword:databasePassword - saltBlock:saltBlock - keySpecBlock:keySpecBlock]; + recordSaltBlock:recordSaltBlock]; } - (void)startupLogging diff --git a/Signal/test/util/OWSDatabaseConverterTest.m b/Signal/test/util/OWSDatabaseConverterTest.m index 45e4fd27e..2f9778e0f 100644 --- a/Signal/test/util/OWSDatabaseConverterTest.m +++ b/Signal/test/util/OWSDatabaseConverterTest.m @@ -17,6 +17,8 @@ NS_ASSUME_NONNULL_BEGIN @interface OWSStorage (OWSDatabaseConverterTest) + (YapDatabaseDeserializer)logOnFailureDeserializer; ++ (void)storeKeyChainValue:(NSData *)data keychainKey:(NSString *)keychainKey; ++ (nullable NSData *)tryToLoadKeyChainValue:(NSString *)keychainKey errorHandle:(NSError **)errorHandle; @end @@ -298,23 +300,21 @@ NS_ASSUME_NONNULL_BEGIN XCTAssertTrue([YapDatabaseCryptoUtils doesDatabaseNeedToBeConverted:databaseFilePath]); __block NSData *_Nullable databaseSalt = nil; - YapDatabaseSaltBlock saltBlock = ^(NSData *saltData) { + __block NSData *_Nullable databaseKeySpec = nil; + YapRecordDatabaseSaltBlock recordSaltBlock = ^(NSData *saltData) { OWSAssert(!databaseSalt); OWSAssert(saltData); databaseSalt = saltData; - }; - __block NSData *_Nullable databaseKeySpec = nil; - YapDatabaseKeySpecBlock keySpecBlock = ^(NSData *keySpecData) { - OWSAssert(!databaseKeySpec); - OWSAssert(keySpecData); + databaseKeySpec = [YapDatabaseCryptoUtils deriveDatabaseKeySpecForPassword:databasePassword saltData:saltData]; + XCTAssert(databaseKeySpec.length == kSQLCipherKeySpecLength); - databaseKeySpec = keySpecData; + return YES; }; + NSError *_Nullable error = [YapDatabaseCryptoUtils convertDatabaseIfNecessary:databaseFilePath databasePassword:databasePassword - saltBlock:saltBlock - keySpecBlock:keySpecBlock]; + recordSaltBlock:recordSaltBlock]; if (error) { DDLogError(@"%s error: %@", __PRETTY_FUNCTION__, error); } @@ -340,23 +340,22 @@ NS_ASSUME_NONNULL_BEGIN XCTAssertTrue([YapDatabaseCryptoUtils doesDatabaseNeedToBeConverted:databaseFilePath]); __block NSData *_Nullable databaseSalt = nil; - YapDatabaseSaltBlock saltBlock = ^(NSData *saltData) { + + __block NSData *_Nullable databaseKeySpec = nil; + YapRecordDatabaseSaltBlock recordSaltBlock = ^(NSData *saltData) { OWSAssert(!databaseSalt); OWSAssert(saltData); databaseSalt = saltData; + databaseKeySpec = [YapDatabaseCryptoUtils deriveDatabaseKeySpecForPassword:databasePassword saltData:saltData]; + XCTAssert(databaseKeySpec.length == kSQLCipherKeySpecLength); + + return YES; }; - __block NSData *_Nullable databaseKeySpec = nil; - YapDatabaseKeySpecBlock keySpecBlock = ^(NSData *keySpecData) { - OWSAssert(!databaseKeySpec); - OWSAssert(keySpecData); - - databaseKeySpec = keySpecData; - }; + NSError *_Nullable error = [YapDatabaseCryptoUtils convertDatabaseIfNecessary:databaseFilePath databasePassword:databasePassword - saltBlock:saltBlock - keySpecBlock:keySpecBlock]; + recordSaltBlock:recordSaltBlock]; if (error) { DDLogError(@"%s error: %@", __PRETTY_FUNCTION__, error); } @@ -374,6 +373,40 @@ NS_ASSUME_NONNULL_BEGIN XCTAssertTrue(isValid); } +// If we fail to record the salt for some reason, we'll be unable to re-open the database +// halt the conversion in hopes that either the failure is intermittent or we can push out +// a patch to fix the problem without having lost the user's DB. +- (void)testDatabaseConversionDoesNotProceedWhenRecordingSaltFails +{ + NSData *databasePassword = [self randomDatabasePassword]; + NSString *_Nullable databaseFilePath = [self createUnconvertedDatabase:databasePassword]; + XCTAssertTrue([YapDatabaseCryptoUtils doesDatabaseNeedToBeConverted:databaseFilePath]); + + __block NSData *_Nullable databaseSalt = nil; + + __block NSData *_Nullable databaseKeySpec = nil; + YapRecordDatabaseSaltBlock recordSaltBlock = ^(NSData *saltData) { + OWSAssert(!databaseSalt); + OWSAssert(saltData); + + // Simulate a failure to record the new salt, e.g. if KDF returns nil + return NO; + }; + + NSError *_Nullable error = [YapDatabaseCryptoUtils convertDatabaseIfNecessary:databaseFilePath + databasePassword:databasePassword + recordSaltBlock:recordSaltBlock]; + + XCTAssertNotNil(error); + XCTAssertTrue([YapDatabaseCryptoUtils doesDatabaseNeedToBeConverted:databaseFilePath]); + + BOOL isValid = [self verifyTestDatabase:databaseFilePath + databasePassword:databasePassword + databaseSalt:nil + databaseKeySpec:databaseKeySpec]; + XCTAssertTrue(isValid); +} + // Verifies that legacy users with non-converted databases can convert. - (void)testDatabaseConversionPerformance_WithKeyspec { @@ -382,7 +415,7 @@ NS_ASSUME_NONNULL_BEGIN const int kItemCount = 50 * 1000; - // Create an populate the unconverted database. + // Create and populate an unconverted database. [self openYapDatabase:databaseFilePath databasePassword:databasePassword databaseSalt:nil @@ -391,7 +424,8 @@ NS_ASSUME_NONNULL_BEGIN YapDatabaseConnection *dbConnection = database.newConnection; [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) { for (int i = 0; i < kItemCount; i++) { - [transaction setObject:@(i) forKey:@"test_key_name" inCollection:@"test_collection_name"]; + NSString *key = [NSString stringWithFormat:@"key-%d", i]; + [transaction setObject:@"test-object" forKey:key inCollection:@"test_collection_name"]; } }]; }]; @@ -399,23 +433,21 @@ NS_ASSUME_NONNULL_BEGIN XCTAssertTrue([YapDatabaseCryptoUtils doesDatabaseNeedToBeConverted:databaseFilePath]); __block NSData *_Nullable databaseSalt = nil; - YapDatabaseSaltBlock saltBlock = ^(NSData *saltData) { + __block NSData *_Nullable databaseKeySpec = nil; + YapRecordDatabaseSaltBlock recordSaltBlock = ^(NSData *saltData) { OWSAssert(!databaseSalt); OWSAssert(saltData); databaseSalt = saltData; - }; - __block NSData *_Nullable databaseKeySpec = nil; - YapDatabaseKeySpecBlock keySpecBlock = ^(NSData *keySpecData) { - OWSAssert(!databaseKeySpec); - OWSAssert(keySpecData); + databaseKeySpec = [YapDatabaseCryptoUtils deriveDatabaseKeySpecForPassword:databasePassword saltData:saltData]; + XCTAssert(databaseKeySpec.length == kSQLCipherKeySpecLength); - databaseKeySpec = keySpecData; + return YES; }; + NSError *_Nullable error = [YapDatabaseCryptoUtils convertDatabaseIfNecessary:databaseFilePath databasePassword:databasePassword - saltBlock:saltBlock - keySpecBlock:keySpecBlock]; + recordSaltBlock:recordSaltBlock]; if (error) { DDLogError(@"%s error: %@", __PRETTY_FUNCTION__, error); } @@ -429,9 +461,9 @@ NS_ASSUME_NONNULL_BEGIN // Verify the contents of the unconverted database. __block BOOL isValid = NO; [self openYapDatabase:databaseFilePath - databasePassword:databasePassword + databasePassword:nil databaseSalt:nil - databaseKeySpec:nil + databaseKeySpec:databaseKeySpec databaseBlock:^(YapDatabase *database) { YapDatabaseConnection *dbConnection = database.newConnection; isValid = [dbConnection numberOfKeysInCollection:@"test_collection_name"] == kItemCount; @@ -449,21 +481,16 @@ NS_ASSUME_NONNULL_BEGIN [self createDatabase:databasePassword databaseSalt:databaseSalt databaseKeySpec:databaseKeySpec]; XCTAssertFalse([YapDatabaseCryptoUtils doesDatabaseNeedToBeConverted:databaseFilePath]); - YapDatabaseSaltBlock saltBlock = ^(NSData *saltData) { + YapRecordDatabaseSaltBlock recordSaltBlock = ^(NSData *saltData) { OWSAssert(saltData); XCTFail(@"%s No conversion should be necessary", __PRETTY_FUNCTION__); - }; - YapDatabaseKeySpecBlock keySpecBlock = ^(NSData *keySpecData) { - OWSAssert(keySpecData); - - XCTFail(@"%s No conversion should be necessary", __PRETTY_FUNCTION__); + return NO; }; NSError *_Nullable error = [YapDatabaseCryptoUtils convertDatabaseIfNecessary:databaseFilePath databasePassword:databasePassword - saltBlock:saltBlock - keySpecBlock:keySpecBlock]; + recordSaltBlock:recordSaltBlock]; if (error) { DDLogError(@"%s error: %@", __PRETTY_FUNCTION__, error); } @@ -487,21 +514,16 @@ NS_ASSUME_NONNULL_BEGIN [self createDatabase:databasePassword databaseSalt:databaseSalt databaseKeySpec:databaseKeySpec]; XCTAssertFalse([YapDatabaseCryptoUtils doesDatabaseNeedToBeConverted:databaseFilePath]); - YapDatabaseSaltBlock saltBlock = ^(NSData *saltData) { + YapRecordDatabaseSaltBlock recordSaltBlock = ^(NSData *saltData) { OWSAssert(saltData); XCTFail(@"%s No conversion should be necessary", __PRETTY_FUNCTION__); - }; - YapDatabaseKeySpecBlock keySpecBlock = ^(NSData *keySpecData) { - OWSAssert(keySpecData); - - XCTFail(@"%s No conversion should be necessary", __PRETTY_FUNCTION__); + return NO; }; NSError *_Nullable error = [YapDatabaseCryptoUtils convertDatabaseIfNecessary:databaseFilePath databasePassword:databasePassword - saltBlock:saltBlock - keySpecBlock:keySpecBlock]; + recordSaltBlock:recordSaltBlock]; if (error) { DDLogError(@"%s error: %@", __PRETTY_FUNCTION__, error); } @@ -929,6 +951,199 @@ NS_ASSUME_NONNULL_BEGIN DDLogInfo(@"%@: %@", label, output); } + +#pragma mark - keychain strategy benchmarks + + +- (void)verifyTestDatabase:(NSString *)databaseFilePath + databaseKeySpecBlock:(NSData *_Nullable (^_Nullable)(void))databaseKeySpecBlock + databasePasswordBlock:(NSData *_Nullable (^_Nullable)(void))databasePasswordBlock + databaseSaltBlock:(NSData *_Nullable (^_Nullable)(void))databaseSaltBlock +{ + NSData *_Nullable databaseKeySpec = databaseKeySpecBlock ? databaseKeySpecBlock() : nil; + NSData *_Nullable databasePassword = databasePasswordBlock ? databasePasswordBlock() : nil; + NSData *_Nullable databaseSalt = databaseSaltBlock ? databaseSaltBlock() : nil; + + [self verifyTestDatabase:databaseFilePath + databasePassword:databasePassword + databaseSalt:databaseSalt + databaseKeySpec:databaseKeySpec]; +} + +- (void)createTestDatabase:(NSString *)databaseFilePath + databaseKeySpecBlock:(NSData *_Nullable (^_Nullable)(void))databaseKeySpecBlock + databasePasswordBlock:(NSData *_Nullable (^_Nullable)(void))databasePasswordBlock + databaseSaltBlock:(NSData *_Nullable (^_Nullable)(void))databaseSaltBlock +{ + NSData *_Nullable databaseKeySpec = databaseKeySpecBlock ? databaseKeySpecBlock() : nil; + NSData *_Nullable databasePassword = databasePasswordBlock ? databasePasswordBlock() : nil; + NSData *_Nullable databaseSalt = databaseSaltBlock ? databaseSaltBlock() : nil; + + [self createTestDatabase:databaseFilePath + databasePassword:databasePassword + databaseSalt:databaseSalt + databaseKeySpec:databaseKeySpec]; +} + +- (void)storeTestPasswordInKeychain:(NSData *)password +{ + // legacy password length + OWSAssert(password.length == 30); + [OWSStorage storeKeyChainValue:password keychainKey:@"_OWSTestingPassword"]; +} + +- (nullable NSData *)fetchTestPasswordFromKeychain +{ + NSError *error; + NSData *password = [OWSStorage tryToLoadKeyChainValue:@"_OWSTestingPassword" errorHandle:&error]; + OWSAssert(password); + OWSAssert(!error); + // legacy password length + OWSAssert(password.length == 30); + + return password; +} + +- (void)storeTestSaltInKeychain:(NSData *)salt +{ + OWSAssert(salt.length == kSQLCipherSaltLength); + [OWSStorage storeKeyChainValue:salt keychainKey:@"_OWSTestingSalt"]; +} + +- (nullable NSData *)fetchTestSaltFromKeychain +{ + NSError *error; + NSData *salt = [OWSStorage tryToLoadKeyChainValue:@"_OWSTestingSalt" errorHandle:&error]; + OWSAssert(salt); + OWSAssert(!error); + OWSAssert(salt.length == kSQLCipherSaltLength); + return salt; +} + +- (void)storeTestKeySpecInKeychain:(NSData *)keySpec +{ + OWSAssert(keySpec.length == kSQLCipherKeySpecLength); + [OWSStorage storeKeyChainValue:keySpec keychainKey:@"_OWSTestingKeySpec"]; +} + +- (nullable NSData *)fetchTestKeySpecFromKeychain +{ + NSError *error; + NSData *keySpec = [OWSStorage tryToLoadKeyChainValue:@"_OWSTestingKeySpec" errorHandle:&error]; + OWSAssert(keySpec); + OWSAssert(!error); + OWSAssert(keySpec.length == kSQLCipherKeySpecLength); + + return keySpec; +} + +- (void)testWidePassphraseFetchingStrategy +{ + NSData *password = [self randomDatabasePassword]; + NSData *salt = [self randomDatabaseSalt]; + + [self measureBlock:^{ + NSString *databaseFilePath = [self createTempDatabaseFilePath]; + + [self createTestDatabase:databaseFilePath + databaseKeySpecBlock:nil + databasePasswordBlock:^() { + return password; + } + databaseSaltBlock:^() { + return salt; + }]; + + [self verifyTestDatabase:databaseFilePath + databaseKeySpecBlock:nil + databasePasswordBlock:^() { + return password; + } + databaseSaltBlock:^() { + return salt; + }]; + }]; +} + +- (void)testGranularPassphraseFetchingStrategy +{ + NSData *password = [self randomDatabasePassword]; + NSData *salt = [self randomDatabaseSalt]; + [self storeTestPasswordInKeychain:password]; + [self storeTestSaltInKeychain:salt]; + + [self measureBlock:^{ + + NSString *databaseFilePath = [self createTempDatabaseFilePath]; + + + [self createTestDatabase:databaseFilePath + databaseKeySpecBlock:nil + databasePasswordBlock:^() { + return [self fetchTestPasswordFromKeychain]; + } + databaseSaltBlock:^() { + return [self fetchTestSaltFromKeychain]; + }]; + + [self verifyTestDatabase:databaseFilePath + databaseKeySpecBlock:nil + databasePasswordBlock:^() { + return [self fetchTestPasswordFromKeychain]; + } + databaseSaltBlock:^() { + return [self fetchTestSaltFromKeychain]; + }]; + }]; +} + +- (void)testGranularKeySpecFetchingStrategy +{ + NSData *keySpec = [self randomDatabaseKeySpec]; + [self storeTestKeySpecInKeychain:keySpec]; + + [self measureBlock:^{ + NSString *databaseFilePath = [self createTempDatabaseFilePath]; + + [self createTestDatabase:databaseFilePath + databaseKeySpecBlock:^() { + return [self fetchTestKeySpecFromKeychain]; + } + databasePasswordBlock:nil + databaseSaltBlock:nil]; + + [self verifyTestDatabase:databaseFilePath + databaseKeySpecBlock:^() { + return [self fetchTestKeySpecFromKeychain]; + } + databasePasswordBlock:nil + databaseSaltBlock:nil]; + }]; +} + +- (void)testWideKeyFetchingStrategy +{ + NSData *keySpec = [self randomDatabaseKeySpec]; + + [self measureBlock:^{ + NSString *databaseFilePath = [self createTempDatabaseFilePath]; + + [self createTestDatabase:databaseFilePath + databaseKeySpecBlock:^() { + return keySpec; + } + databasePasswordBlock:nil + databaseSaltBlock:nil]; + + [self verifyTestDatabase:databaseFilePath + databaseKeySpecBlock:^() { + return keySpec; + } + databasePasswordBlock:nil + databaseSaltBlock:nil]; + }]; +} + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 8657cf54d..73b1af86c 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -1215,7 +1215,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; thread:message.thread attempts:OWSMessageSenderRetryAttempts success:^{ - DDLogInfo(@"Succesfully sent sync transcript."); + DDLogInfo(@"Successfully sent sync transcript."); } failure:^(NSError *error) { // FIXME: We don't yet honor the isRetryable flag here, since sendSyncTranscriptForMessage diff --git a/SignalServiceKit/src/Storage/OWSStorage.h b/SignalServiceKit/src/Storage/OWSStorage.h index 9101cc3cc..83db4afb1 100644 --- a/SignalServiceKit/src/Storage/OWSStorage.h +++ b/SignalServiceKit/src/Storage/OWSStorage.h @@ -75,13 +75,11 @@ extern NSString *const StorageIsReadyNotification; */ + (BOOL)isDatabasePasswordAccessible; -+ (nullable NSData *)tryToLoadDatabasePassword:(NSError **)errorHandle; ++ (nullable NSData *)tryToLoadDatabaseLegacyPassphrase:(NSError **)errorHandle; ++ (void)removeLegacyPassphrase; -+ (nullable NSData *)tryToLoadDatabaseSalt:(NSError **)errorHandle; -+ (void)storeDatabaseSalt:(NSData *)saltData; ++ (void)storeDatabaseCipherKeySpec:(NSData *)cipherKeySpecData; -+ (nullable NSData *)tryToLoadDatabaseKeySpec:(NSError **)errorHandle; -+ (void)storeDatabaseKeySpec:(NSData *)keySpecData; @end diff --git a/SignalServiceKit/src/Storage/OWSStorage.m b/SignalServiceKit/src/Storage/OWSStorage.m index f99e40c06..756ef7a7e 100644 --- a/SignalServiceKit/src/Storage/OWSStorage.m +++ b/SignalServiceKit/src/Storage/OWSStorage.m @@ -27,9 +27,8 @@ NSString *const OWSStorageExceptionName_NoDatabase = @"OWSStorageExceptionName_N NSString *const OWSResetStorageNotification = @"OWSResetStorageNotification"; static NSString *keychainService = @"TSKeyChainService"; -static NSString *keychainDBPassAccount = @"TSDatabasePass"; -static NSString *keychainDBSalt = @"OWSDatabaseSalt"; -static NSString *keychainDBKeySpec = @"OWSDatabaseKeySpec"; +static NSString *keychainDBLegacyPassphrase = @"TSDatabasePass"; +static NSString *keychainDBCipherKeySpec = @"OWSDatabaseCipherKeySpec"; const NSUInteger kDatabasePasswordLength = 30; @@ -381,26 +380,22 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void); - (BOOL)tryToLoadDatabase { - // We determine the database password, salt and key spec first, since a side effect of - // this can be deleting any existing database file (if we're recovering - // from a corrupt keychain). - // - // Although we don't use databasePassword or databaseSalt in this method, - // we use their accessors to ensure that all three exist in the keychain - // and can be loaded or that we reset the database & keychain. - NSData *databasePassword = [self databasePassword]; - OWSAssert(databasePassword.length > 0); - NSData *databaseSalt = [self databaseSalt]; - OWSAssert(databaseSalt.length > 0); - NSData *databaseKeySpec = [self databaseKeySpec]; - OWSAssert(databaseKeySpec.length == kSQLCipherKeySpecLength); - YapDatabaseOptions *options = [[YapDatabaseOptions alloc] init]; options.corruptAction = YapDatabaseCorruptAction_Fail; options.enableMultiProcessSupport = YES; options.cipherKeySpecBlock = ^{ + // Rather than compute this once and capture the value of the key + // in the closure, we prefer to fetch the key from the keychain multiple times + // in order to keep the key out of application memory. + NSData *databaseKeySpec = [self databaseKeySpec]; + OWSAssert(databaseKeySpec.length == kSQLCipherKeySpecLength); return databaseKeySpec; }; + + // We leave a portion of the header decrypted so that iOS will recognize the file + // as a SQLite database. Otherwise, because the database lives in a shared data container, + // and our usage of sqlite's write-ahead logging retains a lock on the database, the OS + // would kill the app/share extension as soon as it is backgrounded. options.cipherUnencryptedHeaderLength = kSqliteHeaderLength; // If any of these asserts fails, we need to verify and update @@ -412,6 +407,11 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void); OWSAssert(options.pragmaJournalSizeLimit == 0); OWSAssert(options.pragmaMMapSize == 0); + // Sanity checking elsewhere asserts we should only regenerate key specs when + // there is no existing database, so rather than lazily generate in the cipherKeySpecBlock + // we must ensure the keyspec exists before we create the database. + [self ensureDatabaseKeySpecExists]; + OWSDatabase *database = [[OWSDatabase alloc] initWithPath:[self databaseFilePath] serializer:nil deserializer:[[self class] logOnFailureDeserializer] @@ -506,7 +506,7 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void); // This might be redundant but in the spirit of thoroughness... [self deleteDatabaseFiles]; - [self deletePasswordFromKeychain]; + [self deleteDBKeys]; if (CurrentAppContext().isMainApp) { [TSAttachmentStream deleteAttachments]; @@ -528,124 +528,62 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void); + (BOOL)isDatabasePasswordAccessible { - [SAMKeychain setAccessibilityType:kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly]; NSError *error; - NSString *dbPassword = [SAMKeychain passwordForService:keychainService account:keychainDBPassAccount error:&error]; + NSData *cipherKeySpec = [self tryToLoadDatabaseCipherKeySpec:&error]; - if (dbPassword && !error) { + if (cipherKeySpec && !error) { return YES; } if (error) { - DDLogWarn(@"Database password couldn't be accessed: %@", error.localizedDescription); + DDLogWarn(@"Database key couldn't be accessed: %@", error.localizedDescription); } return NO; } -+ (nullable NSData *)tryToLoadKeyChainValue:(NSString *)keychainKey errorHandle:(NSError **)errorHandle -{ - OWSAssert(keychainKey.length > 0); - OWSAssert(errorHandle); - - [SAMKeychain setAccessibilityType:kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly]; - - return [SAMKeychain passwordDataForService:keychainService account:keychainKey error:errorHandle]; -} - -+ (nullable NSData *)tryToLoadDatabasePassword:(NSError **)errorHandle -{ - return [self tryToLoadKeyChainValue:keychainDBPassAccount errorHandle:errorHandle]; -} - -+ (nullable NSData *)tryToLoadDatabaseSalt:(NSError **)errorHandle -{ - return [self tryToLoadKeyChainValue:keychainDBSalt errorHandle:errorHandle]; -} - -+ (nullable NSData *)tryToLoadDatabaseKeySpec:(NSError **)errorHandle ++ (nullable NSData *)tryToLoadDatabaseLegacyPassphrase:(NSError **)errorHandle { - return [self tryToLoadKeyChainValue:keychainDBKeySpec errorHandle:errorHandle]; + return [self tryToLoadKeyChainValue:keychainDBLegacyPassphrase errorHandle:errorHandle]; } -- (NSData *)databasePassword ++ (nullable NSData *)tryToLoadDatabaseCipherKeySpec:(NSError **)errorHandle { - return [self loadMetadataOrClearDatabase:^(NSError **_Nullable errorHandle) { - return [OWSStorage tryToLoadDatabasePassword:errorHandle]; - } - createDataBlock:^{ - NSData *passwordData = [self createAndSetNewDatabasePassword]; - NSData *saltData = [self createAndSetNewDatabaseSalt]; - NSData *keySpecData = [self createAndSetNewDatabaseKeySpec]; + NSData *_Nullable data = [self tryToLoadKeyChainValue:keychainDBCipherKeySpec errorHandle:errorHandle]; + OWSAssert(!data || data.length == kSQLCipherKeySpecLength); - OWSAssert(passwordData.length > 0); - OWSAssert(saltData.length == kSQLCipherSaltLength); - OWSAssert(keySpecData.length == kSQLCipherKeySpecLength); - - return passwordData; - } - label:@"Database password"]; + return data; } -- (NSData *)databaseSalt ++ (void)storeDatabaseCipherKeySpec:(NSData *)cipherKeySpecData { - return [self loadMetadataOrClearDatabase:^(NSError **_Nullable errorHandle) { - return [OWSStorage tryToLoadDatabaseSalt:errorHandle]; - } - createDataBlock:^{ - NSData *passwordData = [self createAndSetNewDatabasePassword]; - NSData *saltData = [self createAndSetNewDatabaseSalt]; - NSData *keySpecData = [self createAndSetNewDatabaseKeySpec]; + OWSAssert(cipherKeySpecData.length == kSQLCipherKeySpecLength); - OWSAssert(passwordData.length > 0); - OWSAssert(saltData.length == kSQLCipherSaltLength); - OWSAssert(keySpecData.length == kSQLCipherKeySpecLength); - - return saltData; - } - label:@"Database salt"]; + [self storeKeyChainValue:cipherKeySpecData keychainKey:keychainDBCipherKeySpec]; } -- (NSData *)databaseKeySpec ++ (void)removeLegacyPassphrase { - return [self loadMetadataOrClearDatabase:^(NSError **_Nullable errorHandle) { - return [OWSStorage tryToLoadDatabaseKeySpec:errorHandle]; - } - createDataBlock:^{ - OWSFail(@"%@ It should never be necessary to generate a random key spec.", self.logTag); - - NSData *passwordData = [self createAndSetNewDatabasePassword]; - NSData *saltData = [self createAndSetNewDatabaseSalt]; - NSData *keySpecData = [self createAndSetNewDatabaseKeySpec]; + DDLogInfo(@"%@ removing legacy passphrase", self.logTag); - OWSAssert(passwordData.length > 0); - OWSAssert(saltData.length == kSQLCipherSaltLength); - OWSAssert(keySpecData.length == kSQLCipherKeySpecLength); - - return keySpecData; - } - label:@"Database key spec"]; + [SAMKeychain deletePasswordForService:keychainService account:keychainDBLegacyPassphrase]; } -- (NSData *)loadMetadataOrClearDatabase:(LoadDatabaseMetadataBlock)loadDataBlock - createDataBlock:(CreateDatabaseMetadataBlock)createDataBlock - label:(NSString *)label +- (void)ensureDatabaseKeySpecExists { - OWSAssert(loadDataBlock); - OWSAssert(createDataBlock); - NSError *error; - NSData *_Nullable data = loadDataBlock(&error); + NSData *_Nullable keySpec = [[self class] tryToLoadDatabaseCipherKeySpec:&error]; - if (error) { + if (error || (keySpec.length != kSQLCipherKeySpecLength)) { // Because we use kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly, // the keychain will be inaccessible after device restart until // device is unlocked for the first time. If the app receives // a push notification, we won't be able to access the keychain to // process that notification, so we should just terminate by throwing // an uncaught exception. - NSString *errorDescription = - [NSString stringWithFormat:@"%@ inaccessible. No unlock since device restart? Error: %@", label, error]; + NSString *errorDescription = [NSString + stringWithFormat:@"CipherKeySpec inaccessible. New install or no unlock since device restart? Error: %@", + error]; if (CurrentAppContext().isMainApp) { UIApplicationState applicationState = CurrentAppContext().mainApplicationState; errorDescription = @@ -659,19 +597,17 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void); // 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 backgroundedAppDatabasePasswordInaccessibleWithErrorDescription:errorDescription]; + [self raiseKeySpecInaccessibleExceptionWithErrorDescription:errorDescription]; } } else { - [self backgroundedAppDatabasePasswordInaccessibleWithErrorDescription: - [NSString stringWithFormat:@"%@ inaccessible; not main app.", label]]; + [self raiseKeySpecInaccessibleExceptionWithErrorDescription:@"CipherKeySpec inaccessible; not main app."]; } // 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. - - BOOL shouldHaveDatabaseMetadata = [NSFileManager.defaultManager fileExistsAtPath:[self databaseFilePath]]; - if (shouldHaveDatabaseMetadata) { + BOOL doesDBExist = [NSFileManager.defaultManager fileExistsAtPath:[self databaseFilePath]]; + if (doesDBExist) { OWSFail(@"%@ Could not load database metadata", self.logTag); OWSProdCritical([OWSAnalyticsEvents storageErrorCouldNotLoadDatabaseSecondAttempt]); } @@ -679,47 +615,30 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void); // Try to reset app by deleting database. [OWSStorage resetAllStorage]; - data = createDataBlock(); + keySpec = [Randomness generateRandomBytes:(int)kSQLCipherKeySpecLength]; + [[self class] storeDatabaseCipherKeySpec:keySpec]; } - - return data; -} - -- (NSData *)createAndSetNewDatabasePassword -{ - NSData *password = [[[Randomness generateRandomBytes:kDatabasePasswordLength] base64EncodedString] - dataUsingEncoding:NSUTF8StringEncoding]; - - [OWSStorage storeDatabasePassword:password]; - - return password; -} - -- (NSData *)createAndSetNewDatabaseSalt -{ - NSData *saltData = [Randomness generateRandomBytes:(int)kSQLCipherSaltLength]; - - [OWSStorage storeDatabaseSalt:saltData]; - - return saltData; } -- (NSData *)createAndSetNewDatabaseKeySpec +- (NSData *)databaseKeySpec { - NSData *databasePassword = [self databasePassword]; - OWSAssert(databasePassword.length > 0); - NSData *databaseSalt = [self databaseSalt]; - OWSAssert(databaseSalt.length == kSQLCipherSaltLength); + NSError *error; + NSData *_Nullable keySpec = [[self class] tryToLoadDatabaseCipherKeySpec:&error]; - NSData *keySpecData = [YapDatabaseCryptoUtils databaseKeySpecForPassword:databasePassword saltData:databaseSalt]; - OWSAssert(keySpecData.length == kSQLCipherKeySpecLength); + if (error) { + DDLogError(@"%@ failed to fetch databaseKeySpec with error: %@", self.logTag, error); + [self raiseKeySpecInaccessibleExceptionWithErrorDescription:@"CipherKeySpec inaccessible"]; + } - [OWSStorage storeDatabaseKeySpec:keySpecData]; + if (keySpec.length != kSQLCipherKeySpecLength) { + DDLogError(@"%@ keyspec had length: %lu", self.logTag, (unsigned long)keySpec.length); + [self raiseKeySpecInaccessibleExceptionWithErrorDescription:@"CipherKeySpec invalid"]; + } - return keySpecData; + return keySpec; } -- (void)backgroundedAppDatabasePasswordInaccessibleWithErrorDescription:(NSString *)errorDescription +- (void)raiseKeySpecInaccessibleExceptionWithErrorDescription:(NSString *)errorDescription { OWSAssert(CurrentAppContext().isMainApp && CurrentAppContext().isInBackground); @@ -732,11 +651,10 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void); OWSRaiseException(OWSStorageExceptionName_DatabasePasswordInaccessibleWhileBackgrounded, @"%@", errorDescription); } -+ (void)deletePasswordFromKeychain ++ (void)deleteDBKeys { - [SAMKeychain deletePasswordForService:keychainService account:keychainDBPassAccount]; - [SAMKeychain deletePasswordForService:keychainService account:keychainDBSalt]; - [SAMKeychain deletePasswordForService:keychainService account:keychainDBKeySpec]; + [SAMKeychain deletePasswordForService:keychainService account:keychainDBLegacyPassphrase]; + [SAMKeychain deletePasswordForService:keychainService account:keychainDBCipherKeySpec]; } - (unsigned long long)databaseFileSize @@ -744,6 +662,14 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void); return [OWSFileSystem fileSizeOfPath:self.databaseFilePath].unsignedLongLongValue; } ++ (nullable NSData *)tryToLoadKeyChainValue:(NSString *)keychainKey errorHandle:(NSError **)errorHandle +{ + OWSAssert(keychainKey.length > 0); + OWSAssert(errorHandle); + + return [SAMKeychain passwordDataForService:keychainService account:keychainKey error:errorHandle]; +} + + (void)storeKeyChainValue:(NSData *)data keychainKey:(NSString *)keychainKey { OWSAssert(keychainKey.length > 0); @@ -756,37 +682,16 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void); OWSFail(@"%@ Could not store database metadata", self.logTag); OWSProdCritical([OWSAnalyticsEvents storageErrorCouldNotStoreKeychainValue]); - [OWSStorage deletePasswordFromKeychain]; - // Sleep to give analytics events time to be delivered. [NSThread sleepForTimeInterval:15.0f]; OWSRaiseException( OWSStorageExceptionName_DatabasePasswordUnwritable, @"Setting keychain value failed with error: %@", error); } else { - DDLogWarn(@"Succesfully set new keychain value."); + DDLogWarn(@"Successfully set new keychain value."); } } -+ (void)storeDatabasePassword:(NSData *)passwordData -{ - [self storeKeyChainValue:passwordData keychainKey:keychainDBPassAccount]; -} - -+ (void)storeDatabaseSalt:(NSData *)saltData -{ - OWSAssert(saltData.length == kSQLCipherSaltLength); - - [self storeKeyChainValue:saltData keychainKey:keychainDBSalt]; -} - -+ (void)storeDatabaseKeySpec:(NSData *)keySpecData -{ - OWSAssert(keySpecData.length == kSQLCipherKeySpecLength); - - [self storeKeyChainValue:keySpecData keychainKey:keychainDBKeySpec]; -} - @end NS_ASSUME_NONNULL_END