[mle] validate CSL Channel TLV in HandleChildUpdateRequestOnParent() (#13541)

This commit adds validation of the CSL Channel TLV received in a Child
Update Request on the parent.

Previously, the 16-bit channel value from the TLV was truncated to
`uint8_t` and stored on the `Child` without any range or channel page
check, and then used as the radio channel for every CSL transmission
to that child. This differs from the local `otLinkSetCslChannel()`
path, which validates the channel through `Radio::IsCslChannelValid()`.

Key changes:
- Adds `CslChannelTlvValue`, derived from `ChannelTlvValue`, whose
  `IsValid()` also accepts the special value of zero (CSL channel not
  specified), and uses it in the `CslChannelTlv` definition so the
  zero-or-valid check is encapsulated in the value type.
- Updates `Mle::HandleChildUpdateRequestOnParent()` to switch on the
  result of `Tlv::Find<CslChannelTlv>()`, consistent with how the
  Timeout and Supervision Interval TLVs are parsed, rejecting the
  request with `kErrorParse` if the TLV is malformed or the channel
  is not valid.
- Adds a unit test in `tests/unit/test_mle.cpp` that drives the handler
  with crafted Child Update Requests covering zero, the channel range
  boundaries, out-of-range values, and a value that would alias a
  valid channel after truncation to `uint8_t`.
This commit is contained in:
soma0212
2026-09-08 14:29:11 -07:00
committed by GitHub
parent 5560602b10
commit f34c5e5476
5 changed files with 153 additions and 7 deletions
+1 -1
View File
@@ -3852,7 +3852,7 @@ Error Mle::TxMessage::AppendCslChannelTlv(void)
// CSL channel value of zero indicates that the CSL channel is not
// specified. We can use this value in the TLV as well.
return Tlv::Append<CslChannelTlv>(*this, ChannelTlvValue(Get<Mac::Mac>().GetCslChannel()));
return Tlv::Append<CslChannelTlv>(*this, CslChannelTlvValue(Get<Mac::Mac>().GetCslChannel()));
}
Error Mle::TxMessage::AppendCslTimeoutTlv(void)
+10 -5
View File
@@ -2342,8 +2342,8 @@ void Mle::HandleChildUpdateRequestOnParent(RxInfo &aRxInfo)
#if OPENTHREAD_CONFIG_MAC_CSL_TRANSMITTER_ENABLE
if (child->IsCslSynchronized())
{
ChannelTlvValue cslChannelTlvValue;
uint32_t cslTimeout;
CslChannelTlvValue cslChannelTlvValue;
uint32_t cslTimeout;
switch (Tlv::Find<CslTimeoutTlv>(aRxInfo.mMessage, cslTimeout))
{
@@ -2358,11 +2358,16 @@ void Mle::HandleChildUpdateRequestOnParent(RxInfo &aRxInfo)
ExitNow(error = kErrorNone);
}
if (Tlv::Find<CslChannelTlv>(aRxInfo.mMessage, cslChannelTlvValue) == kErrorNone)
switch (Tlv::Find<CslChannelTlv>(aRxInfo.mMessage, cslChannelTlvValue))
{
// Special value of zero is used to indicate that
// CSL channel is not specified.
case kErrorNone:
VerifyOrExit(cslChannelTlvValue.IsValid(), error = kErrorParse);
child->SetCslChannel(static_cast<uint8_t>(cslChannelTlvValue.GetChannel()));
break;
case kErrorNotFound:
break;
default:
ExitNow(error = kErrorParse);
}
}
#endif // OPENTHREAD_CONFIG_MAC_CSL_TRANSMITTER_ENABLE
+11
View File
@@ -292,6 +292,17 @@ exit:
return isValid;
}
//---------------------------------------------------------------------------------------------------------------------
// CslChannelTlvValue
bool CslChannelTlvValue::IsValid(void) const
{
// Special value of zero is used to indicate that
// CSL channel is not specified.
return (GetChannel() == 0) || ChannelTlvValue::IsValid();
}
//---------------------------------------------------------------------------------------------------------------------
// LeaderDataTlvValue
+32 -1
View File
@@ -703,10 +703,41 @@ private:
*/
typedef SimpleTlvInfo<Tlv::kChannel, ChannelTlvValue> ChannelTlv;
/**
* Implements CSL Channel TLV value format.
*/
OT_TOOL_PACKED_BEGIN
class CslChannelTlvValue : public ChannelTlvValue
{
public:
/**
* Default constructor.
*/
CslChannelTlvValue(void) = default;
/**
* Initializes the `CslChannelTlvValue` with zero channel page and a given channel value.
*
* @param[in] aChannel The channel.
*/
CslChannelTlvValue(uint16_t aChannel)
: ChannelTlvValue(aChannel)
{
}
/**
* Indicates whether or not the CSL Channel and Channel Page values are valid.
*
* @retval TRUE If the Channel and Channel Page values are valid.
* @retval FALSE If the Channel and Channel Page values are not valid.
*/
bool IsValid(void) const;
} OT_TOOL_PACKED_END;
/**
* Defines CSL Channel TLV constants and types.
*/
typedef SimpleTlvInfo<Tlv::kCslChannel, ChannelTlvValue> CslChannelTlv;
typedef SimpleTlvInfo<Tlv::kCslChannel, CslChannelTlvValue> CslChannelTlv;
#if OPENTHREAD_CONFIG_TIME_SYNC_ENABLE
/**
+99
View File
@@ -504,6 +504,85 @@ public:
testFreeInstance(instance);
printf("TestTxChallengeTable passed\n");
}
#if OPENTHREAD_CONFIG_MAC_CSL_TRANSMITTER_ENABLE
static void TestChildUpdateRequestCslChannel(void)
{
static constexpr uint16_t kCslPeriod = 3125;
struct TestCase
{
uint16_t mChannel;
bool mShouldAccept;
};
static const TestCase kTestCases[] = {
{0, true}, // Zero indicates CSL channel is not specified.
{Radio::kChannelMin - 1, false},
{Radio::kChannelMin, true},
{Radio::kChannelMax, true},
{Radio::kChannelMax + 1, false},
{200, false},
{0x0100 + Radio::kChannelMin, false}, // Would be a valid channel if truncated to `uint8_t`.
{0xffff, false},
};
Instance *instance = static_cast<Instance *>(testInitInstance());
Mac::ExtAddress childExtAddress = ExtAddressFromSeed(0x30);
Mle::Mle *mle;
Mle::DeviceMode mode;
Mle::DeviceMode::ModeConfig config;
Child *child;
uint8_t expectedChannel = 0;
printf("TestChildUpdateRequestCslChannel\n");
VerifyOrQuit(instance != nullptr);
mle = &instance->Get<Mle::Mle>();
config.mRxOnWhenIdle = false;
config.mDeviceType = false;
config.mNetworkData = false;
mode.Set(config);
child = mle->mChildTable.GetNewChild();
VerifyOrQuit(child != nullptr);
child->SetExtAddress(childExtAddress);
child->SetRloc16(kNewChildRloc16);
child->SetDeviceMode(mode);
child->SetState(Neighbor::kStateValid);
child->SetCslPeriod(kCslPeriod);
child->SetCslSynchronized(true);
VerifyOrQuit(child->IsCslSynchronized());
VerifyOrQuit(child->GetCslChannel() == expectedChannel);
for (const TestCase &testCase : kTestCases)
{
Message *message = instance->Get<MessagePool>().Allocate(Message::kTypeIp6);
VerifyOrQuit(message != nullptr);
message->SetSubType(Message::kSubTypeMle);
SuccessOrQuit(Tlv::Append<Mle::ModeTlv>(*message, mode.Get()));
SuccessOrQuit(Tlv::Append<Mle::CslChannelTlv>(*message, Mle::CslChannelTlvValue(testCase.mChannel)));
HandleChildUpdateRequest(*mle, *message, childExtAddress);
if (testCase.mShouldAccept)
{
expectedChannel = static_cast<uint8_t>(testCase.mChannel);
}
VerifyOrQuit(child->GetCslChannel() == expectedChannel);
message->Free();
}
testFreeInstance(instance);
printf("TestChildUpdateRequestCslChannel passed\n");
}
#endif // OPENTHREAD_CONFIG_MAC_CSL_TRANSMITTER_ENABLE
#endif // OPENTHREAD_FTD
private:
@@ -712,6 +791,23 @@ private:
message->Free();
testFreeInstance(instance);
}
#if OPENTHREAD_FTD && OPENTHREAD_CONFIG_MAC_CSL_TRANSMITTER_ENABLE
static void HandleChildUpdateRequest(Mle::Mle &aMle, Message &aMessage, const Mac::ExtAddress &aChildExtAddress)
{
Ip6::Address peerAddress;
Ip6::MessageInfo messageInfo;
Mle::Mle::RxInfo rxInfo(aMessage, messageInfo);
peerAddress.InitAsLinkLocalAddress(aChildExtAddress);
messageInfo.SetPeerAddr(peerAddress);
messageInfo.SetSockAddr(aMle.GetLinkLocalAddress());
aMessage.SetOffset(0);
aMle.HandleChildUpdateRequestOnParent(rxInfo);
}
#endif
};
#if OPENTHREAD_FTD && OPENTHREAD_CONFIG_MLE_DEVICE_PROPERTY_LEADER_WEIGHT_ENABLE
@@ -911,6 +1007,9 @@ int main(void)
#if OPENTHREAD_FTD
ot::UnitTester::TestTxChallengeTable();
ot::TestRouterTableRouterIdBounds();
#if OPENTHREAD_CONFIG_MAC_CSL_TRANSMITTER_ENABLE
ot::UnitTester::TestChildUpdateRequestCslChannel();
#endif
#endif
#if OPENTHREAD_FTD && OPENTHREAD_CONFIG_MLE_DEVICE_PROPERTY_LEADER_WEIGHT_ENABLE