From 92eb35f62e188c782db0d8e4ef0c35ed57cdf5f8 Mon Sep 17 00:00:00 2001
From: Matthew Chen <charlesmchen@gmail.com>
Date: Fri, 26 Jan 2018 12:28:12 -0500
Subject: [PATCH 1/7] Add logging in database conversion.

---
 .../Utilities/YapDatabaseCryptoUtils.m        | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/YapDatabase/Utilities/YapDatabaseCryptoUtils.m b/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
index 68a2564a9..1de3a2216 100644
--- a/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
+++ b/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
@@ -164,6 +164,7 @@ + (nullable NSError *)convertDatabaseIfNecessary:(NSString *)databaseFilePath
                                     keySpecBlock:(YapDatabaseKeySpecBlock)keySpecBlock
 {
     if (![self doesDatabaseNeedToBeConverted:databaseFilePath]) {
+        YDBLogInfo(@"%@ convertDatabaseIfNecessary: database does not need to be converted.", self.logTag);
         return nil;
     }
 
@@ -183,6 +184,8 @@ + (nullable NSError *)convertDatabase:(NSString *)databaseFilePath
     YapAssert(saltBlock);
     YapAssert(keySpecBlock);
 
+    YDBLogInfo(@"%@ convertDatabase.", self.logTag);
+
     NSData *saltData;
     {
         NSData *headerData = [self readFirstNBytesOfDatabaseFile:databaseFilePath byteCount:kSqliteHeaderLength];
@@ -197,6 +200,8 @@ + (nullable NSError *)convertDatabase:(NSString *)databaseFilePath
         saltBlock(saltData);
     }
 
+    YDBLogInfo(@"%@ convertDatabase: salt extracted.", self.logTag);
+
     {
         NSData *_Nullable keySpecData = [self databaseKeySpecForPassword:databasePassword saltData:saltData];
         if (!keySpecData || keySpecData.length != kSQLCipherKeySpecLength) {
@@ -211,6 +216,8 @@ + (nullable NSError *)convertDatabase:(NSString *)databaseFilePath
         // unrecoverable state.
         keySpecBlock(keySpecData);
     }
+    
+    YDBLogInfo(@"%@ convertDatabase: key spec derived.", self.logTag);
 
     // -----------------------------------------------------------
     //
@@ -235,6 +242,8 @@ + (nullable NSError *)convertDatabase:(NSString *)databaseFilePath
             return YDBErrorWithDescription(@"Failed to open database");
         }
     }
+    
+    YDBLogInfo(@"%@ convertDatabase: database open.", self.logTag);
 
     // -----------------------------------------------------------
     //
@@ -248,6 +257,8 @@ + (nullable NSError *)convertDatabase:(NSString *)databaseFilePath
             return YDBErrorWithDescription(@"Failed to set SQLCipher key");
         }
     }
+    
+    YDBLogInfo(@"%@ convertDatabase: database keyed.", self.logTag);
 
     // -----------------------------------------------------------
     //
@@ -302,6 +313,8 @@ + (nullable NSError *)convertDatabase:(NSString *)databaseFilePath
         // END DB setup copied from YapDatabase
         // BEGIN SQLCipher migration
     }
+    
+    YDBLogInfo(@"%@ convertDatabase: database configured.", self.logTag);
 
 #ifdef DEBUG
     // We can obtain the database salt in two ways: by reading the first 16 bytes of the encrypted
@@ -313,6 +326,8 @@ + (nullable NSError *)convertDatabase:(NSString *)databaseFilePath
 
         YapAssert([[self hexadecimalStringForData:saltData] isEqualToString:saltString]);
     }
+    
+    YDBLogInfo(@"%@ convertDatabase: salt confirmed.", self.logTag);
 #endif
 
     // -----------------------------------------------------------
@@ -327,6 +342,8 @@ + (nullable NSError *)convertDatabase:(NSString *)databaseFilePath
         if (error) {
             return error;
         }
+        
+        YDBLogInfo(@"%@ convertDatabase: encrypted header configured.", self.logTag);
 
         // Modify the first page, so that SQLCipher will overwrite, respecting our new cipher_plaintext_header_size
         NSString *tableName = [NSString stringWithFormat:@"signal-migration-%@", [NSUUID new].UUIDString];
@@ -340,6 +357,8 @@ + (nullable NSError *)convertDatabase:(NSString *)databaseFilePath
         if (error) {
             return error;
         }
+        
+        YDBLogInfo(@"%@ convertDatabase: database dirtied.", self.logTag);
 
         // Force a checkpoint so that the plaintext is written to the actual DB file, not just living in the WAL.
         int log, ckpt;
@@ -348,8 +367,12 @@ + (nullable NSError *)convertDatabase:(NSString *)databaseFilePath
             YDBLogError(@"%@ Error forcing checkpoint. status: %d, log: %d, ckpt: %d, error: %s", self.logTag, status, log, ckpt, sqlite3_errmsg(db));
             return YDBErrorWithDescription(@"Error forcing checkpoint.");
         }
+        
+        YDBLogInfo(@"%@ convertDatabase: checkpoint completed.", self.logTag);
 
         sqlite3_close(db);
+        
+        YDBLogInfo(@"%@ convertDatabase: database closed.", self.logTag);
     }
 
     return nil;

From e9e2178d9e432c0bb674bcdfded32d137cdc61d0 Mon Sep 17 00:00:00 2001
From: Matthew Chen <charlesmchen@gmail.com>
Date: Tue, 30 Jan 2018 10:00:41 -0500
Subject: [PATCH 2/7] Respond to CR.

---
 .../Utilities/YapDatabaseCryptoUtils.h        |  9 +--
 .../Utilities/YapDatabaseCryptoUtils.m        |  2 +-
 YapDatabase/YapDatabase.m                     | 67 ++++++++++++-------
 YapDatabase/YapDatabaseOptions.h              | 14 +++-
 4 files changed, 63 insertions(+), 29 deletions(-)

diff --git a/YapDatabase/Utilities/YapDatabaseCryptoUtils.h b/YapDatabase/Utilities/YapDatabaseCryptoUtils.h
index d97b1f093..2a6f74386 100644
--- a/YapDatabase/Utilities/YapDatabaseCryptoUtils.h
+++ b/YapDatabase/Utilities/YapDatabaseCryptoUtils.h
@@ -58,13 +58,14 @@ typedef void (^YapDatabaseKeySpecBlock)(NSData *keySpecData);
 // The header does not contain any user data.  See:
 // https://www.sqlite.org/fileformat.html#the_database_header
 //
-// However, Sqlite normally uses the first 16 bytes of the Sqlite header to store
+// However, SQLCipher normally uses the first 16 bytes of the Sqlite header to store
 // a salt value.  Therefore when using unencrypted headers, it is also necessary
 // to explicitly specify a salt value.
 //
 // It is possible to convert SQLCipher databases with encrypted headers to use
 // unencrypted headers.  However, during this conversion, the salt must be extracted
-// and preserved by reading the first 16 bytes of the unconverted file.
+// by reading the first 16 bytes of the unconverted file and preserving it elsewhere,
+// e.g. the keychain.
 //
 //
 // Implementation
@@ -101,7 +102,7 @@ typedef void (^YapDatabaseKeySpecBlock)(NSData *keySpecData);
 // * If convertDatabaseIfNecessary converts the database, it will use its
 //   saltBlock and keySpecBlock parameters to inform you of the salt
 //   and keyspec for this database.  These values will be needed when
-//   opening the database, so they should presumably stored in the
+//   opening the database, so they should presumably be stored in the
 //   keychain (like the database password).
 //
 //
@@ -133,7 +134,7 @@ typedef void (^YapDatabaseKeySpecBlock)(NSData *keySpecData);
 // * If convertDatabaseIfNecessary converts the database, it will use its
 //   saltBlock and keySpecBlock parameters to inform you of the salt
 //   and keyspec for this database.  These values will be needed when
-//   opening the database, so they should presumably stored in the
+//   opening the database, so they should presumably be stored in the
 //   keychain (like the database password).
 + (nullable NSError *)convertDatabaseIfNecessary:(NSString *)databaseFilePath
                                 databasePassword:(NSData *)databasePassword
diff --git a/YapDatabase/Utilities/YapDatabaseCryptoUtils.m b/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
index 1de3a2216..a785ac445 100644
--- a/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
+++ b/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
@@ -433,7 +433,7 @@ + (nullable NSData *)deriveDatabaseKeyForPassword:(NSData *)passwordData saltDat
 
     unsigned char *derivedKeyBytes = malloc((size_t)kSQLCipherDerivedKeyLength);
     YapAssert(derivedKeyBytes);
-    // See: PBKDF2_ITER.
+    // See: PBKDF2_ITER from SQLCipher.
     const unsigned int workfactor = 64000;
 
     int result = CCKeyDerivationPBKDF(kCCPBKDF2,
diff --git a/YapDatabase/YapDatabase.m b/YapDatabase/YapDatabase.m
index 5957fe8ce..4f57ae932 100644
--- a/YapDatabase/YapDatabase.m
+++ b/YapDatabase/YapDatabase.m
@@ -7,6 +7,7 @@
 #import "YapDatabaseConnectionState.h"
 #import "YapDatabaseLogging.h"
 #import "YapDatabaseString.h"
+#import "YapDatabaseCryptoUtils.h"
 
 #import "sqlite3.h"
 
@@ -830,18 +831,50 @@ - (BOOL)configureDatabase:(BOOL)isNewDatabaseFile
 **/
 - (BOOL)configureEncryptionForDatabase:(sqlite3 *)sqlite
 {
+    if (options.cipherUnencryptedHeaderLength > 0) {
+        if (options.cipherKeySpecBlock)
+        {
+            // Do nothing.
+        } else if (!options.cipherKeyBlock ||
+                   options.cipherSaltBlock) {
+            NSAssert(NO, @"If you're using YapDatabaseOptions.cipherUnencryptedHeaderLength, you need to set either cipherKeySpecBlock or both cipherKeyBlock and cipherSaltBlock.");
+            return NO;
+        }
+    }        
+
     if (options.cipherKeyBlock ||
         options.cipherKeySpecBlock)
 	{
         NSData *_Nullable keyData = nil;
         if (options.cipherKeySpecBlock)
         {
-            keyData = options.cipherKeySpecBlock();
-            if (!keyData)
+            if (options.cipherKeyBlock) {
+                NSAssert(NO, @"If you're using YapDatabaseOptions.cipherKeySpecBlock, you don't need to set a cipherKeySpecBlock.");
+                return NO;
+            }
+            if (options.cipherSaltBlock) {
+                NSAssert(NO, @"If you're using YapDatabaseOptions.cipherKeySpecBlock, you don't need to set a cipherSaltBlock.");
+                return NO;
+            }
+
+            NSData *_Nullable keySpecData = options.cipherKeySpecBlock();
+            if (!keySpecData)
             {
                 NSAssert(NO, @"YapDatabaseOptions.cipherKeySpecBlock cannot return nil!");
                 return NO;
             }
+            if (keySpecData.length != kSQLCipherKeySpecLength) {
+                NSAssert(NO, @"YapDatabaseOptions.cipherKeySpecBlock returned a key spec of unexpected length: %zd.", keySpecData.length);
+                return NO;
+            }
+
+            // Use a raw key spec, where the 96 hexadecimal digits are provided
+            // (i.e. 64 hex for the 256 bit key, followed by 32 hex for the 128 bit salt)
+            // using explicit BLOB syntax, e.g.:
+            //
+            // x'98483C6EB40B6C31A448C22A66DED3B5E5E8D5119CAC8327B655C8B5C483648101010101010101010101010101010101'
+            NSString *keySpecString = [NSString stringWithFormat:@"x'%@'", [self hexadecimalStringForData:keySpecData]];
+            keyData = [keySpecString dataUsingEncoding:NSUTF8StringEncoding];
         } else {
             keyData = options.cipherKeyBlock();
             if (!keyData)
@@ -884,27 +917,11 @@ - (BOOL)configureEncryptionForDatabase:(sqlite3 *)sqlite
             }
         }
         
-        if (options.cipherKeySpecBlock) {
-            // Use a raw key spec, where the 96 hexadecimal digits are provided
-            // (i.e. 64 hex for the 256 bit key, followed by 32 hex for the 128 bit salt)
-            // using explicit BLOB syntax, e.g.:
-            //
-            // x'98483C6EB40B6C31A448C22A66DED3B5E5E8D5119CAC8327B655C8B5C483648101010101010101010101010101010101'
-            NSString *keySpecString = [NSString stringWithFormat:@"x'%@'", [self hexadecimalStringForData:keyData]];
-            NSData *keySpecStringData = [keySpecString dataUsingEncoding:NSUTF8StringEncoding];
-            int status = sqlite3_key(sqlite, [keySpecStringData bytes], (int)[keySpecStringData length]);
-            if (status != SQLITE_OK)
-            {
-                YDBLogError(@"Error setting SQLCipher key: %d %s", status, sqlite3_errmsg(sqlite));
-                return NO;
-            }
-        } else {
-            int status = sqlite3_key(sqlite, [keyData bytes], (int)[keyData length]);
-            if (status != SQLITE_OK)
-            {
-                YDBLogError(@"Error setting SQLCipher key: %d %s", status, sqlite3_errmsg(sqlite));
-                return NO;
-            }
+        int status = sqlite3_key(sqlite, [keyData bytes], (int)[keyData length]);
+        if (status != SQLITE_OK)
+        {
+            YDBLogError(@"Error setting SQLCipher key: %d %s", status, sqlite3_errmsg(sqlite));
+            return NO;
         }
         
         if (options.cipherUnencryptedHeaderLength > 0 &&
@@ -923,6 +940,10 @@ - (BOOL)configureEncryptionForDatabase:(sqlite3 *)sqlite
                     NSAssert(NO, @"YapDatabaseOptions.cipherSaltBlock cannot return nil!");
                     return NO;
                 }
+                if (saltData.length != kSQLCipherSaltLength) {
+                    NSAssert(NO, @"YapDatabaseOptions.cipherSaltBlock returned a salt of unexpected length: %zd.", saltData.length);
+                    return NO;
+                }
 
                 {
                     char *errorMsg;
diff --git a/YapDatabase/YapDatabaseOptions.h b/YapDatabase/YapDatabaseOptions.h
index 80032e2df..1b04b0a82 100644
--- a/YapDatabase/YapDatabaseOptions.h
+++ b/YapDatabase/YapDatabaseOptions.h
@@ -244,7 +244,19 @@ typedef NSData *_Nonnull (^YapDatabaseCipherKeyBlock)(void);
 /**
  * Set a block here that returns the key spec (not the key) for the SQLCipher database.
  *
- * This key spec incorporates the "derived key" and the "salt".
+ * The key spec incorporates the "derived key" and the "salt".
+ *
+ * The key spec should be kSQLCipherKeySpecLength bytes in length.
+ *
+ * If you a key spec, you do NOT need to specify the salt (using cipherSaltBlock)
+ * and "key/password" (using cipherKeyBlock).
+ *
+ * For new databases, the key spec can be any N bytes where N is kSQLCipherKeySpecLength.
+ * You should consider generating them with SecRandomCopyBytes().
+ *
+ * For existing databases that were created using a "key/password" (i.e. cipherKeyBlock),
+ * you can derive a key spec using that key/password and the database's salt.  See
+ * comments in YapDatabaseCryptoUtils.h.
  *
  * This block allows you to fetch the key spec from the keychain (or elsewhere)
  * only when you need it, instead of persisting it in memory.

From df39a8514769ca1013977691ff78afce79125791 Mon Sep 17 00:00:00 2001
From: Michael Kirk <michael.code@endoftheworl.de>
Date: Wed, 31 Jan 2018 09:04:06 -0800
Subject: [PATCH 3/7] Separate unencrypted header migration from key-spec
 migration.

---
 .../Utilities/YapDatabaseCryptoUtils.h        | 21 ++++----
 .../Utilities/YapDatabaseCryptoUtils.m        | 51 ++++++-------------
 2 files changed, 25 insertions(+), 47 deletions(-)

diff --git a/YapDatabase/Utilities/YapDatabaseCryptoUtils.h b/YapDatabase/Utilities/YapDatabaseCryptoUtils.h
index 2a6f74386..f394b4ac3 100644
--- a/YapDatabase/Utilities/YapDatabaseCryptoUtils.h
+++ b/YapDatabase/Utilities/YapDatabaseCryptoUtils.h
@@ -10,11 +10,10 @@ NS_ASSUME_NONNULL_BEGIN
 
 extern const NSUInteger kSqliteHeaderLength;
 extern const NSUInteger kSQLCipherSaltLength;
-extern const NSUInteger kSQLCipherDerivedKeyLength;
+extern const NSUInteger kSQLCipherKeyLength;
 extern const NSUInteger kSQLCipherKeySpecLength;
 
 typedef void (^YapDatabaseSaltBlock)(NSData *saltData);
-typedef void (^YapDatabaseKeySpecBlock)(NSData *keySpecData);
 
 // This class contains utility methods for use with SQLCipher encrypted
 // databases, specifically to address an issue around database files that
@@ -100,7 +99,7 @@ typedef void (^YapDatabaseKeySpecBlock)(NSData *keySpecData);
 // * This method should always be pretty fast, and should be safe to
 //   call from within [UIApplicationDelegate application: didFinishLaunchingWithOptions:].
 // * If convertDatabaseIfNecessary converts the database, it will use its
-//   saltBlock and keySpecBlock parameters to inform you of the salt
+//   recordSaltBlock to inform you of the salt
 //   and keyspec for this database.  These values will be needed when
 //   opening the database, so they should presumably be stored in the
 //   keychain (like the database password).
@@ -131,23 +130,21 @@ typedef void (^YapDatabaseKeySpecBlock)(NSData *keySpecData);
 // * This method will have no effect if the YapDatabase has already been converted.
 // * This method should always be pretty fast, and should be safe to
 //   call from within [UIApplicationDelegate application: didFinishLaunchingWithOptions:].
-// * If convertDatabaseIfNecessary converts the database, it will use its
-//   saltBlock and keySpecBlock parameters to inform you of the salt
-//   and keyspec for this database.  These values will be needed when
-//   opening the database, so they should presumably be stored in the
-//   keychain (like the database password).
+// * IMPORTANT: If you fail to record the salt during conversion you will not be able to decrypt
+//   the database in the future, effectively losing all data.  If convertDatabaseIfNecessary
+//   converts the database, it will use its recordSaltBlock parameter to inform you of the salt
+//   for this database. Within that block you must store the salt somewhere durable.
 + (nullable NSError *)convertDatabaseIfNecessary:(NSString *)databaseFilePath
                                 databasePassword:(NSData *)databasePassword
-                                       saltBlock:(YapDatabaseSaltBlock)saltBlock
-                                    keySpecBlock:(YapDatabaseKeySpecBlock)keySpecBlock;
+                                 recordSaltBlock:(YapDatabaseSaltBlock)recordSaltBlock;
 
 // This method can be used to derive a SQLCipher "key spec" from a
 // database password and salt.  Key spec derivation is somewhat costly.
 // The key spec is needed every time the database file is opened
-// (including every time YapDatabse makes a new database connection),
+// (including every time YapDatabase makes a new database connection),
 // So it benefits performance to pass a pre-derived key spec to
 // YapDatabase.
-+ (nullable NSData *)databaseKeySpecForPassword:(NSData *)passwordData saltData:(NSData *)saltData;
++ (nullable NSData *)deriveDatabaseKeySpecForPassword:(NSData *)passwordData saltData:(NSData *)saltData;
 
 #pragma mark - Utils
 
diff --git a/YapDatabase/Utilities/YapDatabaseCryptoUtils.m b/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
index a785ac445..4520229fe 100644
--- a/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
+++ b/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
@@ -92,7 +92,7 @@
 
 const NSUInteger kSqliteHeaderLength = 32;
 const NSUInteger kSQLCipherSaltLength = 16;
-const NSUInteger kSQLCipherDerivedKeyLength = 32;
+const NSUInteger kSQLCipherKeyLength = 32;
 const NSUInteger kSQLCipherKeySpecLength = 48;
 
 NSString *const YapDatabaseErrorDomain = @"YapDatabaseErrorDomain";
@@ -140,7 +140,7 @@ + (BOOL)doesDatabaseNeedToBeConverted:(NSString *)databaseFilePath
 
     if (![[NSFileManager defaultManager] fileExistsAtPath:databaseFilePath]) {
         YDBLogVerbose(@"%@ database file not found.", self.logTag);
-        return nil;
+        return NO;
     }
 
     NSData *headerData = [self readFirstNBytesOfDatabaseFile:databaseFilePath byteCount:kSqliteHeaderLength];
@@ -160,8 +160,7 @@ + (BOOL)doesDatabaseNeedToBeConverted:(NSString *)databaseFilePath
 
 + (nullable NSError *)convertDatabaseIfNecessary:(NSString *)databaseFilePath
                                 databasePassword:(NSData *)databasePassword
-                                       saltBlock:(YapDatabaseSaltBlock)saltBlock
-                                    keySpecBlock:(YapDatabaseKeySpecBlock)keySpecBlock
+                                 recordSaltBlock:(YapDatabaseSaltBlock)recordSaltBlock
 {
     if (![self doesDatabaseNeedToBeConverted:databaseFilePath]) {
         YDBLogInfo(@"%@ convertDatabaseIfNecessary: database does not need to be converted.", self.logTag);
@@ -170,19 +169,16 @@ + (nullable NSError *)convertDatabaseIfNecessary:(NSString *)databaseFilePath
 
     return [self convertDatabase:databaseFilePath
                 databasePassword:databasePassword
-                       saltBlock:saltBlock
-                    keySpecBlock:keySpecBlock];
+                       recordSaltBlock:recordSaltBlock];
 }
 
 + (nullable NSError *)convertDatabase:(NSString *)databaseFilePath
                      databasePassword:(NSData *)databasePassword
-                            saltBlock:(YapDatabaseSaltBlock)saltBlock
-                         keySpecBlock:(YapDatabaseKeySpecBlock)keySpecBlock
+                      recordSaltBlock:(YapDatabaseSaltBlock)recordSaltBlock
 {
     YapAssert(databaseFilePath.length > 0);
     YapAssert(databasePassword.length > 0);
-    YapAssert(saltBlock);
-    YapAssert(keySpecBlock);
+    YapAssert(recordSaltBlock);
 
     YDBLogInfo(@"%@ convertDatabase.", self.logTag);
 
@@ -197,24 +193,8 @@ + (nullable NSError *)convertDatabase:(NSString *)databaseFilePath
         // Make sure we successfully persist the salt (persumably in the keychain) before
         // proceeding with the database conversion or we could leave the app in an
         // unrecoverable state.
-        saltBlock(saltData);
-    }
-
-    YDBLogInfo(@"%@ convertDatabase: salt extracted.", self.logTag);
-
-    {
-        NSData *_Nullable keySpecData = [self databaseKeySpecForPassword:databasePassword saltData:saltData];
-        if (!keySpecData || keySpecData.length != kSQLCipherKeySpecLength) {
-            YDBLogError(@"Error deriving key spec");
-            return YDBErrorWithDescription(@"Invalid key spec");
-        }
-
-        YapAssert(keySpecData.length == kSQLCipherKeySpecLength);
-
-        // Make sure we successfully persist the key spec (persumably in the keychain) before
-        // proceeding with the database conversion or we could leave the app in an
-        // unrecoverable state.
-        keySpecBlock(keySpecData);
+        YDBLogInfo(@"%@ convertDatabase: salt extracted.", self.logTag);
+        recordSaltBlock(saltData);
     }
     
     YDBLogInfo(@"%@ convertDatabase: key spec derived.", self.logTag);
@@ -431,7 +411,8 @@ + (nullable NSData *)deriveDatabaseKeyForPassword:(NSData *)passwordData saltDat
     YapAssert(passwordData.length > 0);
     YapAssert(saltData.length == kSQLCipherSaltLength);
 
-    unsigned char *derivedKeyBytes = malloc((size_t)kSQLCipherDerivedKeyLength);
+    // FIXME memory leak. manage with NSData.
+    unsigned char *derivedKeyBytes = malloc((size_t)kSQLCipherKeyLength);
     YapAssert(derivedKeyBytes);
     // See: PBKDF2_ITER from SQLCipher.
     const unsigned int workfactor = 64000;
@@ -444,14 +425,14 @@ + (nullable NSData *)deriveDatabaseKeyForPassword:(NSData *)passwordData saltDat
         kCCPRFHmacAlgSHA1,
         workfactor,
         derivedKeyBytes,
-        kSQLCipherDerivedKeyLength);
+        kSQLCipherKeyLength);
     if (result != kCCSuccess) {
         YDBLogError(@"Error deriving key: %d", result);
         return nil;
     }
 
-    NSData *_Nullable derivedKeyData = [NSData dataWithBytes:derivedKeyBytes length:kSQLCipherDerivedKeyLength];
-    if (!derivedKeyData || derivedKeyData.length != kSQLCipherDerivedKeyLength) {
+    NSData *_Nullable derivedKeyData = [NSData dataWithBytes:derivedKeyBytes length:kSQLCipherKeyLength];
+    if (!derivedKeyData || derivedKeyData.length != kSQLCipherKeyLength) {
         YDBLogError(@"Invalid derived key: %d", result);
         return nil;
     }
@@ -459,13 +440,13 @@ + (nullable NSData *)deriveDatabaseKeyForPassword:(NSData *)passwordData saltDat
     return derivedKeyData;
 }
 
-+ (nullable NSData *)databaseKeySpecForPassword:(NSData *)passwordData saltData:(NSData *)saltData
++ (nullable NSData *)deriveDatabaseKeySpecForPassword:(NSData *)passwordData saltData:(NSData *)saltData
 {
     YapAssert(passwordData.length > 0);
     YapAssert(saltData.length == kSQLCipherSaltLength);
 
     NSData *_Nullable derivedKeyData = [self deriveDatabaseKeyForPassword:passwordData saltData:saltData];
-    if (!derivedKeyData || derivedKeyData.length != kSQLCipherDerivedKeyLength) {
+    if (!derivedKeyData || derivedKeyData.length != kSQLCipherKeyLength) {
         YDBLogError(@"Error deriving key");
         return nil;
     }
@@ -475,7 +456,7 @@ + (nullable NSData *)databaseKeySpecForPassword:(NSData *)passwordData saltData:
 
     YapAssert(keySpecData.length == kSQLCipherKeySpecLength);
 
-    return keySpecData;
+    return [keySpecData copy];
 }
 
 #pragma mark - Utils

From 461ade93160cf4693f084f15ebdd7cb25b13ccc5 Mon Sep 17 00:00:00 2001
From: Michael Kirk <michael.code@endoftheworl.de>
Date: Wed, 31 Jan 2018 11:00:52 -0800
Subject: [PATCH 4/7] Fix memory leak in key derivation

---
 .../Utilities/YapDatabaseCryptoUtils.m        | 22 +++++++++----------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/YapDatabase/Utilities/YapDatabaseCryptoUtils.m b/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
index 4520229fe..c6a6f842d 100644
--- a/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
+++ b/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
@@ -411,9 +411,12 @@ + (nullable NSData *)deriveDatabaseKeyForPassword:(NSData *)passwordData saltDat
     YapAssert(passwordData.length > 0);
     YapAssert(saltData.length == kSQLCipherSaltLength);
 
-    // FIXME memory leak. manage with NSData.
-    unsigned char *derivedKeyBytes = malloc((size_t)kSQLCipherKeyLength);
-    YapAssert(derivedKeyBytes);
+    NSMutableData *_Nullable derivedKeyData = [NSMutableData dataWithLength:kSQLCipherKeyLength];
+    if (!derivedKeyData) {
+        YapFail(@"failed to allocate derivedKeyData");
+        return nil;
+    }
+
     // See: PBKDF2_ITER from SQLCipher.
     const unsigned int workfactor = 64000;
 
@@ -424,20 +427,15 @@ + (nullable NSData *)deriveDatabaseKeyForPassword:(NSData *)passwordData saltDat
         (size_t)saltData.length,
         kCCPRFHmacAlgSHA1,
         workfactor,
-        derivedKeyBytes,
-        kSQLCipherKeyLength);
+        derivedKeyData.mutableBytes,
+        (size_t)derivedKeyData.length);
+
     if (result != kCCSuccess) {
         YDBLogError(@"Error deriving key: %d", result);
         return nil;
     }
 
-    NSData *_Nullable derivedKeyData = [NSData dataWithBytes:derivedKeyBytes length:kSQLCipherKeyLength];
-    if (!derivedKeyData || derivedKeyData.length != kSQLCipherKeyLength) {
-        YDBLogError(@"Invalid derived key: %d", result);
-        return nil;
-    }
-
-    return derivedKeyData;
+    return [derivedKeyData copy];
 }
 
 + (nullable NSData *)deriveDatabaseKeySpecForPassword:(NSData *)passwordData saltData:(NSData *)saltData

From 56e7869ddfd1c341dd9157caf36e177159135acf Mon Sep 17 00:00:00 2001
From: Michael Kirk <michael.code@endoftheworl.de>
Date: Wed, 31 Jan 2018 11:35:51 -0800
Subject: [PATCH 5/7] CR: revert rename for clarity

---
 YapDatabase/Utilities/YapDatabaseCryptoUtils.h | 2 +-
 YapDatabase/Utilities/YapDatabaseCryptoUtils.m | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/YapDatabase/Utilities/YapDatabaseCryptoUtils.h b/YapDatabase/Utilities/YapDatabaseCryptoUtils.h
index f394b4ac3..f6fc59854 100644
--- a/YapDatabase/Utilities/YapDatabaseCryptoUtils.h
+++ b/YapDatabase/Utilities/YapDatabaseCryptoUtils.h
@@ -10,7 +10,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 extern const NSUInteger kSqliteHeaderLength;
 extern const NSUInteger kSQLCipherSaltLength;
-extern const NSUInteger kSQLCipherKeyLength;
+extern const NSUInteger kSQLCipherDerivedKeyLength;
 extern const NSUInteger kSQLCipherKeySpecLength;
 
 typedef void (^YapDatabaseSaltBlock)(NSData *saltData);
diff --git a/YapDatabase/Utilities/YapDatabaseCryptoUtils.m b/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
index c6a6f842d..0376350d8 100644
--- a/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
+++ b/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
@@ -92,7 +92,7 @@
 
 const NSUInteger kSqliteHeaderLength = 32;
 const NSUInteger kSQLCipherSaltLength = 16;
-const NSUInteger kSQLCipherKeyLength = 32;
+const NSUInteger kSQLCipherDerivedKeyLength = 32;
 const NSUInteger kSQLCipherKeySpecLength = 48;
 
 NSString *const YapDatabaseErrorDomain = @"YapDatabaseErrorDomain";
@@ -169,7 +169,7 @@ + (nullable NSError *)convertDatabaseIfNecessary:(NSString *)databaseFilePath
 
     return [self convertDatabase:databaseFilePath
                 databasePassword:databasePassword
-                       recordSaltBlock:recordSaltBlock];
+                 recordSaltBlock:recordSaltBlock];
 }
 
 + (nullable NSError *)convertDatabase:(NSString *)databaseFilePath
@@ -411,7 +411,7 @@ + (nullable NSData *)deriveDatabaseKeyForPassword:(NSData *)passwordData saltDat
     YapAssert(passwordData.length > 0);
     YapAssert(saltData.length == kSQLCipherSaltLength);
 
-    NSMutableData *_Nullable derivedKeyData = [NSMutableData dataWithLength:kSQLCipherKeyLength];
+    NSMutableData *_Nullable derivedKeyData = [NSMutableData dataWithLength:kSQLCipherDerivedKeyLength];
     if (!derivedKeyData) {
         YapFail(@"failed to allocate derivedKeyData");
         return nil;
@@ -444,7 +444,7 @@ + (nullable NSData *)deriveDatabaseKeySpecForPassword:(NSData *)passwordData sal
     YapAssert(saltData.length == kSQLCipherSaltLength);
 
     NSData *_Nullable derivedKeyData = [self deriveDatabaseKeyForPassword:passwordData saltData:saltData];
-    if (!derivedKeyData || derivedKeyData.length != kSQLCipherKeyLength) {
+    if (!derivedKeyData || derivedKeyData.length != kSQLCipherDerivedKeyLength) {
         YDBLogError(@"Error deriving key");
         return nil;
     }

From 7fb38732b78dc7ba9869fca3184a86b85a5b6b1b Mon Sep 17 00:00:00 2001
From: Michael Kirk <michael.code@endoftheworl.de>
Date: Wed, 31 Jan 2018 12:28:13 -0800
Subject: [PATCH 6/7] Fix parameter assertion

---
 YapDatabase/YapDatabase.m | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/YapDatabase/YapDatabase.m b/YapDatabase/YapDatabase.m
index 4f57ae932..5f5c484c5 100644
--- a/YapDatabase/YapDatabase.m
+++ b/YapDatabase/YapDatabase.m
@@ -835,8 +835,7 @@ - (BOOL)configureEncryptionForDatabase:(sqlite3 *)sqlite
         if (options.cipherKeySpecBlock)
         {
             // Do nothing.
-        } else if (!options.cipherKeyBlock ||
-                   options.cipherSaltBlock) {
+        } else if (!(options.cipherKeyBlock && options.cipherSaltBlock)) {
             NSAssert(NO, @"If you're using YapDatabaseOptions.cipherUnencryptedHeaderLength, you need to set either cipherKeySpecBlock or both cipherKeyBlock and cipherSaltBlock.");
             return NO;
         }

From f71b5be05f896533087ff76d9b1d43c0b89b1c25 Mon Sep 17 00:00:00 2001
From: Michael Kirk <michael.code@endoftheworl.de>
Date: Wed, 31 Jan 2018 16:58:36 -0800
Subject: [PATCH 7/7] Do not proceed with migration if salt was not recorded

---
 YapDatabase/Utilities/YapDatabaseCryptoUtils.h |  9 +++++++--
 YapDatabase/Utilities/YapDatabaseCryptoUtils.m | 10 +++++++---
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/YapDatabase/Utilities/YapDatabaseCryptoUtils.h b/YapDatabase/Utilities/YapDatabaseCryptoUtils.h
index f6fc59854..e5d41dfab 100644
--- a/YapDatabase/Utilities/YapDatabaseCryptoUtils.h
+++ b/YapDatabase/Utilities/YapDatabaseCryptoUtils.h
@@ -13,7 +13,12 @@ extern const NSUInteger kSQLCipherSaltLength;
 extern const NSUInteger kSQLCipherDerivedKeyLength;
 extern const NSUInteger kSQLCipherKeySpecLength;
 
-typedef void (^YapDatabaseSaltBlock)(NSData *saltData);
+// User specified block used to notify the caller of a database's salt when
+// converting SQLCipher headers to plaintext. Failing to properly record the
+// salt will leave the database unreadable.
+// @returns BOOL indicating if the salt was successfully recorded. Conversion
+//          will not proceed if recording the salt fails.
+typedef BOOL (^YapRecordDatabaseSaltBlock)(NSData *saltData);
 
 // This class contains utility methods for use with SQLCipher encrypted
 // databases, specifically to address an issue around database files that
@@ -136,7 +141,7 @@ typedef void (^YapDatabaseSaltBlock)(NSData *saltData);
 //   for this database. Within that block you must store the salt somewhere durable.
 + (nullable NSError *)convertDatabaseIfNecessary:(NSString *)databaseFilePath
                                 databasePassword:(NSData *)databasePassword
-                                 recordSaltBlock:(YapDatabaseSaltBlock)recordSaltBlock;
+                                 recordSaltBlock:(YapRecordDatabaseSaltBlock)recordSaltBlock;
 
 // This method can be used to derive a SQLCipher "key spec" from a
 // database password and salt.  Key spec derivation is somewhat costly.
diff --git a/YapDatabase/Utilities/YapDatabaseCryptoUtils.m b/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
index 0376350d8..4b6584f6d 100644
--- a/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
+++ b/YapDatabase/Utilities/YapDatabaseCryptoUtils.m
@@ -160,7 +160,7 @@ + (BOOL)doesDatabaseNeedToBeConverted:(NSString *)databaseFilePath
 
 + (nullable NSError *)convertDatabaseIfNecessary:(NSString *)databaseFilePath
                                 databasePassword:(NSData *)databasePassword
-                                 recordSaltBlock:(YapDatabaseSaltBlock)recordSaltBlock
+                                 recordSaltBlock:(YapRecordDatabaseSaltBlock)recordSaltBlock
 {
     if (![self doesDatabaseNeedToBeConverted:databaseFilePath]) {
         YDBLogInfo(@"%@ convertDatabaseIfNecessary: database does not need to be converted.", self.logTag);
@@ -174,7 +174,7 @@ + (nullable NSError *)convertDatabaseIfNecessary:(NSString *)databaseFilePath
 
 + (nullable NSError *)convertDatabase:(NSString *)databaseFilePath
                      databasePassword:(NSData *)databasePassword
-                      recordSaltBlock:(YapDatabaseSaltBlock)recordSaltBlock
+                      recordSaltBlock:(YapRecordDatabaseSaltBlock)recordSaltBlock
 {
     YapAssert(databaseFilePath.length > 0);
     YapAssert(databasePassword.length > 0);
@@ -194,7 +194,11 @@ + (nullable NSError *)convertDatabase:(NSString *)databaseFilePath
         // proceeding with the database conversion or we could leave the app in an
         // unrecoverable state.
         YDBLogInfo(@"%@ convertDatabase: salt extracted.", self.logTag);
-        recordSaltBlock(saltData);
+        BOOL success = recordSaltBlock(saltData);
+        if (!success) {
+            YDBLogError(@"Failed to record salt, aborting conversion");
+            return YDBErrorWithDescription(@"Failed to record salt");
+        }
     }
     
     YDBLogInfo(@"%@ convertDatabase: key spec derived.", self.logTag);