espressif/openthread
1
0
Fork 0
mirror of https://github.com/espressif/openthread.git synced 2026-10-26 17:24:00 +00:00
Code Issues Packages Projects Releases Wiki Activity

[mac-frame] improve type safety and helpers for MAC security fields (#13436)

Browse Source 
This commit updates `Mac::Frame` security level and key ID mode helpers.

Specifically:
- Refactors `KeyIdMode` enum values to use raw (unshifted) 2-bit values
  (`0, 1, 2, 3`), matching the structure of `SecurityLevel`.
- Updates `GetSecurityLevel()` and `GetKeyIdMode()` to use strongly-typed
  `SecurityLevel` and `KeyIdMode` parameters.
- Introduces `HasSecurityLevel()` and `HasKeyIdMode()` predicate methods to
  simplify security checks across `Mac`, `SubMac`, and `TrelLink`.
- Adds `ReadSecurityLevel()` and `ReadKeyIdMode()` internal static helpers.
This commit is contained in:
Abtin Keshavarzian
2026-07-30 14:08:00 -07:00
committed by GitHub
parent 8cb9a57bbc
commit 5a39ea6238
9 changed files with 137 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files
examples/platforms/utils/mac_frame.cpp
+2 -12
View File
@@ -216,22 +216,12 @@ bool otMacFrameIsSecurityEnabled(otRadioFrame *aFrame)
bool otMacFrameIsKeyIdMode1(otRadioFrame *aFrame)
{
uint8_t keyIdMode;
otError error;
error = static_cast<const Mac::Frame *>(aFrame)->GetKeyIdMode(keyIdMode);
return (error == OT_ERROR_NONE) ? (keyIdMode == Mac::Frame::kKeyIdMode1) : false;
return static_cast<const Mac::Frame *>(aFrame)->HasKeyIdMode(Mac::Frame::kKeyIdMode1);
}
bool otMacFrameIsKeyIdMode2(otRadioFrame *aFrame)
{
uint8_t keyIdMode;
otError error;
error = static_cast<const Mac::Frame *>(aFrame)->GetKeyIdMode(keyIdMode);
return (error == OT_ERROR_NONE) ? (keyIdMode == Mac::Frame::kKeyIdMode2) : false;
return static_cast<const Mac::Frame *>(aFrame)->HasKeyIdMode(Mac::Frame::kKeyIdMode2);
}
uint8_t otMacFrameGetKeyId(otRadioFrame *aFrame)
src/core/mac/mac.cpp
+7 -17
View File
@@ -807,7 +807,7 @@ bool Mac::IsJoinable(void) const
void Mac::ProcessTransmitSecurity(TxFrame &aFrame)
{
KeyManager &keyManager = Get<KeyManager>();
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<KeyManager>();
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())
{
src/core/mac/mac_frame.cpp
+57 -23
View File
@@ -206,7 +206,7 @@ void TxFrame::BuildInfo::PrepareHeadersIn(TxFrame &aTxFrame) const
if (mSecurityLevel != kSecurityNone)
{
uint8_t secCtl = static_cast<uint8_t>(mSecurityLevel) | static_cast<uint8_t>(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<SecurityLevel>(ReadBits<uint8_t, kScfSecLevelMask>(aSecCtl));
}
Frame::KeyIdMode Frame::ReadKeyIdMode(uint8_t aSecCtl)
{
return static_cast<KeyIdMode>(ReadBits<uint8_t, kScfKeyIdModeMask>(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>(securityLevel);
buildInfo.mKeyIdMode = static_cast<KeyIdMode>(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;
src/core/mac/mac_frame.hpp
+58 -13
View File
@@ -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<uint8_t>(static_cast<uint8_t>(aSecurityLevel) |
(static_cast<uint8_t>(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);
src/core/mac/sub_mac.cpp
+6 -9
View File
@@ -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);
src/core/radio/trel_link.cpp
+1 -4
View File
@@ -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);
}
}
src/core/thread/mle.hpp
+1 -1
View File
@@ -1859,7 +1859,7 @@ private:
private:
static constexpr uint8_t kKeyIdMode2Mic32 =
static_cast<uint8_t>(Mac::Frame::kKeyIdMode2) | static_cast<uint8_t>(Mac::Frame::kSecurityEncMic32);
Mac::Frame::ConstructSecurityControlField(Mac::Frame::kSecurityEncMic32, Mac::Frame::kKeyIdMode2);
uint8_t mSecurityControl;
uint32_t mFrameCounter;
src/posix/platform/rcp_caps_diag.cpp
+1 -1
View File
@@ -247,7 +247,7 @@ template <> otError RcpCapsDiag::HandleSpinelCommand<SPINEL_CMD_PROP_VALUE_SET,
template <> otError RcpCapsDiag::HandleSpinelCommand<SPINEL_CMD_PROP_VALUE_SET, SPINEL_PROP_RCP_MAC_KEY>(void)
{
static constexpr uint8_t keyIdMode1 = 1 << 3;
static constexpr uint8_t keyIdMode1 = 1;
static constexpr uint8_t keyIndex = 100;
otMacKeyMaterial prevKey;
otMacKeyMaterial curKey;
tests/unit/test_mac_frame.cpp
+4 -2
View File
@@ -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",