Skip to content

Commit

Permalink
librbd: check image size when loading encryption
Browse files Browse the repository at this point in the history 
Currently it's done in FormatRequest but not in LoadRequest.  However
an image can be shrunk to a size such that encryption can loaded (i.e.
enough of the header is still present) but nothing else can, breaking
implicit assumptions all around.

Signed-off-by: Ilya Dryomov <[email protected]>
  • Loading branch information
@idryomov
idryomov committed Dec 4, 2022
1 parent 2035609 commit 36c2d58
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 10 deletions.
20 changes: 13 additions & 7 deletions src/librbd/crypto/luks/LoadRequest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,16 +143,11 @@ void LoadRequest<I>::handle_read_header(int r) {
// parse header via libcryptsetup
r = m_header.load(CRYPT_LUKS);
if (r != 0) {
m_image_ctx->image_lock.lock_shared();
auto image_size = m_image_ctx->get_image_size(m_image_ctx->snap_id);
m_image_ctx->image_lock.unlock_shared();

auto max_header_size = std::min(MAXIMUM_HEADER_SIZE, image_size);
if (m_offset < max_header_size) {
if (m_offset < MAXIMUM_HEADER_SIZE) {
// perhaps we did not feed the entire header to libcryptsetup, retry
auto ctx = create_context_callback<
LoadRequest<I>, &LoadRequest<I>::handle_read_header>(this);
read(max_header_size, ctx);
read(MAXIMUM_HEADER_SIZE, ctx);
return;
}

Expand All @@ -179,6 +174,17 @@ void LoadRequest<I>::handle_read_header(int r) {
return;
}

m_image_ctx->image_lock.lock_shared();
uint64_t image_size = m_image_ctx->get_image_size(CEPH_NOSNAP);
m_image_ctx->image_lock.unlock_shared();

if (m_header.get_data_offset() > image_size) {
lderr(m_image_ctx->cct) << "image is too small, data offset "
<< m_header.get_data_offset() << dendl;
finish(-EINVAL);
return;
}

read_volume_key();
return;
}
Expand Down
24 changes: 21 additions & 3 deletions src/test/librbd/crypto/luks/test_mock_LoadRequest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,16 @@ struct TestMockCryptoLuksLoadRequest : public TestMockFixture {
}));
}

void expect_get_image_size() {
void expect_get_image_size(uint64_t size) {
EXPECT_CALL(*mock_image_ctx, get_image_size(_)).WillOnce(
Return(100 * 1024 * 1024));
Return(size));
}
};

TEST_F(TestMockCryptoLuksLoadRequest, AES128) {
generate_header(CRYPT_LUKS2, "aes", 32, "xts-plain64", 4096, false);
expect_image_read(0, DEFAULT_INITIAL_READ_SIZE);
expect_get_image_size(OBJECT_SIZE << 5);
mock_load_request->send();
image_read_request->complete(DEFAULT_INITIAL_READ_SIZE);
ASSERT_EQ(0, finished_cond.wait());
Expand All @@ -123,6 +124,7 @@ TEST_F(TestMockCryptoLuksLoadRequest, AES128) {
TEST_F(TestMockCryptoLuksLoadRequest, AES256) {
generate_header(CRYPT_LUKS2, "aes", 64, "xts-plain64", 4096, false);
expect_image_read(0, DEFAULT_INITIAL_READ_SIZE);
expect_get_image_size(OBJECT_SIZE << 5);
mock_load_request->send();
image_read_request->complete(DEFAULT_INITIAL_READ_SIZE);
ASSERT_EQ(0, finished_cond.wait());
Expand All @@ -137,6 +139,7 @@ TEST_F(TestMockCryptoLuksLoadRequest, LUKS1) {
on_finish);
generate_header(CRYPT_LUKS1, "aes", 32, "xts-plain64", 512, false);
expect_image_read(0, DEFAULT_INITIAL_READ_SIZE);
expect_get_image_size(OBJECT_SIZE << 5);
mock_load_request->send();
image_read_request->complete(DEFAULT_INITIAL_READ_SIZE);
ASSERT_EQ(0, finished_cond.wait());
Expand Down Expand Up @@ -176,13 +179,24 @@ TEST_F(TestMockCryptoLuksLoadRequest, UnsupportedCipherMode) {
ASSERT_EQ("LUKS2", detected_format_name);
}

TEST_F(TestMockCryptoLuksLoadRequest, BadSize) {
generate_header(CRYPT_LUKS2, "aes", 64, "xts-plain64", 4096, false);
expect_image_read(0, DEFAULT_INITIAL_READ_SIZE);
expect_get_image_size(OBJECT_SIZE - 1);
mock_load_request->send();
image_read_request->complete(DEFAULT_INITIAL_READ_SIZE);
ASSERT_EQ(-EINVAL, finished_cond.wait());
ASSERT_EQ(crypto.get(), nullptr);
ASSERT_EQ("LUKS2", detected_format_name);
}

TEST_F(TestMockCryptoLuksLoadRequest, HeaderBiggerThanInitialRead) {
generate_header(CRYPT_LUKS2, "aes", 64, "xts-plain64", 4096, false);
mock_load_request->set_initial_read_size(4096);
expect_image_read(0, 4096);
mock_load_request->send();

expect_get_image_size();
expect_get_image_size(OBJECT_SIZE << 5);
expect_image_read(4096, MAXIMUM_HEADER_SIZE - 4096);
image_read_request->complete(4096); // complete initial read

Expand All @@ -200,6 +214,7 @@ TEST_F(TestMockCryptoLuksLoadRequest, LUKS1FormattedClone) {
on_finish);
generate_header(CRYPT_LUKS1, "aes", 64, "xts-plain64", 512, true);
expect_image_read(0, DEFAULT_INITIAL_READ_SIZE);
expect_get_image_size(OBJECT_SIZE << 5);
mock_load_request->send();
image_read_request->complete(DEFAULT_INITIAL_READ_SIZE);
ASSERT_EQ(0, finished_cond.wait());
Expand All @@ -211,6 +226,7 @@ TEST_F(TestMockCryptoLuksLoadRequest, LUKS2FormattedClone) {
mock_image_ctx->parent = mock_image_ctx;
generate_header(CRYPT_LUKS2, "aes", 64, "xts-plain64", 4096, true);
expect_image_read(0, DEFAULT_INITIAL_READ_SIZE);
expect_get_image_size(OBJECT_SIZE << 5);
mock_load_request->send();
image_read_request->complete(DEFAULT_INITIAL_READ_SIZE);
ASSERT_EQ(0, finished_cond.wait());
Expand All @@ -224,6 +240,7 @@ TEST_F(TestMockCryptoLuksLoadRequest, KeyslotsBiggerThanInitialRead) {
expect_image_read(0, 16384);
mock_load_request->send();

expect_get_image_size(OBJECT_SIZE << 5);
expect_image_read(16384, data_offset - 16384);
image_read_request->complete(16384); // complete initial read

Expand All @@ -240,6 +257,7 @@ TEST_F(TestMockCryptoLuksLoadRequest, WrongPassphrase) {

generate_header(CRYPT_LUKS2, "aes", 64, "xts-plain64", 4096, false);
expect_image_read(0, DEFAULT_INITIAL_READ_SIZE);
expect_get_image_size(OBJECT_SIZE << 5);
mock_load_request->send();

// crypt_volume_key_get will fail, we will retry reading more
Expand Down
51 changes: 51 additions & 0 deletions src/test/librbd/test_librbd.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2574,6 +2574,57 @@ TEST_F(TestLibRBD, EncryptionFormatNoData)
}
}

TEST_F(TestLibRBD, EncryptionLoadBadSize)
{
REQUIRE(!is_feature_enabled(RBD_FEATURE_JOURNALING));

librados::IoCtx ioctx;
ASSERT_EQ(0, _rados.ioctx_create(m_pool_name.c_str(), ioctx));

librbd::RBD rbd;
auto name = get_temp_image_name();
uint64_t luks1_meta_size = 4 << 20;
std::string passphrase = "some passphrase";

{
int order = 0;
ASSERT_EQ(0, create_image_pp(rbd, ioctx, name.c_str(), luks1_meta_size,
&order));
librbd::Image image;
ASSERT_EQ(0, rbd.open(ioctx, image, name.c_str(), nullptr));

librbd::encryption_luks1_format_options_t opts = {
RBD_ENCRYPTION_ALGORITHM_AES256, passphrase};
ASSERT_EQ(0, image.encryption_format(RBD_ENCRYPTION_FORMAT_LUKS1, &opts,
sizeof(opts)));
}

{
librbd::Image image;
ASSERT_EQ(0, rbd.open(ioctx, image, name.c_str(), nullptr));

librbd::encryption_luks_format_options_t opts = {passphrase};
ASSERT_EQ(0, image.encryption_load(RBD_ENCRYPTION_FORMAT_LUKS, &opts,
sizeof(opts)));
uint64_t size;
ASSERT_EQ(0, image.size(&size));
ASSERT_EQ(0, size);
}

{
librbd::Image image;
ASSERT_EQ(0, rbd.open(ioctx, image, name.c_str(), nullptr));
ASSERT_EQ(0, image.resize(luks1_meta_size - 1));

librbd::encryption_luks_format_options_t opts = {passphrase};
ASSERT_EQ(-EINVAL, image.encryption_load(RBD_ENCRYPTION_FORMAT_LUKS, &opts,
sizeof(opts)));
uint64_t size;
ASSERT_EQ(0, image.size(&size));
ASSERT_EQ(luks1_meta_size - 1, size);
}
}

#endif

TEST_F(TestLibRBD, TestIOWithIOHint)
Expand Down

0 comments on commit 36c2d58

Please sign in to comment.