m4heshd / better-sqlite3-multiple-ciphers

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

How do you suggest rekeying with WAL? #64

Closed coltoneshaw closed 10 months ago

coltoneshaw commented 10 months ago

Right now I have journal_mode = WAL set. You cannot run a rekey when this is set. Would you suggest doing journal_mode = delete, rekey, then enabling wal ?

m4heshd commented 10 months ago

I'm not sure if just running a checkpoint would help with this. That's all that came to my mind at the moment. Unfortunately, I'm away at the moment. Can't test anything. Also for the sake of having better knowledge on this, I'd feel much better if @utelle answered this.

utelle commented 10 months ago

Right now I have journal_mode = WAL set. You cannot run a rekey when this is set. Would you suggest doing journal_mode = delete, rekey, then enabling wal ?

Short answer: Yes.

Long answer: The current implementation of the rekeying operation in the underlying SQLite3 Multiple Ciphers library tries to rekey a database in-place if possible, thus keeping the disk storage requirements to a minimum. However, this does not work if WAL journaling is used and could easily lead to database corruption. In principle, rekeying via a vacuum operation could possibly work in WAL mode, if certain restrictions are not violated. However, to be on the safe side SQLite3 Multiple Ciphers currently does not support rekeying in WAL mode and issues an error message. That is, to rekey an encrypted database you need to switch temporarily to journal mode DELETE.

coltoneshaw commented 10 months ago

Long answer: The current implementation of the rekeying operation in the underlying SQLite3 Multiple Ciphers library tries to rekey a database in-place if possible, thus keeping the disk storage requirements to a minimum. However, this does not work if WAL journaling is used and could easily lead to database corruption. In principle, rekeying via a vacuum operation could possibly work in WAL mode, if certain restrictions are not violated. However, to be on the safe side SQLite3 Multiple Ciphers currently does not support rekeying in WAL mode and issues an error message. That is, to rekey an encrypted database you need to switch temporarily to journal mode DELETE.

This is perfect! Thank you! The journal switch operation seems to be fairly fast as well, so no big issues there. What would it look like to do it via a vacuum?

utelle commented 10 months ago

What would it look like to do it via a vacuum?

  1. Independent of whether the database is encrypted or not, vacuuming a database in WAL mode requires free disk space of about twice the database size. In DELETE mode only about once the database size is required.
  2. As said SQLite3 Multiple Ciphers does currently not support rekeying in WAL mode. You could use VACUUM INTO to create a copy of the original database with a different key, but that's not the same as rekeying the original database.
  3. Maybe it is possible to change the implementation of the rekey operation in SQLite3 Multiple Ciphers, so that it uses VACUUM in WAL mode. However, there are additional restrictions: the cipher scheme must be the same as that of the original database and the page size must not be changed.
m4heshd commented 10 months ago

Thank you @utelle. Closing this as solved.

titanism commented 2 months ago

Following up here – @utelle @m4heshd @coltoneshaw what is the recommended approach (including pragma operations/commands)? We're seeing edge cases where our approach with changing a key isn't working (and no errors are being thrown).

const db = new Database('...');
db.pragma('cipher=chacha20');
db.pragma(`key="${decrypt(session.user.password)}"`);

// overwrite deleted content with zeros
// <https://www.sqlite.org/pragma.html#pragma_secure_delete>
db.pragma('secure_delete=ON');

//
// NOTE: we still run a manual vacuum every 24 hours
//
// turn on auto vacuum (for large amounts of deleted content)
// <https://www.sqlite.org/pragma.html#pragma_auto_vacuum>
//
db.pragma('auto_vacuum=FULL');

// <https://litestream.io/tips/#busy-timeout>
db.pragma(`busy_timeout=${config.busyTimeout}`);

// <https://litestream.io/tips/#synchronous-pragma>
db.pragma('synchronous=NORMAL');

// <https://s3.amazonaws.com/bizzabo.file.upload/XTUmoCXSCfit8PqUs2AT_B%20Johnson%20-%20Building%20Production%20Applications%20Using%20Go%20%26%20SQLite.pdf>
db.pragma('foreign_keys=ON');

// <https://www.sqlite.org/pragma.html#pragma_encoding>
db.pragma(`encoding='UTF-8'`);

db.pragma('journal_mode=WAL');
db.pragma('wal_checkpoint(PASSIVE)');

const journalModeResult = db.pragma('journal_mode=DELETE');
if (
  !Array.isArray(journalModeResult) ||
  journalModeResult.length !== 1 ||
  !journalModeResult[0] ||
  typeof journalModeResult[0] !== 'object' ||
  journalModeResult[0].journal_mode !== 'delete'
)
  throw new TypeError('Journal mode could not be changed');

db.pragma(`rekey="${decrypt(payload.new_password)}"`); // <----- IS THE PROBLEM HERE?
db.pragma('journal_mode=WAL'); // <----- IS THE PROBLEM HERE?

// <https://github.com/m4heshd/better-sqlite3-multiple-ciphers/issues/23#issuecomment-1152634207>
db.prepare('VACUUM').run();

db.pragma('optimize');
db.close();
titanism commented 2 months ago

Rekey actually seems to corrupt the database, whereas the old and new password (keys) are not working.

titanism commented 2 months ago

It looks like I was missing a db.prepare('VACUUM').run(); after changing journal mode to delete (in order for change to persist). Then the rekey operation seems to succeed. Otherwise it seems to leave the database in a corrupt state.

titanism commented 2 months ago

Related https://github.com/tursodatabase/libsql/issues/976

titanism commented 2 months ago

Another issue I had was that I was switching back to WAL mode and then closing the db, and then renaming the db file without also realizing that the -whm and -shm files were still there, and so when the DB file was opened again, it didn't see the -whm and shm files (since they weren't of same file name due to the fs.promises.rename call).

m4heshd commented 2 months ago

It looks like I was missing a db.prepare('VACUUM').run();

This is always advised if you're doing a journal mode change.

renaming the db file without also realizing that the -whm and -shm files were still there

If you're properly closing all connections (which should trigger checkpoints), these journal files should go away. I have dedicated logic on all of my applications to check for those files before messing with the database files on storage.