diff --git a/node/src/WebRtcTransport.ts b/node/src/WebRtcTransport.ts index f5735f70c6..1e0a8d5b49 100644 --- a/node/src/WebRtcTransport.ts +++ b/node/src/WebRtcTransport.ts @@ -98,7 +98,6 @@ export type WebRtcTransportOptionsBase = { /** * ICE consent timeout (in seconds). If 0 it is disabled. Default 30. - * For it to be enabled, iceParameters must be given in connect() method. */ iceConsentTimeout?: number; @@ -476,17 +475,14 @@ export class WebRtcTransport< * @override */ async connect({ - iceParameters, dtlsParameters, }: { - iceParameters?: IceParameters; dtlsParameters: DtlsParameters; }): Promise { logger.debug('connect()'); const requestOffset = createConnectRequest({ builder: this.channel.bufferBuilder, - iceParameters, dtlsParameters, }); @@ -837,35 +833,18 @@ export function parseWebRtcTransportDumpResponse( function createConnectRequest({ builder, - iceParameters, dtlsParameters, }: { builder: flatbuffers.Builder; - iceParameters?: IceParameters; dtlsParameters: DtlsParameters; }): number { - let iceParametersOffset: number | undefined; - - // Serialize IceParameters if given. This can throw. - if (iceParameters) { - iceParametersOffset = serializeIceParameters(builder, iceParameters); - } - // Serialize DtlsParameters. This can throw. const dtlsParametersOffset = serializeDtlsParameters(builder, dtlsParameters); - const ConnectRequest = FbsWebRtcTransport.ConnectRequest; - - // Create request. - ConnectRequest.startConnectRequest(builder); - - if (iceParametersOffset) { - ConnectRequest.addIceParameters(builder, iceParametersOffset); - } - - ConnectRequest.addDtlsParameters(builder, dtlsParametersOffset); - - return ConnectRequest.endConnectRequest(builder); + return FbsWebRtcTransport.ConnectRequest.createConnectRequest( + builder, + dtlsParametersOffset + ); } function parseGetStatsResponse( @@ -934,24 +913,6 @@ function parseDtlsParameters( }; } -function serializeIceParameters( - builder: flatbuffers.Builder, - iceParameters: IceParameters -): number { - const usernameFragmentOffset = builder.createString( - iceParameters.usernameFragment ?? '' - ); - const passwordOffset = builder.createString(iceParameters.password ?? ''); - const iceLiteOffset = Boolean(iceParameters.iceLite); - - return FbsWebRtcTransport.IceParameters.createIceParameters( - builder, - usernameFragmentOffset, - passwordOffset, - iceLiteOffset - ); -} - function serializeDtlsParameters( builder: flatbuffers.Builder, dtlsParameters: DtlsParameters diff --git a/node/src/test/test-WebRtcTransport.ts b/node/src/test/test-WebRtcTransport.ts index 203438fc5e..c605c05cb5 100644 --- a/node/src/test/test-WebRtcTransport.ts +++ b/node/src/test/test-WebRtcTransport.ts @@ -314,47 +314,6 @@ test('webRtcTransport.connect() succeeds', async () => { expect(webRtcTransport.dtlsParameters.role).toBe('server'); }, 2000); -test('webRtcTransport.connect() with iceParameters succeeds', async () => { - const webRtcTransport = await ctx.router!.createWebRtcTransport({ - listenInfos: [ - { protocol: 'udp', ip: '127.0.0.1', announcedAddress: '9.9.9.1' }, - ], - }); - - const iceRemoteParameters: mediasoup.types.IceParameters = { - usernameFragment: 'foo', - password: 'xxxx', - }; - - const dtlsRemoteParameters: mediasoup.types.DtlsParameters = { - fingerprints: [ - { - algorithm: 'sha-256', - value: - '82:5A:68:3D:36:C3:0A:DE:AF:E7:32:43:D2:88:83:57:AC:2D:65:E5:80:C4:B6:FB:AF:1A:A0:21:9F:6D:0C:AD', - }, - ], - role: 'client', - }; - - await expect( - webRtcTransport.connect({ - iceParameters: iceRemoteParameters, - dtlsParameters: dtlsRemoteParameters, - }) - ).resolves.toBeUndefined(); - - // Must fail if connected. - await expect( - webRtcTransport.connect({ - iceParameters: iceRemoteParameters, - dtlsParameters: dtlsRemoteParameters, - }) - ).rejects.toThrow(Error); - - expect(webRtcTransport.dtlsParameters.role).toBe('server'); -}, 2000); - test('webRtcTransport.connect() with wrong arguments rejects with TypeError', async () => { const webRtcTransport = await ctx.router!.createWebRtcTransport({ listenInfos: [ diff --git a/rust/examples/echo.rs b/rust/examples/echo.rs index 733f2a4863..d8d751f55f 100644 --- a/rust/examples/echo.rs +++ b/rust/examples/echo.rs @@ -298,10 +298,7 @@ impl Handler for EchoConnection { // synchronous actix::spawn(async move { match transport - .connect(WebRtcTransportRemoteParameters { - ice_parameters: None, - dtls_parameters, - }) + .connect(WebRtcTransportRemoteParameters { dtls_parameters }) .await { Ok(_) => { @@ -349,10 +346,7 @@ impl Handler for EchoConnection { // The same as producer transport, but for consumer transport actix::spawn(async move { match transport - .connect(WebRtcTransportRemoteParameters { - ice_parameters: None, - dtls_parameters, - }) + .connect(WebRtcTransportRemoteParameters { dtls_parameters }) .await { Ok(_) => { diff --git a/rust/examples/multiopus.rs b/rust/examples/multiopus.rs index 456fad1f56..d78e8fd859 100644 --- a/rust/examples/multiopus.rs +++ b/rust/examples/multiopus.rs @@ -331,10 +331,7 @@ impl Handler for EchoConnection { // The same as producer transport, but for consumer transport actix::spawn(async move { match transport - .connect(WebRtcTransportRemoteParameters { - ice_parameters: None, - dtls_parameters, - }) + .connect(WebRtcTransportRemoteParameters { dtls_parameters }) .await { Ok(_) => { diff --git a/rust/examples/svc-simulcast.rs b/rust/examples/svc-simulcast.rs index d48dba6091..4bf34bbf6c 100644 --- a/rust/examples/svc-simulcast.rs +++ b/rust/examples/svc-simulcast.rs @@ -344,10 +344,7 @@ impl Handler for SvcSimulcastConnection { // synchronous actix::spawn(async move { match transport - .connect(WebRtcTransportRemoteParameters { - ice_parameters: None, - dtls_parameters, - }) + .connect(WebRtcTransportRemoteParameters { dtls_parameters }) .await { Ok(_) => { @@ -395,10 +392,7 @@ impl Handler for SvcSimulcastConnection { // The same as producer transport, but for consumer transport actix::spawn(async move { match transport - .connect(WebRtcTransportRemoteParameters { - ice_parameters: None, - dtls_parameters, - }) + .connect(WebRtcTransportRemoteParameters { dtls_parameters }) .await { Ok(_) => { diff --git a/rust/examples/videoroom.rs b/rust/examples/videoroom.rs index 98ffdd493a..379958534e 100644 --- a/rust/examples/videoroom.rs +++ b/rust/examples/videoroom.rs @@ -659,10 +659,7 @@ mod participant { // synchronous actix::spawn(async move { match transport - .connect(WebRtcTransportRemoteParameters { - ice_parameters: None, - dtls_parameters, - }) + .connect(WebRtcTransportRemoteParameters { dtls_parameters }) .await { Ok(_) => { @@ -721,10 +718,7 @@ mod participant { // The same as producer transport, but for consumer transport actix::spawn(async move { match transport - .connect(WebRtcTransportRemoteParameters { - ice_parameters: None, - dtls_parameters, - }) + .connect(WebRtcTransportRemoteParameters { dtls_parameters }) .await { Ok(_) => { diff --git a/rust/src/data_structures.rs b/rust/src/data_structures.rs index 59f7e77420..807dee3990 100644 --- a/rust/src/data_structures.rs +++ b/rust/src/data_structures.rs @@ -153,14 +153,6 @@ pub struct IceParameters { } impl IceParameters { - pub(crate) fn to_fbs(&self) -> web_rtc_transport::IceParameters { - web_rtc_transport::IceParameters { - username_fragment: self.username_fragment.to_string(), - password: self.password.to_string(), - ice_lite: self.ice_lite.unwrap_or(false), - } - } - pub(crate) fn from_fbs(parameters: web_rtc_transport::IceParameters) -> Self { Self { username_fragment: parameters.username_fragment.to_string(), diff --git a/rust/src/messages.rs b/rust/src/messages.rs index 79e86db53c..d06fcaaf52 100644 --- a/rust/src/messages.rs +++ b/rust/src/messages.rs @@ -1408,7 +1408,6 @@ pub(crate) struct WebRtcTransportConnectResponse { #[derive(Debug)] pub(crate) struct WebRtcTransportConnectRequest { - pub(crate) ice_parameters: Option, pub(crate) dtls_parameters: DtlsParameters, } @@ -1419,11 +1418,8 @@ impl Request for WebRtcTransportConnectRequest { fn into_bytes(self, id: u32, handler_id: Self::HandlerId) -> Vec { let mut builder = Builder::new(); - let data = web_rtc_transport::ConnectRequest::create( - &mut builder, - self.ice_parameters.map(|parameters| parameters.to_fbs()), - self.dtls_parameters.to_fbs(), - ); + let data = + web_rtc_transport::ConnectRequest::create(&mut builder, self.dtls_parameters.to_fbs()); let request_body = request::Body::create_web_rtc_transport_connect_request(&mut builder, data); let request = request::Request::create( diff --git a/rust/src/router/webrtc_transport.rs b/rust/src/router/webrtc_transport.rs index 3129da2d5e..e3f8c3babe 100644 --- a/rust/src/router/webrtc_transport.rs +++ b/rust/src/router/webrtc_transport.rs @@ -130,7 +130,6 @@ pub struct WebRtcTransportOptions { /// Default false. pub prefer_tcp: bool, /// ICE consent timeout (in seconds). If 0 it is disabled. - /// For it to be enabled, iceParameters must be given in connect() method. /// Default 30. pub ice_consent_timeout: u8, /// Create a SCTP association. @@ -390,8 +389,6 @@ impl WebRtcTransportStat { #[derive(Debug, Clone, PartialOrd, Eq, PartialEq, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct WebRtcTransportRemoteParameters { - /// Remote ICE parameters. - pub ice_parameters: Option, /// Remote DTLS parameters. pub dtls_parameters: DtlsParameters, } @@ -928,7 +925,7 @@ impl WebRtcTransport { /// /// # Example /// ```rust - /// use mediasoup::data_structures::{IceParameters, DtlsParameters, DtlsRole, DtlsFingerprint}; + /// use mediasoup::data_structures::{DtlsParameters, DtlsRole, DtlsFingerprint}; /// use mediasoup::webrtc_transport::WebRtcTransportRemoteParameters; /// /// # async fn f( @@ -937,11 +934,6 @@ impl WebRtcTransport { /// // Calling connect() on a PlainTransport created with comedia and rtcp_mux set. /// webrtc_transport /// .connect(WebRtcTransportRemoteParameters { - /// ice_parameters: Some(IceParameters { - /// username_fragment: "foo".to_string(), - /// password: "xxxx".to_string(), - /// ice_lite: None, - /// }), /// dtls_parameters: DtlsParameters { /// role: DtlsRole::Server, /// fingerprints: vec![ @@ -971,7 +963,6 @@ impl WebRtcTransport { .request( self.id(), WebRtcTransportConnectRequest { - ice_parameters: remote_parameters.ice_parameters, dtls_parameters: remote_parameters.dtls_parameters, }, ) diff --git a/rust/tests/integration/webrtc_transport.rs b/rust/tests/integration/webrtc_transport.rs index 45e2e60346..c1e9b4bffb 100644 --- a/rust/tests/integration/webrtc_transport.rs +++ b/rust/tests/integration/webrtc_transport.rs @@ -1,8 +1,8 @@ use futures_lite::future; use hash_hasher::HashedSet; use mediasoup::data_structures::{ - AppData, DtlsFingerprint, DtlsParameters, DtlsRole, DtlsState, IceCandidateType, IceParameters, - IceRole, IceState, ListenInfo, Protocol, SctpState, + AppData, DtlsFingerprint, DtlsParameters, DtlsRole, DtlsState, IceCandidateType, IceRole, + IceState, ListenInfo, Protocol, SctpState, }; use mediasoup::prelude::*; use mediasoup::router::{Router, RouterOptions}; @@ -418,7 +418,6 @@ fn connect_succeeds() { transport .connect(WebRtcTransportRemoteParameters { - ice_parameters: None, dtls_parameters: dtls_parameters.clone(), }) .await @@ -427,70 +426,7 @@ fn connect_succeeds() { // Must fail if connected. assert!(matches!( transport - .connect(WebRtcTransportRemoteParameters { - ice_parameters: None, - dtls_parameters - }) - .await, - Err(RequestError::Response { .. }), - )); - - assert_eq!(transport.dtls_parameters().role, DtlsRole::Server); - }); -} - -#[test] -fn connect_with_ice_parameters_succeeds() { - future::block_on(async move { - let (_worker, router) = init().await; - - let transport = router - .create_webrtc_transport(WebRtcTransportOptions::new( - WebRtcTransportListenInfos::new(ListenInfo { - protocol: Protocol::Udp, - ip: IpAddr::V4(Ipv4Addr::LOCALHOST), - announced_address: Some("9.9.9.1".to_string()), - port: None, - flags: None, - send_buffer_size: None, - recv_buffer_size: None, - }), - )) - .await - .expect("Failed to create WebRTC transport"); - - let ice_parameters = IceParameters { - username_fragment: "foo".to_string(), - password: "xxxx".to_string(), - ice_lite: None, - }; - - let dtls_parameters = DtlsParameters { - role: DtlsRole::Client, - fingerprints: vec![DtlsFingerprint::Sha256 { - value: [ - 0x82, 0x5A, 0x68, 0x3D, 0x36, 0xC3, 0x0A, 0xDE, 0xAF, 0xE7, 0x32, 0x43, 0xD2, - 0x88, 0x83, 0x57, 0xAC, 0x2D, 0x65, 0xE5, 0x80, 0xC4, 0xB6, 0xFB, 0xAF, 0x1A, - 0xA0, 0x21, 0x9F, 0x6D, 0x0C, 0xAD, - ], - }], - }; - - transport - .connect(WebRtcTransportRemoteParameters { - ice_parameters: Some(ice_parameters.clone()), - dtls_parameters: dtls_parameters.clone(), - }) - .await - .expect("Failed to establish WebRTC transport connection"); - - // Must fail if connected. - assert!(matches!( - transport - .connect(WebRtcTransportRemoteParameters { - ice_parameters: Some(ice_parameters), - dtls_parameters - }) + .connect(WebRtcTransportRemoteParameters { dtls_parameters }) .await, Err(RequestError::Response { .. }), )); diff --git a/worker/fbs/webRtcTransport.fbs b/worker/fbs/webRtcTransport.fbs index b7afe4f2a9..4d66c7a511 100644 --- a/worker/fbs/webRtcTransport.fbs +++ b/worker/fbs/webRtcTransport.fbs @@ -95,7 +95,6 @@ table IceCandidate { } table ConnectRequest { - ice_parameters: IceParameters; dtls_parameters: DtlsParameters (required); } diff --git a/worker/fuzzer/src/RTC/FuzzerStunPacket.cpp b/worker/fuzzer/src/RTC/FuzzerStunPacket.cpp index 2188d9e7a6..37d567778d 100644 --- a/worker/fuzzer/src/RTC/FuzzerStunPacket.cpp +++ b/worker/fuzzer/src/RTC/FuzzerStunPacket.cpp @@ -41,7 +41,7 @@ void Fuzzer::RTC::StunPacket::Fuzz(const uint8_t* data, size_t len) packet->GetErrorCode(); packet->HasMessageIntegrity(); packet->HasFingerprint(); - packet->CheckAuthentication("foo", "bar", "xxx"); + packet->CheckAuthentication("foo", "xxx"); if (packet->GetClass() == ::RTC::StunPacket::Class::REQUEST) { diff --git a/worker/include/RTC/IceServer.hpp b/worker/include/RTC/IceServer.hpp index 080a0d1bb4..06507e05de 100644 --- a/worker/include/RTC/IceServer.hpp +++ b/worker/include/RTC/IceServer.hpp @@ -7,7 +7,6 @@ #include "RTC/StunPacket.hpp" #include "RTC/TransportTuple.hpp" #include "handles/TimerHandle.hpp" -#include #include #include @@ -86,22 +85,6 @@ namespace RTC { return this->password; } - void SetRemoteUsernameFragmentAndPassword( - const std::string& usernameFragment, const std::string& password) - { - this->remoteUsernameFragment = usernameFragment; - this->remotePassword = password; - - if (IsConsentCheckSupported()) - { - // Restart ICE consent check to avoid authentication issues. - MayRestartConsentCheck(); - } - else - { - MayStopConsentCheck(); - } - } IceState GetState() const { return this->state; @@ -134,27 +117,21 @@ namespace RTC */ RTC::TransportTuple* HasTuple(const RTC::TransportTuple* tuple) const; /** - * Set the given tuple as the selected tuple. Returns true if given tuple - * was not already the selected tuple, false otherwise. + * Set the given tuple as the selected tuple. * NOTE: The given tuple MUST be already stored within the list. */ - bool SetSelectedTuple(RTC::TransportTuple* storedTuple); + void SetSelectedTuple(RTC::TransportTuple* storedTuple); bool IsConsentCheckSupported() const { - return ( - this->consentTimeoutMs != 0u && !this->remoteUsernameFragment.empty() && - !this->remotePassword.empty()); + return this->consentTimeoutMs != 0u; } bool IsConsentCheckRunning() const { return (this->consentCheckTimer && this->consentCheckTimer->IsActive()); } - void MayStartConsentCheck(); - void MayStopConsentCheck(); - void MayRestartConsentCheck(); - void MayStartOrRestartConsentCheck(); - void SendConsentRequest(); - void ConsentTerminated(); + void StartConsentCheck(); + void RestartConsentCheck(); + void StopConsentCheck(); /* Pure virtual methods inherited from TimerHandle::Listener. */ public: @@ -169,14 +146,12 @@ namespace RTC // Others. std::string oldUsernameFragment; std::string oldPassword; - std::string remoteUsernameFragment; - std::string remotePassword; IceState state{ IceState::NEW }; uint32_t remoteNomination{ 0u }; std::list tuples; RTC::TransportTuple* selectedTuple{ nullptr }; TimerHandle* consentCheckTimer{ nullptr }; - std::deque sentConsents; + uint64_t lastConsentRequestReceivedAtMs{ 0u }; }; } // namespace RTC diff --git a/worker/include/RTC/StunPacket.hpp b/worker/include/RTC/StunPacket.hpp index 040fef4ef9..1d781fff43 100644 --- a/worker/include/RTC/StunPacket.hpp +++ b/worker/include/RTC/StunPacket.hpp @@ -193,8 +193,8 @@ namespace RTC return this->hasFingerprint; } Authentication CheckAuthentication( + // The first username fragment in the USERNAME attribute. const std::string& usernameFragment1, - const std::string& usernameFragment2, const std::string& password); StunPacket* CreateSuccessResponse(); StunPacket* CreateErrorResponse(uint16_t errorCode); diff --git a/worker/src/RTC/IceServer.cpp b/worker/src/RTC/IceServer.cpp index 49d8896b7b..756cae8eb5 100644 --- a/worker/src/RTC/IceServer.cpp +++ b/worker/src/RTC/IceServer.cpp @@ -1,5 +1,5 @@ #define MS_CLASS "RTC::IceServer" -#define MS_LOG_DEV_LEVEL 3 +// #define MS_LOG_DEV_LEVEL 3 #include "RTC/IceServer.hpp" #include "DepLibUV.hpp" @@ -12,11 +12,8 @@ namespace RTC static constexpr size_t StunSerializeBufferSize{ 65536 }; thread_local static uint8_t StunSerializeBuffer[StunSerializeBufferSize]; static constexpr size_t MaxTuples{ 8 }; - static const std::string SoftwareAttribute{ "mediasoup" }; - static constexpr uint64_t ConsentCheckIntervalMs{ 5000u }; static constexpr uint8_t ConsentCheckMinTimeoutSec{ 10u }; static constexpr uint8_t ConsentCheckMaxTimeoutSec{ 60u }; - static constexpr uint64_t MaxOngoingSentConsents{ 20u }; /* Class methods. */ IceServer::IceState IceStateFromFbs(FBS::WebRtcTransport::IceState state) @@ -142,9 +139,6 @@ namespace RTC // Unset selected tuple. this->selectedTuple = nullptr; - // Clear queue of ongoing sent ICE consent request. - this->sentConsents.clear(); - // Delete the ICE consent check timer. delete this->consentCheckTimer; this->consentCheckTimer = nullptr; @@ -209,8 +203,12 @@ namespace RTC // yet with old usernameFragment. Wait until we receive a STUN packet // with the new one. - // Restart ICE consent check to avoid authentication issues. - MayRestartConsentCheck(); + // Restart ICE consent check (if running) to give some time to the + // client to establish ICE again. + if (IsConsentCheckSupported() && IsConsentCheckRunning()) + { + RestartConsentCheck(); + } } bool IceServer::IsValidTuple(const RTC::TransportTuple* tuple) const @@ -260,12 +258,20 @@ namespace RTC { this->selectedTuple = nullptr; - // Mark the first tuple as selected tuple (if any). - if (this->tuples.begin() != this->tuples.end()) + // Mark the first tuple as selected tuple (if any) but only if state was + // 'connected' or 'completed'. + if ( + (this->state == IceState::CONNECTED || this->state == IceState::COMPLETED) && + this->tuples.begin() != this->tuples.end()) { SetSelectedTuple(std::addressof(*this->tuples.begin())); - MayStartOrRestartConsentCheck(); + // Restart ICE consent check to let the client send new consent requests + // on the new selected tuple. + if (IsConsentCheckSupported()) + { + RestartConsentCheck(); + } } // Or just emit 'disconnected'. else @@ -279,7 +285,10 @@ namespace RTC // Notify the listener. this->listener->OnIceServerDisconnected(this); - MayStopConsentCheck(); + if (IsConsentCheckSupported() && IsConsentCheckRunning()) + { + StopConsentCheck(); + } } } } @@ -342,8 +351,7 @@ namespace RTC } // Check authentication. - switch (request->CheckAuthentication( - this->usernameFragment, this->remoteUsernameFragment, this->password)) + switch (request->CheckAuthentication(this->usernameFragment, this->password)) { case RTC::StunPacket::Authentication::OK: { @@ -370,7 +378,7 @@ namespace RTC !this->oldUsernameFragment.empty() && !this->oldPassword.empty() && request->CheckAuthentication( - this->oldUsernameFragment, this->remoteUsernameFragment, this->oldPassword + this->oldUsernameFragment, this->oldPassword ) == RTC::StunPacket::Authentication::OK ) // clang-format on @@ -461,26 +469,29 @@ namespace RTC // Handle the tuple. HandleTuple(tuple, request->HasUseCandidate(), request->HasNomination(), nomination); + + // If state is 'connected' or 'completed' after handling the tuple, then + // start or restart ICE consent check (if supported). + if (IsConsentCheckSupported() && (this->state == IceState::CONNECTED || this->state == IceState::COMPLETED)) + { + if (IsConsentCheckRunning()) + { + RestartConsentCheck(); + } + else + { + StartConsentCheck(); + } + } } void IceServer::ProcessStunIndication(RTC::StunPacket* indication) { MS_TRACE(); - MS_DEBUG_DEV("processing STUN indication"); + MS_DEBUG_DEV("STUN indication received, discarded"); - // Must be a Binding method. - if (indication->GetMethod() != RTC::StunPacket::Method::BINDING) - { - MS_WARN_TAG( - ice, - "STUN indication with unknown method %#.3x, discarded", - static_cast(indication->GetMethod())); - - return; - } - - // Nothig else to do. We just discard STUN Binding indications. + // Nothig else to do. We just discard STUN indications. } void IceServer::ProcessStunResponse(RTC::StunPacket* response) @@ -491,123 +502,10 @@ namespace RTC ? "success" : std::to_string(response->GetErrorCode()) + " error"; - MS_DEBUG_DEV("processing STUN %s response", responseType.c_str()); + MS_DEBUG_DEV("processing STUN %s response received, discarded", responseType.c_str()); - // Must be a Binding method. - if (response->GetMethod() != RTC::StunPacket::Method::BINDING) - { - MS_WARN_TAG( - ice, - "STUN %s response with unknown method %#.3x, discarded", - responseType.c_str(), - static_cast(response->GetMethod())); - - return; - } - - // Must have FINGERPRINT attribute. - if (!response->HasFingerprint()) - { - MS_WARN_TAG( - ice, - "STUN Binding %s response without FINGERPRINT attribute, discarded", - responseType.c_str()); - - return; - } - - // Check authentication. - switch (response->CheckAuthentication( - this->remoteUsernameFragment, this->usernameFragment, this->remotePassword)) - { - case RTC::StunPacket::Authentication::OK: - { - break; - } - - case RTC::StunPacket::Authentication::UNAUTHORIZED: - { - MS_WARN_TAG( - ice, "STUN %s response with wrong authentication, discarded", responseType.c_str()); - - return; - } - - case RTC::StunPacket::Authentication::BAD_MESSAGE: - { - MS_WARN_TAG( - ice, - "cannot check authentication in STUN Binding %s response, discarded", - responseType.c_str()); - - return; - } - } - - if (!IsConsentCheckSupported()) - { - MS_WARN_DEV( - "ignoring STUN Binding %s response because ICE consent check is not supported", - responseType.c_str()); - - return; - } - // Ignore if right now ICE consent check is not running since it means that - // ICE is no longer established. - else if (!IsConsentCheckRunning()) - { - MS_DEBUG_DEV( - "ignoring STUN Binding %s response because ICE consent check is not running", - responseType.c_str()); - - return; - } - - // Trick: We only read the fist 4 bytes of the transactionId of the - // response since we know that we generated 4 bytes followed by 8 zeroed - // bytes in the transactionId of the sent consent request. - auto transactionId = Utils::Byte::Get4Bytes(response->GetTransactionId(), 0); - - // Let's iterate all entries in the queue and remove the consent request - // associated to this response and all those that were sent before. - - // Find the associated entry first. - auto it = std::find_if( - this->sentConsents.begin(), - this->sentConsents.end(), - [transactionId](auto& sentConsent) - { return Utils::Byte::Get4Bytes(sentConsent.transactionId, 0) == transactionId; }); - - if (it != this->sentConsents.end()) - { - // If a success response or any non 403 error, let's behave the same way - // by deleting the matching sent consent request and all previous ones. - // This way, if a strict ICE endpoint replies 400 to our consent requests - // (because indeed mediasoup as ICE Lite server should not send requests) - // we know that the endpoint is alive, which is what ICE consent mechanism - // is about anyway. - if (response->GetErrorCode() != 403) - { - this->sentConsents.erase(this->sentConsents.begin(), it + 1); - } - // 403 means that the endpoint has revoked the consent so we should - // disconnect ICE. - else - { - MS_WARN_TAG( - ice, - "ICE consent revoked by the endpoint by means of a 403 response to our ICE consent request, moving to 'disconnected' state"); - - ConsentTerminated(); - } - } - else - { - MS_WARN_TAG( - ice, - "STUN %s response doesn't match any sent consent request, discarded", - responseType.c_str()); - } + // Nothig else to do. We just discard STUN responses because we do not + // generate STUN requests. } void IceServer::MayForceSelectedTuple(const RTC::TransportTuple* tuple) @@ -630,13 +528,7 @@ namespace RTC return; } - // Mark it as selected tuple. - auto isNewSelectedTuple = SetSelectedTuple(storedTuple); - - if (isNewSelectedTuple) - { - MayStartOrRestartConsentCheck(); - } + SetSelectedTuple(storedTuple); } void IceServer::HandleTuple( @@ -664,16 +556,14 @@ namespace RTC // Store the tuple. auto* storedTuple = AddTuple(tuple); - // Mark it as selected tuple. - SetSelectedTuple(storedTuple); - // Update state. this->state = IceState::CONNECTED; + // Mark it as selected tuple. + SetSelectedTuple(storedTuple); + // Notify the listener. this->listener->OnIceServerConnected(this); - - MayStartConsentCheck(); } else { @@ -690,12 +580,12 @@ namespace RTC hasNomination ? "true" : "false", nomination); - // Mark it as selected tuple. - SetSelectedTuple(storedTuple); - // Update state. this->state = IceState::COMPLETED; + // Mark it as selected tuple. + SetSelectedTuple(storedTuple); + // Update nomination. if (hasNomination && nomination > this->remoteNomination) { @@ -704,8 +594,6 @@ namespace RTC // Notify the listener. this->listener->OnIceServerCompleted(this); - - MayStartConsentCheck(); } } @@ -730,16 +618,14 @@ namespace RTC // Store the tuple. auto* storedTuple = AddTuple(tuple); - // Mark it as selected tuple. - SetSelectedTuple(storedTuple); - // Update state. this->state = IceState::CONNECTED; + // Mark it as selected tuple. + SetSelectedTuple(storedTuple); + // Notify the listener. this->listener->OnIceServerConnected(this); - - MayStartConsentCheck(); } else { @@ -756,12 +642,12 @@ namespace RTC hasNomination ? "true" : "false", nomination); - // Mark it as selected tuple. - SetSelectedTuple(storedTuple); - // Update state. this->state = IceState::COMPLETED; + // Mark it as selected tuple. + SetSelectedTuple(storedTuple); + // Update nomination. if (hasNomination && nomination > this->remoteNomination) { @@ -770,8 +656,6 @@ namespace RTC // Notify the listener. this->listener->OnIceServerCompleted(this); - - MayStartConsentCheck(); } } @@ -806,12 +690,12 @@ namespace RTC if ((hasNomination && nomination > this->remoteNomination) || !hasNomination) { - // Mark it as selected tuple. - auto isNewSelectedTuple = SetSelectedTuple(storedTuple); - // Update state. this->state = IceState::COMPLETED; + // Mark it as selected tuple. + SetSelectedTuple(storedTuple); + // Update nomination. if (hasNomination && nomination > this->remoteNomination) { @@ -820,11 +704,6 @@ namespace RTC // Notify the listener. this->listener->OnIceServerCompleted(this); - - if (isNewSelectedTuple) - { - MayStartOrRestartConsentCheck(); - } } } @@ -852,18 +731,13 @@ namespace RTC if ((hasNomination && nomination > this->remoteNomination) || !hasNomination) { // Mark it as selected tuple. - auto isNewSelectedTuple = SetSelectedTuple(storedTuple); + SetSelectedTuple(storedTuple); // Update nomination. if (hasNomination && nomination > this->remoteNomination) { this->remoteNomination = nomination; } - - if (isNewSelectedTuple) - { - MayStartOrRestartConsentCheck(); - } } } @@ -964,36 +838,29 @@ namespace RTC return nullptr; } - bool IceServer::SetSelectedTuple(RTC::TransportTuple* storedTuple) + void IceServer::SetSelectedTuple(RTC::TransportTuple* storedTuple) { MS_TRACE(); // If already the selected tuple do nothing. if (storedTuple == this->selectedTuple) { - return false; + return; } this->selectedTuple = storedTuple; // Notify the listener. this->listener->OnIceServerSelectedTuple(this, this->selectedTuple); - - return true; } - void IceServer::MayStartConsentCheck() + void IceServer::StartConsentCheck() { MS_TRACE(); - if (!IsConsentCheckSupported()) - { - return; - } - else if (IsConsentCheckRunning()) - { - return; - } + MS_ASSERT(IsConsentCheckSupported(), "ICE consent check not supported"); + MS_ASSERT(!IsConsentCheckRunning(), "ICE consent check already running"); + MS_ASSERT(this->selectedTuple, "no selected tuple"); // Create the ICE consent check timer if it doesn't exist. if (!this->consentCheckTimer) @@ -1001,132 +868,28 @@ namespace RTC this->consentCheckTimer = new TimerHandle(this); } - this->consentCheckTimer->Start(ConsentCheckIntervalMs); - } - - void IceServer::MayStopConsentCheck() - { - MS_TRACE(); - - if (this->consentCheckTimer) - { - this->consentCheckTimer->Stop(); - } - - // Clear queue of ongoing sent ICE consent requests. - this->sentConsents.clear(); - } - - void IceServer::MayRestartConsentCheck() - { - MS_TRACE(); - - if (!IsConsentCheckRunning()) - { - return; - } - - // Clear queue of ongoing sent ICE consent requests. - this->sentConsents.clear(); - - MayStartConsentCheck(); - } - - void IceServer::MayStartOrRestartConsentCheck() - { - MS_TRACE(); - - if (!IsConsentCheckRunning()) - { - MayStartConsentCheck(); - } - else - { - MayRestartConsentCheck(); - } + this->consentCheckTimer->Start(this->consentTimeoutMs); } - void IceServer::SendConsentRequest() + void IceServer::RestartConsentCheck() { MS_TRACE(); - MS_ASSERT( - this->state == IceState::CONNECTED || this->state == IceState::COMPLETED, - "cannot send ICE consent request in state other than 'connected' or 'completed'"); - - MS_ASSERT(this->selectedTuple, "cannot send ICE consent request without a selected tuple"); - - // Here we do a trick. We generate a transactionId of 4 bytes and fill the - // latest 8 bytes of the transactionId with zeroes. - uint32_t transactionId = Utils::Crypto::GetRandomUInt(0u, UINT32_MAX); - - auto& sentContext = this->sentConsents.emplace_back(transactionId, DepLibUV::GetTimeMs()); - - // Limit max number of entries in the queue. - if (this->sentConsents.size() > MaxOngoingSentConsents) - { - MS_WARN_TAG(ice, "too many entries in the sent consents queue, removing the oldest one"); - - this->sentConsents.pop_front(); - } - - auto* request = new StunPacket( - StunPacket::Class::REQUEST, StunPacket::Method::BINDING, sentContext.transactionId, nullptr, 0); - - const std::string username = this->remoteUsernameFragment + ":" + this->usernameFragment; - - request->SetUsername(username.c_str(), username.length()); - request->SetPassword(this->remotePassword); - request->SetPriority(1u); - request->SetIceControlled(1u); - request->SetSoftware(SoftwareAttribute.c_str(), SoftwareAttribute.length()); - request->Serialize(StunSerializeBuffer); - - MS_DEBUG_DEV("sending ICE consent request"); + MS_ASSERT(IsConsentCheckSupported(), "ICE consent check not supported"); + MS_ASSERT(IsConsentCheckRunning(), "ICE consent check not running"); + MS_ASSERT(this->selectedTuple, "no selected tuple"); - this->listener->OnIceServerSendStunPacket(this, request, this->selectedTuple); - - delete request; + this->consentCheckTimer->Restart(); } - void IceServer::ConsentTerminated() + void IceServer::StopConsentCheck() { MS_TRACE(); - // There should be a selected tuple. - MS_ASSERT(this->selectedTuple, "ICE consent terminated but there is not selected tuple"); - - auto* disconnectedSelectedTuple = this->selectedTuple; - - // Update state. - this->state = IceState::DISCONNECTED; - - // Reset remote nomination. - this->remoteNomination = 0u; - - // Notify the listener. - this->listener->OnIceServerTupleRemoved(this, disconnectedSelectedTuple); - this->listener->OnIceServerDisconnected(this); - - // Clear all tuples. - for (const auto& it : this->tuples) - { - auto* storedTuple = const_cast(std::addressof(it)); - - // Notify the listener. - this->listener->OnIceServerTupleRemoved(this, storedTuple); - } - - // Clear all tuples. - // NOTE: Do it after notifying the listener since the listener may need to - // use/read the tuple being removed so we cannot free it yet. - this->tuples.clear(); - - // Unset selected tuple. - this->selectedTuple = nullptr; + MS_ASSERT(IsConsentCheckSupported(), "ICE consent check not supported"); + MS_ASSERT(IsConsentCheckRunning(), "ICE consent check not running"); - // Stop ICE consent check. - MayStopConsentCheck(); + this->consentCheckTimer->Stop(); } inline void IceServer::OnTimer(TimerHandle* timer) @@ -1145,39 +908,33 @@ namespace RTC // There should be a selected tuple. MS_ASSERT(this->selectedTuple, "ICE consent check timer fired but there is not selected tuple"); - // Check if there is at least an ongoing expired ICE consent request - // and if so move to disconnected state. + MS_WARN_TAG(ice, "ICE consent expired due to timeout, moving to 'disconnected' state"); - auto nowMs = DepLibUV::GetTimeMs(); - auto consentTimeoutMs = this->consentTimeoutMs; + // Update state. + this->state = IceState::DISCONNECTED; - auto it = std::find_if( - this->sentConsents.begin(), - this->sentConsents.end(), - [nowMs, consentTimeoutMs](auto& sentConsent) - { return nowMs - sentConsent.sentAtMs >= consentTimeoutMs; }); + // Reset remote nomination. + this->remoteNomination = 0u; - if (it == this->sentConsents.end()) + // Clear all tuples. + for (const auto& it : this->tuples) { - // Send a new ICE consent request. - SendConsentRequest(); - - /* - * The interval between ICE consent requests is varied randomly over the - * range [0.8, 1.2] times the calculated interval to prevent - * synchronization of consent checks. - */ - const uint64_t interval = - ConsentCheckIntervalMs + static_cast(Utils::Crypto::GetRandomUInt(8, 12)) / 10; - - this->consentCheckTimer->Start(interval); - } - else - { - MS_WARN_TAG(ice, "ICE consent expired due to timeout, moving to 'disconnected' state"); + auto* storedTuple = const_cast(std::addressof(it)); - ConsentTerminated(); + // Notify the listener. + this->listener->OnIceServerTupleRemoved(this, storedTuple); } + + // Clear all tuples. + // NOTE: Do it after notifying the listener since the listener may need to + // use/read the tuple being removed so we cannot free it yet. + this->tuples.clear(); + + // Unset selected tuple. + this->selectedTuple = nullptr; + + // Notify the listener. + this->listener->OnIceServerDisconnected(this); } } } // namespace RTC diff --git a/worker/src/RTC/StunPacket.cpp b/worker/src/RTC/StunPacket.cpp index 74d92bf3e6..6747afebd1 100644 --- a/worker/src/RTC/StunPacket.cpp +++ b/worker/src/RTC/StunPacket.cpp @@ -470,9 +470,7 @@ namespace RTC } StunPacket::Authentication StunPacket::CheckAuthentication( - const std::string& usernameFragment1, - const std::string& usernameFragment2, - const std::string& password) + const std::string& usernameFragment1, const std::string& password) { MS_TRACE(); @@ -506,35 +504,16 @@ namespace RTC return Authentication::BAD_MESSAGE; } - // If we have both username fragments, do the proper check. - if (!usernameFragment2.empty()) - { - const size_t usernameFragment1Len = usernameFragment1.length(); - const size_t usernameFragment2Len = usernameFragment2.length(); - - if ( - this->username.length() != usernameFragment1Len + 1 + usernameFragment2Len || - this->username.at(usernameFragment1Len) != ':' || - this->username.compare(0, usernameFragment1Len, usernameFragment1) != 0 || - this->username.compare( - usernameFragment1Len + 1, usernameFragment2Len, usernameFragment2) != 0) - { - return Authentication::UNAUTHORIZED; - } - } - // Otherwise check that USERNAME attribute begins with the first - // username fragment plus ":". - else - { - const size_t usernameFragment1Len = usernameFragment1.length(); + // Check that the USERNAME attribute begins with the first username + // fragment plus ":". + const size_t usernameFragment1Len = usernameFragment1.length(); - if ( - this->username.length() <= usernameFragment1Len || - this->username.at(usernameFragment1Len) != ':' || - this->username.compare(0, usernameFragment1Len, usernameFragment1) != 0) - { - return Authentication::UNAUTHORIZED; - } + if ( + this->username.length() <= usernameFragment1Len || + this->username.at(usernameFragment1Len) != ':' || + this->username.compare(0, usernameFragment1Len, usernameFragment1) != 0) + { + return Authentication::UNAUTHORIZED; } break; diff --git a/worker/src/RTC/WebRtcTransport.cpp b/worker/src/RTC/WebRtcTransport.cpp index 75b560cbe9..a5deb26fe0 100644 --- a/worker/src/RTC/WebRtcTransport.cpp +++ b/worker/src/RTC/WebRtcTransport.cpp @@ -441,21 +441,6 @@ namespace RTC const auto* body = request->data->body_as(); - auto iceParametersPresent = - flatbuffers::IsFieldPresent(body, FBS::WebRtcTransport::ConnectRequest::VT_ICEPARAMETERS); - - if (iceParametersPresent) - { - const auto* iceParameters = body->iceParameters(); - const std::string iceUsernameFragment = iceParameters->usernameFragment()->str(); - const std::string icePassword = iceParameters->password()->str(); - - if (!iceUsernameFragment.empty() && !icePassword.empty()) - { - this->iceServer->SetRemoteUsernameFragmentAndPassword(iceUsernameFragment, icePassword); - } - } - const auto* dtlsParameters = body->dtlsParameters(); RTC::DtlsTransport::Fingerprint dtlsRemoteFingerprint;