Skip to content

Commit 3007641

Browse files
kghostbzbarsky-apple
authored andcommitted
Fix GlobalUnencryptedMessageCounter initial value (#19429)
* Fix GlobalUnencryptedMessageCounter initial value * Update src/transport/MessageCounter.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
1 parent ee22333 commit 3007641

File tree

3 files changed

+8
-40
lines changed

3 files changed

+8
-40
lines changed

src/transport/BUILD.gn

-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ static_library("transport") {
2727
"GroupPeerMessageCounter.cpp",
2828
"GroupPeerMessageCounter.h",
2929
"GroupSession.h",
30-
"MessageCounter.cpp",
3130
"MessageCounter.h",
3231
"MessageCounterManagerInterface.h",
3332
"PeerMessageCounter.h",

src/transport/MessageCounter.cpp

-35
This file was deleted.

src/transport/MessageCounter.h

+8-4
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ namespace chip {
3939
class MessageCounter
4040
{
4141
public:
42+
static constexpr uint32_t kMessageCounterRandomInitMask = 0x0FFFFFFF; ///< 28-bit mask
43+
4244
enum Type : uint8_t
4345
{
4446
GlobalUnencrypted,
@@ -50,14 +52,17 @@ class MessageCounter
5052

5153
virtual Type GetType() const = 0;
5254
virtual CHIP_ERROR AdvanceAndConsume(uint32_t & fetch) = 0; /** Advance the counter, and feed the new counter to fetch */
55+
56+
// Note: this function must be called after Crypto is initialized. It can not be called from global variable constructor.
57+
static uint32_t GetDefaultInitialValuePredecessor() { return Crypto::GetRandU32() & kMessageCounterRandomInitMask; }
5358
};
5459

5560
class GlobalUnencryptedMessageCounter : public MessageCounter
5661
{
5762
public:
5863
GlobalUnencryptedMessageCounter() : mLastUsedValue(0) {}
5964

60-
void Init();
65+
void Init() { mLastUsedValue = GetDefaultInitialValuePredecessor(); }
6166

6267
Type GetType() const override { return GlobalUnencrypted; }
6368
CHIP_ERROR AdvanceAndConsume(uint32_t & fetch) override
@@ -73,8 +78,7 @@ class GlobalUnencryptedMessageCounter : public MessageCounter
7378
class LocalSessionMessageCounter : public MessageCounter
7479
{
7580
public:
76-
static constexpr uint32_t kMessageCounterMax = 0xFFFFFFFF;
77-
static constexpr uint32_t kMessageCounterRandomInitMask = 0x0FFFFFFF; ///< 28-bit mask
81+
static constexpr uint32_t kMessageCounterMax = 0xFFFFFFFF;
7882

7983
/**
8084
* Initialize a local message counter with random value between [1, 2^28]. This increases the difficulty of traffic analysis
@@ -83,7 +87,7 @@ class LocalSessionMessageCounter : public MessageCounter
8387
*
8488
* The mLastUsedValue is the predecessor of the initial value, it will be advanced before using, so don't need to add 1 here.
8589
*/
86-
LocalSessionMessageCounter() { mLastUsedValue = (Crypto::GetRandU32() & kMessageCounterRandomInitMask); }
90+
LocalSessionMessageCounter() { mLastUsedValue = GetDefaultInitialValuePredecessor(); }
8791

8892
Type GetType() const override { return Session; }
8993
CHIP_ERROR AdvanceAndConsume(uint32_t & fetch) override

0 commit comments

Comments
 (0)