From 4b39cb3eb69d4cd6b08f6a2d4f0bc04fed16811f Mon Sep 17 00:00:00 2001 From: Jonas Oreland Date: Thu, 30 Jan 2025 10:33:31 +0100 Subject: [PATCH] Reland "Move piggybacking controller from P2PTC to DTLS transport" This reverts commit 4de5839c1117e5bb96148c8575a74a69bde02768. Reason for revert: Bug fixed + DCHECK added Original change's description: > Revert "Move piggybacking controller from P2PTC to DTLS transport" > > This reverts commit 29e639e0a495a537c610182ab9b04aed8cf10426. > > Reason for revert: found bug accessing variable after it has been moved. > > Original change's description: > > Move piggybacking controller from P2PTC to DTLS transport > > > > The DTLS-STUN piggybacking controller is associated with both the DTLS > > transport and the ICE transport (P2PTransportChannel). It turned out to > > be more closely associated with the DTLS transport and requires less > > plumbing when moved there. > > > > The config option to enable the feature remains as part of the ICE > > transport config since the ICE transport does not know its "upstream" > > DTLS transport and hence can not query the config from it. > > > > BUG=webrtc:367395350 > > > > Change-Id: Iafd5abd8b65855bcf32bf840414d96513d8e6300 > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/375283 > > Reviewed-by: Jonas Oreland > > Commit-Queue: Jonas Oreland > > Cr-Commit-Position: refs/heads/main@{#43823} > > Bug: webrtc:367395350 > Change-Id: I2d83de8890b0aa230dd9e21cb5ce2eb03c8d3564 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/375861 > Bot-Commit: rubber-stamper@appspot.gserviceaccount.com > Commit-Queue: Jonas Oreland > Cr-Commit-Position: refs/heads/main@{#43824} Bug: webrtc:367395350 Change-Id: I4b4acccf15de565736b072ca2de88a1551a6378e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/375862 Reviewed-by: Mirko Bonadei Commit-Queue: Jonas Oreland Cr-Commit-Position: refs/heads/main@{#43825} --- p2p/BUILD.gn | 6 +- p2p/base/ice_transport_internal.h | 22 ++--- p2p/base/p2p_transport_channel.cc | 67 ++++++++----- p2p/base/p2p_transport_channel.h | 41 ++++---- p2p/base/p2p_transport_channel_unittest.cc | 62 ------------ p2p/dtls/dtls_ice_integrationtest.cc | 4 +- p2p/dtls/dtls_transport.cc | 107 +++++++++++++++++---- p2p/dtls/dtls_transport.h | 21 ++++ 8 files changed, 182 insertions(+), 148 deletions(-) diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn index 760b602c54..7708f72569 100644 --- a/p2p/BUILD.gn +++ b/p2p/BUILD.gn @@ -245,6 +245,7 @@ rtc_library("dtls_transport") { "dtls/dtls_transport.h", ] deps = [ + ":dtls_stun_piggyback_controller", ":dtls_transport_internal", ":dtls_utils", ":ice_transport_internal", @@ -256,6 +257,7 @@ rtc_library("dtls_transport") { "../api:sequence_checker", "../api/crypto:options", "../api/rtc_event_log", + "../api/transport:stun_types", "../api/units:timestamp", "../logging:ice_log", "../rtc_base:async_packet_socket", @@ -278,6 +280,7 @@ rtc_library("dtls_transport") { "../rtc_base/network:received_packet", "../rtc_base/network:sent_packet", "../rtc_base/system:no_unique_address", + "//third_party/abseil-cpp/absl/functional:any_invocable", "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/strings:string_view", ] @@ -383,6 +386,7 @@ rtc_library("ice_transport_internal") { "../api:field_trials_view", "../api:rtc_error", "../api/transport:enums", + "../api/transport:stun_types", "../rtc_base:callback_list", "../rtc_base:checks", "../rtc_base:network_constants", @@ -415,7 +419,6 @@ rtc_library("p2p_transport_channel") { ":candidate_pair_interface", ":connection", ":connection_info", - ":dtls_stun_piggyback_controller", ":ice_agent_interface", ":ice_controller_factory_interface", ":ice_controller_interface", @@ -467,6 +470,7 @@ rtc_library("p2p_transport_channel") { "../system_wrappers:metrics", "//third_party/abseil-cpp/absl/algorithm:container", "//third_party/abseil-cpp/absl/base:core_headers", + "//third_party/abseil-cpp/absl/functional:any_invocable", "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/strings", "//third_party/abseil-cpp/absl/strings:string_view", diff --git a/p2p/base/ice_transport_internal.h b/p2p/base/ice_transport_internal.h index d2f32de91b..85f48aa5ce 100644 --- a/p2p/base/ice_transport_internal.h +++ b/p2p/base/ice_transport_internal.h @@ -26,6 +26,7 @@ #include "api/field_trials_view.h" #include "api/rtc_error.h" #include "api/transport/enums.h" +#include "api/transport/stun.h" #include "p2p/base/candidate_pair_interface.h" #include "p2p/base/connection.h" #include "p2p/base/connection_info.h" @@ -35,7 +36,6 @@ #include "p2p/base/transport_description.h" #include "rtc_base/callback_list.h" #include "rtc_base/checks.h" -#include "rtc_base/network/received_packet.h" #include "rtc_base/network_constants.h" #include "rtc_base/system/rtc_export.h" #include "rtc_base/third_party/sigslot/sigslot.h" @@ -403,15 +403,14 @@ class RTC_EXPORT IceTransportInternal : public rtc::PacketTransportInternal { virtual const webrtc::FieldTrialsView* field_trials() const { return nullptr; } - void SetPiggybackDtlsDataCallback( - absl::AnyInvocable callback) { - RTC_DCHECK(callback == nullptr || !piggybacked_dtls_callback_); - piggybacked_dtls_callback_ = std::move(callback); - } - virtual void SetDtlsDataToPiggyback(rtc::ArrayView) {} - virtual void SetDtlsHandshakeComplete(bool is_dtls_client, bool is_dtls13) {} - virtual bool IsDtlsPiggybackSupportedByPeer() { return false; } + virtual void SetDtlsPiggybackingCallbacks( + absl::AnyInvocable(StunMessageType)> + dtls_piggyback_get_data, + absl::AnyInvocable(StunMessageType)> + dtls_piggyback_get_ack, + absl::AnyInvocable + dtls_piggyback_report_data) {} protected: void SendGatheringStateEvent() { gathering_state_callback_list_.Send(this); } @@ -433,9 +432,6 @@ class RTC_EXPORT IceTransportInternal : public rtc::PacketTransportInternal { absl::AnyInvocable candidate_pair_change_callback_; - absl::AnyInvocable - piggybacked_dtls_callback_; }; } // namespace cricket diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index 5b0d48af2e..8c7d91aeac 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -26,6 +26,7 @@ #include #include "absl/algorithm/container.h" +#include "absl/functional/any_invocable.h" #include "absl/memory/memory.h" #include "absl/strings/match.h" #include "absl/strings/string_view.h" @@ -200,16 +201,7 @@ P2PTransportChannel::P2PTransportChannel( true /* presume_writable_when_fully_relayed */, REGATHER_ON_FAILED_NETWORKS_INTERVAL, RECEIVING_SWITCHING_DELAY), - field_trials_(field_trials), - dtls_stun_piggyback_controller_( - [this](rtc::ArrayView piggybacked_dtls_packet) { - if (piggybacked_dtls_callback_ == nullptr) { - return; - } - piggybacked_dtls_callback_( - this, rtc::ReceivedPacket(piggybacked_dtls_packet, - rtc::SocketAddress())); - }) { + field_trials_(field_trials) { TRACE_EVENT0("webrtc", "P2PTransportChannel::P2PTransportChannel"); RTC_DCHECK(allocator_ != nullptr); // Validate IceConfig even for mostly built-in constant default values in case @@ -322,16 +314,14 @@ void P2PTransportChannel::AddConnection(Connection* connection) { if (config_.dtls_handshake_in_stun) { connection->RegisterDtlsPiggyback( [this](StunMessageType stun_message_type) { - return dtls_stun_piggyback_controller_.GetDataToPiggyback( - stun_message_type); + return dtls_piggyback_get_data_(stun_message_type); }, [this](StunMessageType stun_message_type) { - return dtls_stun_piggyback_controller_.GetAckToPiggyback( - stun_message_type); + return dtls_piggyback_get_ack_(stun_message_type); }, [this](const StunByteStringAttribute* data, const StunByteStringAttribute* ack) { - dtls_stun_piggyback_controller_.ReportDataPiggybacked(data, ack); + dtls_piggyback_report_data_(data, ack); }); } @@ -1634,15 +1624,6 @@ int P2PTransportChannel::SendPacket(const char* data, return -1; } - // If we try to use DTLS-in-STUN, DTLS packets will be sent as part of STUN - // packets and are consumed here. - rtc::ArrayView data_view = - rtc::MakeArrayView(reinterpret_cast(data), len); - if (config_.dtls_handshake_in_stun && - dtls_stun_piggyback_controller_.MaybeConsumePacket(data_view)) { - return len; - } - // If we don't think the connection is working yet, return ENOTCONN // instead of sending a packet that will probably be dropped. if (!ReadyToSend(selected_connection_)) { @@ -2314,9 +2295,11 @@ void P2PTransportChannel::SetWritable(bool writable) { } SignalWritableState(this); - if (config_.dtls_handshake_in_stun && IsDtlsPiggybackSupportedByPeer()) { + if (config_.dtls_handshake_in_stun && + dtls_piggyback_report_data_ != nullptr) { // Need to STUN ping here to get the last bit of the DTLS handshake across - // as quickly as possible. + // as quickly as possible. Only done when DTLS-in-STUN is configured + // and the data callback has not been reset due to lack of support. SendPingRequestInternal(selected_connection_); } } @@ -2391,4 +2374,36 @@ void P2PTransportChannel::GoogDeltaAckReceived( } } +void P2PTransportChannel::SetDtlsPiggybackingCallbacks( + absl::AnyInvocable(StunMessageType)> + dtls_piggyback_get_data, + absl::AnyInvocable(StunMessageType)> + dtls_piggyback_get_ack, + absl::AnyInvocable + dtls_piggyback_report_data) { + RTC_DCHECK_RUN_ON(network_thread_); + dtls_piggyback_get_data_ = std::move(dtls_piggyback_get_data); + dtls_piggyback_get_ack_ = std::move(dtls_piggyback_get_ack); + dtls_piggyback_report_data_ = std::move(dtls_piggyback_report_data); + + RTC_DCHECK( // either all set + (dtls_piggyback_get_data_ != nullptr && + dtls_piggyback_get_ack_ != nullptr && + dtls_piggyback_report_data_ != nullptr) || + // or all nullptr + (dtls_piggyback_get_data_ == nullptr && + dtls_piggyback_get_ack_ == nullptr && + dtls_piggyback_report_data_ == nullptr)); + + if (dtls_piggyback_get_data_ == nullptr && + dtls_piggyback_get_ack_ == nullptr && + dtls_piggyback_report_data_ == nullptr) { + // Iterate over connections, deregister. + for (auto& connection : connections_) { + connection->DeregisterDtlsPiggyback(); + } + } +} + } // namespace cricket diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h index 7367391234..0e39a82b59 100644 --- a/p2p/base/p2p_transport_channel.h +++ b/p2p/base/p2p_transport_channel.h @@ -30,6 +30,7 @@ #include #include +#include "absl/functional/any_invocable.h" #include "absl/strings/string_view.h" #include "api/array_view.h" #include "api/async_dns_resolver.h" @@ -58,7 +59,6 @@ #include "p2p/base/regathering_controller.h" #include "p2p/base/stun_dictionary.h" #include "p2p/base/transport_description.h" -#include "p2p/dtls/dtls_stun_piggyback_controller.h" #include "rtc_base/async_packet_socket.h" #include "rtc_base/checks.h" #include "rtc_base/dscp.h" @@ -252,27 +252,14 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal, const webrtc::FieldTrialsView* field_trials() const override { return field_trials_; } - - void SetDtlsHandshakeComplete(bool is_dtls_client, bool is_dtls13) override { - dtls_stun_piggyback_controller_.SetDtlsHandshakeComplete(is_dtls_client, - is_dtls13); - } - - bool IsDtlsPiggybackSupportedByPeer() override { - RTC_DCHECK_RUN_ON(network_thread_); - return config_.dtls_handshake_in_stun && - dtls_stun_piggyback_controller_.state() != - DtlsStunPiggybackController::State::OFF; - } - - bool IsDtlsPiggybackHandshaking() { - RTC_DCHECK_RUN_ON(network_thread_); - return config_.dtls_handshake_in_stun && - (dtls_stun_piggyback_controller_.state() == - DtlsStunPiggybackController::State::TENTATIVE || - dtls_stun_piggyback_controller_.state() == - DtlsStunPiggybackController::State::CONFIRMED); - } + void SetDtlsPiggybackingCallbacks( + absl::AnyInvocable(StunMessageType)> + dtls_piggyback_get_data, + absl::AnyInvocable(StunMessageType)> + dtls_piggyback_get_ack, + absl::AnyInvocable + dtls_piggyback_report_data) override; private: P2PTransportChannel( @@ -538,8 +525,14 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal, // A dictionary that tracks attributes from peer. StunDictionaryView stun_dict_view_; - // A controller for piggybacking DTLS in STUN. - DtlsStunPiggybackController dtls_stun_piggyback_controller_; + // DTLS-STUN piggybacking callbacks. + absl::AnyInvocable(StunMessageType)> + dtls_piggyback_get_data_; + absl::AnyInvocable(StunMessageType)> + dtls_piggyback_get_ack_; + absl::AnyInvocable + dtls_piggyback_report_data_; }; } // namespace cricket diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index 22feeccbc3..548961ba10 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -7441,66 +7441,4 @@ TEST_F(P2PTransportChannelTest, TestIceNoOldCandidatesAfterIceRestart) { DestroyChannels(); } -class P2PTransportChannelTestDtlsInStun : public P2PTransportChannelTestBase { - public: - P2PTransportChannelTestDtlsInStun() : P2PTransportChannelTestBase() {} - - protected: - void Run(bool ep1_support, bool ep2_support) { - IceConfig ep1_config; - ep1_config.dtls_handshake_in_stun = ep1_support; - IceConfig ep2_config; - ep2_config.dtls_handshake_in_stun = ep2_support; - CreateChannels(ep1_config, ep2_config); - // DTLS server hello done message as test data. - std::vector dtls_data = { - 0x16, 0xfe, 0xff, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x0c, 0x0e, 0x00, 0x00, 0x00, 0x00, - 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - }; - if (ep1_support) { - ep1_ch1()->SetDtlsDataToPiggyback(dtls_data); - } - if (ep2_support) { - ep2_ch1()->SetDtlsDataToPiggyback(dtls_data); - } - EXPECT_THAT( - webrtc::WaitUntil( - [&] { return CheckConnected(ep1_ch1(), ep2_ch1()); }, IsTrue(), - {.timeout = webrtc::TimeDelta::Millis(kDefaultTimeout), - .clock = &clock_}), - webrtc::IsRtcOk()); - } - - rtc::ScopedFakeClock clock_; -}; - -TEST_F(P2PTransportChannelTestDtlsInStun, NotSupportedByEither) { - Run(false, false); - EXPECT_FALSE(ep1_ch1()->IsDtlsPiggybackSupportedByPeer()); - EXPECT_FALSE(ep2_ch1()->IsDtlsPiggybackSupportedByPeer()); - DestroyChannels(); -} - -TEST_F(P2PTransportChannelTestDtlsInStun, SupportedByClient) { - Run(true, false); - EXPECT_FALSE(ep1_ch1()->IsDtlsPiggybackSupportedByPeer()); - EXPECT_FALSE(ep2_ch1()->IsDtlsPiggybackSupportedByPeer()); - DestroyChannels(); -} - -TEST_F(P2PTransportChannelTestDtlsInStun, SupportedByServer) { - Run(false, true); - EXPECT_FALSE(ep1_ch1()->IsDtlsPiggybackSupportedByPeer()); - EXPECT_FALSE(ep2_ch1()->IsDtlsPiggybackSupportedByPeer()); - DestroyChannels(); -} - -TEST_F(P2PTransportChannelTestDtlsInStun, SupportedByBoth) { - Run(true, true); - EXPECT_TRUE(ep1_ch1()->IsDtlsPiggybackSupportedByPeer()); - EXPECT_TRUE(ep2_ch1()->IsDtlsPiggybackSupportedByPeer()); - DestroyChannels(); -} - } // namespace cricket diff --git a/p2p/dtls/dtls_ice_integrationtest.cc b/p2p/dtls/dtls_ice_integrationtest.cc index 21145ee884..15aff1620b 100644 --- a/p2p/dtls/dtls_ice_integrationtest.cc +++ b/p2p/dtls/dtls_ice_integrationtest.cc @@ -184,9 +184,9 @@ TEST_P(DtlsIceIntegrationTest, SmokeTest) { {.timeout = webrtc::TimeDelta::Millis(kDefaultTimeout), .clock = &fake_clock_}), webrtc::IsRtcOk()); - EXPECT_EQ(client_ice_->IsDtlsPiggybackSupportedByPeer(), + EXPECT_EQ(client_dtls_.IsDtlsPiggybackSupportedByPeer(), client_dtls_stun_piggyback_ && server_dtls_stun_piggyback_); - EXPECT_EQ(server_ice_->IsDtlsPiggybackSupportedByPeer(), + EXPECT_EQ(server_dtls_.IsDtlsPiggybackSupportedByPeer(), client_dtls_stun_piggyback_ && server_dtls_stun_piggyback_); } diff --git a/p2p/dtls/dtls_transport.cc b/p2p/dtls/dtls_transport.cc index 41ebe78e58..f6da36f605 100644 --- a/p2p/dtls/dtls_transport.cc +++ b/p2p/dtls/dtls_transport.cc @@ -18,6 +18,7 @@ #include #include +#include "absl/functional/any_invocable.h" #include "absl/strings/string_view.h" #include "api/array_view.h" #include "api/crypto/crypto_options.h" @@ -26,11 +27,13 @@ #include "api/rtc_event_log/rtc_event_log.h" #include "api/scoped_refptr.h" #include "api/sequence_checker.h" +#include "api/transport/stun.h" #include "api/units/timestamp.h" #include "logging/rtc_event_log/events/rtc_event_dtls_transport_state.h" #include "logging/rtc_event_log/events/rtc_event_dtls_writable_state.h" #include "p2p/base/ice_transport_internal.h" #include "p2p/base/packet_transport_internal.h" +#include "p2p/dtls/dtls_stun_piggyback_controller.h" #include "p2p/dtls/dtls_transport_internal.h" #include "p2p/dtls/dtls_utils.h" #include "rtc_base/buffer.h" @@ -76,6 +79,11 @@ StreamInterfaceChannel::StreamInterfaceChannel( state_(rtc::SS_OPEN), packets_(kMaxPendingPackets, kMaxDtlsPacketLen) {} +void StreamInterfaceChannel::SetDtlsStunPiggybackController( + DtlsStunPiggybackController* dtls_stun_piggyback_controller) { + dtls_stun_piggyback_controller_ = dtls_stun_piggyback_controller; +} + rtc::StreamResult StreamInterfaceChannel::Read(rtc::ArrayView buffer, size_t& read, int& /* error */) { @@ -99,6 +107,15 @@ rtc::StreamResult StreamInterfaceChannel::Write( int& /* error */) { RTC_DCHECK_RUN_ON(&callback_sequence_); + // If we try to use DTLS-in-STUN, DTLS packets will be sent as part of STUN + // packets and are consumed here. + if (ice_transport_->config().dtls_handshake_in_stun && + dtls_stun_piggyback_controller_ && + dtls_stun_piggyback_controller_->MaybeConsumePacket(data)) { + written = data.size(); + return rtc::SR_SUCCESS; + } + // Always succeeds, since this is an unreliable transport anyway. // TODO(zhihuang): Should this block if ice_transport_'s temporarily // unwritable? @@ -143,17 +160,26 @@ DtlsTransport::DtlsTransport(IceTransportInternal* ice_transport, rtc::SSLProtocolVersion max_version) : component_(ice_transport->component()), ice_transport_(ice_transport), - downward_(NULL), + downward_(nullptr), srtp_ciphers_(crypto_options.GetSupportedDtlsSrtpCryptoSuites()), ssl_max_version_(max_version), - event_log_(event_log) { + event_log_(event_log), + dtls_stun_piggyback_controller_( + [this](rtc::ArrayView piggybacked_dtls_packet) { + if (piggybacked_dtls_callback_ == nullptr) { + return; + } + piggybacked_dtls_callback_( + this, rtc::ReceivedPacket(piggybacked_dtls_packet, + rtc::SocketAddress())); + }) { RTC_DCHECK(ice_transport_); ConnectToIceTransport(); } DtlsTransport::~DtlsTransport() { if (ice_transport_) { - ice_transport_->SetPiggybackDtlsDataCallback(nullptr); + ice_transport_->SetDtlsPiggybackingCallbacks(nullptr, nullptr, nullptr); ice_transport_->DeregisterReceivedPacketCallback(this); } } @@ -365,6 +391,8 @@ bool DtlsTransport::SetupDtls() { auto downward = std::make_unique(ice_transport_); StreamInterfaceChannel* downward_ptr = downward.get(); + downward_ptr->SetDtlsStunPiggybackController( + &dtls_stun_piggyback_controller_); dtls_ = rtc::SSLStreamAdapter::Create( std::move(downward), [this](rtc::SSLHandshakeError error) { OnDtlsHandshakeError(error); }, @@ -535,20 +563,33 @@ void DtlsTransport::ConnectToIceTransport() { this, &DtlsTransport::OnReceivingState); ice_transport_->SignalNetworkRouteChanged.connect( this, &DtlsTransport::OnNetworkRouteChanged); - ice_transport_->SetPiggybackDtlsDataCallback( - [this](rtc::PacketTransportInternal* transport, - const rtc::ReceivedPacket& packet) { - RTC_DCHECK(dtls_active_); - RTC_DCHECK(IsDtlsPacket(packet.payload())); - if (!dtls_active_) { - // Not doing DTLS. - return; - } - if (!IsDtlsPacket(packet.payload())) { - return; - } - OnReadPacket(transport, packet); + ice_transport_->SetDtlsPiggybackingCallbacks( + [this](StunMessageType stun_message_type) { + return dtls_stun_piggyback_controller_.GetDataToPiggyback( + stun_message_type); + }, + [this](StunMessageType stun_message_type) { + return dtls_stun_piggyback_controller_.GetAckToPiggyback( + stun_message_type); + }, + [this](const StunByteStringAttribute* data, + const StunByteStringAttribute* ack) { + dtls_stun_piggyback_controller_.ReportDataPiggybacked(data, ack); }); + + SetPiggybackDtlsDataCallback([this](rtc::PacketTransportInternal* transport, + const rtc::ReceivedPacket& packet) { + RTC_DCHECK(dtls_active_); + RTC_DCHECK(IsDtlsPacket(packet.payload())); + if (!dtls_active_) { + // Not doing DTLS. + return; + } + if (!IsDtlsPacket(packet.payload())) { + return; + } + OnReadPacket(ice_transport_, packet); + }); } // The state transition logic here is as follows: @@ -580,13 +621,13 @@ void DtlsTransport::OnWritableState(rtc::PacketTransportInternal* transport) { // the previous DTLS session state beyond repair and no packet // reached the peer. if (ice_transport_->config().dtls_handshake_in_stun && dtls_ && - !was_ever_connected_ && - !ice_transport_->IsDtlsPiggybackSupportedByPeer() && + !was_ever_connected_ && !IsDtlsPiggybackSupportedByPeer() && (dtls_state() == webrtc::DtlsTransportState::kConnecting || dtls_state() == webrtc::DtlsTransportState::kNew)) { RTC_LOG(LS_INFO) << "DTLS piggybacking not supported, restarting..."; - ice_transport_->SetPiggybackDtlsDataCallback(nullptr); + ice_transport_->SetDtlsPiggybackingCallbacks(nullptr, nullptr, nullptr); + downward_->SetDtlsStunPiggybackController(nullptr); dtls_.reset(nullptr); set_dtls_state(webrtc::DtlsTransportState::kNew); set_writable(false); @@ -753,9 +794,10 @@ void DtlsTransport::OnDtlsEvent(int sig, int err) { int ssl_version_bytes; bool ret = dtls_->GetSslVersionBytes(&ssl_version_bytes); RTC_DCHECK(ret); - ice_transport_->SetDtlsHandshakeComplete( + dtls_stun_piggyback_controller_.SetDtlsHandshakeComplete( dtls_role_ == rtc::SSL_CLIENT, ssl_version_bytes == rtc::kDtls13VersionBytes); + downward_->SetDtlsStunPiggybackController(nullptr); set_dtls_state(webrtc::DtlsTransportState::kConnected); set_writable(true); } @@ -936,4 +978,29 @@ void DtlsTransport::ConfigureHandshakeTimeout(bool uses_dtls_in_stun) { } } +void DtlsTransport::SetPiggybackDtlsDataCallback( + absl::AnyInvocable callback) { + RTC_DCHECK(callback == nullptr || !piggybacked_dtls_callback_); + piggybacked_dtls_callback_ = std::move(callback); +} + +bool DtlsTransport::IsDtlsPiggybackSupportedByPeer() { + RTC_DCHECK_RUN_ON(&thread_checker_); + RTC_DCHECK(ice_transport_); + return ice_transport_->config().dtls_handshake_in_stun && + dtls_stun_piggyback_controller_.state() != + DtlsStunPiggybackController::State::OFF; +} + +bool DtlsTransport::IsDtlsPiggybackHandshaking() { + RTC_DCHECK_RUN_ON(&thread_checker_); + RTC_DCHECK(ice_transport_); + return ice_transport_->config().dtls_handshake_in_stun && + (dtls_stun_piggyback_controller_.state() == + DtlsStunPiggybackController::State::TENTATIVE || + dtls_stun_piggyback_controller_.state() == + DtlsStunPiggybackController::State::CONFIRMED); +} + } // namespace cricket diff --git a/p2p/dtls/dtls_transport.h b/p2p/dtls/dtls_transport.h index a2ee3dbf70..b0953b54d4 100644 --- a/p2p/dtls/dtls_transport.h +++ b/p2p/dtls/dtls_transport.h @@ -18,6 +18,7 @@ #include #include +#include "absl/functional/any_invocable.h" #include "absl/strings/string_view.h" #include "api/array_view.h" #include "api/crypto/crypto_options.h" @@ -27,6 +28,7 @@ #include "api/scoped_refptr.h" #include "api/sequence_checker.h" #include "p2p/base/ice_transport_internal.h" +#include "p2p/dtls/dtls_stun_piggyback_controller.h" #include "p2p/dtls/dtls_transport_internal.h" #include "rtc_base/async_packet_socket.h" #include "rtc_base/buffer.h" @@ -55,6 +57,9 @@ class StreamInterfaceChannel : public rtc::StreamInterface { public: explicit StreamInterfaceChannel(IceTransportInternal* ice_transport); + void SetDtlsStunPiggybackController( + DtlsStunPiggybackController* dtls_stun_piggyback_controller); + StreamInterfaceChannel(const StreamInterfaceChannel&) = delete; StreamInterfaceChannel& operator=(const StreamInterfaceChannel&) = delete; @@ -73,6 +78,8 @@ class StreamInterfaceChannel : public rtc::StreamInterface { private: IceTransportInternal* const ice_transport_; // owned by DtlsTransport + DtlsStunPiggybackController* + dtls_stun_piggyback_controller_; // owned by DtlsTransport rtc::StreamState state_ RTC_GUARDED_BY(callback_sequence_); rtc::BufferQueue packets_ RTC_GUARDED_BY(callback_sequence_); }; @@ -221,6 +228,11 @@ class DtlsTransport : public DtlsTransportInternal { return sb.Release(); } + void SetPiggybackDtlsDataCallback( + absl::AnyInvocable callback); + bool IsDtlsPiggybackSupportedByPeer(); + private: void ConnectToIceTransport(); @@ -272,6 +284,15 @@ class DtlsTransport : public DtlsTransportInternal { bool was_ever_connected_ = false; webrtc::RtcEventLog* const event_log_; + + // A controller for piggybacking DTLS in STUN. + DtlsStunPiggybackController dtls_stun_piggyback_controller_; + + bool IsDtlsPiggybackHandshaking(); + + absl::AnyInvocable + piggybacked_dtls_callback_; }; } // namespace cricket