On Mon, May 25, 2020 at 10:57:13AM +0000, Johannes Thumshirn wrote: > On 24/05/2020 21:56, David Sterba wrote: > > On Thu, May 14, 2020 at 11:24:15AM +0200, Johannes Thumshirn wrote: > > For metadata the per-transaction salt is inherently there as the hash is > > calculated with the header included (containing the increasing > > generation) and the filesystem UUID (available via blkid) or chunk tree > > UUID (not so easy to user to read). > > > > So there's an obvious discrepancy in the additional data besides the > > variable contents of the data and metadata blocks. > > > > The weakness of the data blocks may aid some attacks (I don't have a > > concrete suggestion where and how exatly). > > Yes but wouldn't this also need a hash that is prone to a known plaintext > attack or that has known collisions? But it would probably help in > brute-forcing the key K of the filesystem. OTOH fsid, generation and the > chunk-tree UUID can be read in plaintext from the FS as well so this would > only mitigate a rainbow table like attack, wouldn't it? The goal here is to make attacks harder at a small cost. > > Suggested fix is to have a data block "header", with similar contents as > > the metadata blocks, eg. > > > > struct btrfs_hash_header { > > u8 fsid[BTRFS_FSID_SIZE]; > > u8 chunk_tree_uuid[BTRFS_UUID_SIZE]; > > __le64 generation; > > }; > > > > Perhaps also with some extra item for future extensions, set to zeros > > for now. > > This addition would be possible, yes. But if we'd add this header to every > checksum in the checksum tree it would be an incompatible on-disk format > change. No. It's only in-memory and is built from known pieces of information exactly to avoid storing it on disk.