blockstorage: XOR blocksdir *.dat files#28052
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
maflcko
changed the title
|
Concept ACK. Certainly worth doing. |
|
Oops, I clicked wrong. I just meant to Concept ACK. |
|
AFAIK there is software which reads these files intentionally. IMO they can be expected to adapt, but having the option to disable it seems like a good idea in case they aren't ready right away. |
|
Strong Concept ACK. |
|
Thanks for the feedback so far. I've kept the option to disable, but made it a simple bool instead of allowing the user to pick the XOR-key. If someone is interested, a new commit can be added to upgrade the linearize scripts to read the XOR-key. |
|
Concept ACK |
|
Rebased on the latest ##28060 |
fa633aa streams: Teach AutoFile how to XOR (MarcoFalke) 000019e Add AutoFile::detail_fread member function (MarcoFalke) fa7724b refactor: Modernize AutoFile (MarcoFalke) fa8d227 doc: Remove comments that just repeat what the code does (MarcoFalke) fafe2ca refactor: Remove redundant file check from AutoFile shift operators (MarcoFalke) 9999a49 Extract util::Xor, Add key_offset option, Add bench (MarcoFalke) Pull request description: This allows `AutoFile` to roll an XOR pattern while reading or writing to the underlying file. This is needed for bitcoin/bitcoin#28052, but can also be used in any other place. Also, there are tests, so I've split this up from the larger pull to make review easier, hopefully. ACKs for top commit: Crypt-iQ: crACK fa633aa willcl-ark: Lightly tested ACK fa633aa jamesob: reACK fa633aa ([`jamesob/ackr/28060.4.MarcoFalke.util_add_xorfile`](https://github.com/jamesob/bitcoin/tree/ackr/28060.4.MarcoFalke.util_add_xorfile)) Tree-SHA512: 6d66cad0a089a096d3f95e4f2b28bce80b349d4b76f53d09dc9a0bea4fc1b7c0652724469c37971ba27728c7d46398a4c1d289c252af4c0f83bb2fcbc6f8e90b
hodlinator
left a comment
hodlinator
left a comment
There was a problem hiding this comment.
ACK fa895c7
git range-diff fa9d4aa9a1a8df6343f615572479db9c8e26b19d~3 fa9d4aa9a1a8df6343f615572479db9c8e26b19d fa895c72832f9555b52d5bb1dba1093f73de3136
Confirmed only thing that really changed was:
- Merging with mainly 7aa8994 - " refactor: Add FlatFileSeq member variables in BlockManager "
- Correcting CAutoFile -> AutoFile in commit message
- Comment typo in
InitBlocksdirXorKey LogInfoformat string re-jiggling.
Passed make check & test_runner.py.
It would be nice to get to the bottom with why
LogInfo("Using obfuscation key for %s/*.dat files: '%s'\n", fs::PathToString(opts.blocks_dir), HexStr(xor_key));was worse than
LogInfo("Using obfuscation key for blocksdir *.dat files (%s): '%s'\n", fs::PathToString(opts.blocks_dir), HexStr(xor_key));and fix test/lint/run-lint-format-strings.py in the longer term.
|
ACK fa895c7 |
|
Follow-up ideas for this pull requests are:
|
|
Is there some label or something that can assist people in discovering this kind of follow-up work on closed/merged PRs? |
I've moved it to an issue, see #30599 |
14 participants
Currently the *.dat files in the blocksdir store the data received from remote peers as-is. This may be problematic when a program other than Bitcoin Core tries to interpret them by accident. For example, an anti-virus program or other program may scan them and move them into quarantine, or delete them, or corrupt them. This may cause Bitcoin Core to fail a reorg, or fail to reply to block requests (via P2P, RPC, REST, ...).
Fix this, similar to #6650, by rolling a random XOR pattern over the dat files when writing or reading them.
Obviously this can only protect against programs that accidentally and unintentionally are trying to mess with the dat files. Any program that intentionally wants to mess with the dat files can still trivially do so.
The XOR pattern is only applied when the blocksdir is freshly created, and there is an option to disable it (on creation), so that people can disable it, if needed.