From 1721de12bd248a919fa691c9ed18003ca669e3ab Mon Sep 17 00:00:00 2001 From: Jonas Oreland Date: Wed, 20 Nov 2019 12:10:39 +0100 Subject: [PATCH] Add STUN_ATTR_GOOG_MISC_INFO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch adds the new STUN attribute that has been registered at iana, https://www.iana.org/assignments/stun-parameters/stun-parameters.xhtml#stun-parameters-4 This is part of the effort to land https://webrtc-review.googlesource.com/c/src/+/85520. I have merged that patch with upstream, and is now doing privacy review of it. This attribute is hence not yet used. BUG=webrtc:9446 Change-Id: Iaf177b0c28a6aa830a9422260b67436bb05ac756 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/160043 Reviewed-by: Niels Moller Reviewed-by: Björn Terelius Commit-Queue: Jonas Oreland Cr-Commit-Position: refs/heads/master@{#29843} --- api/transport/stun.cc | 35 ++++++++++++++++++++++++++-------- api/transport/stun.h | 13 +++++++++++++ api/transport/stun_unittest.cc | 33 ++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 8 deletions(-) diff --git a/api/transport/stun.cc b/api/transport/stun.cc index 5e00b33c22..7fa3c90bf1 100644 --- a/api/transport/stun.cc +++ b/api/transport/stun.cc @@ -161,6 +161,10 @@ const StunByteStringAttribute* StunMessage::GetByteString(int type) const { return static_cast(GetAttribute(type)); } +const StunUInt16ListAttribute* StunMessage::GetUInt16List(int type) const { + return static_cast(GetAttribute(type)); +} + const StunErrorCodeAttribute* StunMessage::GetErrorCode() const { return static_cast( GetAttribute(STUN_ATTR_ERROR_CODE)); @@ -343,8 +347,9 @@ bool StunMessage::AddFingerprint() { } bool StunMessage::Read(ByteBufferReader* buf) { - if (!buf->ReadUInt16(&type_)) + if (!buf->ReadUInt16(&type_)) { return false; + } if (type_ & 0x8000) { // RTP and RTCP set the MSB of first byte, since first two bits are version, @@ -352,16 +357,19 @@ bool StunMessage::Read(ByteBufferReader* buf) { return false; } - if (!buf->ReadUInt16(&length_)) + if (!buf->ReadUInt16(&length_)) { return false; + } std::string magic_cookie; - if (!buf->ReadString(&magic_cookie, kStunMagicCookieLength)) + if (!buf->ReadString(&magic_cookie, kStunMagicCookieLength)) { return false; + } std::string transaction_id; - if (!buf->ReadString(&transaction_id, kStunTransactionIdLength)) + if (!buf->ReadString(&transaction_id, kStunTransactionIdLength)) { return false; + } uint32_t magic_cookie_int; static_assert(sizeof(magic_cookie_int) == kStunMagicCookieLength, @@ -376,8 +384,9 @@ bool StunMessage::Read(ByteBufferReader* buf) { transaction_id_ = transaction_id; reduced_transaction_id_ = ReduceTransactionId(transaction_id_); - if (length_ != buf->Length()) + if (length_ != buf->Length()) { return false; + } attrs_.resize(0); @@ -396,11 +405,13 @@ bool StunMessage::Read(ByteBufferReader* buf) { if ((attr_length % 4) != 0) { attr_length += (4 - (attr_length % 4)); } - if (!buf->Consume(attr_length)) + if (!buf->Consume(attr_length)) { return false; + } } else { - if (!attr->Read(buf)) + if (!attr->Read(buf)) { return false; + } attrs_.push_back(std::move(attr)); } } @@ -465,6 +476,8 @@ StunAttributeValueType StunMessage::GetAttributeValueType(int type) const { return STUN_VALUE_UINT32; case STUN_ATTR_LAST_ICE_CHECK_RECEIVED: return STUN_VALUE_BYTE_STRING; + case STUN_ATTR_GOOG_MISC_INFO: + return STUN_VALUE_UINT16_LIST; default: return STUN_VALUE_UNKNOWN; } @@ -572,6 +585,11 @@ std::unique_ptr StunAttribute::CreateErrorCode() { STUN_ATTR_ERROR_CODE, StunErrorCodeAttribute::MIN_SIZE); } +std::unique_ptr +StunAttribute::CreateUInt16ListAttribute(uint16_t type) { + return std::make_unique(type, 0); +} + std::unique_ptr StunAttribute::CreateUnknownAttributes() { return std::make_unique(STUN_ATTR_UNKNOWN_ATTRIBUTES, @@ -956,8 +974,9 @@ void StunUInt16ListAttribute::AddType(uint16_t value) { } bool StunUInt16ListAttribute::Read(ByteBufferReader* buf) { - if (length() % 2) + if (length() % 2) { return false; + } for (size_t i = 0; i < length() / 2; i++) { uint16_t attr; diff --git a/api/transport/stun.h b/api/transport/stun.h index e19f196d8c..02b352c55c 100644 --- a/api/transport/stun.h +++ b/api/transport/stun.h @@ -159,6 +159,7 @@ class StunMessage { const StunUInt32Attribute* GetUInt32(int type) const; const StunUInt64Attribute* GetUInt64(int type) const; const StunByteStringAttribute* GetByteString(int type) const; + const StunUInt16ListAttribute* GetUInt16List(int type) const; // Gets these specific attribute values. const StunErrorCodeAttribute* GetErrorCode() const; @@ -257,6 +258,8 @@ class StunAttribute { static std::unique_ptr CreateUInt64(uint16_t type); static std::unique_ptr CreateByteString( uint16_t type); + static std::unique_ptr CreateUInt16ListAttribute( + uint16_t type); static std::unique_ptr CreateErrorCode(); static std::unique_ptr CreateUnknownAttributes(); @@ -615,8 +618,18 @@ enum IceAttributeType { STUN_ATTR_NETWORK_INFO = 0xC057, // Experimental: Transaction ID of the last connectivity check received. STUN_ATTR_LAST_ICE_CHECK_RECEIVED = 0xC058, + // Uint16List. Miscellaneous attributes for future extension. + STUN_ATTR_GOOG_MISC_INFO = 0xC059, }; +// When adding new attributes to STUN_ATTR_GOOG_MISC_INFO +// (which is a list of uint16_t), append the indices of these attributes below +// and do NOT change the exisiting indices. The indices of attributes must be +// 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 IceGoogMiscInfoAttributeIndex {}; + // RFC 5245-defined errors. enum IceErrorCode { STUN_ERROR_ROLE_CONFLICT = 487, diff --git a/api/transport/stun_unittest.cc b/api/transport/stun_unittest.cc index 667746efd1..4baca59057 100644 --- a/api/transport/stun_unittest.cc +++ b/api/transport/stun_unittest.cc @@ -1571,4 +1571,37 @@ TEST_F(StunTest, ReduceTransactionIdIsHostOrderIndependent) { EXPECT_EQ(reduced_transaction_id, 1835954016u); } +TEST_F(StunTest, GoogMiscInfo) { + StunMessage msg; + const size_t size = + /* msg header */ 20 + + /* attr header */ 4 + + /* 3 * 2 rounded to multiple of 4 */ 8; + msg.SetType(STUN_BINDING_REQUEST); + msg.SetTransactionID("ABCDEFGH"); + auto list = + StunAttribute::CreateUInt16ListAttribute(STUN_ATTR_GOOG_MISC_INFO); + list->AddType(0x1U); + list->AddType(0x1000U); + list->AddType(0xAB0CU); + msg.AddAttribute(std::move(list)); + CheckStunHeader(msg, STUN_BINDING_REQUEST, (size - 20)); + + rtc::ByteBufferWriter out; + EXPECT_TRUE(msg.Write(&out)); + ASSERT_EQ(size, out.Length()); + + size_t read_size = ReadStunMessageTestCase( + &msg, reinterpret_cast(out.Data()), out.Length()); + ASSERT_EQ(read_size + 20, size); + CheckStunHeader(msg, STUN_BINDING_REQUEST, read_size); + const StunUInt16ListAttribute* types = + msg.GetUInt16List(STUN_ATTR_GOOG_MISC_INFO); + ASSERT_TRUE(types != NULL); + EXPECT_EQ(3U, types->Size()); + EXPECT_EQ(0x1U, types->GetType(0)); + EXPECT_EQ(0x1000U, types->GetType(1)); + EXPECT_EQ(0xAB0CU, types->GetType(2)); +} + } // namespace cricket