[tcat] clarify mIsCommissioned to mCanOverwriteDataset; fix overwrite tests (#13182)

This reworks mIsCommissioned to a new name (and inverse value)
mCanOverwriteDataset to better reflect its use, which is not fully
related to the device's current 'commissioned' state. Also the related
unit test is clarified (naming and comments) and extended with multiple
attach/detach operations to test when dataset overwriting is allowed and
when not.

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<ActiveDatasetManager>().IsCommissioned();
+    // If already commissioned at start of session, overwriting is never allowed.
+    mCanOverwriteDataset = !Get<ActiveDatasetManager>().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;
 

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<TcatAgent, &TcatAgent::HandleTimer>;

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<TcatAgent>();
@@ -1122,7 +1136,7 @@ public:
         testFreeInstance(instance);
     }
 
-    static void TestTcatAttemptDatasetOverwriteAfterAttach(void)
+    static void TestTcatDatasetOverwriteAfterAttach(void)
     {
         Instance  *instance = TestInitInstanceTcat();
         TcatAgent *agent    = &instance->Get<TcatAgent>();
@@ -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<ActiveDatasetManager>().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<ActiveDatasetManager>().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");