Discard TLS handshake state after handshake has completed

Various bits of data relating to the handshake remained attached to
the Channel even after the handshake completed, but in practice we
only need a few pieces of information. Collect those into a new class,
Active_Connection_State_{12,13}, and discard the rest.

Author: randombit

src/lib/tls/tls12/info.txt

@@ -14,6 +14,7 @@ tls_extensions_12.h
 
 <header:internal>
 tls_channel_impl_12.h
+tls_connection_state_12.h
 tls_client_impl_12.h
 tls_record.h
 tls_server_impl_12.h

src/lib/tls/tls12/tls_channel_impl_12.cpp

@@ -96,28 +96,29 @@ std::shared_ptr<Connection_Cipher_State> Channel_Impl_12::write_cipher_state_epo
 }
 
 std::vector<X509_Certificate> Channel_Impl_12::peer_cert_chain() const {
-   if(const auto* active = active_state()) {
-      return get_peer_cert_chain(*active);
+   if(m_active_state.has_value()) {
+      return m_active_state->peer_certs();
    }
    return std::vector<X509_Certificate>();
 }
 
 std::optional<std::string> Channel_Impl_12::external_psk_identity() const {
-   const auto* state = (active_state() != nullptr) ? active_state() : pending_state();
-   if(state != nullptr) {
+   if(m_active_state.has_value()) {
+      return m_active_state->psk_identity();
+   }
+   if(const auto* state = pending_state()) {
       return state->psk_identity();
-   } else {
-      return std::nullopt;
    }
+   return std::nullopt;
 }
 
 Handshake_State& Channel_Impl_12::create_handshake_state(Protocol_Version version) {
    if(pending_state() != nullptr) {
       throw Internal_Error("create_handshake_state called during handshake");
    }
 
-   if(const auto* active = active_state()) {
-      const Protocol_Version active_version = active->version();
+   if(m_active_state.has_value()) {
+      const Protocol_Version active_version = m_active_state->version();
 
       if(active_version.is_datagram_protocol() != version.is_datagram_protocol()) {
          throw TLS_Exception(Alert::ProtocolVersion,
@@ -156,8 +157,8 @@ Handshake_State& Channel_Impl_12::create_handshake_state(Protocol_Version versio
 
    m_pending_state = new_handshake_state(std::move(io));
 
-   if(const auto* active = active_state()) {
-      m_pending_state->set_version(active->version());
+   if(m_active_state.has_value()) {
+      m_pending_state->set_version(m_active_state->version());
    }
 
    return *m_pending_state;
@@ -177,12 +178,12 @@ void Channel_Impl_12::renegotiate(bool force_full_renegotiation) {
       return;
    }
 
-   if(const auto* active = active_state()) {
+   if(m_active_state.has_value()) {
       if(!force_full_renegotiation) {
          force_full_renegotiation = !policy().allow_resumption_for_renegotiation();
       }
 
-      initiate_handshake(create_handshake_state(active->version()), force_full_renegotiation);
+      initiate_handshake(create_handshake_state(m_active_state->version()), force_full_renegotiation);
    } else {
       throw Invalid_State("Cannot renegotiate on inactive connection");
    }
@@ -245,7 +246,7 @@ void Channel_Impl_12::change_cipher_spec_writer(Connection_Side side) {
 }
 
 bool Channel_Impl_12::is_handshake_complete() const {
-   return (active_state() != nullptr);
+   return m_active_state.has_value();
 }
 
 bool Channel_Impl_12::is_active() const {
@@ -257,10 +258,11 @@ bool Channel_Impl_12::is_closed() const {
 }
 
 void Channel_Impl_12::activate_session() {
-   std::swap(m_active_state, m_pending_state);
-   m_pending_state.reset();
+   BOTAN_ASSERT_NONNULL(m_pending_state);
 
-   if(!m_active_state->version().is_datagram_protocol()) {
+   const auto& state = *m_pending_state;
+
+   if(!state.version().is_datagram_protocol()) {
       // TLS is easy just remove all but the current state
       const uint16_t current_epoch = sequence_numbers().current_write_epoch();
 
@@ -270,6 +272,29 @@ void Channel_Impl_12::activate_session() {
       map_remove_if(not_current_epoch, m_read_cipher_states);
    }
 
+   m_active_state = Active_Connection_State_12(state.version(),
+                                               state.server_hello()->ciphersuite(),
+                                               state.client_hello()->random(),
+                                               application_protocol(),
+                                               get_peer_cert_chain(state),
+                                               state.psk_identity(),
+                                               state.server_hello()->random(),
+                                               Session_ID(state.server_hello()->session_id()),
+                                               state.session_keys().master_secret(),
+                                               state.ciphersuite().prf_algo(),
+                                               state.client_hello()->secure_renegotiation(),
+                                               state.server_hello()->secure_renegotiation(),
+                                               state.client_finished()->verify_data(),
+                                               state.server_finished()->verify_data(),
+                                               state.server_hello()->supports_extended_master_secret());
+
+   // For DTLS, keep the handshake IO for last-flight retransmission.
+   if(m_is_datagram) {
+      m_active_state->set_dtls_handshake_io(m_pending_state->take_handshake_io());
+   }
+
+   m_pending_state.reset();
+
    callbacks().tls_session_activated();
 }
 
@@ -319,10 +344,10 @@ size_t Channel_Impl_12::from_peer(std::span<const uint8_t> data) {
             throw TLS_Exception(Alert::RecordOverflow, "TLS plaintext record is larger than allowed maximum");
          }
 
-         const bool epoch0_restart = m_is_datagram && record.epoch() == 0 && active_state() != nullptr;
+         const bool epoch0_restart = m_is_datagram && record.epoch() == 0 && m_active_state.has_value();
          BOTAN_ASSERT_IMPLICATION(epoch0_restart, allow_epoch0_restart, "Allowed state");
 
-         const bool initial_record = epoch0_restart || (pending_state() == nullptr && active_state() == nullptr);
+         const bool initial_record = epoch0_restart || (pending_state() == nullptr && !m_active_state.has_value());
          bool initial_handshake_message = false;
          if(record.type() == Record_Type::Handshake && !m_record_buf.empty()) {
             const Handshake_Type type = static_cast<Handshake_Type>(m_record_buf[0]);
@@ -340,8 +365,8 @@ size_t Channel_Impl_12::from_peer(std::span<const uint8_t> data) {
                   record.version() != pending->version()) {
                   throw TLS_Exception(Alert::ProtocolVersion, "Received unexpected record version");
                }
-            } else if(const auto* active = active_state()) {
-               if(record.version() != active->version() && !initial_handshake_message) {
+            } else if(m_active_state.has_value()) {
+               if(record.version() != m_active_state->version() && !initial_handshake_message) {
                   throw TLS_Exception(Alert::ProtocolVersion, "Received unexpected record version");
                }
             }
@@ -404,8 +429,10 @@ void Channel_Impl_12::process_handshake_ccs(const secure_vector<uint8_t>& record
             if(epoch == sequence_numbers().current_read_epoch()) {
                create_handshake_state(record_version);
             } else if(epoch == sequence_numbers().current_read_epoch() - 1) {
-               BOTAN_ASSERT(m_active_state, "Have active state here");
-               m_active_state->handshake_io().add_record(record.data(), record.size(), record_type, record_sequence);
+               BOTAN_ASSERT(m_active_state.has_value() && m_active_state->dtls_handshake_io(),
+                            "Have DTLS handshake IO for retransmission");
+               m_active_state->dtls_handshake_io()->add_record(
+                  record.data(), record.size(), record_type, record_sequence);
             }
          } else {
             create_handshake_state(record_version);
@@ -426,7 +453,7 @@ void Channel_Impl_12::process_handshake_ccs(const secure_vector<uint8_t>& record
             break;
          }
 
-         process_handshake_msg(active_state(), *pending, msg.first, msg.second, epoch0_restart);
+         process_handshake_msg(*pending, msg.first, msg.second, epoch0_restart);
 
          if(!m_pending_state) {
             break;
@@ -436,7 +463,7 @@ void Channel_Impl_12::process_handshake_ccs(const secure_vector<uint8_t>& record
 }
 
 void Channel_Impl_12::process_application_data(uint64_t seq_no, const secure_vector<uint8_t>& record) {
-   if(active_state() == nullptr) {
+   if(!m_active_state.has_value()) {
       throw Unexpected_Message("Application data before handshake done");
    }
 
@@ -453,11 +480,10 @@ void Channel_Impl_12::process_alert(const secure_vector<uint8_t>& record) {
    callbacks().tls_alert(alert_msg);
 
    if(alert_msg.is_fatal()) {
-      if(const auto* active = active_state()) {
-         BOTAN_ASSERT_NONNULL(active->server_hello());
-         const auto& session_id = active->server_hello()->session_id();
-         if(!session_id.empty()) {
-            session_manager().remove(Session_Handle(session_id));
+      if(m_active_state.has_value()) {
+         const auto& sid = m_active_state->session_id();
+         if(!sid.empty()) {
+            session_manager().remove(Session_Handle(sid));
          }
       }
    }
@@ -479,10 +505,9 @@ void Channel_Impl_12::write_record(Connection_Cipher_State* cipher_state,
                                    Record_Type record_type,
                                    const uint8_t input[],
                                    size_t length) {
-   BOTAN_ASSERT(m_pending_state || m_active_state, "Some connection state exists");
+   BOTAN_ASSERT(m_pending_state || m_active_state.has_value(), "Some connection state exists");
 
-   const Protocol_Version record_version =
-      (m_pending_state) ? (m_pending_state->version()) : (m_active_state->version());
+   const Protocol_Version record_version = (m_pending_state) ? (m_pending_state->version()) : m_active_state->version();
 
    const uint64_t next_seq = sequence_numbers().next_write_sequence(epoch);
 
@@ -543,11 +568,10 @@ void Channel_Impl_12::send_alert(const Alert& alert) {
    }
 
    if(alert.is_fatal()) {
-      if(const auto* active = active_state()) {
-         BOTAN_ASSERT_NONNULL(active->server_hello());
-         const auto& session_id = active->server_hello()->session_id();
-         if(!session_id.empty()) {
-            session_manager().remove(Session_Handle(Session_ID(session_id)));
+      if(m_active_state.has_value()) {
+         const auto& sid = m_active_state->session_id();
+         if(!sid.empty()) {
+            session_manager().remove(Session_Handle(sid));
          }
       }
       reset_state();
@@ -562,11 +586,8 @@ void Channel_Impl_12::secure_renegotiation_check(const Client_Hello_12* client_h
    BOTAN_ASSERT_NONNULL(client_hello);
    const bool secure_renegotiation = client_hello->secure_renegotiation();
 
-   if(const auto* active = active_state()) {
-      BOTAN_ASSERT_NONNULL(active->client_hello());
-      const bool active_sr = active->client_hello()->secure_renegotiation();
-
-      if(active_sr != secure_renegotiation) {
+   if(m_active_state.has_value()) {
+      if(m_active_state->client_supports_secure_renegotiation() != secure_renegotiation) {
          throw TLS_Exception(Alert::HandshakeFailure, "Client changed its mind about secure renegotiation");
       }
    }
@@ -584,11 +605,8 @@ void Channel_Impl_12::secure_renegotiation_check(const Server_Hello_12* server_h
    BOTAN_ASSERT_NONNULL(server_hello);
    const bool secure_renegotiation = server_hello->secure_renegotiation();
 
-   if(const auto* active = active_state()) {
-      BOTAN_ASSERT_NONNULL(active->server_hello());
-      const bool active_sr = active->server_hello()->secure_renegotiation();
-
-      if(active_sr != secure_renegotiation) {
+   if(m_active_state.has_value()) {
+      if(m_active_state->server_supports_secure_renegotiation() != secure_renegotiation) {
          throw TLS_Exception(Alert::HandshakeFailure, "Server changed its mind about secure renegotiation");
       }
    }
@@ -603,28 +621,25 @@ void Channel_Impl_12::secure_renegotiation_check(const Server_Hello_12* server_h
 }
 
 std::vector<uint8_t> Channel_Impl_12::secure_renegotiation_data_for_client_hello() const {
-   if(const auto* active = active_state()) {
-      BOTAN_ASSERT_NONNULL(active->client_finished());
-      return active->client_finished()->verify_data();
+   if(m_active_state.has_value()) {
+      return m_active_state->client_finished_verify_data();
    }
    return std::vector<uint8_t>();
 }
 
 std::vector<uint8_t> Channel_Impl_12::secure_renegotiation_data_for_server_hello() const {
-   if(const auto* active = active_state()) {
-      BOTAN_ASSERT_NONNULL(active->client_finished());
-      BOTAN_ASSERT_NONNULL(active->server_finished());
-      std::vector<uint8_t> buf = active->client_finished()->verify_data();
-      buf += active->server_finished()->verify_data();
+   if(m_active_state.has_value()) {
+      std::vector<uint8_t> buf = m_active_state->client_finished_verify_data();
+      buf += m_active_state->server_finished_verify_data();
       return buf;
    }
 
    return std::vector<uint8_t>();
 }
 
 bool Channel_Impl_12::secure_renegotiation_supported() const {
-   if(const auto* active = active_state()) {
-      return active->server_hello()->secure_renegotiation();
+   if(m_active_state.has_value()) {
+      return m_active_state->server_supports_secure_renegotiation();
    }
 
    if(const auto* pending = pending_state()) {
@@ -639,35 +654,31 @@ bool Channel_Impl_12::secure_renegotiation_supported() const {
 SymmetricKey Channel_Impl_12::key_material_export(std::string_view label,
                                                   std::string_view context,
                                                   size_t length) const {
-   if(const auto* active = active_state()) {
-      if(pending_state() != nullptr) {
-         throw Invalid_State("Channel_Impl_12::key_material_export cannot export during renegotiation");
-      }
+   if(!m_active_state.has_value()) {
+      throw Invalid_State("Channel_Impl_12::key_material_export connection not active");
+   }
 
-      auto prf = active->protocol_specific_prf();
+   if(pending_state() != nullptr) {
+      throw Invalid_State("Channel_Impl_12::key_material_export cannot export during renegotiation");
+   }
 
-      const secure_vector<uint8_t>& master_secret = active->session_keys().master_secret();
+   auto prf = callbacks().tls12_protocol_specific_kdf(m_active_state->prf_algo());
 
-      BOTAN_ASSERT_NONNULL(active->client_hello());
-      BOTAN_ASSERT_NONNULL(active->server_hello());
-      std::vector<uint8_t> salt;
-      salt += active->client_hello()->random();
-      salt += active->server_hello()->random();
+   std::vector<uint8_t> salt;
+   salt += m_active_state->client_random();
+   salt += m_active_state->server_random();
 
-      if(!context.empty()) {
-         const size_t context_size = context.length();
-         if(context_size > 0xFFFF) {
-            throw Invalid_Argument("key_material_export context is too long");
-         }
-         salt.push_back(get_byte<0>(static_cast<uint16_t>(context_size)));
-         salt.push_back(get_byte<1>(static_cast<uint16_t>(context_size)));
-         salt += as_span_of_bytes(context);
+   if(!context.empty()) {
+      const size_t context_size = context.length();
+      if(context_size > 0xFFFF) {
+         throw Invalid_Argument("key_material_export context is too long");
       }
-
-      return SymmetricKey(prf->derive_key(length, master_secret, salt, as_span_of_bytes(label)));
-   } else {
-      throw Invalid_State("Channel_Impl_12::key_material_export connection not active");
+      salt.push_back(get_byte<0>(static_cast<uint16_t>(context_size)));
+      salt.push_back(get_byte<1>(static_cast<uint16_t>(context_size)));
+      salt += as_span_of_bytes(context);
    }
+
+   return SymmetricKey(prf->derive_key(length, m_active_state->master_secret(), salt, as_span_of_bytes(label)));
 }
 
 }  // namespace Botan::TLS

src/lib/tls/tls12/tls_channel_impl_12.h

@@ -12,6 +12,7 @@
 #include <botan/tls_alert.h>
 #include <botan/tls_session_manager.h>
 #include <botan/internal/tls_channel_impl.h>
+#include <botan/internal/tls_connection_state_12.h>
 #include <map>
 #include <memory>
 #include <string>
@@ -149,8 +150,9 @@ class Channel_Impl_12 : public Channel_Impl {
       bool timeout_check() override;
 
    protected:
-      virtual void process_handshake_msg(const Handshake_State* active_state,
-                                         Handshake_State& pending_state,
+      const std::optional<Active_Connection_State_12>& active_state() const { return m_active_state; }
+
+      virtual void process_handshake_msg(Handshake_State& pending_state,
                                          Handshake_Type type,
                                          const std::vector<uint8_t>& contents,
                                          bool epoch0_restart) = 0;
@@ -206,8 +208,6 @@ class Channel_Impl_12 : public Channel_Impl {
 
       std::shared_ptr<Connection_Cipher_State> write_cipher_state_epoch(uint16_t epoch) const;
 
-      const Handshake_State* active_state() const { return m_active_state.get(); }
-
       const Handshake_State* pending_state() const { return m_pending_state.get(); }
 
       /* methods to handle incoming traffic through Channel_Impl_12::receive_data. */
@@ -235,8 +235,7 @@ class Channel_Impl_12 : public Channel_Impl {
       /* sequence number state */
       std::unique_ptr<Connection_Sequence_Numbers> m_sequence_numbers;
 
-      /* pending and active connection states */
-      std::unique_ptr<Handshake_State> m_active_state;
+      /* pending handshake state (null when no handshake is in progress) */
       std::unique_ptr<Handshake_State> m_pending_state;
 
       /* cipher states for each epoch */
@@ -249,6 +248,8 @@ class Channel_Impl_12 : public Channel_Impl {
       secure_vector<uint8_t> m_record_buf;
 
       bool m_has_been_closed;
+
+      std::optional<Active_Connection_State_12> m_active_state;
 };
 
 }  // namespace TLS