From fb2d153c928e3184955e19e90789883efb2151ff Mon Sep 17 00:00:00 2001 From: Martin Stein Date: Thu, 30 Nov 2023 18:29:17 +0100 Subject: [PATCH] tresor: check hash of all read vba data During one of the many re-factorization steps that were applied to the Tresor library and its predecessor, the CBE library, one of the main features of the project, the integrity check, accidentally received a grave regression. The most recent version of the Tresor still used to check all hashes of meta-data blocks but ignored the hashes of the actual data blocks. With this commit, the hashes of all but yet uninitialized data blocks get checked. The reason for ignoring uninitialized blocks is that they are not actually read from disc but simply generated as an all-zeros block in the driver in order to prevent having to initialize them all to zero in Tresor-Init. That said, the integrity of these blocks cannot be compomised. The according hashes in the meta data remain unset until the data block gets written for the first time. Ref #5062 --- repos/gems/src/lib/tresor/block_io.cc | 1 + .../src/lib/tresor/include/tresor/block_io.h | 4 +-- .../include/tresor/virtual_block_device.h | 2 +- .../src/lib/tresor/virtual_block_device.cc | 28 ++++++++++++++----- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/repos/gems/src/lib/tresor/block_io.cc b/repos/gems/src/lib/tresor/block_io.cc index 3d11c97966..e1c0b6b2ee 100644 --- a/repos/gems/src/lib/tresor/block_io.cc +++ b/repos/gems/src/lib/tresor/block_io.cc @@ -104,6 +104,7 @@ void Block_io_channel::_read_client_data(bool &progress) case REQ_SUBMITTED: _file.read(READ_OK, FILE_ERR, req._pba * BLOCK_SIZE, { (char *)&_blk, BLOCK_SIZE }, progress); break; case READ_OK: + calc_hash(_blk, req._hash); _generate_req( PLAINTEXT_BLK_SUPPLIED, progress, Crypto_request::DECRYPT_CLIENT_DATA, req._client_req_offset, req._client_req_tag, req._key_id, *(Key_value *)0, req._pba, req._vba, _blk); diff --git a/repos/gems/src/lib/tresor/include/tresor/block_io.h b/repos/gems/src/lib/tresor/include/tresor/block_io.h index 0dd33530ec..7da5c7cb3d 100644 --- a/repos/gems/src/lib/tresor/include/tresor/block_io.h +++ b/repos/gems/src/lib/tresor/include/tresor/block_io.h @@ -152,8 +152,8 @@ class Tresor::Block_io : public Module struct Read_client_data : Request { Read_client_data(Module_id m, Module_channel_id c, Physical_block_address p, Virtual_block_address v, - Key_id k, Request_tag t, Request_offset o, bool &s) - : Request(m, c, Request::READ_CLIENT_DATA, o, t, k, p, v, *(Block*)0, *(Hash*)0, s) { } + Key_id k, Request_tag t, Request_offset o, Hash &h, bool &s) + : Request(m, c, Request::READ_CLIENT_DATA, o, t, k, p, v, *(Block*)0, h, s) { } }; Block_io(Vfs::Env &, Xml_node const &); diff --git a/repos/gems/src/lib/tresor/include/tresor/virtual_block_device.h b/repos/gems/src/lib/tresor/include/tresor/virtual_block_device.h index 0d58513a9a..59febe41fe 100644 --- a/repos/gems/src/lib/tresor/include/tresor/virtual_block_device.h +++ b/repos/gems/src/lib/tresor/include/tresor/virtual_block_device.h @@ -123,7 +123,7 @@ class Tresor::Virtual_block_device_channel : public Module_channel void _read_vba(bool &); - bool _check_and_decode_read_blk(bool &, bool); + bool _check_and_decode_read_blk(bool &); Tree_node_index _node_idx(Tree_level_index, Virtual_block_address) const; diff --git a/repos/gems/src/lib/tresor/virtual_block_device.cc b/repos/gems/src/lib/tresor/virtual_block_device.cc index d9588bbfc7..1cd7f969d3 100644 --- a/repos/gems/src/lib/tresor/virtual_block_device.cc +++ b/repos/gems/src/lib/tresor/virtual_block_device.cc @@ -90,7 +90,7 @@ void Virtual_block_device_channel::_read_vba(bool &progress) case READ_BLK_SUCCEEDED: { - if (!_check_and_decode_read_blk(progress, false)) + if (!_check_and_decode_read_blk(progress)) break; if (!_lvl) { @@ -112,7 +112,7 @@ void Virtual_block_device_channel::_read_vba(bool &progress) } else _generate_req( READ_BLK_SUCCEEDED, progress, node.pba, _vba, req._curr_key_id, - req._client_req_tag, req._client_req_offset); + req._client_req_tag, req._client_req_offset, _hash); _lvl--; break; } @@ -147,13 +147,27 @@ void Virtual_block_device_channel::_update_nodes_of_branch_of_written_vba() } -bool Virtual_block_device_channel::_check_and_decode_read_blk(bool &progress, bool check_leaf_data = true) +bool Virtual_block_device_channel::_check_and_decode_read_blk(bool &progress) { - if (!check_leaf_data && !_lvl) - return true; + Request &req { *_req_ptr }; + Hash *node_hash_ptr; + if (_lvl) { + calc_hash(_encoded_blk, _hash); + if (_lvl < snap().max_level) + node_hash_ptr = &_node(_lvl + 1, _vba).hash; + else + node_hash_ptr = &snap().hash; + } else { + Type_1_node &node { _node(_lvl + 1, _vba) }; + if (req._type == Request::READ_VBA) { + if (node.gen == INITIAL_GENERATION) + return true; + } else + calc_hash(_data_blk, _hash); - Hash &hash { _lvl < snap().max_level ? _node(_lvl + 1, _vba).hash : snap().hash }; - if (!check_hash(_lvl ? _encoded_blk : _data_blk, hash)) { + node_hash_ptr = &node.hash; + } + if (_hash != *node_hash_ptr) { _mark_req_failed(progress, "check hash of read block"); return false; }