stun/turn: suppress icecandidateerror for incompatible address family
Suppresses the ice candidate error callback when the STUN/TURN server
address family is not compatible with the local candidate address family.
This is similar to not pairing between candidates that have different
incompatible address families as described in
https://datatracker.ietf.org/doc/html/rfc5245#section-5.7.1
The spec actually says to emit the 701 error if *no* host candidate is able to reach the server:
https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnectioniceerrorevent-errorcode
Also use the same (spec) error code for STUN and TURN, see
https://github.com/webrtc/samples/issues/1215 (error 600 for TURN)
https://github.com/webrtc/samples/issues/1227 (error 701 with AF mismatch)
Drive-by: misc logging fixes
BUG=webrtc:359404135
Change-Id: I99574b7b2b79986a52ab38a7fa58ea1bebab954c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/358961
Commit-Queue: Philipp Hancke <phancke@meta.com>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42830}
diff --git a/api/transport/stun.h b/api/transport/stun.h
index ef5b08e..8300f75 100644
--- a/api/transport/stun.h
+++ b/api/transport/stun.h
@@ -95,13 +95,17 @@
// 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 @@
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 fd1eb72..bdd8830 100644
--- a/p2p/base/stun_port.cc
+++ b/p2p/base/stun_port.cc
@@ -102,7 +102,7 @@
<< 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 @@
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 @@
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 @@
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 842f3e4..08c224a 100644
--- a/p2p/base/stun_port_unittest.cc
+++ b/p2p/base/stun_port_unittest.cc
@@ -44,6 +44,7 @@
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 @@
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 @@
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 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 @@
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 1745019..4dba2a5 100644
--- a/p2p/base/turn_port.cc
+++ b/p2p/base/turn_port.cc
@@ -391,7 +391,8 @@
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 @@
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 @@
} 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 @@
<< 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 @@
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 @@
// 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 @@
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 @@
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 @@
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::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::OnAllocateRequestTimeout() {
- OnAllocateError(SERVER_NOT_REACHABLE_ERROR,
+ OnAllocateError(STUN_ERROR_SERVER_NOT_REACHABLE,
"TURN allocate request timed out.");
}
@@ -1212,7 +1220,8 @@
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 @@
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 0726128..b0f3124 100644
--- a/p2p/base/turn_port_unittest.cc
+++ b/p2p/base/turn_port_unittest.cc
@@ -970,6 +970,27 @@
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 @@
// 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 @@
// 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);
