Update parsing of stun and turn urls for RFC 7064-7065
Main change is deleting support for @userinfo in turn urls. This was
specified in early internet drafts, but never made it into RFC 7065.
Bug: webrtc:6663, webrtc:10422
Change-Id: Idd315a9e6001326f3104be62be3bd0991adc7db4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/128423
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27171}diff --git a/pc/ice_server_parsing.cc b/pc/ice_server_parsing.cc
index 533d597..3d05d66 100644
--- a/pc/ice_server_parsing.cc
+++ b/pc/ice_server_parsing.cc
@@ -20,13 +20,9 @@
#include "rtc_base/ip_address.h"
#include "rtc_base/logging.h"
#include "rtc_base/socket_address.h"
-#include "rtc_base/string_encode.h"
namespace webrtc {
-// The min number of tokens must present in Turn host uri.
-// e.g. user@turn.example.org
-static const size_t kTurnHostTokensNum = 2;
// Number of tokens must be preset when TURN uri has transport param.
static const size_t kTurnTransportTokensNum = 2;
// The default stun port.
@@ -50,15 +46,12 @@
"kValidIceServiceTypes must have as many strings as ServiceType "
"has values.");
-// |in_str| should be of format
-// stunURI = scheme ":" stun-host [ ":" stun-port ]
-// scheme = "stun" / "stuns"
-// stun-host = IP-literal / IPv4address / reg-name
-// stun-port = *DIGIT
-//
-// draft-petithuguenin-behave-turn-uris-01
-// turnURI = scheme ":" turn-host [ ":" turn-port ]
-// turn-host = username@IP-literal / IPv4address / reg-name
+// |in_str| should follow of RFC 7064/7065 syntax, but with an optional
+// "?transport=" already stripped. I.e.,
+// stunURI = scheme ":" host [ ":" port ]
+// scheme = "stun" / "stuns" / "turn" / "turns"
+// host = IP-literal / IPv4address / reg-name
+// port = *DIGIT
static bool GetServiceTypeAndHostnameFromUri(const std::string& in_str,
ServiceType* service_type,
std::string* hostname) {
@@ -139,20 +132,21 @@
const std::string& url,
cricket::ServerAddresses* stun_servers,
std::vector<cricket::RelayServerConfig>* turn_servers) {
- // draft-nandakumar-rtcweb-stun-uri-01
- // stunURI = scheme ":" stun-host [ ":" stun-port ]
+ // RFC 7064
+ // stunURI = scheme ":" host [ ":" port ]
// scheme = "stun" / "stuns"
- // stun-host = IP-literal / IPv4address / reg-name
- // stun-port = *DIGIT
- // draft-petithuguenin-behave-turn-uris-01
- // turnURI = scheme ":" turn-host [ ":" turn-port ]
+ // RFC 7065
+ // turnURI = scheme ":" host [ ":" port ]
// [ "?transport=" transport ]
// scheme = "turn" / "turns"
// transport = "udp" / "tcp" / transport-ext
// transport-ext = 1*unreserved
- // turn-host = IP-literal / IPv4address / reg-name
- // turn-port = *DIGIT
+
+ // RFC 3986
+ // host = IP-literal / IPv4address / reg-name
+ // port = *DIGIT
+
RTC_DCHECK(stun_servers != nullptr);
RTC_DCHECK(turn_servers != nullptr);
std::vector<std::string> tokens;
@@ -191,32 +185,18 @@
// GetServiceTypeAndHostnameFromUri should never give an empty hoststring
RTC_DCHECK(!hoststring.empty());
- // Let's break hostname.
- tokens.clear();
- rtc::tokenize_with_empty_tokens(hoststring, '@', &tokens);
-
- std::string username(server.username);
- if (tokens.size() > kTurnHostTokensNum) {
- RTC_LOG(LS_WARNING) << "Invalid user@hostname format: " << hoststring;
- return RTCErrorType::SYNTAX_ERROR;
- }
- if (tokens.size() == kTurnHostTokensNum) {
- if (tokens[0].empty() || tokens[1].empty()) {
- RTC_LOG(LS_WARNING) << "Invalid user@hostname format: " << hoststring;
- return RTCErrorType::SYNTAX_ERROR;
- }
- username.assign(rtc::s_url_decode(tokens[0]));
- hoststring = tokens[1];
- } else {
- hoststring = tokens[0];
- }
-
int port = kDefaultStunPort;
if (service_type == TURNS) {
port = kDefaultStunTlsPort;
turn_transport_type = cricket::PROTO_TLS;
}
+ if (hoststring.find('@') != std::string::npos) {
+ RTC_LOG(WARNING) << "Invalid url: " << uri_without_transport;
+ RTC_LOG(WARNING)
+ << "Note that user-info@ in turn:-urls is long-deprecated.";
+ return RTCErrorType::SYNTAX_ERROR;
+ }
std::string address;
if (!ParseHostnameAndPortFromString(hoststring, &address, &port)) {
RTC_LOG(WARNING) << "Invalid hostname format: " << uri_without_transport;
@@ -235,10 +215,10 @@
break;
case TURN:
case TURNS: {
- if (username.empty() || server.password.empty()) {
+ if (server.username.empty() || server.password.empty()) {
// The WebRTC spec requires throwing an InvalidAccessError when username
// or credential are ommitted; this is the native equivalent.
- RTC_LOG(LS_ERROR) << "TURN URL without username, or password empty";
+ RTC_LOG(LS_ERROR) << "TURN server with empty username or password";
return RTCErrorType::INVALID_PARAMETER;
}
// If the hostname field is not empty, then the server address must be
@@ -259,8 +239,9 @@
}
socket_address.SetResolvedIP(ip);
}
- cricket::RelayServerConfig config = cricket::RelayServerConfig(
- socket_address, username, server.password, turn_transport_type);
+ cricket::RelayServerConfig config =
+ cricket::RelayServerConfig(socket_address, server.username,
+ server.password, turn_transport_type);
if (server.tls_cert_policy ==
PeerConnectionInterface::kTlsCertPolicyInsecureNoCheck) {
config.tls_cert_policy =
diff --git a/pc/ice_server_parsing_unittest.cc b/pc/ice_server_parsing_unittest.cc
index 0ec59ca..9f1b817 100644
--- a/pc/ice_server_parsing_unittest.cc
+++ b/pc/ice_server_parsing_unittest.cc
@@ -200,16 +200,9 @@
EXPECT_FALSE(ParseTurnUrl("?"));
}
-// Test parsing ICE username contained in URL.
-TEST_F(IceServerParsingTest, ParseUsername) {
- EXPECT_TRUE(ParseTurnUrl("turn:user@hostname"));
- EXPECT_EQ(1U, turn_servers_.size());
- EXPECT_EQ("user", turn_servers_[0].credentials.username);
-
- EXPECT_FALSE(ParseTurnUrl("turn:@hostname"));
- EXPECT_FALSE(ParseTurnUrl("turn:username@"));
- EXPECT_FALSE(ParseTurnUrl("turn:@"));
- EXPECT_FALSE(ParseTurnUrl("turn:user@name@hostname"));
+// Reject pre-RFC 7065 syntax with ICE username contained in URL.
+TEST_F(IceServerParsingTest, ParseRejectsUsername) {
+ EXPECT_FALSE(ParseTurnUrl("turn:user@hostname"));
}
// Test that username and password from IceServer is copied into the resulting
diff --git a/pc/peer_connection_factory_unittest.cc b/pc/peer_connection_factory_unittest.cc
index 1c3eff7..41a5408 100644
--- a/pc/peer_connection_factory_unittest.cc
+++ b/pc/peer_connection_factory_unittest.cc
@@ -57,14 +57,14 @@
namespace {
static const char kStunIceServer[] = "stun:stun.l.google.com:19302";
-static const char kTurnIceServer[] = "turn:test%40hello.com@test.com:1234";
+static const char kTurnIceServer[] = "turn:test.com:1234";
static const char kTurnIceServerWithTransport[] =
- "turn:test@hello.com?transport=tcp";
-static const char kSecureTurnIceServer[] = "turns:test@hello.com?transport=tcp";
+ "turn:hello.com?transport=tcp";
+static const char kSecureTurnIceServer[] = "turns:hello.com?transport=tcp";
static const char kSecureTurnIceServerWithoutTransportParam[] =
- "turns:test_no_transport@hello.com:443";
+ "turns:hello.com:443";
static const char kSecureTurnIceServerWithoutTransportAndPortParam[] =
- "turns:test_no_transport@hello.com";
+ "turns:hello.com";
static const char kTurnIceServerWithNoUsernameInUri[] = "turn:test.com:1234";
static const char kTurnPassword[] = "turnpassword";
static const int kDefaultStunPort = 3478;
@@ -75,8 +75,7 @@
static const char kStunIceServerWithIPv6Address[] = "stun:[2401:fa00:4::]:1234";
static const char kStunIceServerWithIPv6AddressWithoutPort[] =
"stun:[2401:fa00:4::]";
-static const char kTurnIceServerWithIPv6Address[] =
- "turn:test@[2401:fa00:4::]:1234";
+static const char kTurnIceServerWithIPv6Address[] = "turn:[2401:fa00:4::]:1234";
class NullPeerConnectionObserver : public PeerConnectionObserver {
public:
@@ -272,9 +271,11 @@
ice_server.uri = kStunIceServer;
config.servers.push_back(ice_server);
ice_server.uri = kTurnIceServer;
+ ice_server.username = kTurnUsername;
ice_server.password = kTurnPassword;
config.servers.push_back(ice_server);
ice_server.uri = kTurnIceServerWithTransport;
+ ice_server.username = kTurnUsername;
ice_server.password = kTurnPassword;
config.servers.push_back(ice_server);
std::unique_ptr<FakeRTCCertificateGenerator> cert_generator(
@@ -288,10 +289,10 @@
stun_servers.insert(stun1);
VerifyStunServers(stun_servers);
std::vector<cricket::RelayServerConfig> turn_servers;
- cricket::RelayServerConfig turn1("test.com", 1234, "test@hello.com",
+ cricket::RelayServerConfig turn1("test.com", 1234, kTurnUsername,
kTurnPassword, cricket::PROTO_UDP);
turn_servers.push_back(turn1);
- cricket::RelayServerConfig turn2("hello.com", kDefaultStunPort, "test",
+ cricket::RelayServerConfig turn2("hello.com", kDefaultStunPort, kTurnUsername,
kTurnPassword, cricket::PROTO_TCP);
turn_servers.push_back(turn2);
VerifyTurnServers(turn_servers);
@@ -305,6 +306,7 @@
ice_server.urls.push_back(kStunIceServer);
ice_server.urls.push_back(kTurnIceServer);
ice_server.urls.push_back(kTurnIceServerWithTransport);
+ ice_server.username = kTurnUsername;
ice_server.password = kTurnPassword;
config.servers.push_back(ice_server);
std::unique_ptr<FakeRTCCertificateGenerator> cert_generator(
@@ -318,10 +320,10 @@
stun_servers.insert(stun1);
VerifyStunServers(stun_servers);
std::vector<cricket::RelayServerConfig> turn_servers;
- cricket::RelayServerConfig turn1("test.com", 1234, "test@hello.com",
+ cricket::RelayServerConfig turn1("test.com", 1234, kTurnUsername,
kTurnPassword, cricket::PROTO_UDP);
turn_servers.push_back(turn1);
- cricket::RelayServerConfig turn2("hello.com", kDefaultStunPort, "test",
+ cricket::RelayServerConfig turn2("hello.com", kDefaultStunPort, kTurnUsername,
kTurnPassword, cricket::PROTO_TCP);
turn_servers.push_back(turn2);
VerifyTurnServers(turn_servers);
@@ -355,6 +357,7 @@
PeerConnectionInterface::RTCConfiguration config;
webrtc::PeerConnectionInterface::IceServer ice_server;
ice_server.uri = kTurnIceServerWithTransport;
+ ice_server.username = kTurnUsername;
ice_server.password = kTurnPassword;
config.servers.push_back(ice_server);
std::unique_ptr<FakeRTCCertificateGenerator> cert_generator(
@@ -364,7 +367,7 @@
std::move(cert_generator), &observer_));
ASSERT_TRUE(pc.get() != NULL);
std::vector<cricket::RelayServerConfig> turn_servers;
- cricket::RelayServerConfig turn("hello.com", kDefaultStunPort, "test",
+ cricket::RelayServerConfig turn("hello.com", kDefaultStunPort, kTurnUsername,
kTurnPassword, cricket::PROTO_TCP);
turn_servers.push_back(turn);
VerifyTurnServers(turn_servers);
@@ -374,12 +377,15 @@
PeerConnectionInterface::RTCConfiguration config;
webrtc::PeerConnectionInterface::IceServer ice_server;
ice_server.uri = kSecureTurnIceServer;
+ ice_server.username = kTurnUsername;
ice_server.password = kTurnPassword;
config.servers.push_back(ice_server);
ice_server.uri = kSecureTurnIceServerWithoutTransportParam;
+ ice_server.username = kTurnUsername;
ice_server.password = kTurnPassword;
config.servers.push_back(ice_server);
ice_server.uri = kSecureTurnIceServerWithoutTransportAndPortParam;
+ ice_server.username = kTurnUsername;
ice_server.password = kTurnPassword;
config.servers.push_back(ice_server);
std::unique_ptr<FakeRTCCertificateGenerator> cert_generator(
@@ -389,15 +395,16 @@
std::move(cert_generator), &observer_));
ASSERT_TRUE(pc.get() != NULL);
std::vector<cricket::RelayServerConfig> turn_servers;
- cricket::RelayServerConfig turn1("hello.com", kDefaultStunTlsPort, "test",
- kTurnPassword, cricket::PROTO_TLS);
+ cricket::RelayServerConfig turn1("hello.com", kDefaultStunTlsPort,
+ kTurnUsername, kTurnPassword,
+ cricket::PROTO_TLS);
turn_servers.push_back(turn1);
// TURNS with transport param should be default to tcp.
- cricket::RelayServerConfig turn2("hello.com", 443, "test_no_transport",
+ cricket::RelayServerConfig turn2("hello.com", 443, kTurnUsername,
kTurnPassword, cricket::PROTO_TLS);
turn_servers.push_back(turn2);
cricket::RelayServerConfig turn3("hello.com", kDefaultStunTlsPort,
- "test_no_transport", kTurnPassword,
+ kTurnUsername, kTurnPassword,
cricket::PROTO_TLS);
turn_servers.push_back(turn3);
VerifyTurnServers(turn_servers);
@@ -415,6 +422,7 @@
ice_server.uri = kStunIceServerWithIPv6AddressWithoutPort;
config.servers.push_back(ice_server);
ice_server.uri = kTurnIceServerWithIPv6Address;
+ ice_server.username = kTurnUsername;
ice_server.password = kTurnPassword;
config.servers.push_back(ice_server);
std::unique_ptr<FakeRTCCertificateGenerator> cert_generator(
@@ -435,8 +443,8 @@
VerifyStunServers(stun_servers);
std::vector<cricket::RelayServerConfig> turn_servers;
- cricket::RelayServerConfig turn1("2401:fa00:4::", 1234, "test", kTurnPassword,
- cricket::PROTO_UDP);
+ cricket::RelayServerConfig turn1("2401:fa00:4::", 1234, kTurnUsername,
+ kTurnPassword, cricket::PROTO_UDP);
turn_servers.push_back(turn1);
VerifyTurnServers(turn_servers);
}
diff --git a/pc/peer_connection_interface_unittest.cc b/pc/peer_connection_interface_unittest.cc
index 06d077a..7cec6ed 100644
--- a/pc/peer_connection_interface_unittest.cc
+++ b/pc/peer_connection_interface_unittest.cc
@@ -104,7 +104,7 @@
static const char kStunInvalidPort[] = "stun:address:-1";
static const char kStunAddressPortAndMore1[] = "stun:address:port:more";
static const char kStunAddressPortAndMore2[] = "stun:address:port more";
-static const char kTurnIceServerUri[] = "turn:user@turn.example.org";
+static const char kTurnIceServerUri[] = "turn:turn.example.org";
static const char kTurnUsername[] = "user";
static const char kTurnPassword[] = "password";
static const char kTurnHostname[] = "turn.example.org";
@@ -729,10 +729,12 @@
}
void CreatePeerConnectionWithIceServer(const std::string& uri,
+ const std::string& username,
const std::string& password) {
PeerConnectionInterface::RTCConfiguration config;
PeerConnectionInterface::IceServer server;
server.uri = uri;
+ server.username = username;
server.password = password;
config.servers.push_back(server);
CreatePeerConnection(config);
@@ -785,7 +787,7 @@
}
void CreatePeerConnectionWithDifferentConfigurations() {
- CreatePeerConnectionWithIceServer(kStunAddressOnly, "");
+ CreatePeerConnectionWithIceServer(kStunAddressOnly, "", "");
EXPECT_EQ(1u, port_allocator_->stun_servers().size());
EXPECT_EQ(0u, port_allocator_->turn_servers().size());
EXPECT_EQ("address", port_allocator_->stun_servers().begin()->hostname());
@@ -796,7 +798,8 @@
CreatePeerConnectionExpectFail(kStunAddressPortAndMore1);
CreatePeerConnectionExpectFail(kStunAddressPortAndMore2);
- CreatePeerConnectionWithIceServer(kTurnIceServerUri, kTurnPassword);
+ CreatePeerConnectionWithIceServer(kTurnIceServerUri, kTurnUsername,
+ kTurnPassword);
EXPECT_EQ(0u, port_allocator_->stun_servers().size());
EXPECT_EQ(1u, port_allocator_->turn_servers().size());
EXPECT_EQ(kTurnUsername,
