diff --git a/src/core/meshcop/tcat_agent.cpp b/src/core/meshcop/tcat_agent.cpp index 7043c9733..f24d98cae 100644 --- a/src/core/meshcop/tcat_agent.cpp +++ b/src/core/meshcop/tcat_agent.cpp @@ -80,7 +80,7 @@ void TcatAgent::ClearCommissionerState(void) mPskdVerified = false; mPskcVerified = false; mInstallCodeVerified = false; - mIsCommissioned = false; + mCanOverwriteDataset = false; mApplicationResponsePending = false; mHasWrittenActiveDataset = false; } @@ -243,8 +243,8 @@ Error TcatAgent::Connected(MeshCoP::Tls::Extension &aTls) NotifyStateChange(); LogInfo("Connected"); - // This specifically stores the state IsCommissioned at _start_ of session: - mIsCommissioned = Get().IsCommissioned(); + // If already commissioned at start of session, overwriting is never allowed. + mCanOverwriteDataset = !Get().IsCommissioned(); exit: return error; @@ -403,7 +403,7 @@ exit: bool TcatAgent::IsSetActiveDatasetAuthorized(const Dataset *aDataset) const { - return !mIsCommissioned && + return mCanOverwriteDataset && IsCommandClassAuthorizedWithFlags(mCommissionerAuthorizationField.mCommissioningFlags, mDeviceAuthorizationField.mCommissioningFlags, aDataset); } @@ -719,7 +719,7 @@ Error TcatAgent::HandleDecommission(void) #endif mJoinCallback.InvokeIfSet(&GetInstance(), /* aIsJoin */ false, error); - mIsCommissioned = false; // enable repeated commissioning/decommissioning in a session + mCanOverwriteDataset = true; // enable repeated commissioning/decommissioning cycles in a session exit: return error; @@ -1121,7 +1121,7 @@ void TcatAgent::HandleNotifierEvents(Events aEvents) // This event revokes the Commissioner's existing authorization (if any) to rewrite datasets. if (!mHasWrittenActiveDataset && aEvents.ContainsAny(kEventNetworkKeyChanged | kEventThreadExtPanIdChanged)) { - mIsCommissioned = true; + mCanOverwriteDataset = false; } mHasWrittenActiveDataset = false; diff --git a/src/core/meshcop/tcat_agent.hpp b/src/core/meshcop/tcat_agent.hpp index 797f0ba82..d0ba29db6 100644 --- a/src/core/meshcop/tcat_agent.hpp +++ b/src/core/meshcop/tcat_agent.hpp @@ -523,7 +523,7 @@ private: bool mPskdVerified : 1; bool mPskcVerified : 1; bool mInstallCodeVerified : 1; - bool mIsCommissioned : 1; + bool mCanOverwriteDataset : 1; bool mApplicationResponsePending : 1; bool mHasWrittenActiveDataset : 1; using ExpireTimer = TimerMilliIn; diff --git a/tests/unit/test_tcat.cpp b/tests/unit/test_tcat.cpp index 5c0cb8136..c93f735b9 100644 --- a/tests/unit/test_tcat.cpp +++ b/tests/unit/test_tcat.cpp @@ -97,6 +97,16 @@ void otPlatAlarmMilliStartAt(otInstance *, uint32_t aT0, uint32_t aDt) uint32_t otPlatAlarmMilliGetNow(void) { return sNow; } +// Override the weak test platform stub to prevent buffer NULL dereference during the tests. +otRadioFrame *otPlatRadioGetTransmitBuffer(otInstance *) +{ + static otRadioFrame sTxFrame; + static uint8_t sTxPsdu[OT_RADIO_FRAME_MAX_SIZE]; + + sTxFrame.mPsdu = sTxPsdu; + return &sTxFrame; +} + } // extern "C" namespace ot { @@ -561,7 +571,7 @@ private: aAgent->ClearCommissionerState(); aAgent->mCommissionerAuthorizationField = aCommAuth; aAgent->mDeviceAuthorizationField = aDeviceAuth; - aAgent->mIsCommissioned = aIsCommissionedAtStart; + aAgent->mCanOverwriteDataset = !aIsCommissionedAtStart; aAgent->mNextState = (aAgent->mState == TcatAgent::kStateActiveTemporary) ? TcatAgent::kStateStandby : TcatAgent::kStateActive; @@ -621,13 +631,17 @@ private: // just like a real attach. Requires a complete Active Dataset to be present. static void MockDeviceAttachedToNetwork(Instance *aInstance) { - SuccessOrQuit(otIp6SetEnabled(aInstance, true)); - SuccessOrQuit(otThreadSetEnabled(aInstance, true)); SuccessOrQuit(otThreadBecomeLeader(aInstance)); otTaskletsProcess(aInstance); VerifyOrQuit(otThreadGetDeviceRole(aInstance) == OT_DEVICE_ROLE_LEADER); } + static void MockDeviceDetachedFromNetwork(Instance *aInstance) + { + otTaskletsProcess(aInstance); + VerifyOrQuit(otThreadGetDeviceRole(aInstance) == OT_DEVICE_ROLE_DISABLED); + } + public: static void TestTcatCommissioner1Auth(void) { @@ -1066,7 +1080,7 @@ public: testFreeInstance(instance); } - static void TestTcatAttemptDatasetOverwrite(void) + static void TestTcatDatasetOverwrite(void) { Instance *instance = TestInitInstanceTcat(); TcatAgent *agent = &instance->Get(); @@ -1122,7 +1136,7 @@ public: testFreeInstance(instance); } - static void TestTcatAttemptDatasetOverwriteAfterAttach(void) + static void TestTcatDatasetOverwriteAfterAttach(void) { Instance *instance = TestInitInstanceTcat(); TcatAgent *agent = &instance->Get(); @@ -1136,17 +1150,59 @@ public: VerifyOrQuit(IsSetActiveDatasetSuccessful(agent, sFullDataset)); VerifyOrQuit(IsSetActiveDatasetSuccessful(agent, sPartialDataset)); - // Once the device attaches to a Thread network, the TCAT Commissioner can no longer overwrite the - // Active Dataset (this prevents a dataset from propagating to other already-networked devices). + for (int i = 0; i < 3; i++) + { + // Once the device is attached to a Thread network, the TCAT Commissioner can no longer overwrite the + // Active Dataset (this prevents a dataset from propagating to other already-networked devices). + agent->HandleStartThreadInterface(); + MockDeviceAttachedToNetwork(instance); + VerifyOrQuit(!IsSetActiveDatasetSuccessful(agent, sFullDataset)); + VerifyOrQuit(!IsSetActiveDatasetSuccessful(agent, sPartialDataset)); + + // When the device has detached from Thread, the TCAT Commissioner can overwrite the Active Dataset + // even without a Decommission command, because it's still in the same TCAT session. + agent->HandleStopThreadInterface(); + MockDeviceDetachedFromNetwork(instance); + VerifyOrQuit(IsSetActiveDatasetSuccessful(agent, sFullDataset)); + VerifyOrQuit(IsSetActiveDatasetSuccessful(agent, sPartialDataset)); + } + + // Attach to Thread Network again to prepare for the next test. + agent->HandleStartThreadInterface(); MockDeviceAttachedToNetwork(instance); + // Another module/process changes the Active Dataset content significantly (here: a different Network Key). + { + Dataset::Info externalInfo = sFullDataset; + + externalInfo.mNetworkKey.m8[0] ^= 0xff; + MockActiveDatasetChanged(instance, externalInfo); + VerifyOrQuit(!IsSetActiveDatasetSuccessful(agent, sFullDataset)); + VerifyOrQuit(!IsSetActiveDatasetSuccessful(agent, sPartialDataset)); + } + + // Thread is stopped + agent->HandleStopThreadInterface(); + MockDeviceDetachedFromNetwork(instance); VerifyOrQuit(!IsSetActiveDatasetSuccessful(agent, sFullDataset)); VerifyOrQuit(!IsSetActiveDatasetSuccessful(agent, sPartialDataset)); - // After the Active Dataset is cleared (decommissioned), the agent considers the device no longer - // commissioned and allows overwriting again. + // Attach to Thread Network again + agent->HandleStartThreadInterface(); + MockDeviceAttachedToNetwork(instance); + VerifyOrQuit(!IsSetActiveDatasetSuccessful(agent, sFullDataset)); + VerifyOrQuit(!IsSetActiveDatasetSuccessful(agent, sPartialDataset)); + + // After the Active Dataset is cleared (decommissioned) by an external source, the agent + // still cannot set the active dataset. MockActiveDatasetCleared(instance); VerifyOrQuit(!instance->Get().IsCommissioned()); + VerifyOrQuit(!IsSetActiveDatasetSuccessful(agent, sFullDataset)); + VerifyOrQuit(!IsSetActiveDatasetSuccessful(agent, sPartialDataset)); + + // TCAT Commissioner reconnects and then can write a dataset again. + MockCommissionerConnected(agent, sCommAuth, sDeviceAuth, /* aIsCommissionedAtStart */ false); + VerifyOrQuit(!instance->Get().IsCommissioned()); VerifyOrQuit(IsSetActiveDatasetSuccessful(agent, sFullDataset)); VerifyOrQuit(IsSetActiveDatasetSuccessful(agent, sPartialDataset)); @@ -1210,7 +1266,7 @@ public: // A Commissioner connects to the (uncommissioned) device MockCommissionerConnected(agent, sCommAuth, sDeviceAuth, /* aIsCommissionedAtStart */ false); VerifyOrQuit(agent->IsStarted()); - VerifyOrQuit(!agent->mIsCommissioned); + VerifyOrQuit(agent->mCanOverwriteDataset); // Start observing state-changed notifications. SuccessOrQuit(otSetStateChangedCallback(instance, HandleNotifierStateChanged, &observer)); @@ -1239,7 +1295,7 @@ public: // Because the agent saw both events coalesced while mHasWrittenActiveDataset was set, it recognizes the // change as its own and retains the Commissioner's authorization to overwrite the dataset. - VerifyOrQuit(!agent->mIsCommissioned, "a self-made dataset change must not revoke authorization"); + VerifyOrQuit(agent->mCanOverwriteDataset, "a self-made dataset change must not revoke authorization"); } otRemoveStateChangeCallback(instance, HandleNotifierStateChanged, &observer); @@ -1265,8 +1321,8 @@ int main(void) ot::MeshCoP::UnitTester::TestTcatCommissioner1AuthWithDeviceRequirements(); ot::MeshCoP::UnitTester::TestTcatCommissioner2AuthWithDeviceRequirements(); ot::MeshCoP::UnitTester::TestTcatCommissioner4AuthWithExistingPartialDataset(); - ot::MeshCoP::UnitTester::TestTcatAttemptDatasetOverwrite(); - ot::MeshCoP::UnitTester::TestTcatAttemptDatasetOverwriteAfterAttach(); + ot::MeshCoP::UnitTester::TestTcatDatasetOverwrite(); + ot::MeshCoP::UnitTester::TestTcatDatasetOverwriteAfterAttach(); ot::MeshCoP::UnitTester::TestTcatRepeatedCommandActivation(); ot::MeshCoP::UnitTester::TestTcatNotifierCoalescesEvents(); printf("All tests passed\n");