Wipe temporary copies of key material

[mtrossbach]

Hi Ulrich,

this is the next hardening step, with the same background as #267 and #268. It only adds calls to your sqlite3mcSecureZeroMemory().

Type: Enhancement (security hardening)

The problem

The cipher structures are already wiped when they are freed. But some temporary copies of keys stay in memory until something else overwrites them, so they can end up in memory dumps, crash reports or swap:

• PRAGMA hexkey, PRAGMA hexrekey and the URI parameter hexkey decode the key into a buffer that is freed without wiping it.
• PBKDF2 and HMAC in fastpbkdf2.c leave the padded key, the intermediate values and the keyed hash state on the stack. sqlcipher_hmac() does this for every page.
• The VLE context is freed with sqlite3_free(), including its key.
• sqlite3mcCodecTerm() clears the codec with memset(), which the compiler may remove because the memory is freed right after.

The change

sqlite3mcSecureZeroMemory() in these places: 25 added lines in 4 files. fastpbkdf2.c needs a declaration because it is included before memory_secure.c.

SQLCipher does the same: it overwrites key and passphrase buffers when it frees them (sqlcipher_free() in src/sqlcipher.c).

Not included: the key schedules that the AES and ChaCha20 code keeps on the stack for each page. hwaccel changes those files, so I'd rather do that after you have merged it.

Cost: the PBKDF2 wipes run once per key derivation, not per iteration. Per page, sqlcipher_hmac() now also clears its HMAC state (656 bytes for HMAC-SHA512), while it hashes the whole 4 KiB page anyway, so the relative cost stays small on slower hardware as well. On an Apple M4, natively and in WebAssembly under Node.js, the difference is within the measurement noise.

How serious is it? Low in practice: while the database is open, the key is in memory anyway. This is about what is left after it has been closed, and about a point that security reviews routinely check.

How I tested it

• Freed memory: a test program routes all SQLite allocations through an allocator that searches every freed block for the keys. It creates a database with the sqlcipher scheme (legacy=4) and the URI parameter hexkey, opens it with PRAGMA hexkey, changes the key with PRAGMA hexrekey, opens it with the new key, uses VLE and checks that the old key is rejected. Before the change, the first key is found in 3 freed blocks, the second in 2 and the VLE key in 1. After the change, in none.
• This test passes on macOS, Linux aarch64 and x86_64 (GCC and Clang), Android arm64 and WebAssembly.
• The CI recipe (autoreconf, ./configure, make, the SQL tests and cryptotest) passes on Linux aarch64 and x86_64 with GCC and Clang. cryptotest also passes on macOS and WebAssembly.
• No new compiler warnings (compared on macOS, Linux and MinGW-w64). It also builds for armeabi-v7a.

Best regards,
Markus

[utelle]

Hi Markus,

this is the next hardening step, with the same background as #267 and #268. It only adds calls to your sqlite3mcSecureZeroMemory().

I checked your proposed changes and I'm fine with all of them.

I have to admit that I was a bit lazy in adding such memory zeroing calls in the past. If a system is compromised in such a way that an attacker has access to the main memory, that would give easily access to the passphrase, key material etc as well. Additionally, SQLite's page cache and temporary tables are not encrypted at all. And the latter behaviour can't be changed easily - it would require massive changes to SQLite's implementation.

When I started with SQLite3MC I had considered to implement a memory locking mechanism like SQLCipher, but after inspecting and testing the SQLCipher approach I dropped that idea as being a pseudo security feature.

Not included: the key schedules that the AES and ChaCha20 code keeps on the stack for each page. hwaccel changes those files, so I'd rather do that after you have merged it.

Thanks. I guess those changes can happen by the end of the week.

How serious is it? Low in practice: while the database is open, the key is in memory anyway. This is about what is left after it has been closed, and about a point that security reviews routinely check.

Since the performance penalty is usually very small, it is ok to add these measures. However, on a compromised system an attacker simply just needs to intercept the library call where the passphrase is passed to the key derivation function.

The SQLite encryption extension is meant to secure database files at rest in the first place. Securing a running SQLite database application requires other measures.