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

GOOG_PING: improve handshake

Browse Source 
This patch improves handshake wrt GOOG_PING support so that
- if goog_ping_enable: sender send it's goog-ping version until it gets
STUN_BINDING_RESPONSE
- receiver only sends it's goog-ping-version if getting a
goog-ping-version in the request

This means that the overhead of STUN_ATTR_GOOG_MISC_INFO is only
- added on STUN_BINDING_REQUEST until a response is received.
- added on STUN_BINDING_RESPONSE if remote peer request it.

This is wire compatible with older versions so that
- new sender will enable GOOG_PING with new/old receiver.
- old sender will enable GOOG_PING with old receiver.
- old version will not enable GOOG_PING with new receiver
  (receiver expecting sender to announce first).

BUG: webrtc:11100
Change-Id: Ib3434c593988188150f4c7506918139aaf138d0c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/165787
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30269}
This commit is contained in:
Jonas Oreland 2020-01-15 15:29:48 +01:00 committed by Commit Bot
parent a846cef197
commit 219d8ce889
4 changed files with 250 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
api/transport/stun.h
View File

@ -678,7 +678,9 @@ enum IceAttributeType {
// consistent with those used in ConnectionRequest::Prepare when forming a STUN
// message for the ICE connectivity check, and they are used when parsing a
// received STUN message.
enum class IceGoogMiscInfoBindingRequestAttributeIndex {};
enum class IceGoogMiscInfoBindingRequestAttributeIndex {
SUPPORT_GOOG_PING_VERSION = 0,
};
enum class IceGoogMiscInfoBindingResponseAttributeIndex {
SUPPORT_GOOG_PING_VERSION = 0,

58
p2p/base/connection.cc
View File

@ -143,7 +143,11 @@ constexpr int64_t kMinExtraPingDelayMs = 100;
// Default field trials.
const cricket::IceFieldTrials kDefaultFieldTrials;
constexpr int kSupportGoogPingVersionIndex =
constexpr int kSupportGoogPingVersionRequestIndex =
static_cast<int>(cricket::IceGoogMiscInfoBindingRequestAttributeIndex::
SUPPORT_GOOG_PING_VERSION);
constexpr int kSupportGoogPingVersionResponseIndex =
static_cast<int>(cricket::IceGoogMiscInfoBindingResponseAttributeIndex::
SUPPORT_GOOG_PING_VERSION);
@ -224,6 +228,17 @@ void ConnectionRequest::Prepare(StunMessage* request) {
request->AddAttribute(std::make_unique<StunUInt32Attribute>(
STUN_ATTR_PRIORITY, prflx_priority));
if (connection_->field_trials_->enable_goog_ping &&
!connection_->remote_support_goog_ping_.has_value()) {
// Check if remote supports GOOG PING by announcing which version we
// support. This is sent on all STUN_BINDING_REQUEST until we get a
// STUN_BINDING_RESPONSE.
auto list =
StunAttribute::CreateUInt16ListAttribute(STUN_ATTR_GOOG_MISC_INFO);
list->AddTypeAtIndex(kSupportGoogPingVersionRequestIndex, kGoogPingVersion);
request->AddAttribute(std::move(list));
}
if (connection_->ShouldSendGoogPing(request)) {
request->SetType(GOOG_PING_REQUEST);
request->ClearAttributes();
@ -647,10 +662,18 @@ void Connection::SendStunBindingResponse(const StunMessage* request) {
STUN_ATTR_XOR_MAPPED_ADDRESS, remote_candidate_.address()));
if (field_trials_->announce_goog_ping) {
auto list =
StunAttribute::CreateUInt16ListAttribute(STUN_ATTR_GOOG_MISC_INFO);
list->AddTypeAtIndex(kSupportGoogPingVersionIndex, kGoogPingVersion);
response.AddAttribute(std::move(list));
// Check if request contains a announce-request.
auto goog_misc = request->GetUInt16List(STUN_ATTR_GOOG_MISC_INFO);
if (goog_misc != nullptr &&
goog_misc->Size() >= kSupportGoogPingVersionRequestIndex &&
// Which version can we handle...currently any >= 1
goog_misc->GetType(kSupportGoogPingVersionRequestIndex) >= 1) {
auto list =
StunAttribute::CreateUInt16ListAttribute(STUN_ATTR_GOOG_MISC_INFO);
list->AddTypeAtIndex(kSupportGoogPingVersionResponseIndex,
kGoogPingVersion);
response.AddAttribute(std::move(list));
}
}
response.AddMessageIntegrity(local_candidate().password());
@ -1053,12 +1076,18 @@ void Connection::OnConnectionRequestResponse(ConnectionRequest* request,
response->reduced_transaction_id());
if (request->msg()->type() == STUN_BINDING_REQUEST) {
auto goog_misc = response->GetUInt16List(STUN_ATTR_GOOG_MISC_INFO);
if (goog_misc != nullptr &&
goog_misc->Size() >= kSupportGoogPingVersionIndex &&
goog_misc->GetType(kSupportGoogPingVersionIndex) >= kGoogPingVersion) {
// The remote peer has indicated that it supports GOOG_PING.
remote_support_goog_ping_ = true;
if (!remote_support_goog_ping_.has_value()) {
auto goog_misc = response->GetUInt16List(STUN_ATTR_GOOG_MISC_INFO);
if (goog_misc != nullptr &&
goog_misc->Size() >= kSupportGoogPingVersionResponseIndex) {
// The remote peer has indicated that it {does/does not} supports
// GOOG_PING.
remote_support_goog_ping_ =
goog_misc->GetType(kSupportGoogPingVersionResponseIndex) >=
kGoogPingVersion;
} else {
remote_support_goog_ping_ = false;
}
}
MaybeUpdateLocalCandidate(request, response);
@ -1289,12 +1318,15 @@ bool Connection::TooManyOutstandingPings(
}
bool Connection::ShouldSendGoogPing(const StunMessage* message) {
if (remote_support_goog_ping_ && cached_stun_binding_ &&
if (remote_support_goog_ping_ == true && cached_stun_binding_ &&
cached_stun_binding_->EqualAttributes(message, [](int type) {
// Ignore these attributes.
// NOTE: Consider what to do if adding more content to
// STUN_ATTR_GOOG_MISC_INFO
return type != STUN_ATTR_FINGERPRINT &&
type != STUN_ATTR_MESSAGE_INTEGRITY &&
type != STUN_ATTR_RETRANSMIT_COUNT;
type != STUN_ATTR_RETRANSMIT_COUNT &&
type != STUN_ATTR_GOOG_MISC_INFO;
})) {
return true;
}

5
p2p/base/p2p_transport_channel_ice_field_trials.h
View File

@ -32,8 +32,9 @@ struct IceFieldTrials {
// give us chance to find a better connection before starting.
absl::optional<int> initial_select_dampening_ping_received;
// Announce GOOG_PING support in STUN_BINDING_RESPONSE.
bool announce_goog_ping = false;
// Announce GOOG_PING support in STUN_BINDING_RESPONSE if requested
// by peer.
bool announce_goog_ping = true;
// Enable sending GOOG_PING if remote announce it.
bool enable_goog_ping = false;

210
p2p/base/port_unittest.cc
View File

@ -2648,21 +2648,36 @@ TEST_F(PortTest, TestIceLiteConnectivity) {
namespace {
// Utility function for testing goog ping.
absl::optional<int> GetSupportedGoogPingVersion(const StunMessage* response) {
auto goog_misc = response->GetUInt16List(STUN_ATTR_GOOG_MISC_INFO);
absl::optional<int> GetSupportedGoogPingVersion(const StunMessage* msg) {
auto goog_misc = msg->GetUInt16List(STUN_ATTR_GOOG_MISC_INFO);
if (goog_misc == nullptr) {
return absl::nullopt;
}
if (goog_misc->Size() <
static_cast<int>(cricket::IceGoogMiscInfoBindingResponseAttributeIndex::
SUPPORT_GOOG_PING_VERSION)) {
return absl::nullopt;
if (msg->type() == STUN_BINDING_REQUEST) {
if (goog_misc->Size() <
static_cast<int>(cricket::IceGoogMiscInfoBindingRequestAttributeIndex::
SUPPORT_GOOG_PING_VERSION)) {
return absl::nullopt;
}
return goog_misc->GetType(
static_cast<int>(cricket::IceGoogMiscInfoBindingRequestAttributeIndex::
SUPPORT_GOOG_PING_VERSION));
}
return goog_misc->GetType(
static_cast<int>(cricket::IceGoogMiscInfoBindingResponseAttributeIndex::
SUPPORT_GOOG_PING_VERSION));
if (msg->type() == STUN_BINDING_RESPONSE) {
if (goog_misc->Size() <
static_cast<int>(cricket::IceGoogMiscInfoBindingResponseAttributeIndex::
SUPPORT_GOOG_PING_VERSION)) {
return absl::nullopt;
}
return goog_misc->GetType(
static_cast<int>(cricket::IceGoogMiscInfoBindingResponseAttributeIndex::
SUPPORT_GOOG_PING_VERSION));
}
return absl::nullopt;
}
} // namespace
@ -2710,6 +2725,11 @@ TEST_P(GoogPingTest, TestGoogPingAnnounceEnable) {
ASSERT_TRUE_WAIT(port1->last_stun_msg() != NULL, kDefaultTimeout);
const IceMessage* request1 = port1->last_stun_msg();
ASSERT_EQ(trials.enable_goog_ping,
GetSupportedGoogPingVersion(request1) &&
GetSupportedGoogPingVersion(request1) >= kGoogPingVersion);
auto* con = port2->CreateConnection(port1->Candidates()[0],
cricket::Port::ORIGIN_MESSAGE);
con->SetIceFieldTrials(&trials);
@ -2718,8 +2738,8 @@ TEST_P(GoogPingTest, TestGoogPingAnnounceEnable) {
// Then check the response matches the settings.
const auto* response = port2->last_stun_msg();
ASSERT_EQ(response->type(), STUN_BINDING_RESPONSE);
ASSERT_EQ(trials.announce_goog_ping,
EXPECT_EQ(response->type(), STUN_BINDING_RESPONSE);
EXPECT_EQ(trials.enable_goog_ping && trials.announce_goog_ping,
GetSupportedGoogPingVersion(response) &&
GetSupportedGoogPingVersion(response) >= kGoogPingVersion);
@ -2741,6 +2761,10 @@ TEST_P(GoogPingTest, TestGoogPingAnnounceEnable) {
con->SendGoogPingResponse(request2);
} else {
ASSERT_EQ(request2->type(), STUN_BINDING_REQUEST);
// If we sent a BINDING with enable, and we got a reply that
// didn't contain announce, the next ping should not contain
// the enable again.
ASSERT_FALSE(GetSupportedGoogPingVersion(request2).has_value());
con->SendStunBindingResponse(request2);
}
@ -2757,6 +2781,170 @@ TEST_P(GoogPingTest, TestGoogPingAnnounceEnable) {
ch1.Stop();
}
// This test if a someone send a STUN_BINDING with unsupported version
// (kGoogPingVersion == 0)
TEST_F(PortTest, TestGoogPingUnsupportedVersionInStunBinding) {
IceFieldTrials trials;
trials.announce_goog_ping = true;
trials.enable_goog_ping = true;
auto port1_unique =
CreateTestPort(kLocalAddr1, "lfrag", "lpass",
cricket::ICEROLE_CONTROLLING, kTiebreaker1);
auto* port1 = port1_unique.get();
auto port2 = CreateTestPort(kLocalAddr2, "rfrag", "rpass",
cricket::ICEROLE_CONTROLLED, kTiebreaker2);
TestChannel ch1(std::move(port1_unique));
// Block usage of STUN_ATTR_USE_CANDIDATE so that
// ch1.conn() will sent GOOG_PING_REQUEST directly.
// This only makes test a bit shorter...
ch1.SetIceMode(ICEMODE_LITE);
// Start gathering candidates.
ch1.Start();
port2->PrepareAddress();
ASSERT_EQ_WAIT(1, ch1.complete_count(), kDefaultTimeout);
ASSERT_FALSE(port2->Candidates().empty());
ch1.CreateConnection(GetCandidate(port2.get()));
ASSERT_TRUE(ch1.conn() != NULL);
EXPECT_EQ(Connection::STATE_WRITE_INIT, ch1.conn()->write_state());
ch1.conn()->SetIceFieldTrials(&trials);
// Send ping.
ch1.Ping();
ASSERT_TRUE_WAIT(port1->last_stun_msg() != NULL, kDefaultTimeout);
const IceMessage* request1 = port1->last_stun_msg();
ASSERT_TRUE(GetSupportedGoogPingVersion(request1) &&
GetSupportedGoogPingVersion(request1) >= kGoogPingVersion);
// Modify the STUN message request1 to send GetSupportedGoogPingVersion == 0
auto modified_request1 = request1->Clone();
ASSERT_TRUE(modified_request1->RemoveAttribute(STUN_ATTR_GOOG_MISC_INFO) !=
nullptr);
ASSERT_TRUE(modified_request1->RemoveAttribute(STUN_ATTR_MESSAGE_INTEGRITY) !=
nullptr);
{
auto list =
StunAttribute::CreateUInt16ListAttribute(STUN_ATTR_GOOG_MISC_INFO);
list->AddTypeAtIndex(
static_cast<uint16_t>(
cricket::IceGoogMiscInfoBindingRequestAttributeIndex::
SUPPORT_GOOG_PING_VERSION),
/* version */ 0);
modified_request1->AddAttribute(std::move(list));
modified_request1->AddMessageIntegrity("rpass");
}
auto* con = port2->CreateConnection(port1->Candidates()[0],
cricket::Port::ORIGIN_MESSAGE);
con->SetIceFieldTrials(&trials);
con->SendStunBindingResponse(modified_request1.get());
// Then check the response matches the settings.
const auto* response = port2->last_stun_msg();
EXPECT_EQ(response->type(), STUN_BINDING_RESPONSE);
EXPECT_FALSE(GetSupportedGoogPingVersion(response));
ch1.Stop();
}
// This test if a someone send a STUN_BINDING_RESPONSE with unsupported version
// (kGoogPingVersion == 0)
TEST_F(PortTest, TestGoogPingUnsupportedVersionInStunBindingResponse) {
IceFieldTrials trials;
trials.announce_goog_ping = true;
trials.enable_goog_ping = true;
auto port1_unique =
CreateTestPort(kLocalAddr1, "lfrag", "lpass",
cricket::ICEROLE_CONTROLLING, kTiebreaker1);
auto* port1 = port1_unique.get();
auto port2 = CreateTestPort(kLocalAddr2, "rfrag", "rpass",
cricket::ICEROLE_CONTROLLED, kTiebreaker2);
TestChannel ch1(std::move(port1_unique));
// Block usage of STUN_ATTR_USE_CANDIDATE so that
// ch1.conn() will sent GOOG_PING_REQUEST directly.
// This only makes test a bit shorter...
ch1.SetIceMode(ICEMODE_LITE);
// Start gathering candidates.
ch1.Start();
port2->PrepareAddress();
ASSERT_EQ_WAIT(1, ch1.complete_count(), kDefaultTimeout);
ASSERT_FALSE(port2->Candidates().empty());
ch1.CreateConnection(GetCandidate(port2.get()));
ASSERT_TRUE(ch1.conn() != NULL);
EXPECT_EQ(Connection::STATE_WRITE_INIT, ch1.conn()->write_state());
ch1.conn()->SetIceFieldTrials(&trials);
// Send ping.
ch1.Ping();
ASSERT_TRUE_WAIT(port1->last_stun_msg() != NULL, kDefaultTimeout);
const IceMessage* request1 = port1->last_stun_msg();
ASSERT_TRUE(GetSupportedGoogPingVersion(request1) &&
GetSupportedGoogPingVersion(request1) >= kGoogPingVersion);
auto* con = port2->CreateConnection(port1->Candidates()[0],
cricket::Port::ORIGIN_MESSAGE);
con->SetIceFieldTrials(&trials);
con->SendStunBindingResponse(request1);
// Then check the response matches the settings.
const auto* response = port2->last_stun_msg();
EXPECT_EQ(response->type(), STUN_BINDING_RESPONSE);
EXPECT_TRUE(GetSupportedGoogPingVersion(response));
// Modify the STUN message response to contain GetSupportedGoogPingVersion ==
// 0
auto modified_response = response->Clone();
ASSERT_TRUE(modified_response->RemoveAttribute(STUN_ATTR_GOOG_MISC_INFO) !=
nullptr);
ASSERT_TRUE(modified_response->RemoveAttribute(STUN_ATTR_MESSAGE_INTEGRITY) !=
nullptr);
ASSERT_TRUE(modified_response->RemoveAttribute(STUN_ATTR_FINGERPRINT) !=
nullptr);
{
auto list =
StunAttribute::CreateUInt16ListAttribute(STUN_ATTR_GOOG_MISC_INFO);
list->AddTypeAtIndex(
static_cast<uint16_t>(
cricket::IceGoogMiscInfoBindingResponseAttributeIndex::
SUPPORT_GOOG_PING_VERSION),
/* version */ 0);
modified_response->AddAttribute(std::move(list));
modified_response->AddMessageIntegrity("rpass");
modified_response->AddFingerprint();
}
rtc::ByteBufferWriter buf;
modified_response->Write(&buf);
// Feeding the modified respone message back.
ch1.conn()->OnReadPacket(buf.Data(), buf.Length(), /* packet_time_us */ -1);
port1->Reset();
port2->Reset();
ch1.Ping();
ASSERT_TRUE_WAIT(port1->last_stun_msg() != NULL, kDefaultTimeout);
// This should now be a STUN_BINDING...without a kGoogPingVersion
const IceMessage* request2 = port1->last_stun_msg();
EXPECT_EQ(request2->type(), STUN_BINDING_REQUEST);
EXPECT_FALSE(GetSupportedGoogPingVersion(request2));
ch1.Stop();
}
INSTANTIATE_TEST_SUITE_P(GoogPingTest,
GoogPingTest,
// test all combinations of <announce, enable> pairs.