Skip to content

Commit

Permalink
Enforce maximum NONCE
Browse files Browse the repository at this point in the history
  • Loading branch information
kghost committed Jun 7, 2022
1 parent 55ab764 commit eda0ae1
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 10 deletions.
3 changes: 3 additions & 0 deletions src/lib/core/CHIPError.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,9 @@ bool FormatCHIPError(char * buf, uint16_t bufSize, CHIP_ERROR err)
case CHIP_ERROR_DRBG_ENTROPY_SOURCE_FAILED.AsInteger():
desc = "DRBG entropy source failed to generate entropy data";
break;
case CHIP_ERROR_MESSAGE_COUNTER_EXHAUSTED.AsInteger():
desc = "Message counter exhausted";
break;
case CHIP_ERROR_FABRIC_EXISTS.AsInteger():
desc = "Trying to add a NOC for a fabric that already exists";
break;
Expand Down
8 changes: 7 additions & 1 deletion src/lib/core/CHIPError.h
Original file line number Diff line number Diff line change
Expand Up @@ -1554,7 +1554,13 @@ using CHIP_ERROR = ::chip::ChipError;
*/
#define CHIP_ERROR_IM_MALFORMED_STATUS_RESPONSE_MESSAGE CHIP_CORE_ERROR(0x7c)

// unused CHIP_CORE_ERROR(0x7d)
/**
* @def CHIP_ERROR_MESSAGE_COUNTER_EXHAUSTED
*
* @brief
* The message counter of the session is exhausted, the session should be closed.
*/
#define CHIP_ERROR_MESSAGE_COUNTER_EXHAUSTED CHIP_CORE_ERROR(0x7d)

/**
* @def CHIP_ERROR_FABRIC_EXISTS
Expand Down
1 change: 1 addition & 0 deletions src/lib/core/tests/TestCHIPErrorStr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ static const CHIP_ERROR kTestElements[] =
CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_REPORT_IB,
CHIP_ERROR_IM_MALFORMED_EVENT_STATUS_IB,
CHIP_ERROR_IM_MALFORMED_STATUS_RESPONSE_MESSAGE,
CHIP_ERROR_MESSAGE_COUNTER_EXHAUSTED,
CHIP_ERROR_FABRIC_EXISTS,
CHIP_ERROR_KEY_NOT_FOUND_FROM_PEER,
CHIP_ERROR_WRONG_ENCRYPTION_TYPE_FROM_PEER,
Expand Down
2 changes: 1 addition & 1 deletion src/protocols/secure_channel/PairingSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ CHIP_ERROR PairingSession::ActivateSecureSession(const Transport::PeerAddress &
ReturnErrorOnFailure(DeriveSecureSession(secureSession->GetCryptoContext()));
uint16_t peerSessionId = GetPeerSessionId();
secureSession->SetPeerAddress(peerAddress);
secureSession->GetSessionMessageCounter().GetPeerMessageCounter().SetCounter(LocalSessionMessageCounter::kInitialSyncValue);
secureSession->GetSessionMessageCounter().GetPeerMessageCounter().SetCounter(Transport::PeerMessageCounter::kInitialSyncValue);

// Call Activate last, otherwise errors on anything after would lead to
// a partially valid session.
Expand Down
20 changes: 17 additions & 3 deletions src/transport/MessageCounter.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ namespace chip {
* of message counter
*
* 1. Global unencrypted message counter
* 2. Global encrypted message counter
* 3. Session message counter
* 2. Secure session message counter
*
* There will be separate implementations for each type
*/
Expand All @@ -51,6 +50,7 @@ class MessageCounter
virtual ~MessageCounter() = default;

virtual Type GetType() const = 0;
virtual bool IsValid() const = 0;
virtual uint32_t Value() const = 0; /** Get current value */
virtual CHIP_ERROR Advance() = 0; /** Advance the counter */
};
Expand All @@ -63,6 +63,7 @@ class GlobalUnencryptedMessageCounter : public MessageCounter
void Init();

Type GetType() const override { return GlobalUnencrypted; }
bool IsValid() const override { return true; }
uint32_t Value() const override { return mValue; }
CHIP_ERROR Advance() override
{
Expand All @@ -77,7 +78,8 @@ class GlobalUnencryptedMessageCounter : public MessageCounter
class LocalSessionMessageCounter : public MessageCounter
{
public:
static constexpr uint32_t kInitialSyncValue = 0; ///< Used for initializing peer counter
static constexpr uint32_t kInvalidMessageCounter = 0;
static constexpr uint32_t kMaxMessageCounter = 0xFFFFFFFF;
static constexpr uint32_t kMessageCounterRandomInitMask = 0x0FFFFFFF; ///< 28-bit mask

/**
Expand All @@ -88,9 +90,21 @@ class LocalSessionMessageCounter : public MessageCounter
LocalSessionMessageCounter() { mValue = (Crypto::GetRandU32() & kMessageCounterRandomInitMask) + 1; }

Type GetType() const override { return Session; }
bool IsValid() const override { return mValue != kInvalidMessageCounter; }
uint32_t Value() const override { return mValue; }
CHIP_ERROR Advance() override
{
if (mValue == kInvalidMessageCounter)
{
return CHIP_ERROR_MESSAGE_COUNTER_EXHAUSTED;
}

if (mValue == kMaxMessageCounter)
{
mValue = kInvalidMessageCounter;
return CHIP_NO_ERROR;
}

++mValue;
return CHIP_NO_ERROR;
}
Expand Down
3 changes: 2 additions & 1 deletion src/transport/PeerMessageCounter.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ namespace Transport {
class PeerMessageCounter
{
public:
static constexpr size_t kChallengeSize = 8;
static constexpr size_t kChallengeSize = 8;
static constexpr uint32_t kInitialSyncValue = 0;

PeerMessageCounter() : mStatus(Status::NotSynced) {}
~PeerMessageCounter() { Reset(); }
Expand Down
5 changes: 3 additions & 2 deletions src/transport/SessionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P

MessageCounter & counter = session->GetSessionMessageCounter().GetLocalMessageCounter();
uint32_t messageCounter = counter.Value();
VerifyOrReturnError(counter.IsValid(), CHIP_ERROR_MESSAGE_COUNTER_EXHAUSTED);
packetHeader
.SetMessageCounter(messageCounter) //
.SetSessionId(session->GetPeerSessionId()) //
Expand Down Expand Up @@ -408,7 +409,7 @@ CHIP_ERROR SessionManager::InjectPaseSessionWithTestKey(SessionHolder & sessionH
ByteSpan secret(reinterpret_cast<const uint8_t *>(CHIP_CONFIG_TEST_SHARED_SECRET_VALUE), secretLen);
ReturnErrorOnFailure(secureSession->GetCryptoContext().InitFromSecret(
secret, ByteSpan(nullptr, 0), CryptoContext::SessionInfoType::kSessionEstablishment, role));
secureSession->GetSessionMessageCounter().GetPeerMessageCounter().SetCounter(LocalSessionMessageCounter::kInitialSyncValue);
secureSession->GetSessionMessageCounter().GetPeerMessageCounter().SetCounter(Transport::PeerMessageCounter::kInitialSyncValue);
sessionHolder.Grab(session.Value());
return CHIP_NO_ERROR;
}
Expand Down Expand Up @@ -675,7 +676,7 @@ void SessionManager::SecureGroupMessageDispatch(const PacketHeader & packetHeade

// Handle Group message counter here spec 4.7.3
// spec 4.5.1.2 for msg counter
chip::Transport::PeerMessageCounter * counter = nullptr;
Transport::PeerMessageCounter * counter = nullptr;

if (CHIP_NO_ERROR ==
mGroupPeerMsgCounter.FindOrAddPeer(groupContext.fabric_index, packetHeader.GetSourceNodeId().Value(),
Expand Down
4 changes: 2 additions & 2 deletions src/transport/tests/TestPeerMessageCounter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ void UnicastSmallStepTest(nlTestSuite * inSuite, void * inContext)
for (uint32_t k = 1; k <= 2 * CHIP_CONFIG_MESSAGE_COUNTER_WINDOW_SIZE; k++)
{
chip::Transport::PeerMessageCounter counter;
counter.SetCounter(LocalSessionMessageCounter::kInitialSyncValue);
counter.SetCounter(chip::Transport::PeerMessageCounter::kInitialSyncValue);
if (counter.VerifyEncryptedUnicast(n) == CHIP_NO_ERROR)
{
// Act like we got this counter value on the wire.
Expand Down Expand Up @@ -259,7 +259,7 @@ void UnicastLargeStepTest(nlTestSuite * inSuite, void * inContext)
for (uint32_t k = (static_cast<uint32_t>(1 << 31) - 5); k <= (static_cast<uint32_t>(1 << 31) - 1); k++)
{
chip::Transport::PeerMessageCounter counter;
counter.SetCounter(LocalSessionMessageCounter::kInitialSyncValue);
counter.SetCounter(chip::Transport::PeerMessageCounter::kInitialSyncValue);
if (counter.VerifyEncryptedUnicast(n) == CHIP_NO_ERROR)
{
// Act like we got this counter value on the wire.
Expand Down

0 comments on commit eda0ae1

Please sign in to comment.