From a45174f3b1fe547a55c74c5f1200af6c36c6a475 Mon Sep 17 00:00:00 2001 From: Thiesius Date: Tue, 28 Jul 2026 20:55:01 +0200 Subject: [PATCH 1/4] Fix DTLS handshake flight retransmission Handle lost, duplicated, reordered, and stale DTLS handshake flights without creating spurious pending handshakes. Preserve the consumed flight needed to retransmit final responses, including the epoch transition, and add regression coverage. --- doc/api_ref/tls.rst | 15 +- news.rst | 5 + src/bogo_shim/bogo_shim.cpp | 1 + src/bogo_shim/config.json | 1 - src/lib/tls/tls12/tls_channel_impl_12.cpp | 84 +- src/lib/tls/tls12/tls_channel_impl_12.h | 9 +- src/lib/tls/tls12/tls_connection_state_12.h | 2 + src/lib/tls/tls12/tls_handshake_io.cpp | 176 ++- src/lib/tls/tls12/tls_handshake_io.h | 41 + src/lib/tls/tls_channel.h | 12 +- src/lib/tls/tls_channel_impl.h | 14 +- src/lib/tls/tls_client.cpp | 4 + src/lib/tls/tls_client.h | 2 + src/lib/tls/tls_server.cpp | 4 + src/lib/tls/tls_server.h | 2 + src/tests/unit_tls.cpp | 1168 +++++++++++++++++++ 16 files changed, 1511 insertions(+), 29 deletions(-) diff --git a/doc/api_ref/tls.rst b/doc/api_ref/tls.rst index 90c948ea5fa..05c0aebb3bf 100644 --- a/doc/api_ref/tls.rst +++ b/doc/api_ref/tls.rst @@ -257,6 +257,13 @@ available: this connection and the connection has not been subsequently closed. + .. cpp:function:: bool requires_timeout_check() + + Returns true if ``timeout_check`` should be polled to drive DTLS + handshake retransmissions. This may remain true briefly after + ``is_active`` becomes true, because a DTLS peer may not have + received the final handshake flight yet. + .. cpp:function:: bool is_closed() Returns true if and only if either a close notification or a @@ -277,10 +284,10 @@ available: .. cpp:function:: bool timeout_check() This function does nothing unless the channel represents a DTLS - connection and a handshake is actively in progress. In this case - it will check the current timeout state and potentially initiate - retransmission of handshake packets. Returns true if a timeout - condition occurred. + connection and a handshake still needs retransmission handling. This + includes handshakes in progress and locally active sessions whose final + handshake flight may not have reached the peer yet. Returns true if a + timeout condition occurred and handshake packets were retransmitted. .. cpp:function:: void renegotiate(bool force_full_renegotiation = false) diff --git a/news.rst b/news.rst index 0c60348245d..93046122963 100644 --- a/news.rst +++ b/news.rst @@ -102,6 +102,11 @@ Version 3.13.0, Not Yet Released * Upgrade to TLS-Anvil 1.5 (GH #5630) +* Fix DTLS 1.2 handshake retransmission edge cases, including pacing of + repeated timeout checks, replay of final flights after local activation, + partial server-flight delivery, and delayed server-side flight handling. + (GH #2310 #2498 #4022 #4036) + Version 3.12.0, 2026-05-06 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/bogo_shim/bogo_shim.cpp b/src/bogo_shim/bogo_shim.cpp index 7f88d2bc74d..5eefc18f70d 100644 --- a/src/bogo_shim/bogo_shim.cpp +++ b/src/bogo_shim/bogo_shim.cpp @@ -130,6 +130,7 @@ std::string map_to_bogo_error(const std::string& e) noexcept { {"Certificate usage constraints do not allow signing", ":KEY_USAGE_BIT_INCORRECT:"}, {"Can't agree on a ciphersuite with client", ":NO_SHARED_CIPHER:"}, {"Can't interleave application and handshake data", ":UNEXPECTED_RECORD:"}, + {"Unexpected new DTLS handshake message", ":UNEXPECTED_RECORD:"}, {"Certificate chain exceeds policy specified maximum size", ":EXCESSIVE_MESSAGE_SIZE:"}, {"Certificate key type did not match ciphersuite", ":WRONG_CERTIFICATE_TYPE:"}, {"Certificate usage constraints do not allow this ciphersuite", ":KEY_USAGE_BIT_INCORRECT:"}, diff --git a/src/bogo_shim/config.json b/src/bogo_shim/config.json index ff48c9026c9..7d5cd6b31db 100644 --- a/src/bogo_shim/config.json +++ b/src/bogo_shim/config.json @@ -265,7 +265,6 @@ "KeyChangeWithBufferedMessages-DTLS": "No DTLS 1.3", "Renegotiate-DTLS-Server-Forbidden": "Botan tolerates DTLS renegotiation", "Renegotiate-DTLS-Client-Forbidden": "Botan tolerates DTLS renegotiation", - "DTLS12-SendExtraFinished-*": "Botan tolerates extra Finished messages in DTLS", "MixCompleteMessageWithFragments-DTLS-TLS12": "DTLS fragment handling difference", "RejectPSSKeyType-*": "Botan does not reject RSA-PSS key type", "CertificateCipherMismatch-PSS": "Botan does not reject PSS cipher mismatch", diff --git a/src/lib/tls/tls12/tls_channel_impl_12.cpp b/src/lib/tls/tls12/tls_channel_impl_12.cpp index 69808d64188..6ec10537586 100644 --- a/src/lib/tls/tls12/tls_channel_impl_12.cpp +++ b/src/lib/tls/tls12/tls_channel_impl_12.cpp @@ -24,6 +24,21 @@ namespace Botan::TLS { +namespace { + +bool is_new_dtls_association_client_hello(std::span record, Record_Type record_type) { + constexpr size_t DTLS_HANDSHAKE_HEADER_SIZE = 12; + + // Every new DTLS handshake starts at message_seq 0. A cookie-bearing + // ClientHello uses message_seq 1, so one arriving late belongs to the + // previous handshake and must not replace the active association's state. + return record_type == Record_Type::Handshake && record.size() >= DTLS_HANDSHAKE_HEADER_SIZE && + static_cast(record[0]) == Handshake_Type::ClientHello && + load_be(&record[4], 0) == 0; +} + +} // namespace + Channel_Impl_12::Channel_Impl_12(const std::shared_ptr& callbacks, const std::shared_ptr& session_manager, const std::shared_ptr& rng, @@ -54,6 +69,7 @@ Channel_Impl_12::Channel_Impl_12(const std::shared_ptr& callbacks, void Channel_Impl_12::reset_state() { m_active_state.reset(); m_pending_state.reset(); + m_dtls_peer_traffic_seen = true; m_readbuf.clear(); m_write_cipher_states.clear(); m_read_cipher_states.clear(); @@ -63,6 +79,7 @@ void Channel_Impl_12::reset_active_association_state() { // This operation only makes sense for DTLS BOTAN_ASSERT_NOMSG(m_is_datagram); m_active_state.reset(); + m_dtls_peer_traffic_seen = true; m_read_cipher_states.clear(); m_write_cipher_states.clear(); @@ -175,6 +192,12 @@ bool Channel_Impl_12::timeout_check() { return m_pending_state->handshake_io().timeout_check(); } + if(m_is_datagram && !m_has_been_closed && m_active_state.has_value() && !m_dtls_peer_traffic_seen) { + if(auto* dtls_io = m_active_state->dtls_handshake_io()) { + return dtls_io->timeout_check(); + } + } + //FIXME: scan cipher suites and remove epochs older than 2*MSL return false; } @@ -259,6 +282,18 @@ bool Channel_Impl_12::is_active() const { return !is_closed() && is_handshake_complete(); } +bool Channel_Impl_12::requires_timeout_check() const { + if(m_pending_state) { + return true; + } + + // A DTLS peer might not have received our final handshake flight yet, even + // though this side is already active. Keep timer ticks enabled until peer + // traffic confirms that it has moved on to the negotiated epoch. + return m_is_datagram && !m_has_been_closed && m_active_state.has_value() && !m_dtls_peer_traffic_seen && + m_active_state->dtls_handshake_io() != nullptr; +} + bool Channel_Impl_12::is_closed() const { return m_has_been_closed; } @@ -281,8 +316,15 @@ void Channel_Impl_12::activate_session() { // For DTLS, keep the handshake IO for last-flight retransmission. if(m_is_datagram) { m_active_state = Active_Connection_State_12(state, application_protocol(), m_pending_state->take_handshake_io()); + if(auto* dtls_io = m_active_state->dtls_handshake_io()) { + // Mark the just-sent flight complete so timeout_check() may + // retransmit it if the peer never receives it. + dtls_io->conclude_flight(); + } + m_dtls_peer_traffic_seen = false; } else { m_active_state = Active_Connection_State_12(state, application_protocol()); + m_dtls_peer_traffic_seen = true; } m_pending_state.reset(); @@ -407,25 +449,49 @@ void Channel_Impl_12::process_handshake_ccs(const secure_vector& record Protocol_Version record_version, bool epoch0_restart) { if(!m_pending_state) { - // No pending handshake, possibly new: + // With no pending handshake this is either a new handshake attempt or a + // DTLS retransmission from the previous handshake. The latter must not + // create fresh pending state; it only asks us to replay our last flight. + if(record_version.is_datagram_protocol() && epoch0_restart && m_sequence_numbers && m_active_state.has_value()) { + const bool starts_new_handshake = is_new_dtls_association_client_hello(record, record_type); + + if(!starts_new_handshake) { + BOTAN_ASSERT(m_active_state->dtls_handshake_io(), "Have DTLS handshake IO for retransmission"); + m_active_state->dtls_handshake_io()->add_retransmitted_record( + record.data(), record.size(), record_type, record_sequence); + return; + } + } + if(record_version.is_datagram_protocol() && !epoch0_restart) { if(m_sequence_numbers) { - /* - * Might be a peer retransmit under epoch - 1 in which - * case we must retransmit last flight - */ sequence_numbers().read_accept(record_sequence); const uint16_t epoch = record_sequence >> 48; const uint16_t current_epoch = sequence_numbers().current_read_epoch(); if(epoch == current_epoch) { - create_handshake_state(record_version); + // Either endpoint can initiate renegotiation from FINISHED: + // clients send ClientHello, servers send HelloRequest. + const bool starts_new_handshake = + (record_type == Record_Type::Handshake && !record.empty() && + (static_cast(record[0]) == Handshake_Type::ClientHello || + static_cast(record[0]) == Handshake_Type::HelloRequest)); + + if(m_active_state.has_value() && !starts_new_handshake) { + BOTAN_ASSERT(m_active_state->dtls_handshake_io(), "Have DTLS handshake IO for retransmission"); + m_active_state->dtls_handshake_io()->add_retransmitted_record( + record.data(), record.size(), record_type, record_sequence); + return; + } else { + create_handshake_state(record_version); + } } else if(current_epoch > 0 && epoch == current_epoch - 1) { 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( + m_active_state->dtls_handshake_io()->add_retransmitted_record( record.data(), record.size(), record_type, record_sequence); + return; } } else { create_handshake_state(record_version); @@ -467,6 +533,10 @@ void Channel_Impl_12::process_application_data(uint64_t seq_no, const secure_vec throw Unexpected_Message("Application data received in unexpected read epoch"); } + if(m_is_datagram) { + m_dtls_peer_traffic_seen = true; + } + callbacks().tls_record_received(seq_no, record); } diff --git a/src/lib/tls/tls12/tls_channel_impl_12.h b/src/lib/tls/tls12/tls_channel_impl_12.h index 4e15248e91f..a1e1144aafd 100644 --- a/src/lib/tls/tls12/tls_channel_impl_12.h +++ b/src/lib/tls/tls12/tls_channel_impl_12.h @@ -87,6 +87,8 @@ class Channel_Impl_12 : public Channel_Impl { */ bool is_active() const override; + bool requires_timeout_check() const override; + /** * @return true iff the connection has been definitely closed */ @@ -142,10 +144,8 @@ class Channel_Impl_12 : public Channel_Impl { bool secure_renegotiation_supported() const override; /** - * Perform a handshake timeout check. This does nothing unless - * this is a DTLS channel with a pending handshake state, in - * which case we check for timeout and potentially retransmit - * handshake packets. + * Perform a handshake timeout check. This does nothing unless this is a + * DTLS channel that still needs handshake retransmission handling. */ bool timeout_check() override; @@ -246,6 +246,7 @@ class Channel_Impl_12 : public Channel_Impl { secure_vector m_record_buf; bool m_has_been_closed; + bool m_dtls_peer_traffic_seen = true; std::optional m_active_state; }; diff --git a/src/lib/tls/tls12/tls_connection_state_12.h b/src/lib/tls/tls12/tls_connection_state_12.h index e60eec68184..db377d61ac0 100644 --- a/src/lib/tls/tls12/tls_connection_state_12.h +++ b/src/lib/tls/tls12/tls_connection_state_12.h @@ -81,6 +81,8 @@ class Active_Connection_State_12 final { */ Datagram_Handshake_IO* dtls_handshake_io() { return m_dtls_handshake_io.get(); } + const Datagram_Handshake_IO* dtls_handshake_io() const { return m_dtls_handshake_io.get(); } + private: Protocol_Version m_version; uint16_t m_ciphersuite_code = 0; diff --git a/src/lib/tls/tls12/tls_handshake_io.cpp b/src/lib/tls/tls12/tls_handshake_io.cpp index 436040991da..69c8da80b5a 100644 --- a/src/lib/tls/tls12/tls_handshake_io.cpp +++ b/src/lib/tls/tls12/tls_handshake_io.cpp @@ -151,6 +151,8 @@ Protocol_Version Datagram_Handshake_IO::initial_record_version() const { } void Datagram_Handshake_IO::retransmit_last_flight() { + // m_flights keeps an empty trailing slot while waiting for the peer, so the + // last completed flight is normally the one before it. const size_t flight_idx = (m_flights.size() == 1) ? 0 : (m_flights.size() - 2); retransmit_flight(flight_idx); } @@ -160,13 +162,16 @@ void Datagram_Handshake_IO::retransmit_flight(size_t flight_idx) { BOTAN_ASSERT(!flight.empty(), "Nonempty flight to retransmit"); - uint16_t epoch = m_flight_data[flight[0]].epoch; + const uint16_t first_epoch = m_flight_data[flight[0]].epoch; + uint16_t epoch = (first_epoch == 0) ? 0 : first_epoch - 1; for(auto msg_seq : flight) { auto& msg = m_flight_data[msg_seq]; if(msg.epoch != epoch) { - // Epoch gap: insert the CCS + // CCS is not a handshake message and therefore has no message_seq of + // its own. When replaying a cached flight that crosses into a new + // epoch, synthesize the CCS record that originally separated them. const std::vector ccs(1, 1); m_send_hs(epoch, Record_Type::ChangeCipherSpec, ccs); } @@ -177,7 +182,19 @@ void Datagram_Handshake_IO::retransmit_flight(size_t flight_idx) { } bool Datagram_Handshake_IO::have_more_data() const { - return false; + // Future or incomplete fragments remain buffered, but only a complete + // next-in-sequence message is trailing handshake data. + const auto next = m_messages.find(m_in_message_seq); + return next != m_messages.end() && next->second.complete(); +} + +void Datagram_Handshake_IO::conclude_flight() { + // Keep an empty trailing flight to mean "we are waiting for the peer". + // Retransmission then replays the previous, completed flight instead of + // appending to it. + if(!m_flights.rbegin()->empty()) { + m_flights.push_back(std::vector()); + } } bool Datagram_Handshake_IO::timeout_check() { @@ -197,6 +214,8 @@ bool Datagram_Handshake_IO::timeout_check() { retransmit_last_flight(); + // Retransmission restarts the backoff window just like a normal write. + m_last_write = steady_clock_ms(); m_next_timeout = std::min(2 * m_next_timeout, m_max_timeout); return true; } @@ -205,6 +224,109 @@ void Datagram_Handshake_IO::add_record(const uint8_t record[], size_t record_len, Record_Type record_type, uint64_t record_sequence) { + add_record(record, record_len, record_type, record_sequence, false); +} + +void Datagram_Handshake_IO::add_retransmitted_record(const uint8_t record[], + size_t record_len, + Record_Type record_type, + uint64_t record_sequence) { + add_record(record, record_len, record_type, record_sequence, true); +} + +bool Datagram_Handshake_IO::reassemble_retransmitted_fragment(const uint8_t fragment[], + size_t fragment_length, + size_t fragment_offset, + uint16_t epoch, + Handshake_Type msg_type, + size_t msg_length, + uint16_t message_seq) { + auto [i, inserted] = m_retransmitted_messages.try_emplace(msg_type, message_seq, Handshake_Reassembly{}); + + if(!inserted && i->second.first != message_seq) { + i->second = std::make_pair(message_seq, Handshake_Reassembly()); + } + + auto& reassembly = i->second.second; + reassembly.add_fragment(fragment, fragment_length, fragment_offset, epoch, msg_type, msg_length); + + if(!reassembly.complete()) { + return false; + } + + m_retransmitted_messages.erase(i); + return true; +} + +bool Datagram_Handshake_IO::process_previous_handshake_fragment(const uint8_t fragment[], + size_t fragment_length, + size_t fragment_offset, + uint16_t epoch, + Handshake_Type msg_type, + size_t msg_length, + uint16_t message_seq, + bool retransmitted_flight) { + // Empty fragments of non-empty messages add no information and must not + // trigger a flight retransmission. A zero-length message such as + // ServerHelloDone is not an empty fragment in this sense. + if(fragment_length == 0 && msg_length != 0) { + return false; + } + + // ClientHello is special: losing HelloVerifyRequest leaves the server in + // the pending handshake, where it must still replay the cookie. + if(msg_type == Handshake_Type::ClientHello) { + return reassemble_retransmitted_fragment( + fragment, fragment_length, fragment_offset, epoch, msg_type, msg_length, message_seq); + } + + // Other previous-flight messages request an immediate response only after + // the handshake IO was retained by an active association. While pending, + // the normal retransmission timer handles recovery. + if(!retransmitted_flight) { + return false; + } + + if(msg_type == Handshake_Type::ServerHello) { + m_retransmitted_server_hello_complete = reassemble_retransmitted_fragment( + fragment, fragment_length, fragment_offset, epoch, msg_type, msg_length, message_seq); + + if(m_retransmitted_server_hello_complete && m_retransmitted_server_hello_done_complete) { + m_retransmitted_server_hello_complete = false; + m_retransmitted_server_hello_done_complete = false; + return true; + } + } else if(msg_type == Handshake_Type::ServerHelloDone && msg_length == 0) { + if(reassemble_retransmitted_fragment( + fragment, fragment_length, fragment_offset, epoch, msg_type, msg_length, message_seq)) { + m_retransmitted_server_hello_done_complete = true; + if(m_retransmitted_server_hello_complete) { + m_retransmitted_server_hello_complete = false; + m_retransmitted_server_hello_done_complete = false; + return true; + } + } + } else if(msg_type == Handshake_Type::Finished && + reassemble_retransmitted_fragment( + fragment, fragment_length, fragment_offset, epoch, msg_type, msg_length, message_seq)) { + // A final-flight retransmission includes CCS, but UDP may deliver its + // records in either order. Wait until both have been observed. + if(m_retransmitted_ccs_epoch == epoch) { + m_retransmitted_ccs_epoch.reset(); + return true; + } + + m_retransmitted_finished_epoch = epoch; + } + + return false; +} + +void Datagram_Handshake_IO::add_record(const uint8_t record[], + size_t record_len, + Record_Type record_type, + uint64_t record_sequence, + bool retransmitted_flight) { const uint16_t epoch = static_cast(record_sequence >> 48); if(record_type == Record_Type::ChangeCipherSpec) { @@ -214,11 +336,25 @@ void Datagram_Handshake_IO::add_record(const uint8_t record[], // TODO: check this is otherwise empty m_ccs_epochs.insert(epoch); + if(retransmitted_flight) { + // Retransmitted final flights cross the epoch boundary: CCS is sent + // under the previous epoch and Finished under the newly activated one. + // Keep both observations because their datagrams may arrive reordered. + const uint16_t finished_epoch = static_cast(epoch + 1); + if(m_retransmitted_finished_epoch == finished_epoch) { + m_retransmitted_finished_epoch.reset(); + retransmit_last_flight(); + } else { + m_retransmitted_ccs_epoch = finished_epoch; + } + } return; } const size_t DTLS_HANDSHAKE_HEADER_LEN = 12; + bool retransmit_response = false; + while(record_len > 0) { if(record_len < DTLS_HANDSHAKE_HEADER_LEN) { return; // completely bogus? at least degenerate/weird @@ -253,6 +389,16 @@ void Datagram_Handshake_IO::add_record(const uint8_t record[], const size_t max_pending = 4 * m_max_handshake_msg_size; if(message_seq >= m_in_message_seq && (message_seq - m_in_message_seq) < reassembly_window) { + if(retransmitted_flight) { + if(fragment_length == 0) { + record += total_size; + record_len -= total_size; + continue; + } + + throw TLS_Exception(Alert::UnexpectedMessage, "Unexpected new DTLS handshake message"); + } + auto [it, inserted] = m_messages.try_emplace(message_seq); if(inserted) { if(m_max_handshake_msg_size > 0 && m_pending_reassembly_bytes + msg_len > max_pending) { @@ -266,12 +412,23 @@ void Datagram_Handshake_IO::add_record(const uint8_t record[], it->second.add_fragment( &record[DTLS_HANDSHAKE_HEADER_LEN], fragment_length, fragment_offset, epoch, msg_type, msg_len); } else { - // TODO: detect retransmitted flight + retransmit_response |= process_previous_handshake_fragment(&record[DTLS_HANDSHAKE_HEADER_LEN], + fragment_length, + fragment_offset, + epoch, + msg_type, + msg_len, + message_seq, + retransmitted_flight); } record += total_size; record_len -= total_size; } + + if(retransmit_response) { + retransmit_last_flight(); + } } std::pair> Datagram_Handshake_IO::get_next_record(bool expecting_ccs, @@ -428,9 +585,16 @@ std::vector Datagram_Handshake_IO::send_under_epoch(const Handshake_Mes m_send_hs(epoch, Record_Type::ChangeCipherSpec, msg_bits); return std::vector(); // not included in handshake hashes } else if(msg_type == Handshake_Type::HelloVerifyRequest) { - // This message is not included in the handshake hashes - send_message(m_out_message_seq, epoch, msg_type, msg_bits); + // This message is not included in the handshake hashes, but it is a + // DTLS flight and must be available for retransmission if it is lost. + m_flights.rbegin()->push_back(m_out_message_seq); + m_flight_data[m_out_message_seq] = Message_Info(epoch, msg_type, msg_bits); + m_out_message_seq += 1; + m_last_write = steady_clock_ms(); + m_next_timeout = m_initial_timeout; + + send_message(m_out_message_seq - 1, epoch, msg_type, msg_bits); return std::vector(); } diff --git a/src/lib/tls/tls12/tls_handshake_io.h b/src/lib/tls/tls12/tls_handshake_io.h index 2acfcb6e7eb..6ef2565fa68 100644 --- a/src/lib/tls/tls12/tls_handshake_io.h +++ b/src/lib/tls/tls12/tls_handshake_io.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -144,10 +145,42 @@ class Datagram_Handshake_IO final : public Handshake_IO { void add_record(const uint8_t record[], size_t record_len, Record_Type type, uint64_t sequence_number) override; + // Process previous-flight records after this IO object was retained by + // an active DTLS association for final-flight recovery. + void add_retransmitted_record(const uint8_t record[], + size_t record_len, + Record_Type type, + uint64_t sequence_number); + std::pair> get_next_record(bool expecting_ccs, size_t max_message_size) override; + void conclude_flight(); + private: + void add_record(const uint8_t record[], + size_t record_len, + Record_Type record_type, + uint64_t record_sequence, + bool retransmitted_flight); + + bool reassemble_retransmitted_fragment(const uint8_t fragment[], + size_t fragment_length, + size_t fragment_offset, + uint16_t epoch, + Handshake_Type msg_type, + size_t msg_length, + uint16_t message_seq); + + bool process_previous_handshake_fragment(const uint8_t fragment[], + size_t fragment_length, + size_t fragment_offset, + uint16_t epoch, + Handshake_Type msg_type, + size_t msg_length, + uint16_t message_seq, + bool retransmitted_flight); + void retransmit_flight(size_t flight); void retransmit_last_flight(); @@ -215,6 +248,14 @@ class Datagram_Handshake_IO final : public Handshake_IO { std::map m_messages; size_t m_pending_reassembly_bytes = 0; std::set m_ccs_epochs; + + // A retransmitted final flight may deliver CCS and Finished in either + // order. Other terminal messages may themselves be fragmented. + std::optional m_retransmitted_ccs_epoch; + std::optional m_retransmitted_finished_epoch; + std::map> m_retransmitted_messages; + bool m_retransmitted_server_hello_complete = false; + bool m_retransmitted_server_hello_done_complete = false; std::vector> m_flights; std::map m_flight_data; diff --git a/src/lib/tls/tls_channel.h b/src/lib/tls/tls_channel.h index 0ba26c7ce47..fc201030b85 100644 --- a/src/lib/tls/tls_channel.h +++ b/src/lib/tls/tls_channel.h @@ -121,6 +121,12 @@ class BOTAN_PUBLIC_API(2, 0) Channel { */ virtual bool is_active() const = 0; + /** + * @return true iff timeout_check() should be polled to drive handshake + * retransmissions. + */ + virtual bool requires_timeout_check() const { return !is_handshake_complete(); } + /** * Note: For TLS 1.3 a connection is closed only after both peers have * signaled a "close_notify". While TLS 1.2 automatically responded @@ -196,9 +202,9 @@ class BOTAN_PUBLIC_API(2, 0) Channel { * Perform a handshake timeout check. * * This function does nothing unless the channel represents a DTLS - * connection and a handshake is actively in progress. In this case it will - * check the current timeout state and potentially initiate retransmission - * of handshake packets. + * connection and a handshake still needs retransmission handling. This can + * include a locally active DTLS session whose final flight may not have + * reached the peer yet. * * @returns true if a timeout condition occurred */ diff --git a/src/lib/tls/tls_channel_impl.h b/src/lib/tls/tls_channel_impl.h index adf14e936c5..9ad559fb520 100644 --- a/src/lib/tls/tls_channel_impl.h +++ b/src/lib/tls/tls_channel_impl.h @@ -85,6 +85,12 @@ class Channel_Impl { */ virtual bool is_active() const = 0; + /** + * @return true iff timeout_check() should be polled to drive handshake + * retransmissions. + */ + virtual bool requires_timeout_check() const { return !is_handshake_complete(); } + /** * @return true iff the connection has been definitely closed */ @@ -167,10 +173,10 @@ class Channel_Impl { virtual bool secure_renegotiation_supported() const = 0; /** - * Perform a handshake timeout check. This does nothing unless - * this is a DTLS channel with a pending handshake state, in - * which case we check for timeout and potentially retransmit - * handshake packets. + * Perform a handshake timeout check. This does nothing unless this is a + * DTLS channel that still needs handshake retransmission handling. A DTLS + * channel can require this briefly after local activation while its last + * flight may still be lost in transit. */ virtual bool timeout_check() = 0; diff --git a/src/lib/tls/tls_client.cpp b/src/lib/tls/tls_client.cpp index 039979b92be..de8dcc40dd6 100644 --- a/src/lib/tls/tls_client.cpp +++ b/src/lib/tls/tls_client.cpp @@ -119,6 +119,10 @@ bool Client::is_active() const { return m_impl->is_active(); } +bool Client::requires_timeout_check() const { + return m_impl->requires_timeout_check(); +} + bool Client::is_closed() const { return m_impl->is_closed(); } diff --git a/src/lib/tls/tls_client.h b/src/lib/tls/tls_client.h index ccb8f6eb5da..7b0dbbdb9b9 100644 --- a/src/lib/tls/tls_client.h +++ b/src/lib/tls/tls_client.h @@ -100,6 +100,8 @@ class BOTAN_PUBLIC_API(2, 0) Client final : public Channel { bool is_active() const override; + bool requires_timeout_check() const override; + bool is_closed() const override; bool is_closed_for_reading() const override; diff --git a/src/lib/tls/tls_server.cpp b/src/lib/tls/tls_server.cpp index 07ab9348bed..1304a05ba2b 100644 --- a/src/lib/tls/tls_server.cpp +++ b/src/lib/tls/tls_server.cpp @@ -87,6 +87,10 @@ bool Server::is_active() const { return m_impl->is_active(); } +bool Server::requires_timeout_check() const { + return m_impl->requires_timeout_check(); +} + bool Server::is_closed() const { return m_impl->is_closed(); } diff --git a/src/lib/tls/tls_server.h b/src/lib/tls/tls_server.h index b1da439ef04..6e2c50bdb00 100644 --- a/src/lib/tls/tls_server.h +++ b/src/lib/tls/tls_server.h @@ -90,6 +90,8 @@ class BOTAN_PUBLIC_API(2, 0) Server final : public Channel { bool is_active() const override; + bool requires_timeout_check() const override; + bool is_closed() const override; bool is_closed_for_reading() const override; diff --git a/src/tests/unit_tls.cpp b/src/tests/unit_tls.cpp index 56452b7e6e8..a8388570e28 100644 --- a/src/tests/unit_tls.cpp +++ b/src/tests/unit_tls.cpp @@ -11,9 +11,14 @@ */ #include "tests.h" +#include #include #include +#if defined(BOTAN_TARGET_OS_HAS_THREADS) + #include +#endif + #if defined(BOTAN_HAS_TLS) && defined(BOTAN_HAS_RSA) #include @@ -29,6 +34,7 @@ #include #include #include + #include #include #include @@ -1473,6 +1479,1168 @@ BOTAN_REGISTER_TEST("tls", "unit_tls", TLS_Unit_Tests); #if defined(BOTAN_HAS_TLS_12) +class Dtls_Test_Callbacks final : public Botan::TLS::Callbacks { + public: + Dtls_Test_Callbacks(Test::Result& result, std::vector& outbound, std::vector& received) : + m_result(result), m_outbound(outbound), m_received(received) {} + + void tls_emit_data(std::span bits) override { + m_outbound.insert(m_outbound.end(), bits.begin(), bits.end()); + } + + void tls_record_received(uint64_t /*seq*/, std::span bits) override { + m_received.insert(m_received.end(), bits.begin(), bits.end()); + } + + void tls_alert(Botan::TLS::Alert alert) override { + if(alert.is_fatal()) { + m_result.test_failure("unexpected fatal alert: " + alert.type_string()); + } + } + + void tls_session_established(const Botan::TLS::Session_Summary& session) override { + m_session_was_resumption = session.was_resumption(); + ++m_sessions_established; + } + + size_t sessions_established() const { return m_sessions_established; } + + std::optional session_was_resumption() const { return m_session_was_resumption; } + + private: + Test::Result& m_result; + std::vector& m_outbound; + std::vector& m_received; + size_t m_sessions_established = 0; + std::optional m_session_was_resumption; +}; + +class Dtls_PSK_Credentials final : public Botan::Credentials_Manager { + public: + Botan::SymmetricKey psk(const std::string& type, + const std::string& context, + const std::string& /*identity*/) override { + if(type == "tls-server" && context == "session-ticket") { + return Botan::SymmetricKey("AABBCCDDEEFF012345678012345678"); + } + + if(type == "tls-server" && context == "dtls-cookie-secret") { + return Botan::SymmetricKey("4AEA5EAD279CADEB537A594DA0E9DE3A"); + } + + if(context == "localhost" && (type == "tls-client" || type == "tls-server")) { + return Botan::SymmetricKey("20B602D1475F2DF888FCB60D2AE03AFD"); + } + + throw Test_Error("No PSK set for " + type + "/" + context); + } +}; + +class Dtls_PSK_Policy final : public Botan::TLS::Policy { + public: + std::vector allowed_macs() const override { return {"AEAD"}; } + + std::vector allowed_key_exchange_methods() const override { return {"PSK"}; } + + bool allow_tls12() const override { return false; } + + bool allow_dtls12() const override { return true; } + + bool allow_dtls_epoch0_restart() const override { return true; } + + bool allow_server_initiated_renegotiation() const override { return true; } + + bool allow_client_initiated_renegotiation() const override { return true; } + + size_t dtls_initial_timeout() const override { return 1; } + + size_t dtls_maximum_timeout() const override { return 8; } +}; + +/* +These tests exercise the DTLS core without opening real UDP sockets. The test +callbacks collect bytes emitted by TLS::Client/TLS::Server, and deliver() feeds +those bytes into the peer's received_data(). Clearing, splitting, copying, or +delaying those buffers simulates datagram loss, retransmission, duplicates, and +partial flights while still running the real DTLS record, handshake, epoch, and +retransmission logic. +*/ +class DTLS_Core_Regression_Tests final : public Test { + private: + static void deliver(Test::Result& result, + const std::string& label, + std::vector& outbound, + Botan::TLS::Channel& peer) { + if(!result.test_is_true(label + " has data", !outbound.empty())) { + return; + } + + std::vector input; + std::swap(input, outbound); + result.test_no_throw(label, [&] { peer.received_data(input.data(), input.size()); }); + } + + static void deliver_copy(Test::Result& result, + const std::string& label, + const std::vector& outbound, + Botan::TLS::Channel& peer) { + if(!result.test_is_true(label + " has data", !outbound.empty())) { + return; + } + + result.test_no_throw(label, [&] { peer.received_data(outbound.data(), outbound.size()); }); + } + + static bool split_first_dtls_record(Test::Result& result, + const std::vector& records, + std::vector& first_record, + std::vector& remaining_records) { + constexpr size_t dtls_header_len = 13; + + if(!result.test_is_true("DTLS records include a complete header", records.size() >= dtls_header_len)) { + return false; + } + + const size_t record_len = static_cast((static_cast(records[11]) << 8) | records[12]); + const size_t total_len = dtls_header_len + record_len; + + if(!result.test_is_true("DTLS records include a complete first record", records.size() >= total_len)) { + return false; + } + + first_record.assign(records.begin(), records.begin() + total_len); + remaining_records.assign(records.begin() + total_len, records.end()); + return result.test_is_true("DTLS server flight has more than one record", !remaining_records.empty()); + } + + static std::vector dtls_handshake_types(const std::vector& records) { + // Lightweight test-only DTLS record inspection. This deliberately + // looks only at unencrypted handshake records, so labels like + // "retransmitted HelloVerifyRequest" are backed by wire data without + // turning the test into a second DTLS implementation. + constexpr size_t dtls_header_len = 13; + constexpr size_t dtls_handshake_header_len = 12; + + std::vector types; + + size_t offset = 0; + while(offset + dtls_header_len <= records.size()) { + const auto record_type = static_cast(records[offset]); + const size_t record_len = + static_cast((static_cast(records[offset + 11]) << 8) | records[offset + 12]); + const size_t record_end = offset + dtls_header_len + record_len; + + if(record_end > records.size()) { + break; + } + + if(record_type == Botan::TLS::Record_Type::Handshake) { + size_t hs_offset = offset + dtls_header_len; + while(hs_offset + dtls_handshake_header_len <= record_end) { + const auto handshake_type = static_cast(records[hs_offset]); + const size_t fragment_len = (static_cast(records[hs_offset + 9]) << 16) | + (static_cast(records[hs_offset + 10]) << 8) | + records[hs_offset + 11]; + const size_t handshake_end = hs_offset + dtls_handshake_header_len + fragment_len; + + if(handshake_end > record_end) { + break; + } + + types.push_back(handshake_type); + hs_offset = handshake_end; + } + } + + offset = record_end; + } + + return types; + } + + static std::vector dtls_record_types(const std::vector& records) { + constexpr size_t dtls_header_len = 13; + + std::vector types; + + size_t offset = 0; + while(offset + dtls_header_len <= records.size()) { + const auto record_type = static_cast(records[offset]); + const size_t record_len = + static_cast((static_cast(records[offset + 11]) << 8) | records[offset + 12]); + const size_t record_end = offset + dtls_header_len + record_len; + + if(record_end > records.size()) { + break; + } + + types.push_back(record_type); + offset = record_end; + } + + return types; + } + + static bool contains_dtls_handshake_type(const std::vector& records, + Botan::TLS::Handshake_Type expected) { + for(auto type : dtls_handshake_types(records)) { + if(type == expected) { + return true; + } + } + + return false; + } + + static bool contains_dtls_record_type(const std::vector& records, Botan::TLS::Record_Type expected) { + for(auto type : dtls_record_types(records)) { + if(type == expected) { + return true; + } + } + + return false; + } + + static std::vector empty_dtls_handshake_fragment(Botan::TLS::Handshake_Type type, + size_t message_length, + uint16_t message_sequence) { + std::vector record; + record.reserve(25); + + record.push_back(static_cast(Botan::TLS::Record_Type::Handshake)); + record.push_back(0xFE); + record.push_back(0xFD); + record.insert(record.end(), 8, 0); // epoch 0, sequence number 0 + record.push_back(0); + record.push_back(12); + + record.push_back(static_cast(type)); + record.push_back(static_cast((message_length >> 16) & 0xFF)); + record.push_back(static_cast((message_length >> 8) & 0xFF)); + record.push_back(static_cast(message_length & 0xFF)); + record.push_back(static_cast((message_sequence >> 8) & 0xFF)); + record.push_back(static_cast(message_sequence & 0xFF)); + record.insert(record.end(), 6, 0); // fragment offset 0, fragment length 0 + + return record; + } + + template + static bool wait_until(Predicate predicate) { + // DTLS timeouts are clock based. Poll briefly instead of sleeping for + // an exact duration so slow/debug builds and timer granularity do not + // make retransmission tests flaky. + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(1); + + while(std::chrono::steady_clock::now() < deadline) { + if(predicate()) { + return true; + } + + #if defined(BOTAN_TARGET_OS_HAS_THREADS) + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + #endif + } + + return predicate(); + } + + template + static bool remains_false_for(Predicate predicate, std::chrono::milliseconds duration) { + // Negative timeout checks need to observe a small window: a single + // immediate false could pass even though the channel retransmits a few + // milliseconds later. + const auto deadline = std::chrono::steady_clock::now() + duration; + + while(std::chrono::steady_clock::now() < deadline) { + if(predicate()) { + return false; + } + + #if defined(BOTAN_TARGET_OS_HAS_THREADS) + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + #endif + } + + return !predicate(); + } + + static void wait_for_timeout_retransmit(Test::Result& result, + Botan::TLS::Channel& channel, + std::vector& outbound) { + if(!wait_until([&] { return channel.timeout_check() && !outbound.empty(); })) { + result.test_failure("DTLS retransmit was not produced"); + } + } + + static Test::Result test_timeout_check_paces_retransmissions() { + Test::Result result("DTLS timeout_check retransmit pacing"); + + auto rng = Test::new_shared_rng("dtls-core-timeout-pacing"); + auto policy = std::make_shared(); + auto creds = std::make_shared(); + auto client_sessions = std::make_shared(rng); + auto server_sessions = std::make_shared(rng); + + std::vector c2s; + std::vector s2c; + std::vector client_recv; + std::vector server_recv; + + auto server_callbacks = std::make_shared(result, s2c, server_recv); + auto client_callbacks = std::make_shared(result, c2s, client_recv); + + Botan::TLS::Server server(server_callbacks, server_sessions, creds, policy, rng, true); + Botan::TLS::Client client(client_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + deliver(result, "client hello 1", c2s, server); + if(!result.test_is_true("hello verify request was produced", !s2c.empty())) { + return result; + } + s2c.clear(); // simulate losing the HelloVerifyRequest + + wait_for_timeout_retransmit(result, client, c2s); + const auto retransmit_size = c2s.size(); + + result.test_is_false("immediate second timeout is suppressed", client.timeout_check()); + result.test_sz_eq("no immediate second retransmit", c2s.size(), retransmit_size); + + return result; + } + + static Test::Result test_retransmitted_epoch_transition_flight_includes_ccs() { + Test::Result result("DTLS retransmitted epoch-1 flight includes CCS"); + + auto rng = Test::new_shared_rng("dtls-core-retransmitted-ccs"); + auto policy = std::make_shared(); + auto creds = std::make_shared(); + auto client_sessions = std::make_shared(rng); + auto server_sessions = std::make_shared(rng); + + std::vector c2s; + std::vector s2c; + std::vector client_recv; + std::vector server_recv; + + auto server_callbacks = std::make_shared(result, s2c, server_recv); + auto client_callbacks = std::make_shared(result, c2s, client_recv); + + Botan::TLS::Server server(server_callbacks, server_sessions, creds, policy, rng, true); + Botan::TLS::Client client(client_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + deliver(result, "client hello 1", c2s, server); + deliver(result, "hello verify request", s2c, client); + deliver(result, "client hello 2", c2s, server); + deliver(result, "server handshake flight", s2c, client); + deliver(result, "client final flight", c2s, server); + + result.test_is_true("server became active", server.is_active()); + if(!result.test_is_true("server final flight was produced", !s2c.empty())) { + return result; + } + s2c.clear(); // simulate losing the server final flight + + wait_for_timeout_retransmit(result, server, s2c); + + result.test_is_true("retransmitted final flight includes CCS", + contains_dtls_record_type(s2c, Botan::TLS::Record_Type::ChangeCipherSpec)); + result.test_is_true("retransmitted final flight includes Finished record", + contains_dtls_record_type(s2c, Botan::TLS::Record_Type::Handshake)); + + deliver(result, "retransmitted server final flight", s2c, client); + result.test_is_true("client became active", client.is_active()); + + return result; + } + + static Test::Result test_lost_hello_verify_request_retransmits() { + Test::Result result("DTLS lost HelloVerifyRequest retransmits"); + + auto rng = Test::new_shared_rng("dtls-core-lost-hvr"); + auto policy = std::make_shared(); + auto creds = std::make_shared(); + auto client_sessions = std::make_shared(rng); + auto server_sessions = std::make_shared(rng); + + std::vector c2s; + std::vector s2c; + std::vector client_recv; + std::vector server_recv; + + auto server_callbacks = std::make_shared(result, s2c, server_recv); + auto client_callbacks = std::make_shared(result, c2s, client_recv); + + Botan::TLS::Server server(server_callbacks, server_sessions, creds, policy, rng, true); + Botan::TLS::Client client(client_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + deliver(result, "client hello 1", c2s, server); + if(!result.test_is_true("hello verify request was produced", !s2c.empty())) { + return result; + } + result.test_is_true("server response is HelloVerifyRequest", + contains_dtls_handshake_type(s2c, Botan::TLS::Handshake_Type::HelloVerifyRequest)); + s2c.clear(); // simulate losing the HelloVerifyRequest + + wait_for_timeout_retransmit(result, client, c2s); + deliver(result, "client hello 1 retransmit", c2s, server); + if(!result.test_is_true("server retransmitted hello verify request", !s2c.empty())) { + return result; + } + result.test_is_true("server retransmission is HelloVerifyRequest", + contains_dtls_handshake_type(s2c, Botan::TLS::Handshake_Type::HelloVerifyRequest)); + + deliver(result, "retransmitted hello verify request", s2c, client); + deliver(result, "client hello 2", c2s, server); + deliver(result, "server handshake flight", s2c, client); + deliver(result, "client final flight", c2s, server); + deliver(result, "server final flight", s2c, client); + + result.test_is_true("client became active", client.is_active()); + result.test_is_true("server became active", server.is_active()); + + return result; + } + + static Test::Result test_duplicate_hello_verify_request_is_tolerated() { + Test::Result result("DTLS duplicate HelloVerifyRequest is tolerated"); + + auto rng = Test::new_shared_rng("dtls-core-duplicate-hvr"); + auto policy = std::make_shared(); + auto creds = std::make_shared(); + auto client_sessions = std::make_shared(rng); + auto server_sessions = std::make_shared(rng); + + std::vector c2s; + std::vector s2c; + std::vector client_recv; + std::vector server_recv; + + auto server_callbacks = std::make_shared(result, s2c, server_recv); + auto client_callbacks = std::make_shared(result, c2s, client_recv); + + Botan::TLS::Server server(server_callbacks, server_sessions, creds, policy, rng, true); + Botan::TLS::Client client(client_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + deliver(result, "client hello 1", c2s, server); + + const std::vector hello_verify = s2c; + if(!result.test_is_true("hello verify request was produced", !hello_verify.empty())) { + return result; + } + result.test_is_true( + "server response is HelloVerifyRequest", + contains_dtls_handshake_type(hello_verify, Botan::TLS::Handshake_Type::HelloVerifyRequest)); + s2c.clear(); + + deliver_copy(result, "hello verify request", hello_verify, client); + if(!result.test_is_true("client hello 2 was produced", !c2s.empty())) { + return result; + } + std::vector client_hello_2; + std::swap(client_hello_2, c2s); + + deliver_copy(result, "duplicate hello verify request", hello_verify, client); + result.test_is_true("duplicate hello verify request is ignored", c2s.empty()); + + deliver_copy(result, "client hello 2 after duplicate hvr", client_hello_2, server); + deliver(result, "server handshake flight", s2c, client); + deliver(result, "client final flight", c2s, server); + deliver(result, "server final flight", s2c, client); + + result.test_is_true("client became active", client.is_active()); + result.test_is_true("server became active", server.is_active()); + + return result; + } + + static Test::Result test_partial_server_flight_does_not_advance_client() { + Test::Result result("DTLS partial server flight waits for ServerHelloDone"); + + auto rng = Test::new_shared_rng("dtls-core-partial-server-flight"); + auto policy = std::make_shared(); + auto creds = std::make_shared(); + auto client_sessions = std::make_shared(rng); + auto server_sessions = std::make_shared(rng); + + std::vector c2s; + std::vector s2c; + std::vector client_recv; + std::vector server_recv; + + auto server_callbacks = std::make_shared(result, s2c, server_recv); + auto client_callbacks = std::make_shared(result, c2s, client_recv); + + Botan::TLS::Server server(server_callbacks, server_sessions, creds, policy, rng, true); + Botan::TLS::Client client(client_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + deliver(result, "client hello 1", c2s, server); + deliver(result, "hello verify request", s2c, client); + deliver(result, "client hello 2", c2s, server); + + std::vector first_record; + std::vector remaining_records; + if(!split_first_dtls_record(result, s2c, first_record, remaining_records)) { + return result; + } + s2c.clear(); + + result.test_is_true("partial server flight starts with ServerHello", + contains_dtls_handshake_type(first_record, Botan::TLS::Handshake_Type::ServerHello)); + result.test_is_true( + "partial server flight remainder has ServerHelloDone", + contains_dtls_handshake_type(remaining_records, Botan::TLS::Handshake_Type::ServerHelloDone)); + + deliver_copy(result, "partial server flight first record", first_record, client); + result.test_is_false("client waits for the rest of the server flight", client.is_active()); + result.test_is_true("client does not send final flight early", c2s.empty()); + + deliver_copy(result, "partial server flight remaining records", remaining_records, client); + if(!result.test_is_true("client final flight was produced after ServerHelloDone", !c2s.empty())) { + return result; + } + + deliver(result, "client final flight", c2s, server); + deliver(result, "server final flight", s2c, client); + + result.test_is_true("client became active", client.is_active()); + result.test_is_true("server became active", server.is_active()); + + return result; + } + + static Test::Result test_lost_server_flight_retransmits() { + Test::Result result("DTLS lost server flight retransmits"); + + auto rng = Test::new_shared_rng("dtls-core-lost-server-flight"); + auto policy = std::make_shared(); + auto creds = std::make_shared(); + auto client_sessions = std::make_shared(rng); + auto server_sessions = std::make_shared(rng); + + std::vector c2s; + std::vector s2c; + std::vector client_recv; + std::vector server_recv; + + auto server_callbacks = std::make_shared(result, s2c, server_recv); + auto client_callbacks = std::make_shared(result, c2s, client_recv); + + Botan::TLS::Server server(server_callbacks, server_sessions, creds, policy, rng, true); + Botan::TLS::Client client(client_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + deliver(result, "client hello 1", c2s, server); + deliver(result, "hello verify request", s2c, client); + deliver(result, "client hello 2", c2s, server); + + if(!result.test_is_true("server handshake flight was produced", !s2c.empty())) { + return result; + } + s2c.clear(); // simulate losing the server flight + + wait_for_timeout_retransmit(result, client, c2s); + deliver(result, "client hello 2 retransmit", c2s, server); + if(!result.test_is_true("server retransmitted handshake flight", !s2c.empty())) { + return result; + } + + deliver(result, "retransmitted server handshake flight", s2c, client); + deliver(result, "client final flight", c2s, server); + deliver(result, "server final flight", s2c, client); + + result.test_is_true("client became active", client.is_active()); + result.test_is_true("server became active", server.is_active()); + + return result; + } + + static Test::Result test_duplicate_server_flight_defers_to_timer() { + Test::Result result("DTLS duplicate server flight defers replay to timer"); + + auto rng = Test::new_shared_rng("dtls-core-retransmitted-server-flight"); + auto policy = std::make_shared(); + auto creds = std::make_shared(); + auto client_sessions = std::make_shared(rng); + auto server_sessions = std::make_shared(rng); + + std::vector c2s; + std::vector s2c; + std::vector client_recv; + std::vector server_recv; + + auto server_callbacks = std::make_shared(result, s2c, server_recv); + auto client_callbacks = std::make_shared(result, c2s, client_recv); + + Botan::TLS::Server server(server_callbacks, server_sessions, creds, policy, rng, true); + Botan::TLS::Client client(client_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + deliver(result, "client hello 1", c2s, server); + deliver(result, "hello verify request", s2c, client); + deliver(result, "client hello 2", c2s, server); + deliver(result, "server handshake flight", s2c, client); + if(!result.test_is_true("client final flight was produced", !c2s.empty())) { + return result; + } + c2s.clear(); // simulate losing the client's response flight + + wait_for_timeout_retransmit(result, server, s2c); + + std::vector first_server_record; + std::vector remaining_server_records; + if(!split_first_dtls_record(result, s2c, first_server_record, remaining_server_records)) { + return result; + } + s2c.clear(); + + result.test_is_true( + "first retransmitted server record is ServerHello", + contains_dtls_handshake_type(first_server_record, Botan::TLS::Handshake_Type::ServerHello)); + result.test_is_true( + "remaining retransmitted records include ServerHelloDone", + contains_dtls_handshake_type(remaining_server_records, Botan::TLS::Handshake_Type::ServerHelloDone)); + + deliver_copy(result, "retransmitted ServerHello record", first_server_record, client); + result.test_is_true("non-terminal record does not replay client flight", c2s.empty()); + + deliver_copy(result, "rest of retransmitted server flight", remaining_server_records, client); + result.test_is_true("duplicated server flight does not immediately replay client flight", c2s.empty()); + + // The response flight is still recoverable. Deferring to the timer + // avoids injecting an epoch-0 replay after the peer has progressed. + wait_for_timeout_retransmit(result, client, c2s); + result.test_is_true("client timer replays the lost response flight", !c2s.empty()); + + return result; + } + + static Test::Result test_lost_server_final_flight_retransmits() { + Test::Result result("DTLS lost server final flight retransmits"); + + auto rng = Test::new_shared_rng("dtls-core-lost-server-final"); + auto policy = std::make_shared(); + auto creds = std::make_shared(); + auto client_sessions = std::make_shared(rng); + auto server_sessions = std::make_shared(rng); + + std::vector c2s; + std::vector s2c; + std::vector client_recv; + std::vector server_recv; + + auto server_callbacks = std::make_shared(result, s2c, server_recv); + auto client_callbacks = std::make_shared(result, c2s, client_recv); + + Botan::TLS::Server server(server_callbacks, server_sessions, creds, policy, rng, true); + Botan::TLS::Client client(client_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + deliver(result, "client hello 1", c2s, server); + deliver(result, "hello verify request", s2c, client); + deliver(result, "client hello 2", c2s, server); + deliver(result, "server handshake flight", s2c, client); + deliver(result, "client final flight", c2s, server); + + result.test_is_true("server became active", server.is_active()); + result.test_is_false("client is waiting for server final flight", client.is_active()); + if(!result.test_is_true("server final flight was produced", !s2c.empty())) { + return result; + } + s2c.clear(); // simulate losing the server final flight + + wait_for_timeout_retransmit(result, server, s2c); + deliver(result, "retransmitted server final flight", s2c, client); + + result.test_is_true("client became active", client.is_active()); + + return result; + } + + static Test::Result test_stale_client_hello_does_not_replace_active_handshake() { + Test::Result result("DTLS stale ClientHello after server activation"); + + auto rng = Test::new_shared_rng("dtls-core-stale-client-hello"); + auto policy = std::make_shared(); + auto creds = std::make_shared(); + auto client_sessions = std::make_shared(rng); + auto server_sessions = std::make_shared(rng); + + std::vector c2s; + std::vector s2c; + std::vector client_recv; + std::vector server_recv; + + auto server_callbacks = std::make_shared(result, s2c, server_recv); + auto client_callbacks = std::make_shared(result, c2s, client_recv); + + Botan::TLS::Server server(server_callbacks, server_sessions, creds, policy, rng, true); + Botan::TLS::Client client(client_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + deliver(result, "client hello 1", c2s, server); + deliver(result, "hello verify request", s2c, client); + const auto cookie_client_hello = c2s; + deliver(result, "client hello 2", c2s, server); + deliver(result, "server handshake flight", s2c, client); + deliver(result, "client final flight", c2s, server); + + result.test_is_true("server became active", server.is_active()); + result.test_is_false("client is waiting for server final flight", client.is_active()); + s2c.clear(); // simulate losing the server final flight + + // A delayed retransmission of the cookie-bearing ClientHello has + // message_seq 1. It belongs to the completed handshake, whereas a + // genuinely new association starts at message_seq 0. + deliver_copy(result, "stale client hello 2", cookie_client_hello, server); + result.test_is_true("server remains active", server.is_active()); + result.test_is_true("stale ClientHello replays the final server flight", !s2c.empty()); + s2c.clear(); + + wait_for_timeout_retransmit(result, client, c2s); + deliver(result, "retransmitted client final flight", c2s, server); + result.test_is_true("client final flight still receives a response", !s2c.empty()); + + return result; + } + + static Test::Result test_retransmitted_final_flight_then_application_data(bool expect_resumption) { + Test::Result result(expect_resumption ? "DTLS resumed handshake accepts app data" + : "DTLS final flight retransmit before app data"); + + auto rng = Test::new_shared_rng(expect_resumption ? "dtls-core-resumption" : "dtls-core-full"); + auto policy = std::make_shared(); + auto creds = std::make_shared(); + auto client_sessions = std::make_shared(rng); + auto server_sessions = std::make_shared(rng); + + if(expect_resumption) { + run_handshake(result, rng, policy, creds, client_sessions, server_sessions, false); + } + + return run_handshake(result, rng, policy, creds, client_sessions, server_sessions, expect_resumption); + } + + static Test::Result test_resumed_client_final_flight_retransmits_after_activation() { + Test::Result result("DTLS resumed client final flight retransmits after activation"); + + auto rng = Test::new_shared_rng("dtls-core-resumed-client-active"); + auto policy = std::make_shared(); + auto creds = std::make_shared(); + auto client_sessions = std::make_shared(rng); + auto server_sessions = std::make_shared(rng); + + run_handshake(result, rng, policy, creds, client_sessions, server_sessions, false); + + std::vector c2s; + std::vector s2c; + std::vector client_recv; + std::vector server_recv; + + auto server_callbacks = std::make_shared(result, s2c, server_recv); + auto client_callbacks = std::make_shared(result, c2s, client_recv); + + Botan::TLS::Server server(server_callbacks, server_sessions, creds, policy, rng, true); + Botan::TLS::Client client(client_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + deliver(result, "resumed client hello 1", c2s, server); + deliver(result, "resumed hello verify request", s2c, client); + deliver(result, "resumed client hello 2", c2s, server); + deliver(result, "resumed server handshake flight", s2c, client); + + result.test_is_true("client is active after resumed server flight", client.is_active()); + result.test_is_false("server is still waiting for client final flight", server.is_active()); + if(!result.test_is_true("client final flight was produced", !c2s.empty())) { + return result; + } + + c2s.clear(); // simulate losing the client's final flight + + wait_for_timeout_retransmit(result, client, c2s); + deliver(result, "retransmitted resumed client final flight", c2s, server); + + result.test_is_true("server became active from retransmitted client final flight", server.is_active()); + + const std::vector app_data = {0xB0, 0x7A, 0x11}; + result.test_no_throw("server sends application data after completing resumed handshake", + [&] { server.send(app_data); }); + deliver(result, "server application data", s2c, client); + result.test_bin_eq("client received server application data", client_recv, app_data); + + result.test_is_true("client stops retransmitting after peer application data", + remains_false_for([&] { return client.timeout_check(); }, std::chrono::milliseconds(20))); + + return result; + } + + static Test::Result test_reordered_retransmitted_final_flight() { + Test::Result result("DTLS reordered retransmitted final flight"); + + auto rng = Test::new_shared_rng("dtls-core-reordered-retransmitted-final"); + auto policy = std::make_shared(); + auto creds = std::make_shared(); + auto client_sessions = std::make_shared(rng); + auto server_sessions = std::make_shared(rng); + + std::vector c2s; + std::vector s2c; + std::vector client_recv; + std::vector server_recv; + + auto server_callbacks = std::make_shared(result, s2c, server_recv); + auto client_callbacks = std::make_shared(result, c2s, client_recv); + + Botan::TLS::Server server(server_callbacks, server_sessions, creds, policy, rng, true); + Botan::TLS::Client client(client_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + deliver(result, "client hello 1", c2s, server); + deliver(result, "hello verify request", s2c, client); + deliver(result, "client hello 2", c2s, server); + deliver(result, "server handshake flight", s2c, client); + deliver(result, "client final flight", c2s, server); + + result.test_is_true("server became active", server.is_active()); + result.test_is_false("client is waiting for server final flight", client.is_active()); + s2c.clear(); // simulate losing the server's last flight + + wait_for_timeout_retransmit(result, client, c2s); + + std::vector client_key_exchange; + std::vector ccs_and_finished; + if(!split_first_dtls_record(result, c2s, client_key_exchange, ccs_and_finished)) { + return result; + } + + std::vector ccs; + std::vector finished; + if(!split_first_dtls_record(result, ccs_and_finished, ccs, finished)) { + return result; + } + c2s.clear(); + + result.test_is_true( + "first retransmitted record is ClientKeyExchange", + contains_dtls_handshake_type(client_key_exchange, Botan::TLS::Handshake_Type::ClientKeyExchange)); + result.test_is_true("second retransmitted record is CCS", + contains_dtls_record_type(ccs, Botan::TLS::Record_Type::ChangeCipherSpec)); + result.test_is_true("third retransmitted record is Finished", + contains_dtls_record_type(finished, Botan::TLS::Record_Type::Handshake)); + + deliver_copy(result, "retransmitted ClientKeyExchange", client_key_exchange, server); + result.test_is_true("non-terminal retransmitted record produces no response", s2c.empty()); + + deliver_copy(result, "retransmitted Finished before CCS", finished, server); + result.test_is_true("partial retransmitted flight produces no response", s2c.empty()); + + deliver_copy(result, "retransmitted CCS after Finished", ccs, server); + result.test_is_true("complete reordered flight retransmits server flight", !s2c.empty()); + + return result; + } + + static Test::Result test_renegotiation(bool server_initiated) { + Test::Result result(server_initiated ? "DTLS server-initiated renegotiation" + : "DTLS client-initiated renegotiation"); + + auto rng = Test::new_shared_rng(server_initiated ? "dtls-core-server-renegotiation" + : "dtls-core-client-renegotiation"); + auto policy = std::make_shared(); + auto creds = std::make_shared(); + auto client_sessions = std::make_shared(rng); + auto server_sessions = std::make_shared(rng); + + std::vector c2s; + std::vector s2c; + std::vector client_recv; + std::vector server_recv; + + auto server_callbacks = std::make_shared(result, s2c, server_recv); + auto client_callbacks = std::make_shared(result, c2s, client_recv); + + Botan::TLS::Server server(server_callbacks, server_sessions, creds, policy, rng, true); + Botan::TLS::Client client(client_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + deliver(result, "client hello 1", c2s, server); + deliver(result, "hello verify request", s2c, client); + deliver(result, "client hello 2", c2s, server); + deliver(result, "server handshake flight", s2c, client); + deliver(result, "client final flight", c2s, server); + deliver(result, "server final flight", s2c, client); + + if(server_initiated) { + result.test_no_throw("server requests renegotiation", [&] { server.renegotiate(true); }); + deliver(result, "hello request", s2c, client); + } else { + result.test_no_throw("client requests renegotiation", [&] { client.renegotiate(true); }); + } + + deliver(result, "renegotiation client hello", c2s, server); + deliver(result, "renegotiation server handshake flight", s2c, client); + deliver(result, "renegotiation client final flight", c2s, server); + deliver(result, "renegotiation server final flight", s2c, client); + + result.test_is_true("client remains active after renegotiation", client.is_active()); + result.test_is_true("server remains active after renegotiation", server.is_active()); + + return result; + } + + static Test::Result test_empty_old_handshake_fragment_does_not_retransmit() { + Test::Result result("DTLS empty old handshake fragment does not retransmit"); + + auto rng = Test::new_shared_rng("dtls-core-empty-old-fragment"); + auto policy = std::make_shared(); + auto creds = std::make_shared(); + auto client_sessions = std::make_shared(rng); + auto server_sessions = std::make_shared(rng); + + std::vector c2s; + std::vector s2c; + std::vector client_recv; + std::vector server_recv; + + auto server_callbacks = std::make_shared(result, s2c, server_recv); + auto client_callbacks = std::make_shared(result, c2s, client_recv); + + Botan::TLS::Server server(server_callbacks, server_sessions, creds, policy, rng, true); + Botan::TLS::Client client(client_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + deliver(result, "client hello 1", c2s, server); + deliver(result, "hello verify request", s2c, client); + deliver(result, "client hello 2", c2s, server); + deliver(result, "server handshake flight", s2c, client); + deliver(result, "client final flight", c2s, server); + deliver(result, "server final flight", s2c, client); + + result.test_is_true("client became active", client.is_active()); + result.test_is_true("server became active", server.is_active()); + + const auto empty_fragment = empty_dtls_handshake_fragment(Botan::TLS::Handshake_Type::Finished, 12, 0); + + result.test_no_throw("empty old fragment is ignored", + [&] { server.received_data(empty_fragment.data(), empty_fragment.size()); }); + result.test_is_true("server did not retransmit final flight", s2c.empty()); + + return result; + } + + static Test::Result test_resumed_final_flight_and_app_data_in_one_receive() { + Test::Result result("DTLS resumed final flight and app data in one receive"); + + auto rng = Test::new_shared_rng("dtls-core-resumed-final-and-app-data"); + auto policy = std::make_shared(); + auto creds = std::make_shared(); + auto client_sessions = std::make_shared(rng); + auto server_sessions = std::make_shared(rng); + + run_handshake(result, rng, policy, creds, client_sessions, server_sessions, false); + + std::vector c2s; + std::vector s2c; + std::vector client_recv; + std::vector server_recv; + + auto server_callbacks = std::make_shared(result, s2c, server_recv); + auto client_callbacks = std::make_shared(result, c2s, client_recv); + + Botan::TLS::Server server(server_callbacks, server_sessions, creds, policy, rng, true); + Botan::TLS::Client client(client_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + deliver(result, "resumed client hello 1", c2s, server); + deliver(result, "resumed hello verify request", s2c, client); + deliver(result, "resumed client hello 2", c2s, server); + deliver(result, "resumed server handshake flight", s2c, client); + + result.test_is_true("client is active after resumed server flight", client.is_active()); + result.test_is_false("server is still waiting for client final flight", server.is_active()); + if(!result.test_is_true("client final flight was produced", !c2s.empty())) { + return result; + } + + const std::vector app_data = {0x47, 0x82, 0x01}; + result.test_no_throw("client sends application data before server processes final flight", + [&] { client.send(app_data); }); + + deliver(result, "resumed client final flight and application data", c2s, server); + + result.test_is_true("server became active", server.is_active()); + result.test_bin_eq("server received client application data", server_recv, app_data); + + return result; + } + + static Test::Result run_handshake(Test::Result& result, + const std::shared_ptr& rng, + const std::shared_ptr& policy, + const std::shared_ptr& creds, + const std::shared_ptr& client_sessions, + const std::shared_ptr& server_sessions, + bool expect_resumption) { + std::vector c2s; + std::vector s2c; + std::vector client_recv; + std::vector server_recv; + + auto server_callbacks = std::make_shared(result, s2c, server_recv); + auto client_callbacks = std::make_shared(result, c2s, client_recv); + + Botan::TLS::Server server(server_callbacks, server_sessions, creds, policy, rng, true); + Botan::TLS::Client client(client_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + deliver(result, "client hello 1", c2s, server); + deliver(result, "hello verify request", s2c, client); + deliver(result, "client hello 2", c2s, server); + deliver(result, "server handshake flight", s2c, client); + + // For a full handshake this is the client's final flight. The server's + // response is intentionally withheld to make the client retransmit it with + // fresh DTLS record sequence numbers, matching the scenario in #2498. + deliver(result, "client final flight", c2s, server); + result.test_is_true("server became active", server.is_active()); + + if(expect_resumption) { + result.test_is_true("client became active during resumed handshake", client.is_active()); + } else { + if(!result.test_is_true("server final flight is pending", !s2c.empty())) { + return result; + } + + wait_for_timeout_retransmit(result, client, c2s); + deliver(result, "client final flight retransmit", c2s, server); + + deliver(result, "server final flight", s2c, client); + } + + result.test_is_true("client became active", client.is_active()); + + if(client_callbacks->session_was_resumption().has_value()) { + result.test_bool_eq( + "client resumption state", client_callbacks->session_was_resumption().value(), expect_resumption); + } + if(server_callbacks->session_was_resumption().has_value()) { + result.test_bool_eq( + "server resumption state", server_callbacks->session_was_resumption().value(), expect_resumption); + } + + const std::vector app_data = {0xD7, 0x15, 0xA9}; + result.test_no_throw("client sends application data after retransmitted final flight", + [&] { client.send(app_data); }); + deliver(result, "application data", c2s, server); + result.test_bin_eq("server received application data", server_recv, app_data); + + return result; + } + + public: + std::vector run() override { + return {test_timeout_check_paces_retransmissions(), + test_retransmitted_epoch_transition_flight_includes_ccs(), + test_lost_hello_verify_request_retransmits(), + test_duplicate_hello_verify_request_is_tolerated(), + test_partial_server_flight_does_not_advance_client(), + test_lost_server_flight_retransmits(), + test_duplicate_server_flight_defers_to_timer(), + test_lost_server_final_flight_retransmits(), + test_stale_client_hello_does_not_replace_active_handshake(), + test_reordered_retransmitted_final_flight(), + test_empty_old_handshake_fragment_does_not_retransmit(), + test_retransmitted_final_flight_then_application_data(false), + test_retransmitted_final_flight_then_application_data(true), + test_resumed_client_final_flight_retransmits_after_activation(), + test_resumed_final_flight_and_app_data_in_one_receive(), + test_renegotiation(false), + test_renegotiation(true)}; + } +}; + +BOTAN_REGISTER_TEST("tls", "tls_dtls_core_regressions", DTLS_Core_Regression_Tests); + class DTLS_Reconnection_Test : public Test { public: std::vector run() override { From c5372108169629fb7089b699f83602aec65be4a0 Mon Sep 17 00:00:00 2001 From: Thiesius Date: Tue, 28 Jul 2026 20:55:14 +0200 Subject: [PATCH 2/4] Refine DTLS retransmission timing and association restart Keep retransmission timeout processing explicit and distinguish a restarted DTLS association from an ordinary retransmitted ClientHello. Update callers and regression tests for the clarified state transitions. --- doc/api_ref/tls.rst | 13 +-- src/lib/tls/tls12/tls_channel_impl_12.cpp | 77 ++++++++-------- src/lib/tls/tls12/tls_channel_impl_12.h | 10 ++- src/lib/tls/tls12/tls_connection_state_12.h | 7 ++ src/lib/tls/tls12/tls_handshake_io.cpp | 30 ++++--- src/lib/tls/tls12/tls_handshake_io.h | 14 ++- src/lib/tls/tls_channel.h | 7 +- src/lib/tls/tls_channel_impl.h | 6 +- src/lib/tls/tls_client.cpp | 4 +- src/lib/tls/tls_client.h | 2 +- src/lib/tls/tls_server.cpp | 4 +- src/lib/tls/tls_server.h | 2 +- src/tests/unit_tls.cpp | 98 ++++++++++++++++++++- 13 files changed, 202 insertions(+), 72 deletions(-) diff --git a/doc/api_ref/tls.rst b/doc/api_ref/tls.rst index 05c0aebb3bf..4f9c6eeb79e 100644 --- a/doc/api_ref/tls.rst +++ b/doc/api_ref/tls.rst @@ -257,12 +257,15 @@ available: this connection and the connection has not been subsequently closed. - .. cpp:function:: bool requires_timeout_check() + .. cpp:function:: std::optional next_retransmission_timeout() - Returns true if ``timeout_check`` should be polled to drive DTLS - handshake retransmissions. This may remain true briefly after - ``is_active`` becomes true, because a DTLS peer may not have - received the final handshake flight yet. + Returns the remaining time until a call to ``timeout_check`` might + retransmit DTLS handshake data. A zero duration means that a + retransmission is due. If ``std::nullopt`` is returned, no timeout check + is currently needed. + + This may return a duration briefly after ``is_active`` becomes true, + because a DTLS peer may not have received the final handshake flight yet. .. cpp:function:: bool is_closed() diff --git a/src/lib/tls/tls12/tls_channel_impl_12.cpp b/src/lib/tls/tls12/tls_channel_impl_12.cpp index 6ec10537586..d7e0d154d3c 100644 --- a/src/lib/tls/tls12/tls_channel_impl_12.cpp +++ b/src/lib/tls/tls12/tls_channel_impl_12.cpp @@ -21,20 +21,21 @@ #include #include #include +#include namespace Botan::TLS { namespace { -bool is_new_dtls_association_client_hello(std::span record, Record_Type record_type) { +bool is_new_dtls_association_client_hello(std::span msg_and_header, Record_Type record_type) { constexpr size_t DTLS_HANDSHAKE_HEADER_SIZE = 12; // Every new DTLS handshake starts at message_seq 0. A cookie-bearing // ClientHello uses message_seq 1, so one arriving late belongs to the // previous handshake and must not replace the active association's state. - return record_type == Record_Type::Handshake && record.size() >= DTLS_HANDSHAKE_HEADER_SIZE && - static_cast(record[0]) == Handshake_Type::ClientHello && - load_be(&record[4], 0) == 0; + return record_type == Record_Type::Handshake && msg_and_header.size() >= DTLS_HANDSHAKE_HEADER_SIZE && + static_cast(msg_and_header[0]) == Handshake_Type::ClientHello && + load_be(msg_and_header.subspan<4, 2>()) == 0; } } // namespace @@ -69,7 +70,6 @@ Channel_Impl_12::Channel_Impl_12(const std::shared_ptr& callbacks, void Channel_Impl_12::reset_state() { m_active_state.reset(); m_pending_state.reset(); - m_dtls_peer_traffic_seen = true; m_readbuf.clear(); m_write_cipher_states.clear(); m_read_cipher_states.clear(); @@ -79,7 +79,6 @@ void Channel_Impl_12::reset_active_association_state() { // This operation only makes sense for DTLS BOTAN_ASSERT_NOMSG(m_is_datagram); m_active_state.reset(); - m_dtls_peer_traffic_seen = true; m_read_cipher_states.clear(); m_write_cipher_states.clear(); @@ -187,15 +186,31 @@ Handshake_State& Channel_Impl_12::create_handshake_state(Protocol_Version versio return *m_pending_state; } -bool Channel_Impl_12::timeout_check() { +Handshake_IO* Channel_Impl_12::retransmission_io() { + return const_cast(std::as_const(*this).retransmission_io()); +} + +const Handshake_IO* Channel_Impl_12::retransmission_io() const { + if(!m_is_datagram || m_has_been_closed) { + return nullptr; + } + if(m_pending_state) { - return m_pending_state->handshake_io().timeout_check(); + return &m_pending_state->handshake_io(); } - if(m_is_datagram && !m_has_been_closed && m_active_state.has_value() && !m_dtls_peer_traffic_seen) { - if(auto* dtls_io = m_active_state->dtls_handshake_io()) { - return dtls_io->timeout_check(); - } + // Until protected application data confirms that the peer processed our + // final flight, keep it eligible for timeout-driven retransmission. + if(m_active_state.has_value() && !m_active_state->peer_sent_protected_application_data()) { + return m_active_state->dtls_handshake_io(); + } + + return nullptr; +} + +bool Channel_Impl_12::timeout_check() { + if(auto* io = retransmission_io()) { + return io->timeout_check(); } //FIXME: scan cipher suites and remove epochs older than 2*MSL @@ -282,16 +297,12 @@ bool Channel_Impl_12::is_active() const { return !is_closed() && is_handshake_complete(); } -bool Channel_Impl_12::requires_timeout_check() const { - if(m_pending_state) { - return true; +std::optional Channel_Impl_12::next_retransmission_timeout() const { + if(const auto* io = retransmission_io()) { + return io->next_retransmission_timeout(); } - // A DTLS peer might not have received our final handshake flight yet, even - // though this side is already active. Keep timer ticks enabled until peer - // traffic confirms that it has moved on to the negotiated epoch. - return m_is_datagram && !m_has_been_closed && m_active_state.has_value() && !m_dtls_peer_traffic_seen && - m_active_state->dtls_handshake_io() != nullptr; + return std::nullopt; } bool Channel_Impl_12::is_closed() const { @@ -317,14 +328,12 @@ void Channel_Impl_12::activate_session() { if(m_is_datagram) { m_active_state = Active_Connection_State_12(state, application_protocol(), m_pending_state->take_handshake_io()); if(auto* dtls_io = m_active_state->dtls_handshake_io()) { - // Mark the just-sent flight complete so timeout_check() may - // retransmit it if the peer never receives it. - dtls_io->conclude_flight(); + // Explicitly mark the just-sent final flight complete so timeout_check() + // may retransmit it if the peer never receives it. + dtls_io->finalize_handshake(); } - m_dtls_peer_traffic_seen = false; } else { m_active_state = Active_Connection_State_12(state, application_protocol()); - m_dtls_peer_traffic_seen = true; } m_pending_state.reset(); @@ -452,18 +461,18 @@ void Channel_Impl_12::process_handshake_ccs(const secure_vector& record // With no pending handshake this is either a new handshake attempt or a // DTLS retransmission from the previous handshake. The latter must not // create fresh pending state; it only asks us to replay our last flight. - if(record_version.is_datagram_protocol() && epoch0_restart && m_sequence_numbers && m_active_state.has_value()) { + if(epoch0_restart && m_sequence_numbers && m_active_state.has_value()) { const bool starts_new_handshake = is_new_dtls_association_client_hello(record, record_type); if(!starts_new_handshake) { - BOTAN_ASSERT(m_active_state->dtls_handshake_io(), "Have DTLS handshake IO for retransmission"); + BOTAN_ASSERT_NONNULL(m_active_state->dtls_handshake_io()); m_active_state->dtls_handshake_io()->add_retransmitted_record( record.data(), record.size(), record_type, record_sequence); return; } } - if(record_version.is_datagram_protocol() && !epoch0_restart) { + if(m_is_datagram && !epoch0_restart) { if(m_sequence_numbers) { sequence_numbers().read_accept(record_sequence); @@ -479,19 +488,17 @@ void Channel_Impl_12::process_handshake_ccs(const secure_vector& record static_cast(record[0]) == Handshake_Type::HelloRequest)); if(m_active_state.has_value() && !starts_new_handshake) { - BOTAN_ASSERT(m_active_state->dtls_handshake_io(), "Have DTLS handshake IO for retransmission"); + BOTAN_ASSERT_NONNULL(m_active_state->dtls_handshake_io()); m_active_state->dtls_handshake_io()->add_retransmitted_record( record.data(), record.size(), record_type, record_sequence); - return; } else { create_handshake_state(record_version); } } else if(current_epoch > 0 && epoch == current_epoch - 1) { - BOTAN_ASSERT(m_active_state.has_value() && m_active_state->dtls_handshake_io(), - "Have DTLS handshake IO for retransmission"); + BOTAN_ASSERT(m_active_state.has_value(), "Have active DTLS association for retransmission"); + BOTAN_ASSERT_NONNULL(m_active_state->dtls_handshake_io()); m_active_state->dtls_handshake_io()->add_retransmitted_record( record.data(), record.size(), record_type, record_sequence); - return; } } else { create_handshake_state(record_version); @@ -533,9 +540,7 @@ void Channel_Impl_12::process_application_data(uint64_t seq_no, const secure_vec throw Unexpected_Message("Application data received in unexpected read epoch"); } - if(m_is_datagram) { - m_dtls_peer_traffic_seen = true; - } + m_active_state->mark_peer_as_having_sent_protected_application_data(); callbacks().tls_record_received(seq_no, record); } diff --git a/src/lib/tls/tls12/tls_channel_impl_12.h b/src/lib/tls/tls12/tls_channel_impl_12.h index a1e1144aafd..2034814e747 100644 --- a/src/lib/tls/tls12/tls_channel_impl_12.h +++ b/src/lib/tls/tls12/tls_channel_impl_12.h @@ -27,6 +27,7 @@ namespace TLS { class Callbacks; class Connection_Cipher_State; class Connection_Sequence_Numbers; +class Handshake_IO; class Handshake_State; class Handshake_Message; class Client_Hello_12; @@ -87,7 +88,7 @@ class Channel_Impl_12 : public Channel_Impl { */ bool is_active() const override; - bool requires_timeout_check() const override; + std::optional next_retransmission_timeout() const override; /** * @return true iff the connection has been definitely closed @@ -158,7 +159,7 @@ class Channel_Impl_12 : public Channel_Impl { bool epoch0_restart) = 0; Handshake_State& create_handshake_state(Protocol_Version version); - virtual std::unique_ptr new_handshake_state(std::unique_ptr io) = 0; + virtual std::unique_ptr new_handshake_state(std::unique_ptr io) = 0; void inspect_handshake_message(const Handshake_Message& msg); @@ -200,6 +201,10 @@ class Channel_Impl_12 : public Channel_Impl { void reset_state(); + Handshake_IO* retransmission_io(); + + const Handshake_IO* retransmission_io() const; + Connection_Sequence_Numbers& sequence_numbers() const; std::shared_ptr read_cipher_state_epoch(uint16_t epoch) const; @@ -246,7 +251,6 @@ class Channel_Impl_12 : public Channel_Impl { secure_vector m_record_buf; bool m_has_been_closed; - bool m_dtls_peer_traffic_seen = true; std::optional m_active_state; }; diff --git a/src/lib/tls/tls12/tls_connection_state_12.h b/src/lib/tls/tls12/tls_connection_state_12.h index db377d61ac0..7395098e395 100644 --- a/src/lib/tls/tls12/tls_connection_state_12.h +++ b/src/lib/tls/tls12/tls_connection_state_12.h @@ -83,6 +83,12 @@ class Active_Connection_State_12 final { const Datagram_Handshake_IO* dtls_handshake_io() const { return m_dtls_handshake_io.get(); } + // Protected application data proves that the peer processed our final + // handshake flight, so timeout-driven replay is no longer necessary. + bool peer_sent_protected_application_data() const { return m_peer_sent_protected_application_data; } + + void mark_peer_as_having_sent_protected_application_data() { m_peer_sent_protected_application_data = true; } + private: Protocol_Version m_version; uint16_t m_ciphersuite_code = 0; @@ -100,6 +106,7 @@ class Active_Connection_State_12 final { std::vector m_server_finished_verify_data; bool m_supports_extended_master_secret = false; std::unique_ptr m_dtls_handshake_io; + bool m_peer_sent_protected_application_data = false; }; } // namespace Botan::TLS diff --git a/src/lib/tls/tls12/tls_handshake_io.cpp b/src/lib/tls/tls12/tls_handshake_io.cpp index 69c8da80b5a..414f5cbcf4b 100644 --- a/src/lib/tls/tls12/tls_handshake_io.cpp +++ b/src/lib/tls/tls12/tls_handshake_io.cpp @@ -188,7 +188,7 @@ bool Datagram_Handshake_IO::have_more_data() const { return next != m_messages.end() && next->second.complete(); } -void Datagram_Handshake_IO::conclude_flight() { +void Datagram_Handshake_IO::finalize_handshake() { // Keep an empty trailing flight to mean "we are waiting for the peer". // Retransmission then replays the previous, completed flight instead of // appending to it. @@ -198,17 +198,8 @@ void Datagram_Handshake_IO::conclude_flight() { } bool Datagram_Handshake_IO::timeout_check() { - if(m_last_write == 0 || (m_flights.size() > 1 && !m_flights.rbegin()->empty())) { - /* - If we haven't written anything yet obviously no timeout. - Also no timeout possible if we are mid-flight, - */ - return false; - } - - const uint64_t ms_since_write = steady_clock_ms() - m_last_write; - - if(ms_since_write < m_next_timeout) { + const auto timeout = next_retransmission_timeout(); + if(!timeout || timeout->count() > 0) { return false; } @@ -220,6 +211,21 @@ bool Datagram_Handshake_IO::timeout_check() { return true; } +std::optional Datagram_Handshake_IO::next_retransmission_timeout() const { + // Without an outgoing flight, or while constructing one, there is nothing + // complete that timeout_check() could retransmit. + if(m_last_write == 0 || (m_flights.size() > 1 && !m_flights.rbegin()->empty())) { + return std::nullopt; + } + + const uint64_t ms_since_write = steady_clock_ms() - m_last_write; + if(ms_since_write >= m_next_timeout) { + return std::chrono::milliseconds(0); + } + + return std::chrono::milliseconds(m_next_timeout - ms_since_write); +} + void Datagram_Handshake_IO::add_record(const uint8_t record[], size_t record_len, Record_Type record_type, diff --git a/src/lib/tls/tls12/tls_handshake_io.h b/src/lib/tls/tls12/tls_handshake_io.h index 6ef2565fa68..94180d84050 100644 --- a/src/lib/tls/tls12/tls_handshake_io.h +++ b/src/lib/tls/tls12/tls_handshake_io.h @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -51,6 +52,8 @@ class Handshake_IO { virtual bool timeout_check() = 0; + virtual std::optional next_retransmission_timeout() const = 0; + virtual bool have_more_data() const = 0; virtual std::vector format(const std::vector& handshake_msg, @@ -90,6 +93,8 @@ class Stream_Handshake_IO final : public Handshake_IO { bool timeout_check() override { return false; } + std::optional next_retransmission_timeout() const override { return std::nullopt; } + bool have_more_data() const override { return !m_queue.empty(); } std::vector send(const Handshake_Message& msg) override; @@ -134,6 +139,8 @@ class Datagram_Handshake_IO final : public Handshake_IO { bool timeout_check() override; + std::optional next_retransmission_timeout() const override; + bool have_more_data() const override; std::vector send(const Handshake_Message& msg) override; @@ -155,7 +162,12 @@ class Datagram_Handshake_IO final : public Handshake_IO { std::pair> get_next_record(bool expecting_ccs, size_t max_message_size) override; - void conclude_flight(); + /** + * Finalize the terminal outgoing handshake flight after channel + * activation. Intermediate flights are finalized implicitly when the + * peer's next handshake message is requested. + */ + void finalize_handshake(); private: void add_record(const uint8_t record[], diff --git a/src/lib/tls/tls_channel.h b/src/lib/tls/tls_channel.h index fc201030b85..5dd0278f67c 100644 --- a/src/lib/tls/tls_channel.h +++ b/src/lib/tls/tls_channel.h @@ -13,6 +13,7 @@ #include #include +#include #include #include #include @@ -122,10 +123,10 @@ class BOTAN_PUBLIC_API(2, 0) Channel { virtual bool is_active() const = 0; /** - * @return true iff timeout_check() should be polled to drive handshake - * retransmissions. + * @return the remaining time until timeout_check() might retransmit + * handshake data, or std::nullopt if no timeout check is needed. */ - virtual bool requires_timeout_check() const { return !is_handshake_complete(); } + virtual std::optional next_retransmission_timeout() const = 0; /** * Note: For TLS 1.3 a connection is closed only after both peers have diff --git a/src/lib/tls/tls_channel_impl.h b/src/lib/tls/tls_channel_impl.h index 9ad559fb520..6b4a133d971 100644 --- a/src/lib/tls/tls_channel_impl.h +++ b/src/lib/tls/tls_channel_impl.h @@ -85,11 +85,7 @@ class Channel_Impl { */ virtual bool is_active() const = 0; - /** - * @return true iff timeout_check() should be polled to drive handshake - * retransmissions. - */ - virtual bool requires_timeout_check() const { return !is_handshake_complete(); } + virtual std::optional next_retransmission_timeout() const { return std::nullopt; } /** * @return true iff the connection has been definitely closed diff --git a/src/lib/tls/tls_client.cpp b/src/lib/tls/tls_client.cpp index de8dcc40dd6..92080aaa8de 100644 --- a/src/lib/tls/tls_client.cpp +++ b/src/lib/tls/tls_client.cpp @@ -119,8 +119,8 @@ bool Client::is_active() const { return m_impl->is_active(); } -bool Client::requires_timeout_check() const { - return m_impl->requires_timeout_check(); +std::optional Client::next_retransmission_timeout() const { + return m_impl->next_retransmission_timeout(); } bool Client::is_closed() const { diff --git a/src/lib/tls/tls_client.h b/src/lib/tls/tls_client.h index 7b0dbbdb9b9..0f5b1aab19f 100644 --- a/src/lib/tls/tls_client.h +++ b/src/lib/tls/tls_client.h @@ -100,7 +100,7 @@ class BOTAN_PUBLIC_API(2, 0) Client final : public Channel { bool is_active() const override; - bool requires_timeout_check() const override; + std::optional next_retransmission_timeout() const override; bool is_closed() const override; diff --git a/src/lib/tls/tls_server.cpp b/src/lib/tls/tls_server.cpp index 1304a05ba2b..8f24c378b99 100644 --- a/src/lib/tls/tls_server.cpp +++ b/src/lib/tls/tls_server.cpp @@ -87,8 +87,8 @@ bool Server::is_active() const { return m_impl->is_active(); } -bool Server::requires_timeout_check() const { - return m_impl->requires_timeout_check(); +std::optional Server::next_retransmission_timeout() const { + return m_impl->next_retransmission_timeout(); } bool Server::is_closed() const { diff --git a/src/lib/tls/tls_server.h b/src/lib/tls/tls_server.h index 6e2c50bdb00..6c413178f79 100644 --- a/src/lib/tls/tls_server.h +++ b/src/lib/tls/tls_server.h @@ -90,7 +90,7 @@ class BOTAN_PUBLIC_API(2, 0) Server final : public Channel { bool is_active() const override; - bool requires_timeout_check() const override; + std::optional next_retransmission_timeout() const override; bool is_closed() const override; diff --git a/src/tests/unit_tls.cpp b/src/tests/unit_tls.cpp index a8388570e28..ba9f1ca7a6e 100644 --- a/src/tests/unit_tls.cpp +++ b/src/tests/unit_tls.cpp @@ -1525,6 +1525,7 @@ class Dtls_PSK_Credentials final : public Botan::Credentials_Manager { } if(type == "tls-server" && context == "dtls-cookie-secret") { + ++m_dtls_cookie_secret_requests; return Botan::SymmetricKey("4AEA5EAD279CADEB537A594DA0E9DE3A"); } @@ -1534,6 +1535,11 @@ class Dtls_PSK_Credentials final : public Botan::Credentials_Manager { throw Test_Error("No PSK set for " + type + "/" + context); } + + size_t dtls_cookie_secret_requests() const { return m_dtls_cookie_secret_requests; } + + private: + size_t m_dtls_cookie_secret_requests = 0; }; class Dtls_PSK_Policy final : public Botan::TLS::Policy { @@ -1769,7 +1775,10 @@ class DTLS_Core_Regression_Tests final : public Test { static void wait_for_timeout_retransmit(Test::Result& result, Botan::TLS::Channel& channel, std::vector& outbound) { - if(!wait_until([&] { return channel.timeout_check() && !outbound.empty(); })) { + if(!wait_until([&] { + const auto timeout = channel.next_retransmission_timeout(); + return timeout.has_value() && timeout->count() == 0 && channel.timeout_check() && !outbound.empty(); + })) { result.test_failure("DTLS retransmit was not produced"); } } @@ -1809,6 +1818,8 @@ class DTLS_Core_Regression_Tests final : public Test { wait_for_timeout_retransmit(result, client, c2s); const auto retransmit_size = c2s.size(); + result.test_is_true("next retransmission timeout remains available", + client.next_retransmission_timeout().has_value()); result.test_is_false("immediate second timeout is suppressed", client.timeout_check()); result.test_sz_eq("no immediate second retransmit", c2s.size(), retransmit_size); @@ -2254,6 +2265,86 @@ class DTLS_Core_Regression_Tests final : public Test { return result; } + static Test::Result test_epoch0_client_hello_retransmit_while_restart_pending() { + Test::Result result("DTLS epoch-zero ClientHello retransmit while restart pending"); + + auto rng = Test::new_shared_rng("dtls-core-pending-epoch0-restart"); + auto policy = std::make_shared(); + auto creds = std::make_shared(); + auto server_sessions = std::make_shared(rng); + auto client_sessions = std::make_shared(); + + std::vector s2c; + std::vector server_recv; + auto server_callbacks = std::make_shared(result, s2c, server_recv); + Botan::TLS::Server server(server_callbacks, server_sessions, creds, policy, rng, true); + + std::vector c1_c2s; + std::vector client1_recv; + auto client1_callbacks = std::make_shared(result, c1_c2s, client1_recv); + Botan::TLS::Client client1(client1_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + deliver(result, "client 1 hello", c1_c2s, server); + deliver(result, "client 1 hello verify request", s2c, client1); + deliver(result, "client 1 cookie-bearing hello", c1_c2s, server); + deliver(result, "client 1 server handshake flight", s2c, client1); + deliver(result, "client 1 final flight", c1_c2s, server); + deliver(result, "client 1 server final flight", s2c, client1); + + result.test_is_true("first client became active", client1.is_active()); + result.test_is_true("server became active for first client", server.is_active()); + result.test_sz_eq("server established one session", server_callbacks->sessions_established(), 1); + + std::vector c2_c2s; + std::vector client2_recv; + auto client2_callbacks = std::make_shared(result, c2_c2s, client2_recv); + Botan::TLS::Client client2(client2_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + const auto epoch0_client_hello = c2_c2s; + deliver(result, "client 2 epoch-zero hello", c2_c2s, server); + const auto hello_verify_request = s2c; + const auto cookie_secret_requests = creds->dtls_cookie_secret_requests(); + s2c.clear(); + + // Once the pending handshake consumes message_seq 0, receiving it + // again must replay HelloVerifyRequest rather than restart processing. + deliver_copy(result, "retransmitted client 2 epoch-zero hello", epoch0_client_hello, server); + result.test_is_true("pending restart replays HelloVerifyRequest", + contains_dtls_handshake_type(s2c, Botan::TLS::Handshake_Type::HelloVerifyRequest)); + result.test_sz_eq("retransmitted ClientHello does not establish another session", + server_callbacks->sessions_established(), + 1); + result.test_sz_eq("retransmitted ClientHello does not repeat cookie processing", + creds->dtls_cookie_secret_requests(), + cookie_secret_requests); + result.test_is_true("previous association remains active during restart", server.is_active()); + s2c.clear(); + + deliver_copy(result, "client 2 hello verify request", hello_verify_request, client2); + deliver(result, "client 2 cookie-bearing hello", c2_c2s, server); + deliver(result, "client 2 server handshake flight", s2c, client2); + deliver(result, "client 2 final flight", c2_c2s, server); + deliver(result, "client 2 server final flight", s2c, client2); + + result.test_is_true("second client became active", client2.is_active()); + result.test_is_true("server became active for second client", server.is_active()); + result.test_sz_eq("server established the replacement session", server_callbacks->sessions_established(), 2); + + return result; + } + static Test::Result test_retransmitted_final_flight_then_application_data(bool expect_resumption) { Test::Result result(expect_resumption ? "DTLS resumed handshake accepts app data" : "DTLS final flight retransmit before app data"); @@ -2323,6 +2414,8 @@ class DTLS_Core_Regression_Tests final : public Test { deliver(result, "server application data", s2c, client); result.test_bin_eq("client received server application data", client_recv, app_data); + result.test_is_false("client no longer needs timeout checks", + client.next_retransmission_timeout().has_value()); result.test_is_true("client stops retransmitting after peer application data", remains_false_for([&] { return client.timeout_check(); }, std::chrono::milliseconds(20))); @@ -2449,6 +2542,8 @@ class DTLS_Core_Regression_Tests final : public Test { result.test_is_true("client remains active after renegotiation", client.is_active()); result.test_is_true("server remains active after renegotiation", server.is_active()); + result.test_is_true("client tracks the new final flight", client.next_retransmission_timeout().has_value()); + result.test_is_true("server tracks the new final flight", server.next_retransmission_timeout().has_value()); return result; } @@ -2628,6 +2723,7 @@ class DTLS_Core_Regression_Tests final : public Test { test_duplicate_server_flight_defers_to_timer(), test_lost_server_final_flight_retransmits(), test_stale_client_hello_does_not_replace_active_handshake(), + test_epoch0_client_hello_retransmit_while_restart_pending(), test_reordered_retransmitted_final_flight(), test_empty_old_handshake_fragment_does_not_retransmit(), test_retransmitted_final_flight_then_application_data(false), From 87bb3a42b768d271f49c047d35da3e665fc76d55 Mon Sep 17 00:00:00 2001 From: Thiesius Date: Tue, 28 Jul 2026 20:55:25 +0200 Subject: [PATCH 3/4] Retain DTLS final-flight retransmission state after activation Continue retransmitting a retained final handshake flight after the local endpoint becomes active until authenticated peer traffic proves completion. Align FINISHED handling and clean up the retained-flight state and tests. --- doc/api_ref/tls.rst | 9 +- src/bogo_shim/config_no_tls13.json | 1 - src/lib/tls/tls12/tls_channel_impl_12.cpp | 67 ++++------- src/lib/tls/tls12/tls_channel_impl_12.h | 6 +- src/lib/tls/tls12/tls_connection_state_12.h | 16 +-- src/lib/tls/tls12/tls_handshake_io.cpp | 127 +++++++++++++------- src/lib/tls/tls12/tls_handshake_io.h | 20 ++- src/lib/tls/tls12/tls_seq_numbers.h | 58 ++++++--- src/lib/tls/tls_channel.h | 4 +- src/lib/tls/tls_channel_impl.h | 4 +- src/tests/unit_tls.cpp | 92 ++++++++------ 11 files changed, 222 insertions(+), 182 deletions(-) diff --git a/doc/api_ref/tls.rst b/doc/api_ref/tls.rst index 4f9c6eeb79e..21dab5e78d5 100644 --- a/doc/api_ref/tls.rst +++ b/doc/api_ref/tls.rst @@ -264,9 +264,6 @@ available: retransmission is due. If ``std::nullopt`` is returned, no timeout check is currently needed. - This may return a duration briefly after ``is_active`` becomes true, - because a DTLS peer may not have received the final handshake flight yet. - .. cpp:function:: bool is_closed() Returns true if and only if either a close notification or a @@ -287,10 +284,8 @@ available: .. cpp:function:: bool timeout_check() This function does nothing unless the channel represents a DTLS - connection and a handshake still needs retransmission handling. This - includes handshakes in progress and locally active sessions whose final - handshake flight may not have reached the peer yet. Returns true if a - timeout condition occurred and handshake packets were retransmitted. + connection with a handshake in progress. Returns true if a timeout + condition occurred and handshake packets were retransmitted. .. cpp:function:: void renegotiate(bool force_full_renegotiation = false) diff --git a/src/bogo_shim/config_no_tls13.json b/src/bogo_shim/config_no_tls13.json index caf2e7f9004..a280d1f2474 100644 --- a/src/bogo_shim/config_no_tls13.json +++ b/src/bogo_shim/config_no_tls13.json @@ -274,7 +274,6 @@ "KeyChangeWithBufferedMessages-DTLS": "No DTLS 1.3", "Renegotiate-DTLS-Server-Forbidden": "Botan tolerates DTLS renegotiation", "Renegotiate-DTLS-Client-Forbidden": "Botan tolerates DTLS renegotiation", - "DTLS12-SendExtraFinished-*": "Botan tolerates extra Finished messages in DTLS", "MixCompleteMessageWithFragments-DTLS-TLS12": "DTLS fragment handling difference", "RejectPSSKeyType-*": "Botan does not reject RSA-PSS key type", "CertificateCipherMismatch-PSS": "Botan does not reject PSS cipher mismatch", diff --git a/src/lib/tls/tls12/tls_channel_impl_12.cpp b/src/lib/tls/tls12/tls_channel_impl_12.cpp index d7e0d154d3c..4cb49744c8d 100644 --- a/src/lib/tls/tls12/tls_channel_impl_12.cpp +++ b/src/lib/tls/tls12/tls_channel_impl_12.cpp @@ -186,31 +186,9 @@ Handshake_State& Channel_Impl_12::create_handshake_state(Protocol_Version versio return *m_pending_state; } -Handshake_IO* Channel_Impl_12::retransmission_io() { - return const_cast(std::as_const(*this).retransmission_io()); -} - -const Handshake_IO* Channel_Impl_12::retransmission_io() const { - if(!m_is_datagram || m_has_been_closed) { - return nullptr; - } - - if(m_pending_state) { - return &m_pending_state->handshake_io(); - } - - // Until protected application data confirms that the peer processed our - // final flight, keep it eligible for timeout-driven retransmission. - if(m_active_state.has_value() && !m_active_state->peer_sent_protected_application_data()) { - return m_active_state->dtls_handshake_io(); - } - - return nullptr; -} - bool Channel_Impl_12::timeout_check() { - if(auto* io = retransmission_io()) { - return io->timeout_check(); + if(m_is_datagram && !m_has_been_closed && m_pending_state) { + return m_pending_state->handshake_io().timeout_check(); } //FIXME: scan cipher suites and remove epochs older than 2*MSL @@ -298,8 +276,8 @@ bool Channel_Impl_12::is_active() const { } std::optional Channel_Impl_12::next_retransmission_timeout() const { - if(const auto* io = retransmission_io()) { - return io->next_retransmission_timeout(); + if(m_is_datagram && !m_has_been_closed && m_pending_state) { + return m_pending_state->handshake_io().next_retransmission_timeout(); } return std::nullopt; @@ -324,13 +302,18 @@ void Channel_Impl_12::activate_session() { map_remove_if(not_current_epoch, m_read_cipher_states); } - // For DTLS, keep the handshake IO for last-flight retransmission. + // In a full handshake the server sends the terminal flight; in an + // abbreviated handshake the client does. Both endpoints retain handshake + // sequence state, but only the terminal sender replays its outgoing flight. + const bool sent_terminal_dtls_flight = m_is_datagram && (m_is_server == (state.server_hello_done() != nullptr)); + if(m_is_datagram) { m_active_state = Active_Connection_State_12(state, application_protocol(), m_pending_state->take_handshake_io()); if(auto* dtls_io = m_active_state->dtls_handshake_io()) { - // Explicitly mark the just-sent final flight complete so timeout_check() - // may retransmit it if the peer never receives it. - dtls_io->finalize_handshake(); + // Retain receive sequence state on both endpoints to distinguish a + // retransmission from an unexpected new handshake message. Only the + // terminal-flight sender responds by replaying its final flight. + dtls_io->finalize_handshake(sent_terminal_dtls_flight); } } else { m_active_state = Active_Connection_State_12(state, application_protocol()); @@ -387,7 +370,7 @@ size_t Channel_Impl_12::from_peer(std::span data) { throw TLS_Exception(Alert::RecordOverflow, "TLS plaintext record is larger than allowed maximum"); } - const bool epoch0_restart = m_is_datagram && record.epoch() == 0 && m_active_state.has_value(); + const bool epoch0_restart = allow_epoch0_restart && 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 && !m_active_state.has_value()); @@ -457,6 +440,13 @@ void Channel_Impl_12::process_handshake_ccs(const secure_vector& record Record_Type record_type, Protocol_Version record_version, bool epoch0_restart) { + const auto process_retransmitted_record = [&] { + BOTAN_ASSERT(m_active_state.has_value(), "Have active DTLS association for retransmission"); + BOTAN_ASSERT_NONNULL(m_active_state->dtls_handshake_io()); + m_active_state->dtls_handshake_io()->add_retransmitted_record( + record.data(), record.size(), record_type, record_sequence); + }; + if(!m_pending_state) { // With no pending handshake this is either a new handshake attempt or a // DTLS retransmission from the previous handshake. The latter must not @@ -465,9 +455,7 @@ void Channel_Impl_12::process_handshake_ccs(const secure_vector& record const bool starts_new_handshake = is_new_dtls_association_client_hello(record, record_type); if(!starts_new_handshake) { - BOTAN_ASSERT_NONNULL(m_active_state->dtls_handshake_io()); - m_active_state->dtls_handshake_io()->add_retransmitted_record( - record.data(), record.size(), record_type, record_sequence); + process_retransmitted_record(); return; } } @@ -488,17 +476,12 @@ void Channel_Impl_12::process_handshake_ccs(const secure_vector& record static_cast(record[0]) == Handshake_Type::HelloRequest)); if(m_active_state.has_value() && !starts_new_handshake) { - BOTAN_ASSERT_NONNULL(m_active_state->dtls_handshake_io()); - m_active_state->dtls_handshake_io()->add_retransmitted_record( - record.data(), record.size(), record_type, record_sequence); + process_retransmitted_record(); } else { create_handshake_state(record_version); } } else if(current_epoch > 0 && epoch == current_epoch - 1) { - BOTAN_ASSERT(m_active_state.has_value(), "Have active DTLS association for retransmission"); - BOTAN_ASSERT_NONNULL(m_active_state->dtls_handshake_io()); - m_active_state->dtls_handshake_io()->add_retransmitted_record( - record.data(), record.size(), record_type, record_sequence); + process_retransmitted_record(); } } else { create_handshake_state(record_version); @@ -540,8 +523,6 @@ void Channel_Impl_12::process_application_data(uint64_t seq_no, const secure_vec throw Unexpected_Message("Application data received in unexpected read epoch"); } - m_active_state->mark_peer_as_having_sent_protected_application_data(); - callbacks().tls_record_received(seq_no, record); } diff --git a/src/lib/tls/tls12/tls_channel_impl_12.h b/src/lib/tls/tls12/tls_channel_impl_12.h index 2034814e747..9c12b839db2 100644 --- a/src/lib/tls/tls12/tls_channel_impl_12.h +++ b/src/lib/tls/tls12/tls_channel_impl_12.h @@ -146,7 +146,7 @@ class Channel_Impl_12 : public Channel_Impl { /** * Perform a handshake timeout check. This does nothing unless this is a - * DTLS channel that still needs handshake retransmission handling. + * DTLS channel with a handshake in progress. */ bool timeout_check() override; @@ -201,10 +201,6 @@ class Channel_Impl_12 : public Channel_Impl { void reset_state(); - Handshake_IO* retransmission_io(); - - const Handshake_IO* retransmission_io() const; - Connection_Sequence_Numbers& sequence_numbers() const; std::shared_ptr read_cipher_state_epoch(uint16_t epoch) const; diff --git a/src/lib/tls/tls12/tls_connection_state_12.h b/src/lib/tls/tls12/tls_connection_state_12.h index 7395098e395..186e41fa10c 100644 --- a/src/lib/tls/tls12/tls_connection_state_12.h +++ b/src/lib/tls/tls12/tls_connection_state_12.h @@ -39,7 +39,8 @@ class Active_Connection_State_12 final { Active_Connection_State_12(const Handshake_State& state, std::string application_protocol); - // DTLS variant: takes the handshake IO for replay of final flight + // DTLS variant: retains handshake IO for retransmission validation and + // reactive replay of the terminal flight when appropriate. Active_Connection_State_12(const Handshake_State& state, std::string application_protocol, std::unique_ptr io); @@ -75,20 +76,14 @@ class Active_Connection_State_12 final { bool supports_extended_master_secret() const { return m_supports_extended_master_secret; } /** - * For DTLS: the handshake IO from the completed handshake, needed - * to retransmit the last flight when records arrive under the - * previous epoch. Null for stream TLS. + * For DTLS: the handshake IO from the completed handshake, needed to + * validate retransmissions and, for the terminal-flight sender, replay + * the final flight. Null for stream TLS. */ Datagram_Handshake_IO* dtls_handshake_io() { return m_dtls_handshake_io.get(); } const Datagram_Handshake_IO* dtls_handshake_io() const { return m_dtls_handshake_io.get(); } - // Protected application data proves that the peer processed our final - // handshake flight, so timeout-driven replay is no longer necessary. - bool peer_sent_protected_application_data() const { return m_peer_sent_protected_application_data; } - - void mark_peer_as_having_sent_protected_application_data() { m_peer_sent_protected_application_data = true; } - private: Protocol_Version m_version; uint16_t m_ciphersuite_code = 0; @@ -106,7 +101,6 @@ class Active_Connection_State_12 final { std::vector m_server_finished_verify_data; bool m_supports_extended_master_secret = false; std::unique_ptr m_dtls_handshake_io; - bool m_peer_sent_protected_application_data = false; }; } // namespace Botan::TLS diff --git a/src/lib/tls/tls12/tls_handshake_io.cpp b/src/lib/tls/tls12/tls_handshake_io.cpp index 414f5cbcf4b..e04f069d3f5 100644 --- a/src/lib/tls/tls12/tls_handshake_io.cpp +++ b/src/lib/tls/tls12/tls_handshake_io.cpp @@ -20,6 +20,8 @@ namespace Botan::TLS { namespace { +constexpr size_t DTLS_HANDSHAKE_HEADER_SIZE = 12; + inline size_t load_be24(const uint8_t q[3]) { return make_uint32(0, q[0], q[1], q[2]); } @@ -155,46 +157,59 @@ void Datagram_Handshake_IO::retransmit_last_flight() { // last completed flight is normally the one before it. const size_t flight_idx = (m_flights.size() == 1) ? 0 : (m_flights.size() - 2); retransmit_flight(flight_idx); + m_last_write = steady_clock_ms(); } void Datagram_Handshake_IO::retransmit_flight(size_t flight_idx) { - const std::vector& flight = m_flights.at(flight_idx); + const auto& flight = m_flights.at(flight_idx); + const auto& ccs_records = m_flight_ccs.at(flight_idx); + const std::vector ccs = {1}; BOTAN_ASSERT(!flight.empty(), "Nonempty flight to retransmit"); - const uint16_t first_epoch = m_flight_data[flight[0]].epoch; - uint16_t epoch = (first_epoch == 0) ? 0 : first_epoch - 1; - - for(auto msg_seq : flight) { - auto& msg = m_flight_data[msg_seq]; - - if(msg.epoch != epoch) { - // CCS is not a handshake message and therefore has no message_seq of - // its own. When replaying a cached flight that crosses into a new - // epoch, synthesize the CCS record that originally separated them. - const std::vector ccs(1, 1); - m_send_hs(epoch, Record_Type::ChangeCipherSpec, ccs); + size_t ccs_idx = 0; + for(size_t msg_idx = 0; msg_idx != flight.size(); ++msg_idx) { + while(ccs_idx != ccs_records.size() && ccs_records[ccs_idx].first == msg_idx) { + m_send_hs(ccs_records[ccs_idx].second, Record_Type::ChangeCipherSpec, ccs); + ++ccs_idx; } + const auto msg_seq = flight[msg_idx]; + const auto& msg = m_flight_data.at(msg_seq); send_message(msg_seq, msg.epoch, msg.msg_type, msg.msg_bits); - epoch = msg.epoch; + } + + while(ccs_idx != ccs_records.size() && ccs_records[ccs_idx].first == flight.size()) { + m_send_hs(ccs_records[ccs_idx].second, Record_Type::ChangeCipherSpec, ccs); + ++ccs_idx; } } bool Datagram_Handshake_IO::have_more_data() const { + if(m_retransmitted_client_hello.has_value() && m_retransmitted_client_hello->second.complete()) { + return true; + } + // Future or incomplete fragments remain buffered, but only a complete // next-in-sequence message is trailing handshake data. const auto next = m_messages.find(m_in_message_seq); return next != m_messages.end() && next->second.complete(); } -void Datagram_Handshake_IO::finalize_handshake() { +void Datagram_Handshake_IO::finalize_handshake(bool retransmit_terminal_flight) { // Keep an empty trailing flight to mean "we are waiting for the peer". // Retransmission then replays the previous, completed flight instead of // appending to it. if(!m_flights.rbegin()->empty()) { - m_flights.push_back(std::vector()); + m_flights.emplace_back(); + m_flight_ccs.emplace_back(); } + + // RFC 6347 4.2.4 transitions directly to FINISHED after sending the + // terminal flight. Keep the flight for reactive replay when the peer + // retransmits, but do not arm a proactive retransmission timer. + m_finished = true; + m_retransmit_terminal_flight = retransmit_terminal_flight; } bool Datagram_Handshake_IO::timeout_check() { @@ -205,13 +220,15 @@ bool Datagram_Handshake_IO::timeout_check() { retransmit_last_flight(); - // Retransmission restarts the backoff window just like a normal write. - m_last_write = steady_clock_ms(); m_next_timeout = std::min(2 * m_next_timeout, m_max_timeout); return true; } std::optional Datagram_Handshake_IO::next_retransmission_timeout() const { + if(m_finished) { + return std::nullopt; + } + // Without an outgoing flight, or while constructing one, there is nothing // complete that timeout_check() could retransmit. if(m_last_write == 0 || (m_flights.size() > 1 && !m_flights.rbegin()->empty())) { @@ -279,9 +296,20 @@ bool Datagram_Handshake_IO::process_previous_handshake_fragment(const uint8_t fr return false; } - // ClientHello is special: losing HelloVerifyRequest leaves the server in - // the pending handshake, where it must still replay the cookie. + // HelloVerifyRequest has no retransmission timer or cached flight. Feed a + // retransmitted initial ClientHello back to the server handshake logic so + // it can recreate the stateless cookie response instead. if(msg_type == Handshake_Type::ClientHello) { + if(!retransmitted_flight && m_awaiting_cookie_client_hello) { + if(!m_retransmitted_client_hello.has_value() || m_retransmitted_client_hello->first != message_seq) { + m_retransmitted_client_hello = std::make_pair(message_seq, Handshake_Reassembly()); + } + + m_retransmitted_client_hello->second.add_fragment( + fragment, fragment_length, fragment_offset, epoch, msg_type, msg_length); + return false; + } + return reassemble_retransmitted_fragment( fragment, fragment_length, fragment_offset, epoch, msg_type, msg_length, message_seq); } @@ -349,7 +377,9 @@ void Datagram_Handshake_IO::add_record(const uint8_t record[], const uint16_t finished_epoch = static_cast(epoch + 1); if(m_retransmitted_finished_epoch == finished_epoch) { m_retransmitted_finished_epoch.reset(); - retransmit_last_flight(); + if(m_retransmit_terminal_flight) { + retransmit_last_flight(); + } } else { m_retransmitted_ccs_epoch = finished_epoch; } @@ -357,12 +387,10 @@ void Datagram_Handshake_IO::add_record(const uint8_t record[], return; } - const size_t DTLS_HANDSHAKE_HEADER_LEN = 12; - bool retransmit_response = false; while(record_len > 0) { - if(record_len < DTLS_HANDSHAKE_HEADER_LEN) { + if(record_len < DTLS_HANDSHAKE_HEADER_SIZE) { return; // completely bogus? at least degenerate/weird } @@ -382,7 +410,7 @@ void Datagram_Handshake_IO::add_record(const uint8_t record[], const size_t fragment_offset = load_be24(&record[6]); const size_t fragment_length = load_be24(&record[9]); - const size_t total_size = DTLS_HANDSHAKE_HEADER_LEN + fragment_length; + const size_t total_size = DTLS_HANDSHAKE_HEADER_SIZE + fragment_length; if(record_len < total_size) { throw Decoding_Error("Bad lengths in DTLS header"); @@ -416,9 +444,9 @@ void Datagram_Handshake_IO::add_record(const uint8_t record[], m_pending_reassembly_bytes += msg_len; } it->second.add_fragment( - &record[DTLS_HANDSHAKE_HEADER_LEN], fragment_length, fragment_offset, epoch, msg_type, msg_len); + &record[DTLS_HANDSHAKE_HEADER_SIZE], fragment_length, fragment_offset, epoch, msg_type, msg_len); } else { - retransmit_response |= process_previous_handshake_fragment(&record[DTLS_HANDSHAKE_HEADER_LEN], + retransmit_response |= process_previous_handshake_fragment(&record[DTLS_HANDSHAKE_HEADER_SIZE], fragment_length, fragment_offset, epoch, @@ -432,7 +460,7 @@ void Datagram_Handshake_IO::add_record(const uint8_t record[], record_len -= total_size; } - if(retransmit_response) { + if(retransmit_response && (!m_finished || m_retransmit_terminal_flight)) { retransmit_last_flight(); } } @@ -441,7 +469,8 @@ std::pair> Datagram_Handshake_IO::get_next_ size_t /*max_message_size*/) { // Expecting a message means the last flight is concluded if(!m_flights.rbegin()->empty()) { - m_flights.push_back(std::vector()); + m_flights.emplace_back(); + m_flight_ccs.emplace_back(); } if(expecting_ccs) { @@ -455,6 +484,13 @@ std::pair> Datagram_Handshake_IO::get_next_ return std::make_pair(Handshake_Type::None, std::vector()); } + if(m_retransmitted_client_hello.has_value() && m_retransmitted_client_hello->second.complete()) { + auto result = m_retransmitted_client_hello->second.message(); + m_retransmitted_client_hello.reset(); + m_recreating_hello_verify_request = true; + return result; + } + auto i = m_messages.find(m_in_message_seq); if(i == m_messages.end() || !i->second.complete()) { @@ -465,6 +501,10 @@ std::pair> Datagram_Handshake_IO::get_next_ auto result = i->second.message(); + if(result.first == Handshake_Type::ClientHello) { + m_awaiting_cookie_client_hello = false; + } + // Free the reassembly buffer for this delivered slot and uncommit its // bytes against the cap. The entry itself stays in m_messages because // the expecting_ccs branch above uses m_messages.begin()->second.epoch() @@ -588,25 +628,22 @@ std::vector Datagram_Handshake_IO::send_under_epoch(const Handshake_Mes const Handshake_Type msg_type = msg.type(); if(msg_type == Handshake_Type::HandshakeCCS) { + m_flight_ccs.rbegin()->emplace_back(m_flights.rbegin()->size(), epoch); m_send_hs(epoch, Record_Type::ChangeCipherSpec, msg_bits); - return std::vector(); // not included in handshake hashes + return {}; // not included in handshake hashes } else if(msg_type == Handshake_Type::HelloVerifyRequest) { - // This message is not included in the handshake hashes, but it is a - // DTLS flight and must be available for retransmission if it is lost. - m_flights.rbegin()->push_back(m_out_message_seq); - m_flight_data[m_out_message_seq] = Message_Info(epoch, msg_type, msg_bits); - - m_out_message_seq += 1; - m_last_write = steady_clock_ms(); - m_next_timeout = m_initial_timeout; - - send_message(m_out_message_seq - 1, epoch, msg_type, msg_bits); - return std::vector(); + // RFC 6347 3.2.1 explicitly excludes HelloVerifyRequest from timeout + // retransmission. A repeated ClientHello recreates the response using + // the original message sequence number without retaining a flight. + const uint16_t msg_seq = m_recreating_hello_verify_request ? m_out_message_seq - 1 : m_out_message_seq++; + m_awaiting_cookie_client_hello = true; + m_recreating_hello_verify_request = false; + send_message(msg_seq, epoch, msg_type, msg_bits); + return {}; } - // Note: not saving CCS, instead we know it was there due to change in epoch m_flights.rbegin()->push_back(m_out_message_seq); - m_flight_data[m_out_message_seq] = Message_Info(epoch, msg_type, msg_bits); + m_flight_data.emplace(m_out_message_seq, Message_Info(epoch, msg_type, msg_bits)); m_out_message_seq += 1; m_last_write = steady_clock_ms(); @@ -619,8 +656,6 @@ std::vector Datagram_Handshake_IO::send_message(uint16_t msg_seq, uint16_t epoch, Handshake_Type msg_type, const std::vector& msg_bits) { - const size_t DTLS_HANDSHAKE_HEADER_LEN = 12; - auto no_fragment = format_w_seq(msg_bits, msg_type, msg_seq); if(no_fragment.size() + DTLS_HEADER_SIZE <= m_mtu) { @@ -636,7 +671,7 @@ std::vector Datagram_Handshake_IO::send_message(uint16_t msg_seq, * per-record nonce. */ const size_t ciphersuite_overhead = (epoch > 0) ? 128 : 0; - const size_t header_overhead = DTLS_HEADER_SIZE + DTLS_HANDSHAKE_HEADER_LEN; + const size_t header_overhead = DTLS_HEADER_SIZE + DTLS_HANDSHAKE_HEADER_SIZE; if(m_mtu <= (header_overhead + ciphersuite_overhead)) { throw Invalid_Argument("DTLS MTU is too small to send headers"); diff --git a/src/lib/tls/tls12/tls_handshake_io.h b/src/lib/tls/tls12/tls_handshake_io.h index 94180d84050..3905a337484 100644 --- a/src/lib/tls/tls12/tls_handshake_io.h +++ b/src/lib/tls/tls12/tls_handshake_io.h @@ -129,6 +129,7 @@ class Datagram_Handshake_IO final : public Handshake_IO { size_t max_handshake_msg_size) : m_seqs(seq), m_flights(1), + m_flight_ccs(1), m_initial_timeout(initial_timeout_ms), m_max_timeout(max_timeout_ms), m_send_hs(std::move(writer)), @@ -163,11 +164,11 @@ class Datagram_Handshake_IO final : public Handshake_IO { size_t max_message_size) override; /** - * Finalize the terminal outgoing handshake flight after channel - * activation. Intermediate flights are finalized implicitly when the - * peer's next handshake message is requested. + * Enter FINISHED after channel activation. Intermediate outgoing flights + * are finalized implicitly when the peer's next handshake message is + * requested. The terminal-flight sender retains reactive replay behavior. */ - void finalize_handshake(); + void finalize_handshake(bool retransmit_terminal_flight); private: void add_record(const uint8_t record[], @@ -249,8 +250,6 @@ class Datagram_Handshake_IO final : public Handshake_IO { Message_Info(uint16_t e, Handshake_Type mt, const std::vector& msg) : epoch(e), msg_type(mt), msg_bits(msg) {} - Message_Info() : epoch(0xFFFF), msg_type(Handshake_Type::None) {} - uint16_t epoch; // NOLINT(*non-private-member-variable*) Handshake_Type msg_type; // NOLINT(*non-private-member-variable*) std::vector msg_bits; // NOLINT(*non-private-member-variable*) @@ -269,8 +268,17 @@ class Datagram_Handshake_IO final : public Handshake_IO { bool m_retransmitted_server_hello_complete = false; bool m_retransmitted_server_hello_done_complete = false; std::vector> m_flights; + // Each entry records where in the corresponding flight a CCS was sent + // and the epoch under which it was transmitted. + std::vector>> m_flight_ccs; std::map m_flight_data; + std::optional> m_retransmitted_client_hello; + bool m_awaiting_cookie_client_hello = false; + bool m_recreating_hello_verify_request = false; + bool m_finished = false; + bool m_retransmit_terminal_flight = false; + uint64_t m_initial_timeout = 0; uint64_t m_max_timeout = 0; diff --git a/src/lib/tls/tls12/tls_seq_numbers.h b/src/lib/tls/tls12/tls_seq_numbers.h index ee767f7da84..96911817f62 100644 --- a/src/lib/tls/tls12/tls_seq_numbers.h +++ b/src/lib/tls/tls12/tls_seq_numbers.h @@ -93,11 +93,14 @@ class Datagram_Sequence_Numbers final : public Connection_Sequence_Numbers { m_write_seqs[0] = 0; m_write_epoch = 0; m_read_epoch = 0; - m_window_highest = 0; - m_window_bits = 0; + m_read_windows.clear(); + m_read_windows[0] = Replay_Window{}; } - void new_read_cipher_state() override { m_read_epoch++; } + void new_read_cipher_state() override { + m_read_epoch++; + m_read_windows.try_emplace(m_read_epoch); + } void new_write_cipher_state() override { m_write_epoch++; @@ -120,56 +123,71 @@ class Datagram_Sequence_Numbers final : public Connection_Sequence_Numbers { uint64_t next_read_sequence() override { throw Invalid_State("DTLS uses explicit sequence numbers"); } bool already_seen(uint64_t sequence) const override { - const size_t window_size = sizeof(m_window_bits) * 8; + const uint16_t epoch = static_cast(sequence >> 48); + const uint64_t record_sequence = sequence & 0x0000FFFFFFFFFFFF; + const auto window = m_read_windows.find(epoch); + + if(window == m_read_windows.end()) { + return false; + } - if(sequence > m_window_highest) { + const size_t window_size = sizeof(window->second.bits) * 8; + + if(record_sequence > window->second.highest) { return false; } - const uint64_t offset = m_window_highest - sequence; + const uint64_t offset = window->second.highest - record_sequence; if(offset >= window_size) { return true; // really old? } - return (((m_window_bits >> offset) & 1) == 1); + return (((window->second.bits >> offset) & 1) == 1); } void read_accept(uint64_t sequence) override { - const size_t window_size = sizeof(m_window_bits) * 8; + const uint16_t epoch = static_cast(sequence >> 48); + const uint64_t record_sequence = sequence & 0x0000FFFFFFFFFFFF; + auto& window = m_read_windows[epoch]; + const size_t window_size = sizeof(window.bits) * 8; - if(sequence > m_window_highest) { + if(record_sequence > window.highest) { // We've received a later sequence which advances our window - const uint64_t offset = sequence - m_window_highest; - m_window_highest += offset; + const uint64_t offset = record_sequence - window.highest; + window.highest += offset; if(offset >= window_size) { - m_window_bits = 0; + window.bits = 0; } else { - m_window_bits <<= offset; + window.bits <<= offset; } - m_window_bits |= 0x01; + window.bits |= 0x01; } else { - const uint64_t offset = m_window_highest - sequence; + const uint64_t offset = window.highest - record_sequence; if(offset < window_size) { // We've received an old sequence but still within our window - m_window_bits |= (static_cast(1) << offset); + window.bits |= (static_cast(1) << offset); } else { // This occurs only if we have reset state (DTLS reconnection case) - m_window_highest = sequence; - m_window_bits = 0; + window.highest = record_sequence; + window.bits = 0; } } } private: + struct Replay_Window final { + uint64_t highest = 0; + uint64_t bits = 0; + }; + std::map m_write_seqs; + std::map m_read_windows; uint16_t m_write_epoch = 0; uint16_t m_read_epoch = 0; - uint64_t m_window_highest = 0; - uint64_t m_window_bits = 0; }; } // namespace Botan::TLS diff --git a/src/lib/tls/tls_channel.h b/src/lib/tls/tls_channel.h index 5dd0278f67c..4dc949737d0 100644 --- a/src/lib/tls/tls_channel.h +++ b/src/lib/tls/tls_channel.h @@ -203,9 +203,7 @@ class BOTAN_PUBLIC_API(2, 0) Channel { * Perform a handshake timeout check. * * This function does nothing unless the channel represents a DTLS - * connection and a handshake still needs retransmission handling. This can - * include a locally active DTLS session whose final flight may not have - * reached the peer yet. + * connection with a handshake in progress. * * @returns true if a timeout condition occurred */ diff --git a/src/lib/tls/tls_channel_impl.h b/src/lib/tls/tls_channel_impl.h index 6b4a133d971..b119c3a3121 100644 --- a/src/lib/tls/tls_channel_impl.h +++ b/src/lib/tls/tls_channel_impl.h @@ -170,9 +170,7 @@ class Channel_Impl { /** * Perform a handshake timeout check. This does nothing unless this is a - * DTLS channel that still needs handshake retransmission handling. A DTLS - * channel can require this briefly after local activation while its last - * flight may still be lost in transit. + * DTLS channel with a handshake in progress. */ virtual bool timeout_check() = 0; diff --git a/src/tests/unit_tls.cpp b/src/tests/unit_tls.cpp index ba9f1ca7a6e..001f4bdd9fa 100644 --- a/src/tests/unit_tls.cpp +++ b/src/tests/unit_tls.cpp @@ -34,7 +34,6 @@ #include #include #include - #include #include #include @@ -1752,26 +1751,6 @@ class DTLS_Core_Regression_Tests final : public Test { return predicate(); } - template - static bool remains_false_for(Predicate predicate, std::chrono::milliseconds duration) { - // Negative timeout checks need to observe a small window: a single - // immediate false could pass even though the channel retransmits a few - // milliseconds later. - const auto deadline = std::chrono::steady_clock::now() + duration; - - while(std::chrono::steady_clock::now() < deadline) { - if(predicate()) { - return false; - } - - #if defined(BOTAN_TARGET_OS_HAS_THREADS) - std::this_thread::sleep_for(std::chrono::milliseconds(1)); - #endif - } - - return !predicate(); - } - static void wait_for_timeout_retransmit(Test::Result& result, Botan::TLS::Channel& channel, std::vector& outbound) { @@ -1864,7 +1843,8 @@ class DTLS_Core_Regression_Tests final : public Test { } s2c.clear(); // simulate losing the server final flight - wait_for_timeout_retransmit(result, server, s2c); + wait_for_timeout_retransmit(result, client, c2s); + deliver(result, "retransmitted client final flight", c2s, server); result.test_is_true("retransmitted final flight includes CCS", contains_dtls_record_type(s2c, Botan::TLS::Record_Type::ChangeCipherSpec)); @@ -1909,6 +1889,8 @@ class DTLS_Core_Regression_Tests final : public Test { } result.test_is_true("server response is HelloVerifyRequest", contains_dtls_handshake_type(s2c, Botan::TLS::Handshake_Type::HelloVerifyRequest)); + result.test_is_false("server does not arm a HelloVerifyRequest retransmission timer", + server.next_retransmission_timeout().has_value()); s2c.clear(); // simulate losing the HelloVerifyRequest wait_for_timeout_retransmit(result, client, c2s); @@ -2203,12 +2185,30 @@ class DTLS_Core_Regression_Tests final : public Test { if(!result.test_is_true("server final flight was produced", !s2c.empty())) { return result; } - s2c.clear(); // simulate losing the server final flight - wait_for_timeout_retransmit(result, server, s2c); + std::vector delayed_ccs; + std::vector delayed_finished; + if(!split_first_dtls_record(result, s2c, delayed_ccs, delayed_finished)) { + return result; + } + s2c.clear(); // simulate delaying the original server final flight + + result.test_is_false("finished server has no proactive retransmission timer", + server.next_retransmission_timeout().has_value()); + wait_for_timeout_retransmit(result, client, c2s); + deliver(result, "retransmitted client final flight", c2s, server); deliver(result, "retransmitted server final flight", s2c, client); result.test_is_true("client became active", client.is_active()); + result.test_is_false("active client has no retransmission timer", + client.next_retransmission_timeout().has_value()); + + // The original Finished may arrive after the retransmitted flight + // activated the client. It has an unseen DTLS record sequence number + // but an old handshake message_seq and must be discarded silently. + deliver(result, "delayed original server Finished", delayed_finished, client); + result.test_is_true("client remains active after delayed Finished", client.is_active()); + result.test_is_true("client does not respond to delayed Finished", c2s.empty()); return result; } @@ -2318,17 +2318,17 @@ class DTLS_Core_Regression_Tests final : public Test { const auto cookie_secret_requests = creds->dtls_cookie_secret_requests(); s2c.clear(); - // Once the pending handshake consumes message_seq 0, receiving it - // again must replay HelloVerifyRequest rather than restart processing. + // HelloVerifyRequest is not retained as retransmittable flight data. + // A repeated initial ClientHello recreates the cookie response. deliver_copy(result, "retransmitted client 2 epoch-zero hello", epoch0_client_hello, server); result.test_is_true("pending restart replays HelloVerifyRequest", contains_dtls_handshake_type(s2c, Botan::TLS::Handshake_Type::HelloVerifyRequest)); result.test_sz_eq("retransmitted ClientHello does not establish another session", server_callbacks->sessions_established(), 1); - result.test_sz_eq("retransmitted ClientHello does not repeat cookie processing", + result.test_sz_eq("retransmitted ClientHello recreates the cookie response", creds->dtls_cookie_secret_requests(), - cookie_secret_requests); + cookie_secret_requests + 1); result.test_is_true("previous association remains active during restart", server.is_active()); s2c.clear(); @@ -2401,24 +2401,33 @@ class DTLS_Core_Regression_Tests final : public Test { return result; } - c2s.clear(); // simulate losing the client's final flight + std::vector discarded_ccs; + std::vector delayed_finished; + if(!split_first_dtls_record(result, c2s, discarded_ccs, delayed_finished)) { + return result; + } + c2s.clear(); // simulate delaying the original client final flight - wait_for_timeout_retransmit(result, client, c2s); + result.test_is_false("finished client has no proactive retransmission timer", + client.next_retransmission_timeout().has_value()); + wait_for_timeout_retransmit(result, server, s2c); + deliver(result, "retransmitted resumed server flight", s2c, client); deliver(result, "retransmitted resumed client final flight", c2s, server); result.test_is_true("server became active from retransmitted client final flight", server.is_active()); + // As in the full handshake, a delayed copy of the original terminal + // Finished must not turn normal DTLS reordering into a fatal alert. + deliver(result, "delayed original client Finished", delayed_finished, server); + result.test_is_true("server remains active after delayed Finished", server.is_active()); + result.test_is_true("server does not respond to delayed Finished", s2c.empty()); + const std::vector app_data = {0xB0, 0x7A, 0x11}; result.test_no_throw("server sends application data after completing resumed handshake", [&] { server.send(app_data); }); deliver(result, "server application data", s2c, client); result.test_bin_eq("client received server application data", client_recv, app_data); - result.test_is_false("client no longer needs timeout checks", - client.next_retransmission_timeout().has_value()); - result.test_is_true("client stops retransmitting after peer application data", - remains_false_for([&] { return client.timeout_check(); }, std::chrono::milliseconds(20))); - return result; } @@ -2535,6 +2544,13 @@ class DTLS_Core_Regression_Tests final : public Test { result.test_no_throw("client requests renegotiation", [&] { client.renegotiate(true); }); } + // A renegotiation ClientHello is already protected under the active + // epoch. Replaying it must not infer a preceding CCS merely because + // the cached handshake message has a non-zero epoch. + c2s.clear(); // simulate losing the renegotiation ClientHello + wait_for_timeout_retransmit(result, client, c2s); + result.test_is_false("retransmitted renegotiation ClientHello does not include CCS", + contains_dtls_record_type(c2s, Botan::TLS::Record_Type::ChangeCipherSpec)); deliver(result, "renegotiation client hello", c2s, server); deliver(result, "renegotiation server handshake flight", s2c, client); deliver(result, "renegotiation client final flight", c2s, server); @@ -2542,8 +2558,10 @@ class DTLS_Core_Regression_Tests final : public Test { result.test_is_true("client remains active after renegotiation", client.is_active()); result.test_is_true("server remains active after renegotiation", server.is_active()); - result.test_is_true("client tracks the new final flight", client.next_retransmission_timeout().has_value()); - result.test_is_true("server tracks the new final flight", server.next_retransmission_timeout().has_value()); + result.test_is_false("finished client has no proactive retransmission timer", + client.next_retransmission_timeout().has_value()); + result.test_is_false("finished server has no proactive retransmission timer", + server.next_retransmission_timeout().has_value()); return result; } From 463482d94975ad97c27b81f7068b9c5112befd3a Mon Sep 17 00:00:00 2001 From: Thiesius Date: Tue, 28 Jul 2026 00:09:05 +0200 Subject: [PATCH 4/4] Ignore unauthenticated DTLS epoch zero records Do not let malformed or spoofed epoch-zero traffic tear down an established DTLS association or advance its replay state. Continue accepting the handshake records needed for retransmission and explicitly permitted association restarts. --- src/lib/tls/tls12/tls_channel_impl_12.cpp | 31 ++++++-- src/lib/tls/tls12/tls_record.cpp | 2 +- src/tests/unit_tls.cpp | 88 ++++++++++++++++++++++- 3 files changed, 113 insertions(+), 8 deletions(-) diff --git a/src/lib/tls/tls12/tls_channel_impl_12.cpp b/src/lib/tls/tls12/tls_channel_impl_12.cpp index 4cb49744c8d..205475593ff 100644 --- a/src/lib/tls/tls12/tls_channel_impl_12.cpp +++ b/src/lib/tls/tls12/tls_channel_impl_12.cpp @@ -366,7 +366,22 @@ size_t Channel_Impl_12::from_peer(std::span data) { return 0; } + const bool old_unprotected_record = m_is_datagram && record.epoch() == 0 && m_active_state.has_value() && + sequence_numbers().current_read_epoch() > 0; + + // Once encrypted traffic is expected, epoch-zero records are + // unauthenticated. Only handshake records can be useful as part of a + // retransmitted flight or an explicitly allowed association restart. + if(old_unprotected_record && record.type() != Record_Type::Handshake && + record.type() != Record_Type::ChangeCipherSpec) { + continue; + } + if(m_record_buf.size() > MAX_PLAINTEXT_SIZE) { + if(old_unprotected_record) { + continue; + } + throw TLS_Exception(Alert::RecordOverflow, "TLS plaintext record is larger than allowed maximum"); } @@ -380,7 +395,7 @@ size_t Channel_Impl_12::from_peer(std::span data) { initial_handshake_message = (type == Handshake_Type::ClientHello); } - if(record.type() != Record_Type::Alert) { + if(record.type() != Record_Type::Alert && !old_unprotected_record) { if(initial_record) { // For initial records just check for basic sanity if(record.version().major_version() != 3 && record.version().major_version() != 0xFE) { @@ -443,8 +458,16 @@ void Channel_Impl_12::process_handshake_ccs(const secure_vector& record const auto process_retransmitted_record = [&] { BOTAN_ASSERT(m_active_state.has_value(), "Have active DTLS association for retransmission"); BOTAN_ASSERT_NONNULL(m_active_state->dtls_handshake_io()); - m_active_state->dtls_handshake_io()->add_retransmitted_record( - record.data(), record.size(), record_type, record_sequence); + try { + m_active_state->dtls_handshake_io()->add_retransmitted_record( + record.data(), record.size(), record_type, record_sequence); + } catch(...) { + // Epoch-zero records are unauthenticated and may be spoofed. Invalid + // retransmissions must not tear down an established association. + if((record_sequence >> 48) > 0) { + throw; + } + } }; if(!m_pending_state) { @@ -462,8 +485,6 @@ void Channel_Impl_12::process_handshake_ccs(const secure_vector& record if(m_is_datagram && !epoch0_restart) { if(m_sequence_numbers) { - sequence_numbers().read_accept(record_sequence); - const uint16_t epoch = record_sequence >> 48; const uint16_t current_epoch = sequence_numbers().current_read_epoch(); diff --git a/src/lib/tls/tls12/tls_record.cpp b/src/lib/tls/tls12/tls_record.cpp index 6619a8cec72..4efbf4254c3 100644 --- a/src/lib/tls/tls12/tls_record.cpp +++ b/src/lib/tls/tls12/tls_record.cpp @@ -479,7 +479,7 @@ Record_Header read_dtls_record(secure_vector& readbuf, // Unencrypted initial handshake recbuf.assign(readbuf.begin() + DTLS_HEADER_SIZE, readbuf.begin() + DTLS_HEADER_SIZE + record_size); readbuf.clear(); - if(sequence_numbers != nullptr) { + if(sequence_numbers != nullptr && sequence_numbers->current_read_epoch() == 0) { sequence_numbers->read_accept(sequence); } return Record_Header(sequence, version, type); diff --git a/src/tests/unit_tls.cpp b/src/tests/unit_tls.cpp index 001f4bdd9fa..1036671be7d 100644 --- a/src/tests/unit_tls.cpp +++ b/src/tests/unit_tls.cpp @@ -1731,6 +1731,25 @@ class DTLS_Core_Regression_Tests final : public Test { return record; } + static std::vector unprotected_dtls_record(Botan::TLS::Record_Type type, + uint64_t sequence, + std::span payload) { + std::vector record; + record.reserve(13 + payload.size()); + + record.push_back(static_cast(type)); + record.push_back(0xFE); + record.push_back(0xFD); + for(size_t i = 0; i != 8; ++i) { + record.push_back(static_cast(sequence >> (56 - 8 * i))); + } + record.push_back(static_cast(payload.size() >> 8)); + record.push_back(static_cast(payload.size())); + record.insert(record.end(), payload.begin(), payload.end()); + + return record; + } + template static bool wait_until(Predicate predicate) { // DTLS timeouts are clock based. Poll briefly instead of sleeping for @@ -1743,9 +1762,9 @@ class DTLS_Core_Regression_Tests final : public Test { return true; } - #if defined(BOTAN_TARGET_OS_HAS_THREADS) + #if defined(BOTAN_TARGET_OS_HAS_THREADS) std::this_thread::sleep_for(std::chrono::milliseconds(1)); - #endif + #endif } return predicate(); @@ -2611,6 +2630,70 @@ class DTLS_Core_Regression_Tests final : public Test { return result; } + static Test::Result test_spoofed_epoch0_records_do_not_abort_or_poison_retransmission() { + Test::Result result("DTLS spoofed epoch 0 records do not abort or poison retransmission"); + + auto rng = Test::new_shared_rng("dtls-core-spoofed-epoch0"); + auto policy = std::make_shared(); + auto creds = std::make_shared(); + auto client_sessions = std::make_shared(rng); + auto server_sessions = std::make_shared(rng); + + std::vector c2s; + std::vector s2c; + std::vector client_recv; + std::vector server_recv; + + auto server_callbacks = std::make_shared(result, s2c, server_recv); + auto client_callbacks = std::make_shared(result, c2s, client_recv); + + Botan::TLS::Server server(server_callbacks, server_sessions, creds, policy, rng, true); + Botan::TLS::Client client(client_callbacks, + client_sessions, + creds, + policy, + rng, + Botan::TLS::Server_Information("localhost"), + Botan::TLS::Protocol_Version::latest_dtls_version()); + + deliver(result, "client hello 1", c2s, server); + deliver(result, "hello verify request", s2c, client); + deliver(result, "client hello 2", c2s, server); + deliver(result, "server handshake flight", s2c, client); + deliver(result, "client final flight", c2s, server); + + result.test_is_true("server became active", server.is_active()); + result.test_is_false("client is waiting for server final flight", client.is_active()); + s2c.clear(); // simulate losing the server's final flight + + const std::array fatal_alert = {2, 40}; + const auto spoofed_alert = + unprotected_dtls_record(Botan::TLS::Record_Type::Alert, 0x0000FFFFFFFFFFFF, fatal_alert); + result.test_no_throw("spoofed epoch 0 fatal alert is ignored", + [&] { server.received_data(spoofed_alert.data(), spoofed_alert.size()); }); + result.test_is_true("server remains active after spoofed alert", server.is_active()); + result.test_is_true("server does not respond to spoofed alert", s2c.empty()); + + std::array invalid_handshake = {}; + invalid_handshake[0] = 0xFF; + const auto spoofed_handshake = + unprotected_dtls_record(Botan::TLS::Record_Type::Handshake, 0x0000FFFFFFFFFFFE, invalid_handshake); + result.test_no_throw("invalid epoch 0 handshake is ignored", + [&] { server.received_data(spoofed_handshake.data(), spoofed_handshake.size()); }); + result.test_is_true("server remains active after invalid handshake", server.is_active()); + result.test_is_true("server does not respond to invalid handshake", s2c.empty()); + + wait_for_timeout_retransmit(result, client, c2s); + deliver(result, "genuine client final flight retransmit", c2s, server); + result.test_is_true("server retransmits final flight", !s2c.empty()); + deliver(result, "retransmitted server final flight", s2c, client); + + result.test_is_true("client became active", client.is_active()); + result.test_is_true("server remains active", server.is_active()); + + return result; + } + static Test::Result test_resumed_final_flight_and_app_data_in_one_receive() { Test::Result result("DTLS resumed final flight and app data in one receive"); @@ -2744,6 +2827,7 @@ class DTLS_Core_Regression_Tests final : public Test { test_epoch0_client_hello_retransmit_while_restart_pending(), test_reordered_retransmitted_final_flight(), test_empty_old_handshake_fragment_does_not_retransmit(), + test_spoofed_epoch0_records_do_not_abort_or_poison_retransmission(), test_retransmitted_final_flight_then_application_data(false), test_retransmitted_final_flight_then_application_data(true), test_resumed_client_final_flight_retransmits_after_activation(),