Modified STUN verification functions
The new verification makes verification a function on a message.
It also stores the password used in the request message, so that
it is easily accessible when verifying the response.
Bug: chromium:1177125
Change-Id: I505df4b54214643a28a6b292c4e2262b9d97b097
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/209060
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33366}
diff --git a/api/transport/stun.cc b/api/transport/stun.cc
index e1bf03b..1b5bf0c 100644
--- a/api/transport/stun.cc
+++ b/api/transport/stun.cc
@@ -246,6 +246,31 @@
GetAttribute(STUN_ATTR_UNKNOWN_ATTRIBUTES));
}
+StunMessage::IntegrityStatus StunMessage::ValidateMessageIntegrity(
+ const std::string& password) {
+ password_ = password;
+ if (GetByteString(STUN_ATTR_MESSAGE_INTEGRITY)) {
+ if (ValidateMessageIntegrityOfType(
+ STUN_ATTR_MESSAGE_INTEGRITY, kStunMessageIntegritySize,
+ buffer_.c_str(), buffer_.size(), password)) {
+ integrity_ = IntegrityStatus::kIntegrityOk;
+ } else {
+ integrity_ = IntegrityStatus::kIntegrityBad;
+ }
+ } else if (GetByteString(STUN_ATTR_GOOG_MESSAGE_INTEGRITY_32)) {
+ if (ValidateMessageIntegrityOfType(
+ STUN_ATTR_GOOG_MESSAGE_INTEGRITY_32, kStunMessageIntegrity32Size,
+ buffer_.c_str(), buffer_.size(), password)) {
+ integrity_ = IntegrityStatus::kIntegrityOk;
+ } else {
+ integrity_ = IntegrityStatus::kIntegrityBad;
+ }
+ } else {
+ integrity_ = IntegrityStatus::kNoIntegrity;
+ }
+ return integrity_;
+}
+
bool StunMessage::ValidateMessageIntegrity(const char* data,
size_t size,
const std::string& password) {
@@ -353,11 +378,6 @@
password.size());
}
-bool StunMessage::AddMessageIntegrity(const char* key, size_t keylen) {
- return AddMessageIntegrityOfType(STUN_ATTR_MESSAGE_INTEGRITY,
- kStunMessageIntegritySize, key, keylen);
-}
-
bool StunMessage::AddMessageIntegrity32(absl::string_view password) {
return AddMessageIntegrityOfType(STUN_ATTR_GOOG_MESSAGE_INTEGRITY_32,
kStunMessageIntegrity32Size, password.data(),
@@ -395,6 +415,8 @@
// Insert correct HMAC into the attribute.
msg_integrity_attr->CopyBytes(hmac, attr_size);
+ password_.assign(key, keylen);
+ integrity_ = IntegrityStatus::kIntegrityOk;
return true;
}
@@ -473,6 +495,9 @@
}
bool StunMessage::Read(ByteBufferReader* buf) {
+ // Keep a copy of the buffer data around for later verification.
+ buffer_.assign(buf->Data(), buf->Length());
+
if (!buf->ReadUInt16(&type_)) {
return false;
}
diff --git a/api/transport/stun.h b/api/transport/stun.h
index 8893b2a..682a17a 100644
--- a/api/transport/stun.h
+++ b/api/transport/stun.h
@@ -16,6 +16,7 @@
#include <stddef.h>
#include <stdint.h>
+
#include <functional>
#include <memory>
#include <string>
@@ -149,15 +150,24 @@
StunMessage();
virtual ~StunMessage();
+ // The verification status of the message. This is checked on parsing,
+ // or set by AddMessageIntegrity.
+ enum class IntegrityStatus {
+ kNotSet,
+ kNoIntegrity, // Message-integrity attribute missing
+ kIntegrityOk, // Message-integrity checked OK
+ kIntegrityBad, // Message-integrity verification failed
+ };
+
int type() const { return type_; }
size_t length() const { return length_; }
const std::string& transaction_id() const { return transaction_id_; }
uint32_t reduced_transaction_id() const { return reduced_transaction_id_; }
// Returns true if the message confirms to RFC3489 rather than
- // RFC5389. The main difference between two version of the STUN
+ // RFC5389. The main difference between the two versions of the STUN
// protocol is the presence of the magic cookie and different length
- // of transaction ID. For outgoing packets version of the protocol
+ // of transaction ID. For outgoing packets the version of the protocol
// is determined by the lengths of the transaction ID.
bool IsLegacy() const;
@@ -191,19 +201,27 @@
// Remote all attributes and releases them.
void ClearAttributes();
- // Validates that a raw STUN message has a correct MESSAGE-INTEGRITY value.
- // This can't currently be done on a StunMessage, since it is affected by
- // padding data (which we discard when reading a StunMessage).
- static bool ValidateMessageIntegrity(const char* data,
- size_t size,
- const std::string& password);
- static bool ValidateMessageIntegrity32(const char* data,
- size_t size,
- const std::string& password);
+ // Validates that a STUN message has a correct MESSAGE-INTEGRITY value.
+ // This uses the buffered raw-format message stored by Read().
+ IntegrityStatus ValidateMessageIntegrity(const std::string& password);
+
+ // Returns the current integrity status of the message.
+ IntegrityStatus integrity() const { return integrity_; }
+
+ // Shortcut for checking if integrity is verified.
+ bool IntegrityOk() const {
+ return integrity_ == IntegrityStatus::kIntegrityOk;
+ }
+
+ // Returns the password attribute used to set or check the integrity.
+ // Can only be called after adding or checking the integrity.
+ std::string password() const {
+ RTC_DCHECK(integrity_ != IntegrityStatus::kNotSet);
+ return password_;
+ }
// Adds a MESSAGE-INTEGRITY attribute that is valid for the current message.
bool AddMessageIntegrity(const std::string& password);
- bool AddMessageIntegrity(const char* key, size_t keylen);
// Adds a STUN_ATTR_GOOG_MESSAGE_INTEGRITY_32 attribute that is valid for the
// current message.
@@ -244,6 +262,30 @@
bool EqualAttributes(const StunMessage* other,
std::function<bool(int type)> attribute_type_mask) const;
+ // Expose raw-buffer ValidateMessageIntegrity function for testing.
+ static bool ValidateMessageIntegrityForTesting(const char* data,
+ size_t size,
+ const std::string& password) {
+ return ValidateMessageIntegrity(data, size, password);
+ }
+ // Expose raw-buffer ValidateMessageIntegrity function for testing.
+ static bool ValidateMessageIntegrity32ForTesting(
+ const char* data,
+ size_t size,
+ const std::string& password) {
+ return ValidateMessageIntegrity32(data, size, password);
+ }
+ // Validates that a STUN message in byte buffer form
+ // has a correct MESSAGE-INTEGRITY value.
+ // These functions are not recommended and will be deprecated; use
+ // ValidateMessageIntegrity(password) on the parsed form instead.
+ static bool ValidateMessageIntegrity(const char* data,
+ size_t size,
+ const std::string& password);
+ static bool ValidateMessageIntegrity32(const char* data,
+ size_t size,
+ const std::string& password);
+
protected:
// Verifies that the given attribute is allowed for this message.
virtual StunAttributeValueType GetAttributeValueType(int type) const;
@@ -269,6 +311,10 @@
std::string transaction_id_;
uint32_t reduced_transaction_id_;
uint32_t stun_magic_cookie_;
+ // The original buffer for messages created by Read().
+ std::string buffer_;
+ IntegrityStatus integrity_ = IntegrityStatus::kNotSet;
+ std::string password_;
};
// Base class for all STUN/TURN attributes.
diff --git a/api/transport/stun_unittest.cc b/api/transport/stun_unittest.cc
index bf2717e..bf791f2 100644
--- a/api/transport/stun_unittest.cc
+++ b/api/transport/stun_unittest.cc
@@ -1196,24 +1196,24 @@
// Check our STUN message validation code against the RFC5769 test messages.
TEST_F(StunTest, ValidateMessageIntegrity) {
// Try the messages from RFC 5769.
- EXPECT_TRUE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_TRUE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(kRfc5769SampleRequest),
sizeof(kRfc5769SampleRequest), kRfc5769SampleMsgPassword));
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(kRfc5769SampleRequest),
sizeof(kRfc5769SampleRequest), "InvalidPassword"));
- EXPECT_TRUE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_TRUE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(kRfc5769SampleResponse),
sizeof(kRfc5769SampleResponse), kRfc5769SampleMsgPassword));
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(kRfc5769SampleResponse),
sizeof(kRfc5769SampleResponse), "InvalidPassword"));
- EXPECT_TRUE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_TRUE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(kRfc5769SampleResponseIPv6),
sizeof(kRfc5769SampleResponseIPv6), kRfc5769SampleMsgPassword));
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(kRfc5769SampleResponseIPv6),
sizeof(kRfc5769SampleResponseIPv6), "InvalidPassword"));
@@ -1222,40 +1222,40 @@
ComputeStunCredentialHash(kRfc5769SampleMsgWithAuthUsername,
kRfc5769SampleMsgWithAuthRealm,
kRfc5769SampleMsgWithAuthPassword, &key);
- EXPECT_TRUE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_TRUE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(kRfc5769SampleRequestLongTermAuth),
sizeof(kRfc5769SampleRequestLongTermAuth), key));
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(kRfc5769SampleRequestLongTermAuth),
sizeof(kRfc5769SampleRequestLongTermAuth), "InvalidPassword"));
// Try some edge cases.
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(kStunMessageWithZeroLength),
sizeof(kStunMessageWithZeroLength), kRfc5769SampleMsgPassword));
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(kStunMessageWithExcessLength),
sizeof(kStunMessageWithExcessLength), kRfc5769SampleMsgPassword));
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(kStunMessageWithSmallLength),
sizeof(kStunMessageWithSmallLength), kRfc5769SampleMsgPassword));
// Again, but with the lengths matching what is claimed in the headers.
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(kStunMessageWithZeroLength),
kStunHeaderSize + rtc::GetBE16(&kStunMessageWithZeroLength[2]),
kRfc5769SampleMsgPassword));
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(kStunMessageWithExcessLength),
kStunHeaderSize + rtc::GetBE16(&kStunMessageWithExcessLength[2]),
kRfc5769SampleMsgPassword));
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(kStunMessageWithSmallLength),
kStunHeaderSize + rtc::GetBE16(&kStunMessageWithSmallLength[2]),
kRfc5769SampleMsgPassword));
// Check that a too-short HMAC doesn't cause buffer overflow.
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(kStunMessageWithBadHmacAtEnd),
sizeof(kStunMessageWithBadHmacAtEnd), kRfc5769SampleMsgPassword));
@@ -1268,8 +1268,8 @@
if (i > 0)
buf[i - 1] ^= 0x01;
EXPECT_EQ(i >= sizeof(buf) - 8,
- StunMessage::ValidateMessageIntegrity(buf, sizeof(buf),
- kRfc5769SampleMsgPassword));
+ StunMessage::ValidateMessageIntegrityForTesting(
+ buf, sizeof(buf), kRfc5769SampleMsgPassword));
}
}
@@ -1291,7 +1291,7 @@
rtc::ByteBufferWriter buf1;
EXPECT_TRUE(msg.Write(&buf1));
- EXPECT_TRUE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_TRUE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(buf1.Data()), buf1.Length(),
kRfc5769SampleMsgPassword));
@@ -1309,7 +1309,7 @@
rtc::ByteBufferWriter buf3;
EXPECT_TRUE(msg2.Write(&buf3));
- EXPECT_TRUE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_TRUE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(buf3.Data()), buf3.Length(),
kRfc5769SampleMsgPassword));
}
@@ -1317,40 +1317,40 @@
// Check our STUN message validation code against the RFC5769 test messages.
TEST_F(StunTest, ValidateMessageIntegrity32) {
// Try the messages from RFC 5769.
- EXPECT_TRUE(StunMessage::ValidateMessageIntegrity32(
+ EXPECT_TRUE(StunMessage::ValidateMessageIntegrity32ForTesting(
reinterpret_cast<const char*>(kSampleRequestMI32),
sizeof(kSampleRequestMI32), kRfc5769SampleMsgPassword));
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting(
reinterpret_cast<const char*>(kSampleRequestMI32),
sizeof(kSampleRequestMI32), "InvalidPassword"));
// Try some edge cases.
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting(
reinterpret_cast<const char*>(kStunMessageWithZeroLength),
sizeof(kStunMessageWithZeroLength), kRfc5769SampleMsgPassword));
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting(
reinterpret_cast<const char*>(kStunMessageWithExcessLength),
sizeof(kStunMessageWithExcessLength), kRfc5769SampleMsgPassword));
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting(
reinterpret_cast<const char*>(kStunMessageWithSmallLength),
sizeof(kStunMessageWithSmallLength), kRfc5769SampleMsgPassword));
// Again, but with the lengths matching what is claimed in the headers.
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting(
reinterpret_cast<const char*>(kStunMessageWithZeroLength),
kStunHeaderSize + rtc::GetBE16(&kStunMessageWithZeroLength[2]),
kRfc5769SampleMsgPassword));
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting(
reinterpret_cast<const char*>(kStunMessageWithExcessLength),
kStunHeaderSize + rtc::GetBE16(&kStunMessageWithExcessLength[2]),
kRfc5769SampleMsgPassword));
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting(
reinterpret_cast<const char*>(kStunMessageWithSmallLength),
kStunHeaderSize + rtc::GetBE16(&kStunMessageWithSmallLength[2]),
kRfc5769SampleMsgPassword));
// Check that a too-short HMAC doesn't cause buffer overflow.
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting(
reinterpret_cast<const char*>(kStunMessageWithBadHmacAtEnd),
sizeof(kStunMessageWithBadHmacAtEnd), kRfc5769SampleMsgPassword));
@@ -1363,7 +1363,7 @@
if (i > 0)
buf[i - 1] ^= 0x01;
EXPECT_EQ(i >= sizeof(buf) - 8,
- StunMessage::ValidateMessageIntegrity32(
+ StunMessage::ValidateMessageIntegrity32ForTesting(
buf, sizeof(buf), kRfc5769SampleMsgPassword));
}
}
@@ -1384,7 +1384,7 @@
rtc::ByteBufferWriter buf1;
EXPECT_TRUE(msg.Write(&buf1));
- EXPECT_TRUE(StunMessage::ValidateMessageIntegrity32(
+ EXPECT_TRUE(StunMessage::ValidateMessageIntegrity32ForTesting(
reinterpret_cast<const char*>(buf1.Data()), buf1.Length(),
kRfc5769SampleMsgPassword));
@@ -1402,7 +1402,7 @@
rtc::ByteBufferWriter buf3;
EXPECT_TRUE(msg2.Write(&buf3));
- EXPECT_TRUE(StunMessage::ValidateMessageIntegrity32(
+ EXPECT_TRUE(StunMessage::ValidateMessageIntegrity32ForTesting(
reinterpret_cast<const char*>(buf3.Data()), buf3.Length(),
kRfc5769SampleMsgPassword));
}
@@ -1420,14 +1420,14 @@
rtc::ByteBufferWriter buf1;
EXPECT_TRUE(msg.Write(&buf1));
- EXPECT_TRUE(StunMessage::ValidateMessageIntegrity32(
+ EXPECT_TRUE(StunMessage::ValidateMessageIntegrity32ForTesting(
reinterpret_cast<const char*>(buf1.Data()), buf1.Length(), "password1"));
- EXPECT_TRUE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_TRUE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(buf1.Data()), buf1.Length(), "password2"));
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32ForTesting(
reinterpret_cast<const char*>(buf1.Data()), buf1.Length(), "password2"));
- EXPECT_FALSE(StunMessage::ValidateMessageIntegrity(
+ EXPECT_FALSE(StunMessage::ValidateMessageIntegrityForTesting(
reinterpret_cast<const char*>(buf1.Data()), buf1.Length(), "password1"));
}
diff --git a/p2p/base/connection.cc b/p2p/base/connection.cc
index 8adfeb4..0aa2bcb 100644
--- a/p2p/base/connection.cc
+++ b/p2p/base/connection.cc
@@ -480,6 +480,7 @@
// If this is a STUN response, then update the writable bit.
// Log at LS_INFO if we receive a ping on an unwritable connection.
rtc::LoggingSeverity sev = (!writable() ? rtc::LS_INFO : rtc::LS_VERBOSE);
+ msg->ValidateMessageIntegrity(remote_candidate().password());
switch (msg->type()) {
case STUN_BINDING_REQUEST:
RTC_LOG_V(sev) << ToString() << ": Received "
@@ -505,8 +506,7 @@
// id's match.
case STUN_BINDING_RESPONSE:
case STUN_BINDING_ERROR_RESPONSE:
- if (msg->ValidateMessageIntegrity(data, size,
- remote_candidate().password())) {
+ if (msg->IntegrityOk()) {
requests_.CheckResponse(msg.get());
}
// Otherwise silently discard the response message.
@@ -523,8 +523,7 @@
break;
case GOOG_PING_RESPONSE:
case GOOG_PING_ERROR_RESPONSE:
- if (msg->ValidateMessageIntegrity32(data, size,
- remote_candidate().password())) {
+ if (msg->IntegrityOk()) {
requests_.CheckResponse(msg.get());
}
break;
diff --git a/p2p/base/port.cc b/p2p/base/port.cc
index 436da59..79b83b7 100644
--- a/p2p/base/port.cc
+++ b/p2p/base/port.cc
@@ -493,7 +493,8 @@
}
// If ICE, and the MESSAGE-INTEGRITY is bad, fail with a 401 Unauthorized
- if (!stun_msg->ValidateMessageIntegrity(data, size, password_)) {
+ if (stun_msg->ValidateMessageIntegrity(password_) !=
+ StunMessage::IntegrityStatus::kIntegrityOk) {
RTC_LOG(LS_ERROR) << ToString() << ": Received "
<< StunMethodToString(stun_msg->type())
<< " with bad M-I from " << addr.ToSensitiveString()
@@ -559,7 +560,8 @@
// 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) {
- if (!stun_msg->ValidateMessageIntegrity32(data, size, password_)) {
+ if (stun_msg->ValidateMessageIntegrity(password_) !=
+ StunMessage::IntegrityStatus::kIntegrityOk) {
RTC_LOG(LS_ERROR) << ToString() << ": Received "
<< StunMethodToString(stun_msg->type())
<< " with bad M-I from " << addr.ToSensitiveString()
diff --git a/p2p/base/port_unittest.cc b/p2p/base/port_unittest.cc
index bb5bfbd..293a8d1 100644
--- a/p2p/base/port_unittest.cc
+++ b/p2p/base/port_unittest.cc
@@ -1726,9 +1726,8 @@
EXPECT_EQ(kDefaultPrflxPriority, priority_attr->value());
EXPECT_EQ("rfrag:lfrag", username_attr->GetString());
EXPECT_TRUE(msg->GetByteString(STUN_ATTR_MESSAGE_INTEGRITY) != NULL);
- EXPECT_TRUE(StunMessage::ValidateMessageIntegrity(
- lport->last_stun_buf()->data<char>(), lport->last_stun_buf()->size(),
- "rpass"));
+ EXPECT_EQ(StunMessage::IntegrityStatus::kIntegrityOk,
+ msg->ValidateMessageIntegrity("rpass"));
const StunUInt64Attribute* ice_controlling_attr =
msg->GetUInt64(STUN_ATTR_ICE_CONTROLLING);
ASSERT_TRUE(ice_controlling_attr != NULL);
@@ -1767,9 +1766,8 @@
ASSERT_TRUE(addr_attr != NULL);
EXPECT_EQ(lport->Candidates()[0].address(), addr_attr->GetAddress());
EXPECT_TRUE(msg->GetByteString(STUN_ATTR_MESSAGE_INTEGRITY) != NULL);
- EXPECT_TRUE(StunMessage::ValidateMessageIntegrity(
- rport->last_stun_buf()->data<char>(), rport->last_stun_buf()->size(),
- "rpass"));
+ EXPECT_EQ(StunMessage::IntegrityStatus::kIntegrityOk,
+ msg->ValidateMessageIntegrity("rpass"));
EXPECT_TRUE(msg->GetUInt32(STUN_ATTR_FINGERPRINT) != NULL);
EXPECT_TRUE(StunMessage::ValidateFingerprint(
lport->last_stun_buf()->data<char>(), lport->last_stun_buf()->size()));
@@ -1798,9 +1796,8 @@
EXPECT_EQ(STUN_ERROR_SERVER_ERROR, error_attr->code());
EXPECT_EQ(std::string(STUN_ERROR_REASON_SERVER_ERROR), error_attr->reason());
EXPECT_TRUE(msg->GetByteString(STUN_ATTR_MESSAGE_INTEGRITY) != NULL);
- EXPECT_TRUE(StunMessage::ValidateMessageIntegrity(
- rport->last_stun_buf()->data<char>(), rport->last_stun_buf()->size(),
- "rpass"));
+ EXPECT_EQ(StunMessage::IntegrityStatus::kIntegrityOk,
+ msg->ValidateMessageIntegrity("rpass"));
EXPECT_TRUE(msg->GetUInt32(STUN_ATTR_FINGERPRINT) != NULL);
EXPECT_TRUE(StunMessage::ValidateFingerprint(
lport->last_stun_buf()->data<char>(), lport->last_stun_buf()->size()));
diff --git a/p2p/base/stun_request.cc b/p2p/base/stun_request.cc
index 44376ce..2870dcd 100644
--- a/p2p/base/stun_request.cc
+++ b/p2p/base/stun_request.cc
@@ -120,6 +120,18 @@
}
StunRequest* request = iter->second;
+
+ // Now that we know the request, we can see if the response is
+ // integrity-protected or not.
+ // For some tests, the message integrity is not set in the request.
+ // Complain, and then don't check.
+ bool skip_integrity_checking = false;
+ if (request->msg()->integrity() == StunMessage::IntegrityStatus::kNotSet) {
+ skip_integrity_checking = true;
+ } else {
+ msg->ValidateMessageIntegrity(request->msg()->password());
+ }
+
if (!msg->GetNonComprehendedAttributes().empty()) {
// If a response contains unknown comprehension-required attributes, it's
// simply discarded and the transaction is considered failed. See RFC5389
@@ -129,6 +141,9 @@
delete request;
return false;
} else if (msg->type() == GetStunSuccessResponseType(request->type())) {
+ if (!msg->IntegrityOk() && !skip_integrity_checking) {
+ return false;
+ }
request->OnResponse(msg);
} else if (msg->type() == GetStunErrorResponseType(request->type())) {
request->OnErrorResponse(msg);
diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc
index c78a946..2bb4c61 100644
--- a/p2p/base/turn_port.cc
+++ b/p2p/base/turn_port.cc
@@ -724,16 +724,6 @@
return false;
}
- // This must be a response for one of our requests.
- // Check success responses, but not errors, for MESSAGE-INTEGRITY.
- if (IsStunSuccessResponseType(msg_type) &&
- !StunMessage::ValidateMessageIntegrity(data, size, hash())) {
- RTC_LOG(LS_WARNING) << ToString()
- << ": Received TURN message with invalid "
- "message integrity, msg_type: "
- << msg_type;
- return true;
- }
request_manager_.CheckResponse(data, size);
return true;
diff --git a/p2p/base/turn_server.cc b/p2p/base/turn_server.cc
index 25985be..60b3d94 100644
--- a/p2p/base/turn_server.cc
+++ b/p2p/base/turn_server.cc
@@ -306,7 +306,7 @@
}
bool TurnServer::CheckAuthorization(TurnServerConnection* conn,
- const StunMessage* msg,
+ StunMessage* msg,
const char* data,
size_t size,
const std::string& key) {
@@ -322,14 +322,14 @@
const StunByteStringAttribute* nonce_attr =
msg->GetByteString(STUN_ATTR_NONCE);
- // Fail if no M-I.
+ // Fail if no MESSAGE_INTEGRITY.
if (!mi_attr) {
SendErrorResponseWithRealmAndNonce(conn, msg, STUN_ERROR_UNAUTHORIZED,
STUN_ERROR_REASON_UNAUTHORIZED);
return false;
}
- // Fail if there is M-I but no username, nonce, or realm.
+ // Fail if there is MESSAGE_INTEGRITY but no username, nonce, or realm.
if (!username_attr || !realm_attr || !nonce_attr) {
SendErrorResponse(conn, msg, STUN_ERROR_BAD_REQUEST,
STUN_ERROR_REASON_BAD_REQUEST);
@@ -343,9 +343,9 @@
return false;
}
- // Fail if bad username or M-I.
- // We need |data| and |size| for the call to ValidateMessageIntegrity.
- if (key.empty() || !StunMessage::ValidateMessageIntegrity(data, size, key)) {
+ // Fail if bad MESSAGE_INTEGRITY.
+ if (key.empty() || msg->ValidateMessageIntegrity(key) !=
+ StunMessage::IntegrityStatus::kIntegrityOk) {
SendErrorResponseWithRealmAndNonce(conn, msg, STUN_ERROR_UNAUTHORIZED,
STUN_ERROR_REASON_UNAUTHORIZED);
return false;
diff --git a/p2p/base/turn_server.h b/p2p/base/turn_server.h
index 05916ea..c63eeb8 100644
--- a/p2p/base/turn_server.h
+++ b/p2p/base/turn_server.h
@@ -278,7 +278,7 @@
bool GetKey(const StunMessage* msg, std::string* key);
bool CheckAuthorization(TurnServerConnection* conn,
- const StunMessage* msg,
+ StunMessage* msg,
const char* data,
size_t size,
const std::string& key);
diff --git a/test/fuzzers/stun_parser_fuzzer.cc b/test/fuzzers/stun_parser_fuzzer.cc
index 720a699..6ca9eac 100644
--- a/test/fuzzers/stun_parser_fuzzer.cc
+++ b/test/fuzzers/stun_parser_fuzzer.cc
@@ -24,5 +24,6 @@
std::unique_ptr<cricket::IceMessage> stun_msg(new cricket::IceMessage());
rtc::ByteBufferReader buf(message, size);
stun_msg->Read(&buf);
+ stun_msg->ValidateMessageIntegrity("");
}
} // namespace webrtc
diff --git a/test/fuzzers/stun_validator_fuzzer.cc b/test/fuzzers/stun_validator_fuzzer.cc
index 44252fa..421638d 100644
--- a/test/fuzzers/stun_validator_fuzzer.cc
+++ b/test/fuzzers/stun_validator_fuzzer.cc
@@ -18,6 +18,6 @@
const char* message = reinterpret_cast<const char*>(data);
cricket::StunMessage::ValidateFingerprint(message, size);
- cricket::StunMessage::ValidateMessageIntegrity(message, size, "");
+ cricket::StunMessage::ValidateMessageIntegrityForTesting(message, size, "");
}
} // namespace webrtc