From 837c7fd357d3546f96b9b1433d8f1348371b242e Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Thu, 19 Oct 2023 17:36:23 -0700 Subject: [PATCH] [netdata] simplify and update `HandleTmf()` (#9550) This commit updates `HandleTmf()`, which is used to set the Commissioning Dataset. The following changes have been made: - `Tlv::Find()` methods are used to find and parse TLVs in the received request message. - The existing code required the dataset to contain either Joiner UDP or Steering Data TLVs. This check has been removed, making the implementation aligned with the Thread specification. This permits a commissioner to disallow MeshCoP Joining by not including Steering Data. - A new flavor of `SetCommisioningData()` is added, which reads the TLVs directly from a given `aMessage`. This method is now used when processing `kUriCommissionerSet` to avoid using temporary local buffers to read and copy the TLVs. - `SetCommisioningData()` and its related methods have been moved to `network_data_leader_ftd` since they are limited to and used on FTD devices acting as leader. - A new private `UpdateCommissioningData()` method has been added. This method first checks whether or not we can add a Commissioning Data TLV with a given length into Network Data, before removing the current TLV. This is used as a common method when Commissioning Data is set from a message or from a given data buffer. --- src/core/thread/network_data_leader.cpp | 36 ---- src/core/thread/network_data_leader.hpp | 14 -- src/core/thread/network_data_leader_ftd.cpp | 172 ++++++++++---------- src/core/thread/network_data_leader_ftd.hpp | 15 ++ 4 files changed, 105 insertions(+), 132 deletions(-) diff --git a/src/core/thread/network_data_leader.cpp b/src/core/thread/network_data_leader.cpp index 287995554..c1ad8714a 100644 --- a/src/core/thread/network_data_leader.cpp +++ b/src/core/thread/network_data_leader.cpp @@ -428,31 +428,6 @@ exit: return error; } -Error LeaderBase::SetCommissioningData(const void *aValue, uint8_t aValueLength) -{ - Error error = kErrorNone; - CommissioningDataTlv *commissioningDataTlv; - - RemoveCommissioningData(); - - if (aValueLength > 0) - { - VerifyOrExit(aValueLength <= kMaxSize - sizeof(CommissioningDataTlv), error = kErrorNoBufs); - commissioningDataTlv = As(AppendTlv(sizeof(CommissioningDataTlv) + aValueLength)); - VerifyOrExit(commissioningDataTlv != nullptr, error = kErrorNoBufs); - - commissioningDataTlv->Init(); - commissioningDataTlv->SetLength(aValueLength); - memcpy(commissioningDataTlv->GetValue(), aValue, aValueLength); - } - - mVersion++; - SignalNetDataChanged(); - -exit: - return error; -} - const CommissioningDataTlv *LeaderBase::FindCommissioningData(void) const { return NetworkDataTlv::Find(GetTlvsStart(), GetTlvsEnd()); @@ -520,17 +495,6 @@ exit: return isAllowed; } -void LeaderBase::RemoveCommissioningData(void) -{ - CommissioningDataTlv *tlv = FindCommissioningData(); - - VerifyOrExit(tlv != nullptr); - RemoveTlv(tlv); - -exit: - return; -} - Error LeaderBase::SteeringDataCheck(const FilterIndexes &aFilterIndexes) const { Error error = kErrorNone; diff --git a/src/core/thread/network_data_leader.hpp b/src/core/thread/network_data_leader.hpp index 0977f6ff5..f52752221 100644 --- a/src/core/thread/network_data_leader.hpp +++ b/src/core/thread/network_data_leader.hpp @@ -254,18 +254,6 @@ public: */ bool IsJoiningAllowed(void) const; - /** - * Adds Commissioning Data to the Thread Network Data. - * - * @param[in] aValue A pointer to the Commissioning Data value. - * @param[in] aValueLength The length of @p aValue. - * - * @retval kErrorNone Successfully added the Commissioning Data. - * @retval kErrorNoBufs Insufficient space to add the Commissioning Data. - * - */ - Error SetCommissioningData(const void *aValue, uint8_t aValueLength); - /** * Checks if the steering data includes a Joiner. * @@ -339,8 +327,6 @@ private: const PrefixTlv *FindNextMatchingPrefixTlv(const Ip6::Address &aAddress, const PrefixTlv *aPrevTlv) const; - void RemoveCommissioningData(void); - template int CompareRouteEntries(const EntryType &aFirst, const EntryType &aSecond) const; int CompareRouteEntries(int8_t aFirstPreference, uint16_t aFirstRloc, diff --git a/src/core/thread/network_data_leader_ftd.cpp b/src/core/thread/network_data_leader_ftd.cpp index 2fc46ebc6..ea5f56bc5 100644 --- a/src/core/thread/network_data_leader_ftd.cpp +++ b/src/core/thread/network_data_leader_ftd.cpp @@ -186,96 +186,37 @@ exit: template <> void Leader::HandleTmf(Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo) { - uint16_t offset = aMessage.GetOffset(); - uint16_t length = aMessage.GetLength() - aMessage.GetOffset(); - uint8_t tlvs[NetworkData::kMaxSize]; - MeshCoP::StateTlv::State state = MeshCoP::StateTlv::kReject; - bool hasSessionId = false; - bool hasValidTlv = false; - uint16_t sessionId = 0; - CommissioningDataTlv *commDataTlv; - - MeshCoP::Tlv *cur; - MeshCoP::Tlv *end; + MeshCoP::StateTlv::State state = MeshCoP::StateTlv::kReject; + uint16_t borderAgentRloc; + uint16_t sessionId; + uint16_t localSessionId; VerifyOrExit(Get().IsLeader() && !mWaitingForNetDataSync); - VerifyOrExit(length <= sizeof(tlvs)); + // Validate that there is no Border Agent Locator TLV. This also + // validates that all included TLVs are properly formatted. - aMessage.ReadBytes(offset, tlvs, length); + VerifyOrExit(Tlv::Find(aMessage, borderAgentRloc) == kErrorNotFound); - // Session Id and Border Router Locator MUST NOT be set, but accept including unexpected or - // unknown TLV as long as there is at least one valid TLV. - cur = reinterpret_cast(tlvs); - end = reinterpret_cast(tlvs + length); + SuccessOrExit(Tlv::Find(aMessage, sessionId)); - while (cur < end) + if (FindCommissioningSessionId(localSessionId) == kErrorNone) { - MeshCoP::Tlv::Type type; - - VerifyOrExit(((cur + 1) <= end) && !cur->IsExtended() && (cur->GetNext() <= end)); - - type = cur->GetType(); - - if (type == MeshCoP::Tlv::kJoinerUdpPort || type == MeshCoP::Tlv::kSteeringData) - { - hasValidTlv = true; - } - else if (type == MeshCoP::Tlv::kBorderAgentLocator) - { - ExitNow(); - } - else if (type == MeshCoP::Tlv::kCommissionerSessionId) - { - MeshCoP::CommissionerSessionIdTlv *tlv = As(cur); - - VerifyOrExit(tlv->IsValid()); - sessionId = tlv->GetCommissionerSessionId(); - hasSessionId = true; - } - else - { - // do nothing for unexpected or unknown TLV - } - - cur = cur->GetNext(); + VerifyOrExit(sessionId == localSessionId); } - // verify whether or not commissioner session id TLV is included - VerifyOrExit(hasSessionId); + // Add the Border Agent RLOC TLV from Network Data. - // verify whether or not MGMT_COMM_SET.req includes at least one valid TLV - VerifyOrExit(hasValidTlv); - - // Find Commissioning Data TLV - commDataTlv = FindCommissioningData(); - - if (commDataTlv != nullptr) + if (FindBorderAgentRloc(borderAgentRloc) == kErrorNone) { - // Iterate over MeshCoP TLVs and extract desired data - for (cur = reinterpret_cast(commDataTlv->GetValue()); - cur < reinterpret_cast(commDataTlv->GetValue() + commDataTlv->GetLength()); - cur = cur->GetNext()) - { - if (cur->GetType() == MeshCoP::Tlv::kCommissionerSessionId) - { - VerifyOrExit(sessionId == As(cur)->GetCommissionerSessionId()); - } - else if (cur->GetType() == MeshCoP::Tlv::kBorderAgentLocator) - { - VerifyOrExit(length + cur->GetSize() <= sizeof(tlvs)); - memcpy(tlvs + length, reinterpret_cast(cur), cur->GetSize()); - length += cur->GetSize(); - } - } + SuccessOrExit(Tlv::Append(aMessage, borderAgentRloc)); } - IgnoreError(SetCommissioningData(tlvs, static_cast(length))); + SuccessOrExit(SetCommissioningData(aMessage)); state = MeshCoP::StateTlv::kAccept; exit: - if (Get().IsLeader()) { SendCommissioningSetResponse(aMessage, aMessageInfo, state); @@ -337,20 +278,18 @@ void Leader::SendCommissioningSetResponse(const Coap::Message &aRequest, const Ip6::MessageInfo &aMessageInfo, MeshCoP::StateTlv::State aState) { - Error error = kErrorNone; - Coap::Message *message; + Coap::Message *message = Get().NewPriorityResponseMessage(aRequest); - message = Get().NewPriorityResponseMessage(aRequest); - VerifyOrExit(message != nullptr, error = kErrorNoBufs); + VerifyOrExit(message != nullptr); + SuccessOrExit(Tlv::Append(*message, aState)); - SuccessOrExit(error = Tlv::Append(*message, aState)); + SuccessOrExit(Get().SendMessage(*message, aMessageInfo)); + message = nullptr; // `SendMessage` takes ownership on success - SuccessOrExit(error = Get().SendMessage(*message, aMessageInfo)); - - LogInfo("sent %s response", UriToString()); + LogInfo("Sent %s response", UriToString()); exit: - FreeMessageOnError(message, error); + FreeMessage(message); } bool Leader::RlocMatch(uint16_t aFirstRloc16, uint16_t aSecondRloc16, MatchMode aMatchMode) @@ -1362,6 +1301,75 @@ void Leader::HandleNetworkDataRestoredAfterReset(void) } } +Error Leader::UpdateCommissioningData(uint16_t aDataLength, CommissioningDataTlv *&aDataTlv) +{ + // First determine whether or not we can add Commissioning Data + // TLV with the given `aDataLength`, taking into account that we + // would remove the current Commissioning Data TLV. Then remove + // the current TLV and append a new TLV with proper size which is + // returned in `aDataTlv`. + + Error error = kErrorNone; + CommissioningDataTlv *dataTlv = FindCommissioningData(); + uint16_t insertLength; + + if (dataTlv != nullptr) + { + insertLength = (aDataLength <= dataTlv->GetLength()) ? 0 : aDataLength - dataTlv->GetLength(); + } + else + { + insertLength = sizeof(CommissioningDataTlv) + aDataLength; + } + + VerifyOrExit(CanInsert(insertLength), error = kErrorNoBufs); + + if (dataTlv != nullptr) + { + RemoveTlv(dataTlv); + } + + aDataTlv = As(AppendTlv(sizeof(CommissioningDataTlv) + aDataLength)); + + OT_ASSERT(aDataTlv != nullptr); + + aDataTlv->Init(); + aDataTlv->SetLength(static_cast(aDataLength)); + + // The caller would fill the `aDataTlv` value. + + mVersion++; + SignalNetDataChanged(); + +exit: + return error; +} + +Error Leader::SetCommissioningData(const void *aData, uint8_t aDataLength) +{ + Error error = kErrorNone; + CommissioningDataTlv *dataTlv; + + SuccessOrExit(error = UpdateCommissioningData(aDataLength, dataTlv)); + memcpy(dataTlv->GetValue(), aData, aDataLength); + +exit: + return error; +} + +Error Leader::SetCommissioningData(const Message &aMessage) +{ + Error error = kErrorNone; + uint16_t dataLength = aMessage.GetLength() - aMessage.GetOffset(); + CommissioningDataTlv *dataTlv; + + SuccessOrExit(error = UpdateCommissioningData(dataLength, dataTlv)); + aMessage.ReadBytes(aMessage.GetOffset(), dataTlv->GetValue(), dataLength); + +exit: + return error; +} + void Leader::HandleTimer(void) { if (mWaitingForNetDataSync) diff --git a/src/core/thread/network_data_leader_ftd.hpp b/src/core/thread/network_data_leader_ftd.hpp index 563acee26..6b723ac74 100644 --- a/src/core/thread/network_data_leader_ftd.hpp +++ b/src/core/thread/network_data_leader_ftd.hpp @@ -150,6 +150,18 @@ public: */ void RemoveBorderRouter(uint16_t aRloc16, MatchMode aMatchMode); + /** + * Updates Commissioning Data in Network Data. + * + * @param[in] aData A pointer to the Commissioning Data. + * @param[in] aDataLength The length of @p aData. + * + * @retval kErrorNone Successfully updated the Commissioning Data. + * @retval kErrorNoBufs Insufficient space to add the Commissioning Data. + * + */ + Error SetCommissioningData(const void *aData, uint8_t aDataLength); + /** * Synchronizes internal 6LoWPAN Context ID Set with recently obtained Thread Network Data. * @@ -329,6 +341,9 @@ private: UpdateStatus UpdateService(ServiceTlv &aService); UpdateStatus UpdateTlv(NetworkDataTlv &aTlv, const NetworkDataTlv *aSubTlvs); + Error UpdateCommissioningData(uint16_t aDataLength, CommissioningDataTlv *&aDataTlv); + Error SetCommissioningData(const Message &aMessage); + void SendCommissioningSetResponse(const Coap::Message &aRequest, const Ip6::MessageInfo &aMessageInfo, MeshCoP::StateTlv::State aState);