Compare SQLCipher HMAC values in constant time - #264
Merged
Merged
Conversation
The HMAC verification of the SQLCipher cipher scheme used memcmp(), whose execution time may depend on the position of the first differing byte. Use a constant-time comparison instead, as SQLCipher itself does and as the ChaCha20-Poly1305 cipher scheme already does for its Poly1305 tags.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi Ulrich,
here's another small finding, as mentioned in #262. The fix is tiny, so I'm sending it directly as a PR.
The authenticated cipher schemes in sqlite3mc compare their authentication tags in constant time:
poly1305_tagcmp(),aegis_verify_16/32(),ascon_aead_decrypt().The
sqlcipherscheme is the only exception:DecryptPageSQLCipherCipher()insrc/cipher_sqlcipher.cchecks the page HMAC withmemcmp(). SQLCipher itself also uses a constant-time comparison (sqlcipher_memcmp()).Why it matters:
memcmp()may stop at the first differing byte. The time the check takes can therefore reveal how many leading bytes of a forged HMAC are correct (CWE-208). In practice this is hard to exploit, because an attacker needs write access to the file and very precise timing of many page reads. But it's a typical finding in security reviews, and it's easy to fix.The change (only
src/cipher_sqlcipher.c, +18/−1):CompareHmacSQLCipherCipher()combines all byte differences and evaluates the result only at the end, following the same pattern aspoly1305_tagcmp().memcmp()'s convention (0 means equal), so the call site only changes the function name.hwaccelbranch.How I tested it
sqlcipher(legacy=4and the default setting): flipping a single byte in the ciphertext, IV or HMAC of a page is still rejected. Untouched databases open as before.Best regards,
Markus