From b102e4172cc8f21d3d1cc8974f28306eb9d7f37e Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 22 Nov 2022 12:52:21 -0800 Subject: [PATCH] [tlv] add private helper methods to parse and find TLVs in a message (#8421) This commit adds new private type `ParsedInfo` in `Tlv` class which contains parsed TLV info such as TLV type, length, size, value offset. This class provides `ParseFrom()` and `FindIn()` methods to parse or find a TLV from a message. This commit also adds a unit test `test_tlv` to validate the TLV methods. --- src/core/common/tlvs.cpp | 220 +++++++++++++++++--------------------- src/core/common/tlvs.hpp | 14 ++- tests/unit/CMakeLists.txt | 21 ++++ tests/unit/Makefile.am | 5 + tests/unit/test_tlv.cpp | 194 +++++++++++++++++++++++++++++++++ 5 files changed, 331 insertions(+), 123 deletions(-) create mode 100644 tests/unit/test_tlv.cpp diff --git a/src/core/common/tlvs.cpp b/src/core/common/tlvs.cpp index 0ce2eb37e..8fb6487c1 100644 --- a/src/core/common/tlvs.cpp +++ b/src/core/common/tlvs.cpp @@ -61,18 +61,17 @@ Error Tlv::AppendTo(Message &aMessage) const Error Tlv::FindTlv(const Message &aMessage, uint8_t aType, uint16_t aMaxSize, Tlv &aTlv) { - Error error; - uint16_t offset; - uint16_t size; + Error error; + ParsedInfo info; - SuccessOrExit(error = Find(aMessage, aType, &offset, &size, nullptr)); + SuccessOrExit(error = info.FindIn(aMessage, aType)); - if (aMaxSize > size) + if (aMaxSize > info.mSize) { - aMaxSize = size; + aMaxSize = info.mSize; } - aMessage.ReadBytes(offset, &aTlv, aMaxSize); + aMessage.ReadBytes(info.mOffset, &aTlv, aMaxSize); exit: return error; @@ -80,96 +79,106 @@ exit: Error Tlv::FindTlvOffset(const Message &aMessage, uint8_t aType, uint16_t &aOffset) { - return Find(aMessage, aType, &aOffset, nullptr, nullptr); -} + Error error; + ParsedInfo info; -Error Tlv::FindTlvValueOffset(const Message &aMessage, uint8_t aType, uint16_t &aValueOffset, uint16_t &aLength) -{ - Error error; - uint16_t offset; - uint16_t size; - bool isExtendedTlv; - - SuccessOrExit(error = Find(aMessage, aType, &offset, &size, &isExtendedTlv)); - - if (!isExtendedTlv) - { - aValueOffset = offset + sizeof(Tlv); - aLength = size - sizeof(Tlv); - } - else - { - aValueOffset = offset + sizeof(ExtendedTlv); - aLength = size - sizeof(ExtendedTlv); - } + SuccessOrExit(error = info.FindIn(aMessage, aType)); + aOffset = info.mOffset; exit: return error; } -Error Tlv::Find(const Message &aMessage, uint8_t aType, uint16_t *aOffset, uint16_t *aSize, bool *aIsExtendedTlv) +Error Tlv::FindTlvValueOffset(const Message &aMessage, uint8_t aType, uint16_t &aValueOffset, uint16_t &aLength) { - // This static method searches within a `aMessage` for a TLV type - // `aType` and outputs the TLV offset, size, and whether or not it - // is an Extended TLV. - // - // A `nullptr` pointer can be used for output parameters `aOffset`, - // `aSize`, or `aIsExtendedTlv` if the parameter is not required. - // - // Returns `kErrorNone` when found, otherwise `kErrorNotFound`. + Error error; + ParsedInfo info; - Error error = kErrorNotFound; - uint16_t offset = aMessage.GetOffset(); - uint16_t remainingLen = aMessage.GetLength(); - Tlv tlv; - uint32_t size; + SuccessOrExit(error = info.FindIn(aMessage, aType)); - VerifyOrExit(offset <= remainingLen); - remainingLen -= offset; + aValueOffset = info.mValueOffset; + aLength = info.mLength; + +exit: + return error; +} + +Error Tlv::ParsedInfo::ParseFrom(const Message &aMessage, uint16_t aOffset) +{ + // This method reads and parses the TLV info from `aMessage` at + // `aOffset`. This can be used independent of whether the TLV is + // extended or not. It validates that the entire TLV can be read + // from `aMessage`. Returns `kErrorNone` when successfully parsed, + // otherwise `kErrorParse`. + + Error error; + Tlv tlv; + ExtendedTlv extTlv; + uint16_t headerSize; + + SuccessOrExit(error = aMessage.Read(aOffset, tlv)); + + if (!tlv.IsExtended()) + { + mType = tlv.GetType(); + mLength = tlv.GetLength(); + headerSize = sizeof(Tlv); + } + else + { + SuccessOrExit(error = aMessage.Read(aOffset, extTlv)); + + mType = extTlv.GetType(); + mLength = extTlv.GetLength(); + headerSize = sizeof(ExtendedTlv); + } + + // We know that we could successfully read `tlv` or `extTlv` + // (`headerSize` bytes) from the message, so the calculation of the + // remaining length as `aMessage.GetLength() - aOffset - headerSize` + // cannot underflow. + + VerifyOrExit(mLength <= aMessage.GetLength() - aOffset - headerSize, error = kErrorParse); + + // Now that we know the entire TLV is contained within the + // `aMessage`, we can safely calculate `mValueOffset` and `mSize` + // as `uint16_t` and know that there will be no overflow. + + mType = tlv.GetType(); + mOffset = aOffset; + mValueOffset = aOffset + headerSize; + mSize = mLength + headerSize; + +exit: + return error; +} + +Error Tlv::ParsedInfo::FindIn(const Message &aMessage, uint8_t aType) +{ + // This method searches within `aMessage` starting from + // `aMessage.GetOffset()` for a TLV type `aType` and parsed its + // info and validates that the entire TLV can be read from + // `aMessage`. Returns `kErrorNone` when found, otherwise + // `kErrorNotFound`. + + Error error = kErrorNotFound; + uint16_t offset = aMessage.GetOffset(); while (true) { - SuccessOrExit(aMessage.Read(offset, tlv)); + SuccessOrExit(ParseFrom(aMessage, offset)); - if (tlv.mLength != kExtendedLength) + if (mType == aType) { - size = tlv.GetSize(); - } - else - { - ExtendedTlv extTlv; - - SuccessOrExit(aMessage.Read(offset, extTlv)); - - VerifyOrExit(extTlv.GetLength() <= (remainingLen - sizeof(ExtendedTlv))); - size = extTlv.GetSize(); - } - - VerifyOrExit(size <= remainingLen); - - if (tlv.GetType() == aType) - { - if (aOffset != nullptr) - { - *aOffset = offset; - } - - if (aSize != nullptr) - { - *aSize = static_cast(size); - } - - if (aIsExtendedTlv != nullptr) - { - *aIsExtendedTlv = (tlv.mLength == kExtendedLength); - } - error = kErrorNone; ExitNow(); } - offset += size; - remainingLen -= size; + // `ParseFrom()` already validated that `offset + mSize` is + // less than `aMessage.GetLength()` and therefore we can not + // have an overflow here. + + offset += mSize; } exit: @@ -178,15 +187,15 @@ exit: Error Tlv::ReadStringTlv(const Message &aMessage, uint16_t aOffset, uint8_t aMaxStringLength, char *aValue) { - Error error = kErrorNone; - uint16_t valueOffset; - uint16_t length; + Error error = kErrorNone; + ParsedInfo info; + uint16_t length; - SuccessOrExit(error = ReadTlv(aMessage, aOffset, length, valueOffset)); + SuccessOrExit(error = info.ParseFrom(aMessage, aOffset)); - length = Min(length, static_cast(aMaxStringLength)); + length = Min(info.mLength, static_cast(aMaxStringLength)); - aMessage.ReadBytes(valueOffset, aValue, length); + aMessage.ReadBytes(info.mValueOffset, aValue, length); aValue[length + 1] = '\0'; exit: @@ -209,47 +218,16 @@ template Error Tlv::ReadUintTlv(const Message &aMessage, uint16_t aOffs template Error Tlv::ReadUintTlv(const Message &aMessage, uint16_t aOffset, uint16_t &aValue); template Error Tlv::ReadUintTlv(const Message &aMessage, uint16_t aOffset, uint32_t &aValue); -Error Tlv::ReadTlv(const Message &aMessage, uint16_t aOffset, uint16_t &aLength, uint16_t &aValueOffset) -{ - Error error; - Tlv tlv; - uint32_t size; - - SuccessOrExit(error = aMessage.Read(aOffset, tlv)); - - if (!tlv.IsExtended()) - { - aValueOffset = aOffset + sizeof(Tlv); - aLength = tlv.GetLength(); - size = sizeof(Tlv) + aLength; - } - else - { - ExtendedTlv extTlv; - - SuccessOrExit(error = aMessage.Read(aOffset, extTlv)); - aValueOffset = aOffset + sizeof(ExtendedTlv); - aLength = extTlv.GetLength(); - size = sizeof(ExtendedTlv) + aLength; - } - - VerifyOrExit(aOffset + size <= aMessage.GetLength(), error = kErrorParse); - -exit: - return error; -} - Error Tlv::ReadTlvValue(const Message &aMessage, uint16_t aOffset, void *aValue, uint8_t aMinLength) { - Error error; - uint16_t valueOffset; - uint16_t length; + Error error; + ParsedInfo info; - SuccessOrExit(error = ReadTlv(aMessage, aOffset, length, valueOffset)); + SuccessOrExit(error = info.ParseFrom(aMessage, aOffset)); - VerifyOrExit(length >= aMinLength, error = kErrorParse); + VerifyOrExit(info.mLength >= aMinLength, error = kErrorParse); - aMessage.ReadBytes(valueOffset, aValue, aMinLength); + aMessage.ReadBytes(info.mValueOffset, aValue, aMinLength); exit: return error; diff --git a/src/core/common/tlvs.hpp b/src/core/common/tlvs.hpp index 5cab6a7eb..8de307c3b 100644 --- a/src/core/common/tlvs.hpp +++ b/src/core/common/tlvs.hpp @@ -501,8 +501,18 @@ protected: static const uint8_t kExtendedLength = 255; // Extended Length value. private: - static Error ReadTlv(const Message &aMessage, uint16_t aOffset, uint16_t &aLength, uint16_t &aValueOffset); - static Error Find(const Message &aMessage, uint8_t aType, uint16_t *aOffset, uint16_t *aSize, bool *aIsExtendedTlv); + struct ParsedInfo + { + Error ParseFrom(const Message &aMessage, uint16_t aOffset); + Error FindIn(const Message &aMessage, uint8_t aType); + + uint8_t mType; + uint16_t mLength; + uint16_t mOffset; + uint16_t mValueOffset; + uint16_t mSize; + }; + static Error FindTlv(const Message &aMessage, uint8_t aType, void *aValue, uint8_t aLength); static Error AppendTlv(Message &aMessage, uint8_t aType, const void *aValue, uint8_t aLength); static Error ReadStringTlv(const Message &aMessage, uint16_t aOffset, uint8_t aMaxStringLength, char *aValue); diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 2176d4df9..34d9a187f 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -1001,3 +1001,24 @@ target_link_libraries(ot-test-timer ) add_test(NAME ot-test-timer COMMAND ot-test-timer) + +add_executable(ot-test-tlv + test_tlv.cpp +) + +target_include_directories(ot-test-tlv + PRIVATE + ${COMMON_INCLUDES} +) + +target_compile_options(ot-test-tlv + PRIVATE + ${COMMON_COMPILE_OPTIONS} +) + +target_link_libraries(ot-test-tlv + PRIVATE + ${COMMON_LIBS} +) + +add_test(NAME ot-test-tlv COMMAND ot-test-tlv) diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am index 46e7b5622..c1a5e76fb 100644 --- a/tests/unit/Makefile.am +++ b/tests/unit/Makefile.am @@ -155,6 +155,7 @@ check_PROGRAMS += \ ot-test-srp-server \ ot-test-string \ ot-test-timer \ + ot-test-tlv \ $(NULL) if OPENTHREAD_ENABLE_NCP @@ -387,6 +388,10 @@ ot_test_timer_LDADD = $(COMMON_LDADD) ot_test_timer_LIBTOOLFLAGS = $(COMMON_LIBTOOLFLAGS) ot_test_timer_SOURCES = $(COMMON_SOURCES) test_timer.cpp +ot_test_tlv_LDADD = $(COMMON_LDADD) +ot_test_tlv_LIBTOOLFLAGS = $(COMMON_LIBTOOLFLAGS) +ot_test_tlv_SOURCES = $(COMMON_SOURCES) test_tlv.cpp + ot_test_toolchain_LDADD = $(NULL) ot_test_toolchain_SOURCES = test_toolchain.cpp test_toolchain_c.c diff --git a/tests/unit/test_tlv.cpp b/tests/unit/test_tlv.cpp new file mode 100644 index 000000000..c39cfdebc --- /dev/null +++ b/tests/unit/test_tlv.cpp @@ -0,0 +1,194 @@ +/* + * Copyright (c) 2022, The OpenThread Authors. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. Neither the name of the copyright holder nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + +#include "test_platform.h" + +#include + +#include "common/instance.hpp" +#include "common/message.hpp" +#include "common/tlvs.hpp" + +#include "test_util.h" + +namespace ot { + +void TestTlv(void) +{ + Instance * instance = testInitInstance(); + Message * message; + Tlv tlv; + ExtendedTlv extTlv; + uint16_t offset; + uint16_t valueOffset; + uint16_t length; + uint8_t buffer[4]; + + VerifyOrQuit(instance != nullptr); + + VerifyOrQuit((message = instance->Get().Allocate(Message::kTypeIp6)) != nullptr); + VerifyOrQuit(message != nullptr); + + VerifyOrQuit(message->GetOffset() == 0); + VerifyOrQuit(message->GetLength() == 0); + + VerifyOrQuit(Tlv::FindTlvValueOffset(*message, /* aType */ 1, valueOffset, length) == kErrorNotFound); + VerifyOrQuit(Tlv::ReadTlvValue(*message, 0, buffer, 1) == kErrorParse); + + // Add an empty TLV with type 1 and check that we can find it + + offset = message->GetLength(); + + tlv.SetType(1); + tlv.SetLength(0); + SuccessOrQuit(message->Append(tlv)); + + SuccessOrQuit(Tlv::FindTlvValueOffset(*message, /* aType */ 1, valueOffset, length)); + VerifyOrQuit(valueOffset == sizeof(Tlv)); + VerifyOrQuit(length == 0); + SuccessOrQuit(Tlv::ReadTlvValue(*message, offset, buffer, 0)); + VerifyOrQuit(Tlv::ReadTlvValue(*message, offset, buffer, 1) == kErrorParse); + + // Add an empty extended TLV (type 2), and check that we can find it. + + offset = message->GetLength(); + + extTlv.SetType(2); + extTlv.SetLength(0); + SuccessOrQuit(message->Append(extTlv)); + + SuccessOrQuit(Tlv::FindTlvValueOffset(*message, /* aType */ 2, valueOffset, length)); + VerifyOrQuit(valueOffset == offset + sizeof(ExtendedTlv)); + VerifyOrQuit(length == 0); + SuccessOrQuit(Tlv::ReadTlvValue(*message, offset, buffer, 0)); + VerifyOrQuit(Tlv::ReadTlvValue(*message, offset, buffer, 1) == kErrorParse); + + // Add a TLV with type 3 with one byte value and check if we can find it. + + offset = message->GetLength(); + + tlv.SetType(3); + tlv.SetLength(1); + SuccessOrQuit(message->Append(tlv)); + SuccessOrQuit(message->Append(0xff)); + + SuccessOrQuit(Tlv::FindTlvValueOffset(*message, /* aType */ 3, valueOffset, length)); + VerifyOrQuit(valueOffset == offset + sizeof(Tlv)); + VerifyOrQuit(length == 1); + SuccessOrQuit(Tlv::ReadTlvValue(*message, offset, buffer, 1)); + VerifyOrQuit(buffer[0] == 0x0ff); + VerifyOrQuit(Tlv::ReadTlvValue(*message, offset, buffer, 2) == kErrorParse); + + // Add an extended TLV with type 4 with two byte value and check if we can find it. + + offset = message->GetLength(); + + extTlv.SetType(4); + extTlv.SetLength(2); + SuccessOrQuit(message->Append(extTlv)); + SuccessOrQuit(message->Append(0x12)); + SuccessOrQuit(message->Append(0x34)); + + SuccessOrQuit(Tlv::FindTlvValueOffset(*message, /* aType */ 4, valueOffset, length)); + VerifyOrQuit(valueOffset == offset + sizeof(ExtendedTlv)); + VerifyOrQuit(length == 2); + SuccessOrQuit(Tlv::ReadTlvValue(*message, offset, buffer, 1)); + VerifyOrQuit(buffer[0] == 0x12); + SuccessOrQuit(Tlv::ReadTlvValue(*message, offset, buffer, 2)); + VerifyOrQuit(buffer[0] == 0x12); + VerifyOrQuit(buffer[1] == 0x34); + VerifyOrQuit(Tlv::ReadTlvValue(*message, offset, buffer, 3) == kErrorParse); + + // Add a TLV with missing value. + + offset = message->GetLength(); + + tlv.SetType(5); + tlv.SetLength(1); + SuccessOrQuit(message->Append(tlv)); + + VerifyOrQuit(Tlv::FindTlvValueOffset(*message, /* aType */ 5, valueOffset, length) != kErrorNone); + VerifyOrQuit(Tlv::ReadTlvValue(*message, offset, buffer, 0) == kErrorParse); + + // Add the missing value. + SuccessOrQuit(message->Append(0xaa)); + + SuccessOrQuit(Tlv::FindTlvValueOffset(*message, /* aType */ 5, valueOffset, length)); + VerifyOrQuit(valueOffset == offset + sizeof(Tlv)); + VerifyOrQuit(length == 1); + SuccessOrQuit(Tlv::ReadTlvValue(*message, offset, buffer, 1)); + VerifyOrQuit(buffer[0] == 0xaa); + VerifyOrQuit(Tlv::ReadTlvValue(*message, offset, buffer, 2) == kErrorParse); + + // Add an extended TLV with missing value. + + offset = message->GetLength(); + + extTlv.SetType(6); + extTlv.SetLength(2); + SuccessOrQuit(message->Append(extTlv)); + SuccessOrQuit(message->Append(0xbb)); + + VerifyOrQuit(Tlv::FindTlvValueOffset(*message, /* aType */ 6, valueOffset, length) != kErrorNone); + VerifyOrQuit(Tlv::ReadTlvValue(*message, offset, buffer, 1) == kErrorParse); + + SuccessOrQuit(message->Append(0xcc)); + + SuccessOrQuit(Tlv::FindTlvValueOffset(*message, /* aType */ 6, valueOffset, length) != kErrorNone); + VerifyOrQuit(valueOffset == offset + sizeof(ExtendedTlv)); + VerifyOrQuit(length == 2); + SuccessOrQuit(Tlv::ReadTlvValue(*message, offset, buffer, 2)); + VerifyOrQuit(buffer[0] == 0xbb); + VerifyOrQuit(buffer[1] == 0xcc); + VerifyOrQuit(Tlv::ReadTlvValue(*message, offset, buffer, 3) == kErrorParse); + + // Add an extended TLV with overflow length. + + offset = message->GetLength(); + + extTlv.SetType(7); + extTlv.SetLength(0xffff); + SuccessOrQuit(message->Append(extTlv)); + SuccessOrQuit(message->Append(0x11)); + + VerifyOrQuit(Tlv::FindTlvValueOffset(*message, /* aType */ 7, valueOffset, length) != kErrorNone); + VerifyOrQuit(Tlv::ReadTlvValue(*message, offset, buffer, 1) == kErrorParse); + + message->Free(); + + testFreeInstance(instance); +} + +} // namespace ot + +int main(void) +{ + ot::TestTlv(); + printf("All tests passed\n"); + return 0; +}