stephencelis / SQLite.swift

A type-safe, Swift-language layer over SQLite3.
MIT License
9.57k stars 1.55k forks source link

rekey comments incorrect #1098

Closed R4N closed 1 year ago

R4N commented 2 years ago

I was directed over to this project from an issue raised in the SQLCipher project which was linked to from an issue within your project of similar affect.

I saw that you adjusted the documentation to explicitly call out only using the SQLCipher subspec so as not to cause a conflict with standard SQLite which was a good adjustment (even though your documentation previously correctly only referenced the subspec)

One other thing I noticed in both issue reports, which I responded to in the SQLCipher issue report, is that both users believed that rekey encrypts a plaintext database. When correctly linking SQLCipher this is absolutely not correct. You'll need to use the sqlcipher_export() convenience function.

Specifically the second sentence in the comments here is incorrect:

https://github.com/stephencelis/SQLite.swift/blob/master/Sources/SQLite/Extensions/Cipher.swift#L54-L55

In the SQLCipher specific sqlite3.h you'll notice these extra comments above the rekey functions:

/* SQLCipher usage note:

   If the current database is plaintext SQLCipher will NOT encrypt it.
   If the current database is encrypted and pNew==0 or nNew==0, SQLCipher
   will NOT decrypt it.

   This routine will ONLY work on an already encrypted database in order
   to change the key.

   Conversion from plaintext-to-encrypted or encrypted-to-plaintext should
   use an ATTACHed database and the sqlcipher_export() convenience function
   as per the SQLCipher Documentation.
*/

Here's a example using the sample code from your project to create a plaintext database, call rekey on it, then open it without providing a key to display that its not encrypted:

import UIKit
import SQLite

class ViewController: UIViewController {

    let dbPath = URL(fileURLWithPath: NSSearchPathForDirectoriesInDomains(.documentDirectory, .userDomainMask, true)[0]).appendingPathComponent("test.db").path

    override func viewDidLoad() {
        super.viewDidLoad()
        do {
            let fm = FileManager.default
            if (fm.fileExists(atPath: dbPath)) {
                try FileManager.default.removeItem(atPath: dbPath)
            }
        } catch {
            print("Failed to remove db = \(error)")
        }
        createPlaintextDb()
        rekeyPlaintextDb()
        openDbWithoutKey()
    }

    func createPlaintextDb() {
        do {
            let db = try Connection(dbPath)
            // runtime check -- make sure SQLCipher is properly linked
            let cipherVersion = db.cipherVersion
            if (cipherVersion == nil) {
                print("NOT SQLCIPHER!")
                return
            }
            let users = Table("users")
            let id = Expression<Int64>("id")
            let name = Expression<String?>("name")
            let email = Expression<String>("email")

            try db.run(users.create { t in
                t.column(id, primaryKey: true)
                t.column(name)
                t.column(email, unique: true)
            })
            // CREATE TABLE "users" (
            //     "id" INTEGER PRIMARY KEY NOT NULL,
            //     "name" TEXT,
            //     "email" TEXT NOT NULL UNIQUE
            // )

            let insert = users.insert(name <- "Alice", email <- "alice@mac.com")
            let rowid = try db.run(insert)
            // INSERT INTO "users" ("name", "email") VALUES ('Alice', 'alice@mac.com')

            for user in try db.prepare(users) {
                print("id: \(user[id]), name: \(user[name]), email: \(user[email])")
                // id: 1, name: Optional("Alice"), email: alice@mac.com
            }
            // SELECT * FROM "users"

            let alice = users.filter(id == rowid)

            try db.run(alice.update(email <- email.replace("mac.com", with: "me.com")))
            // UPDATE "users" SET "email" = replace("email", 'mac.com', 'me.com')
            // WHERE ("id" = 1)

            try db.run(alice.delete())
            // DELETE FROM "users" WHERE ("id" = 1)

            try db.scalar(users.count) // 0
            // SELECT count(*) FROM "users"
            sqlite3_close(db.handle);
        } catch {
            print (error)
        }
    }

    func rekeyPlaintextDb() {
        do {
            let db = try Connection(dbPath)
            try db.rekey("t3stp4ssReKey")
            let count = try db.scalar("SELECT COUNT(*) FROM sqlite_schema;")
            print("count after rekey = \(count ?? "No Count")")
            sqlite3_close(db.handle)
        } catch {
            print (error)
        }
    }

    func openDbWithoutKey() {
        do {
            let db = try Connection(dbPath)
            // explicitly don't key the db and see that you'll be able to access it without encryption as rekey doesn't encrypt a plaintext db when using SQLCipher
            let count = try db.scalar("SELECT COUNT(*) FROM sqlite_schema;")
            print("count when opening db again without key = \(count ?? "No Count")")
            sqlite3_close(db.handle)
        } catch {
            print(error)
        }
    }
}

Which outputs:

id: 1, name: Optional("Alice"), email: alice@mac.com
count after rekey = 2
count when opening db again without key = 2

Notice that the openDbWithoutKey() was able to access the database without a key because it wasn't encrypted when calling rekey on a plaintext db.

jberkel commented 2 years ago

Thanks for pointing this out, I'll change the comments and clarify this in the docs. It might also be worth to expose sqlcipher_export() in the API of SQLite.swift/SQLCipher.

But I'm wondering, is there a reason why rekey doesn't raise an error when called on an unencrypted db? Seems like an obvious user error to catch.

R4N commented 2 years ago

It might also be worth to expose sqlcipher_export() in the API of SQLite.swift/SQLCipher.

That makes sense to me so it provides your lib's users a streamlined way of going from plaintext <> encrypted

But I'm wondering, is there a reason why rekey doesn't raise an error when called on an unencrypted db? Seems like an obvious user error to catch

That is a fair point, rekey just "does nothing" (and returns SQLITE_OK) when called with a key and the open database is not encrypted. Attempting to key the db on subsequent opens with the same key that was provided during rekey will fail with error though. That being said, we think it might be a prudent to return an error in this scenario.

Going the other direction (attempting to rekey with an empty string when the open db is already encrypted) does fail with SQLITE_ERROR already.