[0.11] dbwrapper: Detect obfuscation #7259

pull MarcoFalke wants to merge 2 commits into bitcoin:0.11 from MarcoFalke:MarcoFalke-2015-dbWrapperObfuscation-0.11 changing 6 files +29 −10

MarcoFalke commented at 9:50 PM on December 26, 2015: member

Replaces #6919. Fixes #7258. Implements #6613 (comment).
[dbwrapper] Detect obfuscation fa24941c46
pstratem commented at 9:52 PM on December 26, 2015: contributor

concept ACK fa24941c46fe0663efa081326940099313df292f
luke-jr commented at 9:52 PM on December 26, 2015: member

Won't this detect the no-op obfuscation too? That wouldn't be a good idea...
pstratem commented at 9:54 PM on December 26, 2015: contributor

@MarcoFalke luke-jr is right you need to also check for the null obfuscation key
MarcoFalke commented at 10:02 PM on December 26, 2015: member

@pstratem Why would someone write a null obfuscation key?
pstratem commented at 10:04 PM on December 26, 2015: contributor

@MarcoFalke it's a quirk of how the obfuscation code works, it's XOR so a null obfuscation key is the same as no obfuscation key
sipa commented at 10:06 PM on December 26, 2015: member

Do we ever write a null key? (as opposed to treating no key as null)
MarcoFalke commented at 10:07 PM on December 26, 2015: member

@pstratem But it is never written to the db. This would be like saying I obfuscate every of my GitHub messages with the null obfuscation key. (It's redundant and not needed)
pstratem commented at 10:18 PM on December 26, 2015: contributor

@MarcoFalke I stand corrected
[init] Fix typo fa3cb4946f
jonasschnelli added the label UTXO Db and Indexes on Dec 27, 2015
laanwj commented at 8:06 AM on January 4, 2016: member

Concept ACK but doubting if this or #6919 makes more sense. Sure, backporting the whole thing involves somewhat more code but it seemed more useful than introducing a little bit less of code to just warn.
MarcoFalke commented at 11:25 AM on January 4, 2016: member

@laanwj This can be cherry-picked to 0.10 without conflicts as well. As this issue seems popular ( #7258 (comment) ) we need either #7259 (this) or #6919, imo.

Keep in mind you can rebase #6919 simply on a commit which reverts #7259 (this), in case you want to reopen after #7259 (this) got merged. ("'Don't Let Perfect Be The Enemy Of Good'")
laanwj commented at 10:44 AM on January 5, 2016: member

Yeah, this is fine for 0.11. It's easier to test that it rejects a database than to test whether it works.

Would be nice to have at least one tested ACK, though.
laanwj merged this on Jan 9, 2016
laanwj closed this on Jan 9, 2016
laanwj referenced this in commit 00aefccb12 on Jan 9, 2016
MarcoFalke deleted the branch on Jan 9, 2016
luke-jr referenced this in commit 4124498ea4 on Jan 10, 2016
luke-jr referenced this in commit 4d548f6fdc on Jan 10, 2016
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#6613 Obfuscate database files #6919 Backport chainstate obfuscation to 0.11 #7258 Assertion `hashPrevBlock == view.GetBestBlock()' failed. Aborted (core dumped)