msg/async/crypto_onwire: perform decryption in place

OpenSSL supports in-place decryption so we can avoid
allocating potentially multi-megabyte and strictly aligned
buffer for each decryption operation.

ProtocolV2 actually gets the alignment wrong: after
read_frame_segment() allocates with cur_rx_desc.alignment,
handle_read_frame_segment() effectively replaces that with
segment_t::DEFAULT_ALIGNMENT.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit fe97a000990632dbe9734e0d3a96df627f104f7a)

diff --git a/src/msg/async/ProtocolV2.cc b/src/msg/async/ProtocolV2.cc
index 10ea6d2eccfb5abcb5712667571cb5ef1139f560..a1ebe6baffbec32b05aa7535d09f33790a577e74 100644 (file)
--- a/src/msg/async/ProtocolV2.cc
+++ b/src/msg/async/ProtocolV2.cc
@@ -1075,8 +1075,7 @@ CtPtr ProtocolV2::handle_read_frame_preamble_main(rx_buffer_t &&buffer, int r) {
     ceph_assert(session_stream_handlers.rx);
 
     session_stream_handlers.rx->reset_rx_handler();
-    preamble = session_stream_handlers.rx->authenticated_decrypt_update(
-      std::move(preamble), segment_t::DEFAULT_ALIGNMENT);
+    session_stream_handlers.rx->authenticated_decrypt_update(preamble);
 
     ldout(cct, 10) << __func__ << " got encrypted preamble."
                    << " after decrypt premable.length()=" << preamble.length()
@@ -1224,11 +1223,12 @@ CtPtr ProtocolV2::handle_read_frame_segment(rx_buffer_t &&rx_buffer, int r) {
 
     auto& new_seg = rx_segments_data.back();
     if (new_seg.length()) {
-      auto padded = session_stream_handlers.rx->authenticated_decrypt_update(
-          std::move(new_seg), segment_t::DEFAULT_ALIGNMENT);
+      session_stream_handlers.rx->authenticated_decrypt_update(new_seg);
       const auto idx = rx_segments_data.size() - 1;
-      new_seg.clear();
-      padded.splice(0, rx_segments_desc[idx].length, &new_seg);
+      if (new_seg.length() > rx_segments_desc[idx].length) {
+        new_seg.splice(rx_segments_desc[idx].length,
+                       new_seg.length() - rx_segments_desc[idx].length);
+      }
 
       ldout(cct, 20) << __func__
                      << " unpadded new_seg.length()=" << new_seg.length()
@@ -1353,9 +1353,8 @@ CtPtr ProtocolV2::handle_read_frame_epilogue_main(rx_buffer_t &&buffer, int r)
     {
       epilogue_bl.push_back(std::move(buffer));
       try {
-        epilogue_bl =
-            session_stream_handlers.rx->authenticated_decrypt_update_final(
-               std::move(epilogue_bl), segment_t::DEFAULT_ALIGNMENT);
+        session_stream_handlers.rx->authenticated_decrypt_update_final(
+            epilogue_bl);
       } catch (ceph::crypto::onwire::MsgAuthError &e) {
         ldout(cct, 5) << __func__ << " message authentication failed: "
                       << e.what() << dendl;

diff --git a/src/msg/async/crypto_onwire.cc b/src/msg/async/crypto_onwire.cc
index 93f17ceb0d554d6c3dbebebdf32cb53be7c85ab2..af9d323089bdb5766f9e0cff71afc4b0021ae70f 100644 (file)
--- a/src/msg/async/crypto_onwire.cc
+++ b/src/msg/async/crypto_onwire.cc
@@ -188,12 +188,8 @@ public:
     return AESGCM_TAG_LEN;
   }
   void reset_rx_handler() override;
-  ceph::bufferlist authenticated_decrypt_update(
-    ceph::bufferlist&& ciphertext,
-    std::uint32_t alignment) override;
-  ceph::bufferlist authenticated_decrypt_update_final(
-    ceph::bufferlist&& ciphertext,
-    std::uint32_t alignment) override;
+  void authenticated_decrypt_update(ceph::bufferlist& bl) override;
+  void authenticated_decrypt_update_final(ceph::bufferlist& bl) override;
 };
 
 void AES128GCM_OnWireRxHandler::reset_rx_handler()
@@ -205,65 +201,36 @@ void AES128GCM_OnWireRxHandler::reset_rx_handler()
   nonce.random_seq = nonce.random_seq + 1;
 }
 
-ceph::bufferlist AES128GCM_OnWireRxHandler::authenticated_decrypt_update(
-  ceph::bufferlist&& ciphertext,
-  std::uint32_t alignment)
+void AES128GCM_OnWireRxHandler::authenticated_decrypt_update(
+  ceph::bufferlist& bl)
 {
-  ceph_assert(ciphertext.length() > 0);
-  //ceph_assert(ciphertext.length() % AESGCM_BLOCK_LEN == 0);
-
-  // NOTE: we might consider in-place transformations in the future. AFAIK
-  // OpenSSL's might sustain that but lack of clear confirmation postpones.
-  auto plainnode = ceph::buffer::ptr_node::create(buffer::create_aligned(
-    ciphertext.length(), alignment));
-  auto* plainbuf = reinterpret_cast<unsigned char*>(plainnode->c_str());
-
-  for (const auto& cipherbuf : ciphertext.buffers()) {
-    // XXX: Why int?
+  // discard cached crcs as we will be writing through c_str()
+  bl.invalidate_crc();
+  for (auto& buf : bl.buffers()) {
+    auto p = reinterpret_cast<unsigned char*>(const_cast<char*>(buf.c_str()));
     int update_len = 0;
 
-    if (1 != EVP_DecryptUpdate(ectx.get(),
-       plainbuf,
-       &update_len,
-       reinterpret_cast<const unsigned char*>(cipherbuf.c_str()),
-       cipherbuf.length())) {
+    if (1 != EVP_DecryptUpdate(ectx.get(), p, &update_len, p, buf.length())) {
       throw std::runtime_error("EVP_DecryptUpdate failed");
     }
     ceph_assert_always(update_len >= 0);
-    ceph_assert(cipherbuf.length() == static_cast<unsigned>(update_len));
-
-    plainbuf += update_len;
+    ceph_assert(static_cast<unsigned>(update_len) == buf.length());
   }
-
-  ceph::bufferlist outbl;
-  outbl.push_back(std::move(plainnode));
-  return outbl;
 }
 
-
-ceph::bufferlist AES128GCM_OnWireRxHandler::authenticated_decrypt_update_final(
-  ceph::bufferlist&& ciphertext_and_tag,
-  std::uint32_t alignment)
+void AES128GCM_OnWireRxHandler::authenticated_decrypt_update_final(
+  ceph::bufferlist& bl)
 {
-  const auto cnt_len = ciphertext_and_tag.length();
-  ceph_assert(cnt_len >= AESGCM_TAG_LEN);
+  unsigned orig_len = bl.length();
+  ceph_assert(orig_len >= AESGCM_TAG_LEN);
 
   // decrypt optional data. Caller is obliged to provide only signature but it
   // may supply ciphertext as well. Combining the update + final is reflected
   // combined together.
-  ceph::bufferlist plainbl;
   ceph::bufferlist auth_tag;
-  {
-    const auto tag_off = cnt_len - AESGCM_TAG_LEN;
-    ceph::bufferlist ciphertext;
-    ciphertext_and_tag.splice(0, tag_off, &ciphertext);
-
-    // the rest is the signature (a.k.a auth tag)
-    auth_tag = std::move(ciphertext_and_tag);
-
-    if (ciphertext.length()) {
-      plainbl = authenticated_decrypt_update(std::move(ciphertext), alignment);
-    }
+  bl.splice(orig_len - AESGCM_TAG_LEN, AESGCM_TAG_LEN, &auth_tag);
+  if (bl.length() > 0) {
+    authenticated_decrypt_update(bl);
   }
 
   // we need to ensure the tag is stored in continuous memory.
@@ -277,18 +244,11 @@ ceph::bufferlist AES128GCM_OnWireRxHandler::authenticated_decrypt_update_final(
   {
     int final_len = 0;
     if (0 >= EVP_DecryptFinal_ex(ectx.get(), nullptr, &final_len)) {
-      ldout(cct, 15) << __func__
-                    << " plainbl.length()=" << plainbl.length()
-                    << " final_len=" << final_len
-                    << dendl;
       throw MsgAuthError();
-    } else {
-      ceph_assert_always(final_len == 0);
-      ceph_assert_always(plainbl.length() + final_len + AESGCM_TAG_LEN == cnt_len);
     }
+    ceph_assert_always(final_len == 0);
+    ceph_assert(bl.length() + AESGCM_TAG_LEN == orig_len);
   }
-
-  return plainbl;
 }
 
 ceph::crypto::onwire::rxtx_t ceph::crypto::onwire::rxtx_t::create_handler_pair(

diff --git a/src/msg/async/crypto_onwire.h b/src/msg/async/crypto_onwire.h
index 0bc58dea0e54f831cfc04ec4cba298d4dab0a71e..9844508963982938da5077c3aa5924550ed2f9c2 100644 (file)
--- a/src/msg/async/crypto_onwire.h
+++ b/src/msg/async/crypto_onwire.h
@@ -103,17 +103,12 @@ public:
   virtual void reset_rx_handler() = 0;
 
   // Perform decryption ciphertext must be ALWAYS aligned to 16 bytes.
-  // TODO: switch to always_aligned_t
-  virtual ceph::bufferlist authenticated_decrypt_update(
-    ceph::bufferlist&& ciphertext,
-    std::uint32_t alignment) = 0;
+  virtual void authenticated_decrypt_update(ceph::bufferlist& bl) = 0;
 
   // Perform decryption of last cipertext's portion and verify signature
   // for overall decryption sequence.
   // Throws on integrity/authenticity checks
-  virtual ceph::bufferlist authenticated_decrypt_update_final(
-    ceph::bufferlist&& ciphertext,
-    std::uint32_t alignment) = 0;
+  virtual void authenticated_decrypt_update_final(ceph::bufferlist& bl) = 0;
 };
 
 struct rxtx_t {