Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rekey comments incorrect #1098

Closed
R4N opened this issue Nov 17, 2021 · 2 comments · Fixed by #1143
Closed

rekey comments incorrect #1098

R4N opened this issue Nov 17, 2021 · 2 comments · Fixed by #1143
Milestone

Comments

@R4N
Copy link

R4N commented Nov 17, 2021

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 <- "[email protected]")
            let rowid = try db.run(insert)
            // INSERT INTO "users" ("name", "email") VALUES ('Alice', '[email protected]')

            for user in try db.prepare(users) {
                print("id: \(user[id]), name: \(user[name]), email: \(user[email])")
                // id: 1, name: Optional("Alice"), email: [email protected]
            }
            // 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: [email protected]
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
Copy link
Collaborator

jberkel commented Nov 17, 2021

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.

@jberkel jberkel added this to the 0.13.2 milestone Nov 17, 2021
@R4N
Copy link
Author

R4N commented Nov 17, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants