diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index a23b25dbc9..ceec13ab0c 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -1172,6 +1172,14 @@ class PeerConnectionObserver { // A new ICE candidate has been gathered. virtual void OnIceCandidate(const IceCandidateInterface* candidate) = 0; + // Gathering of an ICE candidate failed. + // See https://w3c.github.io/webrtc-pc/#event-icecandidateerror + // |host_candidate| is a stringified socket address. + virtual void OnIceCandidateError(const std::string& host_candidate, + const std::string& url, + int error_code, + const std::string& error_text) {} + // Ice candidates have been removed. // TODO(honghaiz): Make this a pure virtual method when all its subclasses // implement it. diff --git a/p2p/base/ice_transport_internal.h b/p2p/base/ice_transport_internal.h index 89b2107c5a..e3d98dbdd1 100644 --- a/p2p/base/ice_transport_internal.h +++ b/p2p/base/ice_transport_internal.h @@ -270,6 +270,9 @@ class RTC_EXPORT IceTransportInternal : public rtc::PacketTransportInternal { sigslot::signal2 SignalCandidateGathered; + sigslot::signal2 + SignalCandidateError; + sigslot::signal2 SignalCandidatesRemoved; diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index d469ff7dd6..d510820d75 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -180,6 +180,8 @@ void P2PTransportChannel::AddAllocatorSession( session->SignalPortsPruned.connect(this, &P2PTransportChannel::OnPortsPruned); session->SignalCandidatesReady.connect( this, &P2PTransportChannel::OnCandidatesReady); + session->SignalCandidateError.connect(this, + &P2PTransportChannel::OnCandidateError); session->SignalCandidatesRemoved.connect( this, &P2PTransportChannel::OnCandidatesRemoved); session->SignalCandidatesAllocationDone.connect( @@ -878,6 +880,13 @@ void P2PTransportChannel::OnCandidatesReady( } } +void P2PTransportChannel::OnCandidateError( + PortAllocatorSession* session, + const IceCandidateErrorEvent& event) { + RTC_DCHECK(network_thread_ == rtc::Thread::Current()); + SignalCandidateError(this, event); +} + void P2PTransportChannel::OnCandidatesAllocationDone( PortAllocatorSession* session) { RTC_DCHECK_RUN_ON(network_thread_); diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h index 5e537ce4af..0bcbe10958 100644 --- a/p2p/base/p2p_transport_channel.h +++ b/p2p/base/p2p_transport_channel.h @@ -307,6 +307,8 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal { const std::vector& ports); void OnCandidatesReady(PortAllocatorSession* session, const std::vector& candidates); + void OnCandidateError(PortAllocatorSession* session, + const IceCandidateErrorEvent& event); void OnCandidatesRemoved(PortAllocatorSession* session, const std::vector& candidates); void OnCandidatesAllocationDone(PortAllocatorSession* session); diff --git a/p2p/base/port.h b/p2p/base/port.h index a0e2606a22..8e6281f689 100644 --- a/p2p/base/port.h +++ b/p2p/base/port.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include "absl/types/optional.h" @@ -130,6 +131,23 @@ struct ProtocolAddress { bool operator!=(const ProtocolAddress& o) const { return !(*this == o); } }; +struct IceCandidateErrorEvent { + IceCandidateErrorEvent() = default; + IceCandidateErrorEvent(std::string host_candidate, + std::string url, + int error_code, + std::string error_text) + : host_candidate(std::move(host_candidate)), + url(std::move(url)), + error_code(error_code), + error_text(std::move(error_text)) {} + + std::string host_candidate; + std::string url; + int error_code = 0; + std::string error_text; +}; + typedef std::set ServerAddresses; // Represents a local communication mechanism that can be used to create @@ -227,9 +245,10 @@ class Port : public PortInterface, // Fired when candidates are discovered by the port. When all candidates // are discovered that belong to port SignalAddressReady is fired. sigslot::signal2 SignalCandidateReady; - // Provides all of the above information in one handy object. const std::vector& Candidates() const override; + // Fired when candidate discovery failed using certain server. + sigslot::signal2 SignalCandidateError; // SignalPortComplete is sent when port completes the task of candidates // allocation. diff --git a/p2p/base/port_allocator.h b/p2p/base/port_allocator.h index d0605b6bb6..d78b6cbc65 100644 --- a/p2p/base/port_allocator.h +++ b/p2p/base/port_allocator.h @@ -271,6 +271,8 @@ class RTC_EXPORT PortAllocatorSession : public sigslot::has_slots<> { SignalPortsPruned; sigslot::signal2&> SignalCandidatesReady; + sigslot::signal2 + SignalCandidateError; // Candidates should be signaled to be removed when the port that generated // the candidates is removed. sigslot::signal2&> diff --git a/p2p/base/stun.cc b/p2p/base/stun.cc index d2897747bb..3a44909a60 100644 --- a/p2p/base/stun.cc +++ b/p2p/base/stun.cc @@ -61,6 +61,7 @@ const char STUN_ERROR_REASON_SERVER_ERROR[] = "Server Error"; const char TURN_MAGIC_COOKIE_VALUE[] = {'\x72', '\xC6', '\x4B', '\xC6'}; const char EMPTY_TRANSACTION_ID[] = "0000000000000000"; const uint32_t STUN_FINGERPRINT_XOR_VALUE = 0x5354554E; +const int SERVER_NOT_REACHABLE_ERROR = 701; // StunMessage diff --git a/p2p/base/stun.h b/p2p/base/stun.h index caaa4745b4..33410b5bae 100644 --- a/p2p/base/stun.h +++ b/p2p/base/stun.h @@ -581,6 +581,9 @@ enum TurnErrorType { STUN_ERROR_WRONG_CREDENTIALS = 441, STUN_ERROR_UNSUPPORTED_PROTOCOL = 442 }; + +extern const int SERVER_NOT_REACHABLE_ERROR; + extern const char STUN_ERROR_REASON_FORBIDDEN[]; extern const char STUN_ERROR_REASON_ALLOCATION_MISMATCH[]; extern const char STUN_ERROR_REASON_WRONG_CREDENTIALS[]; diff --git a/p2p/base/stun_port.cc b/p2p/base/stun_port.cc index 65112f2fec..4662b0d17a 100644 --- a/p2p/base/stun_port.cc +++ b/p2p/base/stun_port.cc @@ -79,7 +79,10 @@ class StunBindingRequest : public StunRequest { << " reason=" << attr->reason(); } - port_->OnStunBindingOrResolveRequestFailed(server_addr_); + port_->OnStunBindingOrResolveRequestFailed( + server_addr_, attr ? attr->number() : STUN_ERROR_GLOBAL_FAILURE, + attr ? attr->reason() + : "STUN binding response with no error code attribute."); int64_t now = rtc::TimeMillis(); if (WithinLifetime(now) && @@ -93,8 +96,9 @@ class StunBindingRequest : public StunRequest { RTC_LOG(LS_ERROR) << "Binding request timed out from " << port_->GetLocalAddress().ToSensitiveString() << " (" << port_->Network()->name() << ")"; - - port_->OnStunBindingOrResolveRequestFailed(server_addr_); + port_->OnStunBindingOrResolveRequestFailed( + server_addr_, SERVER_NOT_REACHABLE_ERROR, + "STUN allocate request timed out."); } private: @@ -104,6 +108,7 @@ class StunBindingRequest : public StunRequest { int lifetime = port_->stun_keepalive_lifetime(); return lifetime < 0 || rtc::TimeDiff(now, start_time_) <= lifetime; } + UDPPort* port_; const rtc::SocketAddress server_addr_; @@ -445,7 +450,8 @@ void UDPPort::OnResolveResult(const rtc::SocketAddress& input, int error) { RTC_LOG(LS_WARNING) << ToString() << ": StunPort: stun host lookup received error " << error; - OnStunBindingOrResolveRequestFailed(input); + OnStunBindingOrResolveRequestFailed(input, SERVER_NOT_REACHABLE_ERROR, + "STUN host lookup received error."); return; } @@ -469,8 +475,10 @@ void UDPPort::SendStunBindingRequest(const rtc::SocketAddress& stun_addr) { } else { // Since we can't send stun messages to the server, we should mark this // port ready. - RTC_LOG(LS_WARNING) << "STUN server address is incompatible."; - OnStunBindingOrResolveRequestFailed(stun_addr); + const char* reason = "STUN server address is incompatible."; + RTC_LOG(LS_WARNING) << reason; + OnStunBindingOrResolveRequestFailed(stun_addr, SERVER_NOT_REACHABLE_ERROR, + reason); } } } @@ -530,7 +538,14 @@ void UDPPort::OnStunBindingRequestSucceeded( } void UDPPort::OnStunBindingOrResolveRequestFailed( - const rtc::SocketAddress& stun_server_addr) { + const rtc::SocketAddress& stun_server_addr, + int error_code, + const std::string& reason) { + rtc::StringBuilder url; + url << "stun:" << stun_server_addr.ToString(); + SignalCandidateError( + this, IceCandidateErrorEvent(GetLocalAddress().ToSensitiveString(), + url.str(), error_code, reason)); if (bind_request_failed_servers_.find(stun_server_addr) != bind_request_failed_servers_.end()) { return; diff --git a/p2p/base/stun_port.h b/p2p/base/stun_port.h index 1ee07279c9..3c4234949a 100644 --- a/p2p/base/stun_port.h +++ b/p2p/base/stun_port.h @@ -222,7 +222,9 @@ class UDPPort : public Port { const rtc::SocketAddress& stun_server_addr, const rtc::SocketAddress& stun_reflected_addr); void OnStunBindingOrResolveRequestFailed( - const rtc::SocketAddress& stun_server_addr); + const rtc::SocketAddress& stun_server_addr, + int error_code, + const std::string& reason); // Sends STUN requests to the server. void OnSendPacket(const void* data, size_t size, StunRequest* req); diff --git a/p2p/base/stun_port_unittest.cc b/p2p/base/stun_port_unittest.cc index a23c063a8f..98e41f9aa3 100644 --- a/p2p/base/stun_port_unittest.cc +++ b/p2p/base/stun_port_unittest.cc @@ -88,6 +88,8 @@ class StunPortTestBase : public ::testing::Test, public sigslot::has_slots<> { stun_port_->SignalPortComplete.connect(this, &StunPortTestBase::OnPortComplete); stun_port_->SignalPortError.connect(this, &StunPortTestBase::OnPortError); + stun_port_->SignalCandidateError.connect( + this, &StunPortTestBase::OnCandidateError); } void CreateSharedUdpPort(const rtc::SocketAddress& server_addr, @@ -145,6 +147,10 @@ class StunPortTestBase : public ::testing::Test, public sigslot::has_slots<> { done_ = true; error_ = true; } + void OnCandidateError(cricket::Port* port, + const cricket::IceCandidateErrorEvent& event) { + error_event_ = event; + } void SetKeepaliveDelay(int delay) { stun_keepalive_delay_ = delay; } void SetKeepaliveLifetime(int lifetime) { @@ -167,6 +173,9 @@ class StunPortTestBase : public ::testing::Test, public sigslot::has_slots<> { bool error_; int stun_keepalive_delay_; int stun_keepalive_lifetime_; + + protected: + cricket::IceCandidateErrorEvent error_event_; }; class StunPortTestWithRealClock : public StunPortTestBase {}; @@ -212,6 +221,15 @@ TEST_F(StunPortTest, TestPrepareAddressFail) { EXPECT_TRUE_SIMULATED_WAIT(done(), kTimeoutMs, fake_clock); EXPECT_TRUE(error()); EXPECT_EQ(0U, port()->Candidates().size()); + EXPECT_EQ_SIMULATED_WAIT(error_event_.error_code, + cricket::SERVER_NOT_REACHABLE_ERROR, kTimeoutMs, + fake_clock); + ASSERT_NE(error_event_.error_text.find("."), std::string::npos); + ASSERT_NE( + error_event_.host_candidate.find(kLocalAddr.HostAsSensitiveURIString()), + std::string::npos); + std::string server_url = "stun:" + kBadAddr.ToString(); + ASSERT_EQ(error_event_.url, server_url); } // Test that we can get an address from a STUN server specified by a hostname. @@ -237,6 +255,8 @@ TEST_F(StunPortTestWithRealClock, TestPrepareAddressHostnameFail) { EXPECT_TRUE_WAIT(done(), kTimeoutMs); EXPECT_TRUE(error()); EXPECT_EQ(0U, port()->Candidates().size()); + EXPECT_EQ_WAIT(error_event_.error_code, cricket::SERVER_NOT_REACHABLE_ERROR, + kTimeoutMs); } // This test verifies keepalive response messages don't result in @@ -303,6 +323,9 @@ TEST_F(StunPortTest, TestMultipleStunServersWithBadServer) { PrepareAddress(); EXPECT_TRUE_SIMULATED_WAIT(done(), kTimeoutMs, fake_clock); EXPECT_EQ(1U, port()->Candidates().size()); + std::string server_url = "stun:" + kBadAddr.ToString(); + ASSERT_EQ_SIMULATED_WAIT(error_event_.url, server_url, kTimeoutMs, + fake_clock); } // Test that two candidates are allocated if the two STUN servers return diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc index bcf574e332..f9104978b3 100644 --- a/p2p/base/turn_port.cc +++ b/p2p/base/turn_port.cc @@ -326,7 +326,8 @@ void TurnPort::PrepareAddress() { if (credentials_.username.empty() || credentials_.password.empty()) { RTC_LOG(LS_ERROR) << "Allocation can't be started without setting the" " TURN server credentials for the user."; - OnAllocateError(); + OnAllocateError(STUN_ERROR_UNAUTHORIZED, + "Missing TURN server credentials."); return; } @@ -343,7 +344,8 @@ void TurnPort::PrepareAddress() { RTC_LOG(LS_ERROR) << "IP address family does not match. server: " << server_address_.address.family() << " local: " << Network()->GetBestIP().family(); - OnAllocateError(); + OnAllocateError(STUN_ERROR_GLOBAL_FAILURE, + "IP address family does not match."); return; } @@ -355,7 +357,8 @@ void TurnPort::PrepareAddress() { << server_address_.address.ToSensitiveString(); if (!CreateTurnClientSocket()) { RTC_LOG(LS_ERROR) << "Failed to create TURN client socket"; - OnAllocateError(); + OnAllocateError(STUN_ERROR_GLOBAL_FAILURE, + "Failed to create TURN client socket."); return; } if (server_address_.proto == PROTO_UDP) { @@ -473,7 +476,9 @@ void TurnPort::OnSocketConnect(rtc::AsyncPacketSocket* socket) { << socket_address.ipaddr().ToString() << ", rather than an address associated with network:" << Network()->ToString() << ". Discarding TURN port."; - OnAllocateError(); + OnAllocateError( + STUN_ERROR_GLOBAL_FAILURE, + "Address not associated with the desired network interface."); return; } } @@ -501,7 +506,8 @@ void TurnPort::OnAllocateMismatch() { RTC_LOG(LS_WARNING) << ToString() << ": Giving up on the port after " << allocate_mismatch_retries_ << " retries for STUN_ERROR_ALLOCATION_MISMATCH"; - OnAllocateError(); + OnAllocateError(STUN_ERROR_ALLOCATION_MISMATCH, + "Maximum retries reached for allocation mismatch."); return; } @@ -786,7 +792,8 @@ void TurnPort::OnResolveResult(rtc::AsyncResolverInterface* resolver) { if (resolver_->GetError() != 0 && (server_address_.proto == PROTO_TCP || server_address_.proto == PROTO_TLS)) { if (!CreateTurnClientSocket()) { - OnAllocateError(); + OnAllocateError(SERVER_NOT_REACHABLE_ERROR, + "TURN host lookup received error."); } return; } @@ -800,7 +807,8 @@ void TurnPort::OnResolveResult(rtc::AsyncResolverInterface* resolver) { RTC_LOG(LS_WARNING) << ToString() << ": TURN host lookup received error " << resolver_->GetError(); error_ = resolver_->GetError(); - OnAllocateError(); + OnAllocateError(SERVER_NOT_REACHABLE_ERROR, + "TURN host lookup received error."); return; } // Signal needs both resolved and unresolved address. After signal is sent @@ -847,14 +855,20 @@ void TurnPort::OnAllocateSuccess(const rtc::SocketAddress& address, ProtoToString(server_address_.proto), // The first hop protocol. "", // TCP canddiate type, empty for turn candidates. RELAY_PORT_TYPE, GetRelayPreference(server_address_.proto), - server_priority_, ReconstructedServerUrl(), true); + server_priority_, ReconstructedServerUrl(false /* use_hostname */), + true); } -void TurnPort::OnAllocateError() { +void TurnPort::OnAllocateError(int error_code, const std::string& reason) { // We will send SignalPortError asynchronously as this can be sent during // port initialization. This way it will not be blocking other port // creation. thread()->Post(RTC_FROM_HERE, this, MSG_ALLOCATE_ERROR); + SignalCandidateError( + this, + IceCandidateErrorEvent(GetLocalAddress().ToSensitiveString(), + ReconstructedServerUrl(true /* use_hostname */), + error_code, reason)); } void TurnPort::OnRefreshError() { @@ -887,7 +901,7 @@ void TurnPort::Release() { void TurnPort::Close() { if (!ready()) { - OnAllocateError(); + OnAllocateError(SERVER_NOT_REACHABLE_ERROR, ""); } request_manager_.Clear(); // Stop the port from creating new connections. @@ -941,7 +955,8 @@ void TurnPort::OnMessage(rtc::Message* message) { } void TurnPort::OnAllocateRequestTimeout() { - OnAllocateError(); + OnAllocateError(SERVER_NOT_REACHABLE_ERROR, + "TURN allocate request timed out."); } void TurnPort::HandleDataIndication(const char* data, @@ -1249,7 +1264,7 @@ bool TurnPort::SetEntryChannelId(const rtc::SocketAddress& address, return true; } -std::string TurnPort::ReconstructedServerUrl() { +std::string TurnPort::ReconstructedServerUrl(bool use_hostname) { // draft-petithuguenin-behave-turn-uris-01 // turnURI = scheme ":" turn-host [ ":" turn-port ] // [ "?transport=" transport ] @@ -1272,8 +1287,10 @@ std::string TurnPort::ReconstructedServerUrl() { break; } rtc::StringBuilder url; - url << scheme << ":" << server_address_.address.ipaddr().ToString() << ":" - << server_address_.address.port() << "?transport=" << transport; + url << scheme << ":" + << (use_hostname ? server_address_.address.hostname() + : server_address_.address.ipaddr().ToString()) + << ":" << server_address_.address.port() << "?transport=" << transport; return url.Release(); } @@ -1388,7 +1405,8 @@ void TurnAllocateRequest::OnErrorResponse(StunMessage* response) { << ": Received TURN allocate error response, id=" << rtc::hex_encode(id()) << ", code=" << error_code << ", rtt=" << Elapsed(); - port_->OnAllocateError(); + const StunErrorCodeAttribute* attr = response->GetErrorCode(); + port_->OnAllocateError(error_code, attr ? attr->reason() : ""); } } @@ -1404,7 +1422,8 @@ void TurnAllocateRequest::OnAuthChallenge(StunMessage* response, int code) { RTC_LOG(LS_WARNING) << port_->ToString() << ": Failed to authenticate with the server " "after challenge."; - port_->OnAllocateError(); + const StunErrorCodeAttribute* attr = response->GetErrorCode(); + port_->OnAllocateError(STUN_ERROR_UNAUTHORIZED, attr ? attr->reason() : ""); return; } @@ -1437,7 +1456,7 @@ void TurnAllocateRequest::OnTryAlternate(StunMessage* response, int code) { // According to RFC 5389 section 11, there are use cases where // authentication of response is not possible, we're not validating // message integrity. - + const StunErrorCodeAttribute* error_code_attr = response->GetErrorCode(); // Get the alternate server address attribute value. const StunAddressAttribute* alternate_server_attr = response->GetAddress(STUN_ATTR_ALTERNATE_SERVER); @@ -1445,11 +1464,13 @@ void TurnAllocateRequest::OnTryAlternate(StunMessage* response, int code) { RTC_LOG(LS_WARNING) << port_->ToString() << ": Missing STUN_ATTR_ALTERNATE_SERVER " "attribute in try alternate error response"; - port_->OnAllocateError(); + port_->OnAllocateError(STUN_ERROR_TRY_ALTERNATE, + error_code_attr ? error_code_attr->reason() : ""); return; } if (!port_->SetAlternateServer(alternate_server_attr->GetAddress())) { - port_->OnAllocateError(); + port_->OnAllocateError(STUN_ERROR_TRY_ALTERNATE, + error_code_attr ? error_code_attr->reason() : ""); return; } diff --git a/p2p/base/turn_port.h b/p2p/base/turn_port.h index e929d3e7cc..5edbb1cc58 100644 --- a/p2p/base/turn_port.h +++ b/p2p/base/turn_port.h @@ -310,7 +310,7 @@ class TurnPort : public Port { void OnStunAddress(const rtc::SocketAddress& address); void OnAllocateSuccess(const rtc::SocketAddress& address, const rtc::SocketAddress& stun_address); - void OnAllocateError(); + void OnAllocateError(int error_code, const std::string& reason); void OnAllocateRequestTimeout(); void HandleDataIndication(const char* data, @@ -349,7 +349,7 @@ class TurnPort : public Port { bool FailAndPruneConnection(const rtc::SocketAddress& address); // Reconstruct the URL of the server which the candidate is gathered from. - std::string ReconstructedServerUrl(); + std::string ReconstructedServerUrl(bool use_hostname); void TurnCustomizerMaybeModifyOutgoingStunMessage(StunMessage* message); bool TurnCustomizerAllowChannelData(const void* data, diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc index e713b2a901..b51a1266b1 100644 --- a/p2p/base/turn_port_unittest.cc +++ b/p2p/base/turn_port_unittest.cc @@ -66,6 +66,7 @@ static const SocketAddress kTurnIPv6IntAddr( static const SocketAddress kTurnUdpIPv6IntAddr( "2400:4030:1:2c00:be30:abcd:efab:cdef", cricket::TURN_SERVER_PORT); +static const SocketAddress kTurnInvalidAddr("www.google.invalid", 3478); static const char kCandidateFoundation[] = "foundation"; static const char kIceUfrag1[] = "TESTICEUFRAG0001"; @@ -176,6 +177,10 @@ class TurnPortTest : public ::testing::Test, void OnTurnPortComplete(Port* port) { turn_ready_ = true; } void OnTurnPortError(Port* port) { turn_error_ = true; } + void OnCandidateError(Port* port, + const cricket::IceCandidateErrorEvent& event) { + error_event_ = event; + } void OnTurnUnknownAddress(PortInterface* port, const SocketAddress& addr, ProtocolType proto, @@ -316,6 +321,8 @@ class TurnPortTest : public ::testing::Test, turn_port_->SignalPortComplete.connect(this, &TurnPortTest::OnTurnPortComplete); turn_port_->SignalPortError.connect(this, &TurnPortTest::OnTurnPortError); + turn_port_->SignalCandidateError.connect(this, + &TurnPortTest::OnCandidateError); turn_port_->SignalUnknownAddress.connect( this, &TurnPortTest::OnTurnUnknownAddress); turn_port_->SignalCreatePermissionResult.connect( @@ -755,6 +762,7 @@ class TurnPortTest : public ::testing::Test, std::vector udp_packets_; rtc::PacketOptions options; std::unique_ptr turn_customizer_; + cricket::IceCandidateErrorEvent error_event_; }; TEST_F(TurnPortTest, TestTurnPortType) { @@ -802,6 +810,17 @@ TEST_F(TurnPortTest, TestTurnAllocate) { EXPECT_NE(0, turn_port_->Candidates()[0].address().port()); } +// Test bad credentials. +TEST_F(TurnPortTest, TestTurnBadCredentials) { + CreateTurnPort(kTurnUsername, "bad", kTurnUdpProtoAddr); + turn_port_->PrepareAddress(); + EXPECT_TRUE_SIMULATED_WAIT(turn_error_, kSimulatedRtt * 3, fake_clock_); + ASSERT_EQ(0U, turn_port_->Candidates().size()); + EXPECT_EQ_SIMULATED_WAIT(error_event_.error_code, STUN_ERROR_UNAUTHORIZED, + kSimulatedRtt * 3, fake_clock_); + EXPECT_EQ(error_event_.error_text, "Unauthorized"); +} + // Testing a normal UDP allocation using TCP connection. TEST_F(TurnPortTest, TestTurnTcpAllocate) { turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP); @@ -861,6 +880,15 @@ TEST_F(TurnPortTest, // Shouldn't take more than 1 RTT to realize the bound address isn't the one // expected. EXPECT_TRUE_SIMULATED_WAIT(turn_error_, kSimulatedRtt, fake_clock_); + EXPECT_EQ_SIMULATED_WAIT(error_event_.error_code, STUN_ERROR_GLOBAL_FAILURE, + kSimulatedRtt, fake_clock_); + ASSERT_NE(error_event_.error_text.find("."), std::string::npos); + ASSERT_NE( + error_event_.host_candidate.find(kLocalAddr2.HostAsSensitiveURIString()), + std::string::npos); + std::string server_url = + "turn:" + kTurnTcpIntAddr.ToString() + "?transport=tcp"; + ASSERT_EQ(error_event_.url, server_url); } // A caveat for the above logic: if the socket ends up bound to one of the IPs @@ -921,14 +949,18 @@ TEST_F(TurnPortTest, TCPPortNotDiscardedIfBoundToTemporaryIP) { TEST_F(TurnPortTest, TestTurnTcpOnAddressResolveFailure) { turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP); CreateTurnPort(kTurnUsername, kTurnPassword, - ProtocolAddress(rtc::SocketAddress("www.google.invalid", 3478), - PROTO_TCP)); + ProtocolAddress(kTurnInvalidAddr, PROTO_TCP)); turn_port_->PrepareAddress(); EXPECT_TRUE_WAIT(turn_error_, kResolverTimeout); // As VSS doesn't provide a DNS resolution, name resolve will fail. TurnPort // will proceed in creating a TCP socket which will fail as there is no // server on the above domain and error will be set to SOCKET_ERROR. EXPECT_EQ(SOCKET_ERROR, turn_port_->error()); + EXPECT_EQ_SIMULATED_WAIT(error_event_.error_code, SERVER_NOT_REACHABLE_ERROR, + kSimulatedRtt, fake_clock_); + std::string server_url = + "turn:" + kTurnInvalidAddr.ToString() + "?transport=tcp"; + ASSERT_EQ(error_event_.url, server_url); } // Testing turn port will attempt to create TLS socket on address resolution @@ -936,8 +968,7 @@ TEST_F(TurnPortTest, TestTurnTcpOnAddressResolveFailure) { TEST_F(TurnPortTest, TestTurnTlsOnAddressResolveFailure) { turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TLS); CreateTurnPort(kTurnUsername, kTurnPassword, - ProtocolAddress(rtc::SocketAddress("www.google.invalid", 3478), - PROTO_TLS)); + ProtocolAddress(kTurnInvalidAddr, PROTO_TLS)); turn_port_->PrepareAddress(); EXPECT_TRUE_WAIT(turn_error_, kResolverTimeout); EXPECT_EQ(SOCKET_ERROR, turn_port_->error()); @@ -947,8 +978,7 @@ TEST_F(TurnPortTest, TestTurnTlsOnAddressResolveFailure) { // and return allocate failure. TEST_F(TurnPortTest, TestTurnUdpOnAddressResolveFailure) { CreateTurnPort(kTurnUsername, kTurnPassword, - ProtocolAddress(rtc::SocketAddress("www.google.invalid", 3478), - PROTO_UDP)); + ProtocolAddress(kTurnInvalidAddr, PROTO_UDP)); turn_port_->PrepareAddress(); EXPECT_TRUE_WAIT(turn_error_, kResolverTimeout); // Error from turn port will not be socket error. diff --git a/p2p/client/basic_port_allocator.cc b/p2p/client/basic_port_allocator.cc index 4f418ee7f6..b1f147dcdc 100644 --- a/p2p/client/basic_port_allocator.cc +++ b/p2p/client/basic_port_allocator.cc @@ -953,6 +953,8 @@ void BasicPortAllocatorSession::AddAllocatedPort(Port* port, port->SignalCandidateReady.connect( this, &BasicPortAllocatorSession::OnCandidateReady); + port->SignalCandidateError.connect( + this, &BasicPortAllocatorSession::OnCandidateError); port->SignalPortComplete.connect(this, &BasicPortAllocatorSession::OnPortComplete); port->SignalDestroyed.connect(this, @@ -1024,6 +1026,15 @@ void BasicPortAllocatorSession::OnCandidateReady(Port* port, } } +void BasicPortAllocatorSession::OnCandidateError( + Port* port, + const IceCandidateErrorEvent& event) { + RTC_DCHECK_RUN_ON(network_thread_); + RTC_DCHECK(FindPort(port)); + + SignalCandidateError(this, event); +} + Port* BasicPortAllocatorSession::GetBestTurnPortForNetwork( const std::string& network_name) const { RTC_DCHECK_RUN_ON(network_thread_); diff --git a/p2p/client/basic_port_allocator.h b/p2p/client/basic_port_allocator.h index 26eea1ef52..13611e724f 100644 --- a/p2p/client/basic_port_allocator.h +++ b/p2p/client/basic_port_allocator.h @@ -231,6 +231,7 @@ class RTC_EXPORT BasicPortAllocatorSession : public PortAllocatorSession, AllocationSequence* seq, bool prepare_address); void OnCandidateReady(Port* port, const Candidate& c); + void OnCandidateError(Port* port, const IceCandidateErrorEvent& event); void OnPortComplete(Port* port); void OnPortError(Port* port); void OnProtocolEnabled(AllocationSequence* seq, ProtocolType proto); diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 93949c6657..56d9a47ed7 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -523,6 +523,8 @@ JsepTransportController::CreateDtlsTransport( this, &JsepTransportController::OnTransportGatheringState_n); dtls->ice_transport()->SignalCandidateGathered.connect( this, &JsepTransportController::OnTransportCandidateGathered_n); + dtls->ice_transport()->SignalCandidateError.connect( + this, &JsepTransportController::OnTransportCandidateError_n); dtls->ice_transport()->SignalCandidatesRemoved.connect( this, &JsepTransportController::OnTransportCandidatesRemoved_n); dtls->ice_transport()->SignalRoleConflict.connect( @@ -1377,6 +1379,14 @@ void JsepTransportController::OnTransportCandidateGathered_n( }); } +void JsepTransportController::OnTransportCandidateError_n( + cricket::IceTransportInternal* transport, + const cricket::IceCandidateErrorEvent& event) { + RTC_DCHECK(network_thread_->IsCurrent()); + + invoker_.AsyncInvoke(RTC_FROM_HERE, signaling_thread_, + [this, event] { SignalIceCandidateError(event); }); +} void JsepTransportController::OnTransportCandidatesRemoved_n( cricket::IceTransportInternal* transport, const cricket::Candidates& candidates) { diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index 995c703ce1..fcae15373f 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -235,6 +235,9 @@ class JsepTransportController : public sigslot::has_slots<> { sigslot::signal2&> SignalIceCandidatesGathered; + sigslot::signal1 + SignalIceCandidateError; + sigslot::signal1&> SignalIceCandidatesRemoved; @@ -375,6 +378,9 @@ class JsepTransportController : public sigslot::has_slots<> { void OnTransportGatheringState_n(cricket::IceTransportInternal* transport); void OnTransportCandidateGathered_n(cricket::IceTransportInternal* transport, const cricket::Candidate& candidate); + void OnTransportCandidateError_n( + cricket::IceTransportInternal* transport, + const cricket::IceCandidateErrorEvent& event); void OnTransportCandidatesRemoved_n(cricket::IceTransportInternal* transport, const cricket::Candidates& candidates); void OnTransportRoleConflict_n(cricket::IceTransportInternal* transport); diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index fde0433278..b94b883989 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -1075,6 +1075,8 @@ bool PeerConnection::Initialize( this, &PeerConnection::OnTransportControllerGatheringState); transport_controller_->SignalIceCandidatesGathered.connect( this, &PeerConnection::OnTransportControllerCandidatesGathered); + transport_controller_->SignalIceCandidateError.connect( + this, &PeerConnection::OnTransportControllerCandidateError); transport_controller_->SignalIceCandidatesRemoved.connect( this, &PeerConnection::OnTransportControllerCandidatesRemoved); transport_controller_->SignalDtlsHandshakeError.connect( @@ -4169,6 +4171,16 @@ void PeerConnection::OnIceCandidate( Observer()->OnIceCandidate(candidate.get()); } +void PeerConnection::OnIceCandidateError(const std::string& host_candidate, + const std::string& url, + int error_code, + const std::string& error_text) { + if (IsClosed()) { + return; + } + Observer()->OnIceCandidateError(host_candidate, url, error_code, error_text); +} + void PeerConnection::OnIceCandidatesRemoved( const std::vector& candidates) { if (IsClosed()) { @@ -6107,6 +6119,12 @@ void PeerConnection::OnTransportControllerCandidatesGathered( } } +void PeerConnection::OnTransportControllerCandidateError( + const cricket::IceCandidateErrorEvent& event) { + OnIceCandidateError(event.host_candidate, event.url, event.error_code, + event.error_text); +} + void PeerConnection::OnTransportControllerCandidatesRemoved( const std::vector& candidates) { // Sanity check. diff --git a/pc/peer_connection.h b/pc/peer_connection.h index d2fb768a33..e7362d9f39 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -413,12 +413,18 @@ class PeerConnection : public PeerConnectionInternal, PeerConnectionInterface::PeerConnectionState new_state) RTC_RUN_ON(signaling_thread()); - // Called any time the IceGatheringState changes + // Called any time the IceGatheringState changes. void OnIceGatheringChange(IceGatheringState new_state) RTC_RUN_ON(signaling_thread()); // New ICE candidate has been gathered. void OnIceCandidate(std::unique_ptr candidate) RTC_RUN_ON(signaling_thread()); + // Gathering of an ICE candidate failed. + void OnIceCandidateError(const std::string& host_candidate, + const std::string& url, + int error_code, + const std::string& error_text) + RTC_RUN_ON(signaling_thread()); // Some local ICE candidates have been removed. void OnIceCandidatesRemoved(const std::vector& candidates) RTC_RUN_ON(signaling_thread()); @@ -1000,6 +1006,9 @@ class PeerConnection : public PeerConnectionInternal, const std::string& transport_name, const std::vector& candidates) RTC_RUN_ON(signaling_thread()); + void OnTransportControllerCandidateError( + const cricket::IceCandidateErrorEvent& event) + RTC_RUN_ON(signaling_thread()); void OnTransportControllerCandidatesRemoved( const std::vector& candidates) RTC_RUN_ON(signaling_thread()); diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc index 395c43768d..3eeeeb8647 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -565,6 +565,9 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, const cricket::Candidate& last_candidate_gathered() const { return last_candidate_gathered_; } + const cricket::IceCandidateErrorEvent& error_event() const { + return error_event_; + } // Sets the mDNS responder for the owned fake network manager and keeps a // reference to the responder. @@ -958,6 +961,13 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, SendIceMessage(candidate->sdp_mid(), candidate->sdp_mline_index(), ice_sdp); last_candidate_gathered_ = candidate->candidate(); } + void OnIceCandidateError(const std::string& host_candidate, + const std::string& url, + int error_code, + const std::string& error_text) override { + error_event_ = cricket::IceCandidateErrorEvent(host_candidate, url, + error_code, error_text); + } void OnDataChannel( rtc::scoped_refptr data_channel) override { RTC_LOG(LS_INFO) << debug_name_ << ": OnDataChannel"; @@ -990,6 +1000,7 @@ class PeerConnectionWrapper : public webrtc::PeerConnectionObserver, int signaling_delay_ms_ = 0; bool signal_ice_candidates_ = true; cricket::Candidate last_candidate_gathered_; + cricket::IceCandidateErrorEvent error_event_; // Store references to the video sources we've created, so that we can stop // them, if required. @@ -5154,6 +5165,46 @@ TEST_P(PeerConnectionIntegrationTest, RegatherAfterChangingIceTransportType) { ClosePeerConnections(); } +TEST_P(PeerConnectionIntegrationTest, OnIceCandidateError) { + webrtc::test::ScopedFieldTrials field_trials( + "WebRTC-GatherOnCandidateFilterChanged/Enabled/"); + static const rtc::SocketAddress turn_server_internal_address{"88.88.88.0", + 3478}; + static const rtc::SocketAddress turn_server_external_address{"88.88.88.1", 0}; + + CreateTurnServer(turn_server_internal_address, turn_server_external_address); + + webrtc::PeerConnectionInterface::IceServer ice_server; + ice_server.urls.push_back("turn:88.88.88.0:3478"); + ice_server.username = "test"; + ice_server.password = "123"; + + PeerConnectionInterface::RTCConfiguration caller_config; + caller_config.servers.push_back(ice_server); + caller_config.type = webrtc::PeerConnectionInterface::kRelay; + caller_config.continual_gathering_policy = PeerConnection::GATHER_CONTINUALLY; + + PeerConnectionInterface::RTCConfiguration callee_config; + callee_config.servers.push_back(ice_server); + callee_config.type = webrtc::PeerConnectionInterface::kRelay; + callee_config.continual_gathering_policy = PeerConnection::GATHER_CONTINUALLY; + + ASSERT_TRUE( + CreatePeerConnectionWrappersWithConfig(caller_config, callee_config)); + + // Do normal offer/answer and wait for ICE to complete. + ConnectFakeSignaling(); + caller()->AddAudioVideoTracks(); + callee()->AddAudioVideoTracks(); + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + EXPECT_EQ_WAIT(401, caller()->error_event().error_code, kDefaultTimeout); + EXPECT_EQ("Unauthorized", caller()->error_event().error_text); + EXPECT_EQ("turn:88.88.88.0:3478?transport=udp", caller()->error_event().url); + EXPECT_NE(std::string::npos, + caller()->error_event().host_candidate.find(":")); +} + INSTANTIATE_TEST_SUITE_P(PeerConnectionIntegrationTest, PeerConnectionIntegrationTest, Values(SdpSemantics::kPlanB,