admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Prevent TURN from connecting to ports < 1024 (except 443)

Browse Source 
This should prevent TURN from being used in NAT slipstream attacks
with native clients.

Bug: webrtc:12497
Change-Id: I3c33df92e97b8e6430b72a05790c137d89661093
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/208582
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Philipp Hancke <philipp.hancke@googlemail.com>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33356}
This commit is contained in:
Harald Alvestrand 2021-02-25 11:18:19 +00:00 committed by Commit Bot
parent bfc3ef0fec
commit df6d4ca3f5
3 changed files with 92 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
p2p/base/turn_port.cc
View File

@ -346,6 +346,15 @@ void TurnPort::PrepareAddress() {
server_address_.address.SetPort(TURN_DEFAULT_PORT);
}
if (!AllowedTurnPort(server_address_.address.port())) {
// 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# "
<< server_address_.address.port();
OnAllocateError(STUN_ERROR_SERVER_ERROR,
"Attempt to start allocation to a disallowed port");
return;
}
if (server_address_.address.IsUnresolvedIP()) {
ResolveTurnAddress(server_address_.address);
} else {
@ -943,6 +952,21 @@ rtc::DiffServCodePoint TurnPort::StunDscpValue() const {
return stun_dscp_value_;
}
// static
bool TurnPort::AllowedTurnPort(int port) {
// Port 443 is used for existing deployments. Ports above 1024 are assumed to
// be OK to use.
if (port == 443 || port >= 1024) {
return true;
}
// Allow any port if relevant field trial is set. This allows disabling the
// check.
if (webrtc::field_trial::IsEnabled("WebRTC-Turn-AllowSystemPorts")) {
return true;
}
return false;
}
void TurnPort::OnMessage(rtc::Message* message) {
switch (message->message_id) {
case MSG_ALLOCATE_ERROR:

13
p2p/base/turn_port.h
View File

@ -68,6 +68,12 @@ class TurnPort : public Port {
if (credentials.username.size() > kMaxTurnUsernameLength) {
return nullptr;
}
// Do not connect to low-numbered ports. The default STUN port is 3478.
if (!AllowedTurnPort(server_address.address.port())) {
RTC_LOG(LS_ERROR) << "Attempt to use TURN to connect to port "
<< server_address.address.port();
return nullptr;
}
// Using `new` to access a non-public constructor.
return absl::WrapUnique(new TurnPort(
thread, factory, network, socket, username, password, server_address,
@ -113,6 +119,12 @@ class TurnPort : public Port {
if (credentials.username.size() > kMaxTurnUsernameLength) {
return nullptr;
}
// Do not connect to low-numbered ports. The default STUN port is 3478.
if (!AllowedTurnPort(server_address.address.port())) {
RTC_LOG(LS_ERROR) << "Attempt to use TURN to connect to port "
<< server_address.address.port();
return nullptr;
}
// Using `new` to access a non-public constructor.
return absl::WrapUnique(
new TurnPort(thread, factory, network, min_port, max_port, username,
@ -296,6 +308,7 @@ class TurnPort : public Port {
typedef std::map<rtc::Socket::Option, int> SocketOptionsMap;
typedef std::set<rtc::SocketAddress> AttemptedServerSet;
static bool AllowedTurnPort(int port);
void OnMessage(rtc::Message* pmsg) override;
bool CreateTurnClientSocket();

55
p2p/base/turn_port_unittest.cc
View File

@ -41,6 +41,7 @@
#include "rtc_base/thread.h"
#include "rtc_base/time_utils.h"
#include "rtc_base/virtual_socket_server.h"
#include "test/field_trial.h"
#include "test/gtest.h"
using rtc::SocketAddress;
@ -58,6 +59,11 @@ static const SocketAddress kTurnTcpIntAddr("99.99.99.4",
static const SocketAddress kTurnUdpExtAddr("99.99.99.5", 0);
static const SocketAddress kTurnAlternateIntAddr("99.99.99.6",
cricket::TURN_SERVER_PORT);
// Port for redirecting to a TCP Web server. Should not work.
static const SocketAddress kTurnDangerousAddr("99.99.99.7", 80);
// Port 443 (the HTTPS port); should work.
static const SocketAddress kTurnPort443Addr("99.99.99.7", 443);
// The default TURN server port.
static const SocketAddress kTurnIntAddr("99.99.99.7",
cricket::TURN_SERVER_PORT);
static const SocketAddress kTurnIPv6IntAddr(
@ -94,6 +100,11 @@ static const cricket::ProtocolAddress kTurnTlsProtoAddr(kTurnTcpIntAddr,
cricket::PROTO_TLS);
static const cricket::ProtocolAddress kTurnUdpIPv6ProtoAddr(kTurnUdpIPv6IntAddr,
cricket::PROTO_UDP);
static const cricket::ProtocolAddress kTurnDangerousProtoAddr(
kTurnDangerousAddr,
cricket::PROTO_TCP);
static const cricket::ProtocolAddress kTurnPort443ProtoAddr(kTurnPort443Addr,
cricket::PROTO_TCP);
static const unsigned int MSG_TESTFINISH = 0;
@ -1785,4 +1796,48 @@ TEST_F(TurnPortTest, TestOverlongUsername) {
CreateTurnPort(overlong_username, kTurnPassword, kTurnTlsProtoAddr));
}
TEST_F(TurnPortTest, TestTurnDangerousServer) {
CreateTurnPort(kTurnUsername, kTurnPassword, kTurnDangerousProtoAddr);
ASSERT_FALSE(turn_port_);
}
TEST_F(TurnPortTest, TestTurnDangerousServerPermits443) {
CreateTurnPort(kTurnUsername, kTurnPassword, kTurnPort443ProtoAddr);
ASSERT_TRUE(turn_port_);
}
TEST_F(TurnPortTest, TestTurnDangerousAlternateServer) {
const ProtocolType protocol_type = PROTO_TCP;
std::vector<rtc::SocketAddress> redirect_addresses;
redirect_addresses.push_back(kTurnDangerousAddr);
TestTurnRedirector redirector(redirect_addresses);
turn_server_.AddInternalSocket(kTurnIntAddr, protocol_type);
turn_server_.AddInternalSocket(kTurnDangerousAddr, protocol_type);
turn_server_.set_redirect_hook(&redirector);
CreateTurnPort(kTurnUsername, kTurnPassword,
ProtocolAddress(kTurnIntAddr, protocol_type));
// Retrieve the address before we run the state machine.
const SocketAddress old_addr = turn_port_->server_address().address;
turn_port_->PrepareAddress();
// This should result in an error event.
EXPECT_TRUE_SIMULATED_WAIT(error_event_.error_code != 0,
TimeToGetAlternateTurnCandidate(protocol_type),
fake_clock_);
// but should NOT result in the port turning ready, and no candidates
// should be gathered.
EXPECT_FALSE(turn_ready_);
ASSERT_EQ(0U, turn_port_->Candidates().size());
}
TEST_F(TurnPortTest, TestTurnDangerousServerAllowedWithFieldTrial) {
webrtc::test::ScopedFieldTrials override_field_trials(
"WebRTC-Turn-AllowSystemPorts/Enabled/");
CreateTurnPort(kTurnUsername, kTurnPassword, kTurnDangerousProtoAddr);
ASSERT_TRUE(turn_port_);
}
} // namespace cricket