From: Casey Bodley <cbodley@redhat.com>
Date: Wed, 28 Jun 2023 20:49:33 +0000 (-0400)
Subject: rgw: BlockDecrypt filter parses manifest parts before construction
X-Git-Tag: v18.2.1~361^2~6
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=f82d6b824d60719ee17d32d51ba2f1d862437d37;p=ceph.git

rgw: BlockDecrypt filter parses manifest parts before construction

users now call a static read_manifest_parts() function, and pass the
resulting vector into the BlockDecrypt constructor

Signed-off-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit d8a82f5aaa1a04b695c19b546767718b832559cb)

Conflicts: optional_yield argument not on reef
	src/rgw/rgw_crypt.cc
	src/rgw/rgw_crypt.h
	src/rgw/rgw_rest_s3.cc
	src/test/rgw/test_rgw_crypto.cc
---

diff --git a/src/rgw/rgw_crypt.cc b/src/rgw/rgw_crypt.cc
index e4fd06168557..6ac2d12df47e 100644
--- a/src/rgw/rgw_crypt.cc
+++ b/src/rgw/rgw_crypt.cc
@@ -635,7 +635,8 @@ bool AES_256_ECB_encrypt(const DoutPrefixProvider* dpp,
 RGWGetObj_BlockDecrypt::RGWGetObj_BlockDecrypt(const DoutPrefixProvider *dpp,
                                                CephContext* cct,
                                                RGWGetObj_Filter* next,
-                                               std::unique_ptr<BlockCrypt> crypt)
+                                               std::unique_ptr<BlockCrypt> crypt,
+                                               std::vector<size_t> parts_len)
     :
     RGWGetObj_Filter(next),
     dpp(dpp),
@@ -644,7 +645,8 @@ RGWGetObj_BlockDecrypt::RGWGetObj_BlockDecrypt(const DoutPrefixProvider *dpp,
     enc_begin_skip(0),
     ofs(0),
     end(0),
-    cache()
+    cache(),
+    parts_len(std::move(parts_len))
 {
   block_size = this->crypt->get_block_size();
 }
@@ -652,8 +654,10 @@ RGWGetObj_BlockDecrypt::RGWGetObj_BlockDecrypt(const DoutPrefixProvider *dpp,
 RGWGetObj_BlockDecrypt::~RGWGetObj_BlockDecrypt() {
 }
 
-int RGWGetObj_BlockDecrypt::read_manifest(const DoutPrefixProvider *dpp, bufferlist& manifest_bl) {
-  parts_len.clear();
+int RGWGetObj_BlockDecrypt::read_manifest_parts(const DoutPrefixProvider *dpp,
+                                                const bufferlist& manifest_bl,
+                                                std::vector<size_t>& parts_len)
+{
   RGWObjManifest manifest;
   if (manifest_bl.length()) {
     auto miter = manifest_bl.cbegin();
@@ -670,10 +674,8 @@ int RGWGetObj_BlockDecrypt::read_manifest(const DoutPrefixProvider *dpp, bufferl
       }
       parts_len.back() += mi.get_stripe_size();
     }
-    if (cct->_conf->subsys.should_gather<ceph_subsys_rgw, 20>()) {
-      for (size_t i = 0; i<parts_len.size(); i++) {
-        ldpp_dout(dpp, 20) << "Manifest part " << i << ", size=" << parts_len[i] << dendl;
-      }
+    for (size_t i = 0; i<parts_len.size(); i++) {
+      ldpp_dout(dpp, 20) << "Manifest part " << i << ", size=" << parts_len[i] << dendl;
     }
   }
   return 0;
diff --git a/src/rgw/rgw_crypt.h b/src/rgw/rgw_crypt.h
index 6008dd05eaea..d8f561eca58f 100644
--- a/src/rgw/rgw_crypt.h
+++ b/src/rgw/rgw_crypt.h
@@ -97,16 +97,16 @@ class RGWGetObj_BlockDecrypt : public RGWGetObj_Filter {
   off_t end; /**< stream offset of last byte that is requested */
   bufferlist cache; /**< stores extra data that could not (yet) be processed by BlockCrypt */
   size_t block_size; /**< snapshot of \ref BlockCrypt.get_block_size() */
+  std::vector<size_t> parts_len; /**< size of parts of multipart object, parsed from manifest */
 
   int process(bufferlist& cipher, size_t part_ofs, size_t size);
 
-protected:
-  std::vector<size_t> parts_len; /**< size of parts of multipart object, parsed from manifest */
 public:
   RGWGetObj_BlockDecrypt(const DoutPrefixProvider *dpp,
                          CephContext* cct,
                          RGWGetObj_Filter* next,
-                         std::unique_ptr<BlockCrypt> crypt);
+                         std::unique_ptr<BlockCrypt> crypt,
+                         std::vector<size_t> parts_len);
   virtual ~RGWGetObj_BlockDecrypt();
 
   virtual int fixup_range(off_t& bl_ofs,
@@ -116,7 +116,9 @@ public:
                           off_t bl_len) override;
   virtual int flush() override;
 
-  int read_manifest(const DoutPrefixProvider *dpp, bufferlist& manifest_bl);
+  static int read_manifest_parts(const DoutPrefixProvider *dpp,
+                                 const bufferlist& manifest_bl,
+                                 std::vector<size_t>& parts_len);
 }; /* RGWGetObj_BlockDecrypt */
 
 
diff --git a/src/rgw/rgw_rest_s3.cc b/src/rgw/rgw_rest_s3.cc
index 56d00dae3ca9..97f2c261ea33 100644
--- a/src/rgw/rgw_rest_s3.cc
+++ b/src/rgw/rgw_rest_s3.cc
@@ -592,21 +592,28 @@ int RGWGetObj_ObjStore_S3::get_decrypt_filter(std::unique_ptr<RGWGetObj_Filter>
     return 0;
   }
 
-  int res = 0;
   std::unique_ptr<BlockCrypt> block_crypt;
-  res = rgw_s3_prepare_decrypt(s, attrs, &block_crypt, crypt_http_responses);
-  if (res == 0) {
-    if (block_crypt != nullptr) {
-      auto f = std::make_unique<RGWGetObj_BlockDecrypt>(s, s->cct, cb, std::move(block_crypt));
-      if (manifest_bl != nullptr) {
-        res = f->read_manifest(this, *manifest_bl);
-        if (res == 0) {
-          *filter = std::move(f);
-        }
-      }
-    }
+  int res = rgw_s3_prepare_decrypt(s, attrs, &block_crypt, crypt_http_responses);
+  if (res < 0) {
+    return res;
   }
-  return res;
+  if (block_crypt == nullptr) {
+    return 0;
+  }
+
+  // in case of a multipart upload, we need to know the part lengths to
+  // correctly decrypt across part boundaries
+  std::vector<size_t> parts_len;
+  res = RGWGetObj_BlockDecrypt::read_manifest_parts(this, *manifest_bl,
+                                                    parts_len);
+  if (res < 0) {
+    return res;
+  }
+
+  *filter = std::make_unique<RGWGetObj_BlockDecrypt>(
+      s, s->cct, cb, std::move(block_crypt),
+      std::move(parts_len));
+  return 0;
 }
 int RGWGetObj_ObjStore_S3::verify_requester(const rgw::auth::StrategyRegistry& auth_registry, optional_yield y) 
 {
@@ -2744,24 +2751,28 @@ int RGWPutObj_ObjStore_S3::get_decrypt_filter(
 {
   std::map<std::string, std::string> crypt_http_responses_unused;
 
-  int res = 0;
   std::unique_ptr<BlockCrypt> block_crypt;
-  res = rgw_s3_prepare_decrypt(s, attrs, &block_crypt, crypt_http_responses_unused);
-  if (res == 0) {
-    if (block_crypt != nullptr) {
-      auto f = std::unique_ptr<RGWGetObj_BlockDecrypt>(new RGWGetObj_BlockDecrypt(s, s->cct, cb, std::move(block_crypt)));
-      //RGWGetObj_BlockDecrypt* f = new RGWGetObj_BlockDecrypt(s->cct, cb, std::move(block_crypt));
-      if (f != nullptr) {
-        if (manifest_bl != nullptr) {
-          res = f->read_manifest(this, *manifest_bl);
-          if (res == 0) {
-            *filter = std::move(f);
-          }
-        }
-      }
-    }
+  int res = rgw_s3_prepare_decrypt(s, attrs, &block_crypt, crypt_http_responses_unused);
+  if (res < 0) {
+    return res;
   }
-  return res;
+  if (block_crypt == nullptr) {
+    return 0;
+  }
+
+  // in case of a multipart upload, we need to know the part lengths to
+  // correctly decrypt across part boundaries
+  std::vector<size_t> parts_len;
+  res = RGWGetObj_BlockDecrypt::read_manifest_parts(this, *manifest_bl,
+                                                    parts_len);
+  if (res < 0) {
+    return res;
+  }
+
+  *filter = std::make_unique<RGWGetObj_BlockDecrypt>(
+      s, s->cct, cb, std::move(block_crypt),
+      std::move(parts_len));
+  return 0;
 }
 
 int RGWPutObj_ObjStore_S3::get_encrypt_filter(
diff --git a/src/test/rgw/test_rgw_crypto.cc b/src/test/rgw/test_rgw_crypto.cc
index 92df403fd09b..b85c78eb2908 100644
--- a/src/test/rgw/test_rgw_crypto.cc
+++ b/src/test/rgw/test_rgw_crypto.cc
@@ -426,7 +426,7 @@ TEST(TestRGWCrypto, verify_RGWGetObj_BlockDecrypt_ranges)
     ut_get_sink get_sink;
     auto cbc = AES_256_CBC_create(&no_dpp, g_ceph_context, &key[0], 32);
     ASSERT_NE(cbc.get(), nullptr);
-    RGWGetObj_BlockDecrypt decrypt(&no_dpp, g_ceph_context, &get_sink, std::move(cbc));
+    RGWGetObj_BlockDecrypt decrypt(&no_dpp, g_ceph_context, &get_sink, std::move(cbc), {});
 
     //random ranges
     off_t begin = (r/3)*r*(r+13)*(r+23)*(r+53)*(r+71) % test_range;
@@ -472,7 +472,7 @@ TEST(TestRGWCrypto, verify_RGWGetObj_BlockDecrypt_chunks)
     ut_get_sink get_sink;
     auto cbc = AES_256_CBC_create(&no_dpp, g_ceph_context, &key[0], 32);
     ASSERT_NE(cbc.get(), nullptr);
-    RGWGetObj_BlockDecrypt decrypt(&no_dpp, g_ceph_context, &get_sink, std::move(cbc));
+    RGWGetObj_BlockDecrypt decrypt(&no_dpp, g_ceph_context, &get_sink, std::move(cbc), {});
 
     //random
     off_t begin = (r/3)*r*(r+13)*(r+23)*(r+53)*(r+71) % test_range;
@@ -518,7 +518,7 @@ TEST(TestRGWCrypto, check_RGWGetObj_BlockDecrypt_fixup)
   ut_get_sink get_sink;
   auto nonecrypt = std::unique_ptr<BlockCrypt>(new BlockCryptNone);
   RGWGetObj_BlockDecrypt decrypt(&no_dpp, g_ceph_context, &get_sink,
-                                 std::move(nonecrypt));
+                                 std::move(nonecrypt), {});
   ASSERT_EQ(fixup_range(&decrypt,0,0),     range_t(0,255));
   ASSERT_EQ(fixup_range(&decrypt,1,256),   range_t(0,511));
   ASSERT_EQ(fixup_range(&decrypt,0,255),   range_t(0,255));
@@ -527,16 +527,6 @@ TEST(TestRGWCrypto, check_RGWGetObj_BlockDecrypt_fixup)
   ASSERT_EQ(fixup_range(&decrypt,513,1024), range_t(512,1024+255));
 }
 
-using parts_len_t = std::vector<size_t>;
-
-class TestRGWGetObj_BlockDecrypt : public RGWGetObj_BlockDecrypt {
-  using RGWGetObj_BlockDecrypt::RGWGetObj_BlockDecrypt;
-public:
-  void set_parts_len(parts_len_t&& other) {
-    parts_len = std::move(other);
-  }
-};
-
 std::vector<size_t> create_mp_parts(size_t obj_size, size_t mp_part_len){
   std::vector<size_t> parts_len;
   size_t part_size;
@@ -559,9 +549,9 @@ TEST(TestRGWCrypto, check_RGWGetObj_BlockDecrypt_fixup_simple)
 
   ut_get_sink get_sink;
   auto nonecrypt = std::make_unique<BlockCryptNone>(4096);
-  TestRGWGetObj_BlockDecrypt decrypt(&no_dpp, g_ceph_context, &get_sink,
-				     std::move(nonecrypt));
-  decrypt.set_parts_len(create_mp_parts(obj_size, part_size));
+  RGWGetObj_BlockDecrypt decrypt(&no_dpp, g_ceph_context, &get_sink,
+				 std::move(nonecrypt),
+				 create_mp_parts(obj_size, part_size));
   ASSERT_EQ(fixup_range(&decrypt,0,0),     range_t(0,4095));
   ASSERT_EQ(fixup_range(&decrypt,1,4096),   range_t(0,8191));
   ASSERT_EQ(fixup_range(&decrypt,0,4095),   range_t(0,4095));
@@ -588,12 +578,13 @@ TEST(TestRGWCrypto, check_RGWGetObj_BlockDecrypt_fixup_non_aligned_obj_size)
 {
   const NoDoutPrefix no_dpp(g_ceph_context, dout_subsys);
 
+  const size_t na_obj_size = obj_size + 1;
+
   ut_get_sink get_sink;
   auto nonecrypt = std::make_unique<BlockCryptNone>(4096);
-  TestRGWGetObj_BlockDecrypt decrypt(&no_dpp, g_ceph_context, &get_sink,
-				     std::move(nonecrypt));
-  auto na_obj_size = obj_size + 1;
-  decrypt.set_parts_len(create_mp_parts(na_obj_size, part_size));
+  RGWGetObj_BlockDecrypt decrypt(&no_dpp, g_ceph_context, &get_sink,
+				 std::move(nonecrypt),
+				 create_mp_parts(na_obj_size, part_size));
 
   // these should be unaffected here
   ASSERT_EQ(fixup_range(&decrypt, 0, part_size - 2), range_t(0, part_size -1));
@@ -616,12 +607,13 @@ TEST(TestRGWCrypto, check_RGWGetObj_BlockDecrypt_fixup_non_aligned_part_size)
 {
   const NoDoutPrefix no_dpp(g_ceph_context, dout_subsys);
 
+  const size_t na_part_size = part_size + 1;
+
   ut_get_sink get_sink;
   auto nonecrypt = std::make_unique<BlockCryptNone>(4096);
-  TestRGWGetObj_BlockDecrypt decrypt(&no_dpp, g_ceph_context, &get_sink,
-				     std::move(nonecrypt));
-  auto na_part_size = part_size + 1;
-  decrypt.set_parts_len(create_mp_parts(obj_size, na_part_size));
+  RGWGetObj_BlockDecrypt decrypt(&no_dpp, g_ceph_context, &get_sink,
+				 std::move(nonecrypt),
+				 create_mp_parts(obj_size, na_part_size));
 
   // na_part_size -2, ie. part_size -1  is aligned to 4095 boundary
   ASSERT_EQ(fixup_range(&decrypt, 0, na_part_size - 2), range_t(0, na_part_size -2));
@@ -650,13 +642,14 @@ TEST(TestRGWCrypto, check_RGWGetObj_BlockDecrypt_fixup_non_aligned)
 {
   const NoDoutPrefix no_dpp(g_ceph_context, dout_subsys);
 
+  const size_t na_part_size = part_size + 1;
+  const size_t na_obj_size = obj_size + 7; // (6*(5MiB + 1) + 1) for the last 1B overflow
+
   ut_get_sink get_sink;
   auto nonecrypt = std::make_unique<BlockCryptNone>(4096);
-  TestRGWGetObj_BlockDecrypt decrypt(&no_dpp, g_ceph_context, &get_sink,
-				     std::move(nonecrypt));
-  auto na_part_size = part_size + 1;
-  auto na_obj_size = obj_size + 7; // (6*(5MiB + 1) + 1) for the last 1B overflow
-  decrypt.set_parts_len(create_mp_parts(na_obj_size, na_part_size));
+  RGWGetObj_BlockDecrypt decrypt(&no_dpp, g_ceph_context, &get_sink,
+				 std::move(nonecrypt),
+				 create_mp_parts(na_obj_size, na_part_size));
 
   // na_part_size -2, ie. part_size -1  is aligned to 4095 boundary
   ASSERT_EQ(fixup_range(&decrypt, 0, na_part_size - 2), range_t(0, na_part_size -2));
@@ -682,10 +675,10 @@ TEST(TestRGWCrypto, check_RGWGetObj_BlockDecrypt_fixup_invalid_ranges)
 
   ut_get_sink get_sink;
   auto nonecrypt = std::make_unique<BlockCryptNone>(4096);
-  TestRGWGetObj_BlockDecrypt decrypt(&no_dpp, g_ceph_context, &get_sink,
-				     std::move(nonecrypt));
+  RGWGetObj_BlockDecrypt decrypt(&no_dpp, g_ceph_context, &get_sink,
+				 std::move(nonecrypt),
+				 create_mp_parts(obj_size, part_size));
 
-  decrypt.set_parts_len(create_mp_parts(obj_size, part_size));
 
   // the ranges below would be mostly unreachable in current code as rgw
   // would've returned a 411 before reaching, but we're just doing this to make
@@ -795,7 +788,8 @@ TEST(TestRGWCrypto, verify_Encrypt_Decrypt)
 
     ut_get_sink get_sink;
     RGWGetObj_BlockDecrypt decrypt(&no_dpp, g_ceph_context, &get_sink,
-                                   AES_256_CBC_create(&no_dpp, g_ceph_context, &key[0], 32));
+                                   AES_256_CBC_create(&no_dpp, g_ceph_context, &key[0], 32),
+                                   {});
 
     off_t bl_ofs = 0;
     off_t bl_end = test_size - 1;