From 2cc0798a971164dded05c491d994f7715a477e93 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Thu, 25 Jul 2024 11:04:37 -0700 Subject: [PATCH] [border-agent] directly respond to MGMT_GET from non-active commissioner (#10524) This commit updates the `BorderAgent` to directly respond to `MGMT_ACTIVE_GET` and `MGMT_PENDING_GET` requests from a non-active commissioner. Requests from an active commissioner are still forwarded to the leader. This aligns the implementation with Thread 1.4 requirements (ephemeral PSKc use case). To achieve this, the following changes are made: - New `State` values are added to distinguish between when a commissioner candidate is connected and when its petition to become the active commissioner is accepted. This determines whether the `MGMT_GET` request should be handled directly or forwarded to the leader. - This state is tracked locally by `BorderAgent` instead of monitoring Network Data to determine whether an active commissioner exists. This ensures correct behavior even when Network Data updates are delayed. - The `DatasetManager` is updated to provide `ProcessGetRequest()` to process an `MGMT_GET` request and prepare the response. This is then used by `DatasetManager` itself and `BorderAgent`. --- src/core/api/border_agent_api.cpp | 17 +++++- src/core/meshcop/border_agent.cpp | 49 ++++++++++++++-- src/core/meshcop/border_agent.hpp | 10 ++-- src/core/meshcop/dataset_manager.cpp | 84 ++++++++++++++++------------ src/core/meshcop/dataset_manager.hpp | 26 ++++++++- 5 files changed, 138 insertions(+), 48 deletions(-) diff --git a/src/core/api/border_agent_api.cpp b/src/core/api/border_agent_api.cpp index 6a9f42f50..c6d321ee9 100644 --- a/src/core/api/border_agent_api.cpp +++ b/src/core/api/border_agent_api.cpp @@ -56,7 +56,22 @@ otError otBorderAgentSetId(otInstance *aInstance, const otBorderAgentId *aId) otBorderAgentState otBorderAgentGetState(otInstance *aInstance) { - return MapEnum(AsCoreType(aInstance).Get().GetState()); + otBorderAgentState state = OT_BORDER_AGENT_STATE_STOPPED; + + switch (AsCoreType(aInstance).Get().GetState()) + { + case MeshCoP::BorderAgent::kStateStopped: + break; + case MeshCoP::BorderAgent::kStateStarted: + state = OT_BORDER_AGENT_STATE_STARTED; + break; + case MeshCoP::BorderAgent::kStateConnected: + case MeshCoP::BorderAgent::kStateAccepted: + state = OT_BORDER_AGENT_STATE_ACTIVE; + break; + } + + return state; } uint16_t otBorderAgentGetUdpPort(otInstance *aInstance) diff --git a/src/core/meshcop/border_agent.cpp b/src/core/meshcop/border_agent.cpp index ae3cc10e1..e084fa63c 100644 --- a/src/core/meshcop/border_agent.cpp +++ b/src/core/meshcop/border_agent.cpp @@ -203,6 +203,7 @@ void BorderAgent::HandleCoapResponse(const ForwardContext &aForwardContext, Get().GetCommissionerAloc(sessionId, mCommissionerAloc.GetAddress()); Get().AddUnicastAddress(mCommissionerAloc); IgnoreError(Get().AddReceiver(mUdpReceiver)); + mState = kStateAccepted; LogInfo("Commissioner accepted - SessionId:%u ALOC:%s", sessionId, mCommissionerAloc.GetAddress().ToString().AsCString()); @@ -462,7 +463,7 @@ void BorderAgent::HandleTmf(Coap::Message &aMessage, const template <> void BorderAgent::HandleTmf(Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo) { - IgnoreError(ForwardToLeader(aMessage, aMessageInfo, kUriActiveGet)); + HandleTmfDatasetGet(aMessage, aMessageInfo, Dataset::kActive); } template <> void BorderAgent::HandleTmf(Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo) @@ -472,7 +473,7 @@ template <> void BorderAgent::HandleTmf(Coap::Message &aMessage, template <> void BorderAgent::HandleTmf(Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo) { - IgnoreError(ForwardToLeader(aMessage, aMessageInfo, kUriPendingGet)); + HandleTmfDatasetGet(aMessage, aMessageInfo, Dataset::kPending); } template <> void BorderAgent::HandleTmf(Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo) @@ -591,6 +592,45 @@ exit: return error; } +void BorderAgent::HandleTmfDatasetGet(Coap::Message &aMessage, + const Ip6::MessageInfo &aMessageInfo, + Dataset::Type aType) +{ + Error error = kErrorNone; + Coap::Message *response = nullptr; + + if (mState == kStateAccepted) + { + Uri uri = (aType == Dataset::kActive) ? kUriActiveGet : kUriPendingGet; + + IgnoreError(ForwardToLeader(aMessage, aMessageInfo, uri)); + ExitNow(); + } + + // When processing `MGMT_GET` request directly on Border Agent, + // the Security Policy flags (O-bit) should be ignore to allow + // the commissioner candidate to get the full Operational Dataset. + + if (aType == Dataset::kActive) + { + response = Get().ProcessGetRequest(aMessage, DatasetManager::kIgnoreSecurityPolicyFlags); + } + else + { + response = Get().ProcessGetRequest(aMessage, DatasetManager::kIgnoreSecurityPolicyFlags); + } + + VerifyOrExit(response != nullptr, error = kErrorParse); + + SuccessOrExit(error = Get().SendMessage(*response, aMessageInfo)); + + LogInfo("Sent %sGet response to non-active commissioner", Dataset::TypeToString(aType)); + +exit: + LogWarnOnError(error, "send Active/PendingGet response"); + FreeMessageOnError(response, error); +} + void BorderAgent::HandleConnected(bool aConnected, void *aContext) { static_cast(aContext)->HandleConnected(aConnected); @@ -601,7 +641,7 @@ void BorderAgent::HandleConnected(bool aConnected) if (aConnected) { LogInfo("Commissioner connected"); - mState = kStateActive; + mState = kStateConnected; mTimer.Start(kKeepAliveTimeout); } else @@ -766,7 +806,8 @@ void BorderAgent::ClearEphemeralKey(void) break; case kStateStopped: - case kStateActive: + case kStateConnected: + case kStateAccepted: // If there is an active commissioner connection, we wait till // it gets disconnected before removing ephemeral key and // restarting the agent. diff --git a/src/core/meshcop/border_agent.hpp b/src/core/meshcop/border_agent.hpp index 933a95c76..4e52ad179 100644 --- a/src/core/meshcop/border_agent.hpp +++ b/src/core/meshcop/border_agent.hpp @@ -46,6 +46,7 @@ #include "common/non_copyable.hpp" #include "common/notifier.hpp" #include "common/tasklet.hpp" +#include "meshcop/dataset.hpp" #include "meshcop/secure_transport.hpp" #include "net/udp6.hpp" #include "thread/tmf.hpp" @@ -94,9 +95,10 @@ public: */ enum State : uint8_t { - kStateStopped = OT_BORDER_AGENT_STATE_STOPPED, ///< Border agent is stopped/disabled. - kStateStarted = OT_BORDER_AGENT_STATE_STARTED, ///< Border agent is started. - kStateActive = OT_BORDER_AGENT_STATE_ACTIVE, ///< Border agent is connected with external commissioner. + kStateStopped, ///< Stopped/disabled. + kStateStarted, ///< Started and listening for connections. + kStateConnected, ///< Connected to an external commissioner candidate, petition pending. + kStateAccepted, ///< Connected to and accepted an external commissioner. }; /** @@ -288,6 +290,7 @@ private: template void HandleTmf(Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo); + void HandleTmfDatasetGet(Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo, Dataset::Type aType); void HandleTimeout(void); #if OPENTHREAD_CONFIG_BORDER_AGENT_EPHEMERAL_KEY_ENABLE @@ -346,7 +349,6 @@ DeclareTmfHandler(BorderAgent, kUriProxyTx); } // namespace MeshCoP -DefineMapEnum(otBorderAgentState, MeshCoP::BorderAgent::State); DefineCoreType(otBorderAgentId, MeshCoP::BorderAgent::Id); } // namespace ot diff --git a/src/core/meshcop/dataset_manager.cpp b/src/core/meshcop/dataset_manager.cpp index 3ebd90d13..a90ae2ef7 100644 --- a/src/core/meshcop/dataset_manager.cpp +++ b/src/core/meshcop/dataset_manager.cpp @@ -531,71 +531,83 @@ exit: void DatasetManager::HandleGet(const Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo) const { - TlvList tlvList; - uint8_t tlvType; - OffsetRange offsetRange; + Error error = kErrorNone; + Coap::Message *response = ProcessGetRequest(aMessage, kCheckSecurityPolicyFlags); - SuccessOrExit(Tlv::FindTlvValueOffsetRange(aMessage, Tlv::kGet, offsetRange)); + VerifyOrExit(response != nullptr); + SuccessOrExit(error = Get().SendMessage(*response, aMessageInfo)); - while (!offsetRange.IsEmpty()) - { - IgnoreError(aMessage.Read(offsetRange, tlvType)); - tlvList.Add(tlvType); - offsetRange.AdvanceOffset(sizeof(uint8_t)); - } - - // MGMT_PENDING_GET.rsp must include Delay Timer TLV (Thread 1.1.1 - // Section 8.7.5.4). - - if (!tlvList.IsEmpty() && IsPendingDataset()) - { - tlvList.Add(Tlv::kDelayTimer); - } + LogInfo("sent %s dataset get response to %s", IsActiveDataset() ? "active" : "pending", + aMessageInfo.GetPeerAddr().ToString().AsCString()); exit: - SendGetResponse(aMessage, aMessageInfo, tlvList); + FreeMessageOnError(response, error); } -void DatasetManager::SendGetResponse(const Coap::Message &aRequest, - const Ip6::MessageInfo &aMessageInfo, - const TlvList &aTlvList) const +Coap::Message *DatasetManager::ProcessGetRequest(const Coap::Message &aRequest, + SecurityPolicyCheckMode aCheckMode) const { - Error error = kErrorNone; - Coap::Message *message; + // Processes a MGMT_ACTIVE_GET or MGMT_PENDING_GET request + // and prepares the response. + + Error error = kErrorNone; + Coap::Message *response = nullptr; Dataset dataset; + TlvList tlvList; + OffsetRange offsetRange; + + if (Tlv::FindTlvValueOffsetRange(aRequest, Tlv::kGet, offsetRange) == kErrorNone) + { + while (!offsetRange.IsEmpty()) + { + uint8_t tlvType; + + IgnoreError(aRequest.Read(offsetRange, tlvType)); + tlvList.Add(tlvType); + offsetRange.AdvanceOffset(sizeof(uint8_t)); + } + + // MGMT_PENDING_GET.rsp must include Delay Timer TLV (Thread 1.1.1 + // Section 8.7.5.4). + + if (!tlvList.IsEmpty() && IsPendingDataset()) + { + tlvList.Add(Tlv::kDelayTimer); + } + } + + // Ignore `Read()` error, since even if no Dataset is saved, we should + // respond with an empty one. IgnoreError(Read(dataset)); - message = Get().NewPriorityResponseMessage(aRequest); - VerifyOrExit(message != nullptr, error = kErrorNoBufs); + response = Get().NewPriorityResponseMessage(aRequest); + VerifyOrExit(response != nullptr, error = kErrorNoBufs); for (const Tlv *tlv = dataset.GetTlvsStart(); tlv < dataset.GetTlvsEnd(); tlv = tlv->GetNext()) { bool shouldAppend = true; - if (!aTlvList.IsEmpty()) + if (!tlvList.IsEmpty()) { - shouldAppend = aTlvList.Contains(tlv->GetType()); + shouldAppend = tlvList.Contains(tlv->GetType()); } - if ((tlv->GetType() == Tlv::kNetworkKey) && !Get().GetSecurityPolicy().mObtainNetworkKeyEnabled) + if ((aCheckMode == kCheckSecurityPolicyFlags) && (tlv->GetType() == Tlv::kNetworkKey) && + !Get().GetSecurityPolicy().mObtainNetworkKeyEnabled) { shouldAppend = false; } if (shouldAppend) { - SuccessOrExit(error = tlv->AppendTo(*message)); + SuccessOrExit(error = tlv->AppendTo(*response)); } } - SuccessOrExit(error = Get().SendMessage(*message, aMessageInfo)); - - LogInfo("sent %s dataset get response to %s", IsActiveDataset() ? "active" : "pending", - aMessageInfo.GetPeerAddr().ToString().AsCString()); - exit: - FreeMessageOnError(message, error); + FreeAndNullMessageOnError(response, error); + return response; } Error DatasetManager::SendSetRequest(const Dataset::Info &aDatasetInfo, diff --git a/src/core/meshcop/dataset_manager.hpp b/src/core/meshcop/dataset_manager.hpp index 270903b8d..9c76df517 100644 --- a/src/core/meshcop/dataset_manager.hpp +++ b/src/core/meshcop/dataset_manager.hpp @@ -65,6 +65,18 @@ public: */ typedef otDatasetMgmtSetCallback MgmtSetCallback; + /** + * Indicates whether to check or ignore Security Policy flag when processing an MGMT_GET request message. + * + * This is used as input in `ProcessGetRequest(). + * + */ + enum SecurityPolicyCheckMode : uint8_t + { + kCheckSecurityPolicyFlags, ///< Check Security Policy flags. + kIgnoreSecurityPolicyFlags, ///< Ignore Security Policy flags. + }; + /** * Returns the network Timestamp. * @@ -219,6 +231,17 @@ public: uint8_t aLength, const otIp6Address *aAddress) const; + /** + * Processes a MGMT_GET request message and prepares the response. + * + * @param[in] aRequest The MGMT_GET request message. + * @param[in] aCheckMode Indicates whether to check or ignore the Security Policy flags. + * + * @returns The prepared response, or `nullptr` if fails to parse the request or cannot allocate message. + * + */ + Coap::Message *ProcessGetRequest(const Coap::Message &aRequest, SecurityPolicyCheckMode aCheckMode) const; + private: static constexpr uint8_t kMaxGetTypes = 64; // Max number of types in MGMT_GET.req static constexpr uint32_t kSendSetDelay = 5000; // in msec. @@ -279,9 +302,6 @@ private: void SignalDatasetChange(void) const; void SyncLocalWithLeader(const Dataset &aDataset); Error SendSetRequest(const Dataset &aDataset); - void SendGetResponse(const Coap::Message &aRequest, - const Ip6::MessageInfo &aMessageInfo, - const TlvList &aTlvList) const; void HandleMgmtSetResponse(Coap::Message *aMessage, const Ip6::MessageInfo *aMessageInfo, Error aError); static void HandleMgmtSetResponse(void *aContext,