Merge pull request #3762 from Rohde-Schwarz/fix/handshake_complete

Introduce TLS::Channel::is_handshake complete()

src/lib/tls/asio/asio_async_ops.h

@@ -278,7 +278,7 @@ class AsyncHandshakeOperation : public AsyncBase<Handler, typename Stream::execu
  return;
  }
 
- if(!m_stream.native_handle()->is_active() && !ec) {
+ if(!m_stream.native_handle()->is_handshake_complete() && !ec) {
  // Read more encrypted TLS data from the network
  m_stream.next_layer().async_read_some(m_stream.input_buffer(), std::move(*this));
  return;

src/lib/tls/asio/asio_stream.h

@@ -229,7 +229,7 @@ class Stream {
  send_pending_encrypted_data(ec);
  }
 
- while(!native_handle()->is_active() && !ec) {
+ while(!native_handle()->is_handshake_complete() && !ec) {
  boost::asio::const_buffer read_buffer{input_buffer().data(), m_nextLayer.read_some(input_buffer(), ec)};
  if(ec) {
  return;

src/lib/tls/tls12/tls_channel_impl_12.cpp

@@ -237,13 +237,14 @@ void Channel_Impl_12::change_cipher_spec_writer(Connection_Side side) {
  m_write_cipher_states[epoch] = write_state;
 }
 
-bool Channel_Impl_12::is_active() const {
- if(is_closed()) {
- return false;
- }
+bool Channel_Impl_12::is_handshake_complete() const {
  return (active_state() != nullptr);
 }
 
+bool Channel_Impl_12::is_active() const {
+ return !is_closed() && is_handshake_complete();
+}
+
 bool Channel_Impl_12::is_closed() const {
  return m_has_been_closed;
 }

src/lib/tls/tls12/tls_channel_impl_12.h

@@ -83,6 +83,11 @@ class Channel_Impl_12 : public Channel_Impl {
  */
  void send_alert(const Alert& alert) override;
 
+ /**
+ * @return true iff the TLS handshake completed successfully
+ */
+ bool is_handshake_complete() const override;
+
  /**
  * @return true iff the connection is active for sending application data
  */

src/lib/tls/tls13/tls_channel_impl_13.cpp

@@ -107,7 +107,7 @@ size_t Channel_Impl_13::from_peer(std::span<const uint8_t> data) {
  if(record.type == Record_Type::Handshake) {
  m_handshake_layer.copy_data(record.fragment);
 
- if(!handshake_finished()) {
+ if(!is_handshake_complete()) {
  while(auto handshake_msg = m_handshake_layer.next_message(policy(), m_transcript_hash)) {
  // RFC 8446 5.1
  // Handshake messages MUST NOT span key changes. Implementations
@@ -316,7 +316,7 @@ SymmetricKey Channel_Impl_13::key_material_export(std::string_view label,
 
 void Channel_Impl_13::update_traffic_keys(bool request_peer_update) {
  BOTAN_STATE_CHECK(!is_downgrading());
- BOTAN_STATE_CHECK(handshake_finished());
+ BOTAN_STATE_CHECK(is_handshake_complete());
  BOTAN_ASSERT_NONNULL(m_cipher_state);
  send_post_handshake_message(Key_Update(request_peer_update));
  m_cipher_state->update_write_keys();

src/lib/tls/tls13/tls_channel_impl_13.h

@@ -193,7 +193,6 @@ class Channel_Impl_13 : public Channel_Impl {
  virtual void process_handshake_msg(Handshake_Message_13 msg) = 0;
  virtual void process_post_handshake_msg(Post_Handshake_Message_13 msg) = 0;
  virtual void process_dummy_change_cipher_spec() = 0;
- virtual bool handshake_finished() const = 0;
 
  /**
  * @return whether a change cipher spec record should be prepended _now_

src/lib/tls/tls13/tls_client_impl_13.cpp

@@ -91,7 +91,7 @@ void Client_Impl_13::process_handshake_msg(Handshake_Message_13 message) {
 }
 
 void Client_Impl_13::process_post_handshake_msg(Post_Handshake_Message_13 message) {
- BOTAN_STATE_CHECK(handshake_finished());
+ BOTAN_STATE_CHECK(is_handshake_complete());
 
  std::visit([&](auto msg) { handle(msg); }, m_handshake_state.received(std::move(message)));
 }
@@ -114,7 +114,7 @@ void Client_Impl_13::process_dummy_change_cipher_spec() {
  // ... no further processing.
 }
 
-bool Client_Impl_13::handshake_finished() const {
+bool Client_Impl_13::is_handshake_complete() const {
  return m_handshake_state.handshake_finished();
 }
 
@@ -418,7 +418,7 @@ void Client_Impl_13::handle(const Certificate_Request_13& certificate_request_ms
  // RFC 8446 4.3.2
  // [The 'context' field] SHALL be zero length unless used for the
  // post-handshake authentication exchanges described in Section 4.6.2.
- if(!m_handshake_state.handshake_finished() && !certificate_request_msg.context().empty()) {
+ if(!is_handshake_complete() && !certificate_request_msg.context().empty()) {
  throw TLS_Exception(Alert::DecodeError, "Certificate_Request context must be empty in the main handshake");
  }
 
@@ -595,7 +595,7 @@ bool Client_Impl_13::prepend_ccs() {
 }
 
 std::string Client_Impl_13::application_protocol() const {
- if(m_handshake_state.handshake_finished()) {
+ if(is_handshake_complete()) {
  const auto& eee = m_handshake_state.encrypted_extensions().extensions();
  if(eee.has<Application_Layer_Protocol_Notification>()) {
  return eee.get<Application_Layer_Protocol_Notification>()->single_protocol();

src/lib/tls/tls13/tls_client_impl_13.h

@@ -68,12 +68,16 @@ class Client_Impl_13 : public Channel_Impl_13 {
  */
  std::optional<std::string> external_psk_identity() const override;
 
+ /**
+ * @return true if the TLS handshake finished successfully
+ */
+ bool is_handshake_complete() const override;
+
  private:
  void process_handshake_msg(Handshake_Message_13 msg) override;
  void process_post_handshake_msg(Post_Handshake_Message_13 msg) override;
  void process_dummy_change_cipher_spec() override;
 
- bool handshake_finished() const override;
  bool prepend_ccs() override;
 
  using Channel_Impl_13::handle;

src/lib/tls/tls13/tls_server_impl_13.cpp

@@ -32,7 +32,7 @@ Server_Impl_13::Server_Impl_13(const std::shared_ptr<Callbacks>& callbacks,
 }
 
 std::string Server_Impl_13::application_protocol() const {
- if(m_handshake_state.handshake_finished()) {
+ if(is_handshake_complete()) {
  const auto& eee = m_handshake_state.encrypted_extensions().extensions();
  if(const auto alpn = eee.get<Application_Layer_Protocol_Notification>()) {
  return alpn->single_protocol();
@@ -67,14 +67,13 @@ bool Server_Impl_13::new_session_ticket_supported() const {
  // regardless of this method indicating no support for tickets.
  //
  // TODO: Implement other PSK KE modes than PSK_DHE_KE
- return m_handshake_state.handshake_finished() &&
- m_handshake_state.client_hello().extensions().has<PSK_Key_Exchange_Modes>() &&
+ return is_handshake_complete() && m_handshake_state.client_hello().extensions().has<PSK_Key_Exchange_Modes>() &&
  value_exists(m_handshake_state.client_hello().extensions().get<PSK_Key_Exchange_Modes>()->modes(),
  PSK_Key_Exchange_Mode::PSK_DHE_KE);
 }
 
 size_t Server_Impl_13::send_new_session_tickets(const size_t tickets) {
- BOTAN_STATE_CHECK(handshake_finished());
+ BOTAN_STATE_CHECK(is_handshake_complete());
 
  if(tickets == 0) {
  return 0;
@@ -125,7 +124,7 @@ void Server_Impl_13::process_handshake_msg(Handshake_Message_13 message) {
 }
 
 void Server_Impl_13::process_post_handshake_msg(Post_Handshake_Message_13 message) {
- BOTAN_STATE_CHECK(handshake_finished());
+ BOTAN_STATE_CHECK(is_handshake_complete());
 
  std::visit([&](auto msg) { handle(msg); }, m_handshake_state.received(std::move(message)));
 }
@@ -148,7 +147,7 @@ void Server_Impl_13::process_dummy_change_cipher_spec() {
  // ... no further processing.
 }
 
-bool Server_Impl_13::handshake_finished() const {
+bool Server_Impl_13::is_handshake_complete() const {
  return m_handshake_state.handshake_finished();
 }
 
@@ -440,7 +439,7 @@ void Server_Impl_13::handle(const Certificate_13& certificate_msg) {
  // RFC 8446 4.3.2
  // certificate_request_context: [...] This field SHALL be zero length
  // unless used for the post-handshake authentication exchanges [...].
- if(!handshake_finished() && !certificate_msg.request_context().empty()) {
+ if(!is_handshake_complete() && !certificate_msg.request_context().empty()) {
  throw TLS_Exception(Alert::DecodeError, "Received a client certificate message with non-empty request context");
  }
 

src/lib/tls/tls13/tls_server_impl_13.h

@@ -33,6 +33,8 @@ class Server_Impl_13 : public Channel_Impl_13 {
  bool new_session_ticket_supported() const override;
  size_t send_new_session_tickets(size_t tickets) override;
 
+ bool is_handshake_complete() const override;
+
  private:
  void process_handshake_msg(Handshake_Message_13 msg) override;
  void process_post_handshake_msg(Post_Handshake_Message_13 msg) override;
@@ -50,8 +52,6 @@ class Server_Impl_13 : public Channel_Impl_13 {
 
  void maybe_handle_compatibility_mode();
 
- bool handshake_finished() const override;
-
  void downgrade();
 
  private:

src/lib/tls/tls_channel.h

@@ -89,6 +89,22 @@ class BOTAN_PUBLIC_API(2, 0) Channel {
  virtual void close() = 0;
 
  /**
+ * Becomes true as soon as the TLS handshake is fully complete and all
+ * security assurances TLS provides can be guaranteed.
+ *
+ * @returns true once the TLS handshake has finished successfully
+ */
+ virtual bool is_handshake_complete() const = 0;
+
+ /**
+ * Check whether the connection is ready to send application data. Note
+ * that a TLS 1.3 server MAY send data _before_ receiving the client's
+ * Finished message. Only _after_ receiving the client's Finished, can the
+ * server be sure about the client's liveness and (optional) identity.
+ *
+ * Consider using is_handshake_complete() if you need to wait until the
+ * handshake if fully complete.
+ *
  * @return true iff the connection is active for sending application data
  */
  virtual bool is_active() const = 0;

src/lib/tls/tls_channel_impl.h

@@ -79,6 +79,11 @@ class Channel_Impl {
  */
  void close() { send_warning_alert(Alert::CloseNotify); }
 
+ /**
+ * @return true iff the TLS handshake has finished successfully
+ */
+ virtual bool is_handshake_complete() const = 0;
+
  /**
  * @return true iff the connection is active for sending application data
  */

src/lib/tls/tls_client.cpp

@@ -97,6 +97,10 @@ size_t Client::from_peer(std::span<const uint8_t> data) {
  return read;
 }
 
+bool Client::is_handshake_complete() const {
+ return m_impl->is_handshake_complete();
+}
+
 bool Client::is_active() const {
  return m_impl->is_active();
 }

src/lib/tls/tls_client.h

@@ -70,6 +70,9 @@ class BOTAN_PUBLIC_API(2, 0) Client final : public Channel {
  std::string application_protocol() const override;
 
  size_t from_peer(std::span<const uint8_t> data) override;
+
+ bool is_handshake_complete() const override;
+
  bool is_active() const override;
 
  bool is_closed() const override;

src/lib/tls/tls_server.cpp

@@ -65,6 +65,10 @@ size_t Server::from_peer(std::span<const uint8_t> data) {
  return read;
 }
 
+bool Server::is_handshake_complete() const {
+ return m_impl->is_handshake_complete();
+}
+
 bool Server::is_active() const {
  return m_impl->is_active();
 }

src/lib/tls/tls_server.h

@@ -66,6 +66,8 @@ class BOTAN_PUBLIC_API(2, 0) Server final : public Channel {
 
  size_t from_peer(std::span<const uint8_t> data) override;
 
+ bool is_handshake_complete() const override;
+
  bool is_active() const override;
 
  bool is_closed() const override;