From dd33295ce9d6a74285b7bd11248e1816e5405da1 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Wed, 6 May 2026 22:32:57 -0700 Subject: [PATCH] [mac] add `RxFrame::IsSecuredWith()` helper method (#13064) This commit introduces a new helper method, `RxFrame::IsSecuredWith()`, which allows callers to cleanly verify if a received MAC frame has security enabled and uses a specific set of allowed Key ID Modes. This eliminates redundant logic in `ThreadLinkInfo::SetFrom()`, where the code previously had to manually check `GetSecurityEnabled()`, extract the Key ID Mode, and validate it against `kKeyIdMode0` or `kKeyIdMode1`. Mac::ProcessCsl()` is updated to use this new method to cleanly enforce that CSL IE processing only occurs on frames secured with Key ID Mode 1 Crucially, this commit also updates `DataPollHandler::HandleDataPoll()` to use this new helper. Previously, it only checked if the frame was secured (`GetSecurityEnabled()`), which would accept frames using any Key ID Mode (including mode 2 with fixed/known keys). By restricting the data poll handling to only accept Key ID Mode 1, we ensure that data polls are only processed if they are secured with a valid Thread network key. --- src/core/mac/data_poll_handler.cpp | 3 ++- src/core/mac/mac.cpp | 8 ++------ src/core/mac/mac_frame.cpp | 26 ++++++++++++++++++++++++++ src/core/mac/mac_frame.hpp | 24 ++++++++++++++++++++++++ src/core/thread/thread_link_info.cpp | 22 ++++------------------ 5 files changed, 58 insertions(+), 25 deletions(-) diff --git a/src/core/mac/data_poll_handler.cpp b/src/core/mac/data_poll_handler.cpp index e567daf9d..0e78d6367 100644 --- a/src/core/mac/data_poll_handler.cpp +++ b/src/core/mac/data_poll_handler.cpp @@ -89,7 +89,8 @@ void DataPollHandler::HandleDataPoll(Mac::RxFrame &aFrame) Child *child; uint16_t indirectMsgCount; - VerifyOrExit(aFrame.GetSecurityEnabled()); + VerifyOrExit(aFrame.IsSecuredWith(Mac::RxFrame::kAllowKeyIdMode1)); + VerifyOrExit(!Get().IsDetached()); SuccessOrExit(aFrame.GetSrcAddr(macSource)); diff --git a/src/core/mac/mac.cpp b/src/core/mac/mac.cpp index 3c5accc2b..ca1a9c648 100644 --- a/src/core/mac/mac.cpp +++ b/src/core/mac/mac.cpp @@ -2491,12 +2491,8 @@ void Mac::ProcessCsl(const RxFrame &aFrame, const Address &aSrcAddr) CslNeighbor *neighbor = nullptr; const CslIe *csl; - uint8_t keyIdMode; - - VerifyOrExit(aFrame.IsVersion2015() && aFrame.GetSecurityEnabled()); - - IgnoreError(aFrame.GetKeyIdMode(keyIdMode)); - VerifyOrExit(keyIdMode == Frame::kKeyIdMode1); + VerifyOrExit(aFrame.IsVersion2015()); + VerifyOrExit(aFrame.IsSecuredWith(RxFrame::kAllowKeyIdMode1)); csl = aFrame.GetCslIe(); VerifyOrExit(csl != nullptr); diff --git a/src/core/mac/mac_frame.cpp b/src/core/mac/mac_frame.cpp index 17d2a1bdd..10de7c873 100644 --- a/src/core/mac/mac_frame.cpp +++ b/src/core/mac/mac_frame.cpp @@ -1561,6 +1561,32 @@ exit: } #endif // OPENTHREAD_CONFIG_WAKEUP_COORDINATOR_ENABLE +bool RxFrame::IsSecuredWith(KeyIdModeFlags aFlags) const +{ + bool isSecure = false; + uint8_t keyIdMode; + + VerifyOrExit(GetSecurityEnabled()); + SuccessOrExit(GetKeyIdMode(keyIdMode)); + + switch (keyIdMode) + { + case kKeyIdMode0: + VerifyOrExit(aFlags & kAllowKeyIdMode0); + break; + case kKeyIdMode1: + VerifyOrExit(aFlags & kAllowKeyIdMode1); + break; + default: + ExitNow(); + } + + isSecure = true; + +exit: + return isSecure; +} + Error RxFrame::ProcessReceiveAesCcm(const ExtAddress &aExtAddress, const KeyMaterial &aMacKey) { #if OPENTHREAD_FTD || OPENTHREAD_MTD diff --git a/src/core/mac/mac_frame.hpp b/src/core/mac/mac_frame.hpp index 0e55fb091..7f44be404 100644 --- a/src/core/mac/mac_frame.hpp +++ b/src/core/mac/mac_frame.hpp @@ -933,6 +933,30 @@ class RxFrame : public Frame public: friend class TxFrame; + /** + * Defines flags to indicate allowed Key ID Modes, used in `IsSecuredWith()`. + */ + enum KeyIdModeFlag : uint8_t + { + kAllowKeyIdMode0 = (1 << 0), ///< Allow Key ID Mode 0. + kAllowKeyIdMode1 = (1 << 1), ///< Allow Key ID Mode 1. + }; + + /** + * Represents a set of `KeyIdModeFlag`s. + */ + typedef uint8_t KeyIdModeFlags; + + /** + * Indicates whether the frame is secured with a given set of allowed Key ID Modes. + * + * @param[in] aFlags A bitmask of `KeyIdModeFlags` specifying the allowed modes. + * + * @retval TRUE The frame has security enabled and uses one of the allowed Key ID Modes. + * @retval FALSE The frame does not have security enabled, or its Key ID Mode is not allowed. + */ + bool IsSecuredWith(KeyIdModeFlags aFlags) const; + /** * Returns the RSSI in dBm used for reception. * diff --git a/src/core/thread/thread_link_info.cpp b/src/core/thread/thread_link_info.cpp index 9b1bf921b..762a7a5dd 100644 --- a/src/core/thread/thread_link_info.cpp +++ b/src/core/thread/thread_link_info.cpp @@ -55,24 +55,10 @@ void ThreadLinkInfo::SetFrom(const Mac::RxFrame &aFrame) mIsDstPanIdBroadcast = (dstPanId == Mac::kPanIdBroadcast); } - if (aFrame.GetSecurityEnabled()) - { - uint8_t keyIdMode; - - // MAC Frame Security was already validated at the MAC - // layer. As a result, `GetKeyIdMode()` will never return - // failure here. - IgnoreError(aFrame.GetKeyIdMode(keyIdMode)); - - mLinkSecurity = (keyIdMode == Mac::Frame::kKeyIdMode0) || (keyIdMode == Mac::Frame::kKeyIdMode1); - } - else - { - mLinkSecurity = false; - } - mChannel = aFrame.GetChannel(); - mRss = aFrame.GetRssi(); - mLqi = aFrame.GetLqi(); + mLinkSecurity = aFrame.IsSecuredWith(Mac::RxFrame::kAllowKeyIdMode0 | Mac::RxFrame::kAllowKeyIdMode1); + mChannel = aFrame.GetChannel(); + mRss = aFrame.GetRssi(); + mLqi = aFrame.GetLqi(); #if OPENTHREAD_CONFIG_TIME_SYNC_ENABLE if (aFrame.GetTimeIe() != nullptr) {