Check for oversized TURN usernames

Bug: chromium:1144646
Change-Id: I8e71a025246708f05e38ba6f397f9655251da788
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/191222
Reviewed-by: Philipp Hancke <philipp.hancke@googlemail.com>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32536}
This commit is contained in:
Harald Alvestrand
2020-11-02 10:16:32 +00:00
committed by Commit Bot
parent 043725fefd
commit 3cb9c6afe7
3 changed files with 38 additions and 13 deletions

View File

@ -33,6 +33,8 @@ class TurnCustomizer;
namespace cricket {
const int kMaxTurnUsernameLength = 509; // RFC 8489 section 14.3
extern const int STUN_ATTR_TURN_LOGGING_ID;
extern const char TURN_PORT_TYPE[];
class TurnAllocateRequest;
@ -61,6 +63,10 @@ class TurnPort : public Port {
int server_priority,
const std::string& origin,
webrtc::TurnCustomizer* customizer) {
// Do basic parameter validation.
if (credentials.username.size() > kMaxTurnUsernameLength) {
return nullptr;
}
// Using `new` to access a non-public constructor.
return absl::WrapUnique(new TurnPort(
thread, factory, network, socket, username, password, server_address,
@ -102,6 +108,10 @@ class TurnPort : public Port {
const std::vector<std::string>& tls_elliptic_curves,
webrtc::TurnCustomizer* customizer,
rtc::SSLCertificateVerifier* tls_cert_verifier = nullptr) {
// Do basic parameter validation.
if (credentials.username.size() > kMaxTurnUsernameLength) {
return nullptr;
}
// Using `new` to access a non-public constructor.
return absl::WrapUnique(
new TurnPort(thread, factory, network, min_port, max_port, username,

View File

@ -236,43 +236,43 @@ class TurnPortTest : public ::testing::Test,
return &networks_.back();
}
void CreateTurnPort(const std::string& username,
bool CreateTurnPort(const std::string& username,
const std::string& password,
const ProtocolAddress& server_address) {
CreateTurnPortWithAllParams(MakeNetwork(kLocalAddr1), username, password,
server_address, std::string());
return CreateTurnPortWithAllParams(MakeNetwork(kLocalAddr1), username,
password, server_address, std::string());
}
void CreateTurnPort(const rtc::SocketAddress& local_address,
bool CreateTurnPort(const rtc::SocketAddress& local_address,
const std::string& username,
const std::string& password,
const ProtocolAddress& server_address) {
CreateTurnPortWithAllParams(MakeNetwork(local_address), username, password,
server_address, std::string());
return CreateTurnPortWithAllParams(MakeNetwork(local_address), username,
password, server_address, std::string());
}
// Should be identical to CreateTurnPort but specifies an origin value
// when creating the instance of TurnPort.
void CreateTurnPortWithOrigin(const rtc::SocketAddress& local_address,
bool CreateTurnPortWithOrigin(const rtc::SocketAddress& local_address,
const std::string& username,
const std::string& password,
const ProtocolAddress& server_address,
const std::string& origin) {
CreateTurnPortWithAllParams(MakeNetwork(local_address), username, password,
server_address, origin);
return CreateTurnPortWithAllParams(MakeNetwork(local_address), username,
password, server_address, origin);
}
void CreateTurnPortWithNetwork(rtc::Network* network,
bool CreateTurnPortWithNetwork(rtc::Network* network,
const std::string& username,
const std::string& password,
const ProtocolAddress& server_address) {
CreateTurnPortWithAllParams(network, username, password, server_address,
std::string());
return CreateTurnPortWithAllParams(network, username, password,
server_address, std::string());
}
// Version of CreateTurnPort that takes all possible parameters; all other
// helper methods call this, such that "SetIceRole" and "ConnectSignals" (and
// possibly other things in the future) only happen in one place.
void CreateTurnPortWithAllParams(rtc::Network* network,
bool CreateTurnPortWithAllParams(rtc::Network* network,
const std::string& username,
const std::string& password,
const ProtocolAddress& server_address,
@ -281,6 +281,9 @@ class TurnPortTest : public ::testing::Test,
turn_port_ = TurnPort::Create(
&main_, &socket_factory_, network, 0, 0, kIceUfrag1, kIcePwd1,
server_address, credentials, 0, origin, {}, {}, turn_customizer_.get());
if (!turn_port_) {
return false;
}
// This TURN port will be the controlling.
turn_port_->SetIceRole(ICEROLE_CONTROLLING);
ConnectSignals();
@ -292,6 +295,7 @@ class TurnPortTest : public ::testing::Test,
turn_port_->SetTlsCertPolicy(
TlsCertPolicy::TLS_CERT_POLICY_INSECURE_NO_CHECK);
}
return true;
}
void CreateSharedTurnPort(const std::string& username,
@ -1774,4 +1778,11 @@ TEST_F(TurnPortTest, TestTurnCustomizerAddAttribute) {
turn_port_.reset(nullptr);
}
TEST_F(TurnPortTest, TestOverlongUsername) {
std::string overlong_username(513, 'x');
RelayCredentials credentials(overlong_username, kTurnPassword);
EXPECT_FALSE(
CreateTurnPort(overlong_username, kTurnPassword, kTurnTlsProtoAddr));
}
} // namespace cricket

View File

@ -28,6 +28,8 @@ std::unique_ptr<Port> TurnPortFactory::Create(
args.username, args.password, *args.server_address,
args.config->credentials, args.config->priority, args.origin,
args.turn_customizer);
if (!port)
return nullptr;
port->SetTlsCertPolicy(args.config->tls_cert_policy);
port->SetTurnLoggingId(args.config->turn_logging_id);
return std::move(port);
@ -42,6 +44,8 @@ std::unique_ptr<Port> TurnPortFactory::Create(const CreateRelayPortArgs& args,
args.config->credentials, args.config->priority, args.origin,
args.config->tls_alpn_protocols, args.config->tls_elliptic_curves,
args.turn_customizer, args.config->tls_cert_verifier);
if (!port)
return nullptr;
port->SetTlsCertPolicy(args.config->tls_cert_policy);
port->SetTurnLoggingId(args.config->turn_logging_id);
return std::move(port);