Skip to content

Commit 3bc8b3f

Browse files
authored
[tcat] use Tlv::Info and OffsetRange in HandleSingleTlv() (openthread#13119)
This commit simplifies and enhances the TLV parsing logic in `TcatAgent` so to use the `Tlv::Info` helper class. This safely and automatically handles both standard and extended TLVs, removing the need for manual type checking and length/offset calculations. Key changes: - Updated `TcatAgent::HandleSingleTlv()` to use `Tlv::Info::ParseFrom()`. - Replaced individual `aOffset` and `aLength` parameters with `const OffsetRange &` across various TLV handler methods (e.g., `HandlePing`, `HandleSetActiveOperationalDataset`, `VerifyHash`). This improves code readability, safety, and consistency with common OpenThread TLV parsing patterns.
1 parent 9d55398 commit 3bc8b3f

4 files changed

Lines changed: 63 additions & 86 deletions

File tree

src/core/meshcop/tcat_agent.cpp

Lines changed: 45 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -396,45 +396,31 @@ Error TcatAgent::HandleSingleTlv(const Message &aIncomingMessage, Message &aOutg
396396
{
397397
Error error;
398398
StatusCode statusCode = kStatusGeneralError;
399-
ot::Tlv tlv;
400-
uint16_t offset = aIncomingMessage.GetOffset();
399+
Tlv::Info tlvInfo;
401400
const uint16_t initialOutgoingMsgLength = aOutgoingMessage.GetLength();
402-
uint16_t length;
403-
bool response = false;
401+
bool response = false;
404402

405403
VerifyOrExit(IsConnected(), error = kErrorInvalidState);
406-
SuccessOrExit(error = aIncomingMessage.Read(offset, tlv));
407404

408-
if (tlv.IsExtended())
409-
{
410-
ot::ExtendedTlv extTlv;
411-
SuccessOrExit(error = aIncomingMessage.Read(offset, extTlv));
412-
length = extTlv.GetLength();
413-
offset += sizeof(ot::ExtendedTlv);
414-
}
415-
else
416-
{
417-
length = tlv.GetLength();
418-
offset += sizeof(ot::Tlv);
419-
}
405+
SuccessOrExit(error = tlvInfo.ParseFrom(aIncomingMessage, aIncomingMessage.GetOffset()));
420406

421-
switch (tlv.GetType())
407+
switch (tlvInfo.GetType())
422408
{
423409
case kTlvDisconnect:
424410
error = kErrorAbort;
425411
response = true; // true - to avoid response-with-status being sent.
426412
break;
427413

428414
case kTlvSetActiveOperationalDataset:
429-
error = HandleSetActiveOperationalDataset(aIncomingMessage, offset, length);
415+
error = HandleSetActiveOperationalDataset(aIncomingMessage, tlvInfo.GetValueOffsetRange());
430416
break;
431417

432418
case kTlvGetActiveOperationalDataset:
433419
error = HandleGetActiveOperationalDataset(aOutgoingMessage, response);
434420
break;
435421

436422
case kTlvGetDiagnosticTlvs:
437-
error = HandleGetDiagnosticTlvs(aIncomingMessage, aOutgoingMessage, offset, length, response);
423+
error = HandleGetDiagnosticTlvs(aIncomingMessage, aOutgoingMessage, tlvInfo.GetValueOffsetRange(), response);
438424
break;
439425

440426
case kTlvStartThreadInterface:
@@ -454,16 +440,16 @@ Error TcatAgent::HandleSingleTlv(const Message &aIncomingMessage, Message &aOutg
454440
case kTlvSendApplicationData3:
455441
case kTlvSendApplicationData4:
456442
case kTlvSendVendorSpecificData:
457-
error = HandleApplicationData(aIncomingMessage, offset, static_cast<TcatApplicationProtocol>(tlv.GetType()),
458-
response);
443+
error = HandleApplicationData(aIncomingMessage, tlvInfo.GetValueOffset(),
444+
static_cast<TcatApplicationProtocol>(tlvInfo.GetType()), response);
459445
break;
460446

461447
case kTlvDecommission:
462448
error = HandleDecommission();
463449
break;
464450

465451
case kTlvPing:
466-
error = HandlePing(aIncomingMessage, aOutgoingMessage, offset, length, response);
452+
error = HandlePing(aIncomingMessage, aOutgoingMessage, tlvInfo.GetValueOffsetRange(), response);
467453
break;
468454

469455
case kTlvGetNetworkName:
@@ -483,15 +469,15 @@ Error TcatAgent::HandleSingleTlv(const Message &aIncomingMessage, Message &aOutg
483469
break;
484470

485471
case kTlvPresentPskdHash:
486-
error = HandlePresentPskdHash(aIncomingMessage, offset, length);
472+
error = HandlePresentPskdHash(aIncomingMessage, tlvInfo.GetValueOffsetRange());
487473
break;
488474

489475
case kTlvPresentPskcHash:
490-
error = HandlePresentPskcHash(aIncomingMessage, offset, length);
476+
error = HandlePresentPskcHash(aIncomingMessage, tlvInfo.GetValueOffsetRange());
491477
break;
492478

493479
case kTlvPresentInstallCodeHash:
494-
error = HandlePresentInstallCodeHash(aIncomingMessage, offset, length);
480+
error = HandlePresentInstallCodeHash(aIncomingMessage, tlvInfo.GetValueOffsetRange());
495481
break;
496482

497483
case kTlvRequestRandomNumChallenge:
@@ -567,18 +553,16 @@ Error TcatAgent::HandleSingleTlv(const Message &aIncomingMessage, Message &aOutg
567553
return error;
568554
}
569555

570-
Error TcatAgent::HandleSetActiveOperationalDataset(const Message &aIncomingMessage, uint16_t aOffset, uint16_t aLength)
556+
Error TcatAgent::HandleSetActiveOperationalDataset(const Message &aIncomingMessage, const OffsetRange &aOffsetRange)
571557
{
572-
Dataset dataset;
573-
OffsetRange offsetRange;
574-
Error error;
575-
uint8_t buf[kCommissionerCertMaxLength];
576-
size_t bufLen = sizeof(buf);
558+
Dataset dataset;
559+
Error error;
560+
uint8_t buf[kCommissionerCertMaxLength];
561+
size_t bufLen = sizeof(buf);
577562

578563
VerifyOrExit(!mIsCommissioned, error = kErrorAlready);
579564

580-
offsetRange.Init(aOffset, aLength);
581-
SuccessOrExit(error = dataset.SetFrom(aIncomingMessage, offsetRange));
565+
SuccessOrExit(error = dataset.SetFrom(aIncomingMessage, aOffsetRange));
582566
SuccessOrExit(error = dataset.ValidateTlvs());
583567
VerifyOrExit(dataset.ContainsTlv(Tlv::kNetworkKey), error = kErrorInvalidArgs);
584568

@@ -623,21 +607,18 @@ Error TcatAgent::HandleGetCommissionerCertificate(Message &aOutgoingMessage, boo
623607
return error;
624608
}
625609

626-
Error TcatAgent::HandleGetDiagnosticTlvs(const Message &aIncomingMessage,
627-
Message &aOutgoingMessage,
628-
uint16_t aOffset,
629-
uint16_t aLength,
630-
bool &aResponse)
610+
Error TcatAgent::HandleGetDiagnosticTlvs(const Message &aIncomingMessage,
611+
Message &aOutgoingMessage,
612+
const OffsetRange &aOffsetRange,
613+
bool &aResponse)
631614
{
632615
Error error = kErrorNone;
633-
OffsetRange offsetRange;
634616
ot::ExtendedTlv extTlv;
635617
uint16_t initialLength;
636618
uint16_t length;
637619

638620
VerifyOrExit(IsCommandClassAuthorized(kCommissioning), error = kErrorRejected);
639621

640-
offsetRange.Init(aOffset, aLength);
641622
initialLength = aOutgoingMessage.GetLength();
642623

643624
// Start with extTlv to avoid the need for a temporary message buffer to calculate reply length
@@ -646,7 +627,7 @@ Error TcatAgent::HandleGetDiagnosticTlvs(const Message &aIncomingMessage,
646627
SuccessOrExit(error = aOutgoingMessage.Append(extTlv));
647628

648629
error =
649-
Get<NetworkDiagnostic::Server>().AppendRequestedTlvsForTcat(aIncomingMessage, aOutgoingMessage, offsetRange);
630+
Get<NetworkDiagnostic::Server>().AppendRequestedTlvsForTcat(aIncomingMessage, aOutgoingMessage, aOffsetRange);
650631

651632
// Ensure enough message buffers are left for transmission of the result. Report error otherwise.
652633
if (Get<MessagePool>().GetFreeBufferCount() < kBufferReserve)
@@ -707,31 +688,31 @@ Error TcatAgent::HandleDecommission(void)
707688
return error;
708689
}
709690

710-
Error TcatAgent::HandlePing(const Message &aIncomingMessage,
711-
Message &aOutgoingMessage,
712-
uint16_t aOffset,
713-
uint16_t aLength,
714-
bool &aResponse)
691+
Error TcatAgent::HandlePing(const Message &aIncomingMessage,
692+
Message &aOutgoingMessage,
693+
const OffsetRange &aOffsetRange,
694+
bool &aResponse)
715695
{
716696
Error error = kErrorNone;
717697
ot::ExtendedTlv extTlv;
718698
ot::Tlv tlv;
719699

720-
VerifyOrExit(aLength <= kPingPayloadMaxLength, error = kErrorParse);
721-
if (aLength > ot::Tlv::kBaseTlvMaxLength)
700+
VerifyOrExit(aOffsetRange.GetLength() <= kPingPayloadMaxLength, error = kErrorParse);
701+
702+
if (aOffsetRange.GetLength() > ot::Tlv::kBaseTlvMaxLength)
722703
{
723704
extTlv.SetType(kTlvResponseWithPayload);
724-
extTlv.SetLength(aLength);
705+
extTlv.SetLength(aOffsetRange.GetLength());
725706
SuccessOrExit(error = aOutgoingMessage.Append(extTlv));
726707
}
727708
else
728709
{
729710
tlv.SetType(kTlvResponseWithPayload);
730-
tlv.SetLength(static_cast<uint8_t>(aLength));
711+
tlv.SetLength(static_cast<uint8_t>(aOffsetRange.GetLength()));
731712
SuccessOrExit(error = aOutgoingMessage.Append(tlv));
732713
}
733714

734-
SuccessOrExit(error = aOutgoingMessage.AppendBytesFromMessage(aIncomingMessage, aOffset, aLength));
715+
SuccessOrExit(error = aOutgoingMessage.AppendBytesFromMessage(aIncomingMessage, aOffsetRange));
735716
aResponse = true;
736717

737718
exit:
@@ -820,22 +801,22 @@ Error TcatAgent::HandleGetProvisioningUrl(Message &aOutgoingMessage, bool &aResp
820801
return error;
821802
}
822803

823-
Error TcatAgent::HandlePresentPskdHash(const Message &aIncomingMessage, uint16_t aOffset, uint16_t aLength)
804+
Error TcatAgent::HandlePresentPskdHash(const Message &aIncomingMessage, const OffsetRange &aOffsetRange)
824805
{
825806
Error error = kErrorNone;
826807

827808
VerifyOrExit(mVendorInfo != nullptr, error = kErrorInvalidState);
828809
VerifyOrExit(mVendorInfo->mPskdString != nullptr, error = kErrorSecurity);
829810

830-
SuccessOrExit(error = VerifyHash(aIncomingMessage, aOffset, aLength, mVendorInfo->mPskdString,
811+
SuccessOrExit(error = VerifyHash(aIncomingMessage, aOffsetRange, mVendorInfo->mPskdString,
831812
StringLength(mVendorInfo->mPskdString, kMaxPskdLength)));
832813
mPskdVerified = true;
833814

834815
exit:
835816
return error;
836817
}
837818

838-
Error TcatAgent::HandlePresentPskcHash(const Message &aIncomingMessage, uint16_t aOffset, uint16_t aLength)
819+
Error TcatAgent::HandlePresentPskcHash(const Message &aIncomingMessage, const OffsetRange &aOffsetRange)
839820
{
840821
Error error = kErrorNone;
841822
Dataset::Info datasetInfo;
@@ -845,21 +826,21 @@ Error TcatAgent::HandlePresentPskcHash(const Message &aIncomingMessage, uint16_t
845826
VerifyOrExit(datasetInfo.IsPresent<Dataset::kPskc>(), error = kErrorSecurity);
846827
pskc = datasetInfo.Get<Dataset::kPskc>();
847828

848-
SuccessOrExit(error = VerifyHash(aIncomingMessage, aOffset, aLength, pskc.m8, Pskc::kSize));
829+
SuccessOrExit(error = VerifyHash(aIncomingMessage, aOffsetRange, pskc.m8, Pskc::kSize));
849830
mPskcVerified = true;
850831

851832
exit:
852833
return error;
853834
}
854835

855-
Error TcatAgent::HandlePresentInstallCodeHash(const Message &aIncomingMessage, uint16_t aOffset, uint16_t aLength)
836+
Error TcatAgent::HandlePresentInstallCodeHash(const Message &aIncomingMessage, const OffsetRange &aOffsetRange)
856837
{
857838
Error error = kErrorNone;
858839

859840
VerifyOrExit(mVendorInfo != nullptr, error = kErrorInvalidState);
860841
VerifyOrExit(mVendorInfo->mInstallCode != nullptr, error = kErrorSecurity);
861842

862-
SuccessOrExit(error = VerifyHash(aIncomingMessage, aOffset, aLength, mVendorInfo->mInstallCode,
843+
SuccessOrExit(error = VerifyHash(aIncomingMessage, aOffsetRange, mVendorInfo->mInstallCode,
863844
StringLength(mVendorInfo->mInstallCode, kInstallCodeMaxSize)));
864845
mInstallCodeVerified = true;
865846

@@ -881,19 +862,18 @@ Error TcatAgent::HandleRequestRandomNumberChallenge(Message &aOutgoingMessage, b
881862
return error;
882863
}
883864

884-
Error TcatAgent::VerifyHash(const Message &aIncomingMessage,
885-
uint16_t aOffset,
886-
uint16_t aLength,
887-
const void *aBuf,
888-
size_t aBufLen)
865+
Error TcatAgent::VerifyHash(const Message &aIncomingMessage,
866+
const OffsetRange &aOffsetRange,
867+
const void *aBuf,
868+
size_t aBufLen)
889869
{
890870
Error error = kErrorNone;
891871
Crypto::HmacSha256::Hash hash;
892872
UptimeSec currentTime = Get<UptimeTracker>().GetUptimeInSeconds();
893873
uint32_t newAttempts = (currentTime - mLastHashVerificationTimestamp) / kHashVerificationAttemptTime;
894874
uint32_t totalAttempts = newAttempts + mHashVerificationAttempts;
895875

896-
VerifyOrExit(aLength == Crypto::HmacSha256::Hash::kSize, error = kErrorSecurity);
876+
VerifyOrExit(aOffsetRange.GetLength() == Crypto::HmacSha256::Hash::kSize, error = kErrorSecurity);
897877
VerifyOrExit(mRandomChallenge != 0, error = kErrorSecurity);
898878

899879
// In case uptime has overflowed (will never happen in practical functional operational life), up to
@@ -907,7 +887,7 @@ Error TcatAgent::VerifyHash(const Message &aIncomingMessage,
907887
SuccessOrExit(error = CalculateHash(mRandomChallenge, reinterpret_cast<const char *>(aBuf), aBufLen, hash));
908888
DumpDebg("Hash", &hash, sizeof(hash));
909889

910-
VerifyOrExit(aIncomingMessage.Compare(aOffset, hash), error = kErrorSecurity);
890+
VerifyOrExit(aIncomingMessage.Compare(aOffsetRange.GetOffset(), hash), error = kErrorSecurity);
911891
mHashVerificationAttempts++;
912892

913893
exit:

src/core/meshcop/tcat_agent.hpp

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -446,26 +446,24 @@ class TcatAgent : public InstanceLocator, private NonCopyable
446446
void Disconnected(void);
447447

448448
Error HandleSingleTlv(const Message &aIncomingMessage, Message &aOutgoingMessage);
449-
Error HandleSetActiveOperationalDataset(const Message &aIncomingMessage, uint16_t aOffset, uint16_t aLength);
449+
Error HandleSetActiveOperationalDataset(const Message &aIncomingMessage, const OffsetRange &aOffsetRange);
450450
Error HandleGetActiveOperationalDataset(Message &aOutgoingMessage, bool &aResponse);
451-
Error HandleGetDiagnosticTlvs(const Message &aIncomingMessage,
452-
Message &aOutgoingMessage,
453-
uint16_t aOffset,
454-
uint16_t aLength,
455-
bool &response);
451+
Error HandleGetDiagnosticTlvs(const Message &aIncomingMessage,
452+
Message &aOutgoingMessage,
453+
const OffsetRange &aOffsetRange,
454+
bool &aResponse);
456455
Error HandleDecommission(void);
457-
Error HandlePing(const Message &aIncomingMessage,
458-
Message &aOutgoingMessage,
459-
uint16_t aOffset,
460-
uint16_t aLength,
461-
bool &aResponse);
456+
Error HandlePing(const Message &aIncomingMessage,
457+
Message &aOutgoingMessage,
458+
const OffsetRange &aOffsetRange,
459+
bool &aResponse);
462460
Error HandleGetNetworkName(Message &aOutgoingMessage, bool &aResponse);
463461
Error HandleGetDeviceId(Message &aOutgoingMessage, bool &aResponse);
464462
Error HandleGetExtPanId(Message &aOutgoingMessage, bool &aResponse);
465463
Error HandleGetProvisioningUrl(Message &aOutgoingMessage, bool &aResponse);
466-
Error HandlePresentPskdHash(const Message &aIncomingMessage, uint16_t aOffset, uint16_t aLength);
467-
Error HandlePresentPskcHash(const Message &aIncomingMessage, uint16_t aOffset, uint16_t aLength);
468-
Error HandlePresentInstallCodeHash(const Message &aIncomingMessage, uint16_t aOffset, uint16_t aLength);
464+
Error HandlePresentPskdHash(const Message &aIncomingMessage, const OffsetRange &aOffsetRange);
465+
Error HandlePresentPskcHash(const Message &aIncomingMessage, const OffsetRange &aOffsetRange);
466+
Error HandlePresentInstallCodeHash(const Message &aIncomingMessage, const OffsetRange &aOffsetRange);
469467
Error HandleRequestRandomNumberChallenge(Message &aOutgoingMessage, bool &aResponse);
470468
Error HandleStartThreadInterface(void);
471469
Error HandleStopThreadInterface(void);
@@ -477,11 +475,10 @@ class TcatAgent : public InstanceLocator, private NonCopyable
477475
bool &aResponse);
478476
void HandleTimer(void);
479477
void AdaptToExistingActivePeriod(uint32_t &aPeriodDelayMs, uint32_t &aPeriodDurationMs);
480-
Error VerifyHash(const Message &aIncomingMessage,
481-
uint16_t aOffset,
482-
uint16_t aLength,
483-
const void *aBuf,
484-
size_t aBufLen);
478+
Error VerifyHash(const Message &aIncomingMessage,
479+
const OffsetRange &aOffsetRange,
480+
const void *aBuf,
481+
size_t aBufLen);
485482
Error CalculateHash(uint64_t aChallenge, const char *aBuf, size_t aBufLen, Crypto::HmacSha256::Hash &aHash);
486483

487484
bool IsCommandClassAuthorizedWithFlags(CommandClassFlags aCommissionerCommandClassFlags,

src/core/thread/network_diagnostic.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ Error Server::AppendRequestedTlvs(const Message &aRequest, Message &aResponse)
280280
}
281281

282282
#if OPENTHREAD_CONFIG_BLE_TCAT_ENABLE
283-
Error Server::AppendRequestedTlvsForTcat(const Message &aRequest, Message &aResponse, OffsetRange &aOffsetRange)
283+
Error Server::AppendRequestedTlvsForTcat(const Message &aRequest, Message &aResponse, const OffsetRange &aOffsetRange)
284284
{
285285
Error error = kErrorNone;
286286
TlvTypeListIterator iterator;

src/core/thread/network_diagnostic.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ class Server : public InstanceLocator, private NonCopyable
144144
Error AppendRequestedTlvs(const Message &aRequest, Message &aResponse);
145145

146146
#if OPENTHREAD_CONFIG_BLE_TCAT_ENABLE
147-
Error AppendRequestedTlvsForTcat(const Message &aRequest, Message &aResponse, OffsetRange &aOffsetRange);
147+
Error AppendRequestedTlvsForTcat(const Message &aRequest, Message &aResponse, const OffsetRange &aOffsetRange);
148148
#endif
149149

150150
#if OPENTHREAD_MTD

0 commit comments

Comments
 (0)