From 25316e8d91d12f271d6c12a9f7e64ac2410b17a6 Mon Sep 17 00:00:00 2001 From: Karsten Sperling Date: Mon, 15 Jul 2024 22:33:15 +1200 Subject: [PATCH 1/2] ThreadOperationalDataset: various bug fixes - Ensure TLVs read have the correct length - Default construct as empty (mLength == 0) - Change MakeRoom() size argument to size_t to avoid chance of truncation - Check for null in SetNetworkName - Use Encoding::BigEndian instead of hand-rolled math - Use ReturnErrorOnFailure / VerifyOrReturnError consistently - Make tests independent from each other (non-static dataset member) - Add more tests --- src/lib/support/ThreadOperationalDataset.cpp | 228 +++++------------- src/lib/support/ThreadOperationalDataset.h | 35 +-- .../tests/TestThreadOperationalDataset.cpp | 170 ++++++++----- 3 files changed, 186 insertions(+), 247 deletions(-) diff --git a/src/lib/support/ThreadOperationalDataset.cpp b/src/lib/support/ThreadOperationalDataset.cpp index 70db620f8b24bc..91e15a95f3bf90 100644 --- a/src/lib/support/ThreadOperationalDataset.cpp +++ b/src/lib/support/ThreadOperationalDataset.cpp @@ -70,7 +70,7 @@ class ThreadTLV final mLength = aLength; } - const void * GetValue() const + const uint8_t * GetValue() const { assert(mLength != kLengthEscape); @@ -79,75 +79,44 @@ class ThreadTLV final return reinterpret_cast(this) + sizeof(*this); } - void * GetValue() { return const_cast(const_cast(this)->GetValue()); } + uint8_t * GetValue() { return const_cast(const_cast(this)->GetValue()); } + + ByteSpan GetValueAsSpan() const { return ByteSpan(static_cast(GetValue()), GetLength()); } void Get64(uint64_t & aValue) const { assert(GetLength() >= sizeof(aValue)); - - const uint8_t * p = reinterpret_cast(GetValue()); - aValue = // - (static_cast(p[0]) << 56) | // - (static_cast(p[1]) << 48) | // - (static_cast(p[2]) << 40) | // - (static_cast(p[3]) << 32) | // - (static_cast(p[4]) << 24) | // - (static_cast(p[5]) << 16) | // - (static_cast(p[6]) << 8) | // - (static_cast(p[7])); + aValue = Encoding::BigEndian::Get64(GetValue()); } - void Get16(uint16_t & aValue) const + void Get32(uint32_t & aValue) const { assert(GetLength() >= sizeof(aValue)); - - const uint8_t * p = static_cast(GetValue()); - - aValue = static_cast(p[0] << 8 | p[1]); + aValue = Encoding::BigEndian::Get32(GetValue()); } - void Get8(uint8_t & aValue) const + void Get16(uint16_t & aValue) const { assert(GetLength() >= sizeof(aValue)); - aValue = *static_cast(GetValue()); + aValue = Encoding::BigEndian::Get16(GetValue()); } void Set64(uint64_t aValue) { - uint8_t * value = static_cast(GetValue()); - - SetLength(sizeof(aValue)); - - value[0] = static_cast((aValue >> 56) & 0xff); - value[1] = static_cast((aValue >> 48) & 0xff); - value[2] = static_cast((aValue >> 40) & 0xff); - value[3] = static_cast((aValue >> 32) & 0xff); - value[4] = static_cast((aValue >> 24) & 0xff); - value[5] = static_cast((aValue >> 16) & 0xff); - value[6] = static_cast((aValue >> 8) & 0xff); - value[7] = static_cast(aValue & 0xff); - } - - void Set16(uint16_t aValue) - { - uint8_t * value = static_cast(GetValue()); - SetLength(sizeof(aValue)); - - value[0] = static_cast(aValue >> 8); - value[1] = static_cast(aValue & 0xff); + Encoding::BigEndian::Put64(GetValue(), aValue); } - void Set8(uint8_t aValue) + void Set32(uint32_t aValue) { SetLength(sizeof(aValue)); - *static_cast(GetValue()) = aValue; + Encoding::BigEndian::Put32(GetValue(), aValue); } - void Set8(int8_t aValue) + void Set16(uint16_t aValue) { SetLength(sizeof(aValue)); - *static_cast(GetValue()) = aValue; + Encoding::BigEndian::Put16(GetValue(), aValue); } void SetValue(const void * aValue, uint8_t aLength) @@ -218,24 +187,16 @@ CHIP_ERROR OperationalDataset::Init(ByteSpan aData) CHIP_ERROR OperationalDataset::GetActiveTimestamp(uint64_t & aActiveTimestamp) const { const ThreadTLV * tlv = Locate(ThreadTLV::kActiveTimestamp); - - if (tlv != nullptr) - { - tlv->Get64(aActiveTimestamp); - return CHIP_NO_ERROR; - } - - return CHIP_ERROR_TLV_TAG_NOT_FOUND; + VerifyOrReturnError(tlv != nullptr, CHIP_ERROR_TLV_TAG_NOT_FOUND); + VerifyOrReturnError(tlv->GetLength() == sizeof(aActiveTimestamp), CHIP_ERROR_INVALID_TLV_ELEMENT); + tlv->Get64(aActiveTimestamp); + return CHIP_NO_ERROR; } CHIP_ERROR OperationalDataset::SetActiveTimestamp(uint64_t aActiveTimestamp) { ThreadTLV * tlv = MakeRoom(ThreadTLV::kActiveTimestamp, sizeof(*tlv) + sizeof(aActiveTimestamp)); - - if (tlv == nullptr) - { - return CHIP_ERROR_NO_MEMORY; - } + VerifyOrReturnError(tlv != nullptr, CHIP_ERROR_NO_MEMORY); tlv->Set64(aActiveTimestamp); @@ -247,26 +208,19 @@ CHIP_ERROR OperationalDataset::SetActiveTimestamp(uint64_t aActiveTimestamp) CHIP_ERROR OperationalDataset::GetChannel(uint16_t & aChannel) const { const ThreadTLV * tlv = Locate(ThreadTLV::kChannel); - - if (tlv != nullptr) - { - const uint8_t * value = reinterpret_cast(tlv->GetValue()); - aChannel = static_cast((value[1] << 8) | value[2]); - return CHIP_NO_ERROR; - } - - return CHIP_ERROR_TLV_TAG_NOT_FOUND; + VerifyOrReturnError(tlv != nullptr, CHIP_ERROR_TLV_TAG_NOT_FOUND); + VerifyOrReturnError(tlv->GetLength() == 3, CHIP_ERROR_INVALID_TLV_ELEMENT); + // Note: The channel page (byte 0) is not returned + const uint8_t * value = tlv->GetValue(); + aChannel = static_cast((value[1] << 8) | value[2]); + return CHIP_NO_ERROR; } CHIP_ERROR OperationalDataset::SetChannel(uint16_t aChannel) { uint8_t value[] = { 0, static_cast(aChannel >> 8), static_cast(aChannel & 0xff) }; ThreadTLV * tlv = MakeRoom(ThreadTLV::kChannel, sizeof(*tlv) + sizeof(value)); - - if (tlv == nullptr) - { - return CHIP_ERROR_NO_MEMORY; - } + VerifyOrReturnError(tlv != nullptr, CHIP_ERROR_NO_MEMORY); tlv->SetValue(value, sizeof(value)); @@ -278,13 +232,7 @@ CHIP_ERROR OperationalDataset::SetChannel(uint16_t aChannel) CHIP_ERROR OperationalDataset::GetExtendedPanId(uint8_t (&aExtendedPanId)[kSizeExtendedPanId]) const { ByteSpan extPanIdSpan; - CHIP_ERROR error = GetExtendedPanIdAsByteSpan(extPanIdSpan); - - if (error != CHIP_NO_ERROR) - { - return error; - } - + ReturnErrorOnFailure(GetExtendedPanIdAsByteSpan(extPanIdSpan)); memcpy(aExtendedPanId, extPanIdSpan.data(), extPanIdSpan.size()); return CHIP_NO_ERROR; } @@ -292,17 +240,8 @@ CHIP_ERROR OperationalDataset::GetExtendedPanId(uint8_t (&aExtendedPanId)[kSizeE CHIP_ERROR OperationalDataset::GetExtendedPanIdAsByteSpan(ByteSpan & span) const { const ThreadTLV * tlv = Locate(ThreadTLV::kExtendedPanId); - - if (tlv == nullptr) - { - return CHIP_ERROR_TLV_TAG_NOT_FOUND; - } - - if (tlv->GetLength() != kSizeExtendedPanId) - { - return CHIP_ERROR_INVALID_TLV_ELEMENT; - } - + VerifyOrReturnError(tlv != nullptr, CHIP_ERROR_TLV_TAG_NOT_FOUND); + VerifyOrReturnError(tlv->GetLength() == kSizeExtendedPanId, CHIP_ERROR_INVALID_TLV_ELEMENT); span = ByteSpan(static_cast(tlv->GetValue()), tlv->GetLength()); return CHIP_NO_ERROR; } @@ -310,11 +249,7 @@ CHIP_ERROR OperationalDataset::GetExtendedPanIdAsByteSpan(ByteSpan & span) const CHIP_ERROR OperationalDataset::SetExtendedPanId(const uint8_t (&aExtendedPanId)[kSizeExtendedPanId]) { ThreadTLV * tlv = MakeRoom(ThreadTLV::kExtendedPanId, sizeof(*tlv) + sizeof(aExtendedPanId)); - - if (tlv == nullptr) - { - return CHIP_ERROR_NO_MEMORY; - } + VerifyOrReturnError(tlv != nullptr, CHIP_ERROR_NO_MEMORY); tlv->SetValue(aExtendedPanId, sizeof(aExtendedPanId)); @@ -328,24 +263,16 @@ CHIP_ERROR OperationalDataset::SetExtendedPanId(const uint8_t (&aExtendedPanId)[ CHIP_ERROR OperationalDataset::GetMasterKey(uint8_t (&aMasterKey)[kSizeMasterKey]) const { const ThreadTLV * tlv = Locate(ThreadTLV::kMasterKey); - - if (tlv != nullptr) - { - memcpy(aMasterKey, tlv->GetValue(), sizeof(aMasterKey)); - return CHIP_NO_ERROR; - } - - return CHIP_ERROR_TLV_TAG_NOT_FOUND; + VerifyOrReturnError(tlv != nullptr, CHIP_ERROR_TLV_TAG_NOT_FOUND); + VerifyOrReturnError(tlv->GetLength() == sizeof(aMasterKey), CHIP_ERROR_INVALID_TLV_ELEMENT); + memcpy(aMasterKey, tlv->GetValue(), sizeof(aMasterKey)); + return CHIP_NO_ERROR; } CHIP_ERROR OperationalDataset::SetMasterKey(const uint8_t (&aMasterKey)[kSizeMasterKey]) { ThreadTLV * tlv = MakeRoom(ThreadTLV::kMasterKey, sizeof(*tlv) + sizeof(aMasterKey)); - - if (tlv == nullptr) - { - return CHIP_ERROR_NO_MEMORY; - } + VerifyOrReturnError(tlv != nullptr, CHIP_ERROR_NO_MEMORY); tlv->SetValue(aMasterKey, sizeof(aMasterKey)); @@ -359,24 +286,16 @@ CHIP_ERROR OperationalDataset::SetMasterKey(const uint8_t (&aMasterKey)[kSizeMas CHIP_ERROR OperationalDataset::GetMeshLocalPrefix(uint8_t (&aMeshLocalPrefix)[kSizeMeshLocalPrefix]) const { const ThreadTLV * tlv = Locate(ThreadTLV::kMeshLocalPrefix); - - if (tlv != nullptr) - { - memcpy(aMeshLocalPrefix, tlv->GetValue(), sizeof(aMeshLocalPrefix)); - return CHIP_NO_ERROR; - } - - return CHIP_ERROR_TLV_TAG_NOT_FOUND; + VerifyOrReturnError(tlv != nullptr, CHIP_ERROR_TLV_TAG_NOT_FOUND); + VerifyOrReturnError(tlv->GetLength() == sizeof(aMeshLocalPrefix), CHIP_ERROR_INVALID_TLV_ELEMENT); + memcpy(aMeshLocalPrefix, tlv->GetValue(), sizeof(aMeshLocalPrefix)); + return CHIP_NO_ERROR; } CHIP_ERROR OperationalDataset::SetMeshLocalPrefix(const uint8_t (&aMeshLocalPrefix)[kSizeMeshLocalPrefix]) { ThreadTLV * tlv = MakeRoom(ThreadTLV::kMeshLocalPrefix, sizeof(*tlv) + sizeof(aMeshLocalPrefix)); - - if (tlv == nullptr) - { - return CHIP_ERROR_NO_MEMORY; - } + VerifyOrReturnError(tlv != nullptr, CHIP_ERROR_NO_MEMORY); tlv->SetValue(aMeshLocalPrefix, sizeof(aMeshLocalPrefix)); @@ -388,32 +307,21 @@ CHIP_ERROR OperationalDataset::SetMeshLocalPrefix(const uint8_t (&aMeshLocalPref CHIP_ERROR OperationalDataset::GetNetworkName(char (&aNetworkName)[kSizeNetworkName + 1]) const { const ThreadTLV * tlv = Locate(ThreadTLV::kNetworkName); - - if (tlv != nullptr) - { - memcpy(aNetworkName, tlv->GetValue(), tlv->GetLength()); - aNetworkName[tlv->GetLength()] = '\0'; - return CHIP_NO_ERROR; - } - - return CHIP_ERROR_TLV_TAG_NOT_FOUND; + VerifyOrReturnError(tlv != nullptr, CHIP_ERROR_TLV_TAG_NOT_FOUND); + VerifyOrReturnError(tlv->GetLength() <= kSizeNetworkName, CHIP_ERROR_INVALID_TLV_ELEMENT); + memcpy(aNetworkName, tlv->GetValue(), tlv->GetLength()); + aNetworkName[tlv->GetLength()] = '\0'; + return CHIP_NO_ERROR; } CHIP_ERROR OperationalDataset::SetNetworkName(const char * aNetworkName) { + VerifyOrReturnError(aNetworkName != nullptr, CHIP_ERROR_INVALID_ARGUMENT); size_t len = strlen(aNetworkName); + VerifyOrReturnError(0 < len && len <= kSizeNetworkName, CHIP_ERROR_INVALID_STRING_LENGTH); - if (len > kSizeNetworkName || len == 0) - { - return CHIP_ERROR_INVALID_STRING_LENGTH; - } - - ThreadTLV * tlv = MakeRoom(ThreadTLV::kNetworkName, static_cast(sizeof(*tlv) + static_cast(len))); - - if (tlv == nullptr) - { - return CHIP_ERROR_NO_MEMORY; - } + ThreadTLV * tlv = MakeRoom(ThreadTLV::kNetworkName, sizeof(*tlv) + len); + VerifyOrReturnError(tlv != nullptr, CHIP_ERROR_NO_MEMORY); tlv->SetValue(aNetworkName, static_cast(len)); @@ -425,24 +333,16 @@ CHIP_ERROR OperationalDataset::SetNetworkName(const char * aNetworkName) CHIP_ERROR OperationalDataset::GetPanId(uint16_t & aPanId) const { const ThreadTLV * tlv = Locate(ThreadTLV::kPanId); - - if (tlv != nullptr) - { - tlv->Get16(aPanId); - return CHIP_NO_ERROR; - } - - return CHIP_ERROR_TLV_TAG_NOT_FOUND; + VerifyOrReturnError(tlv != nullptr, CHIP_ERROR_TLV_TAG_NOT_FOUND); + VerifyOrReturnError(tlv->GetLength() == sizeof(aPanId), CHIP_ERROR_INVALID_TLV_ELEMENT); + tlv->Get16(aPanId); + return CHIP_NO_ERROR; } CHIP_ERROR OperationalDataset::SetPanId(uint16_t aPanId) { ThreadTLV * tlv = MakeRoom(ThreadTLV::kPanId, sizeof(*tlv) + sizeof(aPanId)); - - if (tlv == nullptr) - { - return CHIP_ERROR_NO_MEMORY; - } + VerifyOrReturnError(tlv != nullptr, CHIP_ERROR_NO_MEMORY); tlv->Set16(aPanId); @@ -454,24 +354,16 @@ CHIP_ERROR OperationalDataset::SetPanId(uint16_t aPanId) CHIP_ERROR OperationalDataset::GetPSKc(uint8_t (&aPSKc)[kSizePSKc]) const { const ThreadTLV * tlv = Locate(ThreadTLV::kPSKc); - - if (tlv != nullptr) - { - memcpy(aPSKc, tlv->GetValue(), sizeof(aPSKc)); - return CHIP_NO_ERROR; - } - - return CHIP_ERROR_TLV_TAG_NOT_FOUND; + VerifyOrReturnError(tlv != nullptr, CHIP_ERROR_TLV_TAG_NOT_FOUND); + VerifyOrReturnError(tlv->GetLength() == sizeof(aPSKc), CHIP_ERROR_INVALID_TLV_ELEMENT); + memcpy(aPSKc, tlv->GetValue(), sizeof(aPSKc)); + return CHIP_NO_ERROR; } CHIP_ERROR OperationalDataset::SetPSKc(const uint8_t (&aPSKc)[kSizePSKc]) { ThreadTLV * tlv = MakeRoom(ThreadTLV::kPSKc, sizeof(*tlv) + sizeof(aPSKc)); - - if (tlv == nullptr) - { - return CHIP_ERROR_NO_MEMORY; - } + VerifyOrReturnError(tlv != nullptr, CHIP_ERROR_NO_MEMORY); tlv->SetValue(aPSKc, sizeof(aPSKc)); @@ -533,7 +425,7 @@ void OperationalDataset::Remove(uint8_t aType) } } -ThreadTLV * OperationalDataset::MakeRoom(uint8_t aType, uint8_t aSize) +ThreadTLV * OperationalDataset::MakeRoom(uint8_t aType, size_t aSize) { ThreadTLV * tlv = Locate(aType); diff --git a/src/lib/support/ThreadOperationalDataset.h b/src/lib/support/ThreadOperationalDataset.h index e4de3ddaa8d2c5..7bb528d691c193 100644 --- a/src/lib/support/ThreadOperationalDataset.h +++ b/src/lib/support/ThreadOperationalDataset.h @@ -54,7 +54,6 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully initialized the dataset. * @retval CHIP_ERROR_INVALID_ARGUMENT The dataset length @p aLength is too long or @p data is corrupted. - * */ CHIP_ERROR Init(ByteSpan aData); @@ -65,7 +64,7 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully retrieved the active timestamp. * @retval CHIP_ERROR_TLV_TAG_NOT_FOUND Thread active timestamp is not present in the dataset. - * + * @retval CHIP_ERROR_INVALID_TLV_ELEMENT If the TLV element is invalid. */ CHIP_ERROR GetActiveTimestamp(uint64_t & aActiveTimestamp) const; @@ -76,7 +75,6 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully set the active timestamp. * @retval CHIP_ERROR_NO_MEMORY Insufficient memory in the dataset for setting Thread active timestamp. - * */ CHIP_ERROR SetActiveTimestamp(uint64_t aActiveTimestamp); @@ -87,7 +85,7 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully retrieved the channel. * @retval CHIP_ERROR_TLV_TAG_NOT_FOUND Thread channel is not present in the dataset. - * + * @retval CHIP_ERROR_INVALID_TLV_ELEMENT If the TLV element is invalid. */ CHIP_ERROR GetChannel(uint16_t & aChannel) const; @@ -98,7 +96,6 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully set the channel. * @retval CHIP_ERROR_NO_MEMORY Insufficient memory in the dataset for setting Thread channel. - * */ CHIP_ERROR SetChannel(uint16_t aChannel); @@ -109,7 +106,7 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully retrieved the extended PAN ID. * @retval CHIP_ERROR_TLV_TAG_NOT_FOUND Thread extended PAN ID is not present in the dataset. - * + * @retval CHIP_ERROR_INVALID_TLV_ELEMENT If the TLV element is invalid. */ CHIP_ERROR GetExtendedPanId(uint8_t (&aExtendedPanId)[kSizeExtendedPanId]) const; @@ -121,7 +118,7 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully retrieved the extended PAN ID. * @retval CHIP_ERROR_TLV_TAG_NOT_FOUND Thread extended PAN ID is not present in the dataset. - * + * @retval CHIP_ERROR_INVALID_TLV_ELEMENT If the TLV element is invalid. */ CHIP_ERROR GetExtendedPanIdAsByteSpan(ByteSpan & span) const; @@ -132,7 +129,6 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully set the extended PAN ID. * @retval CHIP_ERROR_NO_MEMORY Insufficient memory in the dataset for setting Thread extended PAN ID. - * */ CHIP_ERROR SetExtendedPanId(const uint8_t (&aExtendedPanId)[kSizeExtendedPanId]); @@ -143,7 +139,7 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully retrieved the master key. * @retval CHIP_ERROR_TLV_TAG_NOT_FOUND Thread master key is not present in the dataset. - * + * @retval CHIP_ERROR_INVALID_TLV_ELEMENT If the TLV element is invalid. */ CHIP_ERROR GetMasterKey(uint8_t (&aMasterKey)[kSizeMasterKey]) const; @@ -154,13 +150,11 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully set the master key. * @retval CHIP_ERROR_NO_MEMORY Insufficient memory in the dataset for setting Thread master key. - * */ CHIP_ERROR SetMasterKey(const uint8_t (&aMasterKey)[kSizeMasterKey]); /** * This method unsets Thread master key to the dataset. - * */ void UnsetMasterKey(void); @@ -171,7 +165,7 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully retrieved the mesh local prefix. * @retval CHIP_ERROR_TLV_TAG_NOT_FOUND Thread mesh local prefix is not present in the dataset. - * + * @retval CHIP_ERROR_INVALID_TLV_ELEMENT If the TLV element is invalid. */ CHIP_ERROR GetMeshLocalPrefix(uint8_t (&aMeshLocalPrefix)[kSizeMeshLocalPrefix]) const; @@ -182,7 +176,6 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully set the Thread mesh local prefix. * @retval CHIP_ERROR_NO_MEMORY Insufficient memory in the dataset for setting Thread mesh local prefix. - * */ CHIP_ERROR SetMeshLocalPrefix(const uint8_t (&aMeshLocalPrefix)[kSizeMeshLocalPrefix]); @@ -193,7 +186,7 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully retrieved the network name. * @retval CHIP_ERROR_TLV_TAG_NOT_FOUND Thread network name is not present in the dataset. - * + * @retval CHIP_ERROR_INVALID_TLV_ELEMENT If the TLV element is invalid. */ CHIP_ERROR GetNetworkName(char (&aNetworkName)[kSizeNetworkName + 1]) const; @@ -204,7 +197,6 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully set the network name. * @retval CHIP_ERROR_NO_MEMORY Insufficient memory in the dataset for setting Thread network name. - * */ CHIP_ERROR SetNetworkName(const char * aNetworkName); @@ -215,7 +207,7 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully retrieved the PAN ID. * @retval CHIP_ERROR_TLV_TAG_NOT_FOUND Thread PAN ID is not present in the dataset. - * + * @retval CHIP_ERROR_INVALID_TLV_ELEMENT If the TLV element is invalid. */ CHIP_ERROR GetPanId(uint16_t & aPanId) const; @@ -226,7 +218,6 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully set the PAN ID. * @retval CHIP_ERROR_NO_MEMORY Insufficient memory in the dataset for setting Thread PAN ID. - * */ CHIP_ERROR SetPanId(uint16_t aPanId); @@ -237,7 +228,7 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully retrieved the PSKc. * @retval CHIP_ERROR_TLV_TAG_NOT_FOUND Thread PSKc is not present in the dataset. - * + * @retval CHIP_ERROR_INVALID_TLV_ELEMENT If the TLV element is invalid. */ CHIP_ERROR GetPSKc(uint8_t (&aPSKc)[kSizePSKc]) const; @@ -248,13 +239,11 @@ class OperationalDataset * * @retval CHIP_NO_ERROR Successfully set the PSKc. * @retval CHIP_ERROR_NO_MEMORY Insufficient memory in the dataset for setting Thread PSKc. - * */ CHIP_ERROR SetPSKc(const uint8_t (&aPSKc)[kSizePSKc]); /** * This method unsets Thread PSKc to the dataset. - * */ void UnsetPSKc(void); @@ -296,12 +285,12 @@ class OperationalDataset ThreadTLV & End(void) { return const_cast(const_cast(this)->End()); } void Remove(uint8_t aType); void Remove(ThreadTLV & aTlv); - ThreadTLV * MakeRoom(uint8_t aType, uint8_t aSize); + ThreadTLV * MakeRoom(uint8_t aType, size_t aSize); bool Has(uint8_t aType) const { return Locate(aType) != nullptr; } uint8_t mData[kSizeOperationalDataset]; - uint8_t mLength; + uint8_t mLength = 0; }; } // namespace Thread -}; // namespace chip +} // namespace chip diff --git a/src/lib/support/tests/TestThreadOperationalDataset.cpp b/src/lib/support/tests/TestThreadOperationalDataset.cpp index d9e13c8c25705f..b37cde4d5308f3 100644 --- a/src/lib/support/tests/TestThreadOperationalDataset.cpp +++ b/src/lib/support/tests/TestThreadOperationalDataset.cpp @@ -30,11 +30,9 @@ class TestThreadOperationalDataset : public ::testing::Test static void SetUpTestSuite() { ASSERT_EQ(chip::Platform::MemoryInit(), CHIP_NO_ERROR); } static void TearDownTestSuite() { chip::Platform::MemoryShutdown(); } - static Thread::OperationalDataset dataset; + Thread::OperationalDataset dataset; }; -Thread::OperationalDataset TestThreadOperationalDataset::dataset; - TEST_F(TestThreadOperationalDataset, TestInit) { @@ -103,6 +101,10 @@ TEST_F(TestThreadOperationalDataset, TestMasterKey) EXPECT_EQ(dataset.SetMasterKey(kMasterKey), CHIP_NO_ERROR); EXPECT_EQ(dataset.GetMasterKey(masterKey), CHIP_NO_ERROR); EXPECT_EQ(memcmp(masterKey, kMasterKey, sizeof(kMasterKey)), 0); + + dataset.UnsetMasterKey(); + EXPECT_EQ(dataset.GetMasterKey(masterKey), CHIP_ERROR_TLV_TAG_NOT_FOUND); + EXPECT_EQ(dataset.SetMasterKey(masterKey), CHIP_NO_ERROR); } TEST_F(TestThreadOperationalDataset, TestMeshLocalPrefix) @@ -129,6 +131,7 @@ TEST_F(TestThreadOperationalDataset, TestNetworkName) EXPECT_EQ(dataset.SetNetworkName("0123456789abcdef"), CHIP_NO_ERROR); EXPECT_EQ(dataset.SetNetworkName("0123456789abcdefg"), CHIP_ERROR_INVALID_STRING_LENGTH); EXPECT_EQ(dataset.SetNetworkName(""), CHIP_ERROR_INVALID_STRING_LENGTH); + EXPECT_EQ(dataset.SetNetworkName(nullptr), CHIP_ERROR_INVALID_ARGUMENT); } TEST_F(TestThreadOperationalDataset, TestPanId) @@ -152,25 +155,7 @@ TEST_F(TestThreadOperationalDataset, TestPSKc) EXPECT_EQ(dataset.SetPSKc(kPSKc), CHIP_NO_ERROR); EXPECT_EQ(dataset.GetPSKc(pskc), CHIP_NO_ERROR); EXPECT_FALSE(memcmp(pskc, kPSKc, sizeof(kPSKc))); -} - -TEST_F(TestThreadOperationalDataset, TestUnsetMasterKey) -{ - - uint8_t masterKey[Thread::kSizeMasterKey] = { 0 }; - EXPECT_EQ(dataset.GetMasterKey(masterKey), CHIP_NO_ERROR); - dataset.UnsetMasterKey(); - EXPECT_EQ(dataset.GetMasterKey(masterKey), CHIP_ERROR_TLV_TAG_NOT_FOUND); - EXPECT_EQ(dataset.SetMasterKey(masterKey), CHIP_NO_ERROR); -} - -TEST_F(TestThreadOperationalDataset, TestUnsetPSKc) -{ - - uint8_t pskc[Thread::kSizePSKc] = { 0 }; - - EXPECT_EQ(dataset.GetPSKc(pskc), CHIP_NO_ERROR); dataset.UnsetPSKc(); EXPECT_EQ(dataset.GetPSKc(pskc), CHIP_ERROR_TLV_TAG_NOT_FOUND); EXPECT_EQ(dataset.SetPSKc(pskc), CHIP_NO_ERROR); @@ -178,45 +163,19 @@ TEST_F(TestThreadOperationalDataset, TestUnsetPSKc) TEST_F(TestThreadOperationalDataset, TestClear) { - - { - uint64_t activeTimestamp; - EXPECT_EQ(dataset.GetActiveTimestamp(activeTimestamp), CHIP_NO_ERROR); - } - - { - uint16_t channel; - EXPECT_EQ(dataset.GetChannel(channel), CHIP_NO_ERROR); - } - - { - uint8_t extendedPanId[Thread::kSizeExtendedPanId] = { 0 }; - EXPECT_EQ(dataset.GetExtendedPanId(extendedPanId), CHIP_NO_ERROR); - } - { + EXPECT_EQ(dataset.SetActiveTimestamp(123), CHIP_NO_ERROR); + EXPECT_EQ(dataset.SetChannel(5), CHIP_NO_ERROR); + uint8_t extendedPanId[Thread::kSizeExtendedPanId] = { 1, 2, 3, 4, 5, 6, 7, 8 }; + EXPECT_EQ(dataset.SetExtendedPanId(extendedPanId), CHIP_NO_ERROR); uint8_t masterKey[Thread::kSizeMasterKey] = { 0 }; - EXPECT_EQ(dataset.GetMasterKey(masterKey), CHIP_NO_ERROR); - } - - { + EXPECT_EQ(dataset.SetMasterKey(masterKey), CHIP_NO_ERROR); uint8_t meshLocalPrefix[Thread::kSizeMeshLocalPrefix] = { 0 }; - EXPECT_EQ(dataset.GetMeshLocalPrefix(meshLocalPrefix), CHIP_NO_ERROR); - } - - { - char networkName[Thread::kSizeNetworkName + 1] = { 0 }; - EXPECT_EQ(dataset.GetNetworkName(networkName), CHIP_NO_ERROR); - } - - { - uint16_t panid; - EXPECT_EQ(dataset.GetPanId(panid), CHIP_NO_ERROR); - } - - { + EXPECT_EQ(dataset.SetMeshLocalPrefix(meshLocalPrefix), CHIP_NO_ERROR); + EXPECT_EQ(dataset.SetNetworkName("w00tw00t"), CHIP_NO_ERROR); + EXPECT_EQ(dataset.SetPanId(0x4242), CHIP_NO_ERROR); uint8_t pskc[Thread::kSizePSKc] = { 0 }; - EXPECT_EQ(dataset.GetPSKc(pskc), CHIP_NO_ERROR); + EXPECT_EQ(dataset.SetPSKc(pskc), CHIP_NO_ERROR); } dataset.Clear(); @@ -262,4 +221,103 @@ TEST_F(TestThreadOperationalDataset, TestClear) } } +TEST_F(TestThreadOperationalDataset, TestExampleDataset) +{ + const uint8_t example[] = { + 0x0e, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, // Active Timestamp 1 + 0x00, 0x03, 0x00, 0x00, 0x0f, // Channel 15 + 0x35, 0x04, 0x07, 0xff, 0xf8, 0x00, // Channel Mask 0x07fff800 + 0x02, 0x08, 0x39, 0x75, 0x8e, 0xc8, 0x14, 0x4b, 0x07, 0xfb, // Ext PAN ID 39758ec8144b07fb + 0x07, 0x08, 0xfd, 0xf1, 0xf1, 0xad, 0xd0, 0x79, 0x7d, 0xc0, // Mesh Local Prefix fdf1:f1ad:d079:7dc0::/64 + 0x05, 0x10, 0xf3, 0x66, 0xce, 0xc7, 0xa4, 0x46, 0xba, 0xb9, 0x78, 0xd9, 0x0d, 0x27, 0xab, 0xe3, 0x8f, 0x23, // Network Key + 0x03, 0x0f, 0x4f, 0x70, 0x65, 0x6e, 0x54, 0x68, 0x72, 0x65, 0x61, 0x64, 0x2d, 0x35, 0x39, 0x33, 0x38, // "OpenThread-5938" + 0x01, 0x02, 0x59, 0x38, // PAN ID : 0x5938 + 0x04, 0x10, 0x3c, 0xa6, 0x7c, 0x96, 0x9e, 0xfb, 0x0d, 0x0c, 0x74, 0xa4, 0xd8, 0xee, 0x92, 0x3b, 0x57, 0x6c, // PKSc + 0x0c, 0x04, 0x02, 0xa0, 0xf7, 0xf8, // Security Policy + }; + EXPECT_EQ(dataset.Init(ByteSpan(example)), CHIP_NO_ERROR); + + uint64_t activeTimestamp; + EXPECT_EQ(dataset.GetActiveTimestamp(activeTimestamp), CHIP_NO_ERROR); + EXPECT_EQ(activeTimestamp, 1u); + + uint16_t channel; + EXPECT_EQ(dataset.GetChannel(channel), CHIP_NO_ERROR); + EXPECT_EQ(channel, 15u); + + uint8_t extPanId[Thread::kSizeExtendedPanId]; + EXPECT_EQ(dataset.GetExtendedPanId(extPanId), CHIP_NO_ERROR); + const uint8_t expectedExtPanId[] = { 0x39, 0x75, 0x8e, 0xc8, 0x14, 0x4b, 0x07, 0xfb }; + EXPECT_TRUE(ByteSpan(extPanId).data_equal(ByteSpan(expectedExtPanId))); + + uint8_t meshLocalPrefix[Thread::kSizeMeshLocalPrefix]; + EXPECT_EQ(dataset.GetMeshLocalPrefix(meshLocalPrefix), CHIP_NO_ERROR); + const uint8_t expectedMeshLocalPrefix[] = { 0xfd, 0xf1, 0xf1, 0xad, 0xd0, 0x79, 0x7d, 0xc0 }; + EXPECT_TRUE(ByteSpan(meshLocalPrefix).data_equal(ByteSpan(expectedMeshLocalPrefix))); + + uint8_t masterKey[Thread::kSizeMasterKey]; + EXPECT_EQ(dataset.GetMasterKey(masterKey), CHIP_NO_ERROR); + const uint8_t expectedMasterKey[] = { 0xf3, 0x66, 0xce, 0xc7, 0xa4, 0x46, 0xba, 0xb9, + 0x78, 0xd9, 0x0d, 0x27, 0xab, 0xe3, 0x8f, 0x23 }; + EXPECT_TRUE(ByteSpan(masterKey).data_equal(ByteSpan(expectedMasterKey))); + + char networkName[Thread::kSizeNetworkName + 1]; + EXPECT_EQ(dataset.GetNetworkName(networkName), CHIP_NO_ERROR); + EXPECT_EQ(strncmp(networkName, "OpenThread-5938", sizeof(networkName)), 0); + + uint16_t panId; + EXPECT_EQ(dataset.GetPanId(panId), CHIP_NO_ERROR); + EXPECT_EQ(panId, 0x5938u); + + uint8_t pksc[Thread::kSizePSKc]; + EXPECT_EQ(dataset.GetPSKc(pksc), CHIP_NO_ERROR); + const uint8_t expectedPksc[] = { + 0x3c, 0xa6, 0x7c, 0x96, 0x9e, 0xfb, 0x0d, 0x0c, 0x74, 0xa4, 0xd8, 0xee, 0x92, 0x3b, 0x57, 0x6c + }; + EXPECT_TRUE(ByteSpan(pksc).data_equal(ByteSpan(expectedPksc))); +} + +TEST_F(TestThreadOperationalDataset, TestInvalidExampleDataset) +{ + const uint8_t invalid[] = { + 0x0e, 0x01, 0x01, // Active Timestamp + 0x00, 0x01, 0x0f, // Channel + 0x35, 0x00, // Channel Mask + 0x02, 0x09, 0x39, 0x75, 0x8e, 0xc8, 0x14, 0x4b, 0x07, 0xfb, 0xff, // Ext PAN ID + 0x07, 0x01, 0x01, // Mesh Local Prefix + 0x05, 0x0f, 0xf3, 0x66, 0xce, 0xc7, 0xa4, 0x46, 0xba, 0xb9, 0x78, 0xd9, 0x0d, 0x27, 0xab, 0xe3, 0x8f, // Network Key + 0x03, 0x11, 0x4f, 0x70, 0x65, 0x6e, 0x54, 0x68, 0x72, 0x65, 0x61, 0x64, 0x2d, 0x35, 0x39, 0x33, 0x38, 0xff, 0xff, // Name + 0x01, 0x01, 0xff, // PAN ID + 0x04, 0x01, 0x3c, // PKSc + 0x0c, 0x01, 0x01, // Security Policy + }; + + // The overall TLV structure is valid, but all TLVs have invalid sizes + EXPECT_EQ(dataset.Init(ByteSpan(invalid)), CHIP_NO_ERROR); + + uint64_t activeTimestamp; + EXPECT_EQ(dataset.GetActiveTimestamp(activeTimestamp), CHIP_ERROR_INVALID_TLV_ELEMENT); + + uint16_t channel; + EXPECT_EQ(dataset.GetChannel(channel), CHIP_ERROR_INVALID_TLV_ELEMENT); + + ByteSpan extPanId; + EXPECT_EQ(dataset.GetExtendedPanIdAsByteSpan(extPanId), CHIP_ERROR_INVALID_TLV_ELEMENT); + + uint8_t meshLocalPrefix[Thread::kSizeMeshLocalPrefix]; + EXPECT_EQ(dataset.GetMeshLocalPrefix(meshLocalPrefix), CHIP_ERROR_INVALID_TLV_ELEMENT); + + uint8_t masterKey[Thread::kSizeMasterKey]; + EXPECT_EQ(dataset.GetMasterKey(masterKey), CHIP_ERROR_INVALID_TLV_ELEMENT); + + char networkName[Thread::kSizeNetworkName + 1]; + EXPECT_EQ(dataset.GetNetworkName(networkName), CHIP_ERROR_INVALID_TLV_ELEMENT); + + uint16_t panId; + EXPECT_EQ(dataset.GetPanId(panId), CHIP_ERROR_INVALID_TLV_ELEMENT); + + uint8_t pksc[Thread::kSizePSKc]; + EXPECT_EQ(dataset.GetPSKc(pksc), CHIP_ERROR_INVALID_TLV_ELEMENT); +} + } // namespace From c8d051ff369eb3676d8dbb59e0d354ff65dd4ebb Mon Sep 17 00:00:00 2001 From: Karsten Sperling Date: Tue, 16 Jul 2024 09:31:09 +1200 Subject: [PATCH 2/2] Fix includes --- src/lib/support/ThreadOperationalDataset.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/lib/support/ThreadOperationalDataset.cpp b/src/lib/support/ThreadOperationalDataset.cpp index 91e15a95f3bf90..9f2ae6a1b1a8ec 100644 --- a/src/lib/support/ThreadOperationalDataset.cpp +++ b/src/lib/support/ThreadOperationalDataset.cpp @@ -15,11 +15,13 @@ * limitations under the License. */ -#include -#include - #include +#include + +#include +#include + namespace chip { namespace Thread {