zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Enforce "comprehension-required" STUN rules.

Browse Source 
If a STUN attribute is in the "comprehension-required" range
(0x0000-0x7FFF), and the implementation does not recognize it, this
should be treated as an error (as per RFC5389), with different behavior
depending on the type of the message received.

Bug: webrtc:9063
Change-Id: Ic31b0cdd3c26772c21d770b44fe4ee4a1b47030a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/64500
Commit-Queue: Taylor <deadbeef@webrtc.org>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30925}
This commit is contained in:
Taylor Brandstetter
2020-03-23 16:00:31 -07:00
committed by Commit Bot
parent 424204d2af
commit fb4351b085
7 changed files with 223 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
api/transport/stun.cc
View File

@ -47,6 +47,7 @@ namespace cricket {
const char STUN_ERROR_REASON_TRY_ALTERNATE_SERVER[] = "Try Alternate Server";
const char STUN_ERROR_REASON_BAD_REQUEST[] = "Bad Request";
const char STUN_ERROR_REASON_UNAUTHORIZED[] = "Unauthorized";
const char STUN_ERROR_REASON_UNKNOWN_ATTRIBUTE[] = "Unknown Attribute";
const char STUN_ERROR_REASON_FORBIDDEN[] = "Forbidden";
const char STUN_ERROR_REASON_STALE_CREDENTIALS[] = "Stale Credentials";
const char STUN_ERROR_REASON_ALLOCATION_MISMATCH[] = "Allocation Mismatch";
@ -140,6 +141,18 @@ void StunMessage::ClearAttributes() {
length_ = 0;
}
std::vector<uint16_t> StunMessage::GetNonComprehendedAttributes() const {
std::vector<uint16_t> unknown_attributes;
for (auto& attr : attrs_) {
// "comprehension-required" range is 0x0000-0x7FFF.
if (attr->type() >= 0x0000 && attr->type() <= 0x7FFF &&
GetAttributeValueType(attr->type()) == STUN_VALUE_UNKNOWN) {
unknown_attributes.push_back(attr->type());
}
}
return unknown_attributes;
}
const StunAddressAttribute* StunMessage::GetAddress(int type) const {
switch (type) {
case STUN_ATTR_MAPPED_ADDRESS: {

4
api/transport/stun.h
View File

@ -163,6 +163,10 @@ class StunMessage {
void SetType(int type) { type_ = static_cast<uint16_t>(type); }
bool SetTransactionID(const std::string& str);
// Get a list of all of the attribute types in the "comprehension required"
// range that were not recognized.
std::vector<uint16_t> GetNonComprehendedAttributes() const;
// Gets the desired attribute value, or NULL if no such attribute type exists.
const StunAddressAttribute* GetAddress(int type) const;
const StunUInt32Attribute* GetUInt32(int type) const;

71
p2p/base/port.cc
View File

@ -469,6 +469,12 @@ bool Port::GetStunMessage(const char* data,
return false;
}
// Get list of attributes in the "comprehension-required" range that were not
// comprehended. If one or more is found, the behavior differs based on the
// type of the incoming message; see below.
std::vector<uint16_t> unknown_attributes =
stun_msg->GetNonComprehendedAttributes();
if (stun_msg->type() == STUN_BINDING_REQUEST) {
// Check for the presence of USERNAME and MESSAGE-INTEGRITY (if ICE) first.
// If not present, fail with a 400 Bad Request.
@ -507,6 +513,15 @@ bool Port::GetStunMessage(const char* data,
STUN_ERROR_REASON_UNAUTHORIZED);
return true;
}
// If a request contains unknown comprehension-required attributes, reply
// with an error. See RFC5389 section 7.3.1.
if (!unknown_attributes.empty()) {
SendUnknownAttributesErrorResponse(stun_msg.get(), addr,
unknown_attributes);
return true;
}
out_username->assign(remote_ufrag);
} else if ((stun_msg->type() == STUN_BINDING_RESPONSE) ||
(stun_msg->type() == STUN_BINDING_ERROR_RESPONSE)) {
@ -527,6 +542,15 @@ bool Port::GetStunMessage(const char* data,
return true;
}
}
// If a response contains unknown comprehension-required attributes, it's
// simply discarded and the transaction is considered failed. See RFC5389
// sections 7.3.3 and 7.3.4.
if (!unknown_attributes.empty()) {
RTC_LOG(LS_ERROR) << ToString()
<< ": Discarding STUN response due to unknown "
"comprehension-required attribute";
return true;
}
// NOTE: Username should not be used in verifying response messages.
out_username->clear();
} else if (stun_msg->type() == STUN_BINDING_INDICATION) {
@ -534,6 +558,15 @@ bool Port::GetStunMessage(const char* data,
<< StunMethodToString(stun_msg->type()) << ": from "
<< addr.ToSensitiveString();
out_username->clear();
// If an indication contains unknown comprehension-required attributes,[]
// it's simply discarded. See RFC5389 section 7.3.2.
if (!unknown_attributes.empty()) {
RTC_LOG(LS_ERROR) << ToString()
<< ": Discarding STUN indication due to "
"unknown comprehension-required attribute";
return true;
}
// No stun attributes will be verified, if it's stun indication message.
// Returning from end of the this method.
} else if (stun_msg->type() == GOOG_PING_REQUEST) {
@ -749,6 +782,44 @@ void Port::SendBindingErrorResponse(StunMessage* request,
<< addr.ToSensitiveString();
}
void Port::SendUnknownAttributesErrorResponse(
StunMessage* request,
const rtc::SocketAddress& addr,
const std::vector<uint16_t>& unknown_types) {
RTC_DCHECK(request->type() == STUN_BINDING_REQUEST);
// Fill in the response message.
StunMessage response;
response.SetType(STUN_BINDING_ERROR_RESPONSE);
response.SetTransactionID(request->transaction_id());
auto error_attr = StunAttribute::CreateErrorCode();
error_attr->SetCode(STUN_ERROR_UNKNOWN_ATTRIBUTE);
error_attr->SetReason(STUN_ERROR_REASON_UNKNOWN_ATTRIBUTE);
response.AddAttribute(std::move(error_attr));
std::unique_ptr<StunUInt16ListAttribute> unknown_attr =
StunAttribute::CreateUnknownAttributes();
for (uint16_t type : unknown_types) {
unknown_attr->AddType(type);
}
response.AddAttribute(std::move(unknown_attr));
response.AddMessageIntegrity(password_);
response.AddFingerprint();
// Send the response message.
rtc::ByteBufferWriter buf;
response.Write(&buf);
rtc::PacketOptions options(StunDscpValue());
options.info_signaled_after_sent.packet_type =
rtc::PacketType::kIceConnectivityCheckResponse;
SendTo(buf.Data(), buf.Length(), addr, options, false);
RTC_LOG(LS_ERROR) << ToString() << ": Sending STUN binding error: reason="
<< STUN_ERROR_UNKNOWN_ATTRIBUTE << " to "
<< addr.ToSensitiveString();
}
void Port::KeepAliveUntilPruned() {
// If it is pruned, we won't bring it up again.
if (state_ == State::INIT) {

4
p2p/base/port.h
View File

@ -295,6 +295,10 @@ class Port : public PortInterface,
const rtc::SocketAddress& addr,
int error_code,
const std::string& reason) override;
void SendUnknownAttributesErrorResponse(
StunMessage* request,
const rtc::SocketAddress& addr,
const std::vector<uint16_t>& unknown_types);
void set_proxy(const std::string& user_agent, const rtc::ProxyInfo& proxy) {
user_agent_ = user_agent;

104
p2p/base/port_unittest.cc
View File

@ -2222,6 +2222,110 @@ TEST_F(PortTest, TestHandleStunMessageBadFingerprint) {
EXPECT_EQ(0, port->last_stun_error_code());
}
// Test handling a STUN message with unknown attributes in the
// "comprehension-required" range. Should respond with an error with the
// unknown attributes' IDs.
TEST_F(PortTest,
TestHandleStunRequestWithUnknownComprehensionRequiredAttribute) {
// Our port will act as the "remote" port.
std::unique_ptr<TestPort> port(CreateTestPort(kLocalAddr2, "rfrag", "rpass"));
std::unique_ptr<IceMessage> in_msg, out_msg;
auto buf = std::make_unique<ByteBufferWriter>();
rtc::SocketAddress addr(kLocalAddr1);
std::string username;
// Build ordinary message with valid ufrag/pass.
in_msg = CreateStunMessageWithUsername(STUN_BINDING_REQUEST, "rfrag:lfrag");
in_msg->AddMessageIntegrity("rpass");
// Add a couple attributes with ID in comprehension-required range.
in_msg->AddAttribute(StunAttribute::CreateUInt32(0x7777));
in_msg->AddAttribute(StunAttribute::CreateUInt32(0x4567));
// ... And one outside the range.
in_msg->AddAttribute(StunAttribute::CreateUInt32(0xdead));
in_msg->AddFingerprint();
WriteStunMessage(*in_msg, buf.get());
ASSERT_TRUE(port->GetStunMessage(buf->Data(), buf->Length(), addr, &out_msg,
&username));
IceMessage* error_response = port->last_stun_msg();
ASSERT_NE(nullptr, error_response);
// Verify that the "unknown attribute" error response has the right error
// code, and includes an attribute that lists out the unrecognized attribute
// types.
EXPECT_EQ(STUN_ERROR_UNKNOWN_ATTRIBUTE, error_response->GetErrorCodeValue());
const StunUInt16ListAttribute* unknown_attributes =
error_response->GetUnknownAttributes();
ASSERT_NE(nullptr, unknown_attributes);
ASSERT_EQ(2u, unknown_attributes->Size());
EXPECT_EQ(0x7777, unknown_attributes->GetType(0));
EXPECT_EQ(0x4567, unknown_attributes->GetType(1));
}
// Similar to the above, but with a response instead of a request. In this
// case the response should just be ignored and transaction treated is failed.
TEST_F(PortTest,
TestHandleStunResponseWithUnknownComprehensionRequiredAttribute) {
// Generic setup.
auto lport = CreateTestPort(kLocalAddr1, "lfrag", "lpass");
lport->SetIceRole(cricket::ICEROLE_CONTROLLING);
auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass");
rport->SetIceRole(cricket::ICEROLE_CONTROLLED);
lport->PrepareAddress();
rport->PrepareAddress();
ASSERT_FALSE(lport->Candidates().empty());
ASSERT_FALSE(rport->Candidates().empty());
Connection* lconn =
lport->CreateConnection(rport->Candidates()[0], Port::ORIGIN_MESSAGE);
Connection* rconn =
rport->CreateConnection(lport->Candidates()[0], Port::ORIGIN_MESSAGE);
// Send request.
lconn->Ping(0);
ASSERT_TRUE_WAIT(lport->last_stun_msg() != NULL, kDefaultTimeout);
rconn->OnReadPacket(lport->last_stun_buf()->data<char>(),
lport->last_stun_buf()->size(), /* packet_time_us */ -1);
// Intercept request and add comprehension required attribute.
ASSERT_TRUE_WAIT(rport->last_stun_msg() != NULL, kDefaultTimeout);
auto modified_response = rport->last_stun_msg()->Clone();
modified_response->AddAttribute(StunAttribute::CreateUInt32(0x7777));
modified_response->RemoveAttribute(STUN_ATTR_FINGERPRINT);
modified_response->AddFingerprint();
ByteBufferWriter buf;
WriteStunMessage(*modified_response, &buf);
lconn->OnReadPacket(buf.Data(), buf.Length(), /* packet_time_us */ -1);
// Response should have been ignored, leaving us unwritable still.
EXPECT_FALSE(lconn->writable());
}
// Similar to the above, but with an indication. As with a response, it should
// just be ignored.
TEST_F(PortTest,
TestHandleStunIndicationWithUnknownComprehensionRequiredAttribute) {
// Generic set up.
auto lport = CreateTestPort(kLocalAddr2, "lfrag", "lpass");
lport->SetIceRole(cricket::ICEROLE_CONTROLLING);
auto rport = CreateTestPort(kLocalAddr2, "rfrag", "rpass");
rport->SetIceRole(cricket::ICEROLE_CONTROLLED);
lport->PrepareAddress();
rport->PrepareAddress();
ASSERT_FALSE(lport->Candidates().empty());
ASSERT_FALSE(rport->Candidates().empty());
Connection* lconn =
lport->CreateConnection(rport->Candidates()[0], Port::ORIGIN_MESSAGE);
// Generate indication with comprehension required attribute and verify it
// doesn't update last_ping_received.
auto in_msg = CreateStunMessage(STUN_BINDING_INDICATION);
in_msg->AddAttribute(StunAttribute::CreateUInt32(0x7777));
in_msg->AddFingerprint();
ByteBufferWriter buf;
WriteStunMessage(*in_msg, &buf);
lconn->OnReadPacket(buf.Data(), buf.Length(), /* packet_time_us */ -1);
EXPECT_EQ(0u, lconn->last_ping_received());
}
// Test handling of STUN binding indication messages . STUN binding
// indications are allowed only to the connection which is in read mode.
TEST_F(PortTest, TestHandleStunBindingIndication) {

10
p2p/base/stun_request.cc
View File

@ -125,7 +125,15 @@ bool StunRequestManager::CheckResponse(StunMessage* msg) {
}
StunRequest* request = iter->second;
if (msg->type() == GetStunSuccessResponseType(request->type())) {
if (!msg->GetNonComprehendedAttributes().empty()) {
// If a response contains unknown comprehension-required attributes, it's
// simply discarded and the transaction is considered failed. See RFC5389
// sections 7.3.3 and 7.3.4.
RTC_LOG(LS_ERROR) << ": Discarding response due to unknown "
"comprehension-required attribute.";
delete request;
return false;
} else if (msg->type() == GetStunSuccessResponseType(request->type())) {
request->OnResponse(msg);
} else if (msg->type() == GetStunErrorResponseType(request->type())) {
request->OnErrorResponse(msg);

18
p2p/base/stun_request_unittest.cc
View File

@ -198,4 +198,22 @@ TEST_F(StunRequestTest, TestNoEmptyRequest) {
delete res;
}
// If the response contains an attribute in the "comprehension required" range
// which is not recognized, the transaction should be considered a failure and
// the response should be ignored.
TEST_F(StunRequestTest, TestUnrecognizedComprehensionRequiredAttribute) {
StunMessage* req = CreateStunMessage(STUN_BINDING_REQUEST, NULL);
manager_.Send(new StunRequestThunker(req, this));
StunMessage* res = CreateStunMessage(STUN_BINDING_ERROR_RESPONSE, req);
res->AddAttribute(StunAttribute::CreateUInt32(0x7777));
EXPECT_FALSE(manager_.CheckResponse(res));
EXPECT_EQ(nullptr, response_);
EXPECT_FALSE(success_);
EXPECT_FALSE(failure_);
EXPECT_FALSE(timeout_);
delete res;
}
} // namespace cricket