Cannot set key again if having removed key

[einhugur]

I am unable to set key again if I Set Key then remove key. Then open the DB again with no key, then neither SetKey or ReKey will work again. (Key is successfully removed as DB Browser for Sqlite can open it at that time)

[resilar]

SetKey on a non-empty database is not supposed to succeed, but ReKey should.

I'm unable to reproduce. Are you using the latest version? Release or git master? Do you set & remove the key using DB Browser, shell or C interface? In addition, .dbinfo output of the database before setting the key first time and after removing the key could be useful.

$ rm -f db
$ ./sqleet 'file:db?key=swordfish' "CREATE TABLE f(x,y)"
$ ./sqleet 'file:db?key=swordfish' "INSERT INTO f VALUES(4,2)"
$ xxd db | head -n1
00000000: 0a98 87dd ae61 7903 e4a8 5d99 c557 0b64  .....ay...]..W.d
$ ./sqleet 'file:db?key=swordfish' "PRAGMA rekey=''" && xxd db | head -n1                                 
00000000: 5351 4c69 7465 2066 6f72 6d61 7420 3300  SQLite format 3.
$ ./sqleet db "PRAGMA rekey='GOD'" 
ok
$ xxd db | head -n1
00000000: af20 0557 5c86 297d af85 ed08 3e75 4983  . .W\.)}....>uI.
$ ./sqleet 'file:db?key=GOD' .dump
PRAGMA foreign_keys=OFF;
BEGIN TRANSACTION;
CREATE TABLE f(x,y);
INSERT INTO f VALUES(4,2);
COMMIT;

[einhugur]

I was using latest release, and the C interface. ReKey works fine, I only get the issue when having done ReKey that removes the key and then put key again on it. I will play a bit more with it and try not using SetKey again in between and see what happens. Thanks.

[rocketcyberdev]

I am having a similar problem using PRAGMA statements.
I have a db that was previously encrypted. I rekeyed it to remove the key. Now when I try to rekey again to add the password its not encrypting.

I've stepped through the code and it appears that when sqlite3_rekey_v2 is calling codec_set_to the codec->pagesize is bunk. it contains a value of : -842150451

Any thoughts?

[resilar]

Thanks for the details @rocketcyberdev

I'm still unable to reproduce the actual sqlite3_rekey_v2() error, but I succeeded to trigger the following (presumably related) valgrind error:

sqlite> PRAGMA rekey='';
sqlite> PRAGMA rekey='GOD';
==55348== Conditional jump or move depends on uninitialised value(s)
==55348==    at 0x1D97FE: codec_set_to (sqleet.c:451)
==55348==    by 0x1D9E89: sqlite3_rekey_v2 (sqleet.c:613)
==55348==    by 0x19303F: sqlite3Pragma (sqlite3.c:123854)
==55348==    by 0x1C577F: yy_reduce (sqlite3.c:152913)
==55348==    by 0x1C6B5C: sqlite3Parser (sqlite3.c:153461)
==55348==    by 0x1C7DCC: sqlite3RunParser (sqlite3.c:154597)
==55348==    by 0x194E0B: sqlite3Prepare (sqlite3.c:124831)
==55348==    by 0x195161: sqlite3LockAndPrepare (sqlite3.c:124924)
==55348==    by 0x19535A: sqlite3_prepare_v2 (sqlite3.c:125008)
==55348==    by 0x1EB488: shell_exec (shell.c:11585)
==55348==    by 0x1F952B: runOneSqlLine (shell.c:18110)
==55348==    by 0x1F9A44: process_input (shell.c:18210)
==55348== 

445 static int codec_set_to(Codec *codec, Btree *pBt)
446 ...
451         if (!codec->pagesize)
452             codec->pagesize = sqlite3BtreeGetPageSize(pBt);

Indeed, codec->pagesize is uninitialized since commit ff10548 (Implement URI API). Commit 4a6f062 adds the missing initialization of codec->pagesize, likely fixing this rekeying issue. Closing, please reopen if the commit does not resolve this issue.

Highly appreciated if @einhugur or @rocketcyberdev could test the fix and report back.

---

I also updated the v0.29.0 release/tag of sqleet to include the fix given the seriousness of the bug. Now thinking about it, this was a little unprofessional because tags should not abruptly change. In future, a better protocol is needed for releasing bugfix releases without "rewriting history".

[rocketcyberdev]

Hi @resilar thanks for this. It appears to have solved the problem we reported.

[resilar]

Great!

[rocketcyberdev]

Hi Resliar Are there any known issues with using sqleet in a multi threaded app? I’ve got an app that has multiple threads trying to read/write to db. It appears that even when I use the KEY pragma after opening the db, some of the threads will get the not a db error. Just wondering if it could be some sort of read/write collision between threads. They are not sharing a db handle. Each thread opens the db separately. From: resilar <notifications@github.com> Reply-To: resilar/sqleet <reply@reply.github.com> Date: Tuesday, September 17, 2019 at 9:13 AM To: resilar/sqleet <sqleet@noreply.github.com> Cc: Developer <developer@rocketcyber.com>, Mention <mention@noreply.github.com> Subject: Re: [resilar/sqleet] Cannot set key again if having removed key (#26) Thanks for the details @rocketcyberdev<https://github.com/rocketcyberdev> I'm still unable to reproduce the actual sqlite3_rekey_v2() error, but I succeeded to trigger the following (presumably related) valgrind error: sqlite> PRAGMA rekey=''; sqlite> PRAGMA rekey='GOD'; ==55348== Conditional jump or move depends on uninitialised value(s) ==55348== at 0x1D97FE: codec_set_to (sqleet.c:451) ==55348== by 0x1D9E89: sqlite3_rekey_v2 (sqleet.c:613) ==55348== by 0x19303F: sqlite3Pragma (sqlite3.c:123854) ==55348== by 0x1C577F: yy_reduce (sqlite3.c:152913) ==55348== by 0x1C6B5C: sqlite3Parser (sqlite3.c:153461) ==55348== by 0x1C7DCC: sqlite3RunParser (sqlite3.c:154597) ==55348== by 0x194E0B: sqlite3Prepare (sqlite3.c:124831) ==55348== by 0x195161: sqlite3LockAndPrepare (sqlite3.c:124924) ==55348== by 0x19535A: sqlite3_prepare_v2 (sqlite3.c:125008) ==55348== by 0x1EB488: shell_exec (shell.c:11585) ==55348== by 0x1F952B: runOneSqlLine (shell.c:18110) ==55348== by 0x1F9A44: process_input (shell.c:18210) ==55348== 445 static int codec_set_to(Codec *codec, Btree *pBt) 446 ... 451 if (!codec->pagesize) 452 codec->pagesize = sqlite3BtreeGetPageSize(pBt); Indeed, codec->pagesize is uninitialized since commit ff10548<ff10548> (Implement URI API). Commit 4a6f062<4a6f062> adds the missing initialization of codec->pagesize, likely fixing this rekeying issue. Closing, please reopen if the commit does not resolve this issue. Highly appreciated if @einhugur<https://github.com/einhugur> or @rocketcyberdev<https://github.com/rocketcyberdev> could test the fix and report back.

…

________________________________ I also updated the v0.29.0 release/tag of sqleet to include the fix given the seriousness of the bug. Now thinking about it, this was a little unprofessional because tags should not abruptly change. In future, a better protocol is needed for releasing bugfix releases without "rewriting history". — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#26?email_source=notifications&email_token=AJD55TRNCPAECRK44OYSAPTQKDQWRA5CNFSM4ISEG4AKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD64VJTA#issuecomment-532239564>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AJD55TURMVYGBQITVXRCCNLQKDQWRANCNFSM4ISEG4AA>.

[resilar]

Interesting. To be honest, I have not tested multi-threaded setup very much. However, there should be no problem since sqleet's sqlite3_key_v2 & sqlite3_rekey_v2 are protected by the database mutex. SQLITE_NOTADB (assuming it's not coming from sqlite3 code) is returned by verify_page1() which is called holding the database mutex in addition to the pager shared lock, so there should be no synchronization issues.

Have you tried compiling sqleet explicitly with -DSQLITE_THREADSAFE=1 or -DSQLITE_THREADSAFE=2? -DSQLITE_THREADSAFE=0 disables mutexes and makes concurrent use of SQLite3 (and sqleet) unsafe.

They are not sharing a db handle. Each thread opens the db separately.

This might be the culprit. https://www.sqlite.org/threadsafe.html says:

---

SQLite supports three different threading modes:

• Single-thread. In this mode, all mutexes are disabled and SQLite is unsafe to use in more than a single thread at once.
• Multi-thread. In this mode, SQLite can be safely used by multiple threads provided that no single database connection is used simultaneously in two or more threads.
• Serialized. In serialized mode, SQLite can be safely used by multiple threads with no restriction.

---

Any idea which mode you are using?

[rocketcyberdev]

Thanks Resilar, Yes we have SQLITE_THREADSAFE=1 (Serialized) set at compile time. I will go back and do some additional testing to make sure that we’re not somehow accidentally opening the db without the key pragma somewhere in the code path or that some runtime db open is overriding the threadsafe setting. From: resilar <notifications@github.com> Reply-To: resilar/sqleet <reply@reply.github.com> Date: Thursday, September 19, 2019 at 2:42 AM To: resilar/sqleet <sqleet@noreply.github.com> Cc: Developer <developer@rocketcyber.com>, Mention <mention@noreply.github.com> Subject: Re: [resilar/sqleet] Cannot set key again if having removed key (#26) Interesting. To be honest, I have not tested multi-threaded setup very much. However, there should be no problem since sqleet's sqlite3_key_v2 & sqlite3_rekey_v2 are protected by the database mutex. SQLITE_NOTADB (assuming it's not coming from sqlite3 code) is returned by verify_page1() which is called holding the database mutex in addition to the pager shared lock, so there should be no synchronization issues. Have you tried compiling sqleet explicitly with -DSQLITE_THREADSAFE=1 or -DSQLITE_THREADSAFE=2? -DSQLITE_THREADSAFE=0 disables mutexes and makes concurrent use of SQLite3 (and sqleet) unsafe. They are not sharing a db handle. Each thread opens the db separately. This might be the culprit. https://www.sqlite.org/threadsafe.html says:

…

________________________________ SQLite supports three different threading modes: * Single-thread. In this mode, all mutexes are disabled and SQLite is unsafe to use in more than a single thread at once. * Multi-thread. In this mode, SQLite can be safely used by multiple threads provided that no single database connection is used simultaneously in two or more threads. * Serialized. In serialized mode, SQLite can be safely used by multiple threads with no restriction.

________________________________ Any idea which mode you are using? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#26?email_source=notifications&email_token=AJD55TX4AIVZ2Y32JP2ORNDQKMUPBA5CNFSM4ISEG4AKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7CRIIY#issuecomment-533009443>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AJD55TSUHRYEIFWJHZ33J3LQKMUPBANCNFSM4ISEG4AA>.

[resilar]

Now thinking about it, a shared pager lock in verify_page1() might not be enough because the function clears the page cache to force reading the first page from the disk. sqlite3PagerExclusiveLock(pager) is maybe worth trying.

I'll look into this and write some concurrency tests when I have time.

[rocketcyberdev]

Sounds good thanks.

…

Sent from my iPhone On Sep 20, 2019, at 2:16 AM, resilar <notifications@github.com<mailto:notifications@github.com>> wrote: Now thinking about it, a shared pager lock in verify_page1() might not be enough because the function clears the page cache to force reading the first page from the disk. sqlite3PagerExclusiveLock(pager) is maybe worth trying. I'll look into this and write some concurrency tests when I have time. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#26?email_source=notifications&email_token=AJD55TTB5PCSVDO3SBUMII3QKR2FJA5CNFSM4ISEG4AKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7F2ETI#issuecomment-533439053>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AJD55TXCVZC2AY4RQYLHKMTQKR2FJANCNFSM4ISEG4AA>.

[resilar]

So far I have written a multi-threaded concurrency smoke test program that continuously (re)opens and reads an encrypted sqleet database from dozens of threads. This has not reproduced the SQLITE_NOTADB error, contrary to my expectations, assuming a concurrency bug in verify_page1(). I'll add writer threads probably later this week to see if concurrent writes triggers the error.