m4heshd / better-sqlite3-multiple-ciphers

better-sqlite3 with multiple-cipher encryption support ๐Ÿ”’
MIT License
137 stars 27 forks source link

file is not a database #98

Closed KotoriMinami closed 4 weeks ago

KotoriMinami commented 1 month ago

I am using this library in an Electron environment, and the usage is as follows:

this.instance = new SQliteDatabase(path, {
  verboze: console.log
});

if (this.secretKey) {
  this.instance.key(Buffer.from(this.secretKey));
}

This code runs every time the application starts.

Assuming the encrypted DB file exists and contains data, it generally works fine most of the time.

However, on rare occasions, the following error occurs:

{ code: 'SQLITE_NOTADB' }

I can confirm that the secretKey and the DB file are the same each time, and usually, restarting the application once resolves the issue.

I'm not sure what is causing this. Could you provide some debugging ideas or insights?

m4heshd commented 1 month ago

Can you please link a minimal reproduction repo for this issue? It seems like you're using an ORM and I'm not sure how to recreate this.

KotoriMinami commented 1 month ago

In fact, I am unable to reproduce this issue because it occurs probabilistically.

According to the application logs, the input database path and key are the same.

The process simplified from the ORM source code is as fo

const SQliteDatabase = require('better-sqlite3-multiple-ciphers');

this.instance = new SQliteDatabase(path, {
    verboze: console.log
});

this.instance.pragma("key='".concat(this.secretKey, "'"));

this.instance.pragma('user_version', {
    simple: true,
})

When the execution reaches user_version, the program throws an exception:

SqliteError: file is not a database.

I am attempting to reproduce it through various methods and will reopen this issue when I manage to.

KotoriMinami commented 1 month ago

I created a repository and wrote a minimal implementation. Based on this code, It will initialize the database when the application starts. I have restarted the application many times but have only managed to reproduce the issue once. It requires some patience. The repository address is as follows:

https://github.com/KotoriMinami/reproduce

This issue has been troubling me for many days....๐Ÿ˜ญ๐Ÿ˜ญ๐Ÿ˜ญ

KotoriMinami commented 1 month ago

image Under the same conditions, only one error occurred๐Ÿ˜‚๐Ÿ˜‚

utelle commented 1 month ago

Assuming the encrypted DB file exists and contains data, it generally works fine most of the time.

If the encrypted database file exists and already contains data, the error SQLITE_NOTADB should never happen.

However, if a new empty database is created, there is a chance to get this error, if 2 (or more) connections to the same database file are established. The reason is that both connections see an empty database file and therefore generate different random data used as cipher salt (which is stored in the header of the database file).

No secondary connection should be established, until the new database was properly initialized (schema created and written to disk).

I can confirm that the secretKey and the DB file are the same each time, and usually, restarting the application once resolves the issue.

Since the database file already exists and is not empty, the error will not occur again.

I'm not sure what is causing this. Could you provide some debugging ideas or insights?

Check whether a secondary connection is established, while a new still empty database file is created. Make sure that no secondary connection is established, until a new empty database file is fully initialized.

One way to force SQLite to write the database header to disk is to execute the SQL command

PRAGMA user_version=0;

or

VACUUM;

after the passphrase has been set with PRAGMA key.

KotoriMinami commented 1 month ago

Thank you for your response. I've also reviewed the ORM source code and need to add a few points:

  1. After the database file is created, it sets PRAGMA user_version=xxx. In the example, you can see the relevant code in setUpWithMigrationsand setUpWithSchema in src/@nozbe/watermelondb/adapters/sqlite/sqlite-node/DatabaseDriver.js. You can also see the output by adding a statement in better-sqlite3-multiple-ciphers/lib/methods/pragma.js. The initial database creation statement is as follows:
    PRAGMA key='NihQ4gszzg4acvGXsLuo7%CRw@EH7tzWB6NE' result: [ { ok: 'ok' } ]
    PRAGMA user_version result: 0
    PRAGMA key='NihQ4gszzg4acvGXsLuo7%CRw@EH7tzWB6NE' result: [ { ok: 'ok' } ]
    PRAGMA key='NihQ4gszzg4acvGXsLuo7%CRw@EH7tzWB6NE' result: [ { ok: 'ok' } ]
    PRAGMA user_version = 7 result: []
    PRAGMA user_version result: 7
    PRAGMA user_version result: 7

    After closing the application and reconnecting, the output is:

    PRAGMA key='NihQ4gszzg4acvGXsLuo7%CRw@EH7tzWB6NE' result: [ { ok: 'ok' } ]
    PRAGMA user_version result: 7
  2. When there is data in the database, the SQLITE_NOTADB error occurs randomly every time the application starts. This issue does not resolve after a single successful connection and continues to appear randomly in subsequent application startups.
  3. Before the errors occur, the database has already been created and contains data.
utelle commented 1 month ago
  1. After the database file is created, it sets PRAGMA user_version=xxx.

In principle, this is a good approach, because it forces SQLite to write the database schema to disk, thus avoiding an empty zero-bytes database file (in case of a fresh empty database).

In the example, you can see the relevant code in setUpWithMigrationsand setUpWithSchema in src/@nozbe/watermelondb/adapters/sqlite/sqlite-node/DatabaseDriver.js. You can also see the output by adding a statement in better-sqlite3-multiple-ciphers/lib/methods/pragma.js.

Sorry, I'm not a user of better-sqlite3-multiple-ciphers, but the developer of the underlying SQLite implementation with encryption support. That is, you will have to do the debugging at the JS level yourself.

The initial database creation statement is as follows:

PRAGMA key='NihQ4gszzg4acvGXsLuo7%CRw@EH7tzWB6NE' result: [ { ok: 'ok' } ]
PRAGMA user_version result: 0
PRAGMA key='NihQ4gszzg4acvGXsLuo7%CRw@EH7tzWB6NE' result: [ { ok: 'ok' } ]
PRAGMA key='NihQ4gszzg4acvGXsLuo7%CRw@EH7tzWB6NE' result: [ { ok: 'ok' } ]
PRAGMA user_version = 7 result: []
PRAGMA user_version result: 7
PRAGMA user_version result: 7

It is unclear whether all those PRAGMA statements are executed in the same database connection. If yes, I don't understand why PRAGMA key is executed several times. Usually, PRAGMA key should be executed only once, immediately after opening the database file. If necessary, only PRAGMA statements for configuring the cipher scheme should be executed before issuing PRAGMA key.

After closing the application and reconnecting, the output is:

PRAGMA key='NihQ4gszzg4acvGXsLuo7%CRw@EH7tzWB6NE' result: [ { ok: 'ok' } ]
PRAGMA user_version result: 7

This looks ok.

  1. When there is data in the database, the SQLITE_NOTADB error occurs randomly every time the application starts. This issue does not resolve after a single successful connection and continues to appear randomly in subsequent application startups.

Now, this is really strange, because establishing a connection to a non-empty encrypted database should always succeed, as long as the right passphrase was given.

Actually, the error SQLITE_NOTADB can occur for a non-empty database file only under the following conditions:

  1. The database file is actually encrypted and a read operation (like PRAGMA user_version) takes place before PRAGMA key was executed. The read operation will fail.
  2. The database file is actually encrypted and PRAGMA key was executed with the wrong passphrase. Any operation on the database will fail.
  3. The database file is actually not encrypted and PRAGMA key was executed. A read operation (like PRAGMA user_version) will fail.

Conclusion: Make sure that the database file is actually encrypted, and make sure that no database read/write operation is executed before PRAGMA key was executed.

  1. Before the errors occur, the database has already been created and contains data.

Good. This eliminates the case of an empty zero-bytes database file.

Assuming that there is only a single database connection, I suspect that a database read (like PRAGMA user_version) can possibly occur before PRAGMA key was executed, causing the error SQLITE_NOTADB.

m4heshd commented 1 month ago

@KotoriMinami I just checked your reproduction repo but that's not in any way a minimal reproduction. I have to go through your whole project and the functionality of multiple dependencies to get to the bottom of it. Unfortunately, that's not an amount of time I can afford at this moment. You have to somehow recreate this issue in a very minimal way to get some assistance.

I'll tell you one thing I see at first glance though. It seems like you're running the DB tasks multi-threaded which will heavily complicate your setup. You have to be extremely careful with what you do using each thread. I'm pretty sure you're running into this issue intermittently because your application is using way too many threads and in the middle of all that, your DB is busy while the next thread is trying to decrypt it.

I advise limiting the number of threads to the number of available CPU threads. Only use the threads to run read operations. All other operations should be run using a single connection in your parent process, which also should be Electron's main process. This is the method I personally use on my massive commercial projects and they have never failed.

Thank you for your input on this as well @utelle.

utelle commented 1 month ago

I'll tell you one thing I see at first glance though. It seems like you're running the DB tasks multi-threaded which will heavily complicate your setup.

In principle, the SQLite API can be called from multiple threads without problems. However, it can depend on the settings for the threading mode. The default threading mode is serialized (see SQLite documentation).

You have to be extremely careful with what you do using each thread. I'm pretty sure you're running into this issue intermittently because your application is using way too many threads and in the middle of all that, your DB is busy while the next thread is trying to decrypt it.

If each thread uses its own database connection, there should not be a problem in accessing existing non-empty encrypted database files. However, if the same connection is used from more than one thread and the threaded functions are not implemented properly, it can happen that PRAGMA key was not yet executed for the database connection, resulting in the observed error.

KotoriMinami commented 4 weeks ago

@m4heshd Bro, you are right. I found that the multithreading didn't work as expected. I have changed it to run in a single thread and will observe it for a while. @utelle @m4heshd Thank you both very much for your help. Thanks a lot!