From a0dd7afe5d8c52dee91e86435c8156a80170ad55 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Tue, 16 Mar 2021 06:50:31 -0700 Subject: [PATCH] Use SecureSessionHandle to indicate if the peer's group key message counter is not synchronized. (#5368) --- src/messaging/ExchangeMgr.cpp | 2 +- src/transport/SecureSessionMgr.cpp | 10 +++++++--- src/transport/SecureSessionMgr.h | 5 +++++ src/transport/raw/MessageHeader.h | 26 ++------------------------ 4 files changed, 15 insertions(+), 28 deletions(-) diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index ba544d1965c279..bec3eb946eeb8c 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -233,7 +233,7 @@ void ExchangeManager::OnMessageReceived(const PacketHeader & packetHeader, const UnsolicitedMessageHandler * matchingUMH = nullptr; bool sendAckAndCloseExchange = false; - if (!IsMsgCounterSyncMessage(payloadHeader) && packetHeader.IsPeerGroupMsgIdNotSynchronized()) + if (!IsMsgCounterSyncMessage(payloadHeader) && session.IsPeerGroupMsgIdNotSynchronized()) { Transport::PeerConnectionState * state = mSessionMgr->GetPeerConnectionState(session); VerifyOrReturn(state != nullptr); diff --git a/src/transport/SecureSessionMgr.cpp b/src/transport/SecureSessionMgr.cpp index 8c4cad670ba81e..74d92154a54f4b 100644 --- a/src/transport/SecureSessionMgr.cpp +++ b/src/transport/SecureSessionMgr.cpp @@ -327,6 +327,7 @@ void SecureSessionMgr::OnMessageReceived(const PacketHeader & packetHeader, cons PacketBufferHandle origMsg; PayloadHeader payloadHeader; + bool peerGroupMsgIdNotSynchronized = false; Transport::AdminPairingInfo * admin = nullptr; VerifyOrExit(!msg.IsNull(), ChipLogError(Inet, "Secure transport received NULL packet, discarding")); @@ -375,14 +376,17 @@ void SecureSessionMgr::OnMessageReceived(const PacketHeader & packetHeader, cons // For all group messages, Set flag if peer group key message counter is not synchronized. if (ChipKeyId::IsAppGroupKey(packetHeader.GetEncryptionKeyID())) { - const_cast(packetHeader).SetPeerGroupMsgIdNotSynchronized(true); + peerGroupMsgIdNotSynchronized = true; } } if (mCB != nullptr) { - mCB->OnMessageReceived(packetHeader, payloadHeader, { state->GetPeerNodeId(), state->GetPeerKeyID(), state->GetAdminId() }, - std::move(msg), this); + SecureSessionHandle session(state->GetPeerNodeId(), state->GetPeerKeyID(), state->GetAdminId()); + + session.SetPeerGroupMsgIdNotSynchronized(peerGroupMsgIdNotSynchronized); + + mCB->OnMessageReceived(packetHeader, payloadHeader, session, std::move(msg), this); } exit: diff --git a/src/transport/SecureSessionMgr.h b/src/transport/SecureSessionMgr.h index eeef685420e596..64d4e57d6c4eb0 100644 --- a/src/transport/SecureSessionMgr.h +++ b/src/transport/SecureSessionMgr.h @@ -57,6 +57,9 @@ class SecureSessionHandle Transport::AdminId GetAdminId() const { return mAdmin; } void SetAdminId(Transport::AdminId adminId) { mAdmin = adminId; } + bool IsPeerGroupMsgIdNotSynchronized() const { return mPeerGroupMsgIdNotSynchronized; } + void SetPeerGroupMsgIdNotSynchronized(bool value) { mPeerGroupMsgIdNotSynchronized = value; } + bool operator==(const SecureSessionHandle & that) const { return mPeerNodeId == that.mPeerNodeId && mPeerKeyId == that.mPeerKeyId && mAdmin == that.mAdmin; @@ -74,6 +77,8 @@ class SecureSessionHandle // to identify an approach that'll allow looking up the corresponding information for // such sessions. Transport::AdminId mAdmin; + + bool mPeerGroupMsgIdNotSynchronized = false; }; /** diff --git a/src/transport/raw/MessageHeader.h b/src/transport/raw/MessageHeader.h index dfe8d8cabdca18..f6d93a1d4618b6 100644 --- a/src/transport/raw/MessageHeader.h +++ b/src/transport/raw/MessageHeader.h @@ -73,12 +73,6 @@ enum class ExFlagValues : uint8_t kExchangeFlag_VendorIdPresent = 0x10, }; -enum class InternalFlagValues : uint8_t -{ - // Header flag indicates that the peer's group key message counter is not synchronized. - kPeerGroupMsgIdNotSynchronized = 0x01, -}; - enum class FlagValues : uint16_t { /// Header flag specifying that a destination node id is included in the header. @@ -95,9 +89,8 @@ enum class FlagValues : uint16_t }; -using Flags = BitFlags; -using ExFlags = BitFlags; -using InternalFlags = BitFlags; +using Flags = BitFlags; +using ExFlags = BitFlags; // Header is a 16-bit value of the form // | 4 bit | 4 bit |8 bit Security Flags| @@ -149,12 +142,6 @@ class PacketHeader /** Check if it's a secure session control message. */ bool IsSecureSessionControlMsg() const { return mFlags.Has(Header::FlagValues::kSecureSessionControlMessage); } - /** Check if the peer's group key message counter is not synchronized. */ - bool IsPeerGroupMsgIdNotSynchronized() const - { - return mInternalFlags.Has(Header::InternalFlagValues::kPeerGroupMsgIdNotSynchronized); - } - Header::EncryptionType GetEncryptionType() const { return mEncryptionType; } PacketHeader & SetSecureSessionControlMsg(bool value) @@ -163,12 +150,6 @@ class PacketHeader return *this; } - PacketHeader & SetPeerGroupMsgIdNotSynchronized(bool value) - { - mInternalFlags.Set(Header::InternalFlagValues::kPeerGroupMsgIdNotSynchronized, value); - return *this; - } - PacketHeader & SetSourceNodeId(NodeId id) { mSourceNodeId.SetValue(id); @@ -328,9 +309,6 @@ class PacketHeader /// Message flags read from the message. Header::Flags mFlags; - /// Message flags not encoded into the packet sent over wire. - Header::InternalFlags mInternalFlags; - /// Represents encryption type used for encrypting current packet Header::EncryptionType mEncryptionType = Header::EncryptionType::kAESCCMTagLen16; };