Implement Message Counter Synchronization Protocol part #4712

yufengwangca · 2021-02-08T06:50:37Z

Problem

Peer message counter synchronization is an essential part of enabling secure CHIP messaging between members of an application group.

Summary of Changes

Implement the protocol used to synchronize message counters between peers when using the same symmetric group key.

This PR does not handle the Initiating Message Counter Synchronization part which is depending on the following issues:
#4628
#4571

Fixes #4626

todo · 2021-02-08T06:50:39Z

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

connectedhomeip/src/messaging/ExchangeContext.h

Lines 191 to 197 in 17fd7cd

    
           // [TODO: #4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual 
        
           // actions with a delegate pattern. 
        
           uint8_t mChallenage[kMsgCounterChallengeSize]; // Challenge number to identify the sychronization request cryptographically. 
        
           BitFlags<uint16_t, ExFlagValues> mFlags; // Internal state flags 
        
           /**

This comment was generated by todo based on a `TODO` comment in `17fd7cd` in #4712. cc @yufengwangca.

todo · 2021-02-08T06:50:42Z

(#4641) We shall also set the C flag in the packet header, this will be used for message protection later.

connectedhomeip/src/messaging/SecureChannelMgr.cpp

Lines 130 to 140 in 17fd7cd

    
               // TODO:(#4641) We shall also set the C flag in the packet header, this will be used for message protection later. 
        
               sendFlags.Set(Messaging::SendMessageFlags::kNoAutoRequestAck, true).Set(Messaging::SendMessageFlags::kExpectResponse, true); 
        
               // Arm a timer to enforce that a MsgCounterSyncRsp is received before kMsgCounterSyncTimeout. 
        
               ec->SetResponseTimeout(kMsgCounterSyncTimeout); 
        
               // Send the message counter synchronization request in a Secure Channel Protocol::MsgCounterSyncReq message. 
        
               err = ec->SendMessage(Protocols::SecureChannel::MsgType::MsgCounterSyncReq, std::move(msgBuf), sendFlags); 
        
               SuccessOrExit(err); 
        
           exit:

This comment was generated by todo based on a `TODO` comment in `17fd7cd` in #4712. cc @yufengwangca.

todo · 2021-02-08T06:50:45Z

(#4628)Initialize/synchronize peer's message counter to FabricState.

connectedhomeip/src/messaging/SecureChannelMgr.cpp

Lines 253 to 263 in 17fd7cd

    
               // ToDo:(#4628)Initialize/synchronize peer's message counter to FabricState. 
        
               VerifyOrExit(syncCounter != 0, err = CHIP_ERROR_READ_FAILED); 
        
           exit: 
        
               if (err != CHIP_NO_ERROR) 
        
               { 
        
                   ChipLogError(ExchangeManager, "Failed to handle MsgCounterSyncResp message with error:%s", ErrorStr(err)); 
        
               } 
        
               if (ec != nullptr) 
        
                   ec->Close();

This comment was generated by todo based on a `ToDo` comment in `17fd7cd` in #4712. cc @yufengwangca.

src/messaging/ReliableMessageMgr.cpp

andy31415 · 2021-02-08T14:14:34Z

src/messaging/SecureChannelMgr.cpp

@@ -0,0 +1,269 @@
+/*
+ *
+ *    Copyright (c) 2021 Project CHIP Authors


Please remove copyright line (years are annoying to update).

@woody-apple - is this ok with you? I find updating years tedious.

I prefer a separate PR to update the Copyright header part unanimously after we finalize what should be kept and what should be removed.

andy31415 · 2021-02-08T14:15:31Z

src/messaging/SecureChannelMgr.cpp

+
+/**
+ *    @file
+ *      This file implements the CHIP Secure Channel protocol.


Do we need this @file ?

andy31415 · 2021-02-08T14:18:12Z

src/messaging/tests/TestSecureChannelMgr.cpp

@@ -0,0 +1,272 @@
+/*
+ *
+ *    Copyright (c) 2020-2021 Project CHIP Authors


If keeping this should be just 2021 probably.

However I would still remove both Copyright and All rights reserved lines.

andy31415 · 2021-02-08T14:19:39Z

src/messaging/tests/TestSecureChannelMgr.cpp

+
+/**
+ *    @file
+ *      This file implements unit tests for the ExchangeManager implementation.


I would drop this @file comment: generally I have not found any of these @file comments valuable when reading code and they seem to have a high probability of being copy & pasted - like this one I believe it should not be ExchangeManager but rather SecureChannelManager.

A comment saying that 'TestSecureChannelMgr.cpp implements unit tests for SecureChannelManager' seems redundant.

todo · 2021-02-08T17:46:26Z

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

connectedhomeip/src/messaging/ExchangeContext.h

Lines 191 to 197 in 492f55a

    
           // [TODO: #4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual 
        
           // actions with a delegate pattern. 
        
           uint8_t mChallenage[kMsgCounterChallengeSize]; // Challenge number to identify the sychronization request cryptographically. 
        
           BitFlags<uint16_t, ExFlagValues> mFlags; // Internal state flags 
        
           /**

This comment was generated by todo based on a `TODO` comment in `492f55a` in #4712. cc @yufengwangca.

todo · 2021-02-08T17:46:29Z

(#4628)Initialize/synchronize peer's message counter to FabricState.

connectedhomeip/src/messaging/SecureChannelMgr.cpp

Lines 253 to 263 in 492f55a

    
               // ToDo:(#4628)Initialize/synchronize peer's message counter to FabricState. 
        
               VerifyOrExit(syncCounter != 0, err = CHIP_ERROR_READ_FAILED); 
        
           exit: 
        
               if (err != CHIP_NO_ERROR) 
        
               { 
        
                   ChipLogError(ExchangeManager, "Failed to handle MsgCounterSyncResp message with error:%s", ErrorStr(err)); 
        
               } 
        
               if (ec != nullptr) 
        
                   ec->Close();

This comment was generated by todo based on a `ToDo` comment in `492f55a` in #4712. cc @yufengwangca.

todo · 2021-02-08T17:58:51Z

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

connectedhomeip/src/messaging/ExchangeContext.h

Lines 191 to 197 in 7e5a1bb

    
           // [TODO: #4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual 
        
           // actions with a delegate pattern. 
        
           uint8_t mChallenage[kMsgCounterChallengeSize]; // Challenge number to identify the sychronization request cryptographically. 
        
           BitFlags<uint16_t, ExFlagValues> mFlags; // Internal state flags 
        
           /**

This comment was generated by todo based on a `TODO` comment in `7e5a1bb` in #4712. cc @yufengwangca.

todo · 2021-02-08T17:58:54Z

(#4628)Initialize/synchronize peer's message counter to FabricState.

connectedhomeip/src/messaging/SecureChannelMgr.cpp

Lines 254 to 264 in 7e5a1bb

    
               // ToDo:(#4628)Initialize/synchronize peer's message counter to FabricState. 
        
               VerifyOrExit(syncCounter != 0, err = CHIP_ERROR_READ_FAILED); 
        
           exit: 
        
               if (err != CHIP_NO_ERROR) 
        
               { 
        
                   ChipLogError(ExchangeManager, "Failed to handle MsgCounterSyncResp message with error:%s", ErrorStr(err)); 
        
               } 
        
               if (ec != nullptr) 
        
                   ec->Close();

This comment was generated by todo based on a `ToDo` comment in `7e5a1bb` in #4712. cc @yufengwangca.

todo · 2021-02-08T18:32:47Z

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

connectedhomeip/src/messaging/ExchangeContext.h

Lines 191 to 197 in 16fd9bb

    
           // [TODO: #4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual 
        
           // actions with a delegate pattern. 
        
           uint8_t mChallenage[kMsgCounterChallengeSize]; // Challenge number to identify the sychronization request cryptographically. 
        
           BitFlags<uint16_t, ExFlagValues> mFlags; // Internal state flags 
        
           /**

This comment was generated by todo based on a `TODO` comment in `16fd9bb` in #4712. cc @yufengwangca.

todo · 2021-02-08T18:32:50Z

(#4628)Initialize/synchronize peer's message counter to FabricState.

connectedhomeip/src/messaging/SecureChannelMgr.cpp

Lines 254 to 264 in 16fd9bb

    
               // ToDo:(#4628)Initialize/synchronize peer's message counter to FabricState. 
        
               VerifyOrExit(syncCounter != 0, err = CHIP_ERROR_READ_FAILED); 
        
           exit: 
        
               if (err != CHIP_NO_ERROR) 
        
               { 
        
                   ChipLogError(ExchangeManager, "Failed to handle MsgCounterSyncResp message with error:%s", ErrorStr(err)); 
        
               } 
        
               if (ec != nullptr) 
        
                   ec->Close();

This comment was generated by todo based on a `ToDo` comment in `16fd9bb` in #4712. cc @yufengwangca.

src/messaging/ExchangeContext.h

todo · 2021-02-08T20:30:35Z

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

connectedhomeip/src/messaging/ExchangeContext.h

Lines 191 to 197 in 1db754e

    
           // [TODO: #4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual 
        
           // actions with a delegate pattern. 
        
           uint8_t mChallenge[kMsgCounterChallengeSize]; // Challenge number to identify the sychronization request cryptographically. 
        
           BitFlags<uint16_t, ExFlagValues> mFlags; // Internal state flags 
        
           /**

This comment was generated by todo based on a `TODO` comment in `1db754e` in #4712. cc @yufengwangca.

todo · 2021-02-08T20:30:38Z

(#4628)Initialize/synchronize peer's message counter to FabricState.

connectedhomeip/src/messaging/SecureChannelMgr.cpp

Lines 254 to 264 in 1db754e

    
               // ToDo:(#4628)Initialize/synchronize peer's message counter to FabricState. 
        
               VerifyOrExit(syncCounter != 0, err = CHIP_ERROR_READ_FAILED); 
        
           exit: 
        
               if (err != CHIP_NO_ERROR) 
        
               { 
        
                   ChipLogError(ExchangeManager, "Failed to handle MsgCounterSyncResp message with error:%s", ErrorStr(err)); 
        
               } 
        
               if (ec != nullptr) 
        
                   ec->Close();

This comment was generated by todo based on a `ToDo` comment in `1db754e` in #4712. cc @yufengwangca.

src/messaging/SecureChannelMgr.cpp

todo · 2021-02-08T22:47:47Z

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

connectedhomeip/src/messaging/ExchangeContext.h

Lines 191 to 197 in b949ac5

    
           // [TODO: #4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual 
        
           // actions with a delegate pattern. 
        
           uint8_t mChallenge[kMsgCounterChallengeSize]; // Challenge number to identify the sychronization request cryptographically. 
        
           BitFlags<uint16_t, ExFlagValues> mFlags; // Internal state flags 
        
           /**

This comment was generated by todo based on a `TODO` comment in `b949ac5` in #4712. cc @yufengwangca.

todo · 2021-02-08T22:47:50Z

(#4628)Initialize/synchronize peer's message counter to FabricState.

connectedhomeip/src/messaging/SecureChannelMgr.cpp

Lines 251 to 261 in b949ac5

    
               // ToDo:(#4628)Initialize/synchronize peer's message counter to FabricState. 
        
               VerifyOrExit(syncCounter != 0, err = CHIP_ERROR_READ_FAILED); 
        
           exit: 
        
               if (err != CHIP_NO_ERROR) 
        
               { 
        
                   ChipLogError(ExchangeManager, "Failed to handle MsgCounterSyncResp message with error:%s", ErrorStr(err)); 
        
               } 
        
               if (exchangeContext != nullptr) 
        
                   exchangeContext->Close();

This comment was generated by todo based on a `ToDo` comment in `b949ac5` in #4712. cc @yufengwangca.

todo · 2021-02-09T20:24:33Z

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

connectedhomeip/src/messaging/ExchangeContext.h

Lines 191 to 197 in 2703eab

    
           // [TODO: #4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual 
        
           // actions with a delegate pattern. 
        
           uint8_t mChallenge[kMsgCounterChallengeSize]; // Challenge number to identify the sychronization request cryptographically. 
        
           BitFlags<uint16_t, ExFlagValues> mFlags; // Internal state flags 
        
           /**

This comment was generated by todo based on a `TODO` comment in `2703eab` in #4712. cc @yufengwangca.

todo · 2021-02-09T20:24:36Z

(#4628)Initialize/synchronize peer's message counter to FabricState.

connectedhomeip/src/messaging/SecureChannelMgr.cpp

Lines 251 to 261 in 2703eab

    
               // ToDo:(#4628)Initialize/synchronize peer's message counter to FabricState. 
        
               VerifyOrExit(syncCounter != 0, err = CHIP_ERROR_READ_FAILED); 
        
           exit: 
        
               if (err != CHIP_NO_ERROR) 
        
               { 
        
                   ChipLogError(ExchangeManager, "Failed to handle MsgCounterSyncResp message with error:%s", ErrorStr(err)); 
        
               } 
        
               if (exchangeContext != nullptr) 
        
                   exchangeContext->Close();

This comment was generated by todo based on a `ToDo` comment in `2703eab` in #4712. cc @yufengwangca.

woody-apple · 2021-02-10T18:52:45Z

@pan-apple ?

todo · 2021-02-11T19:26:30Z

(#4628)Initialize/synchronize peer's message counter to FabricState.

connectedhomeip/src/messaging/MessageCounterSync.cpp

Lines 251 to 261 in 1d4d7d5

    
               // ToDo:(#4628)Initialize/synchronize peer's message counter to FabricState. 
        
               VerifyOrExit(syncCounter != 0, err = CHIP_ERROR_READ_FAILED); 
        
           exit: 
        
               if (err != CHIP_NO_ERROR) 
        
               { 
        
                   ChipLogError(ExchangeManager, "Failed to handle MsgCounterSyncResp message with error:%s", ErrorStr(err)); 
        
               } 
        
               if (exchangeContext != nullptr) 
        
                   exchangeContext->Close();

This comment was generated by todo based on a `ToDo` comment in `1d4d7d5` in #4712. cc @yufengwangca.

yufengwangca · 2021-02-11T19:32:08Z

@yufengwangca , the file name SecureChannelMgr seems to specifically implement MessageCounterSync protocol. The current file name is confusing, given we already have SecureSessionMgr class in the code base. How do you feel about renaming SecureChannelMgr.cpp/.h to MessageCounterSync.cpp/.h?

Acked, SecureChannelMgr was originally designed to run MCSP and CASE protocols, since we have figured out how to implement multiple CASE sessions using ephemeral TCP ports within Transport layer, we can rename SecureChannelMgr to MessageCounterSyncMgr

todo · 2021-02-11T19:35:31Z

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

connectedhomeip/src/messaging/ExchangeContext.h

Lines 191 to 197 in 187aad2

    
           // [TODO: #4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual 
        
           // actions with a delegate pattern. 
        
           uint8_t mChallenge[kMsgCounterChallengeSize]; // Challenge number to identify the sychronization request cryptographically. 
        
           BitFlags<uint16_t, ExFlagValues> mFlags; // Internal state flags 
        
           /**

This comment was generated by todo based on a `TODO` comment in `187aad2` in #4712. cc @yufengwangca.

todo · 2021-02-11T19:35:34Z

(#4628)Initialize/synchronize peer's message counter to FabricState.

connectedhomeip/src/messaging/MessageCounterSync.cpp

Lines 251 to 261 in 187aad2

    
               // ToDo:(#4628)Initialize/synchronize peer's message counter to FabricState. 
        
               VerifyOrExit(syncCounter != 0, err = CHIP_ERROR_READ_FAILED); 
        
           exit: 
        
               if (err != CHIP_NO_ERROR) 
        
               { 
        
                   ChipLogError(ExchangeManager, "Failed to handle MsgCounterSyncResp message with error:%s", ErrorStr(err)); 
        
               } 
        
               if (exchangeContext != nullptr) 
        
                   exchangeContext->Close();

This comment was generated by todo based on a `ToDo` comment in `187aad2` in #4712. cc @yufengwangca.

todo · 2021-02-12T00:12:32Z

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

connectedhomeip/src/messaging/ExchangeContext.h

Lines 191 to 197 in 11dc138

    
           // [TODO: #4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual 
        
           // actions with a delegate pattern. 
        
           uint8_t mChallenge[kMsgCounterChallengeSize]; // Challenge number to identify the sychronization request cryptographically. 
        
           BitFlags<uint16_t, ExFlagValues> mFlags; // Internal state flags 
        
           /**

This comment was generated by todo based on a `TODO` comment in `11dc138` in #4712. cc @yufengwangca.

todo · 2021-02-12T00:12:35Z

(#4628)Initialize/synchronize peer's message counter to FabricState.

connectedhomeip/src/messaging/MessageCounterSync.cpp

Lines 251 to 261 in 11dc138

    
               // ToDo:(#4628)Initialize/synchronize peer's message counter to FabricState. 
        
               VerifyOrExit(syncCounter != 0, err = CHIP_ERROR_READ_FAILED); 
        
           exit: 
        
               if (err != CHIP_NO_ERROR) 
        
               { 
        
                   ChipLogError(ExchangeManager, "Failed to handle MsgCounterSyncResp message with error:%s", ErrorStr(err)); 
        
               } 
        
               if (exchangeContext != nullptr) 
        
                   exchangeContext->Close();

This comment was generated by todo based on a `ToDo` comment in `11dc138` in #4712. cc @yufengwangca.

yufengwangca · 2021-02-12T18:53:14Z

@pan-apple @woody-apple ?

pan-apple

@yufengwangca , there are some minor comments. Once those are addressed, it looks good to me.

src/messaging/ExchangeMgr.h

src/messaging/MessageCounterSync.cpp

pan-apple · 2021-02-15T23:34:25Z

src/messaging/ExchangeContext.h

@@ -33,6 +33,9 @@
 #include <transport/SecureSessionMgr.h>

 namespace chip {
+
+constexpr uint16_t kMsgCounterChallengeSize = 8; // The size of the message counter synchronization request message.


Should we move this to MessageCounterSync.h?

This will cause mutual including during compiling.

The mChallenge should not be declared in ExchangeContext.h, it will be moved to MessageCounterSync.h after:
// [TODO: #4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual actions with a delegate pattern.

todo · 2021-02-16T17:27:29Z

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

connectedhomeip/src/messaging/ExchangeContext.h

Lines 191 to 197 in 979fa5d

    
           // [TODO: #4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual 
        
           // actions with a delegate pattern. 
        
           uint8_t mChallenge[kMsgCounterChallengeSize]; // Challenge number to identify the sychronization request cryptographically. 
        
           BitFlags<uint16_t, ExFlagValues> mFlags; // Internal state flags 
        
           /**

This comment was generated by todo based on a `TODO` comment in `979fa5d` in #4712. cc @yufengwangca.

todo · 2021-02-16T17:27:32Z

(#4628)Initialize/synchronize peer's message counter to FabricState.

connectedhomeip/src/messaging/MessageCounterSync.cpp

Lines 250 to 260 in 979fa5d

    
               // ToDo:(#4628)Initialize/synchronize peer's message counter to FabricState. 
        
               VerifyOrExit(syncCounter != 0, err = CHIP_ERROR_READ_FAILED); 
        
           exit: 
        
               if (err != CHIP_NO_ERROR) 
        
               { 
        
                   ChipLogError(ExchangeManager, "Failed to handle MsgCounterSyncResp message with error:%s", ErrorStr(err)); 
        
               } 
        
               if (exchangeContext != nullptr) 
        
                   exchangeContext->Close();

This comment was generated by todo based on a `ToDo` comment in `979fa5d` in #4712. cc @yufengwangca.

todo · 2021-02-16T18:39:22Z

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

connectedhomeip/src/messaging/ExchangeContext.h

Lines 207 to 213 in 2428bc8

    
           // [TODO: #4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual 
        
           // actions with a delegate pattern. 
        
           uint8_t mChallenge[kMsgCounterChallengeSize]; // Challenge number to identify the sychronization request cryptographically. 
        
           BitFlags<uint16_t, ExFlagValues> mFlags; // Internal state flags 
        
           /**

This comment was generated by todo based on a `TODO` comment in `2428bc8` in #4712. cc @yufengwangca.

todo · 2021-02-16T18:39:25Z

(#4628)Initialize/synchronize peer's message counter to FabricState.

connectedhomeip/src/messaging/MessageCounterSync.cpp

Lines 250 to 260 in 2428bc8

    
               // ToDo:(#4628)Initialize/synchronize peer's message counter to FabricState. 
        
               VerifyOrExit(syncCounter != 0, err = CHIP_ERROR_READ_FAILED); 
        
           exit: 
        
               if (err != CHIP_NO_ERROR) 
        
               { 
        
                   ChipLogError(ExchangeManager, "Failed to handle MsgCounterSyncResp message with error:%s", ErrorStr(err)); 
        
               } 
        
               if (exchangeContext != nullptr) 
        
                   exchangeContext->Close();

This comment was generated by todo based on a `ToDo` comment in `2428bc8` in #4712. cc @yufengwangca.

todo · 2021-02-17T07:46:21Z

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

connectedhomeip/src/messaging/ExchangeContext.h

Lines 207 to 213 in 7bd07d8

    
           // [TODO: #4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual 
        
           // actions with a delegate pattern. 
        
           uint8_t mChallenge[kMsgCounterChallengeSize]; // Challenge number to identify the sychronization request cryptographically. 
        
           BitFlags<uint16_t, ExFlagValues> mFlags; // Internal state flags 
        
           /**

This comment was generated by todo based on a `TODO` comment in `7bd07d8` in #4712. cc @yufengwangca.

todo · 2021-02-17T07:46:24Z

(#4628)Initialize/synchronize peer's message counter to FabricState.

connectedhomeip/src/messaging/MessageCounterSync.cpp

Lines 257 to 267 in 7bd07d8

    
               // ToDo:(#4628)Initialize/synchronize peer's message counter to FabricState. 
        
           exit: 
        
               if (err != CHIP_NO_ERROR) 
        
               { 
        
                   ChipLogError(ExchangeManager, "Failed to handle MsgCounterSyncResp message with error:%s", ErrorStr(err)); 
        
               } 
        
               if (exchangeContext != nullptr) 
        
                   exchangeContext->Close();

This comment was generated by todo based on a `ToDo` comment in `7bd07d8` in #4712. cc @yufengwangca.

todo · 2021-02-17T08:19:18Z

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

connectedhomeip/src/messaging/ExchangeContext.h

Lines 207 to 213 in 2428bc8

    
           // [TODO: #4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual 
        
           // actions with a delegate pattern. 
        
           uint8_t mChallenge[kMsgCounterChallengeSize]; // Challenge number to identify the sychronization request cryptographically. 
        
           BitFlags<uint16_t, ExFlagValues> mFlags; // Internal state flags 
        
           /**

This comment was generated by todo based on a `TODO` comment in `2428bc8` in #4712. cc @yufengwangca.

todo · 2021-02-17T08:19:21Z

(#4628)Initialize/synchronize peer's message counter to FabricState.

connectedhomeip/src/messaging/MessageCounterSync.cpp

Lines 250 to 260 in 2428bc8

    
               // ToDo:(#4628)Initialize/synchronize peer's message counter to FabricState. 
        
               VerifyOrExit(syncCounter != 0, err = CHIP_ERROR_READ_FAILED); 
        
           exit: 
        
               if (err != CHIP_NO_ERROR) 
        
               { 
        
                   ChipLogError(ExchangeManager, "Failed to handle MsgCounterSyncResp message with error:%s", ErrorStr(err)); 
        
               } 
        
               if (exchangeContext != nullptr) 
        
                   exchangeContext->Close();

This comment was generated by todo based on a `ToDo` comment in `2428bc8` in #4712. cc @yufengwangca.

…ect-chip#4712)" This reverts commit a949773.

boring-cyborg bot added the transport label Feb 8, 2021

pullapprove bot requested review from andy31415, BroderickCarlin, bzbarsky-apple, chrisdecenzo, hawk248, jelderton, mspang, saurabhst and woody-apple February 8, 2021 06:51

yufengwangca changed the title ~~Implement Message Counter Synchronization Protocol (MCSP) part~~ Implement Message Counter Synchronization Protocol part Feb 8, 2021

andy31415 approved these changes Feb 8, 2021

View reviewed changes

mspang reviewed Feb 8, 2021

View reviewed changes

src/messaging/ExchangeContext.h Outdated Show resolved Hide resolved

mspang reviewed Feb 8, 2021

View reviewed changes

src/messaging/SecureChannelMgr.cpp Outdated Show resolved Hide resolved

src/messaging/SecureChannelMgr.cpp Outdated Show resolved Hide resolved

pan-apple approved these changes Feb 15, 2021

View reviewed changes

jelderton approved these changes Feb 16, 2021

View reviewed changes

yufengwangca added 3 commits February 16, 2021 10:22

Implement Message Counter Synchronization Protocol (MCSP) part

80d5dd0

Address review comments.

7892d91

Rename SecureChannelMgr to MessageCounterSyncMgr

2428bc8

msandstedt approved these changes Feb 16, 2021

View reviewed changes

boring-cyborg bot added the lib label Feb 17, 2021

msandstedt merged commit a949773 into project-chip:master Feb 17, 2021

msandstedt added a commit to msandstedt/connectedhomeip that referenced this pull request Feb 17, 2021

Revert "Implement Message Counter Synchronization Protocol part (proj…

6e8de2d

…ect-chip#4712)" This reverts commit a949773.

msandstedt mentioned this pull request Feb 17, 2021

Revert "Implement Message Counter Synchronization Protocol part (#4712)" #4910

Closed

yufengwangca mentioned this pull request Feb 17, 2021

Update TestMessageCounterSyncMgr with the latest API change for NewPa… #4913

Merged

yufengwangca deleted the pr/message/mcsp branch February 17, 2021 20:46

Implement Message Counter Synchronization Protocol part #4712

Implement Message Counter Synchronization Protocol part #4712

Conversation

yufengwangca commented Feb 8, 2021

Problem

Summary of Changes

todo bot commented Feb 8, 2021

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

This comment was generated by todo based on a TODO comment in 17fd7cd in #4712. cc @yufengwangca.

todo bot commented Feb 8, 2021

(#4641) We shall also set the C flag in the packet header, this will be used for message protection later.

This comment was generated by todo based on a TODO comment in 17fd7cd in #4712. cc @yufengwangca.

todo bot commented Feb 8, 2021

(#4628)Initialize/synchronize peer's message counter to FabricState.

This comment was generated by todo based on a ToDo comment in 17fd7cd in #4712. cc @yufengwangca.

andy31415 Feb 8, 2021

Choose a reason for hiding this comment

yufengwangca Feb 8, 2021

Choose a reason for hiding this comment

andy31415 Feb 8, 2021

Choose a reason for hiding this comment

andy31415 Feb 8, 2021

Choose a reason for hiding this comment

andy31415 Feb 8, 2021

Choose a reason for hiding this comment

todo bot commented Feb 8, 2021

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

This comment was generated by todo based on a TODO comment in 492f55a in #4712. cc @yufengwangca.

todo bot commented Feb 8, 2021

(#4628)Initialize/synchronize peer's message counter to FabricState.

This comment was generated by todo based on a ToDo comment in 492f55a in #4712. cc @yufengwangca.

todo bot commented Feb 8, 2021

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

This comment was generated by todo based on a TODO comment in 7e5a1bb in #4712. cc @yufengwangca.

todo bot commented Feb 8, 2021

(#4628)Initialize/synchronize peer's message counter to FabricState.

This comment was generated by todo based on a ToDo comment in 7e5a1bb in #4712. cc @yufengwangca.

todo bot commented Feb 8, 2021

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

This comment was generated by todo based on a TODO comment in 16fd9bb in #4712. cc @yufengwangca.

todo bot commented Feb 8, 2021

(#4628)Initialize/synchronize peer's message counter to FabricState.

This comment was generated by todo based on a ToDo comment in 16fd9bb in #4712. cc @yufengwangca.

todo bot commented Feb 8, 2021

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

This comment was generated by todo based on a TODO comment in 1db754e in #4712. cc @yufengwangca.

todo bot commented Feb 8, 2021

(#4628)Initialize/synchronize peer's message counter to FabricState.

This comment was generated by todo based on a ToDo comment in 1db754e in #4712. cc @yufengwangca.

todo bot commented Feb 8, 2021

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

This comment was generated by todo based on a TODO comment in b949ac5 in #4712. cc @yufengwangca.

todo bot commented Feb 8, 2021

(#4628)Initialize/synchronize peer's message counter to FabricState.

This comment was generated by todo based on a ToDo comment in b949ac5 in #4712. cc @yufengwangca.

todo bot commented Feb 9, 2021

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

This comment was generated by todo based on a TODO comment in 2703eab in #4712. cc @yufengwangca.

todo bot commented Feb 9, 2021

(#4628)Initialize/synchronize peer's message counter to FabricState.

This comment was generated by todo based on a ToDo comment in 2703eab in #4712. cc @yufengwangca.

woody-apple commented Feb 10, 2021

todo bot commented Feb 11, 2021

(#4628)Initialize/synchronize peer's message counter to FabricState.

This comment was generated by todo based on a ToDo comment in 1d4d7d5 in #4712. cc @yufengwangca.

yufengwangca commented Feb 11, 2021

todo bot commented Feb 11, 2021

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

This comment was generated by todo based on a TODO comment in 187aad2 in #4712. cc @yufengwangca.

todo bot commented Feb 11, 2021

(#4628)Initialize/synchronize peer's message counter to FabricState.

This comment was generated by todo based on a ToDo comment in 187aad2 in #4712. cc @yufengwangca.

todo bot commented Feb 12, 2021

#4711]: this field need to be moved to appState object which implement 'exchange-specific' contextual

This comment was generated by todo based on a TODO comment in 11dc138 in #4712. cc @yufengwangca.

todo bot commented Feb 12, 2021

(#4628)Initialize/synchronize peer's message counter to FabricState.

This comment was generated by todo based on a ToDo comment in 11dc138 in #4712. cc @yufengwangca.

yufengwangca commented Feb 12, 2021

pan-apple left a comment

This comment was generated by todo based on a `TODO` comment in `17fd7cd` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `TODO` comment in `17fd7cd` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `ToDo` comment in `17fd7cd` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `TODO` comment in `492f55a` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `ToDo` comment in `492f55a` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `TODO` comment in `7e5a1bb` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `ToDo` comment in `7e5a1bb` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `TODO` comment in `16fd9bb` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `ToDo` comment in `16fd9bb` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `TODO` comment in `1db754e` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `ToDo` comment in `1db754e` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `TODO` comment in `b949ac5` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `ToDo` comment in `b949ac5` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `TODO` comment in `2703eab` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `ToDo` comment in `2703eab` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `ToDo` comment in `1d4d7d5` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `TODO` comment in `187aad2` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `ToDo` comment in `187aad2` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `TODO` comment in `11dc138` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `ToDo` comment in `11dc138` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `TODO` comment in `979fa5d` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `ToDo` comment in `979fa5d` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `TODO` comment in `2428bc8` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `ToDo` comment in `2428bc8` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `TODO` comment in `7bd07d8` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `ToDo` comment in `7bd07d8` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `TODO` comment in `2428bc8` in #4712. cc @yufengwangca.

This comment was generated by todo based on a `ToDo` comment in `2428bc8` in #4712. cc @yufengwangca.