[Group][Transport] Add Random first value group counter (#26610)

* Add Random first value group counter * fix CI
project-chip · Jan 15, 2024 · a9f600a · a9f600a
1 parent 3c53a76
commit a9f600a
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 12 deletions.
diff --git a/src/transport/GroupPeerMessageCounter.cpp b/src/transport/GroupPeerMessageCounter.cpp
@@ -23,6 +23,8 @@
 #include <lib/support/DefaultStorageKeyAllocator.h>
 #include <transport/GroupPeerMessageCounter.h>
 
+#include <crypto/RandUtils.h>
+
 namespace chip {
 namespace Transport {
 
@@ -268,7 +270,6 @@ CHIP_ERROR GroupOutgoingCounters::Init(chip::PersistentStorageDelegate * storage
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
 
-    // TODO Implement Logic for first time use / factory reset to be random
     // Spec 4.5.1.3
     mStorage      = storage_delegate;
     uint16_t size = static_cast<uint16_t>(sizeof(uint32_t));
@@ -277,9 +278,8 @@ CHIP_ERROR GroupOutgoingCounters::Init(chip::PersistentStorageDelegate * storage
     err = mStorage->SyncGetKeyValue(DefaultStorageKeyAllocator::GroupControlCounter().KeyName(), &temp, size);
     if (err == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND)
     {
-        // might be the first time we retrieve the value
-        // TODO handle this case
-        mGroupControlCounter = 0; // TODO should be random
+        // First time retrieving the counter
+        mGroupControlCounter = (chip::Crypto::GetRandU32() & kMessageCounterRandomInitMask) + 1;
     }
     else if (err != CHIP_NO_ERROR)
     {
@@ -293,9 +293,8 @@ CHIP_ERROR GroupOutgoingCounters::Init(chip::PersistentStorageDelegate * storage
     err = mStorage->SyncGetKeyValue(DefaultStorageKeyAllocator::GroupDataCounter().KeyName(), &temp, size);
     if (err == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND)
     {
-        // might be the first time we retrieve the value
-        // TODO handle this case
-        mGroupDataCounter = 0; //  TODO should be random
+        // First time retrieving the counter
+        mGroupDataCounter = (chip::Crypto::GetRandU32() & kMessageCounterRandomInitMask) + 1;
     }
     else if (err != CHIP_NO_ERROR)
     {

diff --git a/src/transport/GroupPeerMessageCounter.h b/src/transport/GroupPeerMessageCounter.h
@@ -77,6 +77,8 @@ class GroupPeerTable
 class GroupOutgoingCounters
 {
 public:
+    static constexpr uint32_t kMessageCounterRandomInitMask = 0x0FFFFFFF; ///< 28-bit mask
+
     GroupOutgoingCounters(){};
     GroupOutgoingCounters(chip::PersistentStorageDelegate * storage_delegate);
     CHIP_ERROR Init(chip::PersistentStorageDelegate * storage_delegate);

diff --git a/src/transport/tests/TestGroupMessageCounter.cpp b/src/transport/tests/TestGroupMessageCounter.cpp
@@ -404,15 +404,18 @@ void GroupMessageCounterTest(nlTestSuite * inSuite, void * inContext)
 
     chip::TestPersistentStorageDelegate delegate;
     TestGroupOutgoingCounters groupCientCounter;
+    uint32_t controlCounter = 0, dataCounter = 0;
     CHIP_ERROR err = groupCientCounter.Init(&delegate);
 
     NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
 
     // Start Test with Control counter
-    NL_TEST_ASSERT(inSuite, groupCientCounter.GetCounter(true) == 0);
+    // Counter should be random
+    controlCounter = groupCientCounter.GetCounter(true);
+    dataCounter    = groupCientCounter.GetCounter(false);
     groupCientCounter.IncrementCounter(true);
 
-    NL_TEST_ASSERT(inSuite, groupCientCounter.GetCounter(true) == 1);
+    NL_TEST_ASSERT(inSuite, (groupCientCounter.GetCounter(true) - controlCounter) == 1);
 
     groupCientCounter.SetCounter(true, UINT32_MAX - GROUP_MSG_COUNTER_MIN_INCREMENT);
     NL_TEST_ASSERT(inSuite, groupCientCounter.GetCounter(true) == UINT32_MAX - GROUP_MSG_COUNTER_MIN_INCREMENT);
@@ -421,7 +424,7 @@ void GroupMessageCounterTest(nlTestSuite * inSuite, void * inContext)
     TestGroupOutgoingCounters groupCientCounter2(&delegate);
 
     NL_TEST_ASSERT(inSuite, groupCientCounter2.GetCounter(true) == UINT32_MAX);
-    NL_TEST_ASSERT(inSuite, groupCientCounter2.GetCounter(false) == GROUP_MSG_COUNTER_MIN_INCREMENT);
+    NL_TEST_ASSERT(inSuite, (groupCientCounter2.GetCounter(false) - dataCounter) == GROUP_MSG_COUNTER_MIN_INCREMENT);
 
     // Test Roll over
     groupCientCounter2.IncrementCounter(true);
@@ -433,9 +436,9 @@ void GroupMessageCounterTest(nlTestSuite * inSuite, void * inContext)
     // Redo the test with the second counter
 
     // Start Test with Control counter
-    NL_TEST_ASSERT(inSuite, groupCientCounter.GetCounter(false) == 0);
+    dataCounter = groupCientCounter.GetCounter(false);
     groupCientCounter.IncrementCounter(false);
-    NL_TEST_ASSERT(inSuite, groupCientCounter.GetCounter(false) == 1);
+    NL_TEST_ASSERT(inSuite, (groupCientCounter.GetCounter(false) - dataCounter) == 1);
 
     groupCientCounter.SetCounter(false, UINT32_MAX - GROUP_MSG_COUNTER_MIN_INCREMENT);
     NL_TEST_ASSERT(inSuite, groupCientCounter.GetCounter(false) == UINT32_MAX - GROUP_MSG_COUNTER_MIN_INCREMENT);