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

Don't crash if STUN error message is missing ERROR-CODE attribute.

Browse Source 
This is something a well-behaving STUN server shouldn't do, but we shouldn't
crash if it does happen.

Also adding helper function for the common operation of extracting just
the error code out of a STUN packet.

BUG=chromium:708469

Review-Url: https://codereview.webrtc.org/2837133003
Cr-Commit-Position: refs/heads/master@{#17892}
This commit is contained in:
deadbeef 2017-04-26 09:21:22 -07:00 committed by Commit bot
parent 65881de6c8
commit 996fc6bdb7
7 changed files with 42 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1411,12 +1411,7 @@ void Connection::OnConnectionRequestResponse(ConnectionRequest* request,
void Connection::OnConnectionRequestErrorResponse(ConnectionRequest* request,
StunMessage* response) {
const StunErrorCodeAttribute* error_attr = response->GetErrorCode();
int error_code = STUN_ERROR_GLOBAL_FAILURE;
if (error_attr) {
error_code = error_attr->code();
}
int error_code = response->GetErrorCodeValue();
LOG_J(LS_INFO, this) << "Received STUN error response"
<< " id=" << rtc::hex_encode(request->id())
<< " code=" << error_code

2
webrtc/p2p/base/relayport.cc
View File

@ -824,7 +824,7 @@ void AllocateRequest::OnResponse(StunMessage* response) {
void AllocateRequest::OnErrorResponse(StunMessage* response) {
const StunErrorCodeAttribute* attr = response->GetErrorCode();
if (!attr) {
LOG(INFO) << "Bad allocate response error code";
LOG(LS_ERROR) << "Missing allocate response error code.";
} else {
LOG(INFO) << "Allocate error response:"
<< " code=" << attr->code()

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

@ -115,6 +115,11 @@ const StunErrorCodeAttribute* StunMessage::GetErrorCode() const {
GetAttribute(STUN_ATTR_ERROR_CODE));
}
int StunMessage::GetErrorCodeValue() const {
const StunErrorCodeAttribute* error_attribute = GetErrorCode();
return error_attribute ? error_attribute->code() : STUN_ERROR_GLOBAL_FAILURE;
}
const StunUInt16ListAttribute* StunMessage::GetUnknownAttributes() const {
return static_cast<const StunUInt16ListAttribute*>(
GetAttribute(STUN_ATTR_UNKNOWN_ATTRIBUTES));

3
webrtc/p2p/base/stun.h
View File

@ -156,6 +156,9 @@ class StunMessage {
// Gets these specific attribute values.
const StunErrorCodeAttribute* GetErrorCode() const;
// Returns the code inside the error code attribute, if present, and
// STUN_ERROR_GLOBAL_FAILURE otherwise.
int GetErrorCodeValue() const;
const StunUInt16ListAttribute* GetUnknownAttributes() const;
// Takes ownership of the specified attribute and adds it to the message.

9
webrtc/p2p/base/stun_unittest.cc
View File

@ -1016,6 +1016,15 @@ TEST_F(StunTest, ReadErrorCodeAttribute) {
EXPECT_EQ(kTestErrorNumber, errorcode->number());
EXPECT_EQ(kTestErrorReason, errorcode->reason());
EXPECT_EQ(kTestErrorCode, errorcode->code());
EXPECT_EQ(kTestErrorCode, msg.GetErrorCodeValue());
}
// Test that GetErrorCodeValue returns STUN_ERROR_GLOBAL_FAILURE if the message
// in question doesn't have an error code attribute, rather than crashing.
TEST_F(StunTest, GetErrorCodeValueWithNoErrorAttribute) {
StunMessage msg;
ReadStunMessage(&msg, kStunMessageWithIPv6MappedAddress);
EXPECT_EQ(STUN_ERROR_GLOBAL_FAILURE, msg.GetErrorCodeValue());
}
TEST_F(StunTest, ReadMessageWithAUInt16ListAttribute) {

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

@ -66,7 +66,7 @@ class StunBindingRequest : public StunRequest {
virtual void OnErrorResponse(StunMessage* response) override {
const StunErrorCodeAttribute* attr = response->GetErrorCode();
if (!attr) {
LOG(LS_ERROR) << "Bad allocate response error code";
LOG(LS_ERROR) << "Missing binding response error code.";
} else {
LOG(LS_ERROR) << "Binding error response:"
<< " class=" << attr->eclass()

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

@ -1165,19 +1165,18 @@ void TurnAllocateRequest::OnResponse(StunMessage* response) {
void TurnAllocateRequest::OnErrorResponse(StunMessage* response) {
// Process error response according to RFC5766, Section 6.4.
const StunErrorCodeAttribute* error_code = response->GetErrorCode();
int error_code = response->GetErrorCodeValue();
LOG_J(LS_INFO, port_) << "Received TURN allocate error response"
<< ", id=" << rtc::hex_encode(id())
<< ", code=" << error_code->code()
<< ", rtt=" << Elapsed();
<< ", code=" << error_code << ", rtt=" << Elapsed();
switch (error_code->code()) {
switch (error_code) {
case STUN_ERROR_UNAUTHORIZED: // Unauthrorized.
OnAuthChallenge(response, error_code->code());
OnAuthChallenge(response, error_code);
break;
case STUN_ERROR_TRY_ALTERNATE:
OnTryAlternate(response, error_code->code());
OnTryAlternate(response, error_code);
break;
case STUN_ERROR_ALLOCATION_MISMATCH:
// We must handle this error async because trying to delete the socket in
@ -1186,10 +1185,10 @@ void TurnAllocateRequest::OnErrorResponse(StunMessage* response) {
TurnPort::MSG_ALLOCATE_MISMATCH);
break;
default:
LOG_J(LS_WARNING, port_) << "Received TURN allocate error response"
<< ", id=" << rtc::hex_encode(id())
<< ", code=" << error_code->code()
<< ", rtt=" << Elapsed();
LOG_J(LS_WARNING, port_)
<< "Received TURN allocate error response"
<< ", id=" << rtc::hex_encode(id()) << ", code=" << error_code
<< ", rtt=" << Elapsed();
port_->OnAllocateError();
}
}
@ -1321,20 +1320,20 @@ void TurnRefreshRequest::OnResponse(StunMessage* response) {
}
void TurnRefreshRequest::OnErrorResponse(StunMessage* response) {
const StunErrorCodeAttribute* error_code = response->GetErrorCode();
int error_code = response->GetErrorCodeValue();
if (error_code->code() == STUN_ERROR_STALE_NONCE) {
if (error_code == STUN_ERROR_STALE_NONCE) {
if (port_->UpdateNonce(response)) {
// Send RefreshRequest immediately.
port_->SendRequest(new TurnRefreshRequest(port_), 0);
}
} else {
LOG_J(LS_WARNING, port_) << "Received TURN refresh error response"
<< ", id=" << rtc::hex_encode(id())
<< ", code=" << error_code->code()
<< ", rtt=" << Elapsed();
LOG_J(LS_WARNING, port_)
<< "Received TURN refresh error response"
<< ", id=" << rtc::hex_encode(id()) << ", code=" << error_code
<< ", rtt=" << Elapsed();
port_->OnRefreshError();
port_->SignalTurnRefreshResult(port_, error_code->code());
port_->SignalTurnRefreshResult(port_, error_code);
}
}
@ -1380,13 +1379,12 @@ void TurnCreatePermissionRequest::OnResponse(StunMessage* response) {
}
void TurnCreatePermissionRequest::OnErrorResponse(StunMessage* response) {
const StunErrorCodeAttribute* error_code = response->GetErrorCode();
int error_code = response->GetErrorCodeValue();
LOG_J(LS_WARNING, port_) << "Received TURN create permission error response"
<< ", id=" << rtc::hex_encode(id())
<< ", code=" << error_code->code()
<< ", rtt=" << Elapsed();
<< ", code=" << error_code << ", rtt=" << Elapsed();
if (entry_) {
entry_->OnCreatePermissionError(response, error_code->code());
entry_->OnCreatePermissionError(response, error_code);
}
}
@ -1450,13 +1448,12 @@ void TurnChannelBindRequest::OnResponse(StunMessage* response) {
}
void TurnChannelBindRequest::OnErrorResponse(StunMessage* response) {
const StunErrorCodeAttribute* error_code = response->GetErrorCode();
int error_code = response->GetErrorCodeValue();
LOG_J(LS_WARNING, port_) << "Received TURN channel bind error response"
<< ", id=" << rtc::hex_encode(id())
<< ", code=" << error_code->code()
<< ", rtt=" << Elapsed();
<< ", code=" << error_code << ", rtt=" << Elapsed();
if (entry_) {
entry_->OnChannelBindError(response, error_code->code());
entry_->OnChannelBindError(response, error_code);
}
}