stun/turn: use hostname when reconstructing the url
in the case of an ip address the hostname() call will return
that. This may also avoid leaking IP addresses from DNS resolutions
and is more similar to the url originally passed into the
peerconnection (but will for example produce a fully formed url and
resolves the port if none was given).
BUG=webrtc:13652
Change-Id: I000c66f7988b4b205e38c4dde5b888e48d8f6a0f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/250202
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <philipp.hancke@googlemail.com>
Cr-Commit-Position: refs/heads/main@{#35898}
diff --git a/p2p/base/stun_port.cc b/p2p/base/stun_port.cc
index 3ab3b5b..1ba6486 100644
--- a/p2p/base/stun_port.cc
+++ b/p2p/base/stun_port.cc
@@ -512,7 +512,7 @@
}
rtc::StringBuilder url;
- url << "stun:" << stun_server_addr.ipaddr().ToString() << ":"
+ url << "stun:" << stun_server_addr.hostname() << ":"
<< stun_server_addr.port();
AddAddress(stun_reflected_addr, socket_->GetLocalAddress(), related_address,
UDP_PROTOCOL_NAME, "", "", STUN_PORT_TYPE,
diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc
index e622b62..bde5bf6 100644
--- a/p2p/base/turn_port.cc
+++ b/p2p/base/turn_port.cc
@@ -862,10 +862,9 @@
related_address, // Related address.
UDP_PROTOCOL_NAME,
ProtoToString(server_address_.proto), // The first hop protocol.
- "", // TCP canddiate type, empty for turn candidates.
+ "", // TCP candidate type, empty for turn candidates.
RELAY_PORT_TYPE, GetRelayPreference(server_address_.proto),
- server_priority_, ReconstructedServerUrl(false /* use_hostname */),
- true);
+ server_priority_, ReconstructedServerUrl(), true);
}
void TurnPort::OnAllocateError(int error_code, const std::string& reason) {
@@ -881,9 +880,8 @@
port = 0;
}
SignalCandidateError(
- this, IceCandidateErrorEvent(
- address, port, ReconstructedServerUrl(true /* use_hostname */),
- error_code, reason));
+ this, IceCandidateErrorEvent(address, port, ReconstructedServerUrl(),
+ error_code, reason));
}
void TurnPort::OnRefreshError() {
@@ -1291,7 +1289,7 @@
return true;
}
-std::string TurnPort::ReconstructedServerUrl(bool use_hostname) {
+std::string TurnPort::ReconstructedServerUrl() {
// draft-petithuguenin-behave-turn-uris-01
// turnURI = scheme ":" turn-host [ ":" turn-port ]
// [ "?transport=" transport ]
@@ -1314,10 +1312,8 @@
break;
}
rtc::StringBuilder url;
- url << scheme << ":"
- << (use_hostname ? server_address_.address.hostname()
- : server_address_.address.ipaddr().ToString())
- << ":" << server_address_.address.port() << "?transport=" << transport;
+ url << scheme << ":" << server_address_.address.hostname() << ":"
+ << server_address_.address.port() << "?transport=" << transport;
return url.Release();
}
diff --git a/p2p/base/turn_port.h b/p2p/base/turn_port.h
index 891af76..172dcef 100644
--- a/p2p/base/turn_port.h
+++ b/p2p/base/turn_port.h
@@ -367,7 +367,7 @@
bool FailAndPruneConnection(const rtc::SocketAddress& address);
// Reconstruct the URL of the server which the candidate is gathered from.
- std::string ReconstructedServerUrl(bool use_hostname);
+ std::string ReconstructedServerUrl();
void MaybeAddTurnLoggingId(StunMessage* message);
diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc
index 48d901f..44d2583 100644
--- a/p2p/base/turn_port_unittest.cc
+++ b/p2p/base/turn_port_unittest.cc
@@ -112,6 +112,9 @@
cricket::PROTO_TCP);
static const cricket::ProtocolAddress kTurnPort443ProtoAddr(kTurnPort443Addr,
cricket::PROTO_TCP);
+static const cricket::ProtocolAddress kTurnPortHostnameProtoAddr(
+ kTurnInvalidAddr,
+ cricket::PROTO_UDP);
static const unsigned int MSG_TESTFINISH = 0;
@@ -818,6 +821,18 @@
TestReconstructedServerUrl(PROTO_TLS, "turns:99.99.99.4:3478?transport=tcp");
}
+TEST_F(TurnPortTest, TestReconstructedServerUrlForHostname) {
+ CreateTurnPort(kTurnUsername, kTurnPassword, kTurnPortHostnameProtoAddr);
+ // This test follows the pattern from TestTurnTcpOnAddressResolveFailure.
+ // As VSS doesn't provide DNS resolution, name resolve will fail,
+ // the error will be set and contain the url.
+ turn_port_->PrepareAddress();
+ EXPECT_TRUE_WAIT(turn_error_, kResolverTimeout);
+ std::string server_url =
+ "turn:" + kTurnInvalidAddr.ToString() + "?transport=udp";
+ ASSERT_EQ(error_event_.url, server_url);
+}
+
// Do a normal TURN allocation.
TEST_F(TurnPortTest, TestTurnAllocate) {
CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr);
@@ -1020,7 +1035,7 @@
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
+ // As VSS doesn't provide 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());
