librbd: check image size when loading encryption

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 <idryomov@gmail.com>

diff --git a/src/librbd/crypto/luks/LoadRequest.cc b/src/librbd/crypto/luks/LoadRequest.cc
index b345cdbb824b5ecd10960dfe5bf18489048e26e7..bddf771a8bd68751c50fc2958f889456d610f036 100644 (file)
--- a/src/librbd/crypto/luks/LoadRequest.cc
+++ b/src/librbd/crypto/luks/LoadRequest.cc
@@ -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;
     }
 
@@ -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;
 }

diff --git a/src/test/librbd/crypto/luks/test_mock_LoadRequest.cc b/src/test/librbd/crypto/luks/test_mock_LoadRequest.cc
index c18b1486c9ebbb518710dbb069ccd326b8dfed2f..fb8699de9b1c4dbd07e9bd452c8e3797e63784d8 100644 (file)
--- a/src/test/librbd/crypto/luks/test_mock_LoadRequest.cc
+++ b/src/test/librbd/crypto/luks/test_mock_LoadRequest.cc
@@ -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());
@@ -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());
@@ -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());
@@ -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
 
@@ -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());
@@ -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());
@@ -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
 
@@ -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

diff --git a/src/test/librbd/test_librbd.cc b/src/test/librbd/test_librbd.cc
index 48e0d57db5a57bf84ee864a368dfabc1125cc18e..9e2e7a69e4ca6cd304b01b30f84f5946919f2c08 100644 (file)
--- a/src/test/librbd/test_librbd.cc
+++ b/src/test/librbd/test_librbd.cc
@@ -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)