m4heshd / better-sqlite3-multiple-ciphers

better-sqlite3 with multiple-cipher encryption support 🔒
MIT License
151 stars 27 forks source link

Add key and rekey function to DB which accepts key as a Buffer #41

Closed mivtdole closed 1 year ago

mivtdole commented 1 year ago

In a project we have the key (password) used to encrypt the database is a byte array which does not transfer in a good way to a string to use in the pragma function.

Our case is using the "aes256cbc" cipher which does not accept a hex string in the key pragma. With this new " key" function you can give it a Buffer so we do not get strange string.

Example:

dbHandle.pragma(cipher='aes256cbc') dbHandle.pragma(key='${password}') dbHandle.exec("select * from sqlite_schema"); dbHandle.close()

* New style:

const password = "aPassword"; const db = "test.db"; dbHandle = new sqlite(db, { readonly: true, fileMustExist: true })

dbHandle.pragma(cipher='aes256cbc') dbHandle.key(Buffer.from(password)); dbHandle.exec("select * from sqlite_schema"); dbHandle.close()

mivtdole commented 1 year ago

I allready thought to add the rekey function also. I will add it.

I will add the compiled hpp and cpp files and the extra tests. Will also look into the documententation.

I tried a lot of different ways to keep it as a string but no luck for the cipher being used. Some keys have "ugly" byte combinations. But having a function like the API (sqlite3_key) which just accepts a byte array independant of the bytes in the key is better.

A new pull will follow shortly.

mivtdole commented 1 year ago

Added all requested missing "nuts and bolts".

m4heshd commented 1 year ago

Updates looking good. But the tests seem to be failing and some tests are even hung. It would be preferable if you run tests locally first. Also, the new tests seem to be a little cluttered. Multiple tests are being done in a single node. You can remove the beforeEach() and use this.db = new Database(util.current()) to use the same DB for multiple test nodes. Could you take a look?

mivtdole commented 1 year ago

I have done the test locally first before pushing them. On my Ubuntu 22.04 with NodeJS v18.12.1 it worked.. I will have a look why these are failing. I can clean the tests so they only test at the end of the individual test.

mivtdole commented 1 year ago

Ok I am able to reproduce the error. It only happens when compiled in debug mode (npm run build-debug) when building in release mode and then running test they do not fail.

mivtdole commented 1 year ago

I also have a fix. It works when I skip using the wrapper function "ThrowDatabaseError" and use the non-wrapped function "ThrowSqliteError" directly. Will push shortly.

mivtdole commented 1 year ago

Pushed an update

m4heshd commented 1 year ago

It only happens when compiled in debug mode (npm run build-debug) when building in release mode and then running test they do not fail.

That's how it's supposed to work because that's how the underlying native library gets tested. SQLite provides the debug mode exactly for that. That's why we compile it in that mode to run tests.

Running the tests. I highly advise you NOT to force-push commits because the maintainers are unable to see the exact changes you did to resolve issues in each step. It's alright even if you push 100 commits because they'll be squashed to a single commit at the merge.

m4heshd commented 1 year ago

Everything LGTM. Thank you @mivtdole for improving this library. Merging.