Update Message Header with spec (#10325)

* Update Message Header with spec

* Fix Unit Tests

* Fix style

* fix pr comments

* Fix cirque test

* Fix restyle

examples/shell/shell_common/cmd_ping.cpp

@@ -328,7 +328,7 @@ void StartPinging(streamer_t * stream, char * destination)
     err = EstablishSecureSession(stream, GetEchoPeerAddress());
     SuccessOrExit(err);
 
-    err = gEchoClient.Init(&gExchangeManager, SessionHandle(kTestDeviceNodeId, 0, 0, gFabricIndex));
+    err = gEchoClient.Init(&gExchangeManager, SessionHandle(kTestDeviceNodeId, 1, 1, gFabricIndex));
     SuccessOrExit(err);
 
     // Arrange to get a callback whenever an Echo Response is received.

examples/shell/shell_common/cmd_send.cpp

@@ -127,7 +127,7 @@ CHIP_ERROR SendMessage(streamer_t * stream)
     uint32_t payloadSize = gSendArguments.GetPayloadSize();
 
     // Create a new exchange context.
-    auto * ec = gExchangeManager.NewContext(SessionHandle(kTestDeviceNodeId, 0, 0, gFabricIndex), &gMockAppDelegate);
+    auto * ec = gExchangeManager.NewContext(SessionHandle(kTestDeviceNodeId, 1, 1, gFabricIndex), &gMockAppDelegate);
     VerifyOrExit(ec != nullptr, err = CHIP_ERROR_NO_MEMORY);
 
     payloadBuf = MessagePacketBuffer::New(payloadSize);

src/app/CommandSender.cpp

@@ -48,7 +48,7 @@ CHIP_ERROR CommandSender::SendCommandRequest(NodeId aNodeId, FabricIndex aFabric
     SuccessOrExit(err);
 
     // Create a new exchange context.
-    mpExchangeCtx = mpExchangeMgr->NewContext(secureSession.ValueOr(SessionHandle(aNodeId, 0, 0, aFabricIndex)), this);
+    mpExchangeCtx = mpExchangeMgr->NewContext(secureSession.ValueOr(SessionHandle(aNodeId, 1, 1, aFabricIndex)), this);
     VerifyOrExit(mpExchangeCtx != nullptr, err = CHIP_ERROR_NO_MEMORY);
 
     mpExchangeCtx->SetResponseTimeout(timeout);

src/app/WriteClient.cpp

@@ -258,7 +258,7 @@ CHIP_ERROR WriteClient::SendWriteRequest(NodeId aNodeId, FabricIndex aFabricInde
     ClearExistingExchangeContext();
 
     // Create a new exchange context.
-    mpExchangeCtx = mpExchangeMgr->NewContext(apSecureSession.ValueOr(SessionHandle(aNodeId, 0, 0, aFabricIndex)), this);
+    mpExchangeCtx = mpExchangeMgr->NewContext(apSecureSession.ValueOr(SessionHandle(aNodeId, 1, 1, aFabricIndex)), this);
     VerifyOrExit(mpExchangeCtx != nullptr, err = CHIP_ERROR_NO_MEMORY);
     mpExchangeCtx->SetResponseTimeout(timeout);
 

src/app/tests/integration/chip_im_initiator.cpp

@@ -313,7 +313,7 @@ CHIP_ERROR SendReadRequest()
 
     printf("\nSend read request message to Node: %" PRIu64 "\n", chip::kTestDeviceNodeId);
 
-    chip::app::ReadPrepareParams readPrepareParams(chip::SessionHandle(chip::kTestDeviceNodeId, 0, 0, gFabricIndex));
+    chip::app::ReadPrepareParams readPrepareParams(chip::SessionHandle(chip::kTestDeviceNodeId, 1, 1, gFabricIndex));
     readPrepareParams.mTimeout                     = gMessageTimeoutMsec;
     readPrepareParams.mpAttributePathParamsList    = &attributePathParams;
     readPrepareParams.mAttributePathParamsListSize = 1;
@@ -374,7 +374,7 @@ CHIP_ERROR SendSubscribeRequest()
     CHIP_ERROR err   = CHIP_NO_ERROR;
     gLastMessageTime = chip::System::SystemClock().GetMonotonicMilliseconds();
 
-    chip::app::ReadPrepareParams readPrepareParams(chip::SessionHandle(chip::kTestDeviceNodeId, 0, 0, gFabricIndex));
+    chip::app::ReadPrepareParams readPrepareParams(chip::SessionHandle(chip::kTestDeviceNodeId, 1, 1, gFabricIndex));
     chip::app::EventPathParams eventPathParams[2];
     chip::app::AttributePathParams attributePathParams[1];
     readPrepareParams.mpEventPathParamsList                = eventPathParams;

src/crypto/CHIPCryptoPAL.h

@@ -50,6 +50,8 @@ constexpr size_t kSHA1_Hash_Length                = 20;
 constexpr size_t CHIP_CRYPTO_GROUP_SIZE_BYTES      = kP256_FE_Length;
 constexpr size_t CHIP_CRYPTO_PUBLIC_KEY_SIZE_BYTES = kP256_Point_Length;
 
+constexpr size_t CHIP_CRYPTO_AEAD_MIC_LENGTH_BYTES = 16;
+
 constexpr size_t kMax_ECDH_Secret_Length     = kP256_FE_Length;
 constexpr size_t kMax_ECDSA_Signature_Length = kP256_ECDSA_Signature_Length_Raw;
 constexpr size_t kMAX_FE_Length              = kP256_FE_Length;

src/messaging/ExchangeContext.cpp

@@ -299,7 +299,7 @@ bool ExchangeContext::MatchExchange(SessionHandle session, const PacketHeader &
 
         // TODO: This check should be already implied by the equality of session check,
         // It should be removed after we have implemented the temporary node id for PASE and CASE sessions
-        && (IsEncryptionRequired() == packetHeader.GetFlags().Has(Header::FlagValues::kEncryptedMessage))
+        && (IsEncryptionRequired() == packetHeader.IsEncrypted())
 
         // AND The message was sent by an initiator and the exchange context is a responder (IsInitiator==false)
         //    OR The message was sent by a responder and the exchange context is an initiator (IsInitiator==true) (for the broadcast

src/messaging/ExchangeMgr.cpp

@@ -293,7 +293,7 @@ void ExchangeManager::OnMessageReceived(const PacketHeader & packetHeader, const
         ChipLogDetail(ExchangeManager, "Handling via exchange: " ChipLogFormatExchange ", Delegate: 0x%p", ChipLogValueExchange(ec),
                       ec->GetDelegate());
 
-        if (ec->IsEncryptionRequired() != packetHeader.GetFlags().Has(Header::FlagValues::kEncryptedMessage))
+        if (ec->IsEncryptionRequired() != packetHeader.IsEncrypted())
         {
             ChipLogError(ExchangeManager, "OnMessageReceived failed, err = %s", ErrorStr(CHIP_ERROR_INVALID_MESSAGE_TYPE));
             ec->Close();

src/messaging/tests/echo/echo_requester.cpp

@@ -256,7 +256,7 @@ int main(int argc, char * argv[])
     err = EstablishSecureSession();
     SuccessOrExit(err);
 
-    err = gEchoClient.Init(&gExchangeManager, chip::SessionHandle(chip::kTestDeviceNodeId, 0, 0, gFabricIndex));
+    err = gEchoClient.Init(&gExchangeManager, chip::SessionHandle(chip::kTestDeviceNodeId, 1, 1, gFabricIndex));
     SuccessOrExit(err);
 
     // Arrange to get a callback whenever an Echo Response is received.

src/protocols/secure_channel/PASESession.h

@@ -339,8 +339,10 @@ class SecurePairingUsingTestSecret : public PairingSession
 public:
     SecurePairingUsingTestSecret()
     {
-        SetLocalSessionId(0);
-        SetPeerSessionId(0);
+        // Do not set to 0 to prevent unwanted unsecured session
+        // since the session type is unknown.
+        SetLocalSessionId(1);
+        SetPeerSessionId(1);
     }
 
     SecurePairingUsingTestSecret(uint16_t peerSessionId, uint16_t localSessionId)

src/protocols/secure_channel/SessionIDAllocator.cpp

@@ -24,6 +24,7 @@ namespace chip {
 CHIP_ERROR SessionIDAllocator::Allocate(uint16_t & id)
 {
     VerifyOrReturnError(mNextAvailable < kMaxSessionID, CHIP_ERROR_NO_MEMORY);
+    VerifyOrReturnError(mNextAvailable > kUnsecuredSessionId, CHIP_ERROR_INTERNAL);
     id = mNextAvailable;
 
     // TODO - Update SessionID allocator to use freed session IDs
@@ -34,7 +35,8 @@ CHIP_ERROR SessionIDAllocator::Allocate(uint16_t & id)
 
 void SessionIDAllocator::Free(uint16_t id)
 {
-    if (mNextAvailable > 0 && (mNextAvailable - 1) == id)
+    // As per spec 4.4.1.3 Session ID of 0 is reserved for Unsecure communication
+    if (mNextAvailable > (kUnsecuredSessionId + 1) && (mNextAvailable - 1) == id)
     {
         mNextAvailable--;
     }

src/protocols/secure_channel/SessionIDAllocator.h

@@ -18,6 +18,15 @@
 #pragma once
 
 #include <lib/core/CHIPError.h>
+#include <stdint.h>
+
+// Spec 4.4.1.3
+// ===== Session ID (16 bits)
+// An unsigned integer value identifying the session associated with this message.
+// The session identifies the particular key used to encrypt a message out of the set of
+// available keys (either session or group), and the particular encryption/message
+// integrity algorithm to use for the message.The Session ID field is always present.
+// A Session ID of 0 SHALL indicate an unsecured session with no encryption or message integrity checking.
 
 namespace chip {
 
@@ -34,9 +43,10 @@ class SessionIDAllocator
     uint16_t Peek();
 
 private:
-    // Session ID is a 15 bit value (16th bit indicates unicast/group key)
-    static constexpr uint16_t kMaxSessionID = (1 << 15) - 1;
-    uint16_t mNextAvailable                 = 0;
+    static constexpr uint16_t kMaxSessionID       = UINT16_MAX;
+    static constexpr uint16_t kUnsecuredSessionId = 0;
+
+    uint16_t mNextAvailable = 1;
 };
 
 } // namespace chip

src/protocols/secure_channel/tests/TestCASESession.cpp

@@ -405,6 +405,10 @@ void CASE_SecurePairingSerializeTest(nlTestSuite * inSuite, void * inContext)
     PacketHeader header;
     MessageAuthenticationCode mac;
 
+    header.SetSessionId(1);
+    NL_TEST_ASSERT(inSuite, header.IsEncrypted() == true);
+    NL_TEST_ASSERT(inSuite, header.MICTagLength() == 16);
+
     // Let's try encrypting using original session, and decrypting using deserialized
     {
         CryptoContext session1;

src/protocols/secure_channel/tests/TestPASESession.cpp

@@ -302,6 +302,10 @@ void SecurePairingSerializeTest(nlTestSuite * inSuite, void * inContext)
     PacketHeader header;
     MessageAuthenticationCode mac;
 
+    header.SetSessionId(1);
+    NL_TEST_ASSERT(inSuite, header.IsEncrypted() == true);
+    NL_TEST_ASSERT(inSuite, header.MICTagLength() == 16);
+
     // Let's try encrypting using original session, and decrypting using deserialized
     {
         CryptoContext session1;

src/protocols/secure_channel/tests/TestSessionIDAllocator.cpp

@@ -28,11 +28,11 @@ void TestSessionIDAllocator_Allocate(nlTestSuite * inSuite, void * inContext)
 {
     SessionIDAllocator allocator;
 
-    NL_TEST_ASSERT(inSuite, allocator.Peek() == 0);
+    NL_TEST_ASSERT(inSuite, allocator.Peek() == 1);
 
     uint16_t id;
 
-    for (uint16_t i = 0; i < 16; i++)
+    for (uint16_t i = 1; i < 16; i++)
     {
         CHIP_ERROR err = allocator.Allocate(id);
         NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
@@ -45,11 +45,11 @@ void TestSessionIDAllocator_Free(nlTestSuite * inSuite, void * inContext)
 {
     SessionIDAllocator allocator;
 
-    NL_TEST_ASSERT(inSuite, allocator.Peek() == 0);
+    NL_TEST_ASSERT(inSuite, allocator.Peek() == 1);
 
     uint16_t id;
 
-    for (uint16_t i = 0; i < 16; i++)
+    for (uint16_t i = 1; i < 17; i++)
     {
         CHIP_ERROR err = allocator.Allocate(id);
         NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
@@ -59,15 +59,15 @@ void TestSessionIDAllocator_Free(nlTestSuite * inSuite, void * inContext)
 
     // Free an intermediate ID
     allocator.Free(10);
-    NL_TEST_ASSERT(inSuite, allocator.Peek() == 16);
+    NL_TEST_ASSERT(inSuite, allocator.Peek() == 17);
 
     // Free the last allocated ID
-    allocator.Free(15);
-    NL_TEST_ASSERT(inSuite, allocator.Peek() == 15);
+    allocator.Free(16);
+    NL_TEST_ASSERT(inSuite, allocator.Peek() == 16);
 
     // Free some random unallocated ID
     allocator.Free(100);
-    NL_TEST_ASSERT(inSuite, allocator.Peek() == 15);
+    NL_TEST_ASSERT(inSuite, allocator.Peek() == 16);
 }
 
 void TestSessionIDAllocator_Reserve(nlTestSuite * inSuite, void * inContext)
@@ -76,7 +76,7 @@ void TestSessionIDAllocator_Reserve(nlTestSuite * inSuite, void * inContext)
 
     uint16_t id;
 
-    for (uint16_t i = 0; i < 16; i++)
+    for (uint16_t i = 1; i < 16; i++)
     {
         CHIP_ERROR err = allocator.Allocate(id);
         NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);

src/protocols/user_directed_commissioning/UserDirectedCommissioningClient.cpp

@@ -67,8 +67,6 @@ CHIP_ERROR UserDirectedCommissioningClient::EncodeUDCMessage(System::PacketBuffe
 
     ReturnErrorOnFailure(payloadHeader.EncodeBeforeData(payload));
 
-    packetHeader.SetSessionType(Header::SessionType::kSessionTypeNone);
-
     ReturnErrorOnFailure(packetHeader.EncodeBeforeData(payload));
 
     return CHIP_NO_ERROR;

src/protocols/user_directed_commissioning/UserDirectedCommissioningServer.cpp

@@ -38,7 +38,7 @@ void UserDirectedCommissioningServer::OnMessageReceived(const Transport::PeerAdd
 
     ReturnOnFailure(packetHeader.DecodeAndConsume(msg));
 
-    if (packetHeader.GetFlags().Has(Header::FlagValues::kEncryptedMessage))
+    if (packetHeader.IsEncrypted())
     {
         ChipLogError(AppServer, "UDC encryption flag set - ignoring");
         return;

src/protocols/user_directed_commissioning/tests/TestUdcMessages.cpp

@@ -180,7 +180,7 @@ void TestUserDirectedCommissioningClientMessage(nlTestSuite * inSuite, void * in
     // check the packet header fields
     PacketHeader packetHeader;
     packetHeader.DecodeAndConsume(payloadBuf);
-    NL_TEST_ASSERT(inSuite, !packetHeader.GetFlags().Has(Header::FlagValues::kEncryptedMessage));
+    NL_TEST_ASSERT(inSuite, !packetHeader.IsEncrypted());
 
     // check the payload header fields
     PayloadHeader payloadHeader;

src/transport/CryptoContext.cpp

@@ -163,9 +163,8 @@ CHIP_ERROR CryptoContext::Encrypt(const uint8_t * input, size_t input_length, ui
                                   MessageAuthenticationCode & mac) const
 {
 
-    constexpr Header::SessionType sessionType = Header::SessionType::kAESCCMTagLen16;
+    const size_t taglen = header.MICTagLength();
 
-    const size_t taglen = MessageAuthenticationCode::TagLenForSessionType(sessionType);
     VerifyOrDie(taglen <= kMaxTagLen);
 
     VerifyOrReturnError(mKeyAvailable, CHIP_ERROR_INVALID_USE_OF_SESSION_KEY);
@@ -194,15 +193,15 @@ CHIP_ERROR CryptoContext::Encrypt(const uint8_t * input, size_t input_length, ui
     ReturnErrorOnFailure(AES_CCM_encrypt(input, input_length, AAD, aadLen, mKeys[usage], Crypto::kAES_CCM128_Key_Length, IV,
                                          sizeof(IV), output, tag, taglen));
 
-    mac.SetTag(&header, sessionType, tag, taglen);
+    mac.SetTag(&header, tag, taglen);
 
     return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR CryptoContext::Decrypt(const uint8_t * input, size_t input_length, uint8_t * output, const PacketHeader & header,
                                   const MessageAuthenticationCode & mac) const
 {
-    const size_t taglen = MessageAuthenticationCode::TagLenForSessionType(header.GetSessionType());
+    const size_t taglen = header.MICTagLength();
     const uint8_t * tag = mac.GetTag();
     uint8_t IV[kAESCCMIVLen];
     uint8_t AAD[kMaxAADLen];

src/transport/SecureMessageCodec.cpp

@@ -52,7 +52,8 @@ CHIP_ERROR Encrypt(Transport::SecureSession * state, PayloadHeader & payloadHead
         .SetMessageCounter(messageCounter) //
         .SetSessionId(state->GetPeerSessionId());
 
-    packetHeader.GetFlags().Set(Header::FlagValues::kEncryptedMessage);
+    // TODO set Session Type (Unicast or Group)
+    // packetHeader.SetSessionType(Header::SessionType::kUnicastSession);
 
     ReturnErrorOnFailure(payloadHeader.EncodeBeforeData(msgBuf));
 
@@ -90,7 +91,7 @@ CHIP_ERROR Decrypt(Transport::SecureSession * state, PayloadHeader & payloadHead
     msg->SetDataLength(len);
 #endif
 
-    uint16_t footerLen = MessageAuthenticationCode::TagLenForSessionType(packetHeader.GetSessionType());
+    uint16_t footerLen = packetHeader.MICTagLength();
     VerifyOrReturnError(footerLen <= len, CHIP_ERROR_INVALID_MESSAGE_LENGTH);
 
     uint16_t taglen = 0;

src/transport/SessionManager.cpp

@@ -326,7 +326,7 @@ void SessionManager::OnMessageReceived(const PeerAddress & peerAddress, System::
 
     ReturnOnFailure(packetHeader.DecodeAndConsume(msg));
 
-    if (packetHeader.GetFlags().Has(Header::FlagValues::kEncryptedMessage))
+    if (packetHeader.IsEncrypted())
     {
         SecureMessageDispatch(packetHeader, peerAddress, std::move(msg));
     }
@@ -403,7 +403,7 @@ void SessionManager::SecureMessageDispatch(const PacketHeader & packetHeader, co
                  ChipLogError(Inet, "Secure transport received message, but failed to decode/authenticate it, discarding"));
 
     // Verify message counter
-    if (packetHeader.GetFlags().Has(Header::FlagValues::kSecureSessionControlMessage))
+    if (packetHeader.IsSecureSessionControlMsg())
     {
         // TODO: control message counter is not implemented yet
     }
@@ -462,7 +462,7 @@ void SessionManager::SecureMessageDispatch(const PacketHeader & packetHeader, co
         }
     }
 
-    if (packetHeader.GetFlags().Has(Header::FlagValues::kSecureSessionControlMessage))
+    if (packetHeader.IsSecureSessionControlMsg())
     {
         // TODO: control message counter is not implemented yet
     }