From 7e4388f16eb19ed29df53b335ce2abfbf095e7fc Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Mon, 27 Apr 2020 16:07:46 +0200 Subject: [PATCH] msg/async/crypto_onwire: implement msgr2.1 nonce format Move to a 64-bit counter to avoid wrapping and having to reset the session before the counter repeats. This is in line with NIST Recommendation for GCM [1]: "... this Recommendation suggests, but does not require, that the leading (i.e., leftmost) 32 bits of the IV hold the fixed field; and that the trailing (i.e., rightmost) 64 bits hold the invocation field." See commit bb61e6a5adc3 ("msg/async/ProtocolV2: avoid AES-GCM nonce reuse vulnerabilities"). [1] https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf Signed-off-by: Ilya Dryomov --- src/msg/async/ProtocolV2.cc | 8 +++---- src/msg/async/crypto_onwire.cc | 40 ++++++++++++++++++++++++---------- src/msg/async/crypto_onwire.h | 1 + 3 files changed, 34 insertions(+), 15 deletions(-) diff --git a/src/msg/async/ProtocolV2.cc b/src/msg/async/ProtocolV2.cc index c4e66d7bcd0..5134e13e15e 100644 --- a/src/msg/async/ProtocolV2.cc +++ b/src/msg/async/ProtocolV2.cc @@ -1797,8 +1797,8 @@ CtPtr ProtocolV2::handle_auth_done(ceph::bufferlist &payload) return _fault(); } auth_meta->con_mode = auth_done.con_mode(); - session_stream_handlers = \ - ceph::crypto::onwire::rxtx_t::create_handler_pair(cct, *auth_meta, false); + session_stream_handlers = ceph::crypto::onwire::rxtx_t::create_handler_pair( + cct, *auth_meta, /*new_nonce_format=*/false, /*crossed=*/false); state = AUTH_CONNECTING_SIGN; @@ -2177,8 +2177,8 @@ CtPtr ProtocolV2::finish_auth() ceph_assert(auth_meta); // TODO: having a possibility to check whether we're server or client could // allow reusing finish_auth(). - session_stream_handlers = \ - ceph::crypto::onwire::rxtx_t::create_handler_pair(cct, *auth_meta, true); + session_stream_handlers = ceph::crypto::onwire::rxtx_t::create_handler_pair( + cct, *auth_meta, /*new_nonce_format=*/false, /*crossed=*/true); const auto sig = auth_meta->session_key.empty() ? sha256_digest_t() : auth_meta->session_key.hmac_sha256(cct, pre_auth.rxbuf); diff --git a/src/msg/async/crypto_onwire.cc b/src/msg/async/crypto_onwire.cc index af9d323089b..4e42340603e 100644 --- a/src/msg/async/crypto_onwire.cc +++ b/src/msg/async/crypto_onwire.cc @@ -20,8 +20,8 @@ static constexpr const std::size_t AESGCM_TAG_LEN{16}; static constexpr const std::size_t AESGCM_BLOCK_LEN{16}; struct nonce_t { - ceph_le32 random_seq; - ceph_le64 random_rest; + ceph_le32 fixed; + ceph_le64 counter; bool operator==(const nonce_t& rhs) const { return !memcmp(this, &rhs, sizeof(*this)); @@ -41,15 +41,18 @@ class AES128GCM_OnWireTxHandler : public ceph::crypto::onwire::TxHandler { ceph::bufferlist buffer; nonce_t nonce, initial_nonce; bool used_initial_nonce; + bool new_nonce_format; // 64-bit counter? static_assert(sizeof(nonce) == AESGCM_IV_LEN); public: AES128GCM_OnWireTxHandler(CephContext* const cct, const key_t& key, - const nonce_t& nonce) + const nonce_t& nonce, + bool new_nonce_format) : cct(cct), ectx(EVP_CIPHER_CTX_new(), EVP_CIPHER_CTX_free), - nonce(nonce), initial_nonce(nonce), used_initial_nonce(false) { + nonce(nonce), initial_nonce(nonce), used_initial_nonce(false), + new_nonce_format(new_nonce_format) { ceph_assert_always(ectx); ceph_assert_always(key.size() * CHAR_BIT == 128); @@ -93,7 +96,13 @@ void AES128GCM_OnWireTxHandler::reset_tx_handler(const uint32_t* first, ceph_assert(buffer.get_append_buffer_unused_tail_length() == 0); buffer.reserve(std::accumulate(first, last, AESGCM_TAG_LEN)); - nonce.random_seq = nonce.random_seq + 1; + if (!new_nonce_format) { + // msgr2.0: 32-bit counter followed by 64-bit fixed field, + // susceptible to overflow! + nonce.fixed = nonce.fixed + 1; + } else { + nonce.counter = nonce.counter + 1; + } } void AES128GCM_OnWireTxHandler::authenticated_encrypt_update( @@ -156,16 +165,17 @@ class AES128GCM_OnWireRxHandler : public ceph::crypto::onwire::RxHandler { CephContext* const cct; std::unique_ptr ectx; nonce_t nonce; + bool new_nonce_format; // 64-bit counter? static_assert(sizeof(nonce) == AESGCM_IV_LEN); public: AES128GCM_OnWireRxHandler(CephContext* const cct, const key_t& key, - const nonce_t& nonce) + const nonce_t& nonce, + bool new_nonce_format) : cct(cct), ectx(EVP_CIPHER_CTX_new(), EVP_CIPHER_CTX_free), - nonce(nonce) - { + nonce(nonce), new_nonce_format(new_nonce_format) { ceph_assert_always(ectx); ceph_assert_always(key.size() * CHAR_BIT == 128); @@ -198,7 +208,14 @@ void AES128GCM_OnWireRxHandler::reset_rx_handler() reinterpret_cast(&nonce))) { throw std::runtime_error("EVP_DecryptInit_ex failed"); } - nonce.random_seq = nonce.random_seq + 1; + + if (!new_nonce_format) { + // msgr2.0: 32-bit counter followed by 64-bit fixed field, + // susceptible to overflow! + nonce.fixed = nonce.fixed + 1; + } else { + nonce.counter = nonce.counter + 1; + } } void AES128GCM_OnWireRxHandler::authenticated_decrypt_update( @@ -254,6 +271,7 @@ void AES128GCM_OnWireRxHandler::authenticated_decrypt_update_final( ceph::crypto::onwire::rxtx_t ceph::crypto::onwire::rxtx_t::create_handler_pair( CephContext* cct, const AuthConnectionMeta& auth_meta, + bool new_nonce_format, bool crossed) { if (auth_meta.is_mode_secure()) { @@ -281,9 +299,9 @@ ceph::crypto::onwire::rxtx_t ceph::crypto::onwire::rxtx_t::create_handler_pair( return { std::make_unique( - cct, key, crossed ? tx_nonce : rx_nonce), + cct, key, crossed ? tx_nonce : rx_nonce, new_nonce_format), std::make_unique( - cct, key, crossed ? rx_nonce : tx_nonce) + cct, key, crossed ? rx_nonce : tx_nonce, new_nonce_format) }; } else { return { nullptr, nullptr }; diff --git a/src/msg/async/crypto_onwire.h b/src/msg/async/crypto_onwire.h index 98445089639..55f7550868f 100644 --- a/src/msg/async/crypto_onwire.h +++ b/src/msg/async/crypto_onwire.h @@ -121,6 +121,7 @@ struct rxtx_t { static rxtx_t create_handler_pair( CephContext* ctx, const class AuthConnectionMeta& auth_meta, + bool new_nonce_format, bool crossed); }; -- 2.39.5