]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
librbd: don't decay LUKS{1,2}EncryptionFormat into LUKSEncryptionFormat
authorIlya Dryomov <idryomov@gmail.com>
Fri, 18 Nov 2022 13:53:24 +0000 (14:53 +0100)
committerIlya Dryomov <idryomov@gmail.com>
Sun, 4 Dec 2022 17:19:19 +0000 (18:19 +0100)
Commit 9892ead7fcd9 ("librbd/crypto: allow loading luks format
without specifying version") introduced RBD_ENCRYPTION_FORMAT_LUKS
format identifier, matching cryptsetup's CRYPT_LUKS ("load any LUKS
version happens to be there").  However, in an effort to enable an
obscure "layered encryption with the same passphrase + old QEMU" use
case, it also introduced decaying of RBD_ENCRYPTION_FORMAT_LUKS1 and
RBD_ENCRYPTION_FORMAT_LUKS2 format identifiers, making it impossible
to assert on the format that is being loaded.  This new behavior was
then extended to standalone images.

Treating RBD_ENCRYPTION_FORMAT_LUKS1, RBD_ENCRYPTION_FORMAT_LUKS2
and RBD_ENCRYPTION_FORMAT_LUKS the same when loading encryption can
be construed as an opening for a format downgrade attack.  Let's
resurrect the previous standalone images behavior and extend it to
layered encryption instead.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
src/librbd/crypto/EncryptionFormat.h
src/librbd/crypto/luks/LUKSEncryptionFormat.cc
src/librbd/crypto/luks/LUKSEncryptionFormat.h
src/librbd/crypto/luks/LoadRequest.cc
src/librbd/crypto/luks/LoadRequest.h
src/test/librbd/crypto/luks/test_mock_LoadRequest.cc
src/test/librbd/test_librbd.cc

index 98c62c2169cc7f4ee4220518feaeb8d6942e9766..252592891d5c8da0d9ef7140f2732ccdbd628b50 100644 (file)
@@ -5,7 +5,6 @@
 #define CEPH_LIBRBD_CRYPTO_ENCRYPTION_FORMAT_H
 
 #include <memory>
-#include "common/ref.h"
 
 struct Context;
 
index e739714932508424d62508c6627ed53910137c7c..1f92cf0f72849f0449f4e1dd25625b828205b6a3 100644 (file)
@@ -19,42 +19,60 @@ namespace crypto {
 namespace luks {
 
 template <typename I>
-LUKSEncryptionFormat<I>::LUKSEncryptionFormat(
-        encryption_algorithm_t alg,
-        std::string_view passphrase) : m_passphrase(passphrase), m_alg(alg) {
+void EncryptionFormat<I>::flatten(I* image_ctx, Context* on_finish) {
+  auto req = luks::FlattenRequest<I>::create(image_ctx, on_finish);
+  req->send();
 }
 
 template <typename I>
-LUKSEncryptionFormat<I>::LUKSEncryptionFormat(
-        std::string_view passphrase) : m_passphrase(passphrase) {
+void LUKSEncryptionFormat<I>::format(I* image_ctx, Context* on_finish) {
+  lderr(image_ctx->cct) << "explicit LUKS version required for format" << dendl;
+  on_finish->complete(-EINVAL);
 }
 
 template <typename I>
-void LUKSEncryptionFormat<I>::format(I* image_ctx, Context* on_finish) {
-  if (get_format() == RBD_ENCRYPTION_FORMAT_LUKS) {
-    lderr(image_ctx->cct) << "explicit LUKS version required for format"
-                          << dendl;
-    on_finish->complete(-EINVAL);
-    return;
-  }
+void LUKSEncryptionFormat<I>::load(I* image_ctx,
+                                   std::string* detected_format_name,
+                                   Context* on_finish) {
+  auto req = luks::LoadRequest<I>::create(image_ctx, RBD_ENCRYPTION_FORMAT_LUKS,
+                                          m_passphrase, &this->m_crypto,
+                                          detected_format_name, on_finish);
+  req->send();
+}
 
+template <typename I>
+void LUKS1EncryptionFormat<I>::format(I* image_ctx, Context* on_finish) {
   auto req = luks::FormatRequest<I>::create(
-          image_ctx, get_format(), m_alg, m_passphrase, &m_crypto, on_finish,
-          false);
+      image_ctx, RBD_ENCRYPTION_FORMAT_LUKS1, m_alg, m_passphrase,
+      &this->m_crypto, on_finish, false);
   req->send();
 }
 
 template <typename I>
-void LUKSEncryptionFormat<I>::load(
-        I* image_ctx, std::string* detected_format_name, Context* on_finish) {
+void LUKS1EncryptionFormat<I>::load(I* image_ctx,
+                                    std::string* detected_format_name,
+                                    Context* on_finish) {
   auto req = luks::LoadRequest<I>::create(
-          image_ctx, m_passphrase, &m_crypto, detected_format_name, on_finish);
+      image_ctx, RBD_ENCRYPTION_FORMAT_LUKS1, m_passphrase, &this->m_crypto,
+      detected_format_name, on_finish);
   req->send();
 }
 
 template <typename I>
-void LUKSEncryptionFormat<I>::flatten(I* image_ctx, Context* on_finish) {
-  auto req = luks::FlattenRequest<I>::create(image_ctx, on_finish);
+void LUKS2EncryptionFormat<I>::format(I* image_ctx, Context* on_finish) {
+  auto req = luks::FormatRequest<I>::create(
+      image_ctx, RBD_ENCRYPTION_FORMAT_LUKS2, m_alg, m_passphrase,
+      &this->m_crypto, on_finish, false);
+  req->send();
+}
+
+template <typename I>
+void LUKS2EncryptionFormat<I>::load(I* image_ctx,
+                                    std::string* detected_format_name,
+                                    Context* on_finish) {
+  auto req = luks::LoadRequest<I>::create(
+      image_ctx, RBD_ENCRYPTION_FORMAT_LUKS2, m_passphrase, &this->m_crypto,
+      detected_format_name, on_finish);
   req->send();
 }
 
index 3aa8950f6272c23c062bfbb20831fb754187363f..353bd8933c24c3f0bffe385be5ece77e61fba869 100644 (file)
@@ -16,61 +16,74 @@ struct ImageCtx;
 namespace crypto {
 namespace luks {
 
-// This class is derived from only for the sake of the 'format' operation.
-// The 'load' and 'flatten' operations are handled the same for all three
-// variants.
 template <typename ImageCtxT>
-class LUKSEncryptionFormat : public crypto::EncryptionFormat<ImageCtxT> {
-
+class EncryptionFormat : public crypto::EncryptionFormat<ImageCtxT> {
 public:
-    LUKSEncryptionFormat(std::string_view passphrase);
-    LUKSEncryptionFormat(encryption_algorithm_t alg,
-                         std::string_view passphrase);
-
-    std::unique_ptr<crypto::EncryptionFormat<ImageCtxT>>
-    clone() const override {
-      // clone() should be called only when handling the 'load' operation,
-      // so decaying LUKS{1,2}EncryptionFormat into LUKSEncryptionFormat is fine
-      return std::unique_ptr<crypto::EncryptionFormat<ImageCtxT>>(
-              new LUKSEncryptionFormat(m_passphrase));
-    }
-
-    void format(ImageCtxT* ictx, Context* on_finish) override;
-    void load(ImageCtxT* ictx, std::string* detected_format_name,
-              Context* on_finish) override;
-    void flatten(ImageCtxT* ictx, Context* on_finish) override;
-
-    CryptoInterface* get_crypto() override {
-      ceph_assert(m_crypto);
-      return m_crypto.get();
-    }
+  void flatten(ImageCtxT* ictx, Context* on_finish) override;
+
+  CryptoInterface* get_crypto() override {
+    ceph_assert(m_crypto);
+    return m_crypto.get();
+  }
 
 protected:
-    virtual encryption_format_t get_format() const {
-      return RBD_ENCRYPTION_FORMAT_LUKS;
-    }
+  std::unique_ptr<CryptoInterface> m_crypto;
+};
+
+template <typename ImageCtxT>
+class LUKSEncryptionFormat : public EncryptionFormat<ImageCtxT> {
+public:
+  LUKSEncryptionFormat(std::string_view passphrase)
+      : m_passphrase(passphrase) {}
+
+  std::unique_ptr<crypto::EncryptionFormat<ImageCtxT>> clone() const override {
+    return std::make_unique<LUKSEncryptionFormat>(m_passphrase);
+  }
 
-    std::string_view m_passphrase;
-    encryption_algorithm_t m_alg;
-    std::unique_ptr<CryptoInterface> m_crypto;
+  void format(ImageCtxT* ictx, Context* on_finish) override;
+  void load(ImageCtxT* ictx, std::string* detected_format_name,
+            Context* on_finish) override;
+
+private:
+  std::string_view m_passphrase;
 };
 
 template <typename ImageCtxT>
-class LUKS1EncryptionFormat : public LUKSEncryptionFormat<ImageCtxT> {
-    using LUKSEncryptionFormat<ImageCtxT>::LUKSEncryptionFormat;
+class LUKS1EncryptionFormat : public EncryptionFormat<ImageCtxT> {
+public:
+  LUKS1EncryptionFormat(encryption_algorithm_t alg, std::string_view passphrase)
+      : m_alg(alg), m_passphrase(passphrase) {}
+
+  std::unique_ptr<crypto::EncryptionFormat<ImageCtxT>> clone() const override {
+    return std::make_unique<LUKS1EncryptionFormat>(m_alg, m_passphrase);
+  }
+
+  void format(ImageCtxT* ictx, Context* on_finish) override;
+  void load(ImageCtxT* ictx, std::string* detected_format_name,
+            Context* on_finish) override;
 
-    encryption_format_t get_format() const override {
-      return RBD_ENCRYPTION_FORMAT_LUKS1;
-    }
+private:
+  encryption_algorithm_t m_alg;
+  std::string_view m_passphrase;
 };
 
 template <typename ImageCtxT>
-class LUKS2EncryptionFormat : public LUKSEncryptionFormat<ImageCtxT> {
-    using LUKSEncryptionFormat<ImageCtxT>::LUKSEncryptionFormat;
+class LUKS2EncryptionFormat : public EncryptionFormat<ImageCtxT> {
+public:
+  LUKS2EncryptionFormat(encryption_algorithm_t alg, std::string_view passphrase)
+      : m_alg(alg), m_passphrase(passphrase) {}
+
+  std::unique_ptr<crypto::EncryptionFormat<ImageCtxT>> clone() const override {
+    return std::make_unique<LUKS2EncryptionFormat>(m_alg, m_passphrase);
+  }
+
+  void format(ImageCtxT* ictx, Context* on_finish) override;
+  void load(ImageCtxT* ictx, std::string* detected_format_name,
+            Context* on_finish) override;
 
-    encryption_format_t get_format() const override {
-      return RBD_ENCRYPTION_FORMAT_LUKS2;
-    }
+private:
+  encryption_algorithm_t m_alg;
+  std::string_view m_passphrase;
 };
 
 } // namespace luks
index f23f5de41a636423e7391c7fc07ca9dfd9b1f5cb..b5e16f10016b26c5c07b0a893d02da36d48d0763 100644 (file)
@@ -26,10 +26,11 @@ using librbd::util::create_context_callback;
 
 template <typename I>
 LoadRequest<I>::LoadRequest(
-        I* image_ctx, std::string_view passphrase,
+        I* image_ctx, encryption_format_t format, std::string_view passphrase,
         std::unique_ptr<CryptoInterface>* result_crypto,
         std::string* detected_format_name,
         Context* on_finish) : m_image_ctx(image_ctx),
+                              m_format(format),
                               m_passphrase(passphrase),
                               m_on_finish(on_finish),
                               m_result_crypto(result_crypto),
@@ -140,8 +141,26 @@ void LoadRequest<I>::handle_read_header(int r) {
     return;
   }
 
+  const char* type;
+  switch (m_format) {
+  case RBD_ENCRYPTION_FORMAT_LUKS:
+    type = CRYPT_LUKS;
+    break;
+  case RBD_ENCRYPTION_FORMAT_LUKS1:
+    type = CRYPT_LUKS1;
+    break;
+  case RBD_ENCRYPTION_FORMAT_LUKS2:
+    type = CRYPT_LUKS2;
+    break;
+  default:
+    lderr(m_image_ctx->cct) << "unsupported format type: " << m_format
+                            << dendl;
+    finish(-EINVAL);
+    return;
+  }
+
   // parse header via libcryptsetup
-  r = m_header.load(CRYPT_LUKS);
+  r = m_header.load(type);
   if (r != 0) {
     if (m_offset < MAXIMUM_HEADER_SIZE) {
       // perhaps we did not feed the entire header to libcryptsetup, retry
index 5e64df6ffaee139322498df8f4260a4361c23382..60ed9a4a436aea900ed7912712a5da11040475a2 100644 (file)
@@ -26,15 +26,17 @@ template <typename I>
 class LoadRequest {
 public:
     static LoadRequest* create(
-            I* image_ctx, std::string_view passphrase,
+            I* image_ctx, encryption_format_t format,
+            std::string_view passphrase,
             std::unique_ptr<CryptoInterface>* result_crypto,
             std::string* detected_format_name,
             Context* on_finish) {
-      return new LoadRequest(image_ctx, passphrase, result_crypto,
+      return new LoadRequest(image_ctx, format, passphrase, result_crypto,
                              detected_format_name, on_finish);
     }
 
-    LoadRequest(I* image_ctx, std::string_view passphrase,
+    LoadRequest(I* image_ctx, encryption_format_t format,
+                std::string_view passphrase,
                 std::unique_ptr<CryptoInterface>* result_crypto,
                 std::string* detected_format_name, Context* on_finish);
     void send();
@@ -43,6 +45,7 @@ public:
 
 private:
     I* m_image_ctx;
+    encryption_format_t m_format;
     std::string_view m_passphrase;
     Context* m_on_finish;
     ceph::bufferlist m_bl;
index 313513751f28281104b4a43bd3188e72030ac2a1..5fb566a07bb7458b6c8dbff83b16ca028a09d492 100644 (file)
@@ -49,8 +49,8 @@ struct TestMockCryptoLuksLoadRequest : public TestMockFixture {
     ASSERT_EQ(0, open_image(m_image_name, &ictx));
     mock_image_ctx = new MockImageCtx(*ictx);
     mock_load_request = MockLoadRequest::create(
-            mock_image_ctx, std::move(passphrase), &crypto,
-            &detected_format_name, on_finish);
+        mock_image_ctx, RBD_ENCRYPTION_FORMAT_LUKS2, std::move(passphrase),
+        &crypto, &detected_format_name, on_finish);
     detected_format_name = "";
   }
 
@@ -142,8 +142,8 @@ TEST_F(TestMockCryptoLuksLoadRequest, AES256) {
 TEST_F(TestMockCryptoLuksLoadRequest, LUKS1) {
   delete mock_load_request;
   mock_load_request = MockLoadRequest::create(
-          mock_image_ctx, {passphrase_cstr}, &crypto, &detected_format_name,
-          on_finish);
+      mock_image_ctx, RBD_ENCRYPTION_FORMAT_LUKS1, {passphrase_cstr}, &crypto,
+      &detected_format_name, 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);
@@ -155,7 +155,23 @@ TEST_F(TestMockCryptoLuksLoadRequest, LUKS1) {
   ASSERT_EQ("LUKS1", detected_format_name);
 }
 
-TEST_F(TestMockCryptoLuksLoadRequest, WrongFormat) {
+TEST_F(TestMockCryptoLuksLoadRequest, LUKS1ViaLUKS) {
+  delete mock_load_request;
+  mock_load_request = MockLoadRequest::create(
+      mock_image_ctx, RBD_ENCRYPTION_FORMAT_LUKS, {passphrase_cstr}, &crypto,
+      &detected_format_name, 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);
+  expect_get_stripe_period(OBJECT_SIZE);
+  mock_load_request->send();
+  image_read_request->complete(DEFAULT_INITIAL_READ_SIZE);
+  ASSERT_EQ(0, finished_cond.wait());
+  ASSERT_NE(crypto.get(), nullptr);
+  ASSERT_EQ("LUKS1", detected_format_name);
+}
+
+TEST_F(TestMockCryptoLuksLoadRequest, UnknownFormat) {
   header_bl.append_zero(MAXIMUM_HEADER_SIZE);
   expect_image_read(0, DEFAULT_INITIAL_READ_SIZE);
   mock_load_request->send();
@@ -167,6 +183,21 @@ TEST_F(TestMockCryptoLuksLoadRequest, WrongFormat) {
   ASSERT_EQ("<unknown>", detected_format_name);
 }
 
+TEST_F(TestMockCryptoLuksLoadRequest, WrongFormat) {
+  generate_header(CRYPT_LUKS1, "aes", 32, "xts-plain64", 512, false);
+  expect_image_read(0, DEFAULT_INITIAL_READ_SIZE);
+  mock_load_request->send();
+
+  expect_image_read(DEFAULT_INITIAL_READ_SIZE,
+                    MAXIMUM_HEADER_SIZE - DEFAULT_INITIAL_READ_SIZE);
+  image_read_request->complete(DEFAULT_INITIAL_READ_SIZE);
+  image_read_request->complete(MAXIMUM_HEADER_SIZE - DEFAULT_INITIAL_READ_SIZE);
+
+  ASSERT_EQ(-EINVAL, finished_cond.wait());
+  ASSERT_EQ(crypto.get(), nullptr);
+  ASSERT_EQ("LUKS", detected_format_name);
+}
+
 TEST_F(TestMockCryptoLuksLoadRequest, UnsupportedAlgorithm) {
   generate_header(CRYPT_LUKS2, "twofish", 32, "xts-plain64", 4096, false);
   expect_image_read(0, DEFAULT_INITIAL_READ_SIZE);
@@ -231,8 +262,8 @@ TEST_F(TestMockCryptoLuksLoadRequest, LUKS1FormattedClone) {
   mock_image_ctx->parent = mock_image_ctx;
   delete mock_load_request;
   mock_load_request = MockLoadRequest::create(
-          mock_image_ctx, {passphrase_cstr}, &crypto, &detected_format_name,
-          on_finish);
+      mock_image_ctx, RBD_ENCRYPTION_FORMAT_LUKS1, {passphrase_cstr}, &crypto,
+      &detected_format_name, 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);
@@ -277,7 +308,8 @@ TEST_F(TestMockCryptoLuksLoadRequest, KeyslotsBiggerThanInitialRead) {
 TEST_F(TestMockCryptoLuksLoadRequest, WrongPassphrase) {
   delete mock_load_request;
   mock_load_request = MockLoadRequest::create(
-        mock_image_ctx, "wrong", &crypto, &detected_format_name, on_finish);
+      mock_image_ctx, RBD_ENCRYPTION_FORMAT_LUKS2, "wrong", &crypto,
+      &detected_format_name, on_finish);
 
   generate_header(CRYPT_LUKS2, "aes", 64, "xts-plain64", 4096, false);
   expect_image_read(0, DEFAULT_INITIAL_READ_SIZE);
index 38ea142623d5f08150c3b32670b03343f4bd18b7..c45cf989af8088d295b403df4f124fa4891452dd 100644 (file)
@@ -2254,7 +2254,12 @@ TEST_F(TestLibRBD, TestEncryptionLUKS1)
           _cluster, "rbd_read_from_replica_policy", "balance"));
 
   rbd_image_t image;
-  rbd_encryption_luks1_format_options_t opts = {
+  rbd_encryption_luks1_format_options_t luks1_opts = {
+          .alg = RBD_ENCRYPTION_ALGORITHM_AES256,
+          .passphrase = "password",
+          .passphrase_size = 8,
+  };
+  rbd_encryption_luks2_format_options_t luks2_opts = {
           .alg = RBD_ENCRYPTION_ALGORITHM_AES256,
           .passphrase = "password",
           .passphrase_size = 8,
@@ -2267,14 +2272,24 @@ TEST_F(TestLibRBD, TestEncryptionLUKS1)
 
 #ifndef HAVE_LIBCRYPTSETUP
   ASSERT_EQ(-ENOTSUP, rbd_encryption_format(
-          image, RBD_ENCRYPTION_FORMAT_LUKS1, &opts, sizeof(opts)));
+          image, RBD_ENCRYPTION_FORMAT_LUKS1, &luks1_opts, sizeof(luks1_opts)));
+  ASSERT_EQ(-ENOTSUP, rbd_encryption_load(
+          image, RBD_ENCRYPTION_FORMAT_LUKS1, &luks1_opts, sizeof(luks1_opts)));
+  ASSERT_EQ(-ENOTSUP, rbd_encryption_format(
+          image, RBD_ENCRYPTION_FORMAT_LUKS2, &luks2_opts, sizeof(luks2_opts)));
+  ASSERT_EQ(-ENOTSUP, rbd_encryption_load(
+          image, RBD_ENCRYPTION_FORMAT_LUKS2, &luks2_opts, sizeof(luks2_opts)));
+  ASSERT_EQ(-ENOTSUP, rbd_encryption_format(
+          image, RBD_ENCRYPTION_FORMAT_LUKS, &luks_opts, sizeof(luks_opts)));
   ASSERT_EQ(-ENOTSUP, rbd_encryption_load(
-          image, RBD_ENCRYPTION_FORMAT_LUKS1, &opts, sizeof(opts)));
+          image, RBD_ENCRYPTION_FORMAT_LUKS, &luks_opts, sizeof(luks_opts)));
 #else
+  ASSERT_EQ(-EINVAL, rbd_encryption_format(
+          image, RBD_ENCRYPTION_FORMAT_LUKS, &luks_opts, sizeof(luks_opts)));
   ASSERT_EQ(0, rbd_encryption_format(
-          image, RBD_ENCRYPTION_FORMAT_LUKS1, &opts, sizeof(opts)));
+          image, RBD_ENCRYPTION_FORMAT_LUKS1, &luks1_opts, sizeof(luks1_opts)));
   ASSERT_EQ(-EEXIST, rbd_encryption_load(
-          image, RBD_ENCRYPTION_FORMAT_LUKS1, &opts, sizeof(opts)));
+          image, RBD_ENCRYPTION_FORMAT_LUKS1, &luks1_opts, sizeof(luks1_opts)));
 
   test_io(image);
 
@@ -2282,6 +2297,16 @@ TEST_F(TestLibRBD, TestEncryptionLUKS1)
   ASSERT_EQ(0, rbd_close(image));
 
   ASSERT_EQ(0, rbd_open(ioctx, name.c_str(), &image, NULL));
+  ASSERT_EQ(-EINVAL, rbd_encryption_load(
+          image, RBD_ENCRYPTION_FORMAT_LUKS2, &luks2_opts, sizeof(luks2_opts)));
+  ASSERT_EQ(0, rbd_encryption_load(
+          image, RBD_ENCRYPTION_FORMAT_LUKS1, &luks1_opts, sizeof(luks1_opts)));
+  ASSERT_PASSED(read_test_data, image, "test", 0, 4, 0);
+  ASSERT_EQ(0, rbd_close(image));
+
+  ASSERT_EQ(0, rbd_open(ioctx, name.c_str(), &image, NULL));
+  ASSERT_EQ(-EINVAL, rbd_encryption_load(
+          image, RBD_ENCRYPTION_FORMAT_LUKS2, &luks2_opts, sizeof(luks2_opts)));
   ASSERT_EQ(0, rbd_encryption_load(
           image, RBD_ENCRYPTION_FORMAT_LUKS, &luks_opts, sizeof(luks_opts)));
   ASSERT_PASSED(read_test_data, image, "test", 0, 4, 0);
@@ -2307,7 +2332,12 @@ TEST_F(TestLibRBD, TestEncryptionLUKS2)
           _cluster, "rbd_read_from_replica_policy", "balance"));
 
   rbd_image_t image;
-  rbd_encryption_luks2_format_options_t opts = {
+  rbd_encryption_luks1_format_options_t luks1_opts = {
+          .alg = RBD_ENCRYPTION_ALGORITHM_AES256,
+          .passphrase = "password",
+          .passphrase_size = 8,
+  };
+  rbd_encryption_luks2_format_options_t luks2_opts = {
           .alg = RBD_ENCRYPTION_ALGORITHM_AES256,
           .passphrase = "password",
           .passphrase_size = 8,
@@ -2320,9 +2350,13 @@ TEST_F(TestLibRBD, TestEncryptionLUKS2)
 
 #ifndef HAVE_LIBCRYPTSETUP
   ASSERT_EQ(-ENOTSUP, rbd_encryption_format(
-          image, RBD_ENCRYPTION_FORMAT_LUKS2, &opts, sizeof(opts)));
+          image, RBD_ENCRYPTION_FORMAT_LUKS1, &luks1_opts, sizeof(luks1_opts)));
+  ASSERT_EQ(-ENOTSUP, rbd_encryption_load(
+          image, RBD_ENCRYPTION_FORMAT_LUKS1, &luks1_opts, sizeof(luks1_opts)));
+  ASSERT_EQ(-ENOTSUP, rbd_encryption_format(
+          image, RBD_ENCRYPTION_FORMAT_LUKS2, &luks2_opts, sizeof(luks2_opts)));
   ASSERT_EQ(-ENOTSUP, rbd_encryption_load(
-          image, RBD_ENCRYPTION_FORMAT_LUKS2, &opts, sizeof(opts)));
+          image, RBD_ENCRYPTION_FORMAT_LUKS2, &luks2_opts, sizeof(luks2_opts)));
   ASSERT_EQ(-ENOTSUP, rbd_encryption_format(
           image, RBD_ENCRYPTION_FORMAT_LUKS, &luks_opts, sizeof(luks_opts)));
   ASSERT_EQ(-ENOTSUP, rbd_encryption_load(
@@ -2331,9 +2365,9 @@ TEST_F(TestLibRBD, TestEncryptionLUKS2)
   ASSERT_EQ(-EINVAL, rbd_encryption_format(
           image, RBD_ENCRYPTION_FORMAT_LUKS, &luks_opts, sizeof(luks_opts)));
   ASSERT_EQ(0, rbd_encryption_format(
-          image, RBD_ENCRYPTION_FORMAT_LUKS2, &opts, sizeof(opts)));
+          image, RBD_ENCRYPTION_FORMAT_LUKS2, &luks2_opts, sizeof(luks2_opts)));
   ASSERT_EQ(-EEXIST, rbd_encryption_load(
-          image, RBD_ENCRYPTION_FORMAT_LUKS2, &opts, sizeof(opts)));
+          image, RBD_ENCRYPTION_FORMAT_LUKS2, &luks2_opts, sizeof(luks2_opts)));
 
   test_io(image);
 
@@ -2341,6 +2375,16 @@ TEST_F(TestLibRBD, TestEncryptionLUKS2)
   ASSERT_EQ(0, rbd_close(image));
 
   ASSERT_EQ(0, rbd_open(ioctx, name.c_str(), &image, NULL));
+  ASSERT_EQ(-EINVAL, rbd_encryption_load(
+          image, RBD_ENCRYPTION_FORMAT_LUKS1, &luks1_opts, sizeof(luks1_opts)));
+  ASSERT_EQ(0, rbd_encryption_load(
+          image, RBD_ENCRYPTION_FORMAT_LUKS2, &luks2_opts, sizeof(luks2_opts)));
+  ASSERT_PASSED(read_test_data, image, "test", 0, 4, 0);
+  ASSERT_EQ(0, rbd_close(image));
+
+  ASSERT_EQ(0, rbd_open(ioctx, name.c_str(), &image, NULL));
+  ASSERT_EQ(-EINVAL, rbd_encryption_load(
+          image, RBD_ENCRYPTION_FORMAT_LUKS1, &luks1_opts, sizeof(luks1_opts)));
   ASSERT_EQ(0, rbd_encryption_load(
           image, RBD_ENCRYPTION_FORMAT_LUKS, &luks_opts, sizeof(luks_opts)));
   ASSERT_PASSED(read_test_data, image, "test", 0, 4, 0);
@@ -2426,9 +2470,13 @@ TEST_F(TestLibRBD, TestCloneEncryption)
   ASSERT_EQ(0, rbd_encryption_format(
           child2, RBD_ENCRYPTION_FORMAT_LUKS2, &child2_opts,
           sizeof(child2_opts)));
+  rbd_encryption_luks_format_options_t child2_lopts = {
+          .passphrase = "password",
+          .passphrase_size = 8,
+  };
   ASSERT_EQ(0, rbd_encryption_load(
-        child2, RBD_ENCRYPTION_FORMAT_LUKS2, &child2_opts,
-        sizeof(child2_opts)));
+          child2, RBD_ENCRYPTION_FORMAT_LUKS, &child2_lopts,
+          sizeof(child2_lopts)));
   ASSERT_PASSED(write_test_data, child2, "cccc", 128 << 20, 4, 0);
   ASSERT_EQ(0, rbd_flush(child2));
 
@@ -2864,6 +2912,124 @@ TEST_F(TestLibRBD, EncryptionLoadBadStripePattern)
   }
 }
 
+TEST_F(TestLibRBD, EncryptionLoadFormatMismatch)
+{
+  REQUIRE_FEATURE(RBD_FEATURE_LAYERING);
+  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;
+  std::string name1 = get_temp_image_name();
+  std::string name2 = get_temp_image_name();
+  std::string name3 = get_temp_image_name();
+  std::string passphrase = "some passphrase";
+
+  librbd::encryption_luks1_format_options_t luks1_opts = {
+      RBD_ENCRYPTION_ALGORITHM_AES256, passphrase};
+  librbd::encryption_luks2_format_options_t luks2_opts = {
+      RBD_ENCRYPTION_ALGORITHM_AES256, passphrase};
+  librbd::encryption_luks_format_options_t luks_opts = {passphrase};
+
+#define LUKS_ONE {RBD_ENCRYPTION_FORMAT_LUKS1, &luks1_opts, sizeof(luks1_opts)}
+#define LUKS_TWO {RBD_ENCRYPTION_FORMAT_LUKS2, &luks2_opts, sizeof(luks2_opts)}
+#define LUKS_ANY {RBD_ENCRYPTION_FORMAT_LUKS, &luks_opts, sizeof(luks_opts)}
+
+  const std::vector<librbd::encryption_spec_t> bad_specs[] = {
+      {},
+      {LUKS_ONE},
+      {LUKS_TWO},
+      {LUKS_ONE, LUKS_ONE},
+      {LUKS_ONE, LUKS_TWO},
+      {LUKS_ONE, LUKS_ANY},
+      {LUKS_TWO, LUKS_TWO},
+      {LUKS_ANY, LUKS_TWO},
+      {LUKS_ONE, LUKS_ONE, LUKS_ONE},
+      {LUKS_ONE, LUKS_ONE, LUKS_TWO},
+      {LUKS_ONE, LUKS_ONE, LUKS_ANY},
+      {LUKS_ONE, LUKS_TWO, LUKS_ONE},
+      {LUKS_ONE, LUKS_TWO, LUKS_TWO},
+      {LUKS_ONE, LUKS_TWO, LUKS_ANY},
+      {LUKS_ONE, LUKS_ANY, LUKS_ONE},
+      {LUKS_ONE, LUKS_ANY, LUKS_TWO},
+      {LUKS_ONE, LUKS_ANY, LUKS_ANY},
+      {LUKS_TWO, LUKS_ONE, LUKS_TWO},
+      {LUKS_TWO, LUKS_TWO, LUKS_ONE},
+      {LUKS_TWO, LUKS_TWO, LUKS_TWO},
+      {LUKS_TWO, LUKS_TWO, LUKS_ANY},
+      {LUKS_TWO, LUKS_ANY, LUKS_TWO},
+      {LUKS_ANY, LUKS_ONE, LUKS_TWO},
+      {LUKS_ANY, LUKS_TWO, LUKS_ONE},
+      {LUKS_ANY, LUKS_TWO, LUKS_TWO},
+      {LUKS_ANY, LUKS_TWO, LUKS_ANY},
+      {LUKS_ANY, LUKS_ANY, LUKS_TWO},
+      {LUKS_ANY, LUKS_ANY, LUKS_ANY, LUKS_ANY}};
+
+  const std::vector<librbd::encryption_spec_t> good_specs[] = {
+      {LUKS_ANY},
+      {LUKS_TWO, LUKS_ONE},
+      {LUKS_TWO, LUKS_ANY},
+      {LUKS_ANY, LUKS_ONE},
+      {LUKS_ANY, LUKS_ANY},
+      {LUKS_TWO, LUKS_ONE, LUKS_ONE},
+      {LUKS_TWO, LUKS_ONE, LUKS_ANY},
+      {LUKS_TWO, LUKS_ANY, LUKS_ONE},
+      {LUKS_TWO, LUKS_ANY, LUKS_ANY},
+      {LUKS_ANY, LUKS_ONE, LUKS_ONE},
+      {LUKS_ANY, LUKS_ONE, LUKS_ANY},
+      {LUKS_ANY, LUKS_ANY, LUKS_ONE},
+      {LUKS_ANY, LUKS_ANY, LUKS_ANY}};
+
+  static_assert(std::size(bad_specs) + std::size(good_specs) == 1 + 3 + 9 + 27 + 1);
+
+#undef LUKS_ONE
+#undef LUKS_TWO
+#undef LUKS_ANY
+
+  {
+    int order = 0;
+    ASSERT_EQ(0, create_image_pp(rbd, ioctx, name1.c_str(), 20 << 20, &order));
+    librbd::Image image1;
+    ASSERT_EQ(0, rbd.open(ioctx, image1, name1.c_str(), nullptr));
+    ASSERT_EQ(0, image1.encryption_format(RBD_ENCRYPTION_FORMAT_LUKS1,
+                                          &luks1_opts, sizeof(luks1_opts)));
+
+    ASSERT_EQ(0, image1.snap_create("snap"));
+    ASSERT_EQ(0, image1.snap_protect("snap"));
+    uint64_t features;
+    ASSERT_EQ(0, image1.features(&features));
+    ASSERT_EQ(0, rbd.clone(ioctx, name1.c_str(), "snap", ioctx, name2.c_str(),
+                           features, &order));
+
+    librbd::Image image2;
+    ASSERT_EQ(0, rbd.open(ioctx, image2, name2.c_str(), nullptr));
+    ASSERT_EQ(0, image2.snap_create("snap"));
+    ASSERT_EQ(0, image2.snap_protect("snap"));
+    ASSERT_EQ(0, rbd.clone(ioctx, name2.c_str(), "snap", ioctx, name3.c_str(),
+                           features, &order));
+
+    librbd::Image image3;
+    ASSERT_EQ(0, rbd.open(ioctx, image3, name3.c_str(), nullptr));
+    ASSERT_EQ(0, image3.encryption_format(RBD_ENCRYPTION_FORMAT_LUKS2,
+                                          &luks2_opts, sizeof(luks2_opts)));
+  }
+
+  {
+    librbd::Image image3;
+    ASSERT_EQ(0, rbd.open(ioctx, image3, name3.c_str(), nullptr));
+    for (auto& specs : bad_specs) {
+      ASSERT_EQ(-EINVAL, image3.encryption_load2(specs.data(), specs.size()));
+    }
+  }
+
+  for (auto& specs : good_specs) {
+    librbd::Image image3;
+    ASSERT_EQ(0, rbd.open(ioctx, image3, name3.c_str(), nullptr));
+    ASSERT_EQ(0, image3.encryption_load2(specs.data(), specs.size()));
+  }
+}
+
 TEST_F(TestLibRBD, EncryptedResize)
 {
   REQUIRE(!is_feature_enabled(RBD_FEATURE_JOURNALING));