From 973680045acd578c75f5ae7c8639555c1d1b54bb Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 18 Nov 2025 22:24:39 -0800 Subject: [PATCH] [border-agent] centralize active commissioner session management (#12160) This change moves the state and logic for managing the active commissioner from `CoapDtlsSession` to the `BorderAgent::Manager` class. Previously, each `CoapDtlsSession` instance tracked whether it was the active commissioner using its own `mIsActiveCommissioner` flag, and managed its own `mCommissionerAloc` and `mUdpReceiver`. This commit introduces a single `mCommissionerSession` pointer in the `Manager` to act as the sole source of truth for the currently active commissioner. The responsibility for managing the commissioner ALOC and the associated UDP receiver is also moved to the `Manager`. This approach reinforces the Thread mesh rule of a single active commissioner at a time and simplifies the code by centralizing ALOC and UDP receiver management in one place. --- src/core/meshcop/border_agent.cpp | 162 +++++++++++++++--------------- src/core/meshcop/border_agent.hpp | 33 +++--- 2 files changed, 102 insertions(+), 93 deletions(-) diff --git a/src/core/meshcop/border_agent.cpp b/src/core/meshcop/border_agent.cpp index 01190fd16..3cd750598 100644 --- a/src/core/meshcop/border_agent.cpp +++ b/src/core/meshcop/border_agent.cpp @@ -57,10 +57,14 @@ Manager::Manager(Instance &aInstance) , mEnabled(true) , mIsRunning(false) , mDtlsTransport(aInstance, kNoLinkSecurity) + , mCommissionerSession(nullptr) + , mCommissionerUdpReceiver(HandleUdpReceive, this) #if OPENTHREAD_CONFIG_BORDER_AGENT_ID_ENABLE , mIdInitialized(false) #endif { + mCommissionerAloc.InitAsThreadOriginMeshLocal(); + ClearAllBytes(mCounters); #if OPENTHREAD_CONFIG_BORDER_AGENT_MESHCOP_SERVICE_ENABLE @@ -179,6 +183,11 @@ void Manager::Stop(void) { VerifyOrExit(mIsRunning); + if (mCommissionerSession != nullptr) + { + RevokeRoleIfActiveCommissioner(*mCommissionerSession); + } + mDtlsTransport.Close(); mIsRunning = false; @@ -262,7 +271,7 @@ void Manager::HandleSessionConnected(CoapDtlsSession &aSession) void Manager::HandleSessionDisconnected(CoapDtlsSession &aSession, CoapDtlsSession::ConnectEvent aEvent) { - OT_UNUSED_VARIABLE(aSession); + RevokeRoleIfActiveCommissioner(aSession); #if OPENTHREAD_CONFIG_BORDER_AGENT_EPHEMERAL_KEY_ENABLE if (Get().OwnsSession(aSession)) @@ -279,9 +288,22 @@ void Manager::HandleSessionDisconnected(CoapDtlsSession &aSession, CoapDtlsSessi } } -void Manager::HandleCommissionerPetitionAccepted(CoapDtlsSession &aSession) +void Manager::HandleCommissionerPetitionAccepted(CoapDtlsSession &aSession, uint16_t aSessionId) { - OT_UNUSED_VARIABLE(aSession); + if (mCommissionerSession != nullptr) + { + RevokeRoleIfActiveCommissioner(*mCommissionerSession); + } + + mCommissionerSession = &aSession; + + Get().GetCommissionerAloc(aSessionId, mCommissionerAloc.GetAddress()); + Get().AddUnicastAddress(mCommissionerAloc); + + IgnoreError(Get().AddReceiver(mCommissionerUdpReceiver)); + + LogInfo("Commissioner accepted - SessionId:%u ALOC:%s", aSessionId, + mCommissionerAloc.GetAddress().ToString().AsCString()); #if OPENTHREAD_CONFIG_BORDER_AGENT_EPHEMERAL_KEY_ENABLE if (Get().OwnsSession(aSession)) @@ -295,30 +317,38 @@ void Manager::HandleCommissionerPetitionAccepted(CoapDtlsSession &aSession) } } -Manager::CoapDtlsSession *Manager::FindActiveCommissionerSession(void) +void Manager::RevokeRoleIfActiveCommissioner(CoapDtlsSession &aSession) { - CoapDtlsSession *commissionerSession = nullptr; + VerifyOrExit(IsCommissionerSession(aSession)); - for (SecureSession &session : mDtlsTransport.GetSessions()) - { - CoapDtlsSession &coapSession = static_cast(session); + LogInfo("Commissioner role revoked"); - if (coapSession.IsActiveCommissioner()) - { - commissionerSession = &coapSession; - break; - } - } + IgnoreError(Get().RemoveReceiver(mCommissionerUdpReceiver)); + Get().RemoveUnicastAddress(mCommissionerAloc); -#if OPENTHREAD_CONFIG_BORDER_AGENT_EPHEMERAL_KEY_ENABLE - if ((Get().mCoapDtlsSession != nullptr) && - Get().mCoapDtlsSession->IsActiveCommissioner()) - { - commissionerSession = Get().mCoapDtlsSession; - } -#endif + mCommissionerSession = nullptr; - return commissionerSession; +exit: + return; +} + +bool Manager::HandleUdpReceive(void *aContext, const otMessage *aMessage, const otMessageInfo *aMessageInfo) +{ + return static_cast(aContext)->HandleUdpReceive(AsCoreType(aMessage), AsCoreType(aMessageInfo)); +} + +bool Manager::HandleUdpReceive(const Message &aMessage, const Ip6::MessageInfo &aMessageInfo) +{ + bool didHandle = false; + + VerifyOrExit(mCommissionerSession != nullptr); + VerifyOrExit(aMessageInfo.GetSockAddr() == mCommissionerAloc.GetAddress()); + + mCommissionerSession->ForwardUdpProxyToCommissioner(aMessage, aMessageInfo); + didHandle = true; + +exit: + return didHandle; } template <> void Manager::HandleTmf(Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo) @@ -327,22 +357,12 @@ template <> void Manager::HandleTmf(Coap::Message &aMessage, const OT_UNUSED_VARIABLE(aMessageInfo); - OwnedPtr forwardMessage; - CoapDtlsSession *session; - VerifyOrExit(mIsRunning); VerifyOrExit(aMessage.IsNonConfirmablePostRequest()); - session = FindActiveCommissionerSession(); - VerifyOrExit(session != nullptr); - - forwardMessage.Reset(session->NewPriorityNonConfirmablePostMessage(kUriRelayRx)); - VerifyOrExit(forwardMessage != nullptr); - - SuccessOrExit(session->ForwardToCommissioner(forwardMessage.PassOwnership(), aMessage)); - - LogInfo("Sent to commissioner on RelayRx (c/rx)"); + VerifyOrExit(mCommissionerSession != nullptr); + mCommissionerSession->ForwardUdpRelayToCommissioner(aMessage); exit: return; @@ -483,13 +503,9 @@ exit: Manager::CoapDtlsSession::CoapDtlsSession(Instance &aInstance, Dtls::Transport &aDtlsTransport) : Coap::SecureSession(aInstance, aDtlsTransport) - , mIsActiveCommissioner(false) , mTimer(aInstance, HandleTimer, this) - , mUdpReceiver(HandleUdpReceive, this) , mAllocationTime(aInstance.Get().GetUptime()) { - mCommissionerAloc.InitAsThreadOriginMeshLocal(); - SetResourceHandler(&HandleResource); SetConnectCallback(&HandleConnected, this); } @@ -506,6 +522,8 @@ exit: return error; } +bool Manager::CoapDtlsSession::IsActiveCommissioner(void) const { return Get().IsCommissionerSession(*this); } + void Manager::CoapDtlsSession::Cleanup(void) { while (!mForwardContexts.IsEmpty()) @@ -516,8 +534,8 @@ void Manager::CoapDtlsSession::Cleanup(void) } mTimer.Stop(); - IgnoreError(Get().RemoveReceiver(mUdpReceiver)); - Get().RemoveUnicastAddress(mCommissionerAloc); + + Get().RevokeRoleIfActiveCommissioner(*this); Coap::SecureSession::Cleanup(); } @@ -580,9 +598,6 @@ void Manager::CoapDtlsSession::HandleConnected(ConnectEvent aEvent) else { LogInfo("SecureSession disconnected"); - IgnoreError(Get().RemoveReceiver(mUdpReceiver)); - Get().RemoveUnicastAddress(mCommissionerAloc); - Get().HandleSessionDisconnected(*this, aEvent); } } @@ -590,7 +605,8 @@ void Manager::CoapDtlsSession::HandleConnected(ConnectEvent aEvent) void Manager::CoapDtlsSession::HandleTmfCommissionerKeepAlive(Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo) { - VerifyOrExit(mIsActiveCommissioner); + VerifyOrExit(IsActiveCommissioner()); + SuccessOrExit(ForwardToLeader(aMessage, aMessageInfo, kUriLeaderKeepAlive)); mTimer.Start(kKeepAliveTimeout); #if OPENTHREAD_CONFIG_BORDER_AGENT_EPHEMERAL_KEY_ENABLE && OPENTHREAD_CONFIG_HISTORY_TRACKER_ENABLE @@ -701,29 +717,13 @@ void Manager::CoapDtlsSession::HandleLeaderResponseToFwdTmf(const ForwardContext uint16_t sessionId; SuccessOrExit(error = Tlv::Find(*aResponse, sessionId)); - - Get().GetCommissionerAloc(sessionId, mCommissionerAloc.GetAddress()); - Get().AddUnicastAddress(mCommissionerAloc); - IgnoreError(Get().AddReceiver(mUdpReceiver)); - mIsActiveCommissioner = true; - Get().HandleCommissionerPetitionAccepted(*this); - - LogInfo("Commissioner accepted - SessionId:%u ALOC:%s", sessionId, - mCommissionerAloc.GetAddress().ToString().AsCString()); + Get().HandleCommissionerPetitionAccepted(*this, sessionId); } break; case StateTlv::kReject: - LogInfo("Commissioner rejected"); - - if (mIsActiveCommissioner) - { - IgnoreError(Get().RemoveReceiver(mUdpReceiver)); - Get().RemoveUnicastAddress(mCommissionerAloc); - mIsActiveCommissioner = false; - } - + Get().RevokeRoleIfActiveCommissioner(*this); break; default: @@ -751,26 +751,15 @@ exit: } } -bool Manager::CoapDtlsSession::HandleUdpReceive(void *aContext, - const otMessage *aMessage, - const otMessageInfo *aMessageInfo) -{ - return static_cast(aContext)->HandleUdpReceive(AsCoreType(aMessage), AsCoreType(aMessageInfo)); -} - -bool Manager::CoapDtlsSession::HandleUdpReceive(const Message &aMessage, const Ip6::MessageInfo &aMessageInfo) +void Manager::CoapDtlsSession::ForwardUdpProxyToCommissioner(const Message &aMessage, + const Ip6::MessageInfo &aMessageInfo) { Error error = kErrorNone; OwnedPtr message; - bool didHandle = false; ExtendedTlv extTlv; UdpEncapsulationTlvHeader udpEncapHeader; OffsetRange offsetRange; - VerifyOrExit(aMessageInfo.GetSockAddr() == mCommissionerAloc.GetAddress()); - - didHandle = true; - VerifyOrExit(aMessage.GetLength() > 0); message.Reset(NewPriorityNonConfirmablePostMessage(kUriProxyRx)); @@ -796,8 +785,22 @@ bool Manager::CoapDtlsSession::HandleUdpReceive(const Message &aMessage, const I exit: LogWarnOnError(error, "send ProxyRx (c/ur)"); +} - return didHandle; +void Manager::CoapDtlsSession::ForwardUdpRelayToCommissioner(const Message &aMessage) +{ + OwnedPtr forwardMessage; + Error error = kErrorNone; + + forwardMessage.Reset(NewPriorityNonConfirmablePostMessage(kUriRelayRx)); + VerifyOrExit(forwardMessage != nullptr, error = kErrorNoBufs); + + SuccessOrExit(error = ForwardToCommissioner(forwardMessage.PassOwnership(), aMessage)); + + LogInfo("Sent RelayRx (c/rx) to commissioner"); + +exit: + LogWarnOnError(error, "send RelayRx (c/rx)"); } Error Manager::CoapDtlsSession::ForwardToCommissioner(OwnedPtr aForwardMessage, const Message &aMessage) @@ -808,12 +811,9 @@ Error Manager::CoapDtlsSession::ForwardToCommissioner(OwnedPtr aF offsetRange.InitFromMessageOffsetToEnd(aMessage); SuccessOrExit(error = aForwardMessage->AppendBytesFromMessage(aMessage, offsetRange)); - SuccessOrExit(error = SendMessage(aForwardMessage.PassOwnership())); - - LogInfo("Sent to commissioner"); + error = SendMessage(aForwardMessage.PassOwnership()); exit: - LogWarnOnError(error, "send to commissioner"); return error; } @@ -860,7 +860,7 @@ void Manager::CoapDtlsSession::HandleTmfProxyTx(Coap::Message &aMessage) SuccessOrExit(error = message->AppendBytesFromMessage(aMessage, offsetRange)); messageInfo.SetSockPort(udpEncapHeader.GetSourcePort()); - messageInfo.SetSockAddr(mCommissionerAloc.GetAddress()); + messageInfo.SetSockAddr(Get().GetCommissionerAloc()); messageInfo.SetPeerPort(udpEncapHeader.GetDestinationPort()); SuccessOrExit(error = Tlv::Find(aMessage, messageInfo.GetPeerAddr())); diff --git a/src/core/meshcop/border_agent.hpp b/src/core/meshcop/border_agent.hpp index 3e29111bf..dacca854e 100644 --- a/src/core/meshcop/border_agent.hpp +++ b/src/core/meshcop/border_agent.hpp @@ -256,9 +256,10 @@ private: public: Error SendMessage(OwnedPtr aMessage); - Error ForwardToCommissioner(OwnedPtr aForwardMessage, const Message &aMessage); + void ForwardUdpProxyToCommissioner(const Message &aMessage, const Ip6::MessageInfo &aMessageInfo); + void ForwardUdpRelayToCommissioner(const Message &aMessage); void Cleanup(void); - bool IsActiveCommissioner(void) const { return mIsActiveCommissioner; } + bool IsActiveCommissioner(void) const; uint64_t GetAllocationTime(void) const { return mAllocationTime; } private: @@ -277,6 +278,7 @@ private: CoapDtlsSession(Instance &aInstance, Dtls::Transport &aDtlsTransport); + Error ForwardToCommissioner(OwnedPtr aForwardMessage, const Message &aMessage); void HandleTmfCommissionerKeepAlive(Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo); void HandleTmfRelayTx(Coap::Message &aMessage); void HandleTmfProxyTx(Coap::Message &aMessage); @@ -293,8 +295,6 @@ private: void HandleLeaderResponseToFwdTmf(const ForwardContext &aForwardContext, const Coap::Message *aResponse, Error aResult); - static bool HandleUdpReceive(void *aContext, const otMessage *aMessage, const otMessageInfo *aMessageInfo); - bool HandleUdpReceive(const Message &aMessage, const Ip6::MessageInfo &aMessageInfo); static bool HandleResource(CoapBase &aCoapBase, const char *aUriPath, Coap::Message &aMessage, @@ -303,11 +303,8 @@ private: static void HandleTimer(Timer &aTimer); void HandleTimer(void); - bool mIsActiveCommissioner; LinkedList mForwardContexts; TimerMilliContext mTimer; - Ip6::Udp::Receiver mUdpReceiver; - Ip6::Netif::UnicastAddress mCommissionerAloc; uint64_t mAllocationTime; }; @@ -320,15 +317,23 @@ private: template void HandleTmf(Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo); + // Callbacks used with `Dtls::Transport`. static SecureSession *HandleAcceptSession(void *aContext, const Ip6::MessageInfo &aMessageInfo); CoapDtlsSession *HandleAcceptSession(void); static void HandleRemoveSession(void *aContext, SecureSession &aSession); void HandleRemoveSession(SecureSession &aSession); - CoapDtlsSession *FindActiveCommissionerSession(void); + const Ip6::Address &GetCommissionerAloc(void) const { return mCommissionerAloc.GetAddress(); } + CoapDtlsSession *GetCommissionerSession(void) { return mCommissionerSession; } + + bool IsCommissionerSession(const CoapDtlsSession &aSession) const { return mCommissionerSession == &aSession; } void HandleSessionConnected(CoapDtlsSession &aSession); void HandleSessionDisconnected(CoapDtlsSession &aSession, CoapDtlsSession::ConnectEvent aEvent); - void HandleCommissionerPetitionAccepted(CoapDtlsSession &aSession); + void HandleCommissionerPetitionAccepted(CoapDtlsSession &aSession, uint16_t aSessionId); + void RevokeRoleIfActiveCommissioner(CoapDtlsSession &aSession); + + static bool HandleUdpReceive(void *aContext, const otMessage *aMessage, const otMessageInfo *aMessageInfo); + bool HandleUdpReceive(const Message &aMessage, const Ip6::MessageInfo &aMessageInfo); #if OPENTHREAD_CONFIG_BORDER_AGENT_MESHCOP_SERVICE_ENABLE // Callback from `BorderAgent::TxtData`. @@ -349,9 +354,13 @@ private: static const char kDefaultBaseServiceName[]; #endif - bool mEnabled; - bool mIsRunning; - Dtls::Transport mDtlsTransport; + bool mEnabled; + bool mIsRunning; + Dtls::Transport mDtlsTransport; + CoapDtlsSession *mCommissionerSession; + Ip6::Udp::Receiver mCommissionerUdpReceiver; + Ip6::Netif::UnicastAddress mCommissionerAloc; + #if OPENTHREAD_CONFIG_BORDER_AGENT_ID_ENABLE Id mId; bool mIdInitialized;