diff --git a/examples/platforms/utils/mac_frame.cpp b/examples/platforms/utils/mac_frame.cpp index 09b794798..621023eff 100644 --- a/examples/platforms/utils/mac_frame.cpp +++ b/examples/platforms/utils/mac_frame.cpp @@ -216,22 +216,12 @@ bool otMacFrameIsSecurityEnabled(otRadioFrame *aFrame) bool otMacFrameIsKeyIdMode1(otRadioFrame *aFrame) { - uint8_t keyIdMode; - otError error; - - error = static_cast(aFrame)->GetKeyIdMode(keyIdMode); - - return (error == OT_ERROR_NONE) ? (keyIdMode == Mac::Frame::kKeyIdMode1) : false; + return static_cast(aFrame)->HasKeyIdMode(Mac::Frame::kKeyIdMode1); } bool otMacFrameIsKeyIdMode2(otRadioFrame *aFrame) { - uint8_t keyIdMode; - otError error; - - error = static_cast(aFrame)->GetKeyIdMode(keyIdMode); - - return (error == OT_ERROR_NONE) ? (keyIdMode == Mac::Frame::kKeyIdMode2) : false; + return static_cast(aFrame)->HasKeyIdMode(Mac::Frame::kKeyIdMode2); } uint8_t otMacFrameGetKeyId(otRadioFrame *aFrame) diff --git a/src/core/mac/mac.cpp b/src/core/mac/mac.cpp index 09e6e1a9c..931a6cd78 100644 --- a/src/core/mac/mac.cpp +++ b/src/core/mac/mac.cpp @@ -807,7 +807,7 @@ bool Mac::IsJoinable(void) const void Mac::ProcessTransmitSecurity(TxFrame &aFrame) { KeyManager &keyManager = Get(); - uint8_t keyIdMode; + Frame::KeyIdMode keyIdMode; const ExtAddress *extAddress = nullptr; VerifyOrExit(aFrame.GetSecurityEnabled()); @@ -1651,8 +1651,7 @@ Error Mac::ProcessReceiveSecurity(RxFrame &aFrame, const Address &aSrcAddr, Neig { KeyManager &keyManager = Get(); Error error = kErrorSecurity; - uint8_t securityLevel; - uint8_t keyIdMode; + Frame::KeyIdMode keyIdMode; uint32_t frameCounter; uint32_t keySequence = 0; const KeyMaterial *macKey; @@ -1660,13 +1659,12 @@ Error Mac::ProcessReceiveSecurity(RxFrame &aFrame, const Address &aSrcAddr, Neig VerifyOrExit(aFrame.GetSecurityEnabled(), error = kErrorNone); - IgnoreError(aFrame.GetSecurityLevel(securityLevel)); - VerifyOrExit(securityLevel == Frame::kSecurityEncMic32); + VerifyOrExit(aFrame.HasSecurityLevel(Frame::kSecurityEncMic32)); IgnoreError(aFrame.GetFrameCounter(frameCounter)); LogDebg("Rx security - frame counter %lu", ToUlong(frameCounter)); - IgnoreError(aFrame.GetKeyIdMode(keyIdMode)); + SuccessOrExit(aFrame.GetKeyIdMode(keyIdMode)); switch (keyIdMode) { @@ -1785,10 +1783,8 @@ exit: Error Mac::ProcessEnhAckSecurity(TxFrame &aTxFrame, RxFrame &aAckFrame) { Error error = kErrorSecurity; - uint8_t securityLevel; uint8_t txKeyIndex; uint8_t ackKeyIndex; - uint8_t keyIdMode; uint32_t frameCounter; Address srcAddr; Address dstAddr; @@ -1811,11 +1807,9 @@ Error Mac::ProcessEnhAckSecurity(TxFrame &aTxFrame, RxFrame &aAckFrame) SuccessOrExit(aAckFrame.ValidatePsdu()); - IgnoreError(aAckFrame.GetSecurityLevel(securityLevel)); - VerifyOrExit(securityLevel == Frame::kSecurityEncMic32); + VerifyOrExit(aAckFrame.HasSecurityLevel(Frame::kSecurityEncMic32)); - IgnoreError(aAckFrame.GetKeyIdMode(keyIdMode)); - VerifyOrExit(keyIdMode == Frame::kKeyIdMode1); + VerifyOrExit(aAckFrame.HasKeyIdMode(Frame::kKeyIdMode1)); IgnoreError(aTxFrame.GetKeyIndex(txKeyIndex)); IgnoreError(aAckFrame.GetKeyIndex(ackKeyIndex)); @@ -2035,11 +2029,7 @@ void Mac::HandleReceivedFrame(RxFrame *aFrame, Error aError) if (aFrame->GetSecurityEnabled()) { - uint8_t keyIdMode; - - IgnoreError(aFrame->GetKeyIdMode(keyIdMode)); - - if (keyIdMode == Frame::kKeyIdMode1) + if (aFrame->HasKeyIdMode(Frame::kKeyIdMode1)) { switch (neighbor->GetState()) { diff --git a/src/core/mac/mac_frame.cpp b/src/core/mac/mac_frame.cpp index aec9b1f7c..4ceccdbd6 100644 --- a/src/core/mac/mac_frame.cpp +++ b/src/core/mac/mac_frame.cpp @@ -206,7 +206,7 @@ void TxFrame::BuildInfo::PrepareHeadersIn(TxFrame &aTxFrame) const if (mSecurityLevel != kSecurityNone) { - uint8_t secCtl = static_cast(mSecurityLevel) | static_cast(mKeyIdMode); + uint8_t secCtl = ConstructSecurityControlField(mSecurityLevel, mKeyIdMode); IgnoreError(builder.AppendUint8(secCtl)); builder.AppendLength(CalculateSecurityHeaderSize(secCtl) - sizeof(secCtl)); @@ -604,32 +604,56 @@ exit: return index; } -Error Frame::GetSecurityLevel(uint8_t &aSecurityLevel) const +Error Frame::GetSecurityLevel(SecurityLevel &aSecurityLevel) const { Error error = kErrorNone; uint8_t index = FindSecurityHeaderIndex(); VerifyOrExit(index != kInvalidIndex, error = kErrorParse); - aSecurityLevel = mPsdu[index] & kSecLevelMask; + aSecurityLevel = ReadSecurityLevel(mPsdu[index]); exit: return error; } -Error Frame::GetKeyIdMode(uint8_t &aKeyIdMode) const +bool Frame::HasSecurityLevel(SecurityLevel aSecurityLevel) const +{ + bool has = false; + SecurityLevel securityLevel; + + SuccessOrExit(GetSecurityLevel(securityLevel)); + has = (securityLevel == aSecurityLevel); + +exit: + return has; +} + +Error Frame::GetKeyIdMode(KeyIdMode &aKeyIdMode) const { Error error = kErrorNone; uint8_t index = FindSecurityHeaderIndex(); VerifyOrExit(index != kInvalidIndex, error = kErrorParse); - aKeyIdMode = mPsdu[index] & kKeyIdModeMask; + aKeyIdMode = ReadKeyIdMode(mPsdu[index]); exit: return error; } +bool Frame::HasKeyIdMode(KeyIdMode aKeyIdMode) const +{ + bool has = false; + KeyIdMode keyIdMode; + + SuccessOrExit(GetKeyIdMode(keyIdMode)); + has = (keyIdMode == aKeyIdMode); + +exit: + return has; +} + Error Frame::GetFrameCounter(uint32_t &aFrameCounter) const { Error error = kErrorNone; @@ -673,7 +697,7 @@ uint8_t Frame::CalculateKeySourceSize(uint8_t aSecurityControl) { uint8_t size = 0; - switch (aSecurityControl & kKeyIdModeMask) + switch (ReadKeyIdMode(aSecurityControl)) { case kKeyIdMode0: size = kKeySourceSizeMode0; @@ -808,7 +832,7 @@ uint8_t Frame::CalculateMicSize(uint8_t aSecurityControl) static_assert(kMicSize[kSecurityEncMic64] == kMic64Size, "kMicSize[] array is incorrect"); static_assert(kMicSize[kSecurityEncMic128] == kMic128Size, "kMicSize[] array is incorrect"); - return kMicSize[aSecurityControl & kSecLevelMask]; + return kMicSize[ReadSecurityLevel(aSecurityControl)]; } uint16_t Frame::GetMaxPayloadLength(void) const { return GetMtu() - (GetHeaderLength() + GetFooterLength()); } @@ -870,15 +894,25 @@ uint16_t Frame::DetermineFcfAddrType(const Address &aAddress, uint16_t aBitShift return fcfAddrType; } +Frame::SecurityLevel Frame::ReadSecurityLevel(uint8_t aSecCtl) +{ + return static_cast(ReadBits(aSecCtl)); +} + +Frame::KeyIdMode Frame::ReadKeyIdMode(uint8_t aSecCtl) +{ + return static_cast(ReadBits(aSecCtl)); +} + uint8_t Frame::CalculateSecurityHeaderSize(uint8_t aSecurityControl) { uint8_t size; - VerifyOrExit((aSecurityControl & kSecLevelMask) != kSecurityNone, size = kInvalidSize); + VerifyOrExit(ReadSecurityLevel(aSecurityControl) != kSecurityNone, size = kInvalidSize); size = kSecurityControlSize + kFrameCounterSize + CalculateKeySourceSize(aSecurityControl); - if ((aSecurityControl & kKeyIdModeMask) != kKeyIdMode0) + if (ReadKeyIdMode(aSecurityControl) != kKeyIdMode0) { size += kKeyIndexSize; } @@ -1172,7 +1206,7 @@ Error TxFrame::PerformAesCcm(AesCcmOperation aOperation, const ExtAddress &aExtA Error error; uint32_t frameCounter; - uint8_t securityLevel; + SecurityLevel securityLevel; Crypto::AesCcm aesCcm; Crypto::AesCcm::Nonce nonce; @@ -1215,12 +1249,12 @@ void TxFrame::GenerateImmAck(const RxFrame &aFrame, bool aIsFramePending) #if OPENTHREAD_CONFIG_THREAD_VERSION >= OT_THREAD_VERSION_1_2 Error TxFrame::GenerateEnhAck(const RxFrame &aRxFrame, bool aIsFramePending, const uint8_t *aIeData, uint8_t aIeLength) { - Error error = kErrorNone; - BuildInfo buildInfo; - Address address; - PanId panId; - uint8_t securityLevel = kSecurityNone; - uint8_t keyIdMode = kKeyIdMode0; + Error error = kErrorNone; + BuildInfo buildInfo; + Address address; + PanId panId; + SecurityLevel securityLevel = kSecurityNone; + KeyIdMode keyIdMode = kKeyIdMode0; // Validate the received frame. @@ -1242,8 +1276,8 @@ Error TxFrame::GenerateEnhAck(const RxFrame &aRxFrame, bool aIsFramePending, con if (aRxFrame.GetSecurityEnabled()) { - SuccessOrExit(error = aRxFrame.GetSecurityLevel(securityLevel)); - VerifyOrExit(securityLevel == kSecurityEncMic32, error = kErrorParse); + VerifyOrExit(aRxFrame.HasSecurityLevel(kSecurityEncMic32), error = kErrorParse); + securityLevel = kSecurityEncMic32; SuccessOrExit(error = aRxFrame.GetKeyIdMode(keyIdMode)); } @@ -1266,8 +1300,8 @@ Error TxFrame::GenerateEnhAck(const RxFrame &aRxFrame, bool aIsFramePending, con buildInfo.mType = kTypeAck; buildInfo.mVersion = kVersion2015; - buildInfo.mSecurityLevel = static_cast(securityLevel); - buildInfo.mKeyIdMode = static_cast(keyIdMode); + buildInfo.mSecurityLevel = securityLevel; + buildInfo.mKeyIdMode = keyIdMode; buildInfo.PrepareHeadersIn(*this); @@ -1309,8 +1343,8 @@ Error TxFrame::GenerateWakeupFrame(PanId aPanId, const WakeupRequest &aWakeupReq bool RxFrame::IsSecuredWith(KeyIdModeFlags aFlags) const { - bool isSecure = false; - uint8_t keyIdMode; + bool isSecure = false; + KeyIdMode keyIdMode; VerifyOrExit(GetSecurityEnabled()); SuccessOrExit(GetKeyIdMode(keyIdMode)); @@ -1339,7 +1373,7 @@ Error RxFrame::ProcessReceiveAesCcm(const ExtAddress &aExtAddress, const KeyMate { Error error = kErrorSecurity; uint32_t frameCounter = 0; - uint8_t securityLevel; + SecurityLevel securityLevel; Crypto::AesCcm aesCcm; Crypto::AesCcm::Nonce nonce; diff --git a/src/core/mac/mac_frame.hpp b/src/core/mac/mac_frame.hpp index 8e932dcfd..4e5803133 100644 --- a/src/core/mac/mac_frame.hpp +++ b/src/core/mac/mac_frame.hpp @@ -91,7 +91,8 @@ public: /** * Represents the MAC frame security level. * - * Values match the Security Level field in Security Control Field as an `uint8_t`. + * Values represent the raw (unshifted) Security Level sub-field (3-bit wide) from the Security Control field. + * The enum covers all possible 3-bit values. */ enum SecurityLevel : uint8_t { @@ -108,14 +109,15 @@ public: /** * Represents the MAC frame security key identifier mode. * - * Values match the Key Identifier Mode field in Security Control Field as an `uint8_t`. + * Values represent the raw (unshifted) Key ID Mode sub-field (2-bit wide) from the Security Control field. + * The enum covers all possible 2-bit values. */ enum KeyIdMode : uint8_t { - kKeyIdMode0 = 0 << 3, ///< Key ID Mode 0 - Key is determined implicitly. - kKeyIdMode1 = 1 << 3, ///< Key ID Mode 1 - Key is determined from Key Index field. - kKeyIdMode2 = 2 << 3, ///< Key ID Mode 2 - Key is determined from 4-bytes Key Source and Index fields. - kKeyIdMode3 = 3 << 3, ///< Key ID Mode 3 - Key is determined from 8-bytes Key Source and Index fields. + kKeyIdMode0 = 0, ///< Key ID Mode 0 - Key is determined implicitly. + kKeyIdMode1 = 1, ///< Key ID Mode 1 - Key is determined from Key Index field. + kKeyIdMode2 = 2, ///< Key ID Mode 2 - Key is determined from 4-bytes Key Source and Index fields. + kKeyIdMode3 = 3, ///< Key ID Mode 3 - Key is determined from 8-bytes Key Source and Index fields. }; /** @@ -366,25 +368,48 @@ public: * * @param[out] aSecurityLevel The Security Level Identifier. * - * @retval kErrorNone Successfully retrieved the Security Level Identifier. + * @retval kErrorNone Successfully retrieved the Security Level Identifier. + * @retval kErrorParse Failed to parse MAC or security header. */ - Error GetSecurityLevel(uint8_t &aSecurityLevel) const; + Error GetSecurityLevel(SecurityLevel &aSecurityLevel) const; + + /** + * Indicates whether or not the frame has a specific Security Level. + * + * @param[in] aSecurityLevel The Security Level to check. + * + * @retval TRUE The frame contains a valid security header matching @p aSecurityLevel. + * @retval FALSE The frame does not match @p aSecurityLevel or fails to parse MAC or security header. + */ + bool HasSecurityLevel(SecurityLevel aSecurityLevel) const; /** * Gets the Key Identifier Mode. * * @param[out] aKeyIdMode The Key Identifier Mode. * - * @retval kErrorNone Successfully retrieved the Key Identifier Mode. + * @retval kErrorNone Successfully retrieved the Key Identifier Mode. + * @retval kErrorParse Failed to parse MAC or security header. */ - Error GetKeyIdMode(uint8_t &aKeyIdMode) const; + Error GetKeyIdMode(KeyIdMode &aKeyIdMode) const; + + /** + * Indicates whether or not the frame has a specific Key Identifier Mode. + * + * @param[in] aKeyIdMode The Key Identifier Mode to check. + * + * @retval TRUE The frame contains a valid security header matching @p aKeyIdMode. + * @retval FALSE The frame does not match @p aKeyIdMode or fails to parse MAC or security header. + */ + bool HasKeyIdMode(KeyIdMode aKeyIdMode) const; /** * Gets the Frame Counter. * * @param[out] aFrameCounter The Frame Counter. * - * @retval kErrorNone Successfully retrieved the Frame Counter. + * @retval kErrorNone Successfully retrieved the Frame Counter. + * @retval kErrorParse Failed to parse MAC or security header. */ Error GetFrameCounter(uint32_t &aFrameCounter) const; @@ -611,6 +636,20 @@ public: */ static constexpr uint8_t GetImmAckLength(void) { return kImmAckLength; } + /** + * Constructs a Security Control byte from a given Security Level and Key ID Mode. + * + * @param[in] aSecurityLevel The Security Level. + * @param[in] aKeyIdMode The Key Identifier Mode. + * + * @returns The constructed Security Control byte. + */ + static constexpr uint8_t ConstructSecurityControlField(SecurityLevel aSecurityLevel, KeyIdMode aKeyIdMode) + { + return static_cast(static_cast(aSecurityLevel) | + (static_cast(aKeyIdMode) << kScfKeyIdModeShift)); + } + protected: static constexpr uint8_t kFcfSize = sizeof(uint16_t); static constexpr uint8_t kDsnSize = sizeof(uint8_t); @@ -648,8 +687,10 @@ protected: static constexpr uint16_t kFcfSrcAddrExt = kFcfAddrExt << kFcfSrcAddrShift; static constexpr uint16_t kFcfSrcAddrMask = kFcfAddrMask << kFcfSrcAddrShift; - static constexpr uint8_t kSecLevelMask = 7 << 0; - static constexpr uint8_t kKeyIdModeMask = 3 << 3; + // Security Control field + static constexpr uint8_t kScfKeyIdModeShift = 3; + static constexpr uint8_t kScfSecLevelMask = 7 << 0; + static constexpr uint8_t kScfKeyIdModeMask = 3 << kScfKeyIdModeShift; static constexpr uint8_t kMic0Size = 0; static constexpr uint8_t kMic32Size = 32 / kBitsPerByte; @@ -695,6 +736,10 @@ protected: static uint8_t CalculateKeySourceSize(uint8_t aSecurityControl); static uint8_t CalculateMicSize(uint8_t aSecurityControl); + // Security Control fields + static SecurityLevel ReadSecurityLevel(uint8_t aSecCtl); + static KeyIdMode ReadKeyIdMode(uint8_t aSecCtl); + private: #if OPENTHREAD_CONFIG_MAC_HEADER_IE_SUPPORT typedef bool (&HeaderIeMatcher)(const HeaderIe &aHeaderIe); diff --git a/src/core/mac/sub_mac.cpp b/src/core/mac/sub_mac.cpp index 252acfa11..bb8bfbac3 100644 --- a/src/core/mac/sub_mac.cpp +++ b/src/core/mac/sub_mac.cpp @@ -337,14 +337,11 @@ exit: void SubMac::ProcessTransmitSecurity(void) { const ExtAddress *extAddress = nullptr; - uint8_t keyIdMode; uint8_t keyIndex; VerifyOrExit(mTransmitFrame.GetSecurityEnabled()); VerifyOrExit(!mTransmitFrame.IsSecurityProcessed()); - SuccessOrExit(mTransmitFrame.GetKeyIdMode(keyIdMode)); - if (!mTransmitFrame.IsHeaderUpdated()) { keyIndex = mKeyTrio.GetKeyIndex(); @@ -360,12 +357,12 @@ void SubMac::ProcessTransmitSecurity(void) #if OPENTHREAD_CONFIG_WAKEUP_COORDINATOR_ENABLE if (mTransmitFrame.GetType() == Frame::kTypeMultipurpose) { - VerifyOrExit(keyIdMode == Frame::kKeyIdMode2); + VerifyOrExit(mTransmitFrame.HasKeyIdMode(Frame::kKeyIdMode2)); } else #endif { - VerifyOrExit(keyIdMode == Frame::kKeyIdMode1); + VerifyOrExit(mTransmitFrame.HasKeyIdMode(Frame::kKeyIdMode1)); } mTransmitFrame.SetAesKey(mKeyTrio.SelectKey(keyIndex)); @@ -652,10 +649,10 @@ exit: void SubMac::SignalFrameCounterUsedOnTxDone(const TxFrame &aFrame) { - uint8_t keyIdMode; - uint8_t keyIndex; - uint32_t frameCounter; - bool allowError = false; + Frame::KeyIdMode keyIdMode; + uint8_t keyIndex; + uint32_t frameCounter; + bool allowError = false; OT_UNUSED_VARIABLE(allowError); diff --git a/src/core/radio/trel_link.cpp b/src/core/radio/trel_link.cpp index f1e90fb99..94e32d285 100644 --- a/src/core/radio/trel_link.cpp +++ b/src/core/radio/trel_link.cpp @@ -172,10 +172,7 @@ void Link::BeginTransmit(void) } else { - uint8_t keyIdMode; - - IgnoreError(mTxFrame.GetKeyIdMode(keyIdMode)); - isDiscovery = (keyIdMode == Mac::Frame::kKeyIdMode2); + isDiscovery = mTxFrame.HasKeyIdMode(Mac::Frame::kKeyIdMode2); } } diff --git a/src/core/thread/mle.hpp b/src/core/thread/mle.hpp index cc64bf886..c4fc3989f 100644 --- a/src/core/thread/mle.hpp +++ b/src/core/thread/mle.hpp @@ -1859,7 +1859,7 @@ private: private: static constexpr uint8_t kKeyIdMode2Mic32 = - static_cast(Mac::Frame::kKeyIdMode2) | static_cast(Mac::Frame::kSecurityEncMic32); + Mac::Frame::ConstructSecurityControlField(Mac::Frame::kSecurityEncMic32, Mac::Frame::kKeyIdMode2); uint8_t mSecurityControl; uint32_t mFrameCounter; diff --git a/src/posix/platform/rcp_caps_diag.cpp b/src/posix/platform/rcp_caps_diag.cpp index 5f9f87829..97baf513a 100644 --- a/src/posix/platform/rcp_caps_diag.cpp +++ b/src/posix/platform/rcp_caps_diag.cpp @@ -247,7 +247,7 @@ template <> otError RcpCapsDiag::HandleSpinelCommand otError RcpCapsDiag::HandleSpinelCommand(void) { - static constexpr uint8_t keyIdMode1 = 1 << 3; + static constexpr uint8_t keyIdMode1 = 1; static constexpr uint8_t keyIndex = 100; otMacKeyMaterial prevKey; otMacKeyMaterial curKey; diff --git a/tests/unit/test_mac_frame.cpp b/tests/unit/test_mac_frame.cpp index 455e7281c..c30313d3f 100644 --- a/tests/unit/test_mac_frame.cpp +++ b/tests/unit/test_mac_frame.cpp @@ -399,14 +399,16 @@ void TestMacHeader(void) if (frame.GetSecurityEnabled()) { - uint8_t security; - uint8_t keyIdMode; + Mac::Frame::SecurityLevel security; + Mac::Frame::KeyIdMode keyIdMode; SuccessOrQuit(frame.GetSecurityLevel(security)); VerifyOrQuit(security == testCase.mSecurity); + VerifyOrQuit(frame.HasSecurityLevel(testCase.mSecurity)); SuccessOrQuit(frame.GetKeyIdMode(keyIdMode)); VerifyOrQuit(keyIdMode == testCase.mKeyIdMode); + VerifyOrQuit(frame.HasKeyIdMode(testCase.mKeyIdMode)); } offset = snprintf(string, sizeof(string), "\nver:%s, src[addr:%s, pan:%s], dst[addr:%s, pan:%s], sec:%s",