From b3deeb152a9d20d861d17fc0310030ccfd81f2ce Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Fri, 16 Sep 2022 18:07:03 +0200 Subject: [PATCH] librbd: check stripe pattern when loading encryption Currently it's done in FormatRequest but not in LoadRequest. However an image can be deep copied or exported and imported with a different stripe pattern such that an area boundary would fall in the middle of an object. Signed-off-by: Ilya Dryomov --- src/librbd/crypto/luks/LoadRequest.cc | 8 ++ .../crypto/luks/test_mock_LoadRequest.cc | 25 ++++++ src/test/librbd/test_librbd.cc | 80 +++++++++++++++++++ 3 files changed, 113 insertions(+) diff --git a/src/librbd/crypto/luks/LoadRequest.cc b/src/librbd/crypto/luks/LoadRequest.cc index bddf771a8bd..f23f5de41a6 100644 --- a/src/librbd/crypto/luks/LoadRequest.cc +++ b/src/librbd/crypto/luks/LoadRequest.cc @@ -185,6 +185,14 @@ void LoadRequest::handle_read_header(int r) { return; } + uint64_t stripe_period = m_image_ctx->get_stripe_period(); + if (m_header.get_data_offset() % stripe_period != 0) { + lderr(m_image_ctx->cct) << "incompatible stripe pattern, 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 fb8699de9b1..313513751f2 100644 --- a/src/test/librbd/crypto/luks/test_mock_LoadRequest.cc +++ b/src/test/librbd/crypto/luks/test_mock_LoadRequest.cc @@ -108,12 +108,18 @@ struct TestMockCryptoLuksLoadRequest : public TestMockFixture { EXPECT_CALL(*mock_image_ctx, get_image_size(_)).WillOnce( Return(size)); } + + void expect_get_stripe_period(uint64_t period) { + EXPECT_CALL(*mock_image_ctx, get_stripe_period()).WillOnce( + Return(period)); + } }; 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); + expect_get_stripe_period(OBJECT_SIZE); mock_load_request->send(); image_read_request->complete(DEFAULT_INITIAL_READ_SIZE); ASSERT_EQ(0, finished_cond.wait()); @@ -125,6 +131,7 @@ 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); + expect_get_stripe_period(OBJECT_SIZE); mock_load_request->send(); image_read_request->complete(DEFAULT_INITIAL_READ_SIZE); ASSERT_EQ(0, finished_cond.wait()); @@ -140,6 +147,7 @@ TEST_F(TestMockCryptoLuksLoadRequest, LUKS1) { 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()); @@ -190,6 +198,18 @@ TEST_F(TestMockCryptoLuksLoadRequest, BadSize) { ASSERT_EQ("LUKS2", detected_format_name); } +TEST_F(TestMockCryptoLuksLoadRequest, BadStripePattern) { + 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); + expect_get_stripe_period(OBJECT_SIZE * 3); + 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); @@ -197,6 +217,7 @@ TEST_F(TestMockCryptoLuksLoadRequest, HeaderBiggerThanInitialRead) { mock_load_request->send(); expect_get_image_size(OBJECT_SIZE << 5); + expect_get_stripe_period(OBJECT_SIZE); expect_image_read(4096, MAXIMUM_HEADER_SIZE - 4096); image_read_request->complete(4096); // complete initial read @@ -215,6 +236,7 @@ TEST_F(TestMockCryptoLuksLoadRequest, LUKS1FormattedClone) { 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); + expect_get_stripe_period(OBJECT_SIZE); mock_load_request->send(); image_read_request->complete(DEFAULT_INITIAL_READ_SIZE); ASSERT_EQ(0, finished_cond.wait()); @@ -227,6 +249,7 @@ TEST_F(TestMockCryptoLuksLoadRequest, LUKS2FormattedClone) { 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); + expect_get_stripe_period(OBJECT_SIZE); mock_load_request->send(); image_read_request->complete(DEFAULT_INITIAL_READ_SIZE); ASSERT_EQ(0, finished_cond.wait()); @@ -241,6 +264,7 @@ TEST_F(TestMockCryptoLuksLoadRequest, KeyslotsBiggerThanInitialRead) { mock_load_request->send(); expect_get_image_size(OBJECT_SIZE << 5); + expect_get_stripe_period(OBJECT_SIZE); expect_image_read(16384, data_offset - 16384); image_read_request->complete(16384); // complete initial read @@ -258,6 +282,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); + expect_get_stripe_period(OBJECT_SIZE); 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 9e2e7a69e4c..be9ffabc4c5 100644 --- a/src/test/librbd/test_librbd.cc +++ b/src/test/librbd/test_librbd.cc @@ -2625,6 +2625,86 @@ TEST_F(TestLibRBD, EncryptionLoadBadSize) } } +TEST_F(TestLibRBD, EncryptionLoadBadStripePattern) +{ + REQUIRE_FEATURE(RBD_FEATURE_STRIPINGV2); + REQUIRE(!is_feature_enabled(RBD_FEATURE_JOURNALING)); + + librados::IoCtx ioctx; + ASSERT_EQ(0, _rados.ioctx_create(m_pool_name.c_str(), ioctx)); + + bool old_format; + uint64_t features; + ASSERT_EQ(0, get_features(&old_format, &features)); + ASSERT_FALSE(old_format); + + librbd::RBD rbd; + auto name1 = get_temp_image_name(); + auto name2 = get_temp_image_name(); + auto name3 = get_temp_image_name(); + std::string passphrase = "some passphrase"; + + { + int order = 22; + ASSERT_EQ(0, rbd.create3(ioctx, name1.c_str(), 20 << 20, features, &order, + 2 << 20, 2)); + librbd::Image image; + ASSERT_EQ(0, rbd.open(ioctx, image, name1.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))); + uint64_t size; + ASSERT_EQ(0, image.size(&size)); + ASSERT_EQ(12 << 20, size); + } + + { + librbd::Image image; + ASSERT_EQ(0, rbd.open(ioctx, image, name1.c_str(), nullptr)); + + // different but compatible striping pattern + librbd::ImageOptions image_opts; + ASSERT_EQ(0, image_opts.set(RBD_IMAGE_OPTION_STRIPE_UNIT, 1 << 20)); + ASSERT_EQ(0, image_opts.set(RBD_IMAGE_OPTION_STRIPE_COUNT, 2)); + ASSERT_EQ(0, image.deep_copy(ioctx, name2.c_str(), image_opts)); + } + { + librbd::Image image; + ASSERT_EQ(0, rbd.open(ioctx, image, name2.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(12 << 20, size); + } + + { + librbd::Image image; + ASSERT_EQ(0, rbd.open(ioctx, image, name1.c_str(), nullptr)); + + // incompatible striping pattern + librbd::ImageOptions image_opts; + ASSERT_EQ(0, image_opts.set(RBD_IMAGE_OPTION_STRIPE_UNIT, 1 << 20)); + ASSERT_EQ(0, image_opts.set(RBD_IMAGE_OPTION_STRIPE_COUNT, 3)); + ASSERT_EQ(0, image.deep_copy(ioctx, name3.c_str(), image_opts)); + } + { + librbd::Image image; + ASSERT_EQ(0, rbd.open(ioctx, image, name3.c_str(), nullptr)); + + 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(20 << 20, size); + } +} + #endif TEST_F(TestLibRBD, TestIOWithIOHint) -- 2.39.5