Repository navigation
Re-read pages through the codec when rekeying instead of trusting the cache - #636
LucaCappelletti94 wants to merge 1 commit into
Conversation
|
Hello @LucaCappelletti94 can you provide some background on this PR? Assuming there is data in page cache, it is already verified. If the page was changed by another legitimate connection it would be reloaded anyway. What is the reason that we'd want to discard known good data and raise an error? |
|
The cached copy is authentic, and after rekey the file holds the original content again, the thing that gets lost is the report. When a page's ciphertext changes on disk outside SQLite (I found this while testing whether I could sync the file between native and browser), the change counter stays the same, so the cache is not reset and rekey never reads those bytes. Rekey then writes fresh HMACs under the new key over the whole file, which removes the last evidence (I believe) the old HMAC could have flagged. From then on The outcome also depends on what happened to be cached. With the same tampered file, rekey fails if the page was not read beforehand and succeeds if it was. Clearing the cache makes rekey authenticate every page it rewrites, like any other read under the current key. My understanding is that the cost is one extra read of the pages that were cached, inside a loop that already reads and rewrites every page in the file. |
Changes proposed in this pull request:
sqlite3_rekey_v2 rewrites every page by fetching it with sqlite3PagerGet, which returns a page already in the pager cache without passing it through the codec again. If a page was read before rekey is called and its ciphertext is then changed on disk, rekey never reads or authenticates those bytes. It re-encrypts the cached copy under the new key and returns SQLITE_OK. The database then opens under the new key and passes cipher_integrity_check, and the old key no longer opens it, so the tampering is never reported.
This change clears the pager cache at the start of rekey, right after the existing check that refuses rekey inside a transaction. Every page in the loop is then read from disk and checked by the codec under the current key, so a tampered page makes rekey fail and roll back. The clear sits before the data version is captured, so it does not trip the check that aborts when another connection changed the database.
The new test caches a table, flips one byte of a page on disk, and checks that rekey fails, the new key cannot open the file, and the old key still can.
On beta without the change, rekey reports ok and only the new key works.
The same problem exists on prerelease too (in an extended form which is partially fixed in beta), but I opened it directly on beta as I imagine this is were your efforts are focused now.