From 63737a918bc3fe5ac65c642e07312aa9045e01f1 Mon Sep 17 00:00:00 2001 From: Jonas Oreland Date: Thu, 21 Nov 2019 15:12:14 +0100 Subject: [PATCH] Add new GOOG_PING and GOOG_MESSAGE_INTEGRITY_32 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch adds - Attribute: STUN_ATTR_GOOG_MESSAGE_INTEGRITY_32 which is a ordinary message integrity but truncated to 32-bit - Method: GOOG_PING, which will be used for webrtc:11100 Both the attribute and the method has been registered at iana, https://www.iana.org/assignments/stun-parameters/stun-parameters.xhtml#stun-parameters-4 BUG=webrtc:11100 Change-Id: Iddd5614473fd6f18fbbe76e72d047c617df7123f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/160180 Commit-Queue: Jonas Oreland Reviewed-by: Björn Terelius Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/master@{#29864} --- api/transport/stun.cc | 76 +++++++++++-- api/transport/stun.h | 30 +++++ api/transport/stun_unittest.cc | 199 ++++++++++++++++++++++++++++++++- 3 files changed, 289 insertions(+), 16 deletions(-) diff --git a/api/transport/stun.cc b/api/transport/stun.cc index 7fa3c90bf1..80b7b82d9a 100644 --- a/api/transport/stun.cc +++ b/api/transport/stun.cc @@ -132,6 +132,14 @@ std::unique_ptr StunMessage::RemoveAttribute(int type) { return attribute; } +void StunMessage::ClearAttributes() { + for (auto it = attrs_.rbegin(); it != attrs_.rend(); ++it) { + (*it)->SetOwner(nullptr); + } + attrs_.clear(); + length_ = 0; +} + const StunAddressAttribute* StunMessage::GetAddress(int type) const { switch (type) { case STUN_ATTR_MAPPED_ADDRESS: { @@ -180,11 +188,31 @@ const StunUInt16ListAttribute* StunMessage::GetUnknownAttributes() const { GetAttribute(STUN_ATTR_UNKNOWN_ATTRIBUTES)); } -// Verifies a STUN message has a valid MESSAGE-INTEGRITY attribute, using the -// procedure outlined in RFC 5389, section 15.4. bool StunMessage::ValidateMessageIntegrity(const char* data, size_t size, const std::string& password) { + return ValidateMessageIntegrityOfType(STUN_ATTR_MESSAGE_INTEGRITY, + kStunMessageIntegritySize, data, size, + password); +} + +bool StunMessage::ValidateMessageIntegrity32(const char* data, + size_t size, + const std::string& password) { + return ValidateMessageIntegrityOfType(STUN_ATTR_GOOG_MESSAGE_INTEGRITY_32, + kStunMessageIntegrity32Size, data, size, + password); +} + +// Verifies a STUN message has a valid MESSAGE-INTEGRITY attribute, using the +// procedure outlined in RFC 5389, section 15.4. +bool StunMessage::ValidateMessageIntegrityOfType(int mi_attr_type, + size_t mi_attr_size, + const char* data, + size_t size, + const std::string& password) { + RTC_DCHECK(mi_attr_size <= kStunMessageIntegritySize); + // Verifying the size of the message. if ((size % 4) != 0 || size < kStunHeaderSize) { return false; @@ -206,8 +234,8 @@ bool StunMessage::ValidateMessageIntegrity(const char* data, attr_length = rtc::GetBE16(&data[current_pos + sizeof(attr_type)]); // If M-I, sanity check it, and break out. - if (attr_type == STUN_ATTR_MESSAGE_INTEGRITY) { - if (attr_length != kStunMessageIntegritySize || + if (attr_type == mi_attr_type) { + if (attr_length != mi_attr_size || current_pos + sizeof(attr_type) + sizeof(attr_length) + attr_length > size) { return false; @@ -231,11 +259,11 @@ bool StunMessage::ValidateMessageIntegrity(const char* data, size_t mi_pos = current_pos; std::unique_ptr temp_data(new char[current_pos]); memcpy(temp_data.get(), data, current_pos); - if (size > mi_pos + kStunAttributeHeaderSize + kStunMessageIntegritySize) { + if (size > mi_pos + kStunAttributeHeaderSize + mi_attr_size) { // Stun message has other attributes after message integrity. // Adjust the length parameter in stun message to calculate HMAC. size_t extra_offset = - size - (mi_pos + kStunAttributeHeaderSize + kStunMessageIntegritySize); + size - (mi_pos + kStunAttributeHeaderSize + mi_attr_size); size_t new_adjusted_len = size - extra_offset - kStunHeaderSize; // Writing new length of the STUN message @ Message Length in temp buffer. @@ -252,23 +280,41 @@ bool StunMessage::ValidateMessageIntegrity(const char* data, rtc::ComputeHmac(rtc::DIGEST_SHA_1, password.c_str(), password.size(), temp_data.get(), mi_pos, hmac, sizeof(hmac)); RTC_DCHECK(ret == sizeof(hmac)); - if (ret != sizeof(hmac)) + if (ret != sizeof(hmac)) { return false; + } // Comparing the calculated HMAC with the one present in the message. return memcmp(data + current_pos + kStunAttributeHeaderSize, hmac, - sizeof(hmac)) == 0; + mi_attr_size) == 0; } bool StunMessage::AddMessageIntegrity(const std::string& password) { - return AddMessageIntegrity(password.c_str(), password.size()); + return AddMessageIntegrityOfType(STUN_ATTR_MESSAGE_INTEGRITY, + kStunMessageIntegritySize, password.c_str(), + 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(), + password.length()); +} + +bool StunMessage::AddMessageIntegrityOfType(int attr_type, + size_t attr_size, + const char* key, + size_t keylen) { // Add the attribute with a dummy value. Since this is a known attribute, it // can't fail. + RTC_DCHECK(attr_size <= kStunMessageIntegritySize); auto msg_integrity_attr_ptr = std::make_unique( - STUN_ATTR_MESSAGE_INTEGRITY, std::string(kStunMessageIntegritySize, '0')); + attr_type, std::string(attr_size, '0')); auto* msg_integrity_attr = msg_integrity_attr_ptr.get(); AddAttribute(std::move(msg_integrity_attr_ptr)); @@ -290,7 +336,7 @@ bool StunMessage::AddMessageIntegrity(const char* key, size_t keylen) { } // Insert correct HMAC into the attribute. - msg_integrity_attr->CopyBytes(hmac, sizeof(hmac)); + msg_integrity_attr->CopyBytes(hmac, attr_size); return true; } @@ -973,6 +1019,14 @@ void StunUInt16ListAttribute::AddType(uint16_t value) { SetLength(static_cast(attr_types_->size() * 2)); } +void StunUInt16ListAttribute::AddTypeAtIndex(uint16_t index, uint16_t value) { + if (attr_types_->size() < static_cast(index + 1)) { + attr_types_->resize(index + 1); + } + (*attr_types_)[index] = value; + SetLength(static_cast(attr_types_->size() * 2)); +} + bool StunUInt16ListAttribute::Read(ByteBufferReader* buf) { if (length() % 2) { return false; diff --git a/api/transport/stun.h b/api/transport/stun.h index 02b352c55c..857a381078 100644 --- a/api/transport/stun.h +++ b/api/transport/stun.h @@ -33,6 +33,11 @@ enum StunMessageType { STUN_BINDING_INDICATION = 0x0011, STUN_BINDING_RESPONSE = 0x0101, STUN_BINDING_ERROR_RESPONSE = 0x0111, + + // Method 0x80 + GOOG_PING_REQUEST = 0x200, + GOOG_PING_RESPONSE = 0x300, + GOOG_PING_ERROR_RESPONSE = 0x310, }; // These are all known STUN attributes, defined in RFC 5389 and elsewhere. @@ -119,6 +124,8 @@ const size_t kStunLegacyTransactionIdLength = 16; // STUN Message Integrity HMAC length. const size_t kStunMessageIntegritySize = 20; +// Size of STUN_ATTR_MESSAGE_INTEGRITY_32 +const size_t kStunMessageIntegrity32Size = 4; class StunAddressAttribute; class StunAttribute; @@ -174,16 +181,27 @@ class StunMessage { // Remove the last occurrence of an attribute. std::unique_ptr RemoveAttribute(int type); + // 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); + // 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. + bool AddMessageIntegrity32(absl::string_view password); + // Verifies that a given buffer is STUN by checking for a correct FINGERPRINT. static bool ValidateFingerprint(const char* data, size_t size); @@ -213,6 +231,15 @@ class StunMessage { StunAttribute* CreateAttribute(int type, size_t length) /* const*/; const StunAttribute* GetAttribute(int type) const; static bool IsValidTransactionId(const std::string& transaction_id); + bool AddMessageIntegrityOfType(int mi_attr_type, + size_t mi_attr_size, + const char* key, + size_t keylen); + static bool ValidateMessageIntegrityOfType(int mi_attr_type, + size_t mi_attr_size, + const char* data, + size_t size, + const std::string& password); uint16_t type_; uint16_t length_; @@ -462,6 +489,7 @@ class StunUInt16ListAttribute : public StunAttribute { uint16_t GetType(int index) const; void SetType(int index, uint16_t value); void AddType(uint16_t value); + void AddTypeAtIndex(uint16_t index, uint16_t value); bool Read(rtc::ByteBufferReader* buf) override; bool Write(rtc::ByteBufferWriter* buf) const override; @@ -620,6 +648,8 @@ enum IceAttributeType { STUN_ATTR_LAST_ICE_CHECK_RECEIVED = 0xC058, // Uint16List. Miscellaneous attributes for future extension. STUN_ATTR_GOOG_MISC_INFO = 0xC059, + // MESSAGE-INTEGRITY truncated to 32-bit. + STUN_ATTR_GOOG_MESSAGE_INTEGRITY_32 = 0xC060, }; // When adding new attributes to STUN_ATTR_GOOG_MISC_INFO diff --git a/api/transport/stun_unittest.cc b/api/transport/stun_unittest.cc index 4baca59057..c75fb90500 100644 --- a/api/transport/stun_unittest.cc +++ b/api/transport/stun_unittest.cc @@ -332,6 +332,33 @@ static const unsigned char kRfc5769SampleRequest[] = { 0xe5, 0x7a, 0x3b, 0xcf // CRC32 fingerprint }; +// 2.1. Sample Request +static const unsigned char kSampleRequestMI32[] = { + 0x00, 0x01, 0x00, 0x48, // Request type and message length + 0x21, 0x12, 0xa4, 0x42, // Magic cookie + 0xb7, 0xe7, 0xa7, 0x01, // } + 0xbc, 0x34, 0xd6, 0x86, // } Transaction ID + 0xfa, 0x87, 0xdf, 0xae, // } + 0x80, 0x22, 0x00, 0x10, // SOFTWARE attribute header + 0x53, 0x54, 0x55, 0x4e, // } + 0x20, 0x74, 0x65, 0x73, // } User-agent... + 0x74, 0x20, 0x63, 0x6c, // } ...name + 0x69, 0x65, 0x6e, 0x74, // } + 0x00, 0x24, 0x00, 0x04, // PRIORITY attribute header + 0x6e, 0x00, 0x01, 0xff, // ICE priority value + 0x80, 0x29, 0x00, 0x08, // ICE-CONTROLLED attribute header + 0x93, 0x2f, 0xf9, 0xb1, // } Pseudo-random tie breaker... + 0x51, 0x26, 0x3b, 0x36, // } ...for ICE control + 0x00, 0x06, 0x00, 0x09, // USERNAME attribute header + 0x65, 0x76, 0x74, 0x6a, // } + 0x3a, 0x68, 0x36, 0x76, // } Username (9 bytes) and padding (3 bytes) + 0x59, 0x20, 0x20, 0x20, // } + 0xC0, 0x60, 0x00, 0x04, // MESSAGE-INTEGRITY-32 attribute header + 0x45, 0x45, 0xce, 0x7c, // } HMAC-SHA1 fingerprint (first 32 bit) + 0x80, 0x28, 0x00, 0x04, // FINGERPRINT attribute header + 0xe5, 0x7a, 0x3b, 0xcf // CRC32 fingerprint +}; + // 2.2. Sample IPv4 Response static const unsigned char kRfc5769SampleResponse[] = { 0x01, 0x01, 0x00, 0x3c, // Response type and message length @@ -451,6 +478,14 @@ static const unsigned char kCalculatedHmac1[] = { 0x74, 0x2a, 0xf9, 0xe3 // } }; +// This truncated HMAC differs from kCalculatedHmac1 +// above since the sum is computed including header +// and the header is different since the message is shorter +// than when MESSAGE-INTEGRITY is used. +static const unsigned char kCalculatedHmac1_32[] = { + 0xda, 0x39, 0xde, 0x5d, // } +}; + // Length parameter is changed to 0x1c from 0x3c. // AddMessageIntegrity will add MI information and update the length param // accordingly. @@ -479,6 +514,14 @@ static const unsigned char kCalculatedHmac2[] = { 0x43, 0x14, 0x10, 0x28 // } }; +// This truncated HMAC differs from kCalculatedHmac2 +// above since the sum is computed including header +// and the header is different since the message is shorter +// than when MESSAGE-INTEGRITY is used. +static const unsigned char kCalculatedHmac2_32[] = { + 0xe7, 0x5c, 0xd3, 0x16, // } +}; + // clang-format on // A transaction ID without the 'magic cookie' portion @@ -1271,6 +1314,123 @@ TEST_F(StunTest, AddMessageIntegrity) { kRfc5769SampleMsgPassword)); } +// 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( + reinterpret_cast(kSampleRequestMI32), + sizeof(kSampleRequestMI32), kRfc5769SampleMsgPassword)); + EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32( + reinterpret_cast(kSampleRequestMI32), + sizeof(kSampleRequestMI32), "InvalidPassword")); + + // Try some edge cases. + EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32( + reinterpret_cast(kStunMessageWithZeroLength), + sizeof(kStunMessageWithZeroLength), kRfc5769SampleMsgPassword)); + EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32( + reinterpret_cast(kStunMessageWithExcessLength), + sizeof(kStunMessageWithExcessLength), kRfc5769SampleMsgPassword)); + EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32( + reinterpret_cast(kStunMessageWithSmallLength), + sizeof(kStunMessageWithSmallLength), kRfc5769SampleMsgPassword)); + + // Again, but with the lengths matching what is claimed in the headers. + EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32( + reinterpret_cast(kStunMessageWithZeroLength), + kStunHeaderSize + rtc::GetBE16(&kStunMessageWithZeroLength[2]), + kRfc5769SampleMsgPassword)); + EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32( + reinterpret_cast(kStunMessageWithExcessLength), + kStunHeaderSize + rtc::GetBE16(&kStunMessageWithExcessLength[2]), + kRfc5769SampleMsgPassword)); + EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32( + reinterpret_cast(kStunMessageWithSmallLength), + kStunHeaderSize + rtc::GetBE16(&kStunMessageWithSmallLength[2]), + kRfc5769SampleMsgPassword)); + + // Check that a too-short HMAC doesn't cause buffer overflow. + EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32( + reinterpret_cast(kStunMessageWithBadHmacAtEnd), + sizeof(kStunMessageWithBadHmacAtEnd), kRfc5769SampleMsgPassword)); + + // Test that munging a single bit anywhere in the message causes the + // message-integrity check to fail, unless it is after the M-I attribute. + char buf[sizeof(kSampleRequestMI32)]; + memcpy(buf, kSampleRequestMI32, sizeof(kSampleRequestMI32)); + for (size_t i = 0; i < sizeof(buf); ++i) { + buf[i] ^= 0x01; + if (i > 0) + buf[i - 1] ^= 0x01; + EXPECT_EQ(i >= sizeof(buf) - 8, + StunMessage::ValidateMessageIntegrity32( + buf, sizeof(buf), kRfc5769SampleMsgPassword)); + } +} + +// Validate that we generate correct MESSAGE-INTEGRITY-32 attributes. +TEST_F(StunTest, AddMessageIntegrity32) { + IceMessage msg; + rtc::ByteBufferReader buf( + reinterpret_cast(kRfc5769SampleRequestWithoutMI), + sizeof(kRfc5769SampleRequestWithoutMI)); + EXPECT_TRUE(msg.Read(&buf)); + EXPECT_TRUE(msg.AddMessageIntegrity32(kRfc5769SampleMsgPassword)); + const StunByteStringAttribute* mi_attr = + msg.GetByteString(STUN_ATTR_GOOG_MESSAGE_INTEGRITY_32); + EXPECT_EQ(4U, mi_attr->length()); + EXPECT_EQ(0, memcmp(mi_attr->bytes(), kCalculatedHmac1_32, + sizeof(kCalculatedHmac1_32))); + + rtc::ByteBufferWriter buf1; + EXPECT_TRUE(msg.Write(&buf1)); + EXPECT_TRUE(StunMessage::ValidateMessageIntegrity32( + reinterpret_cast(buf1.Data()), buf1.Length(), + kRfc5769SampleMsgPassword)); + + IceMessage msg2; + rtc::ByteBufferReader buf2( + reinterpret_cast(kRfc5769SampleResponseWithoutMI), + sizeof(kRfc5769SampleResponseWithoutMI)); + EXPECT_TRUE(msg2.Read(&buf2)); + EXPECT_TRUE(msg2.AddMessageIntegrity32(kRfc5769SampleMsgPassword)); + const StunByteStringAttribute* mi_attr2 = + msg2.GetByteString(STUN_ATTR_GOOG_MESSAGE_INTEGRITY_32); + EXPECT_EQ(4U, mi_attr2->length()); + EXPECT_EQ(0, memcmp(mi_attr2->bytes(), kCalculatedHmac2_32, + sizeof(kCalculatedHmac2_32))); + + rtc::ByteBufferWriter buf3; + EXPECT_TRUE(msg2.Write(&buf3)); + EXPECT_TRUE(StunMessage::ValidateMessageIntegrity32( + reinterpret_cast(buf3.Data()), buf3.Length(), + kRfc5769SampleMsgPassword)); +} + +// Validate that the message validates if both MESSAGE-INTEGRITY-32 and +// MESSAGE-INTEGRITY are present in the message. +// This is not expected to be used, but is not forbidden. +TEST_F(StunTest, AddMessageIntegrity32AndMessageIntegrity) { + IceMessage msg; + auto attr = StunAttribute::CreateByteString(STUN_ATTR_USERNAME); + attr->CopyBytes("keso", sizeof("keso")); + msg.AddAttribute(std::move(attr)); + msg.AddMessageIntegrity32("password1"); + msg.AddMessageIntegrity("password2"); + + rtc::ByteBufferWriter buf1; + EXPECT_TRUE(msg.Write(&buf1)); + EXPECT_TRUE(StunMessage::ValidateMessageIntegrity32( + reinterpret_cast(buf1.Data()), buf1.Length(), "password1")); + EXPECT_TRUE(StunMessage::ValidateMessageIntegrity( + reinterpret_cast(buf1.Data()), buf1.Length(), "password2")); + + EXPECT_FALSE(StunMessage::ValidateMessageIntegrity32( + reinterpret_cast(buf1.Data()), buf1.Length(), "password2")); + EXPECT_FALSE(StunMessage::ValidateMessageIntegrity( + reinterpret_cast(buf1.Data()), buf1.Length(), "password1")); +} + // Check our STUN message validation code against the RFC5769 test messages. TEST_F(StunTest, ValidateFingerprint) { EXPECT_TRUE(StunMessage::ValidateFingerprint( @@ -1531,6 +1691,20 @@ TEST_F(StunTest, RemoveAttribute) { EXPECT_EQ(msg.RemoveAttribute(STUN_ATTR_USERNAME), nullptr); } +// Test that we can remove attribute from a message. +TEST_F(StunTest, ClearAttributes) { + StunMessage msg; + + auto attr = StunAttribute::CreateByteString(STUN_ATTR_USERNAME); + attr->CopyBytes("kes", sizeof("kes")); + msg.AddAttribute(std::move(attr)); + size_t len = msg.length(); + + msg.ClearAttributes(); + EXPECT_EQ(msg.length(), len - /* 3 + 1 byte padding + header */ 8); + EXPECT_EQ(nullptr, msg.GetByteString(STUN_ATTR_USERNAME)); +} + // Test CopyStunAttribute TEST_F(StunTest, CopyAttribute) { rtc::ByteBufferWriter buf; @@ -1560,6 +1734,20 @@ TEST_F(StunTest, CopyAttribute) { CheckStunAddressAttribute(static_cast(copy.get()), STUN_ADDRESS_IPV6, kTestMessagePort2, test_ip); } + + { // Test StunAddressAttribute. + rtc::IPAddress test_ip(kIPv6TestAddress2); + auto addr = StunAttribute::CreateAddress(STUN_ATTR_XOR_MAPPED_ADDRESS); + rtc::SocketAddress test_addr(test_ip, kTestMessagePort2); + addr->SetAddress(test_addr); + CheckStunAddressAttribute(addr.get(), STUN_ADDRESS_IPV6, + kTestMessagePort2, test_ip); + + auto copy = CopyStunAttribute(*addr.get(), buffer_ptr); + ASSERT_EQ(copy->value_type(), STUN_VALUE_ADDRESS); + CheckStunAddressAttribute(static_cast(copy.get()), + STUN_ADDRESS_IPV6, kTestMessagePort2, test_ip); + } } } @@ -1581,9 +1769,9 @@ TEST_F(StunTest, GoogMiscInfo) { msg.SetTransactionID("ABCDEFGH"); auto list = StunAttribute::CreateUInt16ListAttribute(STUN_ATTR_GOOG_MISC_INFO); - list->AddType(0x1U); - list->AddType(0x1000U); - list->AddType(0xAB0CU); + list->AddTypeAtIndex(0, 0x1U); + list->AddTypeAtIndex(3, 0x1000U); + list->AddTypeAtIndex(2, 0xAB0CU); msg.AddAttribute(std::move(list)); CheckStunHeader(msg, STUN_BINDING_REQUEST, (size - 20)); @@ -1598,9 +1786,10 @@ TEST_F(StunTest, GoogMiscInfo) { const StunUInt16ListAttribute* types = msg.GetUInt16List(STUN_ATTR_GOOG_MISC_INFO); ASSERT_TRUE(types != NULL); - EXPECT_EQ(3U, types->Size()); + EXPECT_EQ(4U, types->Size()); EXPECT_EQ(0x1U, types->GetType(0)); - EXPECT_EQ(0x1000U, types->GetType(1)); + EXPECT_EQ(0x0U, types->GetType(1)); + EXPECT_EQ(0x1000U, types->GetType(3)); EXPECT_EQ(0xAB0CU, types->GetType(2)); }