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

Fixing the behavior of the candidate filter with pooled candidates.

Browse Source 
According to JSEP, the candidate filter does not affect pooled
candidates because they can be filtered once they're ready to be
surfaced to the application.

So, pooled port allocator sessions will use a filter of CF_ALL, with a
new filter applied when the session is taken by a P2PTransportChannel.

When the filter is applied:
* Some candidates may no longer be returned by ReadyCandidates()
* Some candidates may no longer have a "related address" (for privacy)
* Some ports may no longer be returned by ReadyPorts()

To simplify this, the candidate filtering logic is now moved up from
the Ports to the BasicPortAllocator, with some helper methods to perform
the filtering and stripping out of data.

R=honghaiz@webrtc.org, pthatcher@webrtc.org

Review URL: https://codereview.webrtc.org/1998813002 .

Cr-Commit-Position: refs/heads/master@{#12856}
This commit is contained in:
Taylor Brandstetter 2016-05-23 16:02:19 -07:00
parent 5b53b0eeb4
commit 417eebe5dd
12 changed files with 507 additions and 450 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
webrtc/api/peerconnectioninterface_unittest.cc
View File

@ -1058,7 +1058,6 @@ TEST_F(PeerConnectionInterfaceTest, CreatePeerConnectionWithPooledCandidates) {
EXPECT_EQ(1UL, session->stun_servers().size());
EXPECT_EQ(0U, session->flags() & cricket::PORTALLOCATOR_ENABLE_IPV6);
EXPECT_LT(0U, session->flags() & cricket::PORTALLOCATOR_DISABLE_TCP);
EXPECT_EQ(cricket::CF_RELAY, session->candidate_filter());
}
TEST_F(PeerConnectionInterfaceTest, AddStreams) {
@ -1973,14 +1972,13 @@ TEST_F(PeerConnectionInterfaceTest,
server.uri = kStunAddressOnly;
config.servers.push_back(server);
config.type = PeerConnectionInterface::kRelay;
CreatePeerConnection(config, nullptr);
EXPECT_TRUE(pc_->SetConfiguration(config));
const cricket::FakePortAllocatorSession* session =
static_cast<const cricket::FakePortAllocatorSession*>(
port_allocator_->GetPooledSession());
ASSERT_NE(nullptr, session);
EXPECT_EQ(1UL, session->stun_servers().size());
EXPECT_EQ(cricket::CF_RELAY, session->candidate_filter());
}
// Test that PeerConnection::Close changes the states to closed and all remote

9
webrtc/p2p/base/fakeportallocator.h
View File

@ -103,11 +103,14 @@ class FakePortAllocatorSession : public PortAllocatorSession {
running_(false),
port_config_count_(0),
stun_servers_(allocator->stun_servers()),
turn_servers_(allocator->turn_servers()),
candidate_filter_(allocator->candidate_filter()) {
turn_servers_(allocator->turn_servers()) {
network_.AddIP(rtc::IPAddress(INADDR_LOOPBACK));
}
void SetCandidateFilter(uint32_t filter) override {
candidate_filter_ = filter;
}
void StartGettingPorts() override {
if (!port_) {
port_.reset(TestUDPPort::Create(worker_thread_, factory_, &network_,
@ -181,7 +184,7 @@ class FakePortAllocatorSession : public PortAllocatorSession {
bool allocation_done_ = false;
ServerAddresses stun_servers_;
std::vector<RelayServerConfig> turn_servers_;
uint32_t candidate_filter_;
uint32_t candidate_filter_ = CF_ALL;
int transport_info_update_count_ = 0;
};

6
webrtc/p2p/base/port.cc
View File

@ -150,8 +150,7 @@ Port::Port(rtc::Thread* thread,
enable_port_packets_(false),
ice_role_(ICEROLE_UNKNOWN),
tiebreaker_(0),
shared_socket_(true),
candidate_filter_(CF_ALL) {
shared_socket_(true) {
Construct();
}
@ -180,8 +179,7 @@ Port::Port(rtc::Thread* thread,
enable_port_packets_(false),
ice_role_(ICEROLE_UNKNOWN),
tiebreaker_(0),
shared_socket_(false),
candidate_filter_(CF_ALL) {
shared_socket_(false) {
ASSERT(factory_ != NULL);
Construct();
}

11
webrtc/p2p/base/port.h
View File

@ -293,9 +293,6 @@ class Port : public PortInterface, public rtc::MessageHandler,
// Returns the index of the new local candidate.
size_t AddPrflxCandidate(const Candidate& local);
void set_candidate_filter(uint32_t candidate_filter) {
candidate_filter_ = candidate_filter;
}
int16_t network_cost() const { return network_cost_; }
protected:
@ -349,8 +346,6 @@ class Port : public PortInterface, public rtc::MessageHandler,
return rtc::DSCP_NO_CHANGE;
}
uint32_t candidate_filter() { return candidate_filter_; }
private:
void Construct();
// Called when one of our connections deletes itself.
@ -398,12 +393,6 @@ class Port : public PortInterface, public rtc::MessageHandler,
std::string user_agent_;
rtc::ProxyInfo proxy_;
// Candidate filter is pushed down to Port such that each Port could
// make its own decision on how to create candidates. For example,
// when IceTransportsType is set to relay, both RelayPort and
// TurnPort will hide raddr to avoid local address leakage.
uint32_t candidate_filter_;
// A virtual cost perceived by the user, usually based on the network type
// (WiFi. vs. Cellular). It takes precedence over the priority when
// comparing two connections.

7
webrtc/p2p/base/portallocator.cc
View File

@ -71,8 +71,10 @@ std::unique_ptr<PortAllocatorSession> PortAllocator::CreateSession(
int component,
const std::string& ice_ufrag,
const std::string& ice_pwd) {
return std::unique_ptr<PortAllocatorSession>(
auto session = std::unique_ptr<PortAllocatorSession>(
CreateSessionInternal(content_name, component, ice_ufrag, ice_pwd));
session->SetCandidateFilter(candidate_filter());
return session;
}
std::unique_ptr<PortAllocatorSession> PortAllocator::TakePooledSession(
@ -88,6 +90,9 @@ std::unique_ptr<PortAllocatorSession> PortAllocator::TakePooledSession(
std::unique_ptr<PortAllocatorSession> ret =
std::move(pooled_sessions_.front());
ret->SetIceParameters(content_name, component, ice_ufrag, ice_pwd);
// According to JSEP, a pooled session should filter candidates only after
// it's taken out of the pool.
ret->SetCandidateFilter(candidate_filter());
pooled_sessions_.pop_front();
return ret;
}

8
webrtc/p2p/base/portallocator.h
View File

@ -141,6 +141,13 @@ class PortAllocatorSession : public sigslot::has_slots<> {
const std::string& ice_pwd() const { return ice_pwd_; }
bool pooled() const { return ice_ufrag_.empty(); }
// Setting this filter should affect not only candidates gathered in the
// future, but candidates already gathered and ports already "ready",
// which would be returned by ReadyCandidates() and ReadyPorts().
//
// Default filter should be CF_ALL.
virtual void SetCandidateFilter(uint32_t filter) = 0;
// Starts gathering STUN and Relay configurations.
virtual void StartGettingPorts() = 0;
virtual void StopGettingPorts() = 0;
@ -300,7 +307,6 @@ class PortAllocator : public sigslot::has_slots<> {
uint32_t candidate_filter() { return candidate_filter_; }
void set_candidate_filter(uint32_t filter) {
// TODO(mallinath) - Do transition check?
candidate_filter_ = filter;
}

42
webrtc/p2p/base/portallocator_unittest.cc
View File

@ -15,6 +15,7 @@
#include "webrtc/p2p/base/fakeportallocator.h"
#include "webrtc/p2p/base/portallocator.h"
static const char kSessionId[] = "session id";
static const char kContentName[] = "test content";
// Based on ICE_UFRAG_LENGTH
static const char kIceUfrag[] = "TESTICEUFRAG0000";
@ -37,6 +38,20 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> {
candidate_pool_size);
}
std::unique_ptr<cricket::FakePortAllocatorSession> CreateSession(
const std::string& sid,
const std::string& content_name,
int component,
const std::string& ice_ufrag,
const std::string& ice_pwd) {
return std::unique_ptr<cricket::FakePortAllocatorSession>(
static_cast<cricket::FakePortAllocatorSession*>(
allocator_
->CreateSession(sid, content_name, component, ice_ufrag,
ice_pwd)
.release()));
}
const cricket::FakePortAllocatorSession* GetPooledSession() const {
return static_cast<const cricket::FakePortAllocatorSession*>(
allocator_->GetPooledSession());
@ -76,6 +91,19 @@ TEST_F(PortAllocatorTest, TestDefaults) {
EXPECT_EQ(0, GetAllPooledSessionsReturnCount());
}
// Call CreateSession and verify that the parameters passed in and the
// candidate filter are applied as expected.
TEST_F(PortAllocatorTest, CreateSession) {
allocator_->set_candidate_filter(cricket::CF_RELAY);
auto session = CreateSession(kSessionId, kContentName, 1, kIceUfrag, kIcePwd);
ASSERT_NE(nullptr, session);
EXPECT_EQ(cricket::CF_RELAY, session->candidate_filter());
EXPECT_EQ(kContentName, session->content_name());
EXPECT_EQ(1, session->component());
EXPECT_EQ(kIceUfrag, session->ice_ufrag());
EXPECT_EQ(kIcePwd, session->ice_pwd());
}
TEST_F(PortAllocatorTest, SetConfigurationUpdatesIceServers) {
cricket::ServerAddresses stun_servers_1 = {stun_server_1};
std::vector<cricket::RelayServerConfig> turn_servers_1 = {turn_server_1};
@ -203,3 +231,17 @@ TEST_F(PortAllocatorTest, TakePooledSessionUpdatesIceParameters) {
EXPECT_EQ(kIceUfrag, session->ice_ufrag());
EXPECT_EQ(kIcePwd, session->ice_pwd());
}
// According to JSEP, candidate filtering should be done when the pooled
// candidates are surfaced to the application. This means when a pooled
// session is taken. So a pooled session should gather candidates
// unfiltered until it's returned by TakePooledSession.
TEST_F(PortAllocatorTest, TakePooledSessionUpdatesCandidateFilter) {
allocator_->set_candidate_filter(cricket::CF_RELAY);
SetConfigurationWithPoolSize(1);
auto peeked_session = GetPooledSession();
ASSERT_NE(nullptr, peeked_session);
EXPECT_EQ(cricket::CF_ALL, peeked_session->candidate_filter());
auto session = TakePooledSession();
EXPECT_EQ(cricket::CF_RELAY, session->candidate_filter());
}

5
webrtc/p2p/base/stunport.cc
View File

@ -448,10 +448,7 @@ void UDPPort::OnStunBindingRequestSucceeded(
rtc::SocketAddress related_address = socket_->GetLocalAddress();
// If we can't stamp the related address correctly, empty it to avoid leak.
if (!MaybeSetDefaultLocalAddress(&related_address) ||
!(candidate_filter() & CF_HOST)) {
// If candidate filter doesn't have CF_HOST specified, empty raddr to
// avoid local address leakage.
if (!MaybeSetDefaultLocalAddress(&related_address)) {
related_address = rtc::EmptySocketAddressWithFamily(
related_address.family());
}

5
webrtc/p2p/base/turnport.cc
View File

@ -720,11 +720,6 @@ void TurnPort::OnAllocateSuccess(const rtc::SocketAddress& address,
state_ = STATE_READY;
rtc::SocketAddress related_address = stun_address;
if (!(candidate_filter() & CF_REFLEXIVE)) {
// If candidate filter only allows relay type of address, empty raddr to
// avoid local address leakage.
related_address = rtc::EmptySocketAddressWithFamily(stun_address.family());
}
// For relayed candidate, Base is the candidate itself.
AddAddress(address, // Candidate address.

122
webrtc/p2p/client/basicportallocator.cc
View File

@ -167,6 +167,29 @@ BasicPortAllocatorSession::~BasicPortAllocatorSession() {
delete sequences_[i];
}
void BasicPortAllocatorSession::SetCandidateFilter(uint32_t filter) {
if (filter == candidate_filter_) {
return;
}
// We assume the filter will only change from "ALL" to something else.
RTC_DCHECK(candidate_filter_ == CF_ALL);
candidate_filter_ = filter;
for (PortData& port : ports_) {
if (!port.has_pairable_candidate()) {
continue;
}
const auto& candidates = port.port()->Candidates();
// Setting a filter may cause a ready port to become non-ready
// if it no longer has any pairable candidates.
if (!std::any_of(candidates.begin(), candidates.end(),
[this, &port](const Candidate& candidate) {
return CandidatePairable(candidate, port.port());
})) {
port.set_has_pairable_candidate(false);
}
}
}
void BasicPortAllocatorSession::StartGettingPorts() {
network_thread_ = rtc::Thread::Current();
if (!socket_factory_) {
@ -195,7 +218,7 @@ void BasicPortAllocatorSession::ClearGettingPorts() {
std::vector<PortInterface*> BasicPortAllocatorSession::ReadyPorts() const {
std::vector<PortInterface*> ret;
for (const PortData& port : ports_) {
if (port.ready() || port.complete()) {
if (port.has_pairable_candidate() && !port.error()) {
ret.push_back(port.port());
}
}
@ -214,12 +237,32 @@ std::vector<Candidate> BasicPortAllocatorSession::ReadyCandidates() const {
!data.sequence()->ProtocolEnabled(pvalue)) {
continue;
}
candidates.push_back(candidate);
candidates.push_back(SanitizeRelatedAddress(candidate));
}
}
return candidates;
}
Candidate BasicPortAllocatorSession::SanitizeRelatedAddress(
const Candidate& c) const {
Candidate copy = c;
// If adapter enumeration is disabled or host candidates are disabled,
// clear the raddr of STUN candidates to avoid local address leakage.
bool filter_stun_related_address =
((flags() & PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION) &&
(flags() & PORTALLOCATOR_DISABLE_DEFAULT_LOCAL_CANDIDATE)) ||
!(candidate_filter_ & CF_HOST);
// If the candidate filter doesn't allow reflexive addresses, empty TURN raddr
// to avoid reflexive address leakage.
bool filter_turn_related_address = !(candidate_filter_ & CF_REFLEXIVE);
if ((c.type() == STUN_PORT_TYPE && filter_stun_related_address) ||
(c.type() == RELAY_PORT_TYPE && filter_turn_related_address)) {
copy.set_related_address(
rtc::EmptySocketAddressWithFamily(copy.address().family()));
}
return copy;
}
bool BasicPortAllocatorSession::CandidatesAllocationDone() const {
// Done only if all required AllocationSequence objects
// are created.
@ -483,17 +526,6 @@ void BasicPortAllocatorSession::AddAllocatedPort(Port* port,
port->set_send_retransmit_count_attribute(
(flags() & PORTALLOCATOR_ENABLE_STUN_RETRANSMIT_ATTRIBUTE) != 0);
// Push down the candidate_filter to individual port.
uint32_t candidate_filter = allocator_->candidate_filter();
// When adapter enumeration is disabled, disable CF_HOST at port level so
// local address is not leaked by stunport in the candidate's related address.
if ((flags() & PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION) &&
(flags() & PORTALLOCATOR_DISABLE_DEFAULT_LOCAL_CANDIDATE)) {
candidate_filter &= ~CF_HOST;
}
port->set_candidate_filter(candidate_filter);
PortData data(port, seq);
ports_.push_back(data);
@ -529,44 +561,29 @@ void BasicPortAllocatorSession::OnCandidateReady(
}
ProtocolType pvalue;
bool candidate_signalable = CheckCandidateFilter(c);
// When device enumeration is disabled (to prevent non-default IP addresses
// from leaking), we ping from some local candidates even though we don't
// signal them. However, if host candidates are also disabled (for example, to
// prevent even default IP addresses from leaking), we still don't want to
// ping from them, even if device enumeration is disabled. Thus, we check for
// both device enumeration and host candidates being disabled.
bool network_enumeration_disabled = c.address().IsAnyIP();
bool can_ping_from_candidate =
(port->SharedSocket() || c.protocol() == TCP_PROTOCOL_NAME);
bool host_canidates_disabled = !(allocator_->candidate_filter() & CF_HOST);
bool candidate_pairable =
candidate_signalable ||
(network_enumeration_disabled && can_ping_from_candidate &&
!host_canidates_disabled);
bool candidate_protocol_enabled =
StringToProto(c.protocol().c_str(), &pvalue) &&
data->sequence()->ProtocolEnabled(pvalue);
if (candidate_signalable && candidate_protocol_enabled) {
if (CheckCandidateFilter(c) && candidate_protocol_enabled) {
std::vector<Candidate> candidates;
candidates.push_back(c);
candidates.push_back(SanitizeRelatedAddress(c));
SignalCandidatesReady(this, candidates);
}
// Port has been made ready. Nothing to do here.
if (data->ready()) {
// Port has already been marked as having a pairable candidate.
// Nothing to do here.
if (data->has_pairable_candidate()) {
return;
}
// Move the port to the READY state, either because we have a usable candidate
// from the port, or simply because the port is bound to the any address and
// therefore has no host candidate. This will trigger the port to start
// creating candidate pairs (connections) and issue connectivity checks.
if (candidate_pairable) {
data->set_ready();
// Mark that the port has a pairable candidate, either because we have a
// usable candidate from the port, or simply because the port is bound to the
// any address and therefore has no host candidate. This will trigger the port
// to start creating candidate pairs (connections) and issue connectivity
// checks.
if (CandidatePairable(c, port)) {
data->set_has_pairable_candidate(true);
SignalPortReady(this, port);
}
}
@ -613,8 +630,9 @@ void BasicPortAllocatorSession::OnProtocolEnabled(AllocationSequence* seq,
const std::vector<Candidate>& potentials = it->port()->Candidates();
for (size_t i = 0; i < potentials.size(); ++i) {
if (!CheckCandidateFilter(potentials[i]))
if (!CheckCandidateFilter(potentials[i])) {
continue;
}
ProtocolType pvalue;
bool candidate_protocol_enabled =
StringToProto(potentials[i].protocol().c_str(), &pvalue) &&
@ -631,7 +649,7 @@ void BasicPortAllocatorSession::OnProtocolEnabled(AllocationSequence* seq,
}
bool BasicPortAllocatorSession::CheckCandidateFilter(const Candidate& c) const {
uint32_t filter = allocator_->candidate_filter();
uint32_t filter = candidate_filter_;
// When binding to any address, before sending packets out, the getsockname
// returns all 0s, but after sending packets, it'll be the NIC used to
@ -661,6 +679,26 @@ bool BasicPortAllocatorSession::CheckCandidateFilter(const Candidate& c) const {
return false;
}
bool BasicPortAllocatorSession::CandidatePairable(const Candidate& c,
const Port* port) const {
bool candidate_signalable = CheckCandidateFilter(c);
// When device enumeration is disabled (to prevent non-default IP addresses
// from leaking), we ping from some local candidates even though we don't
// signal them. However, if host candidates are also disabled (for example, to
// prevent even default IP addresses from leaking), we still don't want to
// ping from them, even if device enumeration is disabled. Thus, we check for
// both device enumeration and host candidates being disabled.
bool network_enumeration_disabled = c.address().IsAnyIP();
bool can_ping_from_candidate =
(port->SharedSocket() || c.protocol() == TCP_PROTOCOL_NAME);
bool host_candidates_disabled = !(candidate_filter_ & CF_HOST);
return candidate_signalable ||
(network_enumeration_disabled && can_ping_from_candidate &&
!host_candidates_disabled);
}
void BasicPortAllocatorSession::OnPortAllocationComplete(
AllocationSequence* seq) {
// Send candidate allocation complete signal if all ports are done.

36
webrtc/p2p/client/basicportallocator.h
View File

@ -88,6 +88,7 @@ class BasicPortAllocatorSession : public PortAllocatorSession,
rtc::Thread* network_thread() { return network_thread_; }
rtc::PacketSocketFactory* socket_factory() { return socket_factory_; }
void SetCandidateFilter(uint32_t filter) override;
void StartGettingPorts() override;
void StopGettingPorts() override;
void ClearGettingPorts() override;
@ -113,36 +114,40 @@ class BasicPortAllocatorSession : public PortAllocatorSession,
private:
class PortData {
public:
PortData() : port_(NULL), sequence_(NULL), state_(STATE_INIT) {}
PortData() {}
PortData(Port* port, AllocationSequence* seq)
: port_(port), sequence_(seq), state_(STATE_INIT) {
}
: port_(port), sequence_(seq) {}
Port* port() const { return port_; }
AllocationSequence* sequence() const { return sequence_; }
bool ready() const { return state_ == STATE_READY; }
bool has_pairable_candidate() const { return has_pairable_candidate_; }
bool complete() const { return state_ == STATE_COMPLETE; }
bool error() const { return state_ == STATE_ERROR; }
void set_ready() { ASSERT(state_ == STATE_INIT); state_ = STATE_READY; }
void set_has_pairable_candidate(bool has_pairable_candidate) {
if (has_pairable_candidate) {
ASSERT(state_ == STATE_INPROGRESS);
}
has_pairable_candidate_ = has_pairable_candidate;
}
void set_complete() {
state_ = STATE_COMPLETE;
}
void set_error() {
ASSERT(state_ == STATE_INIT || state_ == STATE_READY);
ASSERT(state_ == STATE_INPROGRESS);
state_ = STATE_ERROR;
}
private:
enum State {
STATE_INIT, // No candidates allocated yet.
STATE_READY, // At least one candidate is ready for process.
STATE_COMPLETE, // All candidates allocated and ready for process.
STATE_ERROR // Error in gathering candidates.
STATE_INPROGRESS, // Still gathering candidates.
STATE_COMPLETE, // All candidates allocated and ready for process.
STATE_ERROR // Error in gathering candidates.
};
Port* port_;
AllocationSequence* sequence_;
State state_;
Port* port_ = nullptr;
AllocationSequence* sequence_ = nullptr;
State state_ = STATE_INPROGRESS;
bool has_pairable_candidate_ = false;
};
void OnConfigReady(PortConfiguration* config);
@ -168,6 +173,10 @@ class BasicPortAllocatorSession : public PortAllocatorSession,
void GetNetworks(std::vector<rtc::Network*>* networks);
bool CheckCandidateFilter(const Candidate& c) const;
bool CandidatePairable(const Candidate& c, const Port* port) const;
// Clear the related address according to the flags and candidate filter
// in order to avoid leaking any information.
Candidate SanitizeRelatedAddress(const Candidate& c) const;
BasicPortAllocator* allocator_;
rtc::Thread* network_thread_;
@ -180,6 +189,7 @@ class BasicPortAllocatorSession : public PortAllocatorSession,
std::vector<PortConfiguration*> configs_;
std::vector<AllocationSequence*> sequences_;
std::vector<PortData> ports_;
uint32_t candidate_filter_ = CF_ALL;
friend class AllocationSequence;
};

702
webrtc/p2p/client/basicportallocator_unittest.cc
View File

File diff suppressed because it is too large Load Diff