From f1ad217525ea5f69d047422c88aa70d698c02efc Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Thu, 23 Apr 2020 19:51:55 +0200 Subject: [PATCH] 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 (cherry picked from commit fe97a000990632dbe9734e0d3a96df627f104f7a) --- src/msg/async/ProtocolV2.cc | 17 ++++---- src/msg/async/crypto_onwire.cc | 78 +++++++++------------------------- src/msg/async/crypto_onwire.h | 9 +--- 3 files changed, 29 insertions(+), 75 deletions(-) diff --git a/src/msg/async/ProtocolV2.cc b/src/msg/async/ProtocolV2.cc index 10ea6d2eccfb5..a1ebe6baffbec 100644 --- 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 93f17ceb0d554..af9d323089bdb 100644 --- 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(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(const_cast(buf.c_str())); int update_len = 0; - if (1 != EVP_DecryptUpdate(ectx.get(), - plainbuf, - &update_len, - reinterpret_cast(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(update_len)); - - plainbuf += update_len; + ceph_assert(static_cast(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 0bc58dea0e54f..9844508963982 100644 --- 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 { -- 2.39.5