diff --git a/api/transport/stun.h b/api/transport/stun.h index ef5b08ecc6..8300f75505 100644 --- a/api/transport/stun.h +++ b/api/transport/stun.h @@ -95,13 +95,17 @@ enum StunAddressFamily { // These are the types of STUN error codes defined in RFC 5389. enum StunErrorCode { + // Not an actual error from RFC 5389 and not emitted via icecandidateerror. + STUN_ERROR_NOT_AN_ERROR = 0, STUN_ERROR_TRY_ALTERNATE = 300, STUN_ERROR_BAD_REQUEST = 400, STUN_ERROR_UNAUTHORIZED = 401, STUN_ERROR_UNKNOWN_ATTRIBUTE = 420, STUN_ERROR_STALE_NONCE = 438, STUN_ERROR_SERVER_ERROR = 500, - STUN_ERROR_GLOBAL_FAILURE = 600 + STUN_ERROR_GLOBAL_FAILURE = 600, + // https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnectioniceerrorevent-errorcode + STUN_ERROR_SERVER_NOT_REACHABLE = 701, }; // Strings for the error codes above. @@ -687,7 +691,8 @@ enum TurnErrorType { STUN_ERROR_UNSUPPORTED_PROTOCOL = 442 }; -extern const int SERVER_NOT_REACHABLE_ERROR; +[[deprecated("Use STUN_ERROR_SERVER_NOT_REACHABLE")]] extern const int + SERVER_NOT_REACHABLE_ERROR; extern const char STUN_ERROR_REASON_FORBIDDEN[]; extern const char STUN_ERROR_REASON_ALLOCATION_MISMATCH[]; diff --git a/p2p/base/stun_port.cc b/p2p/base/stun_port.cc index fd1eb72a92..bdd8830a29 100644 --- a/p2p/base/stun_port.cc +++ b/p2p/base/stun_port.cc @@ -102,7 +102,7 @@ class StunBindingRequest : public StunRequest { << port_->GetLocalAddress().ToSensitiveString() << " (" << port_->Network()->name() << ")"; port_->OnStunBindingOrResolveRequestFailed( - server_addr_, SERVER_NOT_REACHABLE_ERROR, + server_addr_, STUN_ERROR_SERVER_NOT_REACHABLE, "STUN binding request timed out."); } @@ -442,7 +442,7 @@ void UDPPort::OnResolveResult(const rtc::SocketAddress& input, int error) { RTC_LOG(LS_WARNING) << ToString() << ": StunPort: stun host lookup received error " << error; - OnStunBindingOrResolveRequestFailed(input, SERVER_NOT_REACHABLE_ERROR, + OnStunBindingOrResolveRequestFailed(input, STUN_ERROR_SERVER_NOT_REACHABLE, "STUN host lookup received error."); return; } @@ -466,11 +466,13 @@ void UDPPort::SendStunBindingRequest(const rtc::SocketAddress& stun_addr) { new StunBindingRequest(this, stun_addr, rtc::TimeMillis())); } else { // Since we can't send stun messages to the server, we should mark this - // port ready. - const char* reason = "STUN server address is incompatible."; - RTC_LOG(LS_WARNING) << reason; - OnStunBindingOrResolveRequestFailed(stun_addr, SERVER_NOT_REACHABLE_ERROR, - reason); + // port ready. This is not an error but similar to ignoring + // a mismatch of th address family when pairing candidates. + RTC_LOG(LS_WARNING) << ToString() + << ": STUN server address is incompatible."; + OnStunBindingOrResolveRequestFailed( + stun_addr, STUN_ERROR_NOT_AN_ERROR, + "STUN server address is incompatible."); } } } @@ -534,12 +536,14 @@ void UDPPort::OnStunBindingOrResolveRequestFailed( const rtc::SocketAddress& stun_server_addr, int error_code, absl::string_view reason) { - rtc::StringBuilder url; - url << "stun:" << stun_server_addr.ToString(); - SignalCandidateError( - this, IceCandidateErrorEvent(GetLocalAddress().HostAsSensitiveURIString(), - GetLocalAddress().port(), url.str(), - error_code, reason)); + if (error_code != STUN_ERROR_NOT_AN_ERROR) { + rtc::StringBuilder url; + url << "stun:" << stun_server_addr.ToString(); + SignalCandidateError( + this, IceCandidateErrorEvent( + GetLocalAddress().HostAsSensitiveURIString(), + GetLocalAddress().port(), 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_unittest.cc b/p2p/base/stun_port_unittest.cc index 842f3e49c6..08c224ad86 100644 --- a/p2p/base/stun_port_unittest.cc +++ b/p2p/base/stun_port_unittest.cc @@ -44,6 +44,7 @@ static const SocketAddress kStunAddr2("127.0.0.1", 4000); static const SocketAddress kStunAddr3("127.0.0.1", 3000); static const SocketAddress kIPv6StunAddr1("::1", 5000); static const SocketAddress kBadAddr("0.0.0.1", 5000); +static const SocketAddress kIPv6BadAddr("::ffff:0:1", 5000); static const SocketAddress kValidHostnameAddr("valid-hostname", 5000); static const SocketAddress kBadHostnameAddr("not-a-real-hostname", 5000); // STUN timeout (with all retries) is cricket::STUN_TOTAL_TIMEOUT. @@ -300,13 +301,24 @@ TEST_F(StunPortTest, TestPrepareAddressFail) { EXPECT_TRUE(error()); EXPECT_EQ(0U, port()->Candidates().size()); EXPECT_EQ_SIMULATED_WAIT(error_event_.error_code, - cricket::SERVER_NOT_REACHABLE_ERROR, kTimeoutMs, + cricket::STUN_ERROR_SERVER_NOT_REACHABLE, kTimeoutMs, fake_clock); - ASSERT_NE(error_event_.error_text.find('.'), std::string::npos); - ASSERT_NE(error_event_.address.find(kLocalAddr.HostAsSensitiveURIString()), + EXPECT_NE(error_event_.error_text.find('.'), std::string::npos); + EXPECT_NE(error_event_.address.find(kLocalAddr.HostAsSensitiveURIString()), std::string::npos); std::string server_url = "stun:" + kBadAddr.ToString(); - ASSERT_EQ(error_event_.url, server_url); + EXPECT_EQ(error_event_.url, server_url); +} + +// Test that we fail without emitting an error if we try to get an address from +// a STUN server with a different address family. IPv4 local, IPv6 STUN. +TEST_F(StunPortTest, TestServerAddressFamilyMismatch) { + CreateStunPort(kIPv6StunAddr1); + PrepareAddress(); + EXPECT_TRUE_SIMULATED_WAIT(done(), kTimeoutMs, fake_clock); + EXPECT_TRUE(error()); + EXPECT_EQ(0U, port()->Candidates().size()); + EXPECT_EQ(0, error_event_.error_code); } class StunPortWithMockDnsResolverTest : public StunPortTest { @@ -383,8 +395,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); + EXPECT_EQ_WAIT(error_event_.error_code, + cricket::STUN_ERROR_SERVER_NOT_REACHABLE, kTimeoutMs); } // This test verifies keepalive response messages don't result in @@ -658,20 +670,31 @@ TEST_F(StunIPv6PortTest, TestPrepareAddress) { // Test that we fail properly if we can't get an address. TEST_F(StunIPv6PortTest, TestPrepareAddressFail) { - CreateStunPort(kBadAddr); + CreateStunPort(kIPv6BadAddr); PrepareAddress(); 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, + cricket::STUN_ERROR_SERVER_NOT_REACHABLE, kTimeoutMs, fake_clock); - ASSERT_NE(error_event_.error_text.find('.'), std::string::npos); - ASSERT_NE( + EXPECT_NE(error_event_.error_text.find('.'), std::string::npos); + EXPECT_NE( error_event_.address.find(kIPv6LocalAddr.HostAsSensitiveURIString()), std::string::npos); - std::string server_url = "stun:" + kBadAddr.ToString(); - ASSERT_EQ(error_event_.url, server_url); + std::string server_url = "stun:" + kIPv6BadAddr.ToString(); + EXPECT_EQ(error_event_.url, server_url); +} + +// Test that we fail without emitting an error if we try to get an address from +// a STUN server with a different address family. IPv6 local, IPv4 STUN. +TEST_F(StunIPv6PortTest, TestServerAddressFamilyMismatch) { + CreateStunPort(kStunAddr1); + PrepareAddress(); + EXPECT_TRUE_SIMULATED_WAIT(done(), kTimeoutMs, fake_clock); + EXPECT_TRUE(error()); + EXPECT_EQ(0U, port()->Candidates().size()); + EXPECT_EQ(0, error_event_.error_code); } // Test that we handle hostname lookup failures properly with a real clock. @@ -681,8 +704,8 @@ TEST_F(StunIPv6PortTestWithRealClock, 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); + EXPECT_EQ_WAIT(error_event_.error_code, + cricket::STUN_ERROR_SERVER_NOT_REACHABLE, kTimeoutMs); } class StunIPv6PortTestWithMockDnsResolver : public StunIPv6PortTest { diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc index 1745019ca8..4dba2a5708 100644 --- a/p2p/base/turn_port.cc +++ b/p2p/base/turn_port.cc @@ -391,7 +391,8 @@ std::vector TurnPort::GetTlsEllipticCurves() const { void TurnPort::PrepareAddress() { if (credentials_.username.empty() || credentials_.password.empty()) { - RTC_LOG(LS_ERROR) << "Allocation can't be started without setting the" + RTC_LOG(LS_ERROR) << ToString() + << ": Allocation can't be started without setting the" " TURN server credentials for the user."; OnAllocateError(STUN_ERROR_UNAUTHORIZED, "Missing TURN server credentials."); @@ -406,7 +407,8 @@ void TurnPort::PrepareAddress() { if (!AllowedTurnPort(server_address_.address.port(), &field_trials())) { // This can only happen after a 300 ALTERNATE SERVER, since the port can't // be created with a disallowed port number. - RTC_LOG(LS_ERROR) << "Attempt to start allocation with disallowed port# " + RTC_LOG(LS_ERROR) << ToString() + << ": Attempt to start allocation with disallowed port# " << server_address_.address.port(); OnAllocateError(STUN_ERROR_SERVER_ERROR, "Attempt to start allocation to a disallowed port"); @@ -417,11 +419,12 @@ void TurnPort::PrepareAddress() { } else { // If protocol family of server address doesn't match with local, return. if (!IsCompatibleAddress(server_address_.address)) { - RTC_LOG(LS_ERROR) << "IP address family does not match. server: " + RTC_LOG(LS_ERROR) << ToString() + << ": IP address family does not match. Server: " << server_address_.address.family() << " local: " << Network()->GetBestIP().family(); - OnAllocateError(STUN_ERROR_GLOBAL_FAILURE, - "IP address family does not match."); + OnAllocateError(STUN_ERROR_NOT_AN_ERROR, + "TURN server address is incompatible."); return; } @@ -433,8 +436,9 @@ void TurnPort::PrepareAddress() { << ProtoToString(server_address_.proto) << " @ " << server_address_.address.ToSensitiveNameAndAddressString(); if (!CreateTurnClientSocket()) { - RTC_LOG(LS_ERROR) << "Failed to create TURN client socket"; - OnAllocateError(SERVER_NOT_REACHABLE_ERROR, + RTC_LOG(LS_ERROR) << ToString() + << ": Failed to create TURN client socket"; + OnAllocateError(STUN_ERROR_SERVER_NOT_REACHABLE, "Failed to create TURN client socket."); return; } @@ -540,26 +544,26 @@ void TurnPort::OnSocketConnect(rtc::AsyncPacketSocket* socket) { return socket_address.ipaddr() == addr; })) { if (socket->GetLocalAddress().IsLoopbackIP()) { - RTC_LOG(LS_WARNING) << "Socket is bound to the address:" + RTC_LOG(LS_WARNING) << ToString() << ": Socket is bound to the address:" << socket_address.ToSensitiveNameAndAddressString() << ", rather than an address associated with network:" << Network()->ToString() << ". Still allowing it since it's localhost."; } else if (IPIsAny(Network()->GetBestIP())) { RTC_LOG(LS_WARNING) - << "Socket is bound to the address:" + << ToString() << ": Socket is bound to the address:" << socket_address.ToSensitiveNameAndAddressString() << ", rather than an address associated with network:" << Network()->ToString() << ". Still allowing it since it's the 'any' address" ", possibly caused by multiple_routes being disabled."; } else { - RTC_LOG(LS_WARNING) << "Socket is bound to the address:" + RTC_LOG(LS_WARNING) << ToString() << ": Socket is bound to the address:" << socket_address.ToSensitiveNameAndAddressString() << ", rather than an address associated with network:" << Network()->ToString() << ". Discarding TURN port."; OnAllocateError( - STUN_ERROR_GLOBAL_FAILURE, + STUN_ERROR_SERVER_NOT_REACHABLE, "Address not associated with the desired network interface."); return; } @@ -835,7 +839,8 @@ bool TurnPort::SetAlternateServer(const rtc::SocketAddress& address) { // If protocol family of server address doesn't match with local, return. if (!IsCompatibleAddress(address)) { - RTC_LOG(LS_WARNING) << "Server IP address family does not match with " + RTC_LOG(LS_WARNING) << ToString() + << ": Server IP address family does not match with " "local host address family type"; return false; } @@ -876,7 +881,7 @@ void TurnPort::ResolveTurnAddress(const rtc::SocketAddress& address) { if (result.GetError() != 0 && (server_address_.proto == PROTO_TCP || server_address_.proto == PROTO_TLS)) { if (!CreateTurnClientSocket()) { - OnAllocateError(SERVER_NOT_REACHABLE_ERROR, + OnAllocateError(STUN_ERROR_SERVER_NOT_REACHABLE, "TURN host lookup received error."); } return; @@ -891,7 +896,7 @@ void TurnPort::ResolveTurnAddress(const rtc::SocketAddress& address) { RTC_LOG(LS_WARNING) << ToString() << ": TURN host lookup received error " << result.GetError(); error_ = result.GetError(); - OnAllocateError(SERVER_NOT_REACHABLE_ERROR, + OnAllocateError(STUN_ERROR_SERVER_NOT_REACHABLE, "TURN host lookup received error."); return; } @@ -955,8 +960,11 @@ void TurnPort::OnAllocateError(int error_code, absl::string_view reason) { address.clear(); port = 0; } - SignalCandidateError(this, IceCandidateErrorEvent(address, port, server_url_, - error_code, reason)); + if (error_code != STUN_ERROR_NOT_AN_ERROR) { + SignalCandidateError( + this, + IceCandidateErrorEvent(address, port, server_url_, error_code, reason)); + } } void TurnPort::OnRefreshError() { @@ -990,7 +998,7 @@ void TurnPort::Release() { void TurnPort::Close() { if (!ready()) { OnAllocateError( - SERVER_NOT_REACHABLE_ERROR, + STUN_ERROR_SERVER_NOT_REACHABLE, GetProtocol() != PROTO_UDP ? "Failed to establish connection" : ""); } request_manager_.Clear(); @@ -1037,7 +1045,7 @@ void TurnPort::TryAlternateServer() { } void TurnPort::OnAllocateRequestTimeout() { - OnAllocateError(SERVER_NOT_REACHABLE_ERROR, + OnAllocateError(STUN_ERROR_SERVER_NOT_REACHABLE, "TURN allocate request timed out."); } @@ -1212,7 +1220,8 @@ bool TurnPort::UpdateNonce(StunMessage* response) { const StunByteStringAttribute* realm_attr = response->GetByteString(STUN_ATTR_REALM); if (!realm_attr) { - RTC_LOG(LS_ERROR) << "Missing STUN_ATTR_REALM attribute in " + RTC_LOG(LS_ERROR) << ToString() + << ": Missing STUN_ATTR_REALM attribute in " "stale nonce error response."; return false; } @@ -1221,7 +1230,8 @@ bool TurnPort::UpdateNonce(StunMessage* response) { const StunByteStringAttribute* nonce_attr = response->GetByteString(STUN_ATTR_NONCE); if (!nonce_attr) { - RTC_LOG(LS_ERROR) << "Missing STUN_ATTR_NONCE attribute in " + RTC_LOG(LS_ERROR) << ToString() + << ": Missing STUN_ATTR_NONCE attribute in " "stale nonce error response."; return false; } diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc index 0726128fce..b0f3124912 100644 --- a/p2p/base/turn_port_unittest.cc +++ b/p2p/base/turn_port_unittest.cc @@ -970,6 +970,27 @@ TEST_F(TurnPortTest, TestTurnBadCredentials) { EXPECT_EQ(error_event_.error_text, "Unauthorized"); } +// Test that we fail without emitting an error if we try to get an address from +// a TURN server with a different address family. IPv4 local, IPv6 TURN. +TEST_F(TurnPortTest, TestServerAddressFamilyMismatch) { + CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpIPv6ProtoAddr); + turn_port_->PrepareAddress(); + EXPECT_TRUE_SIMULATED_WAIT(turn_error_, kSimulatedRtt * 3, fake_clock_); + ASSERT_EQ(0U, turn_port_->Candidates().size()); + EXPECT_EQ(0, error_event_.error_code); +} + +// Test that we fail without emitting an error if we try to get an address from +// a TURN server with a different address family. IPv6 local, IPv4 TURN. +TEST_F(TurnPortTest, TestServerAddressFamilyMismatch6) { + CreateTurnPort(kLocalIPv6Addr, kTurnUsername, kTurnPassword, + kTurnUdpProtoAddr); + turn_port_->PrepareAddress(); + EXPECT_TRUE_SIMULATED_WAIT(turn_error_, kSimulatedRtt * 3, fake_clock_); + ASSERT_EQ(0U, turn_port_->Candidates().size()); + EXPECT_EQ(0, error_event_.error_code); +} + // Testing a normal UDP allocation using TCP connection. TEST_F(TurnPortTest, TestTurnTcpAllocate) { turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP); @@ -1019,15 +1040,16 @@ 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_.address.find(kLocalAddr2.HostAsSensitiveURIString()), + EXPECT_EQ_SIMULATED_WAIT(error_event_.error_code, + STUN_ERROR_SERVER_NOT_REACHABLE, kSimulatedRtt, + fake_clock_); + EXPECT_NE(error_event_.error_text.find('.'), std::string::npos); + EXPECT_NE(error_event_.address.find(kLocalAddr2.HostAsSensitiveURIString()), std::string::npos); - ASSERT_NE(error_event_.port, 0); + EXPECT_NE(error_event_.port, 0); std::string server_url = "turn:" + kTurnTcpIntAddr.ToString() + "?transport=tcp"; - ASSERT_EQ(error_event_.url, server_url); + EXPECT_EQ(error_event_.url, server_url); } // A caveat for the above logic: if the socket ends up bound to one of the IPs @@ -1095,8 +1117,9 @@ TEST_F(TurnPortTest, TestTurnTcpOnAddressResolveFailure) { // 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_); + EXPECT_EQ_SIMULATED_WAIT(error_event_.error_code, + STUN_ERROR_SERVER_NOT_REACHABLE, kSimulatedRtt, + fake_clock_); std::string server_url = "turn:" + kTurnInvalidAddr.ToString() + "?transport=tcp"; ASSERT_EQ(error_event_.url, server_url);