commit 5181fc9cf2858955c83167b0d32d48c6a41dd843
author Jean-Francois Penven <67962328+jepenven-silabs@users.noreply.github.com> Wed May 24 09:23:45 2023 -0400
committer GitHub <noreply@github.com> Wed May 24 13:23:45 2023 +0000
tree 61468521bf90ba101ce0949559448d969a57ac74
parent 849cd66885fbda10329d82f543a21e4266f0ed35

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

* Add Random first value group counter

* fix CI

src/transport/GroupPeerMessageCounter.cpp

diff --git a/src/transport/GroupPeerMessageCounter.cpp b/src/transport/GroupPeerMessageCounter.cpp
index 29212ee..b93dd9b 100644
--- 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 @@
         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 @@
     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 @@
     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)
     {

src/transport/GroupPeerMessageCounter.h

diff --git a/src/transport/GroupPeerMessageCounter.h b/src/transport/GroupPeerMessageCounter.h
index 6c2b278..25924ec 100644
--- a/src/transport/GroupPeerMessageCounter.h
+++ b/src/transport/GroupPeerMessageCounter.h
@@ -77,6 +77,8 @@
 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);

src/transport/tests/TestGroupMessageCounter.cpp

diff --git a/src/transport/tests/TestGroupMessageCounter.cpp b/src/transport/tests/TestGroupMessageCounter.cpp
index 4a33d38..8e3101e 100644
--- a/src/transport/tests/TestGroupMessageCounter.cpp
+++ b/src/transport/tests/TestGroupMessageCounter.cpp
@@ -404,15 +404,18 @@
 
     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 @@
     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 @@
     // 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);