more granular key access

// FREEBIE
2018-01-31 13:24:58 -08:00 · 2018-01-31 13:24:58 -08:00 · d22fc664f2
parent 426c9baa16
commit d22fc664f2
4 changed files with 292 additions and 6 deletions
--- 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
@ -0,0 +1,52 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+	<key>classNames</key>
+	<dict>
+		<key>OWSDatabaseConverterTest</key>
+		<dict>
+			<key>testGranularKeySpecFetchingStrategy</key>
+			<dict>
+				<key>com.apple.XCTPerformanceMetric_WallClockTime</key>
+				<dict>
+					<key>baselineAverage</key>
+					<real>0.039171</real>
+					<key>baselineIntegrationDisplayName</key>
+					<string>Local Baseline</string>
+				</dict>
+			</dict>
+			<key>testGranularPassphraseFetchingStrategy</key>
+			<dict>
+				<key>com.apple.XCTPerformanceMetric_WallClockTime</key>
+				<dict>
+					<key>baselineAverage</key>
+					<real>0.22846</real>
+					<key>baselineIntegrationDisplayName</key>
+					<string>Local Baseline</string>
+				</dict>
+			</dict>
+			<key>testWideKeyFetchingStrategy</key>
+			<dict>
+				<key>com.apple.XCTPerformanceMetric_WallClockTime</key>
+				<dict>
+					<key>baselineAverage</key>
+					<real>0.039649</real>
+					<key>baselineIntegrationDisplayName</key>
+					<string>Local Baseline</string>
+				</dict>
+			</dict>
+			<key>testWidePassphraseFetchingStrategy</key>
+			<dict>
+				<key>com.apple.XCTPerformanceMetric_WallClockTime</key>
+				<dict>
+					<key>baselineAverage</key>
+					<real>0.21819</real>
+					<key>baselineIntegrationDisplayName</key>
+					<string>Local Baseline</string>
+				</dict>
+			</dict>
+		</dict>
+	</dict>
+</dict>
+</plist>
--- a/Signal.xcodeproj/xcshareddata/xcbaselines/D221A0A9169C9E5F00537ABF.xcbaseline/Info.plist
+++ b/Signal.xcodeproj/xcshareddata/xcbaselines/D221A0A9169C9E5F00537ABF.xcbaseline/Info.plist
@ -0,0 +1,40 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+	<key>runDestinationsByUUID</key>
+	<dict>
+		<key>8A553EB1-B9DF-4DDE-8F93-10474ECF05C2</key>
+		<dict>
+			<key>localComputer</key>
+			<dict>
+				<key>busSpeedInMHz</key>
+				<integer>100</integer>
+				<key>cpuCount</key>
+				<integer>1</integer>
+				<key>cpuKind</key>
+				<string>Intel Core i7</string>
+				<key>cpuSpeedInMHz</key>
+				<integer>2900</integer>
+				<key>logicalCPUCoresPerPackage</key>
+				<integer>8</integer>
+				<key>modelCode</key>
+				<string>MacBookPro13,3</string>
+				<key>physicalCPUCoresPerPackage</key>
+				<integer>4</integer>
+				<key>platformIdentifier</key>
+				<string>com.apple.platform.macosx</string>
+			</dict>
+			<key>targetArchitecture</key>
+			<string>x86_64</string>
+			<key>targetDevice</key>
+			<dict>
+				<key>modelCode</key>
+				<string>iPhone8,1</string>
+				<key>platformIdentifier</key>
+				<string>com.apple.platform.iphonesimulator</string>
+			</dict>
+		</dict>
+	</dict>
+</dict>
+</plist>
--- 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

@ -905,6 +907,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
--- a/SignalServiceKit/src/Storage/OWSStorage.m
+++ b/SignalServiceKit/src/Storage/OWSStorage.m
@ -380,16 +380,15 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void);

 - (BOOL)tryToLoadDatabase
 {
-    // We determine the database key spec first, since a side effect of
-    // this can be deleting any existing database file (if we're recovering
-    // from a corrupt keychain).
-    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;
    };